Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To:     linux-sunxi@googlegroups.com, icenowy@aosc.io
Cc:     Maxime Ripard <maxime.ripard@bootlin.com>,
        David Airlie <airlied@linux.ie>, Chen-Yu Tsai <wens@csie.org>,
        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: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
<jernej.skrabec@gmail.com> =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 <icenowy@aosc.io>
> >
> >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);