Quoting Ville Syrjala (2019-02-04 21:16:44)
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We generally omit register polling from the i915_reg_rw tracepoint.
> Understandable since polling could generate a lot of noise in the
> trace. The downside is that the trace is incomplete. As a compromise
> let's trace the final register value observed while polling. That
> should be generally sufficient to observe what the code should be
> doing next.
Fair compromise; I admit the subject line had me freaking out.
> I suppose in some cases it might make sense to also trace the initial
> register value, and maybe the number of times we polled. But that
> would require a separate tracepoint so let's leave it for the future.
I postulate when you get down to wanting that amount of detail, you
throw in dedicated debug.
> The other users of _NOTRACE() are i915_pmu and i2c bitbanging,
> which I decided to leave alone.
>
> Next we should do something to claw back the tracepoints for
> planes and whatnot which were switched to _FW() a while back.
> I guess just new macros for raw_rw+trace. The question is
> what to call it?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++++--
> drivers/gpu/drm/i915/intel_dp.c | 6 ++++++
> drivers/gpu/drm/i915/intel_uncore.c | 3 +++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7aaa1ac4c99..7de90701f6f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2543,6 +2543,10 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
> static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
> u32 mask, u32 val)
> {
> + i915_reg_t reg = VLV_GTLC_PW_STATUS;
> + u32 reg_value;
> + int ret;
> +
> /* The HW does not like us polling for PW_STATUS frequently, so
> * use the sleeping loop rather than risk the busy spin within
> * intel_wait_for_register().
> @@ -2550,8 +2554,12 @@ static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
> * Transitioning between RC6 states should be at most 2ms (see
> * valleyview_enable_rps) so use a 3ms timeout.
> */
> - return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val,
> - 3);
> + ret = wait_for(((reg_value = I915_READ_NOTRACE(reg)) & mask) == val, 3);
> +
> + /* just trace the final value */
> + trace_i915_reg_rw(false, reg, reg_value, sizeof(reg_value), true);
I feel like there's a pattern here that could benefit from a convenient
macro. Nevertheless,
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
