| Message ID | 20220223145603.121603-19-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov5640: Rework the clock tree programming for MIPI | expand |
Hi Jacopo, On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote: > Implement the init_cfg pad operation to initialize the subdev state > format to the default one. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 6c2c76b61140..c1fc2ab1e0cf 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor) > * over i2c. > */ > /* YUV422 UYVY VGA@30fps */ > + > +static const struct v4l2_mbus_framefmt ov5640_default_fmt = { > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > + .width = 640, > + .height = 480, > + .colorspace = V4L2_COLORSPACE_SRGB, > + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB), > + .quantization = V4L2_QUANTIZATION_FULL_RANGE, > + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB), > + .field = V4L2_FIELD_NONE, > +}; > + > static const struct reg_value ov5640_init_setting[] = { > {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0}, > {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0}, > @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable) > return ret; > } > > +static int ov5640_init_cfg(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state) > +{ > + struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0); Could you run $ ./scripts/checkpatch.pl --strict --max-line-length=80 on the entire set, and see what else is trivial to fix? > + > + *fmt = ov5640_default_fmt; > + > + return 0; > +} > + > static const struct v4l2_subdev_core_ops ov5640_core_ops = { > .s_power = ov5640_s_power, > .log_status = v4l2_ctrl_subdev_log_status,
Hi Sakari, On Wed, Feb 23, 2022 at 10:25:14PM +0200, Sakari Ailus wrote: > Hi Jacopo, > > On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote: > > Implement the init_cfg pad operation to initialize the subdev state > > format to the default one. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++---------- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > index 6c2c76b61140..c1fc2ab1e0cf 100644 > > --- a/drivers/media/i2c/ov5640.c > > +++ b/drivers/media/i2c/ov5640.c > > @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor) > > * over i2c. > > */ > > /* YUV422 UYVY VGA@30fps */ > > + > > +static const struct v4l2_mbus_framefmt ov5640_default_fmt = { > > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > > + .width = 640, > > + .height = 480, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB), > > + .quantization = V4L2_QUANTIZATION_FULL_RANGE, > > + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB), > > + .field = V4L2_FIELD_NONE, > > +}; > > + > > static const struct reg_value ov5640_init_setting[] = { > > {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0}, > > {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0}, > > @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable) > > return ret; > > } > > > > +static int ov5640_init_cfg(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state) > > +{ > > + struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0); > > Could you run > > $ ./scripts/checkpatch.pl --strict --max-line-length=80 > > on the entire set, and see what else is trivial to fix? > I was asked the same by Laurent, and I was honestly surprised as I thought from our last discussions that 100 cols limit was encouraged. Oh well, I suspect that' the price for having dropped the ball with "[PATCH] media: Document coding style requirements" I'll check with 80 cols limits. Thanks j > > + > > + *fmt = ov5640_default_fmt; > > + > > + return 0; > > +} > > + > > static const struct v4l2_subdev_core_ops ov5640_core_ops = { > > .s_power = ov5640_s_power, > > .log_status = v4l2_ctrl_subdev_log_status, > > -- > Regards, > > Sakari Ailus
Hi Jacopo, On Thu, Feb 24, 2022 at 09:08:34AM +0100, Jacopo Mondi wrote: > Hi Sakari, > > On Wed, Feb 23, 2022 at 10:25:14PM +0200, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote: > > > Implement the init_cfg pad operation to initialize the subdev state > > > format to the default one. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++---------- > > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > > index 6c2c76b61140..c1fc2ab1e0cf 100644 > > > --- a/drivers/media/i2c/ov5640.c > > > +++ b/drivers/media/i2c/ov5640.c > > > @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor) > > > * over i2c. > > > */ > > > /* YUV422 UYVY VGA@30fps */ > > > + > > > +static const struct v4l2_mbus_framefmt ov5640_default_fmt = { > > > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > > > + .width = 640, > > > + .height = 480, > > > + .colorspace = V4L2_COLORSPACE_SRGB, > > > + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB), > > > + .quantization = V4L2_QUANTIZATION_FULL_RANGE, > > > + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB), > > > + .field = V4L2_FIELD_NONE, > > > +}; > > > + > > > static const struct reg_value ov5640_init_setting[] = { > > > {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0}, > > > {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0}, > > > @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable) > > > return ret; > > > } > > > > > > +static int ov5640_init_cfg(struct v4l2_subdev *sd, > > > + struct v4l2_subdev_state *state) > > > +{ > > > + struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0); > > > > Could you run > > > > $ ./scripts/checkpatch.pl --strict --max-line-length=80 > > > > on the entire set, and see what else is trivial to fix? > > > > I was asked the same by Laurent, and I was honestly surprised as I > thought from our last discussions that 100 cols limit was encouraged. At least I didn't draw that conclusion from the discussion. It was rather that people had somewhat differing opinions what else should be documented besides what's there right now. > > Oh well, I suspect that' the price for having dropped the ball with > "[PATCH] media: Document coding style requirements" > > I'll check with 80 cols limits. Thank you.
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 6c2c76b61140..c1fc2ab1e0cf 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor) * over i2c. */ /* YUV422 UYVY VGA@30fps */ + +static const struct v4l2_mbus_framefmt ov5640_default_fmt = { + .code = MEDIA_BUS_FMT_UYVY8_2X8, + .width = 640, + .height = 480, + .colorspace = V4L2_COLORSPACE_SRGB, + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB), + .quantization = V4L2_QUANTIZATION_FULL_RANGE, + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB), + .field = V4L2_FIELD_NONE, +}; + static const struct reg_value ov5640_init_setting[] = { {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0}, {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0}, @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable) return ret; } +static int ov5640_init_cfg(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state) +{ + struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0); + + *fmt = ov5640_default_fmt; + + return 0; +} + static const struct v4l2_subdev_core_ops ov5640_core_ops = { .s_power = ov5640_s_power, .log_status = v4l2_ctrl_subdev_log_status, @@ -3529,6 +3551,7 @@ static const struct v4l2_subdev_video_ops ov5640_video_ops = { }; static const struct v4l2_subdev_pad_ops ov5640_pad_ops = { + .init_cfg = ov5640_init_cfg, .enum_mbus_code = ov5640_enum_mbus_code, .get_fmt = ov5640_get_fmt, .set_fmt = ov5640_set_fmt, @@ -3587,7 +3610,6 @@ static int ov5640_probe(struct i2c_client *client) struct device *dev = &client->dev; struct fwnode_handle *endpoint; struct ov5640_dev *sensor; - struct v4l2_mbus_framefmt *fmt; u32 rotation; int ret; @@ -3601,15 +3623,7 @@ static int ov5640_probe(struct i2c_client *client) * default init sequence initialize sensor to * YUV422 UYVY VGA@30fps */ - fmt = &sensor->fmt; - fmt->code = MEDIA_BUS_FMT_UYVY8_2X8; - fmt->colorspace = V4L2_COLORSPACE_SRGB; - fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace); - fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; - fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace); - fmt->width = 640; - fmt->height = 480; - fmt->field = V4L2_FIELD_NONE; + sensor->fmt = ov5640_default_fmt; sensor->frame_interval.numerator = 1; sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS]; sensor->current_fr = OV5640_30_FPS;