Re: [PATCH 1/8] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

This patch seems to have fallen through the cracks..

On 19/03/2018 15:26, Sagar Arun Kamble wrote:
> 
> 
> On 3/16/2018 5:44 PM, Mika Kuoppala wrote:
>> From: Oscar Mateo <oscar.mateo@intel.com>
>>
>> In Gen11, the Video Decode engines (aka VDBOX, aka VCS, aka BSD) and the
>> Video Enhancement engines (aka VEBOX, aka VECS) could be fused off. Also,
>> each VDBOX and VEBOX has its own power well, which only exist if the 
>> related
>> engine exists in the HW.
>>
>> Unfortunately, we have a Catch-22 situation going on: we need the blitter
>> forcewake to read the register with the fuse info, but we cannot 
>> initialize
>> the forcewake domains without knowin about the engines present in the HW.
>> We workaround this problem by allowing the initialization of all 
>> forcewake
>> domains and then pruning the fused off ones, as per the fuse information.
>>
>> Bspec: 20680
>>
>> v2: We were shifting incorrectly for vebox disable (Vinay)
>>
>> v3: Assert mmio is ready and warn if we have attempted to initialize
>>      forcewake for fused-off engines (Paulo)
>>
>> v4:
>>    - Use INTEL_GEN in new code (Tvrtko)
>>    - Shorter local variable (Tvrtko, Michal)
>>    - Keep "if (!...) continue" style (Tvrtko)
>>    - No unnecessary BUG_ON (Tvrtko)
>>    - WARN_ON and cleanup if wrong mask (Tvrtko, Michal)
>>    - Use I915_READ_FW (Michal)
>>    - Use I915_MAX_VCS/VECS macros (Michal)
>>
>> v5: Rebased by Rodrigo fixing conflicts on top of:
>>      commit 33def1ff7b0 ("drm/i915: Simplify intel_engines_init")
>>
>> v6: Fix v5. Remove info->num_rings. (by Oscar)
>>
>> v7: Rebase (Rodrigo).
>>
>> v8:
>>    - s/intel_device_info_fused_off_engines/intel_device_info_init_mmio 
>> (Chris)
>>    - Make vdbox_disable & vebox_disable local variables (Chris)
>>
>> v9:
>>    - Move function declaration to intel_device_info.h (Michal)
>>    - Missing indent in bit fields definitions (Michal)
>>    - When RC6 is enabled by BIOS, the fuse register cannot be read until
>>      the blitter powerwell is awake. Shuffle where the fuse is read, 
>> prune
>>      the forcewake domains after the fact and change the commit message
>>      accordingly (Vinay, Sagar, Chris).
>>
>> v10:
>>    - Improved commit message (Sagar)
>>    - New line in header file (Sagar)
>>    - Specify the message in fw_domain_reset applies to ICL+ (Sagar)
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c          |  4 +++
>>   drivers/gpu/drm/i915/i915_reg.h          |  5 +++
>>   drivers/gpu/drm/i915/intel_device_info.c | 47 
>> +++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_device_info.h |  2 ++
>>   drivers/gpu/drm/i915/intel_uncore.c      | 56 
>> ++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_uncore.h      |  1 +
>>   6 files changed, 115 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 3df5193487f3..83df8e21cec0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1033,6 +1033,10 @@ static int i915_driver_init_mmio(struct 
>> drm_i915_private *dev_priv)
>>       intel_uncore_init(dev_priv);
>> +    intel_device_info_init_mmio(dev_priv);
>> +
>> +    intel_uncore_prune(dev_priv);
>> +
>>       intel_uc_init_mmio(dev_priv);
>>       ret = intel_engines_init_mmio(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index cf7c837d6a09..982e72e73e99 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2545,6 +2545,11 @@ enum i915_power_well_id {
>>   #define GEN10_EU_DISABLE3        _MMIO(0x9140)
>>   #define   GEN10_EU_DIS_SS_MASK        0xff
>> +#define GEN11_GT_VEBOX_VDBOX_DISABLE    _MMIO(0x9140)
>> +#define   GEN11_GT_VDBOX_DISABLE_MASK    0xff
>> +#define   GEN11_GT_VEBOX_DISABLE_SHIFT    16
>> +#define   GEN11_GT_VEBOX_DISABLE_MASK    (0xff << 
>> GEN11_GT_VEBOX_DISABLE_SHIFT)
>> +
>>   #define GEN6_BSD_SLEEP_PSMI_CONTROL    _MMIO(0x12050)
>>   #define   GEN6_BSD_SLEEP_MSG_DISABLE    (1 << 0)
>>   #define   GEN6_BSD_SLEEP_FLUSH_DISABLE    (1 << 2)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
>> b/drivers/gpu/drm/i915/intel_device_info.c
>> index 3dd350f7b8e6..4babfc6ee45b 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -780,3 +780,50 @@ void intel_driver_caps_print(const struct 
>> intel_driver_caps *caps,
>>   {
>>       drm_printf(p, "scheduler: %x\n", caps->scheduler);
>>   }
>> +
>> +/*
>> + * Determine which engines are fused off in our particular hardware. 
>> Since the
>> + * fuse register is in the blitter powerwell, we need forcewake to be 
>> ready at
>> + * this point (but later we need to prune the forcewake domains for 
>> engines that
>> + * are indeed fused off).
>> + */
>> +void intel_device_info_init_mmio(struct drm_i915_private *dev_priv)
>> +{
>> +    struct intel_device_info *info = mkwrite_device_info(dev_priv);
>> +    u8 vdbox_disable, vebox_disable;
>> +    u32 media_fuse;
>> +    int i;
>> +
>> +    if (INTEL_GEN(dev_priv) < 11)
>> +        return;
>> +
>> +    media_fuse = I915_READ(GEN11_GT_VEBOX_VDBOX_DISABLE);
>> +
>> +    vdbox_disable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
>> +    vebox_disable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
>> +            GEN11_GT_VEBOX_DISABLE_SHIFT;
>> +
>> +    DRM_DEBUG_DRIVER("vdbox disable: %04x\n", vdbox_disable);
>> +    for (i = 0; i < I915_MAX_VCS; i++) {
>> +        if (!HAS_ENGINE(dev_priv, _VCS(i)))
>> +            continue;
>> +
>> +        if (!(BIT(i) & vdbox_disable))
>> +            continue;
>> +
>> +        info->ring_mask &= ~ENGINE_MASK(_VCS(i));
>> +        DRM_DEBUG_DRIVER("vcs%u fused off\n", i);
>> +    }
>> +
>> +    DRM_DEBUG_DRIVER("vebox disable: %04x\n", vebox_disable);
>> +    for (i = 0; i < I915_MAX_VECS; i++) {
>> +        if (!HAS_ENGINE(dev_priv, _VECS(i)))
>> +            continue;
>> +
>> +        if (!(BIT(i) & vebox_disable))
>> +            continue;
>> +
>> +        info->ring_mask &= ~ENGINE_MASK(_VECS(i));
>> +        DRM_DEBUG_DRIVER("vecs%u fused off\n", i);
>> +    }
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
>> b/drivers/gpu/drm/i915/intel_device_info.h
>> index 0835752c8b22..0cbb92223013 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -247,6 +247,8 @@ void intel_device_info_dump_runtime(const struct 
>> intel_device_info *info,
>>   void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
>>                        struct drm_printer *p);
>> +void intel_device_info_init_mmio(struct drm_i915_private *dev_priv);
>> +
>>   void intel_driver_caps_print(const struct intel_driver_caps *caps,
>>                    struct drm_printer *p);
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index 4df7c2ef8576..4c616d074a97 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -62,6 +62,11 @@ static inline void
>>   fw_domain_reset(struct drm_i915_private *i915,
>>           const struct intel_uncore_forcewake_domain *d)
>>   {
>> +    /*
>> +     * We don't really know if the powerwell for the forcewake domain 
>> we are
>> +     * trying to reset here does exist at this point (engines could 
>> be fused
>> +     * off in ICL+), so no waiting for acks
>> +     */
>>       __raw_i915_write32(i915, d->reg_set, i915->uncore.fw_reset);
>>   }
>> @@ -1353,6 +1358,23 @@ static void fw_domain_init(struct 
>> drm_i915_private *dev_priv,
>>       fw_domain_reset(dev_priv, d);
>>   }
>> +static void fw_domain_fini(struct drm_i915_private *dev_priv,
>> +               enum forcewake_domain_id domain_id)
>> +{
>> +    struct intel_uncore_forcewake_domain *d;
>> +
>> +    if (WARN_ON(domain_id >= FW_DOMAIN_ID_COUNT))
>> +        return;
>> +
>> +    d = &dev_priv->uncore.fw_domain[domain_id];
>> +
>> +    WARN_ON(d->wake_count);
>> +    WARN_ON(hrtimer_cancel(&d->timer));
>> +    memset(d, 0, sizeof(*d));
>> +
>> +    dev_priv->uncore.fw_domains &= ~BIT(domain_id);
>> +}
>> +
>>   static void intel_uncore_fw_domains_init(struct drm_i915_private 
>> *dev_priv)
>>   {
>>       if (INTEL_GEN(dev_priv) <= 5 || intel_vgpu_active(dev_priv))
>> @@ -1565,6 +1587,40 @@ void intel_uncore_init(struct drm_i915_private 
>> *dev_priv)
>>           &dev_priv->uncore.pmic_bus_access_nb);
>>   }
>> +/*
>> + * 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_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 dfdf444e4bcc..47478d609630 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.h
>> +++ b/drivers/gpu/drm/i915/intel_uncore.h
>> @@ -140,6 +140,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);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx