On Mon, May 27, 2024 at 02:39:35PM +0200, Arnaud Vrac wrote: > On 27/05/2024 14:11, Dmitry Baryshkov wrote: > > On Thu, 23 May 2024 at 18:14, Marc Gonzalez <mgonzalez@xxxxxxxxxx> wrote: > > > > > > From: Arnaud Vrac <avrac@xxxxxxxxxx> > > > > > > Ported from the downstream driver. > > > > > > Signed-off-by: Arnaud Vrac <avrac@xxxxxxxxxx> > > > Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx> > > > --- > > > drivers/gpu/drm/msm/Makefile | 1 + > > > drivers/gpu/drm/msm/hdmi/hdmi.c | 1 + > > > drivers/gpu/drm/msm/hdmi/hdmi.h | 8 + > > > drivers/gpu/drm/msm/hdmi/hdmi.xml.h | 162 ++++ > > > drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 5 + > > > drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 941 +++++++++++++++++++++++ > > > 6 files changed, 1118 insertions(+) > > > create mode 100644 drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c > > > > > > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile > > > index b21ae2880c715..5b5d6aded5233 100644 > > > --- a/drivers/gpu/drm/msm/Makefile > > > +++ b/drivers/gpu/drm/msm/Makefile > > > @@ -26,6 +26,7 @@ msm-$(CONFIG_DRM_MSM_HDMI) += \ > > > hdmi/hdmi_phy.o \ > > > hdmi/hdmi_phy_8960.o \ > > > hdmi/hdmi_phy_8996.o \ > > > + hdmi/hdmi_phy_8998.o \ > > > hdmi/hdmi_phy_8x60.o \ > > > hdmi/hdmi_phy_8x74.o \ > > > hdmi/hdmi_pll_8960.o \ > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > > > index c8ebd75176bba..2a2ce49ef5aa3 100644 > > > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > > > @@ -549,6 +549,7 @@ static void msm_hdmi_dev_remove(struct platform_device *pdev) > > > } > > > > > > static const struct of_device_id msm_hdmi_dt_match[] = { > > > + { .compatible = "qcom,hdmi-tx-8998", .data = &hdmi_tx_8974_config }, > > > > Missing DT bindings. > > > > > { .compatible = "qcom,hdmi-tx-8996", .data = &hdmi_tx_8974_config }, > > > { .compatible = "qcom,hdmi-tx-8994", .data = &hdmi_tx_8974_config }, > > > { .compatible = "qcom,hdmi-tx-8084", .data = &hdmi_tx_8974_config }, > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h > > > index ec57864403915..cad0d50c82fbc 100644 > > > --- a/drivers/gpu/drm/msm/hdmi/hdmi.h > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h > > > @@ -137,6 +137,7 @@ enum hdmi_phy_type { > > > MSM_HDMI_PHY_8960, > > > MSM_HDMI_PHY_8x74, > > > MSM_HDMI_PHY_8996, > > > + MSM_HDMI_PHY_8998, > > > MSM_HDMI_PHY_MAX, > > > }; > > > > > > @@ -154,6 +155,7 @@ extern const struct hdmi_phy_cfg msm_hdmi_phy_8x60_cfg; > > > extern const struct hdmi_phy_cfg msm_hdmi_phy_8960_cfg; > > > extern const struct hdmi_phy_cfg msm_hdmi_phy_8x74_cfg; > > > extern const struct hdmi_phy_cfg msm_hdmi_phy_8996_cfg; > > > +extern const struct hdmi_phy_cfg msm_hdmi_phy_8998_cfg; > > > > > > struct hdmi_phy { > > > struct platform_device *pdev; > > > @@ -184,6 +186,7 @@ void __exit msm_hdmi_phy_driver_unregister(void); > > > #ifdef CONFIG_COMMON_CLK > > > int msm_hdmi_pll_8960_init(struct platform_device *pdev); > > > int msm_hdmi_pll_8996_init(struct platform_device *pdev); > > > +int msm_hdmi_pll_8998_init(struct platform_device *pdev); > > > #else > > > static inline int msm_hdmi_pll_8960_init(struct platform_device *pdev) > > > { > > > @@ -194,6 +197,11 @@ static inline int msm_hdmi_pll_8996_init(struct platform_device *pdev) > > > { > > > return -ENODEV; > > > } > > > + > > > +static inline int msm_hdmi_pll_8998_init(struct platform_device *pdev) > > > +{ > > > + return -ENODEV; > > > +} > > > #endif > > > > > > /* > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h > > > index 973b460486a5a..c9ca1101b5ad4 100644 > > > --- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h > > > @@ -1396,4 +1396,166 @@ static inline uint32_t HDMI_8x60_PHY_REG1_OUTVOL_SWING_CTRL(uint32_t val) > > > #define REG_HDMI_PHY_QSERDES_TX_LX_TX_ALOG_INTF_OBSV 0x00000110 > > > > > > > > > +#define REG_HDMI_8998_PHY_CFG 0x00000000 > > > + > > > +#define REG_HDMI_8998_PHY_PD_CTL 0x00000004 > > > + > > > +#define REG_HDMI_8998_PHY_MODE 0x00000010 > > > + > > > +#define REG_HDMI_8998_PHY_CLOCK 0x0000005c > > > + > > > +#define REG_HDMI_8998_PHY_CMN_CTRL 0x00000068 > > > + > > > +#define REG_HDMI_8998_PHY_STATUS 0x000000b4 > > > + > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_ATB_SEL1 0x00000000 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_ATB_SEL2 0x00000004 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_FREQ_UPDATE 0x00000008 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_BG_TIMER 0x0000000c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_EN_CENTER 0x00000010 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_ADJ_PER1 0x00000014 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_ADJ_PER2 0x00000018 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_PER1 0x0000001c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_PER2 0x00000020 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_STEP_SIZE1 0x00000024 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SSC_STEP_SIZE2 0x00000028 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_POST_DIV 0x0000002c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_POST_DIV_MUX 0x00000030 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_BIAS_EN_CLKBUFLR_EN 0x00000034 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CLK_ENABLE1 0x00000038 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SYS_CLK_CTRL 0x0000003c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SYSCLK_BUF_ENABLE 0x00000040 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_EN 0x00000044 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_IVCO 0x00000048 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CMN_IETRIM 0x0000004c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CMN_IPTRIM 0x00000050 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CP_CTRL_MODE0 0x00000060 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CP_CTRL_MODE1 0x00000064 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_RCTRL_MODE0 0x00000068 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_RCTRL_MODE1 0x0000006c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_CCTRL_MODE0 0x00000070 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_CCTRL_MODE1 0x00000074 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_PLL_CNTRL 0x00000078 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_BIAS_EN_CTRL_BY_PSM 0x0000007c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SYSCLK_EN_SEL 0x00000080 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CML_SYSCLK_SEL 0x00000084 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_RESETSM_CNTRL 0x00000088 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_RESETSM_CNTRL2 0x0000008c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_LOCK_CMP_EN 0x00000090 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_LOCK_CMP_CFG 0x00000094 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_LOCK_CMP1_MODE0 0x00000098 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_LOCK_CMP2_MODE0 0x0000009c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_LOCK_CMP3_MODE0 0x000000a0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DEC_START_MODE0 0x000000b0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DEC_START_MODE1 0x000000b4 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START1_MODE0 0x000000b8 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START2_MODE0 0x000000bc > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START3_MODE0 0x000000c0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START1_MODE1 0x000000c4 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START2_MODE1 0x000000c8 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_DIV_FRAC_START3_MODE1 0x000000cc > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_INITVAL 0x000000d0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_EN 0x000000d4 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_GAIN0_MODE0 0x000000d8 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_GAIN1_MODE0 0x000000dc > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_GAIN0_MODE1 0x000000e0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_INTEGLOOP_GAIN1_MODE1 0x000000e4 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_VCOCAL_DEADMAN_CTRL 0x000000e8 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_VCO_TUNE_CTRL 0x000000ec > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_VCO_TUNE_MAP 0x000000f0 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CMN_STATUS 0x00000124 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_RESET_SM_STATUS 0x00000128 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CLK_SEL 0x00000138 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_HSCLK_SEL 0x0000013c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CORECLK_DIV_MODE0 0x00000148 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SW_RESET 0x00000150 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CORE_CLK_EN 0x00000154 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_C_READY_STATUS 0x00000158 > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_CMN_CONFIG 0x0000015c > > > + > > > +#define REG_HDMI_8998_PHY_QSERDES_COM_SVS_MODE_CLK_SEL 0x00000164 > > > + > > > + > > > +#define REG_HDMI_8998_PHY_TXn_EMP_POST1_LVL 0x00000000 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_INTERFACE_SELECT_TX_BAND 0x00000008 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_CLKBUF_TERM_ENABLE 0x0000000c > > > + > > > +#define REG_HDMI_8998_PHY_TXn_DRV_LVL_RES_CODE_OFFSET 0x00000014 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_DRV_LVL 0x00000018 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_LANE_CONFIG 0x0000001c > > > + > > > +#define REG_HDMI_8998_PHY_TXn_PRE_DRIVER_1 0x00000024 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_PRE_DRIVER_2 0x00000028 > > > + > > > +#define REG_HDMI_8998_PHY_TXn_LANE_MODE 0x0000002c > > > + > > > #endif /* HDMI_XML */ > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c > > > index 88a3423b7f24d..95b3f7535d840 100644 > > > --- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c > > > @@ -118,6 +118,9 @@ static int msm_hdmi_phy_pll_init(struct platform_device *pdev, > > > case MSM_HDMI_PHY_8996: > > > ret = msm_hdmi_pll_8996_init(pdev); > > > break; > > > + case MSM_HDMI_PHY_8998: > > > + ret = msm_hdmi_pll_8998_init(pdev); > > > + break; > > > /* > > > * we don't have PLL support for these, don't report an error for now > > > */ > > > @@ -193,6 +196,8 @@ static const struct of_device_id msm_hdmi_phy_dt_match[] = { > > > .data = &msm_hdmi_phy_8x74_cfg }, > > > { .compatible = "qcom,hdmi-phy-8996", > > > .data = &msm_hdmi_phy_8996_cfg }, > > > + { .compatible = "qcom,hdmi-phy-8998", > > > + .data = &msm_hdmi_phy_8998_cfg }, > > > {} > > > }; > > > > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c > > > new file mode 100644 > > > index 0000000000000..28c4824a30e89 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c > > > @@ -0,0 +1,941 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (c) 2016, The Linux Foundation. All rights reserved. > > > > No changes since 2016? > > > > > + */ > > > + > > > +#include <linux/clk-provider.h> > > > +#include <linux/delay.h> > > > + > > > > [...] > > > > > + > > > +static inline u64 pll_cmp_to_fdata(u32 pll_cmp, unsigned long ref_clk) > > > +{ > > > + u64 fdata = ((u64)pll_cmp) * ref_clk * 10; > > > + > > > + do_div(fdata, HDMI_PLL_CMP_CNT); > > > + > > > + return fdata; > > > +} > > > + > > > +#if 0 > > > > This should probably go away. > > > > > +static int pll_get_post_div(struct hdmi_8998_post_divider *pd, u64 bclk) > > > +{ > > > + /* FIXME: use downstream ratio list ? */ > > > + int ratio[] = { 2, 3, 4, 5, 6, 9, 10, 12, 14, 15, 20, 21, 25, 28, 35 }; > > > + int hs_divsel[] = { 0, 4, 8, 12, 1, 5, 2, 9, 3, 13, 10, 7, 14, 11, 15 }; > > > + int tx_band_sel[] = { 0, 1, 2, 3 }; > > > + u64 vco_freq[60]; > > > + u64 vco, vco_optimal; > > > + int half_rate_mode = 0; > > > + int vco_optimal_index, vco_freq_index; > > > + int i, j; > > > + > > > > So, first of all, the code needs to be cleaned. It contains debugging > > and temporary code all over the place. Such code should be removed > > > > Second, at some point I worked on moving HDMI PHY drivers to > > drivers/phy. Oh my, it was nearly a year ago. > > https://patchwork.freedesktop.org/series/118210/ > > > > I hope to land the HDMI HPD rework this cycle, then get back to the > > HDMI PHY code. No promises though, just wanted to point out that we > > might need to rework this even further in few months. > > > > > > Thanks Dmitry. I wasn't planning on sending the patch anywhere in this > state, but Marc did so I'll ask some of the questions I had when I > wrote this. The first thing I was planning to do was indeed to rebase > on your patch series refactoring PHY drivers. > > First, I understand the XML files describing registers have been > integrated in the kernel tree, so we will have to move the definitions > of the 8998 PHY registers there. Yes > Second, the #if 0 code is the phy setup code I tried to write based on > the simpler 8996 driver, but it doesn't work, hence the actual > compiled code which is a direct port of the downstream code. We'll > probably have to dig a little more to ask more detailed questions on > the phy internals. Let's see, probably we can figure them out. > Last, I tried to implement the recalc_rate callback which does not > exist downstream but I'm not able to get a correct value with the > little data I have on the registers. Is this callback actually needed > ? I don't see why the values programmed in the set_rate call could > change under our feet. The recalc_rate callback is used by CCF to read back the rate from the hardware. For example, when CCF probes all the clocks. Regarding reading the clock rate. As you can see from the 8996 PHY driver, it's easier to follow LOCK_CMPn_MODE0 registers. It *looks* like the code from the refactored 8996 driver should be mostly applicable for 8998 too. Maybe except for the 15 => 1 vs 15 => 35 HSCLK setting). In the end, msm8998 uses what looks like QMP v3 with a different TX register set -- With best wishes Dmitry