Re: [PATCH v2 06/13] media: stm32: dcmipp: configure csi input of all pipes on stm32mp25

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete multi-pad format propagation and hardware configuration logic.
--

commit 11526b79984d515f0895343b0327ef2d55f0975d
Author: Alain Volmat <alain.volmat@foss.st.com>

    media: stm32: dcmipp: configure csi input of all pipes on stm32mp25

This patch adds two additional pixel pipes (source pads 2 and 3) to the
dcmipp-input subdevice for the STM32MP25. It modifies the hardware register
configuration to allow routing input streams to all available pipes.

> diff --git a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-input.c b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-input.c
> index 75421035cfad5..3fc859f1cb14a 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-input.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmipp/dcmipp-input.c
[ ... ]
> @@ -383,7 +389,8 @@ static int dcmipp_inp_configure_parallel(struct dcmipp_inp_device *inp,
>  }
>  
>  static int dcmipp_inp_configure_csi(struct dcmipp_inp_device *inp,
> -				    struct v4l2_subdev_state *state)
> +				    struct v4l2_subdev_state *state,
> +				    u32 pad)
>  {
>  	const struct dcmipp_inp_pix_map *vpix;
>  	struct v4l2_mbus_framefmt *sink_fmt;

[Severity: High]
Since this function now accepts a pad argument, should it be updated to use it
when retrieving the source format?

Currently, dcmipp_inp_configure_csi() hardcodes the format lookup to pad 1:

	/* Get format information */
	sink_fmt = v4l2_subdev_state_get_format(state, 0);
	src_fmt = v4l2_subdev_state_get_format(state, 1);

If a user configures a pipeline using pad 2 or 3, the driver will incorrectly
configure the hardware using pad 1's format parameters. For instance, if pad 1
is left at a default JPEG format, attempting to enable streams on pad 2 could
fail with "JPEG only available on pipe 0" because the driver evaluates the
format of pad 1 instead of pad 2.

[ ... ]
> @@ -533,8 +554,10 @@ struct dcmipp_ent_device *dcmipp_inp_ent_init(const char *entity_name,
>  	struct dcmipp_inp_device *inp;
>  	const unsigned long pads_flag[] = {
>  		MEDIA_PAD_FL_SINK, MEDIA_PAD_FL_SOURCE,
> +		MEDIA_PAD_FL_SOURCE, MEDIA_PAD_FL_SOURCE,
>  	};

[Severity: High]
With the addition of these new source pads (pads 2 and 3), does
dcmipp_inp_set_fmt() also need to be updated to propagate the sink format
to all source pads?

Currently, dcmipp_inp_set_fmt() only propagates the format to pad 1:

	/* When setting the sink format, report that format on the src pad */
	if (IS_SINK(fmt->pad)) {
		mf = v4l2_subdev_state_get_format(sd_state, 1);
		*mf = fmt->format;
		dcmipp_inp_adjust_fmt(inp, mf, 1);
	}

This could leave pads 2 and 3 uninitialized or in a stale state, violating
V4L2 subdev API expectations and potentially causing
v4l2_subdev_link_validate() to fail when setting up a pipeline through the
new pipes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-stm32-dcmipp-pixel-pipes-support-v2-0-3c76b5f93157@foss.st.com?part=6