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

Oscar Mateo
2018-01-10T19:25:38Z

On 01/10/2018 01:36 AM, Chris Wilson wrote:
> Quoting Paulo Zanoni (2018-01-09 23:28:24)
>> 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 to read an
>> MMIO register with the fuse info, but we cannot fully enable MMIO until
>> we read it (since we need the real engines to initialize the forcewake
>> domains). We workaround this problem by reading the fuse after the MMIO
>> is partially ready, but before we initialize forcewake.
>>
>> 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).
>>
>> 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>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 2 ++
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/i915_reg.h | 5 +++
>> drivers/gpu/drm/i915/intel_device_info.c | 53 ++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_device_info.h | 4 +++
>> 5 files changed, 65 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 6c8da9d20c33..60aa09410d94 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1018,6 +1018,8 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
>> if (ret < 0)
>> goto err_bridge;
>>
>> + intel_device_info_fused_off_engines(dev_priv);
> intel_device_info_init_mmio();
>
>> +
>> intel_uncore_init(dev_priv);
>>
>> intel_uc_init_mmio(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 2635e73e0ca5..aa4f2b178d97 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3418,6 +3418,7 @@ void i915_unreserve_fence(struct drm_i915_fence_reg *fence);
>> void i915_gem_revoke_fences(struct drm_i915_private *dev_priv);
>> void i915_gem_restore_fences(struct drm_i915_private *dev_priv);
>>
>> +void intel_device_info_fused_off_engines(struct drm_i915_private *dev_priv);
>> void i915_gem_detect_bit_6_swizzle(struct drm_i915_private *dev_priv);
>> void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj,
>> struct sg_table *pages);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 84a36302066f..c9b62502ce69 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2804,6 +2804,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 25448e38ee76..3316470363a0 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -589,3 +589,56 @@ void intel_device_info_runtime_init(struct intel_device_info *info)
>> /* Initialize command stream timestamp frequency */
>> info->cs_timestamp_frequency_khz = read_timestamp_frequency(dev_priv);
>> }
>> +
>> +/*
>> + * Determine which engines are fused off in our particular hardware.
>> + *
>> + * This function needs to be called after the MMIO has been setup (as we need
>> + * to read registers) but before uncore init (because the powerwell for the
>> + * fused off engines doesn't exist, so we cannot initialize forcewake for them)
>> + */
>> +void intel_device_info_fused_off_engines(struct drm_i915_private *dev_priv)
>> +{
>> + struct intel_device_info *info = mkwrite_device_info(dev_priv);
>> + u32 media_fuse;
>> + int i;
>> +
>> + if (INTEL_GEN(dev_priv) < 11)
>> + return;
>> +
>> + GEM_BUG_ON(!dev_priv->regs);
>> +
>> + media_fuse = I915_READ_FW(GEN11_GT_VEBOX_VDBOX_DISABLE);
>> +
>> + info->vdbox_disable = media_fuse & GEN11_GT_VDBOX_DISABLE_MASK;
>> + info->vebox_disable = (media_fuse & GEN11_GT_VEBOX_DISABLE_MASK) >>
>> + GEN11_GT_VEBOX_DISABLE_SHIFT;
> We don't need to keep these (just locals will do), the permanent
> information is in info->ring_mask.

There are subsequent patches that pass this info to GuC, that's why I
was keeping them. I could retrieve the information back from
info->ring_mask, but it's a pity since we already have it in the right
format here.

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