From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Dafna Hirschfeld <dafna@fastmail.com>
Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
Paul Elder <paul.elder@ideasonboard.com>,
Florian Sylvestre <fsylvestre@baylibre.com>
Subject: Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
Date: Sat, 16 Jul 2022 14:49:49 +0300 [thread overview]
Message-ID: <YtKl3coiUuG8fISt@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20220716061247.dil4rt6zac2jzu6y@guri>
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@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
>
> 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2022-07-16 11:50 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
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 [this message]
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=YtKl3coiUuG8fISt@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=dafna@fastmail.com \
--cc=fsylvestre@baylibre.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=paul.elder@ideasonboard.com \
/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