| Message ID | 20200901040759.29992-3-hoegeun.kwon@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2,1/4] clk: bcm: rpi: Add register to control pixel bvb clk | expand |
Hi Hoegeun, It looks good to me. But, just one comment. On 9/1/20 1:07 PM, Hoegeun Kwon wrote: > 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. > > 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; > + } Generally, enable the clock before using clk and then change the clock rate. I think that you better to change the order between clk_prepare_enable and clk_set_rate. > + > 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; > >
Hi Chanwoo, On 9/1/20 1:27 PM, Chanwoo Choi wrote: > Hi Hoegeun, > > It looks good to me. But, just one comment. > > On 9/1/20 1:07 PM, Hoegeun Kwon wrote: >> 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. >> >> 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; >> + } > Generally, enable the clock before using clk and then change the clock rate. > I think that you better to change the order between clk_prepare_enable and clk_set_rate. Thank you for your comment. As Maxime answered in another patch [1], there is no clear rule of order here. [1] https://lkml.org/lkml/2020/9/1/327 Best regards, Hoegeun
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;
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. 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(+)