Re: [PATCH 1/2] drm/i915/hdmi: add debugfs to contorl HDMI bpc

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

On Thu, Aug 28, 2025 at 08:06:48AM +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.
> This change leverages the existing debugfs (intel_force_link_bpp)
> to manage HDMI bpc, and making it easier to pass HDMI CTS.
> 
> v2: Using exist variable max_requested_bpc.
> v3: Extend intel_force_link_bpp to support HDMI as suggested by Imre.
> v4: Update commit message suggested by Jani.
> 
> 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>
> ---
>  drivers/gpu/drm/i915/display/g4x_hdmi.c      | 5 +----
>  drivers/gpu/drm/i915/display/intel_hdmi.c    | 4 ++++
>  drivers/gpu/drm/i915/display/intel_link_bw.c | 6 +-----
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> index 108ebd97f9e4..b31fb1e4bc1a 100644
> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> @@ -136,11 +136,8 @@ static int g4x_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  
> -	if (HAS_PCH_SPLIT(display)) {
> +	if (HAS_PCH_SPLIT(display))
>  		crtc_state->has_pch_encoder = true;
> -		if (!intel_fdi_compute_pipe_bpp(crtc_state))

g4x_hdmi.c doesn't need to included intel_fdi.h after this change. With
that include removed, the patch looks ok to me:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> -			return -EINVAL;
> -	}
>  
>  	if (display->platform.g4x)
>  		crtc_state->has_hdmi_sink = g4x_compute_has_hdmi_sink(state, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index cbee628eb26b..027e8ed0cea8 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -55,6 +55,7 @@
>  #include "intel_display_regs.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
> +#include "intel_fdi.h"
>  #include "intel_gmbus.h"
>  #include "intel_hdcp.h"
>  #include "intel_hdcp_regs.h"
> @@ -2345,6 +2346,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		pipe_config->pixel_multiplier = 2;
>  
> +	if (!intel_fdi_compute_pipe_bpp(pipe_config))
> +		return -EINVAL;
> +
>  	pipe_config->has_audio =
>  		intel_hdmi_has_audio(encoder, pipe_config, conn_state) &&
>  		intel_audio_compute_config(encoder, pipe_config, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c
> index 3caef7f9c7c4..d194a366ff10 100644
> --- a/drivers/gpu/drm/i915/display/intel_link_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_link_bw.c
> @@ -449,6 +449,7 @@ void intel_link_bw_connector_debugfs_add(struct intel_connector *connector)
>  	switch (connector->base.connector_type) {
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  	case DRM_MODE_CONNECTOR_eDP:
> +	case DRM_MODE_CONNECTOR_HDMIA:
>  		break;
>  	case DRM_MODE_CONNECTOR_VGA:
>  	case DRM_MODE_CONNECTOR_SVIDEO:
> @@ -457,11 +458,6 @@ void intel_link_bw_connector_debugfs_add(struct intel_connector *connector)
>  		if (HAS_FDI(display))
>  			break;
>  
> -		return;
> -	case DRM_MODE_CONNECTOR_HDMIA:
> -		if (HAS_FDI(display) && !HAS_DDI(display))
> -			break;
> -
>  		return;
>  	default:
>  		return;
> -- 
> 2.34.1
>