On Tue, Nov 27, 2018 at 04:13:04PM +0530, Ramalingam C wrote:
> Considering that HDCP2.2 is more secure than HDCP1.4, When a setup
> supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled.
>
> When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is
> enabled.
>
> This change implements a sequence of enabling and disabling of
> HDCP2.2 authentication and HDCP2.2 port encryption.
Patch series suggestion for next time around: First build out the helper
functions, then time them into the big picture like here. Personally I
think that makes it easier to understand, but it's kinda personal choice.
I guess this here works too.
>
> v2:
> Included few optimization suggestions [Chris Wilson]
> Commit message is updated as per the rebased version.
> intel_wait_for_register is used instead of wait_for. [Chris Wilson]
> v3:
> No changes.
> v4:
> Extra comment added and Style issue fixed [Uma]
> v5:
> Rebased as part of patch reordering.
> HDCP2 encryption status is tracked.
> HW state check is moved into WARN_ON [Daniel]
> v6:
> Redefined the mei service functions as per comp redesign.
> Merged patches related to hdcp2.2 enabling and disabling [Sean Paul].
> Required shim functionality is defined [Sean Paul]
> v7:
> Return values are handles [Uma]
> Realigned the code.
> Check for comp_master is removed.
> v8:
> HDCP2.2 is attempted only if mei interface is up.
> Adjust to the new interface
> Avoid bool usage in struct [Tomas]
>
> Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 5 +
> drivers/gpu/drm/i915/intel_hdcp.c | 223 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 214 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bde82f3ada85..3e9f21d23442 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -383,6 +383,10 @@ struct intel_hdcp_shim {
>
> /* Detects the HDCP protocol(DP/HDMI) required on the port */
> enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
> +
> + /* Detects whether Panel is HDCP2.2 capable */
> + int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port,
> + bool *capable);
> };
>
> struct intel_hdcp {
> @@ -396,6 +400,7 @@ struct intel_hdcp {
> /* HDCP2.2 related definitions */
> /* Flag indicates whether this connector supports HDCP2.2 or not. */
> u8 hdcp2_supported;
> + u8 hdcp2_in_use;
>
> /*
> * Content Stream Type defined by content owner. TYPE0(0x0) content can
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 760780f1105c..c1bd1ccd47cd 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -79,6 +79,43 @@ bool intel_hdcp_capable(struct intel_connector *connector)
> return capable;
> }
>
> +/* At present whether mei_hdcp component is binded with i915 master component */
> +static bool intel_hdcp2_mei_binded(struct intel_connector *connector)
> +{
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
> +
> + mutex_lock(&comp->mutex);
> + if (!comp->ops || !comp->dev) {
> + mutex_unlock(&comp->mutex);
> + return false;
> + }
> + mutex_unlock(&comp->mutex);
> +
> + return true;
> +}
> +
> +/* Is HDCP2.2 capable on Platform and Sink */
> +static bool intel_hdcp2_capable(struct intel_connector *connector)
> +{
> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> + struct intel_hdcp *hdcp = &connector->hdcp;
> + bool capable = false;
> +
> + /* I915 support for HDCP2.2 */
> + if (!hdcp->hdcp2_supported)
> + return false;
> +
> + /* MEI services for HDCP2.2 */
> + if (!intel_hdcp2_mei_binded(connector))
> + return false;
Why do we still need this with component? Driver load should be stalled
out until it's all there, that was kinda the entire point of component,
so we don't have to recheck all the time whether hdcp2 is still there or
not.
> +
> + /* Sink's capability for HDCP2.2 */
> + hdcp->shim->hdcp_2_2_capable(intel_dig_port, &capable);
> +
> + return capable;
> +}
> +
> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> const struct intel_hdcp_shim *shim)
> {
> @@ -1114,8 +1151,7 @@ int hdcp2_authenticate_port(struct intel_connector *connector)
> return ret;
> }
>
> -static __attribute__((unused))
> -int hdcp2_close_mei_session(struct intel_connector *connector)
> +static int hdcp2_close_mei_session(struct intel_connector *connector)
> {
> struct mei_hdcp_data *data = &connector->hdcp.mei_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -1137,12 +1173,157 @@ int hdcp2_close_mei_session(struct intel_connector *connector)
> return ret;
> }
>
> -static __attribute__((unused))
> -int hdcp2_deauthenticate_port(struct intel_connector *connector)
> +static int hdcp2_deauthenticate_port(struct intel_connector *connector)
> {
> return hdcp2_close_mei_session(connector);
> }
>
> +static int hdcp2_authenticate_sink(struct intel_connector *connector)
> +{
> + DRM_ERROR("Sink authentication is done in subsequent patches\n");
> +
> + return -EINVAL;
> +}
> +
> +static int hdcp2_enable_encryption(struct intel_connector *connector)
> +{
> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdcp *hdcp = &connector->hdcp;
> + enum port port = connector->encoder->port;
> + int ret;
> +
> + WARN_ON(I915_READ(HDCP2_STATUS_DDI(port)) & LINK_ENCRYPTION_STATUS);
> +
> + if (hdcp->shim->toggle_signalling) {
> + ret = hdcp->shim->toggle_signalling(intel_dig_port, true);
> + if (ret) {
> + DRM_ERROR("Failed to enable HDCP signalling. %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + if (I915_READ(HDCP2_STATUS_DDI(port)) & LINK_AUTH_STATUS) {
> + /* Link is Authenticated. Now set for Encryption */
> + I915_WRITE(HDCP2_CTL_DDI(port),
> + I915_READ(HDCP2_CTL_DDI(port)) |
> + CTL_LINK_ENCRYPTION_REQ);
> + }
> +
> + ret = intel_wait_for_register(dev_priv, HDCP2_STATUS_DDI(port),
> + LINK_ENCRYPTION_STATUS,
> + LINK_ENCRYPTION_STATUS,
> + TIME_FOR_ENCRYPT_STATUS_CHANGE);
> +
> + return ret;
> +}
> +
> +static int hdcp2_disable_encryption(struct intel_connector *connector)
> +{
> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_hdcp *hdcp = &connector->hdcp;
> + enum port port = connector->encoder->port;
> + int ret;
> +
> + WARN_ON(!(I915_READ(HDCP2_STATUS_DDI(port)) & LINK_ENCRYPTION_STATUS));
> +
> + I915_WRITE(HDCP2_CTL_DDI(port),
> + I915_READ(HDCP2_CTL_DDI(port)) & ~CTL_LINK_ENCRYPTION_REQ);
> +
> + ret = intel_wait_for_register(dev_priv, HDCP2_STATUS_DDI(port),
> + LINK_ENCRYPTION_STATUS, 0x0,
> + TIME_FOR_ENCRYPT_STATUS_CHANGE);
> + if (ret == -ETIMEDOUT)
> + DRM_DEBUG_KMS("Disable Encryption Timedout");
> +
> + if (hdcp->shim->toggle_signalling) {
> + ret = hdcp->shim->toggle_signalling(intel_dig_port, false);
> + if (ret) {
> + DRM_ERROR("Failed to disable HDCP signalling. %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector)
> +{
> + int ret, i, tries = 3;
> +
> + for (i = 0; i < tries; i++) {
> + ret = hdcp2_authenticate_sink(connector);
> + if (!ret)
> + break;
> +
> + /* Clearing the mei hdcp session */
> + DRM_DEBUG_KMS("HDCP2.2 Auth %d of %d Failed.(%d)\n",
> + i + 1, tries, ret);
> + if (hdcp2_deauthenticate_port(connector) < 0)
> + DRM_DEBUG_KMS("Port deauth failed.\n");
> + }
> +
> + if (i != tries) {
> + /*
> + * Ensuring the required 200mSec min time interval between
> + * Session Key Exchange and encryption.
> + */
> + msleep(HDCP_2_2_DELAY_BEFORE_ENCRYPTION_EN);
> + ret = hdcp2_enable_encryption(connector);
> + if (ret < 0) {
> + DRM_DEBUG_KMS("Encryption Enable Failed.(%d)\n", ret);
> + if (hdcp2_deauthenticate_port(connector) < 0)
> + DRM_DEBUG_KMS("Port deauth failed.\n");
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int _intel_hdcp2_enable(struct intel_connector *connector)
> +{
> + struct intel_hdcp *hdcp = &connector->hdcp;
> + int ret;
> +
> + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being enabled. Type: %d\n",
> + connector->base.name, connector->base.base.id,
> + hdcp->content_type);
> +
> + ret = hdcp2_authenticate_and_encrypt(connector);
> + if (ret) {
> + DRM_DEBUG_KMS("HDCP2 Type%d Enabling Failed. (%d)\n",
> + hdcp->content_type, ret);
> + return ret;
> + }
> +
> + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is enabled. Type %d\n",
> + connector->base.name, connector->base.base.id,
> + hdcp->content_type);
> +
> + hdcp->hdcp2_in_use = true;
> + return 0;
> +}
> +
> +static int _intel_hdcp2_disable(struct intel_connector *connector)
> +{
> + int ret;
> +
> + DRM_DEBUG_KMS("[%s:%d] HDCP2.2 is being Disabled\n",
> + connector->base.name, connector->base.base.id);
> +
> + ret = hdcp2_disable_encryption(connector);
> +
> + if (hdcp2_deauthenticate_port(connector) < 0)
> + DRM_DEBUG_KMS("Port deauth failed.\n");
> +
> + connector->hdcp.hdcp2_in_use = false;
> +
> + return ret;
> +}
> +
> static int i915_hdcp_component_master_bind(struct device *dev)
> {
> struct drm_i915_private *dev_priv = kdev_to_i915(dev);
> @@ -1342,22 +1523,33 @@ void intel_hdcp_exit(struct drm_i915_private *dev_priv)
> int intel_hdcp_enable(struct intel_connector *connector)
> {
> struct intel_hdcp *hdcp = &connector->hdcp;
> - int ret;
> + int ret = -EINVAL;
>
> if (!hdcp->shim)
> return -ENOENT;
>
> mutex_lock(&hdcp->mutex);
>
> - ret = _intel_hdcp_enable(connector);
> - if (ret)
> - goto out;
> + /*
> + * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
> + * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> + */
> + if (intel_hdcp2_capable(connector))
> + ret = _intel_hdcp2_enable(connector);
> +
> + /* When HDCP2.2 fails, HDCP1.4 will be attempted */
> + if (ret && intel_hdcp_capable(connector)) {
> + ret = _intel_hdcp_enable(connector);
> + if (!ret)
> + schedule_delayed_work(&hdcp->check_work,
> + DRM_HDCP_CHECK_PERIOD_MS);
> + }
> +
> + if (!ret) {
> + hdcp->value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
> + schedule_work(&hdcp->prop_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(&hdcp->mutex);
> return ret;
> }
> @@ -1374,7 +1566,10 @@ int intel_hdcp_disable(struct intel_connector *connector)
>
> if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> - ret = _intel_hdcp_disable(connector);
> + if (hdcp->hdcp2_in_use)
> + ret = _intel_hdcp2_disable(connector);
> + else
> + ret = _intel_hdcp_disable(connector);
Looks reasonable. So one thing we could be doing is already internally
have a DRM_MODE_CONTENT_PROTECTION_ENABLED_TYPE1 for hdcp2. But the uapi
would only return _ENABLED to userspace. But for internal tracking this
would avoid the need for the separate hdcp2_in_use variable.
Probably good cleanup/prep patch for the type0/1 uapi extension, once we
get there.
Aside from the component intergration question seems all reasonable. But I
didn't yet review the hdcp2 flow, will do that once it's more complete in
later patches.
-Daniel
> }
>
> mutex_unlock(&hdcp->mutex);
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
