lkml.org 
[lkml]   [2020]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@cerno.tech> wrote:
> >
> > From: Hoegeun Kwon <hoegeun.kwon@samsung.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@samsung.com>
> > [Maxime: Changed the commit log, used clk_set_min_rate]
> > Signed-off-by: Maxime Ripard <maxime@cerno.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@raspberrypi.com>

I've fixed it, thanks!
Maxime
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-09-07 18:22    [W:2.163 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site