Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[sashiko-bot@kernel.org]

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 <gabriel.petursson@jbtmarel.com>

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/AM9PR06MB78601E2636164181A4689CC281EF2@AM9PR06MB7860.eurprd06.prod.outlook.com?part=1