On Wed, 2025-02-12 at 11:35 +0200, Jani Nikula wrote: > > + /* TODO: support video-optimized refresh rate */ > > + if (timings->flags & (1 << 4)) > > + return NULL; > > Mmh. I'm not sure I'd go this far. The bit indicates *two* timings, one > for which the below *is* correct, and another additional one with > vrefresh * (1000/1001). > > We could just add a drm_dbg_kms(dev, "<message>") here about missing > fractional refresh rate, and proceed with the one non-fractional rate? > Or just have the TODO comment with no checks. I'll go with the former, for now. > Either way, > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> Thank you. ... But! > Are you up for the follow-ups too? And since you've got the hang of it, > maybe fix struct displayid_formula_timings_9 hactive/vactive to __be16 > as well? ... at this moment I realised that both the specs and the legacy code actually indicate it's indeed *little*-endian shorts! I.e. `x[0] | x[1] << 8' -- that's LSB-first. Also, virtually no code in `drm_edid.c' uses big-endian. Thus, I'm fixing both my code and `displayid_detailed_timings_1' (I suppose you meant this struct instead) to use __le16. Egor
