[bug report] drm/i915/dp: Get optimal link config to have best compressed bpp

[Nautiyal, Ankit K]

On 2/5/2025 2:00 PM, Dan Carpenter wrote:
> Hello Ankit Nautiyal,
>
> Commit 1c56e9a39833 ("drm/i915/dp: Get optimal link config to have
> best compressed bpp") from Aug 17, 2023 (linux-next), leads to the
> following Smatch static checker warning:
>
> 	drivers/gpu/drm/i915/display/intel_dp.c:2481 intel_dp_dsc_compute_pipe_bpp_limits()
> 	warn: always clamps to 24
>
> drivers/gpu/drm/i915/display/intel_dp.c
>      2472 static void
>      2473 intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
>      2474                                      struct link_config_limits *limits)
>      2475 {
>      2476         struct intel_display *display = to_intel_display(intel_dp);
>      2477         int dsc_min_bpc = intel_dp_dsc_min_src_input_bpc();
>      2478         int dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
>      2479
>      2480         limits->pipe.max_bpp = clamp(limits->pipe.max_bpp, dsc_min_bpc * 3, dsc_max_bpc * 3);
> --> 2481         limits->pipe.min_bpp = clamp(limits->pipe.min_bpp, dsc_min_bpc * 3, dsc_max_bpc * 3);
>      2482 }
>
> This is an unpublished static checker warning that complains about weird
> clamps() so it only just started showing up now.  The problem is a
> mismatch between intel_dp_min_bpp() and intel_dp_dsc_min_src_input_bpc().
>
> In intel_dp_min_bpp() it uses "6 * 3" but then that gets overwriten with
> "8 * 3" eventually.  The warning is that "6 * 3" might be pointless?
> I haven't followed the code totally, but it seems like potentially the
> checker is correct.

Hi Dan,

The intel_dp_min_bpp helper function specifies the minimum bits per 
pixel (bpp) that the hardware supports for DisplayPort (DP) without 
using Display Stream Compression (DSC).

On the other hand, intel_dp_dsc_min_src_input_bpc() specifies the 
minimum bits per component (bpc) that the hardware supports when DSC is 
used. This is used to derive the min bpp which can be the input to the 
DSC HW.

For a given resolution, the driver first tries different lanes, rates, 
output formats, and bpps within a minimum and maximum bpp range without 
using DSC.

If this is not possible, it then tries to use DSC. With DSC, the minimum 
bpp the hardware can support is 24 bpp, so we update the minimum and 
maximum range accordingly.

>
> drivers/gpu/drm/i915/display/intel_dp.c
>    1175  int intel_dp_min_bpp(enum intel_output_format output_format)
>    1176  {
>    1177          if (output_format == INTEL_OUTPUT_FORMAT_RGB)
>    1178                  return 6 * 3;
>                                 ^^^^^
> Is this pointless?  Should we just always return "8 * 3" since
> that's what it's clamped to in the end?

The 6 * 3 value is the minimum bpp when DSC is not in use and the output 
format is RGB.
This value is not pointless because it represents the minimum bpp 
supported by the hardware in this specific scenario.


>
>    1179          else
>    1180                  return 8 * 3;
>    1181  }
>
> drivers/gpu/drm/i915/display/intel_dp.c
>    2161  int intel_dp_dsc_min_src_input_bpc(void)
>    2162  {
>    2163          /* Min DSC Input BPC for ICL+ is 8 */
>    2164          return 8;
>
> This 8 becomes "8 * 3" in the caller.

This function returns 8, which becomes 8 * 3 in the caller, representing 
the minimum bpp when DSC is used.
If DSC is not used, intel_dp_dsc_compute_pipe_bpp_limits() does not get 
called, and the clamping to 24 bpp does not occur.

I hope this clarifies the distinction and the logic behind the clamping.

Thanks & Regards,
Ankit


>
>    2165  }
>
> regards,
> dan carpenter
>