Re: [Intel-gfx] [PATCH] drm/i915/debugfs: HDCP capability enc NULL check

[Imre Deak <imre.deak@intel.com>]

On Tue, Feb 09, 2021 at 07:39:17AM +0200, Gupta, Anshuman wrote:
> > > > > > [...]
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > > index ae1371c36a32..58af323d189a 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > > @@ -135,11 +135,16 @@ int intel_hdcp_read_valid_bksv(struct intel_digital_port *dig_port,
> > > > > >  /* Is HDCP1.4 capable on Platform and Sink */
> > > > > > bool intel_hdcp_capable(struct intel_connector *connector)  {
> > > > > > -	struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
> > > > > > +	struct intel_digital_port *dig_port;
> > > > > >  	const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > > > > >  	bool capable = false;
> > > > > >  	u8 bksv[5];
> > > > > >
> > > > > > +	if (!connector->encoder)
> > > > > > +		return -ENODEV;
> > > > >
> > > > > I assume this is needed when called from i915_hdcp_sink_capability
> > > > > debugfs entry. That one is lacking the locking for the connector,
> > > > > but is that entry really needed? We print the same info already
> > > > > from the i915_display_info entry which has the proper locking and
> > > > > encoder check.
> > > >
> > > > Historically HDCP capability added to i915_display_info later to
> > > > debug CI machine as i915_display_info available as CI logs.  Now the
> > > > plans i915_display_info  should only show the monitor capability.
> > > > and i915_hdcp_sink_capability will check both sink and platform
> > > > capability.
> > >
> > > Ok, in any case the encoder NULL check and the required locking should
> > > be done in i915_hdcp_sink_capability_show().
>
> Need one input, AFAIU we do require
> drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, NULL)
> lock in i915_hdcp_sink_capability ?

Yes, and there's also drm_modeset_lock_single_interruptible() that could
be used.

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