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 <[email protected]> > Cc: Damien Lespiau <[email protected]> > Signed-off-by: Paulo Zanoni <[email protected]>
Yeah that enum indirection is pointless. Reviewed-by: Daniel Vetter <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
