Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/dgfx: Make failure to setup stolen non-fatal

[Wayne Boyer]

On 9/16/22 10:36 AM, Lucas De Marchi wrote:

There is no reason to consider the setup of Data Stolen Memory fatal on
dgfx and non-fatal on integrated. Move the debug and error propagation
around so both have the same behavior: non-fatal. Before this change,
loading i915 on a system with TGL + DG2 would result in just TGL
succeeding the initialization (without stolen).

Now loading i915 on the same system with an injected failure in
i915_gem_init_stolen():

        $ dmesg | grep stolen
        i915 0000:00:02.0: [drm] Injected failure, disabling use of stolen 
memory
        i915 0000:00:02.0: [drm:init_stolen_smem [i915]] Skip stolen region: 
failed to setup
        i915 0000:03:00.0: [drm] Injected failure, disabling use of stolen 
memory
        i915 0000:03:00.0: [drm:init_stolen_lmem [i915]] Skip stolen region: 
failed to setup

Both GPUs are still available:

        $ sudo build/tools/lsgpu
        card1                    Intel Dg2 (Gen12)                 
drm:/dev/dri/card1
        └─renderD129                                               
drm:/dev/dri/renderD129
        card0                    Intel Tigerlake (Gen12)           
drm:/dev/dri/card0
        └─renderD128                                               
drm:/dev/dri/renderD128

Signed-off-by: Lucas De Marchi <lucas.demar...@intel.com>

Reviewed-by: Wayne Boyer <wayne.bo...@intel.com>

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 6edf4e374f54..c5a4035c99cd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -494,26 +494,26 @@ static int i915_gem_init_stolen(struct 
intel_memory_region *mem)
                drm_notice(&i915->drm,
                           "%s, disabling use of stolen memory\n",
                           "iGVT-g active");
-               return 0;
+               return -ENOSPC;
        }

if (i915_vtd_active(i915) && GRAPHICS_VER(i915) < 8) {

                drm_notice(&i915->drm,
                           "%s, disabling use of stolen memory\n",
                           "DMAR active");
-               return 0;
+               return -ENOSPC;
        }

if (adjust_stolen(i915, &mem->region))

-               return 0;
+               return -ENOSPC;

if (request_smem_stolen(i915, &mem->region))

-               return 0;
+               return -ENOSPC;

i915->dsm = mem->region; if (init_reserved_stolen(i915))

-               return 0;
+               return -ENOSPC;

/* Exclude the reserved region from driver use */

        mem->region.end = i915->dsm_reserved.start - 1;
@@ -527,7 +527,7 @@ static int i915_gem_init_stolen(struct intel_memory_region 
*mem)
                (u64)i915->stolen_usable_size >> 10);

if (i915->stolen_usable_size == 0)

-               return 0;
+               return -ENOSPC;

/* Basic memrange allocator for stolen space. */

        drm_mm_init(&i915->mm.stolen, 0, i915->stolen_usable_size);
@@ -765,11 +765,17 @@ i915_gem_object_create_stolen(struct drm_i915_private 
*i915,

static int init_stolen_smem(struct intel_memory_region *mem)

  {
+       int err;
+
        /*
         * Initialise stolen early so that we may reserve preallocated
         * objects for the BIOS to KMS transition.
         */
-       return i915_gem_init_stolen(mem);
+       err = i915_gem_init_stolen(mem);
+       if (err)
+               drm_dbg(&mem->i915->drm, "Skip stolen region: failed to 
setup\n");
+
+       return 0;
  }

static int release_stolen_smem(struct intel_memory_region *mem)

@@ -786,21 +792,25 @@ static const struct intel_memory_region_ops 
i915_region_stolen_smem_ops = {

static int init_stolen_lmem(struct intel_memory_region *mem)

  {
+       struct drm_i915_private *i915 = mem->i915;
        int err;

if (GEM_WARN_ON(resource_size(&mem->region) == 0))

-               return -ENODEV;
+               return 0;

err = i915_gem_init_stolen(mem);

-       if (err)
-               return err;
+       if (err) {
+               drm_dbg(&mem->i915->drm, "Skip stolen region: failed to 
setup\n");
+               return 0;
+       }

- if (mem->io_size && !io_mapping_init_wc(&mem->iomap,

-                                               mem->io_start,
-                                               mem->io_size)) {
-               err = -EIO;
+       if (mem->io_size &&
+           !io_mapping_init_wc(&mem->iomap, mem->io_start, mem->io_size))
                goto err_cleanup;
-       }
+
+       drm_dbg(&i915->drm, "Stolen Local memory IO start: %pa\n",
+               &mem->io_start);
+       drm_dbg(&i915->drm, "Stolen Local DSM base: %pa\n", &mem->region.start);

return 0; @@ -874,16 +884,6 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,

        if (IS_ERR(mem))
                return mem;

- /*

-        * TODO: consider creating common helper to just print all the
-        * interesting stuff from intel_memory_region, which we can use for all
-        * our probed regions.
-        */
-
-       drm_dbg(&i915->drm, "Stolen Local memory IO start: %pa\n",
-               &mem->io_start);
-       drm_dbg(&i915->drm, "Stolen Local DSM base: %pa\n", &dsm_base);
-
        intel_memory_region_set_name(mem, "stolen-local");

mem->private = true;

@@ -908,6 +908,7 @@ i915_gem_stolen_smem_setup(struct drm_i915_private *i915, 
u16 type,
        intel_memory_region_set_name(mem, "stolen-system");

mem->private = true;

+
        return mem;
  }

--
--
Wayne Boyer
Graphics Software Engineer
VTT-OSGC Platform Enablement