On 28/02/2020 18:44, Chris Wilson wrote:
Quoting Lionel Landwerlin (2020-02-28 16:02:29)
On Gen11 powergating half the execution units is a functional
requirement when using the VME samplers. Not fullfilling this
requirement can lead to hangs.
This unfortunately plays fairly poorly with the NOA requirements. NOA
requires a stable power configuration to maintain its configuration.
As a result using OA (and NOA feeding into it) so far has required us
to use a power configuration that can work for all contexts. The only
power configuration fullfilling this is powergating half the execution
units.
This makes performance analysis for 3D workloads somewhat pointless.
Failing to find a solution that would work for everybody, this change
introduces a new i915-perf stream open parameter that punts the
decision off to userspace. If this parameter is omitted, the existing
Gen11 behavior remains (half EU array powergating).
This change takes the initiative to move all perf related sseu
configuration into i915_perf.c
The code looks fine, your argument is sound. My only reservation is the
danger of this becoming the defacto default and so catching users's
profiling their system by surprise.
As far as I can see, this will only be using non default (default = full
EU array on !gen11, default = half EU array on gen11) on Gen11.
Except Gen11 we don't have those asymetric subslices (remember gen12 has
dual slices of 16 EUs).
It all bad choices :
- let everybody do what they want but risk invalid OA data with no
warning
- force everybody to the same config and only on gen11 risk a hang
if VME samplers end up being used (which is a subset of all media workloads)
@@ -3628,6 +3678,16 @@ static int read_properties_unlocked(struct i915_perf
*perf,
case DRM_I915_PERF_PROP_HOLD_PREEMPTION:
props->hold_preemption = !!value;
break;
+ case DRM_I915_PERF_PROP_GLOBAL_SSEU: {
+ if (copy_from_user(&props->user_sseu,
+ u64_to_user_ptr(value),
+ sizeof(props->user_sseu))) {
+ DRM_DEBUG("Unable to copy global sseu
parameter\n");
+ return -EFAULT;
+ }
Since this affects system state for other users, I would suggest this
has a privilege check
+ props->user_sseu_present = true;
+ break;
i915_perf_ioctl_open_locked:
if (props->user_sseu_present && IS_GEN(11))
privileged_op = true;
?
I'm going to go with priviliged for all gens except if user request ==
default.
Thanks a lot,
-Lionel
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
