Re: [PATCH v3 3/3] arm64: dts: qcom: sm8250: add camcc interconnect cells

[Atanas Filipov <atanas.filipov@oss.qualcomm.com>]

On 10/20/2026 12:45 PM, Dmitry Baryshkov wrote:
> On Tue, Oct 20, 2026 at 11:26:42AM +0300, Atanas Filipov wrote:
>> On 10/19/2026 5:50 PM, Dmitry Baryshkov wrote:
>>> On Fri, Oct 16, 2026 at 08:07:01PM +0300, Atanas Filipov wrote:
>>>> Add #interconnect-cells to the camera clock controller so camera
>>>> consumers can request camera NoC AXI bandwidth through the
>>>> interconnect framework.
>>>
>>> I guess currently we have only one consumer, camss. Could you please
>>> include patches converting it to the bus voting? Be sure to retain
>>> compatibility with older DTs, which don't have ICC provided by the camcc
>>> node.
>>>
>>
>> Before converting CAMSS I would like to agree on the approach, since it
>> cannot simply follow JPEG. In either case the camcc path would be
>> optional, so older DTs keep working as before.
> 
> How does the current camss vote on the clock?  JPEG is not (yet) merged,
> so we can't reference it here. The order in which the patches can land
> would be: icc-clk, camss using the (optional) interconnect, JPEG on top.

It does not vote. Each VFE calls clk_set_rate() directly from
vfe_set_clock_rates(), based on the pixel rate of its own lines only.

Where clk_set_rate() is used on camnoc_axi (SC7280, SC8280XP, SA8775P),
the clock is shared by all VFEs, but:

  - the last VFE to power up wins and may lower the rate under a VFE
    that is already streaming;
  - vfe_check_clock_rates() checks only the VFE's own needs and does not
    notice changes made by other VFEs;
  - vfe_put() never drops the rate.

On the remaining SoCs the camnoc clocks have no levels and are only
enabled. On SM8250 the levels sit on "camnoc_axi_src", which
vfe_match_clock_names() does not match, so they are never applied.

> 
> Please note Krzysztof's comments.

Yes. I will reply to Krzysztof directly in 1/3. The CAMSS behaviour
above is part of that answer: today every consumer sets the shared
camnoc rate directly and nothing aggregates the requests.

> 
>> OPP is per device, and CAMSS is a single device containing several VFEs,
>> each with its own core clock and its own load. If CAMSS used
>> dev_pm_opp_set_opp() on its device like JPEG does, every VFE would overwrite
>> the clock and bandwidth votes of the others, which is the same
>> last-writer-wins problem in a different place.
> 
> For this series just convert the existing approach as is. We have two
> distinct questions here:
>   - Letting camss and JPEG (and others) to vote independently on the
>     clock
>   - Letting camss correctly handle per-VFE votes
> 
> This series must address the first question only.
> 
>> I see two ways to keep the VFEs independent:
>>
>> 1. One OPP table in the CAMSS node, used only as a lookup. Each VFE
>>     finds the OPP for its own required rate, sets its own clock and
>>     votes the bandwidth from that OPP on its own ICC path. The driver
>>     requests a separate path per VFE from the same "interconnects"
>>     entry, so DT describes the hardware connection once and the
>>     interconnect core aggregates the VFE requests with the other
>>     consumers. The rail level is not applied by the OPP core in this
>>     case, so CAMSS keeps handling it as today.
>>
>> 2. An OPP table per VFE, attached to the per-VFE power domain device
>>     that CAMSS already creates, so each VFE calls dev_pm_opp_set_opp()
>>     on its own device. This applies the rail level as well, but the
>>     virtual device shares the CAMSS of_node, so it would pick up all
>>     CAMSS interconnect paths and need indexed OPP tables in DT, which I
>>     have not seen used for such devices.
>>
>> Which one would you prefer, or do you see a better way?
> 
> 3. Add minimal VFE nodes and let camss use those. Converge towards the
> split VFE (IFE) device tree.
> 

Agreed, I will take that up separately. To get it right the first
time, could you clarify what "minimal" covers:

  - child nodes of the camss node, or standalone nodes like the split
    CSIPHY?
  - which resources move into them: only the per-VFE clocks, power
    domain, OPP table and interconnect path, or reg and interrupts as
    well?

--
Best regards,
Atanas