| Message ID | 20240319225122.3048400-4-sean.anderson@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: zynqmp_dp: Misc. patches and debugfs support | expand |
On 20/03/2024 00:51, Sean Anderson wrote: > The feedback we get from the DPRX is per-lane. Make changes using this > information, instead of picking the maximum values from all lanes. This > results in more-consistent training on marginal links. > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- > > (no changes since v1) > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++--------------- > 1 file changed, 8 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 98a32e6a0459..8635b5673386 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp, > u8 link_status[DP_LINK_STATUS_SIZE]) > { > u8 *train_set = dp->train_set; > - u8 voltage = 0, preemphasis = 0; > u8 i; > > for (i = 0; i < dp->mode.lane_cnt; i++) { > - u8 v = drm_dp_get_adjust_request_voltage(link_status, i); > - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i); > + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i); > + u8 preemphasis = > + drm_dp_get_adjust_request_pre_emphasis(link_status, i); > > - if (v > voltage) > - voltage = v; > + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) > + voltage |= DP_TRAIN_MAX_SWING_REACHED; > > - if (p > preemphasis) > - preemphasis = p; > - } > + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) > + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; > > - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) > - voltage |= DP_TRAIN_MAX_SWING_REACHED; > - > - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) > - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; > - > - for (i = 0; i < dp->mode.lane_cnt; i++) > train_set[i] = voltage | preemphasis; > + } > } > > /** Looks fine to me, but a few cosmetic suggestions, feel free to ignore if not to your liking: 1) u8 voltage, preemphasis; voltage = drm_dp_get_adjust_request_voltage(link_status, i); preemphasis = drm_dp_get_adjust_request_pre_emphasis(link_status, i); 2) for (unsigned int i = 0; i < dp->mode.lane_cnt; i++) 3) dp->train_set[i] = voltage | preemphasis; Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Tomi
On 3/20/24 01:57, Tomi Valkeinen wrote: > On 20/03/2024 00:51, Sean Anderson wrote: >> The feedback we get from the DPRX is per-lane. Make changes using this >> information, instead of picking the maximum values from all lanes. This >> results in more-consistent training on marginal links. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> (no changes since v1) >> >> drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++--------------- >> 1 file changed, 8 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> index 98a32e6a0459..8635b5673386 100644 >> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c >> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp, >> u8 link_status[DP_LINK_STATUS_SIZE]) >> { >> u8 *train_set = dp->train_set; >> - u8 voltage = 0, preemphasis = 0; >> u8 i; >> for (i = 0; i < dp->mode.lane_cnt; i++) { >> - u8 v = drm_dp_get_adjust_request_voltage(link_status, i); >> - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i); >> + u8 preemphasis = >> + drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> - if (v > voltage) >> - voltage = v; >> + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> + voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - if (p > preemphasis) >> - preemphasis = p; >> - } >> + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> - voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - >> - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - >> - for (i = 0; i < dp->mode.lane_cnt; i++) >> train_set[i] = voltage | preemphasis; >> + } >> } >> /** > > Looks fine to me, but a few cosmetic suggestions, feel free to ignore if not to your liking: > > 1) > > u8 voltage, preemphasis; > > voltage = drm_dp_get_adjust_request_voltage(link_status, i); > preemphasis = drm_dp_get_adjust_request_pre_emphasis(link_status, i); If the comment here is about the line break, I agree that this looks better but the second line is over 80 characters. > 2) > > for (unsigned int i = 0; i < dp->mode.lane_cnt; i++) Is this allowed now? > 3) > > dp->train_set[i] = voltage | preemphasis; This would be undone in patch 7/8. --Sean > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Tomi >
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index 98a32e6a0459..8635b5673386 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp, u8 link_status[DP_LINK_STATUS_SIZE]) { u8 *train_set = dp->train_set; - u8 voltage = 0, preemphasis = 0; u8 i; for (i = 0; i < dp->mode.lane_cnt; i++) { - u8 v = drm_dp_get_adjust_request_voltage(link_status, i); - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i); + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i); + u8 preemphasis = + drm_dp_get_adjust_request_pre_emphasis(link_status, i); - if (v > voltage) - voltage = v; + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) + voltage |= DP_TRAIN_MAX_SWING_REACHED; - if (p > preemphasis) - preemphasis = p; - } + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) - voltage |= DP_TRAIN_MAX_SWING_REACHED; - - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; - - for (i = 0; i < dp->mode.lane_cnt; i++) train_set[i] = voltage | preemphasis; + } } /**
The feedback we get from the DPRX is per-lane. Make changes using this information, instead of picking the maximum values from all lanes. This results in more-consistent training on marginal links. Signed-off-by: Sean Anderson <sean.anderson@linux.dev> --- (no changes since v1) drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)