public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Damien Lespiau <damien.lespiau@intel.com>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 3/4] drm/i915: unify no_fbc_reason message printing
Date: Fri, 12 Jun 2015 13:17:13 +0100	[thread overview]
Message-ID: <20150612121713.GA28224@strange.ger.corp.intel.com> (raw)
In-Reply-To: <1434049347-2100-4-git-send-email-przanoni@gmail.com>

On Thu, Jun 11, 2015 at 04:02:26PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This commit has two main advantages: simplify intel_fbc_update()
> and deduplicate the strings.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I had some things around that topic as well in feb. May be of interest:

http://lists.freedesktop.org/archives/intel-gfx/2015-February/060850.html

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 49 +++------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c    | 66 +++++++++++++++++++++++++------------
>  3 files changed, 50 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 405022b..eaa567c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
>  
>  	intel_runtime_pm_get(dev_priv);
>  
> -	if (intel_fbc_enabled(dev)) {
> +	if (intel_fbc_enabled(dev))
>  		seq_puts(m, "FBC enabled\n");
> -	} else {
> -		seq_puts(m, "FBC disabled: ");
> -		switch (dev_priv->fbc.no_fbc_reason) {
> -		case FBC_OK:
> -			seq_puts(m, "FBC actived, but currently disabled in hardware");
> -			break;
> -		case FBC_UNSUPPORTED:
> -			seq_puts(m, "unsupported by this chipset");
> -			break;
> -		case FBC_NO_OUTPUT:
> -			seq_puts(m, "no outputs");
> -			break;
> -		case FBC_STOLEN_TOO_SMALL:
> -			seq_puts(m, "not enough stolen memory");
> -			break;
> -		case FBC_UNSUPPORTED_MODE:
> -			seq_puts(m, "mode not supported");
> -			break;
> -		case FBC_MODE_TOO_LARGE:
> -			seq_puts(m, "mode too large");
> -			break;
> -		case FBC_BAD_PLANE:
> -			seq_puts(m, "FBC unsupported on plane");
> -			break;
> -		case FBC_NOT_TILED:
> -			seq_puts(m, "scanout buffer not tiled");
> -			break;
> -		case FBC_MULTIPLE_PIPES:
> -			seq_puts(m, "multiple pipes are enabled");
> -			break;
> -		case FBC_MODULE_PARAM:
> -			seq_puts(m, "disabled per module param (default off)");
> -			break;
> -		case FBC_CHIP_DEFAULT:
> -			seq_puts(m, "disabled per chip default");
> -			break;
> -		case FBC_ROTATION:
> -			seq_puts(m, "rotation not supported");
> -			break;
> -		default:
> -			seq_puts(m, "unknown reason");
> -		}
> -		seq_putc(m, '\n');
> -	}
> +	else
> +		seq_printf(m, "FBC disabled: %s\n",
> +			  intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
>  
>  	if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
>  		seq_printf(m, "Compressing: %s\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b28029a..77f24e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,6 +1255,7 @@ 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);
> +const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>  
>  /* intel_hdmi.c */
>  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 43704a4..9b300bd 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -432,6 +432,39 @@ void intel_fbc_disable(struct drm_device *dev)
>  	dev_priv->fbc.crtc = NULL;
>  }
>  
> +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";
> +	default:
> +		MISSING_CASE(reason);
> +		return "unknown reason";
> +	}
> +}
> +
>  static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>  			      enum no_fbc_reason reason)
>  {
> @@ -439,6 +472,8 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>  		return false;
>  
>  	dev_priv->fbc.no_fbc_reason = reason;
> +	DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
> +
>  	return true;
>  }
>  
> @@ -459,8 +494,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  		if (intel_crtc_active(tmp_crtc) &&
>  		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
>  			if (one_pipe_only && crtc) {
> -				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
> -					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> +				set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES);
>  				return NULL;
>  			}
>  			crtc = tmp_crtc;
> @@ -471,8 +505,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  	}
>  
>  	if (!crtc || crtc->primary->fb == NULL) {
> -		if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
> -			DRM_DEBUG_KMS("no output, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT);
>  		return NULL;
>  	}
>  
> @@ -516,14 +549,12 @@ void intel_fbc_update(struct drm_device *dev)
>  		i915.enable_fbc = 0;
>  
>  	if (i915.enable_fbc < 0) {
> -		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> -			DRM_DEBUG_KMS("disabled per chip default\n");
> +		set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT);
>  		goto out_disable;
>  	}
>  
>  	if (!i915.enable_fbc) {
> -		if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
> -			DRM_DEBUG_KMS("fbc disabled per module param\n");
> +		set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM);
>  		goto out_disable;
>  	}
>  
> @@ -547,9 +578,7 @@ void intel_fbc_update(struct drm_device *dev)
>  
>  	if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
>  	    (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
> -		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> -			DRM_DEBUG_KMS("mode incompatible with compression, "
> -				      "disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE);
>  		goto out_disable;
>  	}
>  
> @@ -565,14 +594,12 @@ void intel_fbc_update(struct drm_device *dev)
>  	}
>  	if (intel_crtc->config->pipe_src_w > max_width ||
>  	    intel_crtc->config->pipe_src_h > max_height) {
> -		if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
> -			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE);
>  		goto out_disable;
>  	}
>  	if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
>  	    intel_crtc->plane != PLANE_A) {
> -		if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
> -			DRM_DEBUG_KMS("plane not A, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_BAD_PLANE);
>  		goto out_disable;
>  	}
>  
> @@ -581,14 +608,12 @@ void intel_fbc_update(struct drm_device *dev)
>  	 */
>  	if (obj->tiling_mode != I915_TILING_X ||
>  	    obj->fence_reg == I915_FENCE_REG_NONE) {
> -		if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
> -			DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_NOT_TILED);
>  		goto out_disable;
>  	}
>  	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>  	    crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
> -		if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
> -			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
> +		set_no_fbc_reason(dev_priv, FBC_ROTATION);
>  		goto out_disable;
>  	}
>  
> @@ -598,8 +623,7 @@ void intel_fbc_update(struct drm_device *dev)
>  
>  	if (i915_gem_stolen_setup_compression(dev, obj->base.size,
>  					      drm_format_plane_cpp(fb->pixel_format, 0))) {
> -		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
> -			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
> +		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
>  		goto out_disable;
>  	}
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2015-06-12 12:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-11 19:02 [PATCH 0/4] FBC trivial patches Paulo Zanoni
2015-06-11 19:02 ` [PATCH 1/4] drm/i915: print FBC compression status on debugfs Paulo Zanoni
2015-06-12 12:01   ` Ville Syrjälä
2015-06-11 19:02 ` [PATCH 2/4] drm/i915: add FBC_ROTATION to enum no_fbc_reason Paulo Zanoni
2015-06-11 19:02 ` [PATCH 3/4] drm/i915: unify no_fbc_reason message printing Paulo Zanoni
2015-06-12  9:02   ` Chris Wilson
2015-06-12 12:17   ` Damien Lespiau [this message]
2015-06-11 19:02 ` [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+ Paulo Zanoni
2015-06-12  9:00   ` Chris Wilson
2015-06-15  6:16   ` shuang.he
2015-06-12  9:03 ` [PATCH 0/4] FBC trivial patches Chris Wilson
2015-06-12 17:36 ` [PATCH 0/4] FBC trivial patches, V2 Paulo Zanoni
2015-06-12 17:36   ` [PATCH 1/4] drm/i915: print FBC compression status on debugfs Paulo Zanoni
2015-06-12 17:36   ` [PATCH 2/4] drm/i915: add FBC_ROTATION to enum no_fbc_reason Paulo Zanoni
2015-06-12 17:36   ` [PATCH 3/4] drm/i915: unify no_fbc_reason message printing Paulo Zanoni
2015-06-12 17:36   ` [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+ Paulo Zanoni
2015-06-12 17:42   ` [PATCH 0/4] FBC trivial patches, V2 Chris Wilson
2015-06-15 16:36     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150612121713.GA28224@strange.ger.corp.intel.com \
    --to=damien.lespiau@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    --cc=przanoni@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox