[PATCH] drm/amdgpu: Raven: don't allow mixing GTT and VRAM

[Christian König]

On 28.07.25 18:38, Alex Deucher wrote:
>>>> Anyway, back to your suggestion, I think we can probably drop the
>>>> checks as you should always get a compatible memory buffer due to
>>>> amdgpu_bo_get_preferred_domain(). Pinning should fail if we can't pin
>>>> in the required domain.  amdgpu_display_supported_domains() will
>>>> ensure you always get VRAM or GTT or VRAM | GTT depending on what the
>>>> chip supports.  Then amdgpu_bo_get_preferred_domain() will either
>>>> leave that as is, or force VRAM or GTT for the STONEY/CARRIZO case.
>>>> On the off chance we do get incompatible memory, something like the
>>>> attached patch should do the trick.
>>
>> Thanks for the patch, this makes sense to me.
>>
>> Somewhat unrelated: I wonder if setting AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS is necessary before
>> bo_pin(). FWIU from chatting with our DCN experts, DCN doesn't really care if the fb is
>> contiguous or not.
> 
> Is this a APU statement or dGPU statement?  At least on older dGPUs,
> they required contiguous VRAM.  This may not be an issue on newer
> chips with DCHUB. At the moment, we use the FB aperture to access VRAM
> directly in the kernel driver, so we do not set up page tables for
> VRAM.  We'd need to do that to support linear mappings of
> non-contiguous VRAM buffers in the kernel driver.  We do support it on
> some MI chips, so it's doable, but it adds overhead.
> 
>>
>> Which begs the question -- what exactly does AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS mean? From git
>> history, it seems setting this flag doesn't necessarily move the bo to be congiguous. But
>> rather:
>>
>>     When we set AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS
>>     - This means contiguous is not mandatory.
>>     - we will try to allocate the contiguous buffer. Say if the
>>       allocation fails, we fallback to allocate the individual pages.
>>
>> https://github.com/torvalds/linux/commit/e362b7c8f8c7af00d06f0ab609629101aebae993
>>
>> Does that mean -- if the buffer is already in the required domain -- that bo_pin() will also
>> attempt to make it contiguous? Or will it just pin it from being moved and leave it at that?
>>
> 
> It means that the VRAM backing for the buffer will be physically contiguous.
> 
>> I guess in any case, it sounds like VRAM_CONTIGUOUS is not necessary for DCN scanout.
>> I can give dropping it a spin and see if IGT complains.
> 
> That won't work unless we change how we manage VRAM in vmid0.  Right
> now we use the FB aperture to directly access it, if we wanted to use
> non-contiguous pages, we'd need to use page tables for VRAM as well.

Yeah, that isn't easily doable. We looked into that when on first HW generation with DCHUB.

In the end it was more trouble managing the page tables for VRAM than allocating VRAM contiguously.

Regards,
Christian.

> 
> Alex