[PATCH weston 3/3] simple-dmabuf-drm: support DRM_FORMAT_LINEAR for NV12 as well

[Pekka Paalanen]

On Tue, 20 Mar 2018 11:36:38 +0100
Guido Günther <agx at sigxcpu.org> wrote:

> This makes --import-format=NV12 testable on e.g. intel
> 
> We only set nv12_format_found to true if we found that format and at
> least one understood modifier. Store modifier verbatim instead of using
> a boolean flag. Last advertised and supported modifier currently wins.
> 
> Signed-off-by: Guido Günther <agx at sigxcpu.org>
> ---
>  clients/simple-dmabuf-drm.c | 54 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index b9681ca4..7175f2d7 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -75,7 +75,7 @@ struct display {
>  	struct zwp_linux_dmabuf_v1 *dmabuf;
>  	int xrgb8888_format_found;
>  	int nv12_format_found;
> -	int nv12_modifier_found;
> +	uint64_t nv12_modifier;
>  	int req_dmabuf_immediate;
>  	int req_dmabuf_modifiers;
>  };
> @@ -273,7 +273,7 @@ fd_device_destroy(struct buffer *buf)
>  #endif /* HAVE_LIBDRM_FREEDRENO */
>  
>  static void
> -fill_content(struct buffer *my_buf)
> +fill_content(struct buffer *my_buf, uint64_t modifier)
>  {
>  	int x = 0, y = 0;
>  	uint32_t *pix;
> @@ -281,11 +281,31 @@ fill_content(struct buffer *my_buf)
>  	assert(my_buf->mmap);
>  
>  	if (my_buf->format == DRM_FORMAT_NV12) {
> -		pix = (uint32_t *) my_buf->mmap;
> -		for (y = 0; y < my_buf->height; y++)
> -			memcpy(&pix[y * my_buf->width / 4],
> -			       &nv12_tiled[my_buf->width * y / 4],
> -			       my_buf->width);
> +		if (modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE) {
> +			pix = (uint32_t *) my_buf->mmap;
> +			for (y = 0; y < my_buf->height; y++)
> +				memcpy(&pix[y * my_buf->width / 4],
> +				       &nv12_tiled[my_buf->width * y / 4],
> +				       my_buf->width);
> +		}
> +		else if (modifier == DRM_FORMAT_MOD_LINEAR) {
> +			uint8_t *pix8;
> +			/* first plane: Y (2/3 of the buffer)	*/
> +			for (y = 0; y < my_buf->height * 2/3; y++) {
> +				pix8 = my_buf->mmap + y * my_buf->stride;
> +				for (x = 0; x < my_buf->width; x++)
> +					*pix8++ = x % 0xff;
> +			}
> +			/* second plane (CbCr) is half the size of Y
> +			   plane (last 1/3 of the buffer) */
> +			for (y = my_buf->height * 2/3; y < my_buf->height; y++) {
> +				pix8 = my_buf->mmap + y * my_buf->stride;
> +				for (x = 0; x < my_buf->width; x+=2) {
> +					*pix8++ = x % 256;
> +					*pix8++ = y % 256;
> +				}
> +			}

Was the LINEAR branch supposed to produce the same image as the XRGB
path? It doesn't, the colors are different on intel, so I assume there
is no such intention.

If there was, I'd call for an image that can be described in words what
it should look like, and would be identifiable if it was flipped or
rotated or had wrong colors.

> +		}
>  	}
>  	else {
>  		for (y = 0; y < my_buf->height; y++) {
> @@ -420,7 +440,6 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  		/* adjust height for allocation of NV12 Y and UV planes */
>  		buffer->height = height * 3 / 2;
>  		buffer->bpp = 8;
> -		modifier = DRM_FORMAT_MOD_SAMSUNG_64_32_TILE;
>  		break;
>  	default:
>  		buffer->height = height;
> @@ -437,7 +456,7 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  		fprintf(stderr, "map_bo failed\n");
>  		goto error2;
>  	}
> -	fill_content(buffer);
> +	fill_content(buffer, modifier);

Doesn't this call have modifier=0 always?
Which by luck turns out to be DRM_FORMAT_MOD_LINEAR.

The first two patches are pushed:
   bff90630..577b3464  master -> master

Without this one issue I would have push the third one too.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180528/81963dec/attachment.sig>