Re: [PATCH 10/15] drm/i915/display: convert HAS_HW_SAGV_WM() to struct intel_display

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Mon, 2024-11-04 at 19:19 +0200, Jani Nikula wrote:
> Convert HAS_HW_SAGV_WM() to struct intel_display. Do minimal drive-by
> conversions to struct intel_display in the callers while at it.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c   |  5 ++-
>  .../drm/i915/display/intel_display_device.h   |  2 +-
>  .../drm/i915/display/skl_universal_plane.c    |  7 ++--
>  drivers/gpu/drm/i915/display/skl_watermark.c  | 33 +++++++++++--------
>  4 files changed, 25 insertions(+), 22 deletions(-)

Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>

> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 9ba77970dab7..ed88a28a3afa 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -619,7 +619,6 @@ static void skl_write_cursor_wm(struct intel_dsb *dsb,
>                                 const struct intel_crtc_state *crtc_state)
>  {
>         struct intel_display *display = to_intel_display(plane->base.dev);
> -       struct drm_i915_private *i915 = to_i915(plane->base.dev);
>         enum plane_id plane_id = plane->id;
>         enum pipe pipe = plane->pipe;
>         const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> @@ -627,14 +626,14 @@ static void skl_write_cursor_wm(struct intel_dsb *dsb,
>                 &crtc_state->wm.skl.plane_ddb[plane_id];
>         int level;
>  
> -       for (level = 0; level < i915->display.wm.num_levels; level++)
> +       for (level = 0; level < display->wm.num_levels; level++)
>                 intel_de_write_dsb(display, dsb, CUR_WM(pipe, level),
>                                    skl_cursor_wm_reg_val(skl_plane_wm_level(pipe_wm, plane_id,
> level)));
>  
>         intel_de_write_dsb(display, dsb, CUR_WM_TRANS(pipe),
>                            skl_cursor_wm_reg_val(skl_plane_trans_wm(pipe_wm, plane_id)));
>  
> -       if (HAS_HW_SAGV_WM(i915)) {
> +       if (HAS_HW_SAGV_WM(display)) {
>                 const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>  
>                 intel_de_write_dsb(display, dsb, CUR_WM_SAGV(pipe),
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> b/drivers/gpu/drm/i915/display/intel_display_device.h
> index c23823769911..e1e718fced3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -162,7 +162,7 @@ struct intel_display_platforms {
>  #define HAS_GMBUS_IRQ(i915)            (DISPLAY_VER(i915) >= 4)
>  #define HAS_GMBUS_BURST_READ(__display)        (DISPLAY_VER(__display) >= 10 || (__display)-
> >platform.kabylake)
>  #define HAS_GMCH(i915)                 (DISPLAY_INFO(i915)->has_gmch)
> -#define HAS_HW_SAGV_WM(i915)           (DISPLAY_VER(i915) >= 13 && !IS_DGFX(i915))
> +#define HAS_HW_SAGV_WM(__display)      (DISPLAY_VER(__display) >= 13 && !(__display)-
> >platform.dgfx)
>  #define HAS_IPC(i915)                  (DISPLAY_INFO(i915)->has_ipc)
>  #define HAS_IPS(__display)             ((__display)->platform.haswell_ult || (__display)-
> >platform.broadwell)
>  #define HAS_LRR(i915)                  (DISPLAY_VER(i915) >= 12)
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 28f7f2405ef3..4c7bcf6806ff 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -736,7 +736,6 @@ static void skl_write_plane_wm(struct intel_dsb *dsb,
>                                const struct intel_crtc_state *crtc_state)
>  {
>         struct intel_display *display = to_intel_display(plane->base.dev);
> -       struct drm_i915_private *i915 = to_i915(plane->base.dev);
>         enum plane_id plane_id = plane->id;
>         enum pipe pipe = plane->pipe;
>         const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal;
> @@ -746,14 +745,14 @@ static void skl_write_plane_wm(struct intel_dsb *dsb,
>                 &crtc_state->wm.skl.plane_ddb_y[plane_id];
>         int level;
>  
> -       for (level = 0; level < i915->display.wm.num_levels; level++)
> +       for (level = 0; level < display->wm.num_levels; level++)
>                 intel_de_write_dsb(display, dsb, PLANE_WM(pipe, plane_id, level),
>                                    skl_plane_wm_reg_val(skl_plane_wm_level(pipe_wm, plane_id,
> level)));
>  
>         intel_de_write_dsb(display, dsb, PLANE_WM_TRANS(pipe, plane_id),
>                            skl_plane_wm_reg_val(skl_plane_trans_wm(pipe_wm, plane_id)));
>  
> -       if (HAS_HW_SAGV_WM(i915)) {
> +       if (HAS_HW_SAGV_WM(display)) {
>                 const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id];
>  
>                 intel_de_write_dsb(display, dsb, PLANE_WM_SAGV(pipe, plane_id),
> @@ -765,7 +764,7 @@ static void skl_write_plane_wm(struct intel_dsb *dsb,
>         intel_de_write_dsb(display, dsb, PLANE_BUF_CFG(pipe, plane_id),
>                            skl_plane_ddb_reg_val(ddb));
>  
> -       if (DISPLAY_VER(i915) < 11)
> +       if (DISPLAY_VER(display) < 11)
>                 intel_de_write_dsb(display, dsb, PLANE_NV12_BUF_CFG(pipe, plane_id),
>                                    skl_plane_ddb_reg_val(ddb_y));
>  }
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 09af693da586..060e0cfcb47f 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -449,6 +449,7 @@ bool intel_can_enable_sagv(struct drm_i915_private *i915,
>  
>  static int intel_compute_sagv_mask(struct intel_atomic_state *state)
>  {
> +       struct intel_display *display = to_intel_display(state);
>         struct drm_i915_private *i915 = to_i915(state->base.dev);
>         int ret;
>         struct intel_crtc *crtc;
> @@ -484,7 +485,7 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state)
>                  * other crtcs can't be allowed to use the more optimal
>                  * normal (ie. non-SAGV) watermarks.
>                  */
> -               pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(i915) &&
> +               pipe_wm->use_sagv_wm = !HAS_HW_SAGV_WM(display) &&
>                         DISPLAY_VER(i915) >= 12 &&
>                         intel_crtc_can_enable_sagv(new_crtc_state);
>  
> @@ -2748,10 +2749,10 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
>                                          const struct skl_pipe_wm *old_pipe_wm,
>                                          const struct skl_pipe_wm *new_pipe_wm)
>  {
> -       struct drm_i915_private *i915 = to_i915(plane->base.dev);
> +       struct intel_display *display = to_intel_display(plane);
>         int level;
>  
> -       for (level = 0; level < i915->display.wm.num_levels; level++) {
> +       for (level = 0; level < display->wm.num_levels; level++) {
>                 /*
>                  * We don't check uv_wm as the hardware doesn't actually
>                  * use it. It only gets used for calculating the required
> @@ -2762,7 +2763,7 @@ static bool skl_plane_selected_wm_equals(struct intel_plane *plane,
>                         return false;
>         }
>  
> -       if (HAS_HW_SAGV_WM(i915)) {
> +       if (HAS_HW_SAGV_WM(display)) {
>                 const struct skl_plane_wm *old_wm = &old_pipe_wm->planes[plane->id];
>                 const struct skl_plane_wm *new_wm = &new_pipe_wm->planes[plane->id];
>  
> @@ -2937,6 +2938,7 @@ static void skl_wm_level_from_reg_val(u32 val, struct skl_wm_level *level)
>  static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>                                      struct skl_pipe_wm *out)
>  {
> +       struct intel_display *display = to_intel_display(crtc);
>         struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>         enum pipe pipe = crtc->pipe;
>         enum plane_id plane_id;
> @@ -2962,7 +2964,7 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>  
>                 skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  
> -               if (HAS_HW_SAGV_WM(i915)) {
> +               if (HAS_HW_SAGV_WM(display)) {
>                         if (plane_id != PLANE_CURSOR)
>                                 val = intel_de_read(i915, PLANE_WM_SAGV(pipe, plane_id));
>                         else
> @@ -3131,6 +3133,7 @@ static void skl_wm_get_hw_state_and_sanitize(struct drm_i915_private *i915)
>  void intel_wm_state_verify(struct intel_atomic_state *state,
>                            struct intel_crtc *crtc)
>  {
> +       struct intel_display *display = to_intel_display(state);
>         struct drm_i915_private *i915 = to_i915(state->base.dev);
>         const struct intel_crtc_state *new_crtc_state =
>                 intel_atomic_get_new_crtc_state(state, crtc);
> @@ -3205,7 +3208,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state,
>                 hw_wm_level = &hw->wm.planes[plane->id].sagv.wm0;
>                 sw_wm_level = &sw_wm->planes[plane->id].sagv.wm0;
>  
> -               if (HAS_HW_SAGV_WM(i915) &&
> +               if (HAS_HW_SAGV_WM(display) &&
>                     !skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
>                         drm_err(&i915->drm,
>                                 "[PLANE:%d:%s] mismatch in SAGV WM (expected e=%d b=%u l=%u, got
> e=%d b=%u l=%u)\n",
> @@ -3221,7 +3224,7 @@ void intel_wm_state_verify(struct intel_atomic_state *state,
>                 hw_wm_level = &hw->wm.planes[plane->id].sagv.trans_wm;
>                 sw_wm_level = &sw_wm->planes[plane->id].sagv.trans_wm;
>  
> -               if (HAS_HW_SAGV_WM(i915) &&
> +               if (HAS_HW_SAGV_WM(display) &&
>                     !skl_wm_level_equals(hw_wm_level, sw_wm_level)) {
>                         drm_err(&i915->drm,
>                                 "[PLANE:%d:%s] mismatch in SAGV trans WM (expected e=%d b=%u l=%u,
> got e=%d b=%u l=%u)\n",
> @@ -3392,17 +3395,19 @@ static void skl_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
>  
>  static void skl_setup_wm_latency(struct drm_i915_private *i915)
>  {
> -       if (HAS_HW_SAGV_WM(i915))
> -               i915->display.wm.num_levels = 6;
> +       struct intel_display *display = &i915->display;
> +
> +       if (HAS_HW_SAGV_WM(display))
> +               display->wm.num_levels = 6;
>         else
> -               i915->display.wm.num_levels = 8;
> +               display->wm.num_levels = 8;
>  
> -       if (DISPLAY_VER(i915) >= 14)
> -               mtl_read_wm_latency(i915, i915->display.wm.skl_latency);
> +       if (DISPLAY_VER(display) >= 14)
> +               mtl_read_wm_latency(i915, display->wm.skl_latency);
>         else
> -               skl_read_wm_latency(i915, i915->display.wm.skl_latency);
> +               skl_read_wm_latency(i915, display->wm.skl_latency);
>  
> -       intel_print_wm_latency(i915, "Gen9 Plane", i915->display.wm.skl_latency);
> +       intel_print_wm_latency(i915, "Gen9 Plane", display->wm.skl_latency);
>  }
>  
>  static const struct intel_wm_funcs skl_wm_funcs = {