Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ VBT's

[Mika Kahola <mika.kahola@intel.com>]

On Mon, 2017-09-04 at 18:04 +0300, Ville Syrjälä wrote:
> On Mon, Sep 04, 2017 at 10:59:32AM +0300, Mika Kahola wrote:
> > 
> > On Fri, 2017-09-01 at 16:43 +0300, Ville Syrjälä wrote:
> > > 
> > > On Fri, Sep 01, 2017 at 10:51:00AM +0300, Mika Kahola wrote:
> > > > 
> > > > 
> > > > According to spec we should send SHUTDOWN before
> > > > MIPI_SEQ_DISPLAY_OFF for
> > > > v3+ VBT's. Testing with VBT v3 the current implementation
> > > > yields
> > > > the
> > > > following error message
> > > > 
> > > > *ERROR* Video mode command 0x00000041 send failed.
> > > > 
> > > > To get rid of this error message, let's limit SHUTDOWN only for
> > > > VBT
> > > > versions 3 or higher.
> > > In the patch you limit it to version 4+, which doesn't make sense
> > > since
> > > AFAIK there is no version 4 of the sequence block.
> > It seems that sending SHUTDOWN signal doesn't make any sense
> > either.
> > Whenever we send that signal it just causes this error message. Do
> > we
> > really need to signal this? From functionality point of view
> > there's no
> > difference.
> Well, the spec doesn't even explain what this "shut down" command
> does.
> Is it actually the DCS "display off" command, or something else?
It seems to do something. At least the intel_wait_for_register() times
out when sending SHUTDOWN signal. Maybe it just shuts down display so
we can't even read the registers after that. 

This brings me to another idea. Maybe we should skip
intel_wait_for_register() if we have sent out SHUTDOWN signal? 

> 
> Did you try reverting bbdf0b2ff32a ("drm/i915/bxt: Disable device
> ready
> before shutdown command")? That looks suspicious to me. But so does
> about half of the DSI code since it never seems to follow the spec,
> and
> we end up doing totally different things on different platforms
> without
> any explanation why that is).
I haven't tried that. I will give it a go and see what happens.

> 
> > 
> > 
> > > 
> > > 
> > > > 
> > > > 
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404
> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > > > b/drivers/gpu/drm/i915/intel_dsi.c
> > > > index 2a0f5d3..b48b9b7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > @@ -916,7 +916,7 @@ static void intel_dsi_disable(struct
> > > > intel_encoder *encoder,
> > > >  	 * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field
> > > > testing
> > > >  	 * has shown that the v3 sequence works for v2 VBTs
> > > > too
> > > >  	 */
> > > > -	if (is_vid_mode(intel_dsi)) {
> > > > +	if (is_vid_mode(intel_dsi) && dev_priv-
> > > > > 
> > > > > vbt.dsi.seq_version > 3) {
> > > >  		/* Send Shutdown command to the panel in LP
> > > > mode
> > > > */
> > > >  		for_each_dsi_port(port, intel_dsi->ports)
> > > >  			dpi_send_cmd(intel_dsi, SHUTDOWN,
> > > > false,
> > > > port);
> > > > -- 
> > > > 2.7.4
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > -- 
> > Mika Kahola - Intel OTC
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx