Change in coreboot[master]: sc7180: Fix prefill requirement and correct the fetch start check

[Ravi kumar (Code Review)]

Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start has evaluated to be more than
v_total which is invalid. Add a check to accommodate the extra h_total addition
in fetch_start calculation. Secondly, made the prefill line requirement
same as Kernel driver.

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/47067/1

diff --git a/src/soc/qualcomm/sc7180/display/mdss.c b/src/soc/qualcomm/sc7180/display/mdss.c
index 2d6bf6f..ce46e8e 100644
--- a/src/soc/qualcomm/sc7180/display/mdss.c
+++ b/src/soc/qualcomm/sc7180/display/mdss.c
@@ -6,7 +6,7 @@
 #include <edid.h>
 #include <soc/display/mdssreg.h>

-#define MDSS_MDP_MAX_PREFILL_FETCH 25
+#define MDSS_MDP_MAX_PREFILL_FETCH 24

static void mdss_source_pipe_config(struct edid *edid)
 {
@@ -91,9 +91,10 @@
    /*
     * MDP programmable fetch is for MDP with rev >= 1.05.
     * Programmable fetch is not needed if vertical back porch
-	 * plus vertical puls width is >= 25.
+	 * plus vertical pulse width plus extra line for the extra h_total
+	 * added during fetch start is >= 24.
     */
-	if ((edid->mode.vbl - edid->mode.vso) >= MDSS_MDP_MAX_PREFILL_FETCH)
+	if ((edid->mode.vbl - edid->mode.vso + 1) >= MDSS_MDP_MAX_PREFILL_FETCH)
    	return;

/*

[Paul Menzel (Code Review)]

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

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

Patch Set 1:

(4 comments)

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@9 
PS1, Line 9: With Innolux panel timings, the fetch_start has evaluated to be more than
           : v_total which is invalid.
> With Innolux panel timings, fetch_start evaluates to be greater than v_total, which is invalid.

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@10 
PS1, Line 10: v_total which is invalid. Add a check to accommodate the extra h_total addition
            : in fetch_start calculation.
No check is added, as far as I can see.

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@11 
PS1, Line 11: made
Present tense: make

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@12 
PS1, Line 12: as Kernel driver
> as Linux kernel driver.

Please also add the commit hash, and the path in the Linux source.

[Vinod Polimera (Code Review)]

Vinod Polimera has uploaded a new patch set (#2) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start evaluates to be more than v_total
which is invalid. Update max prefill fetch check to accommodate the extra
h_total addition in fetch_start calculation. Secondly, make the prefill line
requirement same as Kernel driver.

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/47067/2

[Vinod Polimera (Code Review)]

Vinod Polimera has uploaded a new patch set (#3) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start evaluates to be more than v_total
which is invalid. Update max prefill fetch check to accommodate the extra
h_total addition in fetch_start calculation. Secondly, make the prefill line
requirement same as Kernel driver.
Kernel repo : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/47067/3

[Vinod Polimera (Code Review)]

Vinod Polimera has uploaded a new patch set (#4) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start evaluates to be more than v_total
which is invalid. Update max prefill fetch check to accommodate the extra
h_total addition in fetch_start calculation. Secondly, make the prefill line
requirement same as Kernel driver.

Kernel repo : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/47067/4

[Vinod Polimera (Code Review)]

Vinod Polimera has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

Patch Set 4:

(4 comments)

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@9 
PS1, Line 9: With Innolux panel timings, the fetch_start has evaluated to be more than
           : v_total which is invalid.
> > With Innolux panel timings, fetch_start evaluates to be greater than v_total, which is invalid.

Done

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@11 
PS1, Line 11: made
> Present tense: make

Done

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@10 
PS1, Line 10: v_total which is invalid. Add a check to accommodate the extra h_total addition
            : in fetch_start calculation.
> No check is added, as far as I can see.

updated the check by adding extra line to max prefill fetch check

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@12 
PS1, Line 12: as Kernel driver
> > as Linux kernel driver. […]

Added linux kernel driver source path. This change in kernel is part of bringup code upload, so there is not separate commit hash for this change.

[Ravi kumar (Code Review)]

Hello build bot (Jenkins), mturney mturney, Julius Werner,

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

https://review.coreboot.org/c/coreboot/+/47067

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

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start has evaluated to be more than
v_total which is invalid. Add a check to accommodate the extra h_total addition
in fetch_start calculation. Secondly, made the prefill line requirement
same as Kernel driver.

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/47067/5

[Paul Menzel (Code Review)]

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

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@9 
PS1, Line 9: With Innolux panel timings, the fetch_start has evaluated to be more than
           : v_total which is invalid.
> Done

The patch set 5 reverted your changes again.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

Patch Set 5: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47067/1//COMMIT_MSG@9 
PS1, Line 9: With Innolux panel timings, the fetch_start has evaluated to be more than
           : v_total which is invalid.
> The patch set 5 reverted your changes again.

Sorry Paul, we need this patch really urgently and the time zone difference with India makes it hard to fix this quickly. I hope you won't mind if I let a few commit message grammar issues slip through this once.

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47067 )

Change subject: sc7180: Fix prefill requirement and correct the fetch start check
......................................................................

sc7180: Fix prefill requirement and correct the fetch start check

With Innolux panel timings, the fetch_start has evaluated to be more than
v_total which is invalid. Add a check to accommodate the extra h_total addition
in fetch_start calculation. Secondly, made the prefill line requirement
same as Kernel driver.

Change-Id: If7624c0b28421759fdf47dd92f23214a78058199
Signed-off-by: Vinod Polimera 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47067
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M src/soc/qualcomm/sc7180/display/mdss.c
1 file changed, 4 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/soc/qualcomm/sc7180/display/mdss.c b/src/soc/qualcomm/sc7180/display/mdss.c
index 2d6bf6f..ce46e8e 100644
--- a/src/soc/qualcomm/sc7180/display/mdss.c
+++ b/src/soc/qualcomm/sc7180/display/mdss.c
@@ -6,7 +6,7 @@
 #include <edid.h>
 #include <soc/display/mdssreg.h>

-#define MDSS_MDP_MAX_PREFILL_FETCH 25
+#define MDSS_MDP_MAX_PREFILL_FETCH 24

static void mdss_source_pipe_config(struct edid *edid)
 {
@@ -91,9 +91,10 @@
    /*
     * MDP programmable fetch is for MDP with rev >= 1.05.
     * Programmable fetch is not needed if vertical back porch
-	 * plus vertical puls width is >= 25.
+	 * plus vertical pulse width plus extra line for the extra h_total
+	 * added during fetch start is >= 24.
     */
-	if ((edid->mode.vbl - edid->mode.vso) >= MDSS_MDP_MAX_PREFILL_FETCH)
+	if ((edid->mode.vbl - edid->mode.vso + 1) >= MDSS_MDP_MAX_PREFILL_FETCH)
    	return;

/*