Hi, Am 28.08.20 um 08:30 schrieb Hoegeun Kwon: > 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; > } i think the better solution would be devm_clk_get_optional(), which return NULL in case the clock doesn't exist. Best regards > > 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 >