On Wed, Mar 12, 2025 at 12:38:03AM +0100, Aleksandrs Vinarskis wrote:
> Take into account LTTPR capabilities when selecting maximum allowed
> link rate, number of data lines. Initialize LTTPR before
> msm_dp_panel_read_sink_caps, as
> a) Link params computation need to take into account LTTPR's caps
> b) It appears DPTX shall (re)read DPRX caps after LTTPR detection
... as required by DP 2.1, Section 3.6.7.6.1
Split this into two patches.
>
> Return lttpr_count to prepare for per-segment link training.
And this one is the third one.
>
> Signed-off-by: Aleksandrs Vinarskis <alex.vinars...@gmail.com>
> Reviewed-by: Abel Vesa <abel.v...@linaro.org>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 29 +++++++++++++++++++---------
> drivers/gpu/drm/msm/dp/dp_panel.c | 30 ++++++++++++++++++++---------
> drivers/gpu/drm/msm/dp/dp_panel.h | 2 ++
> 3 files changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index bbc47d86ae9e..d0c2dc7e6648 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -108,6 +108,8 @@ struct msm_dp_display_private {
> struct msm_dp_event event_list[DP_EVENT_Q_MAX];
> spinlock_t event_lock;
>
> + u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE];
It would feel more natural to have lttpr_common_caps inside msm_dp_panel
rather than here.
> +
> bool wide_bus_supported;
>
> struct msm_dp_audio *audio;
> @@ -367,17 +369,21 @@ static int msm_dp_display_send_hpd_notification(struct
> msm_dp_display_private *d
> return 0;
> }
>
> -static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp)
> +static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8
> *dpcd)
Hmm, why? Return code is still unused in this patch. If it is a
preparation for the next one, it should be split into a separate patch.
> {
> - u8 lttpr_caps[DP_LTTPR_COMMON_CAP_SIZE];
> - int rc;
> + int rc, lttpr_count;
>
> - if (drm_dp_read_lttpr_common_caps(dp->aux, dp->panel->dpcd, lttpr_caps))
> - return;
> + if (drm_dp_read_lttpr_common_caps(dp->aux, dpcd, dp->lttpr_common_caps))
> + return 0;
>
> - rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(lttpr_caps));
> - if (rc)
> + lttpr_count = drm_dp_lttpr_count(dp->lttpr_common_caps);
> + rc = drm_dp_lttpr_init(dp->aux, lttpr_count);
> + if (rc) {
> DRM_ERROR("failed to set LTTPRs transparency mode, rc=%d\n",
> rc);
> + return 0;
> + }
> +
> + return lttpr_count;
> }
>
> static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
[...]
> @@ -64,16 +67,24 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel
> *msm_dp_panel)
> major = (link_info->revision >> 4) & 0x0f;
> minor = link_info->revision & 0x0f;
>
> - link_info->rate = drm_dp_max_link_rate(dpcd);
> - link_info->num_lanes = drm_dp_max_lane_count(dpcd);
> + max_source_lanes = msm_dp_panel->max_dp_lanes;
> + max_source_rate = msm_dp_panel->max_dp_link_rate;
>
> - /* Limit data lanes from data-lanes of endpoint property of dtsi */
> - if (link_info->num_lanes > msm_dp_panel->max_dp_lanes)
> - link_info->num_lanes = msm_dp_panel->max_dp_lanes;
> + max_sink_lanes = drm_dp_max_lane_count(dpcd);
> + max_sink_rate = drm_dp_max_link_rate(dpcd);
> +
> + max_lttpr_lanes = drm_dp_lttpr_max_lane_count(lttpr_common_caps);
> + max_lttpr_rate = drm_dp_lttpr_max_link_rate(lttpr_common_caps);
>
> + if (max_lttpr_lanes)
> + max_sink_lanes = min(max_sink_lanes, max_lttpr_lanes);
> + if (max_lttpr_rate)
> + max_sink_rate = min(max_sink_rate, max_lttpr_rate);
> +
> + /* Limit data lanes from data-lanes of endpoint property of dtsi */
> + link_info->num_lanes = min(max_sink_lanes, max_source_lanes);
> /* Limit link rate from link-frequencies of endpoint property of dtsi */
> - if (link_info->rate > msm_dp_panel->max_dp_link_rate)
> - link_info->rate = msm_dp_panel->max_dp_link_rate;
> + link_info->rate = min(max_sink_rate, max_source_rate);
Please keep existing code and extend it to handle max_lttpr_lanes /
max_lttpr_rate instead of rewriting it unnecessarily.
>
> 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);
--
With best wishes
Dmitry
