* [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.