Re: [PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

[Martin Peres <martin.peres@labri.fr>]

On 11/07/2013 13:21, David Herrmann wrote:
> Hi
>
> On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres <martin.peres@labri.fr> wrote:
>> On 07/07/2013 19:17, David Herrmann wrote:
>>> Hi
>>>
>>> This is v2 of the unified VMA offset manager series. The first draft is
>>> available at LWN [1]. This series replaces the VMA offset managers in GEM
>>> and
>>> TTM with a unified implementation.
>>>
>>> The first 4 patches contain the new VMA offset manager and are the only
>>> patches
>>> that I propose for mainline inclusion.
>>> Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
>>> patches in i915-next and I will rebase them once it is merged. Ignore
>>> them if you're not interested.
>>> Patches 9-19 implement mmap access control. Comments are welcome! They are
>>> intended for mainline inclusion, too, but probably require some more
>>> review
>>> rounds. I'd really appreciate if driver authors could comment on the
>>> implementation.
>>> Patch 20 shows the main motivation behind this whole series: render nodes.
>>> It is
>>> marked RFC and I will resend once the VMA manager is merged upstream. Feel
>>> free
>>> to test it.
>>>
>>> One more note regarding DRM-Master: Render-clients are independent of
>>> DRM-Master
>>> (see the DocBook documentation). It really doesn't make sense to
>>> create/bind a
>>> DRM-Master object to render-clients. However, a lot of core DRM code
>>> depends on
>>>    ->master != NULL. Drivers need to take care not to call into those core
>>> modules
>>> if they implement DRIVER_RENDER.
>>> I plan on removing multiple-master-support in the next series. So there
>>> will be
>>> only one global master and each open-file is bound to it. This will make
>>> it very
>>> easy to phase out "drm_master". The planned "modeset" nodes provide a nice
>>> replacement. If anyone knows a **currently used** use-case for
>>> multiple-masters,
>>> please tell me. I couldn't find one that _actually works_.
>>> (SetMaster+DropMaster
>>> will obviously stay functional even with drm_master removed).
>>>
>>>
>>> (series available at: https://github.com/dvdhrm/linux/tree/rnodes)
>>>
>>> Comments welcome!
>>> Cheers
>>> David
>> Hi David,
>>
>> Thanks for this patchset. I am away from my computers for testing but I was
>> wondering if you based your vma rework on Dave's implementation. If so,
>> you may have the same bug that I had with it.
>>
>> I cannot remember the details of the bug, but I could trigger it by
>> rebooting
>> kde around 13 times on radeon. I didn't have this problem with Nouveau.
> It is based on Dave's code, but I rewrote all of it and fixed several
> bugs. For instance, there was a TTM ref-count leak for BOs in TTM core
> and a TTM locking issue. I didn't encounter any bugs so far, but I
> didn't try to restart the xserver 13 times. I will do some more
> stress-tests, but it is quite likely you hit one of those two bugs
> with radeon.

Yeah, the problem I had was related to ref-count and I was trying
to fix the locking too.
>> I am not competent to judge this work but I will try to test it and check
>> with my security tests that I wrote that the problem is indeed solved
>> on nouveau and radeon.
> The changes are actually quite simple. I intentionally kept TTM
> locking as it was before, even though I think we could reduce it now.
> Anyway, I will resend v3 (which now includes tegra and staging) this
> weekend. Hopefully we can get it into linux-next soon.

Very nice! Looking forward to it.