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

[Bob Paauwe]

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."

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



-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193