[PATCH] drm/amdgpu: Support >=4GB GTT memory mapping

[Christian König]

Am 29.01.24 um 17:25 schrieb Philip Yang:
> On 2024-01-29 05:06, Christian König wrote:
>> Am 26.01.24 um 20:47 schrieb Philip Yang:
>>> This is to work around a bug in function drm_prime_pages_to_sg if 
>>> length
>>> of nr_pages >= 4GB, by doing the same check for max_segment and then
>>> calling sg_alloc_table_from_pages_segment directly instead.
>>>
>>> This issue shows up on APU because VRAM is allocated as GTT memory. It
>>> also fixes >=4GB GTT memory mapping for mGPUs with IOMMU isolation 
>>> mode.
>>
>> Well that was talked about before and rejected. If we really want 
>> more than 4GiB in DMA-bufs we need to fix drm_prime_pages_to_sg() 
>> instead.
>
> I sent a patch to fix drm_prime_pages_to_sg but the patch was rejected.
>

Why was that rejected? If this isn't something we want for DRM we 
probably don't want it for AMDGPU either.

> This issue happens on APU, as VRAM is allocated as GTT memory, get to 
> this patch only if IOMMU is isolation mode, with IOMMU off or pt mode, 
> multiple GPUs share the same dma mapping.
>
> Even with the fix patch accepted by drm, we still need this patch to 
> workaround the issue on old kernel version.
>

Yeah, but that's then just a functions fixup for our backporting team 
and shouldn't be worked around like this.

Regards,
Christian.

> Regards,
>
> Philip
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Philip Yang <Philip.Yang at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 50 
>>> ++++++++++++++-------
>>>   1 file changed, 34 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> index 055ba2ea4c12..a203633fd629 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> @@ -171,18 +171,41 @@ static struct sg_table 
>>> *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
>>>       }
>>>         switch (bo->tbo.resource->mem_type) {
>>> -    case TTM_PL_TT:
>>> -        sgt = drm_prime_pages_to_sg(obj->dev,
>>> -                        bo->tbo.ttm->pages,
>>> -                        bo->tbo.ttm->num_pages);
>>> -        if (IS_ERR(sgt))
>>> -            return sgt;
>>> -
>>> -        if (dma_map_sgtable(attach->dev, sgt, dir,
>>> -                    DMA_ATTR_SKIP_CPU_SYNC))
>>> -            goto error_free;
>>> -        break;
>>> +    case TTM_PL_TT: {
>>> +        size_t max_segment = 0;
>>> +        u64 num_pages;
>>> +        int err;
>>> +
>>> +        sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
>>> +        if (!sgt)
>>> +            return ERR_PTR(-ENOMEM);
>>> +
>>> +        if (obj->dev)
>>> +            max_segment = dma_max_mapping_size(obj->dev->dev);
>>> +        if (max_segment == 0)
>>> +            max_segment = UINT_MAX;
>>> +
>>> +        /*
>>> +         * Use u64, otherwise if length of num_pages >= 4GB then size
>>> +         * (num_pages << PAGE_SHIFT) becomes 0
>>> +         */
>>> +        num_pages = bo->tbo.ttm->num_pages;
>>> +        err = sg_alloc_table_from_pages_segment(sgt, 
>>> bo->tbo.ttm->pages,
>>> +                            num_pages, 0,
>>> +                            num_pages << PAGE_SHIFT,
>>> +                            max_segment, GFP_KERNEL);
>>> +        if (err) {
>>> +            kfree(sgt);
>>> +            return ERR_PTR(err);
>>> +        }
>>>   +        if (dma_map_sgtable(attach->dev, sgt, dir, 
>>> DMA_ATTR_SKIP_CPU_SYNC)) {
>>> +            sg_free_table(sgt);
>>> +            kfree(sgt);
>>> +            return ERR_PTR(-EBUSY);
>>> +        }
>>> +        break;
>>> +    }
>>>       case TTM_PL_VRAM:
>>>           r = amdgpu_vram_mgr_alloc_sgt(adev, bo->tbo.resource, 0,
>>>                             bo->tbo.base.size, attach->dev,
>>> @@ -195,11 +218,6 @@ static struct sg_table 
>>> *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
>>>       }
>>>         return sgt;
>>> -
>>> -error_free:
>>> -    sg_free_table(sgt);
>>> -    kfree(sgt);
>>> -    return ERR_PTR(-EBUSY);
>>>   }
>>>     /**
>>