| Message ID | 1504604671-17237-1-git-send-email-vidya.srinivas@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
The revert patch solved the issue *ERROR* Video mode command 0x00000041 send failed. on my setup with APL+MIPI/DSI single link combo. Tested-by: Mika Kahola <mika.kahola@intel.com> On Tue, 2017-09-05 at 15:14 +0530, Vidya Srinivas wrote: > From: Uma Shankar <uma.shankar@intel.com> > > This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282. > > Disable device ready before shutdown command was added previously to > avoid a > split screen issue seen on dual link DSI panels. As of now, dual link > is not > supported and will need some rework in the upstream code. For single > link DSI > panels, the change is not required. This will cause failure in > sending SHUTDOWN > packet during disable. Hence reverting the change. Will handle the > change > as part of dual link enabling in upstream. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > b/drivers/gpu/drm/i915/intel_dsi.c > index 2a0f5d3..fc25d7d 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct > intel_encoder *encoder, > const struct intel_crtc_state > *old_crtc_state, > const struct drm_connector_state > *old_conn_state) > { > - struct drm_device *dev = encoder->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- > >base); > enum port port; > > @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct > intel_encoder *encoder, > intel_panel_disable_backlight(old_conn_state); > > /* > - * Disable Device ready before the port shutdown in order > - * to avoid split screen > - */ > - if (IS_BROXTON(dev_priv)) { > - for_each_dsi_port(port, intel_dsi->ports) > - I915_WRITE(MIPI_DEVICE_READY(port), 0); > - } > - > - /* > * According to the spec we should send SHUTDOWN before > * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing > * has shown that the v3 sequence works for v2 VBTs too
> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of > Vidya Srinivas > Sent: Tuesday, September 5, 2017 3:15 PM > To: intel-gfx@lists.freedesktop.org > Cc: Srinivas, Vidya <vidya.srinivas@intel.com> > Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready > before shutdown command" > > From: Uma Shankar <uma.shankar@intel.com> > > This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282. > > Disable device ready before shutdown command was added previously to > avoid a split screen issue seen on dual link DSI panels. As of now, dual link is > not supported and will need some rework in the upstream code. For single > link DSI panels, the change is not required. This will cause failure in sending > SHUTDOWN packet during disable. Hence reverting the change. Will handle > the change as part of dual link enabling in upstream. Already there are lot of changes wrt to DSI dual link inside intel_dsi.c. Can't we keep this change as well under the condition if (dual_link && Broxton)?? Might help to fetch dual link changes in future instead of losing it. Regards, Madhav > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > b/drivers/gpu/drm/i915/intel_dsi.c > index 2a0f5d3..fc25d7d 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct intel_encoder > *encoder, > const struct intel_crtc_state *old_crtc_state, > const struct drm_connector_state > *old_conn_state) { > - struct drm_device *dev = encoder->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > enum port port; > > @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct intel_encoder > *encoder, > intel_panel_disable_backlight(old_conn_state); > > /* > - * Disable Device ready before the port shutdown in order > - * to avoid split screen > - */ > - if (IS_BROXTON(dev_priv)) { > - for_each_dsi_port(port, intel_dsi->ports) > - I915_WRITE(MIPI_DEVICE_READY(port), 0); > - } > - > - /* > * According to the spec we should send SHUTDOWN before > * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing > * has shown that the v3 sequence works for v2 VBTs too > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 05 Sep 2017, Mika Kahola <mika.kahola@intel.com> wrote: > The revert patch solved the issue > > *ERROR* Video mode command 0x00000041 send failed. > > on my setup with APL+MIPI/DSI single link combo. > > Tested-by: Mika Kahola <mika.kahola@intel.com> Pushed to dinq, thanks for the patch and testing. Alas, I failed to add your Tested-by tag, apologies. BR, Jani. > > > On Tue, 2017-09-05 at 15:14 +0530, Vidya Srinivas wrote: >> From: Uma Shankar <uma.shankar@intel.com> >> >> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282. >> >> Disable device ready before shutdown command was added previously to >> avoid a >> split screen issue seen on dual link DSI panels. As of now, dual link >> is not >> supported and will need some rework in the upstream code. For single >> link DSI >> panels, the change is not required. This will cause failure in >> sending SHUTDOWN >> packet during disable. Hence reverting the change. Will handle the >> change >> as part of dual link enabling in upstream. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dsi.c | 11 ----------- >> 1 file changed, 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c >> b/drivers/gpu/drm/i915/intel_dsi.c >> index 2a0f5d3..fc25d7d 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct >> intel_encoder *encoder, >> const struct intel_crtc_state >> *old_crtc_state, >> const struct drm_connector_state >> *old_conn_state) >> { >> - struct drm_device *dev = encoder->base.dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- >> >base); >> enum port port; >> >> @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct >> intel_encoder *encoder, >> intel_panel_disable_backlight(old_conn_state); >> >> /* >> - * Disable Device ready before the port shutdown in order >> - * to avoid split screen >> - */ >> - if (IS_BROXTON(dev_priv)) { >> - for_each_dsi_port(port, intel_dsi->ports) >> - I915_WRITE(MIPI_DEVICE_READY(port), 0); >> - } >> - >> - /* >> * According to the spec we should send SHUTDOWN before >> * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing >> * has shown that the v3 sequence works for v2 VBTs too
On Tue, 12 Sep 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote: >> -----Original Message----- >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of >> Vidya Srinivas >> Sent: Tuesday, September 5, 2017 3:15 PM >> To: intel-gfx@lists.freedesktop.org >> Cc: Srinivas, Vidya <vidya.srinivas@intel.com> >> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready >> before shutdown command" >> >> From: Uma Shankar <uma.shankar@intel.com> >> >> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282. >> >> Disable device ready before shutdown command was added previously to >> avoid a split screen issue seen on dual link DSI panels. As of now, dual link is >> not supported and will need some rework in the upstream code. For single >> link DSI panels, the change is not required. This will cause failure in sending >> SHUTDOWN packet during disable. Hence reverting the change. Will handle >> the change as part of dual link enabling in upstream. > > Already there are lot of changes wrt to DSI dual link inside intel_dsi.c. > Can't we keep this change as well under the condition if (dual_link && Broxton)?? > Might help to fetch dual link changes in future instead of losing it. I've pushed the patch as-is. I don't think there are grounds to keep it even with dual_link. As Uma says, these things may be panel specific too. BR, Jani.
> -----Original Message----- > From: Nikula, Jani > Sent: Wednesday, September 13, 2017 1:39 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; Srinivas, Vidya > <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device ready > before shutdown command" > > On Tue, 12 Sep 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> > wrote: > >> -----Original Message----- > >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On > >> Behalf Of Vidya Srinivas > >> Sent: Tuesday, September 5, 2017 3:15 PM > >> To: intel-gfx@lists.freedesktop.org > >> Cc: Srinivas, Vidya <vidya.srinivas@intel.com> > >> Subject: [Intel-gfx] [PATCH] Revert "drm/i915/bxt: Disable device > >> ready before shutdown command" > >> > >> From: Uma Shankar <uma.shankar@intel.com> > >> > >> This reverts commit bbdf0b2ff32aa75c7bd167569130e9391d2e6282. > >> > >> Disable device ready before shutdown command was added previously to > >> avoid a split screen issue seen on dual link DSI panels. As of now, > >> dual link is not supported and will need some rework in the upstream > >> code. For single link DSI panels, the change is not required. This > >> will cause failure in sending SHUTDOWN packet during disable. Hence > >> reverting the change. Will handle the change as part of dual link enabling > in upstream. > > > > Already there are lot of changes wrt to DSI dual link inside intel_dsi.c. > > Can't we keep this change as well under the condition if (dual_link && > Broxton)?? > > Might help to fetch dual link changes in future instead of losing it. > > I've pushed the patch as-is. I don't think there are grounds to keep it even > with dual_link. As Uma says, these things may be panel specific too. Hmm..Ok Thanks for clarification. Regards, Madhav > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 2a0f5d3..fc25d7d 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -892,8 +892,6 @@ static void intel_dsi_disable(struct intel_encoder *encoder, const struct intel_crtc_state *old_crtc_state, const struct drm_connector_state *old_conn_state) { - struct drm_device *dev = encoder->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); enum port port; @@ -903,15 +901,6 @@ static void intel_dsi_disable(struct intel_encoder *encoder, intel_panel_disable_backlight(old_conn_state); /* - * Disable Device ready before the port shutdown in order - * to avoid split screen - */ - if (IS_BROXTON(dev_priv)) { - for_each_dsi_port(port, intel_dsi->ports) - I915_WRITE(MIPI_DEVICE_READY(port), 0); - } - - /* * According to the spec we should send SHUTDOWN before * MIPI_SEQ_DISPLAY_OFF only for v3+ VBTs, but field testing * has shown that the v3 sequence works for v2 VBTs too