diff mbox series

[v2,3/3] drm/i915/dgfx: Make failure to setup stolen non-fatal

Message ID 20220915-stolen-v2-3-20ff797de047@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Improvements to stolen memory setup | expand

Commit Message

Lucas De Marchi Sept. 16, 2022, 5:36 p.m. UTC 
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.demarchi@intel.com>

Comments

Wayne Boyer Sept. 20, 2022, 7:18 p.m. UTC | #1 
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.demarchi@intel.com>
> 

Reviewed-by: Wayne Boyer <wayne.boyer@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;
>   }
>   
>
diff mbox series

Patch

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;
 }