Skip to contentSkip to conversation

drm/i915: precedence bug in hsw_compute_wm_results()

PATCH In review Dan Carpenter 4 participants 6 messages Aug 9, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/i915: precedence bug in hsw_compute_wm_results()

The '!' operation has higher precedence than the compare so probably this test is never true.

drivers/gpu/drm/i915/intel_pm.c +1 -1
index a5a9959..0b9d9a7 100644
@@ -2562,7 +2562,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val <= lp_maximums->fbc) {
 			results->enable_fbc_wm = false;
 			lp_results[level - 1].fbc_val = 0;
 		}
#1 lore raw
Quoted 10 lines · Dan Carpenter

The '!' operation has higher precedence than the compare so probably this test is never true.

drivers/gpu/drm/i915/intel_pm.c +0 -0
index a5a9959..0b9d9a7 100644

drivers/gpu/drm/i915/intel_pm.c static void hsw_compute_wm_results(struct drm_device *dev, · line 2565

Quoted 5 lines
drivers/gpu/drm/i915/intel_pm.c +1 -1
@@ -2562,7 +2562,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val <= lp_maximums->fbc) {

I didn't spot that '!' at all. It's a stray and should be just removed. -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#2 lore raw

PATCH v2 1/1 drm/i915: fix a limit check in hsw_compute_wm_results()

The '!' here was not intended. Since '!' has higher precedence than compare, it means the check is never true.

---
v2: My first patch was wrong.

drivers/gpu/drm/i915/intel_pm.c +1 -1
index 96234c6..0f5eb21 100644
@@ -2685,7 +2685,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val > lp_maximums->fbc) {
 			results->enable_fbc_wm = false;
 			lp_results[level - 1].fbc_val = 0;
 		}
Reviewed-by on v2 1/1
#3 lore raw

Re: [Intel-gfx] [patch v2] drm/i915: fix a limit check in hsw_compute_wm_results()

Quoted 4 lines · Dan Carpenter

The '!' here was not intended. Since '!' has higher precedence than compare, it means the check is never true.

The culprit for Daniel:

commit 71fff20ff1bb790f4defe0c880e028581ffab420

Author: Ville Syrjälä <[email protected]>
Date:   Tue Aug 6 22:24:03 2013 +0300

drm/i915: Kill fbc_enable from hsw_lp_wm_results

Quoted 20 lines

---
v2: My first patch was wrong.

drivers/gpu/drm/i915/intel_pm.c +1 -1
index 96234c6..0f5eb21 100644
@@ -2685,7 +2685,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val > lp_maximums->fbc) {
 			results->enable_fbc_wm = false;
 			lp_results[level - 1].fbc_val = 0;
 		}

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 6 lines
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw

Re: [Intel-gfx] [patch v2] drm/i915: fix a limit check in hsw_compute_wm_results()

Quoted 8 lines · Ville Syrjälä
Quoted 4 lines · Dan Carpenter

The '!' here was not intended. Since '!' has higher precedence than compare, it means the check is never true.

Quoted 7 lines

The culprit for Daniel:

commit 71fff20ff1bb790f4defe0c880e028581ffab420

Author: Ville Syrjälä <[email protected]>
Date:   Tue Aug 6 22:24:03 2013 +0300

drm/i915: Kill fbc_enable from hsw_lp_wm_results

Queued for -next, thanks for the patch.
-Daniel

Quoted 25 lines
Quoted 21 lines

---
v2: My first patch was wrong.

drivers/gpu/drm/i915/intel_pm.c +1 -1
index 96234c6..0f5eb21 100644
@@ -2685,7 +2685,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val > lp_maximums->fbc) {
 			results->enable_fbc_wm = false;
 			lp_results[level - 1].fbc_val = 0;
 		}

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines
Ville Syrjälä
Intel OTC
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
In reply to Dan Carpenter
Quoted 10 lines · Dan Carpenter

The '!' operation has higher precedence than the compare so probably this test is never true.

drivers/gpu/drm/i915/intel_pm.c +0 -0
index a5a9959..0b9d9a7 100644

drivers/gpu/drm/i915/intel_pm.c static void hsw_compute_wm_results(struct drm_device *dev, · line 2565

Quoted 5 lines
drivers/gpu/drm/i915/intel_pm.c +1 -1
@@ -2562,7 +2562,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 	 * a WM level. */
 	results->enable_fbc_wm = true;
 	for (level = 1; level <= max_level; level++) {
-		if (!lp_results[level - 1].fbc_val > lp_maximums->fbc) {
+		if (lp_results[level - 1].fbc_val <= lp_maximums->fbc) {

Whoops. My bad.

It should still be > but ! should just be dropped.

Quoted 7 lines
drivers/gpu/drm/i915/intel_pm.c +0 -0
 			results->enable_fbc_wm = false;
 			lp_results[level - 1].fbc_val = 0;
 		}

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 6 lines
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html