* [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 3:56 ` Dixit, Ashutosh
2023-02-15 0:54 ` [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines Umesh Nerlige Ramappa
` (9 subsequent siblings)
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
From: Chris Wilson <chris.p.wilson@linux.intel.com>
If we fail to adjust the GuC run-control on opening the perf stream,
make sure we unwind the wakeref just taken.
Fixes: 01e742746785 ("drm/i915/guc: Support OA when Wa_16011777198 is enabled")
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 18 +++++++++++-------
drivers/gpu/drm/i915/i915_perf_types.h | 6 ++++++
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 824a34ec0b83..393a0da8b7c8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1592,9 +1592,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
/*
* Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
*/
- if (intel_uc_uses_guc_rc(>->uc) &&
- (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
- IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))
+ if (stream->override_gucrc)
drm_WARN_ON(>->i915->drm,
intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc));
@@ -3305,13 +3303,15 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
if (ret) {
drm_dbg(&stream->perf->i915->drm,
"Unable to override gucrc mode\n");
- goto err_config;
+ goto err_fw;
}
+
+ stream->override_gucrc = true;
}
ret = alloc_oa_buffer(stream);
if (ret)
- goto err_oa_buf_alloc;
+ goto err_gucrc;
stream->ops = &i915_oa_stream_ops;
@@ -3344,12 +3344,16 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
free_oa_buffer(stream);
-err_oa_buf_alloc:
- free_oa_configs(stream);
+err_gucrc:
+ if (stream->override_gucrc)
+ intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc);
+err_fw:
intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
intel_engine_pm_put(stream->engine);
+ free_oa_configs(stream);
+
err_config:
free_noa_wait(stream);
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index ca150b7af3f2..e36f046fe2b6 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -316,6 +316,12 @@ struct i915_perf_stream {
* buffer should be checked for available data.
*/
u64 poll_oa_period;
+
+ /**
+ * @override_gucrc: GuC RC has been overridden for the perf stream,
+ * and we need to restore the default configuration on release.
+ */
+ bool override_gucrc:1;
};
/**
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error
2023-02-15 0:54 ` [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error Umesh Nerlige Ramappa
@ 2023-02-16 3:56 ` Dixit, Ashutosh
0 siblings, 0 replies; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 3:56 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Tue, 14 Feb 2023 16:54:11 -0800, Umesh Nerlige Ramappa wrote:
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 824a34ec0b83..393a0da8b7c8 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1592,9 +1592,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
> /*
> * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
> */
> - if (intel_uc_uses_guc_rc(>->uc) &&
> - (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> - IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))
> + if (stream->override_gucrc)
> drm_WARN_ON(>->i915->drm,
> intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc));
>
> @@ -3305,13 +3303,15 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> if (ret) {
> drm_dbg(&stream->perf->i915->drm,
> "Unable to override gucrc mode\n");
> - goto err_config;
> + goto err_fw;
> }
> +
> + stream->override_gucrc = true;
> }
>
> ret = alloc_oa_buffer(stream);
> if (ret)
> - goto err_oa_buf_alloc;
> + goto err_gucrc;
>
> stream->ops = &i915_oa_stream_ops;
>
> @@ -3344,12 +3344,16 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>
> free_oa_buffer(stream);
>
> -err_oa_buf_alloc:
> - free_oa_configs(stream);
> +err_gucrc:
> + if (stream->override_gucrc)
> + intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc);
>
> +err_fw:
> intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
> intel_engine_pm_put(stream->engine);
>
> + free_oa_configs(stream);
> +
> err_config:
> free_noa_wait(stream);
[nice-to-have] The previous naming scheme for labels in the function is the
place from which the goto is issued so err_fw should be named err_gucrc and
err_gucrc should be called the preevious name err_oa_buf_alloc but
otherwise the code seems correct so this is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 3:58 ` Dixit, Ashutosh
2023-02-15 0:54 ` [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch Umesh Nerlige Ramappa
` (8 subsequent siblings)
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Add helper to check for supported OA engines.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@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;
+
+ switch (platform) {
+ default:
+ return engine->class == RENDER_CLASS;
+ }
+}
+
static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
{
struct i915_perf *perf = stream->perf;
@@ -2505,7 +2518,7 @@ static int gen8_configure_context(struct i915_gem_context *ctx,
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
GEM_BUG_ON(ce == ce->engine->kernel_context);
- if (ce->engine->class != RENDER_CLASS)
+ if (!engine_supports_oa(ce->engine))
continue;
/* Otherwise OA settings will be set upon first use */
@@ -2656,7 +2669,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
for_each_uabi_engine(engine, i915) {
struct intel_context *ce = engine->kernel_context;
- if (engine->class != RENDER_CLASS)
+ if (!engine_supports_oa(ce->engine))
continue;
regs[0].value = intel_sseu_make_rpcs(engine->gt, &ce->sseu);
@@ -3369,7 +3382,7 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
{
struct i915_perf_stream *stream;
- if (engine->class != RENDER_CLASS)
+ if (!engine_supports_oa(engine))
return;
/* perf.exclusive_stream serialised by lrc_configure_all_contexts() */
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines
2023-02-15 0:54 ` [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines Umesh Nerlige Ramappa
@ 2023-02-16 3:58 ` Dixit, Ashutosh
2023-02-16 17:30 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 3:58 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
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@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?).
With the changes above this is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines
2023-02-16 3:58 ` Dixit, Ashutosh
@ 2023-02-16 17:30 ` Umesh Nerlige Ramappa
2023-02-17 0:53 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 17:30 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
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@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@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.
Thanks,
Umesh
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines
2023-02-16 17:30 ` Umesh Nerlige Ramappa
@ 2023-02-17 0:53 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-17 0:53 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
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@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@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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 2/9] drm/i915/perf: Add helper to check supported OA engines Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 5:08 ` Dixit, Ashutosh
2023-02-15 0:54 ` [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails Umesh Nerlige Ramappa
` (7 subsequent siblings)
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Validate the OA sseu config after all params are parsed.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index a879ae4bf8d7..0b2097ad000e 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3956,6 +3956,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
u64 __user *uprop = uprops;
u32 i;
int ret;
+ bool config_sseu = false;
+ struct drm_i915_gem_context_param_sseu user_sseu;
memset(props, 0, sizeof(struct perf_open_properties));
props->poll_oa_period = DEFAULT_POLL_PERIOD_NS;
@@ -4082,8 +4084,6 @@ static int read_properties_unlocked(struct i915_perf *perf,
props->hold_preemption = !!value;
break;
case DRM_I915_PERF_PROP_GLOBAL_SSEU: {
- struct drm_i915_gem_context_param_sseu user_sseu;
-
if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 50)) {
drm_dbg(&perf->i915->drm,
"SSEU config not supported on gfx %x\n",
@@ -4098,14 +4098,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
"Unable to copy global sseu parameter\n");
return -EFAULT;
}
-
- ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
- if (ret) {
- drm_dbg(&perf->i915->drm,
- "Invalid SSEU configuration\n");
- return ret;
- }
- props->has_sseu = true;
+ config_sseu = true;
break;
}
case DRM_I915_PERF_PROP_POLL_OA_PERIOD:
@@ -4125,6 +4118,15 @@ static int read_properties_unlocked(struct i915_perf *perf,
uprop += 2;
}
+ if (config_sseu) {
+ ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
+ if (ret) {
+ DRM_DEBUG("Invalid SSEU configuration\n");
+ return ret;
+ }
+ props->has_sseu = true;
+ }
+
return 0;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-15 0:54 ` [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch Umesh Nerlige Ramappa
@ 2023-02-16 5:08 ` Dixit, Ashutosh
2023-02-16 5:36 ` Dixit, Ashutosh
0 siblings, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 5:08 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
>
> Validate the OA sseu config after all params are parsed.
Commit messages for all patches need to answer the "why" or the reason for
the patch. In this case maybe an overkill but probably something like:
Validate the OA sseu config after all params are parsed since the engine
can be passed in as part of perf properties.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_perf.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index a879ae4bf8d7..0b2097ad000e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3956,6 +3956,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
> u64 __user *uprop = uprops;
> u32 i;
> int ret;
> + bool config_sseu = false;
> + struct drm_i915_gem_context_param_sseu user_sseu;
nit: longer lines above shorter lines
>
> memset(props, 0, sizeof(struct perf_open_properties));
> props->poll_oa_period = DEFAULT_POLL_PERIOD_NS;
> @@ -4082,8 +4084,6 @@ static int read_properties_unlocked(struct i915_perf *perf,
> props->hold_preemption = !!value;
> break;
> case DRM_I915_PERF_PROP_GLOBAL_SSEU: {
> - struct drm_i915_gem_context_param_sseu user_sseu;
> -
> if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 50)) {
> drm_dbg(&perf->i915->drm,
> "SSEU config not supported on gfx %x\n",
> @@ -4098,14 +4098,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
> "Unable to copy global sseu parameter\n");
> return -EFAULT;
> }
> -
> - ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
> - if (ret) {
> - drm_dbg(&perf->i915->drm,
> - "Invalid SSEU configuration\n");
> - return ret;
> - }
> - props->has_sseu = true;
> + config_sseu = true;
> break;
> }
> case DRM_I915_PERF_PROP_POLL_OA_PERIOD:
> @@ -4125,6 +4118,15 @@ static int read_properties_unlocked(struct i915_perf *perf,
> uprop += 2;
> }
>
> + if (config_sseu) {
> + ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
> + if (ret) {
> + DRM_DEBUG("Invalid SSEU configuration\n");
drm_dbg? DRM_DEBUG is deprecated?
> + return ret;
> + }
> + props->has_sseu = true;
> + }
> +
> return 0;
> }
After addressing the above comments:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-16 5:08 ` Dixit, Ashutosh
@ 2023-02-16 5:36 ` Dixit, Ashutosh
2023-02-16 16:31 ` Dixit, Ashutosh
2023-02-16 23:11 ` Umesh Nerlige Ramappa
0 siblings, 2 replies; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 5:36 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Wed, 15 Feb 2023 21:08:43 -0800, Dixit, Ashutosh wrote:
>
> On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
> >
> > Validate the OA sseu config after all params are parsed.
>
> Commit messages for all patches need to answer the "why" or the reason for
> the patch. In this case maybe an overkill but probably something like:
>
> Validate the OA sseu config after all params are parsed since the engine
> can be passed in as part of perf properties.
Also, if we do this the patch should probably be later in the series after
the patch which introduces engine class/instance in the perf properties.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-16 5:36 ` Dixit, Ashutosh
@ 2023-02-16 16:31 ` Dixit, Ashutosh
2023-02-16 17:37 ` Umesh Nerlige Ramappa
2023-02-16 23:11 ` Umesh Nerlige Ramappa
1 sibling, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 16:31 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Wed, 15 Feb 2023 21:36:50 -0800, Dixit, Ashutosh wrote:
>
> On Wed, 15 Feb 2023 21:08:43 -0800, Dixit, Ashutosh wrote:
> >
> > On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
> > >
> > > Validate the OA sseu config after all params are parsed.
> >
> > Commit messages for all patches need to answer the "why" or the reason for
> > the patch. In this case maybe an overkill but probably something like:
> >
> > Validate the OA sseu config after all params are parsed since the engine
> > can be passed in as part of perf properties.
>
> Also, if we do this the patch should probably be later in the series after
> the patch which introduces engine class/instance in the perf properties.
General guidelines for submitting a patch series for review (for the
future):
1. The commit message should explain "why" or reason for a patch
2. As far as possible patches should be in a logical order so the series
should "tell a story"
3. The patches should be small (each patch being a single logical change if
possible)
So after we've done the hard part of figuring out the code and getting it
to work, the above guidelines also make the review process easier.
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-16 16:31 ` Dixit, Ashutosh
@ 2023-02-16 17:37 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 17:37 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, Feb 16, 2023 at 08:31:21AM -0800, Dixit, Ashutosh wrote:
>On Wed, 15 Feb 2023 21:36:50 -0800, Dixit, Ashutosh wrote:
>>
>> On Wed, 15 Feb 2023 21:08:43 -0800, Dixit, Ashutosh wrote:
>> >
>> > On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
>> > >
>> > > Validate the OA sseu config after all params are parsed.
>> >
>> > Commit messages for all patches need to answer the "why" or the reason for
>> > the patch. In this case maybe an overkill but probably something like:
>> >
>> > Validate the OA sseu config after all params are parsed since the engine
>> > can be passed in as part of perf properties.
>>
>> Also, if we do this the patch should probably be later in the series after
>> the patch which introduces engine class/instance in the perf properties.
>
>General guidelines for submitting a patch series for review (for the
>future):
>
>1. The commit message should explain "why" or reason for a patch
>2. As far as possible patches should be in a logical order so the series
> should "tell a story"
>3. The patches should be small (each patch being a single logical change if
> possible)
>
>So after we've done the hard part of figuring out the code and getting it
>to work, the above guidelines also make the review process easier.
will do and post a new revision.
Thanks,
Umesh
>
>Thanks.
>--
>Ashutosh
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch
2023-02-16 5:36 ` Dixit, Ashutosh
2023-02-16 16:31 ` Dixit, Ashutosh
@ 2023-02-16 23:11 ` Umesh Nerlige Ramappa
1 sibling, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 23:11 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
On Wed, Feb 15, 2023 at 09:36:50PM -0800, Dixit, Ashutosh wrote:
>On Wed, 15 Feb 2023 21:08:43 -0800, Dixit, Ashutosh wrote:
>>
>> On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
>> >
>> > Validate the OA sseu config after all params are parsed.
>>
>> Commit messages for all patches need to answer the "why" or the reason for
>> the patch. In this case maybe an overkill but probably something like:
>>
>> Validate the OA sseu config after all params are parsed since the engine
>> can be passed in as part of perf properties.
>
>Also, if we do this the patch should probably be later in the series after
>the patch which introduces engine class/instance in the perf properties.
Reg: order of this patch,
I am thinking it still makes sense to have it here (before the
class:instance patch). It's more like a refactor before enabling the
feature so that once the feature is enabled, there are no corner cases.
Thoughts?
I would just add the same description in the commit message.
Thanks,
Umesh
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (2 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 6:10 ` Dixit, Ashutosh
2023-02-15 0:54 ` [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups Umesh Nerlige Ramappa
` (6 subsequent siblings)
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Check for return value from i915_perf_init and fail driver init if perf
init fails.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_driver.c | 4 +++-
drivers/gpu/drm/i915/i915_perf.c | 4 +++-
drivers/gpu/drm/i915/i915_perf.h | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 0c0ae3eabb4b..998ca41c9713 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -477,7 +477,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
if (ret)
return ret;
- i915_perf_init(dev_priv);
+ ret = i915_perf_init(dev_priv);
+ if (ret)
+ return ret;
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 0b2097ad000e..e134523576f8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4845,7 +4845,7 @@ static void i915_perf_init_info(struct drm_i915_private *i915)
* Note: i915-perf initialization is split into an 'init' and 'register'
* phase with the i915_perf_register() exposing state to userspace.
*/
-void i915_perf_init(struct drm_i915_private *i915)
+int i915_perf_init(struct drm_i915_private *i915)
{
struct i915_perf *perf = &i915->perf;
@@ -4962,6 +4962,8 @@ void i915_perf_init(struct drm_i915_private *i915)
oa_init_supported_formats(perf);
}
+
+ return 0;
}
static int destroy_config(int id, void *p, void *data)
diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h
index f96e09a4af04..253637651d5e 100644
--- a/drivers/gpu/drm/i915/i915_perf.h
+++ b/drivers/gpu/drm/i915/i915_perf.h
@@ -18,7 +18,7 @@ struct i915_oa_config;
struct intel_context;
struct intel_engine_cs;
-void i915_perf_init(struct drm_i915_private *i915);
+int i915_perf_init(struct drm_i915_private *i915);
void i915_perf_fini(struct drm_i915_private *i915);
void i915_perf_register(struct drm_i915_private *i915);
void i915_perf_unregister(struct drm_i915_private *i915);
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails
2023-02-15 0:54 ` [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails Umesh Nerlige Ramappa
@ 2023-02-16 6:10 ` Dixit, Ashutosh
2023-02-16 10:43 ` Jani Nikula
0 siblings, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 6:10 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Tue, 14 Feb 2023 16:54:14 -0800, Umesh Nerlige Ramappa wrote:
>
> Check for return value from i915_perf_init and fail driver init if perf
> init fails.
Guess we'll start returning anything other than zero later:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
> drivers/gpu/drm/i915/i915_driver.c | 4 +++-
> drivers/gpu/drm/i915/i915_perf.c | 4 +++-
> drivers/gpu/drm/i915/i915_perf.h | 2 +-
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 0c0ae3eabb4b..998ca41c9713 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -477,7 +477,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
> if (ret)
> return ret;
>
> - i915_perf_init(dev_priv);
> + ret = i915_perf_init(dev_priv);
> + if (ret)
> + return ret;
>
> ret = i915_ggtt_probe_hw(dev_priv);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 0b2097ad000e..e134523576f8 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -4845,7 +4845,7 @@ static void i915_perf_init_info(struct drm_i915_private *i915)
> * Note: i915-perf initialization is split into an 'init' and 'register'
> * phase with the i915_perf_register() exposing state to userspace.
> */
> -void i915_perf_init(struct drm_i915_private *i915)
> +int i915_perf_init(struct drm_i915_private *i915)
> {
> struct i915_perf *perf = &i915->perf;
>
> @@ -4962,6 +4962,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>
> oa_init_supported_formats(perf);
> }
> +
> + return 0;
> }
>
> static int destroy_config(int id, void *p, void *data)
> diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h
> index f96e09a4af04..253637651d5e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.h
> +++ b/drivers/gpu/drm/i915/i915_perf.h
> @@ -18,7 +18,7 @@ struct i915_oa_config;
> struct intel_context;
> struct intel_engine_cs;
>
> -void i915_perf_init(struct drm_i915_private *i915);
> +int i915_perf_init(struct drm_i915_private *i915);
> void i915_perf_fini(struct drm_i915_private *i915);
> void i915_perf_register(struct drm_i915_private *i915);
> void i915_perf_unregister(struct drm_i915_private *i915);
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails
2023-02-16 6:10 ` Dixit, Ashutosh
@ 2023-02-16 10:43 ` Jani Nikula
0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2023-02-16 10:43 UTC (permalink / raw)
To: Dixit, Ashutosh, Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Wed, 15 Feb 2023, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> On Tue, 14 Feb 2023 16:54:14 -0800, Umesh Nerlige Ramappa wrote:
>>
>> Check for return value from i915_perf_init and fail driver init if perf
>> init fails.
>
> Guess we'll start returning anything other than zero later:
And before doing so, this change is useless, and IMO should not be
merged. If it can't or doesn't fail, it should be void. The probe is
complicated enough as is, let's not complicate it unless there's a
reason for it.
Moreover, we'll probably want to consider what parts of the driver probe
are allowed to fail, and the driver can still function. I'm pretty sure
most people prefer a working screen without OA over failing probe.
BR,
Jani.
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_driver.c | 4 +++-
>> drivers/gpu/drm/i915/i915_perf.c | 4 +++-
>> drivers/gpu/drm/i915/i915_perf.h | 2 +-
>> 3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index 0c0ae3eabb4b..998ca41c9713 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -477,7 +477,9 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>> if (ret)
>> return ret;
>>
>> - i915_perf_init(dev_priv);
>> + ret = i915_perf_init(dev_priv);
>> + if (ret)
>> + return ret;
>>
>> ret = i915_ggtt_probe_hw(dev_priv);
>> if (ret)
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 0b2097ad000e..e134523576f8 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -4845,7 +4845,7 @@ static void i915_perf_init_info(struct drm_i915_private *i915)
>> * Note: i915-perf initialization is split into an 'init' and 'register'
>> * phase with the i915_perf_register() exposing state to userspace.
>> */
>> -void i915_perf_init(struct drm_i915_private *i915)
>> +int i915_perf_init(struct drm_i915_private *i915)
>> {
>> struct i915_perf *perf = &i915->perf;
>>
>> @@ -4962,6 +4962,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>>
>> oa_init_supported_formats(perf);
>> }
>> +
>> + return 0;
>> }
>>
>> static int destroy_config(int id, void *p, void *data)
>> diff --git a/drivers/gpu/drm/i915/i915_perf.h b/drivers/gpu/drm/i915/i915_perf.h
>> index f96e09a4af04..253637651d5e 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.h
>> +++ b/drivers/gpu/drm/i915/i915_perf.h
>> @@ -18,7 +18,7 @@ struct i915_oa_config;
>> struct intel_context;
>> struct intel_engine_cs;
>>
>> -void i915_perf_init(struct drm_i915_private *i915);
>> +int i915_perf_init(struct drm_i915_private *i915);
>> void i915_perf_fini(struct drm_i915_private *i915);
>> void i915_perf_register(struct drm_i915_private *i915);
>> void i915_perf_unregister(struct drm_i915_private *i915);
>> --
>> 2.36.1
>>
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (3 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 4/9] drm/i915/perf: Fail modprobe if i915_perf_init fails Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 10:51 ` Jani Nikula
2023-02-15 0:54 ` [Intel-gfx] [PATCH 6/9] drm/i915/perf: Parse 64bit report header formats correctly Umesh Nerlige Ramappa
` (5 subsequent siblings)
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Now that we may have multiple OA units in a single GT as well as on
separate GTs, create an engine group that maps to a single OA unit.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 +
drivers/gpu/drm/i915/gt/intel_sseu.c | 3 +-
drivers/gpu/drm/i915/i915_perf.c | 126 +++++++++++++++++--
drivers/gpu/drm/i915/i915_perf_types.h | 51 +++++++-
4 files changed, 171 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 4fd54fb8810f..8a8b0dce241b 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -53,6 +53,8 @@ struct intel_gt;
struct intel_ring;
struct intel_uncore;
struct intel_breadcrumbs;
+struct intel_engine_cs;
+struct i915_perf_group;
typedef u32 intel_engine_mask_t;
#define ALL_ENGINES ((intel_engine_mask_t)~0ul)
@@ -603,6 +605,8 @@ struct intel_engine_cs {
} props, defaults;
I915_SELFTEST_DECLARE(struct fault_attr reset_timeout);
+
+ struct i915_perf_group *oa_group;
};
static inline bool
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
index 6c6198a257ac..1141f875f5bd 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -6,6 +6,7 @@
#include <linux/string_helpers.h>
#include "i915_drv.h"
+#include "i915_perf_types.h"
#include "intel_engine_regs.h"
#include "intel_gt_regs.h"
#include "intel_sseu.h"
@@ -677,7 +678,7 @@ u32 intel_sseu_make_rpcs(struct intel_gt *gt,
* If i915/perf is active, we want a stable powergating configuration
* on the system. Use the configuration pinned by i915/perf.
*/
- if (gt->perf.exclusive_stream)
+ if (gt->perf.group && gt->perf.group[PERF_GROUP_OAG].exclusive_stream)
req_sseu = >->perf.sseu;
slices = hweight8(req_sseu->slice_mask);
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e134523576f8..fda779b2c16f 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1587,8 +1587,9 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
{
struct i915_perf *perf = stream->perf;
struct intel_gt *gt = stream->engine->gt;
+ struct i915_perf_group *g = stream->engine->oa_group;
- if (WARN_ON(stream != gt->perf.exclusive_stream))
+ if (WARN_ON(stream != g->exclusive_stream))
return;
/*
@@ -1597,7 +1598,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
*
* See i915_oa_init_reg_state() and lrc_configure_all_contexts()
*/
- WRITE_ONCE(gt->perf.exclusive_stream, NULL);
+ WRITE_ONCE(g->exclusive_stream, NULL);
perf->ops.disable_metric_set(stream);
free_oa_buffer(stream);
@@ -3195,6 +3196,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
{
struct drm_i915_private *i915 = stream->perf->i915;
struct i915_perf *perf = stream->perf;
+ struct i915_perf_group *g;
struct intel_gt *gt;
int ret;
@@ -3205,6 +3207,12 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
}
gt = props->engine->gt;
+ g = props->engine->oa_group;
+ if (!g) {
+ DRM_DEBUG("Perf group invalid\n");
+ return -EINVAL;
+ }
+
/*
* If the sysfs metrics/ directory wasn't registered for some
* reason then don't let userspace try their luck with config
@@ -3234,7 +3242,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
* counter reports and marshal to the appropriate client
* we currently only allow exclusive access
*/
- if (gt->perf.exclusive_stream) {
+ if (g->exclusive_stream) {
drm_dbg(&stream->perf->i915->drm,
"OA unit already in use\n");
return -EBUSY;
@@ -3329,7 +3337,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
stream->ops = &i915_oa_stream_ops;
stream->engine->gt->perf.sseu = props->sseu;
- WRITE_ONCE(gt->perf.exclusive_stream, stream);
+ WRITE_ONCE(g->exclusive_stream, stream);
ret = i915_perf_stream_enable_sync(stream);
if (ret) {
@@ -3352,7 +3360,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
return 0;
err_enable:
- WRITE_ONCE(gt->perf.exclusive_stream, NULL);
+ WRITE_ONCE(g->exclusive_stream, NULL);
perf->ops.disable_metric_set(stream);
free_oa_buffer(stream);
@@ -3381,12 +3389,13 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
const struct intel_engine_cs *engine)
{
struct i915_perf_stream *stream;
+ struct i915_perf_group *g = engine->oa_group;
- if (!engine_supports_oa(engine))
+ if (!g)
return;
/* perf.exclusive_stream serialised by lrc_configure_all_contexts() */
- stream = READ_ONCE(engine->gt->perf.exclusive_stream);
+ stream = READ_ONCE(g->exclusive_stream);
if (stream && GRAPHICS_VER(stream->perf->i915) < 12)
gen8_update_reg_state_unlocked(ce, stream);
}
@@ -4755,6 +4764,95 @@ static struct ctl_table oa_table[] = {
{}
};
+static u32 __num_perf_groups_per_gt(struct intel_gt *gt)
+{
+ enum intel_platform platform = INTEL_INFO(gt->i915)->platform;
+
+ switch (platform) {
+ default:
+ return 1;
+ }
+}
+
+static u32 __oa_engine_group(struct intel_engine_cs *engine)
+{
+ if (!engine_supports_oa(engine))
+ return PERF_GROUP_INVALID;
+
+ switch (engine->class) {
+ case RENDER_CLASS:
+ return PERF_GROUP_OAG;
+
+ default:
+ return PERF_GROUP_INVALID;
+ }
+}
+
+static void oa_init_groups(struct intel_gt *gt)
+{
+ int i, num_groups = gt->perf.num_perf_groups;
+ struct i915_perf *perf = >->i915->perf;
+
+ for (i = 0; i < num_groups; i++) {
+ struct i915_perf_group *g = >->perf.group[i];
+
+ /* Fused off engines can result in a group with num_engines == 0 */
+ if (g->num_engines == 0)
+ continue;
+
+ /* Set oa_unit_ids now to ensure ids remain contiguous. */
+ g->oa_unit_id = perf->oa_unit_ids++;
+
+ g->gt = gt;
+ }
+}
+
+static int oa_init_gt(struct intel_gt *gt)
+{
+ u32 num_groups = __num_perf_groups_per_gt(gt);
+ struct intel_engine_cs *engine;
+ struct i915_perf_group *g;
+ intel_engine_mask_t tmp;
+
+ g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
+ if (drm_WARN_ON(>->i915->drm, !g))
+ return -ENOMEM;
+
+ for_each_engine_masked(engine, gt, ALL_ENGINES, tmp) {
+ u32 index;
+
+ index = __oa_engine_group(engine);
+ if (index < num_groups) {
+ g[index].engine_mask |= BIT(engine->id);
+ g[index].num_engines++;
+ engine->oa_group = &g[index];
+ } else {
+ engine->oa_group = NULL;
+ }
+ }
+
+ gt->perf.num_perf_groups = num_groups;
+ gt->perf.group = g;
+
+ oa_init_groups(gt);
+
+ return 0;
+}
+
+static int oa_init_engine_groups(struct i915_perf *perf)
+{
+ struct intel_gt *gt;
+ int i, ret;
+
+ for_each_gt(gt, perf->i915, i) {
+ ret = oa_init_gt(gt);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static void oa_init_supported_formats(struct i915_perf *perf)
{
struct drm_i915_private *i915 = perf->i915;
@@ -4921,7 +5019,7 @@ int i915_perf_init(struct drm_i915_private *i915)
if (perf->ops.enable_metric_set) {
struct intel_gt *gt;
- int i;
+ int i, ret;
for_each_gt(gt, i915, i)
mutex_init(>->perf.lock);
@@ -4960,6 +5058,13 @@ int i915_perf_init(struct drm_i915_private *i915)
perf->i915 = i915;
+ ret = oa_init_engine_groups(perf);
+ if (ret) {
+ drm_err(&i915->drm,
+ "OA initialization failed %d\n", ret);
+ return ret;
+ }
+
oa_init_supported_formats(perf);
}
@@ -4990,10 +5095,15 @@ void i915_perf_sysctl_unregister(void)
void i915_perf_fini(struct drm_i915_private *i915)
{
struct i915_perf *perf = &i915->perf;
+ struct intel_gt *gt;
+ int i;
if (!perf->i915)
return;
+ for_each_gt(gt, perf->i915, i)
+ kfree(gt->perf.group);
+
idr_for_each(&perf->metrics_idr, destroy_config, perf);
idr_destroy(&perf->metrics_idr);
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index e36f046fe2b6..ce99551ad0fd 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -17,6 +17,7 @@
#include <linux/wait.h>
#include <uapi/drm/i915_drm.h>
+#include "gt/intel_engine_types.h"
#include "gt/intel_sseu.h"
#include "i915_reg_defs.h"
#include "intel_wakeref.h"
@@ -30,6 +31,13 @@ struct i915_vma;
struct intel_context;
struct intel_engine_cs;
+enum {
+ PERF_GROUP_OAG = 0,
+
+ PERF_GROUP_MAX,
+ PERF_GROUP_INVALID = U32_MAX,
+};
+
struct i915_oa_format {
u32 format;
int size;
@@ -390,6 +398,35 @@ struct i915_oa_ops {
u32 (*oa_hw_tail_read)(struct i915_perf_stream *stream);
};
+struct i915_perf_group {
+ /*
+ * @type: Identifier for the OA unit.
+ */
+ u32 oa_unit_id;
+
+ /*
+ * @gt: gt that this group belongs to
+ */
+ struct intel_gt *gt;
+
+ /*
+ * @exclusive_stream: The stream currently using the OA unit. This is
+ * sometimes accessed outside a syscall associated to its file
+ * descriptor.
+ */
+ struct i915_perf_stream *exclusive_stream;
+
+ /*
+ * @num_engines: The number of engines using this OA buffer.
+ */
+ u32 num_engines;
+
+ /*
+ * @engine_mask: A mask of engines using a single OA buffer.
+ */
+ intel_engine_mask_t engine_mask;
+};
+
struct i915_perf_gt {
/*
* Lock associated with anything below within this structure.
@@ -402,12 +439,15 @@ struct i915_perf_gt {
*/
struct intel_sseu sseu;
+ /**
+ * @num_perf_groups: number of perf groups per gt.
+ */
+ u32 num_perf_groups;
+
/*
- * @exclusive_stream: The stream currently using the OA unit. This is
- * sometimes accessed outside a syscall associated to its file
- * descriptor.
+ * @group: list of OA groups - one for each OA buffer.
*/
- struct i915_perf_stream *exclusive_stream;
+ struct i915_perf_group *group;
};
struct i915_perf {
@@ -461,6 +501,9 @@ struct i915_perf {
unsigned long format_mask[FORMAT_MASK_SIZE];
atomic64_t noa_programming_delay;
+
+ /* oa unit ids */
+ u32 oa_unit_ids;
};
#endif /* _I915_PERF_TYPES_H_ */
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-15 0:54 ` [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups Umesh Nerlige Ramappa
@ 2023-02-16 10:51 ` Jani Nikula
2023-02-16 17:55 ` Dixit, Ashutosh
2023-02-16 23:44 ` Umesh Nerlige Ramappa
0 siblings, 2 replies; 36+ messages in thread
From: Jani Nikula @ 2023-02-16 10:51 UTC (permalink / raw)
To: Umesh Nerlige Ramappa, intel-gfx; +Cc: Lionel G Landwerlin
On Tue, 14 Feb 2023, Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> wrote:
> Now that we may have multiple OA units in a single GT as well as on
> separate GTs, create an engine group that maps to a single OA unit.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 +
> drivers/gpu/drm/i915/gt/intel_sseu.c | 3 +-
> drivers/gpu/drm/i915/i915_perf.c | 126 +++++++++++++++++--
> drivers/gpu/drm/i915/i915_perf_types.h | 51 +++++++-
> 4 files changed, 171 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 4fd54fb8810f..8a8b0dce241b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -53,6 +53,8 @@ struct intel_gt;
> struct intel_ring;
> struct intel_uncore;
> struct intel_breadcrumbs;
> +struct intel_engine_cs;
> +struct i915_perf_group;
>
> typedef u32 intel_engine_mask_t;
> #define ALL_ENGINES ((intel_engine_mask_t)~0ul)
> @@ -603,6 +605,8 @@ struct intel_engine_cs {
> } props, defaults;
>
> I915_SELFTEST_DECLARE(struct fault_attr reset_timeout);
> +
> + struct i915_perf_group *oa_group;
> };
>
> static inline bool
> diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
> index 6c6198a257ac..1141f875f5bd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_sseu.c
> +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
> @@ -6,6 +6,7 @@
> #include <linux/string_helpers.h>
>
> #include "i915_drv.h"
> +#include "i915_perf_types.h"
> #include "intel_engine_regs.h"
> #include "intel_gt_regs.h"
> #include "intel_sseu.h"
> @@ -677,7 +678,7 @@ u32 intel_sseu_make_rpcs(struct intel_gt *gt,
> * If i915/perf is active, we want a stable powergating configuration
> * on the system. Use the configuration pinned by i915/perf.
> */
> - if (gt->perf.exclusive_stream)
> + if (gt->perf.group && gt->perf.group[PERF_GROUP_OAG].exclusive_stream)
> req_sseu = >->perf.sseu;
>
> slices = hweight8(req_sseu->slice_mask);
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index e134523576f8..fda779b2c16f 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1587,8 +1587,9 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
> {
> struct i915_perf *perf = stream->perf;
> struct intel_gt *gt = stream->engine->gt;
> + struct i915_perf_group *g = stream->engine->oa_group;
>
> - if (WARN_ON(stream != gt->perf.exclusive_stream))
> + if (WARN_ON(stream != g->exclusive_stream))
> return;
>
> /*
> @@ -1597,7 +1598,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
> *
> * See i915_oa_init_reg_state() and lrc_configure_all_contexts()
> */
> - WRITE_ONCE(gt->perf.exclusive_stream, NULL);
> + WRITE_ONCE(g->exclusive_stream, NULL);
> perf->ops.disable_metric_set(stream);
>
> free_oa_buffer(stream);
> @@ -3195,6 +3196,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> {
> struct drm_i915_private *i915 = stream->perf->i915;
> struct i915_perf *perf = stream->perf;
> + struct i915_perf_group *g;
> struct intel_gt *gt;
> int ret;
>
> @@ -3205,6 +3207,12 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> }
> gt = props->engine->gt;
>
> + g = props->engine->oa_group;
> + if (!g) {
> + DRM_DEBUG("Perf group invalid\n");
> + return -EINVAL;
> + }
> +
> /*
> * If the sysfs metrics/ directory wasn't registered for some
> * reason then don't let userspace try their luck with config
> @@ -3234,7 +3242,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> * counter reports and marshal to the appropriate client
> * we currently only allow exclusive access
> */
> - if (gt->perf.exclusive_stream) {
> + if (g->exclusive_stream) {
> drm_dbg(&stream->perf->i915->drm,
> "OA unit already in use\n");
> return -EBUSY;
> @@ -3329,7 +3337,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> stream->ops = &i915_oa_stream_ops;
>
> stream->engine->gt->perf.sseu = props->sseu;
> - WRITE_ONCE(gt->perf.exclusive_stream, stream);
> + WRITE_ONCE(g->exclusive_stream, stream);
>
> ret = i915_perf_stream_enable_sync(stream);
> if (ret) {
> @@ -3352,7 +3360,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> return 0;
>
> err_enable:
> - WRITE_ONCE(gt->perf.exclusive_stream, NULL);
> + WRITE_ONCE(g->exclusive_stream, NULL);
> perf->ops.disable_metric_set(stream);
>
> free_oa_buffer(stream);
> @@ -3381,12 +3389,13 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
> const struct intel_engine_cs *engine)
> {
> struct i915_perf_stream *stream;
> + struct i915_perf_group *g = engine->oa_group;
>
> - if (!engine_supports_oa(engine))
> + if (!g)
> return;
>
> /* perf.exclusive_stream serialised by lrc_configure_all_contexts() */
> - stream = READ_ONCE(engine->gt->perf.exclusive_stream);
> + stream = READ_ONCE(g->exclusive_stream);
> if (stream && GRAPHICS_VER(stream->perf->i915) < 12)
> gen8_update_reg_state_unlocked(ce, stream);
> }
> @@ -4755,6 +4764,95 @@ static struct ctl_table oa_table[] = {
> {}
> };
>
> +static u32 __num_perf_groups_per_gt(struct intel_gt *gt)
> +{
> + enum intel_platform platform = INTEL_INFO(gt->i915)->platform;
> +
> + switch (platform) {
> + default:
> + return 1;
> + }
> +}
> +
> +static u32 __oa_engine_group(struct intel_engine_cs *engine)
> +{
> + if (!engine_supports_oa(engine))
> + return PERF_GROUP_INVALID;
> +
> + switch (engine->class) {
> + case RENDER_CLASS:
> + return PERF_GROUP_OAG;
> +
> + default:
> + return PERF_GROUP_INVALID;
> + }
> +}
> +
> +static void oa_init_groups(struct intel_gt *gt)
> +{
> + int i, num_groups = gt->perf.num_perf_groups;
> + struct i915_perf *perf = >->i915->perf;
> +
> + for (i = 0; i < num_groups; i++) {
> + struct i915_perf_group *g = >->perf.group[i];
> +
> + /* Fused off engines can result in a group with num_engines == 0 */
> + if (g->num_engines == 0)
> + continue;
> +
> + /* Set oa_unit_ids now to ensure ids remain contiguous. */
> + g->oa_unit_id = perf->oa_unit_ids++;
> +
> + g->gt = gt;
> + }
> +}
> +
> +static int oa_init_gt(struct intel_gt *gt)
> +{
> + u32 num_groups = __num_perf_groups_per_gt(gt);
> + struct intel_engine_cs *engine;
> + struct i915_perf_group *g;
> + intel_engine_mask_t tmp;
> +
> + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
> + if (drm_WARN_ON(>->i915->drm, !g))
> + return -ENOMEM;
No warnings or messages on -ENOMEM is standard policy.
> +
> + for_each_engine_masked(engine, gt, ALL_ENGINES, tmp) {
> + u32 index;
> +
> + index = __oa_engine_group(engine);
> + if (index < num_groups) {
> + g[index].engine_mask |= BIT(engine->id);
> + g[index].num_engines++;
> + engine->oa_group = &g[index];
> + } else {
> + engine->oa_group = NULL;
> + }
> + }
> +
> + gt->perf.num_perf_groups = num_groups;
> + gt->perf.group = g;
> +
> + oa_init_groups(gt);
> +
> + return 0;
> +}
> +
> +static int oa_init_engine_groups(struct i915_perf *perf)
> +{
> + struct intel_gt *gt;
> + int i, ret;
> +
> + for_each_gt(gt, perf->i915, i) {
> + ret = oa_init_gt(gt);
> + if (ret)
> + return ret;
If this fails in the middle, you'll leave things in half-initialized
state when returning, and expect the caller to clean it up. But that's a
surprising interface design. If i915_perf_init() returns an error, it's
*not* customary to have to call the corresponding cleanup function. On
the contrary, you only call it on success. Any init failures need to be
cleaned up internally before returning.
> + }
> +
> + return 0;
> +}
> +
> static void oa_init_supported_formats(struct i915_perf *perf)
> {
> struct drm_i915_private *i915 = perf->i915;
> @@ -4921,7 +5019,7 @@ int i915_perf_init(struct drm_i915_private *i915)
>
> if (perf->ops.enable_metric_set) {
> struct intel_gt *gt;
> - int i;
> + int i, ret;
>
> for_each_gt(gt, i915, i)
> mutex_init(>->perf.lock);
> @@ -4960,6 +5058,13 @@ int i915_perf_init(struct drm_i915_private *i915)
>
> perf->i915 = i915;
>
> + ret = oa_init_engine_groups(perf);
> + if (ret) {
> + drm_err(&i915->drm,
> + "OA initialization failed %d\n", ret);
> + return ret;
> + }
> +
> oa_init_supported_formats(perf);
> }
>
> @@ -4990,10 +5095,15 @@ void i915_perf_sysctl_unregister(void)
> void i915_perf_fini(struct drm_i915_private *i915)
> {
> struct i915_perf *perf = &i915->perf;
> + struct intel_gt *gt;
> + int i;
>
> if (!perf->i915)
> return;
>
> + for_each_gt(gt, perf->i915, i)
> + kfree(gt->perf.group);
> +
> idr_for_each(&perf->metrics_idr, destroy_config, perf);
> idr_destroy(&perf->metrics_idr);
>
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index e36f046fe2b6..ce99551ad0fd 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -17,6 +17,7 @@
> #include <linux/wait.h>
> #include <uapi/drm/i915_drm.h>
>
> +#include "gt/intel_engine_types.h"
> #include "gt/intel_sseu.h"
> #include "i915_reg_defs.h"
> #include "intel_wakeref.h"
> @@ -30,6 +31,13 @@ struct i915_vma;
> struct intel_context;
> struct intel_engine_cs;
>
> +enum {
> + PERF_GROUP_OAG = 0,
> +
> + PERF_GROUP_MAX,
> + PERF_GROUP_INVALID = U32_MAX,
> +};
> +
> struct i915_oa_format {
> u32 format;
> int size;
> @@ -390,6 +398,35 @@ struct i915_oa_ops {
> u32 (*oa_hw_tail_read)(struct i915_perf_stream *stream);
> };
>
> +struct i915_perf_group {
> + /*
> + * @type: Identifier for the OA unit.
> + */
> + u32 oa_unit_id;
> +
> + /*
> + * @gt: gt that this group belongs to
> + */
> + struct intel_gt *gt;
> +
> + /*
> + * @exclusive_stream: The stream currently using the OA unit. This is
> + * sometimes accessed outside a syscall associated to its file
> + * descriptor.
> + */
> + struct i915_perf_stream *exclusive_stream;
> +
> + /*
> + * @num_engines: The number of engines using this OA buffer.
> + */
> + u32 num_engines;
> +
> + /*
> + * @engine_mask: A mask of engines using a single OA buffer.
> + */
> + intel_engine_mask_t engine_mask;
> +};
> +
> struct i915_perf_gt {
> /*
> * Lock associated with anything below within this structure.
> @@ -402,12 +439,15 @@ struct i915_perf_gt {
> */
> struct intel_sseu sseu;
>
> + /**
> + * @num_perf_groups: number of perf groups per gt.
> + */
> + u32 num_perf_groups;
> +
> /*
> - * @exclusive_stream: The stream currently using the OA unit. This is
> - * sometimes accessed outside a syscall associated to its file
> - * descriptor.
> + * @group: list of OA groups - one for each OA buffer.
> */
> - struct i915_perf_stream *exclusive_stream;
> + struct i915_perf_group *group;
> };
>
> struct i915_perf {
> @@ -461,6 +501,9 @@ struct i915_perf {
> unsigned long format_mask[FORMAT_MASK_SIZE];
>
> atomic64_t noa_programming_delay;
> +
> + /* oa unit ids */
> + u32 oa_unit_ids;
> };
>
> #endif /* _I915_PERF_TYPES_H_ */
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-16 10:51 ` Jani Nikula
@ 2023-02-16 17:55 ` Dixit, Ashutosh
2023-02-16 18:10 ` Jani Nikula
2023-02-16 23:44 ` Umesh Nerlige Ramappa
1 sibling, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 17:55 UTC (permalink / raw)
To: Jani Nikula; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, 16 Feb 2023 02:51:34 -0800, Jani Nikula wrote:
>
> > +static int oa_init_gt(struct intel_gt *gt)
> > +{
> > + u32 num_groups = __num_perf_groups_per_gt(gt);
> > + struct intel_engine_cs *engine;
> > + struct i915_perf_group *g;
> > + intel_engine_mask_t tmp;
> > +
> > + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
> > + if (drm_WARN_ON(>->i915->drm, !g))
> > + return -ENOMEM;
>
> No warnings or messages on -ENOMEM is standard policy.
Hmm I think this is the only error for which this code is failing the
probe. So if we are not going to fail the probe, we should at least allow a
WARN_ON? Exception proves the rule?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-16 17:55 ` Dixit, Ashutosh
@ 2023-02-16 18:10 ` Jani Nikula
2023-02-16 20:55 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 36+ messages in thread
From: Jani Nikula @ 2023-02-16 18:10 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, 16 Feb 2023, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
> On Thu, 16 Feb 2023 02:51:34 -0800, Jani Nikula wrote:
>>
>> > +static int oa_init_gt(struct intel_gt *gt)
>> > +{
>> > + u32 num_groups = __num_perf_groups_per_gt(gt);
>> > + struct intel_engine_cs *engine;
>> > + struct i915_perf_group *g;
>> > + intel_engine_mask_t tmp;
>> > +
>> > + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
>> > + if (drm_WARN_ON(>->i915->drm, !g))
>> > + return -ENOMEM;
>>
>> No warnings or messages on -ENOMEM is standard policy.
>
> Hmm I think this is the only error for which this code is failing the
> probe. So if we are not going to fail the probe, we should at least allow a
> WARN_ON? Exception proves the rule?
A whole lot of other things are going to go bonkers on -ENOMEM, and
getting that warn isn't going to help anyone...
Maybe we do need to fail probe on this after all, but it just seemed
pointless at the time it was introduced a few patches earlier.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-16 18:10 ` Jani Nikula
@ 2023-02-16 20:55 ` Umesh Nerlige Ramappa
2023-02-16 23:58 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 20:55 UTC (permalink / raw)
To: Jani Nikula; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, Feb 16, 2023 at 08:10:29PM +0200, Jani Nikula wrote:
>On Thu, 16 Feb 2023, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
>> On Thu, 16 Feb 2023 02:51:34 -0800, Jani Nikula wrote:
>>>
>>> > +static int oa_init_gt(struct intel_gt *gt)
>>> > +{
>>> > + u32 num_groups = __num_perf_groups_per_gt(gt);
>>> > + struct intel_engine_cs *engine;
>>> > + struct i915_perf_group *g;
>>> > + intel_engine_mask_t tmp;
>>> > +
>>> > + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
>>> > + if (drm_WARN_ON(>->i915->drm, !g))
>>> > + return -ENOMEM;
>>>
>>> No warnings or messages on -ENOMEM is standard policy.
>>
>> Hmm I think this is the only error for which this code is failing the
>> probe. So if we are not going to fail the probe, we should at least allow a
>> WARN_ON? Exception proves the rule?
>
>A whole lot of other things are going to go bonkers on -ENOMEM, and
>getting that warn isn't going to help anyone...
Should I just add a debug message here instead of warn_ON?
>
>Maybe we do need to fail probe on this after all, but it just seemed
>pointless at the time it was introduced a few patches earlier.
Sorry about that, I will fix the order of patches.
Umesh
>
>BR,
>Jani.
>
>--
>Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-16 20:55 ` Umesh Nerlige Ramappa
@ 2023-02-16 23:58 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 23:58 UTC (permalink / raw)
To: Jani Nikula; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, Feb 16, 2023 at 12:55:59PM -0800, Umesh Nerlige Ramappa wrote:
>On Thu, Feb 16, 2023 at 08:10:29PM +0200, Jani Nikula wrote:
>>On Thu, 16 Feb 2023, "Dixit, Ashutosh" <ashutosh.dixit@intel.com> wrote:
>>>On Thu, 16 Feb 2023 02:51:34 -0800, Jani Nikula wrote:
>>>>
>>>>> +static int oa_init_gt(struct intel_gt *gt)
>>>>> +{
>>>>> + u32 num_groups = __num_perf_groups_per_gt(gt);
>>>>> + struct intel_engine_cs *engine;
>>>>> + struct i915_perf_group *g;
>>>>> + intel_engine_mask_t tmp;
>>>>> +
>>>>> + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
>>>>> + if (drm_WARN_ON(>->i915->drm, !g))
>>>>> + return -ENOMEM;
>>>>
>>>>No warnings or messages on -ENOMEM is standard policy.
>>>
>>>Hmm I think this is the only error for which this code is failing the
>>>probe. So if we are not going to fail the probe, we should at least allow a
>>>WARN_ON? Exception proves the rule?
>>
>>A whole lot of other things are going to go bonkers on -ENOMEM, and
>>getting that warn isn't going to help anyone...
>
>Should I just add a debug message here instead of warn_ON?
nvm, you already mentioned no warn/message on ENOMEM.
Regards,
Umesh
>
>>
>>Maybe we do need to fail probe on this after all, but it just seemed
>>pointless at the time it was introduced a few patches earlier.
>
>Sorry about that, I will fix the order of patches.
>
>Umesh
>>
>>BR,
>>Jani.
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups
2023-02-16 10:51 ` Jani Nikula
2023-02-16 17:55 ` Dixit, Ashutosh
@ 2023-02-16 23:44 ` Umesh Nerlige Ramappa
1 sibling, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 23:44 UTC (permalink / raw)
To: Jani Nikula; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, Feb 16, 2023 at 12:51:34PM +0200, Jani Nikula wrote:
>On Tue, 14 Feb 2023, Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> wrote:
>> Now that we may have multiple OA units in a single GT as well as on
>> separate GTs, create an engine group that maps to a single OA unit.
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 +
>> drivers/gpu/drm/i915/gt/intel_sseu.c | 3 +-
>> drivers/gpu/drm/i915/i915_perf.c | 126 +++++++++++++++++--
>> drivers/gpu/drm/i915/i915_perf_types.h | 51 +++++++-
>> 4 files changed, 171 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> index 4fd54fb8810f..8a8b0dce241b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> @@ -53,6 +53,8 @@ struct intel_gt;
>> struct intel_ring;
>> struct intel_uncore;
>> struct intel_breadcrumbs;
>> +struct intel_engine_cs;
>> +struct i915_perf_group;
>>
>> typedef u32 intel_engine_mask_t;
>> #define ALL_ENGINES ((intel_engine_mask_t)~0ul)
>> @@ -603,6 +605,8 @@ struct intel_engine_cs {
>> } props, defaults;
>>
>> I915_SELFTEST_DECLARE(struct fault_attr reset_timeout);
>> +
>> + struct i915_perf_group *oa_group;
>> };
>>
>> static inline bool
>> diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
>> index 6c6198a257ac..1141f875f5bd 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_sseu.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
>> @@ -6,6 +6,7 @@
>> #include <linux/string_helpers.h>
>>
>> #include "i915_drv.h"
>> +#include "i915_perf_types.h"
>> #include "intel_engine_regs.h"
>> #include "intel_gt_regs.h"
>> #include "intel_sseu.h"
>> @@ -677,7 +678,7 @@ u32 intel_sseu_make_rpcs(struct intel_gt *gt,
>> * If i915/perf is active, we want a stable powergating configuration
>> * on the system. Use the configuration pinned by i915/perf.
>> */
>> - if (gt->perf.exclusive_stream)
>> + if (gt->perf.group && gt->perf.group[PERF_GROUP_OAG].exclusive_stream)
>> req_sseu = >->perf.sseu;
>>
>> slices = hweight8(req_sseu->slice_mask);
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index e134523576f8..fda779b2c16f 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1587,8 +1587,9 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>> {
>> struct i915_perf *perf = stream->perf;
>> struct intel_gt *gt = stream->engine->gt;
>> + struct i915_perf_group *g = stream->engine->oa_group;
>>
>> - if (WARN_ON(stream != gt->perf.exclusive_stream))
>> + if (WARN_ON(stream != g->exclusive_stream))
>> return;
>>
>> /*
>> @@ -1597,7 +1598,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>> *
>> * See i915_oa_init_reg_state() and lrc_configure_all_contexts()
>> */
>> - WRITE_ONCE(gt->perf.exclusive_stream, NULL);
>> + WRITE_ONCE(g->exclusive_stream, NULL);
>> perf->ops.disable_metric_set(stream);
>>
>> free_oa_buffer(stream);
>> @@ -3195,6 +3196,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>> {
>> struct drm_i915_private *i915 = stream->perf->i915;
>> struct i915_perf *perf = stream->perf;
>> + struct i915_perf_group *g;
>> struct intel_gt *gt;
>> int ret;
>>
>> @@ -3205,6 +3207,12 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>> }
>> gt = props->engine->gt;
>>
>> + g = props->engine->oa_group;
>> + if (!g) {
>> + DRM_DEBUG("Perf group invalid\n");
>> + return -EINVAL;
>> + }
>> +
>> /*
>> * If the sysfs metrics/ directory wasn't registered for some
>> * reason then don't let userspace try their luck with config
>> @@ -3234,7 +3242,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>> * counter reports and marshal to the appropriate client
>> * we currently only allow exclusive access
>> */
>> - if (gt->perf.exclusive_stream) {
>> + if (g->exclusive_stream) {
>> drm_dbg(&stream->perf->i915->drm,
>> "OA unit already in use\n");
>> return -EBUSY;
>> @@ -3329,7 +3337,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>> stream->ops = &i915_oa_stream_ops;
>>
>> stream->engine->gt->perf.sseu = props->sseu;
>> - WRITE_ONCE(gt->perf.exclusive_stream, stream);
>> + WRITE_ONCE(g->exclusive_stream, stream);
>>
>> ret = i915_perf_stream_enable_sync(stream);
>> if (ret) {
>> @@ -3352,7 +3360,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>> return 0;
>>
>> err_enable:
>> - WRITE_ONCE(gt->perf.exclusive_stream, NULL);
>> + WRITE_ONCE(g->exclusive_stream, NULL);
>> perf->ops.disable_metric_set(stream);
>>
>> free_oa_buffer(stream);
>> @@ -3381,12 +3389,13 @@ void i915_oa_init_reg_state(const struct intel_context *ce,
>> const struct intel_engine_cs *engine)
>> {
>> struct i915_perf_stream *stream;
>> + struct i915_perf_group *g = engine->oa_group;
>>
>> - if (!engine_supports_oa(engine))
>> + if (!g)
>> return;
>>
>> /* perf.exclusive_stream serialised by lrc_configure_all_contexts() */
>> - stream = READ_ONCE(engine->gt->perf.exclusive_stream);
>> + stream = READ_ONCE(g->exclusive_stream);
>> if (stream && GRAPHICS_VER(stream->perf->i915) < 12)
>> gen8_update_reg_state_unlocked(ce, stream);
>> }
>> @@ -4755,6 +4764,95 @@ static struct ctl_table oa_table[] = {
>> {}
>> };
>>
>> +static u32 __num_perf_groups_per_gt(struct intel_gt *gt)
>> +{
>> + enum intel_platform platform = INTEL_INFO(gt->i915)->platform;
>> +
>> + switch (platform) {
>> + default:
>> + return 1;
>> + }
>> +}
>> +
>> +static u32 __oa_engine_group(struct intel_engine_cs *engine)
>> +{
>> + if (!engine_supports_oa(engine))
>> + return PERF_GROUP_INVALID;
>> +
>> + switch (engine->class) {
>> + case RENDER_CLASS:
>> + return PERF_GROUP_OAG;
>> +
>> + default:
>> + return PERF_GROUP_INVALID;
>> + }
>> +}
>> +
>> +static void oa_init_groups(struct intel_gt *gt)
>> +{
>> + int i, num_groups = gt->perf.num_perf_groups;
>> + struct i915_perf *perf = >->i915->perf;
>> +
>> + for (i = 0; i < num_groups; i++) {
>> + struct i915_perf_group *g = >->perf.group[i];
>> +
>> + /* Fused off engines can result in a group with num_engines == 0 */
>> + if (g->num_engines == 0)
>> + continue;
>> +
>> + /* Set oa_unit_ids now to ensure ids remain contiguous. */
>> + g->oa_unit_id = perf->oa_unit_ids++;
>> +
>> + g->gt = gt;
>> + }
>> +}
>> +
>> +static int oa_init_gt(struct intel_gt *gt)
>> +{
>> + u32 num_groups = __num_perf_groups_per_gt(gt);
>> + struct intel_engine_cs *engine;
>> + struct i915_perf_group *g;
>> + intel_engine_mask_t tmp;
>> +
>> + g = kcalloc(num_groups, sizeof(*g), GFP_KERNEL);
>> + if (drm_WARN_ON(>->i915->drm, !g))
>> + return -ENOMEM;
>
>No warnings or messages on -ENOMEM is standard policy.
Will remove warn ON.
>
>> +
>> + for_each_engine_masked(engine, gt, ALL_ENGINES, tmp) {
>> + u32 index;
>> +
>> + index = __oa_engine_group(engine);
>> + if (index < num_groups) {
>> + g[index].engine_mask |= BIT(engine->id);
>> + g[index].num_engines++;
>> + engine->oa_group = &g[index];
>> + } else {
>> + engine->oa_group = NULL;
>> + }
>> + }
>> +
>> + gt->perf.num_perf_groups = num_groups;
>> + gt->perf.group = g;
>> +
>> + oa_init_groups(gt);
>> +
>> + return 0;
>> +}
>> +
>> +static int oa_init_engine_groups(struct i915_perf *perf)
>> +{
>> + struct intel_gt *gt;
>> + int i, ret;
>> +
>> + for_each_gt(gt, perf->i915, i) {
>> + ret = oa_init_gt(gt);
>> + if (ret)
>> + return ret;
>
>If this fails in the middle, you'll leave things in half-initialized
>state when returning, and expect the caller to clean it up. But that's a
>surprising interface design. If i915_perf_init() returns an error, it's
>*not* customary to have to call the corresponding cleanup function. On
>the contrary, you only call it on success. Any init failures need to be
>cleaned up internally before returning.
ENOMEM is the only error I am expecting here, so no other cleanup is
needed if this fails.
Thanks,
Umesh
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 6/9] drm/i915/perf: Parse 64bit report header formats correctly
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (4 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 5/9] drm/i915/perf: Group engines into respective OA groups Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 7/9] drm/i915/perf: Handle non-power-of-2 reports Umesh Nerlige Ramappa
` (4 subsequent siblings)
10 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Now that OA formats come in flavor of 64 bit reports, the report header
has 64 bit report-id, timestamp, context-id and gpu-ticks fields. When
filtering these reports, use the right width for these fields.
Note that upper dword of context id is reserved, so squash lower dword
only.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 105 ++++++++++++++++++++-----
drivers/gpu/drm/i915/i915_perf_types.h | 6 ++
2 files changed, 92 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index fda779b2c16f..030f3a598229 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -441,6 +441,75 @@ static u32 gen7_oa_hw_tail_read(struct i915_perf_stream *stream)
return oastatus1 & GEN7_OASTATUS1_TAIL_MASK;
}
+#define oa_report_header_64bit(__s) \
+ ((__s)->oa_buffer.format->header == HDR_64_BIT)
+
+static inline u64
+oa_report_id(struct i915_perf_stream *stream, void *report)
+{
+ return oa_report_header_64bit(stream) ? *(u64 *)report : *(u32 *)report;
+}
+
+static inline u64
+oa_report_reason(struct i915_perf_stream *stream, void *report)
+{
+ return (oa_report_id(stream, report) >> OAREPORT_REASON_SHIFT) &
+ (GRAPHICS_VER(stream->perf->i915) == 12 ?
+ OAREPORT_REASON_MASK_EXTENDED :
+ OAREPORT_REASON_MASK);
+}
+
+static inline void
+oa_report_id_clear(struct i915_perf_stream *stream, u32 *report)
+{
+ if (oa_report_header_64bit(stream))
+ *(u64 *)report = 0;
+ else
+ *report = 0;
+}
+
+static inline bool
+oa_report_ctx_invalid(struct i915_perf_stream *stream, void *report)
+{
+ return !(oa_report_id(stream, report) &
+ stream->perf->gen8_valid_ctx_bit) &&
+ GRAPHICS_VER(stream->perf->i915) <= 11;
+}
+
+static inline u64
+oa_timestamp(struct i915_perf_stream *stream, void *report)
+{
+ return oa_report_header_64bit(stream) ?
+ *((u64 *)report + 1) :
+ *((u32 *)report + 1);
+}
+
+static inline void
+oa_timestamp_clear(struct i915_perf_stream *stream, u32 *report)
+{
+ if (oa_report_header_64bit(stream))
+ *(u64 *)&report[2] = 0;
+ else
+ report[1] = 0;
+}
+
+static inline u32
+oa_context_id(struct i915_perf_stream *stream, u32 *report)
+{
+ u32 ctx_id = oa_report_header_64bit(stream) ? report[4] : report[2];
+
+ return ctx_id & stream->specific_ctx_id_mask;
+}
+
+static inline void
+oa_context_id_squash(struct i915_perf_stream *stream, u32 *report)
+{
+ if (oa_report_header_64bit(stream))
+ report[4] = INVALID_CTX_ID;
+ else
+ report[2] = INVALID_CTX_ID;
+}
+
/**
* oa_buffer_check_unlocked - check for data and update tail ptr state
* @stream: i915 stream instance
@@ -521,9 +590,10 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
* If not : (╯°□°)╯︵ ┻━┻
*/
while (OA_TAKEN(tail, aged_tail) >= report_size) {
- u32 *report32 = (void *)(stream->oa_buffer.vaddr + tail);
+ void *report = stream->oa_buffer.vaddr + tail;
- if (report32[0] != 0 || report32[1] != 0)
+ if (oa_report_id(stream, report) ||
+ oa_timestamp(stream, report))
break;
tail = (tail - report_size) & (OA_BUFFER_SIZE - 1);
@@ -702,7 +772,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
u8 *report = oa_buf_base + head;
u32 *report32 = (void *)report;
u32 ctx_id;
- u32 reason;
+ u64 reason;
/*
* All the report sizes factor neatly into the buffer
@@ -725,16 +795,12 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
* triggered this specific report (mostly timer
* triggered or e.g. due to a context switch).
*
- * This field is never expected to be zero so we can
- * check that the report isn't invalid before copying
- * it to userspace...
+ * In MMIO triggered reports, some platforms do not set the
+ * reason bit in this field and it is valid to have a reason
+ * field of zero.
*/
- reason = ((report32[0] >> OAREPORT_REASON_SHIFT) &
- (GRAPHICS_VER(stream->perf->i915) == 12 ?
- OAREPORT_REASON_MASK_EXTENDED :
- OAREPORT_REASON_MASK));
-
- ctx_id = report32[2] & stream->specific_ctx_id_mask;
+ reason = oa_report_reason(stream, report);
+ ctx_id = oa_context_id(stream, report32);
/*
* Squash whatever is in the CTX_ID field if it's marked as
@@ -744,9 +810,10 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
* Note: that we don't clear the valid_ctx_bit so userspace can
* understand that the ID has been squashed by the kernel.
*/
- if (!(report32[0] & stream->perf->gen8_valid_ctx_bit) &&
- GRAPHICS_VER(stream->perf->i915) <= 11)
- ctx_id = report32[2] = INVALID_CTX_ID;
+ if (oa_report_ctx_invalid(stream, report)) {
+ ctx_id = INVALID_CTX_ID;
+ oa_context_id_squash(stream, report32);
+ }
/*
* NB: For Gen 8 the OA unit no longer supports clock gating
@@ -790,7 +857,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
*/
if (stream->ctx &&
stream->specific_ctx_id != ctx_id) {
- report32[2] = INVALID_CTX_ID;
+ oa_context_id_squash(stream, report32);
}
ret = append_oa_sample(stream, buf, count, offset,
@@ -802,11 +869,11 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
}
/*
- * Clear out the first 2 dword as a mean to detect unlanded
+ * Clear out the report id and timestamp as a means to detect unlanded
* reports.
*/
- report32[0] = 0;
- report32[1] = 0;
+ oa_report_id_clear(stream, report32);
+ oa_timestamp_clear(stream, report32);
}
if (start_offset != *offset) {
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index ce99551ad0fd..8ccb0b89d019 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -38,9 +38,15 @@ enum {
PERF_GROUP_INVALID = U32_MAX,
};
+enum report_header {
+ HDR_32_BIT = 0,
+ HDR_64_BIT,
+};
+
struct i915_oa_format {
u32 format;
int size;
+ enum report_header header;
};
struct i915_oa_reg {
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 7/9] drm/i915/perf: Handle non-power-of-2 reports
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (5 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 6/9] drm/i915/perf: Parse 64bit report header formats correctly Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 8/9] drm/i915/perf: Add engine class instance parameters to perf Umesh Nerlige Ramappa
` (3 subsequent siblings)
10 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Some of the newer OA formats are not powers of 2. For those formats,
hw_tail is adjusted accordingly when checking for new reports.
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 50 ++++++++++++++++++--------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 030f3a598229..2e1b305032c0 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -542,6 +542,7 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
bool pollin;
u32 hw_tail;
u64 now;
+ u32 partial_report_size;
/* We have to consider the (unlikely) possibility that read() errors
* could result in an OA buffer reset which might reset the head and
@@ -551,10 +552,16 @@ static bool oa_buffer_check_unlocked(struct i915_perf_stream *stream)
hw_tail = stream->perf->ops.oa_hw_tail_read(stream);
- /* The tail pointer increases in 64 byte increments,
- * not in report_size steps...
+ /* The tail pointer increases in 64 byte increments, whereas report
+ * sizes need not be integral multiples or 64 or powers of 2.
+ * Compute potentially partially landed report in the OA buffer
*/
- hw_tail &= ~(report_size - 1);
+ partial_report_size = OA_TAKEN(hw_tail, stream->oa_buffer.tail);
+ partial_report_size %= report_size;
+
+ /* Subtract partial amount off the tail */
+ hw_tail = gtt_offset + ((hw_tail - partial_report_size) &
+ (stream->oa_buffer.vma->size - 1));
now = ktime_get_mono_fast_ns();
@@ -677,6 +684,8 @@ static int append_oa_sample(struct i915_perf_stream *stream,
{
int report_size = stream->oa_buffer.format->size;
struct drm_i915_perf_record_header header;
+ int report_size_partial;
+ u8 *oa_buf_end;
header.type = DRM_I915_PERF_RECORD_SAMPLE;
header.pad = 0;
@@ -690,8 +699,21 @@ static int append_oa_sample(struct i915_perf_stream *stream,
return -EFAULT;
buf += sizeof(header);
- if (copy_to_user(buf, report, report_size))
+ oa_buf_end = stream->oa_buffer.vaddr +
+ stream->oa_buffer.vma->size;
+ report_size_partial = oa_buf_end - report;
+
+ if (report_size_partial < report_size) {
+ if (copy_to_user(buf, report, report_size_partial))
+ return -EFAULT;
+ buf += report_size_partial;
+
+ if (copy_to_user(buf, stream->oa_buffer.vaddr,
+ report_size - report_size_partial))
+ return -EFAULT;
+ } else if (copy_to_user(buf, report, report_size)) {
return -EFAULT;
+ }
(*offset) += header.size;
@@ -759,8 +781,8 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
* all a power of two).
*/
if (drm_WARN_ONCE(&uncore->i915->drm,
- head > OA_BUFFER_SIZE || head % report_size ||
- tail > OA_BUFFER_SIZE || tail % report_size,
+ head > OA_BUFFER_SIZE ||
+ tail > OA_BUFFER_SIZE,
"Inconsistent OA buffer pointers: head = %u, tail = %u\n",
head, tail))
return -EIO;
@@ -774,22 +796,6 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
u32 ctx_id;
u64 reason;
- /*
- * All the report sizes factor neatly into the buffer
- * size so we never expect to see a report split
- * between the beginning and end of the buffer.
- *
- * Given the initial alignment check a misalignment
- * here would imply a driver bug that would result
- * in an overrun.
- */
- if (drm_WARN_ON(&uncore->i915->drm,
- (OA_BUFFER_SIZE - head) < report_size)) {
- drm_err(&uncore->i915->drm,
- "Spurious OA head ptr: non-integral report offset\n");
- break;
- }
-
/*
* The reason field includes flags identifying what
* triggered this specific report (mostly timer
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 8/9] drm/i915/perf: Add engine class instance parameters to perf
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (6 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 7/9] drm/i915/perf: Handle non-power-of-2 reports Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-15 0:54 ` [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units Umesh Nerlige Ramappa
` (2 subsequent siblings)
10 siblings, 0 replies; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
Current implementation of perf defaults to render to and configures the
default OAG unit. Since there are more OA units on newer hardware, allow
user to pass engine class and instance to program specific OA units.
UMD specific changes for GPUvis support:
https://patchwork.freedesktop.org/patch/522827/?series=114023
https://patchwork.freedesktop.org/patch/522822/?series=114023
https://patchwork.freedesktop.org/patch/522826/?series=114023
https://patchwork.freedesktop.org/patch/522828/?series=114023
https://patchwork.freedesktop.org/patch/522816/?series=114023
https://patchwork.freedesktop.org/patch/522825/?series=114023
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 49 +++++++++++++++++++-------------
include/uapi/drm/i915_drm.h | 20 +++++++++++++
2 files changed, 49 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 2e1b305032c0..8f7306eaf43c 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4038,40 +4038,29 @@ static int read_properties_unlocked(struct i915_perf *perf,
u64 __user *uprop = uprops;
u32 i;
int ret;
+ u8 class, instance;
bool config_sseu = false;
struct drm_i915_gem_context_param_sseu user_sseu;
memset(props, 0, sizeof(struct perf_open_properties));
props->poll_oa_period = DEFAULT_POLL_PERIOD_NS;
- if (!n_props) {
- drm_dbg(&perf->i915->drm,
- "No i915 perf properties given\n");
- return -EINVAL;
- }
-
- /* At the moment we only support using i915-perf on the RCS. */
- props->engine = intel_engine_lookup_user(perf->i915,
- I915_ENGINE_CLASS_RENDER,
- 0);
- if (!props->engine) {
- drm_dbg(&perf->i915->drm,
- "No RENDER-capable engines\n");
- return -EINVAL;
- }
-
/* Considering that ID = 0 is reserved and assuming that we don't
* (currently) expect any configurations to ever specify duplicate
* values for a particular property ID then the last _PROP_MAX value is
* one greater than the maximum number of properties we expect to get
* from userspace.
*/
- if (n_props >= DRM_I915_PERF_PROP_MAX) {
+ if (!n_props || n_props >= DRM_I915_PERF_PROP_MAX) {
drm_dbg(&perf->i915->drm,
- "More i915 perf properties specified than exist\n");
+ "Invalid no. of i915 perf properties given\n");
return -EINVAL;
}
+ /* Defaults when class:instance is not passed */
+ class = I915_ENGINE_CLASS_RENDER;
+ instance = 0;
+
for (i = 0; i < n_props; i++) {
u64 oa_period, oa_freq_hz;
u64 id, value;
@@ -4192,7 +4181,13 @@ static int read_properties_unlocked(struct i915_perf *perf,
}
props->poll_oa_period = value;
break;
- case DRM_I915_PERF_PROP_MAX:
+ case DRM_I915_PERF_PROP_OA_ENGINE_CLASS:
+ class = (u8)value;
+ break;
+ case DRM_I915_PERF_PROP_OA_ENGINE_INSTANCE:
+ instance = (u8)value;
+ break;
+ default:
MISSING_CASE(id);
return -EINVAL;
}
@@ -4200,6 +4195,17 @@ static int read_properties_unlocked(struct i915_perf *perf,
uprop += 2;
}
+ props->engine = intel_engine_lookup_user(perf->i915, class, instance);
+ if (!props->engine) {
+ drm_dbg(&perf->i915->drm,
+ "OA engine class and instance invalid %d:%d\n",
+ class, instance);
+ return -EINVAL;
+ }
+
+ if (!engine_supports_oa(props->engine))
+ return -EINVAL;
+
if (config_sseu) {
ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
if (ret) {
@@ -5210,8 +5216,11 @@ int i915_perf_ioctl_version(void)
*
* 5: Add DRM_I915_PERF_PROP_POLL_OA_PERIOD parameter that controls the
* interval for the hrtimer used to check for OA data.
+ *
+ * 6: Add DRM_I915_PERF_PROP_OA_ENGINE_CLASS and
+ * DRM_I915_PERF_PROP_OA_ENGINE_INSTANCE
*/
- return 5;
+ return 6;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8df261c5ab9b..b6922b52d85c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2758,6 +2758,26 @@ enum drm_i915_perf_property_id {
*/
DRM_I915_PERF_PROP_POLL_OA_PERIOD,
+ /**
+ * In platforms with multiple OA buffers, the engine class instance must
+ * be passed to open a stream to a OA unit corresponding to the engine.
+ * Multiple engines may be mapped to the same OA unit.
+ *
+ * In addition to the class:instance, if a gem context is also passed, then
+ * 1) the report headers of OA reports from other engines are squashed.
+ * 2) OAR is enabled for the class:instance
+ *
+ * This property is available in perf revision 6.
+ */
+ DRM_I915_PERF_PROP_OA_ENGINE_CLASS,
+
+ /**
+ * This parameter specifies the engine instance.
+ *
+ * This property is available in perf revision 6.
+ */
+ DRM_I915_PERF_PROP_OA_ENGINE_INSTANCE,
+
DRM_I915_PERF_PROP_MAX /* non-ABI */
};
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (7 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 8/9] drm/i915/perf: Add engine class instance parameters to perf Umesh Nerlige Ramappa
@ 2023-02-15 0:54 ` Umesh Nerlige Ramappa
2023-02-16 17:27 ` Dixit, Ashutosh
2023-02-15 1:38 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add OAM support for MTL Patchwork
2023-02-15 2:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-15 0:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Lionel G Landwerlin
MTL introduces additional OA units dedicated to media use cases. Add
support for programming these OA units.
UMD specific changes for GPUvis support:
https://patchwork.freedesktop.org/patch/522827/?series=114023
https://patchwork.freedesktop.org/patch/522822/?series=114023
https://patchwork.freedesktop.org/patch/522826/?series=114023
https://patchwork.freedesktop.org/patch/522828/?series=114023
https://patchwork.freedesktop.org/patch/522816/?series=114023
https://patchwork.freedesktop.org/patch/522825/?series=114023
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/i915_perf.c | 247 ++++++++++++++++++++---
drivers/gpu/drm/i915/i915_perf_oa_regs.h | 78 +++++++
drivers/gpu/drm/i915/i915_perf_types.h | 40 ++++
drivers/gpu/drm/i915/intel_device_info.h | 1 +
include/uapi/drm/i915_drm.h | 4 +
7 files changed, 347 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0393273faa09..f3cacbf41c86 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -856,6 +856,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
(INTEL_INFO(dev_priv)->has_oa_bpc_reporting)
#define HAS_OA_SLICE_CONTRIB_LIMITS(dev_priv) \
(INTEL_INFO(dev_priv)->has_oa_slice_contrib_limits)
+#define HAS_OAM(dev_priv) \
+ (INTEL_INFO(dev_priv)->has_oam)
/*
* Set this flag, when platform requires 64K GTT page sizes or larger for
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index a8d942b16223..621730b6551c 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1028,6 +1028,7 @@ static const struct intel_device_info adl_p_info = {
.has_mslice_steering = 1, \
.has_oa_bpc_reporting = 1, \
.has_oa_slice_contrib_limits = 1, \
+ .has_oam = 1, \
.has_rc6 = 1, \
.has_reset_engine = 1, \
.has_rps = 1, \
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 8f7306eaf43c..9fce3ea6a7dc 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -192,6 +192,7 @@
*/
#include <linux/anon_inodes.h>
+#include <linux/nospec.h>
#include <linux/sizes.h>
#include <linux/uuid.h>
@@ -326,6 +327,13 @@ static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
[I915_OAR_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
[I915_OA_FORMAT_A24u40_A14u32_B8_C8] = { 5, 256 },
+ [I915_OAM_FORMAT_MPEC8u64_B8_C8] = { 1, 192, TYPE_OAM, HDR_64_BIT },
+ [I915_OAM_FORMAT_MPEC8u32_B8_C8] = { 2, 128, TYPE_OAM, HDR_64_BIT },
+};
+
+/* PERF_GROUP_OAG is unused for oa_base, drop it for mtl */
+static const u32 mtl_oa_base[] = {
+ [PERF_GROUP_OAM_SAMEDIA_0] = 0x393000,
};
#define SAMPLE_OA_REPORT (1<<0)
@@ -418,11 +426,17 @@ static void free_oa_config_bo(struct i915_oa_config_bo *oa_bo)
kfree(oa_bo);
}
+static inline const
+struct i915_perf_regs *__oa_regs(struct i915_perf_stream *stream)
+{
+ return &stream->oa_buffer.group->regs;
+}
+
static u32 gen12_oa_hw_tail_read(struct i915_perf_stream *stream)
{
struct intel_uncore *uncore = stream->uncore;
- return intel_uncore_read(uncore, GEN12_OAG_OATAILPTR) &
+ return intel_uncore_read(uncore, __oa_regs(stream)->oa_tail_ptr) &
GEN12_OAG_OATAILPTR_MASK;
}
@@ -886,7 +900,8 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
i915_reg_t oaheadptr;
oaheadptr = GRAPHICS_VER(stream->perf->i915) == 12 ?
- GEN12_OAG_OAHEADPTR : GEN8_OAHEADPTR;
+ __oa_regs(stream)->oa_head_ptr :
+ GEN8_OAHEADPTR;
spin_lock_irqsave(&stream->oa_buffer.ptr_lock, flags);
@@ -939,7 +954,8 @@ static int gen8_oa_read(struct i915_perf_stream *stream,
return -EIO;
oastatus_reg = GRAPHICS_VER(stream->perf->i915) == 12 ?
- GEN12_OAG_OASTATUS : GEN8_OASTATUS;
+ __oa_regs(stream)->oa_status :
+ GEN8_OASTATUS;
oastatus = intel_uncore_read(uncore, oastatus_reg);
@@ -1643,6 +1659,18 @@ free_noa_wait(struct i915_perf_stream *stream)
i915_vma_unpin_and_release(&stream->noa_wait, 0);
}
+/*
+ * intel_engine_lookup_user ensures that most of engine specific checks are
+ * taken care of, however, we can run into a case where the OA unit catering to
+ * the engine passed by the user is disabled for some reason. In such cases,
+ * ensure oa unit corresponding to an engine is functional. If there are no
+ * engines in the group, the unit is disabled.
+ */
+static bool oa_unit_functional(const struct intel_engine_cs *engine)
+{
+ return engine->oa_group && engine->oa_group->num_engines;
+}
+
static bool engine_supports_oa(const struct intel_engine_cs *engine)
{
enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
@@ -1651,11 +1679,29 @@ static bool engine_supports_oa(const struct intel_engine_cs *engine)
return false;
switch (platform) {
+ case INTEL_METEORLAKE:
+ return engine->class == RENDER_CLASS ||
+ ((engine->class == VIDEO_DECODE_CLASS ||
+ engine->class == VIDEO_ENHANCEMENT_CLASS) &&
+ engine->gt->type == GT_MEDIA);
default:
return engine->class == RENDER_CLASS;
}
}
+static bool engine_class_supports_oa_format(struct intel_engine_cs *engine, int type)
+{
+ switch (engine->class) {
+ case RENDER_CLASS:
+ return type == TYPE_OAG;
+ case VIDEO_DECODE_CLASS:
+ case VIDEO_ENHANCEMENT_CLASS:
+ return type == TYPE_OAM;
+ default:
+ return false;
+ }
+}
+
static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
{
struct i915_perf *perf = stream->perf;
@@ -1683,7 +1729,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
drm_WARN_ON(>->i915->drm,
intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc));
- intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(stream->uncore, g->fw_domains);
intel_engine_pm_put(stream->engine);
if (stream->ctx)
@@ -1807,8 +1853,8 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
spin_lock_irqsave(&stream->oa_buffer.ptr_lock, flags);
- intel_uncore_write(uncore, GEN12_OAG_OASTATUS, 0);
- intel_uncore_write(uncore, GEN12_OAG_OAHEADPTR,
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_status, 0);
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_head_ptr,
gtt_offset & GEN12_OAG_OAHEADPTR_MASK);
stream->oa_buffer.head = gtt_offset;
@@ -1820,9 +1866,9 @@ static void gen12_init_oa_buffer(struct i915_perf_stream *stream)
* to enable proper functionality of the overflow
* bit."
*/
- intel_uncore_write(uncore, GEN12_OAG_OABUFFER, gtt_offset |
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_buffer, gtt_offset |
OABUFFER_SIZE_16M | GEN8_OABUFFER_MEM_SELECT_GGTT);
- intel_uncore_write(uncore, GEN12_OAG_OATAILPTR,
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_tail_ptr,
gtt_offset & GEN12_OAG_OATAILPTR_MASK);
/* Mark that we need updated tail pointers to read from... */
@@ -2582,7 +2628,8 @@ gen8_modify_self(struct intel_context *ce,
return err;
}
-static int gen8_configure_context(struct i915_gem_context *ctx,
+static int gen8_configure_context(struct i915_perf_stream *stream,
+ struct i915_gem_context *ctx,
struct flex *flex, unsigned int count)
{
struct i915_gem_engines_iter it;
@@ -2592,7 +2639,8 @@ static int gen8_configure_context(struct i915_gem_context *ctx,
for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
GEM_BUG_ON(ce == ce->engine->kernel_context);
- if (!engine_supports_oa(ce->engine))
+ if (!engine_supports_oa(ce->engine) ||
+ ce->engine->class != stream->engine->class)
continue;
/* Otherwise OA settings will be set upon first use */
@@ -2723,7 +2771,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
spin_unlock(&i915->gem.contexts.lock);
- err = gen8_configure_context(ctx, regs, num_regs);
+ err = gen8_configure_context(stream, ctx, regs, num_regs);
if (err) {
i915_gem_context_put(ctx);
return err;
@@ -2743,7 +2791,8 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
for_each_uabi_engine(engine, i915) {
struct intel_context *ce = engine->kernel_context;
- if (!engine_supports_oa(ce->engine))
+ if (!engine_supports_oa(ce->engine) ||
+ ce->engine->class != stream->engine->class)
continue;
regs[0].value = intel_sseu_make_rpcs(engine->gt, &ce->sseu);
@@ -2768,6 +2817,9 @@ gen12_configure_all_contexts(struct i915_perf_stream *stream,
},
};
+ if (stream->engine->class != RENDER_CLASS)
+ return 0;
+
return oa_configure_all_contexts(stream,
regs, ARRAY_SIZE(regs),
active);
@@ -2897,7 +2949,7 @@ gen12_enable_metric_set(struct i915_perf_stream *stream,
_MASKED_BIT_ENABLE(GEN12_DISABLE_DOP_GATING));
}
- intel_uncore_write(uncore, GEN12_OAG_OA_DEBUG,
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_debug,
/* Disable clk ratio reports, like previous Gens. */
_MASKED_BIT_ENABLE(GEN12_OAG_OA_DEBUG_DISABLE_CLK_RATIO_REPORTS |
GEN12_OAG_OA_DEBUG_INCLUDE_CLK_RATIO) |
@@ -2907,7 +2959,7 @@ gen12_enable_metric_set(struct i915_perf_stream *stream,
*/
oag_report_ctx_switches(stream));
- intel_uncore_write(uncore, GEN12_OAG_OAGLBCTXCTRL, periodic ?
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_ctx_ctrl, periodic ?
(GEN12_OAG_OAGLBCTXCTRL_COUNTER_RESUME |
GEN12_OAG_OAGLBCTXCTRL_TIMER_ENABLE |
(period_exponent << GEN12_OAG_OAGLBCTXCTRL_TIMER_PERIOD_SHIFT))
@@ -3061,8 +3113,8 @@ static void gen8_oa_enable(struct i915_perf_stream *stream)
static void gen12_oa_enable(struct i915_perf_stream *stream)
{
- struct intel_uncore *uncore = stream->uncore;
- u32 report_format = stream->oa_buffer.format->format;
+ const struct i915_perf_regs *regs;
+ u32 val;
/*
* If we don't want OA reports from the OA buffer, then we don't even
@@ -3073,9 +3125,11 @@ static void gen12_oa_enable(struct i915_perf_stream *stream)
gen12_init_oa_buffer(stream);
- intel_uncore_write(uncore, GEN12_OAG_OACONTROL,
- (report_format << GEN12_OAG_OACONTROL_OA_COUNTER_FORMAT_SHIFT) |
- GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE);
+ regs = __oa_regs(stream);
+ val = (stream->oa_buffer.format->format << regs->oa_ctrl_counter_format_shift) |
+ GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE;
+
+ intel_uncore_write(stream->uncore, regs->oa_ctrl, val);
}
/**
@@ -3127,9 +3181,9 @@ static void gen12_oa_disable(struct i915_perf_stream *stream)
{
struct intel_uncore *uncore = stream->uncore;
- intel_uncore_write(uncore, GEN12_OAG_OACONTROL, 0);
+ intel_uncore_write(uncore, __oa_regs(stream)->oa_ctrl, 0);
if (intel_wait_for_register(uncore,
- GEN12_OAG_OACONTROL,
+ __oa_regs(stream)->oa_ctrl,
GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE, 0,
50))
drm_err(&stream->perf->i915->drm,
@@ -3332,6 +3386,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
stream->sample_size = sizeof(struct drm_i915_perf_record_header);
+ stream->oa_buffer.group = g;
stream->oa_buffer.format = &perf->oa_formats[props->oa_format];
if (drm_WARN_ON(&i915->drm, stream->oa_buffer.format->size == 0))
return -EINVAL;
@@ -3382,7 +3437,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
* references will effectively disable RC6.
*/
intel_engine_pm_get(stream->engine);
- intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_get(stream->uncore, g->fw_domains);
/*
* Wa_16011777198:dg2: GuC resets render as part of the Wa. This causes
@@ -3443,7 +3498,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
intel_guc_slpc_unset_gucrc_mode(>->uc.guc.slpc);
err_fw:
- intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
+ intel_uncore_forcewake_put(stream->uncore, g->fw_domains);
intel_engine_pm_put(stream->engine);
free_oa_configs(stream);
@@ -4041,6 +4096,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
u8 class, instance;
bool config_sseu = false;
struct drm_i915_gem_context_param_sseu user_sseu;
+ const struct i915_oa_format *f;
memset(props, 0, sizeof(struct perf_open_properties));
props->poll_oa_period = DEFAULT_POLL_PERIOD_NS;
@@ -4206,6 +4262,17 @@ static int read_properties_unlocked(struct i915_perf *perf,
if (!engine_supports_oa(props->engine))
return -EINVAL;
+ if (!oa_unit_functional(props->engine))
+ return -ENODEV;
+
+ i = array_index_nospec(props->oa_format, I915_OA_FORMAT_MAX);
+ f = &perf->oa_formats[i];
+ if (!engine_class_supports_oa_format(props->engine, f->type)) {
+ DRM_DEBUG("Invalid OA format %d for class %d\n",
+ f->type, props->engine->class);
+ return -EINVAL;
+ }
+
if (config_sseu) {
ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
if (ret) {
@@ -4385,6 +4452,14 @@ static const struct i915_range gen12_oa_b_counters[] = {
{}
};
+static const struct i915_range mtl_oam_b_counters[] = {
+ { .start = 0x393000, .end = 0x39301c }, /* GEN12_OAM_STARTTRIG1[1-8] */
+ { .start = 0x393020, .end = 0x39303c }, /* GEN12_OAM_REPORTTRIG1[1-8] */
+ { .start = 0x393040, .end = 0x39307c }, /* GEN12_OAM_CEC[0-7][0-1] */
+ { .start = 0x393200, .end = 0x39323C }, /* MPES[0-7] */
+ {}
+};
+
static const struct i915_range xehp_oa_b_counters[] = {
{ .start = 0xdc48, .end = 0xdc48 }, /* OAA_ENABLE_REG */
{ .start = 0xdd00, .end = 0xdd48 }, /* OAG_LCE0_0 - OAA_LENABLE_REG */
@@ -4431,13 +4506,16 @@ static const struct i915_range gen12_oa_mux_regs[] = {
/*
* Ref: 14010536224:
- * 0x20cc is repurposed on MTL, so use a separate array for MTL.
+ * 0x20cc is repurposed on MTL, so use a separate array for MTL. Also add the
+ * MPES/MPEC registers.
*/
static const struct i915_range mtl_oa_mux_regs[] = {
{ .start = 0x0d00, .end = 0x0d04 }, /* RPM_CONFIG[0-1] */
{ .start = 0x0d0c, .end = 0x0d2c }, /* NOA_CONFIG[0-8] */
{ .start = 0x9840, .end = 0x9840 }, /* GDT_CHICKEN_BITS */
{ .start = 0x9884, .end = 0x9888 }, /* NOA_WRITE */
+ { .start = 0x38d100, .end = 0x38d114}, /* VISACTL */
+ {}
};
static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
@@ -4475,10 +4553,26 @@ static bool gen12_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
return reg_in_range_table(addr, gen12_oa_b_counters);
}
+static bool xehp_is_valid_oam_b_counter_addr(struct i915_perf *perf, u32 addr)
+{
+ enum intel_platform platform = INTEL_INFO(perf->i915)->platform;
+
+ if (!HAS_OAM(perf->i915))
+ return false;
+
+ switch (platform) {
+ case INTEL_METEORLAKE:
+ return reg_in_range_table(addr, mtl_oam_b_counters);
+ default:
+ return false;
+ }
+}
+
static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
{
return reg_in_range_table(addr, xehp_oa_b_counters) ||
- reg_in_range_table(addr, gen12_oa_b_counters);
+ reg_in_range_table(addr, gen12_oa_b_counters) ||
+ xehp_is_valid_oam_b_counter_addr(perf, addr);
}
static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
@@ -4848,11 +4942,39 @@ static u32 __num_perf_groups_per_gt(struct intel_gt *gt)
enum intel_platform platform = INTEL_INFO(gt->i915)->platform;
switch (platform) {
+ case INTEL_METEORLAKE:
+ return 1;
default:
return 1;
}
}
+static u32 __oam_engine_group(struct intel_engine_cs *engine)
+{
+ enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
+ struct intel_gt *gt = engine->gt;
+ u32 group = PERF_GROUP_INVALID;
+
+ switch (platform) {
+ case INTEL_METEORLAKE:
+ /*
+ * There's 1 SAMEDIA gt and 1 OAM per SAMEDIA gt. All media slices
+ * within the gt use the same OAM. All MTL SKUs list 1 SA MEDIA.
+ */
+ drm_WARN_ON(&engine->i915->drm,
+ engine->gt->type != GT_MEDIA);
+
+ group = PERF_GROUP_OAM_SAMEDIA_0;
+ break;
+ default:
+ break;
+ }
+
+ drm_WARN_ON(>->i915->drm, group >= __num_perf_groups_per_gt(gt));
+
+ return group;
+}
+
static u32 __oa_engine_group(struct intel_engine_cs *engine)
{
if (!engine_supports_oa(engine))
@@ -4862,11 +4984,58 @@ static u32 __oa_engine_group(struct intel_engine_cs *engine)
case RENDER_CLASS:
return PERF_GROUP_OAG;
+ case VIDEO_DECODE_CLASS:
+ case VIDEO_ENHANCEMENT_CLASS:
+ return __oam_engine_group(engine);
+
default:
return PERF_GROUP_INVALID;
}
}
+static struct i915_perf_regs __oam_regs(u32 base)
+{
+ return (struct i915_perf_regs) {
+ base,
+ GEN12_OAM_HEAD_POINTER(base),
+ GEN12_OAM_TAIL_POINTER(base),
+ GEN12_OAM_BUFFER(base),
+ GEN12_OAM_CONTEXT_CONTROL(base),
+ GEN12_OAM_CONTROL(base),
+ GEN12_OAM_DEBUG(base),
+ GEN12_OAM_STATUS(base),
+ GEN12_OAM_CONTROL_COUNTER_FORMAT_SHIFT,
+ };
+}
+
+static struct i915_perf_regs __oag_regs(void)
+{
+ return (struct i915_perf_regs) {
+ 0,
+ GEN12_OAG_OAHEADPTR,
+ GEN12_OAG_OATAILPTR,
+ GEN12_OAG_OABUFFER,
+ GEN12_OAG_OAGLBCTXCTRL,
+ GEN12_OAG_OACONTROL,
+ GEN12_OAG_OA_DEBUG,
+ GEN12_OAG_OASTATUS,
+ GEN12_OAG_OACONTROL_OA_COUNTER_FORMAT_SHIFT,
+ };
+}
+
+static void oa_init_regs(struct intel_gt *gt, u32 id)
+{
+ struct i915_perf_group *group = >->perf.group[id];
+ struct i915_perf_regs *regs = &group->regs;
+
+ if (id == PERF_GROUP_OAG && gt->type != GT_MEDIA)
+ *regs = __oag_regs();
+ else if (IS_METEORLAKE(gt->i915))
+ *regs = __oam_regs(mtl_oa_base[id]);
+ else
+ drm_WARN(>->i915->drm, 1, "Unsupported platform for OA\n");
+}
+
static void oa_init_groups(struct intel_gt *gt)
{
int i, num_groups = gt->perf.num_perf_groups;
@@ -4883,6 +5052,24 @@ static void oa_init_groups(struct intel_gt *gt)
g->oa_unit_id = perf->oa_unit_ids++;
g->gt = gt;
+ oa_init_regs(gt, i);
+ g->fw_domains = FORCEWAKE_ALL;
+ if (i == PERF_GROUP_OAG) {
+ g->type = TYPE_OAG;
+
+ /*
+ * Enabling all fw domains for OAG caps the max GT
+ * frequency to media FF max. This could be less than
+ * what the user sets through the sysfs and perf
+ * measurements could be skewed. Since some platforms
+ * have separate OAM units to measure media perf, do not
+ * enable media fw domains for OAG.
+ */
+ if (HAS_OAM(gt->i915))
+ g->fw_domains = FORCEWAKE_GT | FORCEWAKE_RENDER;
+ } else {
+ g->type = TYPE_OAM;
+ }
}
}
@@ -4972,9 +5159,15 @@ static void oa_init_supported_formats(struct i915_perf *perf)
break;
case INTEL_DG2:
+ oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
+ oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
+ break;
+
case INTEL_METEORLAKE:
oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
+ oa_format_add(perf, I915_OAM_FORMAT_MPEC8u64_B8_C8);
+ oa_format_add(perf, I915_OAM_FORMAT_MPEC8u32_B8_C8);
break;
default:
@@ -5219,8 +5412,10 @@ int i915_perf_ioctl_version(void)
*
* 6: Add DRM_I915_PERF_PROP_OA_ENGINE_CLASS and
* DRM_I915_PERF_PROP_OA_ENGINE_INSTANCE
+ *
+ * 7: Add support for video decode and enhancement classes.
*/
- return 6;
+ return 7;
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/i915_perf_oa_regs.h b/drivers/gpu/drm/i915/i915_perf_oa_regs.h
index 381d94101610..ba103875e19f 100644
--- a/drivers/gpu/drm/i915/i915_perf_oa_regs.h
+++ b/drivers/gpu/drm/i915/i915_perf_oa_regs.h
@@ -138,4 +138,82 @@
#define GEN12_SQCNT1_PMON_ENABLE REG_BIT(30)
#define GEN12_SQCNT1_OABPC REG_BIT(29)
+/* Gen12 OAM unit */
+#define GEN12_OAM_HEAD_POINTER_OFFSET (0x1a0)
+#define GEN12_OAM_HEAD_POINTER_MASK 0xffffffc0
+
+#define GEN12_OAM_TAIL_POINTER_OFFSET (0x1a4)
+#define GEN12_OAM_TAIL_POINTER_MASK 0xffffffc0
+
+#define GEN12_OAM_BUFFER_OFFSET (0x1a8)
+#define GEN12_OAM_BUFFER_SIZE_MASK (0x7)
+#define GEN12_OAM_BUFFER_SIZE_SHIFT (3)
+#define GEN12_OAM_BUFFER_MEMORY_SELECT REG_BIT(0) /* 0: PPGTT, 1: GGTT */
+
+#define GEN12_OAM_CONTEXT_CONTROL_OFFSET (0x1bc)
+#define GEN12_OAM_CONTEXT_CONTROL_TIMER_PERIOD_SHIFT 2
+#define GEN12_OAM_CONTEXT_CONTROL_TIMER_ENABLE REG_BIT(1)
+#define GEN12_OAM_CONTEXT_CONTROL_COUNTER_RESUME REG_BIT(0)
+
+#define GEN12_OAM_CONTROL_OFFSET (0x194)
+#define GEN12_OAM_CONTROL_COUNTER_FORMAT_SHIFT 1
+#define GEN12_OAM_CONTROL_COUNTER_ENABLE REG_BIT(0)
+
+#define GEN12_OAM_DEBUG_OFFSET (0x198)
+#define GEN12_OAM_DEBUG_BUFFER_SIZE_SELECT REG_BIT(12)
+#define GEN12_OAM_DEBUG_INCLUDE_CLK_RATIO REG_BIT(6)
+#define GEN12_OAM_DEBUG_DISABLE_CLK_RATIO_REPORTS REG_BIT(5)
+#define GEN12_OAM_DEBUG_DISABLE_GO_1_0_REPORTS REG_BIT(2)
+#define GEN12_OAM_DEBUG_DISABLE_CTX_SWITCH_REPORTS REG_BIT(1)
+
+#define GEN12_OAM_STATUS_OFFSET (0x19c)
+#define GEN12_OAM_STATUS_COUNTER_OVERFLOW REG_BIT(2)
+#define GEN12_OAM_STATUS_BUFFER_OVERFLOW REG_BIT(1)
+#define GEN12_OAM_STATUS_REPORT_LOST REG_BIT(0)
+
+#define GEN12_OAM_MMIO_TRG_OFFSET (0x1d0)
+
+#define GEN12_OAM_MMIO_TRG(base) \
+ _MMIO((base) + GEN12_OAM_MMIO_TRG_OFFSET)
+
+#define GEN12_OAM_HEAD_POINTER(base) \
+ _MMIO((base) + GEN12_OAM_HEAD_POINTER_OFFSET)
+#define GEN12_OAM_TAIL_POINTER(base) \
+ _MMIO((base) + GEN12_OAM_TAIL_POINTER_OFFSET)
+#define GEN12_OAM_BUFFER(base) \
+ _MMIO((base) + GEN12_OAM_BUFFER_OFFSET)
+#define GEN12_OAM_CONTEXT_CONTROL(base) \
+ _MMIO((base) + GEN12_OAM_CONTEXT_CONTROL_OFFSET)
+#define GEN12_OAM_CONTROL(base) \
+ _MMIO((base) + GEN12_OAM_CONTROL_OFFSET)
+#define GEN12_OAM_DEBUG(base) \
+ _MMIO((base) + GEN12_OAM_DEBUG_OFFSET)
+#define GEN12_OAM_STATUS(base) \
+ _MMIO((base) + GEN12_OAM_STATUS_OFFSET)
+
+#define GEN12_OAM_CEC0_0_OFFSET (0x40)
+#define GEN12_OAM_CEC7_1_OFFSET (0x7c)
+#define GEN12_OAM_CEC0_0(base) \
+ _MMIO((base) + GEN12_OAM_CEC0_0_OFFSET)
+#define GEN12_OAM_CEC7_1(base) \
+ _MMIO((base) + GEN12_OAM_CEC7_1_OFFSET)
+
+#define GEN12_OAM_STARTTRIG1_OFFSET (0x00)
+#define GEN12_OAM_STARTTRIG8_OFFSET (0x1c)
+#define GEN12_OAM_STARTTRIG1(base) \
+ _MMIO((base) + GEN12_OAM_STARTTRIG1_OFFSET)
+#define GEN12_OAM_STARTTRIG8(base) \
+ _MMIO((base) + GEN12_OAM_STARTTRIG8_OFFSET)
+
+#define GEN12_OAM_REPORTTRIG1_OFFSET (0x20)
+#define GEN12_OAM_REPORTTRIG8_OFFSET (0x3c)
+#define GEN12_OAM_REPORTTRIG1(base) \
+ _MMIO((base) + GEN12_OAM_REPORTTRIG1_OFFSET)
+#define GEN12_OAM_REPORTTRIG8(base) \
+ _MMIO((base) + GEN12_OAM_REPORTTRIG8_OFFSET)
+
+#define GEN12_OAM_PERF_COUNTER_B0_OFFSET (0x84)
+#define GEN12_OAM_PERF_COUNTER_B(base, idx) \
+ _MMIO((base) + GEN12_OAM_PERF_COUNTER_B0_OFFSET + 4 * (idx))
+
#endif /* __INTEL_PERF_OA_REGS__ */
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index 8ccb0b89d019..5b2c3bab60f8 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -20,6 +20,7 @@
#include "gt/intel_engine_types.h"
#include "gt/intel_sseu.h"
#include "i915_reg_defs.h"
+#include "intel_uncore.h"
#include "intel_wakeref.h"
struct drm_i915_private;
@@ -33,6 +34,7 @@ struct intel_engine_cs;
enum {
PERF_GROUP_OAG = 0,
+ PERF_GROUP_OAM_SAMEDIA_0 = 0,
PERF_GROUP_MAX,
PERF_GROUP_INVALID = U32_MAX,
@@ -43,9 +45,27 @@ enum report_header {
HDR_64_BIT,
};
+struct i915_perf_regs {
+ u32 base;
+ i915_reg_t oa_head_ptr;
+ i915_reg_t oa_tail_ptr;
+ i915_reg_t oa_buffer;
+ i915_reg_t oa_ctx_ctrl;
+ i915_reg_t oa_ctrl;
+ i915_reg_t oa_debug;
+ i915_reg_t oa_status;
+ u32 oa_ctrl_counter_format_shift;
+};
+
+enum {
+ TYPE_OAG,
+ TYPE_OAM,
+};
+
struct i915_oa_format {
u32 format;
int size;
+ int type;
enum report_header header;
};
@@ -317,6 +337,11 @@ struct i915_perf_stream {
* @tail: The last verified tail that can be read by userspace.
*/
u32 tail;
+
+ /**
+ * @group: The group object for this OA buffer.
+ */
+ struct i915_perf_group *group;
} oa_buffer;
/**
@@ -431,6 +456,21 @@ struct i915_perf_group {
* @engine_mask: A mask of engines using a single OA buffer.
*/
intel_engine_mask_t engine_mask;
+
+ /*
+ * @regs: OA buffer register group for programming the OA unit.
+ */
+ struct i915_perf_regs regs;
+
+ /*
+ * @type: Type of OA buffer, OAM, OAG etc.
+ */
+ int type;
+
+ /*
+ * @fw_domains: forcewake domains required for this group.
+ */
+ enum forcewake_domains fw_domains;
};
struct i915_perf_gt {
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 80bda653d61b..45e218327f44 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -166,6 +166,7 @@ enum intel_ppgtt_type {
func(has_mslice_steering); \
func(has_oa_bpc_reporting); \
func(has_oa_slice_contrib_limits); \
+ func(has_oam); \
func(has_one_eu_per_fuse_bit); \
func(has_pxp); \
func(has_rc6); \
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index b6922b52d85c..70bfa6530dbc 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2676,6 +2676,10 @@ enum drm_i915_oa_format {
I915_OAR_FORMAT_A32u40_A4u32_B8_C8,
I915_OA_FORMAT_A24u40_A14u32_B8_C8,
+ /* MTL OAM */
+ I915_OAM_FORMAT_MPEC8u64_B8_C8,
+ I915_OAM_FORMAT_MPEC8u32_B8_C8,
+
I915_OA_FORMAT_MAX /* non-ABI */
};
--
2.36.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units
2023-02-15 0:54 ` [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units Umesh Nerlige Ramappa
@ 2023-02-16 17:27 ` Dixit, Ashutosh
2023-02-16 21:07 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 17:27 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Tue, 14 Feb 2023 16:54:19 -0800, Umesh Nerlige Ramappa wrote:
>
> MTL introduces additional OA units dedicated to media use cases. Add
> support for programming these OA units.
This patch is several patches squashed into a single patch. Sorry for
creating addition work but can we please split this up into smaller
patches. Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units
2023-02-16 17:27 ` Dixit, Ashutosh
@ 2023-02-16 21:07 ` Umesh Nerlige Ramappa
2023-02-16 21:23 ` Dixit, Ashutosh
0 siblings, 1 reply; 36+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-02-16 21:07 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, Feb 16, 2023 at 09:27:47AM -0800, Dixit, Ashutosh wrote:
>On Tue, 14 Feb 2023 16:54:19 -0800, Umesh Nerlige Ramappa wrote:
>>
>> MTL introduces additional OA units dedicated to media use cases. Add
>> support for programming these OA units.
>
>This patch is several patches squashed into a single patch. Sorry for
>creating addition work but can we please split this up into smaller
>patches. Thanks.
I don't recall squashing this, it is still similar to internal repo.
I will however try to see if I can break it down into smaller functional
pieces so that it is easier to review.
Thanks,
Umesh
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units
2023-02-16 21:07 ` Umesh Nerlige Ramappa
@ 2023-02-16 21:23 ` Dixit, Ashutosh
0 siblings, 0 replies; 36+ messages in thread
From: Dixit, Ashutosh @ 2023-02-16 21:23 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: Lionel G Landwerlin, intel-gfx
On Thu, 16 Feb 2023 13:07:25 -0800, Umesh Nerlige Ramappa wrote:
>
> On Thu, Feb 16, 2023 at 09:27:47AM -0800, Dixit, Ashutosh wrote:
> > On Tue, 14 Feb 2023 16:54:19 -0800, Umesh Nerlige Ramappa wrote:
> >>
> >> MTL introduces additional OA units dedicated to media use cases. Add
> >> support for programming these OA units.
> >
> > This patch is several patches squashed into a single patch. Sorry for
> > creating addition work but can we please split this up into smaller
> > patches. Thanks.
>
> I don't recall squashing this, it is still similar to internal repo.
>
> I will however try to see if I can break it down into smaller functional
> pieces so that it is easier to review.
Maybe skip splitting into smaller patches this time, I will review. Next
time we can make sure that right sized patches make it into the internal
repo too :). Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add OAM support for MTL
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (8 preceding siblings ...)
2023-02-15 0:54 ` [Intel-gfx] [PATCH 9/9] drm/i915/perf: Add support for OA media units Umesh Nerlige Ramappa
@ 2023-02-15 1:38 ` Patchwork
2023-02-15 2:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2023-02-15 1:38 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
== Series Details ==
Series: Add OAM support for MTL
URL : https://patchwork.freedesktop.org/series/114033/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for Add OAM support for MTL
2023-02-15 0:54 [Intel-gfx] [PATCH 0/9] Add OAM support for MTL Umesh Nerlige Ramappa
` (9 preceding siblings ...)
2023-02-15 1:38 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Add OAM support for MTL Patchwork
@ 2023-02-15 2:05 ` Patchwork
10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2023-02-15 2:05 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7114 bytes --]
== Series Details ==
Series: Add OAM support for MTL
URL : https://patchwork.freedesktop.org/series/114033/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_12739 -> Patchwork_114033v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_114033v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_114033v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/index.html
Participating hosts (39 -> 39)
------------------------------
Additional (1): bat-rpls-2
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_114033v1:
### IGT changes ###
#### Possible regressions ####
* igt@i915_suspend@basic-s2idle-without-i915:
- fi-cfl-8109u: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_suspend@basic-s2idle-without-i915:
- {bat-rpls-1}: NOTRUN -> [ABORT][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html
Known issues
------------
Here are the changes found in Patchwork_114033v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@hangcheck:
- fi-kbl-soraka: [PASS][4] -> [INCOMPLETE][5] ([i915#7913])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-kbl-soraka/igt@i915_selftest@live@hangcheck.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-kbl-soraka/igt@i915_selftest@live@hangcheck.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
- fi-bsw-n3050: [PASS][6] -> [FAIL][7] ([i915#6298])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- fi-pnv-d510: [FAIL][8] ([i915#7229]) -> [PASS][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-pnv-d510/igt@gem_exec_gttfill@basic.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-pnv-d510/igt@gem_exec_gttfill@basic.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [DMESG-FAIL][10] ([i915#5334] / [i915#7872]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
- fi-apl-guc: [DMESG-FAIL][12] ([i915#5334]) -> [PASS][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@reset:
- {bat-rpls-1}: [ABORT][14] ([i915#4983]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12739/bat-rpls-1/igt@i915_selftest@live@reset.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/bat-rpls-1/igt@i915_selftest@live@reset.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#7102]: https://gitlab.freedesktop.org/drm/intel/issues/7102
[i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
Build changes
-------------
* IGT: IGT_7160 -> IGTPW_8498
* Linux: CI_DRM_12739 -> Patchwork_114033v1
CI-20190529: 20190529
CI_DRM_12739: 5fc904286af94038fbf2c7cda50ed871b70cf4e8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8498: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8498/index.html
IGT_7160: 45da871dd2684227e93a2fc002b87dfc58bd5fd9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_114033v1: 5fc904286af94038fbf2c7cda50ed871b70cf4e8 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
192bc8520091 drm/i915/perf: Add support for OA media units
84b6e24bbdb2 drm/i915/perf: Add engine class instance parameters to perf
f4496df56191 drm/i915/perf: Handle non-power-of-2 reports
318c5541f507 drm/i915/perf: Parse 64bit report header formats correctly
7f5aeb858190 drm/i915/perf: Group engines into respective OA groups
2ea748f55cbc drm/i915/perf: Fail modprobe if i915_perf_init fails
fef287c42287 drm/i915/perf: Validate OA sseu config outside switch
8d941260d3af drm/i915/perf: Add helper to check supported OA engines
b9a48e73e1d7 drm/i915/perf: Drop wakeref on GuC RC error
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114033v1/index.html
[-- Attachment #2: Type: text/html, Size: 6781 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread