public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: paul.elder@ideasonboard.com
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	Dafna Hirschfeld <dafna@fastmail.com>,
	Florian Sylvestre <fsylvestre@baylibre.com>
Subject: Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
Date: Fri, 15 Jul 2022 16:56:35 +0900	[thread overview]
Message-ID: <20220715075635.GG3411283@pyrite.rasen.tech> (raw)
In-Reply-To: <20220616160456.21549-3-laurent.pinchart@ideasonboard.com>

Hi Laurent,

On Thu, Jun 16, 2022 at 07:04:55PM +0300, 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@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  .../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
> +	 * 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 */

  reply	other threads:[~2022-07-15  7:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-16 16:04 [PATCH 0/3] media: rkisp1: Improve DPCC configuration Laurent Pinchart
2022-06-16 16:04 ` [PATCH 1/3] media: rockchip: rkisp1: Set DPCC methods enable bits inside loop Laurent Pinchart
2022-07-15  7:44   ` paul.elder
2022-07-16  5:11   ` Dafna Hirschfeld
2022-06-16 16:04 ` [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters Laurent Pinchart
2022-07-15  7:56   ` paul.elder [this message]
2022-07-16  5:56   ` Dafna Hirschfeld
2022-07-16 11:47     ` Laurent Pinchart
2022-07-16  6:12   ` Dafna Hirschfeld
2022-07-16 11:49     ` Laurent Pinchart
2022-06-16 16:04 ` [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI Laurent Pinchart
2022-07-15  7:49   ` paul.elder
2022-08-23 17:24   ` Laurent Pinchart
2022-08-26 19:04   ` Dafna Hirschfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220715075635.GG3411283@pyrite.rasen.tech \
    --to=paul.elder@ideasonboard.com \
    --cc=dafna@fastmail.com \
    --cc=fsylvestre@baylibre.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox