On Thu, Sep 29, 2016 at 7:33 AM Martin Peres <martin.pe...@linux.intel.com> wrote:
> On 02/09/16 10:08, Martin Peres wrote: > > > > > > On 25/08/16 21:49, Kristian Høgsberg wrote: > >> On Thu, Aug 25, 2016 at 11:38 AM, Chad Versace > >> <chadvers...@chromium.org> wrote: > >>> On Thu 25 Aug 2016, Martin Peres wrote: > >>>> This mirrors the codepath taken by DRI2 in IntelSetTexBuffer2() and > >>>> fixes many applications when using DRI3: > >>>> - Totem with libva on hw-accelerated decoding > >>>> - obs-studio, using Window Capture (Xcomposite) as a Source > >>>> - gstreamer with VAAPI > >>>> > >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71759 > >>>> Signed-off-by: Martin Peres <martin.pe...@linux.intel.com> > >>>> --- > >>>> > >>>> This patch supposedly prevents gnome from running for one Arch Linux > >>>> maintainer. Kenneth Graunke and I failed to reproduce it so I am > >>>> asking for your help/comments on this. > >>>> > >>>> src/mesa/drivers/dri/i965/intel_screen.c | 24 > ++++++++++++++++++++++-- > >>>> 1 file changed, 22 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > >>>> b/src/mesa/drivers/dri/i965/intel_screen.c > >>>> index 7876652..5c0d300 100644 > >>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c > >>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c > >>>> @@ -698,8 +698,11 @@ intel_create_image_from_fds(__DRIscreen *screen, > >>>> int *fds, int num_fds, int *strides, > >>>> int *offsets, > >>>> void *loaderPrivate) > >>>> { > >>>> + GET_CURRENT_CONTEXT(ctx); > >>>> struct intel_screen *intelScreen = screen->driverPrivate; > >>>> + struct brw_context *brw = brw_context(ctx); > >>>> struct intel_image_format *f; > >>>> + dri_bufmgr *bufmgr; > >>>> __DRIimage *image; > >>>> int i, index; > >>>> > >>>> @@ -740,8 +743,25 @@ intel_create_image_from_fds(__DRIscreen *screen, > >>>> size = end; > >>>> } > >>>> > >>>> - image->bo = > drm_intel_bo_gem_create_from_prime(intelScreen->bufmgr, > >>>> - fds[0], size); > >>>> + /* Let's import the buffer into the current context instead of > >>>> the current > >>>> + * screen as some applications like gstreamer, totem, or obs > >>>> create multiple > >>>> + * X connections which end up creating multiple screens and thus > >>>> multiple > >>>> + * buffer managers. They then proceed to use a different X > >>>> connection to call > >>>> + * GLXBindTexImageExt() which should import the buffer in the > >>>> current thread > >>>> + * bound and not the current screen. This is done properly > >>>> upstairs for > >>>> + * texture management so we need to mirror this behaviour if we > >>>> don't want > >>>> + * the kernel rejecting our pushbuffers as the buffers have not > >>>> been imported > >>>> + * by the same bufmgr that sent the pushbuffer. > >>>> + * > >>>> + * If there is no context currently bound, then revert to using > >>>> the screen's > >>>> + * buffer manager and hope for the best... > >>> > >>> Nope. This patch breaks EGL_EXT_image_dma_buf_import. > >>> > >>> When the user calls eglCreateImageKHR(EGLDisplay, EGLContext, ...) with > >>> image type EGL_LINUX_DMA_BUF_EXT, then the spec requires that context > be > >>> NULL. The EGLDisplay parameter determines the namespace of the newly > >>> created > >>> EGLImage. By design, the currently bound context (and its display) DO > >>> NOT affect > >>> eglCreateImage. > >>> > >>> Problem Example: > >>> eglMakeCurrent(dpyA, ..., ctxA); > >>> img = eglCreateImage(dpyB, EGL_NO_CONTEXT, ...); > > > > I see, that may explain the issue Ionut found with gnome. Thanks Chad! > > > >> > >> The difference between DRI2 and DRI3 is that for DRI2 we'd get a > >> DRI2Buffer back from getBuffers, and then open the flink name inside > >> the driver with the current context's bufmgr. In the DRI3 world, we go > >> from prime fd to drm_bo in dri3_get_pixmap_buffer() with the screen > >> that's associated with the current drawable. > > > > Yes, that is the problem indeed. > > > >> > >> I think the fix we're looking for is to make > >> draw->vtable->get_dri_context() also return the __DRIscreen, then use > >> that in dri3_get_pixmap_buffer() to get the right __DRIscreen to pass > >> to loader_dri3_create_image(). > > > > Seems sensible and wouldn't require changing the world! Thanks Kristian! > > I will get to it when I come back from vacation! > > Hey, > > I am finally trying your approach but then I am not sure I understand > what you are saying. > > My patch was changing the import to always happen in the currently-bound > screen, and you said it violates the spec of EGL_EXT_image_dma_buf_import. > > eglCreateImage's dpy is passed all the way down to > intel_create_image_from_fds, which is what the spec mandates. > The problem isn't eglCreateImage, it's when the DRI driver internally imports a prime fd in dri3_get_pixmap_buffer() in src/loader/loader_dri3_helper.c. We want the DRI screen passed to loader_dri3_create_image (line 1150) to come from the current context, not the drawable. We don't have a way to get the DRI screen associated with the current context (__DRIcontext is opaque here), but my suggestion above is that we make draw->vtable->get_dri_context() return it along with the __DRIcontext or perhaps make a new vfunc to get the __DRIscreen for the current context. For platform_x11_dri3.c, egl_dri3_get_dri_context() can get the screen for the current context by following dri2_egl_display(dri2_ctx->base.Resource.Display)->dri_screen. Kristian > So, what do we do? We cannot use one single buffer manager for everyone > unless we always force the authentication dance :s > > Martin >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
