diff mbox

[1/2] drm/radeon: allways add the VM clear duplicate

Message ID 1436342174-6434-1-git-send-email-deathsimple@vodafone.de (mailing list archive)
State New, archived
Headers show

Commit Message

Christian König July 8, 2015, 7:56 a.m. UTC
From: Christian König <christian.koenig@amd.com>

We need to allways add the VM clear duplicate of the BO_VA,
no matter what the old status was.

Signed-off-by: Christian König <christian.koenig@amd.com>
Test-by: hadack@gmx.de
---
 drivers/gpu/drm/radeon/radeon_vm.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

Comments

Alex Deucher July 8, 2015, 3:57 p.m. UTC | #1
On Wed, Jul 8, 2015 at 3:56 AM, Christian König <deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> We need to allways add the VM clear duplicate of the BO_VA,
> no matter what the old status was.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Test-by: hadack@gmx.de

This patch does not apply cleanly.  It also adds an undefined label
error_unreserve.  Is there an additional patch required?

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_vm.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
> index ec10533..0310965 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>         }
>
>         if (bo_va->it.start || bo_va->it.last) {
> -               spin_lock(&vm->status_lock);
> -               if (list_empty(&bo_va->vm_status)) {
> -                       /* add a clone of the bo_va to clear the old address */
> -                       struct radeon_bo_va *tmp;
> -                       spin_unlock(&vm->status_lock);
> -                       tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
> -                       if (!tmp) {
> -                               mutex_unlock(&vm->mutex);
> -                               r = -ENOMEM;
> -                               goto error_unreserve;
> -                       }
> -                       tmp->it.start = bo_va->it.start;
> -                       tmp->it.last = bo_va->it.last;
> -                       tmp->vm = vm;
> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
> -                       spin_lock(&vm->status_lock);
> -                       list_add(&tmp->vm_status, &vm->freed);
> +               /* add a clone of the bo_va to clear the old address */
> +               struct radeon_bo_va *tmp;
> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
> +               if (!tmp) {
> +                       mutex_unlock(&vm->mutex);
> +                       r = -ENOMEM;
> +                       goto error_unreserve;
>                 }
> -               spin_unlock(&vm->status_lock);
> +               tmp->it.start = bo_va->it.start;
> +               tmp->it.last = bo_va->it.last;
> +               tmp->vm = vm;
> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>
>                 interval_tree_remove(&bo_va->it, &vm->va);
>                 bo_va->it.start = 0;
>                 bo_va->it.last = 0;
> +
> +               spin_lock(&vm->status_lock);
> +               list_del_init(&bo_va->vm_status);
> +               list_add(&tmp->vm_status, &vm->freed);
> +               spin_unlock(&vm->status_lock);
>         }
>
>         if (soffset || eoffset) {
> --
> 1.9.1
>
Christian König July 8, 2015, 4:08 p.m. UTC | #2
On 08.07.2015 17:57, Alex Deucher wrote:
> On Wed, Jul 8, 2015 at 3:56 AM, Christian König <deathsimple@vodafone.de> wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> We need to allways add the VM clear duplicate of the BO_VA,
>> no matter what the old status was.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Test-by: hadack@gmx.de
> This patch does not apply cleanly.  It also adds an undefined label
> error_unreserve.  Is there an additional patch required?

That's possible, but I thought the depending patch would have already 
been upstream.

Can you create a drm-fixes-4.2 branch I can rebase on?

Thanks,
Christian.

> Alex
>
>> ---
>>   drivers/gpu/drm/radeon/radeon_vm.c | 34 ++++++++++++++++------------------
>>   1 file changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
>> index ec10533..0310965 100644
>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>>          }
>>
>>          if (bo_va->it.start || bo_va->it.last) {
>> -               spin_lock(&vm->status_lock);
>> -               if (list_empty(&bo_va->vm_status)) {
>> -                       /* add a clone of the bo_va to clear the old address */
>> -                       struct radeon_bo_va *tmp;
>> -                       spin_unlock(&vm->status_lock);
>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>> -                       if (!tmp) {
>> -                               mutex_unlock(&vm->mutex);
>> -                               r = -ENOMEM;
>> -                               goto error_unreserve;
>> -                       }
>> -                       tmp->it.start = bo_va->it.start;
>> -                       tmp->it.last = bo_va->it.last;
>> -                       tmp->vm = vm;
>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>> -                       spin_lock(&vm->status_lock);
>> -                       list_add(&tmp->vm_status, &vm->freed);
>> +               /* add a clone of the bo_va to clear the old address */
>> +               struct radeon_bo_va *tmp;
>> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>> +               if (!tmp) {
>> +                       mutex_unlock(&vm->mutex);
>> +                       r = -ENOMEM;
>> +                       goto error_unreserve;
>>                  }
>> -               spin_unlock(&vm->status_lock);
>> +               tmp->it.start = bo_va->it.start;
>> +               tmp->it.last = bo_va->it.last;
>> +               tmp->vm = vm;
>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>
>>                  interval_tree_remove(&bo_va->it, &vm->va);
>>                  bo_va->it.start = 0;
>>                  bo_va->it.last = 0;
>> +
>> +               spin_lock(&vm->status_lock);
>> +               list_del_init(&bo_va->vm_status);
>> +               list_add(&tmp->vm_status, &vm->freed);
>> +               spin_unlock(&vm->status_lock);
>>          }
>>
>>          if (soffset || eoffset) {
>> --
>> 1.9.1
>>
Alex Deucher July 8, 2015, 4:14 p.m. UTC | #3
On Wed, Jul 8, 2015 at 12:08 PM, Christian König
<deathsimple@vodafone.de> wrote:
> On 08.07.2015 17:57, Alex Deucher wrote:
>>
>> On Wed, Jul 8, 2015 at 3:56 AM, Christian König <deathsimple@vodafone.de>
>> wrote:
>>>
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> We need to allways add the VM clear duplicate of the BO_VA,
>>> no matter what the old status was.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> Test-by: hadack@gmx.de
>>
>> This patch does not apply cleanly.  It also adds an undefined label
>> error_unreserve.  Is there an additional patch required?
>
>
> That's possible, but I thought the depending patch would have already been
> upstream.
>
> Can you create a drm-fixes-4.2 branch I can rebase on?

http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.2-wip

Thanks,

Alex

>
> Thanks,
> Christian.
>
>
>> Alex
>>
>>> ---
>>>   drivers/gpu/drm/radeon/radeon_vm.c | 34
>>> ++++++++++++++++------------------
>>>   1 file changed, 16 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
>>> b/drivers/gpu/drm/radeon/radeon_vm.c
>>> index ec10533..0310965 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device
>>> *rdev,
>>>          }
>>>
>>>          if (bo_va->it.start || bo_va->it.last) {
>>> -               spin_lock(&vm->status_lock);
>>> -               if (list_empty(&bo_va->vm_status)) {
>>> -                       /* add a clone of the bo_va to clear the old
>>> address */
>>> -                       struct radeon_bo_va *tmp;
>>> -                       spin_unlock(&vm->status_lock);
>>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va),
>>> GFP_KERNEL);
>>> -                       if (!tmp) {
>>> -                               mutex_unlock(&vm->mutex);
>>> -                               r = -ENOMEM;
>>> -                               goto error_unreserve;
>>> -                       }
>>> -                       tmp->it.start = bo_va->it.start;
>>> -                       tmp->it.last = bo_va->it.last;
>>> -                       tmp->vm = vm;
>>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>>> -                       spin_lock(&vm->status_lock);
>>> -                       list_add(&tmp->vm_status, &vm->freed);
>>> +               /* add a clone of the bo_va to clear the old address */
>>> +               struct radeon_bo_va *tmp;
>>> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>>> +               if (!tmp) {
>>> +                       mutex_unlock(&vm->mutex);
>>> +                       r = -ENOMEM;
>>> +                       goto error_unreserve;
>>>                  }
>>> -               spin_unlock(&vm->status_lock);
>>> +               tmp->it.start = bo_va->it.start;
>>> +               tmp->it.last = bo_va->it.last;
>>> +               tmp->vm = vm;
>>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>>
>>>                  interval_tree_remove(&bo_va->it, &vm->va);
>>>                  bo_va->it.start = 0;
>>>                  bo_va->it.last = 0;
>>> +
>>> +               spin_lock(&vm->status_lock);
>>> +               list_del_init(&bo_va->vm_status);
>>> +               list_add(&tmp->vm_status, &vm->freed);
>>> +               spin_unlock(&vm->status_lock);
>>>          }
>>>
>>>          if (soffset || eoffset) {
>>> --
>>> 1.9.1
>>>
>
Christian König July 8, 2015, 4:31 p.m. UTC | #4
On 08.07.2015 18:14, Alex Deucher wrote:
> On Wed, Jul 8, 2015 at 12:08 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 08.07.2015 17:57, Alex Deucher wrote:
>>> On Wed, Jul 8, 2015 at 3:56 AM, Christian König <deathsimple@vodafone.de>
>>> wrote:
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> We need to allways add the VM clear duplicate of the BO_VA,
>>>> no matter what the old status was.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> Test-by: hadack@gmx.de
>>> This patch does not apply cleanly.  It also adds an undefined label
>>> error_unreserve.  Is there an additional patch required?
>>
>> That's possible, but I thought the depending patch would have already been
>> upstream.
>>
>> Can you create a drm-fixes-4.2 branch I can rebase on?
> http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.2-wip

Ah, here is the problem. This branch don't yet contains the fixes which 
went upstream with 4.1.

What should we do? I don't want to change those patches and create a 
merge conflict when you merge in 4.1.

Christian.

>
> Thanks,
>
> Alex
>
>> Thanks,
>> Christian.
>>
>>
>>> Alex
>>>
>>>> ---
>>>>    drivers/gpu/drm/radeon/radeon_vm.c | 34
>>>> ++++++++++++++++------------------
>>>>    1 file changed, 16 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
>>>> b/drivers/gpu/drm/radeon/radeon_vm.c
>>>> index ec10533..0310965 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>>>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device
>>>> *rdev,
>>>>           }
>>>>
>>>>           if (bo_va->it.start || bo_va->it.last) {
>>>> -               spin_lock(&vm->status_lock);
>>>> -               if (list_empty(&bo_va->vm_status)) {
>>>> -                       /* add a clone of the bo_va to clear the old
>>>> address */
>>>> -                       struct radeon_bo_va *tmp;
>>>> -                       spin_unlock(&vm->status_lock);
>>>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va),
>>>> GFP_KERNEL);
>>>> -                       if (!tmp) {
>>>> -                               mutex_unlock(&vm->mutex);
>>>> -                               r = -ENOMEM;
>>>> -                               goto error_unreserve;
>>>> -                       }
>>>> -                       tmp->it.start = bo_va->it.start;
>>>> -                       tmp->it.last = bo_va->it.last;
>>>> -                       tmp->vm = vm;
>>>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>>>> -                       spin_lock(&vm->status_lock);
>>>> -                       list_add(&tmp->vm_status, &vm->freed);
>>>> +               /* add a clone of the bo_va to clear the old address */
>>>> +               struct radeon_bo_va *tmp;
>>>> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>>>> +               if (!tmp) {
>>>> +                       mutex_unlock(&vm->mutex);
>>>> +                       r = -ENOMEM;
>>>> +                       goto error_unreserve;
>>>>                   }
>>>> -               spin_unlock(&vm->status_lock);
>>>> +               tmp->it.start = bo_va->it.start;
>>>> +               tmp->it.last = bo_va->it.last;
>>>> +               tmp->vm = vm;
>>>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>
>>>>                   interval_tree_remove(&bo_va->it, &vm->va);
>>>>                   bo_va->it.start = 0;
>>>>                   bo_va->it.last = 0;
>>>> +
>>>> +               spin_lock(&vm->status_lock);
>>>> +               list_del_init(&bo_va->vm_status);
>>>> +               list_add(&tmp->vm_status, &vm->freed);
>>>> +               spin_unlock(&vm->status_lock);
>>>>           }
>>>>
>>>>           if (soffset || eoffset) {
>>>> --
>>>> 1.9.1
>>>>
Alex Deucher July 8, 2015, 4:33 p.m. UTC | #5
On Wed, Jul 8, 2015 at 12:31 PM, Christian König
<deathsimple@vodafone.de> wrote:
> On 08.07.2015 18:14, Alex Deucher wrote:
>>
>> On Wed, Jul 8, 2015 at 12:08 PM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>>
>>> On 08.07.2015 17:57, Alex Deucher wrote:
>>>>
>>>> On Wed, Jul 8, 2015 at 3:56 AM, Christian König
>>>> <deathsimple@vodafone.de>
>>>> wrote:
>>>>>
>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>
>>>>> We need to allways add the VM clear duplicate of the BO_VA,
>>>>> no matter what the old status was.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> Test-by: hadack@gmx.de
>>>>
>>>> This patch does not apply cleanly.  It also adds an undefined label
>>>> error_unreserve.  Is there an additional patch required?
>>>
>>>
>>> That's possible, but I thought the depending patch would have already
>>> been
>>> upstream.
>>>
>>> Can you create a drm-fixes-4.2 branch I can rebase on?
>>
>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.2-wip
>
>
> Ah, here is the problem. This branch don't yet contains the fixes which went
> upstream with 4.1.
>
> What should we do? I don't want to change those patches and create a merge
> conflict when you merge in 4.1.

I'll rebase by tree on Linus' tree.  Dave is on vacation so his tree is lagging.

Alex

>
> Christian.
>
>
>>
>> Thanks,
>>
>> Alex
>>
>>> Thanks,
>>> Christian.
>>>
>>>
>>>> Alex
>>>>
>>>>> ---
>>>>>    drivers/gpu/drm/radeon/radeon_vm.c | 34
>>>>> ++++++++++++++++------------------
>>>>>    1 file changed, 16 insertions(+), 18 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>> b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>> index ec10533..0310965 100644
>>>>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device
>>>>> *rdev,
>>>>>           }
>>>>>
>>>>>           if (bo_va->it.start || bo_va->it.last) {
>>>>> -               spin_lock(&vm->status_lock);
>>>>> -               if (list_empty(&bo_va->vm_status)) {
>>>>> -                       /* add a clone of the bo_va to clear the old
>>>>> address */
>>>>> -                       struct radeon_bo_va *tmp;
>>>>> -                       spin_unlock(&vm->status_lock);
>>>>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va),
>>>>> GFP_KERNEL);
>>>>> -                       if (!tmp) {
>>>>> -                               mutex_unlock(&vm->mutex);
>>>>> -                               r = -ENOMEM;
>>>>> -                               goto error_unreserve;
>>>>> -                       }
>>>>> -                       tmp->it.start = bo_va->it.start;
>>>>> -                       tmp->it.last = bo_va->it.last;
>>>>> -                       tmp->vm = vm;
>>>>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>> -                       spin_lock(&vm->status_lock);
>>>>> -                       list_add(&tmp->vm_status, &vm->freed);
>>>>> +               /* add a clone of the bo_va to clear the old address */
>>>>> +               struct radeon_bo_va *tmp;
>>>>> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>>>>> +               if (!tmp) {
>>>>> +                       mutex_unlock(&vm->mutex);
>>>>> +                       r = -ENOMEM;
>>>>> +                       goto error_unreserve;
>>>>>                   }
>>>>> -               spin_unlock(&vm->status_lock);
>>>>> +               tmp->it.start = bo_va->it.start;
>>>>> +               tmp->it.last = bo_va->it.last;
>>>>> +               tmp->vm = vm;
>>>>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>>
>>>>>                   interval_tree_remove(&bo_va->it, &vm->va);
>>>>>                   bo_va->it.start = 0;
>>>>>                   bo_va->it.last = 0;
>>>>> +
>>>>> +               spin_lock(&vm->status_lock);
>>>>> +               list_del_init(&bo_va->vm_status);
>>>>> +               list_add(&tmp->vm_status, &vm->freed);
>>>>> +               spin_unlock(&vm->status_lock);
>>>>>           }
>>>>>
>>>>>           if (soffset || eoffset) {
>>>>> --
>>>>> 1.9.1
>>>>>
>
Christian König July 8, 2015, 6:28 p.m. UTC | #6
On 08.07.2015 18:33, Alex Deucher wrote:
> On Wed, Jul 8, 2015 at 12:31 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 08.07.2015 18:14, Alex Deucher wrote:
>>> On Wed, Jul 8, 2015 at 12:08 PM, Christian König
>>> <deathsimple@vodafone.de> wrote:
>>>> On 08.07.2015 17:57, Alex Deucher wrote:
>>>>> On Wed, Jul 8, 2015 at 3:56 AM, Christian König
>>>>> <deathsimple@vodafone.de>
>>>>> wrote:
>>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>>
>>>>>> We need to allways add the VM clear duplicate of the BO_VA,
>>>>>> no matter what the old status was.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> Test-by: hadack@gmx.de
>>>>> This patch does not apply cleanly.  It also adds an undefined label
>>>>> error_unreserve.  Is there an additional patch required?
>>>>
>>>> That's possible, but I thought the depending patch would have already
>>>> been
>>>> upstream.
>>>>
>>>> Can you create a drm-fixes-4.2 branch I can rebase on?
>>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.2-wip
>>
>> Ah, here is the problem. This branch don't yet contains the fixes which went
>> upstream with 4.1.
>>
>> What should we do? I don't want to change those patches and create a merge
>> conflict when you merge in 4.1.
> I'll rebase by tree on Linus' tree.  Dave is on vacation so his tree is lagging.

Sounds good, let me know when you have updated the branch and there are 
still conflicts.

Regards,
Christian.

>
> Alex
>
>> Christian.
>>
>>
>>> Thanks,
>>>
>>> Alex
>>>
>>>> Thanks,
>>>> Christian.
>>>>
>>>>
>>>>> Alex
>>>>>
>>>>>> ---
>>>>>>     drivers/gpu/drm/radeon/radeon_vm.c | 34
>>>>>> ++++++++++++++++------------------
>>>>>>     1 file changed, 16 insertions(+), 18 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>> b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>> index ec10533..0310965 100644
>>>>>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device
>>>>>> *rdev,
>>>>>>            }
>>>>>>
>>>>>>            if (bo_va->it.start || bo_va->it.last) {
>>>>>> -               spin_lock(&vm->status_lock);
>>>>>> -               if (list_empty(&bo_va->vm_status)) {
>>>>>> -                       /* add a clone of the bo_va to clear the old
>>>>>> address */
>>>>>> -                       struct radeon_bo_va *tmp;
>>>>>> -                       spin_unlock(&vm->status_lock);
>>>>>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va),
>>>>>> GFP_KERNEL);
>>>>>> -                       if (!tmp) {
>>>>>> -                               mutex_unlock(&vm->mutex);
>>>>>> -                               r = -ENOMEM;
>>>>>> -                               goto error_unreserve;
>>>>>> -                       }
>>>>>> -                       tmp->it.start = bo_va->it.start;
>>>>>> -                       tmp->it.last = bo_va->it.last;
>>>>>> -                       tmp->vm = vm;
>>>>>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>>> -                       spin_lock(&vm->status_lock);
>>>>>> -                       list_add(&tmp->vm_status, &vm->freed);
>>>>>> +               /* add a clone of the bo_va to clear the old address */
>>>>>> +               struct radeon_bo_va *tmp;
>>>>>> +               tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>>>>>> +               if (!tmp) {
>>>>>> +                       mutex_unlock(&vm->mutex);
>>>>>> +                       r = -ENOMEM;
>>>>>> +                       goto error_unreserve;
>>>>>>                    }
>>>>>> -               spin_unlock(&vm->status_lock);
>>>>>> +               tmp->it.start = bo_va->it.start;
>>>>>> +               tmp->it.last = bo_va->it.last;
>>>>>> +               tmp->vm = vm;
>>>>>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>>>
>>>>>>                    interval_tree_remove(&bo_va->it, &vm->va);
>>>>>>                    bo_va->it.start = 0;
>>>>>>                    bo_va->it.last = 0;
>>>>>> +
>>>>>> +               spin_lock(&vm->status_lock);
>>>>>> +               list_del_init(&bo_va->vm_status);
>>>>>> +               list_add(&tmp->vm_status, &vm->freed);
>>>>>> +               spin_unlock(&vm->status_lock);
>>>>>>            }
>>>>>>
>>>>>>            if (soffset || eoffset) {
>>>>>> --
>>>>>> 1.9.1
>>>>>>
Alex Deucher July 8, 2015, 7:03 p.m. UTC | #7
On Wed, Jul 8, 2015 at 2:28 PM, Christian König <deathsimple@vodafone.de> wrote:
> On 08.07.2015 18:33, Alex Deucher wrote:
>>
>> On Wed, Jul 8, 2015 at 12:31 PM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>>
>>> On 08.07.2015 18:14, Alex Deucher wrote:
>>>>
>>>> On Wed, Jul 8, 2015 at 12:08 PM, Christian König
>>>> <deathsimple@vodafone.de> wrote:
>>>>>
>>>>> On 08.07.2015 17:57, Alex Deucher wrote:
>>>>>>
>>>>>> On Wed, Jul 8, 2015 at 3:56 AM, Christian König
>>>>>> <deathsimple@vodafone.de>
>>>>>> wrote:
>>>>>>>
>>>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>>>
>>>>>>> We need to allways add the VM clear duplicate of the BO_VA,
>>>>>>> no matter what the old status was.
>>>>>>>
>>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>>> Test-by: hadack@gmx.de
>>>>>>
>>>>>> This patch does not apply cleanly.  It also adds an undefined label
>>>>>> error_unreserve.  Is there an additional patch required?
>>>>>
>>>>>
>>>>> That's possible, but I thought the depending patch would have already
>>>>> been
>>>>> upstream.
>>>>>
>>>>> Can you create a drm-fixes-4.2 branch I can rebase on?
>>>>
>>>> http://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-fixes-4.2-wip
>>>
>>>
>>> Ah, here is the problem. This branch don't yet contains the fixes which
>>> went
>>> upstream with 4.1.
>>>
>>> What should we do? I don't want to change those patches and create a
>>> merge
>>> conflict when you merge in 4.1.
>>
>> I'll rebase by tree on Linus' tree.  Dave is on vacation so his tree is
>> lagging.
>
>
> Sounds good, let me know when you have updated the branch and there are
> still conflicts.

All set.

Thanks,

Alex

>
> Regards,
> Christian.
>
>
>>
>> Alex
>>
>>> Christian.
>>>
>>>
>>>> Thanks,
>>>>
>>>> Alex
>>>>
>>>>> Thanks,
>>>>> Christian.
>>>>>
>>>>>
>>>>>> Alex
>>>>>>
>>>>>>> ---
>>>>>>>     drivers/gpu/drm/radeon/radeon_vm.c | 34
>>>>>>> ++++++++++++++++------------------
>>>>>>>     1 file changed, 16 insertions(+), 18 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>>> b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>>> index ec10533..0310965 100644
>>>>>>> --- a/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>>> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
>>>>>>> @@ -493,29 +493,27 @@ int radeon_vm_bo_set_addr(struct radeon_device
>>>>>>> *rdev,
>>>>>>>            }
>>>>>>>
>>>>>>>            if (bo_va->it.start || bo_va->it.last) {
>>>>>>> -               spin_lock(&vm->status_lock);
>>>>>>> -               if (list_empty(&bo_va->vm_status)) {
>>>>>>> -                       /* add a clone of the bo_va to clear the old
>>>>>>> address */
>>>>>>> -                       struct radeon_bo_va *tmp;
>>>>>>> -                       spin_unlock(&vm->status_lock);
>>>>>>> -                       tmp = kzalloc(sizeof(struct radeon_bo_va),
>>>>>>> GFP_KERNEL);
>>>>>>> -                       if (!tmp) {
>>>>>>> -                               mutex_unlock(&vm->mutex);
>>>>>>> -                               r = -ENOMEM;
>>>>>>> -                               goto error_unreserve;
>>>>>>> -                       }
>>>>>>> -                       tmp->it.start = bo_va->it.start;
>>>>>>> -                       tmp->it.last = bo_va->it.last;
>>>>>>> -                       tmp->vm = vm;
>>>>>>> -                       tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>>>> -                       spin_lock(&vm->status_lock);
>>>>>>> -                       list_add(&tmp->vm_status, &vm->freed);
>>>>>>> +               /* add a clone of the bo_va to clear the old address
>>>>>>> */
>>>>>>> +               struct radeon_bo_va *tmp;
>>>>>>> +               tmp = kzalloc(sizeof(struct radeon_bo_va),
>>>>>>> GFP_KERNEL);
>>>>>>> +               if (!tmp) {
>>>>>>> +                       mutex_unlock(&vm->mutex);
>>>>>>> +                       r = -ENOMEM;
>>>>>>> +                       goto error_unreserve;
>>>>>>>                    }
>>>>>>> -               spin_unlock(&vm->status_lock);
>>>>>>> +               tmp->it.start = bo_va->it.start;
>>>>>>> +               tmp->it.last = bo_va->it.last;
>>>>>>> +               tmp->vm = vm;
>>>>>>> +               tmp->bo = radeon_bo_ref(bo_va->bo);
>>>>>>>
>>>>>>>                    interval_tree_remove(&bo_va->it, &vm->va);
>>>>>>>                    bo_va->it.start = 0;
>>>>>>>                    bo_va->it.last = 0;
>>>>>>> +
>>>>>>> +               spin_lock(&vm->status_lock);
>>>>>>> +               list_del_init(&bo_va->vm_status);
>>>>>>> +               list_add(&tmp->vm_status, &vm->freed);
>>>>>>> +               spin_unlock(&vm->status_lock);
>>>>>>>            }
>>>>>>>
>>>>>>>            if (soffset || eoffset) {
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
index ec10533..0310965 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -493,29 +493,27 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 	}
 
 	if (bo_va->it.start || bo_va->it.last) {
-		spin_lock(&vm->status_lock);
-		if (list_empty(&bo_va->vm_status)) {
-			/* add a clone of the bo_va to clear the old address */
-			struct radeon_bo_va *tmp;
-			spin_unlock(&vm->status_lock);
-			tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
-			if (!tmp) {
-				mutex_unlock(&vm->mutex);
-				r = -ENOMEM;
-				goto error_unreserve;
-			}
-			tmp->it.start = bo_va->it.start;
-			tmp->it.last = bo_va->it.last;
-			tmp->vm = vm;
-			tmp->bo = radeon_bo_ref(bo_va->bo);
-			spin_lock(&vm->status_lock);
-			list_add(&tmp->vm_status, &vm->freed);
+		/* add a clone of the bo_va to clear the old address */
+		struct radeon_bo_va *tmp;
+		tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
+		if (!tmp) {
+			mutex_unlock(&vm->mutex);
+			r = -ENOMEM;
+			goto error_unreserve;
 		}
-		spin_unlock(&vm->status_lock);
+		tmp->it.start = bo_va->it.start;
+		tmp->it.last = bo_va->it.last;
+		tmp->vm = vm;
+		tmp->bo = radeon_bo_ref(bo_va->bo);
 
 		interval_tree_remove(&bo_va->it, &vm->va);
 		bo_va->it.start = 0;
 		bo_va->it.last = 0;
+
+		spin_lock(&vm->status_lock);
+		list_del_init(&bo_va->vm_status);
+		list_add(&tmp->vm_status, &vm->freed);
+		spin_unlock(&vm->status_lock);
 	}
 
 	if (soffset || eoffset) {