Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr

[Archit Taneja <archit@ti.com>]

On Tuesday 27 September 2011 12:24 PM, Hiremath, Vaibhav wrote:
>> -----Original Message-----
>> From: Valkeinen, Tomi
>> Sent: Tuesday, September 27, 2011 12:19 PM
>> To: Hiremath, Vaibhav
>> Cc: Semwal, Sumit; Taneja, Archit; linux-omap@vger.kernel.org; linux-
>> media@vger.kernel.org
>> Subject: RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in
>> omap_vout_isr
>>
>> On Tue, 2011-09-27 at 12:09 +0530, Hiremath, Vaibhav wrote:
>>> Please look at the patch carefully, it does exactly same thing. I
>>> understand the use-case what Archit explained in the last email but in
>>> this patch context, the use-case change anything here in this patch.
>>
>> With the current code, the ISR code will be ran for a panel connected to
>> LCD1 output when VSYNC for LCD2 happens.
>>
>> After Archit's patch, this no longer happens.
>>
>> I don't know what the ISR code does, so it may not cause any problems,
>> but it sure doesn't sound right running the code when a wrong interrupt
>> happens.
>>
>
> If you look at the patch, the patch barely checks for the condition and
> makes sure that the interrupt is either of VSYNC or VSYNC2, else return. Rest everything is same.

It doesn't only make sure that the interrupt it one of them, it uses it 
later too in the check:

if (!(irqstatus & irq))
	goto vout_isr_err;
...
...

>
> The right fix is in streamon api, where you mask the interrupt before
> registering it.

If this is the right fix, we should have a purely selective method of 
selecting the interrupts. Even for OMAP3, we register interrupts for LCD 
and TV, and then check the interrupt in the handler using panel type. 
Now, since have 2 different interrupts for the same panel type, we have 
to further distinguish using the manager id.

Archit

>
> Thanks,
> Vaibhav
>
>>   Tomi
>>
>
>