[Intel-gfx] [PATCH] drm/i915/perf: introduce global sseu pinning

Chris Wilson chris at chris-wilson.co.uk
Fri Feb 28 16:44:53 UTC 2020


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.

> @@ -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;
?
-Chris


More information about the Intel-gfx mailing list