[Intel-gfx] [PATCH v8 05/35] drm/i915: MEI interface definition
C, Ramalingam
ramalingam.c at intel.com
Fri Dec 7 10:48:25 UTC 2018
On 12/7/2018 11:22 AM, C, Ramalingam wrote:
>
>
> On 12/6/2018 3:53 PM, Daniel Vetter wrote:
>> On Tue, Nov 27, 2018 at 04:13:03PM +0530, Ramalingam C wrote:
>>> Defining the mei-i915 interface functions and initialization of
>>> the interface.
>>>
>>> Signed-off-by: Ramalingam C<ramalingam.c at intel.com>
>>> Signed-off-by: Tomas Winkler<tomas.winkler at intel.com>
>>> ---
>>> 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);
>> Looking ahead, this seems hardwired to constant return value? Or why do we
>> need a function here?
> 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.
>>> };
>>>
>>> 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);
>> 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.
> 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.
>> "Just in case" papering over programming bugs of our own just makes
>> debugging harder.
>>
>>> +
>>> + ret = comp->ops->initiate_hdcp2_session(comp->dev,
>>> + data, ake_data);
>> We should set the port only after successfully initializing this.
> hdcp2_session is created for each port at ME FW. Hence we need the port
> initialized even before calling the initiate_hdcp2_session.
>>> + 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) {
>> These all look like programming mistakes that should result in a WARN_ON.
> 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.
>>> + 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);
>> 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.
> 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.
>>> + mutex_unlock(&comp->mutex);
>> With component do we still need this mutex stuff here?
>>
>> Exact same comments everywhere below.
>>
>>> +
>>> + 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);
>> Need to reset the port here I think.
> 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.
>>> + 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);
>> 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.
> 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.
>>> + 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);
>> 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.
> we have already discussed this.
>>> + if (ret < 0)
>>> + goto out_err;
>>> +
>>> + DRM_INFO("I915 hdcp component master added.\n");
>> You add both the master and the hdcp component here. Output is a bit
>> confusing.
>
> we have component master and a component from mei which will match to
> the master.
>
> Here we are adding the component master.
>
>>> + 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);
>> We need to push this into a per-connector hdcp cleanup function. Or just
>> into the generic connector cleanup.
> We could move it to per connector hdcp cleanup function.
>>> 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 {
>> Imo that should be called mei_hdcp_component_ops and put into the
>> linux/mei_hdcp.h header. Or was that Thomas' review comment?
> Nope this is there for many versions. i915_hdcp_component_ops are
> implemented by mei_hdcp.c and initializes the component with the &ops.
Tomas and Daniel,
Is this fine? Defining the ops at i915_component.h and I915 and mei_hdcp
using it for their purpose?
If it has to be compulsorily defined by the service provider then we need
to move this into the include/linux/mei_hdcp.h. In that we can avoid the
global header from the mei_hdcp Tomas. Please help here to clarify the direction.
-Ram
>> Aside: Review here in public channels instead of in private would be much
>> better for coordination.
> Tomas,
> Could you please help on this ask.?
> --Ram
>>> + /**
>>> + * @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
>>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20181207/bcd1f3e6/attachment-0001.html>
More information about the dri-devel
mailing list