On 3/25/2024 8:47 PM, Jani Nikula wrote:
On Fri, 22 Mar 2024, Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> wrote:
LSPCON can be configured to LS or PCON mode.
Separate the function to set the expected mode from the lspcon probe
function during lspcon init.
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_lspcon.c | 47 ++++++++++++++-------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index 1d048fa98561..62159d3ead56 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -240,18 +240,40 @@ static bool lspcon_wake_native_aux_ch(struct intel_lspcon *lspcon)
return true;
}
-static bool lspcon_probe(struct intel_lspcon *lspcon)
+static bool lspcon_set_expected_mode(struct intel_lspcon *lspcon)
{
- int retry;
- enum drm_dp_dual_mode_type adaptor_type;
struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- struct i2c_adapter *ddc = &intel_dp->aux.ddc;
enum drm_lspcon_mode expected_mode;
expected_mode = lspcon_wake_native_aux_ch(lspcon) ?
DRM_LSPCON_MODE_PCON : DRM_LSPCON_MODE_LS;
You always need to consider the functional changes when aiming to make
non-functional refactoring. This postpones lspcon_wake_native_aux_ch()
until after the probe. But the name has "wake" in it, and you're no
longer waking up as the first thing. Smells fishy.
Git blame leads to f2b667b658f9 ("drm/i915/lspcon: Ensure AUX CH is
awake while in DP Sleep state"). You should read the commit message.
You are right I indeed missed this part. The lspcon_wake_native_aux_ch
was there for a reason, which I completely overlooked.
Thanks for pointing this out. Will take care of this in next version.
Thanks & Regards,
Ankit
BR,
Jani.
+ lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
+
+ /*
+ * In the SW state machine, lets Put LSPCON in PCON mode only.
+ * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
+ * 2.0 sinks.
+ */
+ if (lspcon->mode != DRM_LSPCON_MODE_PCON) {
+ if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
+ drm_err(&i915->drm, "LSPCON mode change to PCON failed\n");
+ return false;
+ }
+ }
+
+ return true;
+}
+
+static bool lspcon_probe(struct intel_lspcon *lspcon)
+{
+ int retry;
+ enum drm_dp_dual_mode_type adaptor_type;
+ struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ struct i2c_adapter *ddc = &intel_dp->aux.ddc;
+
/* Lets probe the adaptor and check its type */
for (retry = 0; retry < 6; retry++) {
if (retry)
@@ -270,19 +292,7 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
/* Yay ... got a LSPCON device */
drm_dbg_kms(&i915->drm, "LSPCON detected\n");
- lspcon->mode = lspcon_wait_mode(lspcon, expected_mode);
- /*
- * In the SW state machine, lets Put LSPCON in PCON mode only.
- * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
- * 2.0 sinks.
- */
- if (lspcon->mode != DRM_LSPCON_MODE_PCON) {
- if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON) < 0) {
- drm_err(&i915->drm, "LSPCON mode change to PCON failed\n");
- return false;
- }
- }
return true;
}
@@ -671,6 +681,11 @@ bool lspcon_init(struct intel_digital_port *dig_port)
return false;
}
+ if (!lspcon_set_expected_mode(lspcon)) {
+ drm_err(&i915->drm, "LSPCON Set expected Mode failed\n");
+ return false;
+ }
+
if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd) != 0) {
drm_err(&i915->drm, "LSPCON DPCD read failed\n");
return false;
