[U-Boot] [PATCH] video: mxsfb: Support data-enable and pixclock polarity

[Anatolij Gustschin]

Hi Michael,

On Wed, 20 Jun 2018 22:55:55 +0200
Michael Trimarchi michael at amarulasolutions.com wrote:

> Add extra feature to support data-enable and clock-polarity
> 
> Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
> ---
>  drivers/video/mxsfb.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 02fde05..4e3e3d7 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -20,6 +20,9 @@
>  
>  #define	PS2KHZ(ps)	(1000000000UL / (ps))
>  
> +#define FB_SYNC_DATA_ENABLE_HIGH_ACT	0x80000000
> +#define FB_SYNC_DOTCLK_FALLING_ACT	0x40000000

shouldn't these be defined in drivers/video/videomodes.h? I assume
that some external code will set these flags in the mode struct, so
these must be known (and included) elsewhere.

> @@ -50,7 +53,7 @@ static void mxs_lcd_init(GraphicDevice *panel,
>  			struct ctfb_res_modes *mode, int bpp)
>  {
>  	struct mxs_lcdif_regs *regs = (struct mxs_lcdif_regs *)MXS_LCDIF_BASE;
> -	uint32_t word_len = 0, bus_width = 0;
> +	uint32_t word_len = 0, bus_width = 0, flags = 0;
>  	uint8_t valid_data = 0;
>  
>  	/* Kick in the LCDIF clock */
> @@ -94,10 +97,22 @@ static void mxs_lcd_init(GraphicDevice *panel,
>  	writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | mode->xres,
>  		&regs->hw_lcdif_transfer_count);
>  
> -	writel(LCDIF_VDCTRL0_ENABLE_PRESENT | LCDIF_VDCTRL0_ENABLE_POL |
> +	if (mode->sync & FB_SYNC_HOR_HIGH_ACT)
> +		flags |= LCDIF_VDCTRL0_VSYNC_POL;

you check for HSYNC high flag, but set VSYNC bit?

> +	if (mode->sync & FB_SYNC_VERT_HIGH_ACT)
> +		flags |= LCDIF_VDCTRL0_HSYNC_POL;

you check for VSYNC high flag, but set HSYNC bit. Please fix.

> +
> +	if (mode->sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
> +		flags |= LCDIF_VDCTRL0_ENABLE_POL;

previously data enable polarity bit was always set in the ctrl register,
now it will be only set if requested by new flag in mode sync. This
is going to break current users, I'm afraid. Who is going to request
this for all driver users? Will it be supplied via environment mode
variable "video=ctfb..." ? This should be compatible with existing
video mode environments then (which we cannot easily update everywhere).

--
Anatolij