Re: [PATCH for-5.3] drm/omap: ensure we have a valid dma_mask

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On 09/08/2019 11:07, Christoph Hellwig wrote:
> On Fri, Aug 09, 2019 at 09:40:32AM +0300, Tomi Valkeinen wrote:
>> We do call dma_set_coherent_mask() in omapdrm's probe() (in omap_drv.c),
>> but apparently that's not enough anymore. Changing that call to
>> dma_coerce_mask_and_coherent() removes the WARN. I can create a patch for
>> that, or Christoph can respin this one.
> 
> Oh, yes - that actually is the right thing to do here.  If you already
> have it please just send it out.
> 
>>
>> I am not too familiar with the dma mask handling, so maybe someone can
>> educate:
>>
>> dma_coerce_mask_and_coherent() overwrites dev->dma_mask. Isn't that a bad
>> thing? What if the platform has set dev->dma_mask, and the driver
>> overwrites it with its value? Or who is supposed to set dev->dma_mask?
> 
> ->dma_mask is a complete mess.  It is a pointer when it really should
> just be a u64, and that means every driver layer has to allocate space
> for it.  We don't really do that for platform_devices, as that breaks
> horribly assumptions in the usb code.  That is why
> dma_coerce_mask_and_coherent exists as a nasty workaround that sets
> the dma_mask to the coherent_dma_mask for devices that don't have
> space for ->dma_mask allocated, which works as long as the device
> doesn't have differnet addressing requirements for both.
> 
> I'm actually working to fix that mess up at the moment, but it is going
> to take a few cycles until everything falls into place.

Alright, thanks for the clarification!

Here's my version.

>From c258309e36fc86076db155aead03a3900b96c3d4 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Fri, 9 Aug 2019 09:54:49 +0300
Subject: [PATCH] drm/omap: ensure we have a valid dma_mask

The omapdrm driver uses dma_set_coherent_mask(), but that's not enough
anymore when LPAE is enabled.

>From Christoph Hellwig <hch@lst.de>:

The traditional arm DMA code ignores, but the generic dma-direct/swiotlb
has stricter checks and thus fails mappings without a DMA mask.  As we
use swiotlb for arm with LPAE now, omapdrm needs to catch up and
actually set a DMA mask.

Change the dma_set_coherent_mask() call to
dma_coerce_mask_and_coherent() so that the dev->dma_mask is also set.

Fixes: ad3c7b18c5b3 ("arm: use swiotlb for bounce buffering on LPAE configs")
Reported-by: "H. Nikolaus Schaller" <hns@goldelico.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 288c59dae56a..1bad0a2cc5c6 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -669,7 +669,7 @@ static int pdev_probe(struct platform_device *pdev)
 	if (omapdss_is_initialized() == false)
 		return -EPROBE_DEFER;
 
-	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to set the DMA mask\n");
 		return ret;




-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki