2019-07-25 14:30:53

by Ezequiel Garcia

Subject: [PATCH v2 1/7] media: hantro: Set DMA max segment size

From: Francois Buergisser <[email protected]>

The Hantro codec is typically used in platforms with an IOMMU,
so we need to set a proper DMA segment size. Devices without an
IOMMU will still fallback to default 64KiB segments.

Cc: [email protected]
Fixes: 775fec69008d3 ("media: add Rockchip VPU JPEG encoder driver")
Signed-off-by: Francois Buergisser <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
drivers/staging/media/hantro/hantro_drv.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index b71a06e9159e..4eae1dbb1ac8 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -731,6 +731,7 @@ static int hantro_probe(struct platform_device *pdev)
dev_err(vpu->dev, "Could not set DMA coherent mask.\n");
return ret;
}
+ vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

for (i = 0; i < vpu->variant->num_irqs; i++) {
const char *irq_name = vpu->variant->irqs[i].name;
--
2.22.0

2019-07-25 18:04:12

by Philipp Zabel

Subject: Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size

On Thu, 2019-07-25 at 11:17 -0300, Ezequiel Garcia wrote:
> From: Francois Buergisser <[email protected]>
>
> The Hantro codec is typically used in platforms with an IOMMU,
> so we need to set a proper DMA segment size.

... to make sure the DMA-mapping subsystem produces contiguous mappings?

> Devices without an
> IOMMU will still fallback to default 64KiB segments.

I don't understand this comment. The default max_seg_size may be 64 KiB,
but if we are always setting it to DMA_BUT_MASK(32), there is no falling
back.

> Cc: [email protected]
> Fixes: 775fec69008d3 ("media: add Rockchip VPU JPEG encoder driver")
> Signed-off-by: Francois Buergisser <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> drivers/staging/media/hantro/hantro_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index b71a06e9159e..4eae1dbb1ac8 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -731,6 +731,7 @@ static int hantro_probe(struct platform_device *pdev)
> dev_err(vpu->dev, "Could not set DMA coherent mask.\n");
> return ret;
> }
> + vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

This should be complemented by a call to
vb2_dma_contig_clear_max_seg_size() in _remove,
to avoid leaking dev->dma_parms.

>
> for (i = 0; i < vpu->variant->num_irqs; i++) {
> const char *irq_name = vpu->variant->irqs[i].name;

regards
Philipp

2019-07-25 18:05:52

by Tomasz Figa

Subject: Re: [PATCH v2 1/7] media: hantro: Set DMA max segment size

On Fri, Jul 26, 2019 at 12:36 AM Philipp Zabel <[email protected]> wrote:
>
> On Thu, 2019-07-25 at 11:17 -0300, Ezequiel Garcia wrote:
> > From: Francois Buergisser <[email protected]>
> >
> > The Hantro codec is typically used in platforms with an IOMMU,
> > so we need to set a proper DMA segment size.
>
> ... to make sure the DMA-mapping subsystem produces contiguous mappings?
>
> > Devices without an
> > IOMMU will still fallback to default 64KiB segments.
>
> I don't understand this comment. The default max_seg_size may be 64 KiB,
> but if we are always setting it to DMA_BUT_MASK(32), there is no falling
> back.
>

DMA mask and segment size are two completely orthogonal parameters.
Please check https://elixir.bootlin.com/linux/v5.3-rc1/source/drivers/iommu/dma-iommu.c#L740
for an example of how the latter is used.

Best regards,
Tomasz