diff mbox series

drm/i915: Make sure dsm_size has correct granularity

Message ID 20230202180243.23637-1-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Make sure dsm_size has correct granularity | expand

Commit Message

Nirmoy Das Feb. 2, 2023, 6:02 p.m. UTC
DSM granularity is 1MB so make sure we stick to that.

v2: replace "1 * SZ_1M" with SZ_1M (Andrzej).

Cc: Matthew Auld <matthew.auld@intel.com>
Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lucas De Marchi Feb. 3, 2023, 6:56 p.m. UTC | #1
On Thu, Feb 02, 2023 at 07:02:43PM +0100, Nirmoy Das wrote:
>DSM granularity is 1MB so make sure we stick to that.

I think we need to be a bit more verbose here, because in future we may
need to refer to this commit if/when things change (e.g. the granularity
or the additional size needed on top of DSM).

The issue this is fixing is that the address set by firmware in GEN12_DSMBASE
and read here doesn't mean "anything above it until the of lmem is part of DSM".
There may be a few KB that is not part of DSM. How large is that space
is platform-dependent, but since it's always less than the DSM
granularity, it can be simplified by simply aligning the size like
is done here.

>
>v2: replace "1 * SZ_1M" with SZ_1M (Andrzej).
>
>Cc: Matthew Auld <matthew.auld@intel.com>
>Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Are you ok with me amending the commit message and applying?

After this patch I think you can follow the process to request committer
access.

Lucas De Marchi

>---
> drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>index 90a967374b1a..d8e06e783e30 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>@@ -909,7 +909,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
> 		dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
> 		if (WARN_ON(lmem_size < dsm_base))
> 			return ERR_PTR(-ENODEV);
>-		dsm_size = lmem_size - dsm_base;
>+		dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
> 	}
>
> 	io_size = dsm_size;
>-- 
>2.39.0
>
Nirmoy Das Feb. 3, 2023, 7 p.m. UTC | #2
Hi Lucas,

On 2/3/2023 7:56 PM, Lucas De Marchi wrote:
> On Thu, Feb 02, 2023 at 07:02:43PM +0100, Nirmoy Das wrote:
>> DSM granularity is 1MB so make sure we stick to that.
>
> I think we need to be a bit more verbose here, because in future we may
> need to refer to this commit if/when things change (e.g. the granularity
> or the additional size needed on top of DSM).
>
> The issue this is fixing is that the address set by firmware in 
> GEN12_DSMBASE
> and read here doesn't mean "anything above it until the of lmem is 
> part of DSM".
> There may be a few KB that is not part of DSM. How large is that space
> is platform-dependent, but since it's always less than the DSM
> granularity, it can be simplified by simply aligning the size like
> is done here.
>
>>
>> v2: replace "1 * SZ_1M" with SZ_1M (Andrzej).
>>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Are you ok with me amending the commit message and applying?


Yes, I fine that, thanks for doing that. I agree this is very short 
description that I have wrote.


>
> After this patch I think you can follow the process to request committer
> access.


Looking for to doing that :)


Nirmoy

>
> Lucas De Marchi
>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> index 90a967374b1a..d8e06e783e30 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> @@ -909,7 +909,7 @@ i915_gem_stolen_lmem_setup(struct 
>> drm_i915_private *i915, u16 type,
>>         dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & 
>> GEN12_BDSM_MASK;
>>         if (WARN_ON(lmem_size < dsm_base))
>>             return ERR_PTR(-ENODEV);
>> -        dsm_size = lmem_size - dsm_base;
>> +        dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
>>     }
>>
>>     io_size = dsm_size;
>> -- 
>> 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 90a967374b1a..d8e06e783e30 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -909,7 +909,7 @@  i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
 		dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE) & GEN12_BDSM_MASK;
 		if (WARN_ON(lmem_size < dsm_base))
 			return ERR_PTR(-ENODEV);
-		dsm_size = lmem_size - dsm_base;
+		dsm_size = ALIGN_DOWN(lmem_size - dsm_base, SZ_1M);
 	}
 
 	io_size = dsm_size;