From: "Govindapillai, Vinod" <vinod.govindapillai@intel.com>
To: "ville.syrjala@linux.intel.com" <ville.syrjala@linux.intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 8/9] drm/i915/wm: Reduce copy-pasta in skl_print_plane_wm_changes()
Date: Wed, 8 Apr 2026 12:04:59 +0000 [thread overview]
Message-ID: <9959bb3a33ce6fe89a506357c4bbb6db9759eaf9.camel@intel.com> (raw)
In-Reply-To: <20260324134843.2364-9-ville.syrjala@linux.intel.com>
On Tue, 2026-03-24 at 15:48 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> skl_print_plane_wm_changes() is rather ugly with the copy-pasted
> massive printk arguments. Reduce the duplication a bit by defining
> a few FMT/ARG macros. Still ugly, but perhaps a bit less fragile.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/skl_watermark.c | 128 ++++++-----------
> --
> 1 file changed, 40 insertions(+), 88 deletions(-)
>
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 1d932c37d768..4bffa27ce02c 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2613,6 +2613,26 @@ skl_print_plane_ddb_changes(struct intel_plane
> *plane,
> skl_ddb_entry_size(old),
> skl_ddb_entry_size(new));
> }
>
> +#define PLANE_WM_EN_FMT
> "%cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm"
> +#define PLANE_WM_EN_ARGS(__wm) \
> + enast((__wm)->wm[0].enable), enast((__wm)->wm[1].enable), \
> + enast((__wm)->wm[2].enable), enast((__wm)->wm[3].enable), \
> + enast((__wm)->wm[4].enable), enast((__wm)->wm[5].enable), \
> + enast((__wm)->wm[6].enable), enast((__wm)->wm[7].enable), \
> + enast((__wm)->trans_wm.enable), \
> + enast((__wm)->sagv.wm0.enable), \
> + enast((__wm)->sagv.trans_wm.enable)
> +
> +#define PLANE_WM_FMT "%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> +#define PLANE_WM_ARGS(__wm, __field) \
> + (__wm)->wm[0].__field, (__wm)->wm[1].__field, \
> + (__wm)->wm[2].__field, (__wm)->wm[3].__field, \
> + (__wm)->wm[4].__field, (__wm)->wm[5].__field, \
> + (__wm)->wm[6].__field, (__wm)->wm[7].__field, \
> + (__wm)->trans_wm.__field, \
> + (__wm)->sagv.wm0.__field, \
> + (__wm)->sagv.trans_wm.__field
> +
> static noinline_for_stack void
> skl_print_plane_wm_changes(struct intel_plane *plane,
> const struct skl_plane_wm *old_wm,
> @@ -2621,112 +2641,44 @@ skl_print_plane_wm_changes(struct
> intel_plane *plane,
> struct intel_display *display = to_intel_display(plane);
>
> drm_dbg_kms(display->drm,
> - "[PLANE:%d:%s] level
> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm"
> - " ->
> %cwm0,%cwm1,%cwm2,%cwm3,%cwm4,%cwm5,%cwm6,%cwm7,%ctwm,%cswm,%cstwm\n"
> ,
> + "[PLANE:%d:%s] level " PLANE_WM_EN_FMT " ->
> " PLANE_WM_EN_FMT "\n",
> plane->base.base.id, plane->base.name,
> - enast(old_wm->wm[0].enable), enast(old_wm-
> >wm[1].enable),
> - enast(old_wm->wm[2].enable), enast(old_wm-
> >wm[3].enable),
> - enast(old_wm->wm[4].enable), enast(old_wm-
> >wm[5].enable),
> - enast(old_wm->wm[6].enable), enast(old_wm-
> >wm[7].enable),
> - enast(old_wm->trans_wm.enable),
> - enast(old_wm->sagv.wm0.enable),
> - enast(old_wm->sagv.trans_wm.enable),
> - enast(new_wm->wm[0].enable), enast(new_wm-
> >wm[1].enable),
> - enast(new_wm->wm[2].enable), enast(new_wm-
> >wm[3].enable),
> - enast(new_wm->wm[4].enable), enast(new_wm-
> >wm[5].enable),
> - enast(new_wm->wm[6].enable), enast(new_wm-
> >wm[7].enable),
> - enast(new_wm->trans_wm.enable),
> - enast(new_wm->sagv.wm0.enable),
> - enast(new_wm->sagv.trans_wm.enable));
> + PLANE_WM_EN_ARGS(old_wm),
> + PLANE_WM_EN_ARGS(new_wm));
>
> drm_dbg_kms(display->drm,
> - "[PLANE:%d:%s] lines
> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d"
> - " ->
> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%4d\n",
> + "[PLANE:%d:%s] lines " PLANE_WM_FMT " -> "
> PLANE_WM_FMT "\n",
> plane->base.base.id, plane->base.name,
> - enast(old_wm->wm[0].ignore_lines), old_wm-
> >wm[0].lines,
> - enast(old_wm->wm[1].ignore_lines), old_wm-
> >wm[1].lines,
> - enast(old_wm->wm[2].ignore_lines), old_wm-
> >wm[2].lines,
> - enast(old_wm->wm[3].ignore_lines), old_wm-
> >wm[3].lines,
> - enast(old_wm->wm[4].ignore_lines), old_wm-
> >wm[4].lines,
> - enast(old_wm->wm[5].ignore_lines), old_wm-
> >wm[5].lines,
> - enast(old_wm->wm[6].ignore_lines), old_wm-
> >wm[6].lines,
> - enast(old_wm->wm[7].ignore_lines), old_wm-
> >wm[7].lines,
> - enast(old_wm->trans_wm.ignore_lines), old_wm-
> >trans_wm.lines,
> - enast(old_wm->sagv.wm0.ignore_lines), old_wm-
> >sagv.wm0.lines,
> - enast(old_wm->sagv.trans_wm.ignore_lines),
> old_wm->sagv.trans_wm.lines,
> - enast(new_wm->wm[0].ignore_lines), new_wm-
> >wm[0].lines,
> - enast(new_wm->wm[1].ignore_lines), new_wm-
> >wm[1].lines,
> - enast(new_wm->wm[2].ignore_lines), new_wm-
> >wm[2].lines,
> - enast(new_wm->wm[3].ignore_lines), new_wm-
> >wm[3].lines,
> - enast(new_wm->wm[4].ignore_lines), new_wm-
> >wm[4].lines,
> - enast(new_wm->wm[5].ignore_lines), new_wm-
> >wm[5].lines,
> - enast(new_wm->wm[6].ignore_lines), new_wm-
> >wm[6].lines,
> - enast(new_wm->wm[7].ignore_lines), new_wm-
> >wm[7].lines,
> - enast(new_wm->trans_wm.ignore_lines), new_wm-
> >trans_wm.lines,
> - enast(new_wm->sagv.wm0.ignore_lines), new_wm-
> >sagv.wm0.lines,
> - enast(new_wm->sagv.trans_wm.ignore_lines),
> new_wm->sagv.trans_wm.lines);
> + PLANE_WM_ARGS(old_wm, lines),
> + PLANE_WM_ARGS(new_wm, lines));
>
> drm_dbg_kms(display->drm,
> - "[PLANE:%d:%s] blocks
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> - " ->
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
> + "[PLANE:%d:%s] blocks " PLANE_WM_FMT " -> "
> PLANE_WM_FMT "\n",
> plane->base.base.id, plane->base.name,
> - old_wm->wm[0].blocks, old_wm->wm[1].blocks,
> - old_wm->wm[2].blocks, old_wm->wm[3].blocks,
> - old_wm->wm[4].blocks, old_wm->wm[5].blocks,
> - old_wm->wm[6].blocks, old_wm->wm[7].blocks,
> - old_wm->trans_wm.blocks,
> - old_wm->sagv.wm0.blocks,
> - old_wm->sagv.trans_wm.blocks,
> - new_wm->wm[0].blocks, new_wm->wm[1].blocks,
> - new_wm->wm[2].blocks, new_wm->wm[3].blocks,
> - new_wm->wm[4].blocks, new_wm->wm[5].blocks,
> - new_wm->wm[6].blocks, new_wm->wm[7].blocks,
> - new_wm->trans_wm.blocks,
> - new_wm->sagv.wm0.blocks,
> - new_wm->sagv.trans_wm.blocks);
> + PLANE_WM_ARGS(old_wm, blocks),
> + PLANE_WM_ARGS(new_wm, blocks));
>
> drm_dbg_kms(display->drm,
> - "[PLANE:%d:%s] min_ddb
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> - " ->
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
> + "[PLANE:%d:%s] min_ddb " PLANE_WM_FMT " -> "
> PLANE_WM_FMT "\n",
> plane->base.base.id, plane->base.name,
> - old_wm->wm[0].min_ddb_alloc, old_wm-
> >wm[1].min_ddb_alloc,
> - old_wm->wm[2].min_ddb_alloc, old_wm-
> >wm[3].min_ddb_alloc,
> - old_wm->wm[4].min_ddb_alloc, old_wm-
> >wm[5].min_ddb_alloc,
> - old_wm->wm[6].min_ddb_alloc, old_wm-
> >wm[7].min_ddb_alloc,
> - old_wm->trans_wm.min_ddb_alloc,
> - old_wm->sagv.wm0.min_ddb_alloc,
> - old_wm->sagv.trans_wm.min_ddb_alloc,
> - new_wm->wm[0].min_ddb_alloc, new_wm-
> >wm[1].min_ddb_alloc,
> - new_wm->wm[2].min_ddb_alloc, new_wm-
> >wm[3].min_ddb_alloc,
> - new_wm->wm[4].min_ddb_alloc, new_wm-
> >wm[5].min_ddb_alloc,
> - new_wm->wm[6].min_ddb_alloc, new_wm-
> >wm[7].min_ddb_alloc,
> - new_wm->trans_wm.min_ddb_alloc,
> - new_wm->sagv.wm0.min_ddb_alloc,
> - new_wm->sagv.trans_wm.min_ddb_alloc);
> + PLANE_WM_ARGS(old_wm, min_ddb_alloc),
> + PLANE_WM_ARGS(new_wm, min_ddb_alloc));
>
> if (DISPLAY_VER(display) >= 11)
> return;
>
> drm_dbg_kms(display->drm,
> - "[PLANE:%d:%s] min_ddb_uv
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d"
> - " ->
> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%5d\n",
> + "[PLANE:%d:%s] min_ddb_uv " PLANE_WM_FMT " -> "
> PLANE_WM_FMT "\n",
> plane->base.base.id, plane->base.name,
> - old_wm->wm[0].min_ddb_alloc_uv, old_wm-
> >wm[1].min_ddb_alloc_uv,
> - old_wm->wm[2].min_ddb_alloc_uv, old_wm-
> >wm[3].min_ddb_alloc_uv,
> - old_wm->wm[4].min_ddb_alloc_uv, old_wm-
> >wm[5].min_ddb_alloc_uv,
> - old_wm->wm[6].min_ddb_alloc_uv, old_wm-
> >wm[7].min_ddb_alloc_uv,
> - old_wm->trans_wm.min_ddb_alloc_uv,
> - old_wm->sagv.wm0.min_ddb_alloc_uv,
> - old_wm->sagv.trans_wm.min_ddb_alloc_uv,
> - new_wm->wm[0].min_ddb_alloc_uv, new_wm-
> >wm[1].min_ddb_alloc_uv,
> - new_wm->wm[2].min_ddb_alloc_uv, new_wm-
> >wm[3].min_ddb_alloc_uv,
> - new_wm->wm[4].min_ddb_alloc_uv, new_wm-
> >wm[5].min_ddb_alloc_uv,
> - new_wm->wm[6].min_ddb_alloc_uv, new_wm-
> >wm[7].min_ddb_alloc_uv,
> - new_wm->trans_wm.min_ddb_alloc_uv,
> - new_wm->sagv.wm0.min_ddb_alloc_uv,
> - new_wm->sagv.trans_wm.min_ddb_alloc_uv);
> + PLANE_WM_ARGS(old_wm, min_ddb_alloc_uv),
> + PLANE_WM_ARGS(new_wm, min_ddb_alloc_uv));
> }
>
> +#undef PLANE_WM_EN_FMT
> +#undef PLANE_WM_EN_ARGS
> +#undef PLANE_WM_FMT
> +#undef PLANE_WM_ARGS
> +
> static void
> skl_print_wm_changes(struct intel_atomic_state *state)
> {
next prev parent reply other threads:[~2026-04-08 12:05 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-24 13:48 [PATCH 0/9] drm/i915/wm: Watermark/SAGV fixes/cleanups/etc Ville Syrjala
2026-03-24 13:48 ` [PATCH 1/9] drm/i915/wm: Reject SAGV consistently when block_time_us==0 Ville Syrjala
2026-04-08 9:56 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 2/9] drm/i915/wm: Don't compute separate SAGV watermarks for RKL Ville Syrjala
2026-04-08 11:48 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 3/9] drm/i915/wm: Consolidate SAGV pipe active/interlace checks to common code Ville Syrjala
2026-04-08 11:49 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 4/9] drm/i915/wm: Verify the correct plane DDB entry Ville Syrjala
2026-04-08 11:53 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 5/9] drm/i915/wm: Extract skl_wm_level_verify() Ville Syrjala
2026-04-08 11:55 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 6/9] drm/i915/wm: Extract skl_ddb_entry_verify() Ville Syrjala
2026-04-08 11:57 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 7/9] drm/i915/wm: Verify 'ddb_y' as well as 'ddb' Ville Syrjala
2026-04-08 11:59 ` Govindapillai, Vinod
2026-03-24 13:48 ` [PATCH 8/9] drm/i915/wm: Reduce copy-pasta in skl_print_plane_wm_changes() Ville Syrjala
2026-04-08 12:04 ` Govindapillai, Vinod [this message]
2026-03-24 13:48 ` [PATCH 9/9] drm/i915/wm: Allow SAGV with multiple pipes on pre-icl Ville Syrjala
2026-04-08 12:10 ` Govindapillai, Vinod
2026-03-24 13:58 ` ✗ CI.checkpatch: warning for drm/i915/wm: Watermark/SAGV fixes/cleanups/etc Patchwork
2026-03-24 14:00 ` ✓ CI.KUnit: success " Patchwork
2026-03-24 14:40 ` ✓ Xe.CI.BAT: " Patchwork
2026-03-25 2:08 ` ✓ Xe.CI.FULL: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9959bb3a33ce6fe89a506357c4bbb6db9759eaf9.camel@intel.com \
--to=vinod.govindapillai@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox