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

[Robert Bragg]

On Thu, Apr 21, 2016 at 12:09 AM, Chris Wilson <chris at chris-wilson.co.uk>
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160421/e3d73dd4/attachment.html>