On Thu, Apr 21, 2016 at 04:43:19PM +0100, Robert Bragg wrote:
> On Wed, Apr 20, 2016 at 11:52 PM, Chris Wilson
> <[1]chris@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 20, 2016 at 03:23:10PM +0100, Robert Bragg wrote:
> > +static int i915_oa_read(struct i915_perf_stream *stream,
> > + struct i915_perf_read_state *read_state)
> > +{
> > + struct drm_i915_private *dev_priv = stream->dev_priv;
> > +
> > + return dev_priv->perf.oa.ops.read(stream, read_state);
> > +}
>
> > + stream->destroy = i915_oa_stream_destroy;
> > + stream->enable = i915_oa_stream_enable;
> > + stream->disable = i915_oa_stream_disable;
> > + stream->can_read = i915_oa_can_read;
> > + stream->wait_unlocked = i915_oa_wait_unlocked;
> > + stream->poll_wait = i915_oa_poll_wait;
> > + stream->read = i915_oa_read;
>
> Why aren't these a const ops table?
>
> No particular reason; I guess it just seemed straightforward enough at the
> time. I suppose it avoids some redundant pointer indirection and could
> suit defining streams in the future that might find it awkward to have
> static ops (don't have anything like that in mind though) but it's at the
> expense of a slightly larger stream struct (though also don't see that as
> a concern currently).
>
> Can change if you like.
I think it is safe to say it is considered best practice to have vfunc
tables in read-only memory. Certainly raises an eyebrow when they look
like they could be modified on the fly.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
