[Intel-gfx] [PATCH v3 2/4] drm/i915: Always set HUC_LOADING_AGENT_GUC bit in WOPCM offset register

[Yaodong Li]

On 04/13/2018 07:26 PM, Michal Wajdeczko wrote:
> On Tue, 10 Apr 2018 02:42:18 +0200, Jackie Li <yaodong.li at intel.com> 
> wrote:
>
>> The enable_guc modparam is used to enable/disable GuC/HuC FW uploading
>> dynamcially during i915 module loading. If WOPCM offset register was
>   ^^^^
> typo
>
D'oh! I really need a tool for this! Thanks, will fix it.
>> locked
>> without having HUC_LOADING_AGENT_GUC bit set to 1, the module reloading
>> with both GuC and HuC FW will fail since we need to set this bit to 1 
>> for
>> HuC FW uploading.
>>
>> Since HUC_LOADING_AGENT_GUC bit has no impact on GuC FW uploading, this
>> patch updates the register updating code to make sure the WOPCM offset
>> register is always locked with HUC_LOADING_AGENT_GUC bit set to 1 which
>> will guarantee successful uploading of both GuC and HuC FW. We will 
>> further
>> take care of the locked values in the following enhancement patch.
>>
>> Signed-off-by: Jackie Li <yaodong.li at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>> Cc: John Spotswood <john.a.spotswood at intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_wopcm.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c 
>> b/drivers/gpu/drm/i915/intel_wopcm.c
>> index 74bf76f..b1c08ca 100644
>> --- a/drivers/gpu/drm/i915/intel_wopcm.c
>> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
>> @@ -238,8 +238,6 @@ static inline int write_and_verify(struct 
>> drm_i915_private *dev_priv,
>>  int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
>>  {
>>      struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
>> -    u32 huc_agent;
>> -    u32 mask;
>>      int err;
>>     if (!USES_GUC(dev_priv))
>> @@ -255,10 +253,10 @@ int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
>>      if (err)
>>          goto err_out;
>> -    huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
>> -    mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
>>      err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
>> -                   wopcm->guc.base | huc_agent, mask,
>> +                   wopcm->guc.base | HUC_LOADING_AGENT_GUC,
>> +                   GUC_WOPCM_OFFSET_MASK | HUC_LOADING_AGENT_GUC |
>> +                   GUC_WOPCM_OFFSET_VALID,
>
> while we can unconditionally set HUC_AGENT bit, there is no need to 
> verify
> it unless we are using HuC, so we can consider leaving old mask intact.
The idea is to verify the written values are exactly we want, so I think 
it better
to keep doing it in this way.

Thanks,
Jackie