[Mesa-dev] [PATCH v2 2/5] panfrost: Allocate shaders in their own BOs

[Tomeu Vizoso]

On Thu, 8 Aug 2019 at 00:47, Rob Herring <robh at kernel.org> wrote:
>
> On Wed, Aug 7, 2019 at 2:37 AM Tomeu Vizoso <tomeu.vizoso at collabora.com> wrote:
> >
> > Instead of all shaders being stored in a single BO, have each shader in
> > its own.
> >
> > This removes the need for a 16MB allocation per context, and allows us
> > to place transient blend shaders in BOs marked as executable (before
> > they were allocated in the transient pool, which shouldn't be
> > executable).
> >
> > v2: - Store compiled blend shaders in a malloc'ed buffer, to avoid
> >       reading from GPU-accessible memory when patching (Alyssa).
> >     - Free struct panfrost_blend_shader (Alyssa).
> >     - Give the job a reference to regular shaders when emitting
> >       (Alyssa).
> >
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
>
>
> > diff --git a/src/gallium/drivers/panfrost/pan_bo_cache.c b/src/gallium/drivers/panfrost/pan_bo_cache.c
> > index fba495c1dd69..7378d0a8abea 100644
> > --- a/src/gallium/drivers/panfrost/pan_bo_cache.c
> > +++ b/src/gallium/drivers/panfrost/pan_bo_cache.c
> > @@ -84,11 +84,10 @@ panfrost_bo_cache_fetch(
> >  {
> >          struct list_head *bucket = pan_bucket(screen, size);
> >
> > -        /* TODO: Honour flags? */
> > -
> >          /* Iterate the bucket looking for something suitable */
> >          list_for_each_entry_safe(struct panfrost_bo, entry, bucket, link) {
> > -                if (entry->size >= size) {
> > +                if (entry->size >= size &&
> > +                    entry->flags == flags) {
>
> This change probably warrants its own patch IMO.

Agreed.

> This is using the
> untranslated flags, but I think it should be the 'translated_flags' as
> those are the ones changing the allocation. For example, I don't think
> there's any reason for DELAYED_MMAP to be used as a match criteria
> (BTW, I'm also not sure if we can reclaim BOs if they remain mmap'ed).
>
> Another problem I see, if we have a 100MB buffer in the cache, would
> we really want to hit on a 4KB allocation? Perhaps a 'entry->size * 2
> < size' check.

Yeah, as mentioned in the v1 discussion, we have plenty of room for
improvements here, but the goal now is just to stop doing memory
allocation so greedily that we reach OOM after launching a few GL
clients.

For 19.3 we could start tracking performance and other metrics such as
peak memory usage, etc.

Cheers,

Tomeu

>
> >                          /* This one works, splice it out of the cache */
> >                          list_del(&entry->link);
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev