On 06-05-2026 19:05, Imre Deak wrote:
On Wed, May 06, 2026 at 06:33:21PM +0530, Dibin Moolakadan Subrahmanian wrote:
gen9_write_dc_state() verifies DC_STATE_EN by reading it back, but it
was comparing the full register value instead of only the DC state bits.
That could trigger false failure messages and unnecessary retries when
unrelated bits differed.
Use intel_de_rmw() to update only the DC state bits and compare only
the masked DC state bits in the read-back check and retry logic.
BSpec: 49437,69115
Signed-off-by: Dibin Moolakadan Subrahmanian
<[email protected]>
---
.../i915/display/intel_display_power_well.c | 21 ++++++++-----------
1 file changed, 9 insertions(+), 12 deletions(-)
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 6fbfd46461b0..75471898e323 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -727,13 +727,13 @@ static void assert_can_disable_dc9(struct intel_display
*display)
}
static void gen9_write_dc_state(struct intel_display *display,
- u32 state)
+ u32 state, u32 mask)
{
int rewrites = 0;
int rereads = 0;
u32 v;
- intel_de_write(display, DC_STATE_EN, state);
+ intel_de_rmw(display, DC_STATE_EN, mask, state);
There is no need to change this to an RMW, since gen9_set_dc_state()
computed the state passed to this function by the equivalent
(intel_de_read(display, DC_STATE_EN) & ~mask) | state
/* It has been observed that disabling the dc6 state sometimes
* doesn't stick and dmc keeps returning old value. Make sure
@@ -742,9 +742,8 @@ static void gen9_write_dc_state(struct intel_display
*display,
*/
do {
v = intel_de_read(display, DC_STATE_EN);
-
- if (v != state) {
- intel_de_write(display, DC_STATE_EN, state);
+ if ((v & mask) != (state & mask)) {
+ intel_de_rmw(display, DC_STATE_EN, mask, state);
Could you provide the flags in the register causing an unexpected
mismatch? I can only see bits that should preserve their state as
written by the driver. The register has also some clear-on-write flags,
like 'Display DC*CO State Status DSI', but not sure how even those can
lead to a mismatch.
Thanks for the review.
I can see some RO bits status getting changed while read back.
In one case, I can see register read back value as 0x400 while
writing DC state 0.
In this case I can see DC state is 0,but other bit have changed.
rewrites++;
rereads = 0;
} else if (rereads++ > 5) {
@@ -753,16 +752,16 @@ static void gen9_write_dc_state(struct intel_display
*display,
} while (rewrites < 100);
- if (v != state)
+ if ((v & mask) != (state & mask))
drm_err(display->drm,
"Writing dc state to 0x%x failed, now 0x%x\n",
- state, v);
+ state & mask, v & mask);
/* Most of the times we need one retry, avoid spam */
if (rewrites > 1)
drm_dbg_kms(display->drm,
"Rewrote dc state to 0x%x %d times\n",
- state, rewrites);
+ state & mask, rewrites);
}
static u32 gen9_dc_mask(struct intel_display *display)
@@ -855,15 +854,13 @@ void gen9_set_dc_state(struct intel_display *display, u32
state)
if (!dc6_was_enabled && enable_dc6)
intel_dmc_update_dc6_allowed_count(display, true);
- val &= ~mask;
- val |= state;
- gen9_write_dc_state(display, val);
+ gen9_write_dc_state(display, state, mask);
if (!enable_dc6 && dc6_was_enabled)
intel_dmc_update_dc6_allowed_count(display, false);
- power_domains->dc_state = val & mask;
+ power_domains->dc_state = state & mask;
}
static void tgl_enable_dc3co(struct intel_display *display)
--
2.43.0
