diff mbox series

[v3,2/3] drm/i915: Don't send MST hotplugs during resume

Message ID 20190129191001.442-3-lyude@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: MST and wakeref leak fixes | expand

Commit Message

Lyude Paul Jan. 29, 2019, 7:10 p.m. UTC
We have a bad habit of calling drm_fb_helper_hotplug_event() far more
then we actually need to. MST appears to be one of these cases, where we
call drm_fb_helper_hotplug_event() if we fail to resume a connected MST
topology in intel_dp_mst_resume(). We don't actually need to do this at
all though since hotplug events are already sent from
drm_dp_connector_destroy_work() every time connectors are unregistered
from userspace's PoV. Additionally, extra calls to
drm_fb_helper_hotplug_event() also just mean more of a chance of doing a
connector probe somewhere we shouldn't.

So, don't send any hotplug events during resume if the MST topology
fails to come up. Just rely on the DP MST helpers to send them for us.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Feb. 6, 2019, 7:32 p.m. UTC | #1
On Tue, Jan 29, 2019 at 02:10:00PM -0500, Lyude Paul wrote:
> We have a bad habit of calling drm_fb_helper_hotplug_event() far more
> then we actually need to. MST appears to be one of these cases, where we
> call drm_fb_helper_hotplug_event() if we fail to resume a connected MST
> topology in intel_dp_mst_resume(). We don't actually need to do this at
> all though since hotplug events are already sent from
> drm_dp_connector_destroy_work() every time connectors are unregistered
> from userspace's PoV. Additionally, extra calls to
> drm_fb_helper_hotplug_event() also just mean more of a chance of doing a
> connector probe somewhere we shouldn't.
> 
> So, don't send any hotplug events during resume if the MST topology
> fails to come up. Just rely on the DP MST helpers to send them for us.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 681e88405ada..c2399acf177b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -7096,7 +7096,10 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
>  			continue;
>  
>  		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
> -		if (ret)
> -			intel_dp_check_mst_status(intel_dp);
> +		if (ret) {
> +			intel_dp->is_mst = false;
> +			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> +							false);

drm_dp_mst_topology_mgr_set_mst()
-> drm_dp_mst_topology_put_mstb()
   -> drm_dp_destroy_mst_branch_device()
      -> drm_dp_mst_topology_put_port()
         -> drm_dp_destroy_port()
	    -> drm_dp_destroy_connector_work()
	       -> drm_kms_helper_hotplug_event()

That of course requires that no one is hanging on to the
kref(s). The lifetime of the references isn't really clear
to me, but I'll take your word that it works.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		}
>  	}
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 681e88405ada..c2399acf177b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -7096,7 +7096,10 @@  void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
 			continue;
 
 		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
-		if (ret)
-			intel_dp_check_mst_status(intel_dp);
+		if (ret) {
+			intel_dp->is_mst = false;
+			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+							false);
+		}
 	}
 }