[Intel-gfx] [PATCH 02/21] drm/i915/gtt: Workaround for HW preload not flushing pdps
Michel Thierry
michel.thierry at intel.com
Fri May 29 05:53:21 PDT 2015
On 5/29/2015 12:05 PM, Michel Thierry wrote:
> On 5/22/2015 6:04 PM, Mika Kuoppala wrote:
>> With BDW/SKL and 32bit addressing mode only, the hardware preloads
>> pdps. However the TLB invalidation only has effect on levels below
>> the pdps. This means that if pdps change, hw might access with
>> stale pdp entry.
>>
>> To combat this problem, preallocate the top pdps so that hw sees
>> them as immutable for each context.
>>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Rafael Barbalho <rafael.barbalho at intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 50
>> +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++
>> drivers/gpu/drm/i915/intel_lrc.c | 15 +----------
>> 3 files changed, 68 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 0ffd459..1a5ad4c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -941,6 +941,48 @@ err_out:
>> return ret;
>> }
>>
>> +/* With some architectures and 32bit legacy mode, hardware pre-loads
>> the
>> + * top level pdps but the tlb invalidation only invalidates the
>> lower levels.
>> + * This might lead to hw fetching with stale pdp entries if top level
>> + * structure changes, ie va space grows with dynamic page tables.
>> + */
>> +static bool hw_wont_flush_pdp_tlbs(struct i915_hw_ppgtt *ppgtt)
>> +{
>> + struct drm_device *dev = ppgtt->base.dev;
>> +
>> + if (GEN8_CTX_ADDRESSING_MODE != LEGACY_32B_CONTEXT)
>> + return false;
>> +
>> + if (IS_BROADWELL(dev) || IS_SKYLAKE(dev))
>> + return true;
> The pd load restriction is also true for chv and bxt.
> And to be safe, we can set reg 0x4030 bit14 to '1' (PD load disable).
> Since this register is not part of the context state, it can be added
> with the other platform workarounds in intel_pm.c.
>
>> +
>> + return false;
>> +}
>> +
>> +static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
>> +{
>> + unsigned long *new_page_dirs, **new_page_tables;
>> + int ret;
>> +
>> + /* We allocate temp bitmap for page tables for no gain
>> + * but as this is for init only, lets keep the things simple
>> + */
>> + ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables);
>> + if (ret)
>> + return ret;
>> +
>> + /* Allocate for all pdps regardless of how the ppgtt
>> + * was defined.
>> + */
>> + ret = gen8_ppgtt_alloc_page_directories(ppgtt, &ppgtt->pdp,
>> + 0, 1ULL << 32,
>> + new_page_dirs);
>> +
>> + free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>> +
>> + return ret;
>> +}
>> +
>> /*
>> * GEN8 legacy ppgtt programming is accomplished through a max 4
>> PDP registers
>> * with a net effect resembling a 2-level page table in normal x86
>> terms. Each
>> @@ -972,6 +1014,14 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
>> *ppgtt)
>>
>> ppgtt->switch_mm = gen8_mm_switch;
>>
>> + if (hw_wont_flush_pdp_tlbs(ppgtt)) {
>> + /* Avoid the tlb flush bug by preallocating
>> + * whole top level pdp structure so it stays
>> + * static even if our va space grows.
>> + */
>> + return gen8_preallocate_top_level_pdps(ppgtt);
>> + }
>> +
Also, we will need the same hw_wont_flush check in the cleanup function,
and iterate each_pdpe (pd) from 0 to 4GiB (otherwise we will leak some
of the preallocated page dirs).
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 6eeba63..334324b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2777,6 +2777,23 @@ enum skl_disp_power_wells {
>> #define VLV_CLK_CTL2 0x101104
>> #define CLK_CTL2_CZCOUNT_30NS_SHIFT 28
>>
>> +/* Context descriptor format bits */
>> +#define GEN8_CTX_VALID (1<<0)
>> +#define GEN8_CTX_FORCE_PD_RESTORE (1<<1)
>> +#define GEN8_CTX_FORCE_RESTORE (1<<2)
>> +#define GEN8_CTX_L3LLC_COHERENT (1<<5)
>> +#define GEN8_CTX_PRIVILEGE (1<<8)
>> +
>> +enum {
>> + ADVANCED_CONTEXT = 0,
>> + LEGACY_32B_CONTEXT,
>> + ADVANCED_AD_CONTEXT,
>> + LEGACY_64B_CONTEXT
>> +};
>> +
>> +#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
>> +#define GEN8_CTX_ADDRESSING_MODE LEGACY_32B_CONTEXT
>> +
>> /*
>> * Overlay regs
>> */
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index 96ae90a..d793d4e 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -183,12 +183,6 @@
>> #define CTX_R_PWR_CLK_STATE 0x42
>> #define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
>>
>> -#define GEN8_CTX_VALID (1<<0)
>> -#define GEN8_CTX_FORCE_PD_RESTORE (1<<1)
>> -#define GEN8_CTX_FORCE_RESTORE (1<<2)
>> -#define GEN8_CTX_L3LLC_COHERENT (1<<5)
>> -#define GEN8_CTX_PRIVILEGE (1<<8)
>> -
>> #define ASSIGN_CTX_PDP(ppgtt, reg_state, n) { \
>> const u64 _addr = test_bit(n, ppgtt->pdp.used_pdpes) ? \
>> ppgtt->pdp.page_directory[n]->daddr : \
>> @@ -198,13 +192,6 @@
>> }
>>
>> enum {
>> - ADVANCED_CONTEXT = 0,
>> - LEGACY_CONTEXT,
>> - ADVANCED_AD_CONTEXT,
>> - LEGACY_64B_CONTEXT
>> -};
>> -#define GEN8_CTX_MODE_SHIFT 3
>> -enum {
>> FAULT_AND_HANG = 0,
>> FAULT_AND_HALT, /* Debug only */
>> FAULT_AND_STREAM,
>> @@ -273,7 +260,7 @@ static uint64_t execlists_ctx_descriptor(struct
>> intel_engine_cs *ring,
>> WARN_ON(lrca & 0xFFFFFFFF00000FFFULL);
>>
>> desc = GEN8_CTX_VALID;
>> - desc |= LEGACY_CONTEXT << GEN8_CTX_MODE_SHIFT;
>> + desc |= GEN8_CTX_ADDRESSING_MODE <<
>> GEN8_CTX_ADDRESSING_MODE_SHIFT;
>> if (IS_GEN8(ctx_obj->base.dev))
>> desc |= GEN8_CTX_L3LLC_COHERENT;
>> desc |= GEN8_CTX_PRIVILEGE;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list