Re: [RFC PATCH 2/3] media: qcom: camss: Add CAMSS Offline Processing Engine driver

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Mar 30, 2026 at 09:07:59PM +0200, Loic Poulain wrote:
> On Mon, Mar 30, 2026 at 4:33 PM Bryan O'Donoghue wrote:
> > On 30/03/2026 15:27, johannes.goede@oss.qualcomm.com wrote:
> > >> That's another reason I bring up CDM again and again. We probably
> > >> don't want to fix to the wrong format for OPE, introduce the CDM
> > >> and then find we have to map from one format to another for large
> > >> and complex data over and over again for each frame or every N
> > >> frames.
> > > 
> > > CDM is a much lower-level API then what is expected from
> > > a media-controller centric V4L2 driver. Basically the OPE
> > > driver will export:
> >
> > My concern is about wrappering one thing inside of another thing and
> > then stuffing it again back into CDM and doing the same on the way out.
> 
> I think there will always be some level of copying involved. That
> said, we can pre‑build the CDM sequence in the drivers and only update
> the variable values, which should avoid significant overhead.
> 
> If we start handling CDM formats directly on the user side, it would
> require exposing a lot of low‑level knowledge there (such as register
> layouts and offsets), and that would diverge from how other ISP
> implementations are structured. I’m concerned this would increase
> complexity and reduce portability.

Agreed, I don't think we should go in that direction. Translating the
parameters buffer to the format expecting by the CDM can be done in the
kernel in userspace context, and work in the IRQ handler will then
become minimal. As far as I understand the CDM expects a buffer that
contains register address and value pairs. This is exactly what the
R-Car V4H does, the rppx1 driver translates the parameters buffer to the
same register addresses and values format, and then passes it to the
VSP (which has the same role as the CDM here).

As mentioned in a separate e-mail, we also support programming the ISP
through MMIO. This creates more work in IRQ context, but is very useful
during development. Switching to MMIO just requires a different code
path in the IRQ handler that iterates over the registers
addresses/values in the VSP buffer, and writes to registers directly.
The architecture is very modular.

> > There are already 50 MMIO writes in the OPE ISR, I don't believe it is
> > sustainable to keep adding MMIO into that.
> 
> Yes, I understand the concern. From our testing so far, however, this
> has not shown to be an issue. In addition, a full reconfiguration
> would only happen in specific cases, such as on explicit full
> configuration changes or during context switching. We can certainly
> look at implementing CDM, but at this stage it didn't seem to bring
> significant benefits, so I prefered to focus on other functional
> aspects, and revisit CDM once there is a clearer need, measurable
> gain, or if it becomes part of the uAPI as discussed here.

I agree. Let's design the driver with CDM in mind to have the right
abstraction layers, and work on CDM support in a second step. If someone
believes this should be done urgently, they can even help by working in
parallel with ISP features enablement.

> > I'm aware of a project in qcom that did something with making the CDM
> > format in libcamera and handed that off to kernel, recommend looking
> > into that.
> 
> I will, thanks, I'm however, concerned about how acceptable this
> approach would be to the wider community and to the maintainers.

-- 
Regards,

Laurent Pinchart