Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3
On Fri, 02 Feb 2018, Ville Syrjälä wrote:
> On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
>> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
>> index = 3, one of which has been kindly provided to me by Jan Brummer,
>> where not working with the i915 driver, giving a black screen on the
>> first modeset.
>>
>> The problem with at least these Dells is that their VBT defines a MIPI
>> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
>> reset in their INIT_OTP sequence, but the deassert must be done before
>> calling intel_dsi_device_ready(), so that is too late.
>>
>> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
>> because the INIT_OTP sequence also sends various MIPI packets to the
>> panel, which can only happen after calling intel_dsi_device_ready().
>>
>> This commit fixes this by splitting the INIT_OTP sequence into everything
>> before the first DSI packet and everything else, including the first DSI
>> packet. The first part (everything before the first DSI packet) is then
>> used as deassert sequence.
>>
>> Changed in v2:
>> -Split the init OTP sequence into a deassert reset and the actual init
>> OTP sequence, instead of calling it earlier and then having the first
>> mipi_exec_send_packet() call call intel_dsi_device_ready().
>>
>> Changes in v3:
>> -Move the whole shebang to intel_bios.c
>>
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
>
> Bugzilla:
>
>> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
>
> References:
>
>> Cc: Jan-Michael Brummer
>> Reported-by: Jan-Michael Brummer
>> Tested-by: Hans de Goede
>> Signed-off-by: Hans de Goede
>
> This one seems good to me, and Jani hasn't complained about anything so
> Reviewed-by: Ville Syrjälä
Thanks for hiding this in one place.
Acked-by: Jani Nikula
>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_bios.c | 83
>> +++
>> 2 files changed, 84 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 081190da0818..1f346266956b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
>> u32 size;
>> u8 *data;
>> const u8 *sequence[MIPI_SEQ_MAX];
>> +u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
>> } dsi;
>>
>> int crt_ddc_pin;
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 64a0d55df28e..cca620f8deb6 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int
>> index, int total)
>> return 0;
>> }
>>
>> +/*
>> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
>> + * skip all delay + gpio operands and stop at the first DSI packet op.
>> + */
>> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private
>> *dev_priv)
>> +{
>> +const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>> +int index, len;
>> +
>> +if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
>> +return 0;
>> +
>> +/* index = 1 to skip sequence byte */
>> +for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
>> +switch (data[index]) {
>> +case MIPI_SEQ_ELEM_SEND_PKT:
>> +return index == 1 ? 0 : index;
>> +case MIPI_SEQ_ELEM_DELAY:
>> +len = 5; /* 1 byte for operand + uint32 */
>> +break;
>> +case MIPI_SEQ_ELEM_GPIO:
>> +len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
>> +break;
>> +default:
>> +return 0;
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/*
>> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
>> + * The deassert must be done before calling intel_dsi_device_ready, so for
>> + * these devices we split the init OTP sequence into a deassert sequence and
>> + * the actual init OTP part.
>> + */
>> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
>> +{
>> +u8 *init_otp;
>> +int len;
>> +
>> +/* Limit this to VLV for now. */
>> +if (!IS_VALLEYVIEW(dev_priv))
>> +return;
>> +
>> +/* Limit this to v1 vid-mode sequences */
>> +if (dev_priv->vbt.dsi.config->is_cmd_mode ||
>> +dev_priv->vbt.dsi.seq_version != 1)
>> +return;
>> +
>> +/* Only do this if there are otp and assert seqs and no deassert seq */
>> +if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
>> +!dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
>> +dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
>> +retur
Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3
On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
> index = 3, one of which has been kindly provided to me by Jan Brummer,
> where not working with the i915 driver, giving a black screen on the
> first modeset.
>
> The problem with at least these Dells is that their VBT defines a MIPI
> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
> reset in their INIT_OTP sequence, but the deassert must be done before
> calling intel_dsi_device_ready(), so that is too late.
>
> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
> because the INIT_OTP sequence also sends various MIPI packets to the
> panel, which can only happen after calling intel_dsi_device_ready().
>
> This commit fixes this by splitting the INIT_OTP sequence into everything
> before the first DSI packet and everything else, including the first DSI
> packet. The first part (everything before the first DSI packet) is then
> used as deassert sequence.
>
> Changed in v2:
> -Split the init OTP sequence into a deassert reset and the actual init
> OTP sequence, instead of calling it earlier and then having the first
> mipi_exec_send_packet() call call intel_dsi_device_ready().
>
> Changes in v3:
> -Move the whole shebang to intel_bios.c
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
Bugzilla:
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
References:
> Cc: Jan-Michael Brummer
> Reported-by: Jan-Michael Brummer
> Tested-by: Hans de Goede
> Signed-off-by: Hans de Goede
This one seems good to me, and Jani hasn't complained about anything so
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_bios.c | 83
> +++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 081190da0818..1f346266956b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
> u32 size;
> u8 *data;
> const u8 *sequence[MIPI_SEQ_MAX];
> + u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
> } dsi;
>
> int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 64a0d55df28e..cca620f8deb6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int
> index, int total)
> return 0;
> }
>
> +/*
> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
> + * skip all delay + gpio operands and stop at the first DSI packet op.
> + */
> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private
> *dev_priv)
> +{
> + const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
> + int index, len;
> +
> + if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
> + return 0;
> +
> + /* index = 1 to skip sequence byte */
> + for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
> + switch (data[index]) {
> + case MIPI_SEQ_ELEM_SEND_PKT:
> + return index == 1 ? 0 : index;
> + case MIPI_SEQ_ELEM_DELAY:
> + len = 5; /* 1 byte for operand + uint32 */
> + break;
> + case MIPI_SEQ_ELEM_GPIO:
> + len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
> + break;
> + default:
> + return 0;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
> + * The deassert must be done before calling intel_dsi_device_ready, so for
> + * these devices we split the init OTP sequence into a deassert sequence and
> + * the actual init OTP part.
> + */
> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
> +{
> + u8 *init_otp;
> + int len;
> +
> + /* Limit this to VLV for now. */
> + if (!IS_VALLEYVIEW(dev_priv))
> + return;
> +
> + /* Limit this to v1 vid-mode sequences */
> + if (dev_priv->vbt.dsi.config->is_cmd_mode ||
> + dev_priv->vbt.dsi.seq_version != 1)
> + return;
> +
> + /* Only do this if there are otp and assert seqs and no deassert seq */
> + if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
> + !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
> + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
> + return;
> +
> + /* The deassert-sequence ends at the first DSI packet */
> + len = get_init_otp_deassert_fragment_len(dev_priv);
> + if (!len)
> + return;
> +
> + DRM_DEBUG_KMS("Usi
[PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3
So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
index = 3, one of which has been kindly provided to me by Jan Brummer,
where not working with the i915 driver, giving a black screen on the
first modeset.
The problem with at least these Dells is that their VBT defines a MIPI
ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
reset in their INIT_OTP sequence, but the deassert must be done before
calling intel_dsi_device_ready(), so that is too late.
Simply doing the INIT_OTP sequence earlier is not enough to fix this,
because the INIT_OTP sequence also sends various MIPI packets to the
panel, which can only happen after calling intel_dsi_device_ready().
This commit fixes this by splitting the INIT_OTP sequence into everything
before the first DSI packet and everything else, including the first DSI
packet. The first part (everything before the first DSI packet) is then
used as deassert sequence.
Changed in v2:
-Split the init OTP sequence into a deassert reset and the actual init
OTP sequence, instead of calling it earlier and then having the first
mipi_exec_send_packet() call call intel_dsi_device_ready().
Changes in v3:
-Move the whole shebang to intel_bios.c
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
Cc: Jan-Michael Brummer
Reported-by: Jan-Michael Brummer
Tested-by: Hans de Goede
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_bios.c | 83 +++
2 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 081190da0818..1f346266956b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,7 @@ struct intel_vbt_data {
u32 size;
u8 *data;
const u8 *sequence[MIPI_SEQ_MAX];
+ u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
} dsi;
int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 64a0d55df28e..cca620f8deb6 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int
index, int total)
return 0;
}
+/*
+ * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
+ * skip all delay + gpio operands and stop at the first DSI packet op.
+ */
+static int get_init_otp_deassert_fragment_len(struct drm_i915_private
*dev_priv)
+{
+ const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
+ int index, len;
+
+ if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
+ return 0;
+
+ /* index = 1 to skip sequence byte */
+ for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
+ switch (data[index]) {
+ case MIPI_SEQ_ELEM_SEND_PKT:
+ return index == 1 ? 0 : index;
+ case MIPI_SEQ_ELEM_DELAY:
+ len = 5; /* 1 byte for operand + uint32 */
+ break;
+ case MIPI_SEQ_ELEM_GPIO:
+ len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
+ break;
+ default:
+ return 0;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
+ * The deassert must be done before calling intel_dsi_device_ready, so for
+ * these devices we split the init OTP sequence into a deassert sequence and
+ * the actual init OTP part.
+ */
+static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
+{
+ u8 *init_otp;
+ int len;
+
+ /* Limit this to VLV for now. */
+ if (!IS_VALLEYVIEW(dev_priv))
+ return;
+
+ /* Limit this to v1 vid-mode sequences */
+ if (dev_priv->vbt.dsi.config->is_cmd_mode ||
+ dev_priv->vbt.dsi.seq_version != 1)
+ return;
+
+ /* Only do this if there are otp and assert seqs and no deassert seq */
+ if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
+ !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
+ dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
+ return;
+
+ /* The deassert-sequence ends at the first DSI packet */
+ len = get_init_otp_deassert_fragment_len(dev_priv);
+ if (!len)
+ return;
+
+ DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
+
+ /* Copy the fragment, update seq byte and terminate it */
+ init_otp = (u8 *)dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
+ dev_priv->vbt.dsi.deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
+ if (!dev_priv->vbt.dsi.deassert_seq)
+ return;
+ dev_priv
