[Mesa-dev] [PATCH 10/14] gallium/radeon: clean up pb_cache bucket/usage determination

[Nicolai Hähnle]

On 03.07.2017 22:26, Marek Olšák wrote:
> On Mon, Jul 3, 2017 at 9:51 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 29.06.2017 21:47, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> ---
>>>    src/gallium/drivers/radeon/radeon_winsys.h    | 20 ++++++++++++++++++++
>>>    src/gallium/winsys/amdgpu/drm/amdgpu_bo.c     | 21 +++++----------------
>>>    src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 21 +++++----------------
>>>    3 files changed, 30 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeon/radeon_winsys.h
>>> b/src/gallium/drivers/radeon/radeon_winsys.h
>>> index 4ecd73f..239b6ab 100644
>>> --- a/src/gallium/drivers/radeon/radeon_winsys.h
>>> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
>>> @@ -658,20 +658,21 @@ static inline void radeon_emit_array(struct
>>> radeon_winsys_cs *cs,
>>>        cs->current.cdw += count;
>>>    }
>>>      enum radeon_heap {
>>>        RADEON_HEAP_VRAM_NO_CPU_ACCESS,
>>>        RADEON_HEAP_VRAM,
>>>        RADEON_HEAP_VRAM_GTT, /* combined heaps */
>>>        RADEON_HEAP_GTT_WC,
>>>        RADEON_HEAP_GTT,
>>>        RADEON_MAX_SLAB_HEAPS,
>>> +    RADEON_MAX_CACHED_HEAPS = RADEON_MAX_SLAB_HEAPS,
>>>    };
>>>      static inline enum radeon_bo_domain radeon_domain_from_heap(enum
>>> radeon_heap heap)
>>>    {
>>>        switch (heap) {
>>>        case RADEON_HEAP_VRAM_NO_CPU_ACCESS:
>>>        case RADEON_HEAP_VRAM:
>>>            return RADEON_DOMAIN_VRAM;
>>>        case RADEON_HEAP_VRAM_GTT:
>>>            return RADEON_DOMAIN_VRAM_GTT;
>>> @@ -692,20 +693,39 @@ static inline unsigned radeon_flags_from_heap(enum
>>> radeon_heap heap)
>>>        case RADEON_HEAP_VRAM:
>>>        case RADEON_HEAP_VRAM_GTT:
>>>        case RADEON_HEAP_GTT_WC:
>>>            return RADEON_FLAG_GTT_WC;
>>>        case RADEON_HEAP_GTT:
>>>        default:
>>>            return 0;
>>>        }
>>>    }
>>>    +/* The pb cache bucket is chosen to minimize pb_cache misses.
>>> + * It must be between 0 and 3 inclusive.
>>> + */
>>> +static inline unsigned radeon_get_pb_cache_bucket_index(enum radeon_heap
>>> heap)
>>> +{
>>> +    switch (heap) {
>>> +    case RADEON_HEAP_VRAM_NO_CPU_ACCESS:
>>> +        return 0;
>>> +    case RADEON_HEAP_VRAM:
>>> +    case RADEON_HEAP_VRAM_GTT:
>>> +        return 1;
>>
>>
>> Why are VRAM and VRAM+GTT in the same heap?
> 
> They are in the same bucket, not heap. Buckets only serve as an optimization.
> 
> Before this patch, multiple heaps could use the same bucket, because
> we had more than 12 heaps and only 4 buckets (pb_cache limit). This
> patch brings it down to 4 heaps (when VRAM_GTT is used, VRAM isn't
> used), which maps perfectly to buckets.

Ah, that makes a lot of sense, thanks. R-b on this patch as well.


>> We don't even use VRAM+GTT on amdgpu anymore, but maybe this affects radeon
>> in an adverse way...
> 
> We use VRAM_GTT on amdgpu < 3.6.0.

Hence not anymore :)

Cheers,
Nicolai

> 
> Marek
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.