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

Dmitry Osipenko
2022-03-16T14:01:33Z

On 3/16/22 16:55, Robin Murphy wrote:
>> To me that NIU quirk should be internal to the clk h/w module, so it
>> doesn't feel nice to mix the clk h/w description with the HDMI h/w
>> description.
>>
>> On the other hand, making clk driver to handle this case indeed will
>> take some effort as I see now. For example, clk driver of NVIDIA Tegra
>> has concept of shared gates, but bringing it to the RK clk driver will
>> be quite messy.
>
> From a quick look, it seems like it could be straightforward
> conceptually at least. Presumably: subclass clk_gate_ops to
> enable/disable a required clock before enabling/disabling normally, have
> rockchip_clk_register_branch() resolve an optional required clock and
> pick gate_ops as appropriate, then the rest is basically just
> boilerplate for describing the dependencies in the first place. However
> I'd agree that in practical implementation terms it does look even
> simpler and cleaner for the clk_hw abstraction to provide the
> appropriate ops and resolution itself.
>
>> Alright, let's work around the clk limitation like you're suggesting. I
>> agree that it shouldn't really be a problem to deprecate the extra clock
>> later on.
>
> If there's a realistic chance that someone will actually work on a
> proper coupled/dependent/whatever clock abstraction before the rest of
> RK3588 is supported well enough for mainline users to start really
> caring about power efficiency, then arguably the simplest and cleanest
> workaround would be the other option that Elaine mentioned, of just
> marking hclk_vo as critical for now. If it's likely to turn into a
> "nothing's as permanent as a temporary fix" situation, though, then the
> DT binding has less functional impact, even if it does leave us
> developers with baggage down the line.

I missed that suggestion about marking hclk_vo as critical. That's a
good idea, I like it.