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

[Kandpal, Suraj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Monday, April 3, 2023 7:39 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Syrjala, Ville <ville.syrjala at intel.com>
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/hdcp: Remove
> drm_modeset_lock in intel_conn_to_vcpi
> 
> On Mon, Apr 03, 2023 at 12:31:25PM +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.
> >
> > --v2
> > -take lock if hdcp function is called from legacy path [Ville]
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 650232c4892b..65cfc4afbcaa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -36,13 +36,16 @@ static int intel_conn_to_vcpi(struct intel_connector
> *connector)
> >  	struct drm_dp_mst_atomic_payload *payload;
> >  	struct drm_dp_mst_topology_state *mst_state;
> >  	int vcpi = 0;
> > +	bool is_locked;
> >
> >  	/* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */
> >  	if (!connector->port)
> >  		return 0;
> >  	mgr = connector->port->mgr;
> > +	is_locked = drm_modeset_is_locked(&mgr->base.lock);
> 
> The fact that *someone* is holding the lock doesn't mean you can just skip
> taking it yourself.
> 

So the reason for doing this here is that if we come here through atomic path
The locks will already be taken for us because of which we wont need to lock but
As you correctly pointed out int the last version that hdcp functions can be called through a legacy path
and this condition is to satisfy that path so we end up taking a lock then

Regards,
Suraj Kandpal

> > +	if (!is_locked)
> > +		drm_modeset_lock(&mgr->base.lock, NULL);
> >
> > -	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)) @@ -54,7 +57,9 @@ static
> int
> > intel_conn_to_vcpi(struct intel_connector *connector)
> >  		goto out;
> >  	}
> >  out:
> > -	drm_modeset_unlock(&mgr->base.lock);
> > +	if (!is_locked)
> > +		drm_modeset_unlock(&mgr->base.lock);
> > +
> >  	return vcpi;
> >  }
> >
> > --
> > 2.25.1
> 
> --
> Ville Syrjälä
> Intel