diff mbox series

[v10,36/40] misc/mei/hdcp: Component framework for I915 Interface

Message ID 1548917996-28081-37-git-send-email-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Implement HDCP2.2 | expand

Commit Message

Ramalingam C Jan. 31, 2019, 6:59 a.m. UTC 
Mei hdcp driver is designed as component slave for the I915 component
master.

v2: Rebased.
v3:
  Notifier chain is adopted for cldev state update [Tomas]
v4:
  Made static dummy functions as inline in mei_hdcp.h
  API for polling client device status
  IS_ENABLED used in header, for config status for mei_hdcp.
v5:
  Replacing the notifier with component framework. [Daniel]
v6:
  Rebased on the I915 comp master redesign.
v7:
  mei_hdcp_component_registered is made static [Uma]
  Need for global static variable mei_cldev is removed.
v8:
  master comp is added to be matched with i915 subcomponent [daniel]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/misc/mei/hdcp/mei_hdcp.c | 90 +++++++++++++++++++++++++++++++++++++++-
 drivers/misc/mei/hdcp/mei_hdcp.h |  5 +++
 2 files changed, 93 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Jan. 31, 2019, 8:23 a.m. UTC | #1 
On Thu, Jan 31, 2019 at 12:29:52PM +0530, Ramalingam C wrote:
> Mei hdcp driver is designed as component slave for the I915 component
> master.
> 
> v2: Rebased.
> v3:
>   Notifier chain is adopted for cldev state update [Tomas]
> v4:
>   Made static dummy functions as inline in mei_hdcp.h
>   API for polling client device status
>   IS_ENABLED used in header, for config status for mei_hdcp.
> v5:
>   Replacing the notifier with component framework. [Daniel]
> v6:
>   Rebased on the I915 comp master redesign.
> v7:
>   mei_hdcp_component_registered is made static [Uma]
>   Need for global static variable mei_cldev is removed.
> v8:
>   master comp is added to be matched with i915 subcomponent [daniel]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Pretty sure v8 has nothing to do with what Uma originally reviewed. If you
keep an r-b with substantial changes pls add an indicator about which old
version this was for. But for completely rewritten patches like this one
here it's best to outright drop it.

> ---
>  drivers/misc/mei/hdcp/mei_hdcp.c | 90 +++++++++++++++++++++++++++++++++++++++-
>  drivers/misc/mei/hdcp/mei_hdcp.h |  5 +++
>  2 files changed, 93 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
> index edfc70fb0617..be2ce12ca460 100644
> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> @@ -23,6 +23,7 @@
>  #include <linux/slab.h>
>  #include <linux/uuid.h>
>  #include <linux/mei_cl_bus.h>
> +#include <linux/component.h>
>  #include <drm/drm_connector.h>
>  #include <drm/i915_component.h>
>  #include <drm/i915_mei_hdcp_interface.h>
> @@ -692,8 +693,7 @@ mei_close_hdcp_session(struct device *dev, struct hdcp_port_data *data)
>  	return 0;
>  }
>  
> -static __attribute__((unused))
> -struct i915_hdcp_component_ops mei_hdcp_ops = {
> +static struct i915_hdcp_component_ops mei_hdcp_ops = {
>  	.owner = THIS_MODULE,
>  	.initiate_hdcp2_session = mei_initiate_hdcp2_session,
>  	.verify_receiver_cert_prepare_km = mei_verify_receiver_cert_prepare_km,
> @@ -708,20 +708,106 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
>  	.close_hdcp_session = mei_close_hdcp_session,
>  };
>  
> +static int mei_component_master_bind(struct device *dev)
> +{
> +	struct mei_cl_device *cldev = to_mei_cl_device(dev);
> +	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
> +	int ret;
> +
> +	dev_info(dev, "%s\n", __func__);
> +	drv_data->comp_master->ops = &mei_hdcp_ops;
> +	drv_data->comp_master->mei_dev = dev;
> +	ret = component_bind_all(dev, drv_data->comp_master);

> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void mei_component_master_unbind(struct device *dev)
> +{
> +	struct mei_cl_device *cldev = to_mei_cl_device(dev);
> +	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
> +
> +	dev_info(dev, "%s\n", __func__);
> +	component_unbind_all(dev, drv_data->comp_master);
> +}
> +
> +static const struct component_master_ops mei_component_master_ops = {
> +	.bind = mei_component_master_bind,
> +	.unbind = mei_component_master_unbind,
> +};
> +
> +static int mei_hdcp_component_match(struct device *dev, int subcomponent,
> +				    void *data)
> +{
> +	return !strcmp(dev->driver->name, "i915") &&
> +	       subcomponent == I915_COMPONENT_HDCP;
> +}
> +
>  static int mei_hdcp_probe(struct mei_cl_device *cldev,
>  			  const struct mei_cl_device_id *id)
>  {
> +	struct mei_hdcp_drv_data *drv_data;
>  	int ret;
>  
>  	ret = mei_cldev_enable(cldev);
>  	if (ret < 0)
>  		dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
>  
> +	drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
> +	if (!drv_data) {
> +		ret = -ENOMEM;
> +		goto drv_data_exit;
> +	}
> +
> +	drv_data->comp_master = kzalloc(sizeof(*drv_data->comp_master),
> +					GFP_KERNEL);
> +	if (!drv_data->comp_master) {
> +		ret = -ENOMEM;
> +		goto comp_master_exit;
> +	}
> +
> +	drv_data->master_match = NULL;
> +	component_match_add_typed(&cldev->dev, &drv_data->master_match,
> +				  mei_hdcp_component_match,
> +				  drv_data->comp_master);
> +	if (IS_ERR_OR_NULL(drv_data->master_match)) {
> +		ret = -ENOMEM;
> +		goto match_add_exit;
> +	}
> +
> +	mei_cldev_set_drvdata(cldev, drv_data);
> +	ret = component_master_add_with_match(&cldev->dev,
> +					      &mei_component_master_ops,
> +					      drv_data->master_match);
> +	if (ret < 0) {
> +		dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
> +		mei_cldev_set_drvdata(cldev, NULL);
> +		goto match_add_exit;
> +	}
> +
> +	return 0;
> +
> +match_add_exit:
> +	kfree(drv_data->comp_master);
> +comp_master_exit:
> +	kfree(drv_data);
> +drv_data_exit:
> +	mei_cldev_disable(cldev);
> +
>  	return ret;
>  }
>  
>  static int mei_hdcp_remove(struct mei_cl_device *cldev)
>  {
> +	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
> +
> +	component_master_del(&cldev->dev, &mei_component_master_ops);
> +	kfree(drv_data->comp_master);
> +	kfree(drv_data);
> +	mei_cldev_set_drvdata(cldev, NULL);
> +
>  	return mei_cldev_disable(cldev);
>  }
>  
> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h b/drivers/misc/mei/hdcp/mei_hdcp.h
> index f831db3cbd54..5c872a57ba11 100644
> --- a/drivers/misc/mei/hdcp/mei_hdcp.h
> +++ b/drivers/misc/mei/hdcp/mei_hdcp.h
> @@ -363,6 +363,11 @@ struct wired_cmd_repeater_auth_stream_req_out {
>  	struct hdcp_port_id	port;
>  } __packed;
>  
> +struct mei_hdcp_drv_data {
> +	struct i915_hdcp_comp_master *comp_master;
> +	struct component_match *master_match;

I think this can just be a local variable in mei_hdcp_probe, and you could
make the comp_master directly your drv_data structure. Memory is released
through the magic of devm_. With that:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +};
> +
>  enum mei_hdcp_ddi {
>  	MEI_DDI_INVALID_PORT = 0x0,
>  
> -- 
> 2.7.4
>
Shankar, Uma Feb. 4, 2019, 4:27 p.m. UTC | #2 
>-----Original Message-----
>From: C, Ramalingam
>Sent: Thursday, January 31, 2019 12:30 PM
>To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>daniel.vetter@ffwll.ch; Winkler, Tomas <tomas.winkler@intel.com>; Shankar,
>Uma <uma.shankar@intel.com>
>Cc: C, Ramalingam <ramalingam.c@intel.com>
>Subject: [PATCH v10 36/40] misc/mei/hdcp: Component framework for I915
>Interface
>
>Mei hdcp driver is designed as component slave for the I915 component master.
>
>v2: Rebased.
>v3:
>  Notifier chain is adopted for cldev state update [Tomas]
>v4:
>  Made static dummy functions as inline in mei_hdcp.h
>  API for polling client device status
>  IS_ENABLED used in header, for config status for mei_hdcp.
>v5:
>  Replacing the notifier with component framework. [Daniel]
>v6:
>  Rebased on the I915 comp master redesign.
>v7:
>  mei_hdcp_component_registered is made static [Uma]
>  Need for global static variable mei_cldev is removed.
>v8:
>  master comp is added to be matched with i915 subcomponent [daniel]
>
>Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>Reviewed-by: Uma Shankar <uma.shankar@intel.com>

This is a new interface change and definitely a lot is changed from the time I reviewed.
You can drop my RB and I would suggest to go with Daniel and Tomas's feedback for the same.

>---
> drivers/misc/mei/hdcp/mei_hdcp.c | 90
>+++++++++++++++++++++++++++++++++++++++-
> drivers/misc/mei/hdcp/mei_hdcp.h |  5 +++
> 2 files changed, 93 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
>b/drivers/misc/mei/hdcp/mei_hdcp.c
>index edfc70fb0617..be2ce12ca460 100644
>--- a/drivers/misc/mei/hdcp/mei_hdcp.c
>+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
>@@ -23,6 +23,7 @@
> #include <linux/slab.h>
> #include <linux/uuid.h>
> #include <linux/mei_cl_bus.h>
>+#include <linux/component.h>
> #include <drm/drm_connector.h>
> #include <drm/i915_component.h>
> #include <drm/i915_mei_hdcp_interface.h> @@ -692,8 +693,7 @@
>mei_close_hdcp_session(struct device *dev, struct hdcp_port_data *data)
> 	return 0;
> }
>
>-static __attribute__((unused))
>-struct i915_hdcp_component_ops mei_hdcp_ops = {
>+static struct i915_hdcp_component_ops mei_hdcp_ops = {
> 	.owner = THIS_MODULE,
> 	.initiate_hdcp2_session = mei_initiate_hdcp2_session,
> 	.verify_receiver_cert_prepare_km =
>mei_verify_receiver_cert_prepare_km,
>@@ -708,20 +708,106 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
> 	.close_hdcp_session = mei_close_hdcp_session,  };
>
>+static int mei_component_master_bind(struct device *dev) {
>+	struct mei_cl_device *cldev = to_mei_cl_device(dev);
>+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
>+	int ret;
>+
>+	dev_info(dev, "%s\n", __func__);
>+	drv_data->comp_master->ops = &mei_hdcp_ops;
>+	drv_data->comp_master->mei_dev = dev;
>+	ret = component_bind_all(dev, drv_data->comp_master);
>+	if (ret < 0)
>+		return ret;
>+
>+	return 0;
>+}
>+
>+static void mei_component_master_unbind(struct device *dev) {
>+	struct mei_cl_device *cldev = to_mei_cl_device(dev);
>+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
>+
>+	dev_info(dev, "%s\n", __func__);
>+	component_unbind_all(dev, drv_data->comp_master); }
>+
>+static const struct component_master_ops mei_component_master_ops = {
>+	.bind = mei_component_master_bind,
>+	.unbind = mei_component_master_unbind, };
>+
>+static int mei_hdcp_component_match(struct device *dev, int subcomponent,
>+				    void *data)
>+{
>+	return !strcmp(dev->driver->name, "i915") &&
>+	       subcomponent == I915_COMPONENT_HDCP; }
>+
> static int mei_hdcp_probe(struct mei_cl_device *cldev,
> 			  const struct mei_cl_device_id *id)  {
>+	struct mei_hdcp_drv_data *drv_data;
> 	int ret;
>
> 	ret = mei_cldev_enable(cldev);
> 	if (ret < 0)
> 		dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
>
>+	drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
>+	if (!drv_data) {
>+		ret = -ENOMEM;
>+		goto drv_data_exit;
>+	}
>+
>+	drv_data->comp_master = kzalloc(sizeof(*drv_data->comp_master),
>+					GFP_KERNEL);
>+	if (!drv_data->comp_master) {
>+		ret = -ENOMEM;
>+		goto comp_master_exit;
>+	}
>+
>+	drv_data->master_match = NULL;
>+	component_match_add_typed(&cldev->dev, &drv_data->master_match,
>+				  mei_hdcp_component_match,
>+				  drv_data->comp_master);
>+	if (IS_ERR_OR_NULL(drv_data->master_match)) {
>+		ret = -ENOMEM;
>+		goto match_add_exit;
>+	}
>+
>+	mei_cldev_set_drvdata(cldev, drv_data);
>+	ret = component_master_add_with_match(&cldev->dev,
>+					      &mei_component_master_ops,
>+					      drv_data->master_match);
>+	if (ret < 0) {
>+		dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
>+		mei_cldev_set_drvdata(cldev, NULL);
>+		goto match_add_exit;
>+	}
>+
>+	return 0;
>+
>+match_add_exit:
>+	kfree(drv_data->comp_master);
>+comp_master_exit:
>+	kfree(drv_data);
>+drv_data_exit:
>+	mei_cldev_disable(cldev);
>+
> 	return ret;
> }
>
> static int mei_hdcp_remove(struct mei_cl_device *cldev)  {
>+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
>+
>+	component_master_del(&cldev->dev, &mei_component_master_ops);
>+	kfree(drv_data->comp_master);
>+	kfree(drv_data);
>+	mei_cldev_set_drvdata(cldev, NULL);
>+
> 	return mei_cldev_disable(cldev);
> }
>
>diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h
>b/drivers/misc/mei/hdcp/mei_hdcp.h
>index f831db3cbd54..5c872a57ba11 100644
>--- a/drivers/misc/mei/hdcp/mei_hdcp.h
>+++ b/drivers/misc/mei/hdcp/mei_hdcp.h
>@@ -363,6 +363,11 @@ struct wired_cmd_repeater_auth_stream_req_out {
> 	struct hdcp_port_id	port;
> } __packed;
>
>+struct mei_hdcp_drv_data {
>+	struct i915_hdcp_comp_master *comp_master;
>+	struct component_match *master_match;
>+};
>+
> enum mei_hdcp_ddi {
> 	MEI_DDI_INVALID_PORT = 0x0,
>
>--
>2.7.4
diff mbox series

Patch

diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index edfc70fb0617..be2ce12ca460 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -23,6 +23,7 @@ 
 #include <linux/slab.h>
 #include <linux/uuid.h>
 #include <linux/mei_cl_bus.h>
+#include <linux/component.h>
 #include <drm/drm_connector.h>
 #include <drm/i915_component.h>
 #include <drm/i915_mei_hdcp_interface.h>
@@ -692,8 +693,7 @@  mei_close_hdcp_session(struct device *dev, struct hdcp_port_data *data)
 	return 0;
 }
 
-static __attribute__((unused))
-struct i915_hdcp_component_ops mei_hdcp_ops = {
+static struct i915_hdcp_component_ops mei_hdcp_ops = {
 	.owner = THIS_MODULE,
 	.initiate_hdcp2_session = mei_initiate_hdcp2_session,
 	.verify_receiver_cert_prepare_km = mei_verify_receiver_cert_prepare_km,
@@ -708,20 +708,106 @@  struct i915_hdcp_component_ops mei_hdcp_ops = {
 	.close_hdcp_session = mei_close_hdcp_session,
 };
 
+static int mei_component_master_bind(struct device *dev)
+{
+	struct mei_cl_device *cldev = to_mei_cl_device(dev);
+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
+	int ret;
+
+	dev_info(dev, "%s\n", __func__);
+	drv_data->comp_master->ops = &mei_hdcp_ops;
+	drv_data->comp_master->mei_dev = dev;
+	ret = component_bind_all(dev, drv_data->comp_master);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void mei_component_master_unbind(struct device *dev)
+{
+	struct mei_cl_device *cldev = to_mei_cl_device(dev);
+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
+
+	dev_info(dev, "%s\n", __func__);
+	component_unbind_all(dev, drv_data->comp_master);
+}
+
+static const struct component_master_ops mei_component_master_ops = {
+	.bind = mei_component_master_bind,
+	.unbind = mei_component_master_unbind,
+};
+
+static int mei_hdcp_component_match(struct device *dev, int subcomponent,
+				    void *data)
+{
+	return !strcmp(dev->driver->name, "i915") &&
+	       subcomponent == I915_COMPONENT_HDCP;
+}
+
 static int mei_hdcp_probe(struct mei_cl_device *cldev,
 			  const struct mei_cl_device_id *id)
 {
+	struct mei_hdcp_drv_data *drv_data;
 	int ret;
 
 	ret = mei_cldev_enable(cldev);
 	if (ret < 0)
 		dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
 
+	drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
+	if (!drv_data) {
+		ret = -ENOMEM;
+		goto drv_data_exit;
+	}
+
+	drv_data->comp_master = kzalloc(sizeof(*drv_data->comp_master),
+					GFP_KERNEL);
+	if (!drv_data->comp_master) {
+		ret = -ENOMEM;
+		goto comp_master_exit;
+	}
+
+	drv_data->master_match = NULL;
+	component_match_add_typed(&cldev->dev, &drv_data->master_match,
+				  mei_hdcp_component_match,
+				  drv_data->comp_master);
+	if (IS_ERR_OR_NULL(drv_data->master_match)) {
+		ret = -ENOMEM;
+		goto match_add_exit;
+	}
+
+	mei_cldev_set_drvdata(cldev, drv_data);
+	ret = component_master_add_with_match(&cldev->dev,
+					      &mei_component_master_ops,
+					      drv_data->master_match);
+	if (ret < 0) {
+		dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
+		mei_cldev_set_drvdata(cldev, NULL);
+		goto match_add_exit;
+	}
+
+	return 0;
+
+match_add_exit:
+	kfree(drv_data->comp_master);
+comp_master_exit:
+	kfree(drv_data);
+drv_data_exit:
+	mei_cldev_disable(cldev);
+
 	return ret;
 }
 
 static int mei_hdcp_remove(struct mei_cl_device *cldev)
 {
+	struct mei_hdcp_drv_data *drv_data = mei_cldev_get_drvdata(cldev);
+
+	component_master_del(&cldev->dev, &mei_component_master_ops);
+	kfree(drv_data->comp_master);
+	kfree(drv_data);
+	mei_cldev_set_drvdata(cldev, NULL);
+
 	return mei_cldev_disable(cldev);
 }
 
diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h b/drivers/misc/mei/hdcp/mei_hdcp.h
index f831db3cbd54..5c872a57ba11 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.h
+++ b/drivers/misc/mei/hdcp/mei_hdcp.h
@@ -363,6 +363,11 @@  struct wired_cmd_repeater_auth_stream_req_out {
 	struct hdcp_port_id	port;
 } __packed;
 
+struct mei_hdcp_drv_data {
+	struct i915_hdcp_comp_master *comp_master;
+	struct component_match *master_match;
+};
+
 enum mei_hdcp_ddi {
 	MEI_DDI_INVALID_PORT = 0x0,