[PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Noralf Trønnes]

dma-buf importing was reworked in commit 7d2cd72a9aa3
("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
unconditionally. Now without the use count set, put pages is called also
on dma-bufs. Fix this by only putting pages if it's not imported.

Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 9825c378dfa6..c8a6547a1757 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
 	if (--shmem->vmap_use_count > 0)
 		return;
 
-	if (obj->import_attach)
+	if (obj->import_attach) {
 		dma_buf_vunmap(obj->import_attach->dmabuf, map);
-	else
+	} else {
 		vunmap(shmem->vaddr);
+		drm_gem_shmem_put_pages(shmem);
+	}
 
 	shmem->vaddr = NULL;
-	drm_gem_shmem_put_pages(shmem);
 }
 
 /*
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2055 bytes --]

Hi

Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
> dma-buf importing was reworked in commit 7d2cd72a9aa3
> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
> unconditionally. Now without the use count set, put pages is called also
> on dma-bufs. Fix this by only putting pages if it's not imported.
> 
> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Just when I saw the error. Nice. :)

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Thomas Zimmermann <tzimmermann@suse.de>

For testing the GUD driver, you may also want to keep an eye at [1]

Best regards
Thomas

[1] 
https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t

> ---
>   drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 9825c378dfa6..c8a6547a1757 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
>   	if (--shmem->vmap_use_count > 0)
>   		return;
>   
> -	if (obj->import_attach)
> +	if (obj->import_attach) {
>   		dma_buf_vunmap(obj->import_attach->dmabuf, map);
> -	else
> +	} else {
>   		vunmap(shmem->vaddr);
> +		drm_gem_shmem_put_pages(shmem);
> +	}
>   
>   	shmem->vaddr = NULL;
> -	drm_gem_shmem_put_pages(shmem);
>   }
>   
>   /*
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Noralf Trønnes]

Den 19.02.2021 14.54, skrev Thomas Zimmermann:
> Hi
> 
> Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
>> dma-buf importing was reworked in commit 7d2cd72a9aa3
>> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
>> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
>> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
>> unconditionally. Now without the use count set, put pages is called also
>> on dma-bufs. Fix this by only putting pages if it's not imported.
>>
>> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> Just when I saw the error. Nice. :)
> 
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
> 

Should I apply this to drm-misc-fixes?

Noralf.

> For testing the GUD driver, you may also want to keep an eye at [1]
> 
> Best regards
> Thomas
> 
> [1]
> https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
> 
> 
>> ---
>>   drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index 9825c378dfa6..c8a6547a1757 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
>> drm_gem_shmem_object *shmem,
>>       if (--shmem->vmap_use_count > 0)
>>           return;
>>   -    if (obj->import_attach)
>> +    if (obj->import_attach) {
>>           dma_buf_vunmap(obj->import_attach->dmabuf, map);
>> -    else
>> +    } else {
>>           vunmap(shmem->vaddr);
>> +        drm_gem_shmem_put_pages(shmem);
>> +    }
>>         shmem->vaddr = NULL;
>> -    drm_gem_shmem_put_pages(shmem);
>>   }
>>     /*
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2549 bytes --]

Hi

Am 24.02.21 um 18:14 schrieb Noralf Trønnes:
> 
> 
> Den 19.02.2021 14.54, skrev Thomas Zimmermann:
>> Hi
>>
>> Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
>>> dma-buf importing was reworked in commit 7d2cd72a9aa3
>>> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
>>> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
>>> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
>>> unconditionally. Now without the use count set, put pages is called also
>>> on dma-bufs. Fix this by only putting pages if it's not imported.
>>>
>>> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>
>> Just when I saw the error. Nice. :)
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
> 
> Should I apply this to drm-misc-fixes?

I think so. Ping me if you want me to add it.

Best regards
Thomas

> 
> Noralf.
> 
>> For testing the GUD driver, you may also want to keep an eye at [1]
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
>>
>>
>>> ---
>>>    drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> index 9825c378dfa6..c8a6547a1757 100644
>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
>>> drm_gem_shmem_object *shmem,
>>>        if (--shmem->vmap_use_count > 0)
>>>            return;
>>>    -    if (obj->import_attach)
>>> +    if (obj->import_attach) {
>>>            dma_buf_vunmap(obj->import_attach->dmabuf, map);
>>> -    else
>>> +    } else {
>>>            vunmap(shmem->vaddr);
>>> +        drm_gem_shmem_put_pages(shmem);
>>> +    }
>>>          shmem->vaddr = NULL;
>>> -    drm_gem_shmem_put_pages(shmem);
>>>    }
>>>      /*
>>>
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Noralf Trønnes]

Den 25.02.2021 09.12, skrev Thomas Zimmermann:
> Hi
> 
> Am 24.02.21 um 18:14 schrieb Noralf Trønnes:
>>
>>
>> Den 19.02.2021 14.54, skrev Thomas Zimmermann:
>>> Hi
>>>
>>> Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
>>>> dma-buf importing was reworked in commit 7d2cd72a9aa3
>>>> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
>>>> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
>>>> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
>>>> unconditionally. Now without the use count set, put pages is called
>>>> also
>>>> on dma-bufs. Fix this by only putting pages if it's not imported.
>>>>
>>>> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>
>>> Just when I saw the error. Nice. :)
>>>
>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>
>> Should I apply this to drm-misc-fixes?
> 
> I think so. Ping me if you want me to add it.
> 

I'd appreciate if you could do it. I haven't applied a patch in almost a
year now and need to dig out my notes on how I do it. -fixes stuff is
for me a "hold my breath and hope I don't screw up anything" exercise.
Stressful :/

One day in the future I hope there's a green button I can push that
says: Merge patch to -fixes and make sure everything is OK. That would
be nice :)

Thanks,
Noralf.

> Best regards
> Thomas
> 
>>
>> Noralf.
>>
>>> For testing the GUD driver, you may also want to keep an eye at [1]
>>>
>>> Best regards
>>> Thomas
>>>
>>> [1]
>>> https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
>>>
>>>
>>>
>>>> ---
>>>>    drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> index 9825c378dfa6..c8a6547a1757 100644
>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
>>>> drm_gem_shmem_object *shmem,
>>>>        if (--shmem->vmap_use_count > 0)
>>>>            return;
>>>>    -    if (obj->import_attach)
>>>> +    if (obj->import_attach) {
>>>>            dma_buf_vunmap(obj->import_attach->dmabuf, map);
>>>> -    else
>>>> +    } else {
>>>>            vunmap(shmem->vaddr);
>>>> +        drm_gem_shmem_put_pages(shmem);
>>>> +    }
>>>>          shmem->vaddr = NULL;
>>>> -    drm_gem_shmem_put_pages(shmem);
>>>>    }
>>>>      /*
>>>>
>>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3511 bytes --]



Am 25.02.21 um 10:25 schrieb Noralf Trønnes:
> 
> 
> Den 25.02.2021 09.12, skrev Thomas Zimmermann:
>> Hi
>>
>> Am 24.02.21 um 18:14 schrieb Noralf Trønnes:
>>>
>>>
>>> Den 19.02.2021 14.54, skrev Thomas Zimmermann:
>>>> Hi
>>>>
>>>> Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
>>>>> dma-buf importing was reworked in commit 7d2cd72a9aa3
>>>>> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
>>>>> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
>>>>> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
>>>>> unconditionally. Now without the use count set, put pages is called
>>>>> also
>>>>> on dma-bufs. Fix this by only putting pages if it's not imported.
>>>>>
>>>>> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>
>>>> Just when I saw the error. Nice. :)
>>>>
>>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>>
>>>
>>> Should I apply this to drm-misc-fixes?
>>
>> I think so. Ping me if you want me to add it.
>>
> 
> I'd appreciate if you could do it. I haven't applied a patch in almost a
> year now and need to dig out my notes on how I do it. -fixes stuff is
> for me a "hold my breath and hope I don't screw up anything" exercise.
> Stressful :/

Applied now to drm-misc-next. If it breaks, I'll also take the blame. :) 
I took the liberty of moving your S-o-b line to the top of the tag. IIRC 
this is the preferred style. Thanks again for the fix.

Best regards
Thomas

> 
> One day in the future I hope there's a green button I can push that
> says: Merge patch to -fixes and make sure everything is OK. That would
> be nice :)
> 
> Thanks,
> Noralf.
> 
>> Best regards
>> Thomas
>>
>>>
>>> Noralf.
>>>
>>>> For testing the GUD driver, you may also want to keep an eye at [1]
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>> [1]
>>>> https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
>>>>
>>>>
>>>>
>>>>> ---
>>>>>     drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>>>>>     1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>>> index 9825c378dfa6..c8a6547a1757 100644
>>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>>> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
>>>>> drm_gem_shmem_object *shmem,
>>>>>         if (--shmem->vmap_use_count > 0)
>>>>>             return;
>>>>>     -    if (obj->import_attach)
>>>>> +    if (obj->import_attach) {
>>>>>             dma_buf_vunmap(obj->import_attach->dmabuf, map);
>>>>> -    else
>>>>> +    } else {
>>>>>             vunmap(shmem->vaddr);
>>>>> +        drm_gem_shmem_put_pages(shmem);
>>>>> +    }
>>>>>           shmem->vaddr = NULL;
>>>>> -    drm_gem_shmem_put_pages(shmem);
>>>>>     }
>>>>>       /*
>>>>>
>>>>
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Daniel Vetter]

On Wed, Feb 24, 2021 at 6:14 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 19.02.2021 14.54, skrev Thomas Zimmermann:
> > Hi
> >
> > Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
> >> dma-buf importing was reworked in commit 7d2cd72a9aa3
> >> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
> >> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
> >> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
> >> unconditionally. Now without the use count set, put pages is called also
> >> on dma-bufs. Fix this by only putting pages if it's not imported.
> >>
> >> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >
> > Just when I saw the error. Nice. :)
> >
> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
>
> Should I apply this to drm-misc-fixes?

drm-misc-next-fixes because we're in the merge window.
-Daniel

>
> Noralf.
>
> > For testing the GUD driver, you may also want to keep an eye at [1]
> >
> > Best regards
> > Thomas
> >
> > [1]
> > https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
> >
> >
> >> ---
> >>   drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> index 9825c378dfa6..c8a6547a1757 100644
> >> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> >> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
> >> drm_gem_shmem_object *shmem,
> >>       if (--shmem->vmap_use_count > 0)
> >>           return;
> >>   -    if (obj->import_attach)
> >> +    if (obj->import_attach) {
> >>           dma_buf_vunmap(obj->import_attach->dmabuf, map);
> >> -    else
> >> +    } else {
> >>           vunmap(shmem->vaddr);
> >> +        drm_gem_shmem_put_pages(shmem);
> >> +    }
> >>         shmem->vaddr = NULL;
> >> -    drm_gem_shmem_put_pages(shmem);
> >>   }
> >>     /*
> >>
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2728 bytes --]

Hi

Am 25.02.21 um 10:43 schrieb Daniel Vetter:
> On Wed, Feb 24, 2021 at 6:14 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>>
>> Den 19.02.2021 14.54, skrev Thomas Zimmermann:
>>> Hi
>>>
>>> Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
>>>> dma-buf importing was reworked in commit 7d2cd72a9aa3
>>>> ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
>>>> drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
>>>> drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
>>>> unconditionally. Now without the use count set, put pages is called also
>>>> on dma-bufs. Fix this by only putting pages if it's not imported.
>>>>
>>>> Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>
>>> Just when I saw the error. Nice. :)
>>>
>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>
>> Should I apply this to drm-misc-fixes?
> 
> drm-misc-next-fixes because we're in the merge window.

Too late. Can/should I cherry-pick the patch from drm-misc-next?

Best regards
Thomas

> -Daniel
> 
>>
>> Noralf.
>>
>>> For testing the GUD driver, you may also want to keep an eye at [1]
>>>
>>> Best regards
>>> Thomas
>>>
>>> [1]
>>> https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
>>>
>>>
>>>> ---
>>>>    drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
>>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> index 9825c378dfa6..c8a6547a1757 100644
>>>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>>>> @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
>>>> drm_gem_shmem_object *shmem,
>>>>        if (--shmem->vmap_use_count > 0)
>>>>            return;
>>>>    -    if (obj->import_attach)
>>>> +    if (obj->import_attach) {
>>>>            dma_buf_vunmap(obj->import_attach->dmabuf, map);
>>>> -    else
>>>> +    } else {
>>>>            vunmap(shmem->vaddr);
>>>> +        drm_gem_shmem_put_pages(shmem);
>>>> +    }
>>>>          shmem->vaddr = NULL;
>>>> -    drm_gem_shmem_put_pages(shmem);
>>>>    }
>>>>      /*
>>>>
>>>
> 
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/shmem-helpers: vunmap: Don't put pages for dma-buf

[Daniel Vetter]

On Thu, Feb 25, 2021 at 10:52:14AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 25.02.21 um 10:43 schrieb Daniel Vetter:
> > On Wed, Feb 24, 2021 at 6:14 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> > > 
> > > 
> > > 
> > > Den 19.02.2021 14.54, skrev Thomas Zimmermann:
> > > > Hi
> > > > 
> > > > Am 19.02.21 um 13:22 schrieb Noralf Trønnes:
> > > > > dma-buf importing was reworked in commit 7d2cd72a9aa3
> > > > > ("drm/shmem-helpers: Simplify dma-buf importing"). Before that commit
> > > > > drm_gem_shmem_prime_import_sg_table() did set ->pages_use_count=1 and
> > > > > drm_gem_shmem_vunmap_locked() could call drm_gem_shmem_put_pages()
> > > > > unconditionally. Now without the use count set, put pages is called also
> > > > > on dma-bufs. Fix this by only putting pages if it's not imported.
> > > > > 
> > > > > Fixes: 7d2cd72a9aa3 ("drm/shmem-helpers: Simplify dma-buf importing")
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > > 
> > > > Just when I saw the error. Nice. :)
> > > > 
> > > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Tested-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > > 
> > > 
> > > Should I apply this to drm-misc-fixes?
> > 
> > drm-misc-next-fixes because we're in the merge window.
> 
> Too late. Can/should I cherry-pick the patch from drm-misc-next?

Maarten just send the pull around, so pls coordinate with him. Maybe best
if Maarten cherry-picks onto drm-misc-fixes after -rc1 is out.
-Daniel

> 
> Best regards
> Thomas
> 
> > -Daniel
> > 
> > > 
> > > Noralf.
> > > 
> > > > For testing the GUD driver, you may also want to keep an eye at [1]
> > > > 
> > > > Best regards
> > > > Thomas
> > > > 
> > > > [1]
> > > > https://lore.kernel.org/dri-devel/02a45c11-fc73-1e5a-3839-30b080950af8@amd.com/T/#t
> > > > 
> > > > 
> > > > > ---
> > > > >    drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++---
> > > > >    1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > > > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > > > index 9825c378dfa6..c8a6547a1757 100644
> > > > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > > > > @@ -357,13 +357,14 @@ static void drm_gem_shmem_vunmap_locked(struct
> > > > > drm_gem_shmem_object *shmem,
> > > > >        if (--shmem->vmap_use_count > 0)
> > > > >            return;
> > > > >    -    if (obj->import_attach)
> > > > > +    if (obj->import_attach) {
> > > > >            dma_buf_vunmap(obj->import_attach->dmabuf, map);
> > > > > -    else
> > > > > +    } else {
> > > > >            vunmap(shmem->vaddr);
> > > > > +        drm_gem_shmem_put_pages(shmem);
> > > > > +    }
> > > > >          shmem->vaddr = NULL;
> > > > > -    drm_gem_shmem_put_pages(shmem);
> > > > >    }
> > > > >      /*
> > > > > 
> > > > 
> > 
> > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel