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@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> 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);
> > }
--
Regards,
Laurent Pinchart
