Re: [PATCH 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

On 2025-03-17 16:02:34 +0100, Krzysztof Kozlowski wrote:
> On 17/03/2025 12:49, Niklas Söderlund wrote:
> > Hi Krzysztof,
> > 
> > Thanks for your feedback.
> > 
> > On 2025-03-17 12:31:51 +0100, Krzysztof Kozlowski wrote:
> >> On Sat, Mar 15, 2025 at 04:27:02PM +0100, Niklas Söderlund wrote:
> >>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>> index c4de4555b753..de9bc739e084 100644
> >>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> >>> @@ -25,19 +25,54 @@ properties:
> >>>            - renesas,r8a779h0-isp # V4M
> >>>        - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> >>>    reg:
> >>> -    maxItems: 1
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>> +
> >>> +  reg-names:
> >>> +    minItems: 1
> >>> +    items:
> >>> +      - const: cs
> >>> +      - const: core
> >>
> >> All of this and further must be restricted per compatible. Otherwise
> >> commit msg should explain why one SoC can have it different on different
> >> boards.
> > 
> > I will expand the commit message. In short this can't be restricted per 
> > compatible, different instances of the IP on the same board can and can 
> > not have a core part.
> 
> s/Same board/same SoC/? Or you really meant that same SoC on different
> boards will have or have not that ISP core?
> 
> Both are odd, first even weirder.
> 
> I wonder if some other difference is not the documented. E.g. same IP
> blocks are not exactly the same, but have different programming model.

The IP block named "ISP" is in fact two different things on R-Car Gen4 
SoCs. I know it's confusing and not logical but that's how they are 
made.

- One part is the ISP Channel Selector, this is a function that sits on 
  the CIS-2 receiver data bus. It is responsible for selecting which 
  CSI-2 Virtual Channel is routed to which DMA capture engine.

  This part is what the rcar-isp.ko driver have always supported and 
  every instance of the ISP that is described in tree deals with just 
  this one function as this is the one we always had documentation for.

  This block is the one the reg-names and clock-names labels "cs".

- One part that we now wish to add is the ISP Core. This is a 
  traditional ISP that act on image data in different ways. This is what 
  I try to model with the reg-name and clock-name labeled "core".

  This is new and we have not had documentation for this until recently.  
  Unfortunately the "core" and "cs" functions is implemented in the same 
  IP block. And to operate the "core" one needs to also deal with "cs".  

To make it more interesting all ISP Channel Selectors (cs) do not have a 
companion ISP Core, but most do. The lack of a ISP core is OK, it just 
means that video capture path can't manipulate the image as much as 
paths that do.

It's not ideal but to support the ISP Core and ISP Channel Selecotr the 
rcar-isp.ko module needs both "core" and "cs" clocks and regs. And to 
support just the Channel Selector it only needs the "cs" resources.


Sorry if I have been confusing. A good example of this is patch 4/7 in 
this series. It shows the V4M board that have 2 ISP instances, one that 
have both cs and core functions, and one that only have cs function.

> 
> What is this ISP core responsible for inside Renesas ISP? How many ISPs
> are inside of SoC?

The ISP core is responsible for image manipulation. ISP Channel Selector 
for CSI-2 channel routing. The number of ISP varies between SoCs:


V3U: Have 4 ISP instances, all 4 have cs and core.
V4H: Have 2 ISP instances, both have cs and core.
V4M: Have 2 ISP instances, one have cs and core, one have only cs.

> 
> And how would it work? You have two exactly the same IP blocks in the
> SoC, but one you program differently than other. How do you know which one?

All cs blocks are the same on all SoCs. The core block is the same on 
V4H and V4M, and different on V3U.

> 
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund