mbox series

[v12,0/5] Add data-lanes and link-frequencies to dp_out endpoint

Message ID 1670967848-31475-1-git-send-email-quic_khsieh@quicinc.com
Headers show
Series Add data-lanes and link-frequencies to dp_out endpoint | expand

Message

Kuogee Hsieh Dec. 13, 2022, 9:44 p.m. UTC
Add DP both data-lanes and link-frequencies property to dp_out endpoint and support
functions to DP driver.

Kuogee Hsieh (5):
  arm64: dts: qcom: add data-lanes and link-freuencies into dp_out
    endpoint
  dt-bindings: msm/dp: add data-lanes and link-frequencies property
  drm/msm/dp: parser data-lanes as property of dp_out endpoint
  drm/msm/dp: parser link-frequencies as property of dp_out endpoint
  drm/msm/dp: add support of max dp link rate

 .../bindings/display/msm/dp-controller.yaml        | 30 ++++++++++++-
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi       |  6 ++-
 arch/arm64/boot/dts/qcom/sc7280-herobrine.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                 | 52 ++++++++++++++++++----
 drivers/gpu/drm/msm/dp/dp_parser.h                 |  2 +
 8 files changed, 93 insertions(+), 15 deletions(-)

Comments

Dmitry Baryshkov Dec. 13, 2022, 10:06 p.m. UTC | #1
On 13 December 2022 23:44:08 EET, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>By default, HBR2 (5.4G) is the max link link be supported. This patch uses the
>actual limit specified by DT and removes the artificial limitation to 5.4 Gbps.
>Supporting HBR3 is a consequence of that.
>
>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
>
>Changes in v4:
>-- delete unnecessary pr_err
>
>Changes in v5:
>-- split parser function into different patch
>
>Changes in v9:
>-- revised commit test
>
>Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>---
> 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 +
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>index bfd0aef..edee550 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 */

Nit: property. And below too.

> 	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;
> };
Dmitry Baryshkov Dec. 13, 2022, 10:07 p.m. UTC | #2
On 13 December 2022 23:44:07 EET, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>Add capability to parser and retrieve max DP link supported rate from
>link-frequencies property of dp_out endpoint.
>
>Changes in v6:
>-- second patch after split parser patch into two patches
>
>Changes in v7:
>-- without checking cnt against DP_MAX_NUM_DP_LANES to retrieve link rate
>
>Changes in v9:
>-- separate parser link-frequencies out of data-lanes
>
>Changes in v10:
>-- add dp_parser_link_frequencies()
>
>Changes in v11:
>-- return 0 if(!endpoint)
>
>Changes in v12:
>-- replace khz with kbytes at dp_parser.h
>
>Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>---
> drivers/gpu/drm/msm/dp/dp_parser.c | 27 +++++++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_parser.h |  2 ++
> 2 files changed, 29 insertions(+)
Stephen Boyd Dec. 13, 2022, 11:11 p.m. UTC | #3
Quoting Kuogee Hsieh (2022-12-13 13:44:06)
> Add capability to parser data-lanes as property of dp_out endpoint.
> Also retain the original capability to parser data-lanes as property
> of mdss_dp node to handle legacy case.
>
> Changes in v6:
> -- first patch after split parser patch into two
>
> Changes in v7:
> -- check "data-lanes" from endpoint first
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Subject says "parser" when it probably should say "parse"?

> ---
>  drivers/gpu/drm/msm/dp/dp_parser.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd73221..b5f7e70 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -94,16 +94,25 @@ 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;
> +       int cnt;
> +
> +       /*
> +        * data-lanes is the property of dp_out endpoint
> +        */
> +       cnt = drm_of_get_data_lanes_count_ep(of_node, 1, 0, 1, DP_MAX_NUM_DP_LANES);
> +       if (cnt > 0)
> +               parser->max_dp_lanes = cnt;
> +       else {

Please add brackets to the above if to match the else.

> +               /*
> +                * legacy code, data-lanes is the property of mdss_dp node
> +                */
> +               cnt = drm_of_get_data_lanes_count(of_node, 1, DP_MAX_NUM_DP_LANES);
> +               if (cnt > 0)
> +                       parser->max_dp_lanes = cnt;
> +               else
> +                       parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
>         }
>
Stephen Boyd Dec. 13, 2022, 11:18 p.m. UTC | #4
Quoting Kuogee Hsieh (2022-12-13 13:44:07)
> Add capability to parser and retrieve max DP link supported rate from

to parse

> link-frequencies property of dp_out endpoint.
>
> Changes in v6:
> -- second patch after split parser patch into two patches
>
> Changes in v7:
> -- without checking cnt against DP_MAX_NUM_DP_LANES to retrieve link rate
>
> Changes in v9:
> -- separate parser link-frequencies out of data-lanes
>
> Changes in v10:
> -- add dp_parser_link_frequencies()
>
> Changes in v11:
> -- return 0 if(!endpoint)
>
> Changes in v12:
> -- replace khz with kbytes at dp_parser.h
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---

Same parser in subject.

>  drivers/gpu/drm/msm/dp/dp_parser.c | 27 +++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/dp/dp_parser.h |  2 ++
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index b5f7e70..5549495 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -91,6 +91,29 @@ static int dp_parser_ctrl_res(struct dp_parser *parser)
>         return 0;
>  }
>
> +static u32 dp_parser_link_frequencies(struct device_node *of_node)
> +{
> +       struct device_node *endpoint;
> +       u64 frequency = 0;
> +       int cnt = 0;

'cnt' doesn't need to be initialized here.

> +
> +       endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0); /* port@1 */
> +       if (!endpoint)
> +               return 0;
> +
> +       cnt = of_property_count_u64_elems(endpoint, "link-frequencies");
> +
> +       if (cnt > 0)
> +               of_property_read_u64_index(endpoint, "link-frequencies",
> +                                               cnt - 1, &frequency);
> +       of_node_put(endpoint);
> +
> +       frequency /= 10;        /* from symbol rate to link rate */
> +       frequency /= 1000;      /* kbytes */

Use do_div(frequency, 10 * 1000)? If you want comments it could maybe be
like this:

	do_div(frequency,
	       10 * /* from symbol rate to link rate */
	       1000); /* kbytes */

> +
> +       return frequency;
> +}
> +
Dmitry Baryshkov Dec. 13, 2022, 11:31 p.m. UTC | #5
On 13 December 2022 23:44:04 EET, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>Move data-lanes property from mdss_dp node to dp_out endpoint. Also
>add link-frequencies property into dp_out endpoint as well. The last
>frequency specified at link-frequencies will be the max link rate
>supported by DP.
>
>Changes in v5:
>-- revert changes at sc7180.dtsi and sc7280.dtsi
>-- add &dp_out to sc7180-trogdor.dtsi and sc7280-herobrine.dtsi
>
>Changes in v6:
>-- add data-lanes and link-frequencies to yaml
>
>Changes in v7:
>-- change 160000000 to 1620000000
>-- separate yaml to different patch
>
>Changes in v8:
>-- correct Bjorn mail address to kernel.org
>
>Changes in v9:
>-- use symbol rate (hz) for link-frequencies at dp_out at sc7180_trogdor.dtsi
>
>Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>---
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi   | 6 +++++-
> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 6 +++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>index eae22e6..93b0cde 100644
>--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>@@ -814,7 +814,11 @@ hp_i2c: &i2c9 {
> 	status = "okay";
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&dp_hot_plug_det>;
>-	data-lanes = <0 1>;
>+};
>+
>+&dp_out {
>+    data-lanes = <0  1>;

Quoting Krzysztof from v12:


Why adding two spaces? Just cut previous line and paste it, don't change it.

>+    link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000>;
> };
> 
> &pm6150_adc {
>diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>index c11e371..3c7a9d8 100644
>--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>@@ -442,7 +442,11 @@ ap_i2c_tpm: &i2c14 {
> 	status = "okay";
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&dp_hot_plug_det>;
>-	data-lanes = <0 1>;
>+};
>+
>+&dp_out {
>+	data-lanes = <0  1>;
>+	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> };
> 
> &mdss_mdp {