Re: [PATCH 4/7] drm/i915/perf: lock powergating configuration to default when active

Chris Wilson
2018-09-05T15:21:58Z

Quoting Tvrtko Ursulin (2018-09-05 15:22:19)
> -static u32 make_rpcs(struct drm_i915_private *dev_priv,
> - struct intel_sseu *ctx_sseu)
> +u32 gen8_make_rpcs(struct drm_i915_private *dev_priv,
> + struct intel_sseu *req_sseu)

Should we retrospectively make this const?

(And anychance for a s/dev_priv/i915?)

> {
> const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
> bool subslice_pg = sseu->has_subslice_pg;
> - u8 slices = hweight8(ctx_sseu->slice_mask);
> - u8 subslices = hweight8(ctx_sseu->subslice_mask);
> + struct intel_sseu ctx_sseu;
> + u8 slices, subslices;
> u32 rpcs = 0;
>
> + /*
> + * If i915/perf is active, we want a stable powergating configuration
> + * on the system. The most natural configuration to take in that case
> + * is the default (i.e maximum the hardware can do).
> + */
> + if (unlikely(dev_priv->perf.oa.exclusive_stream))
> + ctx_sseu = intel_device_default_sseu(dev_priv);
> + else
> + ctx_sseu = *req_sseu;

:(

I'm not sure if I can suggest anything better, but this does feel like a
layering violation.

It makes sense which makes it only feel worse.
-Chris