* [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()
@ 2017-02-22 2:25 Weinan Li
2017-02-22 4:22 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-02-22 7:54 ` [PATCH] " Imre Deak
0 siblings, 2 replies; 6+ messages in thread
From: Weinan Li @ 2017-02-22 2:25 UTC (permalink / raw)
To: intel-gfx, intel-gvt-dev
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@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);
+ return (!*status) && ((val & reply_mask) == reply);
}
/**
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: check status and reply value both in skl_pcode_try_request()
2017-02-22 2:25 [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request() Weinan Li
@ 2017-02-22 4:22 ` Patchwork
2017-02-22 7:54 ` [PATCH] " Imre Deak
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-02-22 4:22 UTC (permalink / raw)
To: Li, Weinan Z; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: check status and reply value both in skl_pcode_try_request()
URL : https://patchwork.freedesktop.org/series/20032/
State : success
== Summary ==
Series 20032v1 drm/i915: check status and reply value both in skl_pcode_try_request()
https://patchwork.freedesktop.org/api/1.0/series/20032/revisions/1/mbox/
fi-bdw-5557u total:253 pass:242 dwarn:0 dfail:0 fail:0 skip:11
fi-bsw-n3050 total:253 pass:214 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-j4205 total:253 pass:234 dwarn:0 dfail:0 fail:0 skip:19
fi-bxt-t5700 total:83 pass:70 dwarn:0 dfail:0 fail:0 skip:12
fi-byt-j1900 total:253 pass:226 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:253 pass:222 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:253 pass:237 dwarn:0 dfail:0 fail:0 skip:16
fi-hsw-4770r total:253 pass:237 dwarn:0 dfail:0 fail:0 skip:16
fi-ilk-650 total:253 pass:203 dwarn:0 dfail:0 fail:0 skip:50
fi-ivb-3520m total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18
fi-ivb-3770 total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18
fi-kbl-7500u total:253 pass:235 dwarn:0 dfail:0 fail:0 skip:18
fi-skl-6260u total:253 pass:243 dwarn:0 dfail:0 fail:0 skip:10
fi-skl-6700hq total:253 pass:236 dwarn:0 dfail:0 fail:0 skip:17
fi-skl-6700k total:253 pass:231 dwarn:4 dfail:0 fail:0 skip:18
fi-skl-6770hq total:253 pass:243 dwarn:0 dfail:0 fail:0 skip:10
fi-snb-2520m total:253 pass:225 dwarn:0 dfail:0 fail:0 skip:28
fi-snb-2600 total:253 pass:224 dwarn:0 dfail:0 fail:0 skip:29
e922d8924920be31fc76f5813bc1fde5d6cbf950 drm-tip: 2017y-02m-21d-16h-18m-14s UTC integration manifest
ec8e9c3 drm/i915: check status and reply value both in skl_pcode_try_request()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3921/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()
2017-02-22 2:25 [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request() Weinan Li
2017-02-22 4:22 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-02-22 7:54 ` Imre Deak
2017-02-22 8:17 ` Li, Weinan Z
1 sibling, 1 reply; 6+ messages in thread
From: Imre Deak @ 2017-02-22 7:54 UTC (permalink / raw)
To: Weinan Li; +Cc: intel-gfx, intel-gvt-dev
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@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);
> + 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().
--Imre
>
> /**
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()
2017-02-22 7:54 ` [PATCH] " Imre Deak
@ 2017-02-22 8:17 ` Li, Weinan Z
2017-02-22 8:56 ` Imre Deak
0 siblings, 1 reply; 6+ messages in thread
From: Li, Weinan Z @ 2017-02-22 8:17 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx, intel-gvt-dev
> -----Original Message-----
> From: Deak, Imre
> Sent: Wednesday, February 22, 2017 3:54 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@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@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()
2017-02-22 8:17 ` Li, Weinan Z
@ 2017-02-22 8:56 ` Imre Deak
2017-02-23 0:53 ` Li, Weinan Z
0 siblings, 1 reply; 6+ messages in thread
From: Imre Deak @ 2017-02-22 8:56 UTC (permalink / raw)
To: Li, Weinan Z; +Cc: intel-gfx, intel-gvt-dev
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@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@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@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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request()
2017-02-22 8:56 ` Imre Deak
@ 2017-02-23 0:53 ` Li, Weinan Z
0 siblings, 0 replies; 6+ messages in thread
From: Li, Weinan Z @ 2017-02-23 0:53 UTC (permalink / raw)
To: Deak, Imre; +Cc: intel-gfx, intel-gvt-dev
Thanks Imre. I see, it's a little hard to read, need to check the final state across 2 level function return value.
Thanks.
Best Regards.
Weinan, LI
> -----Original Message-----
> From: Deak, Imre
> Sent: Wednesday, February 22, 2017 4:56 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@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: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@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org;
> > > intel-gvt-dev@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@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.
>
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-23 0:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-22 2:25 [PATCH] drm/i915: check status and reply value both in skl_pcode_try_request() Weinan Li
2017-02-22 4:22 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-02-22 7:54 ` [PATCH] " Imre Deak
2017-02-22 8:17 ` Li, Weinan Z
2017-02-22 8:56 ` Imre Deak
2017-02-23 0:53 ` Li, Weinan Z
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.