From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
To: Dan Scally <dan.scally@ideasonboard.com>
Cc: "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>,
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 17:10:26 +0100 [thread overview]
Message-ID: <abQ2rGd9GUQZM3mb@zed> (raw)
In-Reply-To: <fc5b4699-37b8-41fe-ab9d-b1ac7fbe33e2@ideasonboard.com>
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;
> >
>
next prev parent reply other threads:[~2026-03-13 16:10 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
2026-03-13 16:10 ` Jacopo Mondi [this message]
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=abQ2rGd9GUQZM3mb@zed \
--to=jacopo.mondi@ideasonboard.com \
--cc=Konstantin.Babin@arm.com \
--cc=Nayden.Kanchev@arm.com \
--cc=anthony.mcgivern@arm.com \
--cc=barnabas.pocze@ideasonboard.com \
--cc=dan.scally@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.