[v9,07/39] drm/i915: MEI interface definition

Submitted by Ramalingam C on Dec. 13, 2018, 4:01 a.m.

Details

Message ID 1544673701-6353-8-git-send-email-ramalingam.c@intel.com
State New
Headers show
Series "drm/i915: Implement HDCP2.2" ( rev: 12 11 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Ramalingam C Dec. 13, 2018, 4:01 a.m.
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]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 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(-)

Patch hide | download patch | download mbox

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;
+
+	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);
+		comp_match_added = true;
+	}
+
 	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.
 	 */

Comments

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@intel.com>
> ---
>  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
>
On 12/19/2018 7:30 PM, Daniel Vetter wrote:
> 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@intel.com>
>> ---
>>   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).

Oops. this is caused by me by calling the hdcp_init even before intel_connector_attach_encoder.

Will fix it.

>
>> +
>> +	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?
This function wont be called till Kconfig is defined in the mei_hdcp 
changes. But yes this chunk should be last patch of these series.
>
>> +		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).
Ok
>
> 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.

the check for CONFIG_INTEL_MEI_HDCP is already added into 
is_hdcp2_supported()

-Ram

>
> 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
>>
On Wed, Dec 19, 2018 at 08:45:41PM +0530, C, Ramalingam wrote:
> 
> On 12/19/2018 7:30 PM, Daniel Vetter wrote:
> > 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@intel.com>
> > > ---
> > >   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).
> 
> Oops. this is caused by me by calling the hdcp_init even before intel_connector_attach_encoder.
> 
> Will fix it.
> 
> > 
> > > +
> > > +	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?
> This function wont be called till Kconfig is defined in the mei_hdcp
> changes. But yes this chunk should be last patch of these series.
> > 
> > > +		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).
> Ok
> > 
> > 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.
> 
> the check for CONFIG_INTEL_MEI_HDCP is already added into
> is_hdcp2_supported()

Indeed, I overlooked that. Maybe highlight it a bit more with a separate

	if (!CONFIG_ENABLED(MEI_HDCP))
		return false;

so it stick out more in the previous patch. Currently it's a bit burried.

With that + the init ordering fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

There's no need to split the patch out anymore, since without the mei
patches you can't enable that Kconfig option and hence no bisect issues.

Cheers, Daniel


> 
> -Ram
> 
> > 
> > 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
> > >
On 12/19/2018 8:51 PM, Daniel Vetter wrote:
> Indeed, I overlooked that. Maybe highlight it a bit more with a separate
>
> 	if (!CONFIG_ENABLED(MEI_HDCP))
> 		return false;
>
> so it stick out more in the previous patch. Currently it's a bit burried.
>
> With that + the init ordering fixed:
>
> Reviewed-by: Daniel Vetter<daniel.vetter@ffwll.ch>

Sure. Thank you.

>
> There's no need to split the patch out anymore, since without the mei
> patches you can't enable that Kconfig option and hence no bisect issues.

Still Kconfig and makefile is added at 22nd patch but component support in mei_hdcp is added at 35th patch.
So even now this chunk needs to be kept after the component support addition.

-Ram

>
> Cheers, Daniel
On Thu, Dec 20, 2018 at 06:48:08PM +0530, C, Ramalingam wrote:
> 
> On 12/19/2018 8:51 PM, Daniel Vetter wrote:
> > Indeed, I overlooked that. Maybe highlight it a bit more with a separate
> > 
> > 	if (!CONFIG_ENABLED(MEI_HDCP))
> > 		return false;
> > 
> > so it stick out more in the previous patch. Currently it's a bit burried.
> > 
> > With that + the init ordering fixed:
> > 
> > Reviewed-by: Daniel Vetter<daniel.vetter@ffwll.ch>
> 
> Sure. Thank you.
> 
> > 
> > There's no need to split the patch out anymore, since without the mei
> > patches you can't enable that Kconfig option and hence no bisect issues.
> 
> Still Kconfig and makefile is added at 22nd patch but component support in mei_hdcp is added at 35th patch.
> So even now this chunk needs to be kept after the component support addition.

IS_ENABLED(CONFIG_FOO) will just always evaluate to false if the Kconfig
symbol isn't defined yet. Kconfig works by injection additional #define
contstants into the build. So current ordering is safe.
-Daniel