Re: [PATCH v2 1/9] drm/i915/perf: store the associated engine of a stream

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/10/2019 17:10, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-10-09 14:48:42)
On 09/10/2019 16:40, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-10-09 14:34:49)
On 09/10/2019 15:45, Chris Wilson wrote:
Quoting Lionel Landwerlin (2019-10-09 13:43:32)
Do you have branch somewhere with this series?
https://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=wip-perf
-Chris

Cheers,


I've modified the top patch to set the nopreempt flag for as long as the
context has been flagged (as long at the perf stream is opened) :
https://github.com/djdeath/linux/commit/d3327b30c6141fac98a3d46f3398c87fe70976aa
That means you are not passing in the ext_perf_config to every batch that
is using it, right? The oa_config tracking also hinges on that you do.
-Chris

Like I mentioned, there are empty batch to drain the context that we
emit without OA reconfiguration.

There could also be a sequence such as :

      - batch0 (includes perf query config=42)

      - batch1 (no perf query, includes timestamp or pipeline query)
But this execbuf is not using the oa_config, so why should it?


I was just working about the problem I guess.

With the patch you sent earlier to not merge request with different flags, that's probably useless.


  The
direction you've suggested is that we should attach the perf state to
the context.

eb_oa_config():
	struct i915_perf_stream *stream;

	/* attached/removed by perf_ioctl */
	stream = rcu_get(eb->gem_context->perf);
	if (!stream)
		return;

	if (stream->oa_config != stream->perf->oa_config) {
		mutex_lock(&perf->lock);
		...
		mutex_unlock(&perf->lock);
	}

	if (stream->hold_preemption)
		eb->request->flags |= NOPREEMPT;
	
	i915_perf_stream_put(stream);

Then instead of an execbuf extension it would either be a perf ioctl to
update the oa_config on the attached specified context, or a context
param. Using the perf ioctl does not seem amiss.

      - batch2 (includes perf query config=42)

      - batch3 (includes perf query config=43)


It sounds reasonable to ensure that all the requests are flagged with
nopreempt to ensure we don't preempt one because we don't have
visibility on what's completed when reloading the execlists ports.

This entire sequence above would be surrounded by open/close of the perf
stream. Once close() returns, then any new request won't be flagged with
nopreempt but it's the application's responsability to have collected
all the queries' results before closing the stream.
Are you happy with associating the i915_perf_stream with the
specific_ctx and controlling all the parameters via perf-ioctl?
-Chris


Yeah sounds like it should work, I would like to test the whole setup.

If you can share the patches changing the config through a perf stream ioctl, I'll update my driver and test.


Thanks a lot,


-Lionel

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux