Re:Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit

["Andy Yan" <andyshrk@163.com>]

Hi Robin,

 Thanks for your comment。

At 2024-10-17 01:38:23, "Robin Murphy" <robin.murphy@arm.com> wrote:
>On 2024-09-20 9:20 am, Andy Yan wrote:
>> From: Andy Yan <andy.yan@rock-chips.com>
>> 
>> The vop mmu support translate physical address upper 4 GB to iova
>> below 4 GB. So set dma mask to 64 bit to indicate we support address
>>> 4GB.
>> 
>> This can avoid warnging message like this on some boards with DDR
>>> 4 GB:
>> 
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 0 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 0 (slots)
>
>There are several things wrong with this...
>
>AFAICS the VOP itself still only supports 32-bit addresses, so the VOP 
>driver should only be setting a 32-bit DMA mask. The IOMMUs support 
>either 32-bit or 40-bit addresses, and the IOMMU driver does set its DMA 
Does that mean we can only use the dev of IOMMU ? If that is true， would you
please give some inspiration on how to implement this? Or is there any other
diver i can follow。Very sorry for that  I'm not familiar with memory management and the IOMMU。


>mask appropriately. None of those numbers is 64, so that's clearly 
>suspicious already. Plus it would seem the claim of the IOMMU being able 
>to address >4GB isn't strictly true for RK3288 (which does supposedly 
>support 8GB of RAM).

We can set DMA mask per device if we can find a right way to do it。

>
>Furthermore, the "display-subsystem" doesn't even exist - it does not 
>represent any actual DMA-capable hardware, so it should not have a DMA 
>mask, and it should not be used for DMA API operations. Buffers for the 
>VOP should be DMA-mapped for the VOP device itself. At the very least
>the rockchip_gem_alloc_dma() path is clearly broken otherwise (I guess 
>this patch possibly *would* make that brokenness apparent).
>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> Tested-by: Derek Foreman <derek.foreman@collabora.com>
>> ---
>> 
>> (no changes since v1)
>> 
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index 04ef7a2c3833..8bc2ff3b04bb 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -445,7 +445,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> -	return 0;
>> +	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>
>Finally as a general thing, please don't misuse 
>dma_coerce_mask_and_coherent() in platform drivers, just use normal 
>dma_set_mask_and_coherent(). The platform bus code has been initialising 
>the dev->dma_mask pointer for years now, drivers should not be messing 
>with it any more.

Got it ， thanks again。

>
>Thanks,
>Robin.
>
>> +
>> +	return ret;
>>   }
>>   
>>   static void rockchip_drm_platform_remove(struct platform_device *pdev)