Hi, On Fri, Sep 04, 2020 at 10:46:26AM +0100, Dave Stevenson wrote: > On Thu, 3 Sep 2020 at 09:03, Maxime Ripard <max...@cerno.tech> wrote: > > > > From: Hoegeun Kwon <hoegeun.k...@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.k...@samsung.com> > > [Maxime: Changed the commit log, used clk_set_min_rate] > > Signed-off-by: Maxime Ripard <max...@cerno.tech> > > Link: > > https://lore.kernel.org/r/20200901040759.29992-3-hoegeun.k...@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.steven...@raspberrypi.com>
I've fixed it, thanks!
Maxime
signature.asc
Description: PGP signature
