drm/i915: Include register polling in reg_rw traces
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Mon, 4 Feb 2019 21:16:44 +0000 (23:16 +0200)
committerVille Syrjälä <ville.syrjala@linux.intel.com>
Tue, 5 Feb 2019 18:44:43 +0000 (20:44 +0200)
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.

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.

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@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190204211644.21967-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
drivers/gpu/drm/i915/i915_drv.c
drivers/gpu/drm/i915/intel_dp.c
drivers/gpu/drm/i915/intel_uncore.c

index f462a4d28af4a03350fecb275aebe915f5a6d131..349ba9e23a5b5b0bd8141ec1c7ed4a16e9011e0e 100644 (file)
@@ -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);
+
+       return ret;
 }
 
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
index d9bc8ee523007f73084f70f660ed757aa7db810b..a15d9fd80641194e85b762ab761577a594abf6a7 100644 (file)
@@ -1061,6 +1061,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
        done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
                                  msecs_to_jiffies_timeout(10));
+
+       /* just trace the final value */
+       trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
+
        if (!done)
                DRM_ERROR("dp aux hw did not signal timeout!\n");
 #undef C
@@ -1227,6 +1231,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
                        break;
                msleep(1);
        }
+       /* just trace the final value */
+       trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
 
        if (try == 3) {
                static u32 last_status = -1;
index e88f0252d77e74c19b1f8c8cf257dd3ac80cac00..75646a1e0051c336b386c60057abaa9807c9c57e 100644 (file)
@@ -1819,6 +1819,9 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
                                 (reg_value & mask) == value,
                                 slow_timeout_ms * 1000, 10, 1000);
 
+       /* just trace the final value */
+       trace_i915_reg_rw(false, reg, reg_value, sizeof(reg_value), true);
+
        if (out_value)
                *out_value = reg_value;