| Message ID | 20241030163439.245035-1-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: imx283: Report correct V4L2_SEL_TGT_CROP | expand |
Hi Stefan On 30/10/24 10:04 pm, Stefan Klug wrote: > The target crop rectangle is initialized with the crop of the default > sensor mode. This is incorrect when a different sensor mode gets > selected. Fix that by updating the crop rectangle when changing the > sensor mode. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > drivers/media/i2c/imx283.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c > index 3174d5ffd2d7..c8863c9e0ccf 100644 > --- a/drivers/media/i2c/imx283.c > +++ b/drivers/media/i2c/imx283.c > @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, > struct v4l2_subdev_state *sd_state, > struct v4l2_subdev_format *fmt) > { > + struct v4l2_rect *crop; > struct v4l2_mbus_framefmt *format; > const struct imx283_mode *mode; > struct imx283 *imx283 = to_imx283(sd); > @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, > > *format = fmt->format; > > + crop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD); > + *crop = mode->crop; > + One thing to note, is the crop for binning modes. Do you need to report mode->crop.width / mode->hbin_ratio mode->crop.height / mode->vbin_ratio for those modes? > return 0; > } >
Hello, On Thu, Oct 31, 2024 at 10:13:13AM +0100, Stefan Klug wrote: > On Thu, Oct 31, 2024 at 11:07:00AM +0530, Umang Jain wrote: > > On 30/10/24 10:04 pm, Stefan Klug wrote: > > > The target crop rectangle is initialized with the crop of the default > > > sensor mode. This is incorrect when a different sensor mode gets > > > selected. Fix that by updating the crop rectangle when changing the > > > sensor mode. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > > drivers/media/i2c/imx283.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c > > > index 3174d5ffd2d7..c8863c9e0ccf 100644 > > > --- a/drivers/media/i2c/imx283.c > > > +++ b/drivers/media/i2c/imx283.c > > > @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, > > > struct v4l2_subdev_state *sd_state, > > > struct v4l2_subdev_format *fmt) > > > { > > > + struct v4l2_rect *crop; > > > struct v4l2_mbus_framefmt *format; > > > const struct imx283_mode *mode; > > > struct imx283 *imx283 = to_imx283(sd); > > > @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, > > > *format = fmt->format; > > > + crop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD); > > > + *crop = mode->crop; > > > + > > > > One thing to note, is the crop for binning modes. > > > > Do you need to report > > > > mode->crop.width / mode->hbin_ratio > > mode->crop.height / mode->vbin_ratio > > > > for those modes? > > Good point. I was naively assuming that it has the same semantics as we > use for ScalerCrop in libcamera where it is explicitly stated that the > coordinates are in sensor pixels without binning. That has the added > advantage that we can deduce the binning factor from TGT_CROP and the > actual output size. However I couldn't find a precise specification for > that in the linux docs. > > Maybe Sakari or Laurent have a definiteve answer there? This is not standardized in V4L2, and different drivers implement different semantics. There's an ongoing effort to fix this, see https://lore.kernel.org/r/20241011075535.588140-1-sakari.ailus@linux.intel.com. Reviews are appreciated :-) > > > return 0; > > > }
diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c index 3174d5ffd2d7..c8863c9e0ccf 100644 --- a/drivers/media/i2c/imx283.c +++ b/drivers/media/i2c/imx283.c @@ -1123,6 +1123,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_format *fmt) { + struct v4l2_rect *crop; struct v4l2_mbus_framefmt *format; const struct imx283_mode *mode; struct imx283 *imx283 = to_imx283(sd); @@ -1149,6 +1150,9 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd, *format = fmt->format; + crop = v4l2_subdev_state_get_crop(sd_state, IMAGE_PAD); + *crop = mode->crop; + return 0; }
The target crop rectangle is initialized with the crop of the default sensor mode. This is incorrect when a different sensor mode gets selected. Fix that by updating the crop rectangle when changing the sensor mode. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- drivers/media/i2c/imx283.c | 4 ++++ 1 file changed, 4 insertions(+)