Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

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