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

Arnaud Vrac avrac at freebox.fr
Mon May 27 12:39:35 UTC 2024


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.

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.

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.

Thanks,
-Arnaud


More information about the Freedreno mailing list