Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200821071045.24501-1-hoegeun.kwon@samsung.com>
 <CGME20200821071122epcas1p3d00dda4665f94192ac5e9ee829d0557d@epcas1p3.samsung.com>
 <20200821071045.24501-4-hoegeun.kwon@samsung.com> <61c199bf-852f-82d3-089a-a0a435343acf@i2se.com>
In-Reply-To: <61c199bf-852f-82d3-089a-a0a435343acf@i2se.com>
From:   Dave Stevenson <dave.stevenson@raspberrypi.com>
Date:   Fri, 28 Aug 2020 10:22:28 +0100
Message-ID: <CAPY8ntABzkR+NPeTvifGbrLGcpom97zWdArxdFjEUq5JnjOU6A@mail.gmail.com>
Subject: Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
To:     Stefan Wahren <stefan.wahren@i2se.com>
Cc:     Hoegeun Kwon <hoegeun.kwon@samsung.com>,
        Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        Eric Anholt <eric@anholt.net>,
        Maxime Ripard <maxime@cerno.tech>, devicetree@vger.kernel.org,
        Tim Gover <tim.gover@raspberrypi.com>, kdasu.kdev@gmail.com,
        sboyd@kernel.org, mturquette@baylibre.com,
        LKML <linux-kernel@vger.kernel.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Phil Elwell <phil@raspberrypi.com>, robh+dt@kernel.org,
        bcm-kernel-feedback-list@broadcom.com,
        linux-rpi-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Stefan & Hoegeun

On Wed, 26 Aug 2020 at 11:04, Stefan Wahren <stefan.wahren@i2se.com> wrote:
>
> Hi Hoeguen,
>
> Am 21.08.20 um 09:10 schrieb Hoegeun Kwon:
> > There is a problem that the output does not work at a resolution
> > exceeding FHD. To solve this, we need to adjust the bvb clock at a
> > resolution exceeding FHD.
>
> this patch introduces a mandatory clock, please update
> brcm,bcm2835-hdmi.yaml first.
>
> Is this clock physically available on BCM283x or only on BCM2711?
>
> I'm a little bit afraid, this change could break with older firmware
> versions on BCM283x.

Thanks for your keen eye on these things.

BVB only exists on 2711, not 283x.

It runs at 2 pixels/clock, must be an integer divider of I believe
600MHz, and between 75 and 300MHz.
This aim of this patch is fine as we currently only go up to 4k30, but
for 4k60 the BVB will need to be set to 300MHz.

Thanks
  Dave

> Best regards
> Stefan
>
> >
> > Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> > ---
> >  drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
> >  drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index 95ec5eedea39..eb3192d1fd86 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -80,6 +80,7 @@
> >  # define VC4_HD_M_ENABLE                     BIT(0)
> >
> >  #define CEC_CLOCK_FREQ 40000
> > +#define VC4_HSM_MID_CLOCK 149985000
> >
> >  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
> >  {
> > @@ -380,6 +381,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);
> >
> > @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder)
> >               return;
> >       }
> >
> > +     ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock,
> > +                     (hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000));
> > +     if (ret) {
> > +             DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
> > +             clk_disable_unprepare(vc4_hdmi->hsm_clock);
> > +             clk_disable_unprepare(vc4_hdmi->pixel_clock);
> > +             return;
> > +     }
> > +
> > +     ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
> > +     if (ret) {
> > +             DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);
> > +             clk_disable_unprepare(vc4_hdmi->hsm_clock);
> > +             clk_disable_unprepare(vc4_hdmi->pixel_clock);
> > +             return;
> > +     }
> > +
> >       if (vc4_hdmi->variant->reset)
> >               vc4_hdmi->variant->reset(vc4_hdmi);
> >
> > @@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
> >               return PTR_ERR(vc4_hdmi->audio_clock);
> >       }
> >
> > +     vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb");
> > +     if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) {
> > +             DRM_ERROR("Failed to get pixel bvb clock\n");
> > +             return PTR_ERR(vc4_hdmi->pixel_bvb_clock);
> > +     }
> > +
> >       vc4_hdmi->reset = devm_reset_control_get(dev, NULL);
> >       if (IS_ERR(vc4_hdmi->reset)) {
> >               DRM_ERROR("Failed to get HDMI reset line\n");
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > index 0806c6d9f24e..63c6f8bddf1d 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > @@ -147,6 +147,7 @@ struct vc4_hdmi {
> >       struct clk *pixel_clock;
> >       struct clk *hsm_clock;
> >       struct clk *audio_clock;
> > +     struct clk *pixel_bvb_clock;
> >
> >       struct reset_control *reset;
> >
>