[Intel-gfx] [PATCH v2 2/8] drm/i915: setup the LMEM region

[Chris Wilson]

Quoting Matthew Auld (2021-01-26 15:12:53)
> Hook up the LMEM region. Addresses will start from zero, and for CPU
> access we get LMEM_BAR which is just a 1:1 mapping of said region.
> 
> Based on a patch from Michel Thierry.
> 
> v2 by Jani:
> - use intel_uncore_read/intel_uncore_write
> - remove trailing blank line
> 
> v3: s/drm_info/drm_dbg for info which in non-pertinent for the user
> 
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c          | 12 ++++++--
>  drivers/gpu/drm/i915/gt/intel_region_lmem.c | 32 +++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_region_lmem.h |  2 ++
>  3 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 989cd2106b09..78866e766989 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -42,7 +42,7 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
>  int intel_gt_probe_lmem(struct intel_gt *gt)
>  {
>         struct drm_i915_private *i915 = gt->i915;
> -       struct intel_memory_region *mem;
> +       struct intel_memory_region *mem = ERR_PTR(-ENODEV);
>         int id;
>         int err;
>  
> @@ -50,7 +50,15 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
>         if (!HAS_REGION(i915, id))
>                 return 0;
>  
> -       mem = intel_gt_setup_fake_lmem(gt);
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +       if (IS_ENABLED(CONFIG_DRM_I915_UNSTABLE_FAKE_LMEM)) {
> +               if (INTEL_GEN(i915) >= 9 && i915_selftest.live < 0 &&
> +                   i915->params.fake_lmem_start)
> +                       mem = intel_gt_setup_fake_lmem(gt);
> +       }
> +#endif
> +       if (IS_ERR(mem))
> +               mem = intel_gt_setup_lmem(gt);

You would rather have fake lmem over real. (Not that two should be able
to co-exist.) Just the semantics of the ordering looks odd.

I would have gone

	mem = intel_gt_setup_lmem(gt);
	if (mem == PTR_ERR(-ENODEV))
		mem = intel_gt_setup_fake_lmem(gt);

>         if (IS_ERR(mem)) {
>                 err = PTR_ERR(mem);
>                 drm_err(&i915->drm,
-Chris