[Intel-gfx] [PATCH 1/8] drm/i915/icl: Check for fused-off VDBOX and VEBOX instances
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Wed Mar 28 11:36:09 UTC 2018
On 28/03/18 00:39, Paulo Zanoni wrote:
> Em Ter, 2018-03-27 às 15:42 -0700, Paulo Zanoni escreveu:
>> Em Sex, 2018-03-23 às 16:28 +0000, Lionel Landwerlin escreveu:
>>> Hi Mika,
>>>
>>> Even after this series, we're still missing support for reading
>>> the
>>> timestamp frequency (read_timestamp_frequency in
>>> intel_device_info.c).
>>> I'm pretty sure someone wrote a patch for it. Do you any idea?
>>>
>>> If not, I can send something.
>> Yes, we have them. I'll see if I missed them while upstreaming and
>> resend in that case.
> https://patchwork.freedesktop.org/patch/196710/
>
> Hey Lionel, the Reviewed-by stamp you gave on the patch was before we
> upstreamed it, so we need you (or someone else) to re-check the patch
> and re-issue the reviewed-by tag. We do this because of the rebasing
> that happened between the R-B tag and the upstreaming, since issues can
> be introduced in between. If you can check the patch again and validate
> the r-b tag (or point the issues) then we can move forward and
> hopefully merge it.
>
> Thanks,
> Paulo
Thanks, just sent another Rb, please push it :)
>
>>> Thanks,
>>>
>>> -
>>> Lionel
>>>
>>> On 16/03/18 12:14, Mika Kuoppala wrote:
>>>> From: Oscar Mateo <oscar.mateo at 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 at intel.com>
>>>> Cc: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>> Signed-off-by: Oscar Mateo <oscar.mateo at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list