Re: [RFCv6 PATCH 04/16] vb2-dma-sg: add allocation context to dma-sg
Hi Pawel,
On 11/16/2014 02:13 PM, Pawel Osciak wrote:
On Mon, Nov 10, 2014 at 8:49 PM, Hans Verkuil hverk...@xs4all.nl wrote:
From: Hans Verkuil hans.verk...@cisco.com
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 hans.verk...@cisco.com
---
[...]
@@ -166,6 +177,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
unsigned long size,
enum dma_data_direction dma_dir)
{
+ struct vb2_dma_sg_conf *conf = alloc_ctx;
struct vb2_dma_sg_buf *buf;
unsigned long first, last;
int num_pages_from_user;
@@ -235,6 +247,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf-num_pages, buf-offset, size, 0))
goto userptr_fail_alloc_table_from_pages;
+ /* Prevent the device from being released while the buffer is used */
+ buf-dev = get_device(conf-dev);
I'm not sure if we should be managing this... As far as I understand
the logic behind taking a ref in alloc, if we are the exporter, we may
have to keep it in case we need to free the buffers after our device
goes away. But for userptr, we only need this for syncs, and in that
case it's triggered by our driver, so I think we don't have to worry
about that. If we do though, then dma-contig should be doing this as
well.
You are correct. I'll remove this for get/put_userptr.
return buf;
userptr_fail_alloc_table_from_pages:
@@ -274,6 +288,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
}
kfree(buf-pages);
vb2_put_vma(buf-vma);
+ put_device(buf-dev);
kfree(buf);
}
@@ -356,6 +371,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
};
EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
+void *vb2_dma_sg_init_ctx(struct device *dev)
+{
+ struct vb2_dma_sg_conf *conf;
+
+ conf = kzalloc(sizeof(*conf), GFP_KERNEL);
+ if (!conf)
+ return ERR_PTR(-ENOMEM);
+
+ conf-dev = dev;
+
+ return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
+
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
+{
+ if (!IS_ERR_OR_NULL(alloc_ctx))
I would prefer not doing this, it's very weird and would really just
be a programming bug.
Erm, I rather like it since it allows you to call cleanup_ctx even if init_ctx
failed. It's actually used like that in the solo driver. Basically for the same
reason that kfree can handle a NULL pointer. Although if I do it here, then it
should also be added to dma-contig.
Regards,
Hans
--
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: [RFCv6 PATCH 04/16] vb2-dma-sg: add allocation context to dma-sg
On Mon, Nov 10, 2014 at 8:49 PM, Hans Verkuil hverk...@xs4all.nl wrote:
From: Hans Verkuil hans.verk...@cisco.com
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 hans.verk...@cisco.com
---
[...]
@@ -166,6 +177,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
unsigned long size,
enum dma_data_direction dma_dir)
{
+ struct vb2_dma_sg_conf *conf = alloc_ctx;
struct vb2_dma_sg_buf *buf;
unsigned long first, last;
int num_pages_from_user;
@@ -235,6 +247,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf-num_pages, buf-offset, size, 0))
goto userptr_fail_alloc_table_from_pages;
+ /* Prevent the device from being released while the buffer is used */
+ buf-dev = get_device(conf-dev);
I'm not sure if we should be managing this... As far as I understand
the logic behind taking a ref in alloc, if we are the exporter, we may
have to keep it in case we need to free the buffers after our device
goes away. But for userptr, we only need this for syncs, and in that
case it's triggered by our driver, so I think we don't have to worry
about that. If we do though, then dma-contig should be doing this as
well.
return buf;
userptr_fail_alloc_table_from_pages:
@@ -274,6 +288,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
}
kfree(buf-pages);
vb2_put_vma(buf-vma);
+ put_device(buf-dev);
kfree(buf);
}
@@ -356,6 +371,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
};
EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
+void *vb2_dma_sg_init_ctx(struct device *dev)
+{
+ struct vb2_dma_sg_conf *conf;
+
+ conf = kzalloc(sizeof(*conf), GFP_KERNEL);
+ if (!conf)
+ return ERR_PTR(-ENOMEM);
+
+ conf-dev = dev;
+
+ return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
+
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
+{
+ if (!IS_ERR_OR_NULL(alloc_ctx))
I would prefer not doing this, it's very weird and would really just
be a programming bug.
--
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
[RFCv6 PATCH 04/16] vb2-dma-sg: add allocation context to dma-sg
From: Hans Verkuil hans.verk...@cisco.com
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 hans.verk...@cisco.com
---
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 | 13 -
drivers/media/platform/marvell-ccic/mcam-core.h | 1 +
drivers/media/v4l2-core/videobuf2-dma-sg.c | 36 +
include/media/videobuf2-dma-sg.h| 3 +++
20 files changed, 108 insertions(+), 10 deletions(-)
diff --git a/drivers/media/pci/cx23885/cx23885-417.c
b/drivers/media/pci/cx23885/cx23885-417.c
index 3948db3..d72a3ec 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 331edda..d452b5c 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 = 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/pci/cx23885/cx23885-dvb.c
b/drivers/media/pci/cx23885/cx23885-dvb.c
index 9da5cf3..e63759e 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -102,6 +102,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 a7c6ef8..1d339a6 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 682a4f9..1b04ab3 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-depth * dev-width * dev-height) 3;
+ alloc_ctxs[0] = dev-alloc_ctx;
return 0;
}
diff --git a/drivers/media/pci/cx23885/cx23885.h
