Re: [PATCH 1/2] drm/i915/display: Clarify target pipe bpp variable name in compute_sink_pipe_bpp()<

["Michał Grzelak" <michal.grzelak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4031 bytes --]

Hello Imre,

Some comments below wrt commit messages. No functional change requested.

On Wed, 11 Feb 2026, Imre Deak wrote:
> Clarify that the baseline pipe BPP - i.e. the non-DP specific
> platform/EDID maximum BPP limited by any user-requested max-bpc setting
> - set by compute_sink_pipe_bpp() is a baseline _target_ pipe BPP. This
> target BPP can get either rejected or adjusted (lowering or increasing
> it as needed) by the encoder state computation based on other
> constrains, like a minimum pipe BPP dictated by a non-RGB output format

s/contrains/contraints

> (24 BPP) or a min/max DSC input BPP dictated by a DSC sink. Whether an
> out-of-bound target BPP is adjusted or rejected depends on the max-bpc
> property's semantic assumed by the driver, which is atm to reject such a

s/semantic/semantics

> request.
>
> A follow-up change will also compute the baseline _maximum_ pipe BPP,
> which is the non-DP specific platform/EDID maximum BPP w/o the requested
> max-bpc adjustment and as such is a hard limit: The encoder state

s/The/the

> computation must ensure that the final BPP selected for the modeset is
> below this maximum. Tracking the baseline maximum pipe BPP separately
> will allow for adjusting the baseline target BPP as needed, clamping it
> to the valid DP min/max pipe BPP range, instead of rejecting an
> out-of-bound BPC/BPP request.
>
> To clarify the above semantics rename bpp in compute_sink_bpp() to

s/compute_sink_bpp()/compute_sink_pipe_bpp()

> target_pipe_bpp in this patch, preparing for a follow-up change also
> computing max_pipe_bpp in the same function.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

BR,
Michał

> ---

> drivers/gpu/drm/i915/display/intel_display.c | 47 +++++++++++---------
> 1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 295f14416be70..ab4b59916d2e7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4350,6 +4350,23 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
> 	return 0;
> }
>
> +static int bpc_to_bpp(int bpc)
> +{
> +	switch (bpc) {
> +	case 6 ... 7:
> +		return 6 * 3;
> +	case 8 ... 9:
> +		return 8 * 3;
> +	case 10 ... 11:
> +		return 10 * 3;
> +	case 12 ... 16:
> +		return 12 * 3;
> +	default:
> +		MISSING_CASE(bpc);
> +		return -EINVAL;
> +	}
> +}
> +
> static int
> compute_sink_pipe_bpp(const struct drm_connector_state *conn_state,
> 		      struct intel_crtc_state *crtc_state)
> @@ -4357,36 +4374,22 @@ compute_sink_pipe_bpp(const struct drm_connector_state *conn_state,
> 	struct intel_display *display = to_intel_display(crtc_state);
> 	struct drm_connector *connector = conn_state->connector;
> 	const struct drm_display_info *info = &connector->display_info;
> -	int bpp;
> +	int target_pipe_bpp;
>
> -	switch (conn_state->max_bpc) {
> -	case 6 ... 7:
> -		bpp = 6 * 3;
> -		break;
> -	case 8 ... 9:
> -		bpp = 8 * 3;
> -		break;
> -	case 10 ... 11:
> -		bpp = 10 * 3;
> -		break;
> -	case 12 ... 16:
> -		bpp = 12 * 3;
> -		break;
> -	default:
> -		MISSING_CASE(conn_state->max_bpc);
> -		return -EINVAL;
> -	}
> +	target_pipe_bpp = bpc_to_bpp(conn_state->max_bpc);
> +	if (target_pipe_bpp < 0)
> +		return target_pipe_bpp;
>
> -	if (bpp < crtc_state->pipe_bpp) {
> +	if (target_pipe_bpp < crtc_state->pipe_bpp) {
> 		drm_dbg_kms(display->drm,
> -			    "[CONNECTOR:%d:%s] Limiting display bpp to %d "
> +			    "[CONNECTOR:%d:%s] Limiting target display pipe bpp to %d "
> 			    "(EDID bpp %d, max requested bpp %d, max platform bpp %d)\n",
> 			    connector->base.id, connector->name,
> -			    bpp, 3 * info->bpc,
> +			    target_pipe_bpp, 3 * info->bpc,
> 			    3 * conn_state->max_requested_bpc,
> 			    crtc_state->pipe_bpp);
>
> -		crtc_state->pipe_bpp = bpp;
> +		crtc_state->pipe_bpp = target_pipe_bpp;
> 	}
>
> 	return 0;
> -- 
> 2.49.1
>
>