From: Dan Scally <dan.scally@ideasonboard.com>
To: "Jacopo Mondi" <jacopo.mondi@ideasonboard.com>,
"Anthony McGivern" <anthony.mcgivern@arm.com>,
"Vincenzo Frascino" <vincenzo.frascino@arm.com>,
"Nayden Kanchev" <Nayden.Kanchev@arm.com>,
"Konstantin Babin" <Konstantin.Babin@arm.com>,
"Barnabás Pőcze" <barnabas.pocze@ideasonboard.com>,
"Mauro Carvalho Chehab" <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH v2 2/6] media: mali-c55: Initialize the ISP in enable_streams()
Date: Fri, 13 Mar 2026 15:59:22 +0000 [thread overview]
Message-ID: <fc5b4699-37b8-41fe-ab9d-b1ac7fbe33e2@ideasonboard.com> (raw)
In-Reply-To: <20260313-mali-c55-fixes-v7-0-v2-2-885c07961f30@ideasonboard.com>
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;
>
next prev parent reply other threads:[~2026-03-13 15:59 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
2026-03-13 14:54 ` [PATCH v2 5/6] media: mali-c55: Bypass the Iridix Tonemap engine Jacopo Mondi
2026-03-13 16:04 ` Dan Scally
2026-03-13 16:18 ` Barnabás Pőcze
2026-03-13 14:54 ` [PATCH v2 6/6] media: mali-c55: Bypass Purple Fringe Correction Jacopo Mondi
2026-03-13 16:04 ` Dan Scally
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc5b4699-37b8-41fe-ab9d-b1ac7fbe33e2@ideasonboard.com \
--to=dan.scally@ideasonboard.com \
--cc=Konstantin.Babin@arm.com \
--cc=Nayden.Kanchev@arm.com \
--cc=anthony.mcgivern@arm.com \
--cc=barnabas.pocze@ideasonboard.com \
--cc=jacopo.mondi@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=stable@vger.kernel.org \
--cc=vincenzo.frascino@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®