Quoting Sean Paul (2017-12-05 05:15:01)
> This patch adds a little more control to a couple wait_for routines such
> that we can avoid open-coding read/wait/timeout patterns which:
> - need the value of the register after the wait_for
> - run arbitrary operation for the read portion
>
> This patch also chooses the correct sleep function (based on
> timers-howto.txt) for the polling interval the caller specifies.
>
> Changes in v2:
> - Added to the series
> Changes in v3:
> - Rebased on drm-intel-next-queued and the new Wmin/max _wait_for
> - Removed msleep option
>
> Suggested-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Signed-off-by: Sean Paul <seanp...@chromium.org>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 17 ++++++++++-------
> drivers/gpu/drm/i915/intel_uncore.c | 23 ++++++++++++++++-------
> drivers/gpu/drm/i915/intel_uncore.h | 14 +++++++++++++-
> 3 files changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 64426d3e078e..852b3d161754 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -41,20 +41,21 @@
> #include <drm/drm_atomic.h>
>
> /**
> - * _wait_for - magic (register) wait macro
> + * __wait_for - magic wait macro
> *
> - * Does the right thing for modeset paths when run under kdgb or similar
> atomic
> - * contexts. Note that it's important that we check the condition again after
> - * having timed out, since the timeout could be due to preemption or similar
> and
> - * we've never had a chance to check the condition before the timeout.
> + * Macro to help avoid open coding check/wait/timeout patterns. Note that
> it's
> + * important that we check the condition again after having timed out, since
> the
> + * timeout could be due to preemption or similar and we've never had a
> chance to
> + * check the condition before the timeout.
> */
> -#define _wait_for(COND, US, Wmin, Wmax) ({ \
> +#define __wait_for(OP, COND, US, Wmin, Wmax) ({ \
> unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \
> long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \
> int ret__; \
> might_sleep(); \
> for (;;) { \
> bool expired__ = time_after(jiffies, timeout__); \
> + OP; \
> if (COND) { \
> ret__ = 0; \
> break; \
> @@ -70,7 +71,9 @@
> ret__; \
> })
>
> -#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000)
> +#define _wait_for(COND, US, Wmin, Wmax) __wait_for(;, (COND), (US),
> (Wmin), \
> + (Wmax))
Hmm, doesn't an empty OP (__wait_for(, ...)) work?
> +int __intel_wait_for_register(struct drm_i915_private *dev_priv,
> i915_reg_t reg,
> u32 mask,
> u32 value,
> - unsigned int timeout_ms)
> + unsigned int fast_timeout_us,
> + unsigned int slow_timeout_ms,
> + u32 *out_value)
> {
> unsigned fw =
> intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ);
> int ret;
> + u32 reg_value;
Before int ret; Try to avoid building a Christmas tree if possible.
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
