On Mon, 2025-09-08 at 10:35 +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.
>
> Signed-off-by: Luca Coelho <luciano.coe...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display_core.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Hi Luca,
I am not sure if these comments by itself help in any understanding of the wm
handling code better -
rather than browsing through the code! IMO, probably you need to have bit
bigger explanation of this
"struct intel_wm_funcs" and the split of legacy/SKL+ usage - before these
comments!
BR
Vinod
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 8c226406c5cd..938971591470 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);
> };
>
