Quoting Luca Coelho (2025-09-08 04:35:34-03:00)
>Make the code a bit clearer by using a switch-case to check the tiling
>mode in skl_compute_plane_wm(), because all the possible states and
>the calculations they use are explicitly handled.
>
>Signed-off-by: Luca Coelho <[email protected]>
>---
> drivers/gpu/drm/i915/display/skl_watermark.c | 24 +++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
>b/drivers/gpu/drm/i915/display/skl_watermark.c
>index dd4bed02c3c0..21f8d52ec1d2 100644
>--- a/drivers/gpu/drm/i915/display/skl_watermark.c
>+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
>@@ -1833,21 +1833,39 @@ static void skl_compute_plane_wm(const struct
>intel_crtc_state *crtc_state,
> latency,
> wp->plane_blocks_per_line);
>
>- if (wp->tiling == WM_TILING_Y_FAMILY) {
>+ switch (wp->tiling) {
>+ case WM_TILING_Y_FAMILY:
> selected_result = max_fixed16(method2, wp->y_tile_minimum);
>- } else {
>+ break;
>+
>+ case WM_TILING_LINEAR:
>+ case WM_TILING_X_TILED:
>+ /*
>+ * Special case for unrealistically small horizontal
>+ * total with plane downscaling.
>+ */
> if ((wp->cpp * crtc_state->hw.pipe_mode.crtc_htotal /
> wp->dbuf_block_size < 1) &&
>- (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
>+ (wp->plane_bytes_per_line / wp->dbuf_block_size < 1)) {
> selected_result = method2;
> } else if (latency >= wp->linetime_us) {
>+ /*
>+ * With display version 9, we use the minimum
>+ * of both methods.
>+ */
Hm... Isn't this saying what is already clear in the code below?
> if (DISPLAY_VER(display) == 9)
> selected_result = min_fixed16(method1,
> method2);
> else
> selected_result = method2;
> } else {
>+ /* everything else with linear/X-tiled uses method 1
>*/
> selected_result = method1;
> }
>+ break;
>+
>+ default:
>+ drm_err(display->drm, "Invalid tiling mode\n", wp->tiling);
>+ break;
If we decide to go with the enumeration solution, I think we should
change this into a warning and use some default behavior here (perhaps
WM_TILING_LINEAR?). Otherwise, selected_result would be used
uninitialized.
--
Gustavo Sousa
> }
>
> blocks = fixed16_to_u32_round_up(selected_result);
>--
>2.50.1
>
