* [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver
@ 2025-01-03 12:58 Abel Vesa
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-03 12:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten
Cc: Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Abel Vesa, Johan Hovold, Imre Deak
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,
--
Abel Vesa <abel.vesa@linaro.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode
2025-01-03 12:58 [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver Abel Vesa
@ 2025-01-03 12:58 ` Abel Vesa
2025-01-03 18:06 ` Dmitry Baryshkov
2025-01-07 9:30 ` Johan Hovold
2025-01-03 12:58 ` [PATCH v3 2/4] drm/nouveau/dp: Use the generic helper to control LTTPR " Abel Vesa
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-03 12:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten
Cc: Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Abel Vesa, Johan Hovold
According to the DisplayPort standard, LTTPRs have two operating
modes:
- non-transparent - it replies to DPCD LTTPR field specific AUX
requests, while passes through all other AUX requests
- transparent - it passes through all AUX requests.
Switching between this two modes is done by the DPTX by issuing
an AUX write to the DPCD PHY_REPEATER_MODE register.
Add a generic helper that allows switching between these modes.
Also add a generic wrapper for the helper that handles the explicit
disabling of non-transparent mode and its disable->enable sequence
mentioned in the DP Standard v2.0 section 3.6.6.1. Do this in order
to move this handling out of the vendor specific driver implementation
into the generic framework.
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/gpu/drm/display/drm_dp_helper.c | 61 +++++++++++++++++++++++++++++++++
include/drm/display/drm_dp_helper.h | 2 ++
2 files changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index da3c8521a7fa7d3c9761377363cdd4b44ab1106e..0f665d9adf02cfd16f0c250eb7ae8fe0461f8fdd 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2817,6 +2817,67 @@ int drm_dp_lttpr_max_link_rate(const u8 caps[DP_LTTPR_COMMON_CAP_SIZE])
}
EXPORT_SYMBOL(drm_dp_lttpr_max_link_rate);
+/**
+ * drm_dp_lttpr_set_transparent_mode - set the LTTPR in transparent mode
+ * @aux: DisplayPort AUX channel
+ * @enable: Enable or disable transparent mode
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_lttpr_set_transparent_mode(struct drm_dp_aux *aux, bool enable)
+{
+ u8 val = enable ? DP_PHY_REPEATER_MODE_TRANSPARENT :
+ DP_PHY_REPEATER_MODE_NON_TRANSPARENT;
+ int ret = drm_dp_dpcd_writeb(aux, DP_PHY_REPEATER_MODE, val);
+
+ if (ret < 0)
+ return ret;
+
+ return (ret == 1) ? 0 : -EIO;
+}
+EXPORT_SYMBOL(drm_dp_lttpr_set_transparent_mode);
+
+/**
+ * drm_dp_lttpr_init - init LTTPR transparency mode according to DP standard
+ *
+ * @aux: DisplayPort AUX channel
+ * @lttpr_count: Number of LTTPRs. Between 0 and 8, according to DP standard.
+ * Negative error code for any non-valid number.
+ * See drm_dp_lttpr_count().
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count)
+{
+ int ret;
+
+ if (!lttpr_count)
+ return 0;
+
+ /*
+ * 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.
+ */
+ ret = drm_dp_lttpr_set_transparent_mode(aux, true);
+ if (ret)
+ return ret;
+
+ if (lttpr_count < 0)
+ return -ENODEV;
+
+ /*
+ * Roll-back to tranparent mode if setting non-tranparent mode failed
+ */
+ if (drm_dp_lttpr_set_transparent_mode(aux, false)) {
+ drm_dp_lttpr_set_transparent_mode(aux, true);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_lttpr_init);
+
/**
* drm_dp_lttpr_max_lane_count - get the maximum lane count supported by all LTTPRs
* @caps: LTTPR common capabilities
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 8f4054a560396a43750570a8c2e95624039ab8ad..3311df3b58255cf0620391d0948ccf6b569a8a34 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -630,6 +630,8 @@ int drm_dp_read_lttpr_phy_caps(struct drm_dp_aux *aux,
u8 caps[DP_LTTPR_PHY_CAP_SIZE]);
int drm_dp_lttpr_count(const u8 cap[DP_LTTPR_COMMON_CAP_SIZE]);
int drm_dp_lttpr_max_link_rate(const u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
+int drm_dp_lttpr_set_transparent_mode(struct drm_dp_aux *aux, bool enable);
+int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count);
int drm_dp_lttpr_max_lane_count(const u8 caps[DP_LTTPR_COMMON_CAP_SIZE]);
bool drm_dp_lttpr_voltage_swing_level_3_supported(const u8 caps[DP_LTTPR_PHY_CAP_SIZE]);
bool drm_dp_lttpr_pre_emphasis_level_3_supported(const u8 caps[DP_LTTPR_PHY_CAP_SIZE]);
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/4] drm/nouveau/dp: Use the generic helper to control LTTPR transparent mode
2025-01-03 12:58 [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver Abel Vesa
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
@ 2025-01-03 12:58 ` Abel Vesa
2025-01-03 12:58 ` [PATCH v3 3/4] drm/i915/dp: " Abel Vesa
2025-01-03 12:58 ` [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling Abel Vesa
3 siblings, 0 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-03 12:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten
Cc: Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Abel Vesa
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.
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/gpu/drm/nouveau/nouveau_dp.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c b/drivers/gpu/drm/nouveau/nouveau_dp.c
index bcda0105160f1450df855281e0d932606a5095dd..55691ec44abaa53c84e73358e33df1949bb1e35c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -79,21 +79,8 @@ nouveau_dp_probe_dpcd(struct nouveau_connector *nv_connector,
!drm_dp_read_lttpr_common_caps(aux, dpcd, outp->dp.lttpr.caps)) {
int nr = drm_dp_lttpr_count(outp->dp.lttpr.caps);
- if (nr) {
- drm_dp_dpcd_writeb(aux, DP_PHY_REPEATER_MODE,
- DP_PHY_REPEATER_MODE_TRANSPARENT);
-
- if (nr > 0) {
- ret = drm_dp_dpcd_writeb(aux, DP_PHY_REPEATER_MODE,
- DP_PHY_REPEATER_MODE_NON_TRANSPARENT);
- if (ret != 1) {
- drm_dp_dpcd_writeb(aux, DP_PHY_REPEATER_MODE,
- DP_PHY_REPEATER_MODE_TRANSPARENT);
- } else {
- outp->dp.lttpr.nr = nr;
- }
- }
- }
+ if (!drm_dp_lttpr_init(aux, nr))
+ outp->dp.lttpr.nr = nr;
}
ret = drm_dp_read_dpcd_caps(aux, dpcd);
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/4] drm/i915/dp: Use the generic helper to control LTTPR transparent mode
2025-01-03 12:58 [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver Abel Vesa
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
2025-01-03 12:58 ` [PATCH v3 2/4] drm/nouveau/dp: Use the generic helper to control LTTPR " Abel Vesa
@ 2025-01-03 12:58 ` Abel Vesa
2025-01-03 18:09 ` Dmitry Baryshkov
2025-01-03 12:58 ` [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling Abel Vesa
3 siblings, 1 reply; 14+ messages in thread
From: Abel Vesa @ 2025-01-03 12:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten
Cc: Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Abel Vesa, Imre Deak
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);
+
return lttpr_count;
out_reset_lttpr_count:
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling
2025-01-03 12:58 [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver Abel Vesa
` (2 preceding siblings ...)
2025-01-03 12:58 ` [PATCH v3 3/4] drm/i915/dp: " Abel Vesa
@ 2025-01-03 12:58 ` Abel Vesa
2025-01-03 18:12 ` Dmitry Baryshkov
2025-01-07 9:32 ` Johan Hovold
3 siblings, 2 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-03 12:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten
Cc: Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Abel Vesa, Johan Hovold
Link Training Tunable PHY Repeaters (LTTPRs) are defined in DisplayPort
1.4a specification. As the name suggests, these PHY repeaters are
capable of adjusting their output for link training purposes.
According to the DisplayPort standard, LTTPRs have two operating
modes:
- non-transparent - it replies to DPCD LTTPR field specific AUX
requests, while passes through all other AUX requests
- transparent - it passes through all AUX requests.
Switching between this two modes is done by the DPTX by issuing
an AUX write to the DPCD PHY_REPEATER_MODE register.
The msm DP driver is currently lacking any handling of LTTPRs.
This means that if at least one LTTPR is found between DPTX and DPRX,
the link training would fail if that LTTPR was not already configured
in transparent mode.
The section 3.6.6.1 from the DisplayPort v2.0 specification mandates
that before link training with the LTTPR is started, the DPTX may place
the LTTPR in non-transparent mode by first switching to transparent mode
and then to non-transparent mode. This operation seems to be needed only
on first link training and doesn't need to be done again until device is
unplugged.
It has been observed on a few X Elite-based platforms which have
such LTTPRs in their board design that the DPTX needs to follow the
procedure described above in order for the link training to be successful.
So add support for reading the LTTPR DPCD caps to figure out the number
of such LTTPRs first. Then, for platforms (or Type-C dongles) that have
at least one such an LTTPR, set its operation mode to transparent mode
first and then to non-transparent, just like the mentioned section of
the specification mandates.
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/gpu/drm/msm/dp/dp_display.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index aff51bb973ebe0835c96420d16547ebae0c6c0f2..6ea8245284ab5b6068bfba64f01a960838f577b2 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -107,6 +107,8 @@ struct msm_dp_display_private {
struct msm_dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
+ u8 lttpr_caps[DP_LTTPR_COMMON_CAP_SIZE];
+
bool wide_bus_supported;
struct msm_dp_audio *audio;
@@ -367,12 +369,27 @@ 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)
+{
+ int rc;
+
+ if (drm_dp_read_lttpr_common_caps(dp->aux, dp->panel->dpcd,
+ dp->lttpr_caps))
+ return;
+
+ rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(dp->lttpr_caps));
+ if (rc)
+ DRM_ERROR("failed to set LTTPRs transparency mode, rc=%d\n", rc);
+}
+
static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
{
struct drm_connector *connector = dp->msm_dp_display.connector;
const struct drm_display_info *info = &connector->display_info;
int rc = 0;
+ msm_dp_display_lttpr_init(dp);
+
rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
if (rc)
goto end;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
@ 2025-01-03 18:06 ` Dmitry Baryshkov
2025-01-07 9:30 ` Johan Hovold
1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-01-03 18:06 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Johan Hovold
On Fri, Jan 03, 2025 at 02:58:15PM +0200, Abel Vesa wrote:
> According to the DisplayPort standard, LTTPRs have two operating
> modes:
> - non-transparent - it replies to DPCD LTTPR field specific AUX
> requests, while passes through all other AUX requests
> - transparent - it passes through all AUX requests.
>
> Switching between this two modes is done by the DPTX by issuing
> an AUX write to the DPCD PHY_REPEATER_MODE register.
>
> Add a generic helper that allows switching between these modes.
>
> Also add a generic wrapper for the helper that handles the explicit
> disabling of non-transparent mode and its disable->enable sequence
> mentioned in the DP Standard v2.0 section 3.6.6.1. Do this in order
> to move this handling out of the vendor specific driver implementation
> into the generic framework.
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 61 +++++++++++++++++++++++++++++++++
> include/drm/display/drm_dp_helper.h | 2 ++
> 2 files changed, 63 insertions(+)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/dp: Use the generic helper to control LTTPR transparent mode
2025-01-03 12:58 ` [PATCH v3 3/4] drm/i915/dp: " Abel Vesa
@ 2025-01-03 18:09 ` Dmitry Baryshkov
2025-01-06 12:45 ` Abel Vesa
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-01-03 18:09 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Imre Deak
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
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling
2025-01-03 12:58 ` [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling Abel Vesa
@ 2025-01-03 18:12 ` Dmitry Baryshkov
2025-01-07 9:32 ` Johan Hovold
1 sibling, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-01-03 18:12 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Johan Hovold
On Fri, Jan 03, 2025 at 02:58:18PM +0200, Abel Vesa wrote:
> Link Training Tunable PHY Repeaters (LTTPRs) are defined in DisplayPort
> 1.4a specification. As the name suggests, these PHY repeaters are
> capable of adjusting their output for link training purposes.
>
> According to the DisplayPort standard, LTTPRs have two operating
> modes:
> - non-transparent - it replies to DPCD LTTPR field specific AUX
> requests, while passes through all other AUX requests
> - transparent - it passes through all AUX requests.
>
> Switching between this two modes is done by the DPTX by issuing
> an AUX write to the DPCD PHY_REPEATER_MODE register.
>
> The msm DP driver is currently lacking any handling of LTTPRs.
> This means that if at least one LTTPR is found between DPTX and DPRX,
> the link training would fail if that LTTPR was not already configured
> in transparent mode.
>
> The section 3.6.6.1 from the DisplayPort v2.0 specification mandates
> that before link training with the LTTPR is started, the DPTX may place
> the LTTPR in non-transparent mode by first switching to transparent mode
> and then to non-transparent mode. This operation seems to be needed only
> on first link training and doesn't need to be done again until device is
> unplugged.
>
> It has been observed on a few X Elite-based platforms which have
> such LTTPRs in their board design that the DPTX needs to follow the
> procedure described above in order for the link training to be successful.
>
> So add support for reading the LTTPR DPCD caps to figure out the number
> of such LTTPRs first. Then, for platforms (or Type-C dongles) that have
> at least one such an LTTPR, set its operation mode to transparent mode
> first and then to non-transparent, just like the mentioned section of
> the specification mandates.
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/dp: Use the generic helper to control LTTPR transparent mode
2025-01-03 18:09 ` Dmitry Baryshkov
@ 2025-01-06 12:45 ` Abel Vesa
2025-01-08 13:11 ` Dmitry Baryshkov
0 siblings, 1 reply; 14+ messages in thread
From: Abel Vesa @ 2025-01-06 12:45 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Imre Deak
On 25-01-03 20:09:42, Dmitry Baryshkov wrote:
> 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).
It is being called if the drm_dp_lttpr_init() returns a non-zero value,
but that is not part of the diff here.
>
> > return lttpr_count;
> >
> > out_reset_lttpr_count:
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
2025-01-03 18:06 ` Dmitry Baryshkov
@ 2025-01-07 9:30 ` Johan Hovold
2025-01-08 13:16 ` Abel Vesa
1 sibling, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2025-01-07 9:30 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, Bjorn Andersson, Konrad Dybcio, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Johan Hovold
On Fri, Jan 03, 2025 at 02:58:15PM +0200, Abel Vesa wrote:
> According to the DisplayPort standard, LTTPRs have two operating
> modes:
> - non-transparent - it replies to DPCD LTTPR field specific AUX
> requests, while passes through all other AUX requests
> - transparent - it passes through all AUX requests.
>
> Switching between this two modes is done by the DPTX by issuing
> an AUX write to the DPCD PHY_REPEATER_MODE register.
>
> Add a generic helper that allows switching between these modes.
>
> Also add a generic wrapper for the helper that handles the explicit
> disabling of non-transparent mode and its disable->enable sequence
> mentioned in the DP Standard v2.0 section 3.6.6.1. Do this in order
> to move this handling out of the vendor specific driver implementation
> into the generic framework.
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> +/**
> + * drm_dp_lttpr_init - init LTTPR transparency mode according to DP standard
> + *
> + * @aux: DisplayPort AUX channel
> + * @lttpr_count: Number of LTTPRs. Between 0 and 8, according to DP standard.
> + * Negative error code for any non-valid number.
> + * See drm_dp_lttpr_count().
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count)
> +{
> + int ret;
> +
> + if (!lttpr_count)
> + return 0;
> +
> + /*
> + * 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.
> + */
> + ret = drm_dp_lttpr_set_transparent_mode(aux, true);
> + if (ret)
> + return ret;
> +
> + if (lttpr_count < 0)
> + return -ENODEV;
> +
> + /*
> + * Roll-back to tranparent mode if setting non-tranparent mode failed
typo: transparent (2x)
> + */
I think this comment now needs to go inside the conditional, if you want
to keep it at all.
> + if (drm_dp_lttpr_set_transparent_mode(aux, false)) {
> + drm_dp_lttpr_set_transparent_mode(aux, true);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_lttpr_init);
This looks much better to me now, so with the above addressed:
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling
2025-01-03 12:58 ` [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling Abel Vesa
2025-01-03 18:12 ` Dmitry Baryshkov
@ 2025-01-07 9:32 ` Johan Hovold
1 sibling, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2025-01-07 9:32 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, Bjorn Andersson, Konrad Dybcio, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Johan Hovold
On Fri, Jan 03, 2025 at 02:58:18PM +0200, Abel Vesa wrote:
> Link Training Tunable PHY Repeaters (LTTPRs) are defined in DisplayPort
> 1.4a specification. As the name suggests, these PHY repeaters are
> capable of adjusting their output for link training purposes.
>
> According to the DisplayPort standard, LTTPRs have two operating
> modes:
> - non-transparent - it replies to DPCD LTTPR field specific AUX
> requests, while passes through all other AUX requests
> - transparent - it passes through all AUX requests.
>
> Switching between this two modes is done by the DPTX by issuing
> an AUX write to the DPCD PHY_REPEATER_MODE register.
>
> The msm DP driver is currently lacking any handling of LTTPRs.
> This means that if at least one LTTPR is found between DPTX and DPRX,
> the link training would fail if that LTTPR was not already configured
> in transparent mode.
>
> The section 3.6.6.1 from the DisplayPort v2.0 specification mandates
> that before link training with the LTTPR is started, the DPTX may place
> the LTTPR in non-transparent mode by first switching to transparent mode
> and then to non-transparent mode. This operation seems to be needed only
> on first link training and doesn't need to be done again until device is
> unplugged.
>
> It has been observed on a few X Elite-based platforms which have
> such LTTPRs in their board design that the DPTX needs to follow the
> procedure described above in order for the link training to be successful.
>
> So add support for reading the LTTPR DPCD caps to figure out the number
> of such LTTPRs first. Then, for platforms (or Type-C dongles) that have
> at least one such an LTTPR, set its operation mode to transparent mode
> first and then to non-transparent, just like the mentioned section of
> the specification mandates.
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/dp: Use the generic helper to control LTTPR transparent mode
2025-01-06 12:45 ` Abel Vesa
@ 2025-01-08 13:11 ` Dmitry Baryshkov
2025-01-08 13:15 ` Abel Vesa
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-01-08 13:11 UTC (permalink / raw)
To: Abel Vesa
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Imre Deak
On Mon, Jan 06, 2025 at 02:45:48PM +0200, Abel Vesa wrote:
> On 25-01-03 20:09:42, Dmitry Baryshkov wrote:
> > 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).
>
> It is being called if the drm_dp_lttpr_init() returns a non-zero value,
> but that is not part of the diff here.
Ack, thanks for the explanation. I'd suggest mentioning that in the
commit message.
>
> >
> > > return lttpr_count;
> > >
> > > out_reset_lttpr_count:
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/dp: Use the generic helper to control LTTPR transparent mode
2025-01-08 13:11 ` Dmitry Baryshkov
@ 2025-01-08 13:15 ` Abel Vesa
0 siblings, 0 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-08 13:15 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
Bjorn Andersson, Konrad Dybcio, Johan Hovold, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Imre Deak
On 25-01-08 15:11:50, Dmitry Baryshkov wrote:
> On Mon, Jan 06, 2025 at 02:45:48PM +0200, Abel Vesa wrote:
> > On 25-01-03 20:09:42, Dmitry Baryshkov wrote:
> > > 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).
> >
> > It is being called if the drm_dp_lttpr_init() returns a non-zero value,
> > but that is not part of the diff here.
>
> Ack, thanks for the explanation. I'd suggest mentioning that in the
> commit message.
Sure. Will do.
>
> >
> > >
> > > > return lttpr_count;
> > > >
> > > > out_reset_lttpr_count:
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry
Thanks for reviewing!
Abel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode
2025-01-07 9:30 ` Johan Hovold
@ 2025-01-08 13:16 ` Abel Vesa
0 siblings, 0 replies; 14+ messages in thread
From: Abel Vesa @ 2025-01-08 13:16 UTC (permalink / raw)
To: Johan Hovold
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Karol Herbst, Lyude Paul, Danilo Krummrich,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, Bjorn Andersson, Konrad Dybcio, dri-devel,
linux-kernel, nouveau, intel-gfx, intel-xe, linux-arm-msm,
freedreno, Johan Hovold
On 25-01-07 10:30:29, Johan Hovold wrote:
> On Fri, Jan 03, 2025 at 02:58:15PM +0200, Abel Vesa wrote:
> > According to the DisplayPort standard, LTTPRs have two operating
> > modes:
> > - non-transparent - it replies to DPCD LTTPR field specific AUX
> > requests, while passes through all other AUX requests
> > - transparent - it passes through all AUX requests.
> >
> > Switching between this two modes is done by the DPTX by issuing
> > an AUX write to the DPCD PHY_REPEATER_MODE register.
> >
> > Add a generic helper that allows switching between these modes.
> >
> > Also add a generic wrapper for the helper that handles the explicit
> > disabling of non-transparent mode and its disable->enable sequence
> > mentioned in the DP Standard v2.0 section 3.6.6.1. Do this in order
> > to move this handling out of the vendor specific driver implementation
> > into the generic framework.
> >
> > Tested-by: Johan Hovold <johan+linaro@kernel.org>
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>
> > +/**
> > + * drm_dp_lttpr_init - init LTTPR transparency mode according to DP standard
> > + *
> > + * @aux: DisplayPort AUX channel
> > + * @lttpr_count: Number of LTTPRs. Between 0 and 8, according to DP standard.
> > + * Negative error code for any non-valid number.
> > + * See drm_dp_lttpr_count().
> > + *
> > + * Returns 0 on success or a negative error code on failure.
> > + */
> > +int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count)
> > +{
> > + int ret;
> > +
> > + if (!lttpr_count)
> > + return 0;
> > +
> > + /*
> > + * 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.
> > + */
> > + ret = drm_dp_lttpr_set_transparent_mode(aux, true);
> > + if (ret)
> > + return ret;
> > +
> > + if (lttpr_count < 0)
> > + return -ENODEV;
> > +
> > + /*
> > + * Roll-back to tranparent mode if setting non-tranparent mode failed
>
> typo: transparent (2x)
Will fix.
>
> > + */
>
> I think this comment now needs to go inside the conditional, if you want
> to keep it at all.
Yes, will move it.
>
> > + if (drm_dp_lttpr_set_transparent_mode(aux, false)) {
> > + drm_dp_lttpr_set_transparent_mode(aux, true);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_dp_lttpr_init);
>
> This looks much better to me now, so with the above addressed:
>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Thanks for reviewing!
Abel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-01-08 13:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-03 12:58 [PATCH v3 0/4] drm/dp: Rework LTTPR transparent mode handling and add support to msm driver Abel Vesa
2025-01-03 12:58 ` [PATCH v3 1/4] drm/dp: Add helper to set LTTPRs in transparent mode Abel Vesa
2025-01-03 18:06 ` Dmitry Baryshkov
2025-01-07 9:30 ` Johan Hovold
2025-01-08 13:16 ` Abel Vesa
2025-01-03 12:58 ` [PATCH v3 2/4] drm/nouveau/dp: Use the generic helper to control LTTPR " Abel Vesa
2025-01-03 12:58 ` [PATCH v3 3/4] drm/i915/dp: " Abel Vesa
2025-01-03 18:09 ` Dmitry Baryshkov
2025-01-06 12:45 ` Abel Vesa
2025-01-08 13:11 ` Dmitry Baryshkov
2025-01-08 13:15 ` Abel Vesa
2025-01-03 12:58 ` [PATCH v3 4/4] drm/msm/dp: Add support for LTTPR handling Abel Vesa
2025-01-03 18:12 ` Dmitry Baryshkov
2025-01-07 9:32 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).