On 2020-04-29 at 15:54:54 -0400, Sean Paul wrote:
> From: Sean Paul <seanp...@chromium.org>
>
> This patch is required for HDCP over MST. If a port is being used for
> multiple HDCP streams, we don't want to fully disable HDCP on a port if
> one of them is disabled. Instead, we just disable the HDCP signalling on
> that particular pipe and exit early. The last pipe to disable HDCP will
> also bring down HDCP on the port.
>
> In order to achieve this, we need to keep a refcount in intel_digital_port
> and protect it using a new hdcp_mutex.
>
> Cc: Ramalingam C <ramalinga...@intel.com>
> Signed-off-by: Sean Paul <seanp...@chromium.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191203173638.94919-8-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191212190230.188505-9-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200117193103.156821-9-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200218220242.107265-9-s...@poorly.run
> #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200305201236.152307-9-s...@poorly.run
> #v5
>
> Changes in v2:
> -Move the toggle_signalling call into _intel_hdcp_disable so it's called from
> check_work
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -Change WARN_ON to drm_WARN_ON
> Changes in v6:
> -None
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 3 ++
> .../drm/i915/display/intel_display_types.h | 5 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +
> drivers/gpu/drm/i915/display/intel_hdcp.c | 53 +++++++++++++++----
> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +
> 5 files changed, 56 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index dc5d39ae4743..11155a8a73c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4801,6 +4801,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv,
> enum port port)
> drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs,
> DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
>
> + mutex_init(&intel_dig_port->hdcp_mutex);
> + intel_dig_port->num_hdcp_streams = 0;
> +
> encoder->hotplug = intel_ddi_hotplug;
> encoder->compute_output_type = intel_ddi_compute_output_type;
> encoder->compute_config = intel_ddi_compute_config;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 383046050c37..69edfab4e266 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1411,6 +1411,11 @@ struct intel_digital_port {
> enum phy_fia tc_phy_fia;
> u8 tc_phy_fia_idx;
>
> + /* protects num_hdcp_streams reference count */
> + struct mutex hdcp_mutex;
> + /* the number of pipes using HDCP signalling out of this port */
> + unsigned int num_hdcp_streams;
> +
> void (*write_infoframe)(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state,
> unsigned int type,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 39c1304fe071..a33ee8f30d14 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -8450,6 +8450,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> intel_encoder = &intel_dig_port->base;
> encoder = &intel_encoder->base;
>
> + mutex_init(&intel_dig_port->hdcp_mutex);
> +
> if (drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
> &intel_dp_enc_funcs, DRM_MODE_ENCODER_TMDS,
> "DP %c", port_name(port)))
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index a4446d47ef27..0c00bbc3f66e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -801,6 +801,19 @@ static int _intel_hdcp_disable(struct intel_connector
> *connector)
> drm_dbg_kms(&dev_priv->drm, "[%s:%d] HDCP is being disabled...\n",
> connector->base.name, connector->base.base.id);
>
> + /*
> + * If there are other connectors on this port using HDCP, don't disable
> + * it. Instead, toggle the HDCP signalling off on that particular
> + * connector/pipe and exit.
> + */
> + if (intel_dig_port->num_hdcp_streams > 0) {
> + ret = hdcp->shim->toggle_signalling(intel_dig_port,
> + cpu_transcoder, false);
> + if (ret)
> + DRM_ERROR("Failed to disable HDCP signalling\n");
> + return ret;
> + }
> +
> hdcp->hdcp_encrypted = false;
> intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0);
> if (intel_de_wait_for_clear(dev_priv,
> @@ -880,6 +893,8 @@ static struct intel_connector
> *intel_hdcp_to_connector(struct intel_hdcp *hdcp)
> static void intel_hdcp_update_value(struct intel_connector *connector,
> u64 value, bool update_property)
> {
> + struct drm_device *dev = connector->base.dev;
> + struct intel_digital_port *intel_dig_port =
> intel_attached_dig_port(connector);
> struct intel_hdcp *hdcp = &connector->hdcp;
>
> drm_WARN_ON(connector->base.dev, !mutex_is_locked(&hdcp->mutex));
> @@ -887,6 +902,15 @@ static void intel_hdcp_update_value(struct
> intel_connector *connector,
> if (hdcp->value == value)
> return;
>
> + drm_WARN_ON(dev, !mutex_is_locked(&intel_dig_port->hdcp_mutex));
> +
> + if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> + if (!drm_WARN_ON(dev, intel_dig_port->num_hdcp_streams == 0))
> + intel_dig_port->num_hdcp_streams--;
> + } else if (value == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> + intel_dig_port->num_hdcp_streams++;
> + }
> +
> hdcp->value = value;
> if (update_property) {
> drm_connector_get(&connector->base);
> @@ -905,6 +929,8 @@ static int intel_hdcp_check_link(struct intel_connector
> *connector)
> int ret = 0;
>
> mutex_lock(&hdcp->mutex);
> + mutex_lock(&intel_dig_port->hdcp_mutex);
> +
> cpu_transcoder = hdcp->cpu_transcoder;
>
> /* Check_link valid only when HDCP1.4 is enabled */
> @@ -958,6 +984,7 @@ static int intel_hdcp_check_link(struct intel_connector
> *connector)
> }
>
> out:
> + mutex_unlock(&intel_dig_port->hdcp_mutex);
> mutex_unlock(&hdcp->mutex);
> return ret;
> }
> @@ -2054,6 +2081,7 @@ int intel_hdcp_enable(struct intel_connector *connector,
> enum transcoder cpu_transcoder, u8 content_type)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_digital_port *intel_dig_port =
> intel_attached_dig_port(connector);
> struct intel_hdcp *hdcp = &connector->hdcp;
> unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
> int ret = -EINVAL;
> @@ -2062,6 +2090,7 @@ int intel_hdcp_enable(struct intel_connector *connector,
> return -ENOENT;
>
> mutex_lock(&hdcp->mutex);
> + mutex_lock(&intel_dig_port->hdcp_mutex);
> drm_WARN_ON(&dev_priv->drm,
> hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED);
> hdcp->content_type = content_type;
> @@ -2096,12 +2125,14 @@ int intel_hdcp_enable(struct intel_connector
> *connector,
> true);
> }
>
> + mutex_unlock(&intel_dig_port->hdcp_mutex);
> mutex_unlock(&hdcp->mutex);
> return ret;
> }
>
> int intel_hdcp_disable(struct intel_connector *connector)
> {
> + struct intel_digital_port *intel_dig_port =
> intel_attached_dig_port(connector);
> struct intel_hdcp *hdcp = &connector->hdcp;
> int ret = 0;
>
> @@ -2109,17 +2140,21 @@ int intel_hdcp_disable(struct intel_connector
> *connector)
> return -ENOENT;
>
> mutex_lock(&hdcp->mutex);
> + mutex_lock(&intel_dig_port->hdcp_mutex);
>
> - if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> - intel_hdcp_update_value(connector,
> - DRM_MODE_CONTENT_PROTECTION_UNDESIRED,
> - false);
> - if (hdcp->hdcp2_encrypted)
> - ret = _intel_hdcp2_disable(connector);
> - else if (hdcp->hdcp_encrypted)
> - ret = _intel_hdcp_disable(connector);
> - }
nice clean up. But unrelated change for this patch, separate patch will
be nice.
> + if (hdcp->value == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> + goto out;
> +
> + intel_hdcp_update_value(connector,
> + DRM_MODE_CONTENT_PROTECTION_UNDESIRED, false);
>
> + if (hdcp->hdcp2_encrypted)
> + ret = _intel_hdcp2_disable(connector);
> + else if (hdcp->hdcp_encrypted)
> + ret = _intel_hdcp_disable(connector);
> +
> +out:
> + mutex_unlock(&intel_dig_port->hdcp_mutex);
> mutex_unlock(&hdcp->mutex);
> cancel_delayed_work_sync(&hdcp->check_work);
> return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 5820a6e94273..93cd8a2a4be1 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3311,6 +3311,8 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>
> intel_encoder = &intel_dig_port->base;
>
> + mutex_init(&intel_dig_port->hdcp_mutex);
> +
> drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
> &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
> "HDMI %c", port_name(port));
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
