Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
On Fri, 2018-09-07 at 22:18 +0300, Ville Syrjälä wrote:
> On Fri, Sep 07, 2018 at 11:31:15AM -0700, Dhinakaran Pandiyan wrote:
> > On Fri, 2018-09-07 at 09:25 -0700, Manasi Navare wrote:
> > > On Fri, Sep 07, 2018 at 05:34:23PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Sep 06, 2018 at 11:21:34PM -0700, Dhinakaran Pandiyan
> > > > wrote:
> > > > > commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link
> > > > > quality
> > > > > check,
> > > > > unconditionally during long pulse"")' applies a work around
> > > > > for
> > > > > monitors
> > > > > that don't signal link loss. Apply this only for external
> > > > > displays as
> > > > > eDP features like PSR when active will have the link turned
> > > > > off
> > > > > and the
> > > > > driver ends up retraining the link seeeing that link is not
> > > > > synchronized.
> > > > >
> > > > > Cc: Lyude Paul
> > > > > Cc: Jan-Marek Glogowski
> > > > > Cc: Ville Syrjälä
> > > > > Cc: Rodrigo Vivi
> > > > > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link
> > > > > quality check, unconditionally during long pulse"")
> > > > > Signed-off-by: Dhinakaran Pandiyan > > > > .com
> > > > > >
> > > > >
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index befdaa076d8c..c59b2f78f41a 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct
> > > > > intel_connector
> > > > > *connector,
> > > > >*/
> > > > > status = connector_status_disconnected;
> > > > > goto out;
> > > > > - } else {
> > > > > + }
> > > > > +
> > > > > + if (!intel_dp_is_edp(intel_dp)) {
> > >
> > > Also what happens when the PSR is not active? We would still need
> > > to
> > > retrain
> > > even on eDP.
> > > eDP tends to get fixed a lot of times during link retraining.
> > >
> > > Manasi
> > >
> > > >
> > > > Hmm. How do things work if we get a short hpd from the sink
> > > > while
> > > > it's
> > > > in PSR link off mode? Won't we try to retrain in that case as
> > > > well?
> > > >
> >
> > Looks like short pulse defers retraining to ->detect(), so with
> > this
> > change we end up not retraining.
>
> Except the ->hotplug() hook will still do the retraining I guess?
> It's a
> bit of a mess now with this getting handled in two places.
>
Yeah, we'll have to fix shortpulse handling in a separate patch. Link
standby v/s link off also determines how we should handle short pulse
from the sink. A retrain is required if PSR put link in standby and
then it went out of sync.
> > But, we do want to retrain when we get
> > a short pulse for a bad link and the link is expected to be
> > synchronized.
> >
> > > > > /*
> > > > >* Check link status in case the link went
> > > > > out
> > > > > of
> > > > >* synchronization and the sink did not tell
> > > > > us.
> > > > > --
> > > > > 2.17.1
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel
> > > > ___
> > > > 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
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
On Fri, Sep 07, 2018 at 11:31:15AM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-09-07 at 09:25 -0700, Manasi Navare wrote:
> > On Fri, Sep 07, 2018 at 05:34:23PM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 06, 2018 at 11:21:34PM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > > > check,
> > > > unconditionally during long pulse"")' applies a work around for
> > > > monitors
> > > > that don't signal link loss. Apply this only for external
> > > > displays as
> > > > eDP features like PSR when active will have the link turned off
> > > > and the
> > > > driver ends up retraining the link seeeing that link is not
> > > > synchronized.
> > > >
> > > > Cc: Lyude Paul
> > > > Cc: Jan-Marek Glogowski
> > > > Cc: Ville Syrjälä
> > > > Cc: Rodrigo Vivi
> > > > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link
> > > > quality check, unconditionally during long pulse"")
> > > > Signed-off-by: Dhinakaran Pandiyan > > > >
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index befdaa076d8c..c59b2f78f41a 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct intel_connector
> > > > *connector,
> > > > */
> > > > status = connector_status_disconnected;
> > > > goto out;
> > > > - } else {
> > > > + }
> > > > +
> > > > + if (!intel_dp_is_edp(intel_dp)) {
> >
> > Also what happens when the PSR is not active? We would still need to
> > retrain
> > even on eDP.
> > eDP tends to get fixed a lot of times during link retraining.
> >
> > Manasi
> >
> > >
> > > Hmm. How do things work if we get a short hpd from the sink while
> > > it's
> > > in PSR link off mode? Won't we try to retrain in that case as well?
> > >
> Looks like short pulse defers retraining to ->detect(), so with this
> change we end up not retraining.
Except the ->hotplug() hook will still do the retraining I guess? It's a
bit of a mess now with this getting handled in two places.
> But, we do want to retrain when we get
> a short pulse for a bad link and the link is expected to be
> synchronized.
>
> > > > /*
> > > > * Check link status in case the link went out
> > > > of
> > > > * synchronization and the sink did not tell us.
> > > > --
> > > > 2.17.1
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> > > ___
> > > 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
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
On Fri, 2018-09-07 at 09:25 -0700, Manasi Navare wrote:
> On Fri, Sep 07, 2018 at 05:34:23PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 06, 2018 at 11:21:34PM -0700, Dhinakaran Pandiyan
> > wrote:
> > > commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > > check,
> > > unconditionally during long pulse"")' applies a work around for
> > > monitors
> > > that don't signal link loss. Apply this only for external
> > > displays as
> > > eDP features like PSR when active will have the link turned off
> > > and the
> > > driver ends up retraining the link seeeing that link is not
> > > synchronized.
> > >
> > > Cc: Lyude Paul
> > > Cc: Jan-Marek Glogowski
> > > Cc: Ville Syrjälä
> > > Cc: Rodrigo Vivi
> > > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link
> > > quality check, unconditionally during long pulse"")
> > > Signed-off-by: Dhinakaran Pandiyan > > >
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index befdaa076d8c..c59b2f78f41a 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct intel_connector
> > > *connector,
> > >*/
> > > status = connector_status_disconnected;
> > > goto out;
> > > - } else {
> > > + }
> > > +
> > > + if (!intel_dp_is_edp(intel_dp)) {
>
> Also what happens when the PSR is not active? We would still need to
> retrain
> even on eDP.
> eDP tends to get fixed a lot of times during link retraining.
>
> Manasi
>
> >
> > Hmm. How do things work if we get a short hpd from the sink while
> > it's
> > in PSR link off mode? Won't we try to retrain in that case as well?
> >
Looks like short pulse defers retraining to ->detect(), so with this
change we end up not retraining. But, we do want to retrain when we get
a short pulse for a bad link and the link is expected to be
synchronized.
> > > /*
> > >* Check link status in case the link went out
> > > of
> > >* synchronization and the sink did not tell us.
> > > --
> > > 2.17.1
> >
> > --
> > Ville Syrjälä
> > Intel
> > ___
> > 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
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
On Fri, Sep 07, 2018 at 05:34:23PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 06, 2018 at 11:21:34PM -0700, Dhinakaran Pandiyan wrote:
> > commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> > unconditionally during long pulse"")' applies a work around for monitors
> > that don't signal link loss. Apply this only for external displays as
> > eDP features like PSR when active will have the link turned off and the
> > driver ends up retraining the link seeeing that link is not synchronized.
> >
> > Cc: Lyude Paul
> > Cc: Jan-Marek Glogowski
> > Cc: Ville Syrjälä
> > Cc: Rodrigo Vivi
> > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> > unconditionally during long pulse"")
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index befdaa076d8c..c59b2f78f41a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct intel_connector
> > *connector,
> > */
> > status = connector_status_disconnected;
> > goto out;
> > - } else {
> > + }
> > +
> > + if (!intel_dp_is_edp(intel_dp)) {
Also what happens when the PSR is not active? We would still need to retrain
even on eDP.
eDP tends to get fixed a lot of times during link retraining.
Manasi
>
> Hmm. How do things work if we get a short hpd from the sink while it's
> in PSR link off mode? Won't we try to retrain in that case as well?
>
> > /*
> > * Check link status in case the link went out of
> > * synchronization and the sink did not tell us.
> > --
> > 2.17.1
>
> --
> Ville Syrjälä
> Intel
> ___
> 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
Re: [Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
On Thu, Sep 06, 2018 at 11:21:34PM -0700, Dhinakaran Pandiyan wrote:
> commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> unconditionally during long pulse"")' applies a work around for monitors
> that don't signal link loss. Apply this only for external displays as
> eDP features like PSR when active will have the link turned off and the
> driver ends up retraining the link seeeing that link is not synchronized.
>
> Cc: Lyude Paul
> Cc: Jan-Marek Glogowski
> Cc: Ville Syrjälä
> Cc: Rodrigo Vivi
> References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> unconditionally during long pulse"")
> Signed-off-by: Dhinakaran Pandiyan
> ---
> drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index befdaa076d8c..c59b2f78f41a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct intel_connector *connector,
>*/
> status = connector_status_disconnected;
> goto out;
> - } else {
> + }
> +
> + if (!intel_dp_is_edp(intel_dp)) {
Hmm. How do things work if we get a short hpd from the sink while it's
in PSR link off mode? Won't we try to retrain in that case as well?
> /*
>* Check link status in case the link went out of
>* synchronization and the sink did not tell us.
> --
> 2.17.1
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/dp: Restrict link retrain workaround to external monitors
commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
unconditionally during long pulse"")' applies a work around for monitors
that don't signal link loss. Apply this only for external displays as
eDP features like PSR when active will have the link turned off and the
driver ends up retraining the link seeeing that link is not synchronized.
Cc: Lyude Paul
Cc: Jan-Marek Glogowski
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
unconditionally during long pulse"")
Signed-off-by: Dhinakaran Pandiyan
---
drivers/gpu/drm/i915/intel_dp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index befdaa076d8c..c59b2f78f41a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5072,7 +5072,9 @@ intel_dp_full_detect(struct intel_connector *connector,
*/
status = connector_status_disconnected;
goto out;
- } else {
+ }
+
+ if (!intel_dp_is_edp(intel_dp)) {
/*
* Check link status in case the link went out of
* synchronization and the sink did not tell us.
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
