From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/3] drm/i915: add i915.dp_link_train_policy option Date: Fri, 25 Apr 2014 11:18:59 +0200 Message-ID: <20140425091859.GQ26374@phenom.ffwll.local> References: <1398374579-5697-1-git-send-email-przanoni@gmail.com> <1398374579-5697-3-git-send-email-przanoni@gmail.com> <20140425080650.GM26374@phenom.ffwll.local> <87y4ytvl59.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id D00596EDFC for ; Fri, 25 Apr 2014 02:19:42 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id e53so2626019eek.30 for ; Fri, 25 Apr 2014 02:19:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87y4ytvl59.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Fri, Apr 25, 2014 at 12:00:34PM +0300, Jani Nikula wrote: > On Fri, 25 Apr 2014, Daniel Vetter wrote: > > On Thu, Apr 24, 2014 at 06:22:59PM -0300, Paulo Zanoni wrote: > >> From: Paulo Zanoni > >> > >> We still have way too many bugs with DP link training. We keep > >> switching between "narrow and fast" and "wide and slow", we recently > >> added 5GHz support, and whenever there's a bug report, we have to ask > >> people to apply patches and test them. > >> > >> Wouldn't it be so much better if we could just ask them to boot with > >> some specific Kernel boot option instead of applying a patch? This > >> will move the situation from "i915.ko is completely broken!" to > >> "i915.ko's default values are broken, but there's an option I can set > >> to fix it, so I won't need to learn how to compile a Kernel!". > >> > >> Some useful values: > >> - i915.dp_link_train_policy=1 for "wide and slow" > >> - i915.dp_link_train_policy=0x120 for DP_LINK_BW_2_7 and 2 lanes, > >> which should be able to fit 1920x1080@60Hz and 24bpp > >> - i915.dp_link_train_policy=0x210 to force DP 5GHz testing on > >> not-so-huge modes > >> > >> The default behavior is still the same. > >> > >> Signed-off-by: Paulo Zanoni > > > > Yeah, I like this. I'll sign up Todd to review this all. > > I guess we'll go with this then, but I'll step back from this particular > patch for a bit, and share my concerns over module parameters and > quirks. > > I am generally opposed to adding module parameters or quirks to > workaround issues in features that should just work. They are an easy > way out for things we should root cause and fix properly. > > Do not mistake me for an idealist for thinking this way, as I'm being > pragmatic. > > The people who report bugs to us are roughly the same people who are > capable of setting the module parameter. Once they figure that out, > they'll stop responding to our requests for testing and info. We've seen > this happen before. We'd hurt our chances of making things work out of > the box for the average user. > > The more we add module parameters, the combinations of them > explode. Debugging *other* problems becomes harder. In the bugs I work > on, the #1 request I have is full dmesg, partially because I want to see > all the wild kernel parameters the user might have set. And all too > often they have. When there are module parameters that fix some bugs, > the blogs and forums get filled with tips about them, and people use > them, whether they strictly have the same bug or not. Search for i915 > invert brightness for example. > > It's also not easy to drop module parameters after we've added them. You > know the drill. Even after we've fixed everything the module parameter > was supposed to fix, dropping it leads to https://xkcd.com/1172/. I fully agree with you. I'm working on a patch (only RFC thus far) which allows you to designate some module parameters as debug knobs. As soon as users touch them they'll get - a stern warning in dmesg - TAINT_USER'ed kernel That should be about as good as we can make it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch