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

[Changhuang Liang]

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@xxxxxxxxxxxxxxxx>
> 
> Thanks
>   j
>