On Tue, 9 Jun 2026, Luca Coelho wrote:
PHY_NONE is not really used, but we define it and, thus, need to check
for it in a few places we use phy. The only potential places where
double space: s/ / /
phy may become PHY_NONE, is in intel_port_to_phy(), where it derives
from port, which can be PORT_NONE. Many of its callers don't check
double space: s/ / /
for PHY_NONE, which can cause unknown behavior. Additionally, this
double space: s/ / /
can only happen if the encoder used has PORT_NONE, which should not be
the case either, without unexpected consequences.
Remove the PHY_NONE definition entirely and add a couple of WARNs at
the relevant places, just to be sure.
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++----
drivers/gpu/drm/i915/display/intel_display.h | 2 --
.../gpu/drm/i915/display/intel_display_power_well.c | 6 +++++-
drivers/gpu/drm/i915/display/intel_hti.c | 3 ---
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index bdf02b67c1d8..58713ad04d37 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1810,9 +1810,7 @@ static void hsw_crtc_disable(struct intel_atomic_state
*state,
/* Prefer intel_encoder_is_combo() */
bool intel_phy_is_combo(struct intel_display *display, enum phy phy)
{
- if (phy == PHY_NONE)
- return false;
- else if (display->platform.alderlake_s)
+ if (display->platform.alderlake_s)
return phy <= PHY_E;
else if (display->platform.dg1 || display->platform.rocketlake)
return phy <= PHY_D;
@@ -1866,7 +1864,7 @@ bool intel_phy_is_snps(struct intel_display *display,
enum phy phy)
* For DG2, and for DG2 only, all four "combo" ports and the TC1 port
* (PHY E) use Synopsis PHYs. See intel_phy_is_tc().
*/
- return display->platform.dg2 && phy > PHY_NONE && phy <= PHY_E;
+ return display->platform.dg2 && phy <= PHY_E;
}
/* Prefer intel_encoder_to_phy() */
@@ -1884,6 +1882,10 @@ enum phy intel_port_to_phy(struct intel_display
*display, enum port port)
port == PORT_D)
return PHY_A;
+ if (drm_WARN(display->drm, port < 0,
+ "PHY is invalid if port < 0 (%d), assuming PHY_A\n", port))
+ return PHY_A;
+
return PHY_A + port - PORT_A;
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 98b589e8360d..a4f621934b33 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -136,8 +136,6 @@ enum tc_port {
};
enum phy {
- PHY_NONE = -1,
-
PHY_A = 0,
I'm wondering if with this change we could remove the assignment above
(PHY_A = 0), since I think enums are starting from 0 by default; but
again I might be missing common codestyle guidelines. Anyways:
Reviewed-by: Michał Grzelak <[email protected]>
BR,
Michał
PHY_B,
PHY_C,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c
b/drivers/gpu/drm/i915/display/intel_display_power_well.c
index 04bd0dde5bed..daea2452a19e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -325,7 +325,11 @@ static enum phy icl_aux_pw_to_phy(struct intel_display
*display,
{
struct intel_encoder *encoder = icl_aux_pw_to_encoder(display,
power_well);
- return encoder ? intel_encoder_to_phy(encoder) : PHY_NONE;
+ if (drm_WARN(display->drm, !encoder,
+ "PHY is invalid if encoder is NULL, assuming PHY_A\n"))
+ return PHY_A;
+
+ return intel_encoder_to_phy(encoder);
}
static bool icl_aux_pw_is_tc_phy(struct intel_display *display,
diff --git a/drivers/gpu/drm/i915/display/intel_hti.c
b/drivers/gpu/drm/i915/display/intel_hti.c
index dc454420c134..56602240ceff 100644
--- a/drivers/gpu/drm/i915/display/intel_hti.c
+++ b/drivers/gpu/drm/i915/display/intel_hti.c
@@ -23,9 +23,6 @@ void intel_hti_init(struct intel_display *display)
bool intel_hti_uses_phy(struct intel_display *display, enum phy phy)
{
- if (drm_WARN_ON(display->drm, phy == PHY_NONE))
- return false;
-
return display->hti.state & HDPORT_ENABLED &&
display->hti.state & HDPORT_DDI_USED(phy);
}
