Re: [PATCH v5] drm/i915/display: Use the recomended min_hblank values

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

On Mon, Jul 28, 2025 at 01:32:14PM +0530, Arun R Murthy wrote:
> Use recommended values as per wa_14021694213 to compare with the
> calculated value and choose minimum of them.
> 
> v2: corrected checkpatch warning and retain the restriction for
> min_hblank (Jani)
> v3: use calculated value to compare with recomended value and choose
> minimum of them (Imre)
> v4: As driver supported min bpc is 8, omit the condition check for
> bpc6 with ycbcr420. Added a note for the same (Imre)
> v5: Add a warn for the unexpected case of 6bpc + uhbr + ycbcr420
> 
> Bspec: 74379
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
> Changes in v5:
> - EDITME: describe what is new in this series revision.
> - EDITME: use bulletpoints and terse descriptions.
> - Link to v4: https://lore.kernel.org/r/20250728-min_hblank-v4-1-b9d83bf03025@intel.com
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 54d88f24b6893e1f8009f00dda87d8dae5dfc137..1cc19f3e7729c25d2066f42601a18bdfa03b2721 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3182,7 +3182,26 @@ int intel_dp_compute_min_hblank(struct intel_crtc_state *crtc_state,
>  	 */
>  	min_hblank = min_hblank - 2;
>  
> -	min_hblank = min(10, min_hblank);
> +	/*
> +	 * min_hblank formula is undergoing a change, to avoid underrun use the
> +	 * recomended value in spec to compare with the calculated one and use the
> +	 * minimum value
> +	 */
> +	if (intel_dp_is_uhbr(crtc_state)) {
> +		/*
> +		 * Note: Driver doesn't support compressed bpp 6.
> +		 * Its unusual to have 6bpc with YCBCR420, for which the spec suggests
                                       ^bpp

> +		 * a minimum hblank value of 2(Hence this condition is omitted).

It's not clear why a compressed bpp of 6 would be unusual. The driver
would use this compressed bpp, just as it would use any other compressed
bpp, if the source (HW) would support it and the available link bandwidth
would require it. It's just that the minimum compressed bpp supported by
the HW or driver is 8. So I'd just say stg like:

		/*
		 * Note: Bspec requires a min_hblank of 2 for YCBCR420
		 * with compressed bpp 6, but the minimum compressed bpp
		 * supported by the driver is 8.
		 */

> +		 */
> +		drm_WARN(display->drm, (crtc_state->dsc.compression_enable &&
> +			 crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +			 crtc_state->dsc.compressed_bpp_x16 == 6),

The above needs a conversion from int to fixed point and it's better to
also assert that the bpp is at least 8, so

			crtc_state->dsc.compressed_bpp_x16 < fxp_q4_from_int(8)

> +			 "Unusual to have compressed bpp 6 with ycbcr420\n");

The condition is self-explanatory, so I'd just use drm_WARN_ON() w/o the
above text part.

> +		min_hblank = min(3, min_hblank);
> +	} else {
> +		min_hblank = min(10, min_hblank);
> +	}
> +
>  	crtc_state->min_hblank = min_hblank;
>  
>  	return 0;
> 
> ---
> base-commit: eb0c2b25072b5c56e8c41d7099050bf53bebaa14
> change-id: 20250728-min_hblank-263ceb40508c
> 
> Best regards,
> -- 
> Arun R Murthy <arun.r.murthy@intel.com>
>