Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jul 16, 2022 at 09:12:47AM +0300, Dafna Hirschfeld wrote:
> On 16.06.2022 19:04, Laurent Pinchart wrote:
> > Restrict the DPCC configuration that can be set by userspace to valid
> > register bits. To do so, reorganize the related register macros to
> > define valid bitmasks, as well as bits of the DPCC mode register.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> >  .../platform/rockchip/rkisp1/rkisp1-params.c  | 44 ++++++++++++-------
> >  .../platform/rockchip/rkisp1/rkisp1-regs.h    | 26 +++++------
> >  2 files changed, 41 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > index 140012fa18f0..94e834cd2a21 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > @@ -58,35 +58,47 @@ static void rkisp1_dpcc_config(struct rkisp1_params *params,
> >  	unsigned int i;
> >  	u32 mode;
> > 
> > -	/* avoid to override the old enable value */
> > +	/*
> > +	 * The enable bit is controlled in rkisp1_isp_isr_other_config() and
> > +	 * must be preserved. The grayscale mode should be configured
> > +	 * automatically based on the media bus code on the ISP sink pad, so
> 
> I see you add RKISP1_CIF_ISP_DPCC_MODE_GRAYSCALE_MODE in this patch.
> Shouldn't you add a patch that set/unset it according to the isp sink pad
> as this doc says?

I should, but the driver doesn't support (yet) greyscale sensors
(MEDIA_BUS_FMT_Y8_1X8 and other bus widths are not mentioned anywhere),
so I can't do that yet.

> > +	 * only the STAGE1_ENABLE bit can be set by userspace.
> > +	 */
> >  	mode = rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_DPCC_MODE);
> > -	mode &= RKISP1_CIF_ISP_DPCC_ENA;
> > -	mode |= arg->mode & ~RKISP1_CIF_ISP_DPCC_ENA;
> > +	mode &= RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE;
> > +	mode |= arg->mode & RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE;
> >  	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_MODE, mode);
> > +
> >  	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_OUTPUT_MODE,
> > -		     arg->output_mode);
> > +		     arg->output_mode & RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_MASK);
> >  	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_SET_USE,
> > -		     arg->set_use);
> > +		     arg->set_use & RKISP1_CIF_ISP_DPCC_SET_USE_MASK);
> > 
> >  	for (i = 0; i < RKISP1_CIF_ISP_DPCC_METHODS_MAX; i++) {
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_METHODS_SET(i),
> > -			     arg->methods[i].method);
> > +			     arg->methods[i].method &
> > +			     RKISP1_CIF_ISP_DPCC_METHODS_SET_MASK);
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_THRESH(i),
> > -			     arg->methods[i].line_thresh);
> > +			     arg->methods[i].line_thresh &
> > +			     RKISP1_CIF_ISP_DPCC_LINE_THRESH_MASK);
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_MAD_FAC(i),
> > -			     arg->methods[i].line_mad_fac);
> > +			     arg->methods[i].line_mad_fac &
> > +			     RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_MASK);
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_PG_FAC(i),
> > -			     arg->methods[i].pg_fac);
> > +			     arg->methods[i].pg_fac &
> > +			     RKISP1_CIF_ISP_DPCC_PG_FAC_MASK);
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_RND_THRESH(i),
> > -			     arg->methods[i].rnd_thresh);
> > +			     arg->methods[i].rnd_thresh &
> > +			     RKISP1_CIF_ISP_DPCC_RND_THRESH_MASK);
> >  		rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_RG_FAC(i),
> > -			     arg->methods[i].rg_fac);
> > +			     arg->methods[i].rg_fac &
> > +			     RKISP1_CIF_ISP_DPCC_RG_FAC_MASK);
> >  	}
> > 
> >  	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_RND_OFFS,
> > -		     arg->rnd_offs);
> > +		     arg->rnd_offs & RKISP1_CIF_ISP_DPCC_RND_OFFS_MASK);
> >  	rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_RO_LIMITS,
> > -		     arg->ro_limits);
> > +		     arg->ro_limits & RKISP1_CIF_ISP_DPCC_RO_LIMIT_MASK);
> >  }
> > 
> >  /* ISP black level subtraction interface function */
> > @@ -1214,11 +1226,11 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> >  		if (module_ens & RKISP1_CIF_ISP_MODULE_DPCC)
> >  			rkisp1_param_set_bits(params,
> >  					      RKISP1_CIF_ISP_DPCC_MODE,
> > -					      RKISP1_CIF_ISP_DPCC_ENA);
> > +					      RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> >  		else
> >  			rkisp1_param_clear_bits(params,
> >  						RKISP1_CIF_ISP_DPCC_MODE,
> > -						RKISP1_CIF_ISP_DPCC_ENA);
> > +						RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> >  	}
> > 
> >  	/* update bls config */
> > @@ -1580,7 +1592,7 @@ void rkisp1_params_configure(struct rkisp1_params *params,
> >  void rkisp1_params_disable(struct rkisp1_params *params)
> >  {
> >  	rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> > -				RKISP1_CIF_ISP_DPCC_ENA);
> > +				RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> >  	rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> >  				RKISP1_CIF_ISP_LSC_CTRL_ENA);
> >  	rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > index dd3e6c38be67..dc01f968c19d 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > @@ -618,19 +618,19 @@
> >  #define RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA_READ(x)	(((x) >> 11) & 1)
> > 
> >  /* DPCC */
> > -/* ISP_DPCC_MODE */
> > -#define RKISP1_CIF_ISP_DPCC_ENA				BIT(0)
> > -#define RKISP1_CIF_ISP_DPCC_MODE_MAX			0x07
> > -#define RKISP1_CIF_ISP_DPCC_OUTPUTMODE_MAX		0x0F
> > -#define RKISP1_CIF_ISP_DPCC_SETUSE_MAX			0x0F
> > -#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RESERVED	0xFFFFE000
> > -#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_RESERVED	0xFFFF0000
> > -#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RESERVED	0xFFFFC0C0
> > -#define RKISP1_CIF_ISP_DPCC_PG_FAC_RESERVED		0xFFFFC0C0
> > -#define RKISP1_CIF_ISP_DPCC_RND_THRESH_RESERVED		0xFFFF0000
> > -#define RKISP1_CIF_ISP_DPCC_RG_FAC_RESERVED		0xFFFFC0C0
> > -#define RKISP1_CIF_ISP_DPCC_RO_LIMIT_RESERVED		0xFFFFF000
> > -#define RKISP1_CIF_ISP_DPCC_RND_OFFS_RESERVED		0xFFFFF000
> > +#define RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE		BIT(0)
> > +#define RKISP1_CIF_ISP_DPCC_MODE_GRAYSCALE_MODE		BIT(1)
> > +#define RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE		BIT(2)
> > +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_MASK		GENMASK(3, 0)
> > +#define RKISP1_CIF_ISP_DPCC_SET_USE_MASK		GENMASK(3, 0)
> > +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_MASK		0x00001f1f
> > +#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_MASK		0x0000ffff
> > +#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_MASK		0x00003f3f
> > +#define RKISP1_CIF_ISP_DPCC_PG_FAC_MASK			0x00003f3f
> > +#define RKISP1_CIF_ISP_DPCC_RND_THRESH_MASK		0x0000ffff
> > +#define RKISP1_CIF_ISP_DPCC_RG_FAC_MASK			0x00003f3f
> > +#define RKISP1_CIF_ISP_DPCC_RO_LIMIT_MASK		0x00000fff
> > +#define RKISP1_CIF_ISP_DPCC_RND_OFFS_MASK		0x00000fff
> > 
> >  /* BLS */
> >  /* ISP_BLS_CTRL */

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux