On 8/27/20 6:49 PM, Stefan Wahren wrote: > Am 27.08.20 um 06:35 schrieb Hoegeun Kwon: >> Hi Stefan, >> >> Thank you for your review. >> >> >> On 8/26/20 7:04 PM, Stefan Wahren 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? >> As far as I know, BCM2711 raspberry pi 4 supports 4k, >> >> don't supported on pi 3 and pi 3+. >> >> Since 4k is not supported in versions prior to Raspberry Pi 4, >> >> I don't think we need to modify the bvb clock. >> >> >> So I think it is better to update 'brcm,bcm2711-hdmi.yaml' >> >> instead of 'brcm,bcm2835-hdmi.yaml'. > You are correct please update only brcm,bcm2711-hdmi.yaml. > > My concern was that the function vc4_hdmi_encoder_pre_crtc_configure() > is called on a non-bcm2711 platform or on a Raspberry Pi 4 with an older > DTB. So making the BVB clock optional might be better? You are right, if use old dtb, we have a problem with the hdmi driver. So how about modifying it like this? @@ -1614,8 +1614,8 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) 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); + DRM_WARN("Failed to get pixel bvb clock\n"); + vc4_hdmi->pixel_bvb_clock = NULL; } vc4_hdmi->reset = devm_reset_control_get(dev, NULL); If we modify like this, if there is no bvb clock in dtb, then pixel_bvb_clock will be null and it will not affect the clk control function below. - clk_set_rate() - clk_prepare_enable() - clk_disable_unprepare() Best regards Hoegeun > >> Please comment, what do you think? >> >>> I'm a little bit afraid, this change could break with older firmware >>> versions on BCM283x. >> Tested it several times with libdrm modetest. >> >> I expect there will be no problem. >> >> >> Best regards, >> >> Hoegeun >> >>> Best regards >>> Stefan >>> >>>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx> >>>> --- >>>> 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; >>>> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >