[Mesa-dev] [RFC] mesa: drop current draw/read buffer when ctx is released

[Rob Herring]

On Fri, Oct 28, 2016 at 9:14 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 28 October 2016 at 13:22, Rob Clark <robdclark at gmail.com> wrote:
>> On Fri, Oct 28, 2016 at 1:24 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>>> On 10/27/2016 01:48 PM, Rob Clark wrote:
>>>>
>>>> On Thu, Oct 27, 2016 at 2:59 AM, Tapani Pälli <tapani.palli at intel.com>
>>>> wrote:
>>>>>
>>>>> On 10/27/2016 12:16 AM, Rob Clark wrote:
>>>>>>
>>>>>> So, not quite sure if this is the *correct* solution, but it is at least
>>>>>> *a* solution to a problem with android wallpaper vs mesa that I've been
>>>>>> debugging.  Basically, what happens is:
>>>>>
>>>>>
>>>>> Could you tell more how to trigger this, is this with some particular
>>>>> live
>>>>> wallpaper and has this been working before (regression)? For me at least
>>>>> these default wallpapers have been working ok.
>>>>
>>>> Actually, it is the default static wallpaper that is problematic.  And
>>>> IIRC, it has never worked, at least not with any of the gallium
>>>> drivers (freedreno, virgl, vc4, etc).  Live-wallpaper did work, but
>>>> does not appear to exist in AOSP builds anymore.
>>>>
>>>> If this works with i965 on android, I'd be curious how.  Or if you're
>>>> android build had some mesa patches that are not upstream?
>>>
>>>
>>> I can confirm that default wallpaper is working on i965. Our Mesa tree
>>> currently looks like this:
>>>
>>> https://github.com/android-ia/external-mesa
>>>
>>> It's now quite a bit behind though because we were dealing with some
>>> non-graphics issues and are planning to rebase soon.
>>
>> I suppose it is possible that it is triggered by something different
>> that mesa/st does vs dri/i965?  Although I find it odd that
>> dri2_make_current() would drop the reference to the EGLSurface when
>> unbinding ctx, but _mesa_make_current() would not drop the reference
>> to the corresponding gl_framebuffer.
>>
> Precisely my line of thinking as well. Both egl and glx drop the
> reference to the objects on !newCtx.
>
> From a quick look the only place where we unref.
> WinSysDrawBuffer/WinSysReadBuffer is in _mesa_free_context_data which
> happens on ctx teardown. Which sounds incomplete -  so I'm a bit
> curious why/how that i965 works fine.
>
>> Maybe the classic driver is holding an extra reference to the
>> EGLSurface so the _eglPutSurface() call in dri2_make_current() does
>> not actually drop the last refcnt?  Which would ensure when the window
>> surface is created it ends up with a different address..
>>
>> but, I wonder if you could try w/ Rob Herring's tree.. this is what we
>> are using for the upstream kernel + AOSP builds[1]:
>>
>>   https://github.com/robherring/mesa/commits/android-m
>>
>> I guess in theory we should both need the same patches on top of
>> mesa..  although also I guess we should also just try to get to the
>> point where we can both use upstream mesa tree directly.
>>
> Indeed. Skimming through the android-ia log - can borrow
> ENABLE_FLINK_SUPPORT and let drm/gbm gralloc have a static inline
> helpers gralloc_drm_get_gem_handle/gralloc_drm_get_prime_fd.
> Thus everyone can polish their gralloc (if interested?) while still
> having something that works across the board. With similar direction
> on the GRALLOC_MODULE_PERFORM_GET_DRM_FD front ?

It's all related. If we use dma-buf fds, then we use the render node
instead of card node and don't need private gralloc access. We just
assume if the buffer handle has an fd that the 1st fd is the dma-buf.

I've been assuming that Tomasz would respin these patches.

Rob