From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH] drm/i915: Allow user modes to exceed DVI 165MHz limit Date: Thu, 27 Mar 2014 14:57:46 +0100 Message-ID: <20140327135746.GC26878@phenom.ffwll.local> References: <1395911325-4981-1-git-send-email-ville.syrjala@linux.intel.com> <20140327093736.GA30820@nuc-i3427.alporthouse.com> <20140327110331.GQ21652@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <20140327110331.GQ21652@intel.com> Sender: stable-owner@vger.kernel.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Chris Wilson , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Mar 27, 2014 at 01:03:31PM +0200, Ville Syrj=E4l=E4 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= =2Ecom 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 sett= ing > > > a user specified mode. > >=20 > > 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? > >=20 > > So it goes like this: > >=20 > > userspace calls GETCONNECTOR > > kernel: fill_modes -> drm_helper_probe_single_connector_modes -> mo= de_valid? > >=20 > > but > >=20 > > userspace calls SETCRTC with a random mode > > kernel: applies random mode without validation > >=20 > > Seriously we don't do any checking that the mode given to SETCRTC i= s > > applicable and not in any way harmful before setting registers? >=20 > 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 we= ll. But yeah, encoders need to share the back-end mode checking functions between mode_valid and compute_config otherwise we'll just let random g= unk 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 --=20 Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch