Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-27 Thread Sivakumar Thulasimani 


On 2/27/2015 1:14 PM, Jani Nikula wrote:

On Fri, 27 Feb 2015, Todd Previte tprev...@gmail.com wrote:

Hi Mika,

On 2/26/2015 2:26 AM, Mika Kahola wrote:

In a case when DP link has been once trained we can reuse
the existing link training parameters i.e. voltage swing
and pre-emphasis levels from cache when there is a need to
restart link training. In a case of eDP we initially try
to train the link by using the parameter set that we can find
from VBT. The fallback on both cases is to reset the link
training parameters and restart.

I think you have a good idea here for the eDP case. But the way it's
coded, there appear to be cases where the code will try to reuse the
same training settings for regular DP as well. That will likely have
some unfortunate results, as there's no guarantee for a normal external
Displayport connection that training settings which worked once will
work the next time. This would be of particular concern when
disconnecting and reconnecting Displayport sink devices, as it appears
as though it would try to retrain the newly connected device using the
previously valid settings for the old device.

Hmm, I thought it would be all right on external DP also if we ensure we
reset the train set when the sink may have changed, i.e. at suspend and
hotplug.

BR,
Jani.
Though VBT fields for Fast Link Training has been available for a long 
time it was not always filled and there was no way to know if the values 
in VBT are valid or not. Hence it is more than likely to be entering the 
failing condition. There was a field added in BDW timeline to say if the 
VBT fields for FLT values is valid or not, so please use them only after 
such a check.


regards,
Sivakumar



IMHO, this really needs to be written as eDP only and such that it can
never occur on an external Displayport device. Otherwise, it seems like
there's a good chance of ending up with a black screen when you plug in
a Displayport monitor.


Signed-off-by: Mika Kahola mika.kah...@intel.com
---
   drivers/gpu/drm/i915/intel_dp.c  | 93 
+++-
   drivers/gpu/drm/i915/intel_drv.h |  1 +
   2 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d1141d3..8482f58 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3288,8 +3288,39 @@ static bool
   intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
uint8_t dp_train_pat)
   {
-   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int i;
+
+   /* in case of eDP, get link training parameters from VBT */
+   if (is_edp(intel_dp)) {
+   for (i=0; iintel_dp-lane_count; i++)
+   intel_dp-train_set[i] =  dev_priv-vbt.edp_vswing | 
dev_priv-vbt.edp_preemphasis;
+   }
+   else
+   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+
+   intel_dp_set_signal_levels(intel_dp, DP);
+   return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
+}
+
+static bool
+intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+ uint8_t dp_train_pat)
+{
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
intel_dp_set_signal_levels(intel_dp, DP);
+
+   I915_WRITE(intel_dp-output_reg, *DP);
+   POSTING_READ(intel_dp-output_reg);
+
+   drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
+ intel_dp-train_set, intel_dp-lane_count);
+
return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
   }
   

I'm not sure you want to be updating the sink device at this point. This
should be checked against the spec to be sure it's not out of sequence.


@@ -3356,6 +3387,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
int voltage_tries, loop_tries;
uint32_t DP = intel_dp-DP;
uint8_t link_config[2];
+   uint8_t link_status[DP_LINK_STATUS_SIZE];
+   bool reuse_train_set = false;
   
   	if (HAS_DDI(dev))

intel_ddi_prepare_link_retrain(encoder);
@@ -3373,20 +3406,44 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
   
   	DP |= DP_PORT_EN;
   
-	/* clock recovery */

-   if (!intel_dp_reset_link_train(intel_dp, DP,
-  DP_TRAINING_PATTERN_1 |
-  DP_LINK_SCRAMBLING_DISABLE)) {
-   DRM_ERROR(failed to enable link training\n);
-   return;
+   /*
+* check if DP has already trained. Reset voltage swing and
+* pre-emphasis levels if

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Jani Nikula 
On Fri, 27 Feb 2015, Todd Previte tprev...@gmail.com wrote:
 Hi Mika,

 On 2/26/2015 2:26 AM, Mika Kahola wrote:
 In a case when DP link has been once trained we can reuse
 the existing link training parameters i.e. voltage swing
 and pre-emphasis levels from cache when there is a need to
 restart link training. In a case of eDP we initially try
 to train the link by using the parameter set that we can find
 from VBT. The fallback on both cases is to reset the link
 training parameters and restart.
 I think you have a good idea here for the eDP case. But the way it's 
 coded, there appear to be cases where the code will try to reuse the 
 same training settings for regular DP as well. That will likely have 
 some unfortunate results, as there's no guarantee for a normal external 
 Displayport connection that training settings which worked once will 
 work the next time. This would be of particular concern when 
 disconnecting and reconnecting Displayport sink devices, as it appears 
 as though it would try to retrain the newly connected device using the 
 previously valid settings for the old device.

Hmm, I thought it would be all right on external DP also if we ensure we
reset the train set when the sink may have changed, i.e. at suspend and
hotplug.

BR,
Jani.


 IMHO, this really needs to be written as eDP only and such that it can 
 never occur on an external Displayport device. Otherwise, it seems like 
 there's a good chance of ending up with a black screen when you plug in 
 a Displayport monitor.

 Signed-off-by: Mika Kahola mika.kah...@intel.com
 ---
   drivers/gpu/drm/i915/intel_dp.c  | 93 
 +++-
   drivers/gpu/drm/i915/intel_drv.h |  1 +
   2 files changed, 84 insertions(+), 10 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c 
 b/drivers/gpu/drm/i915/intel_dp.c
 index d1141d3..8482f58 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3288,8 +3288,39 @@ static bool
   intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
  uint8_t dp_train_pat)
   {
 -memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
 +struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 +struct drm_device *dev = intel_dig_port-base.base.dev;
 +struct drm_i915_private *dev_priv = dev-dev_private;
 +int i;
 +
 +/* in case of eDP, get link training parameters from VBT */
 +if (is_edp(intel_dp)) {
 +for (i=0; iintel_dp-lane_count; i++)
 +intel_dp-train_set[i] =  dev_priv-vbt.edp_vswing | 
 dev_priv-vbt.edp_preemphasis;
 +}
 +else
 +memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
 +
 +intel_dp_set_signal_levels(intel_dp, DP);
 +return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
 +}
 +
 +static bool
 +intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
 +  uint8_t dp_train_pat)
 +{
 +struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 +struct drm_device *dev = intel_dig_port-base.base.dev;
 +struct drm_i915_private *dev_priv = dev-dev_private;
 +
  intel_dp_set_signal_levels(intel_dp, DP);
 +
 +I915_WRITE(intel_dp-output_reg, *DP);
 +POSTING_READ(intel_dp-output_reg);
 +
 +drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
 +  intel_dp-train_set, intel_dp-lane_count);
 +
  return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
   }
   
 I'm not sure you want to be updating the sink device at this point. This 
 should be checked against the spec to be sure it's not out of sequence.

 @@ -3356,6 +3387,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
  int voltage_tries, loop_tries;
  uint32_t DP = intel_dp-DP;
  uint8_t link_config[2];
 +uint8_t link_status[DP_LINK_STATUS_SIZE];
 +bool reuse_train_set = false;
   
  if (HAS_DDI(dev))
  intel_ddi_prepare_link_retrain(encoder);
 @@ -3373,20 +3406,44 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
   
  DP |= DP_PORT_EN;
   
 -/* clock recovery */
 -if (!intel_dp_reset_link_train(intel_dp, DP,
 -   DP_TRAINING_PATTERN_1 |
 -   DP_LINK_SCRAMBLING_DISABLE)) {
 -DRM_ERROR(failed to enable link training\n);
 -return;
 +/*
 + * check if DP has already trained. Reset voltage swing and
 + * pre-emphasis levels if that's not the case.
 + */
 +if (intel_dp-link_trained) {
 +if (!intel_dp_reuse_link_train(intel_dp, DP,
 +   DP_TRAINING_PATTERN_1 |
 +   DP_LINK_SCRAMBLING_DISABLE)) {
 +DRM_DEBUG_KMS(unable to set known link training 
 values\n);
 +return;
 +}
 +DRM_DEBUG_KMS(reuse current link train set\n);
 +

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Ville Syrjälä 
On Thu, Feb 26, 2015 at 06:29:45PM -0700, Todd Previte wrote:
 Hi Mika,
 
 On 2/26/2015 2:26 AM, Mika Kahola wrote:
  In a case when DP link has been once trained we can reuse
  the existing link training parameters i.e. voltage swing
  and pre-emphasis levels from cache when there is a need to
  restart link training. In a case of eDP we initially try
  to train the link by using the parameter set that we can find
  from VBT. The fallback on both cases is to reset the link
  training parameters and restart.
 I think you have a good idea here for the eDP case. But the way it's 
 coded, there appear to be cases where the code will try to reuse the 
 same training settings for regular DP as well. That will likely have 
 some unfortunate results, as there's no guarantee for a normal external 
 Displayport connection that training settings which worked once will 
 work the next time. This would be of particular concern when 
 disconnecting and reconnecting Displayport sink devices, as it appears 
 as though it would try to retrain the newly connected device using the 
 previously valid settings for the old device.
 
 IMHO, this really needs to be written as eDP only and such that it can 
 never occur on an external Displayport device. Otherwise, it seems like 
 there's a good chance of ending up with a black screen when you plug in 
 a Displayport monitor.

I've thought we should retrain with the same parameters we used the
last time iff the link was succesully trained using those parameters
in the past and the sink hasn't been disconnected in the meantime.
We'd do this in the hopes of speeding up subsequent link trainings.
So resetting the saved parameters on link training failure and long
HPD should be good enough here, I think.

And this way the VBT thing could just be a special case where we just
sort of pretend that we succesfully trained the link using those
parameters in the past. Oh and we could also read out the state from
the hardware during init, and assuming the link was up and stable at
the time we could populate the previous good values from whatever
the BIOS actually programmed into the hardware.

Anyway, that's just how I've imagined we should implement this kind of
stuff.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Chris Wilson 
On Thu, Feb 26, 2015 at 02:19:51PM +, Kahola, Mika wrote:
  -Original Message-
  From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
  Sent: Thursday, February 26, 2015 12:51 PM
  To: Kahola, Mika
  Cc: intel-gfx@lists.freedesktop.org
  Subject: Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization
  
  On Thu, Feb 26, 2015 at 11:26:10AM +0200, Mika Kahola wrote:
   In a case when DP link has been once trained we can reuse the existing
   link training parameters i.e. voltage swing and pre-emphasis levels
   from cache when there is a need to restart link training. In a case of
   eDP we initially try to train the link by using the parameter set that
   we can find from VBT. The fallback on both cases is to reset the link
   training parameters and restart.
  
  I don't think the fallback from bad VBT values works as on failure they just
  get reset. Care to enlighten me?
  -Chris
  
  --
  Chris Wilson, Intel Open Source Technology Centre
 
 Thank you for the comment! This was my first patch to gfx development so all 
 comments are welcomed.
 
 The idea that I had in mind was to initially start DP link training with the 
 parameters that are stored in VBT. If we are unlucky and the link cannot be 
 established at once by applying those initial parameters the parameters are 
 reset to zero and link training is restarted. I do agree that the fallback in 
 this case is unnecessary if VBT values are reset to zero in case of a failure.

The VBT values themselves are not reset to zero. What I meant was the
sequence for VBT recovery is:

first time link training
- intel_dp_reset_link_train()
   - use VBT values
- reuse_train_set = true
clock recovery !ok
- reuse_train_set == true
  - intel_dp_reset_link_train
 - use VBT values again
  - reuse_train_set = false

i.e. every time we call reset_link_train we restore the VBT values and
not zero. What you want is something like a
intel_dp-edp_use_vbt_train_set flag that is initially set to
is_edp(intel_edp) and then cleared if clock recovery ever fails.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Kahola, Mika 
 -Original Message-
 From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
 Sent: Thursday, February 26, 2015 12:51 PM
 To: Kahola, Mika
 Cc: intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization
 
 On Thu, Feb 26, 2015 at 11:26:10AM +0200, Mika Kahola wrote:
  In a case when DP link has been once trained we can reuse the existing
  link training parameters i.e. voltage swing and pre-emphasis levels
  from cache when there is a need to restart link training. In a case of
  eDP we initially try to train the link by using the parameter set that
  we can find from VBT. The fallback on both cases is to reset the link
  training parameters and restart.
 
 I don't think the fallback from bad VBT values works as on failure they just
 get reset. Care to enlighten me?
 -Chris
 
 --
 Chris Wilson, Intel Open Source Technology Centre

Thank you for the comment! This was my first patch to gfx development so all 
comments are welcomed.

The idea that I had in mind was to initially start DP link training with the 
parameters that are stored in VBT. If we are unlucky and the link cannot be 
established at once by applying those initial parameters the parameters are 
reset to zero and link training is restarted. I do agree that the fallback in 
this case is unnecessary if VBT values are reset to zero in case of a failure.

Cheers,
Mika
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Mika Kahola 
In a case when DP link has been once trained we can reuse
the existing link training parameters i.e. voltage swing
and pre-emphasis levels from cache when there is a need to
restart link training. In a case of eDP we initially try
to train the link by using the parameter set that we can find
from VBT. The fallback on both cases is to reset the link
training parameters and restart.

Signed-off-by: Mika Kahola mika.kah...@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c  | 93 +++-
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d1141d3..8482f58 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3288,8 +3288,39 @@ static bool
 intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
uint8_t dp_train_pat)
 {
-   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int i;
+
+   /* in case of eDP, get link training parameters from VBT */
+   if (is_edp(intel_dp)) {
+   for (i=0; iintel_dp-lane_count; i++)
+   intel_dp-train_set[i] =  dev_priv-vbt.edp_vswing | 
dev_priv-vbt.edp_preemphasis;
+   }
+   else
+   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+
+   intel_dp_set_signal_levels(intel_dp, DP);
+   return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
+}
+
+static bool
+intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+ uint8_t dp_train_pat)
+{
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
intel_dp_set_signal_levels(intel_dp, DP);
+
+   I915_WRITE(intel_dp-output_reg, *DP);
+   POSTING_READ(intel_dp-output_reg);
+
+   drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
+ intel_dp-train_set, intel_dp-lane_count);
+
return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
 }
 
@@ -3356,6 +3387,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
int voltage_tries, loop_tries;
uint32_t DP = intel_dp-DP;
uint8_t link_config[2];
+   uint8_t link_status[DP_LINK_STATUS_SIZE];
+   bool reuse_train_set = false;
 
if (HAS_DDI(dev))
intel_ddi_prepare_link_retrain(encoder);
@@ -3373,20 +3406,44 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 
DP |= DP_PORT_EN;
 
-   /* clock recovery */
-   if (!intel_dp_reset_link_train(intel_dp, DP,
-  DP_TRAINING_PATTERN_1 |
-  DP_LINK_SCRAMBLING_DISABLE)) {
-   DRM_ERROR(failed to enable link training\n);
-   return;
+   /*
+* check if DP has already trained. Reset voltage swing and
+* pre-emphasis levels if that's not the case.
+*/
+   if (intel_dp-link_trained) {
+   if (!intel_dp_reuse_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_DEBUG_KMS(unable to set known link training 
values\n);
+   return;
+   }
+   DRM_DEBUG_KMS(reuse current link train set\n);
+   reuse_train_set = true;
+   }
+   else {
+   /* reset link training values */
+   if (!intel_dp_reset_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_ERROR(failed to enable link training\n);
+   return;
+   }
+   DRM_DEBUG_KMS(reset link train set\n);
+
+   /*
+* in case of eDP, allow fallback to reset link training set
+* if VBT parameters doesn't apply
+*/
+   if (is_edp(intel_dp))
+   reuse_train_set = true;
+   else
+   reuse_train_set = false;
}
 
voltage = 0xff;
voltage_tries = 0;
loop_tries = 0;
for (;;) {
-   uint8_t link_status[DP_LINK_STATUS_SIZE];
-
drm_dp_link_train_clock_recovery_delay(intel_dp-dpcd);
if (!intel_dp_get_link_status(intel_dp, link_status)) {
DRM_ERROR(failed to get link status\n);
@@ -3397,6 +3454,20 @@

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Chris Wilson 
On Thu, Feb 26, 2015 at 11:26:10AM +0200, Mika Kahola wrote:
 In a case when DP link has been once trained we can reuse
 the existing link training parameters i.e. voltage swing
 and pre-emphasis levels from cache when there is a need to
 restart link training. In a case of eDP we initially try
 to train the link by using the parameter set that we can find
 from VBT. The fallback on both cases is to reset the link
 training parameters and restart.

I don't think the fallback from bad VBT values works as on failure they
just get reset. Care to enlighten me?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-26 Thread Todd Previte 

Hi Mika,

On 2/26/2015 2:26 AM, Mika Kahola wrote:

In a case when DP link has been once trained we can reuse
the existing link training parameters i.e. voltage swing
and pre-emphasis levels from cache when there is a need to
restart link training. In a case of eDP we initially try
to train the link by using the parameter set that we can find
from VBT. The fallback on both cases is to reset the link
training parameters and restart.
I think you have a good idea here for the eDP case. But the way it's 
coded, there appear to be cases where the code will try to reuse the 
same training settings for regular DP as well. That will likely have 
some unfortunate results, as there's no guarantee for a normal external 
Displayport connection that training settings which worked once will 
work the next time. This would be of particular concern when 
disconnecting and reconnecting Displayport sink devices, as it appears 
as though it would try to retrain the newly connected device using the 
previously valid settings for the old device.


IMHO, this really needs to be written as eDP only and such that it can 
never occur on an external Displayport device. Otherwise, it seems like 
there's a good chance of ending up with a black screen when you plug in 
a Displayport monitor.



Signed-off-by: Mika Kahola mika.kah...@intel.com
---
  drivers/gpu/drm/i915/intel_dp.c  | 93 +++-
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  2 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d1141d3..8482f58 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3288,8 +3288,39 @@ static bool
  intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
uint8_t dp_train_pat)
  {
-   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int i;
+
+   /* in case of eDP, get link training parameters from VBT */
+   if (is_edp(intel_dp)) {
+   for (i=0; iintel_dp-lane_count; i++)
+   intel_dp-train_set[i] =  dev_priv-vbt.edp_vswing | 
dev_priv-vbt.edp_preemphasis;
+   }
+   else
+   memset(intel_dp-train_set, 0, sizeof(intel_dp-train_set));
+
+   intel_dp_set_signal_levels(intel_dp, DP);
+   return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
+}
+
+static bool
+intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+ uint8_t dp_train_pat)
+{
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
intel_dp_set_signal_levels(intel_dp, DP);
+
+   I915_WRITE(intel_dp-output_reg, *DP);
+   POSTING_READ(intel_dp-output_reg);
+
+   drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
+ intel_dp-train_set, intel_dp-lane_count);
+
return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
  }
  
I'm not sure you want to be updating the sink device at this point. This 
should be checked against the spec to be sure it's not out of sequence.



@@ -3356,6 +3387,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
int voltage_tries, loop_tries;
uint32_t DP = intel_dp-DP;
uint8_t link_config[2];
+   uint8_t link_status[DP_LINK_STATUS_SIZE];
+   bool reuse_train_set = false;
  
  	if (HAS_DDI(dev))

intel_ddi_prepare_link_retrain(encoder);
@@ -3373,20 +3406,44 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
  
  	DP |= DP_PORT_EN;
  
-	/* clock recovery */

-   if (!intel_dp_reset_link_train(intel_dp, DP,
-  DP_TRAINING_PATTERN_1 |
-  DP_LINK_SCRAMBLING_DISABLE)) {
-   DRM_ERROR(failed to enable link training\n);
-   return;
+   /*
+* check if DP has already trained. Reset voltage swing and
+* pre-emphasis levels if that's not the case.
+*/
+   if (intel_dp-link_trained) {
+   if (!intel_dp_reuse_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_DEBUG_KMS(unable to set known link training 
values\n);
+   return;
+   }
+   DRM_DEBUG_KMS(reuse current link train set\n);
+   reuse_train_set = true;
+   }
+   else {
+   /* reset link training values */
+   if (!intel_dp_reset_link_train(intel_dp, DP,
+

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-11 Thread Daniel Vetter 
On Wed, Feb 11, 2015 at 04:57:06PM +0200, Jani Nikula wrote:
 On Wed, 11 Feb 2015, Mika Kahola mika.kah...@intel.com wrote:
  -
  Intel Finland Oy
  Registered Address: PL 281, 00181 Helsinki 
  Business Identity Code: 0357606 - 4 
  Domiciled in Helsinki 
 
  This e-mail and any attachments may contain confidential material for
  the sole use of the intended recipient(s). Any review or distribution
  by others is strictly prohibited. If you are not the intended
  recipient, please contact the sender and delete all copies.

Also please don't submit patches with this disclaimer, since that
disclaimer and MIT/GPL lincense just don't agree.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-11 Thread Jani Nikula 
On Wed, 11 Feb 2015, Mika Kahola mika.kah...@intel.com wrote:
 Reuse existing DP link training values i.e. voltage swing
 and pre-emphasis levels, if DP port that we are connected
 to hasn't changed. If we are unable to re-initialize DP
 link, the fallback is to reset the link training values,
 and restart.

Sorry, I failed to remind you to sign your work:

Signed-off-by: Mika Kahola mika.kah...@intel.com

see Documentation/SubmittingPatches for details.

BR,
Jani.


   modified:   intel_dp.c
   modified:   intel_drv.h
 ---
  drivers/gpu/drm/i915/intel_dp.c  | 75 
 +++-
  drivers/gpu/drm/i915/intel_drv.h |  2 ++
  2 files changed, 69 insertions(+), 8 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 868a07b..c40d64a 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3286,6 +3286,25 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp, 
 uint32_t *DP,
  }
  
  static bool
 +intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
 +   uint8_t dp_train_pat)
 +{
 + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 + struct drm_device *dev = intel_dig_port-base.base.dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 +
 + intel_dp_set_signal_levels(intel_dp, DP);
 +
 + I915_WRITE(intel_dp-output_reg, *DP);
 + POSTING_READ(intel_dp-output_reg);
 +
 + drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
 +   intel_dp-train_set, intel_dp-lane_count);
 +
 + return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
 +}
 +
 +static bool
  intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
  const uint8_t link_status[DP_LINK_STATUS_SIZE])
  {
 @@ -3348,6 +3367,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
   int voltage_tries, loop_tries;
   uint32_t DP = intel_dp-DP;
   uint8_t link_config[2];
 + uint8_t link_status[DP_LINK_STATUS_SIZE];
 + bool reuse_train_set = false;
  
   if (HAS_DDI(dev))
   intel_ddi_prepare_link_retrain(encoder);
 @@ -3365,20 +3386,36 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
  
   DP |= DP_PORT_EN;
  
 - /* clock recovery */
 - if (!intel_dp_reset_link_train(intel_dp, DP,
 -DP_TRAINING_PATTERN_1 |
 -DP_LINK_SCRAMBLING_DISABLE)) {
 - DRM_ERROR(failed to enable link training\n);
 - return;
 + /*
 +  * check if DP connector has changed. Reset voltage swing and
 +  * pre-emphasis levels if that's the case.
 +  */
 + if (intel_dp-connector_changed) {
 + /* reset link training values */
 + DRM_DEBUG_KMS(resetting link train set\n);
 + if (!intel_dp_reset_link_train(intel_dp, DP,
 +DP_TRAINING_PATTERN_1 |
 +DP_LINK_SCRAMBLING_DISABLE)) {
 + DRM_ERROR(failed to enable link training\n);
 + return;
 + }
 + reuse_train_set = false;
 + }
 + else {
 + DRM_DEBUG_KMS(reusing current link train set\n);
 + if (!intel_dp_reuse_link_train(intel_dp, DP,
 +DP_TRAINING_PATTERN_1 |
 +DP_LINK_SCRAMBLING_DISABLE)) {
 + DRM_DEBUG_KMS(unable to set known link training 
 values\n);
 + return;
 + }
 + reuse_train_set = true;
   }
  
   voltage = 0xff;
   voltage_tries = 0;
   loop_tries = 0;
   for (;;) {
 - uint8_t link_status[DP_LINK_STATUS_SIZE];
 -
   drm_dp_link_train_clock_recovery_delay(intel_dp-dpcd);
   if (!intel_dp_get_link_status(intel_dp, link_status)) {
   DRM_ERROR(failed to get link status\n);
 @@ -3389,6 +3426,20 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
   DRM_DEBUG_KMS(clock recovery OK\n);
   break;
   }
 + /*
 +  * if we used previously trained voltage and pre-emphasis values
 +  * and we don't get clock recovery, reset link training values
 +  */
 + if (reuse_train_set) {
 + DRM_DEBUG_KMS(clock recovery not ok, resetting DP 
 train set\n);
 + if (!intel_dp_reset_link_train(intel_dp, DP,
 +DP_TRAINING_PATTERN_1 |
 +
 DP_LINK_SCRAMBLING_DISABLE)) {
 + DRM_ERROR(failed to enable link training\n);
 + return;
 + }
 +

[Intel-gfx] [PATCH] drm/i915: DP link training optimization

2015-02-11 Thread Mika Kahola 
Reuse existing DP link training values i.e. voltage swing
and pre-emphasis levels, if DP port that we are connected
to hasn't changed. If we are unable to re-initialize DP
link, the fallback is to reset the link training values,
and restart.
modified:   intel_dp.c
modified:   intel_drv.h
---
 drivers/gpu/drm/i915/intel_dp.c  | 75 +++-
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 868a07b..c40d64a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3286,6 +3286,25 @@ intel_dp_reset_link_train(struct intel_dp *intel_dp, 
uint32_t *DP,
 }
 
 static bool
+intel_dp_reuse_link_train(struct intel_dp *intel_dp, uint32_t *DP,
+ uint8_t dp_train_pat)
+{
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port-base.base.dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
+   intel_dp_set_signal_levels(intel_dp, DP);
+
+   I915_WRITE(intel_dp-output_reg, *DP);
+   POSTING_READ(intel_dp-output_reg);
+
+   drm_dp_dpcd_write(intel_dp-aux, DP_TRAINING_LANE0_SET,
+ intel_dp-train_set, intel_dp-lane_count);
+
+   return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
+}
+
+static bool
 intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
   const uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
@@ -3348,6 +3367,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
int voltage_tries, loop_tries;
uint32_t DP = intel_dp-DP;
uint8_t link_config[2];
+   uint8_t link_status[DP_LINK_STATUS_SIZE];
+   bool reuse_train_set = false;
 
if (HAS_DDI(dev))
intel_ddi_prepare_link_retrain(encoder);
@@ -3365,20 +3386,36 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 
DP |= DP_PORT_EN;
 
-   /* clock recovery */
-   if (!intel_dp_reset_link_train(intel_dp, DP,
-  DP_TRAINING_PATTERN_1 |
-  DP_LINK_SCRAMBLING_DISABLE)) {
-   DRM_ERROR(failed to enable link training\n);
-   return;
+   /*
+* check if DP connector has changed. Reset voltage swing and
+* pre-emphasis levels if that's the case.
+*/
+   if (intel_dp-connector_changed) {
+   /* reset link training values */
+   DRM_DEBUG_KMS(resetting link train set\n);
+   if (!intel_dp_reset_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_ERROR(failed to enable link training\n);
+   return;
+   }
+   reuse_train_set = false;
+   }
+   else {
+   DRM_DEBUG_KMS(reusing current link train set\n);
+   if (!intel_dp_reuse_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_DEBUG_KMS(unable to set known link training 
values\n);
+   return;
+   }
+   reuse_train_set = true;
}
 
voltage = 0xff;
voltage_tries = 0;
loop_tries = 0;
for (;;) {
-   uint8_t link_status[DP_LINK_STATUS_SIZE];
-
drm_dp_link_train_clock_recovery_delay(intel_dp-dpcd);
if (!intel_dp_get_link_status(intel_dp, link_status)) {
DRM_ERROR(failed to get link status\n);
@@ -3389,6 +3426,20 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
DRM_DEBUG_KMS(clock recovery OK\n);
break;
}
+   /*
+* if we used previously trained voltage and pre-emphasis values
+* and we don't get clock recovery, reset link training values
+*/
+   if (reuse_train_set) {
+   DRM_DEBUG_KMS(clock recovery not ok, resetting DP 
train set\n);
+   if (!intel_dp_reset_link_train(intel_dp, DP,
+  DP_TRAINING_PATTERN_1 |
+  
DP_LINK_SCRAMBLING_DISABLE)) {
+   DRM_ERROR(failed to enable link training\n);
+   return;
+   }
+   reuse_train_set = false;
+   }
 
/* Check to see if we've tried the max voltage */
for (i = 0; i  intel_dp-lane_count; i++)
@@ -4076,6 +4127,14 @@