On Tue, Feb 04, 2025 at 04:48:43PM +0100, Krzysztof Kozlowski wrote:
> On 04/02/2025 15:28, Dmitry Baryshkov wrote:
> >>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c          | 31 
> >>>> ++++++++++++----------
> >>>>  .../gpu/drm/msm/registers/display/dsi_phy_7nm.xml  | 12 +++++++--
> >>>>  2 files changed, 27 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c 
> >>>> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> >>>> index 
> >>>> 926fd8e3330b2cdfc69d1e0e5d3930abae77b7d8..b61e75a01e1b69f33548ff0adefc5c92980a15d7
> >>>>  100644
> >>>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> >>>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> >>>> @@ -67,8 +67,7 @@ struct dsi_pll_config {
> >>>>  
> >>>>  struct pll_7nm_cached_state {
> >>>>          unsigned long vco_rate;
> >>>> -        u8 bit_clk_div;
> >>>> -        u8 pix_clk_div;
> >>>> +        u8 clk_div;
> >>>>          u8 pll_out_div;
> >>>>          u8 pll_mux;
> >>>>  };
> >>>> @@ -401,12 +400,12 @@ static void dsi_pll_cmn_clk_cfg1_update(struct 
> >>>> dsi_pll_7nm *pll, u32 mask,
> >>>>  
> >>>>  static void dsi_pll_disable_global_clk(struct dsi_pll_7nm *pll)
> >>>>  {
> >>>> -        dsi_pll_cmn_clk_cfg1_update(pll, BIT(5), 0);
> >>>> +        dsi_pll_cmn_clk_cfg1_update(pll, 
> >>>> DSI_7nm_PHY_CMN_CLK_CFG1_CLK_EN, 0);
> >>>>  }
> >>>>  
> >>>>  static void dsi_pll_enable_global_clk(struct dsi_pll_7nm *pll)
> >>>>  {
> >>>> -        u32 cfg_1 = BIT(5) | BIT(4);
> >>>> +        u32 cfg_1 = DSI_7nm_PHY_CMN_CLK_CFG1_CLK_EN | 
> >>>> DSI_7nm_PHY_CMN_CLK_CFG1_CLK_EN_SEL;
> >>>>  
> >>>>          writel(0x04, pll->phy->base + REG_DSI_7nm_PHY_CMN_CTRL_3);
> >>>>          dsi_pll_cmn_clk_cfg1_update(pll, cfg_1, cfg_1);
> >>>> @@ -572,15 +571,17 @@ static void dsi_7nm_pll_save_state(struct 
> >>>> msm_dsi_phy *phy)
> >>>>          cached->pll_out_div &= 0x3;
> >>>>  
> >>>>          cmn_clk_cfg0 = readl(phy_base + REG_DSI_7nm_PHY_CMN_CLK_CFG0);
> >>>> -        cached->bit_clk_div = cmn_clk_cfg0 & 0xf;
> >>>> -        cached->pix_clk_div = (cmn_clk_cfg0 & 0xf0) >> 4;
> >>>> +        cached->clk_div = cmn_clk_cfg0 & 
> >>>> (DSI_7nm_PHY_CMN_CLK_CFG0_DIV_CTRL_3_0__MASK |
> >>>> +                                          
> >>>> DSI_7nm_PHY_CMN_CLK_CFG0_DIV_CTRL_7_4__MASK);
> >>>
> >>> Could you rather store these two fields separately by using FIELD_GET?
> >>
> >> So make the code again more complicated? OK.
> > 
> > It was already there, bit_clk_div and pix_clk_div.
> 
> 
> Yes and I (believe) simplified it. It is subjective, so I don't mind
> going back to two fields, as you asked.

I'd prefer it this way, thank you.

-- 
With best wishes
Dmitry

Reply via email to