Re: [PATCH] dts: rockchip: rk3066: add qos_hdmi and HCLK_HDMI to pmu node

Re: [PATCH] dts: rockchip: rk3066: add qos_hdmi and HCLK_HDMI to pmu node

[Tomasz Figa]

Hi Johan,

On Sun, Dec 16, 2018 at 12:03 AM Johan Jonker <jbx9999@hotmail.com> wrote:
>
> A MK808 TV stick with rk3066 processor boots normal with logo and console.
> After the boot the monitor remains black.
> This patch fixes a vblank timeout crash by adding qos_hdmi and
> HCLK_HDMI to the pmu node.
> The HCLK_HDMI clock and the RK3066_PD_VIO power domain
> will now turn on and off together.
>
> Signed-off-by: Johan Jonker <jbx9999@hotmail.com>
> ---
>  arch/arm/boot/dts/rk3066a.dtsi | 6 ++++--
>  arch/arm/boot/dts/rk3xxx.dtsi  | 5 +++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
>

Thanks for the patch. Unfortunately, it looks like you didn't add the
necessary mailing lists to the recipient list. For reference, the
./scripts/get_maintainer.pl script in the kernel source tree should be
able to give you a reasonable recipient list. For now, I added the
mailing lists on CC and replied without snipping, so people should be
still able to review the patch.

Other than that, It looks reasonable to me, but we need someone with
access to SoC documentation to check it. Heiko, Sandy, is that
something you would be able to help with?

> diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
> index 30dc8af0b..6e7cdde84 100644
> --- a/arch/arm/boot/dts/rk3066a.dtsi
> +++ b/arch/arm/boot/dts/rk3066a.dtsi
> @@ -672,13 +672,15 @@
>                                  <&cru ACLK_IPP>,
>                                  <&cru HCLK_IPP>,
>                                  <&cru ACLK_RGA>,
> -                                <&cru HCLK_RGA>;
> +                                <&cru HCLK_RGA>,
> +                                <&cru HCLK_HDMI>;
>                         pm_qos = <&qos_lcdc0>,
>                                  <&qos_lcdc1>,
>                                  <&qos_cif0>,
>                                  <&qos_cif1>,
>                                  <&qos_ipp>,
> -                                <&qos_rga>;
> +                                <&qos_rga>,
> +                                <&qos_hdmi>;
>                 };
>
>                 pd_video@RK3066_PD_VIDEO {
> diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
> index 97307a405..1f9496e81 100644
> --- a/arch/arm/boot/dts/rk3xxx.dtsi
> +++ b/arch/arm/boot/dts/rk3xxx.dtsi
> @@ -187,6 +187,11 @@
>                 reg = <0x1012f280 0x20>;
>         };
>
> +       qos_hdmi: qos@1012f300 {
> +               compatible = "syscon";
> +               reg = <0x1012f300 0x20>;
> +       };
> +

Is this really common for all rk3xxx SoCs?

>         usb_otg: usb@10180000 {
>                 compatible = "rockchip,rk3066-usb", "snps,dwc2";
>                 reg = <0x10180000 0x40000>;
> --
> 2.11.0
>

Best regards,
Tomasz

Re: [PATCH] dts: rockchip: rk3066: add qos_hdmi and HCLK_HDMI to pmu node

[Heiko Stuebner]

Hi Johan,

Am Montag, 17. Dezember 2018, 15:00:55 CET schrieb Johan Jonker:
> Thanks Tomasz for adding all the mailing lists.
> I prefer to ask first if a qos_hdmi exists before sending it in for
> public review.
> 
> All the clocks in the pmu node seem to have a "quality-of-service" (QoS)
> block.
> So I added one for hdmi too with the question if it exists and which
> address it might have.
> SoC documentation still isn't fully public, not that I'm aware off.
> 
> From Heiko's response I learned that the rk3066 manual does
> not list any hdmi-related QoS blocks.
> We now only add the HCLK_HDMI clock to pmu node in our patch.
> 
> After a small step forward people immediately ask to do a real patch serie.

Of course :-D .
Collecting reviews can take time, so posting things early is helpful,
especially now that you actually can generate output.

That is the one requirement I have ... that things actually work and
the vop-driver alone didn't look that way some weeks back ;-)
[And as we found out was also missing the changed irq].

> The rk3066_hdmi.c file also handles audio, but this is not tested yet.

You can strip the audio parts from the hdmi-encoder and just submit
it with the display parts only.

That way we can possibly pick up the working things already, reducing
the number of patches you have to carry around ;-) .

> The rk3066a-rayeager.dts file gives a idea how it's done,
> but I haven't had the time to get it working with .config and dts.
> Let me know if you can.

I think making this actually work will fall on your shoulders.
I.e. the rk3066 is really old (from 2012 or possibly even earlier), so
people like Tomasz don't have access to this (and also in most cases
not the time or interest to work on those specific socs).

While I do have a rk3066 marsboard here, it is currently not functional
in my test setup, so I also can only look at the code you provide.


Happy hacking
Heiko


> On 12/17/18 6:46 AM, Tomasz Figa wrote:
> 
> > Thanks for the patch. Unfortunately, it looks like you didn't add the
> > necessary mailing lists to the recipient list. For reference, the
> > ./scripts/get_maintainer.pl script in the kernel source tree should be
> > able to give you a reasonable recipient list. For now, I added the
> > mailing lists on CC and replied without snipping, so people should be
> > still able to review the patch.
> > 
> > Other than that, It looks reasonable to me, but we need someone with
> > access to SoC documentation to check it. Heiko, Sandy, is that
> > something you would be able to help with?
> 
>