Re: [Intel-gfx] [PATCH] drm/i915/hdcp: Remove drm_modeset_lock in intel_conn_to_vcpi

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Mar 16, 2023 at 01:52:32PM +0530, Suraj Kandpal wrote:
> Remove drm_modeset_lock in intel_conn_to_vcpi as we don't need it
> anymore since all the required locks are taken in atomic check and
> prepare phases.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 2984d2810e42..f957b4bd9c26 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -41,7 +41,6 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
>  		return 0;
>  	mgr = connector->port->mgr;
>  
> -	drm_modeset_lock(&mgr->base.lock, NULL);
>  	mst_state = to_drm_dp_mst_topology_state(mgr->base.state);
>  	payload = drm_atomic_get_mst_payload_state(mst_state, connector->port);
>  	if (drm_WARN_ON(mgr->dev, !payload))
> @@ -53,7 +52,6 @@ static int intel_conn_to_vcpi(struct intel_connector *connector)
>  		goto out;
>  	}
>  out:
> -	drm_modeset_unlock(&mgr->base.lock);
>  	return vcpi;
>  }

That whole function looks like it something that should be
part of the drm_dp_mst_helper.c.

Also, it's directly accessing mgr->base.state which is just
wrong.

And it looks like it can get called from outside the normal
atomic commit flows (like so many other things in the hdcp
code, sigh), so what you're doing here is also wrong in
that case.

So the whole thing looks just very broken to me. Not to
mention the HDCP vs. MST<->SST switch is also still
fundementally broken. I'm really tempted to just send
a patch to nuke the entire HDCP MST code.

-- 
Ville Syrjälä
Intel