[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <df4cfdb1-66be-4264-aed3-0d5567e721f7@linaro.org>
Date: Mon, 17 Feb 2025 14:37:31 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar
<quic_abhinavk@...cinc.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Jonathan Marek <jonathan@...ek.ca>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Rob Clark <robdclark@...omium.org>
Subject: Re: [PATCH v4 4/4] drm/msm/dsi/phy: Define PHY_CMN_CLK_CFG[01]
bitfields and simplify saving
On 17/02/2025 14:13, Dmitry Baryshkov wrote:
> On Mon, Feb 17, 2025 at 12:53:17PM +0100, Krzysztof Kozlowski wrote:
>> Add bitfields for PHY_CMN_CLK_CFG0 and PHY_CMN_CLK_CFG1 registers to
>> avoid hard-coding bit masks and shifts and make the code a bit more
>> readable. While touching the lines in dsi_7nm_pll_save_state()
>> resulting cached->pix_clk_div assignment would be too big, so just
>> combine pix_clk_div and bit_clk_div into one cached state to make
>> everything simpler.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>
>> ---
>>
>> Changes in v4:
>> 1. Add mising bitfield.h include
>> 2. One more FIELD_GET and DSI_7nm_PHY_CMN_CLK_CFG1_DSICLK_SEL (Dmitry)
>>
>> Changes in v3:
>> 1. Use FIELD_GET
>> 2. Keep separate bit_clk_div and pix_clk_div
>> 3. Rebase (some things moved to previous patches)
>>
>> Changes in v2:
>> 1. New patch
>> ---
>> drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 13 ++++++++-----
>> drivers/gpu/drm/msm/registers/display/dsi_phy_7nm.xml | 1 +
>> 2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> @@ -739,7 +741,8 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
>> u32 data;
>>
>> data = readl(pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>> - writel(data | 3, pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>> + writel(data | DSI_7nm_PHY_CMN_CLK_CFG1_DSICLK_SEL(3),
>> + pll_7nm->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>
> Nit: should this also be using dsi_pll_cmn_clk_cfg1_update() ?
This is before clocks are registered so there is really no chance for
simultaneous access.
It is rather then question of code readability or obviousness.
Best regards,
Krzysztof
Powered by blists - more mailing lists