From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Kuogee Hsieh <khsieh@codeaurora.org>
Cc: freedreno@lists.freedesktop.org, airlied@linux.ie,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, abhinavk@codeaurora.org,
swboyd@chromium.org, tanmay@codeaurora.org,
aravindh@codeaurora.org, sean@poorly.run
Subject: Re: [PATCH v2 2/2] drm/msm/dp: add supported max link rate specified from dtsi
Date: Thu, 18 Feb 2021 16:45:39 -0600 [thread overview]
Message-ID: <YC7uE2L3TPPQhAfS@builder.lan> (raw)
In-Reply-To: <1613681704-12539-1-git-send-email-khsieh@codeaurora.org>
On Thu 18 Feb 14:55 CST 2021, Kuogee Hsieh wrote:
> Allow supported link rate to be limited to the value specified at
> dtsi. If it is not specified, then link rate is derived from dpcd
> directly. Below are examples,
> link-rate = <162000> for max link rate limited at 1.62G
> link-rate = <270000> for max link rate limited at 2.7G
> link-rate = <540000> for max link rate limited at 5.4G
> link-rate = <810000> for max link rate limited at 8.1G
>
> Changes in V2:
> -- allow supported max link rate specified from dtsi
>
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 1 +
> 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 | 13 +++++++++++++
> drivers/gpu/drm/msm/dp/dp_parser.h | 1 +
> 5 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 5a39da6..f633ba6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -322,6 +322,7 @@ 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_link_rate = dp->parser->max_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 9cc8166..be7f102 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -76,9 +76,10 @@ static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
> 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 support of ink rate if specified */
> + if (dp_panel->max_link_rate &&
Make the parser always initialize max_link_rate to something reasonable
and just compare against that here.
> + (link_info->rate > dp_panel->max_link_rate))
No need for the (), nor for line breaking this.
> + link_info->rate = dp_panel->max_link_rate;
>
> DRM_DEBUG_DP("version: %d.%d\n", major, minor);
> DRM_DEBUG_DP("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 9023e5b..1876f5e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> @@ -51,6 +51,7 @@ struct dp_panel {
> u32 vic;
> u32 max_pclk_khz;
> u32 max_dp_lanes;
> + u32 max_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 0519dd3..d8b6898 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -87,6 +87,8 @@ static int dp_parser_misc(struct dp_parser *parser)
> struct device_node *of_node = parser->pdev->dev.of_node;
> int len = 0;
> const char *data_lane_property = "data-lanes";
> + const char *link_rate_property = "link-rate";
There's no reason for stashing these in local variables.
> + u32 rate = 0;
>
> len = of_property_count_elems_of_size(of_node,
> data_lane_property, sizeof(u32));
> @@ -97,6 +99,17 @@ static int dp_parser_misc(struct dp_parser *parser)
> }
>
> parser->max_dp_lanes = len;
> +
> + len = of_property_count_elems_of_size(of_node,
> + link_rate_property, sizeof(u32));
I'm afraid I don't see the reason for this, simply rely on the return
value of of_property_read_u32(), i.e.
ret = of_property_read_u32(node, "link-rate", &rate);
if (!ret)
parser->max_link_rate = rate;
Or if you want to give it some default value:
rate = 1234;
of_property_read_u32(node, "link-rate", &rate);
Which either will overwrite the rate with the value of the property, or
leave it untouched.
Regards,
Bjorn
> +
> + if (len == 1) {
> + of_property_read_u32_index(of_node,
> + link_rate_property, 0, &rate);
> +
> + parser->max_link_rate = rate;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 34b4962..7046fce 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -116,6 +116,7 @@ struct dp_parser {
> struct dp_display_data disp_data;
> const struct dp_regulator_cfg *regulator_cfg;
> u32 max_dp_lanes;
> + u32 max_link_rate;
>
> int (*parse)(struct dp_parser *parser);
> };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-02-18 22:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-18 20:55 [PATCH v2 2/2] drm/msm/dp: add supported max link rate specified from dtsi Kuogee Hsieh
2021-02-18 22:45 ` Bjorn Andersson [this message]
2021-02-18 23:02 ` Stephen Boyd
2021-02-19 16:39 ` khsieh
2021-02-19 22:46 ` Stephen Boyd
2021-02-22 16:30 ` khsieh
2021-02-22 16:55 ` [Freedreno] " Sean Paul
2021-02-22 17:32 ` khsieh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YC7uE2L3TPPQhAfS@builder.lan \
--to=bjorn.andersson@linaro.org \
--cc=abhinavk@codeaurora.org \
--cc=airlied@linux.ie \
--cc=aravindh@codeaurora.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=khsieh@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sean@poorly.run \
--cc=swboyd@chromium.org \
--cc=tanmay@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).