Re: [PATCH v2 1/9] dt-bindings: display: rockchip: analogix-dp: Expand clock-names "hclk" for the third clock

[Damon Ding]

On 3/23/2026 5:24 PM, Krzysztof Kozlowski wrote:
> On 23/03/2026 09:49, Damon Ding wrote:
> > > > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > > index d99b23b88cc5..d2bc8636b626 100644
> > > > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> > > > @@ -26,7 +26,9 @@ properties:
> > > >        items:
> > > >          - const: dp
> > > >          - const: pclk
> > > > -      - const: grf
> > > > +      - enum:
> > > > +          - grf
> > > > +          - hclk
> > >
> > > You just told me it is the same clock, no? I asked that and you
> > > confirmed? Or you replied with an answer not relevant to the question?
> >
> > Aha, I may have misunderstood your earlier advice as meaning that since
> > the RK3576 and RK3588 platforms share the same design, their clock
> > dependencies should also be identical.
> >
> > The 'grf' clock and 'hclk' clock are different.
> > On RK3399, the GRF clock is for accessing VIO GRF registers.
>
> OK
>
> > On RK3588/RK3576, the eDP 'hclk' is required as it is the parent clock
> > of 'pclk' in the AHB bus topology.
>
> Heh, device takes pclk, so you do not need to take its parent.

Emm, my explanation may not have been clear enough. At the clock tree 
level, the hclk and pclk are two separate clocks, while pclk's parent 
clock is hclk in the NOC design, as illustrated below:
(Take the NOC design of RK3576 as example)

PD_VOP
  hclk_vop
    ├─ PD_VO1/hclk_vo1
    │    ├─ hclk_vo1_s (VO1 AHBS)
    │    └─ pclk_vo1_s (VO1 APB)
    └─ PD_VO0/hclk_vo0
         ├─ hclk_vo0_s (VO0 AHB)
         ├─ pclk_vo0   (VO0 AHBS)
         └─ pclk_vo0_s (VO0 APBS)

Then, the clock tree design is:

hclk_vo0_root (200M)
├─ hclk_spdifrx2_en
│    └─ hclk_spdifrx2 (200M)
├─ hclk_spdif2_en
│    └─ hclk_spdif2 (200M)
├─ hclk_sai6_8ch_en
│    └─ hclk_sai6_8ch (200M)
├─ hclk_hdcp0_en
│    └─ hclk_hdcp0 (200M)
├─ hclk_vo0_biu_en
│    └─ hclk_vo0_biu (200M)
└─ hclk_sai5_8ch_en
     └─ hclk_sai5_8ch (200M)

pclk_vo0_root (150M)
├─ pclk_hdmitx0_en
│    └─ pclk_hdmitx0 (150M)
├─ pclk_vo0_grf_en
│    └─ pclk_vo0_grf (150M)
├─ pclk_edp0_en
│    └─ pclk_edp0 (150M)
├─ pclk_dsihost0_en
│    └─ pclk_dsihost0 (150M)
├─ pclk_hdcp0_en
│    └─ pclk_hdcp0 (150M)
└─ pclk_vo0_biu_en
     └─ pclk_vo0_biu (150M)

Therefore, we need take 'hclk' as reference clock.

> > > If this device takes reference to grf, why does it also take grf clock?
> > > Devices MUST NOT take clocks on behalf of other devices, so please
> > > answer with a list of all clock inputs according to datasheet/manual.
> >
> > On earlier platforms, the GRF was not partitioned into separate
> > functional small GRF units as on newer designs. Therefore, the 'grf'
> > clock was designed as the clock dependency for all relevant modules.
> >
> > For RK3588/RK3576 eDP module, the list of all clock inputs is:
> >
> > CLK_EDP0_24M: Reference clock.
> > PCLK_EDP0: AHB clock.
> > HCLK_VO1/HCLK_VO0_ROOT: Video datapath clock.

Best regards,
Damon