On 2/28/2018 11:29 PM, Oscar Mateo wrote:


On 2/26/2018 9:49 PM, Sagar Arun Kamble wrote:


On 2/27/2018 4:34 AM, Oscar Mateo wrote:


On 2/25/2018 9:22 PM, Sagar Arun Kamble wrote:


On 2/23/2018 4:35 AM, Oscar Mateo wrote:


<snip>
+ * We might have detected that some engines are fused off after we initialized + * the forcewake domains. Prune them, to make sure they only reference existing
+ * engines.
+ */
+void intel_uncore_prune(struct drm_i915_private *dev_priv)
+{
+    if (INTEL_GEN(dev_priv) >= 11) {
+        enum forcewake_domains fw_domains = dev_priv->uncore.fw_domains;
+        enum forcewake_domain_id domain_id;
+        int i;
+
+        for (i = 0; i < I915_MAX_VCS; i++) {
+            domain_id = FW_DOMAIN_ID_MEDIA_VDBOX0 + i;
+
+            if (HAS_ENGINE(dev_priv, _VCS(i)))
+                continue;
+
+            if (fw_domains & BIT(domain_id))
fw_domains check seems redundant as it is initialized based on HAS_ENGINE.
we can just have
if (!HAS_ENGINE(dev_priv, _VCS(i)))
    fw_domain_fini(dev_priv, domain_id);

I don't want to call fw_domain_fini on something we never initialized in the first place (e.g. VCS1/3 and VECS1/2/3 on an ICL-LP).

Right. Makes sense.
for loop iterating over engines based on ring_mask can help us rely on only HAS_ENGINE condition and then we can have complete pruning in single for loop.
what do you think?

Hmmm.. I'm not sure I follow: intel_device_info_init_mmio modifies ring_mask, so if I loop over engines based on ring_mask I am going to miss those I want to prune, right?

Oops ... you are right ...
My suggestion about skipping fw_domains check will not work currently. In future may be if we create default ring_mask and runtime ring_mask it can be reworked.

Other suggestion to use single for loop (for_each_engine()) can be done I think. It will make it generic for all engine types.  Below is what I am thinking of as part of intel_uncore_prune:

for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
    if (HAS_ENGINE(dev_priv, i))
        continue;
    if (fw_domains & BIT(i))
        fw_domain_fini(dev_priv, i);
}

This won't work either, because the index for engines and forcewake domains is different. If you think it is important to make the prune function more generic, I can translate between the two (but it will be for naught if, as you mentioned, we are working to create separate default ring_mask and runtime ring_mask in the future).

Yes. Translation will help (I thought of FW_D_ID_MEDIA to be reused for FW_D_ID_MEDIA_VDB0). I think this patch can go in current shape and will pursue other changes as follow up based on inputs.

I feel making it generic will allow pruning to scale across engine types (if that is needed in future). I am not sure if we want to pursue the default/runtime ring_mask change (another use case of this that i think is if user wants to know default config vs fused)
Tvrtko, Chris - what do you think?
+ fw_domain_fini(dev_priv, domain_id);
+        }
+
+        for (i = 0; i < I915_MAX_VECS; i++) {
+            domain_id = FW_DOMAIN_ID_MEDIA_VEBOX0 + i;
+
+            if (HAS_ENGINE(dev_priv, _VECS(i)))
+                continue;
+
+            if (fw_domains & BIT(domain_id))
+                fw_domain_fini(dev_priv, domain_id);
+        }
+    }
+}
+
  void intel_uncore_fini(struct drm_i915_private *dev_priv)
  {
      /* Paranoia: make sure we have disabled everything before we exit. */ diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 53ef77d..28feabf 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -129,6 +129,7 @@ struct intel_uncore {
    void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
  void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_prune(struct drm_i915_private *dev_priv);
  bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);   bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv);
  void intel_uncore_fini(struct drm_i915_private *dev_priv);







--
Thanks,
Sagar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to