[v5,3/3] drm/i915: DP channel EQ check for use of DP link training optimization

Submitted by Kahola, Mika on Jan. 4, 2016, 11:21 a.m.

Details

Message ID 1451906484-4659-4-git-send-email-mika.kahola@intel.com
State New
Headers show
Series "drm/i915: Disable link training optimization if DP config has changed" ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Kahola, Mika Jan. 4, 2016, 11:21 a.m.
Don't use DP link training optimization if channel EQ is not ok. It has
been reported that in case of failure in channel EQ check the link training
optimization can be enabled and therefore may not be able to reuse the
previously computed drive current and pre-emphasis levels.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3137187..0cd1ccb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4354,6 +4354,7 @@  intel_dp_check_link_status(struct intel_dp *intel_dp)
 		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
+		intel_dp->train_set_valid = false;
 		intel_dp_start_link_train(intel_dp);
 		intel_dp_stop_link_train(intel_dp);
 	}

Comments

On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> Don't use DP link training optimization if channel EQ is not ok. It has
> been reported that in case of failure in channel EQ check the link training
> optimization can be enabled and therefore may not be able to reuse the
> previously computed drive current and pre-emphasis levels.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3137187..0cd1ccb 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
>  		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>  			      intel_encoder->base.name);
> +		intel_dp->train_set_valid = false;
>  		intel_dp_start_link_train(intel_dp);
>  		intel_dp_stop_link_train(intel_dp);
>  	}

Should do the same for the MST case I suppose?

> -- 
> 1.9.1
On Mon, Jan 04, 2016 at 06:44:09PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > Don't use DP link training optimization if channel EQ is not ok. It has
> > been reported that in case of failure in channel EQ check the link training
> > optimization can be enabled and therefore may not be able to reuse the
> > previously computed drive current and pre-emphasis levels.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3137187..0cd1ccb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >  		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> >  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> >  			      intel_encoder->base.name);
> > +		intel_dp->train_set_valid = false;
> >  		intel_dp_start_link_train(intel_dp);
> >  		intel_dp_stop_link_train(intel_dp);
> >  	}
> 
> Should do the same for the MST case I suppose?

Also I wonder if we shold maintain some sort of blacklist of unstable
vswing/preemph settings so that we wouldn't even try to retrain with
values we know to be of questionable quality...

> 
> > -- 
> > 1.9.1
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2016-01-04 at 18:44 +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > Don't use DP link training optimization if channel EQ is not ok. It has
> > been reported that in case of failure in channel EQ check the link training
> > optimization can be enabled and therefore may not be able to reuse the
> > previously computed drive current and pre-emphasis levels.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3137187..0cd1ccb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> >  		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> >  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> >  			      intel_encoder->base.name);
> > +		intel_dp->train_set_valid = false;
> >  		intel_dp_start_link_train(intel_dp);
> >  		intel_dp_stop_link_train(intel_dp);
> >  	}
> 
> Should do the same for the MST case I suppose?
Yes, we should do the same for MST case.

Cheers,
Mika
> 
> > -- 
> > 1.9.1
>
On Mon, 2016-01-04 at 18:53 +0200, Ville Syrjälä wrote:
> On Mon, Jan 04, 2016 at 06:44:09PM +0200, Ville Syrjälä wrote:
> > On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > > Don't use DP link training optimization if channel EQ is not ok. It has
> > > been reported that in case of failure in channel EQ check the link training
> > > optimization can be enabled and therefore may not be able to reuse the
> > > previously computed drive current and pre-emphasis levels.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 3137187..0cd1ccb 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > >  		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> > >  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > >  			      intel_encoder->base.name);
> > > +		intel_dp->train_set_valid = false;
> > >  		intel_dp_start_link_train(intel_dp);
> > >  		intel_dp_stop_link_train(intel_dp);
> > >  	}
> > 
> > Should do the same for the MST case I suppose?
> 
> Also I wonder if we shold maintain some sort of blacklist of unstable
> vswing/preemph settings so that we wouldn't even try to retrain with
> values we know to be of questionable quality...
I think it would be quite tricky to maintain such a list. There is quite
a bunch of source/sink combinations out there. For eDP this would be
easier if we could first learn which vsing/preemph settings are unusable
and maintain the blacklist based on that.

Cheers,
Mika 
> 
> > 
> > > -- 
> > > 1.9.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
On Tue, Jan 05, 2016 at 01:08:13PM +0200, Mika Kahola wrote:
> On Mon, 2016-01-04 at 18:53 +0200, Ville Syrjälä wrote:
> > On Mon, Jan 04, 2016 at 06:44:09PM +0200, Ville Syrjälä wrote:
> > > On Mon, Jan 04, 2016 at 01:21:24PM +0200, Mika Kahola wrote:
> > > > Don't use DP link training optimization if channel EQ is not ok. It has
> > > > been reported that in case of failure in channel EQ check the link training
> > > > optimization can be enabled and therefore may not be able to reuse the
> > > > previously computed drive current and pre-emphasis levels.
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91393
> > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 3137187..0cd1ccb 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4354,6 +4354,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > > >  		(!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> > > >  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > > >  			      intel_encoder->base.name);
> > > > +		intel_dp->train_set_valid = false;
> > > >  		intel_dp_start_link_train(intel_dp);
> > > >  		intel_dp_stop_link_train(intel_dp);
> > > >  	}
> > > 
> > > Should do the same for the MST case I suppose?
> > 
> > Also I wonder if we shold maintain some sort of blacklist of unstable
> > vswing/preemph settings so that we wouldn't even try to retrain with
> > values we know to be of questionable quality...
> I think it would be quite tricky to maintain such a list. There is quite
> a bunch of source/sink combinations out there. For eDP this would be
> easier if we could first learn which vsing/preemph settings are unusable
> and maintain the blacklist based on that.

That was what I meant, a purely runtime blacklist.

> 
> Cheers,
> Mika 
> > 
> > > 
> > > > -- 
> > > > 1.9.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
>