v4l: subdev: Add pad config allocator and init

PATCH In review Laurent Pinchart 2 participants 4 messages May 24, 2015 – May 25, 2015 Media (V4L/DVB)

lore

Share & tools

lore

Laurent Pinchart

author

2015-05-24 21:09 UTC #0 lore raw

PATCH v2 1/1 v4l: subdev: Add pad config allocator and init

From: Laurent Pinchart <[email protected]>

Add a new subdev operation to initialize a subdev pad config array, and a helper function to allocate and initialize the array. This can be used by bridge drivers to implement try format based on subdev pad operations.

Signed-off-by Laurent Pinchart <[email protected]>

Acked-by Vaibhav Hiremath <[email protected]>

---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config

drivers/media/v4l2-core/v4l2-subdev.c +18 -1

index 63596063b213..d594fe566be2 100644
@@ -35,7 +35,7 @@
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
+	fh->pad = v4l2_subdev_alloc_pad_config(sd);
 	if (fh->pad == NULL)
 		return -ENOMEM;
 #endif

@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
 		sink, link, &source_fmt, &sink_fmt);
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_pad_config *cfg;
+
+	if (!sd->entity.num_pads)
+		return NULL;
+
+	cfg = kcalloc(sd->entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
+		return NULL;
+
+	v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+	return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)

include/media/v4l2-subdev.h +10 -0

index 8f5da73dacff..cf2ee07b0745 100644
@@ -483,6 +483,8 @@ struct v4l2_subdev_pad_config {
  *                  may be adjusted by the subdev driver to device capabilities.
  */
 struct v4l2_subdev_pad_ops {
+	void (*init_cfg)(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg);
 	int (*enum_mbus_code)(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_mbus_code_enum *code);

@@ -675,7 +677,15 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt);
 int v4l2_subdev_link_validate(struct media_link *link);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
+
+static inline void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
+{
+	kfree(cfg);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
+
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);

Signature · 3 lines

Regards,

Laurent Pinchart

Hans Verkuil

Acked-by on v2 1/1

2015-05-24 22:25 UTC #1 lore raw

Quoted 9 lines · Laurent Pinchart

From: Laurent Pinchart <[email protected]>
Add a new subdev operation to initialize a subdev pad config array, and a helper function to allocate and initialize the array. This can be used by bridge drivers to implement try format based on subdev pad operations.
Signed-off-by Laurent Pinchart <[email protected]>
Acked-by Vaibhav Hiremath <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Note that before this goes in there should be at least one subdev driver that implements init_cfg(). Perhaps adv7604?

Regards,

Hans

Quoted 76 lines

---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config

drivers/media/v4l2-core/v4l2-subdev.c +18 -1

index 63596063b213..d594fe566be2 100644
@@ -35,7 +35,7 @@
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
+	fh->pad = v4l2_subdev_alloc_pad_config(sd);
 	if (fh->pad == NULL)
 		return -ENOMEM;
 #endif

@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
 		sink, link, &source_fmt, &sink_fmt);
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_pad_config *cfg;
+
+	if (!sd->entity.num_pads)
+		return NULL;
+
+	cfg = kcalloc(sd->entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
+		return NULL;
+
+	v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+	return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)

include/media/v4l2-subdev.h +10 -0

index 8f5da73dacff..cf2ee07b0745 100644
@@ -483,6 +483,8 @@ struct v4l2_subdev_pad_config {
  *                  may be adjusted by the subdev driver to device capabilities.
  */
 struct v4l2_subdev_pad_ops {
+	void (*init_cfg)(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg);
 	int (*enum_mbus_code)(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_mbus_code_enum *code);

@@ -675,7 +677,15 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt);
 int v4l2_subdev_link_validate(struct media_link *link);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
+
+static inline void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
+{
+	kfree(cfg);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
+
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);

Laurent Pinchart

author

2015-05-25 11:20 UTC #2 lore raw

Hi Hans,

Quoted 8 lines · On Monday 25 May 2015 00:25:36 Hans Verkuil wrote:

Quoted 7 lines · Laurent Pinchart
From: Laurent Pinchart <[email protected]>
Add a new subdev operation to initialize a subdev pad config array, and a helper function to allocate and initialize the array. This can be used by bridge drivers to implement try format based on subdev pad operations.

Quoted 7 lines

Quoted 2 lines
Signed-off-by Laurent Pinchart <[email protected]>
Acked-by Vaibhav Hiremath <[email protected]>
Acked-by Hans Verkuil <[email protected]>
Note that before this goes in there should be at least one subdev driver that implements init_cfg(). Perhaps adv7604?

I fully agree, this needs to be used by at least one subdev driver.

I'd go even further, I'd like to see v4l2_subdev_alloc_pad_config() used by a bridge driver, to implement VIDIOC_TRY_FMT. I've originally written the patch to implement VIDIOC_TRY_FMT (and VIDIOC_ENUM_FRAMESIZES) in a driver I'm developing, but it's not ready for submission to mainline yet.

Do you think a subdev driver is enough, or do we need a bridge too in order to merge this patch ?

Quoted 9 lines

---

 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config

Signature · 3 lines

Regards,

Laurent Pinchart

Hans Verkuil

2015-05-25 11:41 UTC #3 lore raw

Quoted 21 lines · Laurent Pinchart

Hi Hans,
Quoted 8 lines · On Monday 25 May 2015 00:25:36 Hans Verkuil wrote:
Quoted 7 lines · Laurent Pinchart
From: Laurent Pinchart <[email protected]>
Add a new subdev operation to initialize a subdev pad config array, and a helper function to allocate and initialize the array. This can be used by bridge drivers to implement try format based on subdev pad operations.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Laurent Pinchart <[email protected]>
Acked-by Vaibhav Hiremath <[email protected]>
Acked-by Hans Verkuil <[email protected]>
Note that before this goes in there should be at least one subdev driver that implements init_cfg(). Perhaps adv7604?
I fully agree, this needs to be used by at least one subdev driver.

Quoted 7 lines

I'd go even further, I'd like to see v4l2_subdev_alloc_pad_config() used by a bridge driver, to implement VIDIOC_TRY_FMT. I've originally written the patch to implement VIDIOC_TRY_FMT (and VIDIOC_ENUM_FRAMESIZES) in a driver I'm developing, but it's not ready for submission to mainline yet.
Do you think a subdev driver is enough, or do we need a bridge too in order to merge this patch ?

Ideally a bridge as well, but the one that needs it first is soc-camera, which generally is a pain. On the other hand, it is a good non-trivial use-case.

Regards,

Hans

Quoted 11 lines

Quoted 10 lines
Quoted 10 lines
---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
Changes since v1:
- Added v4l2_subdev_free_pad_config