[PATCH 0/5] Add support for DisplayPort on SA8775P platform

[PATCH 0/5] Add support for DisplayPort on SA8775P platform

[Soutrik Mukhopadhyay]

This series adds support for the DisplayPort controller
and eDP PHY v5 found on the Qualcomm SA8775P platform.

Soutrik Mukhopadhyay (5):
  dt-bindings: phy: Add eDP PHY compatible for sa8775p
  phy: qcom: edp: Introduce aux_cfg array for version specific aux
    settings
  phy: qcom: edp: Add support for eDP PHY on SA8775P
  dt-bindings: display: msm: dp-controller: document SA8775P compatible
  drm/msm/dp: Add DisplayPort controller for SA8775P

 .../bindings/display/msm/dp-controller.yaml   |  1 +
 .../devicetree/bindings/phy/qcom,edp-phy.yaml |  1 +
 drivers/gpu/drm/msm/dp/dp_display.c           |  7 ++
 drivers/phy/qualcomm/phy-qcom-edp.c           | 81 +++++++++++++++----
 4 files changed, 76 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH 1/5] dt-bindings: phy: Add eDP PHY compatible for sa8775p

[Soutrik Mukhopadhyay]

Add compatible string for the supported eDP PHY on sa8775p platform.

Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
---
 Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
index 4e15d90d08b0..293fb6a9b1c3 100644
--- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
@@ -17,6 +17,7 @@ description:
 properties:
   compatible:
     enum:
+      - qcom,sa8775p-edp-phy
       - qcom,sc7280-edp-phy
       - qcom,sc8180x-edp-phy
       - qcom,sc8280xp-dp-phy
-- 
2.17.1

[PATCH 2/5] phy: qcom: edp: Introduce aux_cfg array for version specific aux settings

[Soutrik Mukhopadhyay]

In order to support different HW versions, introduce aux_cfg array
to move v4 specific aux configuration settings.

Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 34 +++++++++++++++++------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index da2b32fb5b45..0f860a807d1b 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -90,6 +90,7 @@ struct phy_ver_ops {
 
 struct qcom_edp_phy_cfg {
 	bool is_edp;
+	u8 *aux_cfg;
 	const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
 	const struct phy_ver_ops *ver_ops;
 };
@@ -186,11 +187,14 @@ static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
 	.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
 };
 
+static u8 edp_phy_aux_cfg_v4[10] = {
+	0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
+};
+
 static int qcom_edp_phy_init(struct phy *phy)
 {
 	struct qcom_edp *edp = phy_get_drvdata(phy);
 	int ret;
-	u8 cfg8;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
 	if (ret)
@@ -222,22 +226,20 @@ static int qcom_edp_phy_init(struct phy *phy)
 	 * even needed.
 	 */
 	if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
-		cfg8 = 0xb7;
-	else
-		cfg8 = 0x37;
+		edp->cfg->aux_cfg[8] = 0xb7;
 
 	writel(0xfc, edp->edp + DP_PHY_MODE);
 
-	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
-	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
-	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
-	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
-	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
-	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
-	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
-	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
-	writel(cfg8, edp->edp + DP_PHY_AUX_CFG8);
-	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
+	writel(edp->cfg->aux_cfg[0], edp->edp + DP_PHY_AUX_CFG0);
+	writel(edp->cfg->aux_cfg[1], edp->edp + DP_PHY_AUX_CFG1);
+	writel(edp->cfg->aux_cfg[2], edp->edp + DP_PHY_AUX_CFG2);
+	writel(edp->cfg->aux_cfg[3], edp->edp + DP_PHY_AUX_CFG3);
+	writel(edp->cfg->aux_cfg[4], edp->edp + DP_PHY_AUX_CFG4);
+	writel(edp->cfg->aux_cfg[5], edp->edp + DP_PHY_AUX_CFG5);
+	writel(edp->cfg->aux_cfg[6], edp->edp + DP_PHY_AUX_CFG6);
+	writel(edp->cfg->aux_cfg[7], edp->edp + DP_PHY_AUX_CFG7);
+	writel(edp->cfg->aux_cfg[8], edp->edp + DP_PHY_AUX_CFG8);
+	writel(edp->cfg->aux_cfg[9], edp->edp + DP_PHY_AUX_CFG9);
 
 	writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
 	       PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
@@ -519,16 +521,19 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
 };
 
 static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
+	.aux_cfg = edp_phy_aux_cfg_v4,
 	.ver_ops = &qcom_edp_phy_ops_v4,
 };
 
 static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
+	.aux_cfg = edp_phy_aux_cfg_v4,
 	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
 	.ver_ops = &qcom_edp_phy_ops_v4,
 };
 
 static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
 	.is_edp = true,
+	.aux_cfg = edp_phy_aux_cfg_v4,
 	.swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
 	.ver_ops = &qcom_edp_phy_ops_v4,
 };
@@ -707,6 +712,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
 };
 
 static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
+	.aux_cfg = edp_phy_aux_cfg_v4,
 	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
 	.ver_ops = &qcom_edp_phy_ops_v6,
 };
-- 
2.17.1

[PATCH 3/5] phy: qcom: edp: Add support for eDP PHY on SA8775P

[Soutrik Mukhopadhyay]

Add support for eDP PHY v5 found on the Qualcomm SA8775P platform.

Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 47 +++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index 0f860a807d1b..34a47cd2919d 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -191,6 +191,45 @@ static u8 edp_phy_aux_cfg_v4[10] = {
 	0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
 };
 
+static const u8 edp_swing_hbr_rbr_v5[4][4] = {
+	{ 0x07, 0x0f, 0x16, 0x1f },
+	{ 0x0d, 0x16, 0x1e, 0xff },
+	{ 0x11, 0x1b, 0xff, 0xff },
+	{ 0x16, 0xff, 0xff, 0xff }
+};
+
+static const u8 edp_pre_emp_hbr_rbr_v5[4][4] = {
+	{ 0x05, 0x11, 0x17, 0x1d },
+	{ 0x05, 0x11, 0x18, 0xff },
+	{ 0x06, 0x11, 0xff, 0xff },
+	{ 0x00, 0xff, 0xff, 0xff }
+};
+
+static const u8 edp_swing_hbr2_hbr3_v5[4][4] = {
+	{ 0x0b, 0x11, 0x17, 0x1c },
+	{ 0x10, 0x19, 0x1f, 0xff },
+	{ 0x19, 0x1f, 0xff, 0xff },
+	{ 0x1f, 0xff, 0xff, 0xff }
+};
+
+static const u8 edp_pre_emp_hbr2_hbr3_v5[4][4] = {
+	{ 0x0c, 0x15, 0x19, 0x1e },
+	{ 0x0b, 0x15, 0x19, 0xff },
+	{ 0x0e, 0x14, 0xff, 0xff },
+	{ 0x0d, 0xff, 0xff, 0xff }
+};
+
+static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v5 = {
+	.swing_hbr_rbr = &edp_swing_hbr_rbr_v5,
+	.swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3_v5,
+	.pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr_v5,
+	.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v5,
+};
+
+static u8 edp_phy_aux_cfg_v5[10] = {
+	0x00, 0x13, 0xa4, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
+};
+
 static int qcom_edp_phy_init(struct phy *phy)
 {
 	struct qcom_edp *edp = phy_get_drvdata(phy);
@@ -520,6 +559,13 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
 	.com_configure_ssc	= qcom_edp_com_configure_ssc_v4,
 };
 
+static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
+	.is_edp = false,
+	.aux_cfg = edp_phy_aux_cfg_v5,
+	.swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
+	.ver_ops = &qcom_edp_phy_ops_v4,
+};
+
 static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
 	.aux_cfg = edp_phy_aux_cfg_v4,
 	.ver_ops = &qcom_edp_phy_ops_v4,
@@ -1114,6 +1160,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_edp_phy_match_table[] = {
+	{ .compatible = "qcom,sa8775p-edp-phy", .data = &sa8775p_dp_phy_cfg, },
 	{ .compatible = "qcom,sc7280-edp-phy", .data = &sc7280_dp_phy_cfg, },
 	{ .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
 	{ .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
-- 
2.17.1

[PATCH 4/5] dt-bindings: display: msm: dp-controller: document SA8775P compatible

[Soutrik Mukhopadhyay]

Add compatible string for the DisplayPort controller found on the
Qualcomm SA8775P platform.

Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
---
 Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index 97993feda193..a212f335d5ff 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,sa8775p-dp
           - qcom,sc7180-dp
           - qcom,sc7280-dp
           - qcom,sc7280-edp
-- 
2.17.1

[PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Soutrik Mukhopadhyay]

The Qualcomm SA8775P platform comes with a DisplayPort controller
with a different base offset than the previous SoCs,
add support for this in the DisplayPort driver.

Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e1228fb093ee..e4954fd99eb0 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -118,6 +118,12 @@ struct msm_dp_desc {
 	bool wide_bus_supported;
 };
 
+static const struct msm_dp_desc sa8775p_dp_descs[] = {
+	{ .io_start = 0xaf54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
+	{ .io_start = 0xaf5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
+	{}
+};
+
 static const struct msm_dp_desc sc7180_dp_descs[] = {
 	{ .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
 	{}
@@ -162,6 +168,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
 };
 
 static const struct of_device_id dp_dt_match[] = {
+	{ .compatible = "qcom,sa8775p-dp", .data = &sa8775p_dp_descs },
 	{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
 	{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
 	{ .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_descs },
-- 
2.17.1

Re: [PATCH 2/5] phy: qcom: edp: Introduce aux_cfg array for version specific aux settings

[Dmitry Baryshkov]

On Wed, 11 Sept 2024 at 13:08, Soutrik Mukhopadhyay
<quic_mukhopad@quicinc.com> wrote:
>
> In order to support different HW versions, introduce aux_cfg array
> to move v4 specific aux configuration settings.
>
> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 34 +++++++++++++++++------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index da2b32fb5b45..0f860a807d1b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -90,6 +90,7 @@ struct phy_ver_ops {
>
>  struct qcom_edp_phy_cfg {
>         bool is_edp;
> +       u8 *aux_cfg;
>         const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>         const struct phy_ver_ops *ver_ops;
>  };
> @@ -186,11 +187,14 @@ static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>         .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
>  };
>
> +static u8 edp_phy_aux_cfg_v4[10] = {

static const u8, please.


> +       0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
> +};
> +
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
>         struct qcom_edp *edp = phy_get_drvdata(phy);
>         int ret;
> -       u8 cfg8;
>
>         ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
>         if (ret)
> @@ -222,22 +226,20 @@ static int qcom_edp_phy_init(struct phy *phy)
>          * even needed.
>          */
>         if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
> -               cfg8 = 0xb7;
> -       else
> -               cfg8 = 0x37;
> +               edp->cfg->aux_cfg[8] = 0xb7;
>
>         writel(0xfc, edp->edp + DP_PHY_MODE);
>
> -       writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
> -       writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
> -       writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
> -       writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
> -       writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
> -       writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
> -       writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
> -       writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
> -       writel(cfg8, edp->edp + DP_PHY_AUX_CFG8);
> -       writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
> +       writel(edp->cfg->aux_cfg[0], edp->edp + DP_PHY_AUX_CFG0);
> +       writel(edp->cfg->aux_cfg[1], edp->edp + DP_PHY_AUX_CFG1);
> +       writel(edp->cfg->aux_cfg[2], edp->edp + DP_PHY_AUX_CFG2);
> +       writel(edp->cfg->aux_cfg[3], edp->edp + DP_PHY_AUX_CFG3);
> +       writel(edp->cfg->aux_cfg[4], edp->edp + DP_PHY_AUX_CFG4);
> +       writel(edp->cfg->aux_cfg[5], edp->edp + DP_PHY_AUX_CFG5);
> +       writel(edp->cfg->aux_cfg[6], edp->edp + DP_PHY_AUX_CFG6);
> +       writel(edp->cfg->aux_cfg[7], edp->edp + DP_PHY_AUX_CFG7);
> +       writel(edp->cfg->aux_cfg[8], edp->edp + DP_PHY_AUX_CFG8);
> +       writel(edp->cfg->aux_cfg[9], edp->edp + DP_PHY_AUX_CFG9);
>
>         writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
>                PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
> @@ -519,16 +521,19 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>  };
>
>  static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
> +       .aux_cfg = edp_phy_aux_cfg_v4,
>         .ver_ops = &qcom_edp_phy_ops_v4,
>  };
>
>  static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> +       .aux_cfg = edp_phy_aux_cfg_v4,
>         .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>         .ver_ops = &qcom_edp_phy_ops_v4,
>  };
>
>  static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>         .is_edp = true,
> +       .aux_cfg = edp_phy_aux_cfg_v4,
>         .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>         .ver_ops = &qcom_edp_phy_ops_v4,
>  };
> @@ -707,6 +712,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>  };
>
>  static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
> +       .aux_cfg = edp_phy_aux_cfg_v4,

Is this correct? Judging by ver_ops, X Elite uses v6 of the PHY, so
maybe it should also use v5 AUX tables?

>         .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>         .ver_ops = &qcom_edp_phy_ops_v6,
>  };
> --
> 2.17.1
>


--
With best wishes
Dmitry

Re: [PATCH 3/5] phy: qcom: edp: Add support for eDP PHY on SA8775P

[Dmitry Baryshkov]

On Wed, 11 Sept 2024 at 13:08, Soutrik Mukhopadhyay
<quic_mukhopad@quicinc.com> wrote:
>
> Add support for eDP PHY v5 found on the Qualcomm SA8775P platform.
>
> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 47 +++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index 0f860a807d1b..34a47cd2919d 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -191,6 +191,45 @@ static u8 edp_phy_aux_cfg_v4[10] = {
>         0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
>  };
>
> +static const u8 edp_swing_hbr_rbr_v5[4][4] = {
> +       { 0x07, 0x0f, 0x16, 0x1f },
> +       { 0x0d, 0x16, 0x1e, 0xff },
> +       { 0x11, 0x1b, 0xff, 0xff },
> +       { 0x16, 0xff, 0xff, 0xff }
> +};

Same as v4

> +
> +static const u8 edp_pre_emp_hbr_rbr_v5[4][4] = {
> +       { 0x05, 0x11, 0x17, 0x1d },
> +       { 0x05, 0x11, 0x18, 0xff },
> +       { 0x06, 0x11, 0xff, 0xff },
> +       { 0x00, 0xff, 0xff, 0xff }
> +};

Could you please confirm that there is a single value difference?

> +
> +static const u8 edp_swing_hbr2_hbr3_v5[4][4] = {
> +       { 0x0b, 0x11, 0x17, 0x1c },
> +       { 0x10, 0x19, 0x1f, 0xff },
> +       { 0x19, 0x1f, 0xff, 0xff },
> +       { 0x1f, 0xff, 0xff, 0xff }
> +};

Same as v4

> +
> +static const u8 edp_pre_emp_hbr2_hbr3_v5[4][4] = {
> +       { 0x0c, 0x15, 0x19, 0x1e },
> +       { 0x0b, 0x15, 0x19, 0xff },
> +       { 0x0e, 0x14, 0xff, 0xff },
> +       { 0x0d, 0xff, 0xff, 0xff }
> +};

This one looks fine

> +
> +static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v5 = {
> +       .swing_hbr_rbr = &edp_swing_hbr_rbr_v5,
> +       .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3_v5,
> +       .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr_v5,
> +       .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v5,
> +};
> +
> +static u8 edp_phy_aux_cfg_v5[10] = {
> +       0x00, 0x13, 0xa4, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
> +};
> +
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
>         struct qcom_edp *edp = phy_get_drvdata(phy);
> @@ -520,6 +559,13 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>         .com_configure_ssc      = qcom_edp_com_configure_ssc_v4,
>  };
>
> +static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
> +       .is_edp = false,
> +       .aux_cfg = edp_phy_aux_cfg_v5,
> +       .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
> +       .ver_ops = &qcom_edp_phy_ops_v4,
> +};
> +
>  static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>         .aux_cfg = edp_phy_aux_cfg_v4,
>         .ver_ops = &qcom_edp_phy_ops_v4,
> @@ -1114,6 +1160,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
>  }
>
>  static const struct of_device_id qcom_edp_phy_match_table[] = {
> +       { .compatible = "qcom,sa8775p-edp-phy", .data = &sa8775p_dp_phy_cfg, },
>         { .compatible = "qcom,sc7280-edp-phy", .data = &sc7280_dp_phy_cfg, },
>         { .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
>         { .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
> --
> 2.17.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Bjorn Andersson]

On Wed, Sep 11, 2024 at 03:38:13PM +0530, Soutrik Mukhopadhyay wrote:
> The Qualcomm SA8775P platform comes with a DisplayPort controller
> with a different base offset than the previous SoCs,
> add support for this in the DisplayPort driver.
> 
> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e1228fb093ee..e4954fd99eb0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -118,6 +118,12 @@ struct msm_dp_desc {
>  	bool wide_bus_supported;
>  };
>  
> +static const struct msm_dp_desc sa8775p_dp_descs[] = {
> +	{ .io_start = 0xaf54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> +	{ .io_start = 0xaf5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },

Why is this list incomplete?

Regards,
Bjorn

> +	{}
> +};
> +
>  static const struct msm_dp_desc sc7180_dp_descs[] = {
>  	{ .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
>  	{}
> @@ -162,6 +168,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
>  };
>  
>  static const struct of_device_id dp_dt_match[] = {
> +	{ .compatible = "qcom,sa8775p-dp", .data = &sa8775p_dp_descs },
>  	{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
>  	{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
>  	{ .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_descs },
> -- 
> 2.17.1
> 

Re: [PATCH 2/5] phy: qcom: edp: Introduce aux_cfg array for version specific aux settings

[Bjorn Andersson]

On Wed, Sep 11, 2024 at 03:38:10PM +0530, Soutrik Mukhopadhyay wrote:
> In order to support different HW versions, introduce aux_cfg array
> to move v4 specific aux configuration settings.
> 
> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-edp.c | 34 +++++++++++++++++------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index da2b32fb5b45..0f860a807d1b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -90,6 +90,7 @@ struct phy_ver_ops {
>  
>  struct qcom_edp_phy_cfg {
>  	bool is_edp;
> +	u8 *aux_cfg;
>  	const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>  	const struct phy_ver_ops *ver_ops;
>  };
> @@ -186,11 +187,14 @@ static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>  	.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
>  };
>  
> +static u8 edp_phy_aux_cfg_v4[10] = {
> +	0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
> +};
> +
>  static int qcom_edp_phy_init(struct phy *phy)
>  {
>  	struct qcom_edp *edp = phy_get_drvdata(phy);
>  	int ret;
> -	u8 cfg8;
>  
>  	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
>  	if (ret)
> @@ -222,22 +226,20 @@ static int qcom_edp_phy_init(struct phy *phy)
>  	 * even needed.
>  	 */
>  	if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
> -		cfg8 = 0xb7;
> -	else
> -		cfg8 = 0x37;
> +		edp->cfg->aux_cfg[8] = 0xb7;

If you have multiple instances of the eDP PHY they will all point to the
same edp_phy_aux_cfg_v4[], so making instance-specific modifications
to that array will not be okay.


Make edp_phy_aux_cfg_v4[] const to reduce the likelihood for someone
else to make this mistake in the future and make a local copy of the
array on the stack so that you can modify it.

Regards,
Bjorn

>  
>  	writel(0xfc, edp->edp + DP_PHY_MODE);
>  
> -	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
> -	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
> -	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
> -	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
> -	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
> -	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
> -	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
> -	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
> -	writel(cfg8, edp->edp + DP_PHY_AUX_CFG8);
> -	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
> +	writel(edp->cfg->aux_cfg[0], edp->edp + DP_PHY_AUX_CFG0);
> +	writel(edp->cfg->aux_cfg[1], edp->edp + DP_PHY_AUX_CFG1);
> +	writel(edp->cfg->aux_cfg[2], edp->edp + DP_PHY_AUX_CFG2);
> +	writel(edp->cfg->aux_cfg[3], edp->edp + DP_PHY_AUX_CFG3);
> +	writel(edp->cfg->aux_cfg[4], edp->edp + DP_PHY_AUX_CFG4);
> +	writel(edp->cfg->aux_cfg[5], edp->edp + DP_PHY_AUX_CFG5);
> +	writel(edp->cfg->aux_cfg[6], edp->edp + DP_PHY_AUX_CFG6);
> +	writel(edp->cfg->aux_cfg[7], edp->edp + DP_PHY_AUX_CFG7);
> +	writel(edp->cfg->aux_cfg[8], edp->edp + DP_PHY_AUX_CFG8);
> +	writel(edp->cfg->aux_cfg[9], edp->edp + DP_PHY_AUX_CFG9);
>  
>  	writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
>  	       PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
> @@ -519,16 +521,19 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>  };
>  
>  static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
> +	.aux_cfg = edp_phy_aux_cfg_v4,
>  	.ver_ops = &qcom_edp_phy_ops_v4,
>  };
>  
>  static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> +	.aux_cfg = edp_phy_aux_cfg_v4,
>  	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>  	.ver_ops = &qcom_edp_phy_ops_v4,
>  };
>  
>  static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>  	.is_edp = true,
> +	.aux_cfg = edp_phy_aux_cfg_v4,
>  	.swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>  	.ver_ops = &qcom_edp_phy_ops_v4,
>  };
> @@ -707,6 +712,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>  };
>  
>  static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
> +	.aux_cfg = edp_phy_aux_cfg_v4,
>  	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>  	.ver_ops = &qcom_edp_phy_ops_v6,
>  };
> -- 
> 2.17.1
> 

Re: [PATCH 2/5] phy: qcom: edp: Introduce aux_cfg array for version specific aux settings

[Soutrik Mukhopadhyay]

On 9/11/2024 4:04 PM, Dmitry Baryshkov wrote:
> On Wed, 11 Sept 2024 at 13:08, Soutrik Mukhopadhyay
> <quic_mukhopad@quicinc.com> wrote:
>> In order to support different HW versions, introduce aux_cfg array
>> to move v4 specific aux configuration settings.
>>
>> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-edp.c | 34 +++++++++++++++++------------
>>   1 file changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index da2b32fb5b45..0f860a807d1b 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -90,6 +90,7 @@ struct phy_ver_ops {
>>
>>   struct qcom_edp_phy_cfg {
>>          bool is_edp;
>> +       u8 *aux_cfg;
>>          const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>>          const struct phy_ver_ops *ver_ops;
>>   };
>> @@ -186,11 +187,14 @@ static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>>          .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
>>   };
>>
>> +static u8 edp_phy_aux_cfg_v4[10] = {
> static const u8, please.


Sure, we will change this to "static const u8" in next version patch.


>
>
>> +       0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
>> +};
>> +
>>   static int qcom_edp_phy_init(struct phy *phy)
>>   {
>>          struct qcom_edp *edp = phy_get_drvdata(phy);
>>          int ret;
>> -       u8 cfg8;
>>
>>          ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
>>          if (ret)
>> @@ -222,22 +226,20 @@ static int qcom_edp_phy_init(struct phy *phy)
>>           * even needed.
>>           */
>>          if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
>> -               cfg8 = 0xb7;
>> -       else
>> -               cfg8 = 0x37;
>> +               edp->cfg->aux_cfg[8] = 0xb7;
>>
>>          writel(0xfc, edp->edp + DP_PHY_MODE);
>>
>> -       writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
>> -       writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
>> -       writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
>> -       writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
>> -       writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
>> -       writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
>> -       writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
>> -       writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
>> -       writel(cfg8, edp->edp + DP_PHY_AUX_CFG8);
>> -       writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
>> +       writel(edp->cfg->aux_cfg[0], edp->edp + DP_PHY_AUX_CFG0);
>> +       writel(edp->cfg->aux_cfg[1], edp->edp + DP_PHY_AUX_CFG1);
>> +       writel(edp->cfg->aux_cfg[2], edp->edp + DP_PHY_AUX_CFG2);
>> +       writel(edp->cfg->aux_cfg[3], edp->edp + DP_PHY_AUX_CFG3);
>> +       writel(edp->cfg->aux_cfg[4], edp->edp + DP_PHY_AUX_CFG4);
>> +       writel(edp->cfg->aux_cfg[5], edp->edp + DP_PHY_AUX_CFG5);
>> +       writel(edp->cfg->aux_cfg[6], edp->edp + DP_PHY_AUX_CFG6);
>> +       writel(edp->cfg->aux_cfg[7], edp->edp + DP_PHY_AUX_CFG7);
>> +       writel(edp->cfg->aux_cfg[8], edp->edp + DP_PHY_AUX_CFG8);
>> +       writel(edp->cfg->aux_cfg[9], edp->edp + DP_PHY_AUX_CFG9);
>>
>>          writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
>>                 PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
>> @@ -519,16 +521,19 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   };
>>
>>   static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>> +       .aux_cfg = edp_phy_aux_cfg_v4,
>>          .ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>>
>>   static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
>> +       .aux_cfg = edp_phy_aux_cfg_v4,
>>          .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>>          .ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>>
>>   static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>>          .is_edp = true,
>> +       .aux_cfg = edp_phy_aux_cfg_v4,
>>          .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>>          .ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>> @@ -707,6 +712,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>>   };
>>
>>   static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
>> +       .aux_cfg = edp_phy_aux_cfg_v4,
> Is this correct? Judging by ver_ops, X Elite uses v6 of the PHY, so
> maybe it should also use v5 AUX tables?


As per the current implementation, v6 uses v4 aux configuration, so we 
updated it to v4.


>
>>          .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>>          .ver_ops = &qcom_edp_phy_ops_v6,
>>   };
>> --
>> 2.17.1
>>
>
> --
> With best wishes
> Dmitry

Re: [PATCH 2/5] phy: qcom: edp: Introduce aux_cfg array for version specific aux settings

[Soutrik Mukhopadhyay]

On 9/12/2024 7:15 AM, Bjorn Andersson wrote:
> On Wed, Sep 11, 2024 at 03:38:10PM +0530, Soutrik Mukhopadhyay wrote:
>> In order to support different HW versions, introduce aux_cfg array
>> to move v4 specific aux configuration settings.
>>
>> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-edp.c | 34 +++++++++++++++++------------
>>   1 file changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index da2b32fb5b45..0f860a807d1b 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -90,6 +90,7 @@ struct phy_ver_ops {
>>   
>>   struct qcom_edp_phy_cfg {
>>   	bool is_edp;
>> +	u8 *aux_cfg;
>>   	const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>>   	const struct phy_ver_ops *ver_ops;
>>   };
>> @@ -186,11 +187,14 @@ static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>>   	.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
>>   };
>>   
>> +static u8 edp_phy_aux_cfg_v4[10] = {
>> +	0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
>> +};
>> +
>>   static int qcom_edp_phy_init(struct phy *phy)
>>   {
>>   	struct qcom_edp *edp = phy_get_drvdata(phy);
>>   	int ret;
>> -	u8 cfg8;
>>   
>>   	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
>>   	if (ret)
>> @@ -222,22 +226,20 @@ static int qcom_edp_phy_init(struct phy *phy)
>>   	 * even needed.
>>   	 */
>>   	if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
>> -		cfg8 = 0xb7;
>> -	else
>> -		cfg8 = 0x37;
>> +		edp->cfg->aux_cfg[8] = 0xb7;
> If you have multiple instances of the eDP PHY they will all point to the
> same edp_phy_aux_cfg_v4[], so making instance-specific modifications
> to that array will not be okay.
>
>
> Make edp_phy_aux_cfg_v4[] const to reduce the likelihood for someone
> else to make this mistake in the future and make a local copy of the
> array on the stack so that you can modify it.
>
> Regards,
> Bjorn


Sure, we will change edp_phy_aux_cfg_v4[] to const and use local copy to 
modify it.


>
>>   
>>   	writel(0xfc, edp->edp + DP_PHY_MODE);
>>   
>> -	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
>> -	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
>> -	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
>> -	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
>> -	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
>> -	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
>> -	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
>> -	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
>> -	writel(cfg8, edp->edp + DP_PHY_AUX_CFG8);
>> -	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
>> +	writel(edp->cfg->aux_cfg[0], edp->edp + DP_PHY_AUX_CFG0);
>> +	writel(edp->cfg->aux_cfg[1], edp->edp + DP_PHY_AUX_CFG1);
>> +	writel(edp->cfg->aux_cfg[2], edp->edp + DP_PHY_AUX_CFG2);
>> +	writel(edp->cfg->aux_cfg[3], edp->edp + DP_PHY_AUX_CFG3);
>> +	writel(edp->cfg->aux_cfg[4], edp->edp + DP_PHY_AUX_CFG4);
>> +	writel(edp->cfg->aux_cfg[5], edp->edp + DP_PHY_AUX_CFG5);
>> +	writel(edp->cfg->aux_cfg[6], edp->edp + DP_PHY_AUX_CFG6);
>> +	writel(edp->cfg->aux_cfg[7], edp->edp + DP_PHY_AUX_CFG7);
>> +	writel(edp->cfg->aux_cfg[8], edp->edp + DP_PHY_AUX_CFG8);
>> +	writel(edp->cfg->aux_cfg[9], edp->edp + DP_PHY_AUX_CFG9);
>>   
>>   	writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
>>   	       PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
>> @@ -519,16 +521,19 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>> +	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
>> +	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>>   	.ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>>   	.is_edp = true,
>> +	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>>   	.ver_ops = &qcom_edp_phy_ops_v4,
>>   };
>> @@ -707,6 +712,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>>   };
>>   
>>   static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
>> +	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>>   	.ver_ops = &qcom_edp_phy_ops_v6,
>>   };
>> -- 
>> 2.17.1
>>

Re: [PATCH 3/5] phy: qcom: edp: Add support for eDP PHY on SA8775P

[Soutrik Mukhopadhyay]

On 9/11/2024 4:10 PM, Dmitry Baryshkov wrote:
> On Wed, 11 Sept 2024 at 13:08, Soutrik Mukhopadhyay
> <quic_mukhopad@quicinc.com> wrote:
>> Add support for eDP PHY v5 found on the Qualcomm SA8775P platform.
>>
>> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-edp.c | 47 +++++++++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index 0f860a807d1b..34a47cd2919d 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -191,6 +191,45 @@ static u8 edp_phy_aux_cfg_v4[10] = {
>>          0x00, 0x13, 0x24, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
>>   };
>>
>> +static const u8 edp_swing_hbr_rbr_v5[4][4] = {
>> +       { 0x07, 0x0f, 0x16, 0x1f },
>> +       { 0x0d, 0x16, 0x1e, 0xff },
>> +       { 0x11, 0x1b, 0xff, 0xff },
>> +       { 0x16, 0xff, 0xff, 0xff }
>> +};
> Same as v4


Yes, we will reuse edp_swing_hbr_rbr for v5.


>
>> +
>> +static const u8 edp_pre_emp_hbr_rbr_v5[4][4] = {
>> +       { 0x05, 0x11, 0x17, 0x1d },
>> +       { 0x05, 0x11, 0x18, 0xff },
>> +       { 0x06, 0x11, 0xff, 0xff },
>> +       { 0x00, 0xff, 0xff, 0xff }
>> +};
> Could you please confirm that there is a single value difference?


Yes, there is a single value difference.


>
>> +
>> +static const u8 edp_swing_hbr2_hbr3_v5[4][4] = {
>> +       { 0x0b, 0x11, 0x17, 0x1c },
>> +       { 0x10, 0x19, 0x1f, 0xff },
>> +       { 0x19, 0x1f, 0xff, 0xff },
>> +       { 0x1f, 0xff, 0xff, 0xff }
>> +};
> Same as v4


Yes, we will reuse edp_swing_hbr2_hbr3 for v5.


>
>> +
>> +static const u8 edp_pre_emp_hbr2_hbr3_v5[4][4] = {
>> +       { 0x0c, 0x15, 0x19, 0x1e },
>> +       { 0x0b, 0x15, 0x19, 0xff },
>> +       { 0x0e, 0x14, 0xff, 0xff },
>> +       { 0x0d, 0xff, 0xff, 0xff }
>> +};
> This one looks fine
>
>> +
>> +static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v5 = {
>> +       .swing_hbr_rbr = &edp_swing_hbr_rbr_v5,
>> +       .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3_v5,
>> +       .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr_v5,
>> +       .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v5,
>> +};
>> +
>> +static u8 edp_phy_aux_cfg_v5[10] = {
>> +       0x00, 0x13, 0xa4, 0x00, 0x0a, 0x26, 0x0a, 0x03, 0x37, 0x03
>> +};
>> +
>>   static int qcom_edp_phy_init(struct phy *phy)
>>   {
>>          struct qcom_edp *edp = phy_get_drvdata(phy);
>> @@ -520,6 +559,13 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>          .com_configure_ssc      = qcom_edp_com_configure_ssc_v4,
>>   };
>>
>> +static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>> +       .is_edp = false,
>> +       .aux_cfg = edp_phy_aux_cfg_v5,
>> +       .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
>> +       .ver_ops = &qcom_edp_phy_ops_v4,
>> +};
>> +
>>   static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>>          .aux_cfg = edp_phy_aux_cfg_v4,
>>          .ver_ops = &qcom_edp_phy_ops_v4,
>> @@ -1114,6 +1160,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
>>   }
>>
>>   static const struct of_device_id qcom_edp_phy_match_table[] = {
>> +       { .compatible = "qcom,sa8775p-edp-phy", .data = &sa8775p_dp_phy_cfg, },
>>          { .compatible = "qcom,sc7280-edp-phy", .data = &sc7280_dp_phy_cfg, },
>>          { .compatible = "qcom,sc8180x-edp-phy", .data = &sc7280_dp_phy_cfg, },
>>          { .compatible = "qcom,sc8280xp-dp-phy", .data = &sc8280xp_dp_phy_cfg, },
>> --
>> 2.17.1
>>
>

Re: [PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Soutrik Mukhopadhyay]

On 9/12/2024 1:32 AM, Bjorn Andersson wrote:
> On Wed, Sep 11, 2024 at 03:38:13PM +0530, Soutrik Mukhopadhyay wrote:
>> The Qualcomm SA8775P platform comes with a DisplayPort controller
>> with a different base offset than the previous SoCs,
>> add support for this in the DisplayPort driver.
>>
>> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index e1228fb093ee..e4954fd99eb0 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -118,6 +118,12 @@ struct msm_dp_desc {
>>   	bool wide_bus_supported;
>>   };
>>   
>> +static const struct msm_dp_desc sa8775p_dp_descs[] = {
>> +	{ .io_start = 0xaf54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
>> +	{ .io_start = 0xaf5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> Why is this list incomplete?
>
> Regards,
> Bjorn


Do you mean to add for mdss_1 as well?

We only added dp controllers for mdss_0 as we plan to validate and 
enable only these.


>
>> +	{}
>> +};
>> +
>>   static const struct msm_dp_desc sc7180_dp_descs[] = {
>>   	{ .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
>>   	{}
>> @@ -162,6 +168,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
>>   };
>>   
>>   static const struct of_device_id dp_dt_match[] = {
>> +	{ .compatible = "qcom,sa8775p-dp", .data = &sa8775p_dp_descs },
>>   	{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
>>   	{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
>>   	{ .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_descs },
>> -- 
>> 2.17.1
>>

Re: [PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Bjorn Andersson]

On Thu, Sep 12, 2024 at 03:34:05PM +0530, Soutrik Mukhopadhyay wrote:
> 
> On 9/12/2024 1:32 AM, Bjorn Andersson wrote:
> > On Wed, Sep 11, 2024 at 03:38:13PM +0530, Soutrik Mukhopadhyay wrote:
> > > The Qualcomm SA8775P platform comes with a DisplayPort controller
> > > with a different base offset than the previous SoCs,
> > > add support for this in the DisplayPort driver.

Please check the line wrapping of this as well; the lines here should be
wrapped at 75 columns.

> > > 
> > > Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
> > > ---
> > >   drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index e1228fb093ee..e4954fd99eb0 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -118,6 +118,12 @@ struct msm_dp_desc {
> > >   	bool wide_bus_supported;
> > >   };
> > > +static const struct msm_dp_desc sa8775p_dp_descs[] = {
> > > +	{ .io_start = 0xaf54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> > > +	{ .io_start = 0xaf5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> > Why is this list incomplete?
> > 
> > Regards,
> > Bjorn
> 
> 
> Do you mean to add for mdss_1 as well?
> 

I don't see a reason for not submitting them as well.

> We only added dp controllers for mdss_0 as we plan to validate and enable
> only these.
> 

As far as I can tell there are 5 DPTX blocks on mdss_0.


It is perfectly fine that you just state in the commit message that
"Only MDSS0 DPTX0 and DPTX1 have been validated", but please add the
others as well in this commit.

Regards,
Bjorn

> 
> > 
> > > +	{}
> > > +};
> > > +
> > >   static const struct msm_dp_desc sc7180_dp_descs[] = {
> > >   	{ .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> > >   	{}
> > > @@ -162,6 +168,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
> > >   };
> > >   static const struct of_device_id dp_dt_match[] = {
> > > +	{ .compatible = "qcom,sa8775p-dp", .data = &sa8775p_dp_descs },
> > >   	{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
> > >   	{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
> > >   	{ .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_descs },
> > > -- 
> > > 2.17.1
> > > 

Re: [PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Soutrik Mukhopadhyay]

On 9/14/2024 2:54 AM, Bjorn Andersson wrote:
> On Thu, Sep 12, 2024 at 03:34:05PM +0530, Soutrik Mukhopadhyay wrote:
>> On 9/12/2024 1:32 AM, Bjorn Andersson wrote:
>>> On Wed, Sep 11, 2024 at 03:38:13PM +0530, Soutrik Mukhopadhyay wrote:
>>>> The Qualcomm SA8775P platform comes with a DisplayPort controller
>>>> with a different base offset than the previous SoCs,
>>>> add support for this in the DisplayPort driver.
> Please check the line wrapping of this as well; the lines here should be
> wrapped at 75 columns.


Yes, it is within 75 columns, we ran checkpatch.pl before posting the patch.


>
>>>> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com>
>>>> ---
>>>>    drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> index e1228fb093ee..e4954fd99eb0 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> @@ -118,6 +118,12 @@ struct msm_dp_desc {
>>>>    	bool wide_bus_supported;
>>>>    };
>>>> +static const struct msm_dp_desc sa8775p_dp_descs[] = {
>>>> +	{ .io_start = 0xaf54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
>>>> +	{ .io_start = 0xaf5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
>>> Why is this list incomplete?
>>>
>>> Regards,
>>> Bjorn
>>
>> Do you mean to add for mdss_1 as well?
>>
> I don't see a reason for not submitting them as well.


Sure, we will add for mdss_1 as well.


>
>> We only added dp controllers for mdss_0 as we plan to validate and enable
>> only these.
>>
> As far as I can tell there are 5 DPTX blocks on mdss_0.
>
>
> It is perfectly fine that you just state in the commit message that
> "Only MDSS0 DPTX0 and DPTX1 have been validated", but please add the
> others as well in this commit.
>
> Regards,
> Bjorn


We cross-checked the mdss architecture and found that there are only 2 
dp controllers for each mdss.

We will add all 4 and mention in the commit message that "Only MDSS0 
DPTX0 and DPTX1 have been validated".


>
>>>> +	{}
>>>> +};
>>>> +
>>>>    static const struct msm_dp_desc sc7180_dp_descs[] = {
>>>>    	{ .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
>>>>    	{}
>>>> @@ -162,6 +168,7 @@ static const struct msm_dp_desc x1e80100_dp_descs[] = {
>>>>    };
>>>>    static const struct of_device_id dp_dt_match[] = {
>>>> +	{ .compatible = "qcom,sa8775p-dp", .data = &sa8775p_dp_descs },
>>>>    	{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
>>>>    	{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
>>>>    	{ .compatible = "qcom,sc7280-edp", .data = &sc7280_dp_descs },
>>>> -- 
>>>> 2.17.1
>>>>

Re: [PATCH 5/5] drm/msm/dp: Add DisplayPort controller for SA8775P

[Konrad Dybcio]

On 17.09.2024 10:12 AM, Soutrik Mukhopadhyay wrote:
> 
> On 9/14/2024 2:54 AM, Bjorn Andersson wrote:
>> On Thu, Sep 12, 2024 at 03:34:05PM +0530, Soutrik Mukhopadhyay wrote:
>>> On 9/12/2024 1:32 AM, Bjorn Andersson wrote:
>>>> On Wed, Sep 11, 2024 at 03:38:13PM +0530, Soutrik Mukhopadhyay wrote:
>>>>> The Qualcomm SA8775P platform comes with a DisplayPort controller
>>>>> with a different base offset than the previous SoCs,
>>>>> add support for this in the DisplayPort driver.
>> Please check the line wrapping of this as well; the lines here should be
>> wrapped at 75 columns.

[...]

> 
> 
> We cross-checked the mdss architecture and found that there are only 2 dp controllers for each mdss.
> 
> We will add all 4 and mention in the commit message that "Only MDSS0 DPTX0 and DPTX1 have been validated".

Looking at some docs, it seems like 2x 2 controllers is indeed the
correct config after all

Konrad