Hi, 在 2024-12-18 00:59:57，"Cristian Ciocaltea" <[email protected]> 写道： >On 12/17/24 6:53 PM, Maxime Ripard wrote: >> On Tue, Dec 17, 2024 at 06:36:41PM +0200, Cristian Ciocaltea wrote: >>> On 12/17/24 5:00 PM, Maxime Ripard wrote: >>>> On Wed, Dec 11, 2024 at 07:01:15PM +0100, Heiko Stübner wrote: >>>>> Am Mittwoch, 11. Dezember 2024, 18:47:44 CET schrieb Maxime Ripard: >>>>>> On Wed, Dec 11, 2024 at 06:23:03PM +0100, Heiko Stübner wrote: >>>>>>> Am Mittwoch, 11. Dezember 2024, 18:07:57 CET schrieb Maxime Ripard: >>>>>>>> On Wed, Dec 11, 2024 at 12:15:07PM +0200, Cristian Ciocaltea wrote: >>>>>>>>> The RK3588 specific implementation is currently quite limited in terms >>>>>>>>> of handling the full range of display modes supported by the connected >>>>>>>>> screens, e.g. 2560x1440@75Hz, 2048x1152@60Hz, 1024x768@60Hz are just a >>>>>>>>> few of them. >>>>>>>>> >>>>>>>>> Additionally, it doesn't cope well with non-integer refresh rates like >>>>>>>>> 59.94, 29.97, 23.98, etc. >>>>>>>>> >>>>>>>>> Make use of HDMI0 PHY PLL as a more accurate DCLK source to handle >>>>>>>>> all display modes up to 4K@60Hz. >>>>>>>>> >>>>>>>>> Tested-by: FUKAUMI Naoki <[email protected]> >>>>>>>>> Signed-off-by: Cristian Ciocaltea <[email protected]> >>>>>>>>> --- >>>>>>>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 34 >>>>>>>>> ++++++++++++++++++++++++++++ >>>>>>>>> 1 file changed, 34 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>>>>>>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>>>>>>> index >>>>>>>>> 8b2f53ffefdbf1cc8737b3a86e630a03a7fd9348..393fe6aa170aaee9663c4a6d98c1cd6a5ef79392 >>>>>>>>> 100644 >>>>>>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >>>>>>>>> @@ -158,6 +158,7 @@ struct vop2_video_port { >>>>>>>>> struct drm_crtc crtc; >>>>>>>>> struct vop2 *vop2; >>>>>>>>> struct clk *dclk; >>>>>>>>> + struct clk *dclk_src; >>>>>>>>> unsigned int id; >>>>>>>>> const struct vop2_video_port_data *data; >>>>>>>>> >>>>>>>>> @@ -212,6 +213,7 @@ struct vop2 { >>>>>>>>> struct clk *hclk; >>>>>>>>> struct clk *aclk; >>>>>>>>> struct clk *pclk; >>>>>>>>> + struct clk *pll_hdmiphy0; >>>>>>>>> >>>>>>>>> /* optional internal rgb encoder */ >>>>>>>>> struct rockchip_rgb *rgb; >>>>>>>>> @@ -220,6 +222,8 @@ struct vop2 { >>>>>>>>> struct vop2_win win[]; >>>>>>>>> }; >>>>>>>>> >>>>>>>>> +#define VOP2_MAX_DCLK_RATE 600000 /* kHz */ >>>>>>>>> + >>>>>>>>> #define vop2_output_if_is_hdmi(x) ((x) == ROCKCHIP_VOP2_EP_HDMI0 >>>>>>>>> || \ >>>>>>>>> (x) == ROCKCHIP_VOP2_EP_HDMI1) >>>>>>>>> >>>>>>>>> @@ -1033,6 +1037,9 @@ static void vop2_crtc_atomic_disable(struct >>>>>>>>> drm_crtc *crtc, >>>>>>>>> >>>>>>>>> vop2_crtc_disable_irq(vp, VP_INT_DSP_HOLD_VALID); >>>>>>>>> >>>>>>>>> + if (vp->dclk_src) >>>>>>>>> + clk_set_parent(vp->dclk, vp->dclk_src); >>>>>>>>> + >>>>>>>>> clk_disable_unprepare(vp->dclk); >>>>>>>>> >>>>>>>>> vop2->enable_count--; >>>>>>>>> @@ -2049,6 +2056,27 @@ static void vop2_crtc_atomic_enable(struct >>>>>>>>> drm_crtc *crtc, >>>>>>>>> >>>>>>>>> vop2_vp_write(vp, RK3568_VP_MIPI_CTRL, 0); >>>>>>>>> >>>>>>>>> + /* >>>>>>>>> + * Switch to HDMI PHY PLL as DCLK source for display modes up >>>>>>>>> + * to 4K@60Hz, if available, otherwise keep using the system >>>>>>>>> CRU. >>>>>>>>> + */ >>>>>>>>> + if (vop2->pll_hdmiphy0 && mode->crtc_clock <= >>>>>>>>> VOP2_MAX_DCLK_RATE) { >>>>>>>>> + drm_for_each_encoder_mask(encoder, crtc->dev, >>>>>>>>> crtc_state->encoder_mask) { >>>>>>>>> + struct rockchip_encoder *rkencoder = >>>>>>>>> to_rockchip_encoder(encoder); >>>>>>>>> + >>>>>>>>> + if (rkencoder->crtc_endpoint_id == >>>>>>>>> ROCKCHIP_VOP2_EP_HDMI0) { >>>>>>>>> + if (!vp->dclk_src) >>>>>>>>> + vp->dclk_src = >>>>>>>>> clk_get_parent(vp->dclk); >>>>>>>>> + >>>>>>>>> + ret = clk_set_parent(vp->dclk, >>>>>>>>> vop2->pll_hdmiphy0); >>>>>>>>> + if (ret < 0) >>>>>>>>> + drm_warn(vop2->drm, >>>>>>>>> + "Could not switch to >>>>>>>>> HDMI0 PHY PLL: %d\n", ret); >>>>>>>>> + break; >>>>>>>>> + } >>>>>>>>> + } >>>>>>>>> + } >>>>>>>>> + >>>>>>>> >>>>>>>> It seems pretty fragile to do it at atomic_enable time, even more so >>>>>>>> since you don't lock the parent either. >>>>>>>> >>>>>>>> Any reason not to do it in the DRM or clock driver probe, and make sure >>>>>>>> you never change the parent somehow? >>>>>>> >>>>>>> On rk3588 we have 3 dclk_s and 2 hdmi controllers. Each video-port can >>>>>>> use the clock generated from either the hdmi0phy or hdmi1phy, depending >>>>>>> on which hdmi-controller it uses. >>>>>>> >>>>>>> So you actually need to know which vpX will output to which hdmiY to >>>>>>> then >>>>>>> reparent that dclk to the hdmiphy output. >>>>>> >>>>>> The Rockchip nomenclature isn't super obvious to me, sorry. Is there a >>>>>> datasheet for this somewhere? Also, does this vpX -> HDMI-Y mapping need >>>>>> to be dynamic? >>>>> >>>>> VPs are CRTCs in drm-language and each of them can drive a differing >>>>> number of output encoders. Those video-ports also have differing output >>>>> characteristics in terms of supported resolution and other properties. >>>>> >>>>> The rk3588 TRM has leaked in a number of places, and if you find a >>>>> TRM-part2, there is a section labeled "Display Output Interface >>>>> Description" >>>>> that has a nice graphic for that. >>>>> >>>>> Or in short: >>>>> - CRTC(VP)0 supports 8K resolution and can drive DP0+1, HDMI0+1, eDP0+1 >>>>> [if I'm reading things correctly, 8K together with CRTC1 somehow) >>>>> - CRTC(VP)1 supports 4K resolution and can drive DP0+1, HDMI0+1, eDP0+1 >>>>> - CRTC(VP)2 supports 4K resolution and can drive DP0+1, HDMI0+1, eDP01, >>>>> DSI0+1 >>>>> - CRTC(VP)3 supports 2K resolution and can drive DSI0+1 and some >>>>> BT1120,BT656 >>>>> >>>>> so for the 3 higher resolution CRTCs there are essentially 6 or 8 output >>>>> options >>>>> depending on the board design >>>> >>>> That's much clearer, thanks. I'm not entirely sure how that links to the >>>> need for the PLL to change its parent depending on the ouput. Do you >>>> need to always have all the outputs on the same PLL? >>> >>> One of the problems is that the PHY PLLs cannot be used as clock sources >>> for resolutions above 4K@60Hz, while VOP2 on RK3588 supports up to 8K@60Hz, >>> which is supposed to be handled by the system CRU. >> >> But can that system CRU drive resolutions lower than 4k@60? If so, why >> do we bother with PHYs? > >It can, but it's not accurate enough to support all modes, e.g. those >having fractional refresh rates, among others. That's actually the >reason we'd want to make use of the PHY PLLs.
Not only that. For resolutions lower than 4k@60, if we use system cur as dclk parent, it can't sync with HDMI TMDS clock , this could lead to stability/compatibility i issues, which may not be easy to encounter, but we have indeed experienced them few timems in a large amount of productization practice. > >>> Moreover, the 2 PLLs are shared between 3 out of the 4 video ports already >>> mentioned by Heiko. There is quite a bit of complexity in downstream >>> driver to handle all possible usecases - see [1] for a brief description on >>> how was that designed to work. >> >> That's a generic allocation issue. Multiple drivers (vc4 for example) >> has this issue for other components. It can be fairly easily dealt with >> at atomic_check time. >> >> Maxime >
