Re: [PATCH 2/6] gl-renderer: introduce a new struct dmabuf_image
On 24/11/15 01:28 PM, Emmanuel Gil Peyrot wrote:
> This struct serves as renderer data for linux-dmabuf buffers, and can
> contain multiple struct egl_image, simplifying this latter in the
> common non-dmabuf case.
>
> Signed-off-by: Emmanuel Gil Peyrot
> Reviewed-by: Pekka Paalanen
> Reviewed-by: Daniel Stone
>
> Differential Revision: https://phabricator.freedesktop.org/D333
> ---
> src/gl-renderer.c | 167
> +++---
> 1 file changed, 109 insertions(+), 58 deletions(-)
>
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index d5356b6..90e4842 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -99,9 +99,12 @@ struct egl_image {
> struct gl_renderer *renderer;
> EGLImageKHR image;
> int refcount;
> +};
>
> - /* Only used for dmabuf imported buffer */
> +struct dmabuf_image {
> struct linux_dmabuf_buffer *dmabuf;
> + int num_images;
> + struct egl_image *images[3];
> struct wl_list link;
> };
>
> @@ -191,6 +194,13 @@ struct gl_renderer {
>
> static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
>
> +static inline const char *
> +dump_format(uint32_t *format)
> +{
> + // FIXME: this won’t work properly on big-endian.
Tiny style complaint, we don't use c++ style comments in weston...
This whole function's pretty scary really - I realize it's just an error
log and it's very hard to hit, but it seems like it's pretty simple to
just fix the FIXME and do it "right"?
Other than that, the important parts look correct, so with this bit fixed,
Reviewed-by: Derek Foreman
> + return (const char *)format;
> +}
> +
> static inline struct gl_output_state *
> get_output_state(struct weston_output *output)
> {
> @@ -222,7 +232,6 @@ egl_image_create(struct gl_renderer *gr, EGLenum target,
> struct egl_image *img;
>
> img = zalloc(sizeof *img);
> - wl_list_init(&img->link);
> img->renderer = gr;
> img->refcount = 1;
> img->image = gr->create_image(gr->egl_display, EGL_NO_CONTEXT,
> @@ -255,16 +264,37 @@ egl_image_unref(struct egl_image *image)
> if (image->refcount > 0)
> return image->refcount;
>
> - if (image->dmabuf)
> - linux_dmabuf_buffer_set_user_data(image->dmabuf, NULL, NULL);
> -
> gr->destroy_image(gr->egl_display, image->image);
> - wl_list_remove(&image->link);
> free(image);
>
> return 0;
> }
>
> +static struct dmabuf_image*
> +dmabuf_image_create(void)
> +{
> + struct dmabuf_image *img;
> +
> + img = zalloc(sizeof *img);
> + wl_list_init(&img->link);
> +
> + return img;
> +}
> +
> +static void
> +dmabuf_image_destroy(struct dmabuf_image *image)
> +{
> + int i;
> +
> + for (i = 0; i < image->num_images; ++i)
> + egl_image_unref(image->images[i]);
> +
> + if (image->dmabuf)
> + linux_dmabuf_buffer_set_user_data(image->dmabuf, NULL, NULL);
> +
> + wl_list_remove(&image->link);
> +}
> +
> static const char *
> egl_error_string(EGLint code)
> {
> @@ -1420,23 +1450,19 @@ gl_renderer_attach_egl(struct weston_surface *es,
> struct weston_buffer *buffer,
> static void
> gl_renderer_destroy_dmabuf(struct linux_dmabuf_buffer *dmabuf)
> {
> - struct egl_image *image = dmabuf->user_data;
> + struct dmabuf_image *image = dmabuf->user_data;
>
> - egl_image_unref(image);
> + dmabuf_image_destroy(image);
> }
>
> static struct egl_image *
> -import_dmabuf(struct gl_renderer *gr,
> - struct linux_dmabuf_buffer *dmabuf)
> +import_simple_dmabuf(struct gl_renderer *gr,
> + struct dmabuf_attributes *attributes)
> {
> struct egl_image *image;
> EGLint attribs[30];
> int atti = 0;
>
> - image = linux_dmabuf_buffer_get_user_data(dmabuf);
> - if (image)
> - return egl_image_ref(image);
> -
> /* This requires the Mesa commit in
>* Mesa 10.3 (08264e5dad4df448e7718e782ad9077902089a07) or
>* Mesa 10.2.7 (55d28925e6109a4afd61f109e845a8a51bd17652).
> @@ -1446,38 +1472,38 @@ import_dmabuf(struct gl_renderer *gr,
>*/
>
> attribs[atti++] = EGL_WIDTH;
> - attribs[atti++] = dmabuf->attributes.width;
> + attribs[atti++] = attributes->width;
> attribs[atti++] = EGL_HEIGHT;
> - attribs[atti++] = dmabuf->attributes.height;
> + attribs[atti++] = attributes->height;
> attribs[atti++] = EGL_LINUX_DRM_FOURCC_EXT;
> - attribs[atti++] = dmabuf->attributes.format;
> + attribs[atti++] = attributes->format;
> /* XXX: Add modifier here when supported */
>
> - if (dmabuf->attributes.n_planes > 0) {
> + if (attributes->n_planes > 0) {
> attribs[atti++] = EGL_DMA_BUF_PLANE0_FD_EXT;
> - attribs[atti++] = dmabuf->attributes.fd[0];
> + attribs[atti++] = attributes->fd[0];
> attribs[atti++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
> -
[PATCH 2/6] gl-renderer: introduce a new struct dmabuf_image
This struct serves as renderer data for linux-dmabuf buffers, and can
contain multiple struct egl_image, simplifying this latter in the
common non-dmabuf case.
Signed-off-by: Emmanuel Gil Peyrot
Reviewed-by: Pekka Paalanen
Reviewed-by: Daniel Stone
Differential Revision: https://phabricator.freedesktop.org/D333
---
src/gl-renderer.c | 167 +++---
1 file changed, 109 insertions(+), 58 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index d5356b6..90e4842 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -99,9 +99,12 @@ struct egl_image {
struct gl_renderer *renderer;
EGLImageKHR image;
int refcount;
+};
- /* Only used for dmabuf imported buffer */
+struct dmabuf_image {
struct linux_dmabuf_buffer *dmabuf;
+ int num_images;
+ struct egl_image *images[3];
struct wl_list link;
};
@@ -191,6 +194,13 @@ struct gl_renderer {
static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
+static inline const char *
+dump_format(uint32_t *format)
+{
+ // FIXME: this won’t work properly on big-endian.
+ return (const char *)format;
+}
+
static inline struct gl_output_state *
get_output_state(struct weston_output *output)
{
@@ -222,7 +232,6 @@ egl_image_create(struct gl_renderer *gr, EGLenum target,
struct egl_image *img;
img = zalloc(sizeof *img);
- wl_list_init(&img->link);
img->renderer = gr;
img->refcount = 1;
img->image = gr->create_image(gr->egl_display, EGL_NO_CONTEXT,
@@ -255,16 +264,37 @@ egl_image_unref(struct egl_image *image)
if (image->refcount > 0)
return image->refcount;
- if (image->dmabuf)
- linux_dmabuf_buffer_set_user_data(image->dmabuf, NULL, NULL);
-
gr->destroy_image(gr->egl_display, image->image);
- wl_list_remove(&image->link);
free(image);
return 0;
}
+static struct dmabuf_image*
+dmabuf_image_create(void)
+{
+ struct dmabuf_image *img;
+
+ img = zalloc(sizeof *img);
+ wl_list_init(&img->link);
+
+ return img;
+}
+
+static void
+dmabuf_image_destroy(struct dmabuf_image *image)
+{
+ int i;
+
+ for (i = 0; i < image->num_images; ++i)
+ egl_image_unref(image->images[i]);
+
+ if (image->dmabuf)
+ linux_dmabuf_buffer_set_user_data(image->dmabuf, NULL, NULL);
+
+ wl_list_remove(&image->link);
+}
+
static const char *
egl_error_string(EGLint code)
{
@@ -1420,23 +1450,19 @@ gl_renderer_attach_egl(struct weston_surface *es,
struct weston_buffer *buffer,
static void
gl_renderer_destroy_dmabuf(struct linux_dmabuf_buffer *dmabuf)
{
- struct egl_image *image = dmabuf->user_data;
+ struct dmabuf_image *image = dmabuf->user_data;
- egl_image_unref(image);
+ dmabuf_image_destroy(image);
}
static struct egl_image *
-import_dmabuf(struct gl_renderer *gr,
- struct linux_dmabuf_buffer *dmabuf)
+import_simple_dmabuf(struct gl_renderer *gr,
+ struct dmabuf_attributes *attributes)
{
struct egl_image *image;
EGLint attribs[30];
int atti = 0;
- image = linux_dmabuf_buffer_get_user_data(dmabuf);
- if (image)
- return egl_image_ref(image);
-
/* This requires the Mesa commit in
* Mesa 10.3 (08264e5dad4df448e7718e782ad9077902089a07) or
* Mesa 10.2.7 (55d28925e6109a4afd61f109e845a8a51bd17652).
@@ -1446,38 +1472,38 @@ import_dmabuf(struct gl_renderer *gr,
*/
attribs[atti++] = EGL_WIDTH;
- attribs[atti++] = dmabuf->attributes.width;
+ attribs[atti++] = attributes->width;
attribs[atti++] = EGL_HEIGHT;
- attribs[atti++] = dmabuf->attributes.height;
+ attribs[atti++] = attributes->height;
attribs[atti++] = EGL_LINUX_DRM_FOURCC_EXT;
- attribs[atti++] = dmabuf->attributes.format;
+ attribs[atti++] = attributes->format;
/* XXX: Add modifier here when supported */
- if (dmabuf->attributes.n_planes > 0) {
+ if (attributes->n_planes > 0) {
attribs[atti++] = EGL_DMA_BUF_PLANE0_FD_EXT;
- attribs[atti++] = dmabuf->attributes.fd[0];
+ attribs[atti++] = attributes->fd[0];
attribs[atti++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT;
- attribs[atti++] = dmabuf->attributes.offset[0];
+ attribs[atti++] = attributes->offset[0];
attribs[atti++] = EGL_DMA_BUF_PLANE0_PITCH_EXT;
- attribs[atti++] = dmabuf->attributes.stride[0];
+ attribs[atti++] = attributes->stride[0];
}
- if (dmabuf->attributes.n_planes > 1) {
+ if (attributes->n_planes > 1) {
attribs[atti++] = EGL_DMA_BUF_PLANE1_FD_EXT;
- attribs[atti++] = dmabuf->attributes.fd[1];
+ attribs[atti++] = attribute
