All of lore.kernel.org
 help / color / mirror / Atom feed
From: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Rob Clark <robdclark@gmail.com>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Jonathan Marek <jonathan@marek.ca>
Cc: linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 5/6] drm/msm/dsi: add support for DSI-PHY on SM8550
Date: Wed, 4 Jan 2023 11:11:50 +0100	[thread overview]
Message-ID: <0e28b301-6980-968c-552d-db16fade6df9@linaro.org> (raw)
In-Reply-To: <aa6724af-99bc-de1d-4c03-82609b59174c@linaro.org>

On 04/01/2023 10:53, Dmitry Baryshkov wrote:
> On 04/01/2023 11:08, Neil Armstrong wrote:
>> SM8550 use a 4nm DSI PHYs, which share register definitions
>> with 7nm DSI PHYs. Rather than duplicating the driver, handle
>> 4nm variant inside the common 5+7nm driver.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/Kconfig               |   4 +-
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c     |   2 +
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h     |   1 +
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 102 ++++++++++++++++++++++++------
>>   4 files changed, 89 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
>> index e7b100d97f88..949b18a29a55 100644
>> --- a/drivers/gpu/drm/msm/Kconfig
>> +++ b/drivers/gpu/drm/msm/Kconfig
>> @@ -140,11 +140,11 @@ config DRM_MSM_DSI_10NM_PHY
>>         Choose this option if DSI PHY on SDM845 is used on the platform.
>>   config DRM_MSM_DSI_7NM_PHY
>> -    bool "Enable DSI 7nm/5nm PHY driver in MSM DRM"
>> +    bool "Enable DSI 7nm/5nm/4nm PHY driver in MSM DRM"
>>       depends on DRM_MSM_DSI
>>       default y
>>       help
>> -      Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SC7280
>> +      Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SM8550/SC7280
>>         is used on the platform.
>>   config DRM_MSM_HDMI
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> index 0c956fdab23e..54e03cc9fbe7 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> @@ -573,6 +573,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>>         .data = &dsi_phy_5nm_8350_cfgs },
>>       { .compatible = "qcom,dsi-phy-5nm-8450",
>>         .data = &dsi_phy_5nm_8450_cfgs },
>> +    { .compatible = "qcom,dsi-phy-4nm-8550",
>> +      .data = &dsi_phy_4nm_8550_cfgs },
>>   #endif
>>       {}
>>   };
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> index f7a907ed2b4b..58f9e09f5224 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> @@ -59,6 +59,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
>> +extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs;
>>   struct msm_dsi_dphy_timing {
>>       u32 clk_zero;
>> 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 7b2c16b3a36c..11629c431c30 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> @@ -47,6 +47,8 @@
>>   #define DSI_PHY_7NM_QUIRK_V4_2        BIT(2)
>>   /* Hardware is V4.3 */
>>   #define DSI_PHY_7NM_QUIRK_V4_3        BIT(3)
>> +/* Hardware is V5.2 */
>> +#define DSI_PHY_7NM_QUIRK_V5_2        BIT(4)
>>   struct dsi_pll_config {
>>       bool enable_ssc;
>> @@ -124,14 +126,25 @@ static void dsi_pll_calc_dec_frac(struct dsi_pll_7nm *pll, struct dsi_pll_config
>>       if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)
>>           config->pll_clock_inverters = 0x28;
>> -    else if (pll_freq <= 1000000000ULL)
>> -        config->pll_clock_inverters = 0xa0;
>> -    else if (pll_freq <= 2500000000ULL)
>> -        config->pll_clock_inverters = 0x20;
>> -    else if (pll_freq <= 3020000000ULL)
>> -        config->pll_clock_inverters = 0x00;
>> -    else
>> -        config->pll_clock_inverters = 0x40;
>> +    else if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (pll_freq <= 1300000000ULL)
>> +            config->pll_clock_inverters = 0xa0;
>> +        else if (pll_freq <= 2500000000ULL)
>> +            config->pll_clock_inverters = 0x20;
>> +        else if (pll_freq <= 4000000000ULL)
>> +            config->pll_clock_inverters = 0x00;
>> +        else
>> +            config->pll_clock_inverters = 0x40;
>> +    } else {
>> +        if (pll_freq <= 1000000000ULL)
>> +            config->pll_clock_inverters = 0xa0;
>> +        else if (pll_freq <= 2500000000ULL)
>> +            config->pll_clock_inverters = 0x20;
>> +        else if (pll_freq <= 3020000000ULL)
>> +            config->pll_clock_inverters = 0x00;
>> +        else
>> +            config->pll_clock_inverters = 0x40;
>> +    }
>>       config->decimal_div_start = dec;
>>       config->frac_div_start = frac;
>> @@ -222,6 +235,13 @@ static void dsi_pll_config_hzindep_reg(struct dsi_pll_7nm *pll)
>>               vco_config_1 = 0x01;
>>       }
>> +    if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (pll->vco_current_rate < 1557000000ULL)
>> +            vco_config_1 = 0x08;
>> +        else
>> +            vco_config_1 = 0x01;
>> +    }
>> +
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_ANALOG_CONTROLS_FIVE_1,
>>                 analog_controls_five_1);
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_VCO_CONFIG_1, vco_config_1);
>> @@ -860,7 +880,8 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>           pr_warn("PLL turned on before configuring PHY\n");
>>       /* Request for REFGEN READY */
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) ||
>> +        (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>>           dsi_phy_write(phy->base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x1);
>>           udelay(500);
>>       }
>> @@ -881,20 +902,38 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>       glbl_str_swi_cal_sel_ctrl = 0x00;
>>       if (phy->cphy_mode) {
>> -        vreg_ctrl_0 = 0x51;
>> -        vreg_ctrl_1 = 0x55;
>> +        if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +            vreg_ctrl_0 = 0x45;
>> +            vreg_ctrl_1 = 0x45;
>> +        } else {
>> +            vreg_ctrl_0 = 0x51;
>> +            vreg_ctrl_1 = 0x55;
>> +        }
> 
> Please move these quirk-specific values down, to the rest of if (QUIRK_5_2) statement.

Ok

> 
>>           glbl_hstx_str_ctrl_0 = 0x00;
>>           glbl_pemph_ctrl_0 = 0x11;
>>           lane_ctrl0 = 0x17;
>>       } else {
>> -        vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
>> -        vreg_ctrl_1 = 0x5c;
>> +        if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +            vreg_ctrl_0 = 0x44;
>> +            vreg_ctrl_1 = 0x19;
>> +        } else {
>> +            vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
>> +            vreg_ctrl_1 = 0x5c;
>> +        }
>>           glbl_hstx_str_ctrl_0 = 0x88;
>>           glbl_pemph_ctrl_0 = 0x00;
>>           lane_ctrl0 = 0x1f;
>>       }
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (phy->cphy_mode) {
>> +            glbl_rescode_top_ctrl = 0x00;
>> +            glbl_rescode_bot_ctrl = 0x00;
>> +        } else {
>> +            glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3c :  0x03;
>> +            glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3c;
>> +        }
>> +    } else if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3)) {
>>           if (phy->cphy_mode) {
>>               glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x01;
>>               glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3b;
>> @@ -943,9 +982,8 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_RBUF_CTRL, 0x00);
>>       /* program CMN_CTRL_4 for minor_ver 2 chipsets*/
>> -    data = dsi_phy_read(base + REG_DSI_7nm_PHY_CMN_REVISION_ID0);
>> -    data = data & (0xf0);
>> -    if (data == 0x20)
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2) ||
>> +        (dsi_phy_read(base + REG_DSI_7nm_PHY_CMN_REVISION_ID0) & (0xf0)) == 0x20)
>>           dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_CTRL_4, 0x04);
> 
> Ugh. I should change this statement to use quirks too.

Sorrt I don't see what you mean, should I change the original REVISION_ID0 to a proper quirk ?

> 
>>       /* Configure PHY lane swap (TODO: we need to calculate this) */
>> @@ -1058,7 +1096,8 @@ static void dsi_7nm_phy_disable(struct msm_dsi_phy *phy)
>>       dsi_phy_hw_v4_0_config_lpcdrx(phy, false);
>>       /* Turn off REFGEN Vote */
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) ||
>> +        (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>>           dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x0);
>>           wmb();
>>           /* Delay to ensure HW removes vote before PHY shut down */
>> @@ -1092,6 +1131,10 @@ static const struct regulator_bulk_data dsi_phy_7nm_97800uA_regulators[] = {
>>       { .supply = "vdds", .init_load_uA = 97800 },
>>   };
>> +static const struct regulator_bulk_data dsi_phy_7nm_98400uA_regulators[] = {
>> +    { .supply = "vdds", .init_load_uA = 98400 },
>> +};
>> +
>>   const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs = {
>>       .has_phy_lane = true,
>>       .regulator_data = dsi_phy_7nm_36mA_regulators,
>> @@ -1201,3 +1244,26 @@ const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs = {
>>       .num_dsi_phy = 2,
>>       .quirks = DSI_PHY_7NM_QUIRK_V4_3,
>>   };
>> +
>> +const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs = {
>> +    .has_phy_lane = true,
>> +    .regulator_data = dsi_phy_7nm_98400uA_regulators,
>> +    .num_regulators = ARRAY_SIZE(dsi_phy_7nm_98400uA_regulators),
>> +    .ops = {
>> +        .enable = dsi_7nm_phy_enable,
>> +        .disable = dsi_7nm_phy_disable,
>> +        .pll_init = dsi_pll_7nm_init,
>> +        .save_pll_state = dsi_7nm_pll_save_state,
>> +        .restore_pll_state = dsi_7nm_pll_restore_state,
>> +        .set_continuous_clock = dsi_7nm_set_continuous_clock,
>> +    },
>> +    .min_pll_rate = 600000000UL,
>> +#ifdef CONFIG_64BIT
>> +    .max_pll_rate = 5000000000UL,
>> +#else
>> +    .max_pll_rate = ULONG_MAX,
>> +#endif
>> +    .io_start = { 0xae95000, 0xae97000 },
>> +    .num_dsi_phy = 2,
>> +    .quirks = DSI_PHY_7NM_QUIRK_V5_2,
>> +};
>>
> 

Thanks,
Neil

WARNING: multiple messages have this Message-ID (diff)
From: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Rob Clark <robdclark@gmail.com>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Jonathan Marek <jonathan@marek.ca>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/6] drm/msm/dsi: add support for DSI-PHY on SM8550
Date: Wed, 4 Jan 2023 11:11:50 +0100	[thread overview]
Message-ID: <0e28b301-6980-968c-552d-db16fade6df9@linaro.org> (raw)
In-Reply-To: <aa6724af-99bc-de1d-4c03-82609b59174c@linaro.org>

On 04/01/2023 10:53, Dmitry Baryshkov wrote:
> On 04/01/2023 11:08, Neil Armstrong wrote:
>> SM8550 use a 4nm DSI PHYs, which share register definitions
>> with 7nm DSI PHYs. Rather than duplicating the driver, handle
>> 4nm variant inside the common 5+7nm driver.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/Kconfig               |   4 +-
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c     |   2 +
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h     |   1 +
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 102 ++++++++++++++++++++++++------
>>   4 files changed, 89 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
>> index e7b100d97f88..949b18a29a55 100644
>> --- a/drivers/gpu/drm/msm/Kconfig
>> +++ b/drivers/gpu/drm/msm/Kconfig
>> @@ -140,11 +140,11 @@ config DRM_MSM_DSI_10NM_PHY
>>         Choose this option if DSI PHY on SDM845 is used on the platform.
>>   config DRM_MSM_DSI_7NM_PHY
>> -    bool "Enable DSI 7nm/5nm PHY driver in MSM DRM"
>> +    bool "Enable DSI 7nm/5nm/4nm PHY driver in MSM DRM"
>>       depends on DRM_MSM_DSI
>>       default y
>>       help
>> -      Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SC7280
>> +      Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SM8550/SC7280
>>         is used on the platform.
>>   config DRM_MSM_HDMI
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> index 0c956fdab23e..54e03cc9fbe7 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> @@ -573,6 +573,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>>         .data = &dsi_phy_5nm_8350_cfgs },
>>       { .compatible = "qcom,dsi-phy-5nm-8450",
>>         .data = &dsi_phy_5nm_8450_cfgs },
>> +    { .compatible = "qcom,dsi-phy-4nm-8550",
>> +      .data = &dsi_phy_4nm_8550_cfgs },
>>   #endif
>>       {}
>>   };
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> index f7a907ed2b4b..58f9e09f5224 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> @@ -59,6 +59,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
>> +extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs;
>>   struct msm_dsi_dphy_timing {
>>       u32 clk_zero;
>> 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 7b2c16b3a36c..11629c431c30 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> @@ -47,6 +47,8 @@
>>   #define DSI_PHY_7NM_QUIRK_V4_2        BIT(2)
>>   /* Hardware is V4.3 */
>>   #define DSI_PHY_7NM_QUIRK_V4_3        BIT(3)
>> +/* Hardware is V5.2 */
>> +#define DSI_PHY_7NM_QUIRK_V5_2        BIT(4)
>>   struct dsi_pll_config {
>>       bool enable_ssc;
>> @@ -124,14 +126,25 @@ static void dsi_pll_calc_dec_frac(struct dsi_pll_7nm *pll, struct dsi_pll_config
>>       if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)
>>           config->pll_clock_inverters = 0x28;
>> -    else if (pll_freq <= 1000000000ULL)
>> -        config->pll_clock_inverters = 0xa0;
>> -    else if (pll_freq <= 2500000000ULL)
>> -        config->pll_clock_inverters = 0x20;
>> -    else if (pll_freq <= 3020000000ULL)
>> -        config->pll_clock_inverters = 0x00;
>> -    else
>> -        config->pll_clock_inverters = 0x40;
>> +    else if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (pll_freq <= 1300000000ULL)
>> +            config->pll_clock_inverters = 0xa0;
>> +        else if (pll_freq <= 2500000000ULL)
>> +            config->pll_clock_inverters = 0x20;
>> +        else if (pll_freq <= 4000000000ULL)
>> +            config->pll_clock_inverters = 0x00;
>> +        else
>> +            config->pll_clock_inverters = 0x40;
>> +    } else {
>> +        if (pll_freq <= 1000000000ULL)
>> +            config->pll_clock_inverters = 0xa0;
>> +        else if (pll_freq <= 2500000000ULL)
>> +            config->pll_clock_inverters = 0x20;
>> +        else if (pll_freq <= 3020000000ULL)
>> +            config->pll_clock_inverters = 0x00;
>> +        else
>> +            config->pll_clock_inverters = 0x40;
>> +    }
>>       config->decimal_div_start = dec;
>>       config->frac_div_start = frac;
>> @@ -222,6 +235,13 @@ static void dsi_pll_config_hzindep_reg(struct dsi_pll_7nm *pll)
>>               vco_config_1 = 0x01;
>>       }
>> +    if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (pll->vco_current_rate < 1557000000ULL)
>> +            vco_config_1 = 0x08;
>> +        else
>> +            vco_config_1 = 0x01;
>> +    }
>> +
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_ANALOG_CONTROLS_FIVE_1,
>>                 analog_controls_five_1);
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_VCO_CONFIG_1, vco_config_1);
>> @@ -860,7 +880,8 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>           pr_warn("PLL turned on before configuring PHY\n");
>>       /* Request for REFGEN READY */
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) ||
>> +        (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>>           dsi_phy_write(phy->base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x1);
>>           udelay(500);
>>       }
>> @@ -881,20 +902,38 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>       glbl_str_swi_cal_sel_ctrl = 0x00;
>>       if (phy->cphy_mode) {
>> -        vreg_ctrl_0 = 0x51;
>> -        vreg_ctrl_1 = 0x55;
>> +        if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +            vreg_ctrl_0 = 0x45;
>> +            vreg_ctrl_1 = 0x45;
>> +        } else {
>> +            vreg_ctrl_0 = 0x51;
>> +            vreg_ctrl_1 = 0x55;
>> +        }
> 
> Please move these quirk-specific values down, to the rest of if (QUIRK_5_2) statement.

Ok

> 
>>           glbl_hstx_str_ctrl_0 = 0x00;
>>           glbl_pemph_ctrl_0 = 0x11;
>>           lane_ctrl0 = 0x17;
>>       } else {
>> -        vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
>> -        vreg_ctrl_1 = 0x5c;
>> +        if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +            vreg_ctrl_0 = 0x44;
>> +            vreg_ctrl_1 = 0x19;
>> +        } else {
>> +            vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
>> +            vreg_ctrl_1 = 0x5c;
>> +        }
>>           glbl_hstx_str_ctrl_0 = 0x88;
>>           glbl_pemph_ctrl_0 = 0x00;
>>           lane_ctrl0 = 0x1f;
>>       }
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>> +        if (phy->cphy_mode) {
>> +            glbl_rescode_top_ctrl = 0x00;
>> +            glbl_rescode_bot_ctrl = 0x00;
>> +        } else {
>> +            glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3c :  0x03;
>> +            glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3c;
>> +        }
>> +    } else if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3)) {
>>           if (phy->cphy_mode) {
>>               glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x01;
>>               glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3b;
>> @@ -943,9 +982,8 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>>       dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_RBUF_CTRL, 0x00);
>>       /* program CMN_CTRL_4 for minor_ver 2 chipsets*/
>> -    data = dsi_phy_read(base + REG_DSI_7nm_PHY_CMN_REVISION_ID0);
>> -    data = data & (0xf0);
>> -    if (data == 0x20)
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2) ||
>> +        (dsi_phy_read(base + REG_DSI_7nm_PHY_CMN_REVISION_ID0) & (0xf0)) == 0x20)
>>           dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_CTRL_4, 0x04);
> 
> Ugh. I should change this statement to use quirks too.

Sorrt I don't see what you mean, should I change the original REVISION_ID0 to a proper quirk ?

> 
>>       /* Configure PHY lane swap (TODO: we need to calculate this) */
>> @@ -1058,7 +1096,8 @@ static void dsi_7nm_phy_disable(struct msm_dsi_phy *phy)
>>       dsi_phy_hw_v4_0_config_lpcdrx(phy, false);
>>       /* Turn off REFGEN Vote */
>> -    if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
>> +    if ((phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) ||
>> +        (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V5_2)) {
>>           dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x0);
>>           wmb();
>>           /* Delay to ensure HW removes vote before PHY shut down */
>> @@ -1092,6 +1131,10 @@ static const struct regulator_bulk_data dsi_phy_7nm_97800uA_regulators[] = {
>>       { .supply = "vdds", .init_load_uA = 97800 },
>>   };
>> +static const struct regulator_bulk_data dsi_phy_7nm_98400uA_regulators[] = {
>> +    { .supply = "vdds", .init_load_uA = 98400 },
>> +};
>> +
>>   const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs = {
>>       .has_phy_lane = true,
>>       .regulator_data = dsi_phy_7nm_36mA_regulators,
>> @@ -1201,3 +1244,26 @@ const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs = {
>>       .num_dsi_phy = 2,
>>       .quirks = DSI_PHY_7NM_QUIRK_V4_3,
>>   };
>> +
>> +const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs = {
>> +    .has_phy_lane = true,
>> +    .regulator_data = dsi_phy_7nm_98400uA_regulators,
>> +    .num_regulators = ARRAY_SIZE(dsi_phy_7nm_98400uA_regulators),
>> +    .ops = {
>> +        .enable = dsi_7nm_phy_enable,
>> +        .disable = dsi_7nm_phy_disable,
>> +        .pll_init = dsi_pll_7nm_init,
>> +        .save_pll_state = dsi_7nm_pll_save_state,
>> +        .restore_pll_state = dsi_7nm_pll_restore_state,
>> +        .set_continuous_clock = dsi_7nm_set_continuous_clock,
>> +    },
>> +    .min_pll_rate = 600000000UL,
>> +#ifdef CONFIG_64BIT
>> +    .max_pll_rate = 5000000000UL,
>> +#else
>> +    .max_pll_rate = ULONG_MAX,
>> +#endif
>> +    .io_start = { 0xae95000, 0xae97000 },
>> +    .num_dsi_phy = 2,
>> +    .quirks = DSI_PHY_7NM_QUIRK_V5_2,
>> +};
>>
> 

Thanks,
Neil

  reply	other threads:[~2023-01-04 10:11 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-04  9:08 [PATCH 0/6] drm/msm: add support for SM8450 Neil Armstrong
2023-01-04  9:08 ` Neil Armstrong
2023-01-04  9:08 ` [PATCH 1/6] dt-bindings: display/msm: document the SM8550 DSI PHY Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-06 15:41   ` Krzysztof Kozlowski
2023-01-06 15:41     ` Krzysztof Kozlowski
2023-01-08 23:09     ` Dmitry Baryshkov
2023-01-08 23:09       ` Dmitry Baryshkov
2023-01-09  8:20       ` Neil Armstrong
2023-01-09  8:20         ` Neil Armstrong
2023-01-04  9:08 ` [PATCH 2/6] dt-bindings: display/msm: document the display hardware for SM8550 Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-05  2:53   ` Rob Herring
2023-01-05  2:53     ` Rob Herring
2023-01-08 23:11   ` Dmitry Baryshkov
2023-01-08 23:11     ` Dmitry Baryshkov
2023-01-04  9:08 ` [PATCH 3/6] drm/msm/dpu: add support " Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-04  9:45   ` Dmitry Baryshkov
2023-01-04  9:45     ` Dmitry Baryshkov
2023-01-04 10:08     ` Neil Armstrong
2023-01-04 10:08       ` Neil Armstrong
2023-01-04 17:48       ` Dmitry Baryshkov
2023-01-04 17:48         ` Dmitry Baryshkov
2023-01-05  9:59         ` neil.armstrong
2023-01-05  9:59           ` neil.armstrong
2023-01-08 23:12   ` Dmitry Baryshkov
2023-01-08 23:12     ` Dmitry Baryshkov
2023-01-04  9:08 ` [PATCH 4/6] drm/msm: mdss: " Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-04  9:46   ` Dmitry Baryshkov
2023-01-04  9:46     ` Dmitry Baryshkov
2023-01-04  9:08 ` [PATCH 5/6] drm/msm/dsi: add support for DSI-PHY on SM8550 Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-04  9:53   ` Dmitry Baryshkov
2023-01-04  9:53     ` Dmitry Baryshkov
2023-01-04 10:11     ` Neil Armstrong [this message]
2023-01-04 10:11       ` Neil Armstrong
2023-01-04 17:20       ` Dmitry Baryshkov
2023-01-04 17:20         ` Dmitry Baryshkov
2023-01-04  9:08 ` [PATCH 6/6] drm/msm/dsi: add support for DSI 2.7.0 Neil Armstrong
2023-01-04  9:08   ` Neil Armstrong
2023-01-04  9:54   ` Dmitry Baryshkov
2023-01-04  9:54     ` Dmitry Baryshkov
2023-01-08 23:09 ` [PATCH 0/6] drm/msm: add support for SM8450 Dmitry Baryshkov
2023-01-08 23:09   ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e28b301-6980-968c-552d-db16fade6df9@linaro.org \
    --to=neil.armstrong@linaro.org \
    --cc=airlied@gmail.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jonathan@marek.ca \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_abhinavk@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sean@poorly.run \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.