diff mbox series

[v2] drm: lcdif: Set and enable FIFO Panic threshold

Message ID 20221101152629.21768-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series [v2] drm: lcdif: Set and enable FIFO Panic threshold | expand

Commit Message

Marek Vasut Nov. 1, 2022, 3:26 p.m. UTC
In case the LCDIFv3 is used to drive a 4k panel via i.MX8MP HDMI bridge,
the LCDIFv3 becomes susceptible to FIFO underflows, these lead to nasty
flicker of the image on the panel, or image being shifted by half frame
horizontally every second frame. The flicker can be easily triggered by
running 3D application on top of weston compositor, like neverball or
chromium. Surprisingly glmark2-es2-wayland or glmark2-es2-drm does not
trigger this effect so easily.

Configure the FIFO Panic threshold register and enable the FIFO Panic
mode, which internally boosts the NoC interconnect priority for LCDIFv3
transactions in case of possible underflow. This mitigates the flicker
effect on 4k panels as well.

Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF variant")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Liu Ying <victor.liu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Marco Felsch <m.felsch@pengutronix.de>
Cc: Martyn Welch <martyn.welch@collabora.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
---
V2: - Include bitfield.h to always obtain FIELD_PREP definition
    - Rename PANIC0_THRES_RANGE to PANIC0_THRES_MAX and set to 511
    - Move threshold configuration and enable before FIFO and
      scanout enable and vice versa for disable
---
 drivers/gpu/drm/mxsfb/lcdif_kms.c  | 16 ++++++++++++++++
 drivers/gpu/drm/mxsfb/lcdif_regs.h |  1 +
 2 files changed, 17 insertions(+)

Comments

Liu Ying Nov. 3, 2022, 9:07 a.m. UTC | #1
On Tue, 2022-11-01 at 16:26 +0100, Marek Vasut wrote:
> In case the LCDIFv3 is used to drive a 4k panel via i.MX8MP HDMI
> bridge,
> the LCDIFv3 becomes susceptible to FIFO underflows, these lead to
> nasty
> flicker of the image on the panel, or image being shifted by half
> frame
> horizontally every second frame. The flicker can be easily triggered
> by
> running 3D application on top of weston compositor, like neverball or
> chromium. Surprisingly glmark2-es2-wayland or glmark2-es2-drm does
> not
> trigger this effect so easily.
> 
> Configure the FIFO Panic threshold register and enable the FIFO Panic
> mode, which internally boosts the NoC interconnect priority for
> LCDIFv3
> transactions in case of possible underflow. This mitigates the
> flicker
> effect on 4k panels as well.
> 
> Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF
> variant")
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Liu Ying <victor.liu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Marco Felsch <m.felsch@pengutronix.de>
> Cc: Martyn Welch <martyn.welch@collabora.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> ---
> V2: - Include bitfield.h to always obtain FIELD_PREP definition
>     - Rename PANIC0_THRES_RANGE to PANIC0_THRES_MAX and set to 511
>     - Move threshold configuration and enable before FIFO and
>       scanout enable and vice versa for disable
> ---
>  drivers/gpu/drm/mxsfb/lcdif_kms.c  | 16 ++++++++++++++++
>  drivers/gpu/drm/mxsfb/lcdif_regs.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> index a5302006c02cd..09280c6c3bbc1 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> @@ -5,6 +5,7 @@
>   * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> @@ -334,6 +335,18 @@ static void lcdif_enable_controller(struct
> lcdif_drm_private *lcdif)
>  {
>  	u32 reg;
>  
> +	/* Set FIFO Panic watermarks, low 1/3, high 2/3 . */
> +	writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_MAX /
> 3) |
> +	       FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 * PANIC0_THRES_MAX
> / 3),

Like I mentioned in v1 review cycle, I still think it's better to
define PANIC0_THRES_{LOW,HIGH}(n) macros in lcdif_regs.h instead of
using FIELD_PREP here directly.  Otherwise,

Tested-by: Liu Ying <victor.liu@nxp.com> # i.MX8mp EVK
"koe,tx26d202vm0bwa" LVDS panel fb console
Reviewed-by: Liu Ying <victor.liu@nxp.com>

[...]

Regards,
Liu Ying
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c
index a5302006c02cd..09280c6c3bbc1 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
@@ -5,6 +5,7 @@ 
  * This code is based on drivers/gpu/drm/mxsfb/mxsfb*
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -334,6 +335,18 @@  static void lcdif_enable_controller(struct lcdif_drm_private *lcdif)
 {
 	u32 reg;
 
+	/* Set FIFO Panic watermarks, low 1/3, high 2/3 . */
+	writel(FIELD_PREP(PANIC0_THRES_LOW_MASK, 1 * PANIC0_THRES_MAX / 3) |
+	       FIELD_PREP(PANIC0_THRES_HIGH_MASK, 2 * PANIC0_THRES_MAX / 3),
+	       lcdif->base + LCDC_V8_PANIC0_THRES);
+
+	/*
+	 * Enable FIFO Panic, this does not generate interrupt, but
+	 * boosts NoC priority based on FIFO Panic watermarks.
+	 */
+	writel(INT_ENABLE_D1_PLANE_PANIC_EN,
+	       lcdif->base + LCDC_V8_INT_ENABLE_D1);
+
 	reg = readl(lcdif->base + LCDC_V8_DISP_PARA);
 	reg |= DISP_PARA_DISP_ON;
 	writel(reg, lcdif->base + LCDC_V8_DISP_PARA);
@@ -361,6 +374,9 @@  static void lcdif_disable_controller(struct lcdif_drm_private *lcdif)
 	reg = readl(lcdif->base + LCDC_V8_DISP_PARA);
 	reg &= ~DISP_PARA_DISP_ON;
 	writel(reg, lcdif->base + LCDC_V8_DISP_PARA);
+
+	/* Disable FIFO Panic NoC priority booster. */
+	writel(0, lcdif->base + LCDC_V8_INT_ENABLE_D1);
 }
 
 static void lcdif_reset_block(struct lcdif_drm_private *lcdif)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h
index fb74eb5ccbf1d..c55dfb236c1d3 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_regs.h
+++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h
@@ -255,6 +255,7 @@ 
 
 #define PANIC0_THRES_LOW_MASK		GENMASK(24, 16)
 #define PANIC0_THRES_HIGH_MASK		GENMASK(8, 0)
+#define PANIC0_THRES_MAX		511
 
 #define LCDIF_MIN_XRES			120
 #define LCDIF_MIN_YRES			120