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

[Jani Nikula <jani.nikula@linux.intel.com>]

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx