Re: [PATCH 3/3] drm/i915: add i915.dp_link_train_policy option

[Jani Nikula <jani.nikula@linux.intel.com>]

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

Ah yes, I remember the patch, that's the one thing I was going to ask
about before my colleagues insisted on leaving for lunch... What's the
status and can we get that in first?

BR,
Jani.


>
> 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

-- 
Jani Nikula, Intel Open Source Technology Center