Hello, On Friday, February 18, 2011 9:14 AM Guennadi Liakhovetski wrote: > Now that soc-camera supports videobuf API v1 and v2, camera-host drivers > can be converted to videobuf2 individually. This patch converts the > mx3_camera driver. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > --- > > This one is also on git.linuxtv.org already. > > drivers/media/video/mx3_camera.c | 342 ++++++++++++++++---------------------- > 1 files changed, 143 insertions(+), 199 deletions(-) > > diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c > index b9cb4a4..d61ff0d 100644 > --- a/drivers/media/video/mx3_camera.c > +++ b/drivers/media/video/mx3_camera.c > @@ -21,7 +21,7 @@ > > #include <media/v4l2-common.h> > #include <media/v4l2-dev.h> > -#include <media/videobuf-dma-contig.h> > +#include <media/videobuf2-dma-contig.h> > #include <media/soc_camera.h> > #include <media/soc_mediabus.h> > > @@ -62,10 +62,16 @@ > > #define MAX_VIDEO_MEM 16 > > +enum csi_buffer_state { > + CSI_BUF_NEEDS_INIT, > + CSI_BUF_PREPARED, > +}; > + > struct mx3_camera_buffer { > /* common v4l buffer stuff -- must be first */ > - struct videobuf_buffer vb; > - enum v4l2_mbus_pixelcode code; > + struct vb2_buffer vb; > + enum csi_buffer_state state; > + struct list_head queue; > > /* One descriptot per scatterlist (per frame) */ > struct dma_async_tx_descriptor *txd; > @@ -108,6 +114,9 @@ struct mx3_camera_dev { > struct list_head capture; > spinlock_t lock; /* Protects video buffer lists */ > struct mx3_camera_buffer *active; > + struct vb2_alloc_ctx *alloc_ctx; > + enum v4l2_field field; > + int sequence; > > /* IDMAC / dmaengine interface */ > struct idmac_channel *idmac_channel[1]; /* We need one channel */ > @@ -130,6 +139,11 @@ static void csi_reg_write(struct mx3_camera_dev *mx3, u32 value, off_t reg) > __raw_writel(value, mx3->base + reg); > } > > +static struct mx3_camera_buffer *to_mx3_vb(struct vb2_buffer *vb) > +{ > + return container_of(vb, struct mx3_camera_buffer, vb); > +} > + > /* Called from the IPU IDMAC ISR */ > static void mx3_cam_dma_done(void *arg) > { > @@ -137,20 +151,20 @@ static void mx3_cam_dma_done(void *arg) > struct dma_chan *chan = desc->txd.chan; > struct idmac_channel *ichannel = to_idmac_chan(chan); > struct mx3_camera_dev *mx3_cam = ichannel->client; > - struct videobuf_buffer *vb; > > dev_dbg(chan->device->dev, "callback cookie %d, active DMA 0x%08x\n", > desc->txd.cookie, mx3_cam->active ? sg_dma_address(&mx3_cam->active->sg) : 0); > > spin_lock(&mx3_cam->lock); > if (mx3_cam->active) { > - vb = &mx3_cam->active->vb; > - > - list_del_init(&vb->queue); > - vb->state = VIDEOBUF_DONE; > - do_gettimeofday(&vb->ts); > - vb->field_count++; > - wake_up(&vb->done); > + struct vb2_buffer *vb = &mx3_cam->active->vb; > + struct mx3_camera_buffer *buf = to_mx3_vb(vb); > + > + list_del_init(&buf->queue); > + do_gettimeofday(&vb->v4l2_buf.timestamp); > + vb->v4l2_buf.field = mx3_cam->field; > + vb->v4l2_buf.sequence = mx3_cam->sequence++; > + vb2_buffer_done(vb, VB2_BUF_STATE_DONE); > } > > if (list_empty(&mx3_cam->capture)) { > @@ -165,38 +179,10 @@ static void mx3_cam_dma_done(void *arg) > } > > mx3_cam->active = list_entry(mx3_cam->capture.next, > - struct mx3_camera_buffer, vb.queue); > - mx3_cam->active->vb.state = VIDEOBUF_ACTIVE; > + struct mx3_camera_buffer, queue); > spin_unlock(&mx3_cam->lock); > } > > -static void free_buffer(struct videobuf_queue *vq, struct mx3_camera_buffer *buf) > -{ > - struct soc_camera_device *icd = vq->priv_data; > - struct videobuf_buffer *vb = &buf->vb; > - struct dma_async_tx_descriptor *txd = buf->txd; > - struct idmac_channel *ichan; > - > - BUG_ON(in_interrupt()); > - > - dev_dbg(icd->dev.parent, "%s (vb=0x%p) 0x%08lx %d\n", __func__, > - vb, vb->baddr, vb->bsize); > - > - /* > - * This waits until this buffer is out of danger, i.e., until it is no > - * longer in STATE_QUEUED or STATE_ACTIVE > - */ > - videobuf_waiton(vq, vb, 0, 0); > - if (txd) { > - ichan = to_idmac_chan(txd->chan); > - async_tx_ack(txd); > - } > - videobuf_dma_contig_free(vq, vb); > - buf->txd = NULL; > - > - vb->state = VIDEOBUF_NEEDS_INIT; > -} > - > /* > * Videobuf operations > */ > @@ -205,10 +191,11 @@ static void free_buffer(struct videobuf_queue *vq, struct mx3_camera_buffer *buf > * Calculate the __buffer__ (not data) size and number of buffers. > * Called with .vb_lock held > */ > -static int mx3_videobuf_setup(struct videobuf_queue *vq, unsigned int *count, > - unsigned int *size) > +static int mx3_videobuf_setup(struct vb2_queue *vq, > + unsigned int *count, unsigned int *num_planes, > + unsigned long sizes[], void *alloc_ctxs[]) > { > - struct soc_camera_device *icd = vq->priv_data; > + struct soc_camera_device *icd = soc_camera_from_vb2q(vq); > struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent); > struct mx3_camera_dev *mx3_cam = ici->priv; > int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, > @@ -220,104 +207,68 @@ static int mx3_videobuf_setup(struct videobuf_queue *vq, unsigned int *count, > if (!mx3_cam->idmac_channel[0]) > return -EINVAL; > > - *size = bytes_per_line * icd->user_height; > + *num_planes = 1; > + > + mx3_cam->sequence = 0; > + sizes[0] = bytes_per_line * icd->user_height; > + alloc_ctxs[0] = mx3_cam->alloc_ctx; > > if (!*count) > *count = 32; > > - if (*size * *count > MAX_VIDEO_MEM * 1024 * 1024) > - *count = MAX_VIDEO_MEM * 1024 * 1024 / *size; > + if (sizes[0] * *count > MAX_VIDEO_MEM * 1024 * 1024) > + *count = MAX_VIDEO_MEM * 1024 * 1024 / sizes[0]; > > return 0; > } > > /* Called with .vb_lock held */ vb_lock has no meaning in vb2, so the above line should be removed imho. > -static int mx3_videobuf_prepare(struct videobuf_queue *vq, > - struct videobuf_buffer *vb, enum v4l2_field field) > +static int mx3_videobuf_prepare(struct vb2_buffer *vb) > { > - struct soc_camera_device *icd = vq->priv_data; > + struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue); > struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent); > struct mx3_camera_dev *mx3_cam = ici->priv; > - struct mx3_camera_buffer *buf = > - container_of(vb, struct mx3_camera_buffer, vb); > + struct idmac_channel *ichan = mx3_cam->idmac_channel[0]; > + struct scatterlist *sg; > + struct mx3_camera_buffer *buf; > size_t new_size; > - int ret; > int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, > icd->current_fmt->host_fmt); > > if (bytes_per_line < 0) > return bytes_per_line; > > - new_size = bytes_per_line * icd->user_height; > + buf = to_mx3_vb(vb); > + sg = &buf->sg; > > - /* > - * I think, in buf_prepare you only have to protect global data, > - * the actual buffer is yours > - */ > - > - if (buf->code != icd->current_fmt->code || > - vb->width != icd->user_width || > - vb->height != icd->user_height || > - vb->field != field) { > - buf->code = icd->current_fmt->code; > - vb->width = icd->user_width; > - vb->height = icd->user_height; > - vb->field = field; > - if (vb->state != VIDEOBUF_NEEDS_INIT) > - free_buffer(vq, buf); > - } > + new_size = bytes_per_line * icd->user_height; > > - if (vb->baddr && vb->bsize < new_size) { > - /* User provided buffer, but it is too small */ > - ret = -ENOMEM; > - goto out; > + if (vb2_plane_size(vb, 0) < new_size) { > + dev_err(icd->dev.parent, "Buffer too small (%lu < %zu)\n", > + vb2_plane_size(vb, 0), new_size); > + return -ENOBUFS; > } > > - if (vb->state == VIDEOBUF_NEEDS_INIT) { > - struct idmac_channel *ichan = mx3_cam->idmac_channel[0]; > - struct scatterlist *sg = &buf->sg; > - > - /* > - * The total size of video-buffers that will be allocated / mapped. > - * *size that we calculated in videobuf_setup gets assigned to > - * vb->bsize, and now we use the same calculation to get vb->size. > - */ > - vb->size = new_size; > - > - /* This actually (allocates and) maps buffers */ > - ret = videobuf_iolock(vq, vb, NULL); > - if (ret) > - goto fail; > - > - /* > - * We will have to configure the IDMAC channel. It has two slots > - * for DMA buffers, we shall enter the first two buffers there, > - * and then submit new buffers in DMA-ready interrupts > - */ > - sg_init_table(sg, 1); > - sg_dma_address(sg) = videobuf_to_dma_contig(vb); > - sg_dma_len(sg) = vb->size; > + if (buf->state == CSI_BUF_NEEDS_INIT) { > + sg_dma_address(sg) = (dma_addr_t)icd->vb2_vidq.mem_ops->cookie( > + vb->planes[0].mem_priv); You should use vb2_dma_contig_plane_paddr() function from include/media/videobuf2-dma-contig.h instead of hacking with cookie directly: sg_dma_address(sg) = vb2_dma_contig_plane_paddr(vb, 0); Everything else look fine! > [snip] Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html