From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id C76E66EE89 for ; Tue, 11 Feb 2020 15:33:47 +0000 (UTC) Date: Tue, 11 Feb 2020 07:33:46 -0800 From: Matt Roper Message-ID: <20200211153346.GA1588865@mdroper-desk1.amr.corp.intel.com> References: <20200211023108.25369-1-imre.deak@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200211023108.25369-1-imre.deak@intel.com> Subject: Re: [igt-dev] [PATCH v2 1/8] lib/igt_draw: Refactor get_tiling calls List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Imre Deak Cc: igt-dev@lists.freedesktop.org, Vanshidhar Konda List-ID: On Tue, Feb 11, 2020 at 04:31:01AM +0200, Imre Deak wrote: > From: Vanshidhar Konda > > Simplify the number of places from which gem_get_tiling method is called > and call it only if the device has support in hardware for tiling. > > For consistency also fix up the gem_mmap__cpu() prot argument in > draw_rect_mmap_cpu(). Atm, gem_mmap__cpu() ignores this, however the > implementation of it may change, so make sure we pass the correct > protection flags. > > v2: > - Use gem_available_fences() to check for HW detiling. > v3: (Matt) > - Fix docbook for draw_rect_render() > - Remove unused swizzle params from the blt/render draw funcs. > - Describe the gem_mmap__cpu() prot argument fix in the commit log. > > Cc: Matt Roper > Signed-off-by: Vanshidhar Konda > Signed-off-by: Imre Deak Reviewed-by: Matt Roper > --- > lib/igt_draw.c | 57 ++++++++++++++++---------------- > lib/igt_draw.h | 5 +-- > tests/kms_frontbuffer_tracking.c | 8 +++-- > 3 files changed, 36 insertions(+), 34 deletions(-) > > diff --git a/lib/igt_draw.c b/lib/igt_draw.c > index 6950bc49..fa8ab562 100644 > --- a/lib/igt_draw.c > +++ b/lib/igt_draw.c > @@ -333,20 +333,19 @@ static void draw_rect_ptr_tiled(void *ptr, uint32_t stride, uint32_t tiling, > } > > static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect, > - uint32_t color) > + uint32_t tiling, uint32_t swizzle, uint32_t color) > { > uint32_t *ptr; > - uint32_t tiling, swizzle; > > gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_CPU, > I915_GEM_DOMAIN_CPU); > - igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle)); > > /* We didn't implement suport for the older tiling methods yet. */ > if (tiling != I915_TILING_NONE) > igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5); > > - ptr = gem_mmap__cpu(fd, buf->handle, 0, PAGE_ALIGN(buf->size), 0); > + ptr = gem_mmap__cpu(fd, buf->handle, 0, PAGE_ALIGN(buf->size), > + PROT_READ | PROT_WRITE); > > switch (tiling) { > case I915_TILING_NONE: > @@ -384,14 +383,12 @@ static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect, > } > > static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect, > - uint32_t color) > + uint32_t tiling, uint32_t swizzle, uint32_t color) > { > uint32_t *ptr; > - uint32_t tiling, swizzle; > > gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_GTT, > I915_GEM_DOMAIN_GTT); > - igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle)); > > /* We didn't implement suport for the older tiling methods yet. */ > if (tiling != I915_TILING_NONE) > @@ -495,12 +492,9 @@ static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf, > } > > static void draw_rect_pwrite(int fd, struct buf_data *buf, > - struct rect *rect, uint32_t color) > + struct rect *rect, uint32_t tiling, > + uint32_t swizzle, uint32_t color) > { > - uint32_t tiling, swizzle; > - > - igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle)); > - > switch (tiling) { > case I915_TILING_NONE: > draw_rect_pwrite_untiled(fd, buf, rect, color); > @@ -517,18 +511,15 @@ static void draw_rect_pwrite(int fd, struct buf_data *buf, > > static void draw_rect_blt(int fd, struct cmd_data *cmd_data, > struct buf_data *buf, struct rect *rect, > - uint32_t color) > + uint32_t tiling, uint32_t color) > { > drm_intel_bo *dst; > struct intel_batchbuffer *batch; > int blt_cmd_len, blt_cmd_tiling, blt_cmd_depth; > uint32_t devid = intel_get_drm_devid(fd); > int gen = intel_gen(devid); > - uint32_t tiling, swizzle; > int pitch; > > - igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle)); > - > dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle); > igt_assert(dst); > > @@ -574,28 +565,25 @@ static void draw_rect_blt(int fd, struct cmd_data *cmd_data, > > static void draw_rect_render(int fd, struct cmd_data *cmd_data, > struct buf_data *buf, struct rect *rect, > - uint32_t color) > + uint32_t tiling, uint32_t color) > { > drm_intel_bo *src, *dst; > uint32_t devid = intel_get_drm_devid(fd); > igt_render_copyfunc_t rendercopy = igt_get_render_copyfunc(devid); > struct igt_buf src_buf = {}, dst_buf = {}; > struct intel_batchbuffer *batch; > - uint32_t tiling, swizzle; > struct buf_data tmp; > int pixel_size = buf->bpp / 8; > > igt_skip_on(!rendercopy); > > - igt_require(gem_get_tiling(fd, buf->handle, &tiling, &swizzle)); > - > /* We create a temporary buffer and copy from it using rendercopy. */ > tmp.size = rect->w * rect->h * pixel_size; > tmp.handle = gem_create(fd, tmp.size); > tmp.stride = rect->w * pixel_size; > tmp.bpp = buf->bpp; > draw_rect_mmap_cpu(fd, &tmp, &(struct rect){0, 0, rect->w, rect->h}, > - color); > + I915_TILING_NONE, I915_BIT_6_SWIZZLE_NONE, color); > > src = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", tmp.handle); > igt_assert(src); > @@ -634,6 +622,7 @@ 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 > + * @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 > * @rect_y: vertical position on the buffer where your rectangle starts > @@ -647,9 +636,12 @@ static void draw_rect_render(int fd, struct cmd_data *cmd_data, > */ > void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context, > uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride, > - enum igt_draw_method method, int rect_x, int rect_y, > - int rect_w, int rect_h, uint32_t color, int bpp) > + 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) > { > + uint32_t buf_tiling, swizzle; > + > struct cmd_data cmd_data = { > .bufmgr = bufmgr, > .context = context, > @@ -667,24 +659,30 @@ void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context, > .h = rect_h, > }; > > + swizzle = I915_BIT_6_SWIZZLE_NONE; > + if (tiling != I915_TILING_NONE && gem_available_fences(fd)) { > + gem_get_tiling(fd, buf_handle, &buf_tiling, &swizzle); > + igt_assert(tiling == buf_tiling); > + } > + > switch (method) { > case IGT_DRAW_MMAP_CPU: > - draw_rect_mmap_cpu(fd, &buf, &rect, color); > + draw_rect_mmap_cpu(fd, &buf, &rect, tiling, swizzle, color); > break; > case IGT_DRAW_MMAP_GTT: > draw_rect_mmap_gtt(fd, &buf, &rect, color); > break; > case IGT_DRAW_MMAP_WC: > - draw_rect_mmap_wc(fd, &buf, &rect, color); > + draw_rect_mmap_wc(fd, &buf, &rect, tiling, swizzle, color); > break; > case IGT_DRAW_PWRITE: > - draw_rect_pwrite(fd, &buf, &rect, color); > + draw_rect_pwrite(fd, &buf, &rect, tiling, swizzle, color); > break; > case IGT_DRAW_BLT: > - draw_rect_blt(fd, &cmd_data, &buf, &rect, color); > + draw_rect_blt(fd, &cmd_data, &buf, &rect, tiling, color); > break; > case IGT_DRAW_RENDER: > - draw_rect_render(fd, &cmd_data, &buf, &rect, color); > + draw_rect_render(fd, &cmd_data, &buf, &rect, tiling, color); > break; > default: > igt_assert(false); > @@ -715,7 +713,8 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr, > int rect_w, int rect_h, uint32_t color) > { > igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, fb->strides[0], > - method, rect_x, rect_y, rect_w, rect_h, color, > + 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 b030131e..ec146754 100644 > --- a/lib/igt_draw.h > +++ b/lib/igt_draw.h > @@ -52,8 +52,9 @@ const char *igt_draw_get_method_name(enum igt_draw_method method); > > void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context, > uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride, > - enum igt_draw_method method, int rect_x, int rect_y, > - int rect_w, int rect_h, uint32_t color, int bpp); > + 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); > > void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr, > drm_intel_context *context, struct igt_fb *fb, > diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c > index 2c765c34..9c2c3a5d 100644 > --- a/tests/kms_frontbuffer_tracking.c > +++ b/tests/kms_frontbuffer_tracking.c > @@ -291,6 +291,7 @@ struct { > int height; > uint32_t color; > int bpp; > + uint32_t tiling; > } busy_thread = { > .stop = true, > }; > @@ -1126,9 +1127,9 @@ static void *busy_thread_func(void *data) > while (!busy_thread.stop) > igt_draw_rect(drm.fd, drm.bufmgr, NULL, busy_thread.handle, > busy_thread.size, busy_thread.stride, > - IGT_DRAW_BLT, 0, 0, busy_thread.width, > - busy_thread.height, busy_thread.color, > - busy_thread.bpp); > + busy_thread.tiling, IGT_DRAW_BLT, 0, 0, > + busy_thread.width, busy_thread.height, > + busy_thread.color, busy_thread.bpp); > > pthread_exit(0); > } > @@ -1146,6 +1147,7 @@ static void start_busy_thread(struct igt_fb *fb) > busy_thread.height = fb->height; > busy_thread.color = pick_color(fb, COLOR_PRIM_BG); > busy_thread.bpp = igt_drm_format_to_bpp(fb->drm_format); > + busy_thread.tiling = igt_fb_mod_to_tiling(fb->modifier); > > rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL); > igt_assert_eq(rc, 0); > -- > 2.23.1 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev