On 10/25/23 02:34, Laurent Pinchart wrote:
The isc_try_fse() function, called from isc_validate(), takes two
parameters, an isc_device pointer, and a v4l2_subdev_state pointer. The
isc_device is accessed but not modified by the function. The state is
modified, including the struct v4l2_subdev_pad_config array it points
to, but they are then never used by the caller. Furthermore, the V4L2
subdev operation called by isc_try_fse() doesn't modify the subdev it is
called on. The isc_try_fse() function has thus no effect, and can just
be dropped.
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
This patch has been compile-tested only. Eugen, would you be able to
test it on hardware ?
Hello Laurent,
Thank you for the patch.
When I initially wrote this code, I definitely had something in mind. I
think it was the fact that the ISC should adjust it's crop area
according to what the subdev is emitting, to avoid other problems (e.g.
reporting bad frame size and v4l2-compliance crying )
Somehow , maybe, that intention is lost somewhere. I agree with you that
this code appears useless.
I will test it out and let you know . (I cannot promise exactly when I
have some time to do it, so bear with me please...)
Eugen
---
.../platform/microchip/microchip-isc-base.c | 39 -------------------
1 file changed, 39 deletions(-)
diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 1f8528844497..540cb1378289 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -851,38 +851,6 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
return 0;
}
-static void isc_try_fse(struct isc_device *isc,
- struct v4l2_subdev_state *sd_state)
-{
- struct v4l2_subdev_frame_size_enum fse = {
- .which = V4L2_SUBDEV_FORMAT_TRY,
- };
- int ret;
-
- /*
- * If we do not know yet which format the subdev is using, we cannot
- * do anything.
- */
- if (!isc->config.sd_format)
- return;
-
- fse.code = isc->try_config.sd_format->mbus_code;
-
- ret = v4l2_subdev_call(isc->current_subdev->sd, pad, enum_frame_size,
- sd_state, &fse);
- /*
- * Attempt to obtain format size from subdev. If not available,
- * just use the maximum ISC can receive.
- */
- if (ret) {
- sd_state->pads->try_crop.width = isc->max_width;
- sd_state->pads->try_crop.height = isc->max_height;
- } else {
- sd_state->pads->try_crop.width = fse.max_width;
- sd_state->pads->try_crop.height = fse.max_height;
- }
-}
-
static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
{
struct v4l2_pix_format *pixfmt = &f->fmt.pix;
@@ -944,10 +912,6 @@ static int isc_validate(struct isc_device *isc)
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
.pad = isc->remote_pad,
};
- struct v4l2_subdev_pad_config pad_cfg = {};
- struct v4l2_subdev_state pad_state = {
- .pads = &pad_cfg,
- };
/* Get current format from subdev */
ret = v4l2_subdev_call(isc->current_subdev->sd, pad, get_fmt, NULL,
@@ -1008,9 +972,6 @@ static int isc_validate(struct isc_device *isc)
if (ret)
return ret;
- /* Obtain frame sizes if possible to have crop requirements ready */
- isc_try_fse(isc, &pad_state);
-
/* Configure ISC pipeline for the config */
ret = isc_try_configure_pipeline(isc);
if (ret)
