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

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

[-- Attachment #1.1: Type: text/plain, Size: 6432 bytes --]


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
>       o This means I915 driver load will get complete only when the
>         mei_hdcp's device and driver are bound.
>       o 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.
>       o when mei_hdcp module is removed i915 will unregister itself.
>       o Becasue of this, Ideally I915 dont expect the device reset
>         from mei for suspend and resume.
>   * At present Mei bus is designed as below:
>       o Device will disappear on FW failures, FW upgrade, suspend of
>         the system etc.
>       o 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,
>       o 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.
>       o 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


[-- Attachment #1.2: Type: text/html, Size: 7525 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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