Skip to contentSkip to conversation

video: mxsfb: Support data-enable and pixclock polarity

PATCH New Michael Trimarchi 2 participants 3 messages Jun 20, 2018 – Jun 21, 2018

Subsystems (2)
Arm i.MX and Layerscape Framebuffer layer
lore
Share & tools
lore

Michael Trimarchi

author
#0 lore raw

PATCH 1/1 video: mxsfb: Support data-enable and pixclock polarity

Add extra feature to support data-enable and clock-polarity

Signed-off-by Michael Trimarchi <[email protected]>
---
 drivers/video/mxsfb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/video/mxsfb.c +18 -3
index 02fde05..4e3e3d7 100644
@@ -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,
 		&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;
+
+	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, &regs->hw_lcdif_vdctrl0);
+		mode->vsync_len | flags, &regs->hw_lcdif_vdctrl0);
 	writel(mode->upper_margin + mode->lower_margin +
 		mode->vsync_len + mode->yres,
 		&regs->hw_lcdif_vdctrl1);
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 video: mxsfb: Support data-enable and pixclock polarity

Hi Michael,

Quoted 12 lines · On Wed, 20 Jun 2018 22:55:55 +0200 Michael Trimarchi michael at…

Add extra feature to support data-enable and clock-polarity

Signed-off-by Michael Trimarchi <[email protected]>
---
 drivers/video/mxsfb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/video/mxsfb.c +0 -0
index 02fde05..4e3e3d7 100644

drivers/video/mxsfb.c line 24

Quoted 5 lines
drivers/video/mxsfb.c +2 -0
@@ -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.

Quoted 10 lines
drivers/video/mxsfb.c +1 -1
@@ -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 */

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 101

Quoted 6 lines
drivers/video/mxsfb.c +2 -1
@@ -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?

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 104

Quoted 2 lines
drivers/video/mxsfb.c +2 -0
+	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.

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 107

Quoted 3 lines
drivers/video/mxsfb.c +3 -0
+
+	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

author
#2 lore raw

PATCH 1/1 video: mxsfb: Support data-enable and pixclock polarity

Hi

Quoted 18 lines · Anatolij Gustschin

Hi Michael,

Quoted 13 lines · On Wed, 20 Jun 2018 22:55:55 +0200 Michael Trimarchi michael at…

Add extra feature to support data-enable and clock-polarity

Signed-off-by Michael Trimarchi <[email protected]>
---
 drivers/video/mxsfb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/video/mxsfb.c +0 -0
index 02fde05..4e3e3d7 100644
@@ -20,6 +20,9 @@
Quoted 8 lines

drivers/video/mxsfb.c line 24

Quoted 4 lines
drivers/video/mxsfb.c +2 -0
 #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

Quoted 20 lines
Quoted 11 lines
drivers/video/mxsfb.c +1 -1
@@ -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 */

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 101

Quoted 6 lines
drivers/video/mxsfb.c +2 -1
@@ -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?

Quoted 5 lines

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 104

Quoted 2 lines
drivers/video/mxsfb.c +2 -0
+     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

Quoted 10 lines

drivers/video/mxsfb.c static void mxs_lcd_init(GraphicDevice *panel, · line 107

Quoted 3 lines
drivers/video/mxsfb.c +3 -0
+
+     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

Quoted 3 lines

--
Anatolij

Signature · 4 lines
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |