From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A1ED72FD69E for ; Mon, 20 Jul 2026 19:22:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784575381; cv=none; b=nScp2YXlYTUKzaGrfT90GfepXA7eS82kALZHDyt8oCMU//Wyp0UhAPOwVpD/xwOniBitYqQ6O3+2UFDnKv10qYqZLOy9XJPYQt48S/DIs+cPVVrJEBBIO+oeubQw/BldOzvm+8hGKaJRf5dzXziZOKnyrRQg44M7yCpQA/cZRZ8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784575381; c=relaxed/simple; bh=OsvP99ela6zAdfVIAq+eCL6sZcIj2qQNMD54cI/1i9s=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=VuapQEsTQdj0JIZ4WrizI8IOeLpcQ4MXCII9YsN9e1E2abZRLZ47Ljp7WkTLWs+xRl6fhZ0hAv98vyoUHYgB0X2PBRbTr/PXOSnOWbUeWNOiRuGATjx6oG7cdAdePsEN3xCw9blkY/YJLyQxq3pQbNoak4d3KdTfRkd1TMvlCM0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=jnWSq+Lo; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="jnWSq+Lo" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1784575377; bh=OsvP99ela6zAdfVIAq+eCL6sZcIj2qQNMD54cI/1i9s=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=jnWSq+Lo4AbMpRQhZKdE9RiEnUqKI7RkAeHfp+TqVXKMmhFuXWYunZK2eTEiRClZR Vu6vYhpB7cxno6FTANC9D+otPZCYlwWvWMfiOd//Kd1Le/VTkK7VdnXBbI34f0uvGQ +GRTOzhpVlLZvturG7s3N3pxjVlgJE+upcHgD4XLZYWPaaxz3UesDjJs8l7k1EHyS/ t0V++NdxCm6DJc7npUYUzEVcoiKaUaoWSX/fjQKn40LbctCDgc8uXxcdkOvILjAIJW cGtYMGI+OnXFXE5cVtDfAkkllbog/2Pift17HprAaSgldYnJ2qqPFUUX1d3kecnZ/L GkNpp6qWTNdYw== Received: from [100.64.0.241] (unknown [100.64.0.241]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange x25519) (No client certificate requested) (Authenticated sender: cristicc) by bali.collaboradmins.com (Postfix) with ESMTPSA id 469CB17E0910; Mon, 20 Jul 2026 21:22:57 +0200 (CEST) Message-ID: Date: Mon, 20 Jul 2026 22:22:56 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 4/8] phy: rockchip: samsung-hdptx: Handle uncommitted PHY config changes To: Dmitry Baryshkov Cc: Vinod Koul , Neil Armstrong , Heiko Stuebner , Algea Cao , Dmitry Baryshkov , kernel@collabora.com, linux-phy@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, =?UTF-8?Q?Thomas_Niederpr=C3=BCm?= , Simon Wright References: <20260612-hdptx-clk-fixes-v4-0-ce5e1d456cda@collabora.com> <20260612-hdptx-clk-fixes-v4-4-ce5e1d456cda@collabora.com> Content-Language: en-US From: Cristian Ciocaltea In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 7/20/26 5:17 PM, Dmitry Baryshkov wrote: > On Fri, Jun 12, 2026 at 02:45:23AM +0300, Cristian Ciocaltea wrote: >> Any changes to the PHY link rate and/or color depth done via the HDMI >> PHY configuration API are not immediately programmed into the hardware, >> but are delayed until the PHY usage count gets incremented from 0 to 1, >> that is when it is powered on or when the PLL clock exposed through >> the CCF API is prepared, whichever comes first. > > Why do you need either-of here? Can it be just one path? The thing is that PLL clock usage is dynamic, depending on the rate, but it can also be disabled permanently. When it's in use, it triggers before power on, hence the hardware must be programmed at that time. >> >> Since the clock might remain in prepared state after subsequent PHY >> config changes, the programming can also be triggered via >> clk_ops.set_rate(). However, from the clock consumer perspective (i.e. >> VOP2 display controller), the (pixel) clock rate doesn't vary with bpc, >> as that is handled internally by the PHY and reflected in the TDMS >> character rate only. >> >> As a consequence, changing the bpc while preserving the modeline may >> lead to out-of-sync issues between CCF and HDMI PHY config state, >> because the .set_rate() callback is not invoked when clock rate remains >> constant. This may also happen when the PHY PLL has been pre-programmed >> by an external entity, e.g. the bootloader, which is actually a >> regression introduced by the recent FRL patches. >> >> Introduce a pll_config_dirty flag to keep track of uncommitted PHY >> config changes and use it in clk_ops.determine_rate() to invalidate the >> current clock rate (as known by CCF) and, consequently, ensure those >> changes are programmed into hardware via clk_ops.set_rate(). >> >> Moreover, proceed with a similar fix in phy_ops.power_on() callback, to >> handle the scenario where the CCF API is not used due to operating in >> FRL mode, while the clock is still in a prepared state and thus >> preventing rk_hdptx_phy_consumer_get() to apply the updated PHY >> configuration. >> >> Fixes: de5dba833118 ("phy: rockchip: samsung-hdptx: Add HDMI 2.1 FRL support") >> Fixes: 9d0ec51d7c22 ("phy: rockchip: samsung-hdptx: Add high color depth management") >> Tested-by: Thomas Niederprüm >> Tested-by: Simon Wright >> Signed-off-by: Cristian Ciocaltea >> --- >> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 84 +++++++++++++---------- >> 1 file changed, 48 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c >> index b210c1a88b25..25bd821cd039 100644 >> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c >> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c >> @@ -413,6 +413,7 @@ struct rk_hdptx_phy { >> >> /* clk provider */ >> struct clk_hw hw; >> + bool pll_config_dirty; > > Which lock protects it? This is not protected by a driver-local lock, as the impacted calls are orchestrated by the DRM core, which should guaranty the expected sequence of operations: 1. encoder-check -> phy_configure -> no HW programming, pll_config_dirty=true 2. VOP2 crtc-enable -> set_rate -> HW commit, pll_config_dirty=false 3. bridge-enable -> phy_power_on, no HW programming (pll_config_dirty already false) > >> bool restrict_rate_change; >> >> atomic_t usage_count; >