All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2
@ 2018-01-26  7:52 Hans de Goede
  2018-01-26  8:15 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Hans de Goede @ 2018-01-26  7:52 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Ville Syrjälä
  Cc: Hans de Goede, intel-gfx, dri-devel, Jan-Michael Brummer

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().

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_dsi.c     |  1 +
 drivers/gpu/drm/i915/intel_dsi.h     |  2 +
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 82 ++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index f67d321376e4..b59ef34d25f6 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1642,6 +1642,7 @@ static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
 	if (intel_dsi->gpio_panel)
 		gpiod_put(intel_dsi->gpio_panel);
 
+	kfree(intel_dsi->deassert_seq);
 	intel_encoder_destroy(encoder);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 7afeb9580f41..5895588144ad 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -46,6 +46,8 @@ struct intel_dsi {
 
 	struct intel_connector *attached_connector;
 
+	u8 *deassert_seq;
+
 	/* bit mask of ports being driven */
 	u16 ports;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
index 91c07b0c8db9..84664f79cbef 100644
--- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
@@ -499,6 +499,86 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
 	return 1;
 }
 
+/*
+ * Get len of pre-fixed deassert from init OTP, skip all delay + gpio operands
+ * and stop at the first DSI packet op.
+ */
+static int intel_vbi_get_deassert_len(const u8 *data, int total)
+{
+	int index, len;
+
+	/* index = 1 to skip sequence byte */
+	for (index = 1; index < total; index += len) {
+		switch (data[index]) {
+		case MIPI_SEQ_ELEM_SEND_PKT:
+			return 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 intel_dsi_fixup_dsi_sequences(struct intel_dsi *intel_dsi)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
+	int init_otp_index, len;
+	u8 *init_otp;
+
+	/* Limit this to VLV for now. */
+	if (!IS_VALLEYVIEW(dev_priv))
+		return;
+
+	/* Limit this to v1 vid-mode sequences */
+	if (intel_dsi->operation_mode != INTEL_DSI_VIDEO_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 */
+	init_otp_index = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] -
+			 (const u8 *)dev_priv->vbt.dsi.data;
+	init_otp = dev_priv->vbt.dsi.data + init_otp_index;
+	len = dev_priv->vbt.dsi.size - init_otp_index;
+	len = intel_vbi_get_deassert_len(init_otp, len);
+	if (!len)
+		return;
+
+	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
+
+	/* Copy the fragment, update seq byte and terminate it */
+	intel_dsi->deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
+	if (!intel_dsi->deassert_seq)
+		return;
+	intel_dsi->deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
+	intel_dsi->deassert_seq[len] = MIPI_SEQ_ELEM_END;
+	/* Use the copy for deassert */
+	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
+		intel_dsi->deassert_seq;
+	/* Replace the last byte of the fragment with init OTP seq byte */
+	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
+	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
+	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
+}
+
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
@@ -794,5 +874,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 		mipi_dsi_attach(intel_dsi->dsi_hosts[port]->device);
 	}
 
+	intel_dsi_fixup_dsi_sequences(intel_dsi);
+
 	return true;
 }
-- 
2.14.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2)
  2018-01-26  7:52 [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Hans de Goede
@ 2018-01-26  8:15 ` Patchwork
  2018-01-26  9:09 ` ✓ Fi.CI.IGT: " Patchwork
  2018-01-26 16:38 ` [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-01-26  8:15 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2)
URL   : https://patchwork.freedesktop.org/series/37105/
State : success

== Summary ==

Series 37105v2 drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence
https://patchwork.freedesktop.org/api/1.0/series/37105/revisions/2/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test gem_ringfill:
        Subgroup basic-default-hang:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101600
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:422s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:428s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:491s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:282s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:481s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:472s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:455s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:573s
fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:525s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:278s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:513s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:394s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:417s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:421s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:458s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:501s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:581s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:427s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:486s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:435s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:409s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:472s

804d087465e907accca7a2631d8a9485729b9a48 drm-tip: 2018y-01m-25d-18h-22m-16s UTC integration manifest
57b4c65e08fb drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7787/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2)
  2018-01-26  7:52 [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Hans de Goede
  2018-01-26  8:15 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2) Patchwork
@ 2018-01-26  9:09 ` Patchwork
  2018-01-26 16:38 ` [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Ville Syrjälä
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-01-26  9:09 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2)
URL   : https://patchwork.freedesktop.org/series/37105/
State : success

== Summary ==

Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252
Test gem_eio:
        Subgroup in-flight-contexts:
                fail       -> PASS       (shard-hsw) fdo#104676
        Subgroup hibernate:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540 +1
Test kms_flip:
        Subgroup modeset-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-apl) fdo#103060
        Subgroup flip-vs-modeset-vs-hang:
                pass       -> DMESG-WARN (shard-snb) fdo#103821
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-apl) fdo#101623
Test drv_suspend:
        Subgroup fence-restore-untiled-hibernate:
                fail       -> SKIP       (shard-snb) fdo#103375

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#104676 https://bugs.freedesktop.org/show_bug.cgi?id=104676
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103821 https://bugs.freedesktop.org/show_bug.cgi?id=103821
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-apl        total:2838 pass:1752 dwarn:1   dfail:0   fail:21  skip:1064 time:12663s
shard-hsw        total:2711 pass:1659 dwarn:1   dfail:0   fail:11  skip:1037 time:11323s
shard-snb        total:2838 pass:1329 dwarn:2   dfail:0   fail:9   skip:1498 time:6636s
Blacklisted hosts:
shard-kbl        total:2771 pass:1793 dwarn:34  dfail:2   fail:21  skip:920 time:9497s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7787/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2
  2018-01-26  7:52 [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Hans de Goede
  2018-01-26  8:15 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2) Patchwork
  2018-01-26  9:09 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-01-26 16:38 ` Ville Syrjälä
  2018-01-27 14:31   ` Hans de Goede
  2 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2018-01-26 16:38 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Jan-Michael Brummer, intel-gfx, dri-devel, Hans de Goede, Rodrigo Vivi

On Fri, Jan 26, 2018 at 08:52:07AM +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().
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
> Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
> Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c     |  1 +
>  drivers/gpu/drm/i915/intel_dsi.h     |  2 +
>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 82 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index f67d321376e4..b59ef34d25f6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1642,6 +1642,7 @@ static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
>  	if (intel_dsi->gpio_panel)
>  		gpiod_put(intel_dsi->gpio_panel);
>  
> +	kfree(intel_dsi->deassert_seq);
>  	intel_encoder_destroy(encoder);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 7afeb9580f41..5895588144ad 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -46,6 +46,8 @@ struct intel_dsi {
>  
>  	struct intel_connector *attached_connector;
>  
> +	u8 *deassert_seq;
> +

Should this perhaps live next to the other DSI VBT stuff? I think that
might make more sense. And should probably also move the
intel_dsi_fixup_dsi_sequences() call to parse_mipi_sequence() as well
since we're actually modifying dev_priv->vbt.data. Doing that from the
encoder init doesn't really feel right to me.

Jani, any thoughts?

>  	/* bit mask of ports being driven */
>  	u16 ports;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> index 91c07b0c8db9..84664f79cbef 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
> @@ -499,6 +499,86 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
>  	return 1;
>  }
>  
> +/*
> + * Get len of pre-fixed deassert from init OTP, skip all delay + gpio operands
> + * and stop at the first DSI packet op.
> + */
> +static int intel_vbi_get_deassert_len(const u8 *data, int total)
> +{
> +	int index, len;

if (WARN_ON(seq_version != 1))
	return 0;

or something might be nice here to document the requirements and
to deter anyone from using this with other seq versions.

> +
> +	/* index = 1 to skip sequence byte */
> +	for (index = 1; index < total; index += len) {
> +		switch (data[index]) {
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +			return index;

What if this is the first element?

Hmm. It does seem to work out in the end. We do end up with
an empty deassert sequence, but I guess hat's fine.

> +		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 intel_dsi_fixup_dsi_sequences(struct intel_dsi *intel_dsi)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
> +	int init_otp_index, len;
> +	u8 *init_otp;
> +
> +	/* Limit this to VLV for now. */
> +	if (!IS_VALLEYVIEW(dev_priv))
> +		return;

Not sure v1 sequences even exist on other platforms. But no
harm in having this check anyway.

> +
> +	/* Limit this to v1 vid-mode sequences */
> +	if (intel_dsi->operation_mode != INTEL_DSI_VIDEO_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 */
> +	init_otp_index = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] -
> +			 (const u8 *)dev_priv->vbt.dsi.data;

Why the cast?

> +	init_otp = dev_priv->vbt.dsi.data + init_otp_index;
> +	len = dev_priv->vbt.dsi.size - init_otp_index;
> +	len = intel_vbi_get_deassert_len(init_otp, len);
> +	if (!len)
> +		return;
> +
> +	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
> +
> +	/* Copy the fragment, update seq byte and terminate it */
> +	intel_dsi->deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
> +	if (!intel_dsi->deassert_seq)
> +		return;
> +	intel_dsi->deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
> +	intel_dsi->deassert_seq[len] = MIPI_SEQ_ELEM_END;
> +	/* Use the copy for deassert */
> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
> +		intel_dsi->deassert_seq;
> +	/* Replace the last byte of the fragment with init OTP seq byte */
> +	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
> +	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
> +}
> +
>  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  {
>  	struct drm_device *dev = intel_dsi->base.base.dev;
> @@ -794,5 +874,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  		mipi_dsi_attach(intel_dsi->dsi_hosts[port]->device);
>  	}
>  
> +	intel_dsi_fixup_dsi_sequences(intel_dsi);
> +
>  	return true;
>  }
> -- 
> 2.14.3

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2
  2018-01-26 16:38 ` [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Ville Syrjälä
@ 2018-01-27 14:31   ` Hans de Goede
  0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2018-01-27 14:31 UTC (permalink / raw)
  To: Ville Syrjälä, Hans de Goede
  Cc: Jan-Michael Brummer, intel-gfx, dri-devel, Rodrigo Vivi

Hi,

On 26-01-18 17:38, Ville Syrjälä wrote:
> On Fri, Jan 26, 2018 at 08:52:07AM +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().
>>
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
>> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
>> Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
>> Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
>> Tested-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dsi.c     |  1 +
>>   drivers/gpu/drm/i915/intel_dsi.h     |  2 +
>>   drivers/gpu/drm/i915/intel_dsi_vbt.c | 82 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 85 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index f67d321376e4..b59ef34d25f6 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1642,6 +1642,7 @@ static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
>>   	if (intel_dsi->gpio_panel)
>>   		gpiod_put(intel_dsi->gpio_panel);
>>   
>> +	kfree(intel_dsi->deassert_seq);
>>   	intel_encoder_destroy(encoder);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index 7afeb9580f41..5895588144ad 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -46,6 +46,8 @@ struct intel_dsi {
>>   
>>   	struct intel_connector *attached_connector;
>>   
>> +	u8 *deassert_seq;
>> +
> 
> Should this perhaps live next to the other DSI VBT stuff? I think that
> might make more sense. And should probably also move the
> intel_dsi_fixup_dsi_sequences() call to parse_mipi_sequence() as well
> since we're actually modifying dev_priv->vbt.data. Doing that from the
> encoder init doesn't really feel right to me.

Sure works for me, shall I submit a v3 with this changed, or do you
want to wait a bit for Jani's input on this?

> Jani, any thoughts?
> 
>>   	/* bit mask of ports being driven */
>>   	u16 ports;
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> index 91c07b0c8db9..84664f79cbef 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>> @@ -499,6 +499,86 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
>>   	return 1;
>>   }
>>   
>> +/*
>> + * Get len of pre-fixed deassert from init OTP, skip all delay + gpio operands
>> + * and stop at the first DSI packet op.
>> + */
>> +static int intel_vbi_get_deassert_len(const u8 *data, int total)
>> +{
>> +	int index, len;
> 
> if (WARN_ON(seq_version != 1))
> 	return 0;
> 
> or something might be nice here to document the requirements and
> to deter anyone from using this with other seq versions.

Ok, will add for v3.

>> +
>> +	/* index = 1 to skip sequence byte */
>> +	for (index = 1; index < total; index += len) {
>> +		switch (data[index]) {
>> +		case MIPI_SEQ_ELEM_SEND_PKT:
>> +			return index;
> 
> What if this is the first element?

Ah good one, I did not think about that.

> Hmm. It does seem to work out in the end. We do end up with
> an empty deassert sequence, but I guess hat's fine.

Ack, lets keep it as is then.

>> +		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 intel_dsi_fixup_dsi_sequences(struct intel_dsi *intel_dsi)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
>> +	int init_otp_index, len;
>> +	u8 *init_otp;
>> +
>> +	/* Limit this to VLV for now. */
>> +	if (!IS_VALLEYVIEW(dev_priv))
>> +		return;
> 
> Not sure v1 sequences even exist on other platforms. But no
> harm in having this check anyway.

Ack.

>> +
>> +	/* Limit this to v1 vid-mode sequences */
>> +	if (intel_dsi->operation_mode != INTEL_DSI_VIDEO_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 */
>> +	init_otp_index = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] -
>> +			 (const u8 *)dev_priv->vbt.dsi.data;
> 
> Why the cast?

dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] is const u8 *,
dev_priv->vbt.dsi.data u8 *, I was expecting gcc to complain without
the cast, but at least current gcc does not complain. Still might
be best to keep the cast to avoid future compiler warnings.

>> +	init_otp = dev_priv->vbt.dsi.data + init_otp_index;
>> +	len = dev_priv->vbt.dsi.size - init_otp_index;
>> +	len = intel_vbi_get_deassert_len(init_otp, len);
>> +	if (!len)
>> +		return;
>> +
>> +	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
>> +
>> +	/* Copy the fragment, update seq byte and terminate it */
>> +	intel_dsi->deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
>> +	if (!intel_dsi->deassert_seq)
>> +		return;
>> +	intel_dsi->deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
>> +	intel_dsi->deassert_seq[len] = MIPI_SEQ_ELEM_END;
>> +	/* Use the copy for deassert */
>> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
>> +		intel_dsi->deassert_seq;
>> +	/* Replace the last byte of the fragment with init OTP seq byte */
>> +	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
>> +	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
>> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
>> +}
>> +
>>   bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>   {
>>   	struct drm_device *dev = intel_dsi->base.base.dev;
>> @@ -794,5 +874,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>   		mipi_dsi_attach(intel_dsi->dsi_hosts[port]->device);
>>   	}
>>   
>> +	intel_dsi_fixup_dsi_sequences(intel_dsi);
>> +
>>   	return true;
>>   }
>> -- 
>> 2.14.3
> 

Regards,

Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-01-27 14:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-26  7:52 [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Hans de Goede
2018-01-26  8:15 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence (rev2) Patchwork
2018-01-26  9:09 ` ✓ Fi.CI.IGT: " Patchwork
2018-01-26 16:38 ` [PATCH] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v2 Ville Syrjälä
2018-01-27 14:31   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.