Re: [igt-dev] [PATCH v7 02/14] fb: Use an igt_fb for the cairo shadow buffer

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 11, 2018 at 10:47:29AM +0200, Maxime Ripard wrote:
> In the case where an igt_fb user wants to get a cairo surface out of that
> framebuffer, if the format of that framebuffer cannot be imported as-is in
> Cairo, the current code will do an anonymous mapping to create a shadow
> buffer where an RGB24 copy of that buffer will be created.
> 
> However, making that shadow buffer into an igt_fb itself will help us do
> further improvements on the conversion code.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  lib/igt_fb.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 542c62610412..dd180c6c8016 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1381,12 +1381,12 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
>  
>  struct fb_convert_blit_upload {
>  	int fd;
> +
>  	struct igt_fb *fb;
> +	uint8_t *ptr;
>  
> -	struct {
> -		uint8_t *map;
> -		unsigned stride, size;
> -	} rgb24;
> +	struct igt_fb shadow_fb;
> +	uint8_t *shadow_ptr;

We seem to be pairing fb and ptr a lot. I wonder if we should suck the
the ptr into the fb struct. But maybe someone wants to map the same
thing multiple times? Not sure.

>  
>  	struct fb_blit_linear linear;
>  };
> @@ -1780,7 +1780,8 @@ static void destroy_cairo_surface__convert(void *arg)
>  			     fb->drm_format);
>  	}
>  
> -	munmap(blit->rgb24.map, blit->rgb24.size);
> +	igt_fb_unmap_buffer(&blit->shadow_fb, blit->shadow_ptr);
> +	igt_remove_fb(blit->fd, &blit->shadow_fb);
>  
>  	if (blit->linear.handle)
>  		free_linear_mapping(blit->fd, blit->fb, &blit->linear);
> @@ -1795,14 +1796,19 @@ static void destroy_cairo_surface__convert(void *arg)
>  static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  {
>  	struct fb_convert_blit_upload *blit = malloc(sizeof(*blit));
> +	int shadow_id;
> +
>  	igt_assert(blit);
>  
>  	blit->fd = fd;
>  	blit->fb = fb;
> -	blit->rgb24.stride = ALIGN(fb->width * 4, 16);
> -	blit->rgb24.size = ALIGN(blit->rgb24.stride * fb->height, sysconf(_SC_PAGESIZE));
> -	blit->rgb24.map = mmap(NULL, blit->rgb24.size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -	igt_assert(blit->rgb24.map != MAP_FAILED);
> +
> +	shadow_id = igt_create_fb(fd, fb->width, fb->height, DRM_FORMAT_RGB888,
> +				  LOCAL_DRM_FORMAT_MOD_NONE, &blit->shadow_fb);
> +	igt_assert(shadow_id > 0);

We can't actually expect addfb to work for RGB888. That thing is not
supported by most hardware. Since my series adding fb_init() etc. hasn't
gotten reviewed I guess we'll just need to populate the igt_fb by hand
here, or something.

> +
> +	blit->shadow_ptr = igt_fb_map_buffer(fd, &blit->shadow_fb);
> +	igt_assert(blit->shadow_ptr);
>  
>  	if (fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>  	    fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
> @@ -1834,10 +1840,10 @@ static void create_cairo_surface__convert(int fd, struct igt_fb *fb)
>  	}
>  
>  	fb->cairo_surface =
> -		cairo_image_surface_create_for_data(blit->rgb24.map,
> +		cairo_image_surface_create_for_data(blit->shadow_ptr,
>  						    CAIRO_FORMAT_RGB24,
>  						    fb->width, fb->height,
> -						    blit->rgb24.stride);
> +						    blit->shadow_fb.stride);
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
>  				    (cairo_user_data_key_t *)create_cairo_surface__convert,
> -- 
> git-series 0.9.1

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev