| Message ID | 1507835618-23051-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: > In case of eDP because the panel has a fixed mode, the link rate > and lane count at which it is trained corresponds to the link BW > required to support the native resolution of the panel. In case of > panles with lower resolutions where fewer lanes are hooked up internally, > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. > So it is pointless to fallback to lower link rate/lane count in case > of link training failure on eDP connector since the lower link BW > will not support the native resolution of the panel and we cannot > prune the preferred mode on the eDP connector. > > In case of Link training failure on the eDP panel, something is wrong > in the HW internally and hence driver errors out with a loud > and clear DRM_ERROR message. > > v2: > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) > > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> This fell through the cracks, looks like it partially fixes https://bugs.freedesktop.org/show_bug.cgi?id=103369 Why link training fails there is not clear. > --- > drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 05907fa..cf8fef8 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > return; > > failure_handling: > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > - intel_connector->base.base.id, > - intel_connector->base.name, > - intel_dp->link_rate, intel_dp->lane_count); > - if (!intel_dp_get_link_train_fallback_values(intel_dp, > - intel_dp->link_rate, > - intel_dp->lane_count)) > - /* Schedule a Hotplug Uevent to userspace to start modeset */ > - schedule_work(&intel_connector->modeset_retry_work); > + /* Dont fallback and prune modes if its eDP */ > + if (!intel_dp_is_edp(intel_dp)) { > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > + intel_connector->base.base.id, > + intel_connector->base.name, > + intel_dp->link_rate, intel_dp->lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + intel_dp->link_rate, > + intel_dp->lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > + intel_connector->base.base.id, > + intel_connector->base.name, > + intel_dp->link_rate, intel_dp->lane_count); > + } > return; > } > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: > On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: > > In case of eDP because the panel has a fixed mode, the link rate > > and lane count at which it is trained corresponds to the link BW > > required to support the native resolution of the panel. In case of > > panles with lower resolutions where fewer lanes are hooked up internally, > > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. > > So it is pointless to fallback to lower link rate/lane count in case > > of link training failure on eDP connector since the lower link BW > > will not support the native resolution of the panel and we cannot > > prune the preferred mode on the eDP connector. > > > > In case of Link training failure on the eDP panel, something is wrong > > in the HW internally and hence driver errors out with a loud > > and clear DRM_ERROR message. > > > > v2: > > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) > > > > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > > Cc: Jim Bride <jim.bride@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Dave Airlie <airlied@redhat.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> > > This fell through the cracks, looks like it partially fixes > https://bugs.freedesktop.org/show_bug.cgi?id=103369 > > Why link training fails there is not clear. Ok, the link training fail turned out to be a race between a modeset link training and a link retraining called from runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that one was fixed meanwhile by commit 42e5e65765265485ecf2a480c244d76c2c624449 Author: Daniel Vetter <daniel.vetter@ffwll.ch> AuthorDate: Mon Nov 13 17:01:40 2017 +0100 Commit: Daniel Vetter <daniel.vetter@ffwll.ch> CommitDate: Thu Nov 23 14:59:07 2017 +0100 drm/i915: sync dp link status checks against atomic commmits I merged now this fix to address the other issue, adding the above bug as reference. Thanks for the patch and the review. --Imre > > > --- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++++++++++++--------- > > 1 file changed, 17 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 05907fa..cf8fef8 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) > > return; > > > > failure_handling: > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > > - intel_connector->base.base.id, > > - intel_connector->base.name, > > - intel_dp->link_rate, intel_dp->lane_count); > > - if (!intel_dp_get_link_train_fallback_values(intel_dp, > > - intel_dp->link_rate, > > - intel_dp->lane_count)) > > - /* Schedule a Hotplug Uevent to userspace to start modeset */ > > - schedule_work(&intel_connector->modeset_retry_work); > > + /* Dont fallback and prune modes if its eDP */ > > + if (!intel_dp_is_edp(intel_dp)) { > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > > + intel_connector->base.base.id, > > + intel_connector->base.name, > > + intel_dp->link_rate, intel_dp->lane_count); > > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > > + intel_dp->link_rate, > > + intel_dp->lane_count)) > > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > > + schedule_work(&intel_connector->modeset_retry_work); > > + } else { > > + DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", > > + intel_connector->base.base.id, > > + intel_connector->base.name, > > + intel_dp->link_rate, intel_dp->lane_count); > > + } > > return; > > } > > -- > > 2.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: > On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: >> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: >> > In case of eDP because the panel has a fixed mode, the link rate >> > and lane count at which it is trained corresponds to the link BW >> > required to support the native resolution of the panel. In case of >> > panles with lower resolutions where fewer lanes are hooked up internally, >> > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. >> > So it is pointless to fallback to lower link rate/lane count in case >> > of link training failure on eDP connector since the lower link BW >> > will not support the native resolution of the panel and we cannot >> > prune the preferred mode on the eDP connector. >> > >> > In case of Link training failure on the eDP panel, something is wrong >> > in the HW internally and hence driver errors out with a loud >> > and clear DRM_ERROR message. >> > >> > v2: >> > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) >> > >> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> >> > Cc: Jim Bride <jim.bride@linux.intel.com> >> > Cc: Jani Nikula <jani.nikula@linux.intel.com> >> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> > Cc: Dave Airlie <airlied@redhat.com> >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> This fell through the cracks, looks like it partially fixes >> https://bugs.freedesktop.org/show_bug.cgi?id=103369 >> >> Why link training fails there is not clear. > > Ok, the link training fail turned out to be a race between a modeset > link training and a link retraining called from > runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that > one was fixed meanwhile by > > commit 42e5e65765265485ecf2a480c244d76c2c624449 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > AuthorDate: Mon Nov 13 17:01:40 2017 +0100 > Commit: Daniel Vetter <daniel.vetter@ffwll.ch> > CommitDate: Thu Nov 23 14:59:07 2017 +0100 > > drm/i915: sync dp link status checks against atomic commmits > > I merged now this fix to address the other issue, adding the above bug > as reference. Thanks for the patch and the review. Thanks for the follow-up... but should we have added a Fixes: or cc: stable tag here? BR, Jani. > > --Imre > >> >> > --- >> > drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++++++++++++--------- >> > 1 file changed, 17 insertions(+), 9 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > index 05907fa..cf8fef8 100644 >> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c >> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) >> > return; >> > >> > failure_handling: >> > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", >> > - intel_connector->base.base.id, >> > - intel_connector->base.name, >> > - intel_dp->link_rate, intel_dp->lane_count); >> > - if (!intel_dp_get_link_train_fallback_values(intel_dp, >> > - intel_dp->link_rate, >> > - intel_dp->lane_count)) >> > - /* Schedule a Hotplug Uevent to userspace to start modeset */ >> > - schedule_work(&intel_connector->modeset_retry_work); >> > + /* Dont fallback and prune modes if its eDP */ >> > + if (!intel_dp_is_edp(intel_dp)) { >> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", >> > + intel_connector->base.base.id, >> > + intel_connector->base.name, >> > + intel_dp->link_rate, intel_dp->lane_count); >> > + if (!intel_dp_get_link_train_fallback_values(intel_dp, >> > + intel_dp->link_rate, >> > + intel_dp->lane_count)) >> > + /* Schedule a Hotplug Uevent to userspace to start modeset */ >> > + schedule_work(&intel_connector->modeset_retry_work); >> > + } else { >> > + DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", >> > + intel_connector->base.base.id, >> > + intel_connector->base.name, >> > + intel_dp->link_rate, intel_dp->lane_count); >> > + } >> > return; >> > } >> > -- >> > 2.1.4 >> > >> > _______________________________________________ >> > Intel-gfx mailing list >> > Intel-gfx@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 23, 2018 at 11:48:22AM +0200, Jani Nikula wrote: > On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: > > On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: > >> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: > >> > In case of eDP because the panel has a fixed mode, the link rate > >> > and lane count at which it is trained corresponds to the link BW > >> > required to support the native resolution of the panel. In case of > >> > panles with lower resolutions where fewer lanes are hooked up internally, > >> > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. > >> > So it is pointless to fallback to lower link rate/lane count in case > >> > of link training failure on eDP connector since the lower link BW > >> > will not support the native resolution of the panel and we cannot > >> > prune the preferred mode on the eDP connector. > >> > > >> > In case of Link training failure on the eDP panel, something is wrong > >> > in the HW internally and hence driver errors out with a loud > >> > and clear DRM_ERROR message. > >> > > >> > v2: > >> > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) > >> > > >> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > >> > Cc: Jim Bride <jim.bride@linux.intel.com> > >> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> > Cc: Dave Airlie <airlied@redhat.com> > >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > >> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> > >> > >> This fell through the cracks, looks like it partially fixes > >> https://bugs.freedesktop.org/show_bug.cgi?id=103369 > >> > >> Why link training fails there is not clear. > > > > Ok, the link training fail turned out to be a race between a modeset > > link training and a link retraining called from > > runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that > > one was fixed meanwhile by > > > > commit 42e5e65765265485ecf2a480c244d76c2c624449 > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > AuthorDate: Mon Nov 13 17:01:40 2017 +0100 > > Commit: Daniel Vetter <daniel.vetter@ffwll.ch> > > CommitDate: Thu Nov 23 14:59:07 2017 +0100 > > > > drm/i915: sync dp link status checks against atomic commmits > > > > I merged now this fix to address the other issue, adding the above bug > > as reference. Thanks for the patch and the review. > > Thanks for the follow-up... but should we have added a Fixes: or cc: > stable tag here? Fixes: 9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link training failure") I wasn't sure about stable, since for me the link training failure happened only due to the bug fixed by 42e5e65765265. In any case I can't see how it could cause problems, so yes let's Cc: stable too. --Imre
On Tue, Jan 23, 2018 at 02:57:04PM +0200, Imre Deak wrote: > On Tue, Jan 23, 2018 at 11:48:22AM +0200, Jani Nikula wrote: > > On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: > > > On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: > > >> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: > > >> > In case of eDP because the panel has a fixed mode, the link rate > > >> > and lane count at which it is trained corresponds to the link BW > > >> > required to support the native resolution of the panel. In case of > > >> > panles with lower resolutions where fewer lanes are hooked up internally, > > >> > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. > > >> > So it is pointless to fallback to lower link rate/lane count in case > > >> > of link training failure on eDP connector since the lower link BW > > >> > will not support the native resolution of the panel and we cannot > > >> > prune the preferred mode on the eDP connector. > > >> > > > >> > In case of Link training failure on the eDP panel, something is wrong > > >> > in the HW internally and hence driver errors out with a loud > > >> > and clear DRM_ERROR message. > > >> > > > >> > v2: > > >> > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) > > >> > > > >> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > > >> > Cc: Jim Bride <jim.bride@linux.intel.com> > > >> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > >> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > >> > Cc: Dave Airlie <airlied@redhat.com> > > >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > >> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > >> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> > > >> > > >> This fell through the cracks, looks like it partially fixes > > >> https://bugs.freedesktop.org/show_bug.cgi?id=103369 > > >> > > >> Why link training fails there is not clear. > > > > > > Ok, the link training fail turned out to be a race between a modeset > > > link training and a link retraining called from > > > runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that > > > one was fixed meanwhile by > > > > > > commit 42e5e65765265485ecf2a480c244d76c2c624449 > > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > > AuthorDate: Mon Nov 13 17:01:40 2017 +0100 > > > Commit: Daniel Vetter <daniel.vetter@ffwll.ch> > > > CommitDate: Thu Nov 23 14:59:07 2017 +0100 > > > > > > drm/i915: sync dp link status checks against atomic commmits > > > > > > I merged now this fix to address the other issue, adding the above bug > > > as reference. Thanks for the patch and the review. > > > > Thanks for the follow-up... but should we have added a Fixes: or cc: > > stable tag here? > > Fixes: 9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link > training failure") > > I wasn't sure about stable, since for me the link training failure > happened only due to the bug fixed by 42e5e65765265. In any case I can't > see how it could cause problems, so yes let's Cc: stable too. > > --Imre Thanks for your feedback Imre and Jani. This patch avoids fallback and modeset retry for link train failure on edp. But lately we have seen a bunch of buggy panels that require just another modeset at same link params to pass link training. So one idea I had and as suggested by DK was that we try harder before failing on eDP. So I was planning to add this code: On eDP, if link train fails -> power cycle the panel -> Just call modeset retry function without fallback -> send a uevent for modeset retry. What do you think? Manasi
On Tue, 23 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: > On Tue, Jan 23, 2018 at 11:48:22AM +0200, Jani Nikula wrote: >> On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: >> > On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: >> >> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: >> >> > In case of eDP because the panel has a fixed mode, the link rate >> >> > and lane count at which it is trained corresponds to the link BW >> >> > required to support the native resolution of the panel. In case of >> >> > panles with lower resolutions where fewer lanes are hooked up internally, >> >> > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. >> >> > So it is pointless to fallback to lower link rate/lane count in case >> >> > of link training failure on eDP connector since the lower link BW >> >> > will not support the native resolution of the panel and we cannot >> >> > prune the preferred mode on the eDP connector. >> >> > >> >> > In case of Link training failure on the eDP panel, something is wrong >> >> > in the HW internally and hence driver errors out with a loud >> >> > and clear DRM_ERROR message. >> >> > >> >> > v2: >> >> > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) >> >> > >> >> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> >> >> > Cc: Jim Bride <jim.bride@linux.intel.com> >> >> > Cc: Jani Nikula <jani.nikula@linux.intel.com> >> >> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> > Cc: Dave Airlie <airlied@redhat.com> >> >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> >> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >> >> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> >> >> This fell through the cracks, looks like it partially fixes >> >> https://bugs.freedesktop.org/show_bug.cgi?id=103369 >> >> >> >> Why link training fails there is not clear. >> > >> > Ok, the link training fail turned out to be a race between a modeset >> > link training and a link retraining called from >> > runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that >> > one was fixed meanwhile by >> > >> > commit 42e5e65765265485ecf2a480c244d76c2c624449 >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch> >> > AuthorDate: Mon Nov 13 17:01:40 2017 +0100 >> > Commit: Daniel Vetter <daniel.vetter@ffwll.ch> >> > CommitDate: Thu Nov 23 14:59:07 2017 +0100 >> > >> > drm/i915: sync dp link status checks against atomic commmits >> > >> > I merged now this fix to address the other issue, adding the above bug >> > as reference. Thanks for the patch and the review. >> >> Thanks for the follow-up... but should we have added a Fixes: or cc: >> stable tag here? > > Fixes: 9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link > training failure") > > I wasn't sure about stable, since for me the link training failure > happened only due to the bug fixed by 42e5e65765265. In any case I can't > see how it could cause problems, so yes let's Cc: stable too. Rodrigo, here's another one to cherry-pick to drm-intel-next-fixes. BR, Jani.
On 30.01.2018 09:38, Jani Nikula wrote: > On Tue, 23 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: >> On Tue, Jan 23, 2018 at 11:48:22AM +0200, Jani Nikula wrote: >>> On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: >>>> On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: >>>>> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: >>>>>> In case of eDP because the panel has a fixed mode, the link rate >>>>>> and lane count at which it is trained corresponds to the link BW >>>>>> required to support the native resolution of the panel. In case of >>>>>> panles with lower resolutions where fewer lanes are hooked up internally, >>>>>> that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. >>>>>> So it is pointless to fallback to lower link rate/lane count in case >>>>>> of link training failure on eDP connector since the lower link BW >>>>>> will not support the native resolution of the panel and we cannot >>>>>> prune the preferred mode on the eDP connector. >>>>>> >>>>>> In case of Link training failure on the eDP panel, something is wrong >>>>>> in the HW internally and hence driver errors out with a loud >>>>>> and clear DRM_ERROR message. >>>>>> >>>>>> v2: >>>>>> * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) >>>>>> >>>>>> Cc: Clinton Taylor <clinton.a.taylor@intel.com> >>>>>> Cc: Jim Bride <jim.bride@linux.intel.com> >>>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >>>>>> Cc: Dave Airlie <airlied@redhat.com> >>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >>>>>> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> >>>>> >>>>> This fell through the cracks, looks like it partially fixes >>>>> https://bugs.freedesktop.org/show_bug.cgi?id=103369 >>>>> >>>>> Why link training fails there is not clear. >>>> >>>> Ok, the link training fail turned out to be a race between a modeset >>>> link training and a link retraining called from >>>> runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that >>>> one was fixed meanwhile by >>>> >>>> commit 42e5e65765265485ecf2a480c244d76c2c624449 >>>> Author: Daniel Vetter <daniel.vetter@ffwll.ch> >>>> AuthorDate: Mon Nov 13 17:01:40 2017 +0100 >>>> Commit: Daniel Vetter <daniel.vetter@ffwll.ch> >>>> CommitDate: Thu Nov 23 14:59:07 2017 +0100 >>>> >>>> drm/i915: sync dp link status checks against atomic commmits >>>> >>>> I merged now this fix to address the other issue, adding the above bug >>>> as reference. Thanks for the patch and the review. >>> >>> Thanks for the follow-up... but should we have added a Fixes: or cc: >>> stable tag here? >> >> Fixes: 9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link >> training failure") >> >> I wasn't sure about stable, since for me the link training failure >> happened only due to the bug fixed by 42e5e65765265. In any case I can't >> see how it could cause problems, so yes let's Cc: stable too. > > Rodrigo, here's another one to cherry-pick to drm-intel-next-fixes. This patch fixes a regression with a BIOS upgrade on a Dell machine, where the screen would stay blank after resume from suspend. So I'd like it to find it's way to 4.15.x if that's still a thing.
On Thu, 12 Apr 2018, Timo Aaltonen <tjaalton@ubuntu.com> wrote: > On 30.01.2018 09:38, Jani Nikula wrote: >> On Tue, 23 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: >>> On Tue, Jan 23, 2018 at 11:48:22AM +0200, Jani Nikula wrote: >>>> On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote: >>>>> On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote: >>>>>> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote: >>>>>>> In case of eDP because the panel has a fixed mode, the link rate >>>>>>> and lane count at which it is trained corresponds to the link BW >>>>>>> required to support the native resolution of the panel. In case of >>>>>>> panles with lower resolutions where fewer lanes are hooked up internally, >>>>>>> that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. >>>>>>> So it is pointless to fallback to lower link rate/lane count in case >>>>>>> of link training failure on eDP connector since the lower link BW >>>>>>> will not support the native resolution of the panel and we cannot >>>>>>> prune the preferred mode on the eDP connector. >>>>>>> >>>>>>> In case of Link training failure on the eDP panel, something is wrong >>>>>>> in the HW internally and hence driver errors out with a loud >>>>>>> and clear DRM_ERROR message. >>>>>>> >>>>>>> v2: >>>>>>> * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) >>>>>>> >>>>>>> Cc: Clinton Taylor <clinton.a.taylor@intel.com> >>>>>>> Cc: Jim Bride <jim.bride@linux.intel.com> >>>>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >>>>>>> Cc: Dave Airlie <airlied@redhat.com> >>>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>>>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >>>>>>> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> >>>>>> >>>>>> This fell through the cracks, looks like it partially fixes >>>>>> https://bugs.freedesktop.org/show_bug.cgi?id=103369 >>>>>> >>>>>> Why link training fails there is not clear. >>>>> >>>>> Ok, the link training fail turned out to be a race between a modeset >>>>> link training and a link retraining called from >>>>> runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that >>>>> one was fixed meanwhile by >>>>> >>>>> commit 42e5e65765265485ecf2a480c244d76c2c624449 >>>>> Author: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>> AuthorDate: Mon Nov 13 17:01:40 2017 +0100 >>>>> Commit: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>> CommitDate: Thu Nov 23 14:59:07 2017 +0100 >>>>> >>>>> drm/i915: sync dp link status checks against atomic commmits >>>>> >>>>> I merged now this fix to address the other issue, adding the above bug >>>>> as reference. Thanks for the patch and the review. >>>> >>>> Thanks for the follow-up... but should we have added a Fixes: or cc: >>>> stable tag here? >>> >>> Fixes: 9301397a63b3 ("drm/i915: Implement Link Rate fallback on Link >>> training failure") >>> >>> I wasn't sure about stable, since for me the link training failure >>> happened only due to the bug fixed by 42e5e65765265. In any case I can't >>> see how it could cause problems, so yes let's Cc: stable too. >> >> Rodrigo, here's another one to cherry-pick to drm-intel-next-fixes. > > This patch fixes a regression with a BIOS upgrade on a Dell machine, > where the screen would stay blank after resume from suspend. So I'd like > it to find it's way to 4.15.x if that's still a thing. I made the backport request to v4.13+, up to stable team which kernels they care about. BR, Jani.
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 05907fa..cf8fef8 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) return; failure_handling: - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", - intel_connector->base.base.id, - intel_connector->base.name, - intel_dp->link_rate, intel_dp->lane_count); - if (!intel_dp_get_link_train_fallback_values(intel_dp, - intel_dp->link_rate, - intel_dp->lane_count)) - /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); + /* Dont fallback and prune modes if its eDP */ + if (!intel_dp_is_edp(intel_dp)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", + intel_connector->base.base.id, + intel_connector->base.name, + intel_dp->link_rate, intel_dp->lane_count); + if (!intel_dp_get_link_train_fallback_values(intel_dp, + intel_dp->link_rate, + intel_dp->lane_count)) + /* Schedule a Hotplug Uevent to userspace to start modeset */ + schedule_work(&intel_connector->modeset_retry_work); + } else { + DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", + intel_connector->base.base.id, + intel_connector->base.name, + intel_dp->link_rate, intel_dp->lane_count); + } return; }
In case of eDP because the panel has a fixed mode, the link rate and lane count at which it is trained corresponds to the link BW required to support the native resolution of the panel. In case of panles with lower resolutions where fewer lanes are hooked up internally, that number is reflected in the MAX_LANE_COUNT DPCD register of the panel. So it is pointless to fallback to lower link rate/lane count in case of link training failure on eDP connector since the lower link BW will not support the native resolution of the panel and we cannot prune the preferred mode on the eDP connector. In case of Link training failure on the eDP panel, something is wrong in the HW internally and hence driver errors out with a loud and clear DRM_ERROR message. v2: * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala) Cc: Clinton Taylor <clinton.a.taylor@intel.com> Cc: Jim Bride <jim.bride@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)