Re: [Intel-gfx] [PATCH 3/7] drm/i915: Move engine-related mmio init to engines_init_mmio

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 6/26/20 7:38 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2020-06-26 00:42:08)
>> All the info we read in intel_device_info_init_mmio are engine-related
>> and since we already have an engine_init_mmio function we can just
>> perform the operations from there.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: Andi Shyti <andi.shyti@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine_cs.c | 72 ++++++++++++++++++++++-
>>   drivers/gpu/drm/i915/i915_drv.c           |  4 --
>>   drivers/gpu/drm/i915/intel_device_info.c  | 66 ---------------------
>>   drivers/gpu/drm/i915/intel_device_info.h  |  2 -
>>   4 files changed, 71 insertions(+), 73 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index be92d1ef9aa9..8497106eb3a6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -450,6 +450,74 @@ void intel_engines_free(struct intel_gt *gt)
>>          }
>>   }
>>   
>> +/*
>> + * 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).
>> + */
>> +static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
>> +{
>> +       struct drm_i915_private *i915 = gt->i915;
>> +       struct intel_device_info *info = mkwrite_device_info(i915);
>> +       struct intel_uncore *uncore = gt->uncore;
>> +       unsigned int logical_vdbox = 0;
>> +       unsigned int i;
>> +       u32 media_fuse;
>> +       u16 vdbox_mask;
>> +       u16 vebox_mask;
> 
> assert_forcewakes_active(uncore, FORCEWAKE_BLITTER) ?
> 
> Since it's called out in the comment, might as well reinforce that with
> an assert.

We don't expect it to be active, just to be initialized/usable.
This comment is there to remind us of the catch-22 issue we have where 
we need forcewake to be initialized to read the fuses, but we need the 
fuses values to know which engines are available and therefore which fw 
domains are there (and that's why we prune the domains later).

Daniele

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