linux-kernel - Re: [PATCH v5 75/80] drm/vc4: hdmi: Add pixel BVB clock control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200907162149.plabkjrgajjqbtiu@gilmour.lan>
Date:   Mon, 7 Sep 2020 18:21:49 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Eric Anholt <eric@...olt.net>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        linux-rpi-kernel@...ts.infradead.org,
        bcm-kernel-feedback-list@...adcom.com,
        linux-arm-kernel@...ts.infradead.org,
        LKML <linux-kernel@...r.kernel.org>,
        Tim Gover <tim.gover@...pberrypi.com>,
        Phil Elwell <phil@...pberrypi.com>,
        Hoegeun Kwon <hoegeun.kwon@...sung.com>
Subject: Re: [PATCH v5 75/80] drm/vc4: hdmi: Add pixel BVB clock control

Hi,

On Fri, Sep 04, 2020 at 10:46:26AM +0100, Dave Stevenson wrote:
> On Thu, 3 Sep 2020 at 09:03, Maxime Ripard <maxime@...no.tech> wrote:
> >
> > From: Hoegeun Kwon <hoegeun.kwon@...sung.com>
> >
> > The BCM2711 has another clock that needs to be ramped up depending on the
> > pixel rate: the pixel BVB clock. Add the code to adjust that clock when
> > changing the mode.
> >
> > Signed-off-by: Hoegeun Kwon <hoegeun.kwon@...sung.com>
> > [Maxime: Changed the commit log, used clk_set_min_rate]
> > Signed-off-by: Maxime Ripard <maxime@...no.tech>
> > Link: https://lore.kernel.org/r/20200901040759.29992-3-hoegeun.kwon@samsung.com
> > ---
> >  drivers/gpu/drm/vc4/vc4_hdmi.c | 23 +++++++++++++++++++++++
> >  drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index ab7abb409de2..39508107dafd 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -54,6 +54,7 @@
> >  #include "vc4_regs.h"
> >
> >  #define CEC_CLOCK_FREQ 40000
> > +#define VC4_HSM_MID_CLOCK 149985000
> 
> I didn't flag it earlier, but this is a bit of a weird name for the
> define. I know it wants to be concise, but it made me do a double take
> as to what it is for.
> I'm currently applying all these patches to our Raspberry Pi tree and
> actually CEC needs a fixed HSM on Pi0-3 to avoid recomputing all the
> timings. So I have a VC4_HSM_CLOCK define which is the fixed clock
> rate for Pi 0-3.
> This one is more a threshold for HSM to control BVB, and my brain
> starts to hurt over what it should be called.
> 
> Unless there are other comments around this patchset (and I hope to
> read through the remaining ones today), then I don't consider it a
> blocker, but we can probably do better as and when we add the next
> threshold for 4k60.
> My current understanding is that the clock has to be an integer divide
> of 600MHz, and at least the pixel rate / 2, so the only link to HSM is
> due to HSM being 101% of pixel rate, but I will try to find
> confirmation of that.

I'm currently working on the 4k60 support, so it will go away soon
(using your suggestion) so there's no need to overthink it :)

> >  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
> >  {
> > @@ -344,6 +345,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder)
> >         HDMI_WRITE(HDMI_VID_CTL,
> >                    HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
> >
> > +       clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);
> >         clk_disable_unprepare(vc4_hdmi->hsm_clock);
> >         clk_disable_unprepare(vc4_hdmi->pixel_clock);
> >
> > @@ -516,6 +518,27 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder)
> >                 return;
> >         }
> >
> > +       /*
> > +        * FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup
> > +        * at 150MHz.
> > +        */
> 
> Typo here. For 4k60 we need 300MHz (pixel clock / 2)
> 
> Otherwise
> Reviewed-by: Dave Stevenson <dave.stevenson@...pberrypi.com>

I've fixed it, thanks!
Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)