| Message ID | 20220702230727.66704-1-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ingenic: Use the highest possible DMA burst size | expand |
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>
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>
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>
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)
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(-)