@@ -459,18 +459,6 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
fbc->deactivate(dev_priv);
}
-static void set_no_fbc_reason(struct drm_i915_private *dev_priv,
- const char *reason)
-{
- struct intel_fbc *fbc = &dev_priv->fbc;
-
- if (fbc->no_fbc_reason == reason)
- return;
-
- fbc->no_fbc_reason = reason;
- DRM_DEBUG_KMS("Disabling FBC: %s\n", reason);
-}
-
static bool crtc_can_fbc(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
@@ -759,18 +747,18 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
struct intel_fbc_state_cache *cache = &fbc->state_cache;
if (!cache->plane.visible) {
- set_no_fbc_reason(dev_priv, "primary plane not visible");
+ fbc->no_fbc_reason = "primary plane not visible";
return false;
}
if ((cache->crtc.mode_flags & DRM_MODE_FLAG_INTERLACE) ||
(cache->crtc.mode_flags & DRM_MODE_FLAG_DBLSCAN)) {
- set_no_fbc_reason(dev_priv, "incompatible mode");
+ fbc->no_fbc_reason = "incompatible mode";
return false;
}
if (!intel_fbc_hw_tracking_covers_screen(crtc)) {
- set_no_fbc_reason(dev_priv, "mode too large for compression");
+ fbc->no_fbc_reason = "mode too large for compression";
return false;
}
@@ -779,29 +767,29 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
*/
if (cache->fb.tiling_mode != I915_TILING_X ||
cache->fb.fence_reg == I915_FENCE_REG_NONE) {
- set_no_fbc_reason(dev_priv, "framebuffer not tiled or fenced");
+ fbc->no_fbc_reason = "framebuffer not tiled or fenced";
return false;
}
if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) &&
cache->plane.rotation != BIT(DRM_ROTATE_0)) {
- set_no_fbc_reason(dev_priv, "rotation unsupported");
+ fbc->no_fbc_reason = "rotation unsupported";
return false;
}
if (!stride_is_valid(dev_priv, cache->fb.stride)) {
- set_no_fbc_reason(dev_priv, "framebuffer stride not supported");
+ fbc->no_fbc_reason = "framebuffer stride not supported";
return false;
}
if (!pixel_format_is_valid(dev_priv, cache->fb.pixel_format)) {
- set_no_fbc_reason(dev_priv, "pixel format is invalid");
+ fbc->no_fbc_reason = "pixel format is invalid";
return false;
}
/* WaFbcExceedCdClockThreshold:hsw,bdw */
if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
cache->crtc.hsw_bdw_pixel_rate >= dev_priv->cdclk_freq * 95 / 100) {
- set_no_fbc_reason(dev_priv, "pixel rate is too big");
+ fbc->no_fbc_reason = "pixel rate is too big";
return false;
}
@@ -817,7 +805,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
* important case, we can implement it later. */
if (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) >
fbc->compressed_fb.size * fbc->threshold) {
- set_no_fbc_reason(dev_priv, "CFB requirements changed");
+ fbc->no_fbc_reason = "CFB requirements changed";
return false;
}
@@ -827,24 +815,25 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
static bool intel_fbc_can_choose(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ struct intel_fbc *fbc = &dev_priv->fbc;
if (intel_vgpu_active(dev_priv->dev)) {
- set_no_fbc_reason(dev_priv, "VGPU is active");
+ fbc->no_fbc_reason = "VGPU is active";
return false;
}
if (i915.enable_fbc < 0) {
- set_no_fbc_reason(dev_priv, "disabled per chip default");
+ fbc->no_fbc_reason = "disabled per chip default";
return false;
}
if (!i915.enable_fbc) {
- set_no_fbc_reason(dev_priv, "disabled per module param");
+ fbc->no_fbc_reason = "disabled per module param";
return false;
}
if (!crtc_can_fbc(crtc)) {
- set_no_fbc_reason(dev_priv, "no enabled pipes can have FBC");
+ fbc->no_fbc_reason = "no enabled pipes can have FBC";
return false;
}
@@ -895,7 +884,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
mutex_lock(&fbc->lock);
if (!multiple_pipes_ok(crtc)) {
- set_no_fbc_reason(dev_priv, "more than one pipe active");
+ fbc->no_fbc_reason = "more than one pipe active";
goto deactivate;
}
@@ -1113,7 +1102,7 @@ void intel_fbc_enable(struct intel_crtc *crtc)
intel_fbc_update_state_cache(crtc);
if (intel_fbc_alloc_cfb(crtc)) {
- set_no_fbc_reason(dev_priv, "not enough stolen memory");
+ fbc->no_fbc_reason = "not enough stolen memory";
goto out;
}
Our dmesg messages started being misleading after we converted to the enable+activate model: we always print "Disabling FBC", even when we're just deactivating it. So, for example, when I boot my machine and do "dmesg | grep -i fbc", I see: [drm:intel_fbc_enable] Enabling FBC on pipe A [drm:set_no_fbc_reason] Disabling FBC: framebuffer not tiled or fenced but then, if I read the debugfs file, I will see: $ sudo cat i915_fbc_status FBC enabled Compressing: yes so we can conclude that dmesg is misleading, since FBC is actually enabled. What happened is that we deactivated FBC due to fbcon not being tiled, but when we silently reactivated it when the display manager started. We don't print activation messages since there may be way too many of these operations per second during normal desktop usage. One possible solution would be to change set_no_fbc_reason to correctly differentiate between disable and deactivation, but we removed support from printing activation/deactivation messages in the past because they were too frequent. So instead of doing this, let's just not print anything on dmesg, and leave the debugfs file if the user needs to investigate something. We already print when we enable and disable FBC anyway on a given pipe, so this should already help triaging bugs. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_fbc.c | 43 +++++++++++++++------------------------- 1 file changed, 16 insertions(+), 27 deletions(-)