On Thu, Dec 13, 2018 at 09:31:09AM +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]
Yeah looks all good. Spotted some small stuff below still.
> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 5 +
> drivers/gpu/drm/i915/intel_hdcp.c | 248 +++++++++++++++++++++++++++++++++++++-
> include/drm/i915_component.h | 7 ++
> 3 files changed, 259 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index dd9371647a8c..191b6e0f086c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -39,6 +39,7 @@
> #include <drm/drm_dp_mst_helper.h>
> #include <drm/drm_rect.h>
> #include <drm/drm_atomic.h>
> +#include <drm/i915_mei_hdcp_interface.h>
> #include <media/cec-notifier.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);
> +
> + /* HDCP adaptation(DP/HDMI) required on the port */
> + enum hdcp_wired_protocol protocol;
> };
>
> struct intel_hdcp {
> @@ -399,6 +403,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 {
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 584d27f3c699..9405fce07b93 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -8,8 +8,10 @@
>
> #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"
> @@ -833,6 +835,232 @@ 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + /* During the connector init encoder might not be initialized */
> + if (data->port == PORT_NONE)
> + data->port = connector->encoder->port;
> +
> + ret = comp->hdcp_ops->initiate_hdcp2_session(comp->mei_dev,
> + data, ake_data);
> + if (ret)
> + DRM_DEBUG_KMS("Prepare_ake_init failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_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);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->verify_hprime(comp->mei_dev, data, rx_hprime);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Verify hprime failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->store_pairing_info(comp->mei_dev, data,
> + pairing_info);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Store pairing info failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->initiate_locality_check(comp->mei_dev, data,
> + lc_init);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Prepare lc_init failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->verify_lprime(comp->mei_dev, data, rx_lprime);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Verify L_Prime failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->get_session_key(comp->mei_dev, data, ske_data);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Get session key failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_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);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->verify_mprime(comp->mei_dev, data, stream_ready);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Verify mprime failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> + int ret;
> +
> + ret = comp->hdcp_ops->enable_hdcp_authentication(comp->mei_dev, data);
> + if (ret < 0)
> + DRM_DEBUG_KMS("Enable hdcp auth failed. %d\n", ret);
> +
> + 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_component_master *comp = dev_priv->comp_master;
> +
> + return comp->hdcp_ops->close_hdcp_session(comp->mei_dev,
> + &connector->hdcp.port_data);
> +}
> +
> +static __attribute__((unused))
> +int hdcp2_deauthenticate_port(struct intel_connector *connector)
> +{
> + return hdcp2_close_mei_session(connector);
> +}
> +
> +static int i915_hdcp_component_match(struct device *dev, void *data)
> +{
> + return !strcmp(dev->driver->name, "mei_hdcp");
> +}
> +
> +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 = PORT_NONE;
> + if (connector->encoder)
> + data->port = connector->encoder->port;
This and the code above in ake_init still look strange. I'm not really
following how we can end up with an intialization sequence where we do not
yet know the encoder and hence can't assign the port?
That still smells like a bug. I think we should be able to set this
uncondtionally (if not, I need to understand why).
> +
> + 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;
> +}
> +
> bool is_hdcp2_supported(struct drm_i915_private *dev_priv)
> {
> return ((INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> @@ -843,10 +1071,28 @@ 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;
> + static bool comp_match_added;
> + int ret;
>
> WARN_ON(!is_hdcp2_supported(dev_priv));
>
> - /* TODO: MEI interface needs to be initialized here */
> + ret = initialize_hdcp_port_data(connector);
> + if (ret) {
> + DRM_DEBUG_KMS("Mei hdcp data init failed\n");
> + return;
> + }
> +
> + /*
> + * Component for mei is common across the connector.
> + * Adding the match once is sufficient.
> + * TODO: Instead of static bool, do we need flag in dev_priv?.
> + */
> + if (!comp_match_added) {
> + component_match_add(dev_priv->drm.dev, &dev_priv->master_match,
> + i915_hdcp_component_match, dev_priv);
Patch series needs to be reordered: We can only do the component_match_add
once the mei component is merged. Maybe just split out this hunk into a
final patch?
> + comp_match_added = true;
This static here kinda works because there's only 1 gpu, but it's a bad
hack. Please move to drm_i915_private (next to the other component stuff).
Also, we need to only do this when CONFIG_MEI_HDCP is enabled, otherwise
hdcp2 init must fail. Maybe best to put that into the is_hdcp2_supported
helper for now. Once we get non-MEI hdcp2 (definitely needed for discrete
gpu) we can split that up into is_mei_hdcp2_support and
is_other_hdcp2_supported.
Otherwise lgtm I think.
-Daniel
> + }
> +
> hdcp->hdcp2_supported = 1;
> }
>
> diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> index 6f94ddd3f2c2..7a7201374cfe 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -24,6 +24,8 @@
> #ifndef _I915_COMPONENT_H_
> #define _I915_COMPONENT_H_
>
> +#include <drm/i915_mei_hdcp_interface.h>
> +
> #include "drm_audio_component.h"
>
> /* MAX_PORT is the number of port
> @@ -50,8 +52,13 @@ struct i915_audio_component {
> * struct i915_component_master - Used for communication between i915
> * and any other drivers for the services
> * of different feature.
> + * @mei_dev: device that provide the HDCP2.2 service from MEI Bus.
> + * @hdcp_ops: Ops implemented by mei_hdcp driver, used by i915 driver.
> */
> struct i915_component_master {
> + struct device *mei_dev;
> + const struct i915_hdcp_component_ops *hdcp_ops;
> +
> /*
> * Add here the interface details between I915 and interested modules.
> */
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
