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 <khs...@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
> 

Reply via email to