Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Modifying global hardware state (`display->cdclk.hw.sanitized`) within 
the stateless atomic check phase violates DRM atomic semantics and breaks 
`TEST_ONLY` commits.
--

commit 8f701efc4d5758e65427e1e613e7e204861dc6ef
Author: Gabríel Arthúr Pétursson <[email protected]>

drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

This commit explicitly triggers CDCLK recalculation after sanitization during
modesetting. It addresses an issue where monitors display a blank screen at
boot accompanied by CPU pipe A FIFO underrun errors.

> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c 
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 189ae2d3cfc9e..38b384d825d10 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c

[ ... ]

> @@ -3596,9 +3599,10 @@ static int intel_modeset_calc_cdclk(struct 
> intel_atomic_state *state)
>  
>  int intel_cdclk_atomic_check(struct intel_atomic_state *state)
>  {
> +     struct intel_display *display = to_intel_display(state);
>       const struct intel_cdclk_state *old_cdclk_state;
>       struct intel_cdclk_state *new_cdclk_state;
> -     bool need_cdclk_calc = false;
> +     bool need_cdclk_calc = display->cdclk.hw.sanitized;

[Severity: High]
Does reading this global hardware state here lack protection against concurrent
atomic checks?

>       int ret;
>  
>       ret = intel_cdclk_modeset_checks(state, &need_cdclk_calc);
> @@ -3629,6 +3633,8 @@ int intel_cdclk_atomic_check(struct intel_atomic_state 
> *state)
>               ret = intel_modeset_calc_cdclk(state);
>               if (ret)
>                       return ret;
> +
> +             display->cdclk.hw.sanitized = false;

[Severity: High]
Does modifying global hardware state within the stateless atomic check phase
violate DRM atomic semantics and break TEST_ONLY commits?

If a userspace Wayland compositor performs a TEST_ONLY atomic commit before a
real commit, it will trigger intel_cdclk_atomic_check() and clear this
sanitized flag. The subsequent real commit will then see sanitized == false
and fail to recalculate CDCLK, potentially resulting in the exact CPU pipe
FIFO underrun and blank screen issues this patch intended to fix.

>       }
>  
>       return 0;

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/am9pr06mb78601e2636164181a4689cc281...@am9pr06mb7860.eurprd06.prod.outlook.com?part=1

Reply via email to