Re: [RFC COMP I/F] drm/i915: Initialize HDCP2.2 and its MEI interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 09, 2018 at 07:01:09PM +0530, Ramalingam C wrote:
> Initialize HDCP2.2 support. This includes the mei interface
> initialization along with required component registration.
> 
> v2:
>   mei interface handle is protected with mutex. [Chris Wilson]
> v3:
>   Notifiers are used for the mei interface state.
> v4:
>   Poll for mei client device state
>   Error msg for out of mem [Uma]
>   Inline req for init function removed [Uma]
> v5:
>   Rebase as Part of reordering.
>   Component is used for the I915 and MEI_HDCP interface [Daniel]
> v6:
>   I915 registration is moved into hdcp component bind [Daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.c   |  33 +++++--
>  drivers/gpu/drm/i915/i915_drv.h   |   3 +
>  drivers/gpu/drm/i915/intel_dp.c   |   3 +-
>  drivers/gpu/drm/i915/intel_drv.h  |   6 +-
>  drivers/gpu/drm/i915/intel_hdcp.c | 193 +++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_hdmi.c |   2 +-
>  include/drm/i915_component.h      |  88 +++++++++++++++++
>  7 files changed, 314 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0db3c83cce29..e6dc0f409a6e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1310,6 +1310,19 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
>  		DRM_INFO("DRM_I915_DEBUG_GEM enabled\n");
>  }
>  
> +void i915_driver_load_tail(struct drm_i915_private *dev_priv)
> +{

This isn't quite enough, we also must delay anything that might do a
modeset. For almost everything this happens when we call
i915_driver_register, but the fbdev emulation gets set up before that. We
need to also include that (and everything after that point) in the
load_tail function here.

> +	i915_driver_register(dev_priv);
> +
> +	intel_runtime_pm_enable(dev_priv);
> +
> +	intel_init_ipc(dev_priv);
> +
> +	intel_runtime_pm_put(dev_priv);
> +
> +	i915_welcome_messages(dev_priv);
> +}
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @pdev: PCI device
> @@ -1389,16 +1402,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret < 0)
>  		goto out_cleanup_hw;
>  
> -	i915_driver_register(dev_priv);
> -
> -	intel_runtime_pm_enable(dev_priv);
> -
> -	intel_init_ipc(dev_priv);
> -
> -	intel_runtime_pm_put(dev_priv);
> -
> -	i915_welcome_messages(dev_priv);
> +	if (is_hdcp2_supported(dev_priv)) {
> +		ret = intel_hdcp_component_init(dev_priv);
> +		if (ret) {
> +			DRM_DEV_ERROR(&pdev->dev, "HDCP comp init failed %d\n",
> +				      ret);
> +			goto out_cleanup_hw;
> +		}
> +		DRM_DEV_INFO(&pdev->dev, "I915 waits for MEI HDCP bind.\n");
> +		return 0;
> +	}
>  
> +	i915_driver_load_tail(dev_priv);

Yup, this is roughly what I had in mind. The only slightly annoying thing
is that this sprinkles is_hdcp2_supported() checks all over the codebase.
What I had in mind instead to have slightly more polished code, and make
this componentn business more useful in general for us:

- Put the component master into i915_dev_priv instead of hiding it in the
  hdcp structure.

- Create a 2nd component that fires when driver loading is complete (i.e.
  right here). This can be a really dumb one with nothing as callbacks.

- Put the driver_load_tail call unconditionally into the master_bind, and
  put the component_master_add_with_match() call also here.

- Don't move the hdcp init around since that's no longer necessary.

>  	return 0;
>  
>  out_cleanup_hw:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 09ab12458244..febb8c8dc457 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2119,6 +2119,8 @@ struct drm_i915_private {
>  
>  	struct i915_pmu pmu;
>  
> +	struct i915_hdcp_component *hdcp_comp;
> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> @@ -2732,6 +2734,7 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv);
>  int intel_engines_init(struct drm_i915_private *dev_priv);
>  
>  u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv);
> +void i915_driver_load_tail(struct drm_i915_private *dev_priv);
>  
>  /* intel_hotplug.c */
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5be07e1d816d..12eb5bd33b7e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6406,7 +6406,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 5799788c8f5d..c40c8283bf79 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -443,7 +443,6 @@ struct intel_hdcp {
>  
>  	/* mei interface related information */
>  	struct mei_hdcp_data mei_data;
> -
>  	struct delayed_work hdcp2_check_work;
>  };
>  
> @@ -1963,11 +1962,14 @@ 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);
>  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
> +int intel_hdcp_component_init(struct drm_i915_private *dev_priv);
> +bool is_hdcp2_supported(struct drm_i915_private *dev_priv);
>  
>  /* intel_psr.c */
>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 65bbe5874eee..928fcd214cf0 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -8,13 +8,19 @@
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_hdcp.h>
> +#include <drm/i915_component.h>
>  #include <linux/i2c.h>
>  #include <linux/random.h>
> +#include <linux/component.h>
>  
>  #include "intel_drv.h"
>  #include "i915_reg.h"
>  
>  #define KEY_LOAD_TRIES	5
> +#define GET_MEI_DDI_INDEX(port)		(((port) == PORT_A) ? DDI_A : \
> +					 (enum hdcp_physical_port)(port))
> +
> +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)
> @@ -743,11 +749,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)
> @@ -757,6 +767,10 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	mutex_init(&hdcp->hdcp_mutex);
>  	INIT_DELAYED_WORK(&hdcp->hdcp_check_work, intel_hdcp_check_work);
>  	INIT_WORK(&hdcp->hdcp_prop_work, intel_hdcp_prop_work);
> +
> +	if (hdcp2_supported)
> +		intel_hdcp2_init(connector);
> +
>  	return 0;
>  }
>  
> @@ -896,3 +910,180 @@ int intel_hdcp_check_link(struct intel_connector *connector)
>  	mutex_unlock(&hdcp->hdcp_mutex);
>  	return ret;
>  }
> +
> +static int i915_hdcp_component_master_bind(struct device *dev)
> +{
> +	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
> +	struct i915_hdcp_component *comp = dev_priv->hdcp_comp;
> +	static bool i915_loaded;
> +	int ret;
> +
> +	mutex_lock(&comp->mutex);
> +	ret = component_bind_all(dev, comp);
> +	if (ret < 0) {
> +		mutex_unlock(&comp->mutex);
> +		return ret;
> +	}
> +
> +	if (!i915_loaded)

I have no idea why you need this one here ...

> +		i915_driver_load_tail(dev_priv);
> +
> +	mutex_unlock(&comp->mutex);

Also not clear on the need for locking here? I thought component takes
care of that for you.

> +	return 0;
> +}
> +
> +static void i915_hdcp_component_master_unbind(struct device *dev)
> +{
> +	struct drm_i915_private *dev_priv = kdev_to_i915(dev);
> +	struct i915_hdcp_component *comp = dev_priv->hdcp_comp;
> +
> +	mutex_lock(&comp->mutex);

Same here.

Cheers, Daniel

> +	component_unbind_all(dev, comp);
> +	WARN_ON(comp->ops);
> +	mutex_unlock(&comp->mutex);
> +}
> +
> +static const struct component_master_ops i915_hdcp_component_master_ops = {
> +	.bind = i915_hdcp_component_master_bind,
> +	.unbind = i915_hdcp_component_master_unbind,
> +};
> +
> +static void intel_hdcp_component_cleanup(struct drm_i915_private *dev_priv)
> +{
> +	component_master_del(dev_priv->drm.dev,
> +			     &i915_hdcp_component_master_ops);
> +	kfree(dev_priv->hdcp_comp);
> +}
> +
> +static void intel_hdcp2_exit(struct intel_connector *connector)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +
> +	/* TO-DO: Disable the HDCP here */
> +	kfree(connector->hdcp.mei_data.streams);
> +
> +	if (dev_priv->hdcp_comp)
> +		intel_hdcp_component_cleanup(dev_priv);
> +}
> +
> +static void intel_pull_down_mei_interface(struct device *kdev)
> +{
> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +	struct drm_device *dev = &dev_priv->drm;
> +	struct intel_connector *intel_connector;
> +	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +
> +	DRM_INFO("MEI Device is Down");
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		intel_connector = to_intel_connector(connector);
> +		if (!intel_connector->hdcp.hdcp2_supported)
> +			continue;
> +
> +		intel_hdcp2_exit(intel_connector);
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +}
> +
> +struct i915_hdcp_component_master_ops master_ops = {
> +	.pull_down_interface = intel_pull_down_mei_interface,
> +};
> +
> +static int i915_hdcp_component_master_match(struct device *dev, void *data)
> +{
> +	return !strcmp(dev->driver->name, "mei_hdcp");
> +}
> +
> +int intel_hdcp_component_init(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_hdcp_component *comp;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	if (dev_priv->hdcp_comp)
> +		return -EINVAL;
> +
> +	comp = kzalloc(sizeof(*comp), GFP_KERNEL);
> +	if (!comp)
> +		return -ENOMEM;
> +
> +	mutex_init(&comp->mutex);
> +	comp->master_ops = &master_ops;
> +	dev_priv->hdcp_comp = comp;
> +
> +	component_match_add(dev_priv->drm.dev, &match,
> +			    i915_hdcp_component_master_match, dev_priv);
> +	ret = component_master_add_with_match(dev_priv->drm.dev,
> +					      &i915_hdcp_component_master_ops,
> +					      match);
> +	if (ret < 0)
> +		goto out_err;
> +
> +	DRM_INFO("I915 hdcp component master added.\n");
> +	return ret;
> +
> +out_err:
> +	component_master_del(dev_priv->drm.dev,
> +			     &i915_hdcp_component_master_ops);
> +	kfree(comp);
> +	dev_priv->hdcp_comp = NULL;
> +	DRM_ERROR("Failed to add i915 hdcp component master (%d)\n", ret);
> +
> +	return ret;
> +}
> +
> +static int initialize_mei_hdcp_data(struct intel_connector *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) {
> +		DRM_ERROR("Out of Memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	data->streams[0].stream_id = 0;
> +	data->streams[0].stream_type = hdcp->content_type;
> +
> +	return 0;
> +}
> +
> +bool is_hdcp2_supported(struct drm_i915_private *dev_priv)
> +{
> +	return ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> +		IS_KABYLAKE(dev_priv)) && IS_ENABLED(CONFIG_INTEL_MEI_HDCP));
> +}
> +
> +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;
> +
> +	WARN_ON(!is_hdcp2_supported(dev_priv));
> +	ret = initialize_mei_hdcp_data(connector);
> +	if (ret)
> +		goto exit;
> +
> +	if (ret)
> +		kfree(hdcp->mei_data.streams);
> +	hdcp->hdcp2_supported = true;
> +
> +exit:
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 8363fbd18ee8..7988f958d835 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2366,7 +2366,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");
>  	}
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index 346b1f5cb180..9986174ee090 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -24,6 +24,10 @@
>  #ifndef _I915_COMPONENT_H_
>  #define _I915_COMPONENT_H_
>  
> +#include <linux/mei_cl_bus.h>
> +#include <linux/mei_hdcp.h>
> +#include <drm/drm_hdcp.h>
> +
>  /* MAX_PORT is the number of port
>   * It must be sync with I915_MAX_PORTS defined i915_drv.h
>   */
> @@ -121,4 +125,88 @@ struct i915_audio_component {
>  	const struct i915_audio_component_audio_ops *audio_ops;
>  };
>  
> +struct i915_hdcp_component_ops {
> +	/**
> +	 * @owner: mei_hdcp module
> +	 */
> +	struct module *owner;
> +	int (*initiate_hdcp2_session)(struct mei_cl_device *cldev,
> +				      struct mei_hdcp_data *data,
> +				      struct hdcp2_ake_init *ake_data);
> +	int
> +	(*verify_receiver_cert_prepare_km)(struct mei_cl_device *cldev,
> +					   struct mei_hdcp_data *data,
> +					   struct hdcp2_ake_send_cert *rx_cert,
> +					   bool *km_stored,
> +					   struct hdcp2_ake_no_stored_km
> +								*ek_pub_km,
> +					   size_t *msg_sz);
> +	int (*verify_hprime)(struct mei_cl_device *cldev,
> +			     struct mei_hdcp_data *data,
> +			     struct hdcp2_ake_send_hprime *rx_hprime);
> +	int (*store_pairing_info)(struct mei_cl_device *cldev,
> +				  struct mei_hdcp_data *data,
> +				  struct hdcp2_ake_send_pairing_info
> +								*pairing_info);
> +	int (*initiate_locality_check)(struct mei_cl_device *cldev,
> +				       struct mei_hdcp_data *data,
> +				       struct hdcp2_lc_init *lc_init_data);
> +	int (*verify_lprime)(struct mei_cl_device *cldev,
> +			     struct mei_hdcp_data *data,
> +			     struct hdcp2_lc_send_lprime *rx_lprime);
> +	int (*get_session_key)(struct mei_cl_device *cldev,
> +			       struct mei_hdcp_data *data,
> +			       struct hdcp2_ske_send_eks *ske_data);
> +	int
> +	(*repeater_check_flow_prepare_ack)(struct mei_cl_device *cldev,
> +					   struct mei_hdcp_data *data,
> +					   struct hdcp2_rep_send_receiverid_list
> +								*rep_topology,
> +					   struct hdcp2_rep_send_ack
> +								*rep_send_ack);
> +	int (*verify_mprime)(struct mei_cl_device *cldev,
> +			     struct mei_hdcp_data *data,
> +			     struct hdcp2_rep_stream_ready *stream_ready);
> +	int (*enable_hdcp_authentication)(struct mei_cl_device *cldev,
> +					  struct mei_hdcp_data *data);
> +	int (*close_hdcp_session)(struct mei_cl_device *cldev,
> +				  struct mei_hdcp_data *data);
> +};
> +
> +struct i915_hdcp_component_master_ops {
> +	void (*pull_down_interface)(struct device *dev);
> +};
> +
> +/**
> + * struct i915_mei_hdcp_component - Used for direct communication between i915
> + * and mei_hdcp drivers
> + */
> +struct i915_hdcp_component {
> +	/**
> +	 * @dev: Kdev of Slave Component. Passed to bind calls.
> +	 */
> +	struct device *dev;
> +	/**
> +	 * @i915_kdev: Kdev of I915. Used from the client component for
> +	 * removing the reference to mei_cldev.
> +	 */
> +	struct device *i915_kdev;
> +	/**
> +	 * @mei_cldev: mei client device, used as parameter for ops
> +	 */
> +	struct mei_cl_device *mei_cldev;
> +	/**
> +	 * @mutex: Mutex to protect the state of mei_cldev
> +	 */
> +	struct mutex mutex;
> +	/**
> +	 * @ops: Ops implemented by mei_hdcp driver, used by i915 driver.
> +	 */
> +	const struct i915_hdcp_component_ops *ops;
> +	/**
> +	 * @master_ops: Ops implemented by I915 driver, used by mei_hdcp driver.
> +	 */
> +	const struct i915_hdcp_component_master_ops *master_ops;
> +};
> +
>  #endif /* _I915_COMPONENT_H_ */
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux