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
