Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a

[Robin Murphy <robin.murphy@arm.com>]

On 2022-05-08 17:53, Peter Geis wrote:
> On Sun, May 8, 2022 at 9:40 AM Piotr Oniszczuk
> <piotr.oniszczuk@gmail.com> wrote:
>>
>>
>>
>>> Wiadomość napisana przez Sascha Hauer <s.hauer@pengutronix.de> w dniu 22.04.2022, o godz. 09:28:
>>>
>>> From: Michael Riesch <michael.riesch@wolfvision.net>
>>>
>>> Enable the RK356x Video Output Processor (VOP) 2 on the Radxa
>>> ROCK3 Model A.
>>>
>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Link: https://lore.kernel.org/r/20220310210352.451136-4-michael.riesch@wolfvision.net
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>
>> Sascha, Michael,
> 
> Good Afternoon,
>>
>> I'm using v11 series on 5.18-rc5 on rk3566 tvbox with great success.
>> Recently i started to work on rock3-a (rk3568).
>> v11 gives me video, audio - but cec is not working on rock3-a.
>>
>> I was told:
>>
>> 32k clock needed for cec and this clock is generated by the rtc which is embedded in the rk8xx regulator.
>> So you should make sure it is enabled when hdmi is powerd on, eg adding it to the RK3568_PD_VO powerdomain should help
>>
>> I was trying to do this in dts https://pastebin.com/67wu9QrH but cec is still non-functional
>>
>> Maybe You have some hints/pointers here?
> 
> Add the following to the HDMI node:
> assigned-clocks = <&cru CLK_HDMI_CEC>;
> assigned-clock-rates = <32768>;
> 
> The issue is the clk_rtc32k_frac clock that feeds clk_rtc_32k which
> feeds clk_hdmi_cec is 24mhz at boot, which is too high for CEC to
> function.
> I submitted a patch to have the hdmi driver handle this, but it broke
> other SoCs because 32k is an optional clock.
> Since this is the case, I'd like Robin to weigh in on going the
> assigned-clock route again.

(did you mean to CC me or have I missed another thread elsewhere?)

FWIW I still think it would be good to fix the clock driver(s) and/or 
DTs to correctly deal with the availability and configuration of xin_32k 
where appropriate. However, much like the HCLK_VO mess I guess that's a 
larger cleanup tangent in its own right, so using "assigned-clocks" for 
this one case in the meantime doesn't seem unreasonable. I was 
optimistic for the cleanest, most generic solution, but if reality gets 
in the way then oh well.

Judging by the datasheet, RK3568 might actually have a similar situation 
with its clk32k_in pin, so you may want "assigned-clock-parents" as well 
to ensure the whole clk_rtc32k branch is really set up the way you 
currently expect - baking any more assumptions into DTBs now only seems 
to add potential for breakage if kernel behaviour changes in future.

Robin.