Re: [PATCH v5 4/5] media: qcom: camss: Add CAMSS Offline Processing Engine driver

["Gjorgji Rosikopulos (Consultant)" <gjorgji.rosikopulos@oss.qualcomm.com>]

Hi Loic,

On 7/24/2026 3:42 PM, Loic Poulain wrote:
> Add an image processing driver for the Qualcomm Offline Processing Engine
> (OPE). OPE is a memory-to-memory ISP block that converts raw Bayer
> frames to YUV, performing white balance, demosaic, chroma enhancement,
> color correction and downscaling.
> 
> The hardware architecture consists of Fetch Engines and Write Engines,
> connected through intermediate pipeline modules for pix processing.
> 
> The driver exposes three video nodes per pipeline instance:
>   - ope_input: Bayer RAW input (V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>   - ope_disp_output: YUV output     (V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>   - ope_params: ISP parameters (V4L2_BUF_TYPE_META_OUTPUT)
> 
> Hardware features:
>   - Stripe-based processing (up to 336 pixels wide per stripe)
>   - White balance (CLC_WB)
>   - Demosaic / Bayer-to-RGB (CLC_DEMO)
>   - RGB-to-YUV conversion (CLC_CHROMA_ENHAN)
>   - Color correction matrix (CLC_CC)
>   - MN downscaler for chroma and luma planes
> 
> Default configuration values are based on public standards such as BT.601.
> 
> Processing Model:
> OPE processes frames in stripes of up to 336 pixels. Therefore, frames
> must be split into stripes for processing. Each stripe is configured after
> the previous one has been acquired (double buffered registers). To minimize
> inter-stripe latency, stripe configurations are generated ahead of time.
> 
> The driver is split into three source files under the ope/ directory:
> 
>   - core.c: the OPE m2m driver itself: probe, power management, V4L2/media
>     device setup, format handling, stripe generation and hardware
>     programming.
> 
>   - pipeline.c/.h: a small declarative media-controller topology builder.
>     Drivers describe their entire media graph, entities (video devices,
>     subdevs, or base entities), their pads, and the links between them, in
>     a static descriptor table. The builder validates the table, allocates
>     and registers all entities, and creates all MC pad links. It is kept
>     generic but currently only used by OPE.
> 
>   - params.c/.h: V4L2 ISP parameter buffer validation and dispatch. It
>     wraps the extensible V4L2 ISP parameters buffer format, validating
>     the buffer size and each per-block header before forwarding every
>     block to its driver-supplied handler.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Co-developed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/camss/Kconfig        |    2 +
>  drivers/media/platform/qcom/camss/Makefile       |    2 +
>  drivers/media/platform/qcom/camss/ope/Kconfig    |   16 +
>  drivers/media/platform/qcom/camss/ope/Makefile   |    9 +
>  drivers/media/platform/qcom/camss/ope/core.c     | 3353 ++++++++++++++++++++++
>  drivers/media/platform/qcom/camss/ope/params.c   |   75 +
>  drivers/media/platform/qcom/camss/ope/params.h   |   62 +
>  drivers/media/platform/qcom/camss/ope/pipeline.c |  399 +++
>  drivers/media/platform/qcom/camss/ope/pipeline.h |  237 ++
>  9 files changed, 4155 insertions(+)
> 

<snip>

> +static int ope_open(struct file *file)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct ope_dev *ope = container_of(vdev->v4l2_dev, struct ope_dev, v4l2_dev);
> +	struct ope_ctx *ctx;
> +	struct v4l2_fh *fh;
> +	int ret = 0;
> +
> +	fh = kzalloc(sizeof(*fh), GFP_KERNEL);
> +	if (!fh)
> +		return -ENOMEM;
> +
> +	if (mutex_lock_interruptible(&ope->mutex)) {
> +		kfree(fh);
> +		return -ERESTARTSYS;
> +	}
> +
> +	/*
> +	 * For now, only a single shared context is supported,
> +	 * until media multi-context support is available.
> +	 */
> +	if (!ope->shared_ctx) {
> +		ctx = ope_ctx_create(ope);
> +		if (IS_ERR(ctx)) {
> +			ret = PTR_ERR(ctx);
> +			goto unlock;
> +		}
> +	} else {
> +		ctx = ope->shared_ctx;
> +	}

I am sure there were multiple discussions on how to implement multi-context. But this exact implementation somehow confuses me (maybe just me).

This allows opening the same video node twice (e.g. the output node). vb2_is_busy protection is present everywhere to prevent races,
but if two applications open the same video node the driver may end up in a situation where the format set by the first application is overridden
by the second, but streaming somehow gets started on the first with the wrong format. The danger is not at the format-set call itself but at start_streaming, 
when the hardware gets programmed with whatever format is current in the shared context.

Maybe it makes sense to have a tiny wrapper on each OPE V4L2 video device that holds an open count and prevents opening the same video node twice,
returning -EBUSY on a second open(), until multi-context support is available as mentioned here.

Regards,
~Gjorgji