| Message ID | 1445349004-16409-2-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 20, 2015 at 11:49:47AM -0200, Paulo Zanoni wrote: > I wanted to add yet another check to intel_fbc_update() and realized > I would need to create yet another enum no_fbc_reason case. So I > remembered this patch series that Damien wrote a long time ago and > nobody ever reviewed, so I decided to reimplement it since the code > changed a lot since then. > > Credits-to: Damien Lespiau <damien.lespiau@intel.com> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Yeah that enum indirection is pointless. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 19 +-------- > drivers/gpu/drm/i915/intel_drv.h | 1 - > drivers/gpu/drm/i915/intel_fbc.c | 77 +++++++++---------------------------- > 4 files changed, 20 insertions(+), 79 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index a3b22bd..6ac4ba3 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1640,7 +1640,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused) > seq_puts(m, "FBC enabled\n"); > else > seq_printf(m, "FBC disabled: %s\n", > - intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason)); > + dev_priv->fbc.no_fbc_reason); > > if (INTEL_INFO(dev_priv)->gen >= 7) > seq_printf(m, "Compressing: %s\n", > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8afda45..c334525 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -925,24 +925,7 @@ struct i915_fbc { > struct drm_framebuffer *fb; > } *fbc_work; > > - enum no_fbc_reason { > - FBC_OK, /* FBC is enabled */ > - FBC_UNSUPPORTED, /* FBC is not supported by this chipset */ > - FBC_NO_OUTPUT, /* no outputs enabled to compress */ > - FBC_STOLEN_TOO_SMALL, /* not enough space for buffers */ > - FBC_UNSUPPORTED_MODE, /* interlace or doublescanned mode */ > - FBC_MODE_TOO_LARGE, /* mode too large for compression */ > - FBC_BAD_PLANE, /* fbc not supported on plane */ > - FBC_NOT_TILED, /* buffer not tiled */ > - FBC_MULTIPLE_PIPES, /* more than one pipe active */ > - FBC_MODULE_PARAM, > - FBC_CHIP_DEFAULT, /* disabled by default on this chip */ > - FBC_ROTATION, /* rotation is not supported */ > - FBC_IN_DBG_MASTER, /* kernel debugger is active */ > - FBC_BAD_STRIDE, /* stride is not supported */ > - FBC_PIXEL_RATE, /* pixel rate is too big */ > - FBC_PIXEL_FORMAT /* pixel format is invalid */ > - } no_fbc_reason; > + const char *no_fbc_reason; > > bool (*fbc_enabled)(struct drm_i915_private *dev_priv); > void (*enable_fbc)(struct intel_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 27dccf3..dc55e4c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1287,7 +1287,6 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, > enum fb_op_origin origin); > void intel_fbc_flush(struct drm_i915_private *dev_priv, > unsigned int frontbuffer_bits, enum fb_op_origin origin); > -const char *intel_no_fbc_reason_str(enum no_fbc_reason reason); > void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv); > > /* intel_hdmi.c */ > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index cf47352..d9d7e54 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -471,55 +471,14 @@ void intel_fbc_disable_crtc(struct intel_crtc *crtc) > mutex_unlock(&dev_priv->fbc.lock); > } > > -const char *intel_no_fbc_reason_str(enum no_fbc_reason reason) > -{ > - switch (reason) { > - case FBC_OK: > - return "FBC enabled but currently disabled in hardware"; > - case FBC_UNSUPPORTED: > - return "unsupported by this chipset"; > - case FBC_NO_OUTPUT: > - return "no output"; > - case FBC_STOLEN_TOO_SMALL: > - return "not enough stolen memory"; > - case FBC_UNSUPPORTED_MODE: > - return "mode incompatible with compression"; > - case FBC_MODE_TOO_LARGE: > - return "mode too large for compression"; > - case FBC_BAD_PLANE: > - return "FBC unsupported on plane"; > - case FBC_NOT_TILED: > - return "framebuffer not tiled or fenced"; > - case FBC_MULTIPLE_PIPES: > - return "more than one pipe active"; > - case FBC_MODULE_PARAM: > - return "disabled per module param"; > - case FBC_CHIP_DEFAULT: > - return "disabled per chip default"; > - case FBC_ROTATION: > - return "rotation unsupported"; > - case FBC_IN_DBG_MASTER: > - return "Kernel debugger is active"; > - case FBC_BAD_STRIDE: > - return "framebuffer stride not supported"; > - case FBC_PIXEL_RATE: > - return "pixel rate is too big"; > - case FBC_PIXEL_FORMAT: > - return "pixel format is invalid"; > - default: > - MISSING_CASE(reason); > - return "unknown reason"; > - } > -} > - > static void set_no_fbc_reason(struct drm_i915_private *dev_priv, > - enum no_fbc_reason reason) > + const char *reason) > { > if (dev_priv->fbc.no_fbc_reason == reason) > return; > > dev_priv->fbc.no_fbc_reason = reason; > - DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason)); > + DRM_DEBUG_KMS("Disabling FBC: %s\n", reason); > } > > static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) > @@ -862,12 +821,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > i915.enable_fbc = 0; > > if (i915.enable_fbc < 0) { > - set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT); > + set_no_fbc_reason(dev_priv, "disabled per chip default"); > goto out_disable; > } > > if (!i915.enable_fbc) { > - set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM); > + set_no_fbc_reason(dev_priv, "disabled per module param"); > goto out_disable; > } > > @@ -882,12 +841,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > */ > crtc = intel_fbc_find_crtc(dev_priv); > if (!crtc) { > - set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT); > + set_no_fbc_reason(dev_priv, "no output"); > goto out_disable; > } > > if (!multiple_pipes_ok(dev_priv)) { > - set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES); > + set_no_fbc_reason(dev_priv, "more than one pipe active"); > goto out_disable; > } > > @@ -898,18 +857,18 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > > if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) || > (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) { > - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE); > + set_no_fbc_reason(dev_priv, "incompatible mode"); > goto out_disable; > } > > if (!intel_fbc_hw_tracking_covers_screen(intel_crtc)) { > - set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE); > + set_no_fbc_reason(dev_priv, "mode too large for compression"); > goto out_disable; > } > > if ((INTEL_INFO(dev_priv)->gen < 4 || HAS_DDI(dev_priv)) && > intel_crtc->plane != PLANE_A) { > - set_no_fbc_reason(dev_priv, FBC_BAD_PLANE); > + set_no_fbc_reason(dev_priv, "FBC unsupported on plane"); > goto out_disable; > } > > @@ -918,28 +877,28 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > */ > if (obj->tiling_mode != I915_TILING_X || > obj->fence_reg == I915_FENCE_REG_NONE) { > - set_no_fbc_reason(dev_priv, FBC_NOT_TILED); > + set_no_fbc_reason(dev_priv, "framebuffer not tiled or fenced"); > goto out_disable; > } > if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) && > crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) { > - set_no_fbc_reason(dev_priv, FBC_ROTATION); > + set_no_fbc_reason(dev_priv, "rotation unsupported"); > goto out_disable; > } > > if (!stride_is_valid(dev_priv, fb->pitches[0])) { > - set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE); > + set_no_fbc_reason(dev_priv, "framebuffer stride not supported"); > goto out_disable; > } > > if (!pixel_format_is_valid(fb)) { > - set_no_fbc_reason(dev_priv, FBC_PIXEL_FORMAT); > + set_no_fbc_reason(dev_priv, "pixel format is invalid"); > goto out_disable; > } > > /* If the kernel debugger is active, always disable compression */ > if (in_dbg_master()) { > - set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER); > + set_no_fbc_reason(dev_priv, "Kernel debugger is active"); > goto out_disable; > } > > @@ -947,12 +906,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) && > ilk_pipe_pixel_rate(intel_crtc->config) >= > dev_priv->cdclk_freq * 95 / 100) { > - set_no_fbc_reason(dev_priv, FBC_PIXEL_RATE); > + set_no_fbc_reason(dev_priv, "pixel rate is too big"); > goto out_disable; > } > > if (intel_fbc_setup_cfb(intel_crtc)) { > - set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL); > + set_no_fbc_reason(dev_priv, "not enough stolen memory"); > goto out_disable; > } > > @@ -995,7 +954,7 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) > } > > intel_fbc_schedule_enable(intel_crtc); > - dev_priv->fbc.no_fbc_reason = FBC_OK; > + dev_priv->fbc.no_fbc_reason = "FBC enabled (not necessarily active)\n"; > return; > > out_disable: > @@ -1088,7 +1047,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) > > if (!HAS_FBC(dev_priv)) { > dev_priv->fbc.enabled = false; > - dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED; > + dev_priv->fbc.no_fbc_reason = "unsupported by this chipset"; > return; > } > > -- > 2.6.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a3b22bd..6ac4ba3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1640,7 +1640,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused) seq_puts(m, "FBC enabled\n"); else seq_printf(m, "FBC disabled: %s\n", - intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason)); + dev_priv->fbc.no_fbc_reason); if (INTEL_INFO(dev_priv)->gen >= 7) seq_printf(m, "Compressing: %s\n", diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8afda45..c334525 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -925,24 +925,7 @@ struct i915_fbc { struct drm_framebuffer *fb; } *fbc_work; - enum no_fbc_reason { - FBC_OK, /* FBC is enabled */ - FBC_UNSUPPORTED, /* FBC is not supported by this chipset */ - FBC_NO_OUTPUT, /* no outputs enabled to compress */ - FBC_STOLEN_TOO_SMALL, /* not enough space for buffers */ - FBC_UNSUPPORTED_MODE, /* interlace or doublescanned mode */ - FBC_MODE_TOO_LARGE, /* mode too large for compression */ - FBC_BAD_PLANE, /* fbc not supported on plane */ - FBC_NOT_TILED, /* buffer not tiled */ - FBC_MULTIPLE_PIPES, /* more than one pipe active */ - FBC_MODULE_PARAM, - FBC_CHIP_DEFAULT, /* disabled by default on this chip */ - FBC_ROTATION, /* rotation is not supported */ - FBC_IN_DBG_MASTER, /* kernel debugger is active */ - FBC_BAD_STRIDE, /* stride is not supported */ - FBC_PIXEL_RATE, /* pixel rate is too big */ - FBC_PIXEL_FORMAT /* pixel format is invalid */ - } no_fbc_reason; + const char *no_fbc_reason; bool (*fbc_enabled)(struct drm_i915_private *dev_priv); void (*enable_fbc)(struct intel_crtc *crtc); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 27dccf3..dc55e4c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1287,7 +1287,6 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, enum fb_op_origin origin); void intel_fbc_flush(struct drm_i915_private *dev_priv, unsigned int frontbuffer_bits, enum fb_op_origin origin); -const char *intel_no_fbc_reason_str(enum no_fbc_reason reason); void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv); /* intel_hdmi.c */ diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index cf47352..d9d7e54 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -471,55 +471,14 @@ void intel_fbc_disable_crtc(struct intel_crtc *crtc) mutex_unlock(&dev_priv->fbc.lock); } -const char *intel_no_fbc_reason_str(enum no_fbc_reason reason) -{ - switch (reason) { - case FBC_OK: - return "FBC enabled but currently disabled in hardware"; - case FBC_UNSUPPORTED: - return "unsupported by this chipset"; - case FBC_NO_OUTPUT: - return "no output"; - case FBC_STOLEN_TOO_SMALL: - return "not enough stolen memory"; - case FBC_UNSUPPORTED_MODE: - return "mode incompatible with compression"; - case FBC_MODE_TOO_LARGE: - return "mode too large for compression"; - case FBC_BAD_PLANE: - return "FBC unsupported on plane"; - case FBC_NOT_TILED: - return "framebuffer not tiled or fenced"; - case FBC_MULTIPLE_PIPES: - return "more than one pipe active"; - case FBC_MODULE_PARAM: - return "disabled per module param"; - case FBC_CHIP_DEFAULT: - return "disabled per chip default"; - case FBC_ROTATION: - return "rotation unsupported"; - case FBC_IN_DBG_MASTER: - return "Kernel debugger is active"; - case FBC_BAD_STRIDE: - return "framebuffer stride not supported"; - case FBC_PIXEL_RATE: - return "pixel rate is too big"; - case FBC_PIXEL_FORMAT: - return "pixel format is invalid"; - default: - MISSING_CASE(reason); - return "unknown reason"; - } -} - static void set_no_fbc_reason(struct drm_i915_private *dev_priv, - enum no_fbc_reason reason) + const char *reason) { if (dev_priv->fbc.no_fbc_reason == reason) return; dev_priv->fbc.no_fbc_reason = reason; - DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason)); + DRM_DEBUG_KMS("Disabling FBC: %s\n", reason); } static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) @@ -862,12 +821,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) i915.enable_fbc = 0; if (i915.enable_fbc < 0) { - set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT); + set_no_fbc_reason(dev_priv, "disabled per chip default"); goto out_disable; } if (!i915.enable_fbc) { - set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM); + set_no_fbc_reason(dev_priv, "disabled per module param"); goto out_disable; } @@ -882,12 +841,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) */ crtc = intel_fbc_find_crtc(dev_priv); if (!crtc) { - set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT); + set_no_fbc_reason(dev_priv, "no output"); goto out_disable; } if (!multiple_pipes_ok(dev_priv)) { - set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES); + set_no_fbc_reason(dev_priv, "more than one pipe active"); goto out_disable; } @@ -898,18 +857,18 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) || (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) { - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE); + set_no_fbc_reason(dev_priv, "incompatible mode"); goto out_disable; } if (!intel_fbc_hw_tracking_covers_screen(intel_crtc)) { - set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE); + set_no_fbc_reason(dev_priv, "mode too large for compression"); goto out_disable; } if ((INTEL_INFO(dev_priv)->gen < 4 || HAS_DDI(dev_priv)) && intel_crtc->plane != PLANE_A) { - set_no_fbc_reason(dev_priv, FBC_BAD_PLANE); + set_no_fbc_reason(dev_priv, "FBC unsupported on plane"); goto out_disable; } @@ -918,28 +877,28 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) */ if (obj->tiling_mode != I915_TILING_X || obj->fence_reg == I915_FENCE_REG_NONE) { - set_no_fbc_reason(dev_priv, FBC_NOT_TILED); + set_no_fbc_reason(dev_priv, "framebuffer not tiled or fenced"); goto out_disable; } if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) && crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) { - set_no_fbc_reason(dev_priv, FBC_ROTATION); + set_no_fbc_reason(dev_priv, "rotation unsupported"); goto out_disable; } if (!stride_is_valid(dev_priv, fb->pitches[0])) { - set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE); + set_no_fbc_reason(dev_priv, "framebuffer stride not supported"); goto out_disable; } if (!pixel_format_is_valid(fb)) { - set_no_fbc_reason(dev_priv, FBC_PIXEL_FORMAT); + set_no_fbc_reason(dev_priv, "pixel format is invalid"); goto out_disable; } /* If the kernel debugger is active, always disable compression */ if (in_dbg_master()) { - set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER); + set_no_fbc_reason(dev_priv, "Kernel debugger is active"); goto out_disable; } @@ -947,12 +906,12 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) && ilk_pipe_pixel_rate(intel_crtc->config) >= dev_priv->cdclk_freq * 95 / 100) { - set_no_fbc_reason(dev_priv, FBC_PIXEL_RATE); + set_no_fbc_reason(dev_priv, "pixel rate is too big"); goto out_disable; } if (intel_fbc_setup_cfb(intel_crtc)) { - set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL); + set_no_fbc_reason(dev_priv, "not enough stolen memory"); goto out_disable; } @@ -995,7 +954,7 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) } intel_fbc_schedule_enable(intel_crtc); - dev_priv->fbc.no_fbc_reason = FBC_OK; + dev_priv->fbc.no_fbc_reason = "FBC enabled (not necessarily active)\n"; return; out_disable: @@ -1088,7 +1047,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) if (!HAS_FBC(dev_priv)) { dev_priv->fbc.enabled = false; - dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED; + dev_priv->fbc.no_fbc_reason = "unsupported by this chipset"; return; }
I wanted to add yet another check to intel_fbc_update() and realized I would need to create yet another enum no_fbc_reason case. So I remembered this patch series that Damien wrote a long time ago and nobody ever reviewed, so I decided to reimplement it since the code changed a lot since then. Credits-to: Damien Lespiau <damien.lespiau@intel.com> Cc: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 19 +-------- drivers/gpu/drm/i915/intel_drv.h | 1 - drivers/gpu/drm/i915/intel_fbc.c | 77 +++++++++---------------------------- 4 files changed, 20 insertions(+), 79 deletions(-)