RE: [PATCH 2/6] drm/i915/wm: Refactor dpkgc value prepration

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Kandpal, Suraj <suraj.kand...@intel.com>
> Sent: 03 December 2024 14:17
> To: intel...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.gol...@intel.com>;
> Kandpal, Suraj <suraj.kand...@intel.com>
> Subject: [PATCH 2/6] drm/i915/wm: Refactor dpkgc value prepration
> 
> Refactor the value getting prepped to be written into the PKG_C_LATENCY
> register by ORing the REG_FIELD_PREP values instead of having val getting
> operated on twice.
> We dont need the clear and val variables to be initialized.
> 
> Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index c40e0173a5bd..df961cb8d51f 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2858,7 +2858,7 @@ static void
>  skl_program_dpkgc_latency(struct drm_i915_private *i915, bool
> enable_dpkgc)  {
>       u32 max_latency = LNL_PKG_C_LATENCY_MASK;
> -     u32 clear = 0, val = 0;

The other way of previous implementation also seems to be ok only.
But, With reference to changes suggested by Vinod in previous revisions: 
https://patchwork.freedesktop.org/patch/624490/?series=141334&rev=1

Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com>

> +     u32 clear, val;
>       u32 added_wake_time = 0;
> 
>       if (DISPLAY_VER(i915) < 20)
> @@ -2872,9 +2872,9 @@ skl_program_dpkgc_latency(struct
> drm_i915_private *i915, bool enable_dpkgc)
>                       i915->display.sagv.block_time_us;
>       }
> 
> -     clear |= LNL_ADDED_WAKE_TIME_MASK |
> LNL_PKG_C_LATENCY_MASK;
> -     val |= REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency);
> -     val |= REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK,
> added_wake_time);
> +     clear = LNL_ADDED_WAKE_TIME_MASK |
> LNL_PKG_C_LATENCY_MASK;
> +     val = REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, max_latency) |
> +             REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK,
> added_wake_time);
> 
>       intel_uncore_rmw(&i915->uncore, LNL_PKG_C_LATENCY, clear, val);
> }
> --
> 2.34.1