[Mesa-dev] [PATCH 3/3] gallium: push offset down to driver

[Emil Velikov]

On 15 May 2016 at 12:34, Stanimir Varbanov <stanimir.varbanov at linaro.org> wrote:
> Push offset down to drivers when importing dmabuf. This is needed
> to more fully support EGL_EXT_image_dma_buf_import when a non-zero
> offset is specified.
>
How did you test this, on which hardware ? Please add such information
to the commit message.

My knowledge in the area is far from perfect, although it seems that
you're pushing the offset check as opposed to the actual offset.
Afaics there is nothing in between that makes use of it (the offset)
so this patch should be mostly a no-op. See below why "mostly".

Perhaps you meant to nuke the in-drivers offset checking, or there's
some more work needed in the area ?

> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -29,6 +29,7 @@
>  #include "util/u_format.h"
>  #include "util/u_memory.h"
>  #include "util/u_pack_color.h"
> +#include "state_tracker/drm_driver.h"

This does not look great. struct winsys_handle is supposed to stay
opaque for the driver. radeon/amdgpu is a nice example of
winsys/driver separation (grep for "_from_handle\>" to see how they've
done it) although I wonder if this is correct... read below.

[And yes, we might want to address freedreno and vc4 and move the
winsys code from $driver_screen.c to gallium/winsys/$driver/ at some
point.]

>  #include <errno.h>
>  #include <inttypes.h>
>
> @@ -1113,6 +1114,9 @@ static struct pipe_resource *r600_texture_from_handle(struct pipe_screen *screen
>         struct radeon_bo_metadata metadata = {};
>         struct r600_texture *rtex;
>
> +       if (whandle->offset != 0)
> +               return NULL;
> +
Afaics some state trackers [1] can provide a a non-zero value and
things are likely to explode there.
Christian, is VDPAU dmabuf going to work with this in place ?


Thanks
Emil

[1] src/mesa/state_tracker/st_vdpau.c