On Wed, Feb 22, 2017 at 10:17:21AM +0200, Li, Weinan Z wrote: > > -----Original Message----- > > From: Deak, Imre > > Sent: Wednesday, February 22, 2017 3:54 PM > > To: Li, Weinan Z <weinan.z.li@xxxxxxxxx> > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx > > Subject: Re: [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@xxxxxxxxx> > > > --- > > > 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. Yes. > > > + 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?## It's not regarded as success, it's regarded as a condition to end the polling. That is either a PCODE error returned in status or the expected reply received (matching reply_mask/reply). status is rechecked at the end of the function. --Imre > ret = 0; > goto out; > } _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx