Re: [PATCH V3 09/15] [media] marvell-ccic: add get_mcam function for marvell-ccic driver
On Sun, 16 Dec 2012, Jonathan Corbet wrote:
> On Sat, 15 Dec 2012 17:57:58 +0800
> Albert Wang wrote:
>
> > This patch adds get_mcam() inline function which is prepared for
> > adding soc_camera support in marvell-ccic driver
>
> Time for a bikeshed moment: "get" generally is understood to mean
> incrementing a reference count in kernel code. Can it have a name like
> vbq_to_mcam() instead?
>
> Also:
>
> > @@ -1073,14 +1073,17 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
> > static void mcam_vb_buf_queue(struct vb2_buffer *vb)
> > {
> > struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
> > - struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
> > + struct mcam_camera *cam = get_mcam(vb->vb2_queue);
> > struct v4l2_pix_format *fmt = &cam->pix_format;
> > unsigned long flags;
> > int start;
> > dma_addr_t dma_handle;
> > + unsigned long size;
> > u32 pixel_count = fmt->width * fmt->height;
> >
> > spin_lock_irqsave(&cam->dev_lock, flags);
> > + size = vb2_plane_size(vb, 0);
> > + vb2_set_plane_payload(vb, 0, size);
> > dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0);
> > BUG_ON(!dma_handle);
> > start = (cam->state == S_BUFWAIT) && !list_empty(&cam->buffers);
>
> There is an unrelated change here that belongs in a separate patch.
Right, agree.
Thanks
Guennadi
> > @@ -1138,9 +1141,12 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq)
> > */
> > static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int
> > count)
> > {
> > - struct mcam_camera *cam = vb2_get_drv_priv(vq);
> > + struct mcam_camera *cam = get_mcam(vq);
> > unsigned int frame;
> >
> > + if (count < 2)
> > + return -EINVAL;
> > +
>
> Here too - unrelated change.
>
> > if (cam->state != S_IDLE) {
> > INIT_LIST_HEAD(&cam->buffers);
> > return -EINVAL;
> > @@ -1170,7 +1176,7 @@ static int mcam_vb_start_streaming(struct vb2_queue
> > *vq, unsigned int count)
> >
> > static int mcam_vb_stop_streaming(struct vb2_queue *vq)
> > {
> > - struct mcam_camera *cam = vb2_get_drv_priv(vq);
> > + struct mcam_camera *cam = get_mcam(vq);
> > unsigned long flags;
> >
> > if (cam->state == S_BUFWAIT) {
> > @@ -1181,6 +1187,7 @@ static int mcam_vb_stop_streaming(struct vb2_queue
> > *vq)
> > if (cam->state != S_STREAMING)
> > return -EINVAL;
> > mcam_ctlr_stop_dma(cam);
> > + cam->state = S_IDLE;
>
> ...and also here ...
>
> jon
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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 V3 09/15] [media] marvell-ccic: add get_mcam function for marvell-ccic driver
Hi, Jonathan
>-Original Message-
>From: Jonathan Corbet [mailto:cor...@lwn.net]
>Sent: Monday, 17 December, 2012 00:25
>To: Albert Wang
>Cc: g.liakhovet...@gmx.de; linux-media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH V3 09/15] [media] marvell-ccic: add get_mcam function for
>marvell-
>ccic driver
>
>On Sat, 15 Dec 2012 17:57:58 +0800
>Albert Wang wrote:
>
>> This patch adds get_mcam() inline function which is prepared for
>> adding soc_camera support in marvell-ccic driver
>
>Time for a bikeshed moment: "get" generally is understood to mean
>incrementing a reference count in kernel code. Can it have a name like
>vbq_to_mcam() instead?
>
[Albert Wang] Sure. It looks your name is more professional. :)
>Also:
>
>> @@ -1073,14 +1073,17 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
>> static void mcam_vb_buf_queue(struct vb2_buffer *vb)
>> {
>> struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
>> -struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
>> +struct mcam_camera *cam = get_mcam(vb->vb2_queue);
>> struct v4l2_pix_format *fmt = &cam->pix_format;
>> unsigned long flags;
>> int start;
>> dma_addr_t dma_handle;
>> +unsigned long size;
>> u32 pixel_count = fmt->width * fmt->height;
>>
>> spin_lock_irqsave(&cam->dev_lock, flags);
>> +size = vb2_plane_size(vb, 0);
>> +vb2_set_plane_payload(vb, 0, size);
>> dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0);
>> BUG_ON(!dma_handle);
>> start = (cam->state == S_BUFWAIT) && !list_empty(&cam->buffers);
>
>There is an unrelated change here that belongs in a separate patch.
>
[Albert Wang] OK
>> @@ -1138,9 +1141,12 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq)
>> */
>> static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count)
>> {
>> -struct mcam_camera *cam = vb2_get_drv_priv(vq);
>> +struct mcam_camera *cam = get_mcam(vq);
>> unsigned int frame;
>>
>> +if (count < 2)
>> +return -EINVAL;
>> +
>
>Here too - unrelated change.
>
[Albert Wang] Em, it looks we should add a new patch to contain these changes.
:)
>> if (cam->state != S_IDLE) {
>> INIT_LIST_HEAD(&cam->buffers);
>> return -EINVAL;
>> @@ -1170,7 +1176,7 @@ static int mcam_vb_start_streaming(struct vb2_queue
>> *vq,
>unsigned int count)
>>
>> static int mcam_vb_stop_streaming(struct vb2_queue *vq)
>> {
>> -struct mcam_camera *cam = vb2_get_drv_priv(vq);
>> +struct mcam_camera *cam = get_mcam(vq);
>> unsigned long flags;
>>
>> if (cam->state == S_BUFWAIT) {
>> @@ -1181,6 +1187,7 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq)
>> if (cam->state != S_STREAMING)
>> return -EINVAL;
>> mcam_ctlr_stop_dma(cam);
>> +cam->state = S_IDLE;
>
>...and also here ...
>
>jon
Thanks
Albert Wang
86-21-61092656
--
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 V3 09/15] [media] marvell-ccic: add get_mcam function for marvell-ccic driver
On Sat, 15 Dec 2012 17:57:58 +0800
Albert Wang wrote:
> This patch adds get_mcam() inline function which is prepared for
> adding soc_camera support in marvell-ccic driver
Time for a bikeshed moment: "get" generally is understood to mean
incrementing a reference count in kernel code. Can it have a name like
vbq_to_mcam() instead?
Also:
> @@ -1073,14 +1073,17 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
> static void mcam_vb_buf_queue(struct vb2_buffer *vb)
> {
> struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
> - struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
> + struct mcam_camera *cam = get_mcam(vb->vb2_queue);
> struct v4l2_pix_format *fmt = &cam->pix_format;
> unsigned long flags;
> int start;
> dma_addr_t dma_handle;
> + unsigned long size;
> u32 pixel_count = fmt->width * fmt->height;
>
> spin_lock_irqsave(&cam->dev_lock, flags);
> + size = vb2_plane_size(vb, 0);
> + vb2_set_plane_payload(vb, 0, size);
> dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0);
> BUG_ON(!dma_handle);
> start = (cam->state == S_BUFWAIT) && !list_empty(&cam->buffers);
There is an unrelated change here that belongs in a separate patch.
> @@ -1138,9 +1141,12 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq)
> */
> static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count)
> {
> - struct mcam_camera *cam = vb2_get_drv_priv(vq);
> + struct mcam_camera *cam = get_mcam(vq);
> unsigned int frame;
>
> + if (count < 2)
> + return -EINVAL;
> +
Here too - unrelated change.
> if (cam->state != S_IDLE) {
> INIT_LIST_HEAD(&cam->buffers);
> return -EINVAL;
> @@ -1170,7 +1176,7 @@ static int mcam_vb_start_streaming(struct vb2_queue
> *vq, unsigned int count)
>
> static int mcam_vb_stop_streaming(struct vb2_queue *vq)
> {
> - struct mcam_camera *cam = vb2_get_drv_priv(vq);
> + struct mcam_camera *cam = get_mcam(vq);
> unsigned long flags;
>
> if (cam->state == S_BUFWAIT) {
> @@ -1181,6 +1187,7 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq)
> if (cam->state != S_STREAMING)
> return -EINVAL;
> mcam_ctlr_stop_dma(cam);
> + cam->state = S_IDLE;
...and also here ...
jon
--
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 V3 09/15] [media] marvell-ccic: add get_mcam function for marvell-ccic driver
This patch adds get_mcam() inline function which is prepared for
adding soc_camera support in marvell-ccic driver
Signed-off-by: Libin Yang
Signed-off-by: Albert Wang
---
drivers/media/platform/marvell-ccic/mcam-core.c | 27 ++-
drivers/media/platform/marvell-ccic/mcam-core.h |5 +
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
b/drivers/media/platform/marvell-ccic/mcam-core.c
index c3c8873..9b5a5e9 100755
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1057,7 +1057,7 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
unsigned int *num_planes, unsigned int sizes[],
void *alloc_ctxs[])
{
- struct mcam_camera *cam = vb2_get_drv_priv(vq);
+ struct mcam_camera *cam = get_mcam(vq);
int minbufs = (cam->buffer_mode == B_DMA_contig) ? 3 : 2;
sizes[0] = cam->pix_format.sizeimage;
@@ -1073,14 +1073,17 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
static void mcam_vb_buf_queue(struct vb2_buffer *vb)
{
struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
- struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+ struct mcam_camera *cam = get_mcam(vb->vb2_queue);
struct v4l2_pix_format *fmt = &cam->pix_format;
unsigned long flags;
int start;
dma_addr_t dma_handle;
+ unsigned long size;
u32 pixel_count = fmt->width * fmt->height;
spin_lock_irqsave(&cam->dev_lock, flags);
+ size = vb2_plane_size(vb, 0);
+ vb2_set_plane_payload(vb, 0, size);
dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0);
BUG_ON(!dma_handle);
start = (cam->state == S_BUFWAIT) && !list_empty(&cam->buffers);
@@ -1121,14 +1124,14 @@ static void mcam_vb_buf_queue(struct vb2_buffer *vb)
*/
static void mcam_vb_wait_prepare(struct vb2_queue *vq)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vq);
+ struct mcam_camera *cam = get_mcam(vq);
mutex_unlock(&cam->s_mutex);
}
static void mcam_vb_wait_finish(struct vb2_queue *vq)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vq);
+ struct mcam_camera *cam = get_mcam(vq);
mutex_lock(&cam->s_mutex);
}
@@ -1138,9 +1141,12 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq)
*/
static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vq);
+ struct mcam_camera *cam = get_mcam(vq);
unsigned int frame;
+ if (count < 2)
+ return -EINVAL;
+
if (cam->state != S_IDLE) {
INIT_LIST_HEAD(&cam->buffers);
return -EINVAL;
@@ -1170,7 +1176,7 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq,
unsigned int count)
static int mcam_vb_stop_streaming(struct vb2_queue *vq)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vq);
+ struct mcam_camera *cam = get_mcam(vq);
unsigned long flags;
if (cam->state == S_BUFWAIT) {
@@ -1181,6 +1187,7 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq)
if (cam->state != S_STREAMING)
return -EINVAL;
mcam_ctlr_stop_dma(cam);
+ cam->state = S_IDLE;
/*
* Reset the CCIC PHY after stopping streaming,
* otherwise, the CCIC may be unstable.
@@ -1216,7 +1223,7 @@ static const struct vb2_ops mcam_vb2_ops = {
static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
{
struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
- struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+ struct mcam_camera *cam = get_mcam(vb->vb2_queue);
int ndesc = cam->pix_format.sizeimage/PAGE_SIZE + 1;
mvb->dma_desc = dma_alloc_coherent(cam->dev,
@@ -1232,7 +1239,7 @@ static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
{
struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
- struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+ struct mcam_camera *cam = get_mcam(vb->vb2_queue);
struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
struct mcam_dma_desc *desc = mvb->dma_desc;
struct scatterlist *sg;
@@ -1252,7 +1259,7 @@ static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+ struct mcam_camera *cam = get_mcam(vb->vb2_queue);
struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
dma_unmap_sg(cam->dev, sgd->sglist, sgd->num_pages, DMA_FROM_DEVICE);
@@ -1261,7 +1268,7 @@ static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
{
- struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+ struct mcam_camera *ca
