Re: [PATCH v5 09/14] staging: media: starfive: Update ISP initialise config for 3A

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Changhuang,

Thank you for the patch.

On Tue, Jul 09, 2024 at 01:38:19AM -0700, Changhuang Liang wrote:
> Upadte ISP initialise for 3A statistics collection data.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../media/starfive/camss/stf-isp-hw-ops.c     | 23 +++++++++++++++++++
>  .../staging/media/starfive/camss/stf-isp.h    | 21 +++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
> index 3b18d09f2cc6..0bc5e36f952e 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
> +++ b/drivers/staging/media/starfive/camss/stf-isp-hw-ops.c
> @@ -300,6 +300,25 @@ static void stf_isp_config_sat(struct stfcamss *stfcamss)
>  	stf_isp_reg_write(stfcamss, ISP_REG_YADJ1, YOMAX(0x3ff) | YOMIN(0x1));
>  }
>  
> +static void stf_isp_config_sc(struct stfcamss *stfcamss)
> +{
> +	stf_isp_reg_write(stfcamss, ISP_REG_SCD_CFG_1, AXI_ID(0));
> +	stf_isp_reg_write(stfcamss, ISP_REG_SC_CFG_0, HSTART(0) | VSTART(0xc));
> +	stf_isp_reg_write(stfcamss, ISP_REG_SC_CFG_1,
> +			  SC_WIDTH(0x1d) | SC_HEIGHT(0x15) |
> +			  AWB_PS_GRB_BA(0x10) | SEL_TYPE(0x3));

There's lots of magic values here and below. A comment would be good.

> +}
> +
> +static void stf_isp_config_yhist(struct stfcamss *stfcamss)
> +{
> +	stf_isp_reg_write(stfcamss, ISP_REG_YHIST_CFG_0, 0);
> +	stf_isp_reg_write(stfcamss, ISP_REG_YHIST_CFG_1,
> +			  YH_WIDTH(0x77f) | YH_HEIGHT(0x437));

Does this hardcode the size of the histogram window to 1920x1080 ? It
should be made configurable from userspace. Maybe that's handled in
further patches in this series.

> +	stf_isp_reg_write(stfcamss, ISP_REG_YHIST_CFG_2,
> +			  YH_DEC_ETW(2) | YH_DEC_ETH(1));
> +	stf_isp_reg_write(stfcamss, ISP_REG_YHIST_CFG_3, 0);
> +}
> +
>  int stf_isp_reset(struct stf_isp_dev *isp_dev)
>  {
>  	stf_isp_reg_set_bit(isp_dev->stfcamss, ISP_REG_ISP_CTRL_0,
> @@ -332,7 +351,11 @@ void stf_isp_init_cfg(struct stf_isp_dev *isp_dev)
>  	stf_isp_config_sharpen(isp_dev->stfcamss);
>  	stf_isp_config_dnyuv(isp_dev->stfcamss);
>  	stf_isp_config_sat(isp_dev->stfcamss);
> +	stf_isp_config_sc(isp_dev->stfcamss);
> +	stf_isp_config_yhist(isp_dev->stfcamss);
>  
> +	stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_DUMP_CFG_1,
> +			  DUMP_BURST_LEN(3) | DUMP_SD(0xb80));
>  	stf_isp_reg_write(isp_dev->stfcamss, ISP_REG_CSI_MODULE_CFG,
>  			  CSI_DUMP_EN | CSI_SC_EN | CSI_AWB_EN |
>  			  CSI_LCCF_EN | CSI_OECF_EN | CSI_OBC_EN | CSI_DEC_EN);
> diff --git a/drivers/staging/media/starfive/camss/stf-isp.h b/drivers/staging/media/starfive/camss/stf-isp.h
> index 0af7b367e57a..eca3ba1ade75 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp.h
> +++ b/drivers/staging/media/starfive/camss/stf-isp.h
> @@ -110,9 +110,19 @@
>  
>  #define ISP_REG_SCD_CFG_0			0x098
>  
> +#define ISP_REG_SCD_CFG_1			0x09c
> +#define AXI_ID(n)				((n) << 24)
> +
> +#define ISP_REG_SC_CFG_0			0x0b8
> +#define VSTART(n)				((n) << 16)
> +#define HSTART(n)				((n) << 0)
> +
>  #define ISP_REG_SC_CFG_1			0x0bc
>  #define ISP_SC_SEL_MASK				GENMASK(31, 30)
>  #define SEL_TYPE(n)				((n) << 30)
> +#define AWB_PS_GRB_BA(n)			((n) << 16)
> +#define SC_HEIGHT(n)				((n) << 8)
> +#define SC_WIDTH(n)				((n) << 0)
>  
>  #define ISP_REG_LCCF_CFG_2			0x0e0
>  #define ISP_REG_LCCF_CFG_3			0x0e4
> @@ -312,6 +322,17 @@
>  #define DNRM_F(n)				((n) << 16)
>  #define CCM_M_DAT(n)				((n) << 0)
>  
> +#define ISP_REG_YHIST_CFG_0			0xcc8
> +
> +#define ISP_REG_YHIST_CFG_1			0xccc
> +#define YH_HEIGHT(n)				((n) << 16)
> +#define YH_WIDTH(n)				((n) << 0)
> +
> +#define ISP_REG_YHIST_CFG_2			0xcd0
> +#define YH_DEC_ETH(n)				((n) << 16)
> +#define YH_DEC_ETW(n)				((n) << 0)
> +
> +#define ISP_REG_YHIST_CFG_3			0xcd4
>  #define ISP_REG_YHIST_CFG_4			0xcd8
>  
>  #define ISP_REG_YHIST_ACC_0			0xd00
> -- 
> 2.25.1
> 

-- 
Regards,

Laurent Pinchart