Re: [Intel-gfx] [PATCH] drm/i915: Allow user modes to exceed DVI 165MHz limit

[Daniel Vetter <daniel@ffwll.ch>]

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@linux.intel.com 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