Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

Rob Clark <robdclark@gmail.com> writes:

> On Tue, Jul 26, 2016 at 7:11 PM, Eric Anholt <eric@anholt.net> wrote:
>> Rob Clark <robdclark@gmail.com> writes:
>>
>>> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
>>>> Overflow memory handling is tricky: While it's still referenced by the
>>>> BPO registers, we want to keep it from being freed.  When we are
>>>> putting a new set of overflow memory in the registers, we need to
>>>> assign the old one to the last rendering job using it.
>>>>
>>>> We were looking at "what's currently running in the binner", but since
>>>> the bin/render submission split, we may end up with the binner
>>>> completing and having no new job while the renderer is still
>>>> processing.  So, if we don't find a bin job at all, look at the
>>>> highest-seqno (last) render job to attach our overflow to.
>>>
>>> so, drive-by comment.. but can you allocate gem bo's without backing
>>> them immediately with pages?  If so, just always allocate the bo
>>> up-front and attach it as a dependency of the batch, and only pin it
>>> to actual pages when you have to overflow?
>>
>> The amount of overflow for a given CL is arbitrary, depending on the
>> geometry submitted, and the overflow pool just gets streamed into by the
>> hardware as you submit bin jobs.  You'll end up allocating [0,n] new
>> overflows per bin job.  I don't see where "allocate gem BOs without
>> backing them immediately with pages" idea would fit into this.
>
> well, even not knowing the size up front shouldn't really be a
> show-stopper, unless you had to mmap it to userspace, perhaps..
> normally backing pages aren't allocated until drm_gem_get_pages() so
> allocating the gem bo as placeholder to track dependencies of the
> batch/submit shouldn't be an issue.  But I noticed you don't use
> drm_gem_get_pages().. maybe w/ cma helpers it is harder to decouple
> allocation of the drm_gem_object from the backing store.

There's no period of time between "I need to allocate an overflow BO"
and "I need pages in the BO", though.

I could have a different setup that allocated a massive (all of CMA?),
fresh overflow BO per CL and populated page ranges in it as I overflow,
but with CMA you really need to never do new allocations in the hot path
because you get to stop and wait approximately forever.  So you'd want
to chunk it up so you could cache the groups of contiguous pages of
overflow, and it turns out we already have a thing for this in the form
of GEM BOs.  Anyway, doing that that means you're losing out on the rest
of the last overflow BO for the new CL, expanding the working set in
your precious 256MB CMA area.

Well, OK, actually I *do* allocate a fresh overflow BO per CL today,
because of leftover bringup code that I think I could just delete at
this point.  I'm not doing that in a -fixes commit, though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]