| Message ID | 20240111114831.656736-5-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,01/11] media: rkisp1: Add and use rkisp1_has_feature() macro | expand |
Hi Paul, Thank you for the patch. On Thu, Jan 11, 2024 at 08:48:24PM +0900, Paul Elder wrote: > Some versions of the ISP supported by the rkisp1 driver, such as the ISP > in the i.MX8MP, lack the dual crop registers and don't support cropping > at the resizer input. They instead rely on cropping in the Image > Stabilization module, at the output of the ISP, to modify the resizer > input size and implement digital zoom. > > Support those ISP versions by addind a dual crop feature flag, and s/addind/adding/ > mapping the resizer input crop rectangle to either the resizer dual crop > module or the image stabilization module. The implementation doesn't match the commit message. You only disable dual-crop here, you don't handle the image stabilization module. I'm fine with that, I think adding IS support belongs to a separate patch, but what this patch is missing is disabling the ability to set a crop rectangle on the resizer input when dual-crop is not supported. > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> > Tested-by: Adam Ford <aford173@gmail.com> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/platform/rockchip/rkisp1/rkisp1-common.h | 2 ++ > drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 6 ++++-- > drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c | 6 ++++-- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index f7c251f79aa9..219d4a2547aa 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -112,6 +112,7 @@ enum rkisp1_isp_pad { > * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver > * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path > * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path > + * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input > * > * The ISP features are stored in a bitmask in &rkisp1_info.features and allow > * the driver to implement support for features present in some ISP versions > @@ -121,6 +122,7 @@ enum rkisp1_feature { > RKISP1_FEATURE_MIPI_CSI2 = BIT(0), > RKISP1_FEATURE_MAIN_STRIDE = BIT(1), > RKISP1_FEATURE_SELF_PATH = BIT(2), > + RKISP1_FEATURE_DUAL_CROP = BIT(3), > }; > > #define rkisp1_has_feature(rkisp1, feature) \ > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > index f48a21985b18..2e40c376cab5 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > @@ -507,7 +507,8 @@ static const struct rkisp1_info px30_isp_info = { > .isr_size = ARRAY_SIZE(px30_isp_isrs), > .isp_ver = RKISP1_V12, > .features = RKISP1_FEATURE_MIPI_CSI2 > - | RKISP1_FEATURE_SELF_PATH, > + | RKISP1_FEATURE_SELF_PATH > + | RKISP1_FEATURE_DUAL_CROP, > }; > > static const char * const rk3399_isp_clks[] = { > @@ -527,7 +528,8 @@ static const struct rkisp1_info rk3399_isp_info = { > .isr_size = ARRAY_SIZE(rk3399_isp_isrs), > .isp_ver = RKISP1_V10, > .features = RKISP1_FEATURE_MIPI_CSI2 > - | RKISP1_FEATURE_SELF_PATH, > + | RKISP1_FEATURE_SELF_PATH > + | RKISP1_FEATURE_DUAL_CROP, > }; > > static const struct of_device_id rkisp1_of_match[] = { > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > index dd77a31e6014..de2eb2c97cc4 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > @@ -635,7 +635,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable) > struct v4l2_subdev_state *sd_state; > > if (!enable) { > - rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); > + if (rkisp1_has_feature(rkisp1, DUAL_CROP)) > + rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); > rkisp1_rsz_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); > return 0; > } > @@ -646,7 +647,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable) > sd_state = v4l2_subdev_lock_and_get_active_state(sd); > > rkisp1_rsz_config(rsz, sd_state, when); > - rkisp1_dcrop_config(rsz, sd_state); > + if (rkisp1_has_feature(rkisp1, DUAL_CROP)) > + rkisp1_dcrop_config(rsz, sd_state); > > v4l2_subdev_unlock_state(sd_state); >
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index f7c251f79aa9..219d4a2547aa 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h @@ -112,6 +112,7 @@ enum rkisp1_isp_pad { * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path + * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input * * The ISP features are stored in a bitmask in &rkisp1_info.features and allow * the driver to implement support for features present in some ISP versions @@ -121,6 +122,7 @@ enum rkisp1_feature { RKISP1_FEATURE_MIPI_CSI2 = BIT(0), RKISP1_FEATURE_MAIN_STRIDE = BIT(1), RKISP1_FEATURE_SELF_PATH = BIT(2), + RKISP1_FEATURE_DUAL_CROP = BIT(3), }; #define rkisp1_has_feature(rkisp1, feature) \ diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c index f48a21985b18..2e40c376cab5 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c @@ -507,7 +507,8 @@ static const struct rkisp1_info px30_isp_info = { .isr_size = ARRAY_SIZE(px30_isp_isrs), .isp_ver = RKISP1_V12, .features = RKISP1_FEATURE_MIPI_CSI2 - | RKISP1_FEATURE_SELF_PATH, + | RKISP1_FEATURE_SELF_PATH + | RKISP1_FEATURE_DUAL_CROP, }; static const char * const rk3399_isp_clks[] = { @@ -527,7 +528,8 @@ static const struct rkisp1_info rk3399_isp_info = { .isr_size = ARRAY_SIZE(rk3399_isp_isrs), .isp_ver = RKISP1_V10, .features = RKISP1_FEATURE_MIPI_CSI2 - | RKISP1_FEATURE_SELF_PATH, + | RKISP1_FEATURE_SELF_PATH + | RKISP1_FEATURE_DUAL_CROP, }; static const struct of_device_id rkisp1_of_match[] = { diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c index dd77a31e6014..de2eb2c97cc4 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c @@ -635,7 +635,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable) struct v4l2_subdev_state *sd_state; if (!enable) { - rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); + if (rkisp1_has_feature(rkisp1, DUAL_CROP)) + rkisp1_dcrop_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); rkisp1_rsz_disable(rsz, RKISP1_SHADOW_REGS_ASYNC); return 0; } @@ -646,7 +647,8 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable) sd_state = v4l2_subdev_lock_and_get_active_state(sd); rkisp1_rsz_config(rsz, sd_state, when); - rkisp1_dcrop_config(rsz, sd_state); + if (rkisp1_has_feature(rkisp1, DUAL_CROP)) + rkisp1_dcrop_config(rsz, sd_state); v4l2_subdev_unlock_state(sd_state);