<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 12/6/2018 3:53 PM, Daniel Vetter wrote: </div> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap="">On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Defining the mei-i915 interface functions and initialization of the interface. Signed-off-by: Ramalingam C <a class="moz-txt-link-rfc2396E" href="mailto:ramalingam.c@intel.com"><ramalingam.c@intel.com></a> Signed-off-by: Tomas Winkler <a class="moz-txt-link-rfc2396E" href="mailto:tomas.winkler@intel.com"><tomas.winkler@intel.com></a> --- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/intel_drv.h | 7 + drivers/gpu/drm/i915/intel_hdcp.c | 442 +++++++++++++++++++++++++++++++++++++- include/drm/i915_component.h | 71 ++++++ 4 files changed, 521 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f763b30f98d9..b68bc980b7cd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2015,6 +2015,8 @@ struct drm_i915_private { struct i915_pmu pmu; + struct i915_hdcp_component_master *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. diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 85a526598096..bde82f3ada85 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -29,6 +29,7 @@ #include <linux/i2c.h> #include <linux/hdmi.h> #include <linux/sched/clock.h> +#include <linux/mei_hdcp.h> #include <drm/i915_drm.h> #include "i915_drv.h" #include <drm/drm_crtc.h> @@ -379,6 +380,9 @@ struct intel_hdcp_shim { /* Detects panel's hdcp capability. This is optional for HDMI. */ int (*hdcp_capable)(struct intel_digital_port *intel_dig_port, bool *hdcp_capable); + + /* Detects the HDCP protocol(DP/HDMI) required on the port */ + enum mei_hdcp_wired_protocol (*hdcp_protocol)(void); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Looking ahead, this seems hardwired to constant return value? Or why do we need a function here?</pre> </blockquote> <pre>This is hardwired based on the connector type(DP/HDMI). Since we have the shim for hdcp's connector based work, I have added this function. Could have done this just with connector_type check, but in that way whole hdcp_shim can be done in that way. So going with the larger design here. </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> }; struct intel_hdcp { @@ -399,6 +403,9 @@ struct intel_hdcp { * content can flow only through a link protected by HDCP2.2. */ u8 content_type; + + /* mei interface related information */ + struct mei_hdcp_data mei_data; }; struct intel_connector { diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index 99dddb540958..760780f1105c 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -8,14 +8,20 @@ #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 TIME_FOR_ENCRYPT_STATUS_CHANGE 50 +#define GET_MEI_DDI_INDEX(p) ({ \ + typeof(p) __p = (p); \ + __p == PORT_A ? MEI_DDI_A : (enum mei_hdcp_ddi)__p;\ +}) static bool intel_hdcp_is_ksv_valid(u8 *ksv) @@ -833,6 +839,417 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port) !IS_CHERRYVIEW(dev_priv) && port < PORT_E); } +static __attribute__((unused)) int +hdcp2_prepare_ake_init(struct intel_connector *connector, + struct hdcp2_ake_init *ake_data) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + if (data->port == MEI_DDI_INVALID_PORT && connector->encoder) + data->port = GET_MEI_DDI_INDEX(connector->encoder->port); + + /* Clear ME FW instance for the port, just incase */ + comp->ops->close_hdcp_session(comp->dev, data); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Sounds like a bug somewhere if we need this? I'd put this code into the ->initiate_hdcp2_session, with a big WARN_ON if it's actually needed.</pre> </blockquote> <pre>Not really. Lets say you have progressed beyond the first step of HDCP2.2 auth along with ME FW. Now if authentication failed due to some reason, then the HDCP2.2 season created with ME FW for that port is not closed yet. So we need to call close_hdcp_session() explicitly on authentication failures. Session has to be closed before restarting the auth on that port with initialite_hdcp_session. If we are closing the hdcp session of the port on all auth errors, we dont need this just before start of the hdcp session. </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> "Just in case" papering over programming bugs of our own just makes debugging harder. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ + ret = comp->ops->initiate_hdcp2_session(comp->dev, + data, ake_data); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> We should set the port only after successfully initializing this.</pre> </blockquote> <pre>hdcp2_session is created for each port at ME FW. Hence we need the port initialized even before calling the initiate_hdcp2_session.</pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_verify_rx_cert_prepare_km(struct intel_connector *connector, + struct hdcp2_ake_send_cert *rx_cert, + bool *paired, + struct hdcp2_ake_no_stored_km *ek_pub_km, + size_t *msg_sz) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> These all look like programming mistakes that should result in a WARN_ON.</pre> </blockquote> <pre>We are using the comp->mutex for protecting the interface during the interface init, usage for mei communication and interface teardown. But what if mei interface teardown happens between mei communications? So when we try to access the mei interface after such possible tear down scenario, we are checking the integrity of the interface. Possible alternate solution is hold the comp->mutex across the authentication steps. But consequence is that mei module removal will be blocked for authentication duration and even if the mei_dev is removed, component unbind will be blocked due to this mutex dependency. </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->verify_receiver_cert_prepare_km(comp->dev, data, + rx_cert, paired, + ek_pub_km, msg_sz); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Error handling here seems a bit strange. You close the session, but don't reset the port. So next op will be totally unaware that things have blown up. Also no warning. If we want to close the session, then I think that should be a decision made higher up.</pre> </blockquote> <pre>This is needed as explained above. But as you have mentioned this can be moved to the end of the authentication on error scenario. I will work on that.</pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ mutex_unlock(&comp->mutex); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> With component do we still need this mutex stuff here? Exact same comments everywhere below. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ + return ret; +} + +static __attribute__((unused)) int +hdcp2_verify_hprime(struct intel_connector *connector, + struct hdcp2_ake_send_hprime *rx_hprime) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->verify_hprime(comp->dev, data, rx_hprime); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_store_pairing_info(struct intel_connector *connector, + struct hdcp2_ake_send_pairing_info *pairing_info) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->store_pairing_info(comp->dev, + data, pairing_info); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_prepare_lc_init(struct intel_connector *connector, + struct hdcp2_lc_init *lc_init) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->initiate_locality_check(comp->dev, + data, lc_init); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_verify_lprime(struct intel_connector *connector, + struct hdcp2_lc_send_lprime *rx_lprime) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->verify_lprime(comp->dev, data, rx_lprime); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) +int hdcp2_prepare_skey(struct intel_connector *connector, + struct hdcp2_ske_send_eks *ske_data) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->get_session_key(comp->dev, data, ske_data); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_verify_rep_topology_prepare_ack(struct intel_connector *connector, + struct hdcp2_rep_send_receiverid_list + *rep_topology, + struct hdcp2_rep_send_ack *rep_send_ack) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->repeater_check_flow_prepare_ack(comp->dev, + data, rep_topology, + rep_send_ack); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) int +hdcp2_verify_mprime(struct intel_connector *connector, + struct hdcp2_rep_stream_ready *stream_ready) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->verify_mprime(comp->dev, data, stream_ready); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) +int hdcp2_authenticate_port(struct intel_connector *connector) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->enable_hdcp_authentication(comp->dev, data); + if (ret < 0) + comp->ops->close_hdcp_session(comp->dev, data); + mutex_unlock(&comp->mutex); + + return ret; +} + +static __attribute__((unused)) +int hdcp2_close_mei_session(struct intel_connector *connector) +{ + struct mei_hdcp_data *data = &connector->hdcp.mei_data; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp; + int ret; + + if (!comp) + return -EINVAL; + + mutex_lock(&comp->mutex); + if (!comp->ops || !comp->dev || + data->port == MEI_DDI_INVALID_PORT) { + mutex_unlock(&comp->mutex); + return -EINVAL; + } + + ret = comp->ops->close_hdcp_session(comp->dev, data); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Need to reset the port here I think.</pre> </blockquote> <pre>What is the reset of the port you are referring to? port is not configured for encryption. And this is the call for marking the port as de-authenticated too. </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ mutex_unlock(&comp->mutex); + return ret; +} + +static __attribute__((unused)) +int hdcp2_deauthenticate_port(struct intel_connector *connector) +{ + return hdcp2_close_mei_session(connector); +} + +static int i915_hdcp_component_master_bind(struct device *dev) +{ + struct drm_i915_private *dev_priv = kdev_to_i915(dev); + + return component_bind_all(dev, dev_priv->hdcp_comp); +} + +static void intel_connectors_hdcp_disable(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = &dev_priv->drm; + struct intel_connector *intel_connector; + struct drm_connector *connector; + struct drm_connector_list_iter conn_iter; + + 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.shim)) + continue; + + intel_hdcp_disable(intel_connector); + } + drm_connector_list_iter_end(&conn_iter); +} + +static void i915_hdcp_component_master_unbind(struct device *dev) +{ + struct drm_i915_private *dev_priv = kdev_to_i915(dev); + + intel_connectors_hdcp_disable(dev_priv); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Why this code? Once we've unregistered the driver, we should have shut down the hardware completely. Which should have shut down all the hdcp users too.</pre> </blockquote> <pre>This unbind might be triggered either due to master_del or component_del. if its triggered from mei through component_del, then before teardown of the i/f in component_unbind(), disable the ongoing HDCP session through hdcp_disable() for each connectors. </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ component_unbind_all(dev, dev_priv->hdcp_comp); +} + +static const struct component_master_ops i915_hdcp_component_master_ops = { + .bind = i915_hdcp_component_master_bind, + .unbind = i915_hdcp_component_master_unbind, +}; + +static int i915_hdcp_component_match(struct device *dev, void *data) +{ + return !strcmp(dev->driver->name, "mei_hdcp"); +} + +static int intel_hdcp_component_init(struct intel_connector *connector) +{ + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct i915_hdcp_component_master *comp; + struct component_match *match = NULL; + int ret; + + comp = kzalloc(sizeof(*comp), GFP_KERNEL); + if (!comp) + return -ENOMEM; + + mutex_init(&comp->mutex); + dev_priv->hdcp_comp = comp; + + component_match_add(dev_priv->drm.dev, &match, + i915_hdcp_component_match, dev_priv); + ret = component_master_add_with_match(dev_priv->drm.dev, + &i915_hdcp_component_master_ops, + match); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> So I'm not sure this will work out well, hiding the master_ops here in hdcp. Definitely needs to be rewritten as soon as i915 needs another component. And might make the load/unload split complicated.</pre> </blockquote> we have already discussed this. <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ if (ret < 0) + goto out_err; + + DRM_INFO("I915 hdcp component master added.\n"); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> You add both the master and the hdcp component here. Output is a bit confusing.</pre> </blockquote> we have component master and a component from mei which will match to the master. Here we are adding the component master. <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ 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 = MEI_HDCP_PORT_TYPE_INTEGRATED; + data->protocol = hdcp->shim->hdcp_protocol(); + + data->k = 1; + if (!data->streams) + data->streams = kcalloc(data->k, sizeof(data->streams[0]), + 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) || @@ -843,10 +1260,25 @@ static void 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)); - /* TODO: MEI interface needs to be initialized here */ + ret = initialize_mei_hdcp_data(connector); + if (ret) { + DRM_DEBUG_KMS("Mei hdcp data init failed\n"); + return; + } + + if (!dev_priv->hdcp_comp) + ret = intel_hdcp_component_init(connector); + + if (ret) { + DRM_DEBUG_KMS("HDCP comp init failed\n"); + kfree(hdcp->mei_data.streams); + return; + } + hdcp->hdcp2_supported = 1; } @@ -894,9 +1326,17 @@ void intel_hdcp_exit(struct drm_i915_private *dev_priv) mutex_lock(&intel_connector->hdcp.mutex); intel_connector->hdcp.hdcp2_supported = 0; intel_connector->hdcp.shim = NULL; + kfree(intel_connector->hdcp.mei_data.streams); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> We need to push this into a per-connector hdcp cleanup function. Or just into the generic connector cleanup.</pre> </blockquote> We could move it to per connector hdcp cleanup function. <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> mutex_unlock(&intel_connector->hdcp.mutex); } drm_connector_list_iter_end(&conn_iter); + + if (dev_priv->hdcp_comp) { + component_master_del(dev_priv->drm.dev, + &i915_hdcp_component_master_ops); + kfree(dev_priv->hdcp_comp); + dev_priv->hdcp_comp = NULL; + } } int intel_hdcp_enable(struct intel_connector *connector) diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h index fca22d463e1b..12268228f4dc 100644 --- a/include/drm/i915_component.h +++ b/include/drm/i915_component.h @@ -24,8 +24,12 @@ #ifndef _I915_COMPONENT_H_ #define _I915_COMPONENT_H_ +#include <linux/mutex.h> + #include "drm_audio_component.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 */ @@ -46,4 +50,71 @@ struct i915_audio_component { int aud_sample_rate[MAX_PORTS]; }; +struct i915_hdcp_component_ops { </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Imo that should be called mei_hdcp_component_ops and put into the linux/mei_hdcp.h header. Or was that Thomas' review comment?</pre> </blockquote> <pre>Nope this is there for many versions. i915_hdcp_component_ops are implemented by mei_hdcp.c and initializes the component with the &ops.</pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> Aside: Review here in public channels instead of in private would be much better for coordination.</pre> </blockquote> <pre>Tomas, </pre> <pre>Could you please help on this ask.?</pre> <pre>--Ram </pre> <blockquote type="cite" cite="mid:20181206102303.GO21184@phenom.ffwll.local"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ /** + * @owner: mei_hdcp module + */ + struct module *owner; + + int (*initiate_hdcp2_session)(struct device *dev, + void *hdcp_data, + struct hdcp2_ake_init *ake_data); + int (*verify_receiver_cert_prepare_km)(struct device *dev, + void *hdcp_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 device *dev, + void *hdcp_data, + struct hdcp2_ake_send_hprime *rx_hprime); + int (*store_pairing_info)(struct device *dev, + void *hdcp_data, + struct hdcp2_ake_send_pairing_info + *pairing_info); + int (*initiate_locality_check)(struct device *dev, + void *hdcp_data, + struct hdcp2_lc_init *lc_init_data); + int (*verify_lprime)(struct device *dev, + void *hdcp_data, + struct hdcp2_lc_send_lprime *rx_lprime); + int (*get_session_key)(struct device *dev, + void *hdcp_data, + struct hdcp2_ske_send_eks *ske_data); + int (*repeater_check_flow_prepare_ack)(struct device *dev, + void *hdcp_data, + struct hdcp2_rep_send_receiverid_list + *rep_topology, + struct hdcp2_rep_send_ack + *rep_send_ack); + int (*verify_mprime)(struct device *dev, + void *hdcp_data, + struct hdcp2_rep_stream_ready *stream_ready); + int (*enable_hdcp_authentication)(struct device *dev, + void *hdcp_data); + int (*close_hdcp_session)(struct device *dev, + void *hdcp_data); +}; + +/** + * struct i915_hdcp_component_master - Used for communication between i915 + * and mei_hdcp for HDCP2.2 services. + */ +struct i915_hdcp_component_master { + /** + * @dev: a device providing hdcp + */ + struct device *dev; + /** + * @mutex: Mutex to protect the state of dev + */ + struct mutex mutex; + /** + * @ops: Ops implemented by mei_hdcp driver, used by i915 driver. + */ + const struct i915_hdcp_component_ops *ops; +}; + #endif /* _I915_COMPONENT_H_ */ -- 2.7.4 </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> </pre> </blockquote> </body> </html>