Re: [Intel-gfx] [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()

2016-04-12 Thread Lyude
I'm going to take back the NAK on this, apparently this hotplugging
issue has been around longer then this patchset.

Reviewed-by: Lyude 


On Tue, 2016-04-12 at 10:11 +0300, Ander Conselvan De Oliveira wrote:
> On Mon, 2016-04-11 at 10:11 -0700, Jim Bride wrote:
> > 
> > In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> > some
> > much needed clean-up was done, but unfortunately part of the change
> > broke DP MST.  The real issue was setting the connector state to
> > disconnected in the MST case, which is good, but the code then
> > (after
> > a goto) checks if the connector state is not connected and shuts
> > down
> > MST if this is the case, which is bad.  With this change both SST
> > and
> > MST seem to be happy.
> > 
> > v2: Add removed check further up in the function to be sure that
> > MST
> > is shut down when we lose the link. (Ander)
> > 
> > Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> > cc: Sivakumar Thulasimani 
> > cc: Shubhangi Shrivastava 
> > cc: Ander Conselvan de Oliveira 
> > cc: Nathan D Ciobanu 
> > Signed-off-by: Jim Bride 
> Reviewed-by: Ander Conselvan de Oliveira 
> 
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 24 +++-
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index da0c3d2..31b222a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> >     intel_dp->compliance_test_type = 0;
> >     intel_dp->compliance_test_data = 0;
> >  
> > +   if (intel_dp->is_mst) {
> > +   DRM_DEBUG_KMS("MST device may have
> > disappeared %d vs
> > %d\n",
> > +     intel_dp->is_mst,
> > +     intel_dp-
> > >mst_mgr.mst_state);
> > +   intel_dp->is_mst = false;
> > +   drm_dp_mst_topology_mgr_set_mst(&intel_dp-
> > >mst_mgr,
> > +   intel_dp-
> > >is_mst);
> > +   }
> > +
> >     goto out;
> >     }
> >  
> > @@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> >     }
> >  
> >  out:
> > -   if (status != connector_status_connected) {
> > +   if ((status != connector_status_connected) &&
> > +   (intel_dp->is_mst == false))
> >     intel_dp_unset_edid(intel_dp);
> > -   /*
> > -    * If we were in MST mode, and device is not
> > there,
> > -    * get out of MST mode
> > -    */
> > -   if (intel_dp->is_mst) {
> > -   DRM_DEBUG_KMS("MST device may have
> > disappeared %d vs
> > %d\n",
> > -     intel_dp->is_mst, intel_dp
> > ->mst_mgr.mst_state);
> > -   intel_dp->is_mst = false;
> > -   drm_dp_mst_topology_mgr_set_mst(&intel_dp-
> > >mst_mgr,
> > -   intel_dp-
> > >is_mst);
> > -   }
> > -   }
> >  
> >     intel_display_power_put(to_i915(dev), power_domain);
> >     return;
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Cheers,
Lyude

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


Re: [Intel-gfx] [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()

2016-04-12 Thread Ander Conselvan De Oliveira
On Mon, 2016-04-11 at 10:11 -0700, Jim Bride wrote:
> In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") some
> much needed clean-up was done, but unfortunately part of the change
> broke DP MST.  The real issue was setting the connector state to
> disconnected in the MST case, which is good, but the code then (after
> a goto) checks if the connector state is not connected and shuts down
> MST if this is the case, which is bad.  With this change both SST and
> MST seem to be happy.
> 
> v2: Add removed check further up in the function to be sure that MST
> is shut down when we lose the link. (Ander)
> 
> Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> cc: Sivakumar Thulasimani 
> cc: Shubhangi Shrivastava 
> cc: Ander Conselvan de Oliveira 
> cc: Nathan D Ciobanu 
> Signed-off-by: Jim Bride 

Reviewed-by: Ander Conselvan de Oliveira 

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 24 +++-
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..31b222a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   intel_dp->compliance_test_type = 0;
>   intel_dp->compliance_test_data = 0;
>  
> + if (intel_dp->is_mst) {
> + DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> +   intel_dp->is_mst,
> +   intel_dp->mst_mgr.mst_state);
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> + intel_dp->is_mst);
> + }
> +
>   goto out;
>   }
>  
> @@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   }
>  
>  out:
> - if (status != connector_status_connected) {
> + if ((status != connector_status_connected) &&
> + (intel_dp->is_mst == false))
>   intel_dp_unset_edid(intel_dp);
> - /*
> -  * If we were in MST mode, and device is not there,
> -  * get out of MST mode
> -  */
> - if (intel_dp->is_mst) {
> - DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> -   intel_dp->is_mst, intel_dp
> ->mst_mgr.mst_state);
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> - intel_dp->is_mst);
> - }
> - }
>  
>   intel_display_power_put(to_i915(dev), power_domain);
>   return;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()

2016-04-11 Thread Lyude Paul
NAK. Try plugging in an MST display, suspending the machine, then resuming it.
Hotplugging still breaks (which I've traced down to this patch)

I wouldn't worry about fixing this up. I'm probably going to be sending a revert
for this anyway soon along with probably some of the other patches.

On Mon, 2016-04-11 at 10:11 -0700, Jim Bride wrote:
> In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") some
> much needed clean-up was done, but unfortunately part of the change
> broke DP MST.  The real issue was setting the connector state to
> disconnected in the MST case, which is good, but the code then (after
> a goto) checks if the connector state is not connected and shuts down
> MST if this is the case, which is bad.  With this change both SST and
> MST seem to be happy.
> 
> v2: Add removed check further up in the function to be sure that MST
> is shut down when we lose the link. (Ander)
> 
> Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
> cc: Sivakumar Thulasimani 
> cc: Shubhangi Shrivastava 
> cc: Ander Conselvan de Oliveira 
> cc: Nathan D Ciobanu 
> Signed-off-by: Jim Bride 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 24 +++-
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d2..31b222a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   intel_dp->compliance_test_type = 0;
>   intel_dp->compliance_test_data = 0;
>  
> + if (intel_dp->is_mst) {
> + DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> +   intel_dp->is_mst,
> +   intel_dp->mst_mgr.mst_state);
> + intel_dp->is_mst = false;
> + drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> + intel_dp->is_mst);
> + }
> +
>   goto out;
>   }
>  
> @@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>   }
>  
>  out:
> - if (status != connector_status_connected) {
> + if ((status != connector_status_connected) &&
> + (intel_dp->is_mst == false))
>   intel_dp_unset_edid(intel_dp);
> - /*
> -  * If we were in MST mode, and device is not there,
> -  * get out of MST mode
> -  */
> - if (intel_dp->is_mst) {
> - DRM_DEBUG_KMS("MST device may have disappeared %d vs
> %d\n",
> -   intel_dp->is_mst, intel_dp-
> >mst_mgr.mst_state);
> - intel_dp->is_mst = false;
> - drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> - intel_dp->is_mst);
> - }
> - }
>  
>   intel_display_power_put(to_i915(dev), power_domain);
>   return;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2] drm/i915/dp/mst: Fix MST logic in intel_dp_long_pulse()

2016-04-11 Thread Jim Bride
In commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") some
much needed clean-up was done, but unfortunately part of the change
broke DP MST.  The real issue was setting the connector state to
disconnected in the MST case, which is good, but the code then (after
a goto) checks if the connector state is not connected and shuts down
MST if this is the case, which is bad.  With this change both SST and
MST seem to be happy.

v2: Add removed check further up in the function to be sure that MST
is shut down when we lose the link. (Ander)

Fixes: commit 7d23e3c3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
cc: Sivakumar Thulasimani 
cc: Shubhangi Shrivastava 
cc: Ander Conselvan de Oliveira 
cc: Nathan D Ciobanu 
Signed-off-by: Jim Bride 
---
 drivers/gpu/drm/i915/intel_dp.c | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index da0c3d2..31b222a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4608,6 +4608,15 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
intel_dp->compliance_test_type = 0;
intel_dp->compliance_test_data = 0;
 
+   if (intel_dp->is_mst) {
+   DRM_DEBUG_KMS("MST device may have disappeared %d vs 
%d\n",
+ intel_dp->is_mst,
+ intel_dp->mst_mgr.mst_state);
+   intel_dp->is_mst = false;
+   drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+   intel_dp->is_mst);
+   }
+
goto out;
}
 
@@ -4665,20 +4674,9 @@ intel_dp_long_pulse(struct intel_connector 
*intel_connector)
}
 
 out:
-   if (status != connector_status_connected) {
+   if ((status != connector_status_connected) &&
+   (intel_dp->is_mst == false))
intel_dp_unset_edid(intel_dp);
-   /*
-* If we were in MST mode, and device is not there,
-* get out of MST mode
-*/
-   if (intel_dp->is_mst) {
-   DRM_DEBUG_KMS("MST device may have disappeared %d vs 
%d\n",
- intel_dp->is_mst, 
intel_dp->mst_mgr.mst_state);
-   intel_dp->is_mst = false;
-   drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
-   intel_dp->is_mst);
-   }
-   }
 
intel_display_power_put(to_i915(dev), power_domain);
return;
-- 
2.5.0

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