dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add data-lanes and link-frequencies to dp_out endpoint
@ 2022-11-17 22:49 Kuogee Hsieh
  2022-11-17 22:49 ` [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into " Kuogee Hsieh
  2022-11-17 22:49 ` [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate Kuogee Hsieh
  0 siblings, 2 replies; 6+ messages in thread
From: Kuogee Hsieh @ 2022-11-17 22:49 UTC (permalink / raw)
  To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
	airlied, agross, dmitry.baryshkov, bjorn.andersson
  Cc: quic_sbillaka, linux-arm-msm, quic_abhinavk, linux-kernel,
	Kuogee Hsieh, freedreno

Add DP both data-lanes and link-frequencies property to dp_out endpoint and support
functions to DP driver.

Kuogee Hsieh (2):
  arm64: dts: qcom: add data-lanes and link-freuencies into dp_out
    endpoint
  drm/msm/dp: add support of max dp link rate

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi   |  9 +++++++-
 arch/arm64/boot/dts/qcom/sc7180.dtsi           |  5 -----
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 10 ++++++++-
 arch/arm64/boot/dts/qcom/sc7280.dtsi           |  6 +-----
 drivers/gpu/drm/msm/dp/dp_display.c            |  4 ++++
 drivers/gpu/drm/msm/dp/dp_panel.c              |  7 +++---
 drivers/gpu/drm/msm/dp/dp_panel.h              |  1 +
 drivers/gpu/drm/msm/dp/dp_parser.c             | 30 +++++++++++++++++++-------
 drivers/gpu/drm/msm/dp/dp_parser.h             |  2 ++
 9 files changed, 51 insertions(+), 23 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into dp_out endpoint
  2022-11-17 22:49 [PATCH v3 0/2] Add data-lanes and link-frequencies to dp_out endpoint Kuogee Hsieh
@ 2022-11-17 22:49 ` Kuogee Hsieh
  2022-11-18 11:01   ` Dmitry Baryshkov
  2022-11-17 22:49 ` [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate Kuogee Hsieh
  1 sibling, 1 reply; 6+ messages in thread
From: Kuogee Hsieh @ 2022-11-17 22:49 UTC (permalink / raw)
  To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
	airlied, agross, dmitry.baryshkov, bjorn.andersson
  Cc: quic_sbillaka, linux-arm-msm, quic_abhinavk, linux-kernel,
	Kuogee Hsieh, freedreno

Add both data-lanes and link-frequencies property to dp_out endpoint.
Also set link-frequencies to 810000 khz at herobrine platform to have
max link rate limited at 810000 khz (HBR3).

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi   |  9 ++++++++-
 arch/arm64/boot/dts/qcom/sc7180.dtsi           |  5 -----
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 10 +++++++++-
 arch/arm64/boot/dts/qcom/sc7280.dtsi           |  5 -----
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 754d2d6..6bf57fc 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -812,7 +812,14 @@ hp_i2c: &i2c9 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&dp_hot_plug_det>;
-	data-lanes = <0 1>;
+	ports {
+		port@1 {
+			reg = <1>;
+			dp_out: endpoint {
+				data-lanes = <0 1>;
+			};
+		};
+	};
 };
 
 &pm6150_adc {
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index c0f6bfd..def5521 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -3116,11 +3116,6 @@
 							remote-endpoint = <&dpu_intf0_out>;
 						};
 					};
-
-					port@1 {
-						reg = <1>;
-						dp_out: endpoint { };
-					};
 				};
 
 				dp_opp_table: opp-table {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index 93e39fc..e8fca18 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -440,7 +440,15 @@ ap_i2c_tpm: &i2c14 {
 	status = "okay";
 	pinctrl-names = "default";
 	pinctrl-0 = <&dp_hot_plug_det>;
-	data-lanes = <0 1>;
+	ports {
+		port@1 {
+			reg = <1>;
+			dp_out: endpoint {
+				data-lanes = <0 1>;
+				link-frequencies=<810000>;
+			};
+		};
+	};
 };
 
 &mdss_mdp {
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index a646405..4afe53b 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3899,11 +3899,6 @@
 							remote-endpoint = <&dpu_intf0_out>;
 						};
 					};
-
-					port@1 {
-						reg = <1>;
-						dp_out: endpoint { };
-					};
 				};
 
 				dp_opp_table: opp-table {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate
  2022-11-17 22:49 [PATCH v3 0/2] Add data-lanes and link-frequencies to dp_out endpoint Kuogee Hsieh
  2022-11-17 22:49 ` [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into " Kuogee Hsieh
@ 2022-11-17 22:49 ` Kuogee Hsieh
  2022-11-18 11:04   ` Dmitry Baryshkov
  1 sibling, 1 reply; 6+ messages in thread
From: Kuogee Hsieh @ 2022-11-17 22:49 UTC (permalink / raw)
  To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
	airlied, agross, dmitry.baryshkov, bjorn.andersson
  Cc: quic_sbillaka, linux-arm-msm, quic_abhinavk, linux-kernel,
	Kuogee Hsieh, freedreno

dp_out endpoint contains both data-lanes and link-frequencies properties.
This patch parser dp_out endpoint properties and acquire dp_max_lanes and
dp_max_link_rate from respective property. Finally, comparing them against
both data lane and link rate read back from sink to ensure both data lane
and link rate are supported by platform.
In the case there is no data-lanes or link-frequencies property defined at
dp_out endpoint, the default value are 4 data lanes with 5.4 Ghz link rate.

Changes in v2:
-- add max link rate from dtsi

Changes in v3:
-- parser max_data_lanes and max_dp_link_rate from dp_out endpoint

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi |  1 +
 drivers/gpu/drm/msm/dp/dp_display.c  |  4 ++++
 drivers/gpu/drm/msm/dp/dp_panel.c    |  7 ++++---
 drivers/gpu/drm/msm/dp/dp_panel.h    |  1 +
 drivers/gpu/drm/msm/dp/dp_parser.c   | 30 ++++++++++++++++++++++--------
 drivers/gpu/drm/msm/dp/dp_parser.h   |  2 ++
 6 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 4afe53b..d456e76 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3897,6 +3897,7 @@
 						reg = <0>;
 						dp_in: endpoint {
 							remote-endpoint = <&dpu_intf0_out>;
+							data-lanes = <0 1 2 3>;
 						};
 					};
 				};
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 29c9845..4fe2092 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -390,6 +390,10 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
 	struct edid *edid;
 
 	dp->panel->max_dp_lanes = dp->parser->max_dp_lanes;
+	dp->panel->max_dp_link_rate = dp->parser->max_dp_link_rate;
+
+	drm_dbg_dp(dp->drm_dev, "max_lanes=%d max_link_rate=%d\n",
+		dp->panel->max_dp_lanes, dp->panel->max_dp_link_rate);
 
 	rc = dp_panel_read_sink_caps(dp->panel, dp->dp_display.connector);
 	if (rc)
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 5149ceb..933fa9c 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -75,12 +75,13 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
 	link_info->rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
 	link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
 
+	/* Limit data lanes from data-lanes of endpoint properity of dtsi */
 	if (link_info->num_lanes > dp_panel->max_dp_lanes)
 		link_info->num_lanes = dp_panel->max_dp_lanes;
 
-	/* Limit support upto HBR2 until HBR3 support is added */
-	if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
-		link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
+	/* Limit link rate from link-frequencies of endpoint properity of dtsi */
+	if (link_info->rate > dp_panel->max_dp_link_rate)
+		link_info->rate = dp_panel->max_dp_link_rate;
 
 	drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
 	drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
index d861197a..f04d021 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.h
+++ b/drivers/gpu/drm/msm/dp/dp_panel.h
@@ -50,6 +50,7 @@ struct dp_panel {
 
 	u32 vic;
 	u32 max_dp_lanes;
+	u32 max_dp_link_rate;
 
 	u32 max_bw_code;
 };
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
index dd73221..667981e 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -94,16 +94,30 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
 static int dp_parser_misc(struct dp_parser *parser)
 {
 	struct device_node *of_node = parser->pdev->dev.of_node;
-	int len;
-
-	len = drm_of_get_data_lanes_count(of_node, 1, DP_MAX_NUM_DP_LANES);
-	if (len < 0) {
-		DRM_WARN("Invalid property \"data-lanes\", default max DP lanes = %d\n",
-			 DP_MAX_NUM_DP_LANES);
-		len = DP_MAX_NUM_DP_LANES;
+	struct device_node *endpoint;
+	int cnt;
+	u32 frequence = 0;
+
+	endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0);
+
+	if (endpoint) {
+		cnt = of_property_count_u32_elems(endpoint, "data-lanes");
+		if (cnt < 0)
+			parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
+		else
+			parser->max_dp_lanes = cnt;
+
+		cnt = of_property_count_u32_elems(endpoint, "link-frequencies");
+		if (cnt < 0) {
+			parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 54000 khz */
+		} else {
+			of_property_read_u32_array(endpoint, "link-frequencies", &frequence, 1);
+			parser->max_dp_link_rate = frequence;
+		}
 	}
 
-	parser->max_dp_lanes = len;
+	pr_err("%s: kuogee, lane=%d frequency=%d\n", __func__, parser->max_dp_lanes, parser->max_dp_link_rate);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
index 866c1a8..76ddb751 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -15,6 +15,7 @@
 #define DP_LABEL "MDSS DP DISPLAY"
 #define DP_MAX_PIXEL_CLK_KHZ	675000
 #define DP_MAX_NUM_DP_LANES	4
+#define DP_LINK_FREQUENCY_HBR2	540000
 
 enum dp_pm_type {
 	DP_CORE_PM,
@@ -119,6 +120,7 @@ struct dp_parser {
 	struct dp_io io;
 	struct dp_display_data disp_data;
 	u32 max_dp_lanes;
+	u32 max_dp_link_rate;
 	struct drm_bridge *next_bridge;
 
 	int (*parse)(struct dp_parser *parser);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into dp_out endpoint
  2022-11-17 22:49 ` [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into " Kuogee Hsieh
@ 2022-11-18 11:01   ` Dmitry Baryshkov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-11-18 11:01 UTC (permalink / raw)
  To: Kuogee Hsieh
  Cc: quic_sbillaka, quic_abhinavk, airlied, freedreno, dri-devel,
	dianders, vkoul, agross, bjorn.andersson, linux-arm-msm, swboyd,
	sean, linux-kernel

On Fri, 18 Nov 2022 at 00:50, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
> Add both data-lanes and link-frequencies property to dp_out endpoint.

Bindings update?
Deprecate the old data-lanes property?

> Also set link-frequencies to 810000 khz at herobrine platform to have
> max link rate limited at 810000 khz (HBR3).

No. As  I stated before, the link-frequencies should list all
supported frequencies (min/max in case the frequencies are
continuous).
Stating just maximum is against the property description.

>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi   |  9 ++++++++-
>  arch/arm64/boot/dts/qcom/sc7180.dtsi           |  5 -----
>  arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 10 +++++++++-
>  arch/arm64/boot/dts/qcom/sc7280.dtsi           |  5 -----
>  4 files changed, 17 insertions(+), 12 deletions(-)
>

[skipped the sc7180 here. All comments noted against sc7280 apply to
sc7180 too].

> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index 93e39fc..e8fca18 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -440,7 +440,15 @@ ap_i2c_tpm: &i2c14 {
>         status = "okay";
>         pinctrl-names = "default";
>         pinctrl-0 = <&dp_hot_plug_det>;
> -       data-lanes = <0 1>;
> +       ports {
> +               port@1 {
> +                       reg = <1>;
> +                       dp_out: endpoint {
> +                               data-lanes = <0 1>;
> +                               link-frequencies=<810000>;

Following the existing examples is nice. Not following them is frowned upon.

> +                       };
> +               };
> +       };

Just:

&dp_out {
    data-lanes = <0  1>;
    link-frequencies = /bits/ 64 <160000000 270000000 540000000 810000000>;
};

>  };
>
>  &mdss_mdp {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index a646405..4afe53b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3899,11 +3899,6 @@
>                                                         remote-endpoint = <&dpu_intf0_out>;
>                                                 };
>                                         };
> -
> -                                       port@1 {
> -                                               reg = <1>;
> -                                               dp_out: endpoint { };
> -                                       };

Please leave it here. It is a part of the SoC, so it should be in SoC dtsi.

>                                 };
>
>                                 dp_opp_table: opp-table {

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate
  2022-11-17 22:49 ` [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate Kuogee Hsieh
@ 2022-11-18 11:04   ` Dmitry Baryshkov
  2022-11-30  0:08     ` Kuogee Hsieh
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-11-18 11:04 UTC (permalink / raw)
  To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
	vkoul, daniel, agross
  Cc: linux-arm-msm, quic_sbillaka, freedreno, quic_abhinavk, linux-kernel

On 18/11/2022 01:49, Kuogee Hsieh wrote:
> dp_out endpoint contains both data-lanes and link-frequencies properties.
> This patch parser dp_out endpoint properties and acquire dp_max_lanes and
> dp_max_link_rate from respective property. Finally, comparing them against
> both data lane and link rate read back from sink to ensure both data lane
> and link rate are supported by platform.
> In the case there is no data-lanes or link-frequencies property defined at
> dp_out endpoint, the default value are 4 data lanes with 5.4 Ghz link rate.
> 
> Changes in v2:
> -- add max link rate from dtsi
> 
> Changes in v3:
> -- parser max_data_lanes and max_dp_link_rate from dp_out endpoint
> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/sc7280.dtsi |  1 +

Should not be a part of this patch.

>   drivers/gpu/drm/msm/dp/dp_display.c  |  4 ++++
>   drivers/gpu/drm/msm/dp/dp_panel.c    |  7 ++++---
>   drivers/gpu/drm/msm/dp/dp_panel.h    |  1 +
>   drivers/gpu/drm/msm/dp/dp_parser.c   | 30 ++++++++++++++++++++++--------
>   drivers/gpu/drm/msm/dp/dp_parser.h   |  2 ++
>   6 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 4afe53b..d456e76 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3897,6 +3897,7 @@
>   						reg = <0>;
>   						dp_in: endpoint {
>   							remote-endpoint = <&dpu_intf0_out>;
> +							data-lanes = <0 1 2 3>;
>   						};
>   					};
>   				};
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 29c9845..4fe2092 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -390,6 +390,10 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
>   	struct edid *edid;
>   
>   	dp->panel->max_dp_lanes = dp->parser->max_dp_lanes;
> +	dp->panel->max_dp_link_rate = dp->parser->max_dp_link_rate;
> +
> +	drm_dbg_dp(dp->drm_dev, "max_lanes=%d max_link_rate=%d\n",
> +		dp->panel->max_dp_lanes, dp->panel->max_dp_link_rate);
>   
>   	rc = dp_panel_read_sink_caps(dp->panel, dp->dp_display.connector);
>   	if (rc)
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 5149ceb..933fa9c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -75,12 +75,13 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>   	link_info->rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
>   	link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
>   
> +	/* Limit data lanes from data-lanes of endpoint properity of dtsi */
>   	if (link_info->num_lanes > dp_panel->max_dp_lanes)
>   		link_info->num_lanes = dp_panel->max_dp_lanes;
>   
> -	/* Limit support upto HBR2 until HBR3 support is added */
> -	if (link_info->rate >= (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
> -		link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
> +	/* Limit link rate from link-frequencies of endpoint properity of dtsi */
> +	if (link_info->rate > dp_panel->max_dp_link_rate)
> +		link_info->rate = dp_panel->max_dp_link_rate;
>   
>   	drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
>   	drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h b/drivers/gpu/drm/msm/dp/dp_panel.h
> index d861197a..f04d021 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -50,6 +50,7 @@ struct dp_panel {
>   
>   	u32 vic;
>   	u32 max_dp_lanes;
> +	u32 max_dp_link_rate;
>   
>   	u32 max_bw_code;
>   };
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd73221..667981e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -94,16 +94,30 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
>   static int dp_parser_misc(struct dp_parser *parser)
>   {
>   	struct device_node *of_node = parser->pdev->dev.of_node;
> -	int len;
> -
> -	len = drm_of_get_data_lanes_count(of_node, 1, DP_MAX_NUM_DP_LANES);
> -	if (len < 0) {
> -		DRM_WARN("Invalid property \"data-lanes\", default max DP lanes = %d\n",
> -			 DP_MAX_NUM_DP_LANES);
> -		len = DP_MAX_NUM_DP_LANES;
> +	struct device_node *endpoint;
> +	int cnt;
> +	u32 frequence = 0;
> +
> +	endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0);
> +
> +	if (endpoint) {
> +		cnt = of_property_count_u32_elems(endpoint, "data-lanes");
> +		if (cnt < 0)
> +			parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
> +		else
> +			parser->max_dp_lanes = cnt;

This should be a separate patch. And now what, 
drm_of_get_data_lanes_count() can be used here too. Why are you dropping 
the generic function for the sake of your custom implementatoin.

> +
> +		cnt = of_property_count_u32_elems(endpoint, "link-frequencies");
> +		if (cnt < 0) {
> +			parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 54000 khz */
> +		} else {
> +			of_property_read_u32_array(endpoint, "link-frequencies", &frequence, 1);

link-frequencies is u64 array.

> +			parser->max_dp_link_rate = frequence;
> +		}
>   	}
>   
> -	parser->max_dp_lanes = len;
> +	pr_err("%s: kuogee, lane=%d frequency=%d\n", __func__, parser->max_dp_lanes, parser->max_dp_link_rate);
> +

Leftover?

>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 866c1a8..76ddb751 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -15,6 +15,7 @@
>   #define DP_LABEL "MDSS DP DISPLAY"
>   #define DP_MAX_PIXEL_CLK_KHZ	675000
>   #define DP_MAX_NUM_DP_LANES	4
> +#define DP_LINK_FREQUENCY_HBR2	540000
>   
>   enum dp_pm_type {
>   	DP_CORE_PM,
> @@ -119,6 +120,7 @@ struct dp_parser {
>   	struct dp_io io;
>   	struct dp_display_data disp_data;
>   	u32 max_dp_lanes;
> +	u32 max_dp_link_rate;
>   	struct drm_bridge *next_bridge;
>   
>   	int (*parse)(struct dp_parser *parser);

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate
  2022-11-18 11:04   ` Dmitry Baryshkov
@ 2022-11-30  0:08     ` Kuogee Hsieh
  0 siblings, 0 replies; 6+ messages in thread
From: Kuogee Hsieh @ 2022-11-30  0:08 UTC (permalink / raw)
  To: Dmitry Baryshkov, dri-devel, robdclark, sean, swboyd, dianders,
	vkoul, daniel, agross
  Cc: linux-arm-msm, quic_sbillaka, freedreno, quic_abhinavk, linux-kernel


On 11/18/2022 3:04 AM, Dmitry Baryshkov wrote:
> On 18/11/2022 01:49, Kuogee Hsieh wrote:
>> dp_out endpoint contains both data-lanes and link-frequencies 
>> properties.
>> This patch parser dp_out endpoint properties and acquire dp_max_lanes 
>> and
>> dp_max_link_rate from respective property. Finally, comparing them 
>> against
>> both data lane and link rate read back from sink to ensure both data 
>> lane
>> and link rate are supported by platform.
>> In the case there is no data-lanes or link-frequencies property 
>> defined at
>> dp_out endpoint, the default value are 4 data lanes with 5.4 Ghz link 
>> rate.
>>
>> Changes in v2:
>> -- add max link rate from dtsi
>>
>> Changes in v3:
>> -- parser max_data_lanes and max_dp_link_rate from dp_out endpoint
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi |  1 +
>
> Should not be a part of this patch.
>
>>   drivers/gpu/drm/msm/dp/dp_display.c  | 4 ++++
>>   drivers/gpu/drm/msm/dp/dp_panel.c    |  7 ++++---
>>   drivers/gpu/drm/msm/dp/dp_panel.h    |  1 +
>>   drivers/gpu/drm/msm/dp/dp_parser.c   | 30 
>> ++++++++++++++++++++++--------
>>   drivers/gpu/drm/msm/dp/dp_parser.h   |  2 ++
>>   6 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 4afe53b..d456e76 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -3897,6 +3897,7 @@
>>                           reg = <0>;
>>                           dp_in: endpoint {
>>                               remote-endpoint = <&dpu_intf0_out>;
>> +                            data-lanes = <0 1 2 3>;
>>                           };
>>                       };
>>                   };
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 29c9845..4fe2092 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -390,6 +390,10 @@ static int dp_display_process_hpd_high(struct 
>> dp_display_private *dp)
>>       struct edid *edid;
>>         dp->panel->max_dp_lanes = dp->parser->max_dp_lanes;
>> +    dp->panel->max_dp_link_rate = dp->parser->max_dp_link_rate;
>> +
>> +    drm_dbg_dp(dp->drm_dev, "max_lanes=%d max_link_rate=%d\n",
>> +        dp->panel->max_dp_lanes, dp->panel->max_dp_link_rate);
>>         rc = dp_panel_read_sink_caps(dp->panel, 
>> dp->dp_display.connector);
>>       if (rc)
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
>> b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index 5149ceb..933fa9c 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -75,12 +75,13 @@ static int dp_panel_read_dpcd(struct dp_panel 
>> *dp_panel)
>>       link_info->rate = 
>> drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
>>       link_info->num_lanes = dpcd[DP_MAX_LANE_COUNT] & 
>> DP_MAX_LANE_COUNT_MASK;
>>   +    /* Limit data lanes from data-lanes of endpoint properity of 
>> dtsi */
>>       if (link_info->num_lanes > dp_panel->max_dp_lanes)
>>           link_info->num_lanes = dp_panel->max_dp_lanes;
>>   -    /* Limit support upto HBR2 until HBR3 support is added */
>> -    if (link_info->rate >= 
>> (drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4)))
>> -        link_info->rate = drm_dp_bw_code_to_link_rate(DP_LINK_BW_5_4);
>> +    /* Limit link rate from link-frequencies of endpoint properity 
>> of dtsi */
>> +    if (link_info->rate > dp_panel->max_dp_link_rate)
>> +        link_info->rate = dp_panel->max_dp_link_rate;
>>         drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
>>       drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h 
>> b/drivers/gpu/drm/msm/dp/dp_panel.h
>> index d861197a..f04d021 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
>> @@ -50,6 +50,7 @@ struct dp_panel {
>>         u32 vic;
>>       u32 max_dp_lanes;
>> +    u32 max_dp_link_rate;
>>         u32 max_bw_code;
>>   };
>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
>> b/drivers/gpu/drm/msm/dp/dp_parser.c
>> index dd73221..667981e 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>> @@ -94,16 +94,30 @@ static int dp_parser_ctrl_res(struct dp_parser 
>> *parser)
>>   static int dp_parser_misc(struct dp_parser *parser)
>>   {
>>       struct device_node *of_node = parser->pdev->dev.of_node;
>> -    int len;
>> -
>> -    len = drm_of_get_data_lanes_count(of_node, 1, DP_MAX_NUM_DP_LANES);
>> -    if (len < 0) {
>> -        DRM_WARN("Invalid property \"data-lanes\", default max DP 
>> lanes = %d\n",
>> -             DP_MAX_NUM_DP_LANES);
>> -        len = DP_MAX_NUM_DP_LANES;
>> +    struct device_node *endpoint;
>> +    int cnt;
>> +    u32 frequence = 0;
>> +
>> +    endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0);
>> +
>> +    if (endpoint) {
>> +        cnt = of_property_count_u32_elems(endpoint, "data-lanes");
>> +        if (cnt < 0)
>> +            parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
>> +        else
>> +            parser->max_dp_lanes = cnt;
>
> This should be a separate patch. And now what, 
> drm_of_get_data_lanes_count() can be used here too. Why are you 
> dropping the generic function for the sake of your custom implementatoin.
drm_of_get_data_lanes_count() expect the parent node of endpoint.

It will locate endpoint first and call of_property_count_u32_elems() to 
retrieve count of elements and there is no similar function for 
link-frequencies.

To get link-frequencies we have to locate endpoint already. so why not 
use same endpoint for both data-lanes and link-frequencies.

So that consistent way are used  to retrieve both data-lanes and 
link-frequencies.




>
>> +
>> +        cnt = of_property_count_u32_elems(endpoint, 
>> "link-frequencies");
>> +        if (cnt < 0) {
>> +            parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 
>> 54000 khz */
>> +        } else {
>> +            of_property_read_u32_array(endpoint, "link-frequencies", 
>> &frequence, 1);
>
> link-frequencies is u64 array.
>
>> +            parser->max_dp_link_rate = frequence;
>> +        }
>>       }
>>   -    parser->max_dp_lanes = len;
>> +    pr_err("%s: kuogee, lane=%d frequency=%d\n", __func__, 
>> parser->max_dp_lanes, parser->max_dp_link_rate);
>> +
>
> Leftover?
>
>>       return 0;
>>   }
>>   diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
>> b/drivers/gpu/drm/msm/dp/dp_parser.h
>> index 866c1a8..76ddb751 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
>> @@ -15,6 +15,7 @@
>>   #define DP_LABEL "MDSS DP DISPLAY"
>>   #define DP_MAX_PIXEL_CLK_KHZ    675000
>>   #define DP_MAX_NUM_DP_LANES    4
>> +#define DP_LINK_FREQUENCY_HBR2    540000
>>     enum dp_pm_type {
>>       DP_CORE_PM,
>> @@ -119,6 +120,7 @@ struct dp_parser {
>>       struct dp_io io;
>>       struct dp_display_data disp_data;
>>       u32 max_dp_lanes;
>> +    u32 max_dp_link_rate;
>>       struct drm_bridge *next_bridge;
>>         int (*parse)(struct dp_parser *parser);
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-30  0:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 22:49 [PATCH v3 0/2] Add data-lanes and link-frequencies to dp_out endpoint Kuogee Hsieh
2022-11-17 22:49 ` [PATCH v3 1/2] arm64: dts: qcom: add data-lanes and link-freuencies into " Kuogee Hsieh
2022-11-18 11:01   ` Dmitry Baryshkov
2022-11-17 22:49 ` [PATCH v3 2/2] drm/msm/dp: add support of max dp link rate Kuogee Hsieh
2022-11-18 11:04   ` Dmitry Baryshkov
2022-11-30  0:08     ` Kuogee Hsieh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).