> -----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Vinod
> Govindapillai
> Sent: Thursday, November 21, 2024 4:57 PM
> To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Cc: Govindapillai, Vinod <vinod.govindapillai at intel.com>; Nikula, Jani
> <jani.nikula at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>; Saarinen, Jani
> <jani.saarinen at intel.com>
> Subject: [PATCH v2 2/3] drm/i915/display: update to relative data rate handling
I think patch header can be re-phrased, something like add a helper for relative
data rate handling
> Move the relative data rate handling to the skl_watermarks.c where other similar
> functions are implemented. Also get rid of
> use_min_ddb() and use use_minimal_wm0() instead to decide whether the
> relative data rate can be returned as 0
This also can be adjusted accordingly.
Overall Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai at intel.com>
> ---
> .../gpu/drm/i915/display/intel_atomic_plane.c | 27 +++++--------------
> drivers/gpu/drm/i915/display/skl_watermark.c | 16 +++++++++++
> drivers/gpu/drm/i915/display/skl_watermark.h | 4 +++
> 3 files changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index d89630b2d5c1..162bd20632cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -207,17 +207,6 @@ unsigned int intel_plane_data_rate(const struct
> intel_crtc_state *crtc_state,
> fb->format->cpp[color_plane];
> }
>
> -static bool
> -use_min_ddb(const struct intel_crtc_state *crtc_state,
> - struct intel_plane *plane)
> -{
> - struct drm_i915_private *i915 = to_i915(plane->base.dev);
> -
> - return DISPLAY_VER(i915) >= 13 &&
> - crtc_state->uapi.async_flip &&
> - plane->async_flip;
> -}
> -
> static unsigned int
> intel_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state, @@ -
> 225,8 +214,8 @@ intel_plane_relative_data_rate(const struct intel_crtc_state
> *crtc_state, {
> struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> const struct drm_framebuffer *fb = plane_state->hw.fb;
> - int width, height;
> unsigned int rel_data_rate;
> + int width, height;
>
> if (plane->id == PLANE_CURSOR)
> return 0;
> @@ -234,14 +223,6 @@ intel_plane_relative_data_rate(const struct
> intel_crtc_state *crtc_state,
> if (!plane_state->uapi.visible)
> return 0;
>
> - /*
> - * We calculate extra ddb based on ratio plane rate/total data rate
> - * in case, in some cases we should not allocate extra ddb for the plane,
> - * so do not count its data rate, if this is the case.
> - */
> - if (use_min_ddb(crtc_state, plane))
> - return 0;
> -
> /*
> * Src coordinates are already rotated by 270 degrees for
> * the 90/270 degree plane rotation cases (to match the @@ -256,7
> +237,11 @@ intel_plane_relative_data_rate(const struct intel_crtc_state
> *crtc_state,
> height /= 2;
> }
>
> - rel_data_rate = width * height * fb->format->cpp[color_plane];
> + rel_data_rate =
> + skl_plane_relative_data_rate(crtc_state, plane, width, height,
> + fb->format->cpp[color_plane]);
> + if (!rel_data_rate)
> + return 0;
>
> return intel_adjusted_rate(&plane_state->uapi.src,
> &plane_state->uapi.dst,
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 7c1c491c2fe0..23ed989f01dc 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -1383,6 +1383,22 @@ use_minimal_wm0_only(const struct intel_crtc_state
> *crtc_state,
> plane->async_flip;
> }
>
> +unsigned int
> +skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
> + struct intel_plane *plane, int width, int height,
> + int cpp)
> +{
> + /*
> + * We calculate extra ddb based on ratio plane rate/total data rate
> + * in case, in some cases we should not allocate extra ddb for the plane,
> + * so do not count its data rate, if this is the case.
> + */
> + if (use_minimal_wm0_only(crtc_state, plane))
> + return 0;
> +
> + return width * height * cpp;
> +}
> +
> static u64
> skl_total_relative_data_rate(const struct intel_crtc_state *crtc_state) { diff --git
> a/drivers/gpu/drm/i915/display/skl_watermark.h
> b/drivers/gpu/drm/i915/display/skl_watermark.h
> index e73baec94873..e79eee80e66f 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> @@ -18,6 +18,7 @@ struct intel_bw_state; struct intel_crtc; struct
> intel_crtc_state; struct intel_plane;
> +struct intel_plane_state;
> struct skl_pipe_wm;
> struct skl_wm_level;
>
> @@ -53,6 +54,9 @@ const struct skl_wm_level *skl_plane_wm_level(const
> struct skl_pipe_wm *pipe_wm,
> int level);
> const struct skl_wm_level *skl_plane_trans_wm(const struct skl_pipe_wm
> *pipe_wm,
> enum plane_id plane_id);
> +unsigned int skl_plane_relative_data_rate(const struct intel_crtc_state
> *crtc_state,
> + struct intel_plane *plane, int width,
> + int height, int cpp);
>
> struct intel_dbuf_state {
> struct intel_global_state base;
> --
> 2.34.1