Re: [igt-dev] [PATCH i-g-t 1/8] lib/igt_draw: Refactor get_tiling calls

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 07 Feb 2020 11:15:17 -0800, Imre Deak wrote:
>  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 +660,32 @@ 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);
> +	}

Probably a nit, but looks a little strange to call gem_get_tiling() to get
the swizzle and then doing an assert. Instead, since igt_draw_rect() has
only two callers how about moving the gem_get_tiling() call into the
callers and passing both tiling and swizzle into igt_draw_rect()?

> 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;

Should not need this if we follow the suggestion above.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev