| Message ID | 20240403040517.3279-1-liankun.yang@mediatek.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v1,1/1] drm/mediatek/dp: The register is written with the parsed DTS SSC value. | expand |
On 03/04/2024 06:05, Liankun Yang wrote: > [Description] > Severe screen flickering has been observed on the external display > when the DP projection function is used with the market expansion dock. > > + if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR)) { > + regmap_update_bits(dp_phy->regs, ssc_reg_offset, > + XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT, read_value); > + } else if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR)) { > + read_value = read_value << 16 | 0x0000; > + regmap_update_bits(dp_phy->regs, ssc_reg_offset, > + XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT, read_value); > + } > + > + return 0; > +} > + > +static struct device_node *mtk_dp_get_ssc_node(struct phy *phy, struct mtk_dp_phy *dp_phy) > +{ > + struct device_node *mode_node = NULL; > + > + mode_node = of_find_node_by_name(dp_phy->dev->of_node, SSC_SETTING); ?!?! You have the node, why do you try to find it? Best regards, Krzysztof
On 03/04/2024 08:41, Krzysztof Kozlowski wrote: > On 03/04/2024 06:05, Liankun Yang wrote: >> [Description] >> Severe screen flickering has been observed on the external display >> when the DP projection function is used with the market expansion dock. >> > >> + if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR)) { >> + regmap_update_bits(dp_phy->regs, ssc_reg_offset, >> + XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT, read_value); >> + } else if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR)) { >> + read_value = read_value << 16 | 0x0000; >> + regmap_update_bits(dp_phy->regs, ssc_reg_offset, >> + XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT, read_value); >> + } >> + >> + return 0; >> +} >> + >> +static struct device_node *mtk_dp_get_ssc_node(struct phy *phy, struct mtk_dp_phy *dp_phy) >> +{ >> + struct device_node *mode_node = NULL; >> + >> + mode_node = of_find_node_by_name(dp_phy->dev->of_node, SSC_SETTING); > > ?!?! > You have the node, why do you try to find it? > Wait, that was brainfuck from my side or -ENOCOFFEE. Ignore. I still have a question though, where did you document new ABI: dependency on the node name here? Also, why you are not going through direct children - of_get_child_by_name()? Best regards, Krzysztof
On Wed, 2024-04-03 at 16:56 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 03/04/2024 08:41, Krzysztof Kozlowski wrote: > > On 03/04/2024 06:05, Liankun Yang wrote: > >> [Description] > >> Severe screen flickering has been observed on the external display > >> when the DP projection function is used with the market expansion > dock. > >> > > > >> +if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR)) { > >> +regmap_update_bits(dp_phy->regs, ssc_reg_offset, > >> + XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT, read_value); > >> +} else if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR)) { > >> +read_value = read_value << 16 | 0x0000; > >> +regmap_update_bits(dp_phy->regs, ssc_reg_offset, > >> + XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT, read_value); > >> +} > >> + > >> +return 0; > >> +} > >> + > >> +static struct device_node *mtk_dp_get_ssc_node(struct phy *phy, > struct mtk_dp_phy *dp_phy) > >> +{ > >> +struct device_node *mode_node = NULL; > >> + > >> +mode_node = of_find_node_by_name(dp_phy->dev->of_node, > SSC_SETTING); > > > > ?!?! > > You have the node, why do you try to find it? > > > > Wait, that was brainfuck from my side or -ENOCOFFEE. Ignore. > > I still have a question though, where did you document new ABI: > dependency on the node name here? > > Also, why you are not going through direct children - > of_get_child_by_name()? > > Best regards, > Krzysztof > > Sorry, there is formatting issue in the previous email. The dp_phy device has already been registered through the mtk_dp_register_phy function in the mtk_dp.c file, so it cannot be redefined in the dts. Avoid using of_get_child_by_name for this purpose. To find the node name, utilize of_find_node_by_name since it has already been registered. Best regards, Liankun Yang >
On 11/04/2024 08:30, LIANKUN YANG (杨连坤) wrote: > On Wed, 2024-04-03 at 16:56 +0200, Krzysztof Kozlowski wrote: >> >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> On 03/04/2024 08:41, Krzysztof Kozlowski wrote: >>> On 03/04/2024 06:05, Liankun Yang wrote: >>>> [Description] >>>> Severe screen flickering has been observed on the external display >>>> when the DP projection function is used with the market expansion >> dock. >>>> >>> >>>> +if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR)) { >>>> +regmap_update_bits(dp_phy->regs, ssc_reg_offset, >>>> + XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT, read_value); >>>> +} else if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR)) { >>>> +read_value = read_value << 16 | 0x0000; >>>> +regmap_update_bits(dp_phy->regs, ssc_reg_offset, >>>> + XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT, read_value); >>>> +} >>>> + >>>> +return 0; >>>> +} >>>> + >>>> +static struct device_node *mtk_dp_get_ssc_node(struct phy *phy, >> struct mtk_dp_phy *dp_phy) >>>> +{ >>>> +struct device_node *mode_node = NULL; >>>> + >>>> +mode_node = of_find_node_by_name(dp_phy->dev->of_node, >> SSC_SETTING); >>> >>> ?!?! >>> You have the node, why do you try to find it? >>> >> >> Wait, that was brainfuck from my side or -ENOCOFFEE. Ignore. >> >> I still have a question though, where did you document new ABI: >> dependency on the node name here? >> >> Also, why you are not going through direct children - >> of_get_child_by_name()? >> >> Best regards, >> Krzysztof >> >> > > Sorry, there is formatting issue in the previous email. > > The dp_phy device has already been registered through the > mtk_dp_register_phy function in the mtk_dp.c file, > so it cannot be redefined in the dts. Avoid using of_get_child_by_name > for this purpose. > > To find the node name, utilize of_find_node_by_name since it has > already been registered. This does not answer my question at all. You speak about driver, I speak about ABI and bindings. Best regards, Krzysztof
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c index d7024a144335..13e5d3c33784 100644 --- a/drivers/phy/mediatek/phy-mtk-dp.c +++ b/drivers/phy/mediatek/phy-mtk-dp.c @@ -25,6 +25,10 @@ #define BIT_RATE_HBR2 2 #define BIT_RATE_HBR3 3 +#define MTK_DP_PHY_DIG_GLB_DA_REG_14 (PHY_OFFSET + 0xD8) +#define XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT GENMASK(15, 0) +#define XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT GENMASK(31, 16) + #define MTK_DP_PHY_DIG_SW_RST (PHY_OFFSET + 0x38) #define DP_GLB_SW_RST_PHYD BIT(0) @@ -78,10 +82,57 @@ #define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \ XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT) +#define SSC_SETTING "dp-ssc-setting" +#define RG_XTP_GLB_TXPLL_SSC_DELTA_RBR "ssc-delta-rbr" +#define RG_XTP_GLB_TXPLL_SSC_DELTA_HBR "ssc-delta-hbr" + struct mtk_dp_phy { struct regmap *regs; + struct device *dev; }; +static int mtk_dp_set_ssc_config(struct phy *phy, struct mtk_dp_phy *dp_phy, + struct device_node *ssc_node, const char *mode_name, u32 ssc_reg_offset) +{ + int ret; + u32 read_value = 0; + + ret = of_property_read_u32(ssc_node, mode_name, &read_value); + if (ret) { + dev_err(&phy->dev, "Read fail,DPTX is not %s\n", mode_name); + return -EINVAL; + } + + if (!read_value) { + dev_err(&phy->dev, "Read value is NULL\n"); + return -ENOMEM; + } + + if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR)) { + regmap_update_bits(dp_phy->regs, ssc_reg_offset, + XTP_GLB_TXPLL_SSC_DELTA_RBR_DEFAULT, read_value); + } else if (!strcmp(mode_name, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR)) { + read_value = read_value << 16 | 0x0000; + regmap_update_bits(dp_phy->regs, ssc_reg_offset, + XTP_GLB_TXPLL_SSC_DELTA_HBR_DEFAULT, read_value); + } + + return 0; +} + +static struct device_node *mtk_dp_get_ssc_node(struct phy *phy, struct mtk_dp_phy *dp_phy) +{ + struct device_node *mode_node = NULL; + + mode_node = of_find_node_by_name(dp_phy->dev->of_node, SSC_SETTING); + if (!mode_node) { + dev_err(&phy->dev, "SSC node is NULL\n"); + return NULL; + } + + return mode_node; +} + static int mtk_dp_phy_init(struct phy *phy) { struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); @@ -109,6 +160,7 @@ static int mtk_dp_phy_init(struct phy *phy) static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts) { struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); + struct device_node *ssc_node = NULL; u32 val; if (opts->dp.set_rate) { @@ -137,6 +189,14 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts) regmap_update_bits(dp_phy->regs, MTK_DP_PHY_DIG_PLL_CTL_1, TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0); + ssc_node = mtk_dp_get_ssc_node(phy, dp_phy); + if (ssc_node) { + mtk_dp_set_ssc_config(phy, dp_phy, ssc_node, RG_XTP_GLB_TXPLL_SSC_DELTA_RBR, + MTK_DP_PHY_DIG_GLB_DA_REG_14); + mtk_dp_set_ssc_config(phy, dp_phy, ssc_node, RG_XTP_GLB_TXPLL_SSC_DELTA_HBR, + MTK_DP_PHY_DIG_GLB_DA_REG_14); + } + return 0; } @@ -186,6 +246,7 @@ static int mtk_dp_phy_probe(struct platform_device *pdev) if (!dev->of_node) phy_create_lookup(phy, "dp", dev_name(dev)); + dp_phy->dev = dev; return 0; }
[Description] Severe screen flickering has been observed on the external display when the DP projection function is used with the market expansion dock. [Root cause] It has been discovered through analysis that severe screen flickering will occur when using the current default settings of SC (Spread Spectrum Clocking) after it is opened. [Solution] Reducing SSC capability on the test platform can resolve the screen flickering issue. By configuring SSC parameters in DTS, locating the DP SSC node in phy-mtk-dp, parsing the current value of SSC, and writing this value into PHY to configure SSC can solve the screen flickering problem. [Test] The SSC configuration values are read from DTS, parsed in the driver, and then written to the hardware. The test results indicate that there is no screen flickering. Signed-off-by: Liankun Yang <liankun.yang@mediatek.com> --- drivers/phy/mediatek/phy-mtk-dp.c | 61 +++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)