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

[Pekka Paalanen]

On Thu, 15 Mar 2018 17:33:21 +0100
Guido Günther <agx at sigxcpu.org> wrote:

> ---
>  Makefile.am                 |  1 +
>  clients/simple-dmabuf-drm.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
>  configure.ac                |  5 +++-
>  3 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index c2d448e7..19319fa2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -626,6 +626,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 492c9667..a445ffb2 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -48,6 +48,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>
> @@ -101,6 +104,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;
> @@ -264,6 +271,62 @@ fd_device_destroy(struct buffer *buf)
>  	fd_device_del(buf->fd_dev);
>  }
>  #endif /* HAVE_LIBDRM_FREEDRENO */
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +#define ROUND_DOWN(v, a) ((v) & ~(a - 1))
> +
> +static int
> +etna_alloc_bo(struct buffer *buf)
> +{
> +	int flags = DRM_ETNA_GEM_CACHE_WC;
> +	int size = buf->width * buf->height * buf->bpp / 8;
> +	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;
> +	buf->stride = ROUND_DOWN(buf->width, 32) * buf->bpp / 8;

Hi,

this still looks strange to me. You get requested to allocate for
width×height. You compute the tight size without any alignment and
allocate that. Then you return the allocated stride rounded down, which
makes it potentially smaller than the stride you use for allocating the
buffer. This means that if the caller iterates a row from x=0 to
x=width-1, it may access pixels past the end, either from the next row
or past the end of the buffer.

Rounding up seemed like the right thing to do, you just need to compute
the stride before size, and compute size from stride to include the
padding you need. Right?


Thanks,
pq

> +	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 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)
> +{
> +}
> +
> +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)
> @@ -331,6 +394,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 dc2fbe93..e86c31fa 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" && \
>  	  "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"; then

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180316/bb7442d5/attachment.sig>