Re: [PATCH v9 35/39] misc/mei/hdcp: Component framework for I915 Interface

["C, Ramalingam" <ramalingam.c@xxxxxxxxx>]

On 12/19/2018 12:15 PM, C, Ramalingam wrote:

Tomas and Daniel,

>From the discussion on this thread, I infer following understanding:

• At present(v9) I915 wants to be hard binded to mei_hdcp device-driver binding status through components
• This means I915 driver load will get complete only when the mei_hdcp's device and driver are bound.
• if mei_hdcp device reset I915 will unregister itself from userspace, and wait for the mei_hdcp device-deriver rebinding.
• Could be due to FW error or any unexpected failures those are rare occurances.
  
• when mei_hdcp module is removed i915 will unregister itself.
• Becasue of this, Ideally I915 dont expect the device reset from mei for suspend and resume.
• At present Mei bus is designed as below:
• Device will disappear on FW failures, FW upgrade, suspend of the system etc.
• And when the errors are handled or on system resume mei device will reappear, hence binding with corresponding driver.
• Mei doesn't plan to avoid the device reset(disappearance and reappearance) for suspend and resume in near future.

Based on above understanding, I propose the below approach. Please correct or approve it.

• At present(v9) component_add from mei_hdcp indicates the mei_hdcp's device-driver binded state.
• Instead lets use component to indicate the mei_hdcp's module availability,
• by adding the component at module_init and removing it from module_exit.
• This way I915 will not be impacted due to the mei device reset at suspend.
• In such scenario I915 will have no idea about the device-driver bind status of mei_hdcp.
• So incase of device is not available, mei_hdcp is responsible to prune such calls with -EIO error.
• This approach avoid any future impact to I915, incase mei intended to support suspend and resume.
  

I am aware this is not the ideal solution we want. But I feel this is the best at present we could do for this I915-mei interface.

Best regards,
Ram

something like (un compiled code)

diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index b22a71e8c5d7..b5b57a883e3b 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -23,11 +23,15 @@
 #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 "mei_hdcp.h"

+struct i915_component_master *i915_master_comp;
+static bool mei_hdcp_component_registered;
+
 /**
  * mei_initiate_hdcp2_session() - Initiate a Wired HDCP2.2 Tx Session in ME FW
  * @dev: device corresponding to the mei_cl_device
@@ -691,8 +695,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,
@@ -707,20 +710,84 @@ struct i915_hdcp_component_ops mei_hdcp_ops = {
        .close_hdcp_session = mei_close_hdcp_session,
 };

+static int mei_hdcp_component_bind(struct device *mei_kdev,
+                                  struct device *i915_kdev, void *data)
+{
+       struct i915_component_master *master_comp = data;
+
+       dev_info(mei_kdev, "MEI HDCP comp bind\n");
+       WARN_ON(master_comp->hdcp_ops);
+       master_comp->hdcp_ops = &mei_hdcp_ops;
+       master_comp->mei_dev = mei_kdev;
+
+       i915_master_comp = master_comp;
+
+       return 0;
+}
+
+static void mei_hdcp_component_unbind(struct device *mei_kdev,
+                                     struct device *i915_kdev, void *data)
+{
+       struct i915_component_master *master_comp = data;
+
+       dev_info(mei_kdev, "MEI HDCP comp unbind\n");
+       master_comp->hdcp_ops = NULL;
+       master_comp->mei_dev = NULL;
+       i915_master_comp = NULL;
+}
+
+static const struct component_ops mei_hdcp_component_bind_ops = {
+       .bind   = mei_hdcp_component_bind,
+       .unbind = mei_hdcp_component_unbind,
+};
+
+static void mei_hdcp_component_init(struct device *dev)
+{
+       int ret;
+
+       if (mei_hdcp_component_registered && i915_master_comp) {
+               i915_master_comp->mei_dev = dev;
+               return;
+       }
+
+       dev_info(dev, "MEI HDCP comp init\n");
+       ret = component_add(dev, &mei_hdcp_component_bind_ops);
+       if (ret < 0) {
+               dev_err(dev, "Failed to add MEI HDCP comp (%d)\n", ret);
+               return;
+       }
+
+       mei_hdcp_component_registered = true;
+}
+
+static void mei_hdcp_component_cleanup(struct device *dev)
+{
+       if (!mei_hdcp_component_registered)
+               return;
+
+       dev_info(dev, "MEI HDCP comp cleanup\n");
+       component_del(dev, &mei_hdcp_component_bind_ops);
+       mei_hdcp_component_registered = false;
+}
+
 static int mei_hdcp_probe(struct mei_cl_device *cldev,
                          const struct mei_cl_device_id *id)
 {
        int ret;

        ret = mei_cldev_enable(cldev);
-       if (ret < 0)
+       if (ret < 0) {
                dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
+               return ret;
+       }
+       mei_hdcp_component_init(&cldev->dev);

-       return ret;
+       return 0;
 }

 static int mei_hdcp_remove(struct mei_cl_device *cldev)
 {
+       i915_master_comp->mei_dev = NULL;
        return mei_cldev_disable(cldev);
 }

@@ -741,7 +808,23 @@ static struct mei_cl_driver mei_hdcp_driver = {
        .remove         = mei_hdcp_remove,
 };

-module_mei_cl_driver(mei_hdcp_driver);
+static int __init mei_hdcp_init(void)
+{
+       int ret;
+
+       ret = mei_cldev_driver_register(mei_hdcp_driver);
+       if (ret)
+               return ret;
+}
+
+static void __exit mei_hdcp_exit(void)
+{
+       mei_hdcp_component_cleanup(&cldev->dev);
+       mei_cldev_driver_unregister(mei_hdcp_driver);
+}
+
+module_init(mei_hdcp_init);
+module_exit(mei_hdcp_exit);

 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("Dual BSD/GPL");
--
2.7.4


-Ram

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