On Tue, 2025-10-07 at 11:07 +0300, Jani Nikula wrote: > On Tue, 07 Oct 2025, Luca Coelho <[email protected]> wrote: > > There are currently two booleans to define three tiling modes, which > > is bad practice because it allows representing an invalid mode. In > > order to simplify this, convert these two booleans into one > > enumeration with three possible tiling modes. > > > > Additionally, introduce the concept of Y "family" of tiling, which > > groups Y, Yf and 4 tiling, since they're effectively treated in the > > same way in the watermark calculations. Describe the grouping in the > > enumeration definition. > > > > v2: - remove redundant "tiled" and get rid of "family" in the enums (Ville) > > - remove holes introduced in the skl_wm_params struct (Ville) > > - improve if-else block to avoid intel_fb_is_tiled_modifier() call > > (Ville) > > > > Signed-off-by: Luca Coelho <[email protected]> > > --- > > drivers/gpu/drm/i915/display/skl_watermark.c | 38 +++++++++++++------- > > 1 file changed, 25 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index 3a982458395e..dc00b5cd3ff7 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -53,13 +53,20 @@ struct intel_dbuf_state { > > #define intel_atomic_get_new_dbuf_state(state) \ > > to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, > > &to_intel_display(state)->dbuf.obj)) > > > > +/* Tiling mode groups relevant to WM calculations */ > > +enum wm_tiling_mode { > > + WM_TILING_LINEAR, > > + WM_TILING_X, > > + WM_TILING_Y_Yf_4, > > +}; > > + > > /* Stores plane specific WM parameters */ > > struct skl_wm_params { > > - bool x_tiled, y_tiled; > > - bool rc_surface; > > - bool is_planar; > > + enum wm_tiling_mode tiling; > > u32 width; > > u8 cpp; > > + bool rc_surface; > > + bool is_planar; > > u32 plane_pixel_rate; > > u32 y_min_scanlines; > > u32 plane_bytes_per_line; > > @@ -618,7 +625,8 @@ static unsigned int skl_wm_latency(struct intel_display > > *display, int level, > > display->platform.cometlake) && skl_watermark_ipc_enabled(display)) > > latency += 4; > > > > - if (skl_needs_memory_bw_wa(display) && wp && wp->x_tiled) > > + if (skl_needs_memory_bw_wa(display) && > > + wp && wp->tiling == WM_TILING_X_TILED) > > latency += 15; > > > > return latency; > > @@ -1659,9 +1667,13 @@ skl_compute_wm_params(const struct intel_crtc_state > > *crtc_state, > > return -EINVAL; > > } > > > > - wp->x_tiled = modifier == I915_FORMAT_MOD_X_TILED; > > - wp->y_tiled = modifier != I915_FORMAT_MOD_X_TILED && > > - intel_fb_is_tiled_modifier(modifier); > > + if (modifier == WM_TILING_LINEAR) > > "Namespace" mistmatch between modifier and WM_TILING_LINEAR?
Oops, sorry, b0rked rebase. I'll fix and submit v3. -- Cheers, Luca.
