video: mxsfb: fix crash when unblanking the display

PATCH New Lothar Waßmann 4 participants 8 messages Nov 22, 2012 – Jan 4, 2013

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Framebuffer layer

lore

Share & tools

lore

Lothar Waßmann

author

2012-11-22 12:49 UTC #0 lore raw

PATCH 1/1 video: mxsfb: fix crash when unblanking the display

The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature. The write in mxsfb_disable_controller() sets the data_cnt for the LCD DMA to 0 which obviously means the max. count for the LCD DMA and leads to overwriting arbitrary memory when the display is unblanked.

Note: I already sent this patch in December 2011 but noone picked it up obviously.

Signed-off-by Lothar Wa?mann <[email protected]>

---
 drivers/video/mxsfb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/video/mxsfb.c +2 -1

index 49619b4..f2a49ef 100644
@@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 		loop--;
 	}
 
-	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+	reg = readl(host->base + LCDC_VDCTRL4);
+	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
 	clk_disable_unprepare(host->clk);

Signature · 1 line

1.7.2.5

Lothar Waßmann

author

2012-11-22 12:49 UTC #1 lore raw

PATCH 1/1 video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'

Signed-off-by Lothar Wa?mann <[email protected]>

---
 arch/arm/mach-mxs/mach-mxs.c |    6 +++---
 drivers/video/mxsfb.c        |    6 +++---
 include/linux/mxsfb.h        |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

arch/arm/mach-mxs/mach-mxs.c +3 -3

index d61b915..6c17cb7 100644
@@ -42,7 +42,7 @@ static struct fb_videomode mx23evk_video_modes[] = {
 		.hsync_len	= 1,
 		.vsync_len	= 1,
 		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

@@ -60,7 +60,7 @@ static struct fb_videomode mx28evk_video_modes[] = {
 		.hsync_len	= 10,
 		.vsync_len	= 10,
 		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

@@ -96,7 +96,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
 		.vsync_len	= 3,
 		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
 				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

drivers/video/mxsfb.c +3 -3

index f2a49ef..eb45d32 100644
@@ -107,7 +107,7 @@
 #define VDCTRL0_ENABLE_PRESENT		(1 << 28)
 #define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
 #define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
-#define VDCTRL0_DOTCLK_ACT_FAILING	(1 << 25)
+#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
 #define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
 #define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
 #define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)

@@ -458,8 +458,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
 	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
-		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
+	if (fb_info->var.sync & FB_SYNC_DOTCLK_FALLING_ACT)
+		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
 
 	writel(vdctrl0, host->base + LCDC_VDCTRL0);

include/linux/mxsfb.h +1 -1

index f14943d..a9eca68 100644
@@ -25,7 +25,7 @@
 #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
 
 #define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
+#define FB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* falling/negative edge sampling */
 
 struct mxsfb_platform_data {
 	struct fb_videomode *mode_list;

Signature · 1 line

1.7.2.5

Lothar Waßmann

author

2012-11-29 11:59 UTC #2 lore raw

PATCH 1/1 video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'

Hi,

adding Juergen Beisert as the driver's author to the recipient list.

Quoted 79 lines · Lothar Wa?mann

Signed-off-by Lothar Wa?mann <[email protected]>

---
 arch/arm/mach-mxs/mach-mxs.c |    6 +++---
 drivers/video/mxsfb.c        |    6 +++---
 include/linux/mxsfb.h        |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

arch/arm/mach-mxs/mach-mxs.c +3 -3

index d61b915..6c17cb7 100644
@@ -42,7 +42,7 @@ static struct fb_videomode mx23evk_video_modes[] = {
 		.hsync_len	= 1,
 		.vsync_len	= 1,
 		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

@@ -60,7 +60,7 @@ static struct fb_videomode mx28evk_video_modes[] = {
 		.hsync_len	= 10,
 		.vsync_len	= 10,
 		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

@@ -96,7 +96,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
 		.vsync_len	= 3,
 		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
 				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+				  FB_SYNC_DOTCLK_FALLING_ACT,
 	},
 };

drivers/video/mxsfb.c +3 -3

index f2a49ef..eb45d32 100644
@@ -107,7 +107,7 @@
 #define VDCTRL0_ENABLE_PRESENT		(1 << 28)
 #define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
 #define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
-#define VDCTRL0_DOTCLK_ACT_FAILING	(1 << 25)
+#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
 #define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
 #define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
 #define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)

@@ -458,8 +458,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
 	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
-		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
+	if (fb_info->var.sync & FB_SYNC_DOTCLK_FALLING_ACT)
+		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
 
 	writel(vdctrl0, host->base + LCDC_VDCTRL0);

include/linux/mxsfb.h +1 -1

index f14943d..a9eca68 100644
@@ -25,7 +25,7 @@
 #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
 
 #define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
+#define FB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* falling/negative edge sampling */
 
 struct mxsfb_platform_data {
 	struct fb_videomode *mode_list;

Signature · 1 line

1.7.2.5

ping?

Lothar Wa?mann

Signature · 9 lines

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

Juergen Beisert

2012-11-29 13:42 UTC #3 lore raw

PATCH 1/1 video: mxsfb: fix typo: 'FAILING' instead of 'FALLING'

Hi Lothar,

Quoted 2 lines · Lothar Wa?mann

[...]
ping?

What a nice typo :))

You can add my:

Acked-by Juergen Beisert <[email protected]>

Regards,
Juergen

Signature · 2 lines

Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

Lothar Waßmann

author

2012-11-29 11:55 UTC #4 lore raw

PATCH 1/1 video: mxsfb: fix crash when unblanking the display

In reply to Lothar Waßmann

Hi,

adding Juergen Beisert as the driver's author to the recipient list.

Quoted 29 lines · Lothar Wa?mann

The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature. The write in mxsfb_disable_controller() sets the data_cnt for the LCD DMA to 0 which obviously means the max. count for the LCD DMA and leads to overwriting arbitrary memory when the display is unblanked.
Note: I already sent this patch in December 2011 but noone picked it up obviously.
Signed-off-by Lothar Wa?mann <[email protected]>
---
 drivers/video/mxsfb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/video/mxsfb.c +2 -1
index 49619b4..f2a49ef 100644
@@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 		loop--;
 	}
 
-	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+	reg = readl(host->base + LCDC_VDCTRL4);
+	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
 	clk_disable_unprepare(host->clk);
 
Signature · 1 line
1.7.2.5

ping?

Lothar Wa?mann

Signature · 9 lines

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

Juergen Beisert

2012-11-29 13:50 UTC #5 lore raw

PATCH 1/1 video: mxsfb: fix crash when unblanking the display

Hi Lothar,

Quoted 4 lines · Lothar Wa?mann

Quoted 4 lines
The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature. The write in mxsfb_disable_controller() sets the data_cnt for the LCD DMA to 0 which obviously means the max. count for the LCD DMA and leads to overwriting arbitrary memory when the display is unblanked.

Overwriting? It should be a reading DMA unit, shouldn't it?

Quoted 20 lines

Note: I already sent this patch in December 2011 but noone picked it up obviously.

Signed-off-by Lothar Wa?mann <[email protected]>

---
 drivers/video/mxsfb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/video/mxsfb.c +2 -1

index 49619b4..f2a49ef 100644
@@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info
 *fb_info) loop--;
 	}
 
-	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+	reg = readl(host->base + LCDC_VDCTRL4);
+	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);

Quoted 6 lines

Quoted 4 lines
drivers/video/mxsfb.c +0 -1
 	clk_disable_unprepare(host->clk);
 
--
1.7.2.5
ping?

I'm not sure if the missing SET/CLR feature for this register is a typo in the i.MX23/i.MX28 data sheets or if it is intended. Lets assume its the truth what they have written:

Acked-by Juergen Beisert <[email protected]>

Regards,
Juergen

Signature · 2 lines

Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

Lauri Hintsala

2013-01-04 07:08 UTC #6 lore raw

PATCH 1/1 video: mxsfb: fix crash when unblanking the display

In reply to Lothar Waßmann

Quoted 13 lines · Lothar Wa?mann

Hi,
adding Juergen Beisert as the driver's author to the recipient list.
Quoted 8 lines · Lothar Wa?mann
The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature. The write in mxsfb_disable_controller() sets the data_cnt for the LCD DMA to 0 which obviously means the max. count for the LCD DMA and leads to overwriting arbitrary memory when the display is unblanked.
Note: I already sent this patch in December 2011 but noone picked it up obviously.
Signed-off-by Lothar Wa?mann <[email protected]>

Tested-by Lauri Hintsala <[email protected]>

This fixes the CPU freezing. I really recommend to apply this patch into mainline soon because unblanking freezes the whole system!!!

Default value of consoleblank parameter is 600 seconds and the whole system crashes every time when framebuffer console is activated after that timeout. I wonder why this issue is still open in mainline kernel.

Should this patch be added into stable versions too?

Best Regards,
Lauri Hintsala

Shawn Guo

2013-01-04 07:42 UTC #7 lore raw

PATCH 1/1 video: mxsfb: fix crash when unblanking the display

Quoted 17 lines · Lauri Hintsala

Quoted 13 lines · Lothar Wa?mann
Hi,
adding Juergen Beisert as the driver's author to the recipient list.
Quoted 8 lines · Lothar Wa?mann
The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature. The write in mxsfb_disable_controller() sets the data_cnt for the LCD DMA to 0 which obviously means the max. count for the LCD DMA and leads to overwriting arbitrary memory when the display is unblanked.
Note: I already sent this patch in December 2011 but noone picked it up obviously.
Signed-off-by Lothar Wa?mann <[email protected]>
Tested-by Lauri Hintsala <[email protected]>

Quoted 7 lines

This fixes the CPU freezing. I really recommend to apply this patch into mainline soon because unblanking freezes the whole system!!!
Default value of consoleblank parameter is 600 seconds and the whole system crashes every time when framebuffer console is activated after that timeout. I wonder why this issue is still open in mainline kernel.

Part of the reason is the FB maintainer's absence. Let me try to send it through arm-soc tree.

Quoted 3 lines

Should this patch be added into stable versions too?

Yes, will do.

Shawn