[i-g-t V2] lib/igt_draw: Fix the usage of FB width & height
B, Jeevan
jeevan.b at intel.com
Thu Feb 1 05:02:03 UTC 2024
> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Sent: Tuesday, January 30, 2024 9:06 PM
> To: B, Jeevan <jeevan.b at intel.com>; igt-dev at lists.freedesktop.org
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> Subject: Re: [i-g-t V2] lib/igt_draw: Fix the usage of FB width & height
>
> Hi Jeevan,
>
> On 30-01-2024 08:51 pm, B, Jeevan wrote:
> >> -----Original Message-----
> >> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> >> Bhanuprakash Modem
> >> Sent: Tuesday, January 30, 2024 5:07 PM
> >> To: igt-dev at lists.freedesktop.org
> >> Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> >> Subject: [i-g-t V2] lib/igt_draw: Fix the usage of FB width & height
> >>
> >> Aliging to xe default alignment is causing the mismatch in derived
> >> height from framebuffer size (height = size/ stride). Instead use the
> >> original width & height from the created fb.
> >>
> >> V2: - Fix issues in CI
> >>
> >> Fixes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/608
> >> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> >> Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> >> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
LGTM
Reviewed-by: Jeevan B <jeevan.b at intel.com>
> >> ---
> >> lib/igt_draw.c | 16 +++++++++++++---
> >> lib/igt_draw.h | 1 +
> >> tests/intel/kms_big_fb.c | 7 +++----
> >> tests/intel/kms_frontbuffer_tracking.c | 1 +
> >> 4 files changed, 18 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/lib/igt_draw.c b/lib/igt_draw.c index
> >> 637b9ba76..0757e9801 100644
> >> --- a/lib/igt_draw.c
> >> +++ b/lib/igt_draw.c
> >> @@ -75,6 +75,8 @@ struct buf_data {
> >> uint32_t handle;
> >> uint32_t size;
> >> uint32_t stride;
> >> + int width;
> >> + int height;
> >> int bpp;
> >> uint8_t pat_index;
> >> };
> >> @@ -648,8 +650,8 @@ static struct intel_buf *create_buf(int fd,
> >> struct buf_ops *bops,
> >> uint64_t region = driver == INTEL_DRIVER_XE ? vram_if_possible(fd,
> >> 0) : - 1;
> >> uint64_t size = from->size;
> >>
> >> - width = from->stride / (from->bpp / 8);
> >> - height = from->size / from->stride;
> >> + width = from->width;
> >> + height = from->height;
> >> if (driver == INTEL_DRIVER_XE)
> >> size = ALIGN(size, xe_get_default_alignment(fd));
> >>
> >> @@ -686,7 +688,7 @@ static void draw_rect_blt(int fd, struct cmd_data
> >> *cmd_data,
> >> intel_bb_add_intel_buf(ibb, dst, true);
> >>
> >> if (HAS_4TILE(intel_get_drm_devid(fd))) {
> >> - int buf_height = buf->size / buf->stride;
> >> + int buf_height = buf->height;
> >>
> >> switch (buf->bpp) {
> >> case 8:
> >> @@ -807,6 +809,8 @@ static void draw_rect_render(int fd, struct
> >> cmd_data *cmd_data,
> >>
> >> tmp.stride = rect->w * pixel_size;
> >> tmp.bpp = buf->bpp;
> >> + tmp.width = rect->w;
> >> + tmp.height = rect->h;
> >
> > Is this required ? I think these are not required.
> > Please correct me if I am wrong.
>
> This is required, otherwise create_buf() [*] may fail due to the garbage values of
> width & height.
>
> [*]: https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/lib/igt_draw.c#n817
>
> - Bhanu
>
> >
> >> if (is_i915_device(fd))
> >> draw_rect_mmap_cpu(fd, &tmp, &(struct rect){0, 0, rect->w,
> >> rect->h},
> >> I915_TILING_NONE,
> >> I915_BIT_6_SWIZZLE_NONE, color); @@ -834,6 +838,8 @@ static void
> >> draw_rect_render(int fd, struct cmd_data *cmd_data,
> >> * @buf_handle: the handle of the buffer where you're going to draw to
> >> * @buf_size: the size of the buffer
> >> * @buf_stride: the stride of the buffer
> >> + * @buf_width: the width of the buffer
> >> + * @buf_height: the height of the buffer
> >> * @tiling: the tiling of the buffer
> >> * @method: method you're going to use to write to the buffer
> >> * @rect_x: horizontal position on the buffer where your rectangle
> >> starts @@ -
> >> 848,6 +854,7 @@ static void draw_rect_render(int fd, struct cmd_data
> >> *cmd_data,
> >> */
> >> void igt_draw_rect(int fd, struct buf_ops *bops, uint32_t ctx,
> >> uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
> >> + int buf_width, int buf_height,
> >> uint32_t tiling, enum igt_draw_method method,
> >> int rect_x, int rect_y, int rect_w, int rect_h,
> >> uint32_t color, int bpp)
> >> @@ -862,6 +869,8 @@ void igt_draw_rect(int fd, struct buf_ops *bops,
> >> uint32_t ctx,
> >> .handle = buf_handle,
> >> .size = buf_size,
> >> .stride = buf_stride,
> >> + .width = buf_width,
> >> + .height = buf_height,
> >> .bpp = bpp,
> >> .pat_index = intel_get_pat_idx_uc(fd),
> >> };
> >> @@ -925,6 +934,7 @@ void igt_draw_rect_fb(int fd, struct buf_ops *bops,
> >> int rect_w, int rect_h, uint32_t color) {
> >> igt_draw_rect(fd, bops, ctx, fb->gem_handle, fb->size,
> >> fb->strides[0],
> >> + fb->width, fb->height,
> >> igt_fb_mod_to_tiling(fb->modifier), method,
> >> rect_x, rect_y, rect_w, rect_h, color,
> >> igt_drm_format_to_bpp(fb->drm_format));
> >> diff --git a/lib/igt_draw.h b/lib/igt_draw.h index
> >> fe7531b79..1dec95e86 100644
> >> --- a/lib/igt_draw.h
> >> +++ b/lib/igt_draw.h
> >> @@ -54,6 +54,7 @@ bool igt_draw_supports_method(int fd, enum
> >> igt_draw_method method);
> >>
> >> void igt_draw_rect(int fd, struct buf_ops *bops, uint32_t ctx,
> >> uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
> >> + int buf_width, int buf_height,
> >> uint32_t tiling, enum igt_draw_method method,
> >> int rect_x, int rect_y, int rect_w, int rect_h,
> >> uint32_t color, int bpp);
> >> diff --git a/tests/intel/kms_big_fb.c b/tests/intel/kms_big_fb.c
> >> index 3eb43515a..0bd79394b 100644
> >> --- a/tests/intel/kms_big_fb.c
> >> +++ b/tests/intel/kms_big_fb.c
> >> @@ -189,18 +189,17 @@ static struct intel_buf *init_buf(data_t *data, {
> >> struct intel_buf *buf;
> >> enum intel_driver driver = buf_ops_get_driver(data->bops);
> >> - uint32_t name, handle, tiling, stride, width, height, bpp, size;
> >> + uint32_t name, handle, tiling, width, height, bpp, size;
> >> uint64_t region = driver == INTEL_DRIVER_XE ?
> >> vram_if_possible(data->drm_fd, 0) : -1;
> >>
> >> igt_assert_eq(fb->offsets[0], 0);
> >>
> >> tiling = igt_fb_mod_to_tiling(fb->modifier);
> >> - stride = fb->strides[0];
> >> bpp = fb->plane_bpp[0];
> >> size = fb->size;
> >> - width = stride / (bpp / 8);
> >> - height = size / stride;
> >> + width = fb->width;
> >> + height = fb->height;
> >>
> >> name = gem_flink(data->drm_fd, fb->gem_handle);
> >> handle = gem_open(data->drm_fd, name); diff --git
> >> a/tests/intel/kms_frontbuffer_tracking.c
> >> b/tests/intel/kms_frontbuffer_tracking.c
> >> index 2a2690d18..912cca3f8 100644
> >> --- a/tests/intel/kms_frontbuffer_tracking.c
> >> +++ b/tests/intel/kms_frontbuffer_tracking.c
> >> @@ -2242,6 +2242,7 @@ static void *busy_thread_func(void *data)
> >> while (!busy_thread.stop)
> >> igt_draw_rect(drm.fd, drm.bops, 0, busy_thread.handle,
> >> busy_thread.size, busy_thread.stride,
> >> + busy_thread.width, busy_thread.height,
> >> busy_thread.tiling, IGT_DRAW_BLT, 0, 0,
> >> busy_thread.width, busy_thread.height,
> >> busy_thread.color, busy_thread.bpp);
> >> --
> >> 2.43.0
> >
More information about the igt-dev
mailing list