atmel_lcdfb: FIFO underflow management
PATCH 1/1 atmel_lcdfb: FIFO underflow management
Manage atmel_lcdfb FIFO underflow
Resetting the LCD and DMA allows to fix screen shifting after a FIFO underflow. It follows reset sequence from errata "LCD Screen Shifting After a Reset".
---drivers/video/atmel_lcdfb.c | 50 +++++++++++++++++++++++++++++++++-include/video/atmel_lcdc.h | 1 2 files changed, 50 insertions(+), 1 deletion(-)
include/video/atmel_lcdc.h +0 -0
@@ -37,6 +37,7 @@ struct atmel_lcdfb_info {
struct fb_info *info; void __iomem *mmio; unsigned long irq_base;
+ struct work_struct task;
unsigned int guard_time; struct platform_device *pdev;
drivers/video/atmel_lcdfb.c +0 -0
@@ -356,6 +356,33 @@ static int atmel_lcdfb_check_var(struct
return 0; }
+/*
+ * LCD reset sequence
+ */
+static void atmel_lcdfb_reset(struct atmel_lcdfb_info *sinfo) +{
+ /* LCD power off */
+ lcdc_writel(sinfo, ATMEL_LCDC_PWRCON, sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET);
+
+ /* wait for the LCDC core to become idle */
+ while (lcdc_readl(sinfo, ATMEL_LCDC_PWRCON) & ATMEL_LCDC_BUSY)
+ msleep(10);
+
+ /* DMA disable */
+ lcdc_writel(sinfo, ATMEL_LCDC_DMACON, 0);
+
+ /* wait for DMA engine to become idle */
+ while (lcdc_readl(sinfo, ATMEL_LCDC_DMACON) & ATMEL_LCDC_DMABUSY)
+ msleep(10);
+
+ /* LCD power on */
+ lcdc_writel(sinfo, ATMEL_LCDC_PWRCON,
+ (sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET) | ATMEL_LCDC_PWR);
+
+ /* DMA enable */
+ lcdc_writel(sinfo, ATMEL_LCDC_DMACON, sinfo->default_dmacon);
+}
+
/**
* atmel_lcdfb_set_par - Alters the hardware state.
* @info: frame buffer structure that represents a single frame buffer
@@ -486,6 +513,8 @@ static int atmel_lcdfb_set_par(struct fb
/* Disable all interrupts */ lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
+ /* Enable FIFO & DMA errors */
+ lcdc_writel(sinfo, ATMEL_LCDC_IER, ATMEL_LCDC_UFLWI | ATMEL_LCDC_OWRI | ATMEL_LCDC_MERI);
/* ...wait for DMA engine to become idle... */ while (lcdc_readl(sinfo, ATMEL_LCDC_DMACON) & ATMEL_LCDC_DMABUSY) @@ -620,10 +649,26 @@ static irqreturn_t atmel_lcdfb_interrupt u32 status; status = lcdc_readl(sinfo, ATMEL_LCDC_ISR);
- lcdc_writel(sinfo, ATMEL_LCDC_IDR, status);
+ if (status & ATMEL_LCDC_UFLWI) {
+ dev_warn(info->device, "FIFO underflow %#x\n", status);
+ /* reset DMA and FIFO to avoid screen shifting */
+ schedule_work(&sinfo->task);
+ }
+ lcdc_writel(sinfo, ATMEL_LCDC_ICR, status); return IRQ_HANDLED; }
+/*
+ * LCD controller task (to reset the LCD)
+ */
+static void atmel_lcdfb_task(struct work_struct *work)
+{
+ struct atmel_lcdfb_info *sinfo =
+ container_of(work, struct atmel_lcdfb_info, task);
+
+ atmel_lcdfb_reset(sinfo);
+}
+
static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
{
struct fb_info *info = sinfo->info;
@@ -795,6 +840,9 @@ static int __init atmel_lcdfb_probe(stru
goto unmap_mmio;
}+ /* Initialize bottom half workqueue */
+ INIT_WORK(&sinfo->task, atmel_lcdfb_task);
+
ret = atmel_lcdfb_init_fbinfo(sinfo);
if (ret < 0) {
dev_err(dev, "init fbinfo failed: %d\n", ret);Quoted 6 lines · On Fri, 16 May 2008 14:34:12 +0200 Nicolas Ferre <[email protected]>…
Manage
atmel_lcdfbFIFO underflowResetting the LCD and DMA allows to fix screen shifting after a FIFO underflow. It follows reset sequence from errata "LCD Screen Shifting After a Reset".
This looks like 2.6.26-worthy bugfix to me? But as commonly happens, that isn't terribly clear.
Review
Quoted 8 lines
(quoted diff) +6
-0+ if (status & ATMEL_LCDC_UFLWI) { + dev_warn(info->device, "FIFO underflow %#x\n", status); + /* reset DMA and FIFO to avoid screen shifting */ + schedule_work(&sinfo->task); + } + lcdc_writel(sinfo, ATMEL_LCDC_ICR, status); return IRQ_HANDLED; }
Shouldn't there be a cancel_work_sync() in atmel_lcdfb_exit()?
Review
Quoted 1 line
(quoted diff) +1
-0+ /* Initialize bottom half workqueue */
Well. "bottom half" is an ancient term for softirqs, not for the new-fangled process-context callbacks.
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
PATCH 1/1 atmel_lcdfb: FIFO underflow management rework
Manage atmel_lcdfb FIFO underflow rework
Resetting the LCD and DMA allows to fix screen shifting after a FIFO underflow. It follows reset sequence from errata "LCD Screen Shifting After a Reset".
Reworked following Andrew Morton comments.
---
Andrew Morton :
Quoted 10 lines
Quoted 4 lines · On Fri, 16 May 2008 14:34:12 +0200 Nicolas Ferre <[email protected]>…
Manage
atmel_lcdfbFIFO underflowResetting the LCD and DMA allows to fix screen shifting after a FIFO underflow. It follows reset sequence from errata "LCD Screen Shifting After a Reset".
This looks like 2.6.26-worthy bugfix to me? But as commonly happens, that isn't terribly clear.
This error may append in latency sensitive cases : slow clock, matrix slot cycle bad configuration, too high pixel clock...
Quoted 10 lines
Review
Quoted 8 lines
(quoted diff) +6
-0+ if (status & ATMEL_LCDC_UFLWI) { + dev_warn(info->device, "FIFO underflow %#x\n", status); + /* reset DMA and FIFO to avoid screen shifting */ + schedule_work(&sinfo->task); + } + lcdc_writel(sinfo, ATMEL_LCDC_ICR, status); return IRQ_HANDLED; }Shouldn't there be a
cancel_work_sync()inatmel_lcdfb_exit()?
Ok incremental patch follows. Tell me if you prefer that I send a reworked one.
I also added might_sleep() annotation functions.
Quoted 4 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ /* Initialize bottom half workqueue */Well. "bottom half" is an ancient term for softirqs, not for the new-fangled process-context callbacks.
Ok, I try to adapt this comment in the reworked patch.
drivers/video/atmel_lcdfb.c +0 -0
@@ -361,6 +361,8 @@ static int atmel_lcdfb_check_var(struct*/
static void atmel_lcdfb_reset(struct atmel_lcdfb_info *sinfo) {
+ might_sleep();
+
/* LCD power off */ lcdc_writel(sinfo, ATMEL_LCDC_PWRCON, sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET);
@@ -405,6 +407,8 @@ static int atmel_lcdfb_set_par(struct fb
unsigned long clk_value_khz; unsigned long bits_per_line;
+ might_sleep();
+
dev_dbg(info->device, "%s:\n", __func__); dev_dbg(info->device, " * resolution: %ux%u (%ux%u virtual)\n", info->var.xres, info->var.yres, @@ -840,7 +844,8 @@ static int __init atmel_lcdfb_probe(stru goto unmap_mmio; }
- /* Initialize bottom half workqueue */
+ /* Some operations on the LCDC might sleep and + * require a preemptible task context */
INIT_WORK(&sinfo->task, atmel_lcdfb_task); ret = atmel_lcdfb_init_fbinfo(sinfo); @@ -885,6 +890,7 @@ static int __init atmel_lcdfb_probe(stru free_cmap: fb_dealloc_cmap(&info->cmap); unregister_irqs: + cancel_work_sync(&sinfo->task); free_irq(sinfo->irq_base, info); unmap_mmio: exit_backlight(sinfo); @@ -922,6 +928,7 @@ static int __exit atmel_lcdfb_remove(str if (!sinfo) return 0;
+ cancel_work_sync(&sinfo->task); exit_backlight(sinfo); if (sinfo->atmel_lcdfb_power_control) sinfo->atmel_lcdfb_power_control(0);