| Message ID | d757ddd6549da140f178563e5fd2bf1d129913fd.1599120059.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Support BCM2711 Display Pipeline | expand |
Hi Maxime 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. > > 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> > + ret = clk_set_min_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); > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h > index 34138e0dd4a6..59639b405b7f 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h > @@ -119,6 +119,7 @@ struct vc4_hdmi { > struct clk *pixel_clock; > struct clk *hsm_clock; > struct clk *audio_clock; > + struct clk *pixel_bvb_clock; > > struct debugfs_regset32 hdmi_regset; > struct debugfs_regset32 hd_regset; > -- > git-series 0.9.1
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
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 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. + */ + ret = clk_set_min_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); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 34138e0dd4a6..59639b405b7f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -119,6 +119,7 @@ struct vc4_hdmi { struct clk *pixel_clock; struct clk *hsm_clock; struct clk *audio_clock; + struct clk *pixel_bvb_clock; struct debugfs_regset32 hdmi_regset; struct debugfs_regset32 hd_regset;