Hi,
On Mon, May 28, 2018 at 04:31:14PM +0300, Pekka Paalanen wrote:
> On Tue, 20 Mar 2018 11:36:38 +0100
> Guido Günther <[email protected]> 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 <[email protected]>
> > ---
> >  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.

There was no intention to reproduce it. I've added a hint how it should
look like to the commit message.

> 
> > +           }
> >     }
> >     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.

I fixed that in my branch a while ago but forget to push that to the
list. Sent out a new version now.

> 
> The first two patches are pushed:
>    bff90630..577b3464  master -> master
> 
> Without this one issue I would have push the third one too.

Thanks!
 -- Guido
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to