[Intel-gfx] [PATCH v4] drm/i915: Add IOCTL Param to control data port coherency.

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-07-09 17:28:02)
> 
> On 09/07/2018 14:20, Tomasz Lis wrote:
> > +static int i915_gem_context_clear_data_port_coherent(struct i915_gem_context *ctx)
> > +{
> > +     int ret;
> > +
> > +     ret = intel_lr_context_modify_data_port_coherency(ctx, false);
> > +     if (!ret)
> > +             __clear_bit(CONTEXT_DATA_PORT_COHERENT, &ctx->flags);
> > +     return ret;
> 
> Is there a good reason you allow userspace to keep emitting unlimited 
> number of commands which actually do not change the status? If not 
> please consider gating the command emission with 
> test_and_set_bit/test_and_clear_bit. Hm.. apart even with that they 
> could keep toggling ad infinitum with no real work in between. Has it 
> been considered to only save the desired state in set param and then 
> emit it, if needed, before next execbuf? Minor thing in any case, just 
> curious since I wasn't following the threads.

The first patch tried to add a bit to execbuf, and having been
mistakenly down that road before, we asked if there was any alternative.
(Now if you've also been following execbuf3 conversations, having a
packet for privileged LRI is definitely something we want.)

Setting the value in the context register is precisely what we want to
do, and trivially serialised with execbuf since we have to serialise
reservation of ring space, i.e. the normal rules of request generation.
(execbuf is just a client and nothing special). From that point of view,
we only care about frequency, if it is very frequent it should be
controlled by userspace inside the batch (but it can't due to there
being dangerous bits inside the reg aiui). At the other end of the
scale, is context_setparam for set-once. And there should be no
inbetween as that requires costly batch flushes.
-Chris