On 1/6/26 13:55, Donet Tom wrote:
> 
> On 12/12/25 2:23 PM, Christian König wrote:
>> On 12/12/25 07:40, Donet Tom wrote:
>>> The SDMA engine has a hardware limitation of 4 MB maximum transfer
>>> size per operation.
>> That is not correct. This is only true on ancient HW.
>>
>> What problems are you seeing here?
>>
>>> AMDGPU_GTT_MAX_TRANSFER_SIZE was hardcoded to
>>> 512 pages, which worked correctly on systems with 4K pages but fails
>>> on systems with larger page sizes.
>>>
>>> This patch divides the max transfer size / AMDGPU_GPU_PAGES_IN_CPU_PAGE
>>> to match with non-4K page size systems.
>> That is actually a bad idea. The value was meant to match the PMD size.
> 
> 
> Hi Christian, Felix, Alex and philip
> 
> Instead of hardcoding the AMDGPU_GTT_MAX_TRANSFER_SIZE value to 512,
> what do you think about doing something like the change below?
> This should work across all architectures and page sizes, so
> AMDGPU_GTT_MAX_TRANSFER_SIZE will always correspond to the PMD
> size on all architectures and with all page sizes.
> 
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 0be2728aa872..c594ed7dff18 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -37,7 +37,7 @@
>  #define AMDGPU_PL_MMIO_REMAP   (TTM_PL_PRIV + 5)
>  #define __AMDGPU_PL_NUM        (TTM_PL_PRIV + 6)
>  
> -#define AMDGPU_GTT_MAX_TRANSFER_SIZE   512
> +#define AMDGPU_GTT_MAX_TRANSFER_SIZE   1 << (PMD_SHIFT - PAGE_SHIFT)
>  #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS  
> 
> Could you please provide your thoughts on above? Is it looking ok to you?

It's at least reasonable. My only concern is that we have patches in the 
pipeline to remove that define and make it independent of the PMD size.

@Pierre-Eric how far along are we with that?

> 
> If this looks good - here is what we were thinking:
> 
> Patches 1-4 are required to fix initial non-4k pagesize support to AMD GPU.
> And since these patches are looking in good shape (since Philip has already
> reviewed [1-3])- We thought it will be good to split the patch series into 
> two.
> I will send a v2 of Part-1 with patches [1-4] (will also address the review 
> comments
> in v2 for Patch-1 & 2 from Philip) and for the rest of the patches [5-8] 
> Part-2, we
> can continue the discussion till other things are sorted. That will also 
> allow us to
> get these initial fixes in Part-1 ready before the 6.20 merge window. 
> 
> Thoughts?

Sounds reasonable to me.

Regards,
Christian.

> 
> 
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Donet Tom <[email protected]>
>>> Signed-off-by: Ritesh Harjani (IBM) <[email protected]>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index 0be2728aa872..9d038feb25b0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -37,7 +37,7 @@
>>>  #define AMDGPU_PL_MMIO_REMAP       (TTM_PL_PRIV + 5)
>>>  #define __AMDGPU_PL_NUM    (TTM_PL_PRIV + 6)
>>>  
>>> -#define AMDGPU_GTT_MAX_TRANSFER_SIZE       512
>>> +#define AMDGPU_GTT_MAX_TRANSFER_SIZE       (512 / 
>>> AMDGPU_GPU_PAGES_IN_CPU_PAGE)
>>>  #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS    2
>>>  
>>>  extern const struct attribute_group amdgpu_vram_mgr_attr_group;

Reply via email to