Re: [Intel-gfx] [PATCH v3 30/40] drm/i915: Initialize HDCP2.2 and its MEI interface

[Ramalingam C <ramalingam.c@xxxxxxxxx>]

On Friday 18 May 2018 06:03 PM, Shankar, Uma wrote:
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
> > Ramalingam C
> > Sent: Tuesday, April 3, 2018 7:28 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> > seanpaul@xxxxxxxxxxxx; daniel@xxxxxxxx; chris@xxxxxxxxxxxxxxxxxx;
> > jani.nikula@xxxxxxxxxxxxxxx; Winkler, Tomas <tomas.winkler@xxxxxxxxx>;
> > Usyskin, Alexander <alexander.usyskin@xxxxxxxxx>
> > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
> > Subject: [Intel-gfx] [PATCH v3 30/40] drm/i915: Initialize HDCP2.2 and its MEI
> > interface
> >
> > Initialize HDCP2.2 support. This includes the mei interface initialization along with
> > required notifier registration.
> >
> > v2:
> >   mei interface handle is protected with mutex. [Chris Wilson]
> > v3:
> >   Notifiers are used for the mei interface state.
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c   |   3 +-
> > drivers/gpu/drm/i915/intel_drv.h  |   5 +-
> > drivers/gpu/drm/i915/intel_hdcp.c | 104
> > +++++++++++++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_hdmi.c |   2 +-
> > 4 files changed, 109 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 9a4a51e79fa1..955a20208097 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6381,7 +6381,8 @@ intel_dp_init_connector(struct intel_digital_port
> > *intel_dig_port,
> > 	intel_dp_add_properties(intel_dp, connector);
> >
> > 	if (is_hdcp_supported(dev_priv, port) && !intel_dp_is_edp(intel_dp)) {
> > -		int ret = intel_hdcp_init(intel_connector, &intel_dp_hdcp_shim);
> > +		int ret = intel_hdcp_init(intel_connector, &intel_dp_hdcp_shim,
> > +					  false);
> > 		if (ret)
> > 			DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
> > 	}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ca06d9a158f6..2f14756b4b0e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -442,7 +442,7 @@ struct intel_hdcp {
> > 	/* mei interface related information */
> > 	struct mei_cl_device *cldev;
> > 	struct mei_hdcp_data mei_data;
> > -
> > +	struct notifier_block mei_cldev_nb;
> > 	struct delayed_work hdcp2_check_work;
> > };
> >
> > @@ -1928,7 +1928,8 @@ void intel_hdcp_atomic_check(struct drm_connector
> > *connector,
> > 			     struct drm_connector_state *old_state,
> > 			     struct drm_connector_state *new_state);  int
> > intel_hdcp_init(struct intel_connector *connector,
> > -		    const struct intel_hdcp_shim *hdcp_shim);
> > +		    const struct intel_hdcp_shim *hdcp_shim,
> > +		    bool hdcp2_supported);
> > int intel_hdcp_enable(struct intel_connector *connector);  int
> > intel_hdcp_disable(struct intel_connector *connector);  int
> > intel_hdcp_check_link(struct intel_connector *connector); diff --git
> > a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index 53d35ee8f683..6eb58a833c7d 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -11,6 +11,7 @@
> > #include <linux/i2c.h>
> > #include <linux/random.h>
> > #include <linux/mei_hdcp.h>
> > +#include <linux/notifier.h>
> >
> > #include "intel_drv.h"
> > #include "i915_reg.h"
> > @@ -25,6 +26,7 @@ static int _intel_hdcp2_enable(struct intel_connector
> > *connector);  static int _intel_hdcp2_disable(struct intel_connector *connector);
> > static void intel_hdcp2_check_work(struct work_struct *work);  static int
> > intel_hdcp2_check_link(struct intel_connector *connector);
> > +static int intel_hdcp2_init(struct intel_connector *connector);
> >
> > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> > 				    const struct intel_hdcp_shim *shim) @@ -
> > 686,11 +688,15 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv,
> > enum port port)  }
> >
> > int intel_hdcp_init(struct intel_connector *connector,
> > -		    const struct intel_hdcp_shim *hdcp_shim)
> > +		    const struct intel_hdcp_shim *hdcp_shim,
> > +		    bool hdcp2_supported)
> > {
> > 	struct intel_hdcp *hdcp = &connector->hdcp;
> > 	int ret;
> >
> > +	if (!hdcp_shim)
> > +		return -EINVAL;
> > +
> > 	ret = drm_connector_attach_content_protection_property(
> > 			&connector->base);
> > 	if (ret)
> > @@ -699,7 +705,12 @@ int intel_hdcp_init(struct intel_connector *connector,
> > 	hdcp->hdcp_shim = hdcp_shim;
> > 	mutex_init(&hdcp->hdcp_mutex);
> > 	INIT_DELAYED_WORK(&hdcp->hdcp_check_work,
> > intel_hdcp_check_work);
> > +	INIT_DELAYED_WORK(&hdcp->hdcp2_check_work,
> > intel_hdcp2_check_work);
> > 	INIT_WORK(&hdcp->hdcp_prop_work, intel_hdcp_prop_work);
> > +
> > +	if (hdcp2_supported)
> > +		intel_hdcp2_init(connector);
> > +
> > 	return 0;
> > }
> >
> > @@ -1565,3 +1576,94 @@ static void intel_hdcp2_check_work(struct
> > work_struct *work)
> > 		schedule_delayed_work(&hdcp->hdcp2_check_work,
> > 				      DRM_HDCP2_CHECK_PERIOD_MS);
> > }
> > +
> > +static inline int initialize_mei_hdcp_data(struct intel_connector
> This is too big to be inline.
Ok. Anyhow not much optimization also as this is called very few times.
I will remove the inline request
> > +*connector) {
> > +	struct intel_hdcp *hdcp = &connector->hdcp;
> > +	struct mei_hdcp_data *data = &hdcp->mei_data;
> > +	enum port port;
> > +
> > +	if (connector->encoder) {
> > +		port = connector->encoder->port;
> > +		data->port = GET_MEI_DDI_INDEX(port);
> > +	}
> > +
> > +	data->port_type = INTEGRATED;
> > +	data->protocol = hdcp->hdcp_shim->hdcp_protocol();
> > +
> > +	data->k = 1;
> > +	if (!data->streams)
> > +		data->streams = kcalloc(data->k,
> > +					sizeof(struct hdcp2_streamid_type),
> > +					GFP_KERNEL);
> > +	if (!data->streams)
> > +		return -ENOMEM;
> > +
> > +	data->streams[0].stream_id = 0;
> > +	data->streams[0].stream_type = hdcp->content_type;
> > +
> > +	return 0;
> > +}
> > +
> > +static void intel_hdcp2_exit(struct intel_connector *connector) {
> > +	intel_hdcp_disable(connector);
> > +	kfree(connector->hdcp.mei_data.streams);
> > +}
> > +
> > +static int mei_cldev_notify(struct notifier_block *nb, unsigned long event,
> > +		      void *cldev)
> > +{
> > +	struct intel_hdcp *hdcp = container_of(nb, struct intel_hdcp,
> > +					       mei_cldev_nb);
> > +	struct intel_connector *intel_connector = container_of(hdcp,
> > +							struct intel_connector,
> > +							hdcp);
> > +
> > +	DRM_ERROR("MEI_HDCP Notification. Interface: %s\n",
> > +		  cldev ? "UP" : "Down");
> > +
> > +	if (event == MEI_CLDEV_ENABLED) {
> > +		hdcp->cldev = cldev;
> > +		initialize_mei_hdcp_data(intel_connector);
> > +	} else {
> > +		hdcp->cldev = NULL;
> > +		intel_hdcp2_exit(intel_connector);
> > +	}
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static inline
> > +bool is_hdcp2_supported(struct drm_i915_private *dev_priv) {
> > +	return (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> > +		IS_KABYLAKE(dev_priv));
> I feel we should add a new field in the platform capability structure, like its done
> for various other capabilities (eg DBUF etc.).
Going forward HDCP2.2 will be supported on all future platforms >= gen10.
So we should be good.
> > +}
> > +
> > +static int intel_hdcp2_init(struct intel_connector *connector) {
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > +	struct intel_hdcp *hdcp = &connector->hdcp;
> > +	int ret;
> > +
> > +	if (!is_hdcp2_supported(dev_priv))
> > +		return -EINVAL;
> > +
> > +	hdcp->hdcp2_supported = true;
> > +
> > +	hdcp->mei_cldev_nb.notifier_call = mei_cldev_notify;
> > +	ret = mei_cldev_register_notify(&hdcp->mei_cldev_nb);
> > +	if (ret) {
> > +		DRM_ERROR("mei_cldev not available. %d\n", ret);
> > +		goto exit;
> > +	}
> > +
> > +	ret = initialize_mei_hdcp_data(connector);
> > +	if (ret)
> 	A debug message for failure should be helpful.
Adding an error msg at the mem alloc failure scenario itself.

Ram
> > +		mei_cldev_unregister_notify(&hdcp->mei_cldev_nb);
> > +
> > +exit:
> > +	if (ret)
> > +		hdcp->hdcp2_supported = false;
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 1baef4ac7ecb..b8b1086c0cbd 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2342,7 +2342,7 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >
> > 	if (is_hdcp_supported(dev_priv, port)) {
> > 		int ret = intel_hdcp_init(intel_connector,
> > -					  &intel_hdmi_hdcp_shim);
> > +					  &intel_hdmi_hdcp_shim, false);
> > 		if (ret)
> > 			DRM_DEBUG_KMS("HDCP init failed, skipping.\n");
> > 	}
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel