Change in ...libgfxinit[master]: dp training: Always end with normal output

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/32731

Change subject: dp training: Always end with normal output
......................................................................

dp training: Always end with normal output

The DP spec mandates that we return to normal output when we abandon
a failed training, before we disable the output. However, the sequence
suggested by Intel always disables the output, even if we try another
time or another frequency. So we also always return to normal frame
delivery.

Change-Id: Ie1f19f2e1d1af62b402ad1dc41373d3d4437e976
Signed-off-by: Nico Huber 
---
M common/hw-gfx-dp_training.adb
1 file changed, 8 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/31/32731/1

diff --git a/common/hw-gfx-dp_training.adb b/common/hw-gfx-dp_training.adb
index a2d00a7..079d49c 100644
--- a/common/hw-gfx-dp_training.adb
+++ b/common/hw-gfx-dp_training.adb
@@ -1,5 +1,5 @@
 --
--- Copyright (C) 2015-2016 secunet Security Networks AG
+-- Copyright (C) 2015-2016, 2019 secunet Security Networks AG
 -- Copyright (C) 2017 Nico Huber 
 --
 -- This program is free software; you can redistribute it and/or modify
@@ -382,20 +382,14 @@
          end loop;
       end if;

-      if Success then
-         if EQ_Done then
-            -- Set_Pattern (TP_None) includes sending the Idle Pattern,
-            -- so tell sink first.
-            Sink_Set_Training_Pattern
-              (DP, Link, DP_Info.TP_None, Success);
-         else
-            Success := False;
-         end if;
-      end if;
+      -- Set_Pattern (TP_None) includes sending the Idle Pattern,
+      -- so tell sink first.
+      Sink_Set_Training_Pattern
+        (DP, Link, DP_Info.TP_None, Success);
+      Set_Pattern (Port, Link, DP_Info.TP_None);

-      if Success then
-         Set_Pattern (Port, Link, DP_Info.TP_None);
-      else
+      Success := Success and then EQ_Done;
+      if not Success then
          Off (Port);
       end if;
    end Train_DP;

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 1: Verified+1

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG@11 
PS1, Line 11: suggested by Intel
Suggested where?

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG@14 
PS1, Line 14: 
Does this fix a bug or is it just to conform with the specification?

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 1: Code-Review+2

[Nico Huber (Code Review)]

Hello Patrick Rudolph, Arthur Heymans, Matt DeVillier,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/libgfxinit/+/32731

to look at the new patch set (#2).

Change subject: dp training: Always end with normal output
......................................................................

dp training: Always end with normal output

The DP spec mandates that we return to normal output when we abandon
a failed training, before we disable the output. However, the sequence
suggested by Intel always disables the output, even if we try another
time or another frequency. So we also always return to normal frame
delivery.

On Ivy Bridge, failure to return to normal output resulted in inability
to disable the display pipe and to restart the training. Thus, after a
failed training, further attempts were futile.

Change-Id: Ie1f19f2e1d1af62b402ad1dc41373d3d4437e976
Signed-off-by: Nico Huber 
---
M common/hw-gfx-dp_training.adb
1 file changed, 8 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/31/32731/2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG@11 
PS1, Line 11: suggested by Intel
> Suggested where?

Any of these [1] that talk about the DP training sequence. Usually
in the "Display" volume, earlier along with the northbridge registers.

[1] https://01.org/linuxgraphics/documentation/hardware-specification-prms

https://review.coreboot.org/#/c/32731/1//COMMIT_MSG@14 
PS1, Line 14:
> Does this fix a bug or is it just to conform with the specification?

Done

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 2: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

Patch Set 2: Code-Review+2

Looks good.

Change in libgfxinit[master]: dp training: Always end with normal output

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/libgfxinit/+/32731 )

Change subject: dp training: Always end with normal output
......................................................................

dp training: Always end with normal output

The DP spec mandates that we return to normal output when we abandon
a failed training, before we disable the output. However, the sequence
suggested by Intel always disables the output, even if we try another
time or another frequency. So we also always return to normal frame
delivery.

On Ivy Bridge, failure to return to normal output resulted in inability
to disable the display pipe and to restart the training. Thus, after a
failed training, further attempts were futile.

Change-Id: Ie1f19f2e1d1af62b402ad1dc41373d3d4437e976
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/32731
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Angel Pons 
Reviewed-by: Arthur Heymans 
Tested-by: Nico Huber 
---
M common/hw-gfx-dp_training.adb
1 file changed, 8 insertions(+), 14 deletions(-)

Approvals:
  Nico Huber: Verified
  HAOUAS Elyes: Looks good to me, but someone else must approve
  Arthur Heymans: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/common/hw-gfx-dp_training.adb b/common/hw-gfx-dp_training.adb
index a2d00a7..079d49c 100644
--- a/common/hw-gfx-dp_training.adb
+++ b/common/hw-gfx-dp_training.adb
@@ -1,5 +1,5 @@
 --
--- Copyright (C) 2015-2016 secunet Security Networks AG
+-- Copyright (C) 2015-2016, 2019 secunet Security Networks AG
 -- Copyright (C) 2017 Nico Huber 
 --
 -- This program is free software; you can redistribute it and/or modify
@@ -382,20 +382,14 @@
          end loop;
       end if;

-      if Success then
-         if EQ_Done then
-            -- Set_Pattern (TP_None) includes sending the Idle Pattern,
-            -- so tell sink first.
-            Sink_Set_Training_Pattern
-              (DP, Link, DP_Info.TP_None, Success);
-         else
-            Success := False;
-         end if;
-      end if;
+      -- Set_Pattern (TP_None) includes sending the Idle Pattern,
+      -- so tell sink first.
+      Sink_Set_Training_Pattern
+        (DP, Link, DP_Info.TP_None, Success);
+      Set_Pattern (Port, Link, DP_Info.TP_None);

-      if Success then
-         Set_Pattern (Port, Link, DP_Info.TP_None);
-      else
+      Success := Success and then EQ_Done;
+      if not Success then
          Off (Port);
       end if;
    end Train_DP;