v4l: yet another video-buf interface update

PATCH In review Gerd Knorr 2 participants 3 messages Nov 8, 2004 Media (V4L/DVB)

Share & tools

Gerd Knorr

author

2004-11-08 09:27 UTC #0 lore raw

PATCH 1/6 v4l: yet another video-buf interface update

This is one more interface fix for the video-buf.c module, the first attempt on that wasn't that clever. Instead of passing the driver private data through all function calls I've just made that an element of the videobuf_queue struct which is passed around everythere _anyway_.

That removes some reduncancy, should be less error prone and gain me some points on rusty's interface design scala ;)

Ah, it also fixes the tvtime crashes which where caused by overviewing one place to fixup in the first attempt.

Signed-off-by Gerd Knorr <[email protected]>

---
 drivers/media/video/video-buf.c |  133 ++++++++++++++++----------------
 include/media/video-buf.h       |   53 +++++++-----
 2 files changed, 97 insertions(+), 89 deletions(-)

include/media/video-buf.h +30 ~~-23~~

Index: linux-2004-11-05/include/media/video-buf.h
===================================================================
@@ -1,5 +1,5 @@
 /*
- * $Id: video-buf.h,v 1.8 2004/10/13 10:39:00 kraxel Exp $
+ * $Id: video-buf.h,v 1.9 2004/11/07 13:17:15 kraxel Exp $
  *
  * generic helper functions for video4linux capture buffers, to handle
  * memory management and PCI DMA.  Right now bttv + saa7134 use it.

@@ -121,7 +121,6 @@ struct videobuf_queue;
 
 struct videobuf_mapping {
 	unsigned int count;
-	int highmem_ok;
 	unsigned long start;
 	unsigned long end;
 	struct videobuf_queue *q;

@@ -167,12 +166,15 @@ struct videobuf_buffer {
 };
 
 struct videobuf_queue_ops {
-	int (*buf_setup)(void *priv,
+	int (*buf_setup)(struct videobuf_queue *q,
 			 unsigned int *count, unsigned int *size);
-	int (*buf_prepare)(void *priv,struct videobuf_buffer *vb,
+	int (*buf_prepare)(struct videobuf_queue *q,
+			   struct videobuf_buffer *vb,
 			   enum v4l2_field field);
-	void (*buf_queue)(void *priv,struct videobuf_buffer *vb);
-	void (*buf_release)(void *priv,struct videobuf_buffer *vb);
+	void (*buf_queue)(struct videobuf_queue *q,
+			  struct videobuf_buffer *vb);
+	void (*buf_release)(struct videobuf_queue *q,
+			    struct videobuf_buffer *vb);
 };
 
 struct videobuf_queue {

@@ -196,6 +198,9 @@ struct videobuf_queue {
 	unsigned int               reading;
 	unsigned int               read_off;
 	struct videobuf_buffer     *read_buf;
+
+	/* driver private data */
+	void                       *priv_data;
 };
 
 void* videobuf_alloc(unsigned int size);

@@ -205,44 +210,46 @@ int videobuf_iolock(struct pci_dev *pci,
 
 void videobuf_queue_init(struct videobuf_queue *q,
 			 struct videobuf_queue_ops *ops,
-			 struct pci_dev *pci, spinlock_t *irqlock,
+			 struct pci_dev *pci,
+			 spinlock_t *irqlock,
 			 enum v4l2_buf_type type,
 			 enum v4l2_field field,
-			 unsigned int msize);
+			 unsigned int msize,
+			 void *priv);
 int  videobuf_queue_is_busy(struct videobuf_queue *q);
-void videobuf_queue_cancel(void *priv, struct videobuf_queue *q);
+void videobuf_queue_cancel(struct videobuf_queue *q);
 
 enum v4l2_field videobuf_next_field(struct videobuf_queue *q);
 void videobuf_status(struct v4l2_buffer *b, struct videobuf_buffer *vb,
 		     enum v4l2_buf_type type);
-int videobuf_reqbufs(void *priv, struct videobuf_queue *q,
+int videobuf_reqbufs(struct videobuf_queue *q,
 		     struct v4l2_requestbuffers *req);
 int videobuf_querybuf(struct videobuf_queue *q, struct v4l2_buffer *b);
-int videobuf_qbuf(void *priv, struct videobuf_queue *q,
+int videobuf_qbuf(struct videobuf_queue *q,
 		  struct v4l2_buffer *b);
-int videobuf_dqbuf(void *priv, struct videobuf_queue *q,
+int videobuf_dqbuf(struct videobuf_queue *q,
 		   struct v4l2_buffer *b, int nonblocking);
-int videobuf_streamon(void *priv, struct videobuf_queue *q);
-int videobuf_streamoff(void *priv, struct videobuf_queue *q);
+int videobuf_streamon(struct videobuf_queue *q);
+int videobuf_streamoff(struct videobuf_queue *q);
 
-int videobuf_read_start(void *priv, struct videobuf_queue *q);
-void videobuf_read_stop(void *priv, struct videobuf_queue *q);
-ssize_t videobuf_read_stream(void *priv, struct videobuf_queue *q,
+int videobuf_read_start(struct videobuf_queue *q);
+void videobuf_read_stop(struct videobuf_queue *q);
+ssize_t videobuf_read_stream(struct videobuf_queue *q,
 			     char __user *data, size_t count, loff_t *ppos,
 			     int vbihack, int nonblocking);
-ssize_t videobuf_read_one(void *priv, struct videobuf_queue *q,
+ssize_t videobuf_read_one(struct videobuf_queue *q,
 			  char __user *data, size_t count, loff_t *ppos,
 			  int nonblocking);
-unsigned int videobuf_poll_stream(struct file *file, void *priv,
+unsigned int videobuf_poll_stream(struct file *file,
 				  struct videobuf_queue *q,
 				  poll_table *wait);
 
-int videobuf_mmap_setup(void *priv, struct videobuf_queue *q,
+int videobuf_mmap_setup(struct videobuf_queue *q,
 			unsigned int bcount, unsigned int bsize,
 			enum v4l2_memory memory);
-int videobuf_mmap_free(void *priv, struct videobuf_queue *q);
-int videobuf_mmap_mapper(struct vm_area_struct *vma,
-			 struct videobuf_queue *q);
+int videobuf_mmap_free(struct videobuf_queue *q);
+int videobuf_mmap_mapper(struct videobuf_queue *q,
+			 struct vm_area_struct *vma);
 
 /* --------------------------------------------------------------------- */

drivers/media/video/video-buf.c +67 ~~-66~~

423 lines not on this page · raw message

Signature · 1 line

#define printk(args...) fprintf(stderr, ## args)

Andrew Morton

2004-11-08 21:49 UTC #1 lore raw

Quoted 5 lines · Gerd Knorr

This is one more interface fix for the video-buf.c module, the first attempt on that wasn't that clever. Instead of passing the driver private data through all function calls I've just made that an element of the videobuf_queue struct which is passed around everythere _anyway_.

This patch throws a reject in videobuf_vm_close() because it's expecting

	map->q->ops->buf_release(vma->vm_file,map->q->bufs[i]);
and not
	map->q->ops->buf_release(vma->vm_file->private_data,map->q->bufs[i]);

I'll fix things up, but please check that.

Gerd Knorr

author

2004-11-08 23:21 UTC #2 lore raw

Quoted 7 lines · Andrew Morton

Quoted 5 lines · Gerd Knorr
This is one more interface fix for the video-buf.c module, the first attempt on that wasn't that clever. Instead of passing the driver private data through all function calls I've just made that an element of the videobuf_queue struct which is passed around everythere _anyway_.

Quoted 7 lines

This patch throws a reject in videobuf_vm_close() because it's expecting
	map->q->ops->buf_release(vma->vm_file,map->q->bufs[i]);
and not
	map->q->ops->buf_release(vma->vm_file->private_data,map->q->bufs[i]);
I'll fix things up, but please check that.

Someone (Peter Osterlund IIRC) mailed a patch with the "vma->vm_file" => "vma->vm_file->private_data" fix to lkml. Thats actually the place I forgot on the first attempt and which causes the tvtime crashes mentioned in the changelog.

My patches are built against a fresh linus bk tree, not against -mm, so its probably just that patch being merged into -mm which causes the rejects. No problem, my patch obsoletes this one-liner fix.

Gerd

Signature · 1 line

#define printk(args...) fprintf(stderr, ## args)