diff mbox series

[v3] drm/i915/mtl: Disable stolen memory backed FB for A0

Message ID 20230404181342.23362-1-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3] drm/i915/mtl: Disable stolen memory backed FB for A0 | expand

Commit Message

Nirmoy Das April 4, 2023, 6:13 p.m. UTC
Stolen memory is not usable for MTL A0 stepping beyond
certain access size and we have no control over userspace
access size of /dev/fb which can be backed by stolen memory.
So disable stolen memory backed fb by setting i915->dsm.usable_size
to zero.

v2: remove hsdes reference and fix commit message(Andi)
v3: use revid as we want to target SOC stepping(Radhakrishna)

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Sripada, Radhakrishna April 4, 2023, 6:23 p.m. UTC | #1
> -----Original Message-----
> From: Das, Nirmoy <nirmoy.das@intel.com>
> Sent: Tuesday, April 4, 2023 11:14 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Das, Nirmoy <nirmoy.das@intel.com>;
> Auld, Matthew <matthew.auld@intel.com>; Andi Shyti
> <andi.shyti@linux.intel.com>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Sripada, Radhakrishna
> <radhakrishna.sripada@intel.com>
> Subject: [PATCH v3] drm/i915/mtl: Disable stolen memory backed FB for A0
> 
> Stolen memory is not usable for MTL A0 stepping beyond
> certain access size and we have no control over userspace
> access size of /dev/fb which can be backed by stolen memory.
> So disable stolen memory backed fb by setting i915->dsm.usable_size
> to zero.
> 
> v2: remove hsdes reference and fix commit message(Andi)
> v3: use revid as we want to target SOC stepping(Radhakrishna)
> 
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>


LGTM,
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 8ac376c24aa2..ee492d823f1b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -535,6 +535,14 @@ static int i915_gem_init_stolen(struct
> intel_memory_region *mem)
>  	/* Basic memrange allocator for stolen space. */
>  	drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size);
> 
> +	/*
> +	 * Access to stolen lmem beyond certain size for MTL A0 stepping
> +	 * would crash the machine. Disable stolen lmem for userspace access
> +	 * by setting usable_size to zero.
> +	 */
> +	if (IS_METEORLAKE(i915) && INTEL_REVID(i915) == 0x0)
> +		i915->dsm.usable_size = 0;
> +
>  	return 0;
>  }
> 
> --
> 2.39.0
Ville Syrjala April 4, 2023, 6:27 p.m. UTC | #2
On Tue, Apr 04, 2023 at 08:13:42PM +0200, Nirmoy Das wrote:
> Stolen memory is not usable for MTL A0 stepping beyond
> certain access size and we have no control over userspace
> access size of /dev/fb which can be backed by stolen memory.
> So disable stolen memory backed fb by setting i915->dsm.usable_size
> to zero.
> 
> v2: remove hsdes reference and fix commit message(Andi)
> v3: use revid as we want to target SOC stepping(Radhakrishna)
> 
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 8ac376c24aa2..ee492d823f1b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -535,6 +535,14 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem)
>  	/* Basic memrange allocator for stolen space. */
>  	drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size);
>  
> +	/*
> +	 * Access to stolen lmem beyond certain size for MTL A0 stepping
> +	 * would crash the machine. Disable stolen lmem for userspace access
> +	 * by setting usable_size to zero.
> +	 */
> +	if (IS_METEORLAKE(i915) && INTEL_REVID(i915) == 0x0)
> +		i915->dsm.usable_size = 0;

That certainly won't prevent FBC from using stolen.
Are we sure that FBC accesses are fine?

> +
>  	return 0;
>  }
>  
> -- 
> 2.39.0
Nirmoy Das April 4, 2023, 9:26 p.m. UTC | #3
On 4/4/2023 8:27 PM, Ville Syrjälä wrote:
> On Tue, Apr 04, 2023 at 08:13:42PM +0200, Nirmoy Das wrote:
>> Stolen memory is not usable for MTL A0 stepping beyond
>> certain access size and we have no control over userspace
>> access size of /dev/fb which can be backed by stolen memory.
>> So disable stolen memory backed fb by setting i915->dsm.usable_size
>> to zero.
>>
>> v2: remove hsdes reference and fix commit message(Andi)
>> v3: use revid as we want to target SOC stepping(Radhakrishna)
>>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> index 8ac376c24aa2..ee492d823f1b 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> @@ -535,6 +535,14 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem)
>>   	/* Basic memrange allocator for stolen space. */
>>   	drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size);
>>   
>> +	/*
>> +	 * Access to stolen lmem beyond certain size for MTL A0 stepping
>> +	 * would crash the machine. Disable stolen lmem for userspace access
>> +	 * by setting usable_size to zero.
>> +	 */
>> +	if (IS_METEORLAKE(i915) && INTEL_REVID(i915) == 0x0)
>> +		i915->dsm.usable_size = 0;
> That certainly won't prevent FBC from using stolen.
> Are we sure that FBC accesses are fine?

I think so. I remember Jouni tested this patch internally to unblock a 
FBC test.

Jouni, could you please share your thoughts. I can't seem to find the 
internal JIRA reference right now.


Regards,

Nirmoy

>
>> +
>>   	return 0;
>>   }
>>   
>> -- 
>> 2.39.0
Hogander, Jouni April 5, 2023, 5:31 a.m. UTC | #4
On Tue, 2023-04-04 at 23:26 +0200, Das, Nirmoy wrote:
> 
> On 4/4/2023 8:27 PM, Ville Syrjälä wrote:
> > On Tue, Apr 04, 2023 at 08:13:42PM +0200, Nirmoy Das wrote:
> > > Stolen memory is not usable for MTL A0 stepping beyond
> > > certain access size and we have no control over userspace
> > > access size of /dev/fb which can be backed by stolen memory.
> > > So disable stolen memory backed fb by setting i915-
> > > >dsm.usable_size
> > > to zero.
> > > 
> > > v2: remove hsdes reference and fix commit message(Andi)
> > > v3: use revid as we want to target SOC stepping(Radhakrishna)
> > > 
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Andi Shyti <andi.shyti@linux.intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > index 8ac376c24aa2..ee492d823f1b 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > @@ -535,6 +535,14 @@ static int i915_gem_init_stolen(struct
> > > intel_memory_region *mem)
> > >         /* Basic memrange allocator for stolen space. */
> > >         drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size);
> > >   
> > > +       /*
> > > +        * Access to stolen lmem beyond certain size for MTL A0
> > > stepping
> > > +        * would crash the machine. Disable stolen lmem for
> > > userspace access
> > > +        * by setting usable_size to zero.
> > > +        */
> > > +       if (IS_METEORLAKE(i915) && INTEL_REVID(i915) == 0x0)
> > > +               i915->dsm.usable_size = 0;
> > That certainly won't prevent FBC from using stolen.
> > Are we sure that FBC accesses are fine?
> 
> I think so. I remember Jouni tested this patch internally to unblock
> a 
> FBC test.
> 
> Jouni, could you please share your thoughts. I can't seem to find the
> internal JIRA reference right now.

I tested this patch and it was fixing the problem it was targeted. I
didn't noticed any issue back then.

> 
> 
> Regards,
> 
> Nirmoy
> 
> > 
> > > +
> > >         return 0;
> > >   }
> > >   
> > > -- 
> > > 2.39.0
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 8ac376c24aa2..ee492d823f1b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -535,6 +535,14 @@  static int i915_gem_init_stolen(struct intel_memory_region *mem)
 	/* Basic memrange allocator for stolen space. */
 	drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size);
 
+	/*
+	 * Access to stolen lmem beyond certain size for MTL A0 stepping
+	 * would crash the machine. Disable stolen lmem for userspace access
+	 * by setting usable_size to zero.
+	 */
+	if (IS_METEORLAKE(i915) && INTEL_REVID(i915) == 0x0)
+		i915->dsm.usable_size = 0;
+
 	return 0;
 }