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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx; Semwal, Sumit; linux-
media@xxxxxxxxxxxxxxx; 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<archit@xxxxxx>
---
  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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux