On Fri, 20 Nov 2015, "Kahola, Mika" <mika.kahola@xxxxxxxxx> wrote: >> -----Original Message----- >> From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] >> Sent: Friday, November 20, 2015 3:47 PM >> To: Kahola, Mika; intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> Subject: Re: [PATCH] drm/i915: Make DP fast link training a >> module parameter >> >> On Fri, 20 Nov 2015, Mika Kahola <mika.kahola@xxxxxxxxx> wrote: >> > There is a bug report >> > >> > https://bugs.freedesktop.org/show_bug.cgi?id=91393 >> > >> > indicating that there are panels that do not support link training >> > starting with non-zero voltage swing and pre-emphasis values. >> > >> > This patch proposes to make this fast link training feature as one >> > module parameter. To take more conservative approach this feature is >> > disabled by default. >> >> Please let's not add any more of these. If we can't make this work >> automatically (we should still keep trying) I'd say we remove it. >> >> Adding features behind module parameters means that most people will >> never use them. Theoretically it doubles our testing efforts because we >> would have to test with the feature on and off; in practise that won't happen >> and only the default will be tested (because we already have so many >> features behind parameters). The code bitrots and breaks even for the >> platforms on which it did work originally. Then there are a minority of people >> who will enable all possible knobs they read about on forums and think will >> improve performance or power savings or something, and report bugs when >> they fail. >> >> IMO there's just too much effort for too little gain if we can't enable this. I >> just wish we would have been able to block more of the module parameters >> earlier... >> > The idea that I had in mind was to keep this feature as selectable > item until we get this working for all panel combinations. That said, > I consider this would have been only a first aid to the problem. I know. It's the same logic behind roughly all of the feature enable module parameters we have. :( BR, Jani. > > Anyway, back to the drawing board... > > Cheers, > Mika > >> >> BR, >> Jani. >> >> >> > >> > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/i915_drv.h | 1 + >> > drivers/gpu/drm/i915/i915_params.c | 4 ++++ >> > drivers/gpu/drm/i915/intel_dp_link_training.c | 11 ++++++++++- >> > 3 files changed, 15 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h >> > b/drivers/gpu/drm/i915/i915_drv.h index a47e0f4..dc2d5a0 100644 >> > --- a/drivers/gpu/drm/i915/i915_drv.h >> > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > @@ -2669,6 +2669,7 @@ struct i915_params { >> > bool verbose_state_checks; >> > bool nuclear_pageflip; >> > int edp_vswing; >> > + bool enable_dp_flt; >> > }; >> > extern struct i915_params i915 __read_mostly; >> > >> > diff --git a/drivers/gpu/drm/i915/i915_params.c >> > b/drivers/gpu/drm/i915/i915_params.c >> > index 835d609..aea5d47 100644 >> > --- a/drivers/gpu/drm/i915/i915_params.c >> > +++ b/drivers/gpu/drm/i915/i915_params.c >> > @@ -56,6 +56,7 @@ struct i915_params i915 __read_mostly = { >> > .edp_vswing = 0, >> > .enable_guc_submission = false, >> > .guc_log_level = -1, >> > + .enable_dp_flt = false, >> > }; >> > >> > module_param_named(modeset, i915.modeset, int, 0400); @@ -202,3 >> > +203,6 @@ MODULE_PARM_DESC(enable_guc_submission, "Enable GuC >> > submission (default:false)") module_param_named(guc_log_level, >> > i915.guc_log_level, int, 0400); MODULE_PARM_DESC(guc_log_level, >> > "GuC firmware logging level (-1:disabled (default), 0-3:enabled)"); >> > + >> > +module_param_named_unsafe(enable_dp_flt, i915.enable_dp_flt, bool, >> > +0400); MODULE_PARM_DESC(enable_dp_flt, "Enable DP fast link training >> > +(default:false)"); >> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c >> > b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > index 8888793..f8b6d69 100644 >> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c >> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > @@ -85,8 +85,17 @@ static bool >> > intel_dp_reset_link_train(struct intel_dp *intel_dp, >> > uint8_t dp_train_pat) >> > { >> > - if (!intel_dp->train_set_valid) >> > + if (i915.enable_dp_flt) { >> > + DRM_DEBUG_KMS("DP flt enabled, train set valid: %s\n", >> > + intel_dp->train_set_valid ? "true" : "false"); >> > + >> > + if (!intel_dp->train_set_valid) >> > + memset(intel_dp->train_set, 0, sizeof(intel_dp- >> >train_set)); >> > + } else { >> > + DRM_DEBUG_KMS("DP flt disabled\n"); >> > memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set)); >> > + } >> > + >> > intel_dp_set_signal_levels(intel_dp); >> > return intel_dp_set_link_train(intel_dp, dp_train_pat); } >> >> -- >> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
