回复: [PATCH v5 04/14] staging: media: starfive: Add a params sink pad and a scd source pad for ISP

Changhuang Liang
2024-07-10T13:19:58Z

Hi, Jacopo

Thanks for your comments.

[...]

> > @@ -151,6 +171,9 @@ static int isp_set_format(struct v4l2_subdev *sd,
> > isp_try_format(isp_dev, state, fmt->pad, &fmt->format);
> > *format = fmt->format;
> >
> > + if (fmt->pad == STF_ISP_PAD_SRC_SCD || fmt->pad ==
> > +STF_ISP_PAD_SINK_PARAMS)
>
> nit: can easily be made < 80 cols (again, not mandatory in Linux but enforced
> in v4l according to
> Documentation/driver-api/media/maintainer-entry-profile.rst)
>

I will add --max-line-length=80 to check patchs again.

> > + return 0;
> > +
> > isp_dev->current_fmt =
> stf_g_fmt_by_mcode(&isp_dev->formats[fmt->pad],
> > fmt->format.code);
> >
> > @@ -202,6 +225,9 @@ static int isp_get_selection(struct v4l2_subdev *sd,
> > struct v4l2_subdev_format fmt = { 0 };
> > struct v4l2_rect *rect;
> >
> > + if (sel->pad == STF_ISP_PAD_SRC_SCD || sel->pad ==
> STF_ISP_PAD_SINK_PARAMS)
> > + return -EINVAL;
> > +
> > switch (sel->target) {
> > case V4L2_SEL_TGT_CROP_BOUNDS:
> > if (sel->pad == STF_ISP_PAD_SINK) { @@ -239,6 +265,9 @@ static
> int
> > isp_set_selection(struct v4l2_subdev *sd,
> > struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> > struct v4l2_rect *rect;
> >
> > + if (sel->pad == STF_ISP_PAD_SRC_SCD || sel->pad ==
> STF_ISP_PAD_SINK_PARAMS)
> > + return -EINVAL;
> > +
> > if (sel->target != V4L2_SEL_TGT_CROP)
> > return -EINVAL;
> >
> > @@ -296,8 +325,38 @@ static int isp_init_formats(struct v4l2_subdev *sd,
> > .height = 1080
> > }
> > };
> > + struct v4l2_subdev_format format_params = {
> > + .pad = STF_ISP_PAD_SINK_PARAMS,
> > + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > + .format = {
> > + .code = MEDIA_BUS_FMT_METADATA_FIXED,
> > + .width = 1,
> > + .height = 1
> > + }
> > + };
> > + struct v4l2_subdev_format format_scd = {
> > + .pad = STF_ISP_PAD_SRC_SCD,
> > + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > + .format = {
> > + .code = MEDIA_BUS_FMT_METADATA_FIXED,
> > + .width = 1,
> > + .height = 1
> > + }
> > + };
> > + int ret;
> > +
> > + /* Init for STF_ISP_PAD_SINK and STF_ISP_PAD_SRC pad */
>
> Does this initialize the format on STF_ISP_PAD_SRC for real ?
>

Yes, it will initialize the PAD_SINK and PAND_SRC formats by isp_set_selection.

> Anyway, was there already so
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>