[v2 0/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support

[v2 0/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support

[Rajeev Nandan]

This series is to add DSI PHY tuning support in Qualcomm Snapdragon
SoCs with 10nm DSI PHY e.g. SC7180

In most cases the default values of DSI PHY tuning registers
should be sufficient as they are fully optimized. However, in
some cases (for example, where extreme board parasitics cause
the eye shape to degrade), the override bits can be used to
improve the signal quality.

Different DSI PHY versions have different configurations to adjust the
drive strength, drive level, de-emphasis, etc. The current series has only
those configuration options supported by 10nm PHY, e.g. drive strength and
drive level. The number of registers to configure the drive strength are
different for 7nm PHY. The design can be extended to other DSI PHY versions
if required, as each PHY version can have its callback to get the input
from DT and prepare register values.

Changes in v2:
 - Addressed dt-bindings comments (Stephen Boyd, Dmitry Baryshkov)
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
 - Fix the backward compatibility (Dmitry Baryshkov)

Rajeev Nandan (3):
  dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties
  drm/msm/dsi: Add dsi phy tuning configuration support
  drm/msm/dsi: Add 10nm dsi phy tuning configuration support

 .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |  3 ++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              | 16 +++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c         | 51 +++++++++++++++++++---
 4 files changed, 97 insertions(+), 6 deletions(-)

-- 
2.7.4

[v2 0/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support

[Rajeev Nandan]

This series is to add DSI PHY tuning support in Qualcomm Snapdragon
SoCs with 10nm DSI PHY e.g. SC7180

In most cases the default values of DSI PHY tuning registers
should be sufficient as they are fully optimized. However, in
some cases (for example, where extreme board parasitics cause
the eye shape to degrade), the override bits can be used to
improve the signal quality.

Different DSI PHY versions have different configurations to adjust the
drive strength, drive level, de-emphasis, etc. The current series has only
those configuration options supported by 10nm PHY, e.g. drive strength and
drive level. The number of registers to configure the drive strength are
different for 7nm PHY. The design can be extended to other DSI PHY versions
if required, as each PHY version can have its callback to get the input
from DT and prepare register values.

Changes in v2:
 - Addressed dt-bindings comments (Stephen Boyd, Dmitry Baryshkov)
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
 - Fix the backward compatibility (Dmitry Baryshkov)

Rajeev Nandan (3):
  dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties
  drm/msm/dsi: Add dsi phy tuning configuration support
  drm/msm/dsi: Add 10nm dsi phy tuning configuration support

 .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |  3 ++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              | 16 +++++++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c         | 51 +++++++++++++++++++---
 4 files changed, 97 insertions(+), 6 deletions(-)

-- 
2.7.4

[v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rajeev Nandan]

In most cases, the default values of DSI PHY tuning registers should be
sufficient as they are fully optimized. However, in some cases where
extreme board parasitics cause the eye shape to degrade, the override
bits can be used to improve the signal quality.

The general guidelines for DSI PHY tuning include:
- High and moderate data rates may benefit from the drive strength and
  drive level tuning.
- Drive strength tuning will affect the output impedance and may be used
  for matching optimization.
- Drive level tuning will affect the output levels without affecting the
  impedance.

The clock and data lanes have a calibration circuitry feature. The drive
strength tuning can be done by adjusting rescode offset for hstop/hsbot,
and the drive level tuning can be done by adjusting the LDO output level
for the HSTX drive.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - More details in the commit text (Stephen Boyd)
 - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
 - Do not take values that are going to be unused (Dmitry Baryshkov)

 .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
index 4399715..d0eb8f6 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
@@ -35,6 +35,35 @@ properties:
       Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
       connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
 
+  phy-rescode-offset-top:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 5
+    maxItems: 5
+    description:
+      Integer array of offset for pull-up legs rescode for all five lanes.
+      To offset the drive strength from the calibrated value in an increasing
+      or decreasing manner, use 6 bit two’s complement values.
+
+  phy-rescode-offset-bot:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 5
+    maxItems: 5
+    description:
+      Integer array of offset for pull-down legs rescode for all five lanes.
+      To offset the drive strength from the calibrated value in an increasing
+      or decreasing manner, use 6 bit two’s complement values.
+
+  phy-drive-ldo-level:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 0
+    maximum: 7
+    description:
+      The PHY LDO has an amplitude tuning feature to adjust the LDO output
+      for the HSTX drive. To offset the drive level from the default value,
+      supported levels are with the following mapping:
+      0 = 375mV, 1 = 400mV, 2 = 425mV, 3 = 450mV, 4 = 475mV, 5 = 500mV,
+      6 = 500mV, 7 = 500mV
+
 required:
   - compatible
   - reg
@@ -64,5 +93,9 @@ examples:
          clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
                   <&rpmhcc RPMH_CXO_CLK>;
          clock-names = "iface", "ref";
+
+         phy-resocde-offset-top = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
+         phy-rescode-offset-bot = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
+         phy-drive-ldo-level = /bits/ 8 <1>;
      };
 ...
-- 
2.7.4

[v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rajeev Nandan]

In most cases, the default values of DSI PHY tuning registers should be
sufficient as they are fully optimized. However, in some cases where
extreme board parasitics cause the eye shape to degrade, the override
bits can be used to improve the signal quality.

The general guidelines for DSI PHY tuning include:
- High and moderate data rates may benefit from the drive strength and
  drive level tuning.
- Drive strength tuning will affect the output impedance and may be used
  for matching optimization.
- Drive level tuning will affect the output levels without affecting the
  impedance.

The clock and data lanes have a calibration circuitry feature. The drive
strength tuning can be done by adjusting rescode offset for hstop/hsbot,
and the drive level tuning can be done by adjusting the LDO output level
for the HSTX drive.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - More details in the commit text (Stephen Boyd)
 - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
 - Do not take values that are going to be unused (Dmitry Baryshkov)

 .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
index 4399715..d0eb8f6 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
@@ -35,6 +35,35 @@ properties:
       Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
       connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
 
+  phy-rescode-offset-top:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 5
+    maxItems: 5
+    description:
+      Integer array of offset for pull-up legs rescode for all five lanes.
+      To offset the drive strength from the calibrated value in an increasing
+      or decreasing manner, use 6 bit two’s complement values.
+
+  phy-rescode-offset-bot:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 5
+    maxItems: 5
+    description:
+      Integer array of offset for pull-down legs rescode for all five lanes.
+      To offset the drive strength from the calibrated value in an increasing
+      or decreasing manner, use 6 bit two’s complement values.
+
+  phy-drive-ldo-level:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    minimum: 0
+    maximum: 7
+    description:
+      The PHY LDO has an amplitude tuning feature to adjust the LDO output
+      for the HSTX drive. To offset the drive level from the default value,
+      supported levels are with the following mapping:
+      0 = 375mV, 1 = 400mV, 2 = 425mV, 3 = 450mV, 4 = 475mV, 5 = 500mV,
+      6 = 500mV, 7 = 500mV
+
 required:
   - compatible
   - reg
@@ -64,5 +93,9 @@ examples:
          clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
                   <&rpmhcc RPMH_CXO_CLK>;
          clock-names = "iface", "ref";
+
+         phy-resocde-offset-top = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
+         phy-rescode-offset-bot = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
+         phy-drive-ldo-level = /bits/ 8 <1>;
      };
 ...
-- 
2.7.4

[v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Add support for MSM DSI PHY tuning configuration. Current design is
to support drive strength and drive level/amplitude tuning for
10nm PHY version, but this can be extended to other PHY versions.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - New.
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)

 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c |  3 +++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 8c65ef6..ee3739d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -739,6 +739,9 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (phy->cfg->ops.tuning_cfg_init)
+		phy->cfg->ops.tuning_cfg_init(phy);
+
 	ret = dsi_phy_regulator_init(phy);
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index b91303a..b559a2b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
 	void (*save_pll_state)(struct msm_dsi_phy *phy);
 	int (*restore_pll_state)(struct msm_dsi_phy *phy);
 	bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool enable);
+	void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
 };
 
 struct msm_dsi_phy_cfg {
@@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
 #define DSI_PIXEL_PLL_CLK		1
 #define NUM_PROVIDED_CLKS		2
 
+#define DSI_LANE_MAX			5
+
+/**
+ * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
+ * @rescode_offset_top: Offset for pull-up legs rescode.
+ * @rescode_offset_bot: Offset for pull-down legs rescode.
+ * @vreg_ctrl: vreg ctrl to drive LDO level
+ */
+struct msm_dsi_phy_tuning_cfg {
+	u8 rescode_offset_top[DSI_LANE_MAX];
+	u8 rescode_offset_bot[DSI_LANE_MAX];
+	u8 vreg_ctrl;
+};
+
 struct msm_dsi_phy {
 	struct platform_device *pdev;
 	void __iomem *base;
@@ -98,6 +113,7 @@ struct msm_dsi_phy {
 
 	struct msm_dsi_dphy_timing timing;
 	const struct msm_dsi_phy_cfg *cfg;
+	struct msm_dsi_phy_tuning_cfg tuning_cfg;
 
 	enum msm_dsi_phy_usecase usecase;
 	bool regulator_ldo_mode;
-- 
2.7.4

[v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Add support for MSM DSI PHY tuning configuration. Current design is
to support drive strength and drive level/amplitude tuning for
10nm PHY version, but this can be extended to other PHY versions.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - New.
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)

 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c |  3 +++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 8c65ef6..ee3739d 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -739,6 +739,9 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (phy->cfg->ops.tuning_cfg_init)
+		phy->cfg->ops.tuning_cfg_init(phy);
+
 	ret = dsi_phy_regulator_init(phy);
 	if (ret)
 		goto fail;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index b91303a..b559a2b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
 	void (*save_pll_state)(struct msm_dsi_phy *phy);
 	int (*restore_pll_state)(struct msm_dsi_phy *phy);
 	bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool enable);
+	void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
 };
 
 struct msm_dsi_phy_cfg {
@@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
 #define DSI_PIXEL_PLL_CLK		1
 #define NUM_PROVIDED_CLKS		2
 
+#define DSI_LANE_MAX			5
+
+/**
+ * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
+ * @rescode_offset_top: Offset for pull-up legs rescode.
+ * @rescode_offset_bot: Offset for pull-down legs rescode.
+ * @vreg_ctrl: vreg ctrl to drive LDO level
+ */
+struct msm_dsi_phy_tuning_cfg {
+	u8 rescode_offset_top[DSI_LANE_MAX];
+	u8 rescode_offset_bot[DSI_LANE_MAX];
+	u8 vreg_ctrl;
+};
+
 struct msm_dsi_phy {
 	struct platform_device *pdev;
 	void __iomem *base;
@@ -98,6 +113,7 @@ struct msm_dsi_phy {
 
 	struct msm_dsi_dphy_timing timing;
 	const struct msm_dsi_phy_cfg *cfg;
+	struct msm_dsi_phy_tuning_cfg tuning_cfg;
 
 	enum msm_dsi_phy_usecase usecase;
 	bool regulator_ldo_mode;
-- 
2.7.4

[v2 3/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support

[Rajeev Nandan]

The clock and data lanes of the DSI PHY have a calibration circuitry
feature. As per the MSM DSI PHY tuning guidelines, the drive strength
tuning can be done by adjusting rescode offset for hstop/hsbot, and
the drive level tuning can be done by adjusting the LDO output level
for the HSTX drive.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
 - Fix the backward compatibility (Dmitry Baryshkov)

 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 51 ++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index d8128f5..40cd0f7 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -775,10 +775,13 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy *phy)
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_CFG2(i), 0x0);
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_CFG3(i),
 			      i == 4 ? 0x80 : 0x0);
-		dsi_phy_write(lane_base +
-			      REG_DSI_10nm_PHY_LN_OFFSET_TOP_CTRL(i), 0x0);
-		dsi_phy_write(lane_base +
-			      REG_DSI_10nm_PHY_LN_OFFSET_BOT_CTRL(i), 0x0);
+
+		/* platform specific dsi phy drive strength adjustment */
+		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_OFFSET_TOP_CTRL(i),
+				phy->tuning_cfg.rescode_offset_top[i]);
+		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_OFFSET_BOT_CTRL(i),
+				phy->tuning_cfg.rescode_offset_bot[i]);
+
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(i),
 			      tx_dctrl[i]);
 	}
@@ -834,8 +837,9 @@ static int dsi_10nm_phy_enable(struct msm_dsi_phy *phy,
 	/* Select MS1 byte-clk */
 	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_GLBL_CTRL, 0x10);
 
-	/* Enable LDO */
-	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_VREG_CTRL, 0x59);
+	/* Enable LDO with platform specific drive level/amplitude adjustment */
+	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_VREG_CTRL,
+		      phy->tuning_cfg.vreg_ctrl);
 
 	/* Configure PHY lane swap (TODO: we need to calculate this) */
 	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_LANE_CFG0, 0x21);
@@ -922,6 +926,39 @@ static void dsi_10nm_phy_disable(struct msm_dsi_phy *phy)
 	DBG("DSI%d PHY disabled", phy->id);
 }
 
+static void dsi_10nm_phy_tuning_cfg_init(struct msm_dsi_phy *phy)
+{
+	struct device *dev = &phy->pdev->dev;
+	u8 offset_top[DSI_LANE_MAX] = { 0 }; /* No offset */
+	u8 offset_bot[DSI_LANE_MAX] = { 0 }; /* No offset */
+	u8 ldo_level = 0x1; /* 400mV */
+	int ret, i;
+
+	/* Drive strength adjustment parameters */
+	ret = of_property_read_u8_array(dev->of_node, "phy-resocde-offset-top",
+					offset_top, DSI_LANE_MAX);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-resocde-offset-top, %d\n", ret);
+
+	for (i = 0; i < DSI_LANE_MAX; i++)
+		phy->tuning_cfg.rescode_offset_top[i] = 0x3f & offset_top[i];
+
+	ret = of_property_read_u8_array(dev->of_node, "phy-resocde-offset-bot",
+					offset_bot, DSI_LANE_MAX);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-resocde-offset-bot, %d\n", ret);
+
+	for (i = 0; i < DSI_LANE_MAX; i++)
+		phy->tuning_cfg.rescode_offset_bot[i] = 0x3f & offset_bot[i];
+
+	/* Drive level/amplitude adjustment parameters */
+	ret = of_property_read_u8(dev->of_node, "phy-drive-ldo-level", &ldo_level);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-drive-ldo-level, %d\n", ret);
+
+	phy->tuning_cfg.vreg_ctrl = 0x58 | (0x7 & ldo_level);
+}
+
 const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs = {
 	.has_phy_lane = true,
 	.reg_cfg = {
@@ -936,6 +973,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs = {
 		.pll_init = dsi_pll_10nm_init,
 		.save_pll_state = dsi_10nm_pll_save_state,
 		.restore_pll_state = dsi_10nm_pll_restore_state,
+		.tuning_cfg_init = dsi_10nm_phy_tuning_cfg_init,
 	},
 	.min_pll_rate = 1000000000UL,
 	.max_pll_rate = 3500000000UL,
@@ -957,6 +995,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs = {
 		.pll_init = dsi_pll_10nm_init,
 		.save_pll_state = dsi_10nm_pll_save_state,
 		.restore_pll_state = dsi_10nm_pll_restore_state,
+		.tuning_cfg_init = dsi_10nm_phy_tuning_cfg_init,
 	},
 	.min_pll_rate = 1000000000UL,
 	.max_pll_rate = 3500000000UL,
-- 
2.7.4

[v2 3/3] drm/msm/dsi: Add 10nm dsi phy tuning configuration support

[Rajeev Nandan]

The clock and data lanes of the DSI PHY have a calibration circuitry
feature. As per the MSM DSI PHY tuning guidelines, the drive strength
tuning can be done by adjusting rescode offset for hstop/hsbot, and
the drive level tuning can be done by adjusting the LDO output level
for the HSTX drive.

Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
---

Changes in v2:
 - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
 - Fix the backward compatibility (Dmitry Baryshkov)

 drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 51 ++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index d8128f5..40cd0f7 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -775,10 +775,13 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy *phy)
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_CFG2(i), 0x0);
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_CFG3(i),
 			      i == 4 ? 0x80 : 0x0);
-		dsi_phy_write(lane_base +
-			      REG_DSI_10nm_PHY_LN_OFFSET_TOP_CTRL(i), 0x0);
-		dsi_phy_write(lane_base +
-			      REG_DSI_10nm_PHY_LN_OFFSET_BOT_CTRL(i), 0x0);
+
+		/* platform specific dsi phy drive strength adjustment */
+		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_OFFSET_TOP_CTRL(i),
+				phy->tuning_cfg.rescode_offset_top[i]);
+		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_OFFSET_BOT_CTRL(i),
+				phy->tuning_cfg.rescode_offset_bot[i]);
+
 		dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(i),
 			      tx_dctrl[i]);
 	}
@@ -834,8 +837,9 @@ static int dsi_10nm_phy_enable(struct msm_dsi_phy *phy,
 	/* Select MS1 byte-clk */
 	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_GLBL_CTRL, 0x10);
 
-	/* Enable LDO */
-	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_VREG_CTRL, 0x59);
+	/* Enable LDO with platform specific drive level/amplitude adjustment */
+	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_VREG_CTRL,
+		      phy->tuning_cfg.vreg_ctrl);
 
 	/* Configure PHY lane swap (TODO: we need to calculate this) */
 	dsi_phy_write(base + REG_DSI_10nm_PHY_CMN_LANE_CFG0, 0x21);
@@ -922,6 +926,39 @@ static void dsi_10nm_phy_disable(struct msm_dsi_phy *phy)
 	DBG("DSI%d PHY disabled", phy->id);
 }
 
+static void dsi_10nm_phy_tuning_cfg_init(struct msm_dsi_phy *phy)
+{
+	struct device *dev = &phy->pdev->dev;
+	u8 offset_top[DSI_LANE_MAX] = { 0 }; /* No offset */
+	u8 offset_bot[DSI_LANE_MAX] = { 0 }; /* No offset */
+	u8 ldo_level = 0x1; /* 400mV */
+	int ret, i;
+
+	/* Drive strength adjustment parameters */
+	ret = of_property_read_u8_array(dev->of_node, "phy-resocde-offset-top",
+					offset_top, DSI_LANE_MAX);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-resocde-offset-top, %d\n", ret);
+
+	for (i = 0; i < DSI_LANE_MAX; i++)
+		phy->tuning_cfg.rescode_offset_top[i] = 0x3f & offset_top[i];
+
+	ret = of_property_read_u8_array(dev->of_node, "phy-resocde-offset-bot",
+					offset_bot, DSI_LANE_MAX);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-resocde-offset-bot, %d\n", ret);
+
+	for (i = 0; i < DSI_LANE_MAX; i++)
+		phy->tuning_cfg.rescode_offset_bot[i] = 0x3f & offset_bot[i];
+
+	/* Drive level/amplitude adjustment parameters */
+	ret = of_property_read_u8(dev->of_node, "phy-drive-ldo-level", &ldo_level);
+	if (ret && ret != -EINVAL)
+		DRM_DEV_ERROR(dev, "failed to parse phy-drive-ldo-level, %d\n", ret);
+
+	phy->tuning_cfg.vreg_ctrl = 0x58 | (0x7 & ldo_level);
+}
+
 const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs = {
 	.has_phy_lane = true,
 	.reg_cfg = {
@@ -936,6 +973,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs = {
 		.pll_init = dsi_pll_10nm_init,
 		.save_pll_state = dsi_10nm_pll_save_state,
 		.restore_pll_state = dsi_10nm_pll_restore_state,
+		.tuning_cfg_init = dsi_10nm_phy_tuning_cfg_init,
 	},
 	.min_pll_rate = 1000000000UL,
 	.max_pll_rate = 3500000000UL,
@@ -957,6 +995,7 @@ const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs = {
 		.pll_init = dsi_pll_10nm_init,
 		.save_pll_state = dsi_10nm_pll_save_state,
 		.restore_pll_state = dsi_10nm_pll_restore_state,
+		.tuning_cfg_init = dsi_10nm_phy_tuning_cfg_init,
 	},
 	.min_pll_rate = 1000000000UL,
 	.max_pll_rate = 3500000000UL,
-- 
2.7.4

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Dmitry Baryshkov]

On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
>
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
>
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
>
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
>
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
>
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
>
>  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> index 4399715..d0eb8f6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> @@ -35,6 +35,35 @@ properties:
>        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
>        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target

Generic note:
I think these properties should be prefixed with "qcom," prefix.

>
> +  phy-rescode-offset-top:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 5
> +    maxItems: 5
> +    description:
> +      Integer array of offset for pull-up legs rescode for all five lanes.
> +      To offset the drive strength from the calibrated value in an increasing
> +      or decreasing manner, use 6 bit two’s complement values.

dtc should support negative values, google hints that <(-2)> should work.

> +
> +  phy-rescode-offset-bot:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 5
> +    maxItems: 5
> +    description:
> +      Integer array of offset for pull-down legs rescode for all five lanes.
> +      To offset the drive strength from the calibrated value in an increasing
> +      or decreasing manner, use 6 bit two’s complement values.
> +
> +  phy-drive-ldo-level:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 0
> +    maximum: 7
> +    description:
> +      The PHY LDO has an amplitude tuning feature to adjust the LDO output
> +      for the HSTX drive. To offset the drive level from the default value,
> +      supported levels are with the following mapping:
> +      0 = 375mV, 1 = 400mV, 2 = 425mV, 3 = 450mV, 4 = 475mV, 5 = 500mV,
> +      6 = 500mV, 7 = 500mV

No encoding please. Specify the values in the dts and convert them
into the register values in the driver.

> +
>  required:
>    - compatible
>    - reg
> @@ -64,5 +93,9 @@ examples:
>           clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>                    <&rpmhcc RPMH_CXO_CLK>;
>           clock-names = "iface", "ref";
> +
> +         phy-resocde-offset-top = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> +         phy-rescode-offset-bot = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> +         phy-drive-ldo-level = /bits/ 8 <1>;

--
With best wishes
Dmitry

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Dmitry Baryshkov]

On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
>
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
>
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
>
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
>
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
>
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
>
>  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> index 4399715..d0eb8f6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> @@ -35,6 +35,35 @@ properties:
>        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
>        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target

Generic note:
I think these properties should be prefixed with "qcom," prefix.

>
> +  phy-rescode-offset-top:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 5
> +    maxItems: 5
> +    description:
> +      Integer array of offset for pull-up legs rescode for all five lanes.
> +      To offset the drive strength from the calibrated value in an increasing
> +      or decreasing manner, use 6 bit two’s complement values.

dtc should support negative values, google hints that <(-2)> should work.

> +
> +  phy-rescode-offset-bot:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 5
> +    maxItems: 5
> +    description:
> +      Integer array of offset for pull-down legs rescode for all five lanes.
> +      To offset the drive strength from the calibrated value in an increasing
> +      or decreasing manner, use 6 bit two’s complement values.
> +
> +  phy-drive-ldo-level:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    minimum: 0
> +    maximum: 7
> +    description:
> +      The PHY LDO has an amplitude tuning feature to adjust the LDO output
> +      for the HSTX drive. To offset the drive level from the default value,
> +      supported levels are with the following mapping:
> +      0 = 375mV, 1 = 400mV, 2 = 425mV, 3 = 450mV, 4 = 475mV, 5 = 500mV,
> +      6 = 500mV, 7 = 500mV

No encoding please. Specify the values in the dts and convert them
into the register values in the driver.

> +
>  required:
>    - compatible
>    - reg
> @@ -64,5 +93,9 @@ examples:
>           clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>                    <&rpmhcc RPMH_CXO_CLK>;
>           clock-names = "iface", "ref";
> +
> +         phy-resocde-offset-top = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> +         phy-rescode-offset-bot = /bits/ 8 <0x0 0x0 0x0 0x0 0x0>;
> +         phy-drive-ldo-level = /bits/ 8 <1>;

--
With best wishes
Dmitry

Re: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Dmitry Baryshkov]

On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
>
> Add support for MSM DSI PHY tuning configuration. Current design is
> to support drive strength and drive level/amplitude tuning for
> 10nm PHY version, but this can be extended to other PHY versions.
>
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
>
> Changes in v2:
>  - New.
>  - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
>
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c |  3 +++
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 8c65ef6..ee3739d 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -739,6 +739,9 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (phy->cfg->ops.tuning_cfg_init)
> +               phy->cfg->ops.tuning_cfg_init(phy);

Please rename to parse_dt_properties() or something like that.

> +
>         ret = dsi_phy_regulator_init(phy);
>         if (ret)
>                 goto fail;
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index b91303a..b559a2b 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
>         void (*save_pll_state)(struct msm_dsi_phy *phy);
>         int (*restore_pll_state)(struct msm_dsi_phy *phy);
>         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool enable);
> +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
>  };
>
>  struct msm_dsi_phy_cfg {
> @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
>  #define DSI_PIXEL_PLL_CLK              1
>  #define NUM_PROVIDED_CLKS              2
>
> +#define DSI_LANE_MAX                   5
> +
> +/**
> + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> + * @rescode_offset_top: Offset for pull-up legs rescode.
> + * @rescode_offset_bot: Offset for pull-down legs rescode.
> + * @vreg_ctrl: vreg ctrl to drive LDO level
> + */
> +struct msm_dsi_phy_tuning_cfg {
> +       u8 rescode_offset_top[DSI_LANE_MAX];
> +       u8 rescode_offset_bot[DSI_LANE_MAX];
> +       u8 vreg_ctrl;
> +};

How generic is this? In other words, you are adding a struct with the
generic name to the generic structure. I'd expect that it would be
common to several PHY generations.

> +
>  struct msm_dsi_phy {
>         struct platform_device *pdev;
>         void __iomem *base;
> @@ -98,6 +113,7 @@ struct msm_dsi_phy {
>
>         struct msm_dsi_dphy_timing timing;
>         const struct msm_dsi_phy_cfg *cfg;
> +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
>
>         enum msm_dsi_phy_usecase usecase;
>         bool regulator_ldo_mode;
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Dmitry Baryshkov]

On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
>
> Add support for MSM DSI PHY tuning configuration. Current design is
> to support drive strength and drive level/amplitude tuning for
> 10nm PHY version, but this can be extended to other PHY versions.
>
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
>
> Changes in v2:
>  - New.
>  - Split into generic code and 10nm-specific part (Dmitry Baryshkov)
>
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c |  3 +++
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 8c65ef6..ee3739d 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -739,6 +739,9 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (phy->cfg->ops.tuning_cfg_init)
> +               phy->cfg->ops.tuning_cfg_init(phy);

Please rename to parse_dt_properties() or something like that.

> +
>         ret = dsi_phy_regulator_init(phy);
>         if (ret)
>                 goto fail;
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index b91303a..b559a2b 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
>         void (*save_pll_state)(struct msm_dsi_phy *phy);
>         int (*restore_pll_state)(struct msm_dsi_phy *phy);
>         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool enable);
> +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
>  };
>
>  struct msm_dsi_phy_cfg {
> @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
>  #define DSI_PIXEL_PLL_CLK              1
>  #define NUM_PROVIDED_CLKS              2
>
> +#define DSI_LANE_MAX                   5
> +
> +/**
> + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> + * @rescode_offset_top: Offset for pull-up legs rescode.
> + * @rescode_offset_bot: Offset for pull-down legs rescode.
> + * @vreg_ctrl: vreg ctrl to drive LDO level
> + */
> +struct msm_dsi_phy_tuning_cfg {
> +       u8 rescode_offset_top[DSI_LANE_MAX];
> +       u8 rescode_offset_bot[DSI_LANE_MAX];
> +       u8 vreg_ctrl;
> +};

How generic is this? In other words, you are adding a struct with the
generic name to the generic structure. I'd expect that it would be
common to several PHY generations.

> +
>  struct msm_dsi_phy {
>         struct platform_device *pdev;
>         void __iomem *base;
> @@ -98,6 +113,7 @@ struct msm_dsi_phy {
>
>         struct msm_dsi_dphy_timing timing;
>         const struct msm_dsi_phy_cfg *cfg;
> +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
>
>         enum msm_dsi_phy_usecase usecase;
>         bool regulator_ldo_mode;
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rob Herring]

On Mon, 10 Jan 2022 18:25:35 +0530, Rajeev Nandan wrote:
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
> 
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
> 
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
> 
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
> 
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
> 
>  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:63:54: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:  mapping values are not allowed in this context
  in "<unicode string>", line 63, column 54
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 46, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context
  in "<unicode string>", line 63, column 54
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml: ignoring, error parsing file
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1577891

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rob Herring]

On Mon, 10 Jan 2022 18:25:35 +0530, Rajeev Nandan wrote:
> In most cases, the default values of DSI PHY tuning registers should be
> sufficient as they are fully optimized. However, in some cases where
> extreme board parasitics cause the eye shape to degrade, the override
> bits can be used to improve the signal quality.
> 
> The general guidelines for DSI PHY tuning include:
> - High and moderate data rates may benefit from the drive strength and
>   drive level tuning.
> - Drive strength tuning will affect the output impedance and may be used
>   for matching optimization.
> - Drive level tuning will affect the output levels without affecting the
>   impedance.
> 
> The clock and data lanes have a calibration circuitry feature. The drive
> strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> and the drive level tuning can be done by adjusting the LDO output level
> for the HSTX drive.
> 
> Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> ---
> 
> Changes in v2:
>  - More details in the commit text (Stephen Boyd)
>  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
>  - Do not take values that are going to be unused (Dmitry Baryshkov)
> 
>  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:63:54: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml:  mapping values are not allowed in this context
  in "<unicode string>", line 63, column 54
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 46, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context
  in "<unicode string>", line 63, column 54
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml: ignoring, error parsing file
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1577891

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rob Herring]

On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
> >
> > In most cases, the default values of DSI PHY tuning registers should be
> > sufficient as they are fully optimized. However, in some cases where
> > extreme board parasitics cause the eye shape to degrade, the override
> > bits can be used to improve the signal quality.
> >
> > The general guidelines for DSI PHY tuning include:
> > - High and moderate data rates may benefit from the drive strength and
> >   drive level tuning.
> > - Drive strength tuning will affect the output impedance and may be used
> >   for matching optimization.
> > - Drive level tuning will affect the output levels without affecting the
> >   impedance.
> >
> > The clock and data lanes have a calibration circuitry feature. The drive
> > strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> > and the drive level tuning can be done by adjusting the LDO output level
> > for the HSTX drive.
> >
> > Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> > ---
> >
> > Changes in v2:
> >  - More details in the commit text (Stephen Boyd)
> >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> >
> >  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > index 4399715..d0eb8f6 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > @@ -35,6 +35,35 @@ properties:
> >        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> >        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> 
> Generic note:
> I think these properties should be prefixed with "qcom," prefix.
> 
> >
> > +  phy-rescode-offset-top:
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    minItems: 5
> > +    maxItems: 5
> > +    description:
> > +      Integer array of offset for pull-up legs rescode for all five lanes.
> > +      To offset the drive strength from the calibrated value in an increasing
> > +      or decreasing manner, use 6 bit two’s complement values.
> 
> dtc should support negative values, google hints that <(-2)> should work.

Yes, but the schema checks don't check negative values correctly yet. So 
you can use 'int8-array', but just don't use negative values in the 
examples. I'm working on changes that will fix this issue.

What does 6-bit mean? 0x3f is negative? Just sign extend the values and 
specify the valid range instead:

minimum: -32
maximum: 31

Rob

Re: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rob Herring]

On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan <quic_rajeevny@quicinc.com> wrote:
> >
> > In most cases, the default values of DSI PHY tuning registers should be
> > sufficient as they are fully optimized. However, in some cases where
> > extreme board parasitics cause the eye shape to degrade, the override
> > bits can be used to improve the signal quality.
> >
> > The general guidelines for DSI PHY tuning include:
> > - High and moderate data rates may benefit from the drive strength and
> >   drive level tuning.
> > - Drive strength tuning will affect the output impedance and may be used
> >   for matching optimization.
> > - Drive level tuning will affect the output levels without affecting the
> >   impedance.
> >
> > The clock and data lanes have a calibration circuitry feature. The drive
> > strength tuning can be done by adjusting rescode offset for hstop/hsbot,
> > and the drive level tuning can be done by adjusting the LDO output level
> > for the HSTX drive.
> >
> > Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> > ---
> >
> > Changes in v2:
> >  - More details in the commit text (Stephen Boyd)
> >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> >
> >  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > index 4399715..d0eb8f6 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > @@ -35,6 +35,35 @@ properties:
> >        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> >        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> 
> Generic note:
> I think these properties should be prefixed with "qcom," prefix.
> 
> >
> > +  phy-rescode-offset-top:
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    minItems: 5
> > +    maxItems: 5
> > +    description:
> > +      Integer array of offset for pull-up legs rescode for all five lanes.
> > +      To offset the drive strength from the calibrated value in an increasing
> > +      or decreasing manner, use 6 bit two’s complement values.
> 
> dtc should support negative values, google hints that <(-2)> should work.

Yes, but the schema checks don't check negative values correctly yet. So 
you can use 'int8-array', but just don't use negative values in the 
examples. I'm working on changes that will fix this issue.

What does 6-bit mean? 0x3f is negative? Just sign extend the values and 
specify the valid range instead:

minimum: -32
maximum: 31

Rob

RE: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rajeev Nandan]

Hi Dmitry, Rob,

Thanks for the review.
> 
> On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan
> <quic_rajeevny@quicinc.com> wrote:
> > >
> > > In most cases, the default values of DSI PHY tuning registers should
> > > be sufficient as they are fully optimized. However, in some cases
> > > where extreme board parasitics cause the eye shape to degrade, the
> > > override bits can be used to improve the signal quality.
> > >
> > > The general guidelines for DSI PHY tuning include:
> > > - High and moderate data rates may benefit from the drive strength and
> > >   drive level tuning.
> > > - Drive strength tuning will affect the output impedance and may be used
> > >   for matching optimization.
> > > - Drive level tuning will affect the output levels without affecting the
> > >   impedance.
> > >
> > > The clock and data lanes have a calibration circuitry feature. The
> > > drive strength tuning can be done by adjusting rescode offset for
> > > hstop/hsbot, and the drive level tuning can be done by adjusting the
> > > LDO output level for the HSTX drive.
> > >
> > > Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> > > ---
> > >
> > > Changes in v2:
> > >  - More details in the commit text (Stephen Boyd)
> > >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> > >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> > >
> > >  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33
> ++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > index 4399715..d0eb8f6 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-
> 10nm.yam
> > > +++ l
> > > @@ -35,6 +35,35 @@ properties:
> > >        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> > >        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> >
> > Generic note:
> > I think these properties should be prefixed with "qcom," prefix.
Sure, I will update in next version along with removing encoding for phy-drive-ldo-level property.

> >
> > >
> > > +  phy-rescode-offset-top:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    minItems: 5
> > > +    maxItems: 5
> > > +    description:
> > > +      Integer array of offset for pull-up legs rescode for all five lanes.
> > > +      To offset the drive strength from the calibrated value in an increasing
> > > +      or decreasing manner, use 6 bit two’s complement values.
> >
> > dtc should support negative values, google hints that <(-2)> should work.
This format is working.
> 
> Yes, but the schema checks don't check negative values correctly yet. So you
> can use 'int8-array', but just don't use negative values in the examples. I'm
> working on changes that will fix this issue.
> 
> What does 6-bit mean? 0x3f is negative? Just sign extend the values and
> specify the valid range instead:
> 
> minimum: -32
> maximum: 31
Yes, Rob. 0x3f is negative value (-1) in 6-bit two's complement.
I will implement your suggestion in the next patch version.

> 
> Rob

Thanks,
Rajeev

RE: [v2 1/3] dt-bindings: msm/dsi: Add 10nm dsi phy tuning properties

[Rajeev Nandan]

Hi Dmitry, Rob,

Thanks for the review.
> 
> On Mon, Jan 10, 2022 at 05:06:03PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 10 Jan 2022 at 15:56, Rajeev Nandan
> <quic_rajeevny@quicinc.com> wrote:
> > >
> > > In most cases, the default values of DSI PHY tuning registers should
> > > be sufficient as they are fully optimized. However, in some cases
> > > where extreme board parasitics cause the eye shape to degrade, the
> > > override bits can be used to improve the signal quality.
> > >
> > > The general guidelines for DSI PHY tuning include:
> > > - High and moderate data rates may benefit from the drive strength and
> > >   drive level tuning.
> > > - Drive strength tuning will affect the output impedance and may be used
> > >   for matching optimization.
> > > - Drive level tuning will affect the output levels without affecting the
> > >   impedance.
> > >
> > > The clock and data lanes have a calibration circuitry feature. The
> > > drive strength tuning can be done by adjusting rescode offset for
> > > hstop/hsbot, and the drive level tuning can be done by adjusting the
> > > LDO output level for the HSTX drive.
> > >
> > > Signed-off-by: Rajeev Nandan <quic_rajeevny@quicinc.com>
> > > ---
> > >
> > > Changes in v2:
> > >  - More details in the commit text (Stephen Boyd)
> > >  - Use human understandable values (Stephen Boyd, Dmitry Baryshkov)
> > >  - Do not take values that are going to be unused (Dmitry Baryshkov)
> > >
> > >  .../bindings/display/msm/dsi-phy-10nm.yaml         | 33
> ++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > index 4399715..d0eb8f6 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-
> 10nm.yam
> > > +++ l
> > > @@ -35,6 +35,35 @@ properties:
> > >        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
> > >        connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
> >
> > Generic note:
> > I think these properties should be prefixed with "qcom," prefix.
Sure, I will update in next version along with removing encoding for phy-drive-ldo-level property.

> >
> > >
> > > +  phy-rescode-offset-top:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    minItems: 5
> > > +    maxItems: 5
> > > +    description:
> > > +      Integer array of offset for pull-up legs rescode for all five lanes.
> > > +      To offset the drive strength from the calibrated value in an increasing
> > > +      or decreasing manner, use 6 bit two’s complement values.
> >
> > dtc should support negative values, google hints that <(-2)> should work.
This format is working.
> 
> Yes, but the schema checks don't check negative values correctly yet. So you
> can use 'int8-array', but just don't use negative values in the examples. I'm
> working on changes that will fix this issue.
> 
> What does 6-bit mean? 0x3f is negative? Just sign extend the values and
> specify the valid range instead:
> 
> minimum: -32
> maximum: 31
Yes, Rob. 0x3f is negative value (-1) in 6-bit two's complement.
I will implement your suggestion in the next patch version.

> 
> Rob

Thanks,
Rajeev

RE: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Hi Dmitry,

> >
> > +       if (phy->cfg->ops.tuning_cfg_init)
> > +               phy->cfg->ops.tuning_cfg_init(phy);
> 
> Please rename to parse_dt_properties() or something like that.
Sure. I didn't understand your comment in v1 to use config_dt() hook. I think, now I understood.
This function can be used to parse PHY version (nm) specific DT properties, currently we will be using it for PHY tuning parameters, and later some other properties can be added.
I will use parse_dt_properties() in next post. Please correct me if I still didn't get it.

> 
> > +
> >         ret = dsi_phy_regulator_init(phy);
> >         if (ret)
> >                 goto fail;
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index b91303a..b559a2b 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > enable);
> > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> >  };
> >
> >  struct msm_dsi_phy_cfg {
> > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> >  #define DSI_PIXEL_PLL_CLK              1
> >  #define NUM_PROVIDED_CLKS              2
> >
> > +#define DSI_LANE_MAX                   5
> > +
> > +/**
> > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > +msm_dsi_phy_tuning_cfg {
> > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > +       u8 vreg_ctrl;
> > +};
> 
> How generic is this? In other words, you are adding a struct with the generic
> name to the generic structure. I'd expect that it would be common to several
> PHY generations.

The 10nm is PHY v3.x, and the PHY tuning register configuration is same across DSI PHY v3.x devices.
Similarly the PHY v4.x devices have same register configuration to adjust PHY tuning parameters.

The v4.x has few changes as compared to v3.x :
- rescode_offset_top:
  In v4.x, the value is not per lane, register is moved from PHY_LN_ block to PHY_CMN_ block. One value needed to configure all the lanes.
  Whereas in v3.x, configuration for each lane can be different.
  In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.

- rescode_offset_bot:
   same as rescode_offset_top comments given above.

- vreg_ctrl:
   v4.x has two registers to adjust drive level, REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
   The first one is the same for v3 and v4, only name is changed (_0 suffix)
   The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to adjust mid-level amplitudes (C-PHY mode only)
   We can add a new member vreg_ctrl_1 in the "struct msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x

Apart from the existing members, the v4.x has a few more register config options for drive strength adjustment and De-emphasis.
We can add new members to address them for v4.x PHY tuning.

The PHY v4.x is the latest PHY version, and most of the new devices are coming with v4.x. So, I can say that the structure member is going to remain the same for some time.
(Things may/may not change in v5, but I never heard of it coming)

Thanks,
Rajeev
> 
> > +
> >  struct msm_dsi_phy {
> >         struct platform_device *pdev;
> >         void __iomem *base;
> > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> >
> >         struct msm_dsi_dphy_timing timing;
> >         const struct msm_dsi_phy_cfg *cfg;
> > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> >
> >         enum msm_dsi_phy_usecase usecase;
> >         bool regulator_ldo_mode;
> > --
> > 2.7.4
> >
> 
> 
> --
> With best wishes
> Dmitry

RE: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Hi Dmitry,

> >
> > +       if (phy->cfg->ops.tuning_cfg_init)
> > +               phy->cfg->ops.tuning_cfg_init(phy);
> 
> Please rename to parse_dt_properties() or something like that.
Sure. I didn't understand your comment in v1 to use config_dt() hook. I think, now I understood.
This function can be used to parse PHY version (nm) specific DT properties, currently we will be using it for PHY tuning parameters, and later some other properties can be added.
I will use parse_dt_properties() in next post. Please correct me if I still didn't get it.

> 
> > +
> >         ret = dsi_phy_regulator_init(phy);
> >         if (ret)
> >                 goto fail;
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index b91303a..b559a2b 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > enable);
> > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> >  };
> >
> >  struct msm_dsi_phy_cfg {
> > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> >  #define DSI_PIXEL_PLL_CLK              1
> >  #define NUM_PROVIDED_CLKS              2
> >
> > +#define DSI_LANE_MAX                   5
> > +
> > +/**
> > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > +msm_dsi_phy_tuning_cfg {
> > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > +       u8 vreg_ctrl;
> > +};
> 
> How generic is this? In other words, you are adding a struct with the generic
> name to the generic structure. I'd expect that it would be common to several
> PHY generations.

The 10nm is PHY v3.x, and the PHY tuning register configuration is same across DSI PHY v3.x devices.
Similarly the PHY v4.x devices have same register configuration to adjust PHY tuning parameters.

The v4.x has few changes as compared to v3.x :
- rescode_offset_top:
  In v4.x, the value is not per lane, register is moved from PHY_LN_ block to PHY_CMN_ block. One value needed to configure all the lanes.
  Whereas in v3.x, configuration for each lane can be different.
  In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.

- rescode_offset_bot:
   same as rescode_offset_top comments given above.

- vreg_ctrl:
   v4.x has two registers to adjust drive level, REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
   The first one is the same for v3 and v4, only name is changed (_0 suffix)
   The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to adjust mid-level amplitudes (C-PHY mode only)
   We can add a new member vreg_ctrl_1 in the "struct msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x

Apart from the existing members, the v4.x has a few more register config options for drive strength adjustment and De-emphasis.
We can add new members to address them for v4.x PHY tuning.

The PHY v4.x is the latest PHY version, and most of the new devices are coming with v4.x. So, I can say that the structure member is going to remain the same for some time.
(Things may/may not change in v5, but I never heard of it coming)

Thanks,
Rajeev
> 
> > +
> >  struct msm_dsi_phy {
> >         struct platform_device *pdev;
> >         void __iomem *base;
> > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> >
> >         struct msm_dsi_dphy_timing timing;
> >         const struct msm_dsi_phy_cfg *cfg;
> > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> >
> >         enum msm_dsi_phy_usecase usecase;
> >         bool regulator_ldo_mode;
> > --
> > 2.7.4
> >
> 
> 
> --
> With best wishes
> Dmitry

Re: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 19:09, Rajeev Nandan <RAJEEVNY@qti.qualcomm.com> wrote:
>
> Hi Dmitry,
>
> > >
> > > +       if (phy->cfg->ops.tuning_cfg_init)
> > > +               phy->cfg->ops.tuning_cfg_init(phy);
> >
> > Please rename to parse_dt_properties() or something like that.
> Sure. I didn't understand your comment in v1 to use config_dt() hook. I think, now I understood.
> This function can be used to parse PHY version (nm) specific DT properties, currently we will be using it for PHY tuning parameters, and later some other properties can be added.
> I will use parse_dt_properties() in next post. Please correct me if I still didn't get it.

You understanding follows my proposal, thank you.

>
> >
> > > +
> > >         ret = dsi_phy_regulator_init(phy);
> > >         if (ret)
> > >                 goto fail;
> > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > index b91303a..b559a2b 100644
> > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> > >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> > >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> > >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > > enable);
> > > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> > >  };
> > >
> > >  struct msm_dsi_phy_cfg {
> > > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> > >  #define DSI_PIXEL_PLL_CLK              1
> > >  #define NUM_PROVIDED_CLKS              2
> > >
> > > +#define DSI_LANE_MAX                   5
> > > +
> > > +/**
> > > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> > > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > > +msm_dsi_phy_tuning_cfg {
> > > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > > +       u8 vreg_ctrl;
> > > +};
> >
> > How generic is this? In other words, you are adding a struct with the generic
> > name to the generic structure. I'd expect that it would be common to several
> > PHY generations.
>
> The 10nm is PHY v3.x, and the PHY tuning register configuration is same across DSI PHY v3.x devices.
> Similarly the PHY v4.x devices have same register configuration to adjust PHY tuning parameters.

What about 14nm (v2.x, sdm660)? Or even 0.0-lpm (apq8016)?

>
> The v4.x has few changes as compared to v3.x :
> - rescode_offset_top:
>   In v4.x, the value is not per lane, register is moved from PHY_LN_ block to PHY_CMN_ block. One value needed to configure all the lanes.
>   Whereas in v3.x, configuration for each lane can be different.
>   In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.

Ugh.

>
> - rescode_offset_bot:
>    same as rescode_offset_top comments given above.
>
> - vreg_ctrl:
>    v4.x has two registers to adjust drive level, REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
>    The first one is the same for v3 and v4, only name is changed (_0 suffix)
>    The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to adjust mid-level amplitudes (C-PHY mode only)
>    We can add a new member vreg_ctrl_1 in the "struct msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x
>
> Apart from the existing members, the v4.x has a few more register config options for drive strength adjustment and De-emphasis.
> We can add new members to address them for v4.x PHY tuning.

I do not like the idea of pushing each and every possible option into
generic structure.
I see two possible solutions:
 - Add opaque void pointer to struct msm_dsi_phy. Allow each driver to
specify it on it's own.

Or:

- add a union of per-nm structures.

From these two options I'm biassed towards the first one. It
encapsulates the data structure with the using code.


>
> The PHY v4.x is the latest PHY version, and most of the new devices are coming with v4.x. So, I can say that the structure member is going to remain the same for some time.
> (Things may/may not change in v5, but I never heard of it coming)
>
> Thanks,
> Rajeev
> >
> > > +
> > >  struct msm_dsi_phy {
> > >         struct platform_device *pdev;
> > >         void __iomem *base;
> > > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> > >
> > >         struct msm_dsi_dphy_timing timing;
> > >         const struct msm_dsi_phy_cfg *cfg;
> > > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> > >
> > >         enum msm_dsi_phy_usecase usecase;
> > >         bool regulator_ldo_mode;
> > > --
> > > 2.7.4
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Re: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Dmitry Baryshkov]

On Wed, 12 Jan 2022 at 19:09, Rajeev Nandan <RAJEEVNY@qti.qualcomm.com> wrote:
>
> Hi Dmitry,
>
> > >
> > > +       if (phy->cfg->ops.tuning_cfg_init)
> > > +               phy->cfg->ops.tuning_cfg_init(phy);
> >
> > Please rename to parse_dt_properties() or something like that.
> Sure. I didn't understand your comment in v1 to use config_dt() hook. I think, now I understood.
> This function can be used to parse PHY version (nm) specific DT properties, currently we will be using it for PHY tuning parameters, and later some other properties can be added.
> I will use parse_dt_properties() in next post. Please correct me if I still didn't get it.

You understanding follows my proposal, thank you.

>
> >
> > > +
> > >         ret = dsi_phy_regulator_init(phy);
> > >         if (ret)
> > >                 goto fail;
> > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > index b91303a..b559a2b 100644
> > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> > >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> > >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> > >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > > enable);
> > > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> > >  };
> > >
> > >  struct msm_dsi_phy_cfg {
> > > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> > >  #define DSI_PIXEL_PLL_CLK              1
> > >  #define NUM_PROVIDED_CLKS              2
> > >
> > > +#define DSI_LANE_MAX                   5
> > > +
> > > +/**
> > > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config parameters.
> > > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > > +msm_dsi_phy_tuning_cfg {
> > > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > > +       u8 vreg_ctrl;
> > > +};
> >
> > How generic is this? In other words, you are adding a struct with the generic
> > name to the generic structure. I'd expect that it would be common to several
> > PHY generations.
>
> The 10nm is PHY v3.x, and the PHY tuning register configuration is same across DSI PHY v3.x devices.
> Similarly the PHY v4.x devices have same register configuration to adjust PHY tuning parameters.

What about 14nm (v2.x, sdm660)? Or even 0.0-lpm (apq8016)?

>
> The v4.x has few changes as compared to v3.x :
> - rescode_offset_top:
>   In v4.x, the value is not per lane, register is moved from PHY_LN_ block to PHY_CMN_ block. One value needed to configure all the lanes.
>   Whereas in v3.x, configuration for each lane can be different.
>   In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.

Ugh.

>
> - rescode_offset_bot:
>    same as rescode_offset_top comments given above.
>
> - vreg_ctrl:
>    v4.x has two registers to adjust drive level, REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
>    The first one is the same for v3 and v4, only name is changed (_0 suffix)
>    The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to adjust mid-level amplitudes (C-PHY mode only)
>    We can add a new member vreg_ctrl_1 in the "struct msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x
>
> Apart from the existing members, the v4.x has a few more register config options for drive strength adjustment and De-emphasis.
> We can add new members to address them for v4.x PHY tuning.

I do not like the idea of pushing each and every possible option into
generic structure.
I see two possible solutions:
 - Add opaque void pointer to struct msm_dsi_phy. Allow each driver to
specify it on it's own.

Or:

- add a union of per-nm structures.

From these two options I'm biassed towards the first one. It
encapsulates the data structure with the using code.


>
> The PHY v4.x is the latest PHY version, and most of the new devices are coming with v4.x. So, I can say that the structure member is going to remain the same for some time.
> (Things may/may not change in v5, but I never heard of it coming)
>
> Thanks,
> Rajeev
> >
> > > +
> > >  struct msm_dsi_phy {
> > >         struct platform_device *pdev;
> > >         void __iomem *base;
> > > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> > >
> > >         struct msm_dsi_dphy_timing timing;
> > >         const struct msm_dsi_phy_cfg *cfg;
> > > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> > >
> > >         enum msm_dsi_phy_usecase usecase;
> > >         bool regulator_ldo_mode;
> > > --
> > > 2.7.4
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

RE: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Hi Dmitry,

> 
> On Wed, 12 Jan 2022 at 19:09, Rajeev Nandan
> <RAJEEVNY@qti.qualcomm.com> wrote:
> >
> > Hi Dmitry,
> >
> > > >
> > > > +       if (phy->cfg->ops.tuning_cfg_init)
> > > > +               phy->cfg->ops.tuning_cfg_init(phy);
> > >
> > > Please rename to parse_dt_properties() or something like that.
> > Sure. I didn't understand your comment in v1 to use config_dt() hook. I
> think, now I understood.
> > This function can be used to parse PHY version (nm) specific DT properties,
> currently we will be using it for PHY tuning parameters, and later some other
> properties can be added.
> > I will use parse_dt_properties() in next post. Please correct me if I still
> didn't get it.
> 
> You understanding follows my proposal, thank you.
> 
> >
> > >
> > > > +
> > > >         ret = dsi_phy_regulator_init(phy);
> > > >         if (ret)
> > > >                 goto fail;
> > > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > index b91303a..b559a2b 100644
> > > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> > > >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> > > >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> > > >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > > > enable);
> > > > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> > > >  };
> > > >
> > > >  struct msm_dsi_phy_cfg {
> > > > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> > > >  #define DSI_PIXEL_PLL_CLK              1
> > > >  #define NUM_PROVIDED_CLKS              2
> > > >
> > > > +#define DSI_LANE_MAX                   5
> > > > +
> > > > +/**
> > > > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config
> parameters.
> > > > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > > > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > > > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > > > +msm_dsi_phy_tuning_cfg {
> > > > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > > > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > > > +       u8 vreg_ctrl;
> > > > +};
> > >
> > > How generic is this? In other words, you are adding a struct with
> > > the generic name to the generic structure. I'd expect that it would
> > > be common to several PHY generations.
> >
> > The 10nm is PHY v3.x, and the PHY tuning register configuration is same
> across DSI PHY v3.x devices.
> > Similarly the PHY v4.x devices have same register configuration to adjust
> PHY tuning parameters.
> 
> What about 14nm (v2.x, sdm660)? Or even 0.0-lpm (apq8016)?

The 14nm (v2.x) has different registers and parameters for the DSI PHY tuning. 
  Drive strength: DSIPHY_HSTX_STR_HSTOP & _HSBOT for each lane (Top/bottom branch drive strength adjustment)
  Drive level: NA
  Pre-emphasis: DSIPHY_PEMPH_STRBOT & _STRTOP for each lane (values are based on HSTX loading on the lane)

The apq8016 is a very old chip and I couldn't find the tuning docs for it.

I think going with your suggestion to allow each driver to specify its structure, we don't need to worry about the details of the tuning configs needed for each PHY versions.

> 
> >
> > The v4.x has few changes as compared to v3.x :
> > - rescode_offset_top:
> >   In v4.x, the value is not per lane, register is moved from PHY_LN_ block to
> PHY_CMN_ block. One value needed to configure all the lanes.
> >   Whereas in v3.x, configuration for each lane can be different.
> >   In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.
> 
> Ugh.
> 
> >
> > - rescode_offset_bot:
> >    same as rescode_offset_top comments given above.
> >
> > - vreg_ctrl:
> >    v4.x has two registers to adjust drive level,
> REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and
> REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
> >    The first one is the same for v3 and v4, only name is changed (_0 suffix)
> >    The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to
> adjust mid-level amplitudes (C-PHY mode only)
> >    We can add a new member vreg_ctrl_1 in the "struct
> > msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x
> >
> > Apart from the existing members, the v4.x has a few more register config
> options for drive strength adjustment and De-emphasis.
> > We can add new members to address them for v4.x PHY tuning.
> 
> I do not like the idea of pushing each and every possible option into generic
> structure.
> I see two possible solutions:
>  - Add opaque void pointer to struct msm_dsi_phy. Allow each driver to
> specify it on it's own.
> 
> Or:
> 
> - add a union of per-nm structures.
> 
> From these two options I'm biassed towards the first one. It encapsulates
> the data structure with the using code.

I agree with you, I will implement the first option.

Thanks,
Rajeev

> 
> 
> >
> > The PHY v4.x is the latest PHY version, and most of the new devices are
> coming with v4.x. So, I can say that the structure member is going to remain
> the same for some time.
> > (Things may/may not change in v5, but I never heard of it coming)
> >
> > Thanks,
> > Rajeev
> > >
> > > > +
> > > >  struct msm_dsi_phy {
> > > >         struct platform_device *pdev;
> > > >         void __iomem *base;
> > > > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> > > >
> > > >         struct msm_dsi_dphy_timing timing;
> > > >         const struct msm_dsi_phy_cfg *cfg;
> > > > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> > > >
> > > >         enum msm_dsi_phy_usecase usecase;
> > > >         bool regulator_ldo_mode;
> > > > --
> > > > 2.7.4
> > > >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> 
> 
> 
> --
> With best wishes
> Dmitry

RE: [v2 2/3] drm/msm/dsi: Add dsi phy tuning configuration support

[Rajeev Nandan]

Hi Dmitry,

> 
> On Wed, 12 Jan 2022 at 19:09, Rajeev Nandan
> <RAJEEVNY@qti.qualcomm.com> wrote:
> >
> > Hi Dmitry,
> >
> > > >
> > > > +       if (phy->cfg->ops.tuning_cfg_init)
> > > > +               phy->cfg->ops.tuning_cfg_init(phy);
> > >
> > > Please rename to parse_dt_properties() or something like that.
> > Sure. I didn't understand your comment in v1 to use config_dt() hook. I
> think, now I understood.
> > This function can be used to parse PHY version (nm) specific DT properties,
> currently we will be using it for PHY tuning parameters, and later some other
> properties can be added.
> > I will use parse_dt_properties() in next post. Please correct me if I still
> didn't get it.
> 
> You understanding follows my proposal, thank you.
> 
> >
> > >
> > > > +
> > > >         ret = dsi_phy_regulator_init(phy);
> > > >         if (ret)
> > > >                 goto fail;
> > > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > index b91303a..b559a2b 100644
> > > > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > > > @@ -25,6 +25,7 @@ struct msm_dsi_phy_ops {
> > > >         void (*save_pll_state)(struct msm_dsi_phy *phy);
> > > >         int (*restore_pll_state)(struct msm_dsi_phy *phy);
> > > >         bool (*set_continuous_clock)(struct msm_dsi_phy *phy, bool
> > > > enable);
> > > > +       void (*tuning_cfg_init)(struct msm_dsi_phy *phy);
> > > >  };
> > > >
> > > >  struct msm_dsi_phy_cfg {
> > > > @@ -81,6 +82,20 @@ struct msm_dsi_dphy_timing {
> > > >  #define DSI_PIXEL_PLL_CLK              1
> > > >  #define NUM_PROVIDED_CLKS              2
> > > >
> > > > +#define DSI_LANE_MAX                   5
> > > > +
> > > > +/**
> > > > + * struct msm_dsi_phy_tuning_cfg - Holds PHY tuning config
> parameters.
> > > > + * @rescode_offset_top: Offset for pull-up legs rescode.
> > > > + * @rescode_offset_bot: Offset for pull-down legs rescode.
> > > > + * @vreg_ctrl: vreg ctrl to drive LDO level  */ struct
> > > > +msm_dsi_phy_tuning_cfg {
> > > > +       u8 rescode_offset_top[DSI_LANE_MAX];
> > > > +       u8 rescode_offset_bot[DSI_LANE_MAX];
> > > > +       u8 vreg_ctrl;
> > > > +};
> > >
> > > How generic is this? In other words, you are adding a struct with
> > > the generic name to the generic structure. I'd expect that it would
> > > be common to several PHY generations.
> >
> > The 10nm is PHY v3.x, and the PHY tuning register configuration is same
> across DSI PHY v3.x devices.
> > Similarly the PHY v4.x devices have same register configuration to adjust
> PHY tuning parameters.
> 
> What about 14nm (v2.x, sdm660)? Or even 0.0-lpm (apq8016)?

The 14nm (v2.x) has different registers and parameters for the DSI PHY tuning. 
  Drive strength: DSIPHY_HSTX_STR_HSTOP & _HSBOT for each lane (Top/bottom branch drive strength adjustment)
  Drive level: NA
  Pre-emphasis: DSIPHY_PEMPH_STRBOT & _STRTOP for each lane (values are based on HSTX loading on the lane)

The apq8016 is a very old chip and I couldn't find the tuning docs for it.

I think going with your suggestion to allow each driver to specify its structure, we don't need to worry about the details of the tuning configs needed for each PHY versions.

> 
> >
> > The v4.x has few changes as compared to v3.x :
> > - rescode_offset_top:
> >   In v4.x, the value is not per lane, register is moved from PHY_LN_ block to
> PHY_CMN_ block. One value needed to configure all the lanes.
> >   Whereas in v3.x, configuration for each lane can be different.
> >   In case of v4.x, we can use rescode_offset_top[0] and ignore rest values.
> 
> Ugh.
> 
> >
> > - rescode_offset_bot:
> >    same as rescode_offset_top comments given above.
> >
> > - vreg_ctrl:
> >    v4.x has two registers to adjust drive level,
> REG_DSI_7nm_PHY_CMN_VREG_CTRL_0 and
> REG_DSI_7nm_PHY_CMN_VREG_CTRL_1
> >    The first one is the same for v3 and v4, only name is changed (_0 suffix)
> >    The second one REG_DSI_7nm_PHY_CMN_VREG_CTRL_1 is added to
> adjust mid-level amplitudes (C-PHY mode only)
> >    We can add a new member vreg_ctrl_1 in the "struct
> > msm_dsi_phy_tuning_cfg" while adding PHY tuning support for V4.x
> >
> > Apart from the existing members, the v4.x has a few more register config
> options for drive strength adjustment and De-emphasis.
> > We can add new members to address them for v4.x PHY tuning.
> 
> I do not like the idea of pushing each and every possible option into generic
> structure.
> I see two possible solutions:
>  - Add opaque void pointer to struct msm_dsi_phy. Allow each driver to
> specify it on it's own.
> 
> Or:
> 
> - add a union of per-nm structures.
> 
> From these two options I'm biassed towards the first one. It encapsulates
> the data structure with the using code.

I agree with you, I will implement the first option.

Thanks,
Rajeev

> 
> 
> >
> > The PHY v4.x is the latest PHY version, and most of the new devices are
> coming with v4.x. So, I can say that the structure member is going to remain
> the same for some time.
> > (Things may/may not change in v5, but I never heard of it coming)
> >
> > Thanks,
> > Rajeev
> > >
> > > > +
> > > >  struct msm_dsi_phy {
> > > >         struct platform_device *pdev;
> > > >         void __iomem *base;
> > > > @@ -98,6 +113,7 @@ struct msm_dsi_phy {
> > > >
> > > >         struct msm_dsi_dphy_timing timing;
> > > >         const struct msm_dsi_phy_cfg *cfg;
> > > > +       struct msm_dsi_phy_tuning_cfg tuning_cfg;
> > > >
> > > >         enum msm_dsi_phy_usecase usecase;
> > > >         bool regulator_ldo_mode;
> > > > --
> > > > 2.7.4
> > > >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> 
> 
> 
> --
> With best wishes
> Dmitry