All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Rename __force_wake_get to __force_wake_auto
Date: Thu, 24 Mar 2016 15:19:40 +0000	[thread overview]
Message-ID: <56F4058C.70904@linux.intel.com> (raw)
In-Reply-To: <1458829907-26596-1-git-send-email-chris@chris-wilson.co.uk>


On 24/03/16 14:31, Chris Wilson wrote:
> __force_wake_get() only acquire a temporary wakeref on forcewake that is
> automatically releases when a timer expires. When reading the code
> again, I confused __intel_uncore_forcewake_get for __force_wake_get and
> to my shame thought I found a bug in an unbalanced wake_count handling.
>
> I claim that if the function had been called __force_wake_auto instead I
> would not have embarrassed myself.

Confused myself more than once so on the basis on that I think it is a 
good naming distinction to make.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 96799392c2c7..165ebce5b638 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -716,8 +716,8 @@ __gen2_read(64)
>   	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
>   	return val
>
> -static inline void __force_wake_get(struct drm_i915_private *dev_priv,
> -				    enum forcewake_domains fw_domains)
> +static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
> +				     enum forcewake_domains fw_domains)
>   {
>   	struct intel_uncore_forcewake_domain *domain;
>   	enum forcewake_domain_id id;
> @@ -745,7 +745,7 @@ static u##x \
>   gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
>   	GEN6_READ_HEADER(x); \
>   	if (NEEDS_FORCE_WAKE(offset)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +		__force_wake_auto(dev_priv, FORCEWAKE_RENDER); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
>   	GEN6_READ_FOOTER; \
>   }
> @@ -762,7 +762,7 @@ vlv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
>   	else if (FORCEWAKE_VLV_MEDIA_RANGE_OFFSET(offset)) \
>   		fw_engine = FORCEWAKE_MEDIA; \
>   	if (fw_engine) \
> -		__force_wake_get(dev_priv, fw_engine); \
> +		__force_wake_auto(dev_priv, fw_engine); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
>   	GEN6_READ_FOOTER; \
>   }
> @@ -781,7 +781,7 @@ chv_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
>   	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
>   		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
>   	if (fw_engine) \
> -		__force_wake_get(dev_priv, fw_engine); \
> +		__force_wake_auto(dev_priv, fw_engine); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
>   	GEN6_READ_FOOTER; \
>   }
> @@ -805,7 +805,7 @@ gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
>   	else \
>   		fw_engine = FORCEWAKE_BLITTER; \
>   	if (fw_engine) \
> -		__force_wake_get(dev_priv, fw_engine); \
> +		__force_wake_auto(dev_priv, fw_engine); \
>   	val = __raw_i915_read##x(dev_priv, reg); \
>   	GEN6_READ_FOOTER; \
>   }
> @@ -969,7 +969,7 @@ static void \
>   gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
>   	GEN6_WRITE_HEADER; \
>   	if (NEEDS_FORCE_WAKE(offset) && !is_gen8_shadowed(dev_priv, reg)) \
> -		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
> +		__force_wake_auto(dev_priv, FORCEWAKE_RENDER); \
>   	__raw_i915_write##x(dev_priv, reg, val); \
>   	GEN6_WRITE_FOOTER; \
>   }
> @@ -989,7 +989,7 @@ chv_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
>   	else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(offset)) \
>   		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
>   	if (fw_engine) \
> -		__force_wake_get(dev_priv, fw_engine); \
> +		__force_wake_auto(dev_priv, fw_engine); \
>   	__raw_i915_write##x(dev_priv, reg, val); \
>   	GEN6_WRITE_FOOTER; \
>   }
> @@ -1036,7 +1036,7 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \
>   	else \
>   		fw_engine = FORCEWAKE_BLITTER; \
>   	if (fw_engine) \
> -		__force_wake_get(dev_priv, fw_engine); \
> +		__force_wake_auto(dev_priv, fw_engine); \
>   	__raw_i915_write##x(dev_priv, reg, val); \
>   	GEN6_WRITE_FOOTER; \
>   }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2016-03-24 15:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-24 14:31 [PATCH] drm/i915: Rename __force_wake_get to __force_wake_auto Chris Wilson
2016-03-24 15:12 ` Dave Gordon
2016-03-24 15:28   ` Chris Wilson
2016-03-24 15:19 ` Tvrtko Ursulin [this message]
2016-03-24 16:02 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-03-30 10:19   ` Mika Kuoppala
2016-03-31 11:13 ` ✗ Fi.CI.BAT: failure " Patchwork

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=56F4058C.70904@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.