Re: [PATCH 01/11] drm/amdgpu: Align amdgpu_gtt_mgr entries to TLB size on Tahiti

Christian König
2026-04-23T13:32:39Z

On 4/23/26 14:18, Timur Kristóf wrote:
> On Thursday, April 23, 2026 1:04:53 PM Central European Summer Time Christian
> König wrote:
>> On 4/23/26 03:16, Timur Kristóf wrote:
>>> The TLB is organized in groups of 8 entries, each one is 4K.
>>> On Tahiti, the HW requires these GART entries to be 32K-aligned.
>>>
>>> This fixes a VCE 1 firmware validation failure that can happen
>>> after suspend/resume since we use amdgpu_gtt_mgr for VCE 1.
>>>
>>> Fixes: 698fa62f56aa ("drm/amdgpu: Add helper to alloc GART entries")
>>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
>>> ---
>>>
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index
>>> 9b0bcf6aca445..673e9e08c66a0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> @@ -198,12 +198,19 @@ int amdgpu_gtt_mgr_alloc_entries(struct
>>> amdgpu_gtt_mgr *mgr,>
>>> u64 num_pages,
>>> enum drm_mm_insert_mode mode)
>>>
>>> {
>>>
>>> + u32 alignment = 0;
>>>
>>> struct amdgpu_device *adev = container_of(mgr, typeof(*adev),
>>> mman.gtt_mgr); int r;
>>
>> Only a nit, but reverse xmas tree order please for variable declarations.
>
> I haven't found this in the Linux coding style guide, can you elaborate what
> you are referring to exactly?

It's not general coding style rule, but some subsystem maintainers usually suggest to have longer lines with constant initializers (e.g. like adev in this example) first. Then variables with longer names like "alignment" and finally temporary variables like "i", "r", etc...

I think it keeps code a bit more readable sometimes.

Regards,
Christian.

>
>>
>>> + /* Align to TLB size on Tahiti */
>>
>> Oh that needs improvement.
>>
>> Maybe something like /* Align to TLB L2 cache entry size to work around V
>> bit HW bug */
>>
>> Mostly nobody will know what that "V bit HW bug" is, but at least AMD people
>> can search for that in the HW docs.
>
> Sounds good, will add those details to the comments (and commit message)
> in the next version of the series.
>
>>
>> With that fixed Reviewed-by: Christian König <christian.koenig@amd.com>.
>>
>> Thanks,
>> Christian.
>
> Thanks!
>
>>
>>> + if (adev->asic_type == CHIP_TAHITI) {
>>> + alignment = 32 * 1024 / AMDGPU_GPU_PAGE_SIZE;
>>> + num_pages = ALIGN(num_pages, alignment);
>>> + }
>>> +
>>>
>>> spin_lock(&mgr->lock);
>>> r = drm_mm_insert_node_in_range(&mgr->mm, mm_node, num_pages,
>>>
>>> - 0,
> GART_ENTRY_WITHOUT_BO_COLOR, 0,
>>> + alignment,
> GART_ENTRY_WITHOUT_BO_COLOR, 0,
>>>
>>> adev->gmc.gart_size >>
> PAGE_SHIFT,
>>> mode);
>>>
>>> spin_unlock(&mgr->lock);
>
>
>
>