diff mbox

[06/47] v4l: Add pad-level DV timings subdev operations

Message ID 1391618558-5580-7-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Pinchart Feb. 5, 2014, 4:41 p.m. UTC
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/v4l2-subdev.h    | 4 ++++
 include/uapi/linux/videodev2.h | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Hans Verkuil Feb. 5, 2014, 5:13 p.m. UTC | #1
On 02/05/2014 05:41 PM, Laurent Pinchart wrote:
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/media/v4l2-subdev.h    | 4 ++++
>  include/uapi/linux/videodev2.h | 8 ++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d67210a..2c7355a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -505,6 +505,10 @@ struct v4l2_subdev_pad_ops {
>  			     struct v4l2_subdev_selection *sel);
>  	int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
>  	int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
> +	int (*dv_timings_cap)(struct v4l2_subdev *sd,
> +			      struct v4l2_dv_timings_cap *cap);
> +	int (*enum_dv_timings)(struct v4l2_subdev *sd,
> +			       struct v4l2_enum_dv_timings *timings);
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	int (*link_validate)(struct v4l2_subdev *sd, struct media_link *link,
>  			     struct v4l2_subdev_format *source_fmt,
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 6ae7bbe..b75970d 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1086,12 +1086,14 @@ struct v4l2_dv_timings {
>  
>  /** struct v4l2_enum_dv_timings - DV timings enumeration
>   * @index:	enumeration index
> + * @pad:	the pad number for which to query capabilities

Add something like: "pad is only used with v4l-subdev nodes."

>   * @reserved:	must be zeroed
>   * @timings:	the timings for the given index
>   */
>  struct v4l2_enum_dv_timings {
>  	__u32 index;
> -	__u32 reserved[3];
> +	__u32 pad;
> +	__u32 reserved[2];
>  	struct v4l2_dv_timings timings;
>  };
>  
> @@ -1129,11 +1131,13 @@ struct v4l2_bt_timings_cap {
>  
>  /** struct v4l2_dv_timings_cap - DV timings capabilities
>   * @type:	the type of the timings (same as in struct v4l2_dv_timings)
> + * @pad:	the pad number for which to query capabilities

Ditto.

>   * @bt:		the BT656/1120 timings capabilities
>   */
>  struct v4l2_dv_timings_cap {
>  	__u32 type;
> -	__u32 reserved[3];
> +	__u32 pad;
> +	__u32 reserved[2];
>  	union {
>  		struct v4l2_bt_timings_cap bt;
>  		__u32 raw_data[32];
> 

See also my comments for patch 27.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Feb. 6, 2014, 5:33 p.m. UTC | #2
Hi Laurent,

On Wed, Feb 05, 2014 at 05:41:57PM +0100, Laurent Pinchart wrote:
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/media/v4l2-subdev.h    | 4 ++++
>  include/uapi/linux/videodev2.h | 8 ++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d67210a..2c7355a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -505,6 +505,10 @@ struct v4l2_subdev_pad_ops {
>  			     struct v4l2_subdev_selection *sel);
>  	int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
>  	int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
> +	int (*dv_timings_cap)(struct v4l2_subdev *sd,
> +			      struct v4l2_dv_timings_cap *cap);
> +	int (*enum_dv_timings)(struct v4l2_subdev *sd,
> +			       struct v4l2_enum_dv_timings *timings);

Do you think there would be use for these in the user space API? The
argument structs are defined in the user space header. The driver does also
export a sub-device node.
Laurent Pinchart Feb. 7, 2014, 12:50 a.m. UTC | #3
Hi Sakari,

Thank you for the review.

On Thursday 06 February 2014 19:33:23 Sakari Ailus wrote:
> Hi Laurent,
> 
> On Wed, Feb 05, 2014 at 05:41:57PM +0100, Laurent Pinchart wrote:
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  include/media/v4l2-subdev.h    | 4 ++++
> >  include/uapi/linux/videodev2.h | 8 ++++++--
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index d67210a..2c7355a 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -505,6 +505,10 @@ struct v4l2_subdev_pad_ops {
> > 
> >  			     struct v4l2_subdev_selection *sel);
> >  	
> >  	int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid 
*edid);
> >  	int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid 
*edid);
> > 
> > +	int (*dv_timings_cap)(struct v4l2_subdev *sd,
> > +			      struct v4l2_dv_timings_cap *cap);
> > +	int (*enum_dv_timings)(struct v4l2_subdev *sd,
> > +			       struct v4l2_enum_dv_timings *timings);
> 
> Do you think there would be use for these in the user space API? The
> argument structs are defined in the user space header. The driver does also
> export a sub-device node.

Please have a look at

[PATCH 27/47] v4l: Add support for DV timings ioctls on subdev nodes

:-)
diff mbox

Patch

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d67210a..2c7355a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -505,6 +505,10 @@  struct v4l2_subdev_pad_ops {
 			     struct v4l2_subdev_selection *sel);
 	int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
 	int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid);
+	int (*dv_timings_cap)(struct v4l2_subdev *sd,
+			      struct v4l2_dv_timings_cap *cap);
+	int (*enum_dv_timings)(struct v4l2_subdev *sd,
+			       struct v4l2_enum_dv_timings *timings);
 #ifdef CONFIG_MEDIA_CONTROLLER
 	int (*link_validate)(struct v4l2_subdev *sd, struct media_link *link,
 			     struct v4l2_subdev_format *source_fmt,
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 6ae7bbe..b75970d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1086,12 +1086,14 @@  struct v4l2_dv_timings {
 
 /** struct v4l2_enum_dv_timings - DV timings enumeration
  * @index:	enumeration index
+ * @pad:	the pad number for which to query capabilities
  * @reserved:	must be zeroed
  * @timings:	the timings for the given index
  */
 struct v4l2_enum_dv_timings {
 	__u32 index;
-	__u32 reserved[3];
+	__u32 pad;
+	__u32 reserved[2];
 	struct v4l2_dv_timings timings;
 };
 
@@ -1129,11 +1131,13 @@  struct v4l2_bt_timings_cap {
 
 /** struct v4l2_dv_timings_cap - DV timings capabilities
  * @type:	the type of the timings (same as in struct v4l2_dv_timings)
+ * @pad:	the pad number for which to query capabilities
  * @bt:		the BT656/1120 timings capabilities
  */
 struct v4l2_dv_timings_cap {
 	__u32 type;
-	__u32 reserved[3];
+	__u32 pad;
+	__u32 reserved[2];
 	union {
 		struct v4l2_bt_timings_cap bt;
 		__u32 raw_data[32];