From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from layka.disroot.org (layka.disroot.org [178.21.23.139]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 92100397321; Sun, 9 Aug 2026 07:19:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=178.21.23.139 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786259963; cv=none; b=dvJ3eEAF5TR2FyL4sDkszXY8ulKPrvXIdWgk6vtoy4z/G9a45rT7BwFk5gtGDiswVE+UflsJJhkEUyoQeHe6CSBP2gPCHwUBZatz0jKELFZZC8gJgCnVYs3SmUyzDBnlWOM4ufPq16/DVDXJWuFOLcLlqJughw2cTfLn9XyCzA0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786259963; c=relaxed/simple; bh=ikalTkv5Ibx6UZVhDzEkB2X3qHV5gdGfm5vXWpBzMJM=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=aTpYy1U/wssuKleNcXNqzAiybqkFQnr6fqRF677c+qRo0WTpwj+3IBe3Lek43cFOFaGc7tkhXbNPkFW7NDzzVSyDJ1XanHcbAUQcs6D//13M4AfpGvk6COEeZPm1nxWaKooe9cRp+BjArZTPhxFFdyOjiFIDnQNAmcsC90L++VQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=disroot.org; spf=pass smtp.mailfrom=disroot.org; dkim=pass (2048-bit key) header.d=disroot.org header.i=@disroot.org header.b=HaVpSR0e; arc=none smtp.client-ip=178.21.23.139 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=disroot.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=disroot.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=disroot.org header.i=@disroot.org header.b="HaVpSR0e" Received: from mail01.layka.lan (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id 09FC142C48; Sun, 09 Aug 2026 09:19:12 +0200 (CEST) X-Virus-Scanned: SPAM Filter at disroot.org Received: from layka.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Z7krVjHtXELU; Sun, 9 Aug 2026 09:19:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1786259951; bh=ikalTkv5Ibx6UZVhDzEkB2X3qHV5gdGfm5vXWpBzMJM=; h=Date:Cc:Subject:From:To:References:In-Reply-To; b=HaVpSR0eV4lwJI3Zo1GkAKJhw9U43pdq8FdUwseBaQzOltKIsfqBStvT8SPjiLrEH OH6v26uZYqUSWVzp1x4DZ8NH7EYYvbhFSlV4oez66uDn5TZY7GRE6ZRDMcip9rd80d BOPXp4YVPvfHLMdtWTcdd7Df04txGyiFtWSFX7O/PcOKfkjyXy6Amx47OCzs1PIcSb GNskEFhkSM3zcG2cQL6kSYRS4xzFw5dtT239nxsOoUKhwfgyYFV57oQYdW8FZpn7mv Dp6DNbO4Bo0pRp5uEHumbM7n8Mno/LRqvWji5jiRNLg7nhBPw3S6McvNeZj00YpBF0 THqpME1Ne7Ncw== Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Sun, 09 Aug 2026 12:48:59 +0530 Message-Id: Cc: "Jagan Teki" , "Marek Szyprowski" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "Laurent Pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "Luca Ceresoli" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Seung-Woo Kim" , "Kyungmin Park" , "Krzysztof Kozlowski" , "Peter Griffin" , "Alim Akhtar" , , , , Subject: Re: [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability From: "Kaustabh Chakraborty" To: "Inki Dae" , "Kaustabh Chakraborty" References: <20260723-exynos-dsim-fixes-v3-0-0c31ae1dbecc@disroot.org> <20260723-exynos-dsim-fixes-v3-2-0c31ae1dbecc@disroot.org> In-Reply-To: On 2026-08-02 15:56 +09:00, Inki Dae wrote: > HI, > > 2026=EB=85=84 7=EC=9B=94 23=EC=9D=BC (=EB=AA=A9) =EC=98=A4=EC=A0=84 4:04,= Kaustabh Chakraborty =EB=8B=98=EC=9D=B4 =EC=9E=91= =EC=84=B1: >> >> Stabilizing PLL needs to be waited for. This is done using a loop, >> checking the PLL_STABLE bit in the status register. >> >> DSIM fires an interrupt when the PLL is stabilized. Rely on this >> functionality for stabilization wait, getting rid of the implicit loop. >> >> This has been tested on a Galaxy J6 (Exynos 7870). Unfortunately, since >> testing on all supported devices is less feasible, introduce a stop-gap >> measure where the timeout has a gracious lower bound of 100 >> microseconds. This will (hopefully) prevent regressions due to timeout >> on other devices. >> >> Suggested-by: Inki Dae >> Link: https://lore.kernel.org/r/CAAQKjZMLMbwDVZRb5+Xb_5yz3AEP4uuzFJMuuZy= 9NFDu13VU5w@mail.gmail.com >> Tested-by: Marek Szyprowski >> Signed-off-by: Kaustabh Chakraborty >> --- >> drivers/gpu/drm/bridge/samsung-dsim.c | 41 +++++++++++++++++++++++-----= ------- >> include/drm/bridge/samsung-dsim.h | 1 + >> 2 files changed, 28 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bri= dge/samsung-dsim.c >> index da753ff6eed4..866cff205e71 100644 >> --- a/drivers/gpu/drm/bridge/samsung-dsim.c >> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c ... >> >> - timeout =3D 3000; >> - do { >> - if (timeout-- =3D=3D 0) { >> - dev_err(dsi->dev, "PLL failed to stabilize\n"); >> - return 0; >> - } >> - if (driver_data->has_legacy_status_reg) >> - reg =3D samsung_dsim_read(dsi, DSIM_STATUS_REG); >> - else >> - reg =3D samsung_dsim_read(dsi, DSIM_LINK_STATUS_= REG); >> - } while ((reg & BIT(driver_data->pll_stable_bit)) =3D=3D 0); >> + if (wait_for_completion_timeout(&dsi->pll_stabilized, >> + usecs_to_jiffies(timeout))) { >> + dev_err(dsi->dev, "PLL failed to stabilize\n"); >> + return 0; >> + } > > This is the main problem: the condition is inverted. > > wait_for_completion_timeout() returns 0 on timeout, and the number of > remaining jiffies (> 0) on completion. As written, this reports an > error and returns 0 exactly when the PLL *did* stabilize, and silently > succeeds when it timed out. > > Since samsung_dsim_set_pll() returning 0 makes its caller > samsung_dsim_enable_clock() bail out with -EFAULT, the display would > fail to come up entirely the moment a PLL_STABLE interrupt actually > arrives. > > if (!wait_for_completion_timeout(&dsi->pll_stabilized, > usecs_to_jiffies(timeout))) { > > The reason this still passed testing is the second issue below: the > PLL_STABLE interrupt never fires in the first place. The two bugs are > masking each other. Unfortunately, fixing these my hardware does not issue the PLL_STABLE interrupt for the first panel reset during boot. For subsequent ones it does issue the same. One lazy solution is to try to peek at the PLL stable bit after the timeout and continue execution if it indicates that PLL is stable. In that case we should be able to validate it with a fallback. I should also remember to drop the Reviewed-by: tag from this patch.