[PATCH] OMAP: DSS: dispc: enable/disable clocks in error handler

[PATCH] OMAP: DSS: dispc: enable/disable clocks in error handler

[Dima Zavin]

There's no guarantee that the error handler worker thread
will run while the dispc clocks are on. Explicitly enable/disable
them.

Signed-off-by: Dima Zavin <dima@android.com>
---
 drivers/video/omap2/dss/dispc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 62aa77c..2458248 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3292,6 +3292,8 @@ static void dispc_error_worker(struct work_struct *work)
 	dispc.error_irqs = 0;
 	spin_unlock_irqrestore(&dispc.irq_lock, flags);
 
+	dispc_runtime_get();
+
 	if (errors & DISPC_IRQ_GFX_FIFO_UNDERFLOW) {
 		DSSERR("GFX_FIFO_UNDERFLOW, disabling GFX\n");
 		for (i = 0; i < omap_dss_get_num_overlays(); ++i) {
@@ -3478,6 +3480,8 @@ static void dispc_error_worker(struct work_struct *work)
 	dispc.irq_error_mask |= errors;
 	_omap_dispc_set_irqs();
 	spin_unlock_irqrestore(&dispc.irq_lock, flags);
+
+	dispc_runtime_put();
 }
 
 int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
-- 
1.7.3.1

Re: [PATCH] OMAP: DSS: dispc: enable/disable clocks in error handler

[Archit Taneja]

Hi,

On Monday 27 June 2011 10:31 AM, Dima Zavin wrote:
> There's no guarantee that the error handler worker thread
> will run while the dispc clocks are on. Explicitly enable/disable
> them.

I agree with this.

Tomi,

We could get prevent scheduling of the error worker by registering 
omap_dispc_irq_handler() as an interrupt thread.

Archit

>
> Signed-off-by: Dima Zavin<dima@android.com>
> ---
>   drivers/video/omap2/dss/dispc.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 62aa77c..2458248 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -3292,6 +3292,8 @@ static void dispc_error_worker(struct work_struct *work)
>   	dispc.error_irqs = 0;
>   	spin_unlock_irqrestore(&dispc.irq_lock, flags);
>
> +	dispc_runtime_get();
> +
>   	if (errors&  DISPC_IRQ_GFX_FIFO_UNDERFLOW) {
>   		DSSERR("GFX_FIFO_UNDERFLOW, disabling GFX\n");
>   		for (i = 0; i<  omap_dss_get_num_overlays(); ++i) {
> @@ -3478,6 +3480,8 @@ static void dispc_error_worker(struct work_struct *work)
>   	dispc.irq_error_mask |= errors;
>   	_omap_dispc_set_irqs();
>   	spin_unlock_irqrestore(&dispc.irq_lock, flags);
> +
> +	dispc_runtime_put();
>   }
>
>   int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)

Re: [PATCH] OMAP: DSS: dispc: enable/disable clocks in error

[Tomi Valkeinen]

On Tue, 2011-06-28 at 09:19 -0700, Archit Taneja wrote:
> Hi,
> 
> On Monday 27 June 2011 10:31 AM, Dima Zavin wrote:
> > There's no guarantee that the error handler worker thread
> > will run while the dispc clocks are on. Explicitly enable/disable
> > them.
> 
> I agree with this.

Yes, I think this patch is fine. I'll apply it to DSS tree.

> Tomi,
> 
> We could get prevent scheduling of the error worker by registering 
> omap_dispc_irq_handler() as an interrupt thread.

But then we would get extra latency on the interrupt handlers. I wanted
to keep handling DSS interrupts in interrupt context, because some use
cases may require very fast reaction to an interrupt.

 Tomi

Re: [PATCH] OMAP: DSS: dispc: enable/disable clocks in error handler

[Archit Taneja]

Hi,

On Tuesday 28 June 2011 09:58 PM, Valkeinen, Tomi wrote:
> On Tue, 2011-06-28 at 09:19 -0700, Archit Taneja wrote:
>> Hi,
>>
>> On Monday 27 June 2011 10:31 AM, Dima Zavin wrote:
>>> There's no guarantee that the error handler worker thread
>>> will run while the dispc clocks are on. Explicitly enable/disable
>>> them.
>>
>> I agree with this.
>
> Yes, I think this patch is fine. I'll apply it to DSS tree.
>
>> Tomi,
>>
>> We could get prevent scheduling of the error worker by registering
>> omap_dispc_irq_handler() as an interrupt thread.
>
> But then we would get extra latency on the interrupt handlers. I wanted
> to keep handling DSS interrupts in interrupt context, because some use
> cases may require very fast reaction to an interrupt.

Okay..we don't need to handle the errors in interrupt context..the 
request_threaded_irq() takes 2 functions I think, we could have have 
omap_dispc_irq_handler() as the primary handler, and the error_worker as 
the thread.

I think the change I'm just suggesting just leads to some beautification 
in the end :), we would still need to ensure that clocks are enabled in 
the error_worker path.

Archit