[Mesa-dev] [Request for Testing] i965: import prime buffers in the current context, not screen

[Martin Peres]

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.

So, what do we do? We cannot use one single buffer manager for everyone 
unless we always force the authentication dance :s

Martin