[libcamera-devel,11/25] media: ov5647: Implement enum_frame_size()

Message ID 20200623164224.44476-1-jacopo@jmondi.org
State Not Applicable
Delegated to: Jacopo Mondi
Headers show
Series
  • media: ov5647: Support RaspberryPi Camera Module v1
Related show

Commit Message

Jacopo Mondi June 23, 2020, 4:42 p.m. UTC
Implement the .enum_frame_size subdev pad operation.

As the driver only supports one format and one resolution at the moment
the implementation is trivial.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Sakari Ailus Aug. 18, 2020, 7:33 a.m. UTC | #1
Hi Jacopo,

On Tue, Jun 23, 2020 at 06:42:20PM +0200, Jacopo Mondi wrote:
> Implement the .enum_frame_size subdev pad operation.
> 
> As the driver only supports one format and one resolution at the moment
> the implementation is trivial.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/i2c/ov5647.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index bb9ff77f49fe0..859cc5b0d14a3 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -482,6 +482,24 @@ static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int ov5647_enum_frame_size(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_pad_config *cfg,
> +				  struct v4l2_subdev_frame_size_enum *fse)
> +{
> +	if (fse->index)
> +		return -EINVAL;
> +
> +	if (fse->code != MEDIA_BUS_FMT_SBGGR8_1X8)
> +		return -EINVAL;
> +
> +	fse->min_width = 640;
> +	fse->max_width = 640;
> +	fse->min_height = 480;
> +	fse->max_height = 480;

Could this be somehow connected to the mode used? Or at least use a macro
defined close to the mode? This seems to be a 1,3 MP sensor and the driver
only supports VGA...

> +
> +	return 0;
> +}
> +
>  static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_pad_config *cfg,
>  			      struct v4l2_subdev_format *format)
> @@ -500,9 +518,10 @@ static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
>  }
>  
>  static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
> -	.enum_mbus_code = ov5647_enum_mbus_code,
> -	.set_fmt =	  ov5647_set_get_fmt,
> -	.get_fmt =	  ov5647_set_get_fmt,
> +	.enum_mbus_code		= ov5647_enum_mbus_code,
> +	.enum_frame_size	= ov5647_enum_frame_size,
> +	.set_fmt		= ov5647_set_get_fmt,
> +	.get_fmt		= ov5647_set_get_fmt,
>  };
>  
>  static const struct v4l2_subdev_ops ov5647_subdev_ops = {
> -- 
> 2.27.0
>
Sakari Ailus Aug. 18, 2020, 7:38 a.m. UTC | #2
> Could this be somehow connected to the mode used? Or at least use a macro
> defined close to the mode? This seems to be a 1,3 MP sensor and the driver
> only supports VGA...

Please ignore the comment. I see it's actually addressed by the next patch.

Patch

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index bb9ff77f49fe0..859cc5b0d14a3 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -482,6 +482,24 @@  static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int ov5647_enum_frame_size(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_pad_config *cfg,
+				  struct v4l2_subdev_frame_size_enum *fse)
+{
+	if (fse->index)
+		return -EINVAL;
+
+	if (fse->code != MEDIA_BUS_FMT_SBGGR8_1X8)
+		return -EINVAL;
+
+	fse->min_width = 640;
+	fse->max_width = 640;
+	fse->min_height = 480;
+	fse->max_height = 480;
+
+	return 0;
+}
+
 static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_format *format)
@@ -500,9 +518,10 @@  static int ov5647_set_get_fmt(struct v4l2_subdev *sd,
 }
 
 static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
-	.enum_mbus_code = ov5647_enum_mbus_code,
-	.set_fmt =	  ov5647_set_get_fmt,
-	.get_fmt =	  ov5647_set_get_fmt,
+	.enum_mbus_code		= ov5647_enum_mbus_code,
+	.enum_frame_size	= ov5647_enum_frame_size,
+	.set_fmt		= ov5647_set_get_fmt,
+	.get_fmt		= ov5647_set_get_fmt,
 };
 
 static const struct v4l2_subdev_ops ov5647_subdev_ops = {