[v4 1/3] lib/igt_draw: Dont mmap, if the buffer is already mmapped
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Wed Mar 27 05:09:04 UTC 2024
On Tue, Mar 26, 2024 at 07:44:20PM +0530, Vandita Kulkarni wrote:
> This will help add support to tests which would reuse the
> same fbs and would not want to mmap again and would just
> want to update the fbs.
>
> v2: Handle non null buf ptr cases in non WC mmap cases.
> Squash changes in kms_frontbuffer_tracking which is using
> this function. (Zbigniew)
>
> v3: Fix the buf ptr handling in non WC mmap cases (Zbigniew)
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
We discussed about this offline and solution with passing fb mapping
in driver_priv field seems to be simplest way atm.
> ---
> lib/igt_draw.c | 61 ++++++++++++++++----------
> lib/igt_draw.h | 2 +-
> tests/intel/kms_frontbuffer_tracking.c | 2 +-
> 3 files changed, 39 insertions(+), 26 deletions(-)
>
> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> index 2c01d7b02..a4d5f2f0e 100644
> --- a/lib/igt_draw.c
> +++ b/lib/igt_draw.c
> @@ -75,6 +75,7 @@ struct buf_data {
> uint32_t handle;
> uint32_t size;
> uint32_t stride;
> + void *buf_ptr;
> int width;
> int height;
> int bpp;
> @@ -448,6 +449,8 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
> {
> uint32_t *ptr;
>
> + igt_assert(buf->buf_ptr == NULL);
> +
> gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_CPU,
> I915_GEM_DOMAIN_CPU);
>
> @@ -483,6 +486,8 @@ static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect,
> {
> uint32_t *ptr;
>
> + igt_assert(buf->buf_ptr == NULL);
> +
> gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_GTT,
> I915_GEM_DOMAIN_GTT);
>
> @@ -499,28 +504,32 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
> {
> uint32_t *ptr;
>
> - if (is_i915_device(fd)) {
> - gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_GTT,
> - I915_GEM_DOMAIN_GTT);
> -
> - /* We didn't implement suport for the older tiling methods yet. */
> - if (tiling != I915_TILING_NONE)
> - igt_require(intel_display_ver(intel_get_drm_devid(fd)) >= 5);
> -
> - if (gem_has_lmem(fd))
> - ptr = gem_mmap_offset__fixed(fd, buf->handle, 0,
> - PAGE_ALIGN(buf->size),
> - PROT_READ | PROT_WRITE);
> - else if (gem_has_legacy_mmap(fd))
> - ptr = gem_mmap__wc(fd, buf->handle, 0, PAGE_ALIGN(buf->size),
> - PROT_READ | PROT_WRITE);
> - else
> - ptr = gem_mmap_offset__wc(fd, buf->handle, 0,
> - PAGE_ALIGN(buf->size),
> - PROT_READ | PROT_WRITE);
> + if (!buf->buf_ptr) {
> + if (is_i915_device(fd)) {
> + gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_GTT,
> + I915_GEM_DOMAIN_GTT);
> +
> + /* We didn't implement suport for the older tiling methods yet. */
> + if (tiling != I915_TILING_NONE)
> + igt_require(intel_display_ver(intel_get_drm_devid(fd)) >= 5);
> +
> + if (gem_has_lmem(fd))
> + ptr = gem_mmap_offset__fixed(fd, buf->handle, 0,
> + PAGE_ALIGN(buf->size),
> + PROT_READ | PROT_WRITE);
> + else if (gem_has_legacy_mmap(fd))
> + ptr = gem_mmap__wc(fd, buf->handle, 0, PAGE_ALIGN(buf->size),
> + PROT_READ | PROT_WRITE);
> + else
> + ptr = gem_mmap_offset__wc(fd, buf->handle, 0,
> + PAGE_ALIGN(buf->size),
> + PROT_READ | PROT_WRITE);
> + } else {
> + ptr = xe_bo_mmap_ext(fd, buf->handle, buf->size,
> + PROT_READ | PROT_WRITE);
> + }
> } else {
> - ptr = xe_bo_mmap_ext(fd, buf->handle, buf->size,
> - PROT_READ | PROT_WRITE);
> + ptr = buf->buf_ptr;
> }
>
> switch (tiling) {
> @@ -538,7 +547,8 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
> break;
> }
>
> - igt_assert(gem_munmap(ptr, buf->size) == 0);
> + if (ptr != buf->buf_ptr)
> + igt_assert(gem_munmap(ptr, buf->size) == 0);
> }
>
> static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
> @@ -839,6 +849,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_ptr: if required can be used to pass already mmapped buffer ptr
> + * only in case of wc mmap, can be NULL in any other case.
I think you should write 'should be NULL' instead 'can be NULL' as you
don't support cpu/gtt mapping.
With this minor fix:
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
--
Zbigniew
> * @buf_width: the width of the buffer
> * @buf_height: the height of the buffer
> * @tiling: the tiling of the buffer
> @@ -855,7 +867,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,
> + void *buf_ptr, 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)
> @@ -870,6 +882,7 @@ void igt_draw_rect(int fd, struct buf_ops *bops, uint32_t ctx,
> .handle = buf_handle,
> .size = buf_size,
> .stride = buf_stride,
> + .buf_ptr = buf_ptr,
> .width = buf_width,
> .height = buf_height,
> .bpp = bpp,
> @@ -935,7 +948,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,
> + fb->driver_priv, 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 1dec95e86..865641ef6 100644
> --- a/lib/igt_draw.h
> +++ b/lib/igt_draw.h
> @@ -54,7 +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,
> + void *buf_ptr, 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_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index db2db0ff1..392539408 100644
> --- a/tests/intel/kms_frontbuffer_tracking.c
> +++ b/tests/intel/kms_frontbuffer_tracking.c
> @@ -2254,7 +2254,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.size, busy_thread.stride, NULL,
> busy_thread.width, busy_thread.height,
> busy_thread.tiling, IGT_DRAW_BLT, 0, 0,
> busy_thread.width, busy_thread.height,
> --
> 2.43.2
>
More information about the igt-dev
mailing list