diff mbox series

[v6,12/28] media: v4l: subdev: Add helpers for format, crop and compose pointers

Message ID 20231003120813.77726-3-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Generic line based metadata support, internal pads | expand

Commit Message

Sakari Ailus Oct. 3, 2023, 12:07 p.m. UTC
Add a helper for obtaining format, crop and compose pointers. These are
convenient for drivers, independently of the driver uses streams or not.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 54 ++++++++++++++++++++++----
 include/media/v4l2-subdev.h           | 56 +++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 8 deletions(-)

Comments

Tomi Valkeinen Oct. 5, 2023, 1:12 p.m. UTC | #1
On 03/10/2023 15:07, Sakari Ailus wrote:
> Add a helper for obtaining format, crop and compose pointers. These are
> convenient for drivers, independently of the driver uses streams or not.

If we go with these, should we deprecate 
v4l2_subdev_state_get_stream_format() and v4l2_subdev_get_pad_format()?

Having three different ways to get the fmt seems a bit excessive.

Should we add 'num_pads' to the state? That would remove the need pass 
the subdevice to these functions.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   drivers/media/v4l2-core/v4l2-subdev.c | 54 ++++++++++++++++++++++----
>   include/media/v4l2-subdev.h           | 56 +++++++++++++++++++++++++++
>   2 files changed, 102 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index d295a4e87b66..854f9d4db923 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1573,19 +1573,57 @@ v4l2_subdev_init_stream_configs(struct v4l2_subdev_stream_configs *stream_config
>   	return 0;
>   }
>   
> +struct v4l2_mbus_framefmt
> +*v4l2_subdev_get_fmt_ptr(struct v4l2_subdev *sd,
> +			 struct v4l2_subdev_state *state, unsigned int pad,
> +			 unsigned int stream)
> +{
> +	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
> +		return v4l2_subdev_state_get_stream_format(state, pad, stream);
> +
> +	if (pad < sd->entity.num_pads && stream == 0)
> +		return v4l2_subdev_get_pad_format(sd, state, pad);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt_ptr);
> +
> +struct v4l2_rect
> +*v4l2_subdev_get_crop_ptr(struct v4l2_subdev *sd,
> +			  struct v4l2_subdev_state *state, unsigned int pad,
> +			  unsigned int stream)
> +{
> +	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
> +		return v4l2_subdev_state_get_stream_crop(state, pad, stream);
> +
> +	if (pad < sd->entity.num_pads && stream == 0)
> +		return v4l2_subdev_get_pad_crop(sd, state, pad);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_crop_ptr);
> +
> +struct v4l2_rect
> +*v4l2_subdev_get_compose_ptr(struct v4l2_subdev *sd,
> +			     struct v4l2_subdev_state *state, unsigned int pad,
> +			     unsigned int stream)
> +{
> +	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
> +		return v4l2_subdev_state_get_stream_compose(state, pad, stream);
> +
> +	if (pad < sd->entity.num_pads && stream == 0)
> +		return v4l2_subdev_get_pad_compose(sd, state, pad);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_compose_ptr);
> +
>   int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
>   			struct v4l2_subdev_format *format)
>   {
>   	struct v4l2_mbus_framefmt *fmt;
>   
> -	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
> -		fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
> -							  format->stream);
> -	else if (format->pad < sd->entity.num_pads && format->stream == 0)
> -		fmt = v4l2_subdev_get_pad_format(sd, state, format->pad);
> -	else
> -		fmt = NULL;
> -
> +	fmt = v4l2_subdev_get_fmt_ptr(sd, state, format->pad, format->stream);
>   	if (!fmt)
>   		return -EINVAL;
>   
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 5f59ff0796b7..7c34243ffed9 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1479,6 +1479,62 @@ v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
>   
>   #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>   
> +/**
> + * v4l2_subdev_get_fmt_ptr - Obtain a pointer to V4L2 sub-device format for pad
> + *			     and stream
> + * @sd: subdevice
> + * @state: subdevice state
> + * @pad: the pad on the sub-device
> + * @stream: stream in the pad
> + *
> + * For given pad and stream, obtain a pointer to the mbus format from the
> + * sub-device.
> + *
> + * Returns NULL if the format is not found or the parameters are invalid.
> + */
> +struct v4l2_mbus_framefmt *
> +v4l2_subdev_get_fmt_ptr(struct v4l2_subdev *sd,
> +			struct v4l2_subdev_state *state, unsigned int pad,
> +			unsigned int stream);
> +
> +/**
> + * v4l2_subdev_get_crop_ptr - Obtain a pointer to V4L2 sub-device crop
> + *			      rectangle for pad and stream
> + * @sd: subdevice
> + * @state: subdevice state
> + * @pad: the pad on the sub-device
> + * @stream: stream in the pad
> + *
> + * For given pad and stream, obtain a pointer to the crop selection rectangle
> + * from the sub-device.
> + *
> + * Returns NULL if the selection rectangle is not found or the parameters are
> + * invalid.
> + */
> +struct v4l2_rect *
> +v4l2_subdev_get_crop_ptr(struct v4l2_subdev *sd,
> +			 struct v4l2_subdev_state *state, unsigned int pad,
> +			 unsigned int stream);
> +
> +/**
> + * v4l2_subdev_get_compose_ptr - Obtain a pointer to V4L2 sub-device compose
> + *				 rectangle for pad and stream
> + * @sd: subdevice
> + * @state: subdevice state
> + * @pad: the pad on the sub-device
> + * @stream: stream in the pad
> + *
> + * For given pad and stream, obtain a pointer to the compose selection rectangle
> + * from the sub-device.
> + *
> + * Returns NULL if the selection rectangle is not found or the parameters are
> + * invalid.
> + */
> +struct v4l2_rect *
> +v4l2_subdev_get_compose_ptr(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *state, unsigned int pad,
> +			    unsigned int stream);
> +
>   /**
>    * v4l2_subdev_get_fmt() - Fill format based on state
>    * @sd: subdevice
Sakari Ailus Oct. 13, 2023, 6:05 a.m. UTC | #2
Moi,

On Thu, Oct 05, 2023 at 04:12:00PM +0300, Tomi Valkeinen wrote:
> On 03/10/2023 15:07, Sakari Ailus wrote:
> > Add a helper for obtaining format, crop and compose pointers. These are
> > convenient for drivers, independently of the driver uses streams or not.
> 
> If we go with these, should we deprecate
> v4l2_subdev_state_get_stream_format() and v4l2_subdev_get_pad_format()?
> 
> Having three different ways to get the fmt seems a bit excessive.
> 
> Should we add 'num_pads' to the state? That would remove the need pass the
> subdevice to these functions.

Good question. This would make it easier to refactor the code later,
drivers would still need to begin calling v4l2_subdev_init_finalize(). But
it's one step forward I think.

I'll add this earlier in the set.
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index d295a4e87b66..854f9d4db923 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1573,19 +1573,57 @@  v4l2_subdev_init_stream_configs(struct v4l2_subdev_stream_configs *stream_config
 	return 0;
 }
 
+struct v4l2_mbus_framefmt
+*v4l2_subdev_get_fmt_ptr(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_state *state, unsigned int pad,
+			 unsigned int stream)
+{
+	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
+		return v4l2_subdev_state_get_stream_format(state, pad, stream);
+
+	if (pad < sd->entity.num_pads && stream == 0)
+		return v4l2_subdev_get_pad_format(sd, state, pad);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_fmt_ptr);
+
+struct v4l2_rect
+*v4l2_subdev_get_crop_ptr(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_state *state, unsigned int pad,
+			  unsigned int stream)
+{
+	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
+		return v4l2_subdev_state_get_stream_crop(state, pad, stream);
+
+	if (pad < sd->entity.num_pads && stream == 0)
+		return v4l2_subdev_get_pad_crop(sd, state, pad);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_crop_ptr);
+
+struct v4l2_rect
+*v4l2_subdev_get_compose_ptr(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_state *state, unsigned int pad,
+			     unsigned int stream)
+{
+	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
+		return v4l2_subdev_state_get_stream_compose(state, pad, stream);
+
+	if (pad < sd->entity.num_pads && stream == 0)
+		return v4l2_subdev_get_pad_compose(sd, state, pad);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_compose_ptr);
+
 int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
 			struct v4l2_subdev_format *format)
 {
 	struct v4l2_mbus_framefmt *fmt;
 
-	if (sd->flags & V4L2_SUBDEV_FL_STREAMS)
-		fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
-							  format->stream);
-	else if (format->pad < sd->entity.num_pads && format->stream == 0)
-		fmt = v4l2_subdev_get_pad_format(sd, state, format->pad);
-	else
-		fmt = NULL;
-
+	fmt = v4l2_subdev_get_fmt_ptr(sd, state, format->pad, format->stream);
 	if (!fmt)
 		return -EINVAL;
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 5f59ff0796b7..7c34243ffed9 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1479,6 +1479,62 @@  v4l2_subdev_lock_and_get_active_state(struct v4l2_subdev *sd)
 
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 
+/**
+ * v4l2_subdev_get_fmt_ptr - Obtain a pointer to V4L2 sub-device format for pad
+ *			     and stream
+ * @sd: subdevice
+ * @state: subdevice state
+ * @pad: the pad on the sub-device
+ * @stream: stream in the pad
+ *
+ * For given pad and stream, obtain a pointer to the mbus format from the
+ * sub-device.
+ *
+ * Returns NULL if the format is not found or the parameters are invalid.
+ */
+struct v4l2_mbus_framefmt *
+v4l2_subdev_get_fmt_ptr(struct v4l2_subdev *sd,
+			struct v4l2_subdev_state *state, unsigned int pad,
+			unsigned int stream);
+
+/**
+ * v4l2_subdev_get_crop_ptr - Obtain a pointer to V4L2 sub-device crop
+ *			      rectangle for pad and stream
+ * @sd: subdevice
+ * @state: subdevice state
+ * @pad: the pad on the sub-device
+ * @stream: stream in the pad
+ *
+ * For given pad and stream, obtain a pointer to the crop selection rectangle
+ * from the sub-device.
+ *
+ * Returns NULL if the selection rectangle is not found or the parameters are
+ * invalid.
+ */
+struct v4l2_rect *
+v4l2_subdev_get_crop_ptr(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_state *state, unsigned int pad,
+			 unsigned int stream);
+
+/**
+ * v4l2_subdev_get_compose_ptr - Obtain a pointer to V4L2 sub-device compose
+ *				 rectangle for pad and stream
+ * @sd: subdevice
+ * @state: subdevice state
+ * @pad: the pad on the sub-device
+ * @stream: stream in the pad
+ *
+ * For given pad and stream, obtain a pointer to the compose selection rectangle
+ * from the sub-device.
+ *
+ * Returns NULL if the selection rectangle is not found or the parameters are
+ * invalid.
+ */
+struct v4l2_rect *
+v4l2_subdev_get_compose_ptr(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *state, unsigned int pad,
+			    unsigned int stream);
+
 /**
  * v4l2_subdev_get_fmt() - Fill format based on state
  * @sd: subdevice