[v2,2/4] drm/dp_mst: Remove port validation in drm_dp_atomic_find_vcpi_slots()

Submitted by Lyude Paul on Feb. 1, 2019, 1:14 a.m.

Details

Message ID 20190201011506.21055-3-lyude@redhat.com
State New
Headers show
Series "drm/dp_mst: Fix regressions from new atomic VCPI helpers" ( rev: 1 ) in Nouveau

Not browsing as part of any series.

Commit Message

Lyude Paul Feb. 1, 2019, 1:14 a.m.
Since we now have an easy way of refcounting drm_dp_mst_port structs and
safely accessing their contents, there isn't any good reason to keep
validating ports here. It doesn't prevent us from performing modesets on
branch devices that have been removed either, and we already disallow
enabling new displays on unregistered connectors in
update_connector_routing() in drm_atomic_check_modeset(). All it does is
cause us to have to make weird special exceptions in our atomic
modesetting code. So, get rid of it entirely.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: eceae1472467 ("drm/dp_mst: Start tracking per-port VCPI allocations")
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_dp_mst_topology.c   | 12 ++----------
 drivers/gpu/drm/i915/intel_dp_mst.c     | 17 ++++++-----------
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  3 +--
 3 files changed, 9 insertions(+), 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index abb0ea8ba9d9..4325e1518286 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3117,10 +3117,6 @@  int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
 	if (IS_ERR(topology_state))
 		return PTR_ERR(topology_state);
 
-	port = drm_dp_mst_topology_get_port_validated(mgr, port);
-	if (port == NULL)
-		return -EINVAL;
-
 	/* Find the current allocation for this port, if any */
 	list_for_each_entry(pos, &topology_state->vcpis, next) {
 		if (pos->port == port) {
@@ -3153,10 +3149,8 @@  int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
 	/* Add the new allocation to the state */
 	if (!vcpi) {
 		vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL);
-		if (!vcpi) {
-			ret = -ENOMEM;
-			goto out;
-		}
+		if (!vcpi)
+			return -ENOMEM;
 
 		drm_dp_mst_get_port_malloc(port);
 		vcpi->port = port;
@@ -3165,8 +3159,6 @@  int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
 	vcpi->vcpi = req_slots;
 
 	ret = req_slots;
-out:
-	drm_dp_mst_topology_put_port(port);
 	return ret;
 }
 EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index cdb83d294cdd..fb67cd931117 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -80,17 +80,12 @@  static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
 	pipe_config->pbn = mst_pbn;
 
-	/* Zombie connectors can't have VCPI slots */
-	if (!drm_connector_is_unregistered(connector)) {
-		slots = drm_dp_atomic_find_vcpi_slots(state,
-						      &intel_dp->mst_mgr,
-						      port,
-						      mst_pbn);
-		if (slots < 0) {
-			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
-				      slots);
-			return slots;
-		}
+	slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, port,
+					      mst_pbn);
+	if (slots < 0) {
+		DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
+			      slots);
+		return slots;
 	}
 
 	intel_link_compute_m_n(bpp, lane_count,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2e8a5fd9b262..60d858c2f2ce 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -771,8 +771,7 @@  nv50_msto_atomic_check(struct drm_encoder *encoder,
 	mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
 					 bpp);
 
-	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
-	    !drm_connector_is_unregistered(connector)) {
+	if (drm_atomic_crtc_needs_modeset(crtc_state)) {
 		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
 						      mstc->port, mstc->pbn);
 		if (slots < 0)

Comments

On Thu, Jan 31, 2019 at 08:14:49PM -0500, Lyude Paul wrote:
> Since we now have an easy way of refcounting drm_dp_mst_port structs and
> safely accessing their contents, there isn't any good reason to keep
> validating ports here. It doesn't prevent us from performing modesets on
> branch devices that have been removed either, and we already disallow
> enabling new displays on unregistered connectors in
> update_connector_routing() in drm_atomic_check_modeset(). All it does is
> cause us to have to make weird special exceptions in our atomic
> modesetting code. So, get rid of it entirely.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: eceae1472467 ("drm/dp_mst: Start tracking per-port VCPI allocations")
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c   | 12 ++----------
>  drivers/gpu/drm/i915/intel_dp_mst.c     | 17 ++++++-----------
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  3 +--
>  3 files changed, 9 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index abb0ea8ba9d9..4325e1518286 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3117,10 +3117,6 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
>  	if (IS_ERR(topology_state))
>  		return PTR_ERR(topology_state);
>  
> -	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> -	if (port == NULL)
> -		return -EINVAL;
> -
>  	/* Find the current allocation for this port, if any */
>  	list_for_each_entry(pos, &topology_state->vcpis, next) {
>  		if (pos->port == port) {

Also noticed that the WARN_ON() return -EINVAL; here gets fixed with this
patch.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> @@ -3153,10 +3149,8 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
>  	/* Add the new allocation to the state */
>  	if (!vcpi) {
>  		vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL);
> -		if (!vcpi) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +		if (!vcpi)
> +			return -ENOMEM;
>  
>  		drm_dp_mst_get_port_malloc(port);
>  		vcpi->port = port;
> @@ -3165,8 +3159,6 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
>  	vcpi->vcpi = req_slots;
>  
>  	ret = req_slots;
> -out:
> -	drm_dp_mst_topology_put_port(port);
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots);
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index cdb83d294cdd..fb67cd931117 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -80,17 +80,12 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  	pipe_config->pbn = mst_pbn;
>  
> -	/* Zombie connectors can't have VCPI slots */
> -	if (!drm_connector_is_unregistered(connector)) {
> -		slots = drm_dp_atomic_find_vcpi_slots(state,
> -						      &intel_dp->mst_mgr,
> -						      port,
> -						      mst_pbn);
> -		if (slots < 0) {
> -			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
> -				      slots);
> -			return slots;
> -		}
> +	slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, port,
> +					      mst_pbn);
> +	if (slots < 0) {
> +		DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
> +			      slots);
> +		return slots;
>  	}
>  
>  	intel_link_compute_m_n(bpp, lane_count,
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 2e8a5fd9b262..60d858c2f2ce 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -771,8 +771,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  	mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
>  					 bpp);
>  
> -	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> -	    !drm_connector_is_unregistered(connector)) {
> +	if (drm_atomic_crtc_needs_modeset(crtc_state)) {
>  		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
>  						      mstc->port, mstc->pbn);
>  		if (slots < 0)
> -- 
> 2.20.1
>