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 <naoki@xxxxxxxxx>
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
> > ---
> > 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.
