Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control

[Stefan Wahren]

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?

>
> 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