> -----Original Message----- > From: Nikula, Jani <jani.nikula@xxxxxxxxx> > Sent: Friday, August 12, 2022 12:25 PM > To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: De Marchi, Lucas <lucas.demarchi@xxxxxxxxx> > Subject: RE: [PATCH 06/39] drm/i915: move wm_disp funcs to > display.funcs > > On Fri, 12 Aug 2022, "Murthy, Arun R" <arun.r.murthy@xxxxxxxxx> wrote: > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h > >> b/drivers/gpu/drm/i915/display/intel_display_core.h > >> index 98c6ccdc9100..a6843ebcca5a 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_core.h > >> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h > >> @@ -8,6 +8,7 @@ > >> > >> #include <linux/types.h> > >> > >> +struct drm_i915_private; > >> struct intel_atomic_state; > >> struct intel_cdclk_funcs; > >> struct intel_clock_gating_funcs; > >> @@ -31,6 +32,23 @@ struct intel_display_funcs { > >> void (*commit_modeset_enables)(struct intel_atomic_state > >> *state); }; > >> > >> +/* functions used for watermark calcs for display. */ struct > >> +intel_wm_funcs { > >> + /* update_wm is for legacy wm management */ > >> + void (*update_wm)(struct drm_i915_private *dev_priv); > >> + int (*compute_pipe_wm)(struct intel_atomic_state *state, > >> + struct intel_crtc *crtc); > >> + int (*compute_intermediate_wm)(struct intel_atomic_state *state, > >> + struct intel_crtc *crtc); > >> + void (*initial_watermarks)(struct intel_atomic_state *state, > >> + struct intel_crtc *crtc); > >> + void (*atomic_update_watermarks)(struct intel_atomic_state *state, > >> + struct intel_crtc *crtc); > >> + void (*optimize_watermarks)(struct intel_atomic_state *state, > >> + struct intel_crtc *crtc); > >> + int (*compute_global_watermarks)(struct intel_atomic_state > >> +*state); > >> }; > >> + > >> struct intel_display { > >> /* Display functions */ > >> struct { > >> @@ -48,6 +66,9 @@ struct intel_display { > >> > >> /* pm private clock gating functions */ > >> const struct intel_clock_gating_funcs *clock_gating; > >> + > >> + /* pm display functions */ > >> + const struct intel_wm_funcs *wm; > >> } funcs; > > > > Can the wm, dbuf, clock related move to a struct intel_pm ? which > > makes it more meaningful else again we end up creating a struct > > intel_display a long one like i915_private. > > Well, I think the display wm/dbuf stuff also needs to be split from intel_pm.c > so it's not so clear cut. And for now, I'm keeping the functions together. Ok, lets take it up in the later. But adding a TODO would make it clear that some more clean up in this regards is in queue. Reviewed-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx> Thanks and Regards, Arun R Murthy --------------------