| Message ID | 20250103-drm-dp-msm-add-lttpr-transparent-mode-set-v3-0-5c367f4b0763@linaro.org |
|---|---|
| Headers | show |
| Series | drm/dp: Rework LTTPR transparent mode handling and add support to msm driver | expand |
On Fri, Jan 03, 2025 at 02:58:17PM +0200, Abel Vesa wrote: > LTTPRs operating modes are defined by the DisplayPort standard and the > generic framework now provides a helper to switch between them, which > is handling the explicit disabling of non-transparent mode and its > disable->enable sequence mentioned in the DP Standard v2.0 section > 3.6.6.1. > > So use the new drm generic helper instead as it makes the code a bit > cleaner. > > Acked-by: Imre Deak <imre.deak@intel.com> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > .../gpu/drm/i915/display/intel_dp_link_training.c | 24 +++++----------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index 8b1977cfec503c70f07af716ee2c00e7605c6adf..c5bad311edf7b9a5cebb633b9e9692bae397f9ed 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -119,9 +119,6 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable) > u8 val = enable ? DP_PHY_REPEATER_MODE_TRANSPARENT : > DP_PHY_REPEATER_MODE_NON_TRANSPARENT; > > - if (drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE, &val, 1) != 1) > - return false; > - > intel_dp->lttpr_common_caps[DP_PHY_REPEATER_MODE - > DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV] = val; > > @@ -146,6 +143,7 @@ static bool intel_dp_lttpr_transparent_mode_enabled(struct intel_dp *intel_dp) > static int intel_dp_init_lttpr_phys(struct intel_dp *intel_dp, const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > { > int lttpr_count; > + int ret; > > if (!intel_dp_read_lttpr_common_caps(intel_dp, dpcd)) > return 0; > @@ -172,22 +170,8 @@ static int intel_dp_init_lttpr_phys(struct intel_dp *intel_dp, const u8 dpcd[DP_ > return lttpr_count; > } > > - /* > - * See DP Standard v2.0 3.6.6.1. about the explicit disabling of > - * non-transparent mode and the disable->enable non-transparent mode > - * sequence. > - */ > - intel_dp_set_lttpr_transparent_mode(intel_dp, true); > - > - /* > - * In case of unsupported number of LTTPRs or failing to switch to > - * non-transparent mode fall-back to transparent link training mode, > - * still taking into account any LTTPR common lane- rate/count limits. > - */ > - if (lttpr_count < 0) > - goto out_reset_lttpr_count; > - > - if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) { > + ret = drm_dp_lttpr_init(&intel_dp->aux, lttpr_count); > + if (ret) { > lt_dbg(intel_dp, DP_PHY_DPRX, > "Switching to LTTPR non-transparent LT mode failed, fall-back to transparent mode\n"); > > @@ -196,6 +180,8 @@ static int intel_dp_init_lttpr_phys(struct intel_dp *intel_dp, const u8 dpcd[DP_ > goto out_reset_lttpr_count; > } > > + intel_dp_set_lttpr_transparent_mode(intel_dp, false); > + I think the code now misses a way to update intel_dp->lttpr_common_caps in a transparent-mode case: intel_dp_set_lttpr_transparent_mode(intel_dp, true). > return lttpr_count; > > out_reset_lttpr_count: > > -- > 2.34.1 >
Looking at both i915 and nouveau DP drivers, both are setting the first LTTPR (if found) in transparent mode first and then in non-transparent mode, just like the DP v2.0 specification mentions in section 3.6.6.1. Being part of the standard, setting the LTTPR in a specific operation mode can be easily moved in the generic framework. So do that by adding a new helper. Then, the msm DP driver is lacking any kind of support for LTTPR handling, so add it by reading the LTTPR caps for figuring out the number of LTTPRs found on plug detect and then do exactly what the i915 and nouveau drivers do with respect to toggling through operating modes, just like the up-mentioned section from DP spec describes. At some point, link training per sub-segment will probably be needed, but for now, toggling the operating modes seems to be enough at least for the X Elite-based platforms that this patchset has been tested on. Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- Changes in v3: - Picked-up T-b tag from Johan for the drm/dp transparent mode set helper patch - Re-worked the return value of the drm/dp transparet mode set helper - Added some more details about what the values of the lttpr_count arg is expected to be for the drm_dp_lttpr_init(), like Johan suggested. - Re-worked the non-transparent mode disable->enable so that the rollback doesn't happen unless enable failed. - Picked-up Lyude's R-b tag for the nouveau patch. - Dropped extra parantesis at the end of the drm_dp_lttpr_init() call in i915 patch. - Picked-up Johan's T-b tag for the drm/msm/dp patch. - Added some error handling and an error message in the msm_dp_display_lttpr_init(), while dropping the unnecessary lttpr_count local variable. - Link to v2: https://lore.kernel.org/r/20241211-drm-dp-msm-add-lttpr-transparent-mode-set-v2-0-d5906ed38b28@linaro.org Changes in v2: - Added new wrapper over the set_transparent new helper in order to move the non-transparent disable and the its enable->disable sequence mentioned in the DP standard section 3.6.6.1 entirely in the generic implemetation. - Switch all 3 drivers to use the new wrapper. - Fixed the return value of the helper to return 0 on success and negative value on error. - Added explanation about the transparent/non-transparent modes into the msm dp commit message. - Dropped the condition for non-eDP in msm DP driver since it is allowed to try to get the number of LTTPRs even on eDP and it will be always 0 anyway. - Dropped the RFC prefix - Link to v1: https://lore.kernel.org/r/20241031-drm-dp-msm-add-lttpr-transparent-mode-set-v1-0-cafbb9855f40@linaro.org --- Abel Vesa (4): drm/dp: Add helper to set LTTPRs in transparent mode drm/nouveau/dp: Use the generic helper to control LTTPR transparent mode drm/i915/dp: Use the generic helper to control LTTPR transparent mode drm/msm/dp: Add support for LTTPR handling drivers/gpu/drm/display/drm_dp_helper.c | 61 ++++++++++++++++++++++ .../gpu/drm/i915/display/intel_dp_link_training.c | 24 ++------- drivers/gpu/drm/msm/dp/dp_display.c | 17 ++++++ drivers/gpu/drm/nouveau/nouveau_dp.c | 17 +----- include/drm/display/drm_dp_helper.h | 2 + 5 files changed, 87 insertions(+), 34 deletions(-) --- base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2 change-id: 20241031-drm-dp-msm-add-lttpr-transparent-mode-set-136cd5bfde07 Best regards,