1. Patchew
  2. linux
  3. [PATCH v3 00/15] VOP Support for rk3576
  4. View patch

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

Andy Yan posted 15 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit
Posted by Andy Yan 2 months, 1 week ago 
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)

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));
+
+	return ret;
 }
 
 static void rockchip_drm_platform_remove(struct platform_device *pdev)
-- 
2.34.1
Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit
Posted by Robin Murphy 1 month, 1 week ago 
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 
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).

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.

Thanks,
Robin.

> +
> +	return ret;
>   }
>   
>   static void rockchip_drm_platform_remove(struct platform_device *pdev)
Re:Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit
Posted by Andy Yan 1 month, 1 week ago 

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)
Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit
Posted by Jonas Karlman 1 month, 1 week ago 
On 2024-10-17 09:06, Andy Yan wrote:
> 
> 
> 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。

Removing the use of custom rockchip_drm_gem and use the common gem dma
fops should also allow import of framebuffers in >4GB address.

I played around with that [1] last year but never took it further
because it broke multiple VOPs/IOMMUSs on e.g. rk3288. Only IOMMU dte
address handling fixes for >4GB support was sent and got merged.

When I tested [1] on an RK3568 back then it was possible to import video
framebuffers located in >4GB memory and display them on screen without a
spam of "swiotlb buffer is full" lines.

Maybe there is some part of the current custom rockchip_drm_gem code
that can be adjusted to work closer to the common gem dma fops?, or
maybe fully drop rockchip_drm_gem in favor of common gem dma fops could
be an alternative solution?

[1] https://github.com/Kwiboo/linux-rockchip/commit/70695c8f868adec630592fef536364e59793de81

Regards,
Jonas

> 
>>
>> 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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.