On Thu, Oct 09, 2025 at 10:54:32AM +0300, Luca Coelho wrote: > Some of the ops in struct intel_wm_funcs are used only for legacy > watermark management, while others are only for SKL+ or both. Clarify > that in the struct definition.
We have (roughly) three vintages of wm stuff right now. pre-g4x without proper atomic watermark code currently (though I do have it in a branch somewhere...): - .compute_watermarks() doesn't really do what it says on the tin here, but I just needed a place to hide the ugliness from higher level code - .update_wm() g4x/vlv/chv/ilk+ (hw with single buffered wm registers) .compute_watermarks() .initial_watermarks() .optimize_watermarks() .atomic_update_watermarks() .get_hw_state() .sanitize() skl+ (hw with double buffered wm registers) .compute_global_watermarks() .get_hw_state() .sanitize() Most of the differences between the three are more accidental than intentional, and should be unified. I think if we managed to clean this up properly then we would be left with this: .compute() .sanitize() .get_hw_state() .initial_watermarks() (pre-skl only) .optimize_watermarks() (pre-skl only) .atomic_update_watermarks() (pre-skl only) Getting there would be mostly a matter of figuring out the right order to do things in intel_atomic_check(). > > Signed-off-by: Luca Coelho <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_display_core.h | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h > b/drivers/gpu/drm/i915/display/intel_display_core.h > index df4da52cbdb3..7144b61fb1ff 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_core.h > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h > @@ -78,7 +78,7 @@ struct intel_display_funcs { > > /* functions used for watermark calcs for display. */ > struct intel_wm_funcs { > - /* update_wm is for legacy wm management */ > + /* these are only for legacy wm management */ > void (*update_wm)(struct intel_display *display); > int (*compute_watermarks)(struct intel_atomic_state *state, > struct intel_crtc *crtc); > @@ -88,8 +88,12 @@ struct intel_wm_funcs { > struct intel_crtc *crtc); > void (*optimize_watermarks)(struct intel_atomic_state *state, > struct intel_crtc *crtc); > + > + /* these are for SKL+ wm management */ > int (*compute_global_watermarks)(struct intel_atomic_state *state); > void (*get_hw_state)(struct intel_display *display); > + > + /* this is used by both legacy and SKL+ */ > void (*sanitize)(struct intel_display *display); > }; > > -- > 2.51.0 -- Ville Syrjälä Intel
