[Intel-gfx] [PATCH resend 12/15] drm/i915/dsi: Document always using v3 SHUTDOWN / MIPI_SEQ_DISPLAY_OFF order

[Hans de Goede]

Hi,

On 24-02-17 18:02, Bob Paauwe wrote:
> On Mon, 20 Feb 2017 15:08:42 +0100
> Hans de Goede <hdegoede at redhat.com> wrote:
>
>> According to the spec for v2 VBTs we should call MIPI_SEQ_DISPLAY_OFF
>> before sending SHUTDOWN, where as for v3 VBTs we should send SHUTDOWN
>> first.
>>
>> Since the v2 order has known issues, we use the v3 order everywhere,
>> add a comment documenting this.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index a8d0948..1914311 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -732,6 +732,11 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder,
>>  			I915_WRITE(MIPI_DEVICE_READY(port), 0);
>>  	}
>>
>> +	/*
>> +	 * XXX: According to the spec we should send SHUTDOWN before
>> +	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but testing in the field
>> +	 * has shown that we should do this for v2 VBTs too?
> drop the '?'

I added that because I'm not 100% sure this is true, looking through git
history (and android x86 kernel patch-sets) I managed to piece together that
at one point in time the v2 sequence was used, but that yielded problems
during some testing, what the commits do not tell if is that testing was
using boards with v3 VBTs, but assuming v2 tables are out there in the
wild then it seems that the v3 order works fine for v2 too.

TLDR I'm not 100% sure about this hence the '?', my main goal with this
patch is to document that we're deviating from the spec for v2 tables here.

>> +	 */
>>  	if (is_vid_mode(intel_dsi)) {
>>  		/* Send Shutdown command to the panel in LP mode */
>>  		for_each_dsi_port(port, intel_dsi->ports)
>> @@ -764,6 +769,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
>>  	/*
>>  	 * if disable packets are sent before sending shutdown packet then in
>>  	 * some next enable sequence send turn on packet error is observed
>> +	 * XXX spec specifies SHUTDOWN before MIPI_SEQ_DISPLAY_OFF for
>> +	 * v3 VBTs, but not for v2 VBTs?
>>  	 */
>>  	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
>>
>
> Should XXX be replaced with something?

XXX is used in many places in intel_dsi.c to indicate code which may need
work / which may needs to be investigated further. I followed that and
added XXX here since this code is deviating from the spec.

Regards,

Hans