Re: [v2] drm/i915/hdmi: add debugfs to contorl HDMI bpc

[Imre Deak <imre.deak@intel.com>]

On Fri, Aug 08, 2025 at 09:16:02AM +0000, Lee Shawn C wrote:
> While performing HDMI compliance testing, test equipment may request
> different bpc output for signal measurement. However, display driver
> typically determines the maximum available bpc based on HW bandwidth.
> 
> Introduce a new debugfs that allows user to configure dedicated bpc
> manually, and making HDMI compliance test much easier.
> 
> v2: Using exist variable max_requested_bpc.

How come this doesn't get reset after a hotplug as you described for the
case when the property is used, even though both the property and this
debug entries use the same state variable? (Not saying that the reset
happing after a hotplug is a valid justification for a new debugfs entry,
the hotplug could be also handled by the user, but you could argue the
debugfs entry is more convenient.)

Have you also considered enabling the force_link_bpp debugfs entry for
all HDMI connectors instead?

> Cc: Shankar Uma <uma.shankar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index ce3f9810c42d..5ef2bcffefc5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1208,6 +1208,47 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
>  	.write = i915_dsc_fractional_bpp_write
>  };
>  
> +static int i915_force_bpc_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct drm_connector *conn = &connector->base;
> +
> +	seq_printf(m, "%u\n", conn->state->max_requested_bpc);
> +
> +	return 0;
> +}
> +
> +static ssize_t i915_force_bpc_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_display *display = to_intel_display(connector);
> +	struct drm_connector *conn = &connector->base;
> +	int new_bpc, ret;
> +
> +	ret = kstrtoint_from_user(ubuf, len, 0, &new_bpc);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (new_bpc) {
> +	case 8:
> +	case 10:
> +	case 12:
> +		break;
> +	default:
> +		drm_dbg_kms(display->drm, "Invalid bpc value (%u)\n", new_bpc);
> +		return -EINVAL;
> +	}
> +
> +	conn->state->max_requested_bpc = new_bpc;
> +
> +	*offp += len;
> +	return len;
> +}
> +DEFINE_SHOW_STORE_ATTRIBUTE(i915_force_bpc);
> +
>  /*
>   * Returns the Current CRTC's bpc.
>   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> @@ -1359,6 +1400,11 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  	    connector_type == DRM_MODE_CONNECTOR_HDMIB)
>  		debugfs_create_file("i915_lpsp_capability", 0444, root,
>  				    connector, &i915_lpsp_capability_fops);
> +
> +	if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> +	    connector_type == DRM_MODE_CONNECTOR_HDMIB)
> +		debugfs_create_file("i915_force_bpc", 0644, root,
> +				    connector, &i915_force_bpc_fops);
>  }
>  
>  /**
> -- 
> 2.34.1
>