From: Matt Roper <matthew.d.roper@intel.com>
To: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 1/3] drm/xe: Refactor emit_clear_main_copy
Date: Thu, 7 May 2026 10:45:17 -0700 [thread overview]
Message-ID: <20260507174517.GO2131374@mdroper-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <20260507171643.GM2131374@mdroper-desk1.amr.corp.intel.com>
On Thu, May 07, 2026 at 10:16:43AM -0700, Matt Roper wrote:
> On Thu, May 07, 2026 at 12:12:31PM +0530, Balasubramani Vivekanandan wrote:
> > Implement a function which returns the length of XY_FAST_COLOR_BLT
> > instruction instead of hardcoding it inside the emit_clear_main_copy.
> > In future platforms, the length of this instruction is expected to
> > change and this patch helps in preparing for it.
BTW, it's worth noting that even if this instruction changes length on
future platforms, I don't think we expect to use this instruction for
our current code flows. Everything Xe2 and later (and also PVC)
supports service copy engines and the new MEM_SET/MEM_COPY instructions
that were added as part of that hardware change. So
"emit_clear_main_copy" is really a function that's only intended for use
on non-PVC Xe1 platforms.
That said, there's always the possibility that we'll need to use the
legacy XY_FAST_COLOR / XY_FAST_COPY instructions for something specific
in the future (e.g., a hardware workaround that asks us to use those
instead of MEM_SET/MEM_COPY), so it's still good to capture the
instruction changes in the driver, even if we don't expect them to be
relevant for now.
Matt
> >
> > Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > ---
> > .../gpu/drm/xe/instructions/xe_gpu_commands.h | 2 +-
> > drivers/gpu/drm/xe/xe_migrate.c | 16 ++++++++++++----
> > 2 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h b/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
> > index 885fcf211e6d..082f2926feab 100644
> > --- a/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
> > +++ b/drivers/gpu/drm/xe/instructions/xe_gpu_commands.h
> > @@ -20,7 +20,7 @@
> >
> > #define XY_FAST_COLOR_BLT_CMD (2 << 29 | 0x44 << 22)
> > #define XY_FAST_COLOR_BLT_DEPTH_32 (2 << 19)
> > -#define XY_FAST_COLOR_BLT_DW 16
> > +#define XY_FAST_COLOR_BLT_LEN_DW 16
>
> It might be best to just drop the #define completely if it's only going
> to apply to a subset of our platforms. Leaving the define here risks
> someone seeing it and trying to use it elsewhere in the driver where it
> isn't actually correct. E.g., it looks like the existing usage in
> emit_clear_cmd_len() was probably incorrect since Xe_LP platforms
> actually needed a different value (11)?
>
>
> Matt
>
> > #define XY_FAST_COLOR_BLT_MOCS_MASK GENMASK(27, 22)
> > #define XE2_XY_FAST_COLOR_BLT_MOCS_INDEX_MASK GENMASK(27, 24)
> > #define XY_FAST_COLOR_BLT_MEM_TYPE_SHIFT 31
> > diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> > index 078a9bc2821d..e4afe011f246 100644
> > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> > @@ -1409,15 +1409,21 @@ static void emit_clear_link_copy(struct xe_gt *gt, struct xe_bb *bb, u64 src_ofs
> > bb->len += len;
> > }
> >
> > +static u32 blt_fast_color_cmd_len(struct xe_device *xe)
> > +{
> > + if (GRAPHICS_VERx100(xe) >= 1250)
> > + return XY_FAST_COLOR_BLT_LEN_DW;
> > + else
> > + return 11;
> > +}
> > +
> > static void emit_clear_main_copy(struct xe_gt *gt, struct xe_bb *bb,
> > u64 src_ofs, u32 size, u32 pitch, bool is_vram)
> > {
> > struct xe_device *xe = gt_to_xe(gt);
> > u32 *cs = bb->cs + bb->len;
> > - u32 len = XY_FAST_COLOR_BLT_DW;
> > + u32 len = blt_fast_color_cmd_len(xe);
> >
> > - if (GRAPHICS_VERx100(xe) < 1250)
> > - len = 11;
> >
> > *cs++ = XY_FAST_COLOR_BLT_CMD | XY_FAST_COLOR_BLT_DEPTH_32 |
> > (len - 2);
> > @@ -1466,10 +1472,12 @@ static bool has_service_copy_support(struct xe_gt *gt)
> >
> > static u32 emit_clear_cmd_len(struct xe_gt *gt)
> > {
> > + struct xe_device *xe = gt_to_xe(gt);
> > +
> > if (has_service_copy_support(gt))
> > return PVC_MEM_SET_CMD_LEN_DW;
> > else
> > - return XY_FAST_COLOR_BLT_DW;
> > + return blt_fast_color_cmd_len(xe);
> > }
> >
> > static void emit_clear(struct xe_gt *gt, struct xe_bb *bb, u64 src_ofs,
> > --
> > 2.43.0
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
next prev parent reply other threads:[~2026-05-07 17:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-07 6:42 [PATCH 0/3] Refactor functions implementing the blt batch buffer Balasubramani Vivekanandan
2026-05-07 6:42 ` [PATCH 1/3] drm/xe: Refactor emit_clear_main_copy Balasubramani Vivekanandan
2026-05-07 17:16 ` Matt Roper
2026-05-07 17:45 ` Matt Roper [this message]
2026-05-07 6:42 ` [PATCH 2/3] drm/xe: Refactor emit_clear_link_copy Balasubramani Vivekanandan
2026-05-07 17:18 ` Matt Roper
2026-05-07 6:42 ` [PATCH 3/3] drm/xe: Refactor emit_xy_fast_copy and emit_mem_copy functions Balasubramani Vivekanandan
2026-05-07 6:44 ` ✓ CI.KUnit: success for Refactor functions implementing the blt batch buffer Patchwork
2026-05-07 7:45 ` ✓ Xe.CI.BAT: " Patchwork
2026-05-07 17:35 ` ✗ Xe.CI.FULL: failure " 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=20260507174517.GO2131374@mdroper-desk1.amr.corp.intel.com \
--to=matthew.d.roper@intel.com \
--cc=balasubramani.vivekanandan@intel.com \
--cc=intel-xe@lists.freedesktop.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).