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

[Rob Herring]

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. 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.

>                          /* This one works, splice it out of the cache */
>                          list_del(&entry->link);
>