| Message ID | 20171017210156.54601-1-puthik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: > intel_dp_long_pulse() is always checking link status because > there has been known issues of link loss triggerring long pulse. > > However this is not needed for eDP display since we won't have > link loss for internal display. Also there are reports that > screens are flickering during link status check. (repro by > running modetest command repeatedly) > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > --- > drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 4b65cf137f79..75a77ef257e2 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > */ > status = connector_status_disconnected; > goto out; > - } else { > + } else if (status != connector_status_connected || > + intel_encoder->type != INTEL_OUTPUT_EDP) { > /* > * If display is now connected check links status, > * there has been known issues of link loss triggerring > @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > * going back up soon after. And once that happens we must > * retrain the link to get a picture. That's in case no > * userspace component reacted to intermittent HPD dip. > + * > + * Skip checking links status for connected eDP display. > + * There are known issues of display blinking during checking > + * link status and we don't have link loss for internal display. > */ Inside intel_dp_check_link_status(), it actually checks if link status is good by checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. So in case of eDP panel, if there is no link loss then it should always return link status as good and not retrain. So IMHO I dont think we need to explicitly avoid this for eDP. When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? Also look at this patch: https://patchwork.freedesktop.org/series/30670/ This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the link is good then it should never retrain even in intel_dp_check_link_status() for eDP. Manasi > intel_dp_check_link_status(intel_dp); > } > -- > 2.15.0.rc0.271.g36b669edcc-goog > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
I tried https://patchwork.freedesktop.org/series/30670/ but it doesn't work. > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? Yes. Repro is running "modetest" while PSR is on (easier to repro with PSR2 but PSR1 is fine too) > lane_align = dp_link_status(link_status, > DP_LANE_ALIGN_STATUS_UPDATED); > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) > return false; I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) So drm_dp_channel_eq_ok() always return false. If I disable PSR, drm_dp_channel_eq_ok() will return true and no blinking occurs. On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare <manasi.d.navare@intel.com> wrote: > On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: >> intel_dp_long_pulse() is always checking link status because >> there has been known issues of link loss triggerring long pulse. >> >> However this is not needed for eDP display since we won't have >> link loss for internal display. Also there are reports that >> screens are flickering during link status check. (repro by >> running modetest command repeatedly) >> >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 4b65cf137f79..75a77ef257e2 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> */ >> status = connector_status_disconnected; >> goto out; >> - } else { >> + } else if (status != connector_status_connected || >> + intel_encoder->type != INTEL_OUTPUT_EDP) { >> /* >> * If display is now connected check links status, >> * there has been known issues of link loss triggerring >> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> * going back up soon after. And once that happens we must >> * retrain the link to get a picture. That's in case no >> * userspace component reacted to intermittent HPD dip. >> + * >> + * Skip checking links status for connected eDP display. >> + * There are known issues of display blinking during checking >> + * link status and we don't have link loss for internal display. >> */ > > Inside intel_dp_check_link_status(), it actually checks if link status is good by > checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. > So in case of eDP panel, if there is no link loss then it should always return link > status as good and not retrain. > So IMHO I dont think we need to explicitly avoid this for eDP. > > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? > Also look at this patch: https://patchwork.freedesktop.org/series/30670/ > This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the > link is good then it should never retrain even in intel_dp_check_link_status() for eDP. > > Manasi >> intel_dp_check_link_status(intel_dp); >> } >> -- >> 2.15.0.rc0.271.g36b669edcc-goog >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 17, 2017 at 03:46:00PM -0700, Puthikorn Voravootivat wrote: > I tried https://patchwork.freedesktop.org/series/30670/ but it doesn't work. > > > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? > Yes. Repro is running "modetest" while PSR is on (easier to repro with > PSR2 but PSR1 is fine too) > > > lane_align = dp_link_status(link_status, > > DP_LANE_ALIGN_STATUS_UPDATED); > > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) > > return false; > > I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) > So drm_dp_channel_eq_ok() always return false. Hmm, it looks like there is a bug somewhere else that is causing the link status to get updated and bit LINK_STATUS_UPDATED from LANE_ALIGN_STATUS_UPDATED register is not being read so not being cleared causing the link to be retrained again. So I still feel that this patch is a workaround for another bug in the PSR code. But I will let others comment on this. Regards Manasi > > If I disable PSR, drm_dp_channel_eq_ok() will return true and no > blinking occurs. > > On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare > <manasi.d.navare@intel.com> wrote: > > On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: > >> intel_dp_long_pulse() is always checking link status because > >> there has been known issues of link loss triggerring long pulse. > >> > >> However this is not needed for eDP display since we won't have > >> link loss for internal display. Also there are reports that > >> screens are flickering during link status check. (repro by > >> running modetest command repeatedly) > >> > >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > >> --- > >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >> index 4b65cf137f79..75a77ef257e2 100644 > >> --- a/drivers/gpu/drm/i915/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/intel_dp.c > >> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > >> */ > >> status = connector_status_disconnected; > >> goto out; > >> - } else { > >> + } else if (status != connector_status_connected || > >> + intel_encoder->type != INTEL_OUTPUT_EDP) { > >> /* > >> * If display is now connected check links status, > >> * there has been known issues of link loss triggerring > >> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > >> * going back up soon after. And once that happens we must > >> * retrain the link to get a picture. That's in case no > >> * userspace component reacted to intermittent HPD dip. > >> + * > >> + * Skip checking links status for connected eDP display. > >> + * There are known issues of display blinking during checking > >> + * link status and we don't have link loss for internal display. > >> */ > > > > Inside intel_dp_check_link_status(), it actually checks if link status is good by > > checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. > > So in case of eDP panel, if there is no link loss then it should always return link > > status as good and not retrain. > > So IMHO I dont think we need to explicitly avoid this for eDP. > > > > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? > > Also look at this patch: https://patchwork.freedesktop.org/series/30670/ > > This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the > > link is good then it should never retrain even in intel_dp_check_link_status() for eDP. > > > > Manasi > >> intel_dp_check_link_status(intel_dp); > >> } > >> -- > >> 2.15.0.rc0.271.g36b669edcc-goog > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
+ Vathsala for PSR related code. On Tue, Oct 17, 2017 at 4:29 PM, Manasi Navare <manasi.d.navare@intel.com> wrote: > On Tue, Oct 17, 2017 at 03:46:00PM -0700, Puthikorn Voravootivat wrote: >> I tried https://patchwork.freedesktop.org/series/30670/ but it doesn't work. >> >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? >> Yes. Repro is running "modetest" while PSR is on (easier to repro with >> PSR2 but PSR1 is fine too) >> >> > lane_align = dp_link_status(link_status, >> > DP_LANE_ALIGN_STATUS_UPDATED); >> > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) >> > return false; >> >> I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) >> So drm_dp_channel_eq_ok() always return false. > > Hmm, it looks like there is a bug somewhere else that is causing the > link status to get updated and bit LINK_STATUS_UPDATED from LANE_ALIGN_STATUS_UPDATED register > is not being read so not being cleared causing the link to be retrained again. > > So I still feel that this patch is a workaround for another bug in the PSR code. > But I will let others comment on this. > > Regards > Manasi > >> >> If I disable PSR, drm_dp_channel_eq_ok() will return true and no >> blinking occurs. >> >> On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare >> <manasi.d.navare@intel.com> wrote: >> > On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: >> >> intel_dp_long_pulse() is always checking link status because >> >> there has been known issues of link loss triggerring long pulse. >> >> >> >> However this is not needed for eDP display since we won't have >> >> link loss for internal display. Also there are reports that >> >> screens are flickering during link status check. (repro by >> >> running modetest command repeatedly) >> >> >> >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> >> >> --- >> >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- >> >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> >> index 4b65cf137f79..75a77ef257e2 100644 >> >> --- a/drivers/gpu/drm/i915/intel_dp.c >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> >> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> >> */ >> >> status = connector_status_disconnected; >> >> goto out; >> >> - } else { >> >> + } else if (status != connector_status_connected || >> >> + intel_encoder->type != INTEL_OUTPUT_EDP) { >> >> /* >> >> * If display is now connected check links status, >> >> * there has been known issues of link loss triggerring >> >> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> >> * going back up soon after. And once that happens we must >> >> * retrain the link to get a picture. That's in case no >> >> * userspace component reacted to intermittent HPD dip. >> >> + * >> >> + * Skip checking links status for connected eDP display. >> >> + * There are known issues of display blinking during checking >> >> + * link status and we don't have link loss for internal display. >> >> */ >> > >> > Inside intel_dp_check_link_status(), it actually checks if link status is good by >> > checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. >> > So in case of eDP panel, if there is no link loss then it should always return link >> > status as good and not retrain. >> > So IMHO I dont think we need to explicitly avoid this for eDP. >> > >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? >> > Also look at this patch: https://patchwork.freedesktop.org/series/30670/ >> > This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the >> > link is good then it should never retrain even in intel_dp_check_link_status() for eDP. >> > >> > Manasi >> >> intel_dp_check_link_status(intel_dp); >> >> } >> >> -- >> >> 2.15.0.rc0.271.g36b669edcc-goog >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Puthikorn, could you please test the patch Manasi had pointed out [1] to see if that solves your issue instead of your patch? If that one solves it you put a Tested-by on that one to help that getting merged. If that is not the case so we need to hunt down PSR2 bugs that are killing the link status. [1]: https://patchwork.freedesktop.org/patch/178035/ Thanks, Rodrigo. On Tue, Oct 17, 2017 at 11:31:46PM +0000, Puthikorn Voravootivat wrote: > + Vathsala for PSR related code. > > On Tue, Oct 17, 2017 at 4:29 PM, Manasi Navare > <manasi.d.navare@intel.com> wrote: > > On Tue, Oct 17, 2017 at 03:46:00PM -0700, Puthikorn Voravootivat wrote: > >> I tried https://patchwork.freedesktop.org/series/30670/ but it doesn't work. > >> > >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? > >> Yes. Repro is running "modetest" while PSR is on (easier to repro with > >> PSR2 but PSR1 is fine too) > >> > >> > lane_align = dp_link_status(link_status, > >> > DP_LANE_ALIGN_STATUS_UPDATED); > >> > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) > >> > return false; > >> > >> I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) > >> So drm_dp_channel_eq_ok() always return false. > > > > Hmm, it looks like there is a bug somewhere else that is causing the > > link status to get updated and bit LINK_STATUS_UPDATED from LANE_ALIGN_STATUS_UPDATED register > > is not being read so not being cleared causing the link to be retrained again. > > > > So I still feel that this patch is a workaround for another bug in the PSR code. > > But I will let others comment on this. > > > > Regards > > Manasi > > > >> > >> If I disable PSR, drm_dp_channel_eq_ok() will return true and no > >> blinking occurs. > >> > >> On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare > >> <manasi.d.navare@intel.com> wrote: > >> > On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: > >> >> intel_dp_long_pulse() is always checking link status because > >> >> there has been known issues of link loss triggerring long pulse. > >> >> > >> >> However this is not needed for eDP display since we won't have > >> >> link loss for internal display. Also there are reports that > >> >> screens are flickering during link status check. (repro by > >> >> running modetest command repeatedly) > >> >> > >> >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > >> >> --- > >> >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- > >> >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >> >> index 4b65cf137f79..75a77ef257e2 100644 > >> >> --- a/drivers/gpu/drm/i915/intel_dp.c > >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c > >> >> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > >> >> */ > >> >> status = connector_status_disconnected; > >> >> goto out; > >> >> - } else { > >> >> + } else if (status != connector_status_connected || > >> >> + intel_encoder->type != INTEL_OUTPUT_EDP) { > >> >> /* > >> >> * If display is now connected check links status, > >> >> * there has been known issues of link loss triggerring > >> >> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > >> >> * going back up soon after. And once that happens we must > >> >> * retrain the link to get a picture. That's in case no > >> >> * userspace component reacted to intermittent HPD dip. > >> >> + * > >> >> + * Skip checking links status for connected eDP display. > >> >> + * There are known issues of display blinking during checking > >> >> + * link status and we don't have link loss for internal display. > >> >> */ > >> > > >> > Inside intel_dp_check_link_status(), it actually checks if link status is good by > >> > checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. > >> > So in case of eDP panel, if there is no link loss then it should always return link > >> > status as good and not retrain. > >> > So IMHO I dont think we need to explicitly avoid this for eDP. > >> > > >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? > >> > Also look at this patch: https://patchwork.freedesktop.org/series/30670/ > >> > This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the > >> > link is good then it should never retrain even in intel_dp_check_link_status() for eDP. > >> > > >> > Manasi > >> >> intel_dp_check_link_status(intel_dp); > >> >> } > >> >> -- > >> >> 2.15.0.rc0.271.g36b669edcc-goog > >> >> > >> >> _______________________________________________ > >> >> Intel-gfx mailing list > >> >> Intel-gfx@lists.freedesktop.org > >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Rodrigo The previous email (quote below) is tested with that patch applied. >> >> > lane_align = dp_link_status(link_status, >> >> > DP_LANE_ALIGN_STATUS_UPDATED); >> >> > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) >> >> > return false; >> >> >> >> I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) >> >> So drm_dp_channel_eq_ok() always return false. Thanks On Tue, Oct 17, 2017 at 4:39 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > Puthikorn, > > could you please test the patch Manasi had pointed out [1] to see if that > solves your issue instead of your patch? > > If that one solves it you put a Tested-by on that one to help that getting merged. > > If that is not the case so we need to hunt down PSR2 bugs that are killing the link status. > > [1]: https://patchwork.freedesktop.org/patch/178035/ > > Thanks, > Rodrigo. > > On Tue, Oct 17, 2017 at 11:31:46PM +0000, Puthikorn Voravootivat wrote: >> + Vathsala for PSR related code. >> >> On Tue, Oct 17, 2017 at 4:29 PM, Manasi Navare >> <manasi.d.navare@intel.com> wrote: >> > On Tue, Oct 17, 2017 at 03:46:00PM -0700, Puthikorn Voravootivat wrote: >> >> I tried https://patchwork.freedesktop.org/series/30670/ but it doesn't work. >> >> >> >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? >> >> Yes. Repro is running "modetest" while PSR is on (easier to repro with >> >> PSR2 but PSR1 is fine too) >> >> >> >> > lane_align = dp_link_status(link_status, >> >> > DP_LANE_ALIGN_STATUS_UPDATED); >> >> > if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) >> >> > return false; >> >> >> >> I always got lane_align = 128 (DP_LINK_STATUS_UPDATED) >> >> So drm_dp_channel_eq_ok() always return false. >> > >> > Hmm, it looks like there is a bug somewhere else that is causing the >> > link status to get updated and bit LINK_STATUS_UPDATED from LANE_ALIGN_STATUS_UPDATED register >> > is not being read so not being cleared causing the link to be retrained again. >> > >> > So I still feel that this patch is a workaround for another bug in the PSR code. >> > But I will let others comment on this. >> > >> > Regards >> > Manasi >> > >> >> >> >> If I disable PSR, drm_dp_channel_eq_ok() will return true and no >> >> blinking occurs. >> >> >> >> On Tue, Oct 17, 2017 at 2:21 PM, Manasi Navare >> >> <manasi.d.navare@intel.com> wrote: >> >> > On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote: >> >> >> intel_dp_long_pulse() is always checking link status because >> >> >> there has been known issues of link loss triggerring long pulse. >> >> >> >> >> >> However this is not needed for eDP display since we won't have >> >> >> link loss for internal display. Also there are reports that >> >> >> screens are flickering during link status check. (repro by >> >> >> running modetest command repeatedly) >> >> >> >> >> >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> >> >> >> --- >> >> >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- >> >> >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> >> >> index 4b65cf137f79..75a77ef257e2 100644 >> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c >> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> >> >> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> >> >> */ >> >> >> status = connector_status_disconnected; >> >> >> goto out; >> >> >> - } else { >> >> >> + } else if (status != connector_status_connected || >> >> >> + intel_encoder->type != INTEL_OUTPUT_EDP) { >> >> >> /* >> >> >> * If display is now connected check links status, >> >> >> * there has been known issues of link loss triggerring >> >> >> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) >> >> >> * going back up soon after. And once that happens we must >> >> >> * retrain the link to get a picture. That's in case no >> >> >> * userspace component reacted to intermittent HPD dip. >> >> >> + * >> >> >> + * Skip checking links status for connected eDP display. >> >> >> + * There are known issues of display blinking during checking >> >> >> + * link status and we don't have link loss for internal display. >> >> >> */ >> >> > >> >> > Inside intel_dp_check_link_status(), it actually checks if link status is good by >> >> > checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain. >> >> > So in case of eDP panel, if there is no link loss then it should always return link >> >> > status as good and not retrain. >> >> > So IMHO I dont think we need to explicitly avoid this for eDP. >> >> > >> >> > When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false? >> >> > Also look at this patch: https://patchwork.freedesktop.org/series/30670/ >> >> > This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the >> >> > link is good then it should never retrain even in intel_dp_check_link_status() for eDP. >> >> > >> >> > Manasi >> >> >> intel_dp_check_link_status(intel_dp); >> >> >> } >> >> >> -- >> >> >> 2.15.0.rc0.271.g36b669edcc-goog >> >> >> >> >> >> _______________________________________________ >> >> >> Intel-gfx mailing list >> >> >> Intel-gfx@lists.freedesktop.org >> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4b65cf137f79..75a77ef257e2 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) */ status = connector_status_disconnected; goto out; - } else { + } else if (status != connector_status_connected || + intel_encoder->type != INTEL_OUTPUT_EDP) { /* * If display is now connected check links status, * there has been known issues of link loss triggerring @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) * going back up soon after. And once that happens we must * retrain the link to get a picture. That's in case no * userspace component reacted to intermittent HPD dip. + * + * Skip checking links status for connected eDP display. + * There are known issues of display blinking during checking + * link status and we don't have link loss for internal display. */ intel_dp_check_link_status(intel_dp); }
intel_dp_long_pulse() is always checking link status because there has been known issues of link loss triggerring long pulse. However this is not needed for eDP display since we won't have link loss for internal display. Also there are reports that screens are flickering during link status check. (repro by running modetest command repeatedly) Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> --- drivers/gpu/drm/i915/intel_dp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)