From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?Christian_K=F6nig?= <deathsimple@vodafone.de>
Subject: Re: [PATCH 10/10] drm/radeon: make page table updates async
Date: Mon, 13 Aug 2012 18:53:47 +0200
Message-ID: <5029311B.5030307@vodafone.de>
References: <1344853568-3870-1-git-send-email-deathsimple@vodafone.de>
	<1344853568-3870-11-git-send-email-deathsimple@vodafone.de>
	<CAH3drwafnzNuSkVXtFAH9O2ojUKpjeL7DUpndCoNFDSvZ7ENrQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"; Format="flowed"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de
	[139.7.28.128])
	by gabe.freedesktop.org (Postfix) with ESMTP id B44F29E75D
	for <dri-devel@lists.freedesktop.org>;
	Mon, 13 Aug 2012 09:53:50 -0700 (PDT)
In-Reply-To: <CAH3drwafnzNuSkVXtFAH9O2ojUKpjeL7DUpndCoNFDSvZ7ENrQ@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Jerome Glisse <j.glisse@gmail.com>
Cc: dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

On 13.08.2012 18:19, Jerome Glisse wrote:
> On Mon, Aug 13, 2012 at 6:26 AM, Christian K=F6nig
> <deathsimple@vodafone.de> wrote:
>> Currently doing the update with the CP.
>>
>> Signed-off-by: Christian K=F6nig <deathsimple@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 *rd=
ev,
>>                  return -EINVAL;
>>          }
>>
>> -       if (bo_va->valid && mem)
>> +       if (bo_va->valid =3D=3D !!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 =3D=3D 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.