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

[Martin Peres]

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!

>
> Kristian
>
>>> +    */
>>> +   if (brw)
>>> +       bufmgr = brw->bufmgr;
>>> +   else
>>> +       bufmgr = intelScreen->bufmgr;
>>> +
>>> +   image->bo = drm_intel_bo_gem_create_from_prime(bufmgr, fds[0], size);
>>>     if (image->bo == NULL) {
>>>        free(image);
>>>        return NULL;
>>> --
>>> 2.9.3
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>