[PATCH weston 3/3 v4] simple-dmabuf-drm: use GBM generic calls
Daniel Stone
daniel at fooishbar.org
Fri Jul 13 15:45:16 UTC 2018
Hi Emilio,
On Thu, 12 Jul 2018 at 12:46, Emilio Pozuelo Monfort <pochu27 at gmail.com> wrote:
> @@ -343,17 +143,16 @@ fill_content(struct buffer *my_buf, uint64_t modifier)
> }
> 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++) {
> + /* first plane: Y */
> + for (y = 0; y < my_buf->height; y++) {
> pix8 = my_buf->mmap + y * my_buf->stride;
> for (x = 0; x < my_buf->width; x++)
> - *pix8++ = x % 0xff;
> + *pix8++ = x % 256;
> }
> - /* 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) {
> + /* second plane (CbCr) is half the size of Y */
> + for (y = 0; y < my_buf->height; y++) {
> + pix8 = my_buf->mmap2 + y * my_buf->stride2;
> + for (x = 0; x < my_buf->width; x++) {
This should only allocate and fill half the width/height. I'll squash
that in when I push.
> - drmVersionPtr version = drmGetVersion(buf->drm_fd);
> + close(buf->dmabuf_fd);
> + close(buf->dmabuf_fd2);
As there was no initialisation to -1, this can close stdout instead.
> +static struct gbm_bo *
> +create_bo(struct buffer *buffer, int format, const uint64_t *modifiers, const unsigned int count)
> +{
> + struct gbm_bo *bo;
> +
> + if (!modifiers)
> + bo = gbm_bo_create(buffer->dev, buffer->width, buffer->height, format, 0 /* usage */);
> + else
> + bo = gbm_bo_create_with_modifiers(buffer->dev, buffer->width, buffer->height, GBM_FORMAT_GR88,
> + modifiers, count);
I've fixed the hardcoded format. These are also some seriously long
lines ... we don't stick strictly to 80 in this file, but these are
something like 110.
> + if (buffer->bo2) gbm_bo_unmap(buffer->bo2, buffer->mmap2);
> + buffer->dmabuf_fd = gbm_bo_get_fd(buffer->bo);
> + if (buffer->bo2) buffer->dmabuf_fd2 = gbm_bo_get_fd(buffer->bo2);
Please also put conditionals and statements on separate lines. There
are also a fair few if ((foo = thing) == NULL) tests, which is
something we avoid.
> @@ -391,7 +391,7 @@ AC_ARG_ENABLE(simple-dmabuf-drm-client,
> [do not build the simple dmabuf drm client]),,
> enable_simple_dmabuf_drm_client="auto")
> if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
> - PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm], [have_simple_dmabuf_libs=yes],
> + PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm gbm], [have_simple_dmabuf_libs=yes],
This is missing a version check for the modifiers API.
I was going to fix these up and push with review, but whilst this
works for NV12, it breaks RGB for me on Intel:
[1495104.378] -> wl_compositor at 4.create_surface(new id wl_surface at 3)
[1495104.389] -> zxdg_shell_v6 at 6.get_xdg_surface(new id
zxdg_surface_v6 at 7, wl_surface at 3)
[1495104.404] -> zxdg_surface_v6 at 7.get_toplevel(new id zxdg_toplevel_v6 at 8)
[1495104.414] -> zxdg_toplevel_v6 at 8.set_title("simple-dmabuf")
[1495104.422] -> wl_surface at 3.commit()
Missing separate debuginfo for /lib64/libstdc++.so.6
Try: dnf --enablerepo='*debug*' install
/usr/lib/debug/.build-id/a0/41a97a2ee53859845097dc52b0f8cc39738ecb.debug
[New Thread 0x7ffff4829700 (LWP 20761)]
weston-simple-dmabuf-drm:
../src/mesa/drivers/dri/i965/brw_bufmgr.c:658: brw_bo_unreference:
Assertion `p_atomic_read(&bo->refcount) > 0' failed.
Thread 1 "weston-simple-d" received signal SIGABRT, Aborted.
__GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 return ret;
(gdb) bt
#0 __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007ffff5db2561 in __GI_abort () at abort.c:79
#2 0x00007ffff5db2431 in __assert_fail_base (fmt=0x7ffff5f151a8
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7ffff56a3500
"p_atomic_read(&bo->refcount) > 0", file=0x7ffff56a32d8
"../src/mesa/drivers/dri/i965/brw_bufmgr.c", line=658,
function=0x7ffff56a3930 <__PRETTY_FUNCTION__.46376>
"brw_bo_unreference") at assert.c:92
#3 0x00007ffff5dc0692 in __GI___assert_fail
(assertion=assertion at entry=0x7ffff56a3500
"p_atomic_read(&bo->refcount) > 0", file=file at entry=0x7ffff56a32d8
"../src/mesa/drivers/dri/i965/brw_bufmgr.c", line=line at entry=658,
function=function at entry=0x7ffff56a3930 <__PRETTY_FUNCTION__.46376>
"brw_bo_unreference") at assert.c:101
#4 0x00007ffff5143cbc in brw_bo_unreference (bo=0x7ffff7e30000) at
../src/mesa/drivers/dri/i965/brw_bufmgr.c:635
#5 0x00007ffff79bc91c in gbm_dri_bo_unmap (_bo=<optimized out>,
map_data=<optimized out>) at ../src/gbm/backends/dri/gbm_dri.c:1282
#6 0x0000000000402395 in create_dmabuf_buffer (display=0x61f260,
buffer=0x624e68, width=256, height=256, format=875713112, opts=0) at
../clients/simple-dmabuf-drm.c:323
#7 0x0000000000402869 in create_window (display=0x61f260, width=256,
height=256, format=875713112, opts=0) at
../clients/simple-dmabuf-drm.c:475
#8 0x000000000040312e in main (argc=1, argv=0x7fffffffd3a8) at
../clients/simple-dmabuf-drm.c:776
It doesn't seem to make a difference if I force the use of
gbm_bo_create_with_modifiers over gbm_bo_create. I also fixed up the
call to gbm_bo_map(), where 0 was being passed for the flags, however
the API documentation notes that setting flags (READ/WRITE) is
mandatory.
Could you please fix these up (including RGB) and resubmit? I'll push
the first patches in the meantime.
Cheers,
Daniel
More information about the wayland-devel
mailing list