Re: [PATCH v5 04/49] media: davinci: vpif_display: release buffers in case start_streaming() call back fails

2014-05-23 Thread Hans Verkuil
On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 this patch adds support to release the buffer by calling
 vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED
 if start_streaming() call back fails.
 
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/media/platform/davinci/vpif_display.c |   42 
 +++--
  1 file changed, 26 insertions(+), 16 deletions(-)
 
 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index 8bb9f02..1a17a45 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -196,26 +196,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   struct channel_obj *ch = vb2_get_drv_priv(vq);
   struct common_obj *common = ch-common[VPIF_VIDEO_INDEX];
   struct vpif_params *vpif = ch-vpifparams;
 - unsigned long addr = 0;
 - unsigned long flags;
 + struct vpif_disp_buffer *buf, *tmp;
 + unsigned long addr, flags;
   int ret;
  
   spin_lock_irqsave(common-irqlock, flags);
  
 - /* Get the next frame from the buffer queue */
 - common-next_frm = common-cur_frm =
 - list_entry(common-dma_queue.next,
 -struct vpif_disp_buffer, list);
 -
 - list_del(common-cur_frm-list);
 - spin_unlock_irqrestore(common-irqlock, flags);
 - /* Mark state of the current frame to active */
 - common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;
 -
   /* Initialize field_id and started member */
   ch-field_id = 0;
   common-started = 1;
 - addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
 +
   /* Calculate the offset for Y and C data  in the buffer */
   vpif_calculate_offsets(ch);
  
 @@ -225,7 +215,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   || (!ch-vpifparams.std_info.frm_fmt
(common-fmt.fmt.pix.field == V4L2_FIELD_NONE))) {
   vpif_err(conflict in field format and std format\n);
 - return -EINVAL;
 + ret = -EINVAL;
 + goto err;
   }
  
   /* clock settings */
 @@ -234,17 +225,28 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   ycmux_mode, ch-vpifparams.std_info.hd_sd);
   if (ret  0) {
   vpif_err(can't set clock\n);
 - return ret;
 + goto err;
   }
   }
  
   /* set the parameters and addresses */
   ret = vpif_set_video_params(vpif, ch-channel_id + 2);
   if (ret  0)
 - return ret;
 + goto err;
  
   common-started = ret;
   vpif_config_addr(ch, ret);
 + /* Get the next frame from the buffer queue */
 + common-next_frm = common-cur_frm =
 + list_entry(common-dma_queue.next,
 +struct vpif_disp_buffer, list);
 +
 + list_del(common-cur_frm-list);
 + spin_unlock_irqrestore(common-irqlock, flags);
 + /* Mark state of the current frame to active */
 + common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;

There is no need to set this, all buffers queued to the driver are always in 
state
ACTIVE. The vb2 core sets that for you. In general drivers never need to change 
the
state manually.

It happens twice in this driver and in both cases the assignment can be removed.

Regards,

Hans

 +
 + addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
   common-set_addr((addr + common-ytop_off),
   (addr + common-ybtm_off),
   (addr + common-ctop_off),
 @@ -271,6 +273,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   }
  
   return 0;
 +
 +err:
 + list_for_each_entry_safe(buf, tmp, common-dma_queue, list) {
 + list_del(buf-list);
 + vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED);
 + }
 +
 + return ret;
  }
  
  /* abort streaming and wait for last buffer */
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 04/49] media: davinci: vpif_display: release buffers in case start_streaming() call back fails

2014-05-23 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Fri, May 23, 2014 at 2:05 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 this patch adds support to release the buffer by calling
 vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED
 if start_streaming() call back fails.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/media/platform/davinci/vpif_display.c |   42 
 +++--
  1 file changed, 26 insertions(+), 16 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index 8bb9f02..1a17a45 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -196,26 +196,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   struct channel_obj *ch = vb2_get_drv_priv(vq);
   struct common_obj *common = ch-common[VPIF_VIDEO_INDEX];
   struct vpif_params *vpif = ch-vpifparams;
 - unsigned long addr = 0;
 - unsigned long flags;
 + struct vpif_disp_buffer *buf, *tmp;
 + unsigned long addr, flags;
   int ret;

   spin_lock_irqsave(common-irqlock, flags);

 - /* Get the next frame from the buffer queue */
 - common-next_frm = common-cur_frm =
 - list_entry(common-dma_queue.next,
 -struct vpif_disp_buffer, list);
 -
 - list_del(common-cur_frm-list);
 - spin_unlock_irqrestore(common-irqlock, flags);
 - /* Mark state of the current frame to active */
 - common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;
 -
   /* Initialize field_id and started member */
   ch-field_id = 0;
   common-started = 1;
 - addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
 +
   /* Calculate the offset for Y and C data  in the buffer */
   vpif_calculate_offsets(ch);

 @@ -225,7 +215,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   || (!ch-vpifparams.std_info.frm_fmt
(common-fmt.fmt.pix.field == V4L2_FIELD_NONE))) {
   vpif_err(conflict in field format and std format\n);
 - return -EINVAL;
 + ret = -EINVAL;
 + goto err;
   }

   /* clock settings */
 @@ -234,17 +225,28 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   ycmux_mode, ch-vpifparams.std_info.hd_sd);
   if (ret  0) {
   vpif_err(can't set clock\n);
 - return ret;
 + goto err;
   }
   }

   /* set the parameters and addresses */
   ret = vpif_set_video_params(vpif, ch-channel_id + 2);
   if (ret  0)
 - return ret;
 + goto err;

   common-started = ret;
   vpif_config_addr(ch, ret);
 + /* Get the next frame from the buffer queue */
 + common-next_frm = common-cur_frm =
 + list_entry(common-dma_queue.next,
 +struct vpif_disp_buffer, list);
 +
 + list_del(common-cur_frm-list);
 + spin_unlock_irqrestore(common-irqlock, flags);
 + /* Mark state of the current frame to active */
 + common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;

 There is no need to set this, all buffers queued to the driver are always in 
 state
 ACTIVE. The vb2 core sets that for you. In general drivers never need to 
 change the
 state manually.

 It happens twice in this driver and in both cases the assignment can be 
 removed.

OK, will drop this.

Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 04/49] media: davinci: vpif_display: release buffers in case start_streaming() call back fails

2014-05-16 Thread Lad, Prabhakar
From: Lad, Prabhakar prabhakar.cse...@gmail.com

this patch adds support to release the buffer by calling
vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED
if start_streaming() call back fails.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/media/platform/davinci/vpif_display.c |   42 +++--
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c 
b/drivers/media/platform/davinci/vpif_display.c
index 8bb9f02..1a17a45 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -196,26 +196,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
struct channel_obj *ch = vb2_get_drv_priv(vq);
struct common_obj *common = ch-common[VPIF_VIDEO_INDEX];
struct vpif_params *vpif = ch-vpifparams;
-   unsigned long addr = 0;
-   unsigned long flags;
+   struct vpif_disp_buffer *buf, *tmp;
+   unsigned long addr, flags;
int ret;
 
spin_lock_irqsave(common-irqlock, flags);
 
-   /* Get the next frame from the buffer queue */
-   common-next_frm = common-cur_frm =
-   list_entry(common-dma_queue.next,
-  struct vpif_disp_buffer, list);
-
-   list_del(common-cur_frm-list);
-   spin_unlock_irqrestore(common-irqlock, flags);
-   /* Mark state of the current frame to active */
-   common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;
-
/* Initialize field_id and started member */
ch-field_id = 0;
common-started = 1;
-   addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
+
/* Calculate the offset for Y and C data  in the buffer */
vpif_calculate_offsets(ch);
 
@@ -225,7 +215,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
|| (!ch-vpifparams.std_info.frm_fmt
 (common-fmt.fmt.pix.field == V4L2_FIELD_NONE))) {
vpif_err(conflict in field format and std format\n);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err;
}
 
/* clock settings */
@@ -234,17 +225,28 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
ycmux_mode, ch-vpifparams.std_info.hd_sd);
if (ret  0) {
vpif_err(can't set clock\n);
-   return ret;
+   goto err;
}
}
 
/* set the parameters and addresses */
ret = vpif_set_video_params(vpif, ch-channel_id + 2);
if (ret  0)
-   return ret;
+   goto err;
 
common-started = ret;
vpif_config_addr(ch, ret);
+   /* Get the next frame from the buffer queue */
+   common-next_frm = common-cur_frm =
+   list_entry(common-dma_queue.next,
+  struct vpif_disp_buffer, list);
+
+   list_del(common-cur_frm-list);
+   spin_unlock_irqrestore(common-irqlock, flags);
+   /* Mark state of the current frame to active */
+   common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;
+
+   addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
common-set_addr((addr + common-ytop_off),
(addr + common-ybtm_off),
(addr + common-ctop_off),
@@ -271,6 +273,14 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
unsigned int count)
}
 
return 0;
+
+err:
+   list_for_each_entry_safe(buf, tmp, common-dma_queue, list) {
+   list_del(buf-list);
+   vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED);
+   }
+
+   return ret;
 }
 
 /* abort streaming and wait for last buffer */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html