| Message ID | 20230123125205.622152-26-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: atomisp: Big power-management changes + lots of fixes | expand |
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 >
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 */
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. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 3 --- 1 file changed, 3 deletions(-)