Re: [PATCH] drm/i915/hdcp: Create force_hdcp14 debug fs entry

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/13/2025 1:55 PM, Suraj Kandpal wrote:
> Testing HDCP 1.4 becomes tough since the only way our code comes to
> HDCP 1.4 pathway is if the monitor only supports HDCP 1.4 which
> becomes tough to find sometimes.
> Setting this debug_fs entry will force use to use the HDCP 1.4 path
> so that more robust HDCP 1.4 testing can take place.
>
> --v2
> -Move the code to intel_hdcp.c [Jani]
> -Remove useless debug logging [Jani]
> -Remove Force_HDCP from the debug file [Jani]
>
> --v3
> -Remove leftover debug loggings [Jani]
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   .../drm/i915/display/intel_display_types.h    |  2 +
>   drivers/gpu/drm/i915/display/intel_hdcp.c     | 75 ++++++++++++++++++-
>   2 files changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 6a82c6ade549..c78dd77ef74c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -496,6 +496,8 @@ struct intel_hdcp {
>   	enum transcoder cpu_transcoder;
>   	/* Only used for DP MST stream encryption */
>   	enum transcoder stream_transcoder;
> +	/* Used to force HDCP 1.4 bypassing HDCP 2.x */
> +	bool force_hdcp14;
>   };
>   
>   struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 1aa4c0fd216f..0e663e0e72bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2472,13 +2472,16 @@ static int _intel_hdcp_enable(struct intel_atomic_state *state,
>   	 * 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_get_capability(connector)) {
> +	if (!hdcp->force_hdcp14 && intel_hdcp2_get_capability(connector)) {
>   		ret = _intel_hdcp2_enable(state, connector);
>   		if (!ret)
>   			check_link_interval =
>   				DRM_HDCP2_CHECK_PERIOD_MS;
>   	}
>   
> +	if (hdcp->force_hdcp14)
> +		drm_dbg_kms(display->drm, "Forcing HDCP 1.4\n");
> +
>   	/*
>   	 * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
>   	 * be attempted.
> @@ -2797,6 +2800,74 @@ static int intel_hdcp_sink_capability_show(struct seq_file *m, void *data)
>   }
>   DEFINE_SHOW_ATTRIBUTE(intel_hdcp_sink_capability);
>   
> +static ssize_t intel_hdcp_force_14_write(struct file *file,
> +					 const char __user *ubuf,
> +					 size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct intel_connector *connector = m->private;
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +	bool force_hdcp14 = false;
> +	int ret;
> +
> +	if (len == 0)
> +		return 0;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &force_hdcp14);
> +	if (ret < 0)
> +		return ret;
> +
> +	hdcp->force_hdcp14 = force_hdcp14;
> +
> +	*offp += len;

I think you missed to add blank line, as suggested by Jani.


> +	return len;
> +}
> +
> +static int intel_hdcp_force_14_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct intel_display *display = to_intel_display(connector);
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +	struct drm_crtc *crtc;
> +	int ret;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	ret = drm_modeset_lock_single_interruptible(&display->drm->mode_config.connection_mutex);
> +	if (ret)
> +		return ret;
> +
> +	crtc = connector->base.state->crtc;
> +	if (connector->base.status != connector_status_connected || !crtc) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	seq_printf(m, "%s\n",
> +		   str_yes_no(hdcp->force_hdcp14));
> +out:
> +	drm_modeset_unlock(&display->drm->mode_config.connection_mutex);

Ditto.


Apart from styling issues above, patch looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


> +	return ret;
> +}
> +
> +static int intel_hdcp_force_14_open(struct inode *inode,
> +				    struct file *file)
> +{
> +	return single_open(file, intel_hdcp_force_14_show,
> +			   inode->i_private);
> +}
> +
> +static const struct file_operations intel_hdcp_force_14_fops = {
> +	.owner = THIS_MODULE,
> +	.open = intel_hdcp_force_14_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = intel_hdcp_force_14_write
> +};
> +
>   void intel_hdcp_connector_debugfs_add(struct intel_connector *connector)
>   {
>   	struct dentry *root = connector->base.debugfs_entry;
> @@ -2807,5 +2878,7 @@ void intel_hdcp_connector_debugfs_add(struct intel_connector *connector)
>   	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>   		debugfs_create_file("i915_hdcp_sink_capability", 0444, root,
>   				    connector, &intel_hdcp_sink_capability_fops);
> +		debugfs_create_file("i915_force_hdcp14", 0644, root,
> +				    connector, &intel_hdcp_force_14_fops);
>   	}
>   }