* [PATCH 0/3] media: rkisp1: Improve DPCC configuration
@ 2022-06-16 16:04 Laurent Pinchart
2022-06-16 16:04 ` [PATCH 1/3] media: rockchip: rkisp1: Set DPCC methods enable bits inside loop Laurent Pinchart
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-06-16 16:04 UTC (permalink / raw)
To: linux-media
Cc: linux-rockchip, Dafna Hirschfeld, Paul Elder, Florian Sylvestre
Hello,
This small patch series improves the configuration of the Defective
Pixel Cluster Correction (DPCC) configuration. Please see individual
patches for details.
The series is based on top of "[PATCH v4 00/21] media: rkisp1: Misc bug
fixes and cleanups" ([1]).
[1] https://lore.kernel.org/linux-media/20220421234240.1694-1-laurent.pinchart@ideasonboard.com/
Laurent Pinchart (3):
media: rockchip: rkisp1: Set DPCC methods enable bits inside loop
media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI
.../platform/rockchip/rkisp1/rkisp1-params.c | 52 ++++++++-----
.../platform/rockchip/rkisp1/rkisp1-regs.h | 25 +++---
include/uapi/linux/rkisp1-config.h | 77 +++++++++++++++----
3 files changed, 104 insertions(+), 50 deletions(-)
base-commit: 4d846800b527933a46ffd5ff48b87f5a03ca087a
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] media: rockchip: rkisp1: Set DPCC methods enable bits inside loop
2022-06-16 16:04 [PATCH 0/3] media: rkisp1: Improve DPCC configuration Laurent Pinchart
@ 2022-06-16 16:04 ` 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-06-16 16:04 ` [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI Laurent Pinchart
2 siblings, 2 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-06-16 16:04 UTC (permalink / raw)
To: linux-media
Cc: linux-rockchip, Dafna Hirschfeld, Paul Elder, Florian Sylvestre
The rkisp1_dpcc_config() function looks over methods sets to configure
them, but sets the RKISP1_CIF_ISP_DPCC_METHODS_SET_* registers outside
of the loop with hand-unrolled code. Move this to the loop to simplify
the code.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index c88a9c0fa86e..140012fa18f0 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -18,6 +18,8 @@
#define RKISP1_ISP_PARAMS_REQ_BUFS_MIN 2
#define RKISP1_ISP_PARAMS_REQ_BUFS_MAX 8
+#define RKISP1_ISP_DPCC_METHODS_SET(n) \
+ (RKISP1_CIF_ISP_DPCC_METHODS_SET_1 + 0x4 * (n))
#define RKISP1_ISP_DPCC_LINE_THRESH(n) \
(RKISP1_CIF_ISP_DPCC_LINE_THRESH_1 + 0x14 * (n))
#define RKISP1_ISP_DPCC_LINE_MAD_FAC(n) \
@@ -66,13 +68,9 @@ static void rkisp1_dpcc_config(struct rkisp1_params *params,
rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_SET_USE,
arg->set_use);
- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_1,
- arg->methods[0].method);
- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_2,
- arg->methods[1].method);
- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_3,
- arg->methods[2].method);
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);
rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_THRESH(i),
arg->methods[i].line_thresh);
rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_MAD_FAC(i),
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
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-06-16 16:04 ` Laurent Pinchart
2022-07-15 7:56 ` paul.elder
` (2 more replies)
2022-06-16 16:04 ` [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI Laurent Pinchart
2 siblings, 3 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-06-16 16:04 UTC (permalink / raw)
To: linux-media
Cc: linux-rockchip, Dafna Hirschfeld, Paul Elder, Florian Sylvestre
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
+ * 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI
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-06-16 16:04 ` [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters Laurent Pinchart
@ 2022-06-16 16:04 ` Laurent Pinchart
2022-07-15 7:49 ` paul.elder
` (2 more replies)
2 siblings, 3 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-06-16 16:04 UTC (permalink / raw)
To: linux-media
Cc: linux-rockchip, Dafna Hirschfeld, Paul Elder, Florian Sylvestre
Extend the UAPI rkisp1-config.h header with macros for all DPCC
configuration fields. While at it, clarify of fix issues in the DPCC
documentation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-regs.h | 1 -
include/uapi/linux/rkisp1-config.h | 77 +++++++++++++++----
2 files changed, 61 insertions(+), 17 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
index dc01f968c19d..a931f7216e9b 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
@@ -620,7 +620,6 @@
/* DPCC */
#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
diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
index 583ca0d9a79d..730673ecc63d 100644
--- a/include/uapi/linux/rkisp1-config.h
+++ b/include/uapi/linux/rkisp1-config.h
@@ -117,7 +117,46 @@
/*
* Defect Pixel Cluster Correction
*/
-#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
+#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
+
+#define RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE (1U << 2)
+
+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER (1U << 0)
+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER (1U << 1)
+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_G_3X3 (1U << 2)
+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_RB_3X3 (1U << 3)
+
+/* 0-2 for sets 1-3 */
+#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_SET(n) ((n) << 0)
+#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET (1U << 3)
+
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE (1U << 0)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE (1U << 1)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE (1U << 2)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE (1U << 3)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE (1U << 4)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE (1U << 8)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE (1U << 9)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE (1U << 10)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE (1U << 11)
+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE (1U << 12)
+
+#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(v) ((v) << 0)
+#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(v) ((v) << 8)
+#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(v) ((v) << 0)
+#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(v) ((v) << 8)
+#define RKISP1_CIF_ISP_DPCC_PG_FAC_G(v) ((v) << 0)
+#define RKISP1_CIF_ISP_DPCC_PG_FAC_RB(v) ((v) << 8)
+#define RKISP1_CIF_ISP_DPCC_RND_THRESH_G(v) ((v) << 0)
+#define RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(v) ((v) << 8)
+#define RKISP1_CIF_ISP_DPCC_RG_FAC_G(v) ((v) << 0)
+#define RKISP1_CIF_ISP_DPCC_RG_FAC_RB(v) ((v) << 8)
+
+#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(n, v) ((v) << ((n) * 4))
+#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(n, v) ((v) << ((n) * 4 + 2))
+
+#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(n, v) ((v) << ((n) * 4))
+#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(n, v) ((v) << ((n) * 4 + 2))
/*
* Denoising pre filter
@@ -249,16 +288,20 @@ struct rkisp1_cif_isp_bls_config {
};
/**
- * struct rkisp1_cif_isp_dpcc_methods_config - Methods Configuration used by DPCC
+ * struct rkisp1_cif_isp_dpcc_methods_config - DPCC methods set configuration
*
- * Methods Configuration used by Defect Pixel Cluster Correction
+ * This structure stores the configuration of one set of methods for the DPCC
+ * algorithm. Multiple methods can be selected in each set (independently for
+ * the Green and Red/Blue components) through the @method field, the result is
+ * the logical AND of all enabled methods. The remaining fields set thresholds
+ * and factors for each method.
*
- * @method: Method enable bits
- * @line_thresh: Line threshold
- * @line_mad_fac: Line MAD factor
- * @pg_fac: Peak gradient factor
- * @rnd_thresh: Rank Neighbor Difference threshold
- * @rg_fac: Rank gradient factor
+ * @method: Method enable bits (RKISP1_CIF_ISP_DPCC_METHODS_SET_*)
+ * @line_thresh: Line threshold (RKISP1_CIF_ISP_DPCC_LINE_THRESH_*)
+ * @line_mad_fac: Line Mean Absolute Difference factor (RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_*)
+ * @pg_fac: Peak gradient factor (RKISP1_CIF_ISP_DPCC_PG_FAC_*)
+ * @rnd_thresh: Rank Neighbor Difference threshold (RKISP1_CIF_ISP_DPCC_RND_THRESH_*)
+ * @rg_fac: Rank gradient factor (RKISP1_CIF_ISP_DPCC_RG_FAC_*)
*/
struct rkisp1_cif_isp_dpcc_methods_config {
__u32 method;
@@ -272,14 +315,16 @@ struct rkisp1_cif_isp_dpcc_methods_config {
/**
* struct rkisp1_cif_isp_dpcc_config - Configuration used by DPCC
*
- * Configuration used by Defect Pixel Cluster Correction
+ * Configuration used by Defect Pixel Cluster Correction. Three sets of methods
+ * can be configured and selected through the @set_use field. The result is the
+ * logical OR of all enabled sets.
*
- * @mode: dpcc output mode
- * @output_mode: whether use hard coded methods
- * @set_use: stage1 methods set
- * @methods: methods config
- * @ro_limits: rank order limits
- * @rnd_offs: differential rank offsets for rank neighbor difference
+ * @mode: DPCC mode (RKISP1_CIF_ISP_DPCC_MODE_*)
+ * @output_mode: Interpolation output mode (RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_*)
+ * @set_use: Methods sets selection (RKISP1_CIF_ISP_DPCC_SET_USE_*)
+ * @methods: Methods sets configuration
+ * @ro_limits: Rank order limits (RKISP1_CIF_ISP_DPCC_RO_LIMITS_*)
+ * @rnd_offs: Differential rank offsets for rank neighbor difference (RKISP1_CIF_ISP_DPCC_RND_OFFS_*)
*/
struct rkisp1_cif_isp_dpcc_config {
__u32 mode;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] media: rockchip: rkisp1: Set DPCC methods enable bits inside loop
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
1 sibling, 0 replies; 14+ messages in thread
From: paul.elder @ 2022-07-15 7:44 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Dafna Hirschfeld, Florian Sylvestre
Hi Laurent,
On Thu, Jun 16, 2022 at 07:04:54PM +0300, Laurent Pinchart wrote:
> The rkisp1_dpcc_config() function looks over methods sets to configure
> them, but sets the RKISP1_CIF_ISP_DPCC_METHODS_SET_* registers outside
> of the loop with hand-unrolled code. Move this to the loop to simplify
> the code.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> index c88a9c0fa86e..140012fa18f0 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> @@ -18,6 +18,8 @@
> #define RKISP1_ISP_PARAMS_REQ_BUFS_MIN 2
> #define RKISP1_ISP_PARAMS_REQ_BUFS_MAX 8
>
> +#define RKISP1_ISP_DPCC_METHODS_SET(n) \
> + (RKISP1_CIF_ISP_DPCC_METHODS_SET_1 + 0x4 * (n))
> #define RKISP1_ISP_DPCC_LINE_THRESH(n) \
> (RKISP1_CIF_ISP_DPCC_LINE_THRESH_1 + 0x14 * (n))
> #define RKISP1_ISP_DPCC_LINE_MAD_FAC(n) \
> @@ -66,13 +68,9 @@ static void rkisp1_dpcc_config(struct rkisp1_params *params,
> rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_SET_USE,
> arg->set_use);
>
> - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_1,
> - arg->methods[0].method);
> - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_2,
> - arg->methods[1].method);
> - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_3,
> - arg->methods[2].method);
> 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);
> rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_THRESH(i),
> arg->methods[i].line_thresh);
> rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_MAD_FAC(i),
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI
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
2 siblings, 0 replies; 14+ messages in thread
From: paul.elder @ 2022-07-15 7:49 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Dafna Hirschfeld, Florian Sylvestre
Hi Laurent,
On Thu, Jun 16, 2022 at 07:04:56PM +0300, Laurent Pinchart wrote:
> Extend the UAPI rkisp1-config.h header with macros for all DPCC
> configuration fields. While at it, clarify of fix issues in the DPCC
> documentation.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-regs.h | 1 -
> include/uapi/linux/rkisp1-config.h | 77 +++++++++++++++----
> 2 files changed, 61 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> index dc01f968c19d..a931f7216e9b 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> @@ -620,7 +620,6 @@
> /* DPCC */
> #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
> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> index 583ca0d9a79d..730673ecc63d 100644
> --- a/include/uapi/linux/rkisp1-config.h
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -117,7 +117,46 @@
> /*
> * Defect Pixel Cluster Correction
> */
> -#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
> +#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
> +
> +#define RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE (1U << 2)
> +
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER (1U << 0)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER (1U << 1)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_G_3X3 (1U << 2)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_RB_3X3 (1U << 3)
> +
> +/* 0-2 for sets 1-3 */
> +#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_SET(n) ((n) << 0)
> +#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET (1U << 3)
> +
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE (1U << 0)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE (1U << 1)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE (1U << 2)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE (1U << 3)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE (1U << 4)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE (1U << 8)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE (1U << 9)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE (1U << 10)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE (1U << 11)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE (1U << 12)
> +
> +#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_PG_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_PG_FAC_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_RND_THRESH_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_RG_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_RG_FAC_RB(v) ((v) << 8)
> +
> +#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(n, v) ((v) << ((n) * 4))
> +#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(n, v) ((v) << ((n) * 4 + 2))
> +
> +#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(n, v) ((v) << ((n) * 4))
> +#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(n, v) ((v) << ((n) * 4 + 2))
>
> /*
> * Denoising pre filter
> @@ -249,16 +288,20 @@ struct rkisp1_cif_isp_bls_config {
> };
>
> /**
> - * struct rkisp1_cif_isp_dpcc_methods_config - Methods Configuration used by DPCC
> + * struct rkisp1_cif_isp_dpcc_methods_config - DPCC methods set configuration
> *
> - * Methods Configuration used by Defect Pixel Cluster Correction
> + * This structure stores the configuration of one set of methods for the DPCC
> + * algorithm. Multiple methods can be selected in each set (independently for
> + * the Green and Red/Blue components) through the @method field, the result is
> + * the logical AND of all enabled methods. The remaining fields set thresholds
> + * and factors for each method.
> *
> - * @method: Method enable bits
> - * @line_thresh: Line threshold
> - * @line_mad_fac: Line MAD factor
> - * @pg_fac: Peak gradient factor
> - * @rnd_thresh: Rank Neighbor Difference threshold
> - * @rg_fac: Rank gradient factor
> + * @method: Method enable bits (RKISP1_CIF_ISP_DPCC_METHODS_SET_*)
> + * @line_thresh: Line threshold (RKISP1_CIF_ISP_DPCC_LINE_THRESH_*)
> + * @line_mad_fac: Line Mean Absolute Difference factor (RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_*)
> + * @pg_fac: Peak gradient factor (RKISP1_CIF_ISP_DPCC_PG_FAC_*)
> + * @rnd_thresh: Rank Neighbor Difference threshold (RKISP1_CIF_ISP_DPCC_RND_THRESH_*)
> + * @rg_fac: Rank gradient factor (RKISP1_CIF_ISP_DPCC_RG_FAC_*)
> */
> struct rkisp1_cif_isp_dpcc_methods_config {
> __u32 method;
> @@ -272,14 +315,16 @@ struct rkisp1_cif_isp_dpcc_methods_config {
> /**
> * struct rkisp1_cif_isp_dpcc_config - Configuration used by DPCC
> *
> - * Configuration used by Defect Pixel Cluster Correction
> + * Configuration used by Defect Pixel Cluster Correction. Three sets of methods
> + * can be configured and selected through the @set_use field. The result is the
> + * logical OR of all enabled sets.
> *
> - * @mode: dpcc output mode
> - * @output_mode: whether use hard coded methods
> - * @set_use: stage1 methods set
> - * @methods: methods config
> - * @ro_limits: rank order limits
> - * @rnd_offs: differential rank offsets for rank neighbor difference
> + * @mode: DPCC mode (RKISP1_CIF_ISP_DPCC_MODE_*)
> + * @output_mode: Interpolation output mode (RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_*)
> + * @set_use: Methods sets selection (RKISP1_CIF_ISP_DPCC_SET_USE_*)
> + * @methods: Methods sets configuration
> + * @ro_limits: Rank order limits (RKISP1_CIF_ISP_DPCC_RO_LIMITS_*)
> + * @rnd_offs: Differential rank offsets for rank neighbor difference (RKISP1_CIF_ISP_DPCC_RND_OFFS_*)
> */
> struct rkisp1_cif_isp_dpcc_config {
> __u32 mode;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
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 6:12 ` Dafna Hirschfeld
2 siblings, 0 replies; 14+ messages in thread
From: paul.elder @ 2022-07-15 7:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Dafna Hirschfeld, Florian Sylvestre
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 */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] media: rockchip: rkisp1: Set DPCC methods enable bits inside loop
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
1 sibling, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2022-07-16 5:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
hi,
On 16.06.2022 19:04, Laurent Pinchart wrote:
>The rkisp1_dpcc_config() function looks over methods sets to configure
>them, but sets the RKISP1_CIF_ISP_DPCC_METHODS_SET_* registers outside
>of the loop with hand-unrolled code. Move this to the loop to simplify
>the code.
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com>
>---
> drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>index c88a9c0fa86e..140012fa18f0 100644
>--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
>@@ -18,6 +18,8 @@
> #define RKISP1_ISP_PARAMS_REQ_BUFS_MIN 2
> #define RKISP1_ISP_PARAMS_REQ_BUFS_MAX 8
>
>+#define RKISP1_ISP_DPCC_METHODS_SET(n) \
>+ (RKISP1_CIF_ISP_DPCC_METHODS_SET_1 + 0x4 * (n))
> #define RKISP1_ISP_DPCC_LINE_THRESH(n) \
> (RKISP1_CIF_ISP_DPCC_LINE_THRESH_1 + 0x14 * (n))
> #define RKISP1_ISP_DPCC_LINE_MAD_FAC(n) \
>@@ -66,13 +68,9 @@ static void rkisp1_dpcc_config(struct rkisp1_params *params,
> rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_SET_USE,
> arg->set_use);
>
>- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_1,
>- arg->methods[0].method);
>- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_2,
>- arg->methods[1].method);
>- rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_DPCC_METHODS_SET_3,
>- arg->methods[2].method);
> 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);
> rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_THRESH(i),
> arg->methods[i].line_thresh);
> rkisp1_write(params->rkisp1, RKISP1_ISP_DPCC_LINE_MAD_FAC(i),
>--
>Regards,
>
>Laurent Pinchart
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
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
2 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2022-07-16 5:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
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
>+ * 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)
Why are these two masks use GENMASK and other don't?
Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com>
>+#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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
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 6:12 ` Dafna Hirschfeld
2022-07-16 11:49 ` Laurent Pinchart
2 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2022-07-16 6:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
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?
Thanks,
Dafna
>+ * 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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
2022-07-16 5:56 ` Dafna Hirschfeld
@ 2022-07-16 11:47 ` Laurent Pinchart
0 siblings, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-07-16 11:47 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
Hi Dafna,
On Sat, Jul 16, 2022 at 08:56:48AM +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
> > + * 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)
>
> Why are these two masks use GENMASK and other don't?
Good question. Probably because the above two are new and I didn't think
about changing the other ones.
For RKISP1_CIF_ISP_DPCC_METHODS_SET_MASK, would you rather keep
#define RKISP1_CIF_ISP_DPCC_METHODS_SET_MASK 0x00001f1f
or have
#define RKISP1_CIF_ISP_DPCC_METHODS_SET_MASK (GENMASK(12 ,8) | GENMASK(4, 0))
? I think the former is easier to read and match against datasheets, but
I don't mind much either way.
> Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com>
>
> > +#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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: rockchip: rkisp1: Mask invalid bits in DPCC parameters
2022-07-16 6:12 ` Dafna Hirschfeld
@ 2022-07-16 11:49 ` Laurent Pinchart
0 siblings, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-07-16 11:49 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI
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
2 siblings, 0 replies; 14+ messages in thread
From: Laurent Pinchart @ 2022-08-23 17:24 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
Hi Dafna,
Would you be able to review this patch ?
On Thu, Jun 16, 2022 at 07:04:56PM +0300, Laurent Pinchart wrote:
> Extend the UAPI rkisp1-config.h header with macros for all DPCC
> configuration fields. While at it, clarify of fix issues in the DPCC
> documentation.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-regs.h | 1 -
> include/uapi/linux/rkisp1-config.h | 77 +++++++++++++++----
> 2 files changed, 61 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> index dc01f968c19d..a931f7216e9b 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> @@ -620,7 +620,6 @@
> /* DPCC */
> #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
> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> index 583ca0d9a79d..730673ecc63d 100644
> --- a/include/uapi/linux/rkisp1-config.h
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -117,7 +117,46 @@
> /*
> * Defect Pixel Cluster Correction
> */
> -#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
> +#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
> +
> +#define RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE (1U << 2)
> +
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER (1U << 0)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER (1U << 1)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_G_3X3 (1U << 2)
> +#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_RB_3X3 (1U << 3)
> +
> +/* 0-2 for sets 1-3 */
> +#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_SET(n) ((n) << 0)
> +#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET (1U << 3)
> +
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE (1U << 0)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE (1U << 1)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE (1U << 2)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE (1U << 3)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE (1U << 4)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE (1U << 8)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE (1U << 9)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE (1U << 10)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE (1U << 11)
> +#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE (1U << 12)
> +
> +#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_PG_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_PG_FAC_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_RND_THRESH_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(v) ((v) << 8)
> +#define RKISP1_CIF_ISP_DPCC_RG_FAC_G(v) ((v) << 0)
> +#define RKISP1_CIF_ISP_DPCC_RG_FAC_RB(v) ((v) << 8)
> +
> +#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(n, v) ((v) << ((n) * 4))
> +#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(n, v) ((v) << ((n) * 4 + 2))
> +
> +#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(n, v) ((v) << ((n) * 4))
> +#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(n, v) ((v) << ((n) * 4 + 2))
>
> /*
> * Denoising pre filter
> @@ -249,16 +288,20 @@ struct rkisp1_cif_isp_bls_config {
> };
>
> /**
> - * struct rkisp1_cif_isp_dpcc_methods_config - Methods Configuration used by DPCC
> + * struct rkisp1_cif_isp_dpcc_methods_config - DPCC methods set configuration
> *
> - * Methods Configuration used by Defect Pixel Cluster Correction
> + * This structure stores the configuration of one set of methods for the DPCC
> + * algorithm. Multiple methods can be selected in each set (independently for
> + * the Green and Red/Blue components) through the @method field, the result is
> + * the logical AND of all enabled methods. The remaining fields set thresholds
> + * and factors for each method.
> *
> - * @method: Method enable bits
> - * @line_thresh: Line threshold
> - * @line_mad_fac: Line MAD factor
> - * @pg_fac: Peak gradient factor
> - * @rnd_thresh: Rank Neighbor Difference threshold
> - * @rg_fac: Rank gradient factor
> + * @method: Method enable bits (RKISP1_CIF_ISP_DPCC_METHODS_SET_*)
> + * @line_thresh: Line threshold (RKISP1_CIF_ISP_DPCC_LINE_THRESH_*)
> + * @line_mad_fac: Line Mean Absolute Difference factor (RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_*)
> + * @pg_fac: Peak gradient factor (RKISP1_CIF_ISP_DPCC_PG_FAC_*)
> + * @rnd_thresh: Rank Neighbor Difference threshold (RKISP1_CIF_ISP_DPCC_RND_THRESH_*)
> + * @rg_fac: Rank gradient factor (RKISP1_CIF_ISP_DPCC_RG_FAC_*)
> */
> struct rkisp1_cif_isp_dpcc_methods_config {
> __u32 method;
> @@ -272,14 +315,16 @@ struct rkisp1_cif_isp_dpcc_methods_config {
> /**
> * struct rkisp1_cif_isp_dpcc_config - Configuration used by DPCC
> *
> - * Configuration used by Defect Pixel Cluster Correction
> + * Configuration used by Defect Pixel Cluster Correction. Three sets of methods
> + * can be configured and selected through the @set_use field. The result is the
> + * logical OR of all enabled sets.
> *
> - * @mode: dpcc output mode
> - * @output_mode: whether use hard coded methods
> - * @set_use: stage1 methods set
> - * @methods: methods config
> - * @ro_limits: rank order limits
> - * @rnd_offs: differential rank offsets for rank neighbor difference
> + * @mode: DPCC mode (RKISP1_CIF_ISP_DPCC_MODE_*)
> + * @output_mode: Interpolation output mode (RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_*)
> + * @set_use: Methods sets selection (RKISP1_CIF_ISP_DPCC_SET_USE_*)
> + * @methods: Methods sets configuration
> + * @ro_limits: Rank order limits (RKISP1_CIF_ISP_DPCC_RO_LIMITS_*)
> + * @rnd_offs: Differential rank offsets for rank neighbor difference (RKISP1_CIF_ISP_DPCC_RND_OFFS_*)
> */
> struct rkisp1_cif_isp_dpcc_config {
> __u32 mode;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: rockchip: rkisp1: Define macros for DPCC configurations in UAPI
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
2 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2022-08-26 19:04 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-rockchip, Paul Elder, Florian Sylvestre
On 16.06.2022 19:04, Laurent Pinchart wrote:
>Extend the UAPI rkisp1-config.h header with macros for all DPCC
>configuration fields. While at it, clarify of fix issues in the DPCC
>documentation.
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Dafna Hirschfeld <dafna@fastmail.com>
>---
> .../platform/rockchip/rkisp1/rkisp1-regs.h | 1 -
> include/uapi/linux/rkisp1-config.h | 77 +++++++++++++++----
> 2 files changed, 61 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
>index dc01f968c19d..a931f7216e9b 100644
>--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
>+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
>@@ -620,7 +620,6 @@
> /* DPCC */
> #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
>diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
>index 583ca0d9a79d..730673ecc63d 100644
>--- a/include/uapi/linux/rkisp1-config.h
>+++ b/include/uapi/linux/rkisp1-config.h
>@@ -117,7 +117,46 @@
> /*
> * Defect Pixel Cluster Correction
> */
>-#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
>+#define RKISP1_CIF_ISP_DPCC_METHODS_MAX 3
>+
>+#define RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE (1U << 2)
>+
>+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER (1U << 0)
>+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER (1U << 1)
>+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_G_3X3 (1U << 2)
>+#define RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_RB_3X3 (1U << 3)
>+
>+/* 0-2 for sets 1-3 */
>+#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_SET(n) ((n) << 0)
>+#define RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET (1U << 3)
>+
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE (1U << 0)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE (1U << 1)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE (1U << 2)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE (1U << 3)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE (1U << 4)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE (1U << 8)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE (1U << 9)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE (1U << 10)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE (1U << 11)
>+#define RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE (1U << 12)
>+
>+#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(v) ((v) << 0)
>+#define RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(v) ((v) << 8)
>+#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(v) ((v) << 0)
>+#define RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(v) ((v) << 8)
>+#define RKISP1_CIF_ISP_DPCC_PG_FAC_G(v) ((v) << 0)
>+#define RKISP1_CIF_ISP_DPCC_PG_FAC_RB(v) ((v) << 8)
>+#define RKISP1_CIF_ISP_DPCC_RND_THRESH_G(v) ((v) << 0)
>+#define RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(v) ((v) << 8)
>+#define RKISP1_CIF_ISP_DPCC_RG_FAC_G(v) ((v) << 0)
>+#define RKISP1_CIF_ISP_DPCC_RG_FAC_RB(v) ((v) << 8)
>+
>+#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(n, v) ((v) << ((n) * 4))
>+#define RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(n, v) ((v) << ((n) * 4 + 2))
>+
>+#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(n, v) ((v) << ((n) * 4))
>+#define RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(n, v) ((v) << ((n) * 4 + 2))
>
> /*
> * Denoising pre filter
>@@ -249,16 +288,20 @@ struct rkisp1_cif_isp_bls_config {
> };
>
> /**
>- * struct rkisp1_cif_isp_dpcc_methods_config - Methods Configuration used by DPCC
>+ * struct rkisp1_cif_isp_dpcc_methods_config - DPCC methods set configuration
> *
>- * Methods Configuration used by Defect Pixel Cluster Correction
>+ * This structure stores the configuration of one set of methods for the DPCC
>+ * algorithm. Multiple methods can be selected in each set (independently for
>+ * the Green and Red/Blue components) through the @method field, the result is
>+ * the logical AND of all enabled methods. The remaining fields set thresholds
>+ * and factors for each method.
> *
>- * @method: Method enable bits
>- * @line_thresh: Line threshold
>- * @line_mad_fac: Line MAD factor
>- * @pg_fac: Peak gradient factor
>- * @rnd_thresh: Rank Neighbor Difference threshold
>- * @rg_fac: Rank gradient factor
>+ * @method: Method enable bits (RKISP1_CIF_ISP_DPCC_METHODS_SET_*)
>+ * @line_thresh: Line threshold (RKISP1_CIF_ISP_DPCC_LINE_THRESH_*)
>+ * @line_mad_fac: Line Mean Absolute Difference factor (RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_*)
>+ * @pg_fac: Peak gradient factor (RKISP1_CIF_ISP_DPCC_PG_FAC_*)
>+ * @rnd_thresh: Rank Neighbor Difference threshold (RKISP1_CIF_ISP_DPCC_RND_THRESH_*)
>+ * @rg_fac: Rank gradient factor (RKISP1_CIF_ISP_DPCC_RG_FAC_*)
> */
> struct rkisp1_cif_isp_dpcc_methods_config {
> __u32 method;
>@@ -272,14 +315,16 @@ struct rkisp1_cif_isp_dpcc_methods_config {
> /**
> * struct rkisp1_cif_isp_dpcc_config - Configuration used by DPCC
> *
>- * Configuration used by Defect Pixel Cluster Correction
>+ * Configuration used by Defect Pixel Cluster Correction. Three sets of methods
>+ * can be configured and selected through the @set_use field. The result is the
>+ * logical OR of all enabled sets.
> *
>- * @mode: dpcc output mode
>- * @output_mode: whether use hard coded methods
>- * @set_use: stage1 methods set
>- * @methods: methods config
>- * @ro_limits: rank order limits
>- * @rnd_offs: differential rank offsets for rank neighbor difference
>+ * @mode: DPCC mode (RKISP1_CIF_ISP_DPCC_MODE_*)
>+ * @output_mode: Interpolation output mode (RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_*)
>+ * @set_use: Methods sets selection (RKISP1_CIF_ISP_DPCC_SET_USE_*)
>+ * @methods: Methods sets configuration
>+ * @ro_limits: Rank order limits (RKISP1_CIF_ISP_DPCC_RO_LIMITS_*)
>+ * @rnd_offs: Differential rank offsets for rank neighbor difference (RKISP1_CIF_ISP_DPCC_RND_OFFS_*)
> */
> struct rkisp1_cif_isp_dpcc_config {
> __u32 mode;
>--
>Regards,
>
>Laurent Pinchart
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-08-26 19:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).