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

[Hans de Goede]

Hi,

On 27-02-17 18:16, Bob Paauwe wrote:
> On Sat, 25 Feb 2017 11:42:09 +0100
> Hans de Goede <hdegoede at redhat.com> wrote:
>
>> 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 anyone else, Jani?, has more information about this, that would be
> good to know.
>
> I'd be OK with just stating that "field testing has shown that the v3
> sequence works with v2 VBT's so just use that."

Ok, done for v2.

Regards,

Hans


>
>>
>>>> +	 */
>>>>  	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
>
>
>