[Intel-gfx] [PATCH v13 5/6] drm/i915/guc: Check the locking status of GuC WOPCM registers

[Joonas Lahtinen]

Quoting Jackie Li (2018-03-14 02:32:53)
> GuC WOPCM registers are write-once registers. Current driver code accesses
> these registers without checking the accessibility to these registers which
> will lead to unpredictable driver behaviors if these registers were touch
> by other components (such as faulty BIOS code).
> 
> This patch moves the GuC WOPCM registers updating code into intel_wopcm.c
> and adds check before and after the update to GuC WOPCM registers so that
> we can make sure the driver is in a known state after writing to these
> write-once registers.
> 
> v6:
>  - Made sure module reloading won't bug the kernel while doing
>    locking status checking
> 
> v7:
>  - Fixed patch format issues
> 
> v8:
>  - Fixed coding style issue on register lock bit macro definition (Sagar)
> 
> v9:
>  - Avoided to use redundant !! to cast uint to bool (Chris)
>  - Return error code instead of GEM_BUG_ON for locked with invalid register
>    values case (Sagar)
>  - Updated guc_wopcm_hw_init to use guc_wopcm as first parameter (Michal)
>  - Added code to set and validate the HuC_LOADING_AGENT_GUC bit in GuC
>    WOPCM offset register based on the presence of HuC firmware (Michal)
>  - Use bit fields instead of macros for GuC WOPCM flags (Michal)
> 
> v10:
>  - Refined variable names, removed redundant comments (Joonas)
>  - Introduced lockable_reg to handle the write once register write and
>    propagate the write error to caller (Joonas)
>  - Used lockable_reg abstraction to avoid locking bit check on generic
>    i915_reg_t (Michal)
>  - Added log message for error paths (Michal)
>  - Removed hw_updated flag and only relies on real hardware status
> 
> v11:
>  - Replaced lockable_reg with simplified function (Michal)
>  - Used new macros for locking bits of WOPCM size/offset registers instead
>    of using BIT(0) directly (Michal)
>  - use intel_wopcm_init_hw() called from intel_gem_init_hw() to do GuC
>    WOPCM register setup instead of calling from intel_uc_init_hw() (Michal)
> 
> v12:
>  - Updated function kernel-doc to align with code changes (Michal)
>  - Updated code to use wopcm pointer directly (Michal)
> 
> v13:
>  - Updated the ordering of s-o-b/cc/r-b tags (Sagar)
> 
> BSpec: 10875, 10833
> 
> Signed-off-by: Jackie Li <yaodong.li at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com> (v11)
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com> (v12)

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5147,6 +5147,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>                 goto out;
>         }
>  
> +       ret = intel_wopcm_init_hw(&dev_priv->wopcm);
> +       if (ret) {
> +               DRM_ERROR("Enabling WOPCM failed (%d)\n", ret);

This (or the other instance inside) can still be removed, it's a duplicate
message.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

Regards, Joonas