| Message ID | 1504252261-28964-2-git-send-email-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > > 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
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. > > > > > > > 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
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? 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). > > > > > > > > > > > > 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
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
>-----Original Message----- >From: Kahola, Mika >Sent: Tuesday, September 5, 2017 2:03 PM >To: Ville Syrjälä <ville.syrjala@linux.intel.com> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com> >Subject: Re: [Intel-gfx] [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. 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@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
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@linux.intel.com> > >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com> > >Subject: Re: [Intel-gfx] [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. > > 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@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 >
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, September 5, 2017 6:46 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: Kahola, Mika <mika.kahola@intel.com>; Srinivas, Vidya ><vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [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@linux.intel.com> >> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma >> ><uma.shankar@intel.com> >> >Subject: Re: [Intel-gfx] [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. 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@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 >> > >-- >Ville Syrjälä >Intel OTC
On Tue, Sep 05, 2017 at 02:55:31PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Tuesday, September 5, 2017 6:46 PM > >To: Shankar, Uma <uma.shankar@intel.com> > >Cc: Kahola, Mika <mika.kahola@intel.com>; Srinivas, Vidya > ><vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [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@linux.intel.com> > >> >Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma > >> ><uma.shankar@intel.com> > >> >Subject: Re: [Intel-gfx] [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@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 > >> > > > >-- > >Ville Syrjälä > >Intel OTC
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);
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. 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(-)