RE: [PATCH v3 19/40] drm/i915: wrapping all hdcp var into intel_hdcp

["Shankar, Uma" <uma.shankar@intel.com>]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Ramalingam C
>Sent: Tuesday, April 3, 2018 7:28 PM
>To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>seanpaul@chromium.org; daniel@ffwll.ch; chris@chris-wilson.co.uk;
>jani.nikula@linux.intel.com; Winkler, Tomas <tomas.winkler@intel.com>;
>Usyskin, Alexander <alexander.usyskin@intel.com>
>Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>Subject: [PATCH v3 19/40] drm/i915: wrapping all hdcp var into intel_hdcp
>
>Considering the upcoming significant no HDCP2.2 variables, it will be clean to

Rephrase this sentence.

>have separate struct fo HDCP.

Typo here.

>
>New structure called intel_hdcp is introduced.
>
>v2:
>  struct hdcp statically allocated. [Sean Paul]
>  enable and disable function parameters are retained.[Sean Paul]
>v3:
>  No Changes.
>
>Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>---
> drivers/gpu/drm/i915/intel_display.c |  7 +--
> drivers/gpu/drm/i915/intel_drv.h     | 14 ++++--
> drivers/gpu/drm/i915/intel_hdcp.c    | 94 ++++++++++++++++++++----------------
> 3 files changed, 66 insertions(+), 49 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c
>b/drivers/gpu/drm/i915/intel_display.c
>index 331084082545..2d7c47135e1a 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -15395,9 +15395,10 @@ static void intel_hpd_poll_fini(struct drm_device
>*dev)
> 	for_each_intel_connector_iter(connector, &conn_iter) {
> 		if (connector->modeset_retry_work.func)
> 			cancel_work_sync(&connector->modeset_retry_work);
>-		if (connector->hdcp_shim) {
>-			cancel_delayed_work_sync(&connector-
>>hdcp_check_work);
>-			cancel_work_sync(&connector->hdcp_prop_work);
>+		if (connector->hdcp.hdcp_shim) {
>+			cancel_delayed_work_sync(
>+					&connector->hdcp.hdcp_check_work);
>+			cancel_work_sync(&connector->hdcp.hdcp_prop_work);
> 		}
> 	}
> 	drm_connector_list_iter_end(&conn_iter);
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index d4368589b355..fdffcb833cd2 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -377,6 +377,14 @@ struct intel_hdcp_shim {
> 			    bool *hdcp_capable);
> };
>
>+struct intel_hdcp {
>+	const struct intel_hdcp_shim *hdcp_shim;
>+	struct mutex hdcp_mutex;
>+	uint64_t hdcp_value; /* protected by hdcp_mutex */
>+	struct delayed_work hdcp_check_work;
>+	struct work_struct hdcp_prop_work;
>+};
>+
> struct intel_connector {
> 	struct drm_connector base;
> 	/*
>@@ -409,11 +417,7 @@ struct intel_connector {
> 	/* Work struct to schedule a uevent on link train failure */
> 	struct work_struct modeset_retry_work;
>
>-	const struct intel_hdcp_shim *hdcp_shim;
>-	struct mutex hdcp_mutex;
>-	uint64_t hdcp_value; /* protected by hdcp_mutex */
>-	struct delayed_work hdcp_check_work;
>-	struct work_struct hdcp_prop_work;
>+	struct intel_hdcp hdcp;
> };
>
> struct intel_digital_connector_state {
>diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
>b/drivers/gpu/drm/i915/intel_hdcp.c
>index 14ca5d3057a7..1cca4f349064 100644
>--- a/drivers/gpu/drm/i915/intel_hdcp.c
>+++ b/drivers/gpu/drm/i915/intel_hdcp.c
>@@ -547,6 +547,7 @@ struct intel_digital_port *conn_to_dig_port(struct
>intel_connector *connector)
>
> static int _intel_hdcp_disable(struct intel_connector *connector)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> 	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> 	enum port port = intel_dig_port->base.port; @@ -562,7 +563,7 @@
>static int _intel_hdcp_disable(struct intel_connector *connector)
> 		return -ETIMEDOUT;
> 	}
>
>-	ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);
>+	ret = hdcp->hdcp_shim->toggle_signalling(intel_dig_port, false);
> 	if (ret) {
> 		DRM_ERROR("Failed to disable HDCP signalling\n");
> 		return ret;
>@@ -574,6 +575,7 @@ static int _intel_hdcp_disable(struct intel_connector
>*connector)
>
> static int _intel_hdcp_enable(struct intel_connector *connector)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> 	int i, ret, tries = 3;
>
>@@ -599,7 +601,7 @@ static int _intel_hdcp_enable(struct intel_connector
>*connector)
> 	/* Incase of authentication failures, HDCP spec expects reauth. */
> 	for (i = 0; i < tries; i++) {
> 		ret = intel_hdcp_auth(conn_to_dig_port(connector),
>-				      connector->hdcp_shim);
>+				      hdcp->hdcp_shim);
> 		if (!ret)
> 			return 0;
>
>@@ -615,36 +617,42 @@ static int _intel_hdcp_enable(struct intel_connector
>*connector)
>
> static void intel_hdcp_check_work(struct work_struct *work)  {
>-	struct intel_connector *connector =
>container_of(to_delayed_work(work),
>+	struct intel_hdcp *hdcp = container_of(to_delayed_work(work),
>+					       struct intel_hdcp,
>+					       hdcp_check_work);
>+	struct intel_connector *connector = container_of(hdcp,
> 							 struct intel_connector,
>-							 hdcp_check_work);
>+							 hdcp);
>+
> 	if (!intel_hdcp_check_link(connector))
>-		schedule_delayed_work(&connector->hdcp_check_work,
>+		schedule_delayed_work(&hdcp->hdcp_check_work,
> 				      DRM_HDCP_CHECK_PERIOD_MS);
> }
>
> static void intel_hdcp_prop_work(struct work_struct *work)  {
>-	struct intel_connector *connector = container_of(work,
>+	struct intel_hdcp *hdcp = container_of(work, struct intel_hdcp,
>+					       hdcp_prop_work);
>+	struct intel_connector *connector = container_of(hdcp,
> 							 struct intel_connector,
>-							 hdcp_prop_work);
>+							 hdcp);
> 	struct drm_device *dev = connector->base.dev;
> 	struct drm_connector_state *state;
>
> 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>-	mutex_lock(&connector->hdcp_mutex);
>+	mutex_lock(&hdcp->hdcp_mutex);
>
> 	/*
> 	 * This worker is only used to flip between ENABLED/DESIRED. Either of
> 	 * those to UNDESIRED is handled by core. If hdcp_value == UNDESIRED,
> 	 * we're running just after hdcp has been disabled, so just exit
> 	 */
>-	if (connector->hdcp_value !=
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>+	if (hdcp->hdcp_value !=
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> 		state = connector->base.state;
>-		state->content_protection = connector->hdcp_value;
>+		state->content_protection = hdcp->hdcp_value;
> 	}
>
>-	mutex_unlock(&connector->hdcp_mutex);
>+	mutex_unlock(&hdcp->hdcp_mutex);
> 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> }
>
>@@ -658,6 +666,7 @@ bool is_hdcp_supported(struct drm_i915_private
>*dev_priv, enum port port)  int intel_hdcp_init(struct intel_connector *connector,
> 		    const struct intel_hdcp_shim *hdcp_shim)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	int ret;
>
> 	ret = drm_connector_attach_content_protection_property(
>@@ -665,51 +674,53 @@ int intel_hdcp_init(struct intel_connector *connector,
> 	if (ret)
> 		return ret;
>
>-	connector->hdcp_shim = hdcp_shim;
>-	mutex_init(&connector->hdcp_mutex);
>-	INIT_DELAYED_WORK(&connector->hdcp_check_work,
>intel_hdcp_check_work);
>-	INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);
>+	hdcp->hdcp_shim = hdcp_shim;
>+	mutex_init(&hdcp->hdcp_mutex);
>+	INIT_DELAYED_WORK(&hdcp->hdcp_check_work,
>intel_hdcp_check_work);
>+	INIT_WORK(&hdcp->hdcp_prop_work, intel_hdcp_prop_work);
> 	return 0;
> }
>
> int intel_hdcp_enable(struct intel_connector *connector)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	int ret;
>
>-	if (!connector->hdcp_shim)
>+	if (!hdcp->hdcp_shim)
> 		return -ENOENT;
>
>-	mutex_lock(&connector->hdcp_mutex);
>+	mutex_lock(&hdcp->hdcp_mutex);
>
> 	ret = _intel_hdcp_enable(connector);
> 	if (ret)
> 		goto out;
>
>-	connector->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_ENABLED;
>-	schedule_work(&connector->hdcp_prop_work);
>-	schedule_delayed_work(&connector->hdcp_check_work,
>+	hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
>+	schedule_work(&hdcp->hdcp_prop_work);
>+	schedule_delayed_work(&hdcp->hdcp_check_work,
> 			      DRM_HDCP_CHECK_PERIOD_MS);
> out:
>-	mutex_unlock(&connector->hdcp_mutex);
>+	mutex_unlock(&hdcp->hdcp_mutex);
> 	return ret;
> }
>
> int intel_hdcp_disable(struct intel_connector *connector)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	int ret = 0;
>
>-	if (!connector->hdcp_shim)
>+	if (!hdcp->hdcp_shim)
> 		return -ENOENT;
>
>-	mutex_lock(&connector->hdcp_mutex);
>+	mutex_lock(&hdcp->hdcp_mutex);
>
>-	if (connector->hdcp_value !=
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>-		connector->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>+	if (hdcp->hdcp_value !=
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>+		hdcp->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> 		ret = _intel_hdcp_disable(connector);
> 	}
>
>-	mutex_unlock(&connector->hdcp_mutex);
>-	cancel_delayed_work_sync(&connector->hdcp_check_work);
>+	mutex_unlock(&hdcp->hdcp_mutex);
>+	cancel_delayed_work_sync(&hdcp->hdcp_check_work);
> 	return ret;
> }
>
>@@ -749,17 +760,18 @@ void intel_hdcp_atomic_check(struct drm_connector
>*connector,
> /* Implements Part 3 of the HDCP authorization procedure */  int
>intel_hdcp_check_link(struct intel_connector *connector)  {
>+	struct intel_hdcp *hdcp = &connector->hdcp;
> 	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> 	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> 	enum port port = intel_dig_port->base.port;
> 	int ret = 0;
>
>-	if (!connector->hdcp_shim)
>+	if (!hdcp->hdcp_shim)
> 		return -ENOENT;
>
>-	mutex_lock(&connector->hdcp_mutex);
>+	mutex_lock(&hdcp->hdcp_mutex);
>
>-	if (connector->hdcp_value ==
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>+	if (hdcp->hdcp_value ==
>DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> 		goto out;
>
> 	if (!(I915_READ(PORT_HDCP_STATUS(port)) & HDCP_STATUS_ENC)) {
>@@ -767,17 +779,17 @@ int intel_hdcp_check_link(struct intel_connector
>*connector)
> 			  connector->base.name, connector->base.base.id,
> 			  I915_READ(PORT_HDCP_STATUS(port)));
> 		ret = -ENXIO;
>-		connector->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>-		schedule_work(&connector->hdcp_prop_work);
>+		hdcp->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>+		schedule_work(&hdcp->hdcp_prop_work);
> 		goto out;
> 	}
>
>-	if (connector->hdcp_shim->check_link(intel_dig_port)) {
>-		if (connector->hdcp_value !=
>+	if (hdcp->hdcp_shim->check_link(intel_dig_port)) {
>+		if (hdcp->hdcp_value !=
> 		    DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>-			connector->hdcp_value =
>+			hdcp->hdcp_value =
>
>	DRM_MODE_CONTENT_PROTECTION_ENABLED;
>-			schedule_work(&connector->hdcp_prop_work);
>+			schedule_work(&hdcp->hdcp_prop_work);
> 		}
> 		goto out;
> 	}
>@@ -788,20 +800,20 @@ int intel_hdcp_check_link(struct intel_connector
>*connector)
> 	ret = _intel_hdcp_disable(connector);
> 	if (ret) {
> 		DRM_ERROR("Failed to disable hdcp (%d)\n", ret);
>-		connector->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>-		schedule_work(&connector->hdcp_prop_work);
>+		hdcp->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>+		schedule_work(&hdcp->hdcp_prop_work);
> 		goto out;
> 	}
>
> 	ret = _intel_hdcp_enable(connector);
> 	if (ret) {
> 		DRM_ERROR("Failed to enable hdcp (%d)\n", ret);
>-		connector->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>-		schedule_work(&connector->hdcp_prop_work);
>+		hdcp->hdcp_value =
>DRM_MODE_CONTENT_PROTECTION_DESIRED;
>+		schedule_work(&hdcp->hdcp_prop_work);
> 		goto out;
> 	}
>
> out:
>-	mutex_unlock(&connector->hdcp_mutex);
>+	mutex_unlock(&hdcp->hdcp_mutex);
> 	return ret;
> }
>--
>2.7.4
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel