On Thu, Mar 27, 2014 at 01:03:31PM +0200, Ville Syrjälä wrote: > On Thu, Mar 27, 2014 at 09:37:36AM +0000, Chris Wilson wrote: > > On Thu, Mar 27, 2014 at 11:08:45AM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > > So relax the checks a bit, and apply the single-link DVI dotclock limit > > > only when filtering the mode list, and ignore the limit when setting > > > a user specified mode. > > > > Mind enlightening me as to how this actually works? I thought all > > display modes were validated before we used them, so how come this > > sneaks through? > > > > So it goes like this: > > > > userspace calls GETCONNECTOR > > kernel: fill_modes -> drm_helper_probe_single_connector_modes -> mode_valid? > > > > but > > > > userspace calls SETCRTC with a random mode > > kernel: applies random mode without validation > > > > Seriously we don't do any checking that the mode given to SETCRTC is > > applicable and not in any way harmful before setting registers? > > Pretty much. Calling our user mode validation even "minimal" is a bit > of a stretch. And the checks we have in the .mode_valid() hooks are > lacking as well. I've had patches floating around which implemented mode_valid in terms of compute_config (or well, mode_adjust as it was called back then). But there are slight differences in semantics so that didn't pan out too well. But yeah, encoders need to share the back-end mode checking functions between mode_valid and compute_config otherwise we'll just let random gunk get through. Checking at the crtc level is better since modes without a valid pll config won't get through. Well, if we'd compute the pll settings a bit earlier ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html