Re: [PATCH 5/9] drm/i915: Enable i915 perf stream for Haswell OA unit

On Wed, Apr 20, 2016 at 03:23:10PM +0100, Robert Bragg wrote:

+static void i915_oa_stream_enable(struct i915_perf_stream *stream) +{

• struct drm_i915_private *dev_priv = stream->dev_priv;
• dev_priv->perf.oa.ops.oa_enable(dev_priv);
• if (dev_priv->perf.oa.periodic)

• hrtimer_start(&dev_priv->perf.oa.poll_check_timer,
  

• 	      ns_to_ktime(POLL_PERIOD),
  

• 	      HRTIMER_MODE_REL_PINNED);
  

+}

+static void i915_oa_stream_disable(struct i915_perf_stream *stream) +{

• struct drm_i915_private *dev_priv = stream->dev_priv;
• dev_priv->perf.oa.ops.oa_disable(dev_priv);
• if (dev_priv->perf.oa.periodic)

• hrtimer_cancel(&dev_priv->perf.oa.poll_check_timer);
  

+}

+static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer) +{

• struct drm_i915_private *dev_priv =

• container_of(hrtimer, typeof(*dev_priv),
  

• 	     perf.oa.poll_check_timer);
  

• if (!dev_priv->perf.oa.ops.oa_buffer_is_empty(dev_priv))

• wake_up(&dev_priv->perf.oa.poll_wq);
  

• hrtimer_forward_now(hrtimer, ns_to_ktime(POLL_PERIOD));
• return HRTIMER_RESTART;

+}

@@ -424,8 +1313,37 @@ void i915_perf_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev);

• if (!IS_HASWELL(dev))

• return;
  

• hrtimer_init(&dev_priv->perf.oa.poll_check_timer,

•      CLOCK_MONOTONIC, HRTIMER_MODE_REL);
  

• dev_priv->perf.oa.poll_check_timer.function = oa_poll_check_timer_cb;
• init_waitqueue_head(&dev_priv->perf.oa.poll_wq);

This timer only serves to wake up pollers / wait_unlocked, right? So why is it always running (when the stream is enabled)?

What happens to poll / wait_unlocked if oa.periodic is not set? It seems like those functions would block indefinitely. -Chris