Re: [PATCH i-g-t] lib: Pass tiling constant where that's expected

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Hi,

On 10/11/2016 10:12, Tomeu Vizoso wrote:
> We were passing in two places a framebuffer modifier constant instead of
> a tiling constant.
>
> Also adds igt_fb_mod_to_tiling so tests can do that by themselves.
>
> Cc: Tvrtko Ursulin <tursulin@ursulin.net>
> Fixes: 8a1a38661f56 ("lib: Add igt_create_bo_with_dimensions")
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  lib/igt_fb.c     | 48 +++++++++++++++++++++++++++++-------------------
>  lib/igt_fb.h     |  2 ++
>  tests/kms_flip.c |  2 +-
>  3 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 47472f4399e9..bb87869f18ce 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -180,6 +180,32 @@ void igt_calc_fb_size(int fd, int width, int height, int bpp, uint64_t tiling,
>  	*size_ret = size;
>  }
>
> +/**
> + * igt_fb_mod_to_tiling:
> + * @modifier: DRM framebuffer modifier
> + *
> + * This function converts a DRM framebuffer modifier to its corresponding
> + * tiling constant.
> + *
> + * Returns:
> + * A tiling constant
> + */
> +uint64_t igt_fb_mod_to_tiling(uint64_t modifier)
> +{
> +	switch (modifier) {
> +	case LOCAL_DRM_FORMAT_MOD_NONE:
> +		return I915_TILING_NONE;
> +	case LOCAL_I915_FORMAT_MOD_X_TILED:
> +		return I915_TILING_X;
> +	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> +		return I915_TILING_Y;
> +	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> +		return I915_TILING_Yf;
> +	default:
> +		igt_assert(0);
> +	}
> +}
> +
>  /* helpers to create nice-looking framebuffers */
>  static int create_bo_for_fb(int fd, int width, int height, uint32_t format,
>  			    uint64_t tiling, unsigned size, unsigned stride,
> @@ -206,7 +232,7 @@ static int create_bo_for_fb(int fd, int width, int height, uint32_t format,
>  			uint32_t *ptr;
>
>  			bo = gem_create(fd, size);
> -			gem_set_tiling(fd, bo, tiling, stride);
> +			gem_set_tiling(fd, bo, igt_fb_mod_to_tiling(tiling), stride);

This is still not correct because the kernel does not know about Yf or 
Ys in the set_tiling ioctl.

Regards,

Tvrtko

>
>  			/* Ensure the framebuffer is preallocated */
>  			ptr = gem_mmap__gtt(fd, bo, size, PROT_READ);
> @@ -956,27 +982,11 @@ struct fb_blit_upload {
>  	} linear;
>  };
>
> -static unsigned int fb_mod_to_obj_tiling(uint64_t fb_mod)
> -{
> -	switch (fb_mod) {
> -	case LOCAL_DRM_FORMAT_MOD_NONE:
> -		return I915_TILING_NONE;
> -	case LOCAL_I915_FORMAT_MOD_X_TILED:
> -		return I915_TILING_X;
> -	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> -		return I915_TILING_Y;
> -	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> -		return I915_TILING_Yf;
> -	default:
> -		igt_assert(0);
> -	}
> -}
> -
>  static void destroy_cairo_surface__blit(void *arg)
>  {
>  	struct fb_blit_upload *blit = arg;
>  	struct igt_fb *fb = blit->fb;
> -	unsigned int obj_tiling = fb_mod_to_obj_tiling(fb->tiling);
> +	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
>
>  	munmap(blit->linear.map, blit->linear.size);
>  	fb->cairo_surface = NULL;
> @@ -1005,7 +1015,7 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
>  {
>  	struct fb_blit_upload *blit;
>  	cairo_format_t cairo_format;
> -	unsigned int obj_tiling = fb_mod_to_obj_tiling(fb->tiling);
> +	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
>
>  	blit = malloc(sizeof(*blit));
>  	igt_assert(blit);
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index e1c4c1fa1f73..4a680cefb16d 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -129,6 +129,8 @@ int igt_create_bo_with_dimensions(int fd, int width, int height, uint32_t format
>  				  unsigned *stride_ret, unsigned *size_ret,
>  				  bool *is_dumb);
>
> +uint64_t igt_fb_mod_to_tiling(uint64_t modifier);
> +
>  /* cairo-based painting */
>  cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb);
>  void igt_paint_color(cairo_t *cr, int x, int y, int w, int h,
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 6a1549e7b857..2a9fe2e3f702 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -370,7 +370,7 @@ static int _emit_dummy_load__rcs(struct test_output *o, int limit, int timeout)
>  	sb[2].bo = gem_handle_to_libdrm_bo(bufmgr, drm_fd, "imported", fb_info->gem_handle);
>  	igt_assert(sb[2].bo);
>  	sb[2].size = sb[2].bo->size;
> -	sb[2].tiling = fb_info->tiling;
> +	sb[2].tiling = igt_fb_mod_to_tiling(fb_info->tiling);
>  	sb[2].data = NULL;
>  	sb[2].num_tiles = sb[2].bo->size;
>  	sb[2].stride = fb_info->stride;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx