On Mon, 19 Mar 2018 13:38:11 +0100
Guido Günther <[email protected]> wrote:

> Signed-off-by: Guido Günther <[email protected]>
> ---
>  Makefile.am                 |  1 +
>  clients/simple-dmabuf-drm.c | 74 
> +++++++++++++++++++++++++++++++++++++++++++++
>  configure.ac                |  5 ++-
>  3 files changed, 79 insertions(+), 1 deletion(-)
> 

Hi,

I see freedreno has the same problems as mentioned below.

> diff --git a/Makefile.am b/Makefile.am
> index 69ca6cba..64a8006c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES =         \
>  weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
> $(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
>  weston_simple_dmabuf_drm_LDADD = $(SIMPLE_DMABUF_DRM_CLIENT_LIBS) \
>       $(LIBDRM_PLATFORM_FREEDRENO_LIBS) \
> +     $(LIBDRM_PLATFORM_ETNAVIV_LIBS)   \
>       $(LIBDRM_PLATFORM_INTEL_LIBS)     \
>       libshared.la
>  endif
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index 4f26e4a9..174d0f85 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -49,6 +49,9 @@
>  #ifdef HAVE_LIBDRM_FREEDRENO
>  #include <freedreno/freedreno_drmif.h>
>  #endif
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +#include <etnaviv_drmif.h>
> +#endif
>  #include <drm_fourcc.h>
>  
>  #include <wayland-client.h>
> @@ -107,6 +110,10 @@ struct buffer {
>       struct fd_device *fd_dev;
>       struct fd_bo *fd_bo;
>  #endif /* HAVE_LIBDRM_FREEDRENO */
> +#if HAVE_LIBDRM_ETNAVIV
> +     struct etna_device *etna_dev;
> +     struct etna_bo *etna_bo;
> +#endif /* HAVE_LIBDRM_ETNAVIV */
>  
>       uint32_t gem_handle;
>       int dmabuf_fd;
> @@ -270,6 +277,63 @@ fd_device_destroy(struct buffer *buf)
>       fd_device_del(buf->fd_dev);
>  }
>  #endif /* HAVE_LIBDRM_FREEDRENO */
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +#define ETNA_ALIGN(v, a) ((v + a - 1) & ~(a - 1))

This is the same as freedreno has, you can just move it outside of the
#ifdef block. I think this is really the expected form of an ALIGN
macro. If something else needs to compute something else, it will use a
macro named something else.

> +
> +static int
> +etna_alloc_bo(struct buffer *buf)
> +{
> +     int flags = DRM_ETNA_GEM_CACHE_WC;
> +     int size;
> +
> +     buf->stride = ETNA_ALIGN(buf->width, 32) * buf->bpp / 8;
> +     size =  buf->stride * buf->height * buf->bpp / 8;

Oops, multiplying twice by the bytes-per-pixel.

The next patch introduces the same bug to freedreno path.

Stride must be in bytes already, at least that is how fill_content()
and create_dmabuf_buffer() use it, so the multiplication for size by
bytes-per-pixel is too much.

> +     buf->etna_dev = etna_device_new(buf->drm_fd);
> +     buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
> +
> +     if (!buf->etna_bo)
> +             return 0;
> +     return 1;
> +}
> +
> +static void
> +etna_free_bo(struct buffer *buf)
> +{
> +     etna_bo_del(buf->etna_bo);
> +}
> +
> +static int
> +etna_bo_export_to_prime(struct buffer *buf)
> +{
> +     buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
> +     if (buf->dmabuf_fd > 0)

Zero is a valid file descriptor, it should be allowed.

> +             return 0;
> +
> +     return 1;
> +}
> +
> +static int
> +etna_map_bo(struct buffer *buf)
> +{
> +     buf->mmap = etna_bo_map(buf->etna_bo);
> +
> +     if (buf->mmap != NULL)
> +             return 1;
> +
> +     return 0;
> +}
> +
> +static void
> +etna_unmap_bo(struct buffer *buf)
> +{

Missing implementation. Isn't this missing a memory flush after CPU
write access?

> +}
> +
> +static void
> +etna_device_destroy(struct buffer *buf)
> +{
> +     etna_device_del(buf->etna_dev);
> +}
> +#endif /* HAVE_LIBDRM_ENTAVIV */
>  
>  static void
>  fill_content(struct buffer *my_buf)
> @@ -337,6 +401,16 @@ drm_device_init(struct buffer *buf)
>               dev->unmap_bo = fd_unmap_bo;
>               dev->device_destroy = fd_device_destroy;
>       }
> +#endif
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +     else if (!strcmp(dev->name, "etnaviv")) {
> +             dev->alloc_bo = etna_alloc_bo;
> +             dev->free_bo = etna_free_bo;
> +             dev->export_bo_to_prime = etna_bo_export_to_prime;
> +             dev->map_bo = etna_map_bo;
> +             dev->unmap_bo = etna_unmap_bo;
> +             dev->device_destroy = etna_device_destroy;
> +     }
>  #endif
>       else {
>               fprintf(stderr, "Error: drm device %s unsupported.\n",
> diff --git a/configure.ac b/configure.ac
> index 90ffc88d..29926388 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
> then
>    PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
>        AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
> have_simple_dmabuf_drm_client=yes,
>        [true])
> +  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
> +      AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
> have_simple_dmabuf_drm_client=yes,
> +      [have_etnaviv=no])
>  
>    if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \
>         "x$have_simple_dmabuf_libs" = "xno" && \
>       test "x$enable_simple_dmabuf_drm_client" = "xyes"; then
> -    AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
> libdrm_freedreno not found])
> +    AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but neither 
> libdrm_intel, libdrm_freedreno or libdrm_etnaviv found])
>    fi
>  
>    if test "x$have_simple_dmabuf_drm_client" = "xyes" -a 
> "x$have_simple_dmabuf_libs" = "xyes"; then

Thanks,
pq

Attachment: pgpBMmgMCQ0IG.pgp
Description: OpenPGP digital signature

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

Reply via email to