[Intel-gfx] [PATCH 3/3] drm/i915: add i915.dp_link_train_policy option

[Daniel Vetter]

On Fri, Apr 25, 2014 at 12:00:34PM +0300, Jani Nikula wrote:
> On Fri, 25 Apr 2014, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Thu, Apr 24, 2014 at 06:22:59PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >> 
> >> 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 at 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 <paulo.r.zanoni at intel.com>
> >
> > 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