| Message ID | 20180129144735.32412-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <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> This one seems good to me, and Jani hasn't complained about anything so Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > 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->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET; > + dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END; > + /* Use the copy for deassert */ > + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] = > + dev_priv->vbt.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; > +} > + > static void > parse_mipi_sequence(struct drm_i915_private *dev_priv, > const struct bdb_header *bdb) > @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv, > dev_priv->vbt.dsi.size = seq_size; > dev_priv->vbt.dsi.seq_version = sequence->version; > > + fixup_mipi_sequences(dev_priv); > + > DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n"); > return; > > @@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv) > */ > void intel_bios_cleanup(struct drm_i915_private *dev_priv) > { > + kfree(dev_priv->vbt.dsi.deassert_seq); > kfree(dev_priv->vbt.dsi.data); > kfree(dev_priv->vbt.dsi.pps); > kfree(dev_priv->vbt.dsi.config); > -- > 2.14.3
On Fri, 02 Feb 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> 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 <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> > > This one seems good to me, and Jani hasn't complained about anything so > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks for hiding this in one place. Acked-by: Jani Nikula <jani.nikula@intel.com> > >> --- >> 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->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET; >> + dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END; >> + /* Use the copy for deassert */ >> + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] = >> + dev_priv->vbt.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; >> +} >> + >> static void >> parse_mipi_sequence(struct drm_i915_private *dev_priv, >> const struct bdb_header *bdb) >> @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv, >> dev_priv->vbt.dsi.size = seq_size; >> dev_priv->vbt.dsi.seq_version = sequence->version; >> >> + fixup_mipi_sequences(dev_priv); >> + >> DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n"); >> return; >> >> @@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv) >> */ >> void intel_bios_cleanup(struct drm_i915_private *dev_priv) >> { >> + kfree(dev_priv->vbt.dsi.deassert_seq); >> kfree(dev_priv->vbt.dsi.data); >> kfree(dev_priv->vbt.dsi.pps); >> kfree(dev_priv->vbt.dsi.config); >> -- >> 2.14.3
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->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET; + dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END; + /* Use the copy for deassert */ + dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] = + dev_priv->vbt.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; +} + static void parse_mipi_sequence(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv, dev_priv->vbt.dsi.size = seq_size; dev_priv->vbt.dsi.seq_version = sequence->version; + fixup_mipi_sequences(dev_priv); + DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n"); return; @@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv) */ void intel_bios_cleanup(struct drm_i915_private *dev_priv) { + kfree(dev_priv->vbt.dsi.deassert_seq); kfree(dev_priv->vbt.dsi.data); kfree(dev_priv->vbt.dsi.pps); kfree(dev_priv->vbt.dsi.config);