All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
@ 2014-01-17  3:06 Todd Previte
  2014-01-17  6:30 ` Daniel Vetter
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Todd Previte @ 2014-01-17  3:06 UTC (permalink / raw)
  To: intel-gfx

For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
DPCD in order to use HBR2.
---
 drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7df5085..f92d1c0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
 	case DP_LINK_BW_2_7:
 		break;
 	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
-		max_link_bw = DP_LINK_BW_2_7;
+        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
+            max_link_bw = DP_LINK_BW_5_4;
+        else
+            max_link_bw = DP_LINK_BW_2_7;
 		break;
 	default:
 		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
@@ -805,9 +808,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
-	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
+    /* Conveniently, the link BW constants become indices with a shift...*/
+    int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
 	int bpp, mode_rate;
-	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
+	static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
 	int link_avail, link_clock;
 
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
@@ -2621,10 +2625,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
+    uint32_t training_pattern = DP_TRAINING_PATTERN_2;
+
+    /* Training Pattern 3 for HBR2 */
+    if (intel_dp->link_bw == DP_LINK_BW_5_4)
+        training_pattern = DP_TRAINING_PATTERN_3;
 
 	/* channel equalization */
 	if (!intel_dp_set_link_train(intel_dp, &DP,
-				     DP_TRAINING_PATTERN_2 |
+				     training_pattern |
 				     DP_LINK_SCRAMBLING_DISABLE)) {
 		DRM_ERROR("failed to start channel equalization\n");
 		return;
@@ -2652,7 +2661,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			cr_tries++;
 			continue;
@@ -2668,7 +2677,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			intel_dp_link_down(intel_dp);
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			tries = 0;
 			cr_tries++;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17  3:06 [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices Todd Previte
@ 2014-01-17  6:30 ` Daniel Vetter
  2014-01-17 14:58   ` Todd Previte
  2014-01-17 11:55 ` Damien Lespiau
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2014-01-17  6:30 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Fri, Jan 17, 2014 at 4:06 AM, Todd Previte <tprevite@gmail.com> wrote:
> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
> sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
> DPCD in order to use HBR2.

sob line missing.

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7df5085..f92d1c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>         case DP_LINK_BW_2_7:
>                 break;
>         case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> -               max_link_bw = DP_LINK_BW_2_7;
> +        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
> +            max_link_bw = DP_LINK_BW_5_4;
> +        else
> +            max_link_bw = DP_LINK_BW_2_7;

Is this really required, i.e. do we have dp 1.1 machines in the wild
which advertise 5.4 but can't? In any case you also need to have a
IS_HSW || IS_BDW check here, since only those two platforms support
5.4 GHz.

>                 break;
>         default:
>                 WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> @@ -805,9 +808,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>         struct intel_connector *intel_connector = intel_dp->attached_connector;
>         int lane_count, clock;
>         int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> -       int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
> +    /* Conveniently, the link BW constants become indices with a shift...*/
> +    int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
>         int bpp, mode_rate;
> -       static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> +       static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
>         int link_avail, link_clock;
>
>         if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
> @@ -2621,10 +2625,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>         bool channel_eq = false;
>         int tries, cr_tries;
>         uint32_t DP = intel_dp->DP;
> +    uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> +
> +    /* Training Pattern 3 for HBR2 */
> +    if (intel_dp->link_bw == DP_LINK_BW_5_4)
> +        training_pattern = DP_TRAINING_PATTERN_3;

Hm, I've thought that with 5.4 we're supposed to do both pattern 2&3.
But tbh I didn't bother to dig out the spec ;-) Does it hurt not to?
Also, is there any harm in using pattern 3 for all dp 1.2 capable
sinks? I've thought that it should give us more reliable link
training, so might be beneficial not just for 5.4 mode.
-Daniel

>         /* channel equalization */
>         if (!intel_dp_set_link_train(intel_dp, &DP,
> -                                    DP_TRAINING_PATTERN_2 |
> +                                    training_pattern |
>                                      DP_LINK_SCRAMBLING_DISABLE)) {
>                 DRM_ERROR("failed to start channel equalization\n");
>                 return;
> @@ -2652,7 +2661,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>                 if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
>                         intel_dp_start_link_train(intel_dp);
>                         intel_dp_set_link_train(intel_dp, &DP,
> -                                               DP_TRAINING_PATTERN_2 |
> +                                               training_pattern |
>                                                 DP_LINK_SCRAMBLING_DISABLE);
>                         cr_tries++;
>                         continue;
> @@ -2668,7 +2677,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>                         intel_dp_link_down(intel_dp);
>                         intel_dp_start_link_train(intel_dp);
>                         intel_dp_set_link_train(intel_dp, &DP,
> -                                               DP_TRAINING_PATTERN_2 |
> +                                               training_pattern |
>                                                 DP_LINK_SCRAMBLING_DISABLE);
>                         tries = 0;
>                         cr_tries++;
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17  3:06 [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices Todd Previte
  2014-01-17  6:30 ` Daniel Vetter
@ 2014-01-17 11:55 ` Damien Lespiau
  2014-01-17 13:32   ` Jani Nikula
  2014-01-17 15:00   ` Todd Previte
  2014-01-17 16:58 ` [PATCH V2] " Todd Previte
  2014-01-20 17:19 ` [PATCH v3] " Todd Previte
  3 siblings, 2 replies; 15+ messages in thread
From: Damien Lespiau @ 2014-01-17 11:55 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Thu, Jan 16, 2014 at 08:06:08PM -0700, Todd Previte wrote:
> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
> sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
> DPCD in order to use HBR2.
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7df5085..f92d1c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>  	case DP_LINK_BW_2_7:
>  		break;
>  	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> -		max_link_bw = DP_LINK_BW_2_7;
> +        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
> +            max_link_bw = DP_LINK_BW_5_4;
> +        else
> +            max_link_bw = DP_LINK_BW_2_7;
>  		break;

I see spaces instead of tabs. You can use the useful checkpatch.pl
script on patches to catch those pesky style issues (from within a linux
tree):

$ ./scripts/checkpatch.pl 0001-drm-i915-Enable-5.4Ghz-HBR2-link-rate-for-Displaypor.patch

[...]

total: 6 errors, 9 warnings, 55 lines checked

-- 
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 11:55 ` Damien Lespiau
@ 2014-01-17 13:32   ` Jani Nikula
  2014-01-17 15:22     ` Todd Previte
  2014-01-17 15:00   ` Todd Previte
  1 sibling, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2014-01-17 13:32 UTC (permalink / raw)
  To: Damien Lespiau, Todd Previte; +Cc: intel-gfx

On Fri, 17 Jan 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> I see spaces instead of tabs. You can use the useful checkpatch.pl
> script on patches to catch those pesky style issues (from within a linux
> tree):
>
> $ ./scripts/checkpatch.pl 0001-drm-i915-Enable-5.4Ghz-HBR2-link-rate-for-Displaypor.patch
>
> [...]
>
> total: 6 errors, 9 warnings, 55 lines checked

Should anyone find this useful, I have these to check branches in my
local repos directly:

alias checkpatch='/path/to/checkpatch.pl -q --emacs --strict'

checkbranch()
{
    local commit
    local range

    if [ -z "$1" ]; then
	range="origin..HEAD"
    elif [ -n "`echo $1 | grep '\.\.'`" ]; then
	range="$1"
    else
	range="$1..HEAD"
    fi

    for commit in `git rev-list --reverse $range`; do
	git --no-pager log --oneline -1 $commit
	git format-patch --stdout -1 $commit | checkpatch -
    done
}

Then I can do:

Check local patches against origin:
$ checkbranch

Check local patches against drm-intel/drm-intel-nightly:
$ checkbranch drm-intel/drm-intel-nightly

Check a revision range:
$ checkbranch commit1..commit2

Do note that checkpatch is not the law. But it helps you get some of the
little things straight.


HTH,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17  6:30 ` Daniel Vetter
@ 2014-01-17 14:58   ` Todd Previte
  2014-01-17 15:08     ` Damien Lespiau
  0 siblings, 1 reply; 15+ messages in thread
From: Todd Previte @ 2014-01-17 14:58 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On 1/16/2014 11:30 PM, Daniel Vetter wrote:
> On Fri, Jan 17, 2014 at 4:06 AM, Todd Previte <tprevite@gmail.com> wrote:
>> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
>> sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
>> DPCD in order to use HBR2.
> sob line missing.
Oops. I'll add

Signed-off-by: Todd Previte <tprevite@gmail.com>

to the next revision.

>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7df5085..f92d1c0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>>          case DP_LINK_BW_2_7:
>>                  break;
>>          case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
>> -               max_link_bw = DP_LINK_BW_2_7;
>> +        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
>> +            max_link_bw = DP_LINK_BW_5_4;
>> +        else
>> +            max_link_bw = DP_LINK_BW_2_7;
> Is this really required, i.e. do we have dp 1.1 machines in the wild
> which advertise 5.4 but can't? In any case you also need to have a
> IS_HSW || IS_BDW check here, since only those two platforms support
> 5.4 GHz.
I've not seen a case where a 1.1a capable device advertises HBR2, no. I 
*have* seen the case where the sink reports that it only supports RBR 
(1.62Ghz) but is in fact capable of 2.7Ghz. This is more of a safety 
measure to eliminate potential training problems, but is not strictly 
necessary to support HBR2. It does need the IS_HSW || IS_BDW though, so 
I'll fix that and resend.

>>                  break;
>>          default:
>>                  WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
>> @@ -805,9 +808,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>          struct intel_connector *intel_connector = intel_dp->attached_connector;
>>          int lane_count, clock;
>>          int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>> -       int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
>> +    /* Conveniently, the link BW constants become indices with a shift...*/
>> +    int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
>>          int bpp, mode_rate;
>> -       static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
>> +       static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
>>          int link_avail, link_clock;
>>
>>          if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
>> @@ -2621,10 +2625,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>>          bool channel_eq = false;
>>          int tries, cr_tries;
>>          uint32_t DP = intel_dp->DP;
>> +    uint32_t training_pattern = DP_TRAINING_PATTERN_2;
>> +
>> +    /* Training Pattern 3 for HBR2 */
>> +    if (intel_dp->link_bw == DP_LINK_BW_5_4)
>> +        training_pattern = DP_TRAINING_PATTERN_3;
> Hm, I've thought that with 5.4 we're supposed to do both pattern 2&3.
> But tbh I didn't bother to dig out the spec ;-) Does it hurt not to?
> Also, is there any harm in using pattern 3 for all dp 1.2 capable
> sinks? I've thought that it should give us more reliable link
> training, so might be beneficial not just for 5.4 mode.
> -Daniel
Nope. For 5.4Ghz the source is supposed to use TPS3 in place of TPS2 in 
the channel equalization phase. I think it would cause problems if we 
try to use both TPS2 and TPS3. I suspect the sink device would simple 
not lock onto the pattern from TPS2 since it's expecting pattern 3 when 
set for HBR2. So all that would do is effectively delay link training 
until such time as TPS3 was being transmitted.

There is a bit in the DPCD (bit 6, DPCD 0x002) that indicates whether or 
not a sink device supports TPS3. For these devices, we should be using 
TPS3 instead of TPS2 for the channel equalization phase, yes. The code 
doesn't currently check this bit but it should. I'll add that into the 
configuration computation to enable TPS3 for 1.2 devices that support it.

>>          /* channel equalization */
>>          if (!intel_dp_set_link_train(intel_dp, &DP,
>> -                                    DP_TRAINING_PATTERN_2 |
>> +                                    training_pattern |
>>                                       DP_LINK_SCRAMBLING_DISABLE)) {
>>                  DRM_ERROR("failed to start channel equalization\n");
>>                  return;
>> @@ -2652,7 +2661,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>>                  if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
>>                          intel_dp_start_link_train(intel_dp);
>>                          intel_dp_set_link_train(intel_dp, &DP,
>> -                                               DP_TRAINING_PATTERN_2 |
>> +                                               training_pattern |
>>                                                  DP_LINK_SCRAMBLING_DISABLE);
>>                          cr_tries++;
>>                          continue;
>> @@ -2668,7 +2677,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>>                          intel_dp_link_down(intel_dp);
>>                          intel_dp_start_link_train(intel_dp);
>>                          intel_dp_set_link_train(intel_dp, &DP,
>> -                                               DP_TRAINING_PATTERN_2 |
>> +                                               training_pattern |
>>                                                  DP_LINK_SCRAMBLING_DISABLE);
>>                          tries = 0;
>>                          cr_tries++;
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 11:55 ` Damien Lespiau
  2014-01-17 13:32   ` Jani Nikula
@ 2014-01-17 15:00   ` Todd Previte
  1 sibling, 0 replies; 15+ messages in thread
From: Todd Previte @ 2014-01-17 15:00 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On 1/17/2014 4:55 AM, Damien Lespiau wrote:
> On Thu, Jan 16, 2014 at 08:06:08PM -0700, Todd Previte wrote:
>> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
>> sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
>> DPCD in order to use HBR2.
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 7df5085..f92d1c0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>>   	case DP_LINK_BW_2_7:
>>   		break;
>>   	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
>> -		max_link_bw = DP_LINK_BW_2_7;
>> +        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
>> +            max_link_bw = DP_LINK_BW_5_4;
>> +        else
>> +            max_link_bw = DP_LINK_BW_2_7;
>>   		break;
> I see spaces instead of tabs. You can use the useful checkpatch.pl
> script on patches to catch those pesky style issues (from within a linux
> tree):
>
> $ ./scripts/checkpatch.pl 0001-drm-i915-Enable-5.4Ghz-HBR2-link-rate-for-Displaypor.patch
>
> [...]
>
> total: 6 errors, 9 warnings, 55 lines checked
>
Gah. That's what happens when I make changes with different editors. 
I'll fix those in the next revision.

-T

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 14:58   ` Todd Previte
@ 2014-01-17 15:08     ` Damien Lespiau
  2014-01-17 15:22       ` Todd Previte
  0 siblings, 1 reply; 15+ messages in thread
From: Damien Lespiau @ 2014-01-17 15:08 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Fri, Jan 17, 2014 at 07:58:58AM -0700, Todd Previte wrote:
> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>index 7df5085..f92d1c0 100644
> >>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>@@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
> >>         case DP_LINK_BW_2_7:
> >>                 break;
> >>         case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> >>-               max_link_bw = DP_LINK_BW_2_7;
> >>+        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
> >>+            max_link_bw = DP_LINK_BW_5_4;
> >>+        else
> >>+            max_link_bw = DP_LINK_BW_2_7;
> >Is this really required, i.e. do we have dp 1.1 machines in the wild
> >which advertise 5.4 but can't? In any case you also need to have a
> >IS_HSW || IS_BDW check here, since only those two platforms support
> >5.4 GHz.
> I've not seen a case where a 1.1a capable device advertises HBR2,
> no. I *have* seen the case where the sink reports that it only
> supports RBR (1.62Ghz) but is in fact capable of 2.7Ghz. This is
> more of a safety measure to eliminate potential training problems,
> but is not strictly necessary to support HBR2. It does need the
> IS_HSW || IS_BDW though, so I'll fix that and resend.

Can we make it IS_HSW || INTEL_INFO(dev)->gen >= 8, we can't quite
predict the future but new platforms supporting what old platforms do
support is a bet we take elsewere.

-- 
Damien

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 13:32   ` Jani Nikula
@ 2014-01-17 15:22     ` Todd Previte
  0 siblings, 0 replies; 15+ messages in thread
From: Todd Previte @ 2014-01-17 15:22 UTC (permalink / raw)
  To: Jani Nikula, Damien Lespiau; +Cc: intel-gfx

On 1/17/2014 6:32 AM, Jani Nikula wrote:
> On Fri, 17 Jan 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
>> I see spaces instead of tabs. You can use the useful checkpatch.pl
>> script on patches to catch those pesky style issues (from within a linux
>> tree):
>>
>> $ ./scripts/checkpatch.pl 0001-drm-i915-Enable-5.4Ghz-HBR2-link-rate-for-Displaypor.patch
>>
>> [...]
>>
>> total: 6 errors, 9 warnings, 55 lines checked
> Should anyone find this useful, I have these to check branches in my
> local repos directly:
>
> alias checkpatch='/path/to/checkpatch.pl -q --emacs --strict'
>
> checkbranch()
> {
>      local commit
>      local range
>
>      if [ -z "$1" ]; then
> 	range="origin..HEAD"
>      elif [ -n "`echo $1 | grep '\.\.'`" ]; then
> 	range="$1"
>      else
> 	range="$1..HEAD"
>      fi
>
>      for commit in `git rev-list --reverse $range`; do
> 	git --no-pager log --oneline -1 $commit
> 	git format-patch --stdout -1 $commit | checkpatch -
>      done
> }
>
> Then I can do:
>
> Check local patches against origin:
> $ checkbranch
>
> Check local patches against drm-intel/drm-intel-nightly:
> $ checkbranch drm-intel/drm-intel-nightly
>
> Check a revision range:
> $ checkbranch commit1..commit2
>
> Do note that checkpatch is not the law. But it helps you get some of the
> little things straight.
>
>
> HTH,
> Jani.
>
>
>
Cool thanks Jani. I'll give that a try.

-T

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 15:08     ` Damien Lespiau
@ 2014-01-17 15:22       ` Todd Previte
  0 siblings, 0 replies; 15+ messages in thread
From: Todd Previte @ 2014-01-17 15:22 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On 1/17/2014 8:08 AM, Damien Lespiau wrote:
> On Fri, Jan 17, 2014 at 07:58:58AM -0700, Todd Previte wrote:
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index 7df5085..f92d1c0 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -102,7 +102,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>>>>          case DP_LINK_BW_2_7:
>>>>                  break;
>>>>          case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
>>>> -               max_link_bw = DP_LINK_BW_2_7;
>>>> +        if (intel_dp->dpcd[DP_DPCD_REV] == 0x12)
>>>> +            max_link_bw = DP_LINK_BW_5_4;
>>>> +        else
>>>> +            max_link_bw = DP_LINK_BW_2_7;
>>> Is this really required, i.e. do we have dp 1.1 machines in the wild
>>> which advertise 5.4 but can't? In any case you also need to have a
>>> IS_HSW || IS_BDW check here, since only those two platforms support
>>> 5.4 GHz.
>> I've not seen a case where a 1.1a capable device advertises HBR2,
>> no. I *have* seen the case where the sink reports that it only
>> supports RBR (1.62Ghz) but is in fact capable of 2.7Ghz. This is
>> more of a safety measure to eliminate potential training problems,
>> but is not strictly necessary to support HBR2. It does need the
>> IS_HSW || IS_BDW though, so I'll fix that and resend.
> Can we make it IS_HSW || INTEL_INFO(dev)->gen >= 8, we can't quite
> predict the future but new platforms supporting what old platforms do
> support is a bet we take elsewere.
>
Sounds like a better solution. I should have that integrated shortly.

-T

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V2] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17  3:06 [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices Todd Previte
  2014-01-17  6:30 ` Daniel Vetter
  2014-01-17 11:55 ` Damien Lespiau
@ 2014-01-17 16:58 ` Todd Previte
  2014-01-17 16:58   ` [PATCH] " Todd Previte
  2014-01-20 17:19 ` [PATCH v3] " Todd Previte
  3 siblings, 1 reply; 15+ messages in thread
From: Todd Previte @ 2014-01-17 16:58 UTC (permalink / raw)
  To: intel-gfx

Clean up and adjustments per the feedback above.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 16:58 ` [PATCH V2] " Todd Previte
@ 2014-01-17 16:58   ` Todd Previte
  2014-01-17 17:34     ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Todd Previte @ 2014-01-17 16:58 UTC (permalink / raw)
  To: intel-gfx

   For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
   sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
   DPCD in order to use HBR2.

   V2:
     - Added training pattern 3 flag to intel_dp struct
     - Added check for appropriate hardware to 5.4Ghz link rate configuration
     - Added a check for TPS3 supprt in the DPCD read
     - Adjusted channel equalization to use TPS3 when appropriate
     - Cleaned up whitespace
Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 30 ++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7df5085..fd87c37 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -102,7 +102,11 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
 	case DP_LINK_BW_2_7:
 		break;
 	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
-		max_link_bw = DP_LINK_BW_2_7;
+		if ((IS_HSW(dev) || (INTEL_INFO(dev)->gen >= 8)) &&
+		   (intel_dp->dpcd[DP_DPCD_REV] >= 0x12))
+			max_link_bw = DP_LINK_BW_5_4;
+		else
+			max_link_bw = DP_LINK_BW_2_7;
 		break;
 	default:
 		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
@@ -805,9 +809,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
-	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
+    /* Conveniently, the link BW constants become indices with a shift...*/
+	int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
 	int bpp, mode_rate;
-	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
+	static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
 	int link_avail, link_clock;
 
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
@@ -2621,10 +2626,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
+	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
+
+    /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
+	if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
+		training_pattern = DP_TRAINING_PATTERN_3;
 
 	/* channel equalization */
 	if (!intel_dp_set_link_train(intel_dp, &DP,
-				     DP_TRAINING_PATTERN_2 |
+				     training_pattern |
 				     DP_LINK_SCRAMBLING_DISABLE)) {
 		DRM_ERROR("failed to start channel equalization\n");
 		return;
@@ -2652,7 +2662,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			cr_tries++;
 			continue;
@@ -2668,7 +2678,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			intel_dp_link_down(intel_dp);
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			tries = 0;
 			cr_tries++;
@@ -2810,6 +2820,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		}
 	}
 
+	/* Training Pattern 3 support */
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
+		intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED) {
+		intel_dp->use_tps3 = true;
+		DRM_DEBUG_KMS("Displayport TPS3 supported");
+	} else
+		intel_dp->use_tps3 = false;
+
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
 	      DP_DWN_STRM_PORT_PRESENT))
 		return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9841f78..e934d8d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,7 @@ struct intel_dp {
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
 	bool psr_setup_done;
+	bool use_tps3;
 	struct intel_connector *attached_connector;
 };
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17 16:58   ` [PATCH] " Todd Previte
@ 2014-01-17 17:34     ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2014-01-17 17:34 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

Looks good imo, just a bit patch polish. And maybe wait for Ville's
confirmation about the stuff we're discussing in private.
-Daniel

On Fri, Jan 17, 2014 at 09:58:03AM -0700, Todd Previte wrote:
>    For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
>    sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
>    DPCD in order to use HBR2.
> 
>    V2:
>      - Added training pattern 3 flag to intel_dp struct
>      - Added check for appropriate hardware to 5.4Ghz link rate configuration
>      - Added a check for TPS3 supprt in the DPCD read
>      - Adjusted channel equalization to use TPS3 when appropriate
>      - Cleaned up whitespace

Commit message shouldn't be indented (git will do that when displaying a
patch automatically) and should be limited to about 70 chars (for to have
a bit of space in 80 char mails when replying). Also a white line before
the sob-section.

> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 30 ++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7df5085..fd87c37 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -102,7 +102,11 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp)
>  	case DP_LINK_BW_2_7:
>  		break;
>  	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> -		max_link_bw = DP_LINK_BW_2_7;
> +		if ((IS_HSW(dev) || (INTEL_INFO(dev)->gen >= 8)) &&
> +		   (intel_dp->dpcd[DP_DPCD_REV] >= 0x12))

Checkpatch is still a bit unhappy about the alignment of the 2-line
condition here, and I concur - imo doing this as suggest helps readiblity
quite a bit.

> +			max_link_bw = DP_LINK_BW_5_4;
> +		else
> +			max_link_bw = DP_LINK_BW_2_7;
>  		break;
>  	default:
>  		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> @@ -805,9 +809,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  	int lane_count, clock;
>  	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> -	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
> +    /* Conveniently, the link BW constants become indices with a shift...*/
> +	int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
>  	int bpp, mode_rate;
> -	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> +	static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
>  	int link_avail, link_clock;
>  
>  	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
> @@ -2621,10 +2626,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  	bool channel_eq = false;
>  	int tries, cr_tries;
>  	uint32_t DP = intel_dp->DP;
> +	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> +
> +    /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/

Indentation of the comment.

> +	if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> +		training_pattern = DP_TRAINING_PATTERN_3;
>  
>  	/* channel equalization */
>  	if (!intel_dp_set_link_train(intel_dp, &DP,
> -				     DP_TRAINING_PATTERN_2 |
> +				     training_pattern |
>  				     DP_LINK_SCRAMBLING_DISABLE)) {
>  		DRM_ERROR("failed to start channel equalization\n");
>  		return;
> @@ -2652,7 +2662,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
>  			intel_dp_start_link_train(intel_dp);
>  			intel_dp_set_link_train(intel_dp, &DP,
> -						DP_TRAINING_PATTERN_2 |
> +						training_pattern |
>  						DP_LINK_SCRAMBLING_DISABLE);
>  			cr_tries++;
>  			continue;
> @@ -2668,7 +2678,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  			intel_dp_link_down(intel_dp);
>  			intel_dp_start_link_train(intel_dp);
>  			intel_dp_set_link_train(intel_dp, &DP,
> -						DP_TRAINING_PATTERN_2 |
> +						training_pattern |
>  						DP_LINK_SCRAMBLING_DISABLE);
>  			tries = 0;
>  			cr_tries++;
> @@ -2810,6 +2820,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> +	/* Training Pattern 3 support */
> +	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
> +		intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED) {

Another 2-line condition which should be aligned.

> +		intel_dp->use_tps3 = true;
> +		DRM_DEBUG_KMS("Displayport TPS3 supported");
> +	} else
> +		intel_dp->use_tps3 = false;
> +
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>  	      DP_DWN_STRM_PORT_PRESENT))
>  		return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9841f78..e934d8d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -488,6 +488,7 @@ struct intel_dp {
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
>  	bool psr_setup_done;
> +	bool use_tps3;
>  	struct intel_connector *attached_connector;
>  };
>  
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-17  3:06 [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices Todd Previte
                   ` (2 preceding siblings ...)
  2014-01-17 16:58 ` [PATCH V2] " Todd Previte
@ 2014-01-20 17:19 ` Todd Previte
  2014-01-20 17:19   ` Todd Previte
  3 siblings, 1 reply; 15+ messages in thread
From: Todd Previte @ 2014-01-20 17:19 UTC (permalink / raw)
  To: intel-gfx

More whitepsace cleanup.

One caveat with this patch: current link policy dictates that the driver will train the                
"wide and slow", i.e. max lanes at low speed. It will increase lanes and speed when the
specified resolution demands greater bandwidth. Consequently, the resolution will need to
be set such that the data rate exceeds the bandwidth provided by 4 lanes @ 2.4Ghz before 
the driver will use the new 5.4Ghz rate.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-20 17:19 ` [PATCH v3] " Todd Previte
@ 2014-01-20 17:19   ` Todd Previte
  2014-01-21  9:26     ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Todd Previte @ 2014-01-20 17:19 UTC (permalink / raw)
  To: intel-gfx

For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
DPCD in order to use HBR2.

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 31 +++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7df5085..7fb02f6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -96,13 +96,18 @@ static int
 intel_dp_max_link_bw(struct intel_dp *intel_dp)
 {
 	int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
+	struct drm_device *dev = intel_dp->attached_connector->base.dev;
 
 	switch (max_link_bw) {
 	case DP_LINK_BW_1_62:
 	case DP_LINK_BW_2_7:
 		break;
 	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
-		max_link_bw = DP_LINK_BW_2_7;
+		if ((IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) &&
+		    intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
+			max_link_bw = DP_LINK_BW_5_4;
+		else
+			max_link_bw = DP_LINK_BW_2_7;
 		break;
 	default:
 		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
@@ -805,9 +810,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
-	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
+	/* Conveniently, the link BW constants become indices with a shift...*/
+	int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
 	int bpp, mode_rate;
-	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
+	static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
 	int link_avail, link_clock;
 
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
@@ -2621,10 +2627,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
+	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
+
+	/* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
+	if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
+		training_pattern = DP_TRAINING_PATTERN_3;
 
 	/* channel equalization */
 	if (!intel_dp_set_link_train(intel_dp, &DP,
-				     DP_TRAINING_PATTERN_2 |
+				     training_pattern |
 				     DP_LINK_SCRAMBLING_DISABLE)) {
 		DRM_ERROR("failed to start channel equalization\n");
 		return;
@@ -2652,7 +2663,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			cr_tries++;
 			continue;
@@ -2668,7 +2679,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			intel_dp_link_down(intel_dp);
 			intel_dp_start_link_train(intel_dp);
 			intel_dp_set_link_train(intel_dp, &DP,
-						DP_TRAINING_PATTERN_2 |
+						training_pattern |
 						DP_LINK_SCRAMBLING_DISABLE);
 			tries = 0;
 			cr_tries++;
@@ -2810,6 +2821,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		}
 	}
 
+	/* Training Pattern 3 support */
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
+	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED) {
+		intel_dp->use_tps3 = true;
+		DRM_DEBUG_KMS("Displayport TPS3 supported");
+	} else
+		intel_dp->use_tps3 = false;
+
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
 	      DP_DWN_STRM_PORT_PRESENT))
 		return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9841f78..e934d8d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -488,6 +488,7 @@ struct intel_dp {
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
 	bool psr_setup_done;
+	bool use_tps3;
 	struct intel_connector *attached_connector;
 };
 
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices
  2014-01-20 17:19   ` Todd Previte
@ 2014-01-21  9:26     ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2014-01-21  9:26 UTC (permalink / raw)
  To: Todd Previte; +Cc: intel-gfx

On Mon, Jan 20, 2014 at 10:19:39AM -0700, Todd Previte wrote:
> For HSW+ platforms, enable the 5.4Ghz (HBR2) link rate for devices that support it. The
> sink device must report that is supports Displayport 1.2 and the HBR2 bit rate in the
> DPCD in order to use HBR2.
> 
> Signed-off-by: Todd Previte <tprevite@gmail.com>

As mentioned on irc, please add a per-patch changelog before the sob
section in the future. Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 31 +++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7df5085..7fb02f6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -96,13 +96,18 @@ static int
>  intel_dp_max_link_bw(struct intel_dp *intel_dp)
>  {
>  	int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
> +	struct drm_device *dev = intel_dp->attached_connector->base.dev;
>  
>  	switch (max_link_bw) {
>  	case DP_LINK_BW_1_62:
>  	case DP_LINK_BW_2_7:
>  		break;
>  	case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> -		max_link_bw = DP_LINK_BW_2_7;
> +		if ((IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) &&
> +		    intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
> +			max_link_bw = DP_LINK_BW_5_4;
> +		else
> +			max_link_bw = DP_LINK_BW_2_7;
>  		break;
>  	default:
>  		WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> @@ -805,9 +810,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
>  	int lane_count, clock;
>  	int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> -	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
> +	/* Conveniently, the link BW constants become indices with a shift...*/
> +	int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
>  	int bpp, mode_rate;
> -	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> +	static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
>  	int link_avail, link_clock;
>  
>  	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
> @@ -2621,10 +2627,15 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  	bool channel_eq = false;
>  	int tries, cr_tries;
>  	uint32_t DP = intel_dp->DP;
> +	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> +
> +	/* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
> +	if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> +		training_pattern = DP_TRAINING_PATTERN_3;
>  
>  	/* channel equalization */
>  	if (!intel_dp_set_link_train(intel_dp, &DP,
> -				     DP_TRAINING_PATTERN_2 |
> +				     training_pattern |
>  				     DP_LINK_SCRAMBLING_DISABLE)) {
>  		DRM_ERROR("failed to start channel equalization\n");
>  		return;
> @@ -2652,7 +2663,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  		if (!drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
>  			intel_dp_start_link_train(intel_dp);
>  			intel_dp_set_link_train(intel_dp, &DP,
> -						DP_TRAINING_PATTERN_2 |
> +						training_pattern |
>  						DP_LINK_SCRAMBLING_DISABLE);
>  			cr_tries++;
>  			continue;
> @@ -2668,7 +2679,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  			intel_dp_link_down(intel_dp);
>  			intel_dp_start_link_train(intel_dp);
>  			intel_dp_set_link_train(intel_dp, &DP,
> -						DP_TRAINING_PATTERN_2 |
> +						training_pattern |
>  						DP_LINK_SCRAMBLING_DISABLE);
>  			tries = 0;
>  			cr_tries++;
> @@ -2810,6 +2821,14 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> +	/* Training Pattern 3 support */
> +	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
> +	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED) {
> +		intel_dp->use_tps3 = true;
> +		DRM_DEBUG_KMS("Displayport TPS3 supported");
> +	} else
> +		intel_dp->use_tps3 = false;
> +
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>  	      DP_DWN_STRM_PORT_PRESENT))
>  		return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9841f78..e934d8d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -488,6 +488,7 @@ struct intel_dp {
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
>  	bool psr_setup_done;
> +	bool use_tps3;
>  	struct intel_connector *attached_connector;
>  };
>  
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-21  9:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-17  3:06 [PATCH] drm/i915: Enable 5.4Ghz (HBR2) link rate for Displayport 1.2-capable devices Todd Previte
2014-01-17  6:30 ` Daniel Vetter
2014-01-17 14:58   ` Todd Previte
2014-01-17 15:08     ` Damien Lespiau
2014-01-17 15:22       ` Todd Previte
2014-01-17 11:55 ` Damien Lespiau
2014-01-17 13:32   ` Jani Nikula
2014-01-17 15:22     ` Todd Previte
2014-01-17 15:00   ` Todd Previte
2014-01-17 16:58 ` [PATCH V2] " Todd Previte
2014-01-17 16:58   ` [PATCH] " Todd Previte
2014-01-17 17:34     ` Daniel Vetter
2014-01-20 17:19 ` [PATCH v3] " Todd Previte
2014-01-20 17:19   ` Todd Previte
2014-01-21  9:26     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.