On Wed, 30 Nov 2022 at 02:12, Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> wrote:
>
> Both data-lanes and link-frequencies are property of endpoint. This
> patch parser endpoint to retrieve max data lanes and max link rate
> supported specified at dp_out endpoint. In the case where no endpoint
> specified, then 4 data lanes with HBR2 link rate (5.4G) will be the
> default link configuration.
So, you have two changes in a single patch.
1) Moving the data-lanes to the endpoint
2) Adding link-frequencies.
Please split the patch accordingly. Also keep in mind that you have to
provide backwards compatibility for the data-lanes property.
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/dp/dp_parser.c | 34 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/msm/dp/dp_parser.h | 2 ++
> 2 files changed, 28 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..9367f8c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -94,16 +94,34 @@ 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;
> + u64 frequence[4];
frequency
> +
> + endpoint = of_graph_get_endpoint_by_regs(of_node, 1, 0); /* port@1 */
> + 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_u64_elems(endpoint, "link-frequencies");
> + if (cnt < 0) {
> + parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 54000 khz */
Wrong number of zeroes
> + } else {
> + if (cnt > 4) /* 4 frequency at most */
> + cnt = 4;
'4 frequencies'. Not to mention that magic '4' should be defined
somewhere. Or removed completely. See below.
> + of_property_read_u64_array(endpoint, "link-frequencies", frequence, cnt);
Can you please use of_property_read_u64_index() instead? It also has a
nice feature of modifying the out_value only if the proper data was
found. So you can set the default and then override it with the
of_property_read function. And then divide it by 1000 to get the value
in KHz.
> + parser->max_dp_link_rate = (u32)frequence[cnt -1];
> + parser->max_dp_link_rate /= 1000; /* khz */
The HDR3 rate is 8100 Mb/s. 8 100 000 000. This doesn't fit into u32
(U32_MAX = 4 294 967 295).
> + }
> + } else {
> + /* default */
> + parser->max_dp_lanes = DP_MAX_NUM_DP_LANES; /* 4 lanes */
> + parser->max_dp_link_rate = DP_LINK_FREQUENCY_HBR2; /* 54000 khz */
Wrong number of zeroes. Better use Mb/s or Gb/s directly. Also it is a
rate, not a frequency, so the define should also use 'RATE' in its
name.
> }
>
> - parser->max_dp_lanes = len;
> 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
>
--
With best wishes
Dmitry
