| Message ID | 20231129092759.242641-6-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,01/11] media: rkisp1: Add and use rkisp1_has_feature() macro | expand |
Hi Paul, On 29/11/2023 11:27, Paul Elder wrote: > The ISP8000Nano, found in the i.MX8MP, has a different architecture to > crop at the resizer input. Instead of the "dual crop" block between the > ISP and the resizers found in the RK3399, cropping has been moved to the > input of the resizer blocks. As a result, the resizer CFG_UPD and > CFG_UPD_AUTO bits have been moved to make space for a new CROP_ENABLE > bit. > > Fix the resizer shadow update accordingly, using the DUAL_CROP feature > to infer whether or not the resizer implements cropping. Support for > resizer cropping itself will be added in a subsequent commit. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I don't think this one is correct. The above is perhaps true for ISP8000, but ISP8000Nano does not have CROP_ENABLE bit, and the CFG_UPD and CFG_UPD_AUTO are at the same locations as on RK3399. I don't have documentation to prove this, but experimentation shows that this is the case. Tomi > --- > Changes since v3: > > - Condition on RKISP1_FEATURE_DUAL_CROP feature > - Update commit message > > Changes since v2: > > - Condition on RKISP1_FEATURE_RSZ_CROP feature > - Rename bits > - Use the rkisp1_has_feature() macro > > .../media/platform/rockchip/rkisp1/rkisp1-regs.h | 5 +++++ > .../platform/rockchip/rkisp1/rkisp1-resizer.c | 15 +++++++++++---- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > index 3b19c8411360..95646b45f28b 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > @@ -168,6 +168,11 @@ > #define RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO BIT(9) > #define RKISP1_CIF_RSZ_SCALER_FACTOR BIT(16) > > +/* For resizer instances that support cropping */ > +#define RKISP1_CIF_RSZ_CTRL_CROP_ENABLE BIT(8) > +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD BIT(9) > +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO BIT(10) > + > /* MI_IMSC - MI_MIS - MI_RIS - MI_ICR - MI_ISR */ > #define RKISP1_CIF_MI_FRAME(stream) BIT((stream)->id) > #define RKISP1_CIF_MI_MBLK_LINE BIT(2) > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > index c1aaeed58acc..6d6ebc53c6e5 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > @@ -178,10 +178,17 @@ static void rkisp1_rsz_update_shadow(struct rkisp1_resizer *rsz, > { > u32 ctrl_cfg = rkisp1_rsz_read(rsz, RKISP1_CIF_RSZ_CTRL); > > - if (when == RKISP1_SHADOW_REGS_ASYNC) > - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; > - else > - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; > + if (when == RKISP1_SHADOW_REGS_ASYNC) { > + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; > + else > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO; > + } else { > + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; > + else > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD; > + } > > rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CTRL, ctrl_cfg); > }
On Mon, Dec 18, 2023 at 05:31:18PM +0200, Tomi Valkeinen wrote: > Hi Paul, > > On 29/11/2023 11:27, Paul Elder wrote: > > The ISP8000Nano, found in the i.MX8MP, has a different architecture to > > crop at the resizer input. Instead of the "dual crop" block between the > > ISP and the resizers found in the RK3399, cropping has been moved to the > > input of the resizer blocks. As a result, the resizer CFG_UPD and > > CFG_UPD_AUTO bits have been moved to make space for a new CROP_ENABLE > > bit. > > > > Fix the resizer shadow update accordingly, using the DUAL_CROP feature > > to infer whether or not the resizer implements cropping. Support for > > resizer cropping itself will be added in a subsequent commit. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I don't think this one is correct. > > The above is perhaps true for ISP8000, but ISP8000Nano does not have > CROP_ENABLE bit, and the CFG_UPD and CFG_UPD_AUTO are at the same > locations as on RK3399. > > I don't have documentation to prove this, but experimentation shows that > this is the case. I agree with you. The missing CROP_ENABLE bit matches the missing resizer input crop capability in the i.MX8MP. I don't know if that's specific to the i.MX8MP, specific to the ISP8000Nano, or common to all ISP8000 versions when the instance is synthesized with a single path (which may be what ISP8000Nano is). > > --- > > Changes since v3: > > > > - Condition on RKISP1_FEATURE_DUAL_CROP feature > > - Update commit message > > > > Changes since v2: > > > > - Condition on RKISP1_FEATURE_RSZ_CROP feature > > - Rename bits > > - Use the rkisp1_has_feature() macro > > > > .../media/platform/rockchip/rkisp1/rkisp1-regs.h | 5 +++++ > > .../platform/rockchip/rkisp1/rkisp1-resizer.c | 15 +++++++++++---- > > 2 files changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > > index 3b19c8411360..95646b45f28b 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > > @@ -168,6 +168,11 @@ > > #define RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO BIT(9) > > #define RKISP1_CIF_RSZ_SCALER_FACTOR BIT(16) > > > > +/* For resizer instances that support cropping */ > > +#define RKISP1_CIF_RSZ_CTRL_CROP_ENABLE BIT(8) > > +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD BIT(9) > > +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO BIT(10) > > + > > /* MI_IMSC - MI_MIS - MI_RIS - MI_ICR - MI_ISR */ > > #define RKISP1_CIF_MI_FRAME(stream) BIT((stream)->id) > > #define RKISP1_CIF_MI_MBLK_LINE BIT(2) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > > index c1aaeed58acc..6d6ebc53c6e5 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > > @@ -178,10 +178,17 @@ static void rkisp1_rsz_update_shadow(struct rkisp1_resizer *rsz, > > { > > u32 ctrl_cfg = rkisp1_rsz_read(rsz, RKISP1_CIF_RSZ_CTRL); > > > > - if (when == RKISP1_SHADOW_REGS_ASYNC) > > - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; > > - else > > - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; > > + if (when == RKISP1_SHADOW_REGS_ASYNC) { > > + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) > > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; > > + else > > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO; > > + } else { > > + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) > > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; > > + else > > + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD; > > + } > > > > rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CTRL, ctrl_cfg); > > }
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h index 3b19c8411360..95646b45f28b 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h @@ -168,6 +168,11 @@ #define RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO BIT(9) #define RKISP1_CIF_RSZ_SCALER_FACTOR BIT(16) +/* For resizer instances that support cropping */ +#define RKISP1_CIF_RSZ_CTRL_CROP_ENABLE BIT(8) +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD BIT(9) +#define RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO BIT(10) + /* MI_IMSC - MI_MIS - MI_RIS - MI_ICR - MI_ISR */ #define RKISP1_CIF_MI_FRAME(stream) BIT((stream)->id) #define RKISP1_CIF_MI_MBLK_LINE BIT(2) diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c index c1aaeed58acc..6d6ebc53c6e5 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c @@ -178,10 +178,17 @@ static void rkisp1_rsz_update_shadow(struct rkisp1_resizer *rsz, { u32 ctrl_cfg = rkisp1_rsz_read(rsz, RKISP1_CIF_RSZ_CTRL); - if (when == RKISP1_SHADOW_REGS_ASYNC) - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; - else - ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; + if (when == RKISP1_SHADOW_REGS_ASYNC) { + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD_AUTO; + else + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD_AUTO; + } else { + if (rkisp1_has_feature(rsz->rkisp1, DUAL_CROP)) + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CFG_UPD; + else + ctrl_cfg |= RKISP1_CIF_RSZ_CTRL_CROP_CFG_UPD; + } rkisp1_rsz_write(rsz, RKISP1_CIF_RSZ_CTRL, ctrl_cfg); }