On Tue, Sep 05, 2017 at 02:55:31PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >Sent: Tuesday, September 5, 2017 6:46 PM > >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >Cc: Kahola, Mika <mika.kahola@xxxxxxxxx>; Srinivas, Vidya > ><vidya.srinivas@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >Subject: Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only for v3+ > >VBT's > > > >On Tue, Sep 05, 2017 at 09:27:47AM +0000, Shankar, Uma wrote: > >> > >> > >> >-----Original Message----- > >> >From: Kahola, Mika > >> >Sent: Tuesday, September 5, 2017 2:03 PM > >> >To: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Shankar, Uma > >> ><uma.shankar@xxxxxxxxx> > >> >Subject: Re: [PATCH 1/2] drm/i915/dsi: Send SHUTDOWN only > >> >for v3+ VBT's > >> > > >> >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. > >> > > >> > >> The issue is happening due to Device Ready setting. It was added to > >> resolve a DSI split screen issue in dual link DSI panels. Since DSI > >> dual link is not enabled and will require a bit of work in upstream, > >> we should revert this change. Vidya will send the change and also work on > >enabling dual link support in upstream. > >> > >> FYI - SHUTDOWN is a short packet peripheral command that turns off the > >> display in a Video Mode display module for power saving. > > > >There is no SHUTDOWN comamnd in the DCS spec that I can find. > > > > Hi Ville, > It's part of MIPI_DSI_specification - (v1-3 is the latest) > 8.8.5 Shutdown Peripheral Command, Data Type = 10 0010 (0x22) > Shutdown Peripheral command is a Short packet command that turns off the display > in a Video Mode display module for power saving. Note the interface shall remain powered > in order to receive the turn-on, or wake-up, command. Ah, thanks. I was only looking at the DCS spec. > > Regards, > Uma Shankar > > >> > >> For DSI, there have been issues with different panels requiring some > >> unconventional changes, and also bspec sometimes doesn't cover > >> everything generically. So, it gets a bit challenging :( > >> > >> Regards, > >> Uma Shankar > >> > >> >> > > >> >> > > >> >> > > > >> >> > > > >> >> > > > > >> >> > > > > >> >> > > > > >> >> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102404 > >> >> > > > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> > >> >> > > > --- > >> >> > > > 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@xxxxxxxxxxxxxxxxxxxxx > >> >> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> >> > -- > >> >> > Mika Kahola - Intel OTC > >> >-- > >> >Mika Kahola - Intel OTC > >> > > > >-- > >Ville Syrjälä > >Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
