[Intel-gfx] [PATCH 1/2] drm/i915/wopcm: Try to use already locked WOPCM layout

[Daniele Ceraolo Spurio]

On 8/14/19 12:51 PM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/14/19 4:38 AM, Michal Wajdeczko wrote:
>> If WOPCM layout is already locked in HW we shouldn't continue
>> with our own partitioning as it could be likely different and
>> we will be unable to enforce it and fail. Instead we should try
>> to reuse what is already programmed, maybe there will be a fit.
>>
>> This should enable us to reload driver with slightly different
>> HuC firmware (or even without HuC) without need to reboot.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_wopcm.c | 29 +++++++++++++++++++++++++++--
>>   1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c 
>> b/drivers/gpu/drm/i915/intel_wopcm.c
>> index 2bda24200498..e5bc7b8a433e 100644
>> --- a/drivers/gpu/drm/i915/intel_wopcm.c
>> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
>> @@ -154,6 +154,21 @@ static inline int check_hw_restriction(struct 
>> drm_i915_private *i915,
>>       return err;
>>   }
>> +static bool __wopcm_regs_locked(struct intel_uncore *uncore,
>> +                u32 *guc_wopcm_base, u32 *guc_wopcm_size)
>> +{
>> +    u32 reg_base = intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET);
>> +    u32 reg_size = intel_uncore_read(uncore, GUC_WOPCM_SIZE);
>> +
>> +    if (!(reg_size & GUC_WOPCM_SIZE_LOCKED) ||
>> +        !(reg_base & GUC_WOPCM_OFFSET_VALID))
>> +        return false;
> 
> Should we at least WARN in the unlikely case where only one of the 2 
> regs is locked?
> 
>> +
>> +    *guc_wopcm_base = reg_base & GUC_WOPCM_OFFSET_MASK;
>> +    *guc_wopcm_size = reg_size & GUC_WOPCM_SIZE_MASK;
>> +    return true;
>> +}
>> +
>>   /**
>>    * intel_wopcm_init() - Initialize the WOPCM structure.
>>    * @wopcm: pointer to intel_wopcm.
>> @@ -167,8 +182,9 @@ static inline int check_hw_restriction(struct 
>> drm_i915_private *i915,
>>   void intel_wopcm_init(struct intel_wopcm *wopcm)
>>   {
>>       struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
>> -    u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915->gt.uc.guc.fw);
>> -    u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915->gt.uc.huc.fw);
>> +    struct intel_gt *gt = &i915->gt;
>> +    u32 guc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.guc.fw);
>> +    u32 huc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.huc.fw);
>>       u32 ctx_rsvd = context_reserved_size(i915);
>>       u32 guc_wopcm_base;
>>       u32 guc_wopcm_size;
>> @@ -185,6 +201,14 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>>       if (i915_inject_probe_failure(i915))
>>           return;
>> +    if (__wopcm_regs_locked(gt->uncore, &guc_wopcm_base, 
>> &guc_wopcm_size)) {
>> +        DRM_DEV_DEBUG_DRIVER(i915->drm.dev,
>> +                     "GuC WOPCM is already locked [%uK, %uK)\n",
>> +                     guc_wopcm_base / SZ_1K,
>> +                     guc_wopcm_size / SZ_1K);
>> +        goto check;
>> +    }
>> +
>>       if (guc_fw_size >= wopcm->size) {
>>           DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
>>                 guc_fw_size / 1024);
>> @@ -211,6 +235,7 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>>       DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
>>                guc_wopcm_base / 1024, guc_wopcm_size / 1024);
>> +check:
> 
> The checks here don't really verify that the sizes we're locked with are 
> enough to fit the binaries. We need to at least print an error in that 
> case so we can debug why GuC/HuC fails to load later if the sizes are 
> not ok.

Here I meant to refer to HuC only, since guc size is verified below (I'm 
too used to refer to them as a pair :P )

Daniele

> 
> Daniele
> 
>>       guc_wopcm_rsvd = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
>>       if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
>>           DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
>>