[Intel-gfx] [PATCH v9] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

[Oscar Mateo]

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?

>>>> + 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);
>>>
>>
>