[PATCH weston v4 4/4] simple-dmabuf-drm: support etnaviv drm as well

[Derek Foreman]

On 2018-03-19 11:45 AM, Guido Günther wrote:
> Signed-off-by: Guido Günther <agx at sigxcpu.org>
> ---
>   Makefile.am                 |  1 +
>   clients/simple-dmabuf-drm.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
>   configure.ac                |  5 ++-
>   3 files changed, 82 insertions(+), 1 deletion(-)
> 
> 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 22891639..0bc99a59 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -39,6 +39,7 @@
>   #include <signal.h>
>   #include <fcntl.h>
>   #include <getopt.h>
> +#include <errno.h>
>   
>   #include <xf86drm.h>
>   
> @@ -49,6 +50,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>
> @@ -108,6 +112,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;
> @@ -272,6 +280,65 @@ fd_device_destroy(struct buffer *buf)
>   	fd_device_del(buf->fd_dev);
>   }
>   #endif /* HAVE_LIBDRM_FREEDRENO */
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +
> +static int
> +etna_alloc_bo(struct buffer *buf)
> +{
> +	int flags = DRM_ETNA_GEM_CACHE_WC;
> +	int size;
> +
> +	buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
> +	size = 	buf->stride * buf->height;
> +	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)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +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)
> +{
> +	if (munmap(buf->mmap, buf->stride * buf->height) < 0)
> +		fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));

Pekka had said something about cache flushing here in a previous 
comment, I think.  I think this munmap() is enough, but if it isn't I'd 
like to know what's required too.

> +	buf->mmap = NULL;
> +}
> +
> +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)
> @@ -339,6 +406,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])

why the have_etnaviv=no here when the rest are all [true]?

With that explained, this is
Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

But I'm about to start the release process for the beta, so I think this 
will need to wait a while to land.

>   
>     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 

Technically, "neither" is grammatically incorrect here, as it is used 
for 2 things...

Thanks,
Derek

libdrm_etnaviv found])
>     fi
>   
>     if test "x$have_simple_dmabuf_drm_client" = "xyes" -a "x$have_simple_dmabuf_libs" = "xyes"; then
>