* [PATCH] drm/i915: Make DP fast link training a module parameter @ 2015-11-20 13:25 Mika Kahola 2015-11-20 13:47 ` Jani Nikula 0 siblings, 1 reply; 4+ messages in thread From: Mika Kahola @ 2015-11-20 13:25 UTC (permalink / raw) To: intel-gfx 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. Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- 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); } -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Make DP fast link training a module parameter 2015-11-20 13:25 [PATCH] drm/i915: Make DP fast link training a module parameter Mika Kahola @ 2015-11-20 13:47 ` Jani Nikula 2015-11-20 13:52 ` Kahola, Mika 0 siblings, 1 reply; 4+ messages in thread From: Jani Nikula @ 2015-11-20 13:47 UTC (permalink / raw) To: Mika Kahola, intel-gfx On Fri, 20 Nov 2015, Mika Kahola <mika.kahola@intel.com> 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... BR, Jani. > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Make DP fast link training a module parameter 2015-11-20 13:47 ` Jani Nikula @ 2015-11-20 13:52 ` Kahola, Mika 2015-11-20 14:07 ` Jani Nikula 0 siblings, 1 reply; 4+ messages in thread From: Kahola, Mika @ 2015-11-20 13:52 UTC (permalink / raw) To: Jani Nikula, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Friday, November 20, 2015 3:47 PM > To: Kahola, Mika; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Make DP fast link training a > module parameter > > On Fri, 20 Nov 2015, Mika Kahola <mika.kahola@intel.com> 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. Anyway, back to the drawing board... Cheers, Mika > > BR, > Jani. > > > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > --- > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Make DP fast link training a module parameter 2015-11-20 13:52 ` Kahola, Mika @ 2015-11-20 14:07 ` Jani Nikula 0 siblings, 0 replies; 4+ messages in thread From: Jani Nikula @ 2015-11-20 14:07 UTC (permalink / raw) To: Kahola, Mika, intel-gfx@lists.freedesktop.org On Fri, 20 Nov 2015, "Kahola, Mika" <mika.kahola@intel.com> wrote: >> -----Original Message----- >> From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >> Sent: Friday, November 20, 2015 3:47 PM >> To: Kahola, Mika; intel-gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Make DP fast link training a >> module parameter >> >> On Fri, 20 Nov 2015, Mika Kahola <mika.kahola@intel.com> 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@intel.com> >> > --- >> > 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-20 14:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-20 13:25 [PATCH] drm/i915: Make DP fast link training a module parameter Mika Kahola 2015-11-20 13:47 ` Jani Nikula 2015-11-20 13:52 ` Kahola, Mika 2015-11-20 14:07 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox