RE: [PATCH v6 05/35] drm/i915: wrapping all hdcp var into intel_hdcp

-----Original Message----- From: C, Ramalingam Sent: Saturday, July 14, 2018 8:45 AM To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; daniel@ffwll.ch; seanpaul@chromium.org; Winkler, Tomas tomas.winkler@intel.com; Usyskin, Alexander alexander.usyskin@intel.com; Shankar, Uma uma.shankar@intel.com Cc: Sharma, Shashank shashank.sharma@intel.com; C, Ramalingam ramalingam.c@intel.com Subject: [PATCH v6 05/35] drm/i915: wrapping all hdcp var into intel_hdcp

Considering significant number of HDCP specific variables, it will be clean to have separate struct for HDCP.

New structure called intel_hdcp is added within intel_connector.

v2: struct hdcp statically allocated. [Sean Paul] enable and disable function parameters are retained.[Sean Paul] v3: No Changes. v4: Commit msg is rephrased [Uma] v5: Comment for mutex definition. v6: hdcp_ prefix from all intel_hdcp members are removed [Sean Paul] inline function intel_hdcp_to_connector is defined [Sean Paul]

Changes look ok to me. Reviewed-by: Uma Shankar uma.shankar@intel.com

Signed-off-by: Ramalingam C ramalingam.c@intel.com Reviewed-by: Sean Paul seanpaul@chromium.org

---

drivers/gpu/drm/i915/intel_display.c | 6 +- drivers/gpu/drm/i915/intel_drv.h | 15 +++-- drivers/gpu/drm/i915/intel_hdcp.c | 107 +++++++++++++++++++---------------- 3 files changed, 72 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8f3199b06d1f..b8106f0fd64b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15878,9 +15878,9 @@ 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.shim) {
  

• 	cancel_delayed_work_sync(&connector-
  

...

hdcp.check_work);

• 	cancel_work_sync(&connector->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 61e715ddd0d5..c32665136d5d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -378,6 +378,15 @@ struct intel_hdcp_shim { bool *hdcp_capable); };

+struct intel_hdcp {

• const struct intel_hdcp_shim *shim;
• /* Mutex for hdcp state of the connector */
• struct mutex mutex;
• uint64_t value;
• struct delayed_work check_work;
• struct work_struct prop_work;

+};

struct intel_connector { struct drm_connector base; /* @@ -410,11 +419,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 0cc6a861bcf8..55bc4d423187 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -626,6 +626,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; @@ -641,7 +642,7 @@

static int _intel_hdcp_disable(struct intel_connector *connector) return -ETIMEDOUT; }

• ret = connector->hdcp_shim->toggle_signalling(intel_dig_port, false);

• ret = hdcp->shim->toggle_signalling(intel_dig_port, false); if (ret) { DRM_ERROR("Failed to disable HDCP signalling\n"); return ret;

@@ -653,6 +654,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;

@@ -677,8 +679,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);
  

• ret = intel_hdcp_auth(conn_to_dig_port(connector), hdcp-
  

...

shim);

if (!ret)
	return 0;

@@ -692,38 +693,46 @@ static int _intel_hdcp_enable(struct intel_connector *connector) return ret; }

+static inline +struct intel_connector *intel_hdcp_to_connector(struct intel_hdcp +*hdcp) {

• return container_of(hdcp, struct intel_connector, hdcp); }

static void intel_hdcp_check_work(struct work_struct *work) {

• struct intel_connector *connector =

container_of(to_delayed_work(work),

• 					 struct intel_connector,
  

• 					 hdcp_check_work);
  

• struct intel_hdcp *hdcp = container_of(to_delayed_work(work),

• 			       struct intel_hdcp,
  

• 			       check_work);
  

• struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
• if (!intel_hdcp_check_link(connector))

• schedule_delayed_work(&connector->hdcp_check_work,
  

• schedule_delayed_work(&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_connector,
  

• 					 hdcp_prop_work);
  

• struct intel_hdcp *hdcp = container_of(work, struct intel_hdcp,

• 			       prop_work);
  

• struct intel_connector *connector = intel_hdcp_to_connector(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->mutex);

  /*

  • This worker is only used to flip between ENABLED/DESIRED. Either of

• * those to UNDESIRED is handled by core. If hdcp_value == UNDESIRED,
  

• * those to UNDESIRED is handled by core. If 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->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { state = connector->base.state;

• state->content_protection = connector->hdcp_value;
  

• state->content_protection = hdcp->value;
  

  }

• mutex_unlock(&connector->hdcp_mutex);

• mutex_unlock(&hdcp->mutex); drm_modeset_unlock(&dev->mode_config.connection_mutex);

}

@@ -735,8 +744,9 @@ 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)
  

•     const struct intel_hdcp_shim *shim)
  

{

• struct intel_hdcp *hdcp = &connector->hdcp; int ret;

  ret = drm_connector_attach_content_protection_property(

@@ -744,51 +754,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->shim = shim;
• mutex_init(&hdcp->mutex);
• INIT_DELAYED_WORK(&hdcp->check_work, intel_hdcp_check_work);
• INIT_WORK(&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->shim) return -ENOENT;

• mutex_lock(&connector->hdcp_mutex);

• mutex_lock(&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->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
• schedule_work(&hdcp->prop_work);
• schedule_delayed_work(&hdcp->check_work, DRM_HDCP_CHECK_PERIOD_MS);

out:

• mutex_unlock(&connector->hdcp_mutex);

• mutex_unlock(&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->shim) return -ENOENT;

• mutex_lock(&connector->hdcp_mutex);

• mutex_lock(&hdcp->mutex);

• if (connector->hdcp_value !=

DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {

• connector->hdcp_value =
  

DRM_MODE_CONTENT_PROTECTION_UNDESIRED;

• if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {

• 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->mutex);
• cancel_delayed_work_sync(&hdcp->check_work); return ret;

}

@@ -828,17 +840,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->shim) return -ENOENT;

• mutex_lock(&connector->hdcp_mutex);

• mutex_lock(&hdcp->mutex);

• if (connector->hdcp_value ==

DRM_MODE_CONTENT_PROTECTION_UNDESIRED)

• if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_UNDESIRED) goto out;

  if (!(I915_READ(PORT_HDCP_STATUS(port)) & HDCP_STATUS_ENC)) {

@@ -846,17 +859,15 @@ 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->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
  

• schedule_work(&hdcp->prop_work);
  

  goto out; }

• if (connector->hdcp_shim->check_link(intel_dig_port)) {

• if (connector->hdcp_value !=
  

•     DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
  

• 	connector->hdcp_value =
  

• DRM_MODE_CONTENT_PROTECTION_ENABLED;

• 	schedule_work(&connector->hdcp_prop_work);
  

• if (hdcp->shim->check_link(intel_dig_port)) {

• if (hdcp->value !=
  

DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {

• 	hdcp->value =
  

DRM_MODE_CONTENT_PROTECTION_ENABLED;

• 	schedule_work(&hdcp->prop_work);
  

  } goto out; }

@@ -867,20 +878,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->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
  

• schedule_work(&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->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
  

• schedule_work(&hdcp->prop_work);
  

  goto out; }

out:

• mutex_unlock(&connector->hdcp_mutex);

• mutex_unlock(&hdcp->mutex); return ret;

}

2.7.4