diff mbox series

[v4,05/11] media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP

Message ID 20231129092759.242641-6-paul.elder@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series media: rkisp1: Add support for i.MX8MP | expand

Commit Message

Paul Elder Nov. 29, 2023, 9:27 a.m. UTC
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>
---
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(-)

Comments

Tomi Valkeinen Dec. 18, 2023, 3:31 p.m. UTC | #1
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);
>   }
Laurent Pinchart Dec. 18, 2023, 3:48 p.m. UTC | #2
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 mbox series

Patch

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);
 }