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
