Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams()
From: Dan Scally
Date: Mon Mar 16 2026 - 05:36:24 EST
Morning Jacopo, happy Monday.
On 13/03/2026 17:00, Jacopo Mondi wrote:
Hi Dan
On Fri, Mar 13, 2026 at 04:14:09PM +0000, Dan Scally wrote:
Hi Jacopo
On 13/03/2026 16:10, Jacopo Mondi wrote:
Hi Dan
On Fri, Mar 13, 2026 at 03:59:22PM +0000, Dan Scally wrote:
Hi Jacopo - thanks for the patches
On 13/03/2026 14:53, Jacopo Mondi wrote:
The Mali C55 driver initializes the ISP in two points:
1) At probe time it disables ISP blocks by configuring them in bypass
mode
2) At enable_streams() it initializes the crop rectangles and the image
processing pipeline using the current image format
However, as ISP blocks are configured by userspace, if their
configuration is not reset, from the second enable_streams() call
onwards the ISP configuration will depend on the previous streaming
session configuration.
To re-initialize the ISP completely at enable_strems() time consolidate
s/enable_strems/enable_streams
the ISP block bypass configuration and the image processing path
configuration in a single function to be called at enabled_streams()
time.
I'm slightly confused; the change seems fine, but as far as I can see it's
non-functional...or is this just preliminary reorganisation to make the next
patch easier?
Isn't mali_c55_init_context() only called at probe() time ?
IOW all the bypass configuration that were only performed at probe
time are now performed at every streaming start.
Or have I missed something ?
But they're context rather than hardware writes; they're stored in the
context's registers buffer and should persist past power off / on and then
be re-written to the hardware when mali_c55_config_write() is called in
mali_c55_isp_start() the next time streaming started...at least that's
what's supposed to happen.
I'm not so concerned about power on/off but rather about configs from a
streaming session being applied to the following one.
In the case of the bypasses that are configured in
mali_c55_init_context(), at the moment there is no uAPI to enable
them.
However, rather soon we'll introduce blocks in the uAPI to enable
them, and I think there still is value in resetting the ISP
configuration in full at start streaming time. Do you agree ?
Yes; and indeed when we add uAPI blocks that allow users to configure these particular registers they would need resetting at stream-start too, so:
Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
Thanks
Dan
Thanks
j
Thanks
Dan
Cc: stable@xxxxxxxxxxxxxxx
Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
---
.../media/platform/arm/mali-c55/mali-c55-common.h | 2 +
.../media/platform/arm/mali-c55/mali-c55-core.c | 35 -----------
drivers/media/platform/arm/mali-c55/mali-c55-isp.c | 37 ++---------
.../media/platform/arm/mali-c55/mali-c55-params.c | 72 ++++++++++++++++++++++
4 files changed, 79 insertions(+), 67 deletions(-)
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-common.h b/drivers/media/platform/arm/mali-c55/mali-c55-common.h
index 31c1deaca146..13a3e9dc4243 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-common.h
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-common.h
@@ -306,5 +306,7 @@ bool mali_c55_pipeline_ready(struct mali_c55 *mali_c55);
void mali_c55_stats_fill_buffer(struct mali_c55 *mali_c55,
enum mali_c55_config_spaces cfg_space);
void mali_c55_params_write_config(struct mali_c55 *mali_c55);
+void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55,
+ const struct v4l2_subdev_state *state);
#endif /* _MALI_C55_COMMON_H */
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-core.c b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
index 43b834459ccf..c1a562cd214e 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
@@ -663,41 +663,6 @@ static int mali_c55_init_context(struct mali_c55 *mali_c55,
mali_c55->base + config_space_addrs[MALI_C55_CONFIG_PING],
MALI_C55_CONFIG_SPACE_SIZE);
- /*
- * Some features of the ISP need to be disabled by default and only
- * enabled at the same time as they're configured by a parameters buffer
- */
-
- /* Bypass the sqrt and square compression and expansion modules */
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_1,
- MALI_C55_REG_BYPASS_1_FE_SQRT,
- MALI_C55_REG_BYPASS_1_FE_SQRT);
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_3,
- MALI_C55_REG_BYPASS_3_SQUARE_BE,
- MALI_C55_REG_BYPASS_3_SQUARE_BE);
-
- /* Bypass the temper module */
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_2,
- MALI_C55_REG_BYPASS_2_TEMPER);
-
- /* Disable the temper module's DMA read/write */
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_TEMPER_DMA_IO, 0x0);
-
- /* Bypass the colour noise reduction */
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_4,
- MALI_C55_REG_BYPASS_4_CNR);
-
- /* Disable the sinter module */
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_SINTER_CONFIG,
- MALI_C55_SINTER_ENABLE_MASK, 0);
-
- /* Disable the RGB Gamma module for each output */
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_FR_GAMMA_RGB_ENABLE, 0);
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_DS_GAMMA_RGB_ENABLE, 0);
-
- /* Disable the colour correction matrix */
- mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0);
-
return 0;
}
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-isp.c b/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
index 497f25fbdd13..4c0fd1ec741c 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
@@ -112,9 +112,6 @@ static int mali_c55_isp_start(struct mali_c55 *mali_c55,
const struct v4l2_subdev_state *state)
{
struct mali_c55_context *ctx = mali_c55_get_active_context(mali_c55);
- const struct mali_c55_isp_format_info *cfg;
- const struct v4l2_mbus_framefmt *format;
- const struct v4l2_rect *crop;
u32 val;
int ret;
@@ -122,35 +119,11 @@ static int mali_c55_isp_start(struct mali_c55 *mali_c55,
MALI_C55_REG_MCU_CONFIG_WRITE_MASK,
MALI_C55_REG_MCU_CONFIG_WRITE_PING);
- /* Apply input windowing */
- crop = v4l2_subdev_state_get_crop(state, MALI_C55_ISP_PAD_SINK_VIDEO);
- format = v4l2_subdev_state_get_format(state,
- MALI_C55_ISP_PAD_SINK_VIDEO);
- cfg = mali_c55_isp_get_mbus_config_by_code(format->code);
-
- mali_c55_write(mali_c55, MALI_C55_REG_HC_START,
- MALI_C55_HC_START(crop->left));
- mali_c55_write(mali_c55, MALI_C55_REG_HC_SIZE,
- MALI_C55_HC_SIZE(crop->width));
- mali_c55_write(mali_c55, MALI_C55_REG_VC_START_SIZE,
- MALI_C55_VC_START(crop->top) |
- MALI_C55_VC_SIZE(crop->height));
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BASE_ADDR,
- MALI_C55_REG_ACTIVE_WIDTH_MASK, format->width);
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BASE_ADDR,
- MALI_C55_REG_ACTIVE_HEIGHT_MASK,
- format->height << 16);
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BAYER_ORDER,
- MALI_C55_BAYER_ORDER_MASK, cfg->order);
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_INPUT_WIDTH,
- MALI_C55_INPUT_WIDTH_MASK,
- MALI_C55_INPUT_WIDTH_20BIT);
-
- mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_ISP_RAW_BYPASS,
- MALI_C55_ISP_RAW_BYPASS_BYPASS_MASK,
- cfg->bypass ? MALI_C55_ISP_RAW_BYPASS_BYPASS_MASK :
- 0x00);
-
+ /*
+ * Apply default ISP configuration and the apply configurations from
+ * the first available parameters buffer.
+ */
+ mali_c55_params_init_isp_config(mali_c55, state);
mali_c55_params_write_config(mali_c55);
ret = mali_c55_config_write(ctx, MALI_C55_CONFIG_PING, true);
if (ret) {
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index c03a6120ddbf..c84a6047a570 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -732,6 +732,78 @@ void mali_c55_params_write_config(struct mali_c55 *mali_c55)
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
}
+void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55,
+ const struct v4l2_subdev_state *state)
+{
+ const struct mali_c55_isp_format_info *cfg;
+ const struct v4l2_mbus_framefmt *format;
+ const struct v4l2_rect *crop;
+
+ /* Apply input windowing */
+ crop = v4l2_subdev_state_get_crop(state, MALI_C55_ISP_PAD_SINK_VIDEO);
+ format = v4l2_subdev_state_get_format(state,
+ MALI_C55_ISP_PAD_SINK_VIDEO);
+ cfg = mali_c55_isp_get_mbus_config_by_code(format->code);
+
+ mali_c55_write(mali_c55, MALI_C55_REG_HC_START,
+ MALI_C55_HC_START(crop->left));
+ mali_c55_write(mali_c55, MALI_C55_REG_HC_SIZE,
+ MALI_C55_HC_SIZE(crop->width));
+ mali_c55_write(mali_c55, MALI_C55_REG_VC_START_SIZE,
+ MALI_C55_VC_START(crop->top) |
+ MALI_C55_VC_SIZE(crop->height));
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BASE_ADDR,
+ MALI_C55_REG_ACTIVE_WIDTH_MASK, format->width);
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BASE_ADDR,
+ MALI_C55_REG_ACTIVE_HEIGHT_MASK,
+ format->height << 16);
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BAYER_ORDER,
+ MALI_C55_BAYER_ORDER_MASK, cfg->order);
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_INPUT_WIDTH,
+ MALI_C55_INPUT_WIDTH_MASK,
+ MALI_C55_INPUT_WIDTH_20BIT);
+
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_ISP_RAW_BYPASS,
+ MALI_C55_ISP_RAW_BYPASS_BYPASS_MASK,
+ cfg->bypass ? MALI_C55_ISP_RAW_BYPASS_BYPASS_MASK :
+ 0x00);
+
+ /*
+ * Some features of the ISP need to be disabled by default and only
+ * enabled at the same time as they're configured by a parameters buffer
+ */
+
+ /* Bypass the sqrt and square compression and expansion modules */
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_1,
+ MALI_C55_REG_BYPASS_1_FE_SQRT,
+ MALI_C55_REG_BYPASS_1_FE_SQRT);
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_3,
+ MALI_C55_REG_BYPASS_3_SQUARE_BE,
+ MALI_C55_REG_BYPASS_3_SQUARE_BE);
+
+ /* Bypass the temper module */
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_2,
+ MALI_C55_REG_BYPASS_2_TEMPER);
+
+ /* Disable the temper module's DMA read/write */
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_TEMPER_DMA_IO, 0x0);
+
+ /* Bypass the colour noise reduction */
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_4,
+ MALI_C55_REG_BYPASS_4_CNR);
+
+ /* Disable the sinter module */
+ mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_SINTER_CONFIG,
+ MALI_C55_SINTER_ENABLE_MASK, 0);
+
+ /* Disable the RGB Gamma module for each output */
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_FR_GAMMA_RGB_ENABLE, 0);
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_DS_GAMMA_RGB_ENABLE, 0);
+
+ /* Disable the colour correction matrix */
+ mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0);
+}
+
void mali_c55_unregister_params(struct mali_c55 *mali_c55)
{
struct mali_c55_params *params = &mali_c55->params;