Am 16.05.2016 um 22:16 schrieb Rob Clark:
On Mon, May 16, 2016 at 3:53 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
On 15 May 2016 at 12:34, Stanimir Varbanov <stanimir.varba...@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 ?
jfwiw, freedreno already handles the offset, egl just needs to pass it
down. This is why you aren't seeing a hunk for freedreno.
The checks to other drivers for offset!=0 would presumably be removed
when they add support for non-zero offsets. So IMO this change is
correct, at least for the drivers the ignore the offset.
--- 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.]
tbh, being portable to use freedreno (or probably vc4) on windows
doesn't seem terribly important ;-)
For r600/amdgpu we don't want to add this include. For
nouveau/vc4/freedreno it is ok.. they already ignore the winsys
abstraction.
(fwiw, for ilo the offset!=0 check should probably go in
intel_winsys_import_handle())
#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 ?
yeah, looks like r600 does already handle the offset, so I think we
could drop this hunk. It might be the same for amdgpu, not sure.
Yes, both amdgpu as well as radeon already handle that.
And yes the offset is used for VDPAU dma-buf based interop and adding
those checks would break that.
Christian.
BR,
-R
Thanks
Emil
[1] src/mesa/state_tracker/st_vdpau.c
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
