From 28020ff57728d07ae0715fc15696b8fe40337b3d Mon Sep 17 00:00:00 2001 From: emersion Date: Tue, 29 May 2018 22:38:00 +0100 Subject: [PATCH] Only allow one modifier per DMA-BUF, split attributes struct in render/ --- backend/drm/renderer.c | 4 +-- include/wlr/render/dmabuf.h | 29 ++++++++++++++++++ include/wlr/render/egl.h | 6 ++-- include/wlr/render/gles2.h | 2 +- include/wlr/render/interface.h | 6 ++-- include/wlr/render/wlr_renderer.h | 2 +- include/wlr/render/wlr_texture.h | 4 +-- include/wlr/types/wlr_linux_dmabuf.h | 30 ++---------------- render/egl.c | 19 ++++++------ render/gles2/renderer.c | 6 ++-- render/gles2/texture.c | 4 +-- render/wlr_renderer.c | 4 +-- render/wlr_texture.c | 2 +- types/wlr_linux_dmabuf.c | 46 ++++++++++++++++++---------- types/wlr_surface.c | 1 + 15 files changed, 91 insertions(+), 74 deletions(-) create mode 100644 include/wlr/render/dmabuf.h diff --git a/backend/drm/renderer.c b/backend/drm/renderer.c index d5bcef2b..72a0254b 100644 --- a/backend/drm/renderer.c +++ b/backend/drm/renderer.c @@ -186,15 +186,15 @@ static struct wlr_texture *get_tex_for_bo(struct wlr_drm_renderer *renderer, return NULL; } - struct wlr_dmabuf_buffer_attribs attribs = { + struct wlr_dmabuf_attributes attribs = { .n_planes = 1, .width = gbm_bo_get_width(bo), .height = gbm_bo_get_height(bo), .format = gbm_bo_get_format(bo), + .modifier = DRM_FORMAT_MOD_LINEAR, }; attribs.offset[0] = 0; attribs.stride[0] = gbm_bo_get_stride_for_plane(bo, 0); - attribs.modifier[0] = DRM_FORMAT_MOD_LINEAR; attribs.fd[0] = gbm_bo_get_fd(bo); tex->tex = wlr_texture_from_dmabuf(renderer->wlr_rend, &attribs); diff --git a/include/wlr/render/dmabuf.h b/include/wlr/render/dmabuf.h new file mode 100644 index 00000000..2e13fe75 --- /dev/null +++ b/include/wlr/render/dmabuf.h @@ -0,0 +1,29 @@ +#ifndef WLR_RENDER_DMABUF_H +#define WLR_RENDER_DMABUF_H + +// So we don't have to pull in linux specific drm headers +#ifndef DRM_FORMAT_MOD_INVALID +#define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1) +#endif + +#define WLR_DMABUF_MAX_PLANES 4 + +enum wlr_dmabuf_attributes_flags { + WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT = 1, + WLR_DMABUF_ATTRIBUTES_FLAGS_INTERLACED = 2, + WLR_DMABUF_ATTRIBUTES_FLAGS_BOTTOM_FIRST = 4, +}; + +struct wlr_dmabuf_attributes { + int32_t width, height; + uint32_t format; + uint32_t flags; // enum wlr_dmabuf_attributes_flags + uint64_t modifier; + + int n_planes; + uint32_t offset[WLR_DMABUF_MAX_PLANES]; + uint32_t stride[WLR_DMABUF_MAX_PLANES]; + int fd[WLR_DMABUF_MAX_PLANES]; +}; + +#endif diff --git a/include/wlr/render/egl.h b/include/wlr/render/egl.h index 4d837138..7cd5c5ca 100644 --- a/include/wlr/render/egl.h +++ b/include/wlr/render/egl.h @@ -6,7 +6,7 @@ #include #include #include -#include +#include struct wlr_egl { EGLDisplay display; @@ -65,14 +65,14 @@ EGLImageKHR wlr_egl_create_image_from_wl_drm(struct wlr_egl *egl, * of the dmabuf with wlr_egl_check_import_dmabuf once first. */ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer_attribs *attributes); + struct wlr_dmabuf_attributes *attributes); /** * Try to import the given dmabuf. On success return true false otherwise. * If this succeeds the dmabuf can be used for rendering on a texture */ bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer *dmabuf); + struct wlr_dmabuf_attributes *attributes); /** * Get the available dmabuf formats diff --git a/include/wlr/render/gles2.h b/include/wlr/render/gles2.h index 65bb36c1..866c6658 100644 --- a/include/wlr/render/gles2.h +++ b/include/wlr/render/gles2.h @@ -14,6 +14,6 @@ struct wlr_texture *wlr_gles2_texture_from_pixels(struct wlr_egl *egl, struct wlr_texture *wlr_gles2_texture_from_wl_drm(struct wlr_egl *egl, struct wl_resource *data); struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer_attribs *attribs); + struct wlr_dmabuf_attributes *attribs); #endif diff --git a/include/wlr/render/interface.h b/include/wlr/render/interface.h index 2267d376..87b3c929 100644 --- a/include/wlr/render/interface.h +++ b/include/wlr/render/interface.h @@ -8,8 +8,8 @@ #include #include #include -#include #include +#include struct wlr_renderer_impl { void (*begin)(struct wlr_renderer *renderer, uint32_t width, @@ -31,7 +31,7 @@ struct wlr_renderer_impl { void (*wl_drm_buffer_get_size)(struct wlr_renderer *renderer, struct wl_resource *buffer, int *width, int *height); bool (*check_import_dmabuf)(struct wlr_renderer *renderer, - struct wlr_dmabuf_buffer *dmabuf); + struct wlr_dmabuf_attributes *attribs); int (*get_dmabuf_formats)(struct wlr_renderer *renderer, int **formats); int (*get_dmabuf_modifiers)(struct wlr_renderer *renderer, int format, uint64_t **modifiers); @@ -47,7 +47,7 @@ struct wlr_renderer_impl { struct wlr_texture *(*texture_from_wl_drm)(struct wlr_renderer *renderer, struct wl_resource *data); struct wlr_texture *(*texture_from_dmabuf)(struct wlr_renderer *renderer, - struct wlr_dmabuf_buffer_attribs *attribs); + struct wlr_dmabuf_attributes *attribs); void (*destroy)(struct wlr_renderer *renderer); void (*init_wl_display)(struct wlr_renderer *renderer, struct wl_display *wl_display); diff --git a/include/wlr/render/wlr_renderer.h b/include/wlr/render/wlr_renderer.h index 039bb66e..dd62944f 100644 --- a/include/wlr/render/wlr_renderer.h +++ b/include/wlr/render/wlr_renderer.h @@ -89,7 +89,7 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *renderer, int format, * If this succeeds the dmabuf can be used for rendering on a texture */ bool wlr_renderer_check_import_dmabuf(struct wlr_renderer *renderer, - struct wlr_dmabuf_buffer *dmabuf); + struct wlr_dmabuf_attributes *attributes); /** * Reads out of pixels of the currently bound surface into data. `stride` is in * bytes. diff --git a/include/wlr/render/wlr_texture.h b/include/wlr/render/wlr_texture.h index 239fc51b..c1633820 100644 --- a/include/wlr/render/wlr_texture.h +++ b/include/wlr/render/wlr_texture.h @@ -5,7 +5,7 @@ #include #include #include -#include +#include struct wlr_renderer; struct wlr_texture_impl; @@ -33,7 +33,7 @@ struct wlr_texture *wlr_texture_from_wl_drm(struct wlr_renderer *renderer, * Create a new texture from a DMA-BUF. The returned texture is immutable. */ struct wlr_texture *wlr_texture_from_dmabuf(struct wlr_renderer *renderer, - struct wlr_dmabuf_buffer_attribs *attribs); + struct wlr_dmabuf_attributes *attribs); /** * Get the texture width and height. diff --git a/include/wlr/types/wlr_linux_dmabuf.h b/include/wlr/types/wlr_linux_dmabuf.h index 531e68ab..ea219020 100644 --- a/include/wlr/types/wlr_linux_dmabuf.h +++ b/include/wlr/types/wlr_linux_dmabuf.h @@ -1,40 +1,16 @@ #ifndef WLR_TYPES_WLR_LINUX_DMABUF_H #define WLR_TYPES_WLR_LINUX_DMABUF_H -#define WLR_LINUX_DMABUF_MAX_PLANES 4 - #include #include - -/* So we don't have to pull in linux specific drm headers */ -#ifndef DRM_FORMAT_MOD_INVALID -#define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1) -#endif - -enum { - WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_Y_INVERT = 1, - WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_INTERLACED = 2, - WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_BOTTOM_FIRST = 4, -}; - -struct wlr_dmabuf_buffer_attribs { - /* set via params_add */ - int n_planes; - uint32_t offset[WLR_LINUX_DMABUF_MAX_PLANES]; - uint32_t stride[WLR_LINUX_DMABUF_MAX_PLANES]; - uint64_t modifier[WLR_LINUX_DMABUF_MAX_PLANES]; - int fd[WLR_LINUX_DMABUF_MAX_PLANES]; - /* set via params_create */ - int32_t width, height; - uint32_t format; - uint32_t flags; -}; +#include struct wlr_dmabuf_buffer { struct wlr_renderer *renderer; struct wl_resource *buffer_resource; struct wl_resource *params_resource; - struct wlr_dmabuf_buffer_attribs attributes; + struct wlr_dmabuf_attributes attributes; + bool has_modifier; }; /** diff --git a/render/egl.c b/render/egl.c index 579bb5fe..6ac3ee2a 100644 --- a/render/egl.c +++ b/render/egl.c @@ -340,9 +340,9 @@ EGLImageKHR wlr_egl_create_image_from_wl_drm(struct wlr_egl *egl, } EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer_attribs *attributes) { + struct wlr_dmabuf_attributes *attributes) { bool has_modifier = false; - if (attributes->modifier[0] != DRM_FORMAT_MOD_INVALID) { + if (attributes->modifier != DRM_FORMAT_MOD_INVALID) { if (!egl->egl_exts.dmabuf_import_modifiers) { return NULL; } @@ -364,7 +364,7 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, EGLint pitch; EGLint mod_lo; EGLint mod_hi; - } attr_names[WLR_LINUX_DMABUF_MAX_PLANES] = { + } attr_names[WLR_DMABUF_MAX_PLANES] = { { EGL_DMA_BUF_PLANE0_FD_EXT, EGL_DMA_BUF_PLANE0_OFFSET_EXT, @@ -401,9 +401,9 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, attribs[atti++] = attributes->stride[i]; if (has_modifier) { attribs[atti++] = attr_names[i].mod_lo; - attribs[atti++] = attributes->modifier[i] & 0xFFFFFFFF; + attribs[atti++] = attributes->modifier & 0xFFFFFFFF; attribs[atti++] = attr_names[i].mod_hi; - attribs[atti++] = attributes->modifier[i] >> 32; + attribs[atti++] = attributes->modifier >> 32; } } attribs[atti++] = EGL_NONE; @@ -414,11 +414,11 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, } #ifndef DRM_FORMAT_BIG_ENDIAN -# define DRM_FORMAT_BIG_ENDIAN 0x80000000 +#define DRM_FORMAT_BIG_ENDIAN 0x80000000 #endif bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer *dmabuf) { - switch (dmabuf->attributes.format & ~DRM_FORMAT_BIG_ENDIAN) { + struct wlr_dmabuf_attributes *attribs) { + switch (attribs->format & ~DRM_FORMAT_BIG_ENDIAN) { /* TODO: YUV based formats not yet supported, require multiple * wlr_create_image_from_dmabuf */ case WL_SHM_FORMAT_YUYV: @@ -431,8 +431,7 @@ bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl, break; } - EGLImage egl_image = wlr_egl_create_image_from_dmabuf(egl, - &dmabuf->attributes); + EGLImage egl_image = wlr_egl_create_image_from_dmabuf(egl, attribs); if (egl_image) { /* We can import the image, good. No need to keep it since wlr_texture_upload_dmabuf will import it again */ diff --git a/render/gles2/renderer.c b/render/gles2/renderer.c index 5cea5c3b..e68bb83f 100644 --- a/render/gles2/renderer.c +++ b/render/gles2/renderer.c @@ -243,9 +243,9 @@ static int gles2_get_dmabuf_modifiers(struct wlr_renderer *wlr_renderer, } static bool gles2_check_import_dmabuf(struct wlr_renderer *wlr_renderer, - struct wlr_dmabuf_buffer *dmabuf) { + struct wlr_dmabuf_attributes *attribs) { struct wlr_gles2_renderer *renderer = gles2_get_renderer(wlr_renderer); - return wlr_egl_check_import_dmabuf(renderer->egl, dmabuf); + return wlr_egl_check_import_dmabuf(renderer->egl, attribs); } static bool gles2_read_pixels(struct wlr_renderer *wlr_renderer, @@ -299,7 +299,7 @@ static struct wlr_texture *gles2_texture_from_wl_drm( static struct wlr_texture *gles2_texture_from_dmabuf( struct wlr_renderer *wlr_renderer, - struct wlr_dmabuf_buffer_attribs *attribs) { + struct wlr_dmabuf_attributes *attribs) { struct wlr_gles2_renderer *renderer = gles2_get_renderer(wlr_renderer); return wlr_gles2_texture_from_dmabuf(renderer->egl, attribs); } diff --git a/render/gles2/texture.c b/render/gles2/texture.c index 37424802..2627e292 100644 --- a/render/gles2/texture.c +++ b/render/gles2/texture.c @@ -199,7 +199,7 @@ struct wlr_texture *wlr_gles2_texture_from_wl_drm(struct wlr_egl *egl, } struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl, - struct wlr_dmabuf_buffer_attribs *attribs) { + struct wlr_dmabuf_attributes *attribs) { assert(wlr_egl_is_current(egl)); if (!glEGLImageTargetTexture2DOES) { @@ -225,7 +225,7 @@ struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl, texture->type = WLR_GLES2_TEXTURE_DMABUF; texture->has_alpha = true; texture->inverted_y = - (attribs->flags & WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_Y_INVERT) != 0; + (attribs->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT) != 0; texture->image = wlr_egl_create_image_from_dmabuf(egl, attribs); if (texture->image == NULL) { diff --git a/render/wlr_renderer.c b/render/wlr_renderer.c index aed821c9..32b0a779 100644 --- a/render/wlr_renderer.c +++ b/render/wlr_renderer.c @@ -136,11 +136,11 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *r, int format, } bool wlr_renderer_check_import_dmabuf(struct wlr_renderer *r, - struct wlr_dmabuf_buffer *dmabuf) { + struct wlr_dmabuf_attributes *attribs) { if (!r->impl->check_import_dmabuf) { return false; } - return r->impl->check_import_dmabuf(r, dmabuf); + return r->impl->check_import_dmabuf(r, attribs); } bool wlr_renderer_read_pixels(struct wlr_renderer *r, enum wl_shm_format fmt, diff --git a/render/wlr_texture.c b/render/wlr_texture.c index 9aecfd98..38c75d28 100644 --- a/render/wlr_texture.c +++ b/render/wlr_texture.c @@ -35,7 +35,7 @@ struct wlr_texture *wlr_texture_from_wl_drm(struct wlr_renderer *renderer, } struct wlr_texture *wlr_texture_from_dmabuf(struct wlr_renderer *renderer, - struct wlr_dmabuf_buffer_attribs *attribs) { + struct wlr_dmabuf_attributes *attribs) { if (!renderer->impl->texture_from_dmabuf) { return NULL; } diff --git a/types/wlr_linux_dmabuf.c b/types/wlr_linux_dmabuf.c index 9bcd473f..fd4bd334 100644 --- a/types/wlr_linux_dmabuf.c +++ b/types/wlr_linux_dmabuf.c @@ -66,7 +66,7 @@ static void params_destroy(struct wl_client *client, } static void params_add(struct wl_client *client, - struct wl_resource *params_resource, int32_t name_fd, + struct wl_resource *params_resource, int32_t fd, uint32_t plane_idx, uint32_t offset, uint32_t stride, uint32_t modifier_hi, uint32_t modifier_lo) { struct wlr_dmabuf_buffer *buffer = @@ -76,33 +76,42 @@ static void params_add(struct wl_client *client, wl_resource_post_error(params_resource, ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_ALREADY_USED, "params was already used to create a wl_buffer"); - close(name_fd); + close(fd); return; } - if (plane_idx >= WLR_LINUX_DMABUF_MAX_PLANES) { + if (plane_idx >= WLR_DMABUF_MAX_PLANES) { wl_resource_post_error(params_resource, ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_PLANE_IDX, - "plane index %u > %u", plane_idx, WLR_LINUX_DMABUF_MAX_PLANES); - close(name_fd); + "plane index %u > %u", plane_idx, WLR_DMABUF_MAX_PLANES); + close(fd); return; } if (buffer->attributes.fd[plane_idx] != -1) { wl_resource_post_error(params_resource, ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_PLANE_SET, - "a dmabuf with id %d has already been added for plane %u", - buffer->attributes.fd[plane_idx], - plane_idx); - close(name_fd); + "a dmabuf with FD %d has already been added for plane %u", + buffer->attributes.fd[plane_idx], plane_idx); + close(fd); return; } - buffer->attributes.fd[plane_idx] = name_fd; + uint64_t modifier = ((uint64_t)modifier_hi << 32) | modifier_lo; + if (buffer->has_modifier && modifier != buffer->attributes.modifier) { + wl_resource_post_error(params_resource, + ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_INVALID_FORMAT, + "sent modifier %lu for plane %u, expected modifier %lu like other planes", + modifier, plane_idx, buffer->attributes.modifier); + close(fd); + return; + } + buffer->attributes.modifier = modifier; + buffer->has_modifier = true; + + buffer->attributes.fd[plane_idx] = fd; buffer->attributes.offset[plane_idx] = offset; buffer->attributes.stride[plane_idx] = stride; - buffer->attributes.modifier[plane_idx] = - ((uint64_t)modifier_hi << 32) | modifier_lo; buffer->attributes.n_planes++; } @@ -181,7 +190,8 @@ static void params_create_common(struct wl_client *client, } off_t size = lseek(buffer->attributes.fd[i], 0, SEEK_END); - if (size == -1) { /* Skip checks if kernel does no support seek on buffer */ + if (size == -1) { + // Skip checks if kernel does no support seek on buffer continue; } if (buffer->attributes.offset[i] >= size) { @@ -200,8 +210,9 @@ static void params_create_common(struct wl_client *client, goto err_out; } - if (i == 0 && /* planes > 0 might be subsampled according to fourcc format */ - buffer->attributes.offset[i] + buffer->attributes.stride[i] * height > size) { + // planes > 0 might be subsampled according to fourcc format + if (i == 0 && buffer->attributes.offset[i] + + buffer->attributes.stride[i] * height >= size) { wl_resource_post_error(params_resource, ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_OUT_OF_BOUNDS, "invalid buffer stride or height for plane %d", i); @@ -218,7 +229,8 @@ static void params_create_common(struct wl_client *client, } /* Check if dmabuf is usable */ - if (!wlr_renderer_check_import_dmabuf(buffer->renderer, buffer)) { + if (!wlr_renderer_check_import_dmabuf(buffer->renderer, + &buffer->attributes)) { goto err_failed; } @@ -319,7 +331,7 @@ static void linux_dmabuf_create_params(struct wl_client *client, goto err; } - for (int i = 0; i < WLR_LINUX_DMABUF_MAX_PLANES; i++) { + for (int i = 0; i < WLR_DMABUF_MAX_PLANES; i++) { buffer->attributes.fd[i] = -1; } diff --git a/types/wlr_surface.c b/types/wlr_surface.c index 8a05657e..3949f3e2 100644 --- a/types/wlr_surface.c +++ b/types/wlr_surface.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include