Re: [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
Hi Hans,
Thank you for working on this!
On Fri, Sep 12, 2014 at 8:59 PM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil
[...]
> diff --git a/drivers/media/pci/cx23885/cx23885-core.c
> b/drivers/media/pci/cx23885/cx23885-core.c
> index cb94366b..8a36fcd 100644
> --- a/drivers/media/pci/cx23885/cx23885-core.c
> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> @@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
> if (!pci_dma_supported(pci_dev, 0x)) {
> printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
> err = -EIO;
> - goto fail_irq;
> + goto fail_context;
> }
>
> + dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
> + if (IS_ERR(dev->alloc_ctx)) {
> + err = -ENOMEM;
err = PTR_ERR(dev->alloc_ctx) ?
> + goto fail_context;
> + }
> err = request_irq(pci_dev->irq, cx23885_irq,
> IRQF_SHARED, dev->name, dev);
> if (err < 0) {
> @@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
> return 0;
>
> fail_irq:
> + vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> +fail_context:
> cx23885_dev_unregister(dev);
> fail_ctrl:
> v4l2_ctrl_handler_free(hdl);
> @@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
> free_irq(pci_dev->irq, dev);
>
> cx23885_dev_unregister(dev);
> + vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> v4l2_ctrl_handler_free(&dev->ctrl_handler);
> v4l2_device_unregister(v4l2_dev);
> kfree(dev);
[...]
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
> b/drivers/media/platform/marvell-ccic/mcam-core.h
> index e0e628c..7b8c201 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
> @@ -176,6 +176,7 @@ struct mcam_camera {
> /* DMA buffers - DMA modes */
> struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
> struct vb2_alloc_ctx *vb_alloc_ctx;
> + struct vb2_alloc_ctx *vb_alloc_ctx_sg;
Should this be under #ifdef MCAM_MODE_DMA_SG?
>
> /* Mode-specific ops, set at open time */
> void (*dma_setup)(struct mcam_camera *cam);
[...]
> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> index 313d977..d77e397 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
>
> static void vb2_vmalloc_put(void *buf_priv);
>
> -static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, int
> write,
> + gfp_t gfp_flags)
I agree with Laurent that "write" is confusing, this could be a
direction flag, but the dma direction allows bidirectional, which we
would not be using. So I would personally prefer a binary flag or
enum. So perhaps we should keep this, only documenting it please.
--
Best regards,
Pawel Osciak
--
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: [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
Hi Hans,
Thank you for the patch.
On Friday 12 September 2014 14:59:51 Hans Verkuil wrote:
> From: Hans Verkuil
>
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/pci/cx23885/cx23885-417.c | 1 +
> drivers/media/pci/cx23885/cx23885-core.c| 10 +-
> drivers/media/pci/cx23885/cx23885-dvb.c | 1 +
> drivers/media/pci/cx23885/cx23885-vbi.c | 1 +
> drivers/media/pci/cx23885/cx23885-video.c | 1 +
> drivers/media/pci/cx23885/cx23885.h | 1 +
> drivers/media/pci/saa7134/saa7134-core.c| 18 +++---
> drivers/media/pci/saa7134/saa7134-ts.c | 1 +
> drivers/media/pci/saa7134/saa7134-vbi.c | 1 +
> drivers/media/pci/saa7134/saa7134-video.c | 1 +
> drivers/media/pci/saa7134/saa7134.h | 1 +
> drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 10 ++
> drivers/media/pci/solo6x10/solo6x10.h | 1 +
> drivers/media/pci/tw68/tw68-core.c | 15 +++--
> drivers/media/pci/tw68/tw68-video.c | 1 +
> drivers/media/pci/tw68/tw68.h | 1 +
> drivers/media/platform/marvell-ccic/mcam-core.c | 7
> drivers/media/platform/marvell-ccic/mcam-core.h | 1 +
> drivers/media/v4l2-core/videobuf2-core.c| 3 +-
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++-
> drivers/media/v4l2-core/videobuf2-dma-sg.c | 44 --
> drivers/media/v4l2-core/videobuf2-vmalloc.c | 3 +-
> include/media/videobuf2-core.h | 3 +-
> include/media/videobuf2-dma-sg.h| 3 ++
> 24 files changed, 118 insertions(+), 15 deletions(-)
[snip]
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index e5247a4..087cd62 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -189,6 +189,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
> static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
> {
> struct vb2_queue *q = vb->vb2_queue;
> + int write = !V4L2_TYPE_IS_OUTPUT(q->type);
> void *mem_priv;
> int plane;
>
> @@ -200,7 +201,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
> unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
>
> mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
> - size, q->gfp_flags);
> + size, write, q->gfp_flags);
> if (IS_ERR_OR_NULL(mem_priv))
> goto free;
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 4a02ade..6675f12
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -155,7 +155,8 @@ static void vb2_dc_put(void *buf_priv)
> kfree(buf);
> }
>
> -static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, int write,
> + gfp_t gfp_flags)
> {
> struct vb2_dc_conf *conf = alloc_ctx;
> struct device *dev = conf->dev;
> @@ -176,6 +177,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
> size, gfp_t gfp_flags) /* Prevent the device from being released while the
> buffer is used */ buf->dev = get_device(dev);
> buf->size = size;
> + buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
This is an unrelated bug fix, it should be split to a separate patch.
> buf->handler.refcount = &buf->refcount;
> buf->handler.put = vb2_dc_put;
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index adefc31..9b7a041 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -30,11 +30,17 @@ module_param(debug, int, 0644);
> printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg); \
> } while (0)
>
> +struct vb2_dma_sg_conf {
> + struct device *dev;
> +};
> +
> struct vb2_dma_sg_buf {
> + struct device *dev;
> void*vaddr;
> struct page **pages;
> int write;
> int offset;
> + enum dma_data_direction dma_dir;
> struct sg_table sg_table;
> size_t size;
> unsigned intnum_pages;
> @@ -86,22 +92,27 @@ static int vb2_dma_sg_alloc_compacted(struct
> vb2_dma_sg_buf *buf, return 0;
> }
>
> -static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb
[RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
From: Hans Verkuil
Require that dma-sg also uses an allocation context. This is in preparation
for adding prepare/finish memops to sync the memory between DMA and CPU.
Signed-off-by: Hans Verkuil
---
drivers/media/pci/cx23885/cx23885-417.c | 1 +
drivers/media/pci/cx23885/cx23885-core.c| 10 +-
drivers/media/pci/cx23885/cx23885-dvb.c | 1 +
drivers/media/pci/cx23885/cx23885-vbi.c | 1 +
drivers/media/pci/cx23885/cx23885-video.c | 1 +
drivers/media/pci/cx23885/cx23885.h | 1 +
drivers/media/pci/saa7134/saa7134-core.c| 18 +++---
drivers/media/pci/saa7134/saa7134-ts.c | 1 +
drivers/media/pci/saa7134/saa7134-vbi.c | 1 +
drivers/media/pci/saa7134/saa7134-video.c | 1 +
drivers/media/pci/saa7134/saa7134.h | 1 +
drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c | 10 ++
drivers/media/pci/solo6x10/solo6x10.h | 1 +
drivers/media/pci/tw68/tw68-core.c | 15 +++--
drivers/media/pci/tw68/tw68-video.c | 1 +
drivers/media/pci/tw68/tw68.h | 1 +
drivers/media/platform/marvell-ccic/mcam-core.c | 7
drivers/media/platform/marvell-ccic/mcam-core.h | 1 +
drivers/media/v4l2-core/videobuf2-core.c| 3 +-
drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++-
drivers/media/v4l2-core/videobuf2-dma-sg.c | 44 +++--
drivers/media/v4l2-core/videobuf2-vmalloc.c | 3 +-
include/media/videobuf2-core.h | 3 +-
include/media/videobuf2-dma-sg.h| 3 ++
24 files changed, 118 insertions(+), 15 deletions(-)
diff --git a/drivers/media/pci/cx23885/cx23885-417.c
b/drivers/media/pci/cx23885/cx23885-417.c
index 6973055..b7e143a 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1148,6 +1148,7 @@ static int queue_setup(struct vb2_queue *q, const struct
v4l2_format *fmt,
dev->ts1.ts_packet_count = mpeglines;
*num_planes = 1;
sizes[0] = mpeglinesize * mpeglines;
+ alloc_ctxs[0] = dev->alloc_ctx;
*num_buffers = mpegbufs;
return 0;
}
diff --git a/drivers/media/pci/cx23885/cx23885-core.c
b/drivers/media/pci/cx23885/cx23885-core.c
index cb94366b..8a36fcd 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
if (!pci_dma_supported(pci_dev, 0x)) {
printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
err = -EIO;
- goto fail_irq;
+ goto fail_context;
}
+ dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+ if (IS_ERR(dev->alloc_ctx)) {
+ err = -ENOMEM;
+ goto fail_context;
+ }
err = request_irq(pci_dev->irq, cx23885_irq,
IRQF_SHARED, dev->name, dev);
if (err < 0) {
@@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
return 0;
fail_irq:
+ vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
+fail_context:
cx23885_dev_unregister(dev);
fail_ctrl:
v4l2_ctrl_handler_free(hdl);
@@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
free_irq(pci_dev->irq, dev);
cx23885_dev_unregister(dev);
+ vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
v4l2_device_unregister(v4l2_dev);
kfree(dev);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
b/drivers/media/pci/cx23885/cx23885-dvb.c
index 332e6fa..80e2356 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -97,6 +97,7 @@ static int queue_setup(struct vb2_queue *q, const struct
v4l2_format *fmt,
port->ts_packet_count = 32;
*num_planes = 1;
sizes[0] = port->ts_packet_size * port->ts_packet_count;
+ alloc_ctxs[0] = port->dev->alloc_ctx;
*num_buffers = 32;
return 0;
}
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c
b/drivers/media/pci/cx23885/cx23885-vbi.c
index 67b71f9..a0e5b3f 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -132,6 +132,7 @@ static int queue_setup(struct vb2_queue *q, const struct
v4l2_format *fmt,
lines = VBI_NTSC_LINE_COUNT;
*num_planes = 1;
sizes[0] = lines * VBI_LINE_LENGTH * 2;
+ alloc_ctxs[0] = dev->alloc_ctx;
return 0;
}
diff --git a/drivers/media/pci/cx23885/cx23885-video.c
b/drivers/media/pci/cx23885/cx23885-video.c
index f0ea904..be67836 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -323,6 +323,7 @@ static int queue_setup(struct vb2_queue *q, const struct
v4l2_format *fmt,
*num_planes = 1;
sizes[0] = (dev->fmt
