[Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines

[Umesh Nerlige Ramappa]

On Thu, Feb 16, 2023 at 09:30:57AM -0800, Umesh Nerlige Ramappa wrote:
>On Wed, Feb 15, 2023 at 07:58:02PM -0800, Dixit, Ashutosh wrote:
>>On Tue, 14 Feb 2023 16:54:12 -0800, Umesh Nerlige Ramappa wrote:
>>>
>>>Add helper to check for supported OA engines.
>>>
>>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>>>---
>>> drivers/gpu/drm/i915/i915_perf.c | 19 ++++++++++++++++---
>>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>>>index 393a0da8b7c8..a879ae4bf8d7 100644
>>>--- a/drivers/gpu/drm/i915/i915_perf.c
>>>+++ b/drivers/gpu/drm/i915/i915_perf.c
>>>@@ -1570,6 +1570,19 @@ free_noa_wait(struct i915_perf_stream *stream)
>>>	i915_vma_unpin_and_release(&stream->noa_wait, 0);
>>> }
>>>
>>>+static bool engine_supports_oa(const struct intel_engine_cs *engine)
>>>+{
>>>+	enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
>>>+
>>>+	if (intel_engine_is_virtual(engine))
>>>+		return false;
>>
>>Let's move this check to a different (or a separate) patch (with
>>explanation about OA and virtual engines in case of separate patch). It is
>>strange to see this check in this patch since previously we have only
>>supported render (I think there is only a single render engine so no
>>virtual render engines are possible, correct?).
>
>will do.
>
>>
>>With the changes above this is:
>>
>>Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
>>
>>Is there anything intrinsically wrong with virtual engines that we cannot
>>get OA data for them? Since we get OA data from an OA unit not engines so
>>wondering why this restriction. So if I have a virtual engine consisting of
>>two VDBOX engines attached to a single OAM unit we cannot get OA data for
>>this virtual engine?
>>
>>Or is just that we haven't handled such cases? In that case it is fine to
>>disallow virtual engines till we can support them. Sorry just trying to
>>understand the restriction.
>
>iirc, we cannot modify the context image for the virtual engine.  
>Something to do with lrc state for such engines. Also OA supports only 
>a concept of one engine instance passed to it, so a virtual engine 
>does not fit the interface definition.

Actually, it's not the lrc state, but getting engine_pm wakeref for 
virtual engine was failing, so we had to use this check, but at this 
moment the check is not required in OA like you said - there's just one 
render.

Thanks,
Umesh
>
>Thanks,
>Umesh