[Intel-gfx] [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()

[Li, Weinan Z]

> -----Original Message-----
> From: Deak, Imre
> Sent: Wednesday, February 22, 2017 3:54 PM
> To: Li, Weinan Z <weinan.z.li at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; intel-gvt-dev at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: check status and reply value both
> in skl_pcode_try_request()
> 
> On Wed, Feb 22, 2017 at 10:25:44AM +0800, Weinan Li wrote:
> > skl_pcode_try_request() call sandybridge_pcode_read(), check both
> > return status and value simultanously, ensure it got correct value without
> error.
> >
> > Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c index ae2c0bb..e7b12ec 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -7882,7 +7882,7 @@ static bool skl_pcode_try_request(struct
> > drm_i915_private *dev_priv, u32 mbox,
> >
> >  	*status = sandybridge_pcode_read(dev_priv, mbox, &val);
> >
> > -	return *status || ((val & reply_mask) == reply);
*status == 0 means success, otherwise error happened. 
> > +	return (!*status) && ((val & reply_mask) == reply);
> >  }
> 
> The original looks ok to me. The condition becomes true if PCODE reports an
> error in *status or we get the expected reply. *status is then rechecked in
> skl_pcode_request().
int skl_pcode_request(struct drm_i915_private *dev_priv, u32 mbox, u32 request,
		      u32 reply_mask, u32 reply, int timeout_base_ms)
{
	u32 status;
	int ret;

	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));

#define COND skl_pcode_try_request(dev_priv, mbox, request, reply_mask, reply, \
				   &status)

	/*
	 * Prime the PCODE by doing a request first. Normally it guarantees
	 * that a subsequent request, at most @timeout_base_ms later, succeeds.
	 * _wait_for() doesn't guarantee when its passed condition is evaluated
	 * first, so send the first request explicitly.
	 */
	if (COND) {##here will deal as success although pcode_read() get error happened.
		            Is this expected behavior?##
		ret = 0;
		goto out;
	}
> 
> --Imre
> 
> >
> >  /**
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx