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

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

On Tue, Mar 24, 2026 at 11:00:21AM +0000, Bryan O'Donoghue wrote:
> On 23/03/2026 15:31, Loic Poulain wrote:
> > > > +
> > > > +static void ope_prog_bayer2rgb(struct ope_dev *ope)
> > > > +{
> > > > +     /* Fixed Settings */
> > > > +     ope_write_pp(ope, 0x860, 0x4001);
> > > > +     ope_write_pp(ope, 0x868, 128);
> > > > +     ope_write_pp(ope, 0x86c, 128 << 20);
> > > > +     ope_write_pp(ope, 0x870, 102);
> > > What are the magic numbers about ? Please define bit-fields and offsets.
> > There are some registers I can't disclose today, which have to be
> > configured with working values,
> > Similarly to some sensor configuration in media/i2c.
> 
> Not really the same thing, all of the offsets in upstream CAMSS and its CLC
> are documented. Sensor values are typically upstreamed by people who don't
> control the documentation, that is not the case with Qcom submitting this
> code upstream now.
> 
> Are you guys doing an upstream implementation or not ?

And there are enough upstream implementations, even coming from the
vendors, without (or with the minimal) register specifications.

> 
> > As far as I understand, CDM could also be implemented in a generic way
> > within CAMSS, since other CAMSS blocks make use of CDM as well.
> > This is something we should discuss further.
> My concern is even conservatively if each module adds another 10 ? writes by
> the time we get to denoising, sharpening, lens shade correction, those
> writes could easily look more like 100.
> 
> What user-space should submit is well documented data-structures which then
> get translated into CDM buffers by the OPE and IFE for the various bits of
> the pipeline.

I hope here you have accent on the well-documented (ideally some kind of
the vendor-independent ABI).

-- 
With best wishes
Dmitry