From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f67.google.com ([209.85.221.67]:36327 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729290AbeHOQqF (ORCPT ); Wed, 15 Aug 2018 12:46:05 -0400 From: Jernej =?utf-8?B?xaBrcmFiZWM=?= To: linux-sunxi@googlegroups.com, icenowy@aosc.io Cc: Maxime Ripard , David Airlie , Chen-Yu Tsai , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [linux-sunxi] [PATCH] drm: sun4i: exclusively set HDMI-related clocks for dw-hdmi Date: Wed, 15 Aug 2018 15:53:45 +0200 Message-ID: <4239222.d0pFiuuR7S@jernej-laptop> In-Reply-To: <2890910B-FFBD-4B05-8275-270B2DB73DEA@aosc.io> References: <20180815120745.36593-1-icenowy@aosc.io> <5309847.hPHKHXY3qr@jernej-laptop> <2890910B-FFBD-4B05-8275-270B2DB73DEA@aosc.io> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Sender: stable-owner@vger.kernel.org List-ID: Hi! Dne sreda, 15. avgust 2018 ob 15:43:19 CEST je Icenowy Zheng napisal(a): > =E4=BA=8E 2018=E5=B9=B48=E6=9C=8815=E6=97=A5 GMT+08:00 =E4=B8=8B=E5=8D=88= 9:39:44, "Jernej =C5=A0krabec"=20 =E5=86=99=E5=88=B0: > >Hi! > > > >Dne sreda, 15. avgust 2018 ob 14:07:45 CEST je Icenowy Zheng > > > >napisal(a): > >> The glue in sun4i-drm of dw-hdmi currently doesn't set the clocks of > >> dw-hdmi exclusively, which will lead the display fails to initialize > > > >in > > > >> some situations. > >>=20 > >> Add the exclusivity to sun8i-dw-hdmi and sun8i-hdmi-phy. > >>=20 > >> Cc: stable@vger.kernel.org # v4.17+ > >> Signed-off-by: Icenowy Zheng > > > >Given that you want this patch to be merged in stable, I have to ask > >what kind > >of tests did you run to prove it works as expected? > > > >In the past, there were some issues with TCON exclusive rate setting. > >It turns > >out that as many scenarios as possible have to be tested before you can > >say > >it's working. > > > >I imagine that it has to be tested on at least A83T, H3 (or H5) and R40 > >(or > >A64). They all have their own specifics which needs to be covered. > > > >Tests which I remember from top of my head: > >- run with monitor connected at boot >=20 > BTW I think the bootloader will affect the state of displsy even > without the patch. I know it can be an issue. For this test I imagine you can just disable=20 display driver in U-Boot so the PLL doesn't get set. >=20 > When could your U-Boot tweaks patchset to be done? I think they will > be needed for a really reliable test. I plan to do them after A64 driver is merged, so I can sync up dts files an= d=20 switch driver to be enabled trough them. AFAIK, A64 patches are more or les= s=20 ready and they will be merged after 4.19-rc1, right? >=20 > >- run with monitor disconnected at boot and connect it later > >- playing with resolution switching (at least a few non-standard, e.g > >something other than 1080p or 720p) >=20 > How to do it without starting a X server? libdrm has in it's source superb testing tool called modetest. It is useful= =20 for resolution testing, plane testing (scaling, positioning, color formats,= =20 blending, ...) and setting atomic properties (like zpos). While it doesn't= =20 recognize sun4i-drm driver automatically, you can specify it through comman= d=20 line parameter. Best regards, Jernej >=20 > >Best regards, > >Jernej > > > >> --- > >>=20 > >> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 11 ++++++++++- > >> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 7 +++++-- > >> 2 files changed, 15 insertions(+), 3 deletions(-) > >>=20 > >> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index > > > >31875b636434..a10220518548 > > > >> 100644 > >> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c > >> @@ -137,10 +137,16 @@ static int sun8i_dw_hdmi_bind(struct device > > > >*dev, > > > >> struct device *master, goto err_assert_ctrl_reset; > >>=20 > >> } > >>=20 > >> + ret =3D clk_rate_exclusive_get(hdmi->clk_tmds); > >> + if (ret) { > >> + dev_err(dev, "Could not get exclusivity over the tmds clock\n"); > >> + goto err_disable_clk_tmds; > >> + } > >> + > >>=20 > >> phy_node =3D of_parse_phandle(dev->of_node, "phys", 0); > >> if (!phy_node) { > >> =09 > >> dev_err(dev, "Can't found PHY phandle\n"); > >>=20 > >> - goto err_disable_clk_tmds; > >> + goto err_put_clk_tmds_exclusivity; > >>=20 > >> } > >> =09 > >> ret =3D sun8i_hdmi_phy_probe(hdmi, phy_node); > >>=20 > >> @@ -179,6 +185,8 @@ static int sun8i_dw_hdmi_bind(struct device *dev, > > > >struct > > > >> device *master, cleanup_encoder: > >> drm_encoder_cleanup(encoder); > >> sun8i_hdmi_phy_remove(hdmi); > >>=20 > >> +err_put_clk_tmds_exclusivity: > >> + clk_rate_exclusive_put(hdmi->clk_tmds); > >>=20 > >> err_disable_clk_tmds: > >> clk_disable_unprepare(hdmi->clk_tmds); > >> =20 > >> err_assert_ctrl_reset: > >> @@ -194,6 +202,7 @@ static void sun8i_dw_hdmi_unbind(struct device > > > >*dev, > > > >> struct device *master, > >>=20 > >> dw_hdmi_unbind(hdmi->hdmi); > >> sun8i_hdmi_phy_remove(hdmi); > >>=20 > >> + clk_rate_exclusive_put(hdmi->clk_tmds); > >>=20 > >> clk_disable_unprepare(hdmi->clk_tmds); > >> reset_control_assert(hdmi->rst_ctrl); > >> =20 > >> } > >>=20 > >> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > >> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index > > > >82502b351aec..1e0b1d9bc0fb > > > >> 100644 > >> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > >> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > >> @@ -511,13 +511,14 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi > > > >*hdmi, > > > >> struct device_node *node) } > >>=20 > >> clk_prepare_enable(phy->clk_phy); > >>=20 > >> + clk_rate_exclusive_get(phy->clk_phy); > >>=20 > >> } > >> =09 > >> phy->rst_phy =3D of_reset_control_get_shared(node, "phy"); > >> if (IS_ERR(phy->rst_phy)) { > >> =09 > >> dev_err(dev, "Could not get phy reset control\n"); > >> ret =3D PTR_ERR(phy->rst_phy); > >>=20 > >> - goto err_disable_clk_phy; > >> + goto err_put_clk_phy_exclusivity; > >>=20 > >> } > >> =09 > >> ret =3D reset_control_deassert(phy->rst_phy); > >>=20 > >> @@ -548,7 +549,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi > > > >*hdmi, > > > >> struct device_node *node) reset_control_assert(phy->rst_phy); > >>=20 > >> err_put_rst_phy: > >> reset_control_put(phy->rst_phy); > >>=20 > >> -err_disable_clk_phy: > >> +err_put_clk_phy_exclusivity: > >> + clk_rate_exclusive_put(phy->clk_phy); > >>=20 > >> clk_disable_unprepare(phy->clk_phy); > >> =20 > >> err_put_clk_pll1: > >> clk_put(phy->clk_pll1); > >>=20 > >> @@ -568,6 +570,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi > > > >*hdmi) > > > >> clk_disable_unprepare(phy->clk_mod); > >> clk_disable_unprepare(phy->clk_bus); > >>=20 > >> + clk_rate_exclusive_put(phy->clk_phy); > >>=20 > >> clk_disable_unprepare(phy->clk_phy); > >> =09 > >> reset_control_assert(phy->rst_phy);