Re: [RFC PATCH 2/2] drm/amdgpu/uvd: Ensure vcpu bos are within the uvd segment

[John Olender <john.olender@gmail.com>]

On 6/3/25 12:26 PM, Christian König wrote:
> 
> 
> On 6/3/25 16:34, John Olender wrote:
>>>> Oh, that's a very interesting find. Could you try to turn around the way the patch works?
>>>>
>>>> E.g. instead of forcing the UVD FW into the first segment, change amdgpu_uvd_force_into_uvd_segment() so that the BOs are forced into the same segment as the UVD firmware.
>>>>
>>
>> I started implementing this and I realized two main problems with this
>> approach.
>>
>> First, there's currently no guarantee the UVD FW does not cross a 256MB
>> boundary.
> 
> There actually is a guarantee for that.
> 
> FW BOs are always allocated contiguously, and the allocation backend makes sure that for example an 16MiB (rounded up) FW is always aligned to a 16MiB boundary.
> 
> So FW memory never crosses a segment boundary larger than it's rounded up size. Without that tons of things in our driver would fall apart, not just UVD :)

Aha, thanks!

> 
>   Checking for this and providing a fallback is going to make
>> this patch... not really any less complex than the original.
>>
>> Second, most of time this is just going to end up selecting the first
>> segment anyway.  I'll go more into this below.
> 
> That's fine with me. I'm just concerned about that 1% of people where it potentially worked before and we are now breaking it.
> 
>>
>>>> That would resolve my concern that this could overload the first segment. The feedback and message BO are usually rather small (4 or 128k IIRC), but the firmware is a couple of megabytes in size.
>>>>
>>>> When we have other FW and VGA emulation buffers in the first segment as well then that could result into clashing that segment to much.
>>>>

Okay this makes much more sense.  I read this as general concern and put
the brakes on, especially after missing the above.

I'll resume work on the simplified patch.

Thanks again,
John