Re: [PATCH] drm/mgag200: reject pixel clocks outside PLL range

[Thomas Zimmermann]

Hi

Am 23.05.26 um 15:58 schrieb Berkant Koc:
> > The other models never did this check. Why is this now a problem?
> You are right, and my Fixes tag is wrong. I checked the split again.
> 877507bb954e moved the per-model compute routines into separate files; it
> did not remove a check. Only g200se ever had the permitteddelta guard (the
> 0.5% VESA comment lives only there). The other six helpers never had it,
> before or after the split. So this is not a regression. Please drop the
> Fixes tag and the stable Cc; I should not have framed it that way.
>
> What remains is a latent robustness gap, not a bug users hit.
> mgag200_crtc_helper_mode_valid does not bound the pixel clock, so a
> hand-crafted out-of-range modeline can reach the search loop, match
> nothing, and leave m/n/p/s at 0. atomic_update then does pixpllc->m - 1,
> which underflows, and programs garbage into PIXPLLC. Normal EDID modes stay
> inside the VCO window and never trigger this.
>
> If you think hardening the six helpers to match g200se is still worth it, I
> will resend without the Fixes/stable tags. Do you prefer a per-helper guard
> like g200se, or one shared check after the loop? I will follow your call on
> scope.

Adding that test could possibly regress someone's system. If they run 
the display at a higher delta than 5%, it might still work.

Before rejecting those modes, there needs to be a check in the CRTC's 
mode_valid [1] against the limits of PIXPLLC, similar to 
pixpllc_atomic_check. That would filter out display modes that are not 
supported anyway.

https://elixir.bootlin.com/linux/v7.0.10/source/drivers/gpu/drm/mgag200/mgag200_mode.c#L581

Best regards
Thomas

> Thanks for catching the framing.
>
> Berkant Koc

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)