[Intel-gfx] [PATCH] drm/i915: Make DP fast link training a module parameter

[Kahola, Mika]

> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
> Sent: Friday, November 20, 2015 3:47 PM
> To: Kahola, Mika; intel-gfx at 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 at 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 at 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