Re: [PATCH 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code from omap_vout_isr

2011-09-25 Thread Archit Taneja 

Hi,

On Wednesday 21 September 2011 04:15 PM, Taneja, Archit wrote:

Hi,

On Wednesday 21 September 2011 03:35 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 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code
from omap_vout_isr

Currently, there is a lot of redundant code is between DPI and VENC panels,
this
can be made common by moving out field/interlace specific code to a
separate
function called omapvid_handle_interlace_display(). There is no functional
change made.

Signed-off-by: Archit Tanejaarc...@ti.com
---
   drivers/media/video/omap/omap_vout.c |  172 -
-




[Hiremath, Vaibhav]
Have you tested TV out functionality?


I haven't checked it yet to be totally honest. Its hard to find a VENC
TV! I wanted to anyway get some kind of Ack from you before starting to
test this. Since you also feel that this clean up is needed, I'll start
testing this out :)


I tested the TV out functionality. It works fine. I have left the extra 
fid == 0 check so that the code is more clear. Will post out the new 
patch soon.


Archit
--
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 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code from omap_vout_isr

2011-09-21 Thread Hiremath, Vaibhav 

 -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 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code
 from omap_vout_isr
 
 Currently, there is a lot of redundant code is between DPI and VENC panels,
 this
 can be made common by moving out field/interlace specific code to a
 separate
 function called omapvid_handle_interlace_display(). There is no functional
 change made.
 
 Signed-off-by: Archit Taneja arc...@ti.com
 ---
  drivers/media/video/omap/omap_vout.c |  172 -
 -
  1 files changed, 82 insertions(+), 90 deletions(-)
 
 diff --git a/drivers/media/video/omap/omap_vout.c
 b/drivers/media/video/omap/omap_vout.c
 index e14c82b..c5f2ea0 100644
 --- a/drivers/media/video/omap/omap_vout.c
 +++ b/drivers/media/video/omap/omap_vout.c
 @@ -524,10 +524,50 @@ static int omapvid_apply_changes(struct
 omap_vout_device *vout)
   return 0;
  }
 
 +static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
 + unsigned int irqstatus, struct timeval timevalue)
 +{
 + u32 fid;
 +
 + if (vout-first_int) {
 + vout-first_int = 0;
 + goto err;
 + }
 +
 + if (irqstatus  DISPC_IRQ_EVSYNC_ODD)
 + fid = 1;
 + else if (irqstatus  DISPC_IRQ_EVSYNC_EVEN)
 + fid = 0;
 + else
 + goto err;
 +
 + vout-field_id ^= 1;
 + if (fid != vout-field_id) {
 + /* reset field ID */
 + vout-field_id = 0;
[Hiremath, Vaibhav] You should check whether fid == 0 before resetting it.

 + } else if (0 == fid) {
[Hiremath, Vaibhav] This is not matching with the original code, probably I 
have to be more careful here. I need to spend more time here...


 + if (vout-cur_frm == vout-next_frm)
 + goto err;
 +
 + vout-cur_frm-ts = timevalue;
 + vout-cur_frm-state = VIDEOBUF_DONE;
 + wake_up_interruptible(vout-cur_frm-done);
 + vout-cur_frm = vout-next_frm;
 + } else {
 + if (list_empty(vout-dma_queue) ||
 + (vout-cur_frm != vout-next_frm))
 + goto err;
 + }
 +
 + return vout-field_id;
 +err:
 + return 0;
 +}
 +
  static void omap_vout_isr(void *arg, unsigned int irqstatus)
  {
 - int ret;
 - u32 addr, fid;
 + int ret, fid;
 + u32 addr;
   struct omap_overlay *ovl;
   struct timeval timevalue;
   struct omapvideo_info *ovid;
 @@ -548,107 +588,59 @@ static void omap_vout_isr(void *arg, unsigned int
 irqstatus)
   spin_lock(vout-vbq_lock);
   do_gettimeofday(timevalue);
 
 - if (cur_display-type != OMAP_DISPLAY_TYPE_VENC) {
 - switch (cur_display-type) {
 - case OMAP_DISPLAY_TYPE_DPI:
 - if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
 - goto vout_isr_err;
 - break;
 - case OMAP_DISPLAY_TYPE_HDMI:
 - if (!(irqstatus  DISPC_IRQ_EVSYNC_EVEN))
 - goto vout_isr_err;
 - break;
 - default:
 + switch (cur_display-type) {
 + case OMAP_DISPLAY_TYPE_DPI:
 + if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
   goto vout_isr_err;
 - }
 - if (!vout-first_int  (vout-cur_frm != vout-next_frm)) {
 - vout-cur_frm-ts = timevalue;
 - vout-cur_frm-state = VIDEOBUF_DONE;
 - wake_up_interruptible(vout-cur_frm-done);
 - vout-cur_frm = vout-next_frm;
 - }
 - vout-first_int = 0;
 - if (list_empty(vout-dma_queue))
 + break;
 + case OMAP_DISPLAY_TYPE_VENC:
 + fid = omapvid_handle_interlace_display(vout, irqstatus,
 + timevalue);
 + if (!fid)
   goto vout_isr_err;
[Hiremath, Vaibhav] 
Have you tested TV out functionality? 

 + break;
 + case OMAP_DISPLAY_TYPE_HDMI:
 + if (!(irqstatus  DISPC_IRQ_EVSYNC_EVEN))
 + goto vout_isr_err;
 + break;
 + default:
 + goto vout_isr_err;
 + }
 
 - vout-next_frm = list_entry(vout-dma_queue.next,
 - struct videobuf_buffer, queue);
 - list_del(vout-next_frm-queue);
 -
 - vout-next_frm-state = VIDEOBUF_ACTIVE;
 -
 - addr = (unsigned long) vout-queued_buf_addr[vout-next_frm-
 i]
 - + vout-cropped_offset;
 + if (!vout-first_int  (vout-cur_frm != vout-next_frm)) {
 + vout-cur_frm-ts = timevalue;
 + vout-cur_frm-state = VIDEOBUF_DONE

Re: [PATCH 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code from omap_vout_isr

2011-09-21 Thread Archit Taneja 

Hi,

On Wednesday 21 September 2011 03:35 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 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code
from omap_vout_isr

Currently, there is a lot of redundant code is between DPI and VENC panels,
this
can be made common by moving out field/interlace specific code to a
separate
function called omapvid_handle_interlace_display(). There is no functional
change made.

Signed-off-by: Archit Tanejaarc...@ti.com
---
  drivers/media/video/omap/omap_vout.c |  172 -
-
  1 files changed, 82 insertions(+), 90 deletions(-)

diff --git a/drivers/media/video/omap/omap_vout.c
b/drivers/media/video/omap/omap_vout.c
index e14c82b..c5f2ea0 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -524,10 +524,50 @@ static int omapvid_apply_changes(struct
omap_vout_device *vout)
return 0;
  }

+static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
+   unsigned int irqstatus, struct timeval timevalue)
+{
+   u32 fid;
+
+   if (vout-first_int) {
+   vout-first_int = 0;
+   goto err;
+   }
+
+   if (irqstatus  DISPC_IRQ_EVSYNC_ODD)
+   fid = 1;
+   else if (irqstatus  DISPC_IRQ_EVSYNC_EVEN)
+   fid = 0;
+   else
+   goto err;
+
+   vout-field_id ^= 1;
+   if (fid != vout-field_id) {
+   /* reset field ID */
+   vout-field_id = 0;

[Hiremath, Vaibhav] You should check whether fid == 0 before resetting it.


+   } else if (0 == fid) {

[Hiremath, Vaibhav] This is not matching with the original code, probably I 
have to be more careful here. I need to spend more time here...


If you do a dry run of it you'll see that it does the same thing 
functionally. If fid was 1, vout-field_id would have been 0 anyway.
So the check for fid == 0 looked a bit redundant to me. However, if you 
think that doing this makes the code less clear, we can surely keep this 
check.






+   if (vout-cur_frm == vout-next_frm)
+   goto err;
+
+   vout-cur_frm-ts = timevalue;
+   vout-cur_frm-state = VIDEOBUF_DONE;
+   wake_up_interruptible(vout-cur_frm-done);
+   vout-cur_frm = vout-next_frm;
+   } else {
+   if (list_empty(vout-dma_queue) ||
+   (vout-cur_frm != vout-next_frm))
+   goto err;
+   }
+
+   return vout-field_id;
+err:
+   return 0;
+}
+
  static void omap_vout_isr(void *arg, unsigned int irqstatus)
  {
-   int ret;
-   u32 addr, fid;
+   int ret, fid;
+   u32 addr;
struct omap_overlay *ovl;
struct timeval timevalue;
struct omapvideo_info *ovid;
@@ -548,107 +588,59 @@ static void omap_vout_isr(void *arg, unsigned int
irqstatus)
spin_lock(vout-vbq_lock);
do_gettimeofday(timevalue);

-   if (cur_display-type != OMAP_DISPLAY_TYPE_VENC) {
-   switch (cur_display-type) {
-   case OMAP_DISPLAY_TYPE_DPI:
-   if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
-   goto vout_isr_err;
-   break;
-   case OMAP_DISPLAY_TYPE_HDMI:
-   if (!(irqstatus  DISPC_IRQ_EVSYNC_EVEN))
-   goto vout_isr_err;
-   break;
-   default:
+   switch (cur_display-type) {
+   case OMAP_DISPLAY_TYPE_DPI:
+   if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
goto vout_isr_err;
-   }
-   if (!vout-first_int  (vout-cur_frm != vout-next_frm)) {
-   vout-cur_frm-ts = timevalue;
-   vout-cur_frm-state = VIDEOBUF_DONE;
-   wake_up_interruptible(vout-cur_frm-done);
-   vout-cur_frm = vout-next_frm;
-   }
-   vout-first_int = 0;
-   if (list_empty(vout-dma_queue))
+   break;
+   case OMAP_DISPLAY_TYPE_VENC:
+   fid = omapvid_handle_interlace_display(vout, irqstatus,
+   timevalue);
+   if (!fid)
goto vout_isr_err;

[Hiremath, Vaibhav]
Have you tested TV out functionality?


I haven't checked it yet to be totally honest. Its hard to find a VENC 
TV! I wanted to anyway get some kind of Ack from you before starting to 
test this. Since you also feel that this clean up is needed, I'll start 
testing this out :)





+   break;
+   case OMAP_DISPLAY_TYPE_HDMI:
+   if (!(irqstatus

[PATCH 2/5] [media]: OMAP_VOUT: CLEANUP: Remove redundant code from omap_vout_isr

2011-09-16 Thread Archit Taneja 
Currently, there is a lot of redundant code is between DPI and VENC panels, this
can be made common by moving out field/interlace specific code to a separate
function called omapvid_handle_interlace_display(). There is no functional
change made.

Signed-off-by: Archit Taneja arc...@ti.com
---
 drivers/media/video/omap/omap_vout.c |  172 --
 1 files changed, 82 insertions(+), 90 deletions(-)

diff --git a/drivers/media/video/omap/omap_vout.c 
b/drivers/media/video/omap/omap_vout.c
index e14c82b..c5f2ea0 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -524,10 +524,50 @@ static int omapvid_apply_changes(struct omap_vout_device 
*vout)
return 0;
 }
 
+static int omapvid_handle_interlace_display(struct omap_vout_device *vout,
+   unsigned int irqstatus, struct timeval timevalue)
+{
+   u32 fid;
+
+   if (vout-first_int) {
+   vout-first_int = 0;
+   goto err;
+   }
+
+   if (irqstatus  DISPC_IRQ_EVSYNC_ODD)
+   fid = 1;
+   else if (irqstatus  DISPC_IRQ_EVSYNC_EVEN)
+   fid = 0;
+   else
+   goto err;
+
+   vout-field_id ^= 1;
+   if (fid != vout-field_id) {
+   /* reset field ID */
+   vout-field_id = 0;
+   } else if (0 == fid) {
+   if (vout-cur_frm == vout-next_frm)
+   goto err;
+
+   vout-cur_frm-ts = timevalue;
+   vout-cur_frm-state = VIDEOBUF_DONE;
+   wake_up_interruptible(vout-cur_frm-done);
+   vout-cur_frm = vout-next_frm;
+   } else {
+   if (list_empty(vout-dma_queue) ||
+   (vout-cur_frm != vout-next_frm))
+   goto err;
+   }
+
+   return vout-field_id;
+err:
+   return 0;
+}
+
 static void omap_vout_isr(void *arg, unsigned int irqstatus)
 {
-   int ret;
-   u32 addr, fid;
+   int ret, fid;
+   u32 addr;
struct omap_overlay *ovl;
struct timeval timevalue;
struct omapvideo_info *ovid;
@@ -548,107 +588,59 @@ static void omap_vout_isr(void *arg, unsigned int 
irqstatus)
spin_lock(vout-vbq_lock);
do_gettimeofday(timevalue);
 
-   if (cur_display-type != OMAP_DISPLAY_TYPE_VENC) {
-   switch (cur_display-type) {
-   case OMAP_DISPLAY_TYPE_DPI:
-   if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
-   goto vout_isr_err;
-   break;
-   case OMAP_DISPLAY_TYPE_HDMI:
-   if (!(irqstatus  DISPC_IRQ_EVSYNC_EVEN))
-   goto vout_isr_err;
-   break;
-   default:
+   switch (cur_display-type) {
+   case OMAP_DISPLAY_TYPE_DPI:
+   if (!(irqstatus  (DISPC_IRQ_VSYNC | DISPC_IRQ_VSYNC2)))
goto vout_isr_err;
-   }
-   if (!vout-first_int  (vout-cur_frm != vout-next_frm)) {
-   vout-cur_frm-ts = timevalue;
-   vout-cur_frm-state = VIDEOBUF_DONE;
-   wake_up_interruptible(vout-cur_frm-done);
-   vout-cur_frm = vout-next_frm;
-   }
-   vout-first_int = 0;
-   if (list_empty(vout-dma_queue))
+   break;
+   case OMAP_DISPLAY_TYPE_VENC:
+   fid = omapvid_handle_interlace_display(vout, irqstatus,
+   timevalue);
+   if (!fid)
goto vout_isr_err;
+   break;
+   case OMAP_DISPLAY_TYPE_HDMI:
+   if (!(irqstatus  DISPC_IRQ_EVSYNC_EVEN))
+   goto vout_isr_err;
+   break;
+   default:
+   goto vout_isr_err;
+   }
 
-   vout-next_frm = list_entry(vout-dma_queue.next,
-   struct videobuf_buffer, queue);
-   list_del(vout-next_frm-queue);
-
-   vout-next_frm-state = VIDEOBUF_ACTIVE;
-
-   addr = (unsigned long) vout-queued_buf_addr[vout-next_frm-i]
-   + vout-cropped_offset;
+   if (!vout-first_int  (vout-cur_frm != vout-next_frm)) {
+   vout-cur_frm-ts = timevalue;
+   vout-cur_frm-state = VIDEOBUF_DONE;
+   wake_up_interruptible(vout-cur_frm-done);
+   vout-cur_frm = vout-next_frm;
+   }
 
-   /* First save the configuration in ovelray structure */
-   ret = omapvid_init(vout, addr);
-   if (ret)
-   printk(KERN_ERR VOUT_NAME
-   failed to set overlay info\n);
-   /* Enable the pipeline and set the Go bit */
-   ret = omapvid_apply_changes(vout);
-   if (ret)
-   printk(KERN_ERR