2022-06-16 21:46:03

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH] drm/sun4i: Add DMA mask and segment size

Kernel occasionally complains that there is mismatch in segment size
when trying to render HW decoded videos and rendering them directly with
sun4i DRM driver.

Fix that by setting DMA mask and segment size.

Signed-off-by: Jernej Skrabec <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_drv.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 275f7e4a03ae..83f4e87f77f6 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -7,6 +7,7 @@
*/

#include <linux/component.h>
+#include <linux/dma-mapping.h>
#include <linux/kfifo.h>
#include <linux/module.h>
#include <linux/of_graph.h>
@@ -367,6 +368,9 @@ static int sun4i_drv_probe(struct platform_device *pdev)

INIT_KFIFO(list.fifo);

+ dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
+
for (i = 0;; i++) {
struct device_node *pipeline = of_parse_phandle(np,
"allwinner,pipelines",
--
2.36.1


2022-06-17 03:19:22

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] drm/sun4i: Add DMA mask and segment size

Hi Jernej,

On 6/16/22 4:32 PM, Jernej Skrabec wrote:
> Kernel occasionally complains that there is mismatch in segment size
> when trying to render HW decoded videos and rendering them directly with
> sun4i DRM driver.
>
> Fix that by setting DMA mask and segment size.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun4i_drv.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 275f7e4a03ae..83f4e87f77f6 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/component.h>
> +#include <linux/dma-mapping.h>
> #include <linux/kfifo.h>
> #include <linux/module.h>
> #include <linux/of_graph.h>
> @@ -367,6 +368,9 @@ static int sun4i_drv_probe(struct platform_device *pdev)
>
> INIT_KFIFO(list.fifo);
>
> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));

Isn't this already the default, from of_dma_configure_id or setup_pdev_dma_masks?

> + dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));

This looks like a good change. In fact, I think we need a similar change in some
other drivers.

Regards,
Samuel

2022-06-17 05:10:45

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH] drm/sun4i: Add DMA mask and segment size

Dne petek, 17. junij 2022 ob 05:03:11 CEST je Samuel Holland napisal(a):
> Hi Jernej,
>
> On 6/16/22 4:32 PM, Jernej Skrabec wrote:
> > Kernel occasionally complains that there is mismatch in segment size
> > when trying to render HW decoded videos and rendering them directly with
> > sun4i DRM driver.
> >
> > Fix that by setting DMA mask and segment size.
> >
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> >
> > drivers/gpu/drm/sun4i/sun4i_drv.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > b/drivers/gpu/drm/sun4i/sun4i_drv.c index 275f7e4a03ae..83f4e87f77f6
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -7,6 +7,7 @@
> >
> > */
> >
> > #include <linux/component.h>
> >
> > +#include <linux/dma-mapping.h>
> >
> > #include <linux/kfifo.h>
> > #include <linux/module.h>
> > #include <linux/of_graph.h>
> >
> > @@ -367,6 +368,9 @@ static int sun4i_drv_probe(struct platform_device
> > *pdev)>
> > INIT_KFIFO(list.fifo);
> >
> > + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>
> Isn't this already the default, from of_dma_configure_id or
> setup_pdev_dma_masks?

Not sure, I need to check.

> > + dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>
> This looks like a good change. In fact, I think we need a similar change in
> some other drivers.

Should be DMA_BIT_MASK(25) as in your other patch?

Best regards,
Jernej

>
> Regards,
> Samuel




2022-06-17 05:16:31

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] drm/sun4i: Add DMA mask and segment size

On 6/16/22 11:13 PM, Jernej Škrabec wrote:
> Dne petek, 17. junij 2022 ob 05:03:11 CEST je Samuel Holland napisal(a):
>> Hi Jernej,
>>
>> On 6/16/22 4:32 PM, Jernej Skrabec wrote:
>>> Kernel occasionally complains that there is mismatch in segment size
>>> when trying to render HW decoded videos and rendering them directly with
>>> sun4i DRM driver.
>>>
>>> Fix that by setting DMA mask and segment size.
>>>
>>> Signed-off-by: Jernej Skrabec <[email protected]>
>>> ---
>>>
>>> drivers/gpu/drm/sun4i/sun4i_drv.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
>>> b/drivers/gpu/drm/sun4i/sun4i_drv.c index 275f7e4a03ae..83f4e87f77f6
>>> 100644
>>> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>>> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>>> @@ -7,6 +7,7 @@
>>>
>>> */
>>>
>>> #include <linux/component.h>
>>>
>>> +#include <linux/dma-mapping.h>
>>>
>>> #include <linux/kfifo.h>
>>> #include <linux/module.h>
>>> #include <linux/of_graph.h>
>>>
>>> @@ -367,6 +368,9 @@ static int sun4i_drv_probe(struct platform_device
>>> *pdev)>
>>> INIT_KFIFO(list.fifo);
>>>
>>> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>>
>> Isn't this already the default, from of_dma_configure_id or
>> setup_pdev_dma_masks?
>
> Not sure, I need to check.

Looking at the DE2 and DE3 spec to answer your question, I noticed that all of
the address fields have a corresponding 8-bit "high address" field, so from a
hardware perspective, the DMA mask should be 40 bits.

However, currently we do not use these fields, so we are limited to 32 bits. I
would suggest we comment that, so I am fine with setting the mask even if
redundant, as it provides a good place to add the comment.

>>> + dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>>
>> This looks like a good change. In fact, I think we need a similar change in
>> some other drivers.
>
> Should be DMA_BIT_MASK(25) as in your other patch?

No, that limit is specific to the other DMA engine. I do not see any limit for
the display engine. BSP uses sg_tables only for dma-buf, and then it puts the
entire framebuffer in one segment. So I think DMA_BIT_MASK(32) (the maximum
possible value) is correct.

Regards,
Samuel