On 05-01-2026 18:25, Jani Nikula wrote:
On Tue, 09 Dec 2025, Dibin Moolakadan
Subrahmanian<[email protected]> wrote:
Introduce dc3co_allow in struct intel_display and determine DC3CO
eligibility during atomic_check(). DC3CO is permitted only when:
- the active pipe drives eDP,
- the pipe is single-pipe (no joiner),
- the pipe/port combination supports DC3CO.
When eligible, intel_atomic_commit_tail() programs the target DC state
as DC_STATE_EN_UPTO_DC3CO; otherwise we fall back to DC6. Update the
PSR vblank enable/disable path to follow the same policy.
Also extend get_allowed_dc_mask() to expose DC3CO support on
DISPLAY_VER >= 35.
Signed-off-by: Dibin Moolakadan
Subrahmanian<[email protected]>
---
drivers/gpu/drm/i915/display/intel_display.c | 75 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_display.h | 1 +
.../gpu/drm/i915/display/intel_display_core.h | 3 +
.../drm/i915/display/intel_display_power.c | 4 +-
drivers/gpu/drm/i915/display/intel_psr.c | 13 ++--
5 files changed, 87 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 9c6d3ecdb589..205f55a87736 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6295,6 +6295,75 @@ static int intel_joiner_add_affected_crtcs(struct
intel_atomic_state *state)
return 0;
}
+bool intel_dc3co_allowed(struct intel_display *display)
+{
+ return display->power.dc3co_allow;
Very few files should touch display->power, and this is not one of them.
'git grep "display->power" -- drivers/gpu/drm/i915/display'
Yes, git grep shows few files , I will try to move all dc3co functions to
drivers/gpu/drm/i915/display/intel_display_power.c.
When is it okay to call this function and expect to get sane results?
display->power.dc3co_allow is only updated in intel_dc3co_allow_check() which
is called from
intel_atomic_commit_tail().intel_dc3co_allowed() only intended to be called
from intel_post_plane_update()
path(ALPM/PSR), which executes as part of intel_atomic_commit_tail().
+}
+
+static bool intel_dc3co_port_pipe_compatible(struct intel_dp *intel_dp,
+ const struct intel_crtc_state
*crtc_state)
+{
+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+ enum pipe pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
+ enum port port = dig_port->base.port;
+ int num_pipes = intel_crtc_num_joined_pipes(crtc_state);
+
+ if (num_pipes != 1)
+ return false;
+
+ if (!(pipe <= PIPE_B && port <= PORT_B))
+ return false;
+
+ return true;
That's a really complicated way to say
return num_pipes == 1 && pipe <= PIPEB && port <= PORT_B;
I will update this.
+}
+
+static void intel_dc3co_allow_check(struct intel_atomic_state *state)
What does "check" mean here? Or in *any* function?
Check sounds like something that's a pure function that doesn't change
anything... but this does.
I will split this function to two , one for check and one for initialization
+{
+ struct intel_display *display = to_intel_display(state);
+ struct intel_crtc *crtc;
+ struct intel_crtc_state *new_crtc_state;
+ struct intel_encoder *encoder;
+ struct intel_dp *intel_dp;
+ int i;
+ struct i915_power_domains *power_domains = &display->power.domains;
+ bool any_active = false;
+ bool allow = true;
+
+ display->power.dc3co_allow = 0;
That's now cached state with no stated rules on when it's valid and when
it's not.
+
+ if ((power_domains->allowed_dc_mask & DC_STATE_EN_UPTO_DC3CO) !=
DC_STATE_EN_UPTO_DC3CO)
+ return;
+
+ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+ if (!new_crtc_state->hw.active)
+ continue;
+
+ any_active = true;
+
+ for_each_intel_encoder_mask(display->drm, encoder,
+ new_crtc_state->uapi.encoder_mask) {
+ /* If any active pipe not eDP disable*/
What?
I will correct comment /* Disallow DC3CO if any active pipe is not eDP */
+ if (!intel_encoder_is_dp(encoder) ||
+ encoder->type != INTEL_OUTPUT_EDP) {
+ allow = false;
+ goto out;
+ }
+ intel_dp = enc_to_intel_dp(encoder);
+ /* Port, joiner, pipe placement checks */
Is that a helpful comment?
+ if (!intel_dc3co_port_pipe_compatible(intel_dp,
new_crtc_state)) {
+ allow = false;
+ goto out;
+ }
+ }
+ }
+
+ if (!any_active)
+ allow = false;
+
+out:
+ display->power.dc3co_allow = allow;
+}
+
intel_display.[ch] is not the dumping ground for random new code. The
goal is to *reduce* the size of it, not increase.
This function needs encoder,port and pipe information, which is why I added
it in intel_display.c.However,I agree it is updating dc3co_allow . I will check
if
it can be moved to intel_display_power.c.
static int intel_atomic_check_config(struct intel_atomic_state *state,
struct intel_link_bw_limits *limits,
enum pipe *failed_pipe)
@@ -6565,6 +6634,8 @@ int intel_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+ intel_dc3co_allow_check(state);
+
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
intel_color_assert_luts(new_crtc_state);
@@ -7601,6 +7672,10 @@ static void intel_atomic_commit_tail(struct
intel_atomic_state *state)
*/
intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore);
}
+ if (intel_dc3co_allowed(display))
+ intel_display_power_set_target_dc_state(display,
DC_STATE_EN_UPTO_DC3CO);
+ else
+ intel_display_power_set_target_dc_state(display,
DC_STATE_EN_UPTO_DC6);
/*
* Delay re-enabling DC states by 17 ms to avoid the off->on->off
* toggling overhead at and above 60 FPS.
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index f8e6e4e82722..97987f082560 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display *display, enum
port port);
bool intel_scanout_needs_vtd_wa(struct intel_display *display);
int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
+bool intel_dc3co_allowed(struct intel_display *display);
#endif
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
b/drivers/gpu/drm/i915/display/intel_display_core.h
index d708d322aa85..fa567c95029c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -538,6 +538,9 @@ struct intel_display {
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
+
+ /* mark dc3co entry is allowed*/
^- space missing
will add space.
+ bool dc3co_allow;
Still unclear when this is valid.
} power;
struct {
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 0961b194554c..e99552f18756 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -956,7 +956,9 @@ static u32 get_allowed_dc_mask(struct intel_display
*display, int enable_dc)
if (!HAS_DISPLAY(display))
return 0;
- if (DISPLAY_VER(display) >= 20)
+ if (DISPLAY_VER(display) >= 35)
+ max_dc = 3;
+ else if (DISPLAY_VER(display) >= 20)
max_dc = 2;
else if (display->platform.dg2)
max_dc = 1;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 753359069044..9c616f449ad6 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3903,14 +3903,11 @@ void intel_psr_notify_vblank_enable_disable(struct
intel_display *display,
return;
}
- /*
- * NOTE: intel_display_power_set_target_dc_state is used
- * only by PSR * code for DC3CO handling. DC3CO target
- * state is currently disabled in * PSR code. If DC3CO
- * is taken into use we need take that into account here
- * as well.
- */
- intel_display_power_set_target_dc_state(display, enable ?
DC_STATE_DISABLE :
+ if (intel_dc3co_allowed(display))
+ intel_display_power_set_target_dc_state(display, enable ?
DC_STATE_DISABLE :
+ DC_STATE_EN_UPTO_DC3CO);
+ else
+ intel_display_power_set_target_dc_state(display, enable ?
DC_STATE_DISABLE :
DC_STATE_EN_UPTO_DC6);
}
