* [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass
@ 2026-03-13 14:53 Jacopo Mondi
2026-03-13 14:53 ` [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types Jacopo Mondi
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Jacopo Mondi @ 2026-03-13 14:53 UTC (permalink / raw)
To: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev,
Konstantin Babin, Daniel Scally, Barnabás Pőcze,
Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Jacopo Mondi, stable
The ISP is initialized in two different points (s_stream and probe).
This cause two consecutive start streaming to use settings from the
previous one.
Also, not all the blocks that userspace can configure are properly
reset.
Fix that and also bypass a few ISP blocks which if not bypassed might
interfere with the image processing.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Changes in v2:
- Collect tags and fix a wrong "Fixes:" tag
- Link to v1: https://lore.kernel.org/r/20260313-mali-c55-fixes-v7-0-v1-0-21805b2b516b@ideasonboard.com
---
Daniel Scally (1):
media: mali-c55: Fix Iridix bypass macros
Jacopo Mondi (5):
media: mali-c55: Fix wrong comment of ISP block types
media: mali-c55: Initialize the ISP in enable_streams()
media: mali-c55: Fully reset the ISP configuration
media: mali-c55: Bypass the Iridix Tonemap engine
media: mali-c55: Bypass Purple Fringe Correction
.../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 | 134 ++++++++++++++++++++-
.../platform/arm/mali-c55/mali-c55-registers.h | 4 +-
5 files changed, 140 insertions(+), 72 deletions(-)
---
base-commit: f6390408a846aacc2171c17d88b062e202d84e86
change-id: 20260311-mali-c55-fixes-v7-0-5db2a04ea818
Best regards,
--
Jacopo Mondi <jacopo.mondi@ideasonboard.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types 2026-03-13 14:53 [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass Jacopo Mondi @ 2026-03-13 14:53 ` Jacopo Mondi 2026-03-13 16:22 ` Barnabás Pőcze 2026-03-13 14:53 ` [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() Jacopo Mondi ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 14:53 UTC (permalink / raw) To: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Daniel Scally, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, Jacopo Mondi, stable Some bad copy&paste happened in the description of the ISP block types and AWB_CONFIG got mixed up with SHADING_CONFIG. Fix it by assigning to each block the correct type. As only the comment is changed, there is no uABI breakage or regression. Cc: stable@vger.kernel.org Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- drivers/media/platform/arm/mali-c55/mali-c55-params.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 be0e909bcf29..c03a6120ddbf 100644 --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c @@ -43,9 +43,9 @@ * @digital_gain: For header->type == MALI_C55_PARAM_BLOCK_DIGITAL_GAIN * @awb_gains: For header->type == MALI_C55_PARAM_BLOCK_AWB_GAINS and * header->type = MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP - * @awb_config: For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG - * @shading_config: For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION - * @shading_selection: For header->type == MALI_C55_PARAM_BLOCK_SENSOR_OFFS + * @awb_config: For header->type == MALI_C55_PARAM_BLOCK_AWB_CONFIG + * @shading_config: For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG + * @shading_selection: For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION * @data: Allows easy initialisation of a union variable with a * pointer into a __u8 array. */ -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types 2026-03-13 14:53 ` [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types Jacopo Mondi @ 2026-03-13 16:22 ` Barnabás Pőcze 0 siblings, 0 replies; 12+ messages in thread From: Barnabás Pőcze @ 2026-03-13 16:22 UTC (permalink / raw) To: Jacopo Mondi, Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Daniel Scally, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, stable 2026. 03. 13. 15:53 keltezéssel, Jacopo Mondi írta: > Some bad copy&paste happened in the description of the ISP block types > and AWB_CONFIG got mixed up with SHADING_CONFIG. > > Fix it by assigning to each block the correct type. > > As only the comment is changed, there is no uABI breakage or regression. > > Cc: stable@vger.kernel.org > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > drivers/media/platform/arm/mali-c55/mali-c55-params.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > 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 be0e909bcf29..c03a6120ddbf 100644 > --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c > @@ -43,9 +43,9 @@ > * @digital_gain: For header->type == MALI_C55_PARAM_BLOCK_DIGITAL_GAIN > * @awb_gains: For header->type == MALI_C55_PARAM_BLOCK_AWB_GAINS and > * header->type = MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP > - * @awb_config: For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG > - * @shading_config: For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION > - * @shading_selection: For header->type == MALI_C55_PARAM_BLOCK_SENSOR_OFFS > + * @awb_config: For header->type == MALI_C55_PARAM_BLOCK_AWB_CONFIG > + * @shading_config: For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG > + * @shading_selection: For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION > * @data: Allows easy initialisation of a union variable with a > * pointer into a __u8 array. > */ > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 14:53 [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass Jacopo Mondi 2026-03-13 14:53 ` [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types Jacopo Mondi @ 2026-03-13 14:53 ` Jacopo Mondi 2026-03-13 15:59 ` Dan Scally 2026-03-13 14:53 ` [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration Jacopo Mondi 2026-03-13 14:54 ` [PATCH v2 4/6] media: mali-c55: Fix Iridix bypass macros Jacopo Mondi 3 siblings, 1 reply; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 14:53 UTC (permalink / raw) To: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Daniel Scally, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, Jacopo Mondi, stable 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 the ISP block bypass configuration and the image processing path configuration in a single function to be called at enabled_streams() time. Cc: stable@vger.kernel.org Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- .../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; -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 14:53 ` [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() Jacopo Mondi @ 2026-03-13 15:59 ` Dan Scally 2026-03-13 16:10 ` Jacopo Mondi 0 siblings, 1 reply; 12+ messages in thread From: Dan Scally @ 2026-03-13 15:59 UTC (permalink / raw) To: Jacopo Mondi, Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, stable 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? Thanks Dan > Cc: stable@vger.kernel.org > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > .../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; > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 15:59 ` Dan Scally @ 2026-03-13 16:10 ` Jacopo Mondi 2026-03-13 16:14 ` Dan Scally 0 siblings, 1 reply; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 16:10 UTC (permalink / raw) To: Dan Scally Cc: Jacopo Mondi, Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab, linux-media, linux-kernel, stable 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 ? Thanks j > > Thanks > Dan > > > Cc: stable@vger.kernel.org > > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > .../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; > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 16:10 ` Jacopo Mondi @ 2026-03-13 16:14 ` Dan Scally 2026-03-13 17:00 ` Jacopo Mondi 0 siblings, 1 reply; 12+ messages in thread From: Dan Scally @ 2026-03-13 16:14 UTC (permalink / raw) To: Jacopo Mondi Cc: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab, linux-media, linux-kernel, stable 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. Thanks Dan > > Thanks > j > >> >> Thanks >> Dan >> >>> Cc: stable@vger.kernel.org >>> Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> --- >>> .../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; >>> >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 16:14 ` Dan Scally @ 2026-03-13 17:00 ` Jacopo Mondi 2026-03-16 9:34 ` Dan Scally 0 siblings, 1 reply; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 17:00 UTC (permalink / raw) To: Dan Scally Cc: Jacopo Mondi, Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab, linux-media, linux-kernel, stable 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 ? > > Thanks > Dan > > > > > Thanks > > j > > > > > > > > Thanks > > > Dan > > > > > > > Cc: stable@vger.kernel.org > > > > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > --- > > > > .../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; > > > > > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() 2026-03-13 17:00 ` Jacopo Mondi @ 2026-03-16 9:34 ` Dan Scally 0 siblings, 0 replies; 12+ messages in thread From: Dan Scally @ 2026-03-16 9:34 UTC (permalink / raw) To: Jacopo Mondi Cc: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab, linux-media, linux-kernel, stable 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@ideasonboard.com> > > >> >> Thanks >> Dan >> >>> >>> Thanks >>> j >>> >>>> >>>> Thanks >>>> Dan >>>> >>>>> Cc: stable@vger.kernel.org >>>>> Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") >>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>>> --- >>>>> .../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; >>>>> >>>> >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration 2026-03-13 14:53 [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass Jacopo Mondi 2026-03-13 14:53 ` [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types Jacopo Mondi 2026-03-13 14:53 ` [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() Jacopo Mondi @ 2026-03-13 14:53 ` Jacopo Mondi 2026-03-13 16:01 ` Dan Scally 2026-03-13 14:54 ` [PATCH v2 4/6] media: mali-c55: Fix Iridix bypass macros Jacopo Mondi 3 siblings, 1 reply; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 14:53 UTC (permalink / raw) To: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Daniel Scally, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, Jacopo Mondi, stable The Mali C55 driver uses an auto-suspend delay of 2000 milli-seconds. As the delay is quite large, it is certainly possible that two consecutive calls to enable_streams() do not go through a suspend of the peripheral, meaning we cannot rely on POW register values for the ISP configuration. To prevent a streaming session to be initialized with settings from the previous one, reset the full ISP configuration to know state disabling or bypassing all the ISP blocks the driver supports. Cc: stable@vger.kernel.org Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- .../media/platform/arm/mali-c55/mali-c55-params.c | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) 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 c84a6047a570..773e5b6a2b7e 100644 --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c @@ -781,6 +781,43 @@ void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55, MALI_C55_REG_BYPASS_3_SQUARE_BE, MALI_C55_REG_BYPASS_3_SQUARE_BE); + /* Bypass the sensor offset correction (BLS) module */ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_3, + MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH, + MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH); + + /* Configure 1x digital gain. */ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_DIGITAL_GAIN, + MALI_C55_DIGITAL_GAIN_MASK, 256); + + /* Set all AWB gains to 1x. at both AWB configuration points*/ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1, + MALI_C55_AWB_GAIN00_MASK, 256); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1, + MALI_C55_AWB_GAIN01_MASK, + MALI_C55_AWB_GAIN01(256)); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2, + MALI_C55_AWB_GAIN10_MASK, 256); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2, + MALI_C55_AWB_GAIN11_MASK, + MALI_C55_AWB_GAIN11(256)); + + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1_AEXP, + MALI_C55_AWB_GAIN00_MASK, 256); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1_AEXP, + MALI_C55_AWB_GAIN01_MASK, + MALI_C55_AWB_GAIN01(256)); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2_AEXP, + MALI_C55_AWB_GAIN10_MASK, 256); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2_AEXP, + MALI_C55_AWB_GAIN11_MASK, + MALI_C55_AWB_GAIN11(256)); + + /* Bypass mesh shading corrections (LSC). */ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_MESH_SHADING_CONFIG, + MALI_C55_MESH_SHADING_ENABLE_MASK, + false); + /* Bypass the temper module */ mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_2, MALI_C55_REG_BYPASS_2_TEMPER); @@ -802,6 +839,19 @@ void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55, /* Disable the colour correction matrix */ mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0); + + /* Disable AWB stats. */ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, + MALI_C55_AWB_DISABLE_MASK, + MALI_C55_AWB_DISABLE_MASK); + + /* Disable auto-exposure 1024-bin histograms at both tap points. */ + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, + MALI_C55_AEXP_HIST_DISABLE_MASK, + MALI_C55_AEXP_HIST_DISABLE); + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, + MALI_C55_AEXP_IHIST_DISABLE_MASK, + MALI_C55_AEXP_IHIST_DISABLE); } void mali_c55_unregister_params(struct mali_c55 *mali_c55) -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration 2026-03-13 14:53 ` [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration Jacopo Mondi @ 2026-03-13 16:01 ` Dan Scally 0 siblings, 0 replies; 12+ messages in thread From: Dan Scally @ 2026-03-13 16:01 UTC (permalink / raw) To: Jacopo Mondi, Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, stable Hi Jacopo On 13/03/2026 14:53, Jacopo Mondi wrote: > The Mali C55 driver uses an auto-suspend delay of 2000 milli-seconds. > > As the delay is quite large, it is certainly possible that two > consecutive calls to enable_streams() do not go through a suspend of the > peripheral, meaning we cannot rely on POW register values for the ISP > configuration. > > To prevent a streaming session to be initialized with settings from the > previous one, reset the full ISP configuration to know state disabling or > bypassing all the ISP blocks the driver supports. Oh that's annoying - good spot...perhaps the auto-suspend delay should be reduced too, though I think this change is prudent anyway: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > Cc: stable@vger.kernel.org > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > .../media/platform/arm/mali-c55/mali-c55-params.c | 50 ++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > 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 c84a6047a570..773e5b6a2b7e 100644 > --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c > @@ -781,6 +781,43 @@ void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55, > MALI_C55_REG_BYPASS_3_SQUARE_BE, > MALI_C55_REG_BYPASS_3_SQUARE_BE); > > + /* Bypass the sensor offset correction (BLS) module */ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_BYPASS_3, > + MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH, > + MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH); > + > + /* Configure 1x digital gain. */ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_DIGITAL_GAIN, > + MALI_C55_DIGITAL_GAIN_MASK, 256); > + > + /* Set all AWB gains to 1x. at both AWB configuration points*/ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1, > + MALI_C55_AWB_GAIN00_MASK, 256); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1, > + MALI_C55_AWB_GAIN01_MASK, > + MALI_C55_AWB_GAIN01(256)); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2, > + MALI_C55_AWB_GAIN10_MASK, 256); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2, > + MALI_C55_AWB_GAIN11_MASK, > + MALI_C55_AWB_GAIN11(256)); > + > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1_AEXP, > + MALI_C55_AWB_GAIN00_MASK, 256); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS1_AEXP, > + MALI_C55_AWB_GAIN01_MASK, > + MALI_C55_AWB_GAIN01(256)); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2_AEXP, > + MALI_C55_AWB_GAIN10_MASK, 256); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_AWB_GAINS2_AEXP, > + MALI_C55_AWB_GAIN11_MASK, > + MALI_C55_AWB_GAIN11(256)); > + > + /* Bypass mesh shading corrections (LSC). */ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_MESH_SHADING_CONFIG, > + MALI_C55_MESH_SHADING_ENABLE_MASK, > + false); > + > /* Bypass the temper module */ > mali_c55_ctx_write(mali_c55, MALI_C55_REG_BYPASS_2, > MALI_C55_REG_BYPASS_2_TEMPER); > @@ -802,6 +839,19 @@ void mali_c55_params_init_isp_config(struct mali_c55 *mali_c55, > > /* Disable the colour correction matrix */ > mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0); > + > + /* Disable AWB stats. */ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, > + MALI_C55_AWB_DISABLE_MASK, > + MALI_C55_AWB_DISABLE_MASK); > + > + /* Disable auto-exposure 1024-bin histograms at both tap points. */ > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, > + MALI_C55_AEXP_HIST_DISABLE_MASK, > + MALI_C55_AEXP_HIST_DISABLE); > + mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_METERING_CONFIG, > + MALI_C55_AEXP_IHIST_DISABLE_MASK, > + MALI_C55_AEXP_IHIST_DISABLE); > } > > void mali_c55_unregister_params(struct mali_c55 *mali_c55) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/6] media: mali-c55: Fix Iridix bypass macros 2026-03-13 14:53 [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass Jacopo Mondi ` (2 preceding siblings ...) 2026-03-13 14:53 ` [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration Jacopo Mondi @ 2026-03-13 14:54 ` Jacopo Mondi 3 siblings, 0 replies; 12+ messages in thread From: Jacopo Mondi @ 2026-03-13 14:54 UTC (permalink / raw) To: Anthony McGivern, Vincenzo Frascino, Nayden Kanchev, Konstantin Babin, Daniel Scally, Barnabás Pőcze, Mauro Carvalho Chehab Cc: linux-media, linux-kernel, Jacopo Mondi, stable From: Daniel Scally <dan.scally@ideasonboard.com> The Mali C55 Iridix block has a digital gain function and tone mapping function, whose enablement is controlled by two different bits in the BYPASS_3 register. Unfortunately, the "Gain" and "Tonemap" bypass bit definitions are the wrong way around. Swap them. Cc: stable@vger.kernel.org Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver") Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- drivers/media/platform/arm/mali-c55/mali-c55-registers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h index f5a148add1c8..f098effde7b4 100644 --- a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h +++ b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h @@ -128,8 +128,8 @@ enum mali_c55_interrupts { #define MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH BIT(1) #define MALI_C55_REG_BYPASS_3_MESH_SHADING BIT(3) #define MALI_C55_REG_BYPASS_3_WHITE_BALANCE BIT(4) -#define MALI_C55_REG_BYPASS_3_IRIDIX BIT(5) -#define MALI_C55_REG_BYPASS_3_IRIDIX_GAIN BIT(6) +#define MALI_C55_REG_BYPASS_3_IRIDIX_GAIN BIT(5) +#define MALI_C55_REG_BYPASS_3_IRIDIX BIT(6) #define MALI_C55_REG_BYPASS_4 0x18ec0 #define MALI_C55_REG_BYPASS_4_DEMOSAIC_RGB BIT(1) #define MALI_C55_REG_BYPASS_4_PF_CORRECTION BIT(3) -- 2.53.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-03-16 9:34 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-13 14:53 [PATCH v2 0/6] media: mali-c55: Fix ISP reset and blocks bypass Jacopo Mondi 2026-03-13 14:53 ` [PATCH v2 1/6] media: mali-c55: Fix wrong comment of ISP block types Jacopo Mondi 2026-03-13 16:22 ` Barnabás Pőcze 2026-03-13 14:53 ` [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams() Jacopo Mondi 2026-03-13 15:59 ` Dan Scally 2026-03-13 16:10 ` Jacopo Mondi 2026-03-13 16:14 ` Dan Scally 2026-03-13 17:00 ` Jacopo Mondi 2026-03-16 9:34 ` Dan Scally 2026-03-13 14:53 ` [PATCH v2 3/6] media: mali-c55: Fully reset the ISP configuration Jacopo Mondi 2026-03-13 16:01 ` Dan Scally 2026-03-13 14:54 ` [PATCH v2 4/6] media: mali-c55: Fix Iridix bypass macros Jacopo Mondi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox