RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
-Original Message- From: Semwal, Sumit Sent: Tuesday, September 27, 2011 11:12 AM To: Hiremath, Vaibhav Cc: Taneja, Archit; Valkeinen, Tomi; linux-omap@vger.kernel.org; linux- me...@vger.kernel.org Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr Hi Vaibhav, On Mon, Sep 26, 2011 at 3:49 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: Taneja, Archit Sent: Thursday, September 22, 2011 11:46 AM To: Hiremath, Vaibhav Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux- me...@vger.kernel.org Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr Hi, snip - if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2))) + if (mgr_id == OMAP_DSS_CHANNEL_LCD) + irq = DISPC_IRQ_VSYNC; + else if (mgr_id == OMAP_DSS_CHANNEL_LCD2) + irq = DISPC_IRQ_VSYNC2; + else + goto vout_isr_err; + + if (!(irqstatus irq)) goto vout_isr_err; break; I understand what this patch meant for, but I am more curious to know What is value addition of this patch? if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2))) Vs if (mgr_id == OMAP_DSS_CHANNEL_LCD) irq = DISPC_IRQ_VSYNC; else if (mgr_id == OMAP_DSS_CHANNEL_LCD2) irq = DISPC_IRQ_VSYNC2; Isn't this same, because we are not doing anything separate and special for VSYNC and VSYNC2? Consider a use case where the primary LCD panel(connected to LCD manager) is configured at a 60 Hz refresh rate, and the secondary panel(connected to LCD2) refreshes at 30 Hz. Let the overlay configuration be something like this: /dev/video1-overlay1-LCD manager-Primary panel(60 Hz) /dev/video2-overlay2-LCD2 manager-Secondary Panel(30 Hz) Now if we are doing streaming on both video1 and video2, since we deque on either VSYNC or VSYNC2, video2 buffers will deque faster than the panels refresh, which is incorrect. So for video2, we should only deque when we get a VSYNC2 interrupt. Thats why there is a need to correlate the interrupt and the overlay manager. Archit, I think I am still not understood or convinced with your description above, The code snippet which we are referring here, does check whether the interrupt is either VSYNC or VSYNC2, else fall back to vout_isr_err. I am not quite sure I understand what is the confusing part here - below is my understanding; please correct me if you think otherwise. As I understand, this patch creates a (missing) correlation between a manager and the corresponding ISR. The earlier code would accept a VSYNC2 for LCD1 manager, which is not the correct thing to do. That is why the check of 'if ((mgr==LCD) (IRQ==VSYNC))' kind of thing is needed; Which part of this do you think the above patch doesn't do? Or, do you think it is not needed / done correctly? Sumit, 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. Can you review it carefully again? Thanks, Vaibhav For me both are doing same thing, the original code is optimized way of implementation. Can you please review it again? Thanks, Vaibhav Thanks and best regards, ~Sumit. snip -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
Hi Vaibhav, On Tue, Sep 27, 2011 at 12:09 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: Semwal, Sumit Sent: Tuesday, September 27, 2011 11:12 AM To: Hiremath, Vaibhav Cc: Taneja, Archit; Valkeinen, Tomi; linux-omap@vger.kernel.org; linux- me...@vger.kernel.org Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr Hi Vaibhav, snip Archit, I think I am still not understood or convinced with your description above, The code snippet which we are referring here, does check whether the interrupt is either VSYNC or VSYNC2, else fall back to vout_isr_err. I am not quite sure I understand what is the confusing part here - below is my understanding; please correct me if you think otherwise. As I understand, this patch creates a (missing) correlation between a manager and the corresponding ISR. The earlier code would accept a VSYNC2 for LCD1 manager, which is not the correct thing to do. That is why the check of 'if ((mgr==LCD) (IRQ==VSYNC))' kind of thing is needed; Which part of this do you think the above patch doesn't do? Or, do you think it is not needed / done correctly? Sumit, 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. Can you review it carefully again? Thanks - I did review it carefully (the first time, and again), and maybe it is something that you're able to see which I can't. Could you please explain why you think (1) if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2))) goto vout_isr_err; is *exactly* the same as (2) if (!((mgr==LCD) (irqstatus DISPC_IRQ_VSYNC)) || (mgr==LCD2) (irqstatus DISPC_IRQ_VSYNC2)) ) goto vout_isr_err; [which I understand is what Archit's patch does] I am not able to see any correlation in (1) between mgr and irq, whereas it is quite clear in (2). Let me know if I missed something? Best regards, ~Sumit. Thanks, Vaibhav snip -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
-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- me...@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. The right fix is in streamon api, where you mask the interrupt before registering it. Thanks, Vaibhav Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
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- me...@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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
On Tue, 2011-09-27 at 12:24 +0530, Hiremath, Vaibhav wrote:
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.
This is what the current code does, in clearer form and slightly pseudo
code:
if (irq == VSYNC || irq == VSYNC2) {
do isr stuff
}
This is what it does after Archit's patch:
if ((lcd == LCD1 irq == VSYNC) || (lcd == LCD2 irq == VSYNC2)) {
do isr stuff;
}
I see a clear difference there. Or am I missing something?
The right fix is in streamon api, where you mask the interrupt before
registering it.
I'm not familiar with v4l so I don't know what that means, but yes, it
would be better if it's possible to only register for the needed
interrupts.
But the ISR code is still needed. If you are using both LCDs, you will
get both interrupts and you need to check if the interrupt is for the
right output.
Tomi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
-Original Message- From: Taneja, Archit Sent: Tuesday, September 27, 2011 12:32 PM To: Hiremath, Vaibhav Cc: Valkeinen, Tomi; Semwal, Sumit; linux-omap@vger.kernel.org; linux- me...@vger.kernel.org Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr 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- me...@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. I have to agree here with you that we do not have this available now. Also I had reviewed the patch again, and I think I now understand what usecase you are referring here. My bad, I concluded early on this Thanks, Vaibhav Archit Thanks, Vaibhav Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
-Original Message-
From: Taneja, Archit
Sent: Thursday, September 22, 2011 11:46 AM
To: Hiremath, Vaibhav
Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux-
me...@vger.kernel.org
Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in
omap_vout_isr
Hi,
On Wednesday 21 September 2011 07:04 PM, Hiremath, Vaibhav wrote:
-Original Message-
From: Taneja, Archit
Sent: Friday, September 16, 2011 3:31 PM
To: Hiremath, Vaibhav
Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux-
me...@vger.kernel.org; Taneja, Archit
Subject: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in
omap_vout_isr
Currently, in omap_vout_isr(), if the panel type is DPI, and if we
get either VSYNC or VSYNC2 interrupts, we proceed ahead to set the
current buffers state to VIDEOBUF_DONE and prepare to display the
next frame in the queue.
On OMAP4, because we have 2 LCD managers, the panel type itself is not
sufficient to tell if we have received the correct irq, i.e, we
shouldn't
proceed ahead if we get a VSYNC interrupt for LCD2 manager, or a VSYNC2
interrupt for LCD manager.
Fix this by correlating LCD manager to VSYNC interrupt and LCD2 manager
to VSYNC2 interrupt.
Signed-off-by: Archit Tanejaarc...@ti.com
---
drivers/media/video/omap/omap_vout.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/omap/omap_vout.c
b/drivers/media/video/omap/omap_vout.c
index c5f2ea0..20638c3 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -566,8 +566,8 @@ err:
static void omap_vout_isr(void *arg, unsigned int irqstatus)
{
- int ret, fid;
- u32 addr;
+ int ret, fid, mgr_id;
+ u32 addr, irq;
struct omap_overlay *ovl;
struct timeval timevalue;
struct omapvideo_info *ovid;
@@ -583,6 +583,7 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
if (!ovl-manager || !ovl-manager-device)
return;
+ mgr_id = ovl-manager-id;
cur_display = ovl-manager-device;
spin_lock(vout-vbq_lock);
@@ -590,7 +591,14 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
switch (cur_display-type) {
case OMAP_DISPLAY_TYPE_DPI:
- if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
+ if (mgr_id == OMAP_DSS_CHANNEL_LCD)
+ irq = DISPC_IRQ_VSYNC;
+ else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
+ irq = DISPC_IRQ_VSYNC2;
+ else
+ goto vout_isr_err;
+
+ if (!(irqstatus irq))
goto vout_isr_err;
break;
I understand what this patch meant for, but I am more curious to know
What is value addition of this patch?
if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
Vs
if (mgr_id == OMAP_DSS_CHANNEL_LCD)
irq = DISPC_IRQ_VSYNC;
else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
irq = DISPC_IRQ_VSYNC2;
Isn't this same, because we are not doing anything separate and special
for VSYNC and VSYNC2?
Consider a use case where the primary LCD panel(connected to LCD
manager) is configured at a 60 Hz refresh rate, and the secondary
panel(connected to LCD2) refreshes at 30 Hz. Let the overlay
configuration be something like this:
/dev/video1-overlay1-LCD manager-Primary panel(60 Hz)
/dev/video2-overlay2-LCD2 manager-Secondary Panel(30 Hz)
Now if we are doing streaming on both video1 and video2, since we deque
on either VSYNC or VSYNC2, video2 buffers will deque faster than the
panels refresh, which is incorrect. So for video2, we should only deque
when we get a VSYNC2 interrupt. Thats why there is a need to correlate
the interrupt and the overlay manager.
Archit,
I think I am still not understood or convinced with your description above,
The code snippet which we are referring here, does check whether the
interrupt is either VSYNC or VSYNC2, else fall back to vout_isr_err.
For me both are doing same thing, the original code is optimized way of
implementation. Can you please review it again?
Thanks,
Vaibhav
Regards,
Archit
Thanks,
Vaibhav
case OMAP_DISPLAY_TYPE_VENC:
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
Hi Vaibhav, On Mon, Sep 26, 2011 at 3:49 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: Taneja, Archit Sent: Thursday, September 22, 2011 11:46 AM To: Hiremath, Vaibhav Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux- me...@vger.kernel.org Subject: Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr Hi, snip - if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2))) + if (mgr_id == OMAP_DSS_CHANNEL_LCD) + irq = DISPC_IRQ_VSYNC; + else if (mgr_id == OMAP_DSS_CHANNEL_LCD2) + irq = DISPC_IRQ_VSYNC2; + else + goto vout_isr_err; + + if (!(irqstatus irq)) goto vout_isr_err; break; I understand what this patch meant for, but I am more curious to know What is value addition of this patch? if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2))) Vs if (mgr_id == OMAP_DSS_CHANNEL_LCD) irq = DISPC_IRQ_VSYNC; else if (mgr_id == OMAP_DSS_CHANNEL_LCD2) irq = DISPC_IRQ_VSYNC2; Isn't this same, because we are not doing anything separate and special for VSYNC and VSYNC2? Consider a use case where the primary LCD panel(connected to LCD manager) is configured at a 60 Hz refresh rate, and the secondary panel(connected to LCD2) refreshes at 30 Hz. Let the overlay configuration be something like this: /dev/video1-overlay1-LCD manager-Primary panel(60 Hz) /dev/video2-overlay2-LCD2 manager-Secondary Panel(30 Hz) Now if we are doing streaming on both video1 and video2, since we deque on either VSYNC or VSYNC2, video2 buffers will deque faster than the panels refresh, which is incorrect. So for video2, we should only deque when we get a VSYNC2 interrupt. Thats why there is a need to correlate the interrupt and the overlay manager. Archit, I think I am still not understood or convinced with your description above, The code snippet which we are referring here, does check whether the interrupt is either VSYNC or VSYNC2, else fall back to vout_isr_err. I am not quite sure I understand what is the confusing part here - below is my understanding; please correct me if you think otherwise. As I understand, this patch creates a (missing) correlation between a manager and the corresponding ISR. The earlier code would accept a VSYNC2 for LCD1 manager, which is not the correct thing to do. That is why the check of 'if ((mgr==LCD) (IRQ==VSYNC))' kind of thing is needed; Which part of this do you think the above patch doesn't do? Or, do you think it is not needed / done correctly? For me both are doing same thing, the original code is optimized way of implementation. Can you please review it again? Thanks, Vaibhav Thanks and best regards, ~Sumit. snip -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
Hi,
On Wednesday 21 September 2011 07:04 PM, Hiremath, Vaibhav wrote:
-Original Message-
From: Taneja, Archit
Sent: Friday, September 16, 2011 3:31 PM
To: Hiremath, Vaibhav
Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux-
me...@vger.kernel.org; Taneja, Archit
Subject: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in
omap_vout_isr
Currently, in omap_vout_isr(), if the panel type is DPI, and if we
get either VSYNC or VSYNC2 interrupts, we proceed ahead to set the
current buffers state to VIDEOBUF_DONE and prepare to display the
next frame in the queue.
On OMAP4, because we have 2 LCD managers, the panel type itself is not
sufficient to tell if we have received the correct irq, i.e, we shouldn't
proceed ahead if we get a VSYNC interrupt for LCD2 manager, or a VSYNC2
interrupt for LCD manager.
Fix this by correlating LCD manager to VSYNC interrupt and LCD2 manager
to VSYNC2 interrupt.
Signed-off-by: Archit Tanejaarc...@ti.com
---
drivers/media/video/omap/omap_vout.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/omap/omap_vout.c
b/drivers/media/video/omap/omap_vout.c
index c5f2ea0..20638c3 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -566,8 +566,8 @@ err:
static void omap_vout_isr(void *arg, unsigned int irqstatus)
{
- int ret, fid;
- u32 addr;
+ int ret, fid, mgr_id;
+ u32 addr, irq;
struct omap_overlay *ovl;
struct timeval timevalue;
struct omapvideo_info *ovid;
@@ -583,6 +583,7 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
if (!ovl-manager || !ovl-manager-device)
return;
+ mgr_id = ovl-manager-id;
cur_display = ovl-manager-device;
spin_lock(vout-vbq_lock);
@@ -590,7 +591,14 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
switch (cur_display-type) {
case OMAP_DISPLAY_TYPE_DPI:
- if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
+ if (mgr_id == OMAP_DSS_CHANNEL_LCD)
+ irq = DISPC_IRQ_VSYNC;
+ else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
+ irq = DISPC_IRQ_VSYNC2;
+ else
+ goto vout_isr_err;
+
+ if (!(irqstatus irq))
goto vout_isr_err;
break;
I understand what this patch meant for, but I am more curious to know
What is value addition of this patch?
if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
Vs
if (mgr_id == OMAP_DSS_CHANNEL_LCD)
irq = DISPC_IRQ_VSYNC;
else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
irq = DISPC_IRQ_VSYNC2;
Isn't this same, because we are not doing anything separate and special
for VSYNC and VSYNC2?
Consider a use case where the primary LCD panel(connected to LCD
manager) is configured at a 60 Hz refresh rate, and the secondary
panel(connected to LCD2) refreshes at 30 Hz. Let the overlay
configuration be something like this:
/dev/video1-overlay1-LCD manager-Primary panel(60 Hz)
/dev/video2-overlay2-LCD2 manager-Secondary Panel(30 Hz)
Now if we are doing streaming on both video1 and video2, since we deque
on either VSYNC or VSYNC2, video2 buffers will deque faster than the
panels refresh, which is incorrect. So for video2, we should only deque
when we get a VSYNC2 interrupt. Thats why there is a need to correlate
the interrupt and the overlay manager.
Regards,
Archit
Thanks,
Vaibhav
case OMAP_DISPLAY_TYPE_VENC:
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in omap_vout_isr
-Original Message-
From: Taneja, Archit
Sent: Friday, September 16, 2011 3:31 PM
To: Hiremath, Vaibhav
Cc: Valkeinen, Tomi; linux-omap@vger.kernel.org; Semwal, Sumit; linux-
me...@vger.kernel.org; Taneja, Archit
Subject: [PATCH 3/5] [media]: OMAP_VOUT: Fix VSYNC IRQ handling in
omap_vout_isr
Currently, in omap_vout_isr(), if the panel type is DPI, and if we
get either VSYNC or VSYNC2 interrupts, we proceed ahead to set the
current buffers state to VIDEOBUF_DONE and prepare to display the
next frame in the queue.
On OMAP4, because we have 2 LCD managers, the panel type itself is not
sufficient to tell if we have received the correct irq, i.e, we shouldn't
proceed ahead if we get a VSYNC interrupt for LCD2 manager, or a VSYNC2
interrupt for LCD manager.
Fix this by correlating LCD manager to VSYNC interrupt and LCD2 manager
to VSYNC2 interrupt.
Signed-off-by: Archit Taneja arc...@ti.com
---
drivers/media/video/omap/omap_vout.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/omap/omap_vout.c
b/drivers/media/video/omap/omap_vout.c
index c5f2ea0..20638c3 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -566,8 +566,8 @@ err:
static void omap_vout_isr(void *arg, unsigned int irqstatus)
{
- int ret, fid;
- u32 addr;
+ int ret, fid, mgr_id;
+ u32 addr, irq;
struct omap_overlay *ovl;
struct timeval timevalue;
struct omapvideo_info *ovid;
@@ -583,6 +583,7 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
if (!ovl-manager || !ovl-manager-device)
return;
+ mgr_id = ovl-manager-id;
cur_display = ovl-manager-device;
spin_lock(vout-vbq_lock);
@@ -590,7 +591,14 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
switch (cur_display-type) {
case OMAP_DISPLAY_TYPE_DPI:
- if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
+ if (mgr_id == OMAP_DSS_CHANNEL_LCD)
+ irq = DISPC_IRQ_VSYNC;
+ else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
+ irq = DISPC_IRQ_VSYNC2;
+ else
+ goto vout_isr_err;
+
+ if (!(irqstatus irq))
goto vout_isr_err;
break;
I understand what this patch meant for, but I am more curious to know
What is value addition of this patch?
if (!(irqstatus (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
Vs
if (mgr_id == OMAP_DSS_CHANNEL_LCD)
irq = DISPC_IRQ_VSYNC;
else if (mgr_id == OMAP_DSS_CHANNEL_LCD2)
irq = DISPC_IRQ_VSYNC2;
Isn't this same, because we are not doing anything separate and special
for VSYNC and VSYNC2?
Thanks,
Vaibhav
case OMAP_DISPLAY_TYPE_VENC:
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
