[PATCH] drm/i915: clarify logging about reasons for disabling FBC

[PATCH] drm/i915: clarify logging about reasons for disabling FBC

[Jani Nikula]

Previously FBC disabling due to per-chip default was logged as being
disabled per module parameter. Distinguish between the two.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

only compile tested...
---
 drivers/gpu/drm/i915/i915_debugfs.c |    5 ++++-
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/intel_pm.c     |   14 ++++++++------
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0911fcd..275954c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1294,7 +1294,10 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 			seq_printf(m, "multiple pipes are enabled");
 			break;
 		case FBC_MODULE_PARAM:
-			seq_printf(m, "disabled per module param (default off)");
+			seq_printf(m, "disabled per module param");
+			break;
+		case FBC_PER_CHIP_DEFAULT:
+			seq_printf(m, "per chip default is disabled");
 			break;
 		default:
 			seq_printf(m, "unknown reason");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1f6a09..1d552df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -453,6 +453,7 @@ enum no_fbc_reason {
 	FBC_NOT_TILED, /* buffer not tiled */
 	FBC_MULTIPLE_PIPES, /* more than one pipe active */
 	FBC_MODULE_PARAM,
+	FBC_PER_CHIP_DEFAULT,
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 61fee7f..9b3cdf4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -437,12 +437,14 @@ void intel_update_fbc(struct drm_device *dev)
 
 	enable_fbc = i915_enable_fbc;
 	if (enable_fbc < 0) {
-		DRM_DEBUG_KMS("fbc set to per-chip default\n");
-		enable_fbc = 1;
-		if (INTEL_INFO(dev)->gen <= 6)
-			enable_fbc = 0;
-	}
-	if (!enable_fbc) {
+		enable_fbc = INTEL_INFO(dev)->gen > 6;
+		DRM_DEBUG_KMS("fbc set to per-chip default: %s\n",
+			      enable_fbc ? "enabled" : "disabled");
+		if (!enable_fbc) {
+			dev_priv->no_fbc_reason = FBC_PER_CHIP_DEFAULT;
+			goto out_disable;
+		}
+	} else if (!enable_fbc) {
 		DRM_DEBUG_KMS("fbc disabled per module param\n");
 		dev_priv->no_fbc_reason = FBC_MODULE_PARAM;
 		goto out_disable;
-- 
1.7.9.5

Re: [PATCH] drm/i915: clarify logging about reasons for disabling FBC

[Chris Wilson]

On Thu, Feb 14, 2013 at 01:52:42PM +0200, Jani Nikula wrote:
> Previously FBC disabling due to per-chip default was logged as being
> disabled per module parameter. Distinguish between the two.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Don't even squawk in the debug log if FBC is explicitly disabled.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v2] drm/i915: clarify logging about reasons for disabling FBC

[Jani Nikula]

Previously FBC disabling due to per-chip default was logged as being
disabled per module parameter. Distinguish between the two.

v2: Don't even squawk in the debug log if FBC is explicitly disabled (Chris
Wilson).

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    5 ++++-
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/intel_pm.c     |   15 ++++++++-------
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0911fcd..275954c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1294,7 +1294,10 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 			seq_printf(m, "multiple pipes are enabled");
 			break;
 		case FBC_MODULE_PARAM:
-			seq_printf(m, "disabled per module param (default off)");
+			seq_printf(m, "disabled per module param");
+			break;
+		case FBC_PER_CHIP_DEFAULT:
+			seq_printf(m, "per chip default is disabled");
 			break;
 		default:
 			seq_printf(m, "unknown reason");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1f6a09..1d552df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -453,6 +453,7 @@ enum no_fbc_reason {
 	FBC_NOT_TILED, /* buffer not tiled */
 	FBC_MULTIPLE_PIPES, /* more than one pipe active */
 	FBC_MODULE_PARAM,
+	FBC_PER_CHIP_DEFAULT,
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 61fee7f..9b2d7dd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -437,13 +437,14 @@ void intel_update_fbc(struct drm_device *dev)
 
 	enable_fbc = i915_enable_fbc;
 	if (enable_fbc < 0) {
-		DRM_DEBUG_KMS("fbc set to per-chip default\n");
-		enable_fbc = 1;
-		if (INTEL_INFO(dev)->gen <= 6)
-			enable_fbc = 0;
-	}
-	if (!enable_fbc) {
-		DRM_DEBUG_KMS("fbc disabled per module param\n");
+		enable_fbc = INTEL_INFO(dev)->gen > 6;
+		DRM_DEBUG_KMS("fbc set to per-chip default: %s\n",
+			      enable_fbc ? "enabled" : "disabled");
+		if (!enable_fbc) {
+			dev_priv->no_fbc_reason = FBC_PER_CHIP_DEFAULT;
+			goto out_disable;
+		}
+	} else if (!enable_fbc) {
 		dev_priv->no_fbc_reason = FBC_MODULE_PARAM;
 		goto out_disable;
 	}
-- 
1.7.9.5

Re: [PATCH v2] drm/i915: clarify logging about reasons for disabling FBC

[Chris Wilson]

On Tue, Feb 19, 2013 at 09:18:20AM +0200, Jani Nikula wrote:
> Previously FBC disabling due to per-chip default was logged as being
> disabled per module parameter. Distinguish between the two.
> 
> v2: Don't even squawk in the debug log if FBC is explicitly disabled (Chris
> Wilson).

I'm still not entirely happy, as I still get to see the repeated message
many time a second, but it is an improvement over the current message.

I would be happiest if just the state changes were logged. Something
like in out_disable:
 if (reason != dev_priv->no_fbc_reason) {
   DEBUG("disabling fbc: %s\n", reason);
   dev_priv->no_fbc_reason = reason;
 }
and move the enum->string conversion from debugfs and just store the
(static) string.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2] drm/i915: clarify logging about reasons for disabling FBC

[Jani Nikula]

On Tue, 19 Feb 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Feb 19, 2013 at 09:18:20AM +0200, Jani Nikula wrote:
>> Previously FBC disabling due to per-chip default was logged as being
>> disabled per module parameter. Distinguish between the two.
>> 
>> v2: Don't even squawk in the debug log if FBC is explicitly disabled (Chris
>> Wilson).
>
> I'm still not entirely happy, as I still get to see the repeated message
> many time a second, but it is an improvement over the current message.

Right, I was just trying to get rid of printing misleading information
(disabled per module parameter when it was the per chip default) and
ignored the noise part (which can be more annoying).

Jani.