[PATCH] drm/ingenic: Use the highest possible DMA burst size

[PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Until now, when running at the maximum resolution of 1280x720 at 32bpp
on the JZ4770 SoC the output was garbled, the X/Y position of the
top-left corner of the framebuffer warping to a random position with
the whole image being offset accordingly, every time a new frame was
being submitted.

This problem can be eliminated by using a bigger burst size for the DMA.

Set in each soc_info structure the maximum burst size supported by the
corresponding SoC, and use it in the driver.

Set the new value using regmap_update_bits() instead of
regmap_set_bits(), since we do want to override the old value of the
burst size. (Note that regmap_set_bits() wasn't really valid before for
the same reason, but it never seemed to be a problem).

Cc: <stable@vger.kernel.org>
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 10 ++++++++--
 drivers/gpu/drm/ingenic/ingenic-drm.h     |  3 +++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 2c559885347a..8ad6080b32b2 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -70,6 +70,7 @@ struct jz_soc_info {
 	bool map_noncoherent;
 	bool use_extended_hwdesc;
 	bool plane_f0_not_working;
+	u32 max_burst;
 	unsigned int max_width, max_height;
 	const u32 *formats_f0, *formats_f1;
 	unsigned int num_formats_f0, num_formats_f1;
@@ -319,8 +320,9 @@ static void ingenic_drm_crtc_update_timings(struct ingenic_drm *priv,
 		regmap_write(priv->map, JZ_REG_LCD_REV, mode->htotal << 16);
 	}
 
-	regmap_set_bits(priv->map, JZ_REG_LCD_CTRL,
-			JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
+	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
+			   JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_MASK,
+			   JZ_LCD_CTRL_OFUP | priv->soc_info->max_burst);
 
 	/*
 	 * IPU restart - specify how much time the LCDC will wait before
@@ -1519,6 +1521,7 @@ static const struct jz_soc_info jz4740_soc_info = {
 	.map_noncoherent = false,
 	.max_width = 800,
 	.max_height = 600,
+	.max_burst = JZ_LCD_CTRL_BURST_16,
 	.formats_f1 = jz4740_formats,
 	.num_formats_f1 = ARRAY_SIZE(jz4740_formats),
 	/* JZ4740 has only one plane */
@@ -1530,6 +1533,7 @@ static const struct jz_soc_info jz4725b_soc_info = {
 	.map_noncoherent = false,
 	.max_width = 800,
 	.max_height = 600,
+	.max_burst = JZ_LCD_CTRL_BURST_16,
 	.formats_f1 = jz4725b_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4725b_formats_f1),
 	.formats_f0 = jz4725b_formats_f0,
@@ -1542,6 +1546,7 @@ static const struct jz_soc_info jz4770_soc_info = {
 	.map_noncoherent = true,
 	.max_width = 1280,
 	.max_height = 720,
+	.max_burst = JZ_LCD_CTRL_BURST_64,
 	.formats_f1 = jz4770_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
 	.formats_f0 = jz4770_formats_f0,
@@ -1556,6 +1561,7 @@ static const struct jz_soc_info jz4780_soc_info = {
 	.plane_f0_not_working = true,	/* REVISIT */
 	.max_width = 4096,
 	.max_height = 2048,
+	.max_burst = JZ_LCD_CTRL_BURST_64,
 	.formats_f1 = jz4770_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
 	.formats_f0 = jz4770_formats_f0,
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index cb1d09b62588..e5bd007ea93d 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -106,6 +106,9 @@
 #define JZ_LCD_CTRL_BURST_4			(0x0 << 28)
 #define JZ_LCD_CTRL_BURST_8			(0x1 << 28)
 #define JZ_LCD_CTRL_BURST_16			(0x2 << 28)
+#define JZ_LCD_CTRL_BURST_32			(0x3 << 28)
+#define JZ_LCD_CTRL_BURST_64			(0x4 << 28)
+#define JZ_LCD_CTRL_BURST_MASK			(0x7 << 28)
 #define JZ_LCD_CTRL_RGB555			BIT(27)
 #define JZ_LCD_CTRL_OFUP			BIT(26)
 #define JZ_LCD_CTRL_FRC_GRAYSCALE_16		(0x0 << 24)
-- 
2.35.1

[PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Until now, when running at the maximum resolution of 1280x720 at 32bpp
on the JZ4770 SoC the output was garbled, the X/Y position of the
top-left corner of the framebuffer warping to a random position with
the whole image being offset accordingly, every time a new frame was
being submitted.

This problem can be eliminated by using a bigger burst size for the DMA.

Set in each soc_info structure the maximum burst size supported by the
corresponding SoC, and use it in the driver.

Set the new value using regmap_update_bits() instead of
regmap_set_bits(), since we do want to override the old value of the
burst size. (Note that regmap_set_bits() wasn't really valid before for
the same reason, but it never seemed to be a problem).

Cc: <stable@vger.kernel.org>
Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 10 ++++++++--
 drivers/gpu/drm/ingenic/ingenic-drm.h     |  3 +++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 2c559885347a..8ad6080b32b2 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -70,6 +70,7 @@ struct jz_soc_info {
 	bool map_noncoherent;
 	bool use_extended_hwdesc;
 	bool plane_f0_not_working;
+	u32 max_burst;
 	unsigned int max_width, max_height;
 	const u32 *formats_f0, *formats_f1;
 	unsigned int num_formats_f0, num_formats_f1;
@@ -319,8 +320,9 @@ static void ingenic_drm_crtc_update_timings(struct ingenic_drm *priv,
 		regmap_write(priv->map, JZ_REG_LCD_REV, mode->htotal << 16);
 	}
 
-	regmap_set_bits(priv->map, JZ_REG_LCD_CTRL,
-			JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
+	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
+			   JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_MASK,
+			   JZ_LCD_CTRL_OFUP | priv->soc_info->max_burst);
 
 	/*
 	 * IPU restart - specify how much time the LCDC will wait before
@@ -1519,6 +1521,7 @@ static const struct jz_soc_info jz4740_soc_info = {
 	.map_noncoherent = false,
 	.max_width = 800,
 	.max_height = 600,
+	.max_burst = JZ_LCD_CTRL_BURST_16,
 	.formats_f1 = jz4740_formats,
 	.num_formats_f1 = ARRAY_SIZE(jz4740_formats),
 	/* JZ4740 has only one plane */
@@ -1530,6 +1533,7 @@ static const struct jz_soc_info jz4725b_soc_info = {
 	.map_noncoherent = false,
 	.max_width = 800,
 	.max_height = 600,
+	.max_burst = JZ_LCD_CTRL_BURST_16,
 	.formats_f1 = jz4725b_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4725b_formats_f1),
 	.formats_f0 = jz4725b_formats_f0,
@@ -1542,6 +1546,7 @@ static const struct jz_soc_info jz4770_soc_info = {
 	.map_noncoherent = true,
 	.max_width = 1280,
 	.max_height = 720,
+	.max_burst = JZ_LCD_CTRL_BURST_64,
 	.formats_f1 = jz4770_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
 	.formats_f0 = jz4770_formats_f0,
@@ -1556,6 +1561,7 @@ static const struct jz_soc_info jz4780_soc_info = {
 	.plane_f0_not_working = true,	/* REVISIT */
 	.max_width = 4096,
 	.max_height = 2048,
+	.max_burst = JZ_LCD_CTRL_BURST_64,
 	.formats_f1 = jz4770_formats_f1,
 	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
 	.formats_f0 = jz4770_formats_f0,
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index cb1d09b62588..e5bd007ea93d 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -106,6 +106,9 @@
 #define JZ_LCD_CTRL_BURST_4			(0x0 << 28)
 #define JZ_LCD_CTRL_BURST_8			(0x1 << 28)
 #define JZ_LCD_CTRL_BURST_16			(0x2 << 28)
+#define JZ_LCD_CTRL_BURST_32			(0x3 << 28)
+#define JZ_LCD_CTRL_BURST_64			(0x4 << 28)
+#define JZ_LCD_CTRL_BURST_MASK			(0x7 << 28)
 #define JZ_LCD_CTRL_RGB555			BIT(27)
 #define JZ_LCD_CTRL_OFUP			BIT(26)
 #define JZ_LCD_CTRL_FRC_GRAYSCALE_16		(0x0 << 24)
-- 
2.35.1

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Sam Ravnborg]

Hi Paul,

On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
> Until now, when running at the maximum resolution of 1280x720 at 32bpp
> on the JZ4770 SoC the output was garbled, the X/Y position of the
> top-left corner of the framebuffer warping to a random position with
> the whole image being offset accordingly, every time a new frame was
> being submitted.
> 
> This problem can be eliminated by using a bigger burst size for the DMA.

Are there any alignment constraints of the framebuffer that depends on
the burst size? I am hit by this with some atmel IP - which is why I
ask.

Patch looks good and is a-b.

> 
> Set in each soc_info structure the maximum burst size supported by the
> corresponding SoC, and use it in the driver.
> 
> Set the new value using regmap_update_bits() instead of
> regmap_set_bits(), since we do want to override the old value of the
> burst size. (Note that regmap_set_bits() wasn't really valid before for
> the same reason, but it never seemed to be a problem).
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Sam Ravnborg]

Hi Paul,

On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
> Until now, when running at the maximum resolution of 1280x720 at 32bpp
> on the JZ4770 SoC the output was garbled, the X/Y position of the
> top-left corner of the framebuffer warping to a random position with
> the whole image being offset accordingly, every time a new frame was
> being submitted.
> 
> This problem can be eliminated by using a bigger burst size for the DMA.

Are there any alignment constraints of the framebuffer that depends on
the burst size? I am hit by this with some atmel IP - which is why I
ask.

Patch looks good and is a-b.

> 
> Set in each soc_info structure the maximum burst size supported by the
> corresponding SoC, and use it in the driver.
> 
> Set the new value using regmap_update_bits() instead of
> regmap_set_bits(), since we do want to override the old value of the
> burst size. (Note that regmap_set_bits() wasn't really valid before for
> the same reason, but it never seemed to be a problem).
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Christophe Branchereau]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

Hello, fixes the hdmi glitches for me on jz4770.

Tested-by: Christophe Branchereau <cbranchereau@gmail.com>

On Sun, Jul 3, 2022 at 8:43 AM Sam Ravnborg <sam@ravnborg.org> wrote:

> Hi Paul,
>
> On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
> > Until now, when running at the maximum resolution of 1280x720 at 32bpp
> > on the JZ4770 SoC the output was garbled, the X/Y position of the
> > top-left corner of the framebuffer warping to a random position with
> > the whole image being offset accordingly, every time a new frame was
> > being submitted.
> >
> > This problem can be eliminated by using a bigger burst size for the DMA.
>
> Are there any alignment constraints of the framebuffer that depends on
> the burst size? I am hit by this with some atmel IP - which is why I
> ask.
>
> Patch looks good and is a-b.
>
> >
> > Set in each soc_info structure the maximum burst size supported by the
> > corresponding SoC, and use it in the driver.
> >
> > Set the new value using regmap_update_bits() instead of
> > regmap_set_bits(), since we do want to override the old value of the
> > burst size. (Note that regmap_set_bits() wasn't really valid before for
> > the same reason, but it never seemed to be a problem).
> >
> > Cc: <stable@vger.kernel.org>
> > Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx SoCs")
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>

[-- Attachment #2: Type: text/html, Size: 2177 bytes --]

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Hi Sam,

Le dim., juil. 3 2022 at 08:43:37 +0200, Sam Ravnborg 
<sam@ravnborg.org> a écrit :
> Hi Paul,
> 
> On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
>>  Until now, when running at the maximum resolution of 1280x720 at 
>> 32bpp
>>  on the JZ4770 SoC the output was garbled, the X/Y position of the
>>  top-left corner of the framebuffer warping to a random position with
>>  the whole image being offset accordingly, every time a new frame was
>>  being submitted.
>> 
>>  This problem can be eliminated by using a bigger burst size for the 
>> DMA.
> 
> Are there any alignment constraints of the framebuffer that depends on
> the burst size? I am hit by this with some atmel IP - which is why I
> ask.

I would think that the framebuffer needs to be aligned with the burst 
size, indeed. Here, our buffers are always page-aligned so that's not a 
problem.

> Patch looks good and is a-b.

Thanks!

Cheers,
-Paul

>> 
>>  Set in each soc_info structure the maximum burst size supported by 
>> the
>>  corresponding SoC, and use it in the driver.
>> 
>>  Set the new value using regmap_update_bits() instead of
>>  regmap_set_bits(), since we do want to override the old value of the
>>  burst size. (Note that regmap_set_bits() wasn't really valid before 
>> for
>>  the same reason, but it never seemed to be a problem).
>> 
>>  Cc: <stable@vger.kernel.org>
>>  Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx 
>> SoCs")
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Hi Sam,

Le dim., juil. 3 2022 at 08:43:37 +0200, Sam Ravnborg 
<sam@ravnborg.org> a écrit :
> Hi Paul,
> 
> On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
>>  Until now, when running at the maximum resolution of 1280x720 at 
>> 32bpp
>>  on the JZ4770 SoC the output was garbled, the X/Y position of the
>>  top-left corner of the framebuffer warping to a random position with
>>  the whole image being offset accordingly, every time a new frame was
>>  being submitted.
>> 
>>  This problem can be eliminated by using a bigger burst size for the 
>> DMA.
> 
> Are there any alignment constraints of the framebuffer that depends on
> the burst size? I am hit by this with some atmel IP - which is why I
> ask.

I would think that the framebuffer needs to be aligned with the burst 
size, indeed. Here, our buffers are always page-aligned so that's not a 
problem.

> Patch looks good and is a-b.

Thanks!

Cheers,
-Paul

>> 
>>  Set in each soc_info structure the maximum burst size supported by 
>> the
>>  corresponding SoC, and use it in the driver.
>> 
>>  Set the new value using regmap_update_bits() instead of
>>  regmap_set_bits(), since we do want to override the old value of the
>>  burst size. (Note that regmap_set_bits() wasn't really valid before 
>> for
>>  the same reason, but it never seemed to be a problem).
>> 
>>  Cc: <stable@vger.kernel.org>
>>  Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx 
>> SoCs")
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Hi Sam,

Le dim., juil. 3 2022 at 08:43:37 +0200, Sam Ravnborg 
<sam@ravnborg.org> a écrit :
> Hi Paul,
> 
> On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
>>  Until now, when running at the maximum resolution of 1280x720 at 
>> 32bpp
>>  on the JZ4770 SoC the output was garbled, the X/Y position of the
>>  top-left corner of the framebuffer warping to a random position with
>>  the whole image being offset accordingly, every time a new frame was
>>  being submitted.
>> 
>>  This problem can be eliminated by using a bigger burst size for the 
>> DMA.
> 
> Are there any alignment constraints of the framebuffer that depends on
> the burst size? I am hit by this with some atmel IP - which is why I
> ask.

I verified this, and everything behaves correctly when the source 
address is not aligned to the burst size. So I believe in our case the 
DMA is smart enough to auto-select the best burst size, up to the 
configured value.

Cheers,
-Paul

> 
> Patch looks good and is a-b.
> 
>> 
>>  Set in each soc_info structure the maximum burst size supported by 
>> the
>>  corresponding SoC, and use it in the driver.
>> 
>>  Set the new value using regmap_update_bits() instead of
>>  regmap_set_bits(), since we do want to override the old value of the
>>  burst size. (Note that regmap_set_bits() wasn't really valid before 
>> for
>>  the same reason, but it never seemed to be a problem).
>> 
>>  Cc: <stable@vger.kernel.org>
>>  Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx 
>> SoCs")
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH] drm/ingenic: Use the highest possible DMA burst size

[Paul Cercueil]

Hi Sam,

Le dim., juil. 3 2022 at 08:43:37 +0200, Sam Ravnborg 
<sam@ravnborg.org> a écrit :
> Hi Paul,
> 
> On Sun, Jul 03, 2022 at 12:07:27AM +0100, Paul Cercueil wrote:
>>  Until now, when running at the maximum resolution of 1280x720 at 
>> 32bpp
>>  on the JZ4770 SoC the output was garbled, the X/Y position of the
>>  top-left corner of the framebuffer warping to a random position with
>>  the whole image being offset accordingly, every time a new frame was
>>  being submitted.
>> 
>>  This problem can be eliminated by using a bigger burst size for the 
>> DMA.
> 
> Are there any alignment constraints of the framebuffer that depends on
> the burst size? I am hit by this with some atmel IP - which is why I
> ask.

I verified this, and everything behaves correctly when the source 
address is not aligned to the burst size. So I believe in our case the 
DMA is smart enough to auto-select the best burst size, up to the 
configured value.

Cheers,
-Paul

> 
> Patch looks good and is a-b.
> 
>> 
>>  Set in each soc_info structure the maximum burst size supported by 
>> the
>>  corresponding SoC, and use it in the driver.
>> 
>>  Set the new value using regmap_update_bits() instead of
>>  regmap_set_bits(), since we do want to override the old value of the
>>  burst size. (Note that regmap_set_bits() wasn't really valid before 
>> for
>>  the same reason, but it never seemed to be a problem).
>> 
>>  Cc: <stable@vger.kernel.org>
>>  Fixes: 90b86fcc47b4 ("DRM: Add KMS driver for the Ingenic JZ47xx 
>> SoCs")
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>