On Thu, Apr 21, 2016 at 12:09 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
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.
Right, it's unecessary. I'll look at limitting it to just while polling or for blocking reads.
Good point about the blocking case too.
I just started testing that scenario yesterday, writting an MI_RPC unit test which opens a stream without requesting periodic sampling, but didn't poll or read in that case so far so didn't hit this yet.
At least for the read() this is partially considered by returning -EIO if attempting a blocking read while the stream is disabled, but it doesn't consider the case that the stream is enabled but periodic sampling isn't enabled.
Regards,
- Robert
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
