Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Ville Syrjälä]

On Wed, Feb 22, 2017 at 08:00:38PM -0800, Palmer Dabbelt wrote:
> On Thu, Feb 16, 2017 at 9:46 AM, Ville Syrjälä
>  wrote:
> > On Thu, Feb 16, 2017 at 09:24:09AM -0800, Manasi Navare wrote:
> >> On Thu, Feb 16, 2017 at 07:18:57PM +0200, Ville Syrjälä wrote:
> >> > On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote:
> >> > > On Thu, Feb 16, 2017 at 05:30:07PM +0200, 
> >> > > ville.syrj...@linux.intel.com wrote:
> >> > > > From: Ville Syrjälä 
> >> > > >
> >> > > > Apparently some DP sinks are a little nuts and cause HPD to drop
> >> > > > intermittently during modesets. This happens eg. on an ASUS PB287Q.
> >> > > > In oder to recover from this we can't really use the previous
> >> > > > connector status to determine if the link needs retraining, so let's
> >> > > > just ignore that piece of information and do the retrain
> >> > > > unconditionally. We do of course still check whether the link is
> >> > > > supposed to be running or not.
> >> > > >
> >> > > > Cc: sta...@vger.kernel.org
> >> > > > Cc: Palmer Dabbelt 
> >> > > > Reported-by: Palmer Dabbelt 
> >> > > > References: 
> >> > > > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> >> > > > Signed-off-by: Ville Syrjälä 
> >> > > > ---
> >> > > >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
> >> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> >> > > >
> >> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >> > > > b/drivers/gpu/drm/i915/intel_dp.c
> >> > > > index 024798a9c016..37a746f7fbc3 100644
> >> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> >> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> > > > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> >> > > > *intel_connector)
> >> > > >  */
> >> > > > status = connector_status_disconnected;
> >> > > > goto out;
> >> > > > -   } else if (connector->status == connector_status_connected) {
> >> > > > +   } else {
> >> > > > /*
> >> > > > -* If display was connected already and is still 
> >> > > > connected
> >> > > > -* check links status, there has been known issues of
> >> > > > -* link loss triggerring long pulse
> >> > > > +* If display is now connected check links status,
> >> > > > +* there has been known issues of link loss 
> >> > > > triggerring
> >> > > > +* long pulse.
> >> > > > +*
> >> > > > +* Some sinks (eg. ASUS PB287Q) seem to perform some
> >> > > > +* weird HPD ping pong during modesets. So we can 
> >> > > > apparely
> >> > > > +* end up with HPD going low during a modeset, and 
> >> > > > then
> >> > > > +* 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.
> >> > > >  */
> >> > > > drm_modeset_lock(>mode_config.connection_mutex, 
> >> > > > NULL);
> >> > > > intel_dp_check_link_status(intel_dp);
> >> > > > --
> >> > >
> >> > > So here we basically just ignore the connector status and retrain 
> >> > > irrespectively.
> >> >
> >> > We ignore the _previous_ connector status.
> >> >
> >> > > But that means even if we have newer values now for max link rate/lane 
> >> > > count from
> >> > > DPCD, during this retrain we are just using the stale value of 
> >> > > intel_dp->link_rate
> >> > > and intel_dp->lane_count. I think intel_dp->link_rate and lane count 
> >> > > values
> >> > > should be set to 0 on HPD pulse, they would be set only during a 
> >> > > modeset.
> >> >
> >> > The DPCD has already been parsed by this time.
> >> >
> >> > --
> >> > Ville Syrjälä
> >> > Intel OTC
> >>
> >> Yes, we have parsed the DPCD but we dont write to intel_dp->link-rate and 
> >> intel_dp->lane_count
> >> until we do a modeset (these get written during pre_enable) and these 
> >> values get used
> >> during the retraining of the link. So at this point we will still use 
> >> stale values.
> >
> > They're not stale. They're exactly what we used when we set up the
> > mode that's still being drive out. This is just retraining the link
> > at the same parameters.
> >
> >> Shouldnt we set the link status to BAD and send a hotplug event, like the 
> >> solution we implemneted
> >> for retraining?
> >>
> >> Regards
> >> Manasi
> >>
> >> Regards
> >> Manasi
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> Sorry, I forgot about this.  A log is attached of my machine booting
> with the monitor attached, me detaching the monitor, and then plugging
> it back in.

I was actually able to reproduce 

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Palmer Dabbelt]

On Thu, Feb 16, 2017 at 9:46 AM, Ville Syrjälä
 wrote:
> On Thu, Feb 16, 2017 at 09:24:09AM -0800, Manasi Navare wrote:
>> On Thu, Feb 16, 2017 at 07:18:57PM +0200, Ville Syrjälä wrote:
>> > On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote:
>> > > On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrj...@linux.intel.com 
>> > > wrote:
>> > > > From: Ville Syrjälä 
>> > > >
>> > > > Apparently some DP sinks are a little nuts and cause HPD to drop
>> > > > intermittently during modesets. This happens eg. on an ASUS PB287Q.
>> > > > In oder to recover from this we can't really use the previous
>> > > > connector status to determine if the link needs retraining, so let's
>> > > > just ignore that piece of information and do the retrain
>> > > > unconditionally. We do of course still check whether the link is
>> > > > supposed to be running or not.
>> > > >
>> > > > Cc: sta...@vger.kernel.org
>> > > > Cc: Palmer Dabbelt 
>> > > > Reported-by: Palmer Dabbelt 
>> > > > References: 
>> > > > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
>> > > > Signed-off-by: Ville Syrjälä 
>> > > > ---
>> > > >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
>> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> > > > b/drivers/gpu/drm/i915/intel_dp.c
>> > > > index 024798a9c016..37a746f7fbc3 100644
>> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
>> > > > *intel_connector)
>> > > >  */
>> > > > status = connector_status_disconnected;
>> > > > goto out;
>> > > > -   } else if (connector->status == connector_status_connected) {
>> > > > +   } else {
>> > > > /*
>> > > > -* If display was connected already and is still 
>> > > > connected
>> > > > -* check links status, there has been known issues of
>> > > > -* link loss triggerring long pulse
>> > > > +* If display is now connected check links status,
>> > > > +* there has been known issues of link loss triggerring
>> > > > +* long pulse.
>> > > > +*
>> > > > +* Some sinks (eg. ASUS PB287Q) seem to perform some
>> > > > +* weird HPD ping pong during modesets. So we can 
>> > > > apparely
>> > > > +* end up with HPD going low during a modeset, and then
>> > > > +* 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.
>> > > >  */
>> > > > drm_modeset_lock(>mode_config.connection_mutex, 
>> > > > NULL);
>> > > > intel_dp_check_link_status(intel_dp);
>> > > > --
>> > >
>> > > So here we basically just ignore the connector status and retrain 
>> > > irrespectively.
>> >
>> > We ignore the _previous_ connector status.
>> >
>> > > But that means even if we have newer values now for max link rate/lane 
>> > > count from
>> > > DPCD, during this retrain we are just using the stale value of 
>> > > intel_dp->link_rate
>> > > and intel_dp->lane_count. I think intel_dp->link_rate and lane count 
>> > > values
>> > > should be set to 0 on HPD pulse, they would be set only during a modeset.
>> >
>> > The DPCD has already been parsed by this time.
>> >
>> > --
>> > Ville Syrjälä
>> > Intel OTC
>>
>> Yes, we have parsed the DPCD but we dont write to intel_dp->link-rate and 
>> intel_dp->lane_count
>> until we do a modeset (these get written during pre_enable) and these values 
>> get used
>> during the retraining of the link. So at this point we will still use stale 
>> values.
>
> They're not stale. They're exactly what we used when we set up the
> mode that's still being drive out. This is just retraining the link
> at the same parameters.
>
>> Shouldnt we set the link status to BAD and send a hotplug event, like the 
>> solution we implemneted
>> for retraining?
>>
>> Regards
>> Manasi
>>
>> Regards
>> Manasi
>
> --
> Ville Syrjälä
> Intel OTC

Sorry, I forgot about this.  A log is attached of my machine booting
with the monitor attached, me detaching the monitor, and then plugging
it back in.


log.gz
Description: GNU Zip compressed data
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Ville Syrjälä]

On Thu, Feb 16, 2017 at 09:24:09AM -0800, Manasi Navare wrote:
> On Thu, Feb 16, 2017 at 07:18:57PM +0200, Ville Syrjälä wrote:
> > On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote:
> > > On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrj...@linux.intel.com 
> > > wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > Apparently some DP sinks are a little nuts and cause HPD to drop
> > > > intermittently during modesets. This happens eg. on an ASUS PB287Q.
> > > > In oder to recover from this we can't really use the previous
> > > > connector status to determine if the link needs retraining, so let's
> > > > just ignore that piece of information and do the retrain
> > > > unconditionally. We do of course still check whether the link is
> > > > supposed to be running or not.
> > > > 
> > > > Cc: sta...@vger.kernel.org
> > > > Cc: Palmer Dabbelt 
> > > > Reported-by: Palmer Dabbelt 
> > > > References: 
> > > > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> > > > Signed-off-by: Ville Syrjälä 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 024798a9c016..37a746f7fbc3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> > > > *intel_connector)
> > > >  */
> > > > status = connector_status_disconnected;
> > > > goto out;
> > > > -   } else if (connector->status == connector_status_connected) {
> > > > +   } else {
> > > > /*
> > > > -* If display was connected already and is still 
> > > > connected
> > > > -* check links status, there has been known issues of
> > > > -* link loss triggerring long pulse
> > > > +* If display is now connected check links status,
> > > > +* there has been known issues of link loss triggerring
> > > > +* long pulse.
> > > > +*
> > > > +* Some sinks (eg. ASUS PB287Q) seem to perform some
> > > > +* weird HPD ping pong during modesets. So we can 
> > > > apparely
> > > > +* end up with HPD going low during a modeset, and then
> > > > +* 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.
> > > >  */
> > > > drm_modeset_lock(>mode_config.connection_mutex, 
> > > > NULL);
> > > > intel_dp_check_link_status(intel_dp);
> > > > --
> > > 
> > > So here we basically just ignore the connector status and retrain 
> > > irrespectively.
> > 
> > We ignore the _previous_ connector status.
> > 
> > > But that means even if we have newer values now for max link rate/lane 
> > > count from
> > > DPCD, during this retrain we are just using the stale value of 
> > > intel_dp->link_rate
> > > and intel_dp->lane_count. I think intel_dp->link_rate and lane count 
> > > values
> > > should be set to 0 on HPD pulse, they would be set only during a modeset.
> > 
> > The DPCD has already been parsed by this time.
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> Yes, we have parsed the DPCD but we dont write to intel_dp->link-rate and 
> intel_dp->lane_count
> until we do a modeset (these get written during pre_enable) and these values 
> get used
> during the retraining of the link. So at this point we will still use stale 
> values.

They're not stale. They're exactly what we used when we set up the
mode that's still being drive out. This is just retraining the link
at the same parameters.

> Shouldnt we set the link status to BAD and send a hotplug event, like the 
> solution we implemneted
> for retraining?
> 
> Regards
> Manasi
> 
> Regards
> Manasi

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Manasi Navare]

On Thu, Feb 16, 2017 at 07:18:57PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote:
> > On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrj...@linux.intel.com 
> > wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Apparently some DP sinks are a little nuts and cause HPD to drop
> > > intermittently during modesets. This happens eg. on an ASUS PB287Q.
> > > In oder to recover from this we can't really use the previous
> > > connector status to determine if the link needs retraining, so let's
> > > just ignore that piece of information and do the retrain
> > > unconditionally. We do of course still check whether the link is
> > > supposed to be running or not.
> > > 
> > > Cc: sta...@vger.kernel.org
> > > Cc: Palmer Dabbelt 
> > > Reported-by: Palmer Dabbelt 
> > > References: 
> > > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 024798a9c016..37a746f7fbc3 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> > > *intel_connector)
> > >*/
> > >   status = connector_status_disconnected;
> > >   goto out;
> > > - } else if (connector->status == connector_status_connected) {
> > > + } else {
> > >   /*
> > > -  * If display was connected already and is still connected
> > > -  * check links status, there has been known issues of
> > > -  * link loss triggerring long pulse
> > > +  * If display is now connected check links status,
> > > +  * there has been known issues of link loss triggerring
> > > +  * long pulse.
> > > +  *
> > > +  * Some sinks (eg. ASUS PB287Q) seem to perform some
> > > +  * weird HPD ping pong during modesets. So we can apparely
> > > +  * end up with HPD going low during a modeset, and then
> > > +  * 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.
> > >*/
> > >   drm_modeset_lock(>mode_config.connection_mutex, NULL);
> > >   intel_dp_check_link_status(intel_dp);
> > > --
> > 
> > So here we basically just ignore the connector status and retrain 
> > irrespectively.
> 
> We ignore the _previous_ connector status.
> 
> > But that means even if we have newer values now for max link rate/lane 
> > count from
> > DPCD, during this retrain we are just using the stale value of 
> > intel_dp->link_rate
> > and intel_dp->lane_count. I think intel_dp->link_rate and lane count values
> > should be set to 0 on HPD pulse, they would be set only during a modeset.
> 
> The DPCD has already been parsed by this time.
> 
> -- 
> Ville Syrjälä
> Intel OTC

Yes, we have parsed the DPCD but we dont write to intel_dp->link-rate and 
intel_dp->lane_count
until we do a modeset (these get written during pre_enable) and these values 
get used
during the retraining of the link. So at this point we will still use stale 
values.
Shouldnt we set the link status to BAD and send a hotplug event, like the 
solution we implemneted
for retraining?

Regards
Manasi

Regards
Manasi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Ville Syrjälä]

On Thu, Feb 16, 2017 at 09:07:53AM -0800, Manasi Navare wrote:
> On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Apparently some DP sinks are a little nuts and cause HPD to drop
> > intermittently during modesets. This happens eg. on an ASUS PB287Q.
> > In oder to recover from this we can't really use the previous
> > connector status to determine if the link needs retraining, so let's
> > just ignore that piece of information and do the retrain
> > unconditionally. We do of course still check whether the link is
> > supposed to be running or not.
> > 
> > Cc: sta...@vger.kernel.org
> > Cc: Palmer Dabbelt 
> > Reported-by: Palmer Dabbelt 
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 024798a9c016..37a746f7fbc3 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> > *intel_connector)
> >  */
> > status = connector_status_disconnected;
> > goto out;
> > -   } else if (connector->status == connector_status_connected) {
> > +   } else {
> > /*
> > -* If display was connected already and is still connected
> > -* check links status, there has been known issues of
> > -* link loss triggerring long pulse
> > +* If display is now connected check links status,
> > +* there has been known issues of link loss triggerring
> > +* long pulse.
> > +*
> > +* Some sinks (eg. ASUS PB287Q) seem to perform some
> > +* weird HPD ping pong during modesets. So we can apparely
> > +* end up with HPD going low during a modeset, and then
> > +* 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.
> >  */
> > drm_modeset_lock(>mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);
> > --
> 
> So here we basically just ignore the connector status and retrain 
> irrespectively.

We ignore the _previous_ connector status.

> But that means even if we have newer values now for max link rate/lane count 
> from
> DPCD, during this retrain we are just using the stale value of 
> intel_dp->link_rate
> and intel_dp->lane_count. I think intel_dp->link_rate and lane count values
> should be set to 0 on HPD pulse, they would be set only during a modeset.

The DPCD has already been parsed by this time.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Manasi Navare]

On Thu, Feb 16, 2017 at 05:30:07PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Apparently some DP sinks are a little nuts and cause HPD to drop
> intermittently during modesets. This happens eg. on an ASUS PB287Q.
> In oder to recover from this we can't really use the previous
> connector status to determine if the link needs retraining, so let's
> just ignore that piece of information and do the retrain
> unconditionally. We do of course still check whether the link is
> supposed to be running or not.
> 
> Cc: sta...@vger.kernel.org
> Cc: Palmer Dabbelt 
> Reported-by: Palmer Dabbelt 
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 024798a9c016..37a746f7fbc3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> *intel_connector)
>*/
>   status = connector_status_disconnected;
>   goto out;
> - } else if (connector->status == connector_status_connected) {
> + } else {
>   /*
> -  * If display was connected already and is still connected
> -  * check links status, there has been known issues of
> -  * link loss triggerring long pulse
> +  * If display is now connected check links status,
> +  * there has been known issues of link loss triggerring
> +  * long pulse.
> +  *
> +  * Some sinks (eg. ASUS PB287Q) seem to perform some
> +  * weird HPD ping pong during modesets. So we can apparely
> +  * end up with HPD going low during a modeset, and then
> +  * 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.
>*/
>   drm_modeset_lock(>mode_config.connection_mutex, NULL);
>   intel_dp_check_link_status(intel_dp);
> --

So here we basically just ignore the connector status and retrain 
irrespectively.
But that means even if we have newer values now for max link rate/lane count 
from
DPCD, during this retrain we are just using the stale value of 
intel_dp->link_rate
and intel_dp->lane_count. I think intel_dp->link_rate and lane count values
should be set to 0 on HPD pulse, they would be set only during a modeset.

Regards
Manasi

 
> 2.10.2
> 
> ___
> 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] drm/i915: Perform link quality check unconditionally during long pulse

[Ville Syrjälä]

On Thu, Feb 16, 2017 at 07:39:29AM -0800, Palmer Dabbelt wrote:
> On Thu, 16 Feb 2017 07:30:07 PST (-0800), ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> >
> > Apparently some DP sinks are a little nuts and cause HPD to drop
> > intermittently during modesets. This happens eg. on an ASUS PB287Q.
> > In oder to recover from this we can't really use the previous
> > connector status to determine if the link needs retraining, so let's
> > just ignore that piece of information and do the retrain
> > unconditionally. We do of course still check whether the link is
> > supposed to be running or not.
> 
> With this patch on top of linux-4.9 I have DPMS on/off working again, but I
> don't have the correct resolutions on my monitor.  I just see
> 
> DP1 connected 1024x768+2880+0 (normal left inverted right x axis y axis) 0mm 
> x 0mm
>1024x768  60.00*
>800x600   60.3256.25
>848x480   60.00
>640x480   59.94

Hmm. That would indicate that the EDID reads are also failing now.
I wonder if your monitor is just a lemon.

Anyways, there were no failed EDID reads visible in the logs you
provided. You should keep the drm.debug=0xe and rerun xrandr (do not
use the --current knob), and then we should check the dmesg again
to see what the kernel thinks the mode list should look like.

> 
> >
> > Cc: sta...@vger.kernel.org
> > Cc: Palmer Dabbelt 
> > Reported-by: Palmer Dabbelt 
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 024798a9c016..37a746f7fbc3 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> > *intel_connector)
> >  */
> > status = connector_status_disconnected;
> > goto out;
> > -   } else if (connector->status == connector_status_connected) {
> > +   } else {
> > /*
> > -* If display was connected already and is still connected
> > -* check links status, there has been known issues of
> > -* link loss triggerring long pulse
> > +* If display is now connected check links status,
> > +* there has been known issues of link loss triggerring
> > +* long pulse.
> > +*
> > +* Some sinks (eg. ASUS PB287Q) seem to perform some
> > +* weird HPD ping pong during modesets. So we can apparely
> > +* end up with HPD going low during a modeset, and then
> > +* 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.
> >  */
> > drm_modeset_lock(>mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[Palmer Dabbelt]

On Thu, 16 Feb 2017 07:30:07 PST (-0800), ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
>
> Apparently some DP sinks are a little nuts and cause HPD to drop
> intermittently during modesets. This happens eg. on an ASUS PB287Q.
> In oder to recover from this we can't really use the previous
> connector status to determine if the link needs retraining, so let's
> just ignore that piece of information and do the retrain
> unconditionally. We do of course still check whether the link is
> supposed to be running or not.

With this patch on top of linux-4.9 I have DPMS on/off working again, but I
don't have the correct resolutions on my monitor.  I just see

DP1 connected 1024x768+2880+0 (normal left inverted right x axis y axis) 0mm x 
0mm
   1024x768  60.00*
   800x600   60.3256.25
   848x480   60.00
   640x480   59.94

>
> Cc: sta...@vger.kernel.org
> Cc: Palmer Dabbelt 
> Reported-by: Palmer Dabbelt 
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 024798a9c016..37a746f7fbc3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
> *intel_connector)
>*/
>   status = connector_status_disconnected;
>   goto out;
> - } else if (connector->status == connector_status_connected) {
> + } else {
>   /*
> -  * If display was connected already and is still connected
> -  * check links status, there has been known issues of
> -  * link loss triggerring long pulse
> +  * If display is now connected check links status,
> +  * there has been known issues of link loss triggerring
> +  * long pulse.
> +  *
> +  * Some sinks (eg. ASUS PB287Q) seem to perform some
> +  * weird HPD ping pong during modesets. So we can apparely
> +  * end up with HPD going low during a modeset, and then
> +  * 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.
>*/
>   drm_modeset_lock(>mode_config.connection_mutex, NULL);
>   intel_dp_check_link_status(intel_dp);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Perform link quality check unconditionally during long pulse

[ville . syrjala]

From: Ville Syrjälä 

Apparently some DP sinks are a little nuts and cause HPD to drop
intermittently during modesets. This happens eg. on an ASUS PB287Q.
In oder to recover from this we can't really use the previous
connector status to determine if the link needs retraining, so let's
just ignore that piece of information and do the retrain
unconditionally. We do of course still check whether the link is
supposed to be running or not.

Cc: sta...@vger.kernel.org
Cc: Palmer Dabbelt 
Reported-by: Palmer Dabbelt 
References: 
https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_dp.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 024798a9c016..37a746f7fbc3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4648,11 +4648,18 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
 */
status = connector_status_disconnected;
goto out;
-   } else if (connector->status == connector_status_connected) {
+   } else {
/*
-* If display was connected already and is still connected
-* check links status, there has been known issues of
-* link loss triggerring long pulse
+* If display is now connected check links status,
+* there has been known issues of link loss triggerring
+* long pulse.
+*
+* Some sinks (eg. ASUS PB287Q) seem to perform some
+* weird HPD ping pong during modesets. So we can apparely
+* end up with HPD going low during a modeset, and then
+* 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.
 */
drm_modeset_lock(>mode_config.connection_mutex, NULL);
intel_dp_check_link_status(intel_dp);
-- 
2.10.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx