diff mbox series

[3/4] drm/i915: allow volatile buffers to use ttm pool allocator

Message ID 20220503191316.1145124-4-bob.beckett@collabora.com (mailing list archive)
State New, archived
Headers show
Series ttm for internal | expand

Commit Message

Bob Beckett May 3, 2022, 7:13 p.m. UTC
internal buffers should be shmem backed.
if a volatile buffer is requested, allow ttm to use the pool allocator
to provide volatile pages as backing

Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Thomas Hellström May 11, 2022, 12:42 p.m. UTC | #1
Hi, Bob,

On Tue, 2022-05-03 at 19:13 +0000, Robert Beckett wrote:
> internal buffers should be shmem backed.
> if a volatile buffer is requested, allow ttm to use the pool
> allocator
> to provide volatile pages as backing
> 
> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index 4c25d9b2f138..fdb3a1c18cb6 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -309,7 +309,8 @@ static struct ttm_tt *i915_ttm_tt_create(struct
> ttm_buffer_object *bo,
>                 page_flags |= TTM_TT_FLAG_ZERO_ALLOC;
>  
>         caching = i915_ttm_select_tt_caching(obj);
> -       if (i915_gem_object_is_shrinkable(obj) && caching ==
> ttm_cached) {
> +       if (i915_gem_object_is_shrinkable(obj) && caching ==
> ttm_cached &&
> +           !i915_gem_object_is_volatile(obj)) {
>                 page_flags |= TTM_TT_FLAG_EXTERNAL |
>                               TTM_TT_FLAG_EXTERNAL_MAPPABLE;
>                 i915_tt->is_shmem = true;

While this is ok, I think it also needs adjustment in the i915_ttm
shrink callback. If someone creates a volatile smem object which then
hits the shrinker, I think we might hit asserts that it's a is_shem
ttm?

In this case, the shrink callback should just i915_ttm_purge().

/Thomas
Bob Beckett May 23, 2022, 3:53 p.m. UTC | #2
On 11/05/2022 13:42, Thomas Hellström wrote:
> Hi, Bob,
> 
> On Tue, 2022-05-03 at 19:13 +0000, Robert Beckett wrote:
>> internal buffers should be shmem backed.
>> if a volatile buffer is requested, allow ttm to use the pool
>> allocator
>> to provide volatile pages as backing
>>
>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> index 4c25d9b2f138..fdb3a1c18cb6 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> @@ -309,7 +309,8 @@ static struct ttm_tt *i915_ttm_tt_create(struct
>> ttm_buffer_object *bo,
>>                  page_flags |= TTM_TT_FLAG_ZERO_ALLOC;
>>   
>>          caching = i915_ttm_select_tt_caching(obj);
>> -       if (i915_gem_object_is_shrinkable(obj) && caching ==
>> ttm_cached) {
>> +       if (i915_gem_object_is_shrinkable(obj) && caching ==
>> ttm_cached &&
>> +           !i915_gem_object_is_volatile(obj)) {
>>                  page_flags |= TTM_TT_FLAG_EXTERNAL |
>>                                TTM_TT_FLAG_EXTERNAL_MAPPABLE;
>>                  i915_tt->is_shmem = true;
> 
> While this is ok, I think it also needs adjustment in the i915_ttm
> shrink callback. If someone creates a volatile smem object which then
> hits the shrinker, I think we might hit asserts that it's a is_shem
> ttm?
> 
> In this case, the shrink callback should just i915_ttm_purge().

agreed. nice catch.
I'll fix for v2

looks like we could maybe do with some extra shrinker testing too? looks 
like nothing caught this during CI testing

> 
> /Thomas
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 4c25d9b2f138..fdb3a1c18cb6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -309,7 +309,8 @@  static struct ttm_tt *i915_ttm_tt_create(struct ttm_buffer_object *bo,
 		page_flags |= TTM_TT_FLAG_ZERO_ALLOC;
 
 	caching = i915_ttm_select_tt_caching(obj);
-	if (i915_gem_object_is_shrinkable(obj) && caching == ttm_cached) {
+	if (i915_gem_object_is_shrinkable(obj) && caching == ttm_cached &&
+	    !i915_gem_object_is_volatile(obj)) {
 		page_flags |= TTM_TT_FLAG_EXTERNAL |
 			      TTM_TT_FLAG_EXTERNAL_MAPPABLE;
 		i915_tt->is_shmem = true;