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

Add extra feature to support data-enable and clock-polarity
Signed-off-by: Michael Trimarchi michael@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 + static GraphicDevice panel; struct mxs_dma_desc desc;
@@ -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, ®s->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; + + if (mode->sync & FB_SYNC_VERT_HIGH_ACT) + flags |= LCDIF_VDCTRL0_HSYNC_POL; + + if (mode->sync & FB_SYNC_DATA_ENABLE_HIGH_ACT) + flags |= LCDIF_VDCTRL0_ENABLE_POL; + + if (mode->sync & FB_SYNC_DOTCLK_FALLING_ACT) + flags |= LCDIF_VDCTRL0_DOTCLK_POL; + + writel(LCDIF_VDCTRL0_ENABLE_PRESENT | LCDIF_VDCTRL0_VSYNC_PERIOD_UNIT | LCDIF_VDCTRL0_VSYNC_PULSE_WIDTH_UNIT | - mode->vsync_len, ®s->hw_lcdif_vdctrl0); + mode->vsync_len | flags, ®s->hw_lcdif_vdctrl0); writel(mode->upper_margin + mode->lower_margin + mode->vsync_len + mode->yres, ®s->hw_lcdif_vdctrl1);

Hi Michael,
On Wed, 20 Jun 2018 22:55:55 +0200 Michael Trimarchi michael@amarulasolutions.com wrote:
Add extra feature to support data-enable and clock-polarity
Signed-off-by: Michael Trimarchi michael@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, ®s->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

Hi
On Thu, Jun 21, 2018 at 9:17 AM, Anatolij Gustschin agust@denx.de wrote:
Hi Michael,
On Wed, 20 Jun 2018 22:55:55 +0200 Michael Trimarchi michael@amarulasolutions.com wrote:
Add extra feature to support data-enable and clock-polarity
Signed-off-by: Michael Trimarchi michael@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.
No, those are what we call host->flags and they can not land there. Those mapping is already used for those special host implementation and we can not diverge from linux. I found two different way to work with panel, video and display
@@ -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, ®s->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.
Yes right
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).
I have notice this one, but the previsius implementation was wrong. I'm planning to move this one in DM_VIDEO anyway later
Michael
-- Anatolij
participants (3)
-
Anatolij Gustschin
-
Michael Nazzareno Trimarchi
-
Michael Trimarchi