[Mesa-dev] [Request for Testing] i965: import prime buffers in the current context, not screen
Kristian Høgsberg
hoegsberg at gmail.com
Mon Oct 3 20:43:15 UTC 2016
On Thu, Sep 29, 2016 at 7:33 AM Martin Peres <martin.peres at 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
> >> <chadversary at 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.peres at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161003/6c37bcdb/attachment-0001.html>
More information about the mesa-dev
mailing list