Hi Niklas,
On Friday, 19 January 2018 02:46:03 EET Niklas Söderlund wrote:
> Hi Laurent,
>
> Thanks for your comments.
>
> Apart from the issue with the input API Hans pointed which indicates I
> need to keep that around until it's fixed in the framework I agree with
> all your comments but one.
>
> <snip>
>
> On 2017-12-08 12:14:05 +0200, Laurent Pinchart wrote:
> >> @@ -628,7 +628,8 @@ static int rvin_setup(struct rvin_dev *vin)
> >> /* Default to TB */
> >> vnmc = VNMC_IM_FULL;
> >> /* Use BT if video standard can be read and is 60 Hz format */
> >> - if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
> >> + if (!vin->info->use_mc &&
> >> + !v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
> >> if (std & V4L2_STD_525_60)
> >> vnmc = VNMC_IM_FULL | VNMC_FOC;
> >> }
> >
> > I think the subdev should be queried in rcar-v4l2.c and the information
> > cached in the rvin_dev structure instead of queried here. Interactions
> > with the subdev should be minimized in rvin-dma.c. You can fix this in a
> > separate patch if you prefer.
>
> This can't be a cached value it needs to be read at stream on time. The
> input source could have changed format, e.g. the user may have
> disconnected a NTSC source and plugged in a PAL.
Please note that in that case the source subdev will not have changed its
format yet, it only does so when the s_std operation is called.
> This is a shame as I otherwise agree with you that interactions with the
> subdevice should be kept at a minimum.
>
> <snip>
--
Regards,
Laurent Pinchart
