Re: [PATCH 25/57] media: atomisp: Stop overriding padding w/h to 12 on BYT

Andy Shevchenko
2023-01-23T17:59:42Z

On Mon, Jan 23, 2023 at 01:51:33PM +0100, Hans de Goede wrote:
> atomisp_set_fmt() first does:
>
> v4l2_fill_mbus_format(&vformat.format, ...);
> vformat.format.height += padding_h;
> vformat.format.width += padding_w;
>
> ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, pad,
> set_fmt, NULL, &vformat);
> if (ret)
> return ret;
>
> f->fmt.pix.width = vformat.format.width - padding_w;
> f->fmt.pix.height = vformat.format.height - padding_h;
>
> this happens with the original padding w/h = 16 values and then later
> on it calls:
>
> ret = atomisp_set_fmt_to_snr(vdev, &s_fmt,
> f->fmt.pix.pixelformat, padding_w,
> padding_h, dvs_env_w, dvs_env_h);
>
> Which repeats the above structure. If at that point padding w/h are
> changed to 12 then it will now request a different output-size of
> the sensor driver.
>
> The sensor drivers so far have actually been ignoring this since they use
> v4l2_find_nearest_size() on a fixed resolution list and the nearest
> resolution will be the one from the earlier calls where padding w/h
> was 16.
>
> But there really is no reason for sensor drivers to use a fixed
> resolution list. They make lower resolutions using cropping so they
> can make any resolution as long as width/height are even numbers.
>
> Dropping the fixed-resolution list limit from sensors on BYT results
> in trying to start streaming failing because the resolution set to
> the sensor now no longer matches with the resolution used during
> the initial part of the configuration done by atomisp_set_fmt().
>
> Drop the BYT specific overriding of the padding_w/h to 12, so that
> the padding in the first and second s_fmt calls made to the sensor
> matches, to fix stream start failing when the fixed resolution list
> is dropped.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_cmd.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> index eb05288d8fb1..47f18ac5e40e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> @@ -5163,9 +5163,6 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
> if (!atomisp_subdev_format_conversion(asd, source_pad)) {
> padding_w = 0;
> padding_h = 0;
> - } else if (IS_BYT) {
> - padding_w = 12;
> - padding_h = 12;
> }
>
> /* construct resolution supported by isp */
> --
> 2.39.0
>

--
With Best Regards,
Andy Shevchenko