Re: [PATCH RESEND v2 1/5] media: dt-bindings: Add CAMSS device for SM8750

[Bryan O'Donoghue]

On 19/02/2026 01:08, Vijay Kumar Tumati wrote:
> Hi Bryan,
>
> On 2/18/2026 4:50 PM, Bryan O'Donoghue wrote:
> > On 20/01/2026 06:42, Hangxiang Ma wrote:
> > > +      - description: Registers for ICP (Imaging Control Processor) 0
> > > +      - description: Registers for ICP 0 SYS
> > > +      - description: Registers for ICP 1
> > > +      - description: Registers for ICP 1 SYS
> > > +      - description: Registers for IPE (Image Processing Engine)
> > > +      - description: Registers for JPEG DMA & Downscaler 0
> > > +      - description: Registers for JPEG Encoder 0
> > > +      - description: Registers for JPEG DMA & Downscaler 1
> > > +      - description: Registers for JPEG Encoder 1
> > > +      - description: Registers for OFE (Offline Front End)
> >
> > This is a weird map - it doesn't seem to have a BPS ?
> There is no BPS module on SM8750.
> > > +      - description: Registers for RT CDM (Camera Data Mover) 0
> > > +      - description: Registers for RT CDM 1
> > > +      - description: Registers for RT CDM 2
> > > +      - description: Registers for RT CDM 3
> > > +      - description: Registers for RT CDM 4
> >
> > I actually think these should be standalone nodes.
> >
> > I've done some prototyping work on Hamoa to bring up the BPS and IPE 
> > using the ICP and the HFI protocol.
> >
> > An absolute torrent of TLAs there but one thing that pops out of that 
> > is the current CAMSS bindings we have kind of match how camx works 
> > when there is an ICP.
> >
> > Linux/HLOS programs up the PHYs, CSID, IFE, sensor and then the ICP is 
> > tasked with owning the BPS, IPE and hiding away the complexity of the 
> > CDM.
> >
> > So to me that says we should keep CAMSS bindings as they are largely.
> >
> > I think its just messy to keep jamming registers into this map - it 
> > really is an enormous list.
> >
> > Lets revert to the simpler version and add new nodes as we enable them 
> > for OPE, IPE, BPS and ICP instead.
> >
> > OTOH I will publish the CSIPHY code you were asking for either 
> > tomorrow Thursday or Friday and I'd be obliged if you could review and 
> > ideally align with that.
> >
> > A humongous blob of a camera block seems like a legacy sin we should 
> > just fix.
> I am not sure I followed this entirely. Firstly, you weren't referring 
> to the RT CDM register blocks (although you added your comment there), 
> are you? 

I think what I'm saying is - lets leave CAMSS bindings as they are. We 
can add nodes like OPE, IPE, BPS, ICP as peers, along with CCI existing 
and CSIPHY - in progress.

This way also stops us tying our hands with bindings. For preference we 
describe all the hardware but, having played with the ICP - I have it 
booting and sending me a few cursory messages - I think extending the 
blob of CAMSS bindings is a "hiding to nothing" - sure it checks the box 
of documenting absolutely everything but it also locks us into a very 
rigid binding.

TL;DR I was wrong about that ;)

Secondly, I thought you wanted the complete HW description as
> it is supposed to be in the DT bindings, isn't it? I am not sure why you 
> think it is based on how CAMX works, rather purely based on the HW 
> blocks and registers available in the Camera sub system on SM8750 and it 
> is up to the driver, whether or not to use the ICP for offline modules, 

Yes understood but to me it seems simpler/cleaner to the use the ICP - 
for example HLOS doesn't have to know or care how to shift data in/out 
of the IPE with the CDM..

> although using ICP is generally advised for the stripe based processing.
>
> CSIPHY nodes - sure, if you advise that, we can.
Feels like a nicer interim solution.

"Do the right thing" separating out the PHYs while giving ourselves 
scope to investigate doing IPE, JPEG or whatever - likely as separate 
devices a-la ~ every other SoC.

Rockchip and Broadcom as examples.

---
bod