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

[Emil Velikov]

On 28 October 2016 at 15:33, Rob Herring <robh at kernel.org> wrote:
> 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.
>
Point I was [trying to] make is that this is very iffy way to pick
render node device. Afaict every kernel nowadays has at least two -
vgem and the hw one (i915,nouveau).

Although far from ideal, the DRM_FD allows you to keep the heuristics
(shared device in drm_gralloc case and pick a random one* for
gbm_gralloc). That is, until liballoc comes about and we could nuke
most/all of that stuff into orbit ;-)

* Just do some basic probing/test on the device (/me is pitching for
drmDevice, but anything will do). Not to mention that one can override
the heuristics - be that device, drm module or the exact render node
name via the standard Android property_get().

-Emil