[PATCH v8 09/24] drm/rockchip: dw_hdmi: Add support for niu clk

[Sascha Hauer]

On Mon, Mar 14, 2022 at 08:54:27PM +0300, Dmitry Osipenko wrote:
> On 3/14/22 11:18, Sascha Hauer wrote:
> > On Sun, Mar 13, 2022 at 12:07:56AM +0300, Dmitry Osipenko wrote:
> >> On 3/11/22 11:33, Sascha Hauer wrote:
> >>> The rk3568 HDMI has an additional clock that needs to be enabled for the
> >>> HDMI controller to work. This clock is not needed for the HDMI
> >>> controller itself, but to make the SoC internal bus logic work. From the
> >>> reference manual:
> >>>
> >>>> 2.8.6 NIU Clock gating reliance
> >>>>
> >>>> A part of niu clocks have a dependence on another niu clock in order to
> >>>> sharing the internal bus. When these clocks are in use, another niu
> >>>> clock must be opened, and cannot be gated.  These clocks and the special
> >>>> clock on which they are relied are as following:
> >>>>
> >>>> Clocks which have dependency     The clock which can not be gated
> >>>> -----------------------------------------------------------------
> >>>> ...
> >>>> pclk_vo_niu, hclk_vo_s_niu       hclk_vo_niu
> >>>> ...
> >>> The clock framework does not support turning on a clock whenever another
> >>> clock is turned on, so this patch adds support for the dependent clock
> >>> to the HDMI driver. We call it "NIU", which is for "Native Interface
> >>> Unit"
> >>
> >> This still doesn't make sense to me. You're saying that "pclk_vo_niu,
> >> hclk_vo_s_niu" depend on "hclk_vo_niu", but HDMI doesn't use pclk_vo, it
> >> uses pclk_hdmi.
> > 
> > pclk_hdmi_host is a child clock of pclk_vo:
> > 
> >      aclk_vo                  2        2        0   300000000          0     0  50000         Y
> >         aclk_hdcp             0        0        0   300000000          0     0  50000         N
> >         pclk_vo               2        3        0    75000000          0     0  50000         Y
> >            pclk_edp_ctrl      0        0        0    75000000          0     0  50000         N
> >            pclk_dsitx_1       0        0        0    75000000          0     0  50000         N
> >            pclk_dsitx_0       1        2        0    75000000          0     0  50000         Y
> >            pclk_hdmi_host     1        2        0    75000000          0     0  50000         Y
> >            pclk_hdcp          0        0        0    75000000          0     0  50000         N
> >         hclk_vo               2        5        0   150000000          0     0  50000         Y
> >            hclk_hdcp          0        0        0   150000000          0     0  50000         N
> >            hclk_vop           0        2        0   150000000          0     0  50000         N
> 
> It was unclear that the pclk_hdmi is the child of pclk_vo by looking at
> the clk driver's code, thank you!
> 
> Won't be better if the implicit clk dependency would be handled
> internally by the RK clk driver?

I have considered handling something like coupled clocks in the clock
framework, but I have not yet considered handling this internally in the
Rockchip clock driver.

I just had a quick look at the driver. While it sounds like an easy task
to enable gate A whenever gate B is enabled I haven't found a good way to
integrate that into the clk driver. It seems to me that it's not easier
to implement in the clk driver than it is to implement it in the clk
framework where it could be used by others as well.

> For example, by making the common gate
> shared/refcounted. Have you considered this variant? Then we won't need
> to change the DT bindings.

For the DT bindings it is just an additional clock. Should we have a
better way to handle that case in the future we could simply ignore the
additional clock. I wouldn't bother too much about this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |