The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); }
-static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc, - bool phy_is_ready, bool phy_is_owned) +static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc, + bool phy_is_ready, bool phy_is_owned) { struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready); + if (DISPLAY_VER(display) < 20) { + drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned; + return phy_is_ready && phy_is_owned; + } else { + return phy_is_owned; + } }
static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) + if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT; @@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) { + if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
-----Original Message----- From: Intel-xe intel-xe-bounces@lists.freedesktop.org On Behalf Of Imre Deak Sent: Tuesday, 5 August 2025 10.37 To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Cc: stable@vger.kernel.org; Lin, Charlton charlton.lin@intel.com; Almahallawy, Khaled khaled.almahallawy@intel.com Subject: [PATCH 01/19] drm/i915/lnl+/tc: Fix handling of an enabled/disconnected dp-alt sink
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP- alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com
Reviewed-by: Mika Kahola mika.kahola@intel.com
Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); }
-static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned){ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
}
static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT; @@ -1352,7 +1356,7 @@
tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned))
+{ mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT); -- 2.49.1
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)
Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
This is a nitpick, obviously, so I'll leave it up to you.
Regardless:
Reviewed-by: Luca Coelho luciano.coelho@intel.com
-- Cheers, Luca.
{ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
} static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT;
@@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote:
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
The question the function is meant to answer is "Is the PHY in legacy or DP-alt mode?". The return value is true (yes) if the PHY is in either legacy or DP-alt mode, false (no) if the PHY is neither in legacy or DP-alt mode. There are many other uses of "or" in function names in this sense, so not sure how else I'd name this one. Simply leaving out "or" would make it less clear that the legacy and DP-alt modes are two separate modes.
This is a nitpick, obviously, so I'll leave it up to you.
Regardless:
Reviewed-by: Luca Coelho luciano.coelho@intel.com
-- Cheers, Luca.
{ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
} static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT;
@@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote:
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
The question the function is meant to answer is "Is the PHY in legacy or DP-alt mode?". The return value is true (yes) if the PHY is in either legacy or DP-alt mode, false (no) if the PHY is neither in legacy or DP-alt mode. There are many other uses of "or" in function names in this sense, so not sure how else I'd name this one. Simply leaving out "or" would make it less clear that the legacy and DP-alt modes are two separate modes.
What's the opposite of "Is the PHY in legacy or DP-alt mode"?
Would that lead to a simpler name, with the reversed return value?
BR, Jani.
This is a nitpick, obviously, so I'll leave it up to you.
Regardless:
Reviewed-by: Luca Coelho luciano.coelho@intel.com
-- Cheers, Luca.
{ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
} static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT;
@@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
On Thu, Aug 07, 2025 at 03:19:17PM +0300, Jani Nikula wrote:
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote:
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
The question the function is meant to answer is "Is the PHY in legacy or DP-alt mode?". The return value is true (yes) if the PHY is in either legacy or DP-alt mode, false (no) if the PHY is neither in legacy or DP-alt mode. There are many other uses of "or" in function names in this sense, so not sure how else I'd name this one. Simply leaving out "or" would make it less clear that the legacy and DP-alt modes are two separate modes.
What's the opposite of "Is the PHY in legacy or DP-alt mode"?
Would that lead to a simpler name, with the reversed return value?
The opposite is either TBT-alt or disconnected mode, so the reversal would result in the same function name format.
BR, Jani.
This is a nitpick, obviously, so I'll leave it up to you.
Regardless:
Reviewed-by: Luca Coelho luciano.coelho@intel.com
-- Cheers, Luca.
{ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
} static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT;
@@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
-- Jani Nikula, Intel
On Thu, Aug 07, 2025 at 03:33:04PM +0300, Imre Deak wrote:
On Thu, Aug 07, 2025 at 03:19:17PM +0300, Jani Nikula wrote:
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote:
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
The question the function is meant to answer is "Is the PHY in legacy or DP-alt mode?". The return value is true (yes) if the PHY is in either legacy or DP-alt mode, false (no) if the PHY is neither in legacy or DP-alt mode. There are many other uses of "or" in function names in this sense, so not sure how else I'd name this one. Simply leaving out "or" would make it less clear that the legacy and DP-alt modes are two separate modes.
What's the opposite of "Is the PHY in legacy or DP-alt mode"?
Would that lead to a simpler name, with the reversed return value?
The opposite is either TBT-alt or disconnected mode, so the reversal would result in the same function name format.
Would you be ok with
tc_phy_owned_by_display()
?
BR, Jani.
This is a nitpick, obviously, so I'll leave it up to you.
Regardless:
Reviewed-by: Luca Coelho luciano.coelho@intel.com
-- Cheers, Luca.
{ struct intel_display *display = to_intel_display(tc->dig_port);
- drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- if (DISPLAY_VER(display) < 20) {
drm_WARN_ON(display->drm, phy_is_owned && !phy_is_ready);
- return phy_is_ready && phy_is_owned;
return phy_is_ready && phy_is_owned;- } else {
return phy_is_owned;- }
} static bool tc_phy_is_connected(struct intel_tc_port *tc, @@ -1244,7 +1248,7 @@ static bool tc_phy_is_connected(struct intel_tc_port *tc, bool phy_is_owned = tc_phy_is_owned(tc); bool is_connected;
- if (tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned))
- if (tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) is_connected = port_pll_type == ICL_PORT_DPLL_MG_PHY; else is_connected = port_pll_type == ICL_PORT_DPLL_DEFAULT;
@@ -1352,7 +1356,7 @@ tc_phy_get_current_mode(struct intel_tc_port *tc) phy_is_ready = tc_phy_is_ready(tc); phy_is_owned = tc_phy_is_owned(tc);
- if (!tc_phy_is_ready_and_owned(tc, phy_is_ready, phy_is_owned)) {
- if (!tc_phy_in_legacy_or_dp_alt_mode(tc, phy_is_ready, phy_is_owned)) { mode = get_tc_mode_in_phy_not_owned_state(tc, live_mode); } else { drm_WARN_ON(display->drm, live_mode == TC_PORT_TBT_ALT);
-- Jani Nikula, Intel
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
Would you be ok with
tc_phy_owned_by_display()
Sounds good to me. Maybe add a brief comment "Is the PHY in legacy or DP-alt mode" or something above the function.
BR, Jani.
On Thu, Aug 07, 2025 at 04:05:14PM +0300, Jani Nikula wrote:
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
Would you be ok with
tc_phy_owned_by_display()
Sounds good to me. Maybe add a brief comment "Is the PHY in legacy or DP-alt mode" or something above the function.
Ok.
BR, Jani.
-- Jani Nikula, Intel
On Thu, 2025-08-07 at 15:50 +0300, Imre Deak wrote:
On Thu, Aug 07, 2025 at 03:33:04PM +0300, Imre Deak wrote:
On Thu, Aug 07, 2025 at 03:19:17PM +0300, Jani Nikula wrote:
On Thu, 07 Aug 2025, Imre Deak imre.deak@intel.com wrote:
On Thu, Aug 07, 2025 at 01:59:21PM +0300, Luca Coelho wrote:
On Tue, 2025-08-05 at 10:36 +0300, Imre Deak wrote:
The TypeC PHY HW readout during driver loading and system resume determines which TypeC mode the PHY is in (legacy/DP-alt/TBT-alt) and whether the PHY is connected, based on the PHY's Owned and Ready flags. For the PHY to be in DP-alt or legacy mode and for the PHY to be in the connected state in these modes, both the Owned (set by the BIOS/driver) and the Ready (set by the HW) flags should be set.
On ICL-MTL the HW kept the PHY's Ready flag set after the driver connected the PHY by acquiring the PHY ownership (by setting the Owned flag), until the driver disconnected the PHY by releasing the PHY ownership (by clearing the Owned flag). On LNL+ this has changed, in that the HW clears the Ready flag as soon as the sink gets disconnected, even if the PHY ownership was acquired already and hence the PHY is being used by the display.
When inheriting the HW state from BIOS for a PHY connected in DP-alt mode on which the sink got disconnected - i.e. in a case where the sink was connected while BIOS/GOP was running and so the sink got enabled connecting the PHY, but the user disconnected the sink by the time the driver loaded - the PHY Owned but not Ready state must be accounted for on LNL+ according to the above. Do that by assuming on LNL+ that the PHY is connected in DP-alt mode whenever the PHY Owned flag is set, regardless of the PHY Ready flag.
This fixes a problem on LNL+, where the PHY TypeC mode / connected state was detected incorrectly for a DP-alt sink, which got connected and then disconnected by the user in the above way.
Cc: stable@vger.kernel.org # v6.8+ Reported-by: Charlton Lin charlton.lin@intel.com Tested-by: Khaled Almahallawy khaled.almahallawy@intel.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/i915/display/intel_tc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index 3bc57579fe53e..73a08bd84a70a 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -1226,14 +1226,18 @@ static void tc_phy_get_hw_state(struct intel_tc_port *tc) tc->phy_ops->get_hw_state(tc); } -static bool tc_phy_is_ready_and_owned(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)+static bool tc_phy_in_legacy_or_dp_alt_mode(struct intel_tc_port *tc,
bool phy_is_ready, bool phy_is_owned)Personally I don't like the "or" in the function name. You're returning a boolean which is true or false. The return value is akin to answering "Yes/No" to the question "Is it black or white".
The question the function is meant to answer is "Is the PHY in legacy or DP-alt mode?". The return value is true (yes) if the PHY is in either legacy or DP-alt mode, false (no) if the PHY is neither in legacy or DP-alt mode. There are many other uses of "or" in function names in this sense, so not sure how else I'd name this one. Simply leaving out "or" would make it less clear that the legacy and DP-alt modes are two separate modes.
Right, I missed that. But one shouldn't have to go read the function implementation to know what it's doing.
What's the opposite of "Is the PHY in legacy or DP-alt mode"?
Would that lead to a simpler name, with the reversed return value?
The opposite is either TBT-alt or disconnected mode, so the reversal would result in the same function name format.
Would you be ok with
tc_phy_owned_by_display()
?
This sounds much better! And it's indeed what you're looking for. It doesn't matter if it's legacy DP-alt, TBT-alt or whatever. What you're checking is actually whether display owns the PHY. So this is a much better choice IMHO. :)
-- Cheers, Luca.
linux-stable-mirror@lists.linaro.org
-
Imre Deak -
Jani Nikula -
Kahola, Mika -
Luca Coelho