[PATCH v11 08/42] drm/i915: MEI interface definition
C, Ramalingam
ramalingam.c at intel.com
Fri Feb 8 08:48:19 UTC 2019
On 2/8/2019 1:18 AM, Daniel Vetter wrote:
> On Thu, Feb 07, 2019 at 08:40:08PM +0100, Daniel Vetter wrote:
>> On Thu, Feb 07, 2019 at 02:33:57AM +0530, Ramalingam C wrote:
>>> Defining the mei-i915 interface functions and initialization of
>>> the interface.
>>>
>>> v2:
>>> Adjust to the new interface changes. [Tomas]
>>> Added further debug logs for the failures at MEI i/f.
>>> port in hdcp_port data is equipped to handle -ve values.
>>> v3:
>>> mei comp is matched for global i915 comp master. [Daniel]
>>> In hdcp_shim hdcp_protocol() is replaced with const variable. [Daniel]
>>> mei wrappers are adjusted as per the i/f change [Daniel]
>>> v4:
>>> port initialization is done only at hdcp2_init only [Danvet]
>>> v5:
>>> I915 registers a subcomponent to be matched with mei_hdcp [Daniel]
>>> v6:
>>> HDCP_disable for all connectors incase of comp_unbind.
>>> Tear down HDCP comp interface at i915_unload [Daniel]
>>>
>>> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_drv.c | 1 +
>>> drivers/gpu/drm/i915/i915_drv.h | 7 +
>>> drivers/gpu/drm/i915/intel_connector.c | 2 +
>>> drivers/gpu/drm/i915/intel_drv.h | 6 +
>>> drivers/gpu/drm/i915/intel_hdcp.c | 423 ++++++++++++++++++++++++++++++++-
>>> include/drm/i915_component.h | 3 +
>>> 6 files changed, 441 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>>> index 7de90701f6f1..9c4218b5d153 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>> @@ -904,6 +904,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>>> mutex_init(&dev_priv->av_mutex);
>>> mutex_init(&dev_priv->wm.wm_mutex);
>>> mutex_init(&dev_priv->pps_mutex);
>>> + mutex_init(&dev_priv->hdcp_comp_mutex);
>>>
>>> i915_memcpy_init_early(dev_priv);
>>> intel_runtime_pm_init_early(dev_priv);
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index a2293152cb6a..e3d030b73b5b 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -55,6 +55,7 @@
>>> #include <drm/drm_util.h>
>>> #include <drm/drm_dsc.h>
>>> #include <drm/drm_connector.h>
>>> +#include <drm/i915_mei_hdcp_interface.h>
>>>
>>> #include "i915_fixed.h"
>>> #include "i915_params.h"
>>> @@ -2043,6 +2044,12 @@ struct drm_i915_private {
>>>
>>> struct i915_pmu pmu;
>>>
>>> + struct i915_hdcp_comp_master *hdcp_master;
>>> + bool hdcp_comp_added;
>>> +
>>> + /* Mutex to protect the above hdcp component related values. */
>>> + struct mutex hdcp_comp_mutex;
>>> +
>>> /*
>>> * 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_connector.c b/drivers/gpu/drm/i915/intel_connector.c
>>> index ee16758747c5..66ed3ee5998a 100644
>>> --- a/drivers/gpu/drm/i915/intel_connector.c
>>> +++ b/drivers/gpu/drm/i915/intel_connector.c
>>> @@ -88,6 +88,8 @@ void intel_connector_destroy(struct drm_connector *connector)
>>>
>>> kfree(intel_connector->detect_edid);
>>>
>>> + intel_hdcp_cleanup(intel_connector);
>>> +
>>> if (!IS_ERR_OR_NULL(intel_connector->edid))
>>> kfree(intel_connector->edid);
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index 030d962697dd..b5c54c688256 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -41,6 +41,7 @@
>>> #include <drm/drm_rect.h>
>>> #include <drm/drm_vblank.h>
>>> #include <drm/drm_atomic.h>
>>> +#include <drm/i915_mei_hdcp_interface.h>
>>> #include <media/cec-notifier.h>
>>>
>>> struct drm_printer;
>>> @@ -385,6 +386,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);
>>> +
>>> + /* HDCP adaptation(DP/HDMI) required on the port */
>>> + enum hdcp_wired_protocol protocol;
>>> };
>>>
>>> struct intel_hdcp {
>>> @@ -405,6 +409,7 @@ struct intel_hdcp {
>>> * content can flow only through a link protected by HDCP2.2.
>>> */
>>> u8 content_type;
>>> + struct hdcp_port_data port_data;
>>> };
>>>
>>> struct intel_connector {
>>> @@ -2070,6 +2075,7 @@ int intel_hdcp_disable(struct intel_connector *connector);
>>> int intel_hdcp_check_link(struct intel_connector *connector);
>>> bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
>>> bool intel_hdcp_capable(struct intel_connector *connector);
>>> +void intel_hdcp_cleanup(struct intel_connector *connector);
>>>
>>> /* intel_psr.c */
>>> #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
>>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
>>> index 7b1097d79fb8..79f8979b9150 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdcp.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
>>> @@ -7,8 +7,10 @@
>>> */
>>>
>>> #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"
>>> @@ -832,6 +834,360 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port)
>>> return INTEL_GEN(dev_priv) >= 9 && port < PORT_E;
>>> }
>>>
>>> +static __attribute__((unused)) int
>>> +hdcp2_prepare_ake_init(struct intel_connector *connector,
>>> + struct hdcp2_ake_init *ake_data)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->initiate_hdcp2_session(comp->mei_dev, data, ake_data);
>>> + if (ret)
>>> + DRM_DEBUG_KMS("Prepare_ake_init failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_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 hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->verify_receiver_cert_prepare_km(comp->mei_dev, data,
>>> + rx_cert, paired,
>>> + ek_pub_km, msg_sz);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Verify rx_cert failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused)) int
>>> +hdcp2_verify_hprime(struct intel_connector *connector,
>>> + struct hdcp2_ake_send_hprime *rx_hprime)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->verify_hprime(comp->mei_dev, data, rx_hprime);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Verify hprime failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_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 hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->store_pairing_info(comp->mei_dev, data, pairing_info);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Store pairing info failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused)) int
>>> +hdcp2_prepare_lc_init(struct intel_connector *connector,
>>> + struct hdcp2_lc_init *lc_init)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->initiate_locality_check(comp->mei_dev, data, lc_init);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Prepare lc_init failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused)) int
>>> +hdcp2_verify_lprime(struct intel_connector *connector,
>>> + struct hdcp2_lc_send_lprime *rx_lprime)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->verify_lprime(comp->mei_dev, data, rx_lprime);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Verify L_Prime failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused))
>>> +int hdcp2_prepare_skey(struct intel_connector *connector,
>>> + struct hdcp2_ske_send_eks *ske_data)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->get_session_key(comp->mei_dev, data, ske_data);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Get session key failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_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 hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->repeater_check_flow_prepare_ack(comp->mei_dev, data,
>>> + rep_topology,
>>> + rep_send_ack);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Verify rep topology failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused)) int
>>> +hdcp2_verify_mprime(struct intel_connector *connector,
>>> + struct hdcp2_rep_stream_ready *stream_ready)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->verify_mprime(comp->mei_dev, data, stream_ready);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Verify mprime failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused))
>>> +int hdcp2_authenticate_port(struct intel_connector *connector)
>>> +{
>>> + struct hdcp_port_data *data = &connector->hdcp.port_data;
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->enable_hdcp_authentication(comp->mei_dev, data);
>>> + if (ret < 0)
>>> + DRM_DEBUG_KMS("Enable hdcp auth failed. %d\n", ret);
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static __attribute__((unused))
>>> +int hdcp2_close_mei_session(struct intel_connector *connector)
>>> +{
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> + struct i915_hdcp_comp_master *comp;
>>> + int ret;
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + comp = dev_priv->hdcp_master;
>>> +
>>> + if (!comp || !comp->ops) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = comp->ops->close_hdcp_session(comp->mei_dev,
>>> + &connector->hdcp.port_data);
>>> + mutex_unlock(&dev_priv->hdcp_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_bind(struct device *i915_kdev,
>>> + struct device *mei_kdev, void *data)
>>> +{
>>> + struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>>> +
>>> + DRM_DEBUG("I915 HDCP comp bind\n");
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + dev_priv->hdcp_master = (struct i915_hdcp_comp_master *)data;
>>> + dev_priv->hdcp_master->mei_dev = mei_kdev;
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void i915_hdcp_component_unbind(struct device *i915_kdev,
>>> + struct device *mei_kdev, void *data)
>>> +{
>>> + struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>>> + struct drm_device *dev = &dev_priv->drm;
>>> + struct intel_connector *intel_connector;
>>> + struct drm_connector *connector;
>>> + struct drm_connector_list_iter conn_iter;
>>> +
>>> + DRM_DEBUG("I915 HDCP comp unbind\n");
>>> +
>>> + /* Once the component is unbind, Disable HDCP2.2 on all connectors */
>>> + drm_connector_list_iter_begin(dev, &conn_iter);
>>> + drm_for_each_connector_iter(connector, &conn_iter) {
>>> + intel_connector = to_intel_connector(connector);
>>> + intel_hdcp_disable(intel_connector);
>>> + }
>>> + drm_connector_list_iter_end(&conn_iter);
>> I played this through, and I think we have a bug here.
>>
>> 1. Userspace asks for hdcp, sets connector_state->content_protection to
>> DESIRED
>> 2. hdcp is enabled, userspace visible value is now ENABLED
>> 2. mei unloads
>> 3. we disable content protection here
>> 4. hdcp->value is set to UNDESIRED
>> 5. intel_hdcp_prop_work does _not_ update
>> connector_state->content_protection (this is correct, because otherwise
>> dpms off/on would break and we would not try to restore)
>> 6. Userspace still sees ENABLED, which is wrong
>>
>> I think the better solution is to not call intel_hdcp_disable here at all,
>> but instead wait for the link status check work to notice that ME is gone,
>> which will the correctly change status from ENABLED to DESIRED.
Daniel,
It is a embarrassing miss from my side.
But can be fixed using the __intel_hdcp(1/2)_disable and setting the
hdcp->val to desired and invoking the prop_work.
Perhaps, as you mentioned we could leave it to the link integrity check
also to do the same. But it will be need to kill it here.
-Ram
>>
>> So just the below code is needed I think
>>
>> Otherwise looks good to me, with the ebove loop removed:
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>
>>
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + dev_priv->hdcp_master = NULL;
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +}
>>> +
>>> +static const struct component_ops i915_hdcp_component_ops = {
>>> + .bind = i915_hdcp_component_bind,
>>> + .unbind = i915_hdcp_component_unbind,
>>> +};
>>> +
>>> +static int initialize_hdcp_port_data(struct intel_connector *connector)
>>> +{
>>> + struct intel_hdcp *hdcp = &connector->hdcp;
>>> + struct hdcp_port_data *data = &hdcp->port_data;
>>> +
>>> + data->port = connector->encoder->port;
>>> + data->port_type = (u8)HDCP_PORT_TYPE_INTEGRATED;
>>> + data->protocol = (u8)hdcp->shim->protocol;
>>> +
>>> + data->k = 1;
>>> + if (!data->streams)
>>> + data->streams = kcalloc(data->k,
>>> + sizeof(struct hdcp2_streamid_type),
>>> + 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;
>>> +}
>>> +
>>> static bool is_hdcp2_supported(struct drm_i915_private *dev_priv)
>>> {
>>> if (!IS_ENABLED(CONFIG_INTEL_MEI_HDCP))
>>> @@ -843,9 +1199,42 @@ static bool is_hdcp2_supported(struct drm_i915_private *dev_priv)
>>>
>>> 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;
>>> + bool add_component = false;
>>> + int ret;
>>> +
>>> + ret = initialize_hdcp_port_data(connector);
>>> + if (ret) {
>>> + DRM_DEBUG_KMS("Mei hdcp data init failed\n");
>>> + return;
>>> + }
>>> +
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + /*
>>> + * Component for mei is common across the connector.
>>> + * Adding the component once is sufficient.
>>> + */
>>> + if (!dev_priv->hdcp_comp_added) {
>>> + dev_priv->hdcp_comp_added = true;
>>> + add_component = true;
>>> + }
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> +
>>> + if (add_component) {
> This one (and the similar code in the destroy path) is a bit awkward for
> two reasons:
> - Because of dp mst hotplug/unplug you need a lock to make sure we
> register only once, usually setup & teardown code doesn't need any
> locking at all because it's all single-threaded.
> - ->hdcp_comp_added boils down to "have we loaded i915".
Added because of the place we are adding the component . i.e
connector_init which happens per connector. If we move this out of
connector_init we dont need this flag.
>
> I think it would be nice to have a separate intel_hdcp_mei_comp_init/fini,
> which is called only once (from mode_config setup/teardown functions
> perhaps) in the driver load/unload code, which would avoid both issues.
Sure I will move it out.
Thanks
-Ram.
>
> Because the code is already protected with the mutex against the mei hdcp
> driver appearing/disappearing any time it wants to this will not cause an
> additional problem. So would be nice to simplify.
>
> Cheers, Daniel
>
>>> + ret = component_add_typed(dev_priv->drm.dev,
>>> + &i915_hdcp_component_ops,
>>> + I915_COMPONENT_HDCP);
>>> + if (ret < 0) {
>>> + DRM_DEBUG_KMS("Failed at component add(%d)\n", ret);
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + dev_priv->hdcp_comp_added = false;
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + kfree(hdcp->port_data.streams);
>>> + return;
>>> + }
>>> + }
>>>
>>> - /* TODO: MEI interface needs to be initialized here */
>>> hdcp->hdcp2_supported = true;
>>> }
>>>
>>> @@ -917,6 +1306,38 @@ int intel_hdcp_disable(struct intel_connector *connector)
>>> return ret;
>>> }
>>>
>>> +void intel_hdcp_cleanup(struct intel_connector *connector)
>>> +{
>>> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>> +
>>> + if (!connector->hdcp.shim)
>>> + return;
>>> +
>>> + mutex_lock(&connector->hdcp.mutex);
>>> + if (!connector->hdcp.hdcp2_supported) {
>>> + mutex_unlock(&connector->hdcp.mutex);
>>> + return;
>>> + }
>>> +
>>> + kfree(connector->hdcp.port_data.streams);
>>> + mutex_lock(&dev_priv->hdcp_comp_mutex);
>>> + /*
>>> + * Component for mei is common across the connector.
>>> + * Removing the component once is sufficient.
>>> + */
>>> + if (!dev_priv->hdcp_comp_added) {
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + mutex_unlock(&connector->hdcp.mutex);
>>> + return;
>>> + }
>>> +
>>> + dev_priv->hdcp_comp_added = false;
>>> + mutex_unlock(&dev_priv->hdcp_comp_mutex);
>>> + mutex_unlock(&connector->hdcp.mutex);
>>> +
>>> + component_del(dev_priv->drm.dev, &i915_hdcp_component_ops);
>>> +}
>>> +
>>> void intel_hdcp_atomic_check(struct drm_connector *connector,
>>> struct drm_connector_state *old_state,
>>> struct drm_connector_state *new_state)
>>> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
>>> index 72fbb037f9b3..f3851ba3e4c3 100644
>>> --- a/include/drm/i915_component.h
>>> +++ b/include/drm/i915_component.h
>>> @@ -24,10 +24,13 @@
>>> #ifndef _I915_COMPONENT_H_
>>> #define _I915_COMPONENT_H_
>>>
>>> +#include <linux/device.h>
>>> +
>>> #include "drm_audio_component.h"
>>>
>>> enum i915_component_type {
>>> I915_COMPONENT_AUDIO = 1,
>>> + I915_COMPONENT_HDCP,
>>> };
>>>
>>> /* MAX_PORT is the number of port
>>> --
>>> 2.7.4
>>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
More information about the dri-devel
mailing list