Re: [PATCH 3/4] drm/i915/display: Use intel_de_write_fw in intel_pipe_fastset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing Wakelock / DC State Hardware Hang

Pre-existing issues:
- [High] Sleeping in Atomic Context (Incomplete Fix)
--

commit b928342b55781fe54de15efd0d013221c43c3091
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/i915/display: Use intel_de_write_fw in intel_pipe_fastset

This commit updates multiple display configuration functions to use the
_fw register write variants to bypass DMC wakelock acquisition. This is
intended to fix a sleep-in-atomic-context deadlock on PREEMPT_RT when
modifying display registers during vblank evasion.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5bc8e6ea10a50..f5dac380aa7ff 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
[ ... ]
> @@ -2607,14 +2607,14 @@ void intel_set_m_n(struct intel_display *display,
>  		   intel_reg_t data_m_reg, intel_reg_t data_n_reg,
>  		   intel_reg_t link_m_reg, intel_reg_t link_n_reg)
>  {
> -	intel_de_write(display, data_m_reg, TU_SIZE(m_n->tu) | m_n->data_m);
> -	intel_de_write(display, data_n_reg, m_n->data_n);
> -	intel_de_write(display, link_m_reg, m_n->link_m);
> +	intel_de_write_fw(display, data_m_reg, TU_SIZE(m_n->tu) | m_n->data_m);
> +	intel_de_write_fw(display, data_n_reg, m_n->data_n);
> +	intel_de_write_fw(display, link_m_reg, m_n->link_m);
>  	/*
>  	 * On BDW+ writing LINK_N arms the double buffered update
>  	 * of all the M/N registers, so it must be written last.
>  	 */
> -	intel_de_write(display, link_n_reg, m_n->link_n);
> +	intel_de_write_fw(display, link_n_reg, m_n->link_n);

[Severity: High]
Does using intel_de_write_fw() here risk writing to hardware while it is in a
deep sleep DC state?

When intel_set_m_n() is called asynchronously by intel_drrs_downclock_work()
through intel_drrs_set_refresh_rate_m_n():

drivers/gpu/drm/i915/display/intel_drrs.c:intel_drrs_set_refresh_rate_m_n() {
    ...
    intel_cpu_transcoder_set_m1_n1(crtc, crtc->drrs.cpu_transcoder,
                                   refresh_rate == DRRS_REFRESH_RATE_LOW ?
                                   &crtc->drrs.m2_n2 : &crtc->drrs.m_n);
    ...
}

This async work runs when the display is idle and holds no outer DMC
wakelocks. Bypassing the implicit wakelock acquisition could lead to the
transcoder M/N registers being written without waking up the DMC,
potentially causing dropped writes or hardware hangs.

>  }
>  
>  bool intel_cpu_transcoder_has_m2_n2(struct intel_display *display,
[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index bffbdee76ee1c..5b09a17185b9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
[ ... ]
> @@ -923,8 +923,8 @@ static void intel_vrr_tg_enable(const struct intel_crtc_state *crtc_state,
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 vrr_ctl;
>  
> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> -		       trans_vrr_push(crtc_state, false));
> +	intel_de_write_fw(display, TRANS_PUSH(display, cpu_transcoder),
> +			  trans_vrr_push(crtc_state, false));
>  
>  	vrr_ctl = VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state);
>  

[Severity: High]
This is a pre-existing issue, but while you are converting these functions to
avoid sleeping in atomic context, do intel_vrr_set_transcoder_timings(),
intel_vrr_set_vrr_timings(), and intel_vrr_enable_dc_balancing() also need
to be converted to use the _fw variants?

They are called during the vblank evasion loop (which disables interrupts and
preemption) and still use intel_de_write(), which acquires the DMC wakelock
and uncore spinlock, both of which can sleep on PREEMPT_RT:

intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
               trans_vrr_ctl(crtc_state));

Will leaving these unconverted result in the same sleep-in-atomic-context
deadlocks during display mode sets or fastsets with VRR enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702080346.431508-1-dev@lankhorst.se?part=3