[igt-dev] [PATCH i-g-t] tests/i915: Tests modified to use COPY Command modification for gen12+ platforms
Kumar, Sinjan
sinjan.kumar at intel.com
Mon Dec 5 12:45:37 UTC 2022
Hi Vikas,
Please find the review comments inline. In general, please mention changes in each version.
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Vikas
> Srivastava
> Sent: Thursday, December 1, 2022 6:59 PM
> To: igt-dev at lists.freedesktop.org; kamil.konieczny at linux.intel.com
> Subject: [igt-dev] [PATCH i-g-t] tests/i915: Tests modified to use COPY
> Command modification for gen12+ platforms
>
> From: Arjun Melkaveri <arjun.melkaveri at intel.com>
>
> gem_partial_pwrite_pread , gem_evict_everythig and gem_evict_alignment
> were using legacy command which is not supported on gen12+ platforms.
> Modified test to use XY_FAST_COPY_BLT.
>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> Signed-off-by: vikas srivastava <vikas.srivastava at intel.com>
> ---
> tests/i915/gem_evict_alignment.c | 25 ++++++++++++++++---------
> tests/i915/gem_evict_everything.c | 25 ++++++++++++++++---------
> tests/i915/gem_partial_pwrite_pread.c | 17 +++++++++++------
> 3 files changed, 43 insertions(+), 24 deletions(-)
>
> diff --git a/tests/i915/gem_evict_alignment.c
> b/tests/i915/gem_evict_alignment.c
> index 0b560ab03..36045de20 100644
> --- a/tests/i915/gem_evict_alignment.c
> +++ b/tests/i915/gem_evict_alignment.c
> @@ -64,15 +64,22 @@ copy(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo,
> struct drm_i915_gem_execbuffer2 exec;
> uint32_t handle;
> int n, i=0;
> -
> - batch[i++] = (XY_SRC_COPY_BLT_CMD |
> - XY_SRC_COPY_BLT_WRITE_ALPHA |
> - XY_SRC_COPY_BLT_WRITE_RGB | 6);
> - if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> - batch[i - 1] += 2;
> - batch[i++] = (3 << 24) | /* 32 bits */
> - (0xcc << 16) | /* copy ROP */
> - WIDTH*4;
> + uint32_t devid;
> +
> + devid = intel_get_drm_devid(fd);
> + if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
> + batch[i++] = XY_FAST_COPY_BLT;
> + batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
> + } else {
> + batch[i++] = (XY_SRC_COPY_BLT_CMD |
> + XY_SRC_COPY_BLT_WRITE_ALPHA |
> + XY_SRC_COPY_BLT_WRITE_RGB | 6);
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + batch[i - 1] += 2;
> + batch[i++] = (3 << 24) | /* 32 bits */
> + (0xcc << 16) | /* copy ROP */
> + WIDTH*4;
> + }
> batch[i++] = 0; /* dst x1,y1 */
> batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> batch[i++] = 0; /* dst reloc */
> diff --git a/tests/i915/gem_evict_everything.c
> b/tests/i915/gem_evict_everything.c
> index 120f89072..995df2e82 100644
> --- a/tests/i915/gem_evict_everything.c
> +++ b/tests/i915/gem_evict_everything.c
> @@ -64,15 +64,22 @@ copy(int fd, uint32_t dst, uint32_t src, uint32_t *all_bo,
> int n_bo)
> struct drm_i915_gem_execbuffer2 exec;
> uint32_t handle;
> int n, ret, i=0;
> -
> - batch[i++] = (XY_SRC_COPY_BLT_CMD |
> - XY_SRC_COPY_BLT_WRITE_ALPHA |
> - XY_SRC_COPY_BLT_WRITE_RGB | 6);
> - if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> - batch[i - 1] += 2;
> - batch[i++] = (3 << 24) | /* 32 bits */
> - (0xcc << 16) | /* copy ROP */
> - WIDTH*4;
> + uint32_t devid;
> +
> + devid = intel_get_drm_devid(fd);
> + if (intel_graphics_ver(devid) >= IP_VER(12, 60)) {
> + batch[i++] = XY_FAST_COPY_BLT;
> + batch[i++] = XY_FAST_COPY_COLOR_DEPTH_32 | WIDTH*4;
> + } else {
> + batch[i++] = (XY_SRC_COPY_BLT_CMD |
> + XY_SRC_COPY_BLT_WRITE_ALPHA |
> + XY_SRC_COPY_BLT_WRITE_RGB | 6);
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + batch[i - 1] += 2;
> + batch[i++] = (3 << 24) | /* 32 bits */
> + (0xcc << 16) | /* copy ROP */
> + WIDTH*4;
> + }
> batch[i++] = 0; /* dst x1,y1 */
> batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> batch[i++] = 0; /* dst reloc */
> diff --git a/tests/i915/gem_partial_pwrite_pread.c
> b/tests/i915/gem_partial_pwrite_pread.c
> index 474149d48..9b5b05700 100644
> --- a/tests/i915/gem_partial_pwrite_pread.c
> +++ b/tests/i915/gem_partial_pwrite_pread.c
> @@ -86,12 +86,17 @@ static void copy_bo(struct intel_bb *ibb,
> intel_bb_add_intel_buf(ibb, src, false);
> intel_bb_add_intel_buf(ibb, dst, true);
>
> - intel_bb_out(ibb,
> - XY_SRC_COPY_BLT_CMD |
> - XY_SRC_COPY_BLT_WRITE_ALPHA |
> - XY_SRC_COPY_BLT_WRITE_RGB |
> - (6 + 2 * has_64b_reloc));
> - intel_bb_out(ibb, 3 << 24 | 0xcc << 16 | 4096);
> + if (intel_graphics_ver(ibb->devid) >= IP_VER(12, 60)) {
> + intel_bb_out(ibb, XY_FAST_COPY_BLT);
> + intel_bb_out(ibb, XY_FAST_COPY_COLOR_DEPTH_32 | 4096);
[Kumar, Sinjan] IMO, it's better to #define some meaningful name for 4096 as this value may change on future chipsets
> + } else {
> + intel_bb_out(ibb,
> + XY_SRC_COPY_BLT_CMD |
> + XY_SRC_COPY_BLT_WRITE_ALPHA |
> + XY_SRC_COPY_BLT_WRITE_RGB |
> + (6 + 2 * has_64b_reloc));
> + intel_bb_out(ibb, 3 << 24 | 0xcc << 16 | 4096);
> + }
> intel_bb_out(ibb, 0 << 16 | 0);
> intel_bb_out(ibb, (BO_SIZE/4096) << 16 | 1024);
> intel_bb_emit_reloc_fenced(ibb, dst->handle,
> --
> 2.25.1
[Kumar, Sinjan] Rest LGTM.
More information about the igt-dev
mailing list