[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4c2ebf98-7c0b-418a-a9de-ebe54acd21af@kwiboo.se>
Date: Thu, 17 Oct 2024 10:58:08 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Andy Yan <andyshrk@....com>, Robin Murphy <robin.murphy@....com>
Cc: heiko@...ech.de, hjc@...k-chips.com, krzk+dt@...nel.org, robh@...nel.org,
conor+dt@...nel.org, s.hauer@...gutronix.de, devicetree@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
derek.foreman@...labora.com, minhuadotchen@...il.com,
detlev.casanova@...labora.com, xxm@...k-chips.com,
Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit
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@....com> wrote:
>> On 2024-09-20 9:20 am, Andy Yan wrote:
>>> From: Andy Yan <andy.yan@...k-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@...k-chips.com>
>>> Tested-by: Derek Foreman <derek.foreman@...labora.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)
Powered by blists - more mailing lists