[RFC PATCH v1] drm/msm: add msm8998 hdmi phy/pll support

Dmitry Baryshkov dmitry.baryshkov at linaro.org
Tue May 28 00:15:11 UTC 2024


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 at freebox.fr> wrote:
> > > 
> > > From: Arnaud Vrac <avrac at freebox.fr>
> > > 
> > > Ported from the downstream driver.
> > > 
> > > Signed-off-by: Arnaud Vrac <avrac at freebox.fr>
> > > Signed-off-by: Marc Gonzalez <mgonzalez at freebox.fr>
> > > ---
> > >   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


More information about the Freedreno mailing list