<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Sep 29, 2016 at 7:33 AM Martin Peres <<a href="mailto:martin.peres@linux.intel.com">martin.peres@linux.intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 >> <<a href="mailto:chadversary@chromium.org" class="gmail_msg" target="_blank">chadversary@chromium.org</a>> 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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=71759" rel="noreferrer" class="gmail_msg" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=71759</a> >>>> Signed-off-by: Martin Peres <<a href="mailto:martin.peres@linux.intel.com" class="gmail_msg" target="_blank">martin.peres@linux.intel.com</a>> >>>> --- >>>> >>>> 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. </blockquote><div> </div><div>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.</div><div> </div><div>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.</div><div> </div><div>Kristian</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> So, what do we do? We cannot use one single buffer manager for everyone unless we always force the authentication dance :s Martin </blockquote></div></div>