On 13.08.2012 18:19, Jerome Glisse wrote:
On Mon, Aug 13, 2012 at 6:26 AM, Christian König
<deathsim...@vodafone.de> wrote:
Currently doing the update with the CP.

Signed-off-by: Christian König <deathsim...@vodafone.de>
NAK until point below are addressed
[SNIP]
For this to work properly you will need patch :
http://people.freedesktop.org/~glisse/0001-drm-radeon-make-sure-ib-bo-is-properly-bound-and-up-.patch

Otherwise there is a chance that ib bo pagetable is out of sync.
Oh yes indeed. Going to sort in your patch directly before of this one.

Also I haven't tested suspend/resume with this set yet, need to change that before sending it upstream.

[SNIP]
@@ -832,7 +829,7 @@ int radeon_vm_bo_update_pte(struct radeon_device *rdev,
                 return -EINVAL;
         }

-       if (bo_va->valid && mem)
+       if (bo_va->valid == !!mem)
                 return 0;
Logic is wrong, we want to return either if valid is true and mem not
null, which means bo is already bound and its pagetable entry are up
to date as it did not move since page table was last updated. Or
return if valid is false and mem is null, meaning the pagetable
already contain invalid page entry and we are unbinding the bo. So the
proper test should be :

if ((bo_va->valid && mem) || (!bo_va->valid && mem == NULL)) {
     return 0;
}
Isn't that identical? Ok your version is definitely easier to read, so I'm going to use that one anyway.

Thanks,
Christian.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to