[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <91d75479-8569-40e1-914a-27268d66b5c0@linux.dev>
Date: Sat, 2 Nov 2024 23:31:24 +0800
From: Sui Jingfeng <sui.jingfeng@...ux.dev>
To: Lucas Stach <l.stach@...gutronix.de>,
Xiaolei Wang <xiaolei.wang@...driver.com>, linux+etnaviv@...linux.org.uk,
christian.gmeiner@...il.com, airlied@...il.com, daniel@...ll.ch
Cc: etnaviv@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] drm/etnaviv: Request pages from DMA32 zone on
addressing_limited
Hi,
On 2024/10/1 20:17, Lucas Stach wrote:
> Hi Xiaolei,
>
> Am Dienstag, dem 03.09.2024 um 10:08 +0800 schrieb Xiaolei Wang:
>> Remove __GFP_HIGHMEM when requesting a page from DMA32 zone,
>> and since all vivante GPUs in the system will share the same
>> DMA constraints, move the check of whether to get a page from
>> DMA32 to etnaviv_bind().
>>
>> Fixes: b72af445cd38 ("drm/etnaviv: request pages from DMA32 zone when needed")
>> Suggested-by: Sui Jingfeng <sui.jingfeng@...ux.dev>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
>> ---
>>
>> change log
>>
>> v1:
>> https://patchwork.kernel.org/project/dri-devel/patch/20240806104733.2018783-1-xiaolei.wang@windriver.com/
>>
>> v2:
>> Modify the issue of not retaining GFP_USER in v1 and update the commit log.
>>
>> v3:
>> Use "priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;"
>> instead of
>> "priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;"
> I don't understand this part of the changes in the new version. Why
> should we drop the HIGHMEM bit always and not only in the case where
> dma addressing is limited? This seems overly restrictive.
While reading the implementation of the dma_alloc_attrs() function,
except allocate memory from device coherent pool, the rest implementation
just mask out __GFP_DMA, __GFP_DMA32 and __GFP_HIGHMEM anyway.
So ?
```
void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t flag, unsigned long attrs)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
void *cpu_addr;
WARN_ON_ONCE(!dev->coherent_dma_mask);
/*
* DMA allocations can never be turned back into a page pointer, so
* requesting compound pages doesn't make sense (and can't even be
* supported at all by various backends).
*/
if (WARN_ON_ONCE(flag & __GFP_COMP))
return NULL;
if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr))
return cpu_addr;
/* let the implementation decide on the zone to allocate from: */
flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
if (dma_alloc_direct(dev, ops))
cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
else if (use_dma_iommu(dev))
cpu_addr = iommu_dma_alloc(dev, size, dma_handle, flag, attrs);
else if (ops->alloc)
cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs);
else
return NULL;
return cpu_addr;
}
```
> Regards,
> Lucas
>
>> and move the check of whether to get a page from DMA32 to etnaviv_bind().
>>
>> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +++++++++-
>> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 --------
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> index 6500f3999c5f..8cb2c3ec8e5d 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> @@ -536,7 +536,15 @@ static int etnaviv_bind(struct device *dev)
>> mutex_init(&priv->gem_lock);
>> INIT_LIST_HEAD(&priv->gem_list);
>> priv->num_gpus = 0;
>> - priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
>> + priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
>> +
>> + /*
>> + * If the GPU is part of a system with DMA addressing limitations,
>> + * request pages for our SHM backend buffers from the DMA32 zone to
>> + * hopefully avoid performance killing SWIOTLB bounce buffering.
>> + */
>> + if (dma_addressing_limited(dev))
>> + priv->shm_gfp_mask |= GFP_DMA32;
>>
>> priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
>> if (IS_ERR(priv->cmdbuf_suballoc)) {
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> index 7c7f97793ddd..5e753dd42f72 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> @@ -839,14 +839,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>> if (ret)
>> goto fail;
>>
>> - /*
>> - * If the GPU is part of a system with DMA addressing limitations,
>> - * request pages for our SHM backend buffers from the DMA32 zone to
>> - * hopefully avoid performance killing SWIOTLB bounce buffering.
>> - */
>> - if (dma_addressing_limited(gpu->dev))
>> - priv->shm_gfp_mask |= GFP_DMA32;
>> -
>> /* Create buffer: */
>> ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, &gpu->buffer,
>> PAGE_SIZE);
--
Best regards,
Sui
Powered by blists - more mailing lists