[PATCH] drm: mxsfb: Enable recovery on underflow

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Mon, Jun 21, 2021 at 12:47:01AM +0200, Marek Vasut wrote:
> There is some sort of corner case behavior of the controller,
> which could rarely be triggered at least on i.MX6SX connected
> to 800x480 DPI panel and i.MX8MM connected to DPI->DSI->LVDS
> bridged 1920x1080 panel (and likely on other setups too), where
> the image on the panel shifts to the right and wraps around.
> This happens either when the controller is enabled on boot or
> even later during run time. The condition does not correct
> itself automatically, i.e. the display image remains shifted.
> 
> It seems this problem is known and is due to sporadic underflows
> of the LCDIF FIFO. While the LCDIF IP does have underflow/overflow
> IRQs, neither of the IRQs trigger and neither IRQ status bit is
> asserted when this condition occurs.
> 
> All known revisions of the LCDIF IP have CTRL1 RECOVER_ON_UNDERFLOW
> bit, which is described in the reference manual since i.MX23 as
> "
>   Set this bit to enable the LCDIF block to recover in the next
>   field/frame if there was an underflow in the current field/frame.
> "
> Enable this bit to mitigate the sporadic underflows.
> 
> Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Daniel Abrecht <public at danielabrecht.ch>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Lucas Stach <l.stach at pengutronix.de>
> Cc: Stefan Agner <stefan at agner.ch>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c  | 29 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index 300e7bab0f43..01e0f525360f 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -115,6 +115,35 @@ static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
>  	reg |= VDCTRL4_SYNC_SIGNALS_ON;
>  	writel(reg, mxsfb->base + LCDC_VDCTRL4);
>  
> +	/*
> +	 * Enable recovery on underflow.
> +	 *
> +	 * There is some sort of corner case behavior of the controller,
> +	 * which could rarely be triggered at least on i.MX6SX connected
> +	 * to 800x480 DPI panel and i.MX8MM connected to DPI->DSI->LVDS
> +	 * bridged 1920x1080 panel (and likely on other setups too), where
> +	 * the image on the panel shifts to the right and wraps around.
> +	 * This happens either when the controller is enabled on boot or
> +	 * even later during run time. The condition does not correct
> +	 * itself automatically, i.e. the display image remains shifted.
> +	 *
> +	 * It seems this problem is known and is due to sporadic underflows
> +	 * of the LCDIF FIFO. While the LCDIF IP does have underflow/overflow
> +	 * IRQs, neither of the IRQs trigger and neither IRQ status bit is
> +	 * asserted when this condition occurs.
> +	 *
> +	 * All known revisions of the LCDIF IP have CTRL1 RECOVER_ON_UNDERFLOW
> +	 * bit, which is described in the reference manual since i.MX23 as
> +	 * "
> +	 *   Set this bit to enable the LCDIF block to recover in the next
> +	 *   field/frame if there was an underflow in the current field/frame.
> +	 * "
> +	 * Enable this bit to mitigate the sporadic underflows.
> +	 */
> +	reg = readl(mxsfb->base + LCDC_CTRL1);
> +	reg |= CTRL1_RECOVER_ON_UNDERFLOW;
> +	writel(reg, mxsfb->base + LCDC_CTRL1);

Looks good to me. Thanks for the detailed explanation.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
>  	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
>  }
>  
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> index 55d28a27f912..df90e960f495 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> @@ -54,6 +54,7 @@
>  #define CTRL_DF24			BIT(1)
>  #define CTRL_RUN			BIT(0)
>  
> +#define CTRL1_RECOVER_ON_UNDERFLOW	BIT(24)
>  #define CTRL1_FIFO_CLEAR		BIT(21)
>  #define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
>  #define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)

-- 
Regards,

Laurent Pinchart