diff mbox

[01/14] drm/i915/guc: Do not use 0 for GuC doorbell cookie

Message ID 20171019183619.6235-2-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Oct. 19, 2017, 6:36 p.m. UTC
Apparently, this value is reserved and may be interpreted as changing
doorbell ownership. Even though we're not observing any side effects
now, let's skip over it to be consistent with the spec.

Suggested-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

sagar.a.kamble@intel.com Oct. 21, 2017, 6:28 a.m. UTC | #1
On 10/20/2017 12:06 AM, Michał Winiarski wrote:
> Apparently, this value is reserved and may be interpreted as changing
> doorbell ownership. Even though we're not observing any side effects
> now, let's skip over it to be consistent with the spec.
>
> Suggested-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a2e8114b739d..02ca2a412c62 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -475,9 +475,10 @@ static void guc_ring_doorbell(struct i915_guc_client *client)
>   	/* pointer of current doorbell cacheline */
>   	db = __get_doorbell(client);
>   
> -	/* we're not expecting the doorbell cookie to change behind our back */
> +	/* we're not expecting the doorbell cookie to change behind our back,
> +	 * we also need to treat 0 as a reserved value */
Need to fix /* & */.
Change looks good to me.
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@gmail.com>
>   	cookie = READ_ONCE(db->cookie);
> -	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1) != cookie);
> +	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1 ?: cookie + 2) != cookie);
>   
>   	/* XXX: doorbell was lost and need to acquire it again */
>   	GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a2e8114b739d..02ca2a412c62 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -475,9 +475,10 @@  static void guc_ring_doorbell(struct i915_guc_client *client)
 	/* pointer of current doorbell cacheline */
 	db = __get_doorbell(client);
 
-	/* we're not expecting the doorbell cookie to change behind our back */
+	/* we're not expecting the doorbell cookie to change behind our back,
+	 * we also need to treat 0 as a reserved value */
 	cookie = READ_ONCE(db->cookie);
-	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1) != cookie);
+	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1 ?: cookie + 2) != cookie);
 
 	/* XXX: doorbell was lost and need to acquire it again */
 	GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);