Quoting Kuogee Hsieh (2021-07-06 10:20:20)
> Main link symbol locked is achieved at end of link training 2. Some
> dongle main link symbol may become unlocked again if host did not end
> link training soon enough after completion of link training 2. Host
> have to re train main link if loss of symbol lock detected before
> end link training so that the coming video stream can be transmitted
> to sink properly.
>
> Signed-off-by: Kuogee Hsieh <khsieh@xxxxxxxxxxxxxx>
I guess this is a fix for the original driver, so it should be tagged
with Fixes appropriately.
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 0cb01a9..e616ab2 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1661,6 +1661,25 @@ static bool dp_ctrl_any_lane_cr_lose(struct dp_ctrl_private *ctrl,
> return false;
> }
>
> +static bool dp_ctrl_loss_symbol_lock(struct dp_ctrl_private *ctrl)
> +{
> + u8 link_status[6];
Can we use link_status[DP_LINK_STATUS_SIZE] instead?
> + u8 status;
> + int i;
> + int lane = ctrl->link->link_params.num_lanes;
s/lane/num_lanes/
would make the code easier to read
> +
> + dp_ctrl_read_link_status(ctrl, link_status);
> +
> + for (i = 0; i < lane; i++) {
> + status = link_status[i / 2];
> + status >>= ((i % 2) * 4);
> + if (!(status & DP_LANE_SYMBOL_LOCKED))
> + return true;
> + }
> +
> + return false;
> +}
> +
> int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
> {
> int rc = 0;
> @@ -1777,6 +1796,17 @@ int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl)
> return rc;
> }
>
> +static int dp_ctrl_link_retrain(struct dp_ctrl_private *ctrl)
> +{
> + int ret = 0;
Please drop init of ret.
> + u8 cr_status[2];
> + int training_step = DP_TRAINING_NONE;
> +
> + ret = dp_ctrl_setup_main_link(ctrl, cr_status, &training_step);
as it is assigned here.
> +
> + return ret;
And indeed, it could be 'return dp_ctrl_setup_main_link()' instead.
> +}
> +
> int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
> {
> int ret = 0;
> @@ -1802,6 +1832,10 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
> }
> }
>
> + /* if loss symbol lock happen, then retaining the link */
retain or retrain? The comment seems to be saying what the code says "if
loss retrain", so the comment is not very useful.
> + if (dp_ctrl_loss_symbol_lock(ctrl))
> + dp_ctrl_link_retrain(ctrl);
> +
> /* stop txing train pattern to end link training */
> dp_ctrl_clear_training_pattern(ctrl);
>
