BUG: Atmel AT91 LCD Blanking hang (atmel_hlcdfb)

Commit Message

Sylvain Rochet Oct. 22, 2013, 2:44 p.m. UTC

Hi,

Kernel used: linux-3.10.y stable Git branch (currently 3.10.17)
  + linux-3.10-at91 Git branch

# echo "4" > /sys/class/graphics/fb0/blank 
never ends

atmel_lcdfb_core.c, atmel_lcdfb_blank() calls atmel_hlcdfb.c, 
atmel_hlcdfb_stop() which waits for ATMEL_LCDC_BASEISR.LCDC_BASEISR_DMA 
to be set if ATMEL_LCDC_STOP_NOWAIT flag is not set.

But it never happens, thus atmel_hlcdfb_stop() is going to wait 
indefinitely. It never happens because ATMEL_LCDC_BASEISR status 
register is cleared upon reading and interrupt routine 
atmel_hlcdfb_interrupt() is also reading the same register. Any 
triggered interrupt will clear this DMA bit while atmel_hlcdfb_stop() is 
currently msleep()ing.

The issue does not seem obvious to fix for me, since it requires some 
synching between the interrupt handler and the userland request to 
blank. I am not proud of, but I currently fixed the issue by asserting 
the ATMEL_LCDC_STOP_NOWAIT flag to all atmel_hlcdfb_stop() calls. I 
attached the patch for reference purposes, but it is broken since it 
also change the atmel_lcdfb.c behavior, hardware I don't have and which 
probably doesn't need to be fixed.

Anyway, I am not a design guru, but reading a register which clears 
itself at reading at two or more locations should probably be something 
to avoid.

Regards,
Sylvain

Patch

diff --git a/drivers/video/atmel_lcdfb_core.c b/drivers/video/atmel_lcdfb_core.c
index df7eb27..08669ce 100644
--- a/drivers/video/atmel_lcdfb_core.c
+++ b/drivers/video/atmel_lcdfb_core.c
@@ -304,7 +304,7 @@  static void atmel_lcdfb_reset(struct atmel_lcdfb_info *sinfo)
 	might_sleep();
 
 	if (sinfo->dev_data->stop)
-		sinfo->dev_data->stop(sinfo, 0);
+		sinfo->dev_data->stop(sinfo, ATMEL_LCDC_STOP_NOWAIT);
 	if (sinfo->dev_data->start)
 		sinfo->dev_data->start(sinfo);
 }
@@ -477,7 +477,7 @@  static int atmel_lcdfb_blank(int blank_mode, struct fb_info *info)
 		break;
 	case FB_BLANK_POWERDOWN:
 		if (sinfo->dev_data->stop)
-			sinfo->dev_data->stop(sinfo, 0);
+			sinfo->dev_data->stop(sinfo, ATMEL_LCDC_STOP_NOWAIT);
 		break;
 	default:
 		return -EINVAL;