* Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
2015-08-20 16:37 [PATCH] drm/i915: Check DP link status on long hpd too ville.syrjala
@ 2015-08-21 6:40 ` Jani Nikula
2015-08-26 9:04 ` Daniel Vetter
2015-08-31 15:50 ` Jani Nikula
2015-09-01 18:06 ` Sivakumar Thulasimani
2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2015-08-21 6:40 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Dave Airlie, stable
On Thu, 20 Aug 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We are no longer checkling the DP link status on long hpd. We used to do
> that from the .hot_plug() handler, but it was removed when MST got
> introduced.
>
> If there's no userspace we now fail to retrain the link if the sink
> power is toggled (or cable yanked and replugged), meaning the user is
> left staring at a blank screen. With the retraining put back that should
> be fixed.
>
> Also remove the leftover comment that referred to the old retraining
> from .hot_plug().
>
> Fixes a regression introduced in:
> commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> Author: Dave Airlie <airlied@redhat.com>
> Date: Fri May 2 14:02:48 2014 +1000
>
> drm/i915: add DP 1.2 MST support (v0.7)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
Tested-by: Palmer Dabbelt <palmer@dabbelt.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..b014158 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>
> intel_dp_probe_oui(intel_dp);
>
> - if (!intel_dp_probe_mst(intel_dp))
> + if (!intel_dp_probe_mst(intel_dp)) {
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> + intel_dp_check_link_status(intel_dp);
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> goto mst_fail;
> -
> + }
> } else {
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> }
>
> if (!intel_dp->is_mst) {
> - /*
> - * we'll check the link status via the normal hot plug path later -
> - * but for short hpds we should check it now
> - */
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> intel_dp_check_link_status(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
2015-08-21 6:40 ` [Intel-gfx] " Jani Nikula
@ 2015-08-26 9:04 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-08-26 9:04 UTC (permalink / raw)
To: Jani Nikula; +Cc: ville.syrjala, intel-gfx, Dave Airlie, stable
On Fri, Aug 21, 2015 at 09:40:12AM +0300, Jani Nikula wrote:
> On Thu, 20 Aug 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > We are no longer checkling the DP link status on long hpd. We used to do
> > that from the .hot_plug() handler, but it was removed when MST got
> > introduced.
> >
> > If there's no userspace we now fail to retrain the link if the sink
> > power is toggled (or cable yanked and replugged), meaning the user is
> > left staring at a blank screen. With the retraining put back that should
> > be fixed.
> >
> > Also remove the leftover comment that referred to the old retraining
> > from .hot_plug().
> >
> > Fixes a regression introduced in:
> > commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> > Author: Dave Airlie <airlied@redhat.com>
> > Date: Fri May 2 14:02:48 2014 +1000
> >
> > drm/i915: add DP 1.2 MST support (v0.7)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
>
> Tested-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index d32ce48..b014158 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >
> > intel_dp_probe_oui(intel_dp);
> >
> > - if (!intel_dp_probe_mst(intel_dp))
> > + if (!intel_dp_probe_mst(intel_dp)) {
> > + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > + intel_dp_check_link_status(intel_dp);
> > + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > goto mst_fail;
> > -
> > + }
> > } else {
> > if (intel_dp->is_mst) {
> > if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> > @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > }
> >
> > if (!intel_dp->is_mst) {
> > - /*
> > - * we'll check the link status via the normal hot plug path later -
> > - * but for short hpds we should check it now
> > - */
> > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);
> > drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
2015-08-20 16:37 [PATCH] drm/i915: Check DP link status on long hpd too ville.syrjala
2015-08-21 6:40 ` [Intel-gfx] " Jani Nikula
@ 2015-08-31 15:50 ` Jani Nikula
2015-09-01 18:06 ` Sivakumar Thulasimani
2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-08-31 15:50 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Dave Airlie, stable
On Thu, 20 Aug 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We are no longer checkling the DP link status on long hpd. We used to do
> that from the .hot_plug() handler, but it was removed when MST got
> introduced.
>
> If there's no userspace we now fail to retrain the link if the sink
> power is toggled (or cable yanked and replugged), meaning the user is
> left staring at a blank screen. With the retraining put back that should
> be fixed.
>
> Also remove the leftover comment that referred to the old retraining
> from .hot_plug().
>
> Fixes a regression introduced in:
> commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> Author: Dave Airlie <airlied@redhat.com>
> Date: Fri May 2 14:02:48 2014 +1000
>
> drm/i915: add DP 1.2 MST support (v0.7)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-next-fixes.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..b014158 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>
> intel_dp_probe_oui(intel_dp);
>
> - if (!intel_dp_probe_mst(intel_dp))
> + if (!intel_dp_probe_mst(intel_dp)) {
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> + intel_dp_check_link_status(intel_dp);
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> goto mst_fail;
> -
> + }
> } else {
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> }
>
> if (!intel_dp->is_mst) {
> - /*
> - * we'll check the link status via the normal hot plug path later -
> - * but for short hpds we should check it now
> - */
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> intel_dp_check_link_status(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
2015-08-20 16:37 [PATCH] drm/i915: Check DP link status on long hpd too ville.syrjala
2015-08-21 6:40 ` [Intel-gfx] " Jani Nikula
2015-08-31 15:50 ` Jani Nikula
@ 2015-09-01 18:06 ` Sivakumar Thulasimani
2015-09-01 18:30 ` Ville Syrjälä
2 siblings, 1 reply; 6+ messages in thread
From: Sivakumar Thulasimani @ 2015-09-01 18:06 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Dave Airlie, stable
On 8/20/2015 10:07 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We are no longer checkling the DP link status on long hpd. We used to do
> that from the .hot_plug() handler, but it was removed when MST got
> introduced.
>
> If there's no userspace we now fail to retrain the link if the sink
> power is toggled (or cable yanked and replugged), meaning the user is
> left staring at a blank screen. With the retraining put back that should
> be fixed.
>
> Also remove the leftover comment that referred to the old retraining
> from .hot_plug().
>
> Fixes a regression introduced in:
> commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> Author: Dave Airlie <airlied@redhat.com>
> Date: Fri May 2 14:02:48 2014 +1000
>
> drm/i915: add DP 1.2 MST support (v0.7)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..b014158 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>
> intel_dp_probe_oui(intel_dp);
>
> - if (!intel_dp_probe_mst(intel_dp))
> + if (!intel_dp_probe_mst(intel_dp)) {
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> + intel_dp_check_link_status(intel_dp);
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
couple of queries for my understanding.
> why should we check for link status for long pulse, which is supposed
to be for plug in or plug out ?
> goto mst_fail will indicate we are falling back to intel_dp_detect,
isnt this better suited there ?
Also the following bug indicates failure in mst panel, but the changes
here are in non-mst path
https://bugs.freedesktop.org/show_bug.cgi?id=89453, how is this
patch helping this bug ?
> goto mst_fail;
> -
> + }
> } else {
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> }
>
> if (!intel_dp->is_mst) {
> - /*
> - * we'll check the link status via the normal hot plug path later -
> - * but for short hpds we should check it now
> - */
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> intel_dp_check_link_status(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
--
regards,
Sivakumar
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too
2015-09-01 18:06 ` Sivakumar Thulasimani
@ 2015-09-01 18:30 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2015-09-01 18:30 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx, Dave Airlie, stable
On Tue, Sep 01, 2015 at 11:36:25PM +0530, Sivakumar Thulasimani wrote:
>
>
> On 8/20/2015 10:07 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> >
> > We are no longer checkling the DP link status on long hpd. We used to do
> > that from the .hot_plug() handler, but it was removed when MST got
> > introduced.
> >
> > If there's no userspace we now fail to retrain the link if the sink
> > power is toggled (or cable yanked and replugged), meaning the user is
> > left staring at a blank screen. With the retraining put back that should
> > be fixed.
> >
> > Also remove the leftover comment that referred to the old retraining
> > from .hot_plug().
> >
> > Fixes a regression introduced in:
> > commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> > Author: Dave Airlie <airlied@redhat.com>
> > Date: Fri May 2 14:02:48 2014 +1000
> >
> > drm/i915: add DP 1.2 MST support (v0.7)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index d32ce48..b014158 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >
> > intel_dp_probe_oui(intel_dp);
> >
> > - if (!intel_dp_probe_mst(intel_dp))
> > + if (!intel_dp_probe_mst(intel_dp)) {
> > + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > + intel_dp_check_link_status(intel_dp);
> > + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> couple of queries for my understanding.
> > why should we check for link status for long pulse, which is supposed
> to be for plug in or plug out ?
> > goto mst_fail will indicate we are falling back to intel_dp_detect,
> isnt this better suited there ?
->detect() should do what it says and no more.
In any case I think it's better to have the code to maintain the current
link in one place. That should also make the locking rules easier to
understand since we hpd_pulse and detect are executed from different
works.
>
> Also the following bug indicates failure in mst panel, but the changes
> here are in non-mst path
> https://bugs.freedesktop.org/show_bug.cgi?id=89453, how is this
> patch helping this bug ?
Presumably the monitor is being used in non-MST mode.
> > goto mst_fail;
> > -
> > + }
> > } else {
> > if (intel_dp->is_mst) {
> > if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> > @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > }
> >
> > if (!intel_dp->is_mst) {
> > - /*
> > - * we'll check the link status via the normal hot plug path later -
> > - * but for short hpds we should check it now
> > - */
> > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);
> > drm_modeset_unlock(&dev->mode_config.connection_mutex);
>
> --
> regards,
> Sivakumar
--
Ville Syrj�l�
Intel OTC
^ permalink raw reply [flat|nested] 6+ messages in thread