Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Marek Olšák]

Hi,

Given how our memory manager works and the guesswork that UMDs have to
do to determine whether to set the flag, I think the flag isn't
useful.

I'm proposing that CPU_ACCESS_REQUIRED:
- will be deprecated.
- It will remain to be accepted by the kernel driver, but it will
either not have any effect, or it will serve as a hint that might or
might not be followed.
- The only flag that UMDs are expected to set with regard to CPU
access is NO_CPU_ACCESS.

The main motivation is the reduction of "virtual" heaps for UMD buffer
suballocators and reusable buffer pools. A higher number of heaps
means that more memory can be wasted by UMDs.

Opinions?

Thanks,
Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Christian König]

Yeah, I was thinking something similar.

See the intention behind CPU_ACCESS_REQUIRED is to always guarantee that 
CPU access is immediately possible.

If you ask me that is not really useful for the UMD and was never meant 
to be used by Mesa (only the closed source UMD and some kernel internal 
use cases).

I would like to keep the behavior in the kernel driver as it is, but we 
should really stop using this as a hint in Mesa.

Regards,
Christian.

Am 29.06.2017 um 16:41 schrieb Marek Olšák:
> Hi,
>
> Given how our memory manager works and the guesswork that UMDs have to
> do to determine whether to set the flag, I think the flag isn't
> useful.
>
> I'm proposing that CPU_ACCESS_REQUIRED:
> - will be deprecated.
> - It will remain to be accepted by the kernel driver, but it will
> either not have any effect, or it will serve as a hint that might or
> might not be followed.
> - The only flag that UMDs are expected to set with regard to CPU
> access is NO_CPU_ACCESS.
>
> The main motivation is the reduction of "virtual" heaps for UMD buffer
> suballocators and reusable buffer pools. A higher number of heaps
> means that more memory can be wasted by UMDs.
>
> Opinions?
>
> Thanks,
> Marek
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Marek Olšák]

Do you have any concern if we also stop using the CPU_ACCESS flag on radeon?

Thanks,
Marek

On Thu, Jun 29, 2017 at 4:51 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Yeah, I was thinking something similar.
>
> See the intention behind CPU_ACCESS_REQUIRED is to always guarantee that CPU
> access is immediately possible.
>
> If you ask me that is not really useful for the UMD and was never meant to
> be used by Mesa (only the closed source UMD and some kernel internal use
> cases).
>
> I would like to keep the behavior in the kernel driver as it is, but we
> should really stop using this as a hint in Mesa.
>
> Regards,
> Christian.
>
>
> Am 29.06.2017 um 16:41 schrieb Marek Olšák:
>>
>> Hi,
>>
>> Given how our memory manager works and the guesswork that UMDs have to
>> do to determine whether to set the flag, I think the flag isn't
>> useful.
>>
>> I'm proposing that CPU_ACCESS_REQUIRED:
>> - will be deprecated.
>> - It will remain to be accepted by the kernel driver, but it will
>> either not have any effect, or it will serve as a hint that might or
>> might not be followed.
>> - The only flag that UMDs are expected to set with regard to CPU
>> access is NO_CPU_ACCESS.
>>
>> The main motivation is the reduction of "virtual" heaps for UMD buffer
>> suballocators and reusable buffer pools. A higher number of heaps
>> means that more memory can be wasted by UMDs.
>>
>> Opinions?
>>
>> Thanks,
>> Marek
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 12:03 AM, Marek Olšák wrote:
> Do you have any concern if we also stop using the CPU_ACCESS flag on radeon?

No concern from my side for radeon.


> On Thu, Jun 29, 2017 at 4:51 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Yeah, I was thinking something similar.
>>
>> See the intention behind CPU_ACCESS_REQUIRED is to always guarantee that CPU
>> access is immediately possible.
>>
>> If you ask me that is not really useful for the UMD and was never meant to
>> be used by Mesa (only the closed source UMD and some kernel internal use
>> cases).
>>
>> I would like to keep the behavior in the kernel driver as it is, but we
>> should really stop using this as a hint in Mesa.

So we'd have a flag in the userspace ABI which is only used by closed
source userspace, and which can be used to artificially create pressure
on CPU visible VRAM. I know somebody who would argue vehemently against
adding something like that. :)

What does the closed source UMD use the flag for?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Mao, David]

[-- Attachment #1.1: Type: text/plain, Size: 2229 bytes --]

Vulkan allows the application to decide whether it wants the allocation to be host visible and device local.
If we drop the flag, what will happen if we did not set the NO_CPU_ACCESS flag?
Will it fail the map in case the allocation could be placed in invisible heap then?

Thanks.
Best Regards,
David
On 29 Jun 2017, at 11:03 PM, Marek Olšák <maraeo@gmail.com<mailto:maraeo@gmail.com>> wrote:

Do you have any concern if we also stop using the CPU_ACCESS flag on radeon?

Thanks,
Marek

On Thu, Jun 29, 2017 at 4:51 PM, Christian König
<deathsimple@vodafone.de<mailto:deathsimple@vodafone.de>> wrote:
Yeah, I was thinking something similar.

See the intention behind CPU_ACCESS_REQUIRED is to always guarantee that CPU
access is immediately possible.

If you ask me that is not really useful for the UMD and was never meant to
be used by Mesa (only the closed source UMD and some kernel internal use
cases).

I would like to keep the behavior in the kernel driver as it is, but we
should really stop using this as a hint in Mesa.

Regards,
Christian.


Am 29.06.2017 um 16:41 schrieb Marek Olšák:

Hi,

Given how our memory manager works and the guesswork that UMDs have to
do to determine whether to set the flag, I think the flag isn't
useful.

I'm proposing that CPU_ACCESS_REQUIRED:
- will be deprecated.
- It will remain to be accepted by the kernel driver, but it will
either not have any effect, or it will serve as a hint that might or
might not be followed.
- The only flag that UMDs are expected to set with regard to CPU
access is NO_CPU_ACCESS.

The main motivation is the reduction of "virtual" heaps for UMD buffer
suballocators and reusable buffer pools. A higher number of heaps
means that more memory can be wasted by UMDs.

Opinions?

Thanks,
Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 4208 bytes --]

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 10:55 AM, Mao, David wrote:
> Vulkan allows the application to decide whether it wants the allocation
> to be host visible and device local. 
> If we drop the flag, what will happen if we did not set the
> NO_CPU_ACCESS flag?
> Will it fail the map in case the allocation could be placed in invisible
> heap then?

No, it'll work just as well. On attempted CPU access,
amdgpu_bo_fault_reserve_notify will ensure that it's CPU accessible.

The difference is that it'll allow BOs which aren't being actively
accessed by the CPU to be in CPU invisible VRAM, reducing pressure on
CPU visible VRAM.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Mao, David]

[-- Attachment #1.1: Type: text/plain, Size: 1549 bytes --]

Sounds good!
One thing to confirm, If the original location is already in the invisible, will the notifier callback to move the bo from invisible to visible?  if it is true and the logic is already available in the kernel, can we use NO_CPU_ACCESS flag by default to accomplish the similar purpose for now?
It also reminds me of another related topic, can we always take visible heap as priority against to the remote in this case?
So far, kernel don’t have the heap priority.
IIRC, if the LFB bo moved to GTT, it will never be moved back since GTT is also its preferred heap. (Kernel seems to add the GTT even if the UMD only ask for LFB).

Thanks.
Best Regards,
David
On 30 Jun 2017, at 11:36 AM, Michel Dänzer <michel@daenzer.net<mailto:michel@daenzer.net>> wrote:

On 30/06/17 10:55 AM, Mao, David wrote:
Vulkan allows the application to decide whether it wants the allocation
to be host visible and device local.
If we drop the flag, what will happen if we did not set the
NO_CPU_ACCESS flag?
Will it fail the map in case the allocation could be placed in invisible
heap then?

No, it'll work just as well. On attempted CPU access,
amdgpu_bo_fault_reserve_notify will ensure that it's CPU accessible.

The difference is that it'll allow BOs which aren't being actively
accessed by the CPU to be in CPU invisible VRAM, reducing pressure on
CPU visible VRAM.


--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer


[-- Attachment #1.2: Type: text/html, Size: 3367 bytes --]

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Christian König]

Am 30.06.2017 um 03:36 schrieb Michel Dänzer:
> On 30/06/17 12:03 AM, Marek Olšák wrote:
>> Do you have any concern if we also stop using the CPU_ACCESS flag on radeon?
> No concern from my side for radeon.
>
>
>> On Thu, Jun 29, 2017 at 4:51 PM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>> Yeah, I was thinking something similar.
>>>
>>> See the intention behind CPU_ACCESS_REQUIRED is to always guarantee that CPU
>>> access is immediately possible.
>>>
>>> If you ask me that is not really useful for the UMD and was never meant to
>>> be used by Mesa (only the closed source UMD and some kernel internal use
>>> cases).
>>>
>>> I would like to keep the behavior in the kernel driver as it is, but we
>>> should really stop using this as a hint in Mesa.
> So we'd have a flag in the userspace ABI which is only used by closed
> source userspace, and which can be used to artificially create pressure
> on CPU visible VRAM. I know somebody who would argue vehemently against
> adding something like that. :)

Yeah, and I really tried hard to prevent that :)

But unfortunately the milk is already spilled, so not much we can do 
about that.

> What does the closed source UMD use the flag for?

Well it doesn't use the flag, but it has the concept of separate heaps 
for visible and invisible VRAM and maps that to setting the flag 
appropriately.

But putting the closed source UMD asside, my primary concern is rather 
in kernel users of the flag.

Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 03:44 PM, Christian König wrote:
> Am 30.06.2017 um 03:36 schrieb Michel Dänzer:
>> On 30/06/17 12:03 AM, Marek Olšák wrote:
>>> On Thu, Jun 29, 2017 at 4:51 PM, Christian König
>>> <deathsimple@vodafone.de> wrote:
>>>> Yeah, I was thinking something similar.
>>>>
>>>> See the intention behind CPU_ACCESS_REQUIRED is to always guarantee
>>>> that CPU
>>>> access is immediately possible.
>>>>
>>>> If you ask me that is not really useful for the UMD and was never
>>>> meant to
>>>> be used by Mesa (only the closed source UMD and some kernel internal
>>>> use
>>>> cases).
>>>>
>>>> I would like to keep the behavior in the kernel driver as it is, but we
>>>> should really stop using this as a hint in Mesa.
>> So we'd have a flag in the userspace ABI which is only used by closed
>> source userspace, and which can be used to artificially create pressure
>> on CPU visible VRAM. I know somebody who would argue vehemently against
>> adding something like that. :)
> 
> Yeah, and I really tried hard to prevent that :)
> 
> But unfortunately the milk is already spilled, so not much we can do
> about that.

Right, sorry I didn't realize this issue when we added this flag.


>> What does the closed source UMD use the flag for?
> 
> Well it doesn't use the flag, but it has the concept of separate heaps
> for visible and invisible VRAM and maps that to setting the flag
> appropriately.

That doesn't require the strict semantics you're defending. John has
proven with a lot of hard work that the looser semantics work out better
in general.


> But putting the closed source UMD asside, my primary concern is rather
> in kernel users of the flag.

We can deal with that internally in the kernel, while fixing the
existing flag for userspace.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Christian König]

Am 30.06.2017 um 08:51 schrieb Michel Dänzer:
> We can deal with that internally in the kernel, while fixing the
> existing flag for userspace.

And as I said, NAK to that approach. I'm not going to add a 
CPU_ACCESS_REALLY_REQUIRED flag in the kernel just because mesa has 
messed up it's use case.

We could agree on filtering that flag from userspace when BOs are 
created and/or map it to a CREATE_CPU_ACCESS_HINT flag. But I would 
rather want to avoid that as well.

Christian.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 01:00 PM, Mao, David wrote:
> Sounds good!
> One thing to confirm, If the original location is already in the
> invisible, will the notifier callback to move the bo from invisible to
> visible?

Yes.


> if it is true and the logic is already available in the kernel, can we
> use NO_CPU_ACCESS flag by default to accomplish the similar purpose for
> now?

You mean set the NO_CPU_ACCESS flag for BOs in the "CPU invisible heap"?
Yes, that's a good idea.

However, we can also improve the kernel driver's handling of the
CPU_ACCESS_REQUIRED flag so that userspace code can continue using it
the way it has been.


> It also reminds me of another related topic, can we always take visible
> heap as priority against to the remote in this case?
> So far, kernel don’t have the heap priority.
> IIRC, if the LFB bo moved to GTT, it will never be moved back since GTT
> is also its preferred heap.

That can happen if userspace specifies both VRAM and GTT as preferred
domains. It's one reason why that isn't recommended.

> (Kernel seems to add the GTT even if the UMD only ask for LFB).

I can only see

                robj->allowed_domains = robj->prefered_domains;
                if (robj->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
                        robj->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;

which adds GTT as an *allowed* domain for BOs which only have VRAM as
the preferred domain. Since VRAM is the only preferred domain, the
driver will attempt to move the BO from GTT to VRAM on userspace command
stream submissions (subject to throttling).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 03:59 PM, Christian König wrote:
> Am 30.06.2017 um 08:51 schrieb Michel Dänzer:
>> We can deal with that internally in the kernel, while fixing the
>> existing flag for userspace.
> 
> And as I said, NAK to that approach. I'm not going to add a
> CPU_ACCESS_REALLY_REQUIRED flag in the kernel just because mesa has
> messed up it's use case.
> 
> We could agree on filtering that flag from userspace when BOs are
> created and/or map it to a CREATE_CPU_ACCESS_HINT flag.

Then I propose the following:

One patch:

Convert AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED to a kernel internal flag
AMDGPU_GEM_CPU_ACCESS_HINT in amdgpu_gem_create_ioctl, which is
initially treated the same way as AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED.

Another patch:

Change the treatment of AMDGPU_GEM_CPU_ACCESS_HINT according to John's
patch 4 in the latest series, or a variation of that as discussed on IRC.


If any regressions are reported, we will be able to differentiate
whether they are due to the addition of the new flag itself or due to
the change in its handling.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Christian König]

Am 30.06.2017 um 09:14 schrieb Michel Dänzer:
> On 30/06/17 03:59 PM, Christian König wrote:
>> Am 30.06.2017 um 08:51 schrieb Michel Dänzer:
>>> We can deal with that internally in the kernel, while fixing the
>>> existing flag for userspace.
>> And as I said, NAK to that approach. I'm not going to add a
>> CPU_ACCESS_REALLY_REQUIRED flag in the kernel just because mesa has
>> messed up it's use case.
>>
>> We could agree on filtering that flag from userspace when BOs are
>> created and/or map it to a CREATE_CPU_ACCESS_HINT flag.
> Then I propose the following:
>
> One patch:
>
> Convert AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED to a kernel internal flag
> AMDGPU_GEM_CPU_ACCESS_HINT in amdgpu_gem_create_ioctl, which is
> initially treated the same way as AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED.
>
> Another patch:
>
> Change the treatment of AMDGPU_GEM_CPU_ACCESS_HINT according to John's
> patch 4 in the latest series, or a variation of that as discussed on IRC.
>
>
> If any regressions are reported, we will be able to differentiate
> whether they are due to the addition of the new flag itself or due to
> the change in its handling.


It just occurred to me that there is a simpler way of handling this: We 
just never clear the flag on kernel allocations.

See my main concern are the in kernel users of the flag which use it as 
guarantee that the BO is CPU accessible.

If we handle those specially there shouldn't be a problem clearing the 
flag for the UMD BOs.

Christian.

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Marek Olšák]

On Fri, Jun 30, 2017 at 12:34 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 30.06.2017 um 09:14 schrieb Michel Dänzer:
>>
>> On 30/06/17 03:59 PM, Christian König wrote:
>>>
>>> Am 30.06.2017 um 08:51 schrieb Michel Dänzer:
>>>>
>>>> We can deal with that internally in the kernel, while fixing the
>>>> existing flag for userspace.
>>>
>>> And as I said, NAK to that approach. I'm not going to add a
>>> CPU_ACCESS_REALLY_REQUIRED flag in the kernel just because mesa has
>>> messed up it's use case.
>>>
>>> We could agree on filtering that flag from userspace when BOs are
>>> created and/or map it to a CREATE_CPU_ACCESS_HINT flag.
>>
>> Then I propose the following:
>>
>> One patch:
>>
>> Convert AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED to a kernel internal flag
>> AMDGPU_GEM_CPU_ACCESS_HINT in amdgpu_gem_create_ioctl, which is
>> initially treated the same way as AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED.
>>
>> Another patch:
>>
>> Change the treatment of AMDGPU_GEM_CPU_ACCESS_HINT according to John's
>> patch 4 in the latest series, or a variation of that as discussed on IRC.
>>
>>
>> If any regressions are reported, we will be able to differentiate
>> whether they are due to the addition of the new flag itself or due to
>> the change in its handling.
>
>
>
> It just occurred to me that there is a simpler way of handling this: We just
> never clear the flag on kernel allocations.
>
> See my main concern are the in kernel users of the flag which use it as
> guarantee that the BO is CPU accessible.
>
> If we handle those specially there shouldn't be a problem clearing the flag
> for the UMD BOs.

Hi,

I don't know what is being talked about here anymore, but I wouldn't
like to use CPU_ACCESS_REQUIRED or CPU_ACCESS_REALLY_REQUIRED in
userspace. The reason is that userspace doesn't and can't know whether
CPU access will be required, and the frequency at which it will be
required. 3 heaps {no CPU access, no flag, CPU access required} are
too many. Userspace mostly doesn't use the "no flag" heap for VRAM. It
uses "CPU access required" for almost everything except tiled
textures, which use "no CPU access".

I've been trying to trim down the number of heaps. So far, I have:
- VRAM_NO_CPU_ACCESS (implies WC)
- VRAM (implies WC)
- VRAM_GTT (combined, implies WC)
- GTT_WC
- GTT

See, you can't forbid CPU access for the combined VRAM_GTT heap. It's
one of the compromises there.

The more heaps we have, the more memory can be wasted by
suballocators. It's silly to have more than 3 suballocators just for
VRAM.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH RFC 0/2] Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[John Brooks]

> On 30/06/17 03:59 PM, Christian König wrote:
>> Am 30.06.2017 um 08:51 schrieb Michel Dänzer:
>>> We can deal with that internally in the kernel, while fixing the
>>> existing flag for userspace.
>> And as I said, NAK to that approach. I'm not going to add a
>> CPU_ACCESS_REALLY_REQUIRED flag in the kernel just because mesa has
>> messed up it's use case.
>>
>> We could agree on filtering that flag from userspace when BOs are
>> created and/or map it to a CREATE_CPU_ACCESS_HINT flag.
> Then I propose the following:
>
> One patch:
>
> Convert AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED to a kernel internal flag
> AMDGPU_GEM_CPU_ACCESS_HINT in amdgpu_gem_create_ioctl, which is
> initially treated the same way as AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED.
>
> Another patch:
>
> Change the treatment of AMDGPU_GEM_CPU_ACCESS_HINT according to John's
> patch 4 in the latest series, or a variation of that as discussed on IRC.
>
>
> If any regressions are reported, we will be able to differentiate
> whether they are due to the addition of the new flag itself or due to
> the change in its handling.
>

How about this?
Note: I haven't tested this beyond compiling.

See replies for:
[PATCH RFC 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS
[PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault

John

>
> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH RFC 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS

[John Brooks]

For userspace BO allocations, replace AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED
with a new AMDGPU_BO_FLAG_CPU_ACCESS flag. This flag will be used to
indicate that a BO should currently be CPU accessible. Unlike the
CPU_ACCESS_REQUIRED flag, it is meant to be an ephemeral rather than a
permanent constraint. Currently, however, it is treated no differently.

Signed-off-by: John Brooks <john@fastquake.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 ++++++++-
 include/uapi/drm/amdgpu_drm.h              | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 12d61ed..a724e4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -411,6 +411,9 @@ struct amdgpu_bo_va {
 
 #define AMDGPU_GEM_DOMAIN_MAX		0x3
 
+/* BO internal flags */
+#define AMDGPU_BO_FLAG_CPU_ACCESS	(AMDGPU_GEM_CREATE_MAX << 1)
+
 struct amdgpu_bo {
 	/* Protected by tbo.reserved */
 	u32				prefered_domains;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8ee6965..fa8aeca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -128,7 +128,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev,
 		places[c].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
 			TTM_PL_FLAG_VRAM;
 
-		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
+		if (flags & (AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
+			     AMDGPU_BO_FLAG_CPU_ACCESS))
 			places[c].lpfn = visible_pfn;
 		else
 			places[c].flags |= TTM_PL_FLAG_TOPDOWN;
@@ -361,6 +362,12 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
 	if (!kernel && bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
 		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
 
+	if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
+		flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
+		/* Treat CPU_ACCESS_REQUIRED only as a hint if given by UMD */
+		if (!kernel)
+			flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+	}
 	bo->flags = flags;
 
 #ifdef CONFIG_X86_32
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index d9aa4a3..473076f 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -87,6 +87,7 @@ extern "C" {
 #define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
 /* Flag that allocating the BO should use linear VRAM */
 #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
+#define AMDGPU_GEM_CREATE_MAX			(1 << 5)
 
 struct drm_amdgpu_gem_create_in  {
 	/** the requested memory size */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[John Brooks]

When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This allows it
to potentially later move to invisible VRAM if the CPU does not access it
again.

Setting the CPU_ACCESS flag in amdgpu_fault_reserve_notify() also means
that we can remove the loop to restrict lpfn to the end of visible VRAM,
because amdgpu_ttm_placement_init() will do it for us.

Signed-off-by: John Brooks <john@fastquake.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index fa8aeca..19bd2fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -953,6 +953,9 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
 		return 0;
 
 	abo = container_of(bo, struct amdgpu_bo, tbo);
+
+	abo->flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
+
 	if (bo->mem.mem_type != TTM_PL_VRAM)
 		return 0;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c9b131b..cc65cdd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -417,6 +417,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 				struct ttm_mem_reg *new_mem)
 {
 	struct amdgpu_device *adev;
+	struct amdgpu_bo *abo;
 	struct ttm_mem_reg *old_mem = &bo->mem;
 	struct ttm_mem_reg tmp_mem;
 	struct ttm_placement placement;
@@ -424,6 +425,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 	int r;
 
 	adev = amdgpu_ttm_adev(bo->bdev);
+	abo = container_of(bo, struct amdgpu_bo, tbo);
 	tmp_mem = *new_mem;
 	tmp_mem.mm_node = NULL;
 	placement.num_placement = 1;
@@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
+
+	/* The page fault handler will re-set this if the CPU accesses the BO
+	 * after it's moved.
+	 */
+	abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
+
 out_cleanup:
 	ttm_bo_mem_put(bo, &tmp_mem);
 	return r;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH RFC v2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[John Brooks]

When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This allows it
to potentially later move to invisible VRAM if the CPU does not access it
again.

Setting the CPU_ACCESS flag in amdgpu_bo_fault_reserve_notify() also means
that we can remove the loop to restrict lpfn to the end of visible VRAM,
because amdgpu_ttm_placement_init() will do it for us.

Signed-off-by: John Brooks <john@fastquake.com>
---

Whoops, I forgot to actually remove that loop.
Also, in the changelog: amdgpu_fault_reserve_notify -> amdgpu_bo_fault_reserve_notify

 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 15 +++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 ++++++++
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index fa8aeca..7164f8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -946,13 +946,16 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
 	struct amdgpu_bo *abo;
-	unsigned long offset, size, lpfn;
-	int i, r;
+	unsigned long offset, size;
+	int r;
 
 	if (!amdgpu_ttm_bo_is_amdgpu_bo(bo))
 		return 0;
 
 	abo = container_of(bo, struct amdgpu_bo, tbo);
+
+	abo->flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
+
 	if (bo->mem.mem_type != TTM_PL_VRAM)
 		return 0;
 
@@ -969,14 +972,6 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
 	/* hurrah the memory is not visible ! */
 	atomic64_inc(&adev->num_vram_cpu_page_faults);
 	amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM);
-	lpfn =	adev->mc.visible_vram_size >> PAGE_SHIFT;
-	for (i = 0; i < abo->placement.num_placement; i++) {
-		/* Force into visible VRAM */
-		if ((abo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
-		    (!abo->placements[i].lpfn ||
-		     abo->placements[i].lpfn > lpfn))
-			abo->placements[i].lpfn = lpfn;
-	}
 	r = ttm_bo_validate(bo, &abo->placement, false, false);
 	if (unlikely(r == -ENOMEM)) {
 		amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_GTT);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c9b131b..cc65cdd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -417,6 +417,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 				struct ttm_mem_reg *new_mem)
 {
 	struct amdgpu_device *adev;
+	struct amdgpu_bo *abo;
 	struct ttm_mem_reg *old_mem = &bo->mem;
 	struct ttm_mem_reg tmp_mem;
 	struct ttm_placement placement;
@@ -424,6 +425,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 	int r;
 
 	adev = amdgpu_ttm_adev(bo->bdev);
+	abo = container_of(bo, struct amdgpu_bo, tbo);
 	tmp_mem = *new_mem;
 	tmp_mem.mm_node = NULL;
 	placement.num_placement = 1;
@@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
 	if (unlikely(r)) {
 		goto out_cleanup;
 	}
+
+	/* The page fault handler will re-set this if the CPU accesses the BO
+	 * after it's moved.
+	 */
+	abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
+
 out_cleanup:
 	ttm_bo_mem_put(bo, &tmp_mem);
 	return r;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS

[Christian König]

Am 30.06.2017 um 17:18 schrieb John Brooks:
> For userspace BO allocations, replace AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED
> with a new AMDGPU_BO_FLAG_CPU_ACCESS flag. This flag will be used to
> indicate that a BO should currently be CPU accessible. Unlike the
> CPU_ACCESS_REQUIRED flag, it is meant to be an ephemeral rather than a
> permanent constraint. Currently, however, it is treated no differently.
>
> Signed-off-by: John Brooks <john@fastquake.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 3 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 ++++++++-
>   include/uapi/drm/amdgpu_drm.h              | 1 +
>   3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 12d61ed..a724e4f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -411,6 +411,9 @@ struct amdgpu_bo_va {
>   
>   #define AMDGPU_GEM_DOMAIN_MAX		0x3
>   
> +/* BO internal flags */
> +#define AMDGPU_BO_FLAG_CPU_ACCESS	(AMDGPU_GEM_CREATE_MAX << 1)
Better put this into amdgpu_drm.h to keep all the flags together. We can 
reject kernel internal flags when we create the BO.

But in general I would still say we should stop clearing the flag when 
"bo->tbo.type != ttm_bo_type_device".

This way we are always on the save side and don't modify flags for 
kernel BOs.

Regards,
Christian.

> +
>   struct amdgpu_bo {
>   	/* Protected by tbo.reserved */
>   	u32				prefered_domains;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 8ee6965..fa8aeca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -128,7 +128,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev,
>   		places[c].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>   			TTM_PL_FLAG_VRAM;
>   
> -		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
> +		if (flags & (AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> +			     AMDGPU_BO_FLAG_CPU_ACCESS))
>   			places[c].lpfn = visible_pfn;
>   		else
>   			places[c].flags |= TTM_PL_FLAG_TOPDOWN;
> @@ -361,6 +362,12 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
>   	if (!kernel && bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>   		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>   
> +	if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +		flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
> +		/* Treat CPU_ACCESS_REQUIRED only as a hint if given by UMD */
> +		if (!kernel)
> +			flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +	}
>   	bo->flags = flags;
>   
>   #ifdef CONFIG_X86_32
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index d9aa4a3..473076f 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -87,6 +87,7 @@ extern "C" {
>   #define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
>   /* Flag that allocating the BO should use linear VRAM */
>   #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
> +#define AMDGPU_GEM_CREATE_MAX			(1 << 5)
>   
>   struct drm_amdgpu_gem_create_in  {
>   	/** the requested memory size */


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH RFC v2 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS

[John Brooks]

For userspace BO allocations, replace AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED
with a new AMDGPU_BO_FLAG_CPU_ACCESS flag. This flag will be used to
indicate that a BO should currently be CPU accessible. Unlike the
CPU_ACCESS_REQUIRED flag, it is meant to be an ephemeral rather than a
permanent constraint. Currently, however, it is treated no differently.

v2: Put the new flag in amdgpu_drm.h, only clear CPU_ACCESS_REQUIRED if
    bo->tbo.type == ttm_bo_type_device (Christian König)

Signed-off-by: John Brooks <john@fastquake.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 ++++++++-
 include/uapi/drm/amdgpu_drm.h              | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8ee6965..a5e73f8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -128,7 +128,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev,
 		places[c].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
 			TTM_PL_FLAG_VRAM;
 
-		if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)
+		if (flags & (AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
+			     AMDGPU_BO_FLAG_CPU_ACCESS))
 			places[c].lpfn = visible_pfn;
 		else
 			places[c].flags |= TTM_PL_FLAG_TOPDOWN;
@@ -361,6 +362,12 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
 	if (!kernel && bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
 		bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
 
+	if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
+		flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
+		/* Treat CPU_ACCESS_REQUIRED only as a hint if given by UMD */
+		if (type == ttm_bo_type_device)
+			flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+	}
 	bo->flags = flags;
 
 #ifdef CONFIG_X86_32
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index d9aa4a3..aeff910 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -87,6 +87,8 @@ extern "C" {
 #define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
 /* Flag that allocating the BO should use linear VRAM */
 #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS	(1 << 5)
+/* Flag that the BO is actively being used by the CPU. Internal use only */
+#define AMDGPU_BO_FLAG_CPU_ACCESS		(1 << 6)
 
 struct drm_amdgpu_gem_create_in  {
 	/** the requested memory size */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Christian König]

Am 30.06.2017 um 17:18 schrieb John Brooks:
> When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This allows it
> to potentially later move to invisible VRAM if the CPU does not access it
> again.
>
> Setting the CPU_ACCESS flag in amdgpu_fault_reserve_notify() also means
> that we can remove the loop to restrict lpfn to the end of visible VRAM,
> because amdgpu_ttm_placement_init() will do it for us.
>
> Signed-off-by: John Brooks <john@fastquake.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 8 ++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index fa8aeca..19bd2fd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -953,6 +953,9 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
>   		return 0;
>   
>   	abo = container_of(bo, struct amdgpu_bo, tbo);
> +
> +	abo->flags |= AMDGPU_BO_FLAG_CPU_ACCESS;
> +
>   	if (bo->mem.mem_type != TTM_PL_VRAM)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index c9b131b..cc65cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -417,6 +417,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
>   				struct ttm_mem_reg *new_mem)
>   {
>   	struct amdgpu_device *adev;
> +	struct amdgpu_bo *abo;
>   	struct ttm_mem_reg *old_mem = &bo->mem;
>   	struct ttm_mem_reg tmp_mem;
>   	struct ttm_placement placement;
> @@ -424,6 +425,7 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
>   	int r;
>   
>   	adev = amdgpu_ttm_adev(bo->bdev);
> +	abo = container_of(bo, struct amdgpu_bo, tbo);
>   	tmp_mem = *new_mem;
>   	tmp_mem.mm_node = NULL;
>   	placement.num_placement = 1;
> @@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
>   	if (unlikely(r)) {
>   		goto out_cleanup;
>   	}
> +
> +	/* The page fault handler will re-set this if the CPU accesses the BO
> +	 * after it's moved.
> +	 */
> +	abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
> +

This is the wrong place for clearing the flag. This code path is only 
called when we move things back in after suspend/resume (or run out of 
GTT space).

Regards,
Christian.

>   out_cleanup:
>   	ttm_bo_mem_put(bo, &tmp_mem);
>   	return r;


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Michel Dänzer]

On 02/07/17 09:52 PM, Christian König wrote:
> Am 30.06.2017 um 17:18 schrieb John Brooks:
>> When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This
>> allows it
>> to potentially later move to invisible VRAM if the CPU does not access it
>> again.
>>
>> Setting the CPU_ACCESS flag in amdgpu_fault_reserve_notify() also means
>> that we can remove the loop to restrict lpfn to the end of visible VRAM,
>> because amdgpu_ttm_placement_init() will do it for us.
>>
>> Signed-off-by: John Brooks <john@fastquake.com>

[...]

>> @@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct
>> ttm_buffer_object *bo,
>>       if (unlikely(r)) {
>>           goto out_cleanup;
>>       }
>> +
>> +    /* The page fault handler will re-set this if the CPU accesses
>> the BO
>> +     * after it's moved.
>> +     */

Maybe say "amdgpu_bo_fault_reserve_notify" explicitly here instead of
"The page fault handler".


>> +    abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
>> +
> 
> This is the wrong place for clearing the flag. This code path is only
> called when we move things back in after suspend/resume (or run out of
> GTT space).

Surely amdgpu_move_ram_vram is called whenever a BO is moved to VRAM,
for any reason. I suggested clearing the flag here to John on IRC. The
idea is briefly described in the commit log, let me elaborate a bit on that:

When a BO is moved to VRAM which has the AMDGPU_BO_FLAG_CPU_ACCESS flag
set, it is put in CPU visible VRAM, and the flag is cleared. If the CPU
doesn't access the BO, the next time it will be moved to VRAM (after it
was evicted from there, for any reason), the flag will no longer be set,
and the BO will likely be moved to CPU invisible VRAM.

If the BO is accessed by the CPU again though (no matter where the BO is
currently located at that time), the flag is set again, and the cycle
from the previous paragraph starts over.

The end result should be similar as with the timestamp based solution in
John's earlier series: BOs which are at least occasionally accessed by
the CPU will tend to be in CPU visible VRAM, those which are never
accessed by the CPU can be in CPU invisible VRAM.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Christian König]

Am 03.07.2017 um 03:34 schrieb Michel Dänzer:
> On 02/07/17 09:52 PM, Christian König wrote:
>> Am 30.06.2017 um 17:18 schrieb John Brooks:
>>> When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This
>>> allows it
>>> to potentially later move to invisible VRAM if the CPU does not access it
>>> again.
>>>
>>> Setting the CPU_ACCESS flag in amdgpu_fault_reserve_notify() also means
>>> that we can remove the loop to restrict lpfn to the end of visible VRAM,
>>> because amdgpu_ttm_placement_init() will do it for us.
>>>
>>> Signed-off-by: John Brooks <john@fastquake.com>
> [...]
>
>>> @@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct
>>> ttm_buffer_object *bo,
>>>        if (unlikely(r)) {
>>>            goto out_cleanup;
>>>        }
>>> +
>>> +    /* The page fault handler will re-set this if the CPU accesses
>>> the BO
>>> +     * after it's moved.
>>> +     */
> Maybe say "amdgpu_bo_fault_reserve_notify" explicitly here instead of
> "The page fault handler".
>
>
>>> +    abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
>>> +
>> This is the wrong place for clearing the flag. This code path is only
>> called when we move things back in after suspend/resume (or run out of
>> GTT space).
> Surely amdgpu_move_ram_vram is called whenever a BO is moved to VRAM,

No, that isn't even remotely correct. amdgpu_move_ram_vram() is only 
called when the BO is moved directly from the system domain to the VRAM 
domain.

Normally BOs are only moved from the GTT domain to the VRAM domain, 
except after resume and when we ran out of GTT space.

> for any reason. I suggested clearing the flag here to John on IRC. The
> idea is briefly described in the commit log, let me elaborate a bit on that:
>
> When a BO is moved to VRAM which has the AMDGPU_BO_FLAG_CPU_ACCESS flag
> set, it is put in CPU visible VRAM, and the flag is cleared. If the CPU
> doesn't access the BO, the next time it will be moved to VRAM (after it
> was evicted from there, for any reason), the flag will no longer be set,
> and the BO will likely be moved to CPU invisible VRAM.
>
> If the BO is accessed by the CPU again though (no matter where the BO is
> currently located at that time), the flag is set again, and the cycle
> from the previous paragraph starts over.
>
> The end result should be similar as with the timestamp based solution in
> John's earlier series: BOs which are at least occasionally accessed by
> the CPU will tend to be in CPU visible VRAM, those which are never
> accessed by the CPU can be in CPU invisible VRAM.
Yeah, I understand the intention. But the implementation isn't even 
remotely correct.

First of all the flag must be cleared in the CS which wants to move the 
BO, not in the move functions when the decision where to put it is 
already made.

Second currently the flag is set on page fault, but never cleared 
because the place where the code to clear it was added is just 
completely incorrect (see above).

Instead of messing with all this I suggest that we just add a jiffies 
based timeout to the BO when we can clear the flag. For kernel BOs this 
timeout is just infinity.

Then we check in amdgpu_cs_bo_validate() before generating the 
placements if we could clear the flag and do so based on the timeout.

I can help implementing this when I'm done getting ride of the BO move 
size limitation (swapped all of this stuff for that task back into my 
brain anyway).

Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Michel Dänzer]

On 03/07/17 04:06 PM, Christian König wrote:
> Am 03.07.2017 um 03:34 schrieb Michel Dänzer:
> 
>> [...] I suggested clearing the flag here to John on IRC. The
>> idea is briefly described in the commit log, let me elaborate a bit on
>> that:
>>
>> When a BO is moved to VRAM which has the AMDGPU_BO_FLAG_CPU_ACCESS flag
>> set, it is put in CPU visible VRAM, and the flag is cleared. If the CPU
>> doesn't access the BO, the next time it will be moved to VRAM (after it
>> was evicted from there, for any reason), the flag will no longer be set,
>> and the BO will likely be moved to CPU invisible VRAM.
>>
>> If the BO is accessed by the CPU again though (no matter where the BO is
>> currently located at that time), the flag is set again, and the cycle
>> from the previous paragraph starts over.
>>
>> The end result should be similar as with the timestamp based solution in
>> John's earlier series: BOs which are at least occasionally accessed by
>> the CPU will tend to be in CPU visible VRAM, those which are never
>> accessed by the CPU can be in CPU invisible VRAM.
> Yeah, I understand the intention. But the implementation isn't even
> remotely correct.
> 
> First of all the flag must be cleared in the CS which wants to move the
> BO, not in the move functions when the decision where to put it is
> already made.

For the purpose of this patch, we should clear the flag when the BO is
actually moved to VRAM, regardless of how or why.


> Second currently the flag is set on page fault, but never cleared
> because the place where the code to clear it was added is just
> completely incorrect (see above).

My bad, thanks for pointing this out. The following at the end of
amdgpu_bo_move should do the trick:

	if (new_mem->mem_type == TTM_PL_VRAM &&
	    old_mem->mem_type != TTM_PL_VRAM) {
		/* amdgpu_bo_fault_reserve_notify will re-set this if
		 * the CPU accesses the BO after it's moved.
		 */
		abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
	}


> Instead of messing with all this I suggest that we just add a jiffies
> based timeout to the BO when we can clear the flag. For kernel BOs this
> timeout is just infinity.
> 
> Then we check in amdgpu_cs_bo_validate() before generating the
> placements if we could clear the flag and do so based on the timeout.

The idea for this patch was to save the memory and CPU cycles needed for
that approach.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC v2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Michel Dänzer]

On 01/07/17 12:31 AM, John Brooks wrote:
> When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This allows it
> to potentially later move to invisible VRAM if the CPU does not access it
> again.
> 
> Setting the CPU_ACCESS flag in amdgpu_bo_fault_reserve_notify() also means
> that we can remove the loop to restrict lpfn to the end of visible VRAM,
> because amdgpu_ttm_placement_init() will do it for us.
> 
> Signed-off-by: John Brooks <john@fastquake.com>

[...]

> @@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object *bo,
>  	if (unlikely(r)) {
>  		goto out_cleanup;
>  	}
> +
> +	/* The page fault handler will re-set this if the CPU accesses the BO
> +	 * after it's moved.
> +	 */
> +	abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;

I've come to realize that the flag also needs to be cleared at the end
of amdgpu_bo_create_restricted, otherwise we will incorrectly assume
that every BO created with this flag has been accessed by the CPU.


BTW, there's also a minor issue here in that the flags member is u64,
but the flags are defined as ints. Probably doesn't matter so far, but
it would as soon as any flag's value is >= (1 << 32).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 30/06/17 08:43 PM, Marek Olšák wrote:
> 
> I don't know what is being talked about here anymore, but I wouldn't
> like to use CPU_ACCESS_REQUIRED or CPU_ACCESS_REALLY_REQUIRED in
> userspace. The reason is that userspace doesn't and can't know whether
> CPU access will be required, and the frequency at which it will be
> required. 3 heaps {no CPU access, no flag, CPU access required} are
> too many. Userspace mostly doesn't use the "no flag" heap for VRAM. It
> uses "CPU access required" for almost everything except tiled
> textures, which use "no CPU access".

FWIW, the difference between setting CPU_ACCESS_REQUIRED and not setting
it for a BO created in VRAM will be: If it's set, the BO is initially
created in CPU visible VRAM, otherwise it's most likely created in CPU
invisible VRAM.

If userspace knows that a BO will likely be accessed by the CPU first,
setting the flag could save a move from CPU invisible to CPU visible
VRAM when the CPU access happens. Conversely, if a BO will likely never
be accessed by the CPU, not setting the flag may reduce pressure on CPU
visible VRAM.

Not sure radeonsi can make this distinction though.


> I've been trying to trim down the number of heaps. So far, I have:
> - VRAM_NO_CPU_ACCESS (implies WC)
> - VRAM (implies WC)
> - VRAM_GTT (combined, implies WC)

Is this useful? It means:

* The BO may be created in VRAM, or if there's no space, in GTT.
* Once the BO is in GTT for any reason, it will never go back to VRAM.

Such BOs will tend to end up in GTT after some time, at the latest after
suspend/resume.

I think it would be better for radeonsi to choose either VRAM or GTT as
the preferred domain, and let the kernel handle it.


> - GTT_WC
> - GTT




-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Christian König]

Am 03.07.2017 um 11:49 schrieb Michel Dänzer:
>> Instead of messing with all this I suggest that we just add a jiffies
>> based timeout to the BO when we can clear the flag. For kernel BOs this
>> timeout is just infinity.
>>
>> Then we check in amdgpu_cs_bo_validate() before generating the
>> placements if we could clear the flag and do so based on the timeout.
> The idea for this patch was to save the memory and CPU cycles needed for
> that approach.
But when we clear the flag on the end of the move we already moved the 
BO to visible VRAM again.

Only on after the next swapout/swapin cycle we see an effect of that change.

Is that the intended approach?

Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Marek Olšák]

On Mon, Jul 3, 2017 at 12:08 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 30/06/17 08:43 PM, Marek Olšák wrote:
>>
>> I don't know what is being talked about here anymore, but I wouldn't
>> like to use CPU_ACCESS_REQUIRED or CPU_ACCESS_REALLY_REQUIRED in
>> userspace. The reason is that userspace doesn't and can't know whether
>> CPU access will be required, and the frequency at which it will be
>> required. 3 heaps {no CPU access, no flag, CPU access required} are
>> too many. Userspace mostly doesn't use the "no flag" heap for VRAM. It
>> uses "CPU access required" for almost everything except tiled
>> textures, which use "no CPU access".
>
> FWIW, the difference between setting CPU_ACCESS_REQUIRED and not setting
> it for a BO created in VRAM will be: If it's set, the BO is initially
> created in CPU visible VRAM, otherwise it's most likely created in CPU
> invisible VRAM.
>
> If userspace knows that a BO will likely be accessed by the CPU first,
> setting the flag could save a move from CPU invisible to CPU visible
> VRAM when the CPU access happens. Conversely, if a BO will likely never
> be accessed by the CPU, not setting the flag may reduce pressure on CPU
> visible VRAM.
>
> Not sure radeonsi can make this distinction though.

It can't.

Either all mappable BOs set CPU_ACCESS_REQUIRED, or all mappable BOs
don't set it. Either way, there is only one combination of flags for
mappable BOs in VRAM, and therefore only one kind of behavior the
kernel can follow.

>
>
>> I've been trying to trim down the number of heaps. So far, I have:
>> - VRAM_NO_CPU_ACCESS (implies WC)
>> - VRAM (implies WC)
>> - VRAM_GTT (combined, implies WC)
>
> Is this useful? It means:
>
> * The BO may be created in VRAM, or if there's no space, in GTT.
> * Once the BO is in GTT for any reason, it will never go back to VRAM.
>
> Such BOs will tend to end up in GTT after some time, at the latest after
> suspend/resume.
>
> I think it would be better for radeonsi to choose either VRAM or GTT as
> the preferred domain, and let the kernel handle it.

Currently, radeonsi on amdgpu doesn't use VRAM_GTT with the current kernel.

I'm aware of the limited usefulness.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM

[Michel Dänzer]

On 03/07/17 08:47 PM, Christian König wrote:
> Am 03.07.2017 um 11:49 schrieb Michel Dänzer:
>>> Instead of messing with all this I suggest that we just add a jiffies
>>> based timeout to the BO when we can clear the flag. For kernel BOs this
>>> timeout is just infinity.
>>>
>>> Then we check in amdgpu_cs_bo_validate() before generating the
>>> placements if we could clear the flag and do so based on the timeout.
>> The idea for this patch was to save the memory and CPU cycles needed for
>> that approach.
> But when we clear the flag on the end of the move we already moved the
> BO to visible VRAM again.

Right. Clearing the flag before the move would make the flag
ineffective. We want to put the BO in CPU visible VRAM when the flag is
set, because it means the BO has been accessed by the CPU since it was
created or since it was last moved to VRAM.


> Only on after the next swapout/swapin cycle we see an effect of that
> change.

Right, clearing the flag cannot have any effect before the next time the
BO is moved to VRAM anyway.


> Is that the intended approach?

So yes, it is.


The only significant difference to the timestamp based approach (for
which John already posted patches before) is that this patch will
remember any CPU access until the next time the BO is moved to VRAM, no
matter how much time passes in between.

BTW, one issue with the timestamp based approach is that we only get a
page fault on the first CPU access after the BO was moved. So the
timestamp only says how much time has passed since the first CPU access,
not since the last CPU access.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Michel Dänzer]

On 03/07/17 10:03 PM, Marek Olšák wrote:
> On Mon, Jul 3, 2017 at 12:08 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 30/06/17 08:43 PM, Marek Olšák wrote:
>>>
>>> I don't know what is being talked about here anymore, but I wouldn't
>>> like to use CPU_ACCESS_REQUIRED or CPU_ACCESS_REALLY_REQUIRED in
>>> userspace. The reason is that userspace doesn't and can't know whether
>>> CPU access will be required, and the frequency at which it will be
>>> required. 3 heaps {no CPU access, no flag, CPU access required} are
>>> too many. Userspace mostly doesn't use the "no flag" heap for VRAM. It
>>> uses "CPU access required" for almost everything except tiled
>>> textures, which use "no CPU access".
>>
>> FWIW, the difference between setting CPU_ACCESS_REQUIRED and not setting
>> it for a BO created in VRAM will be: If it's set, the BO is initially
>> created in CPU visible VRAM, otherwise it's most likely created in CPU
>> invisible VRAM.
>>
>> If userspace knows that a BO will likely be accessed by the CPU first,
>> setting the flag could save a move from CPU invisible to CPU visible
>> VRAM when the CPU access happens. Conversely, if a BO will likely never
>> be accessed by the CPU, not setting the flag may reduce pressure on CPU
>> visible VRAM.
>>
>> Not sure radeonsi can make this distinction though.
> 
> It can't.

Not sure why, but anyway.


> Either all mappable BOs set CPU_ACCESS_REQUIRED, or all mappable BOs
> don't set it.

FWIW, with kernels without John's fixes, the flag shouldn't be set for
all such BOs, because it would prevent them from ever going to CPU
invisible VRAM.

With John's fixes, one possibility would be to set the flag based on the
existing heuristics or some variant, and use it when a BO is newly
created from the kernel, but to ignore the flag when reusing a BO from
the cache (with John's fixes, whether or not the flag was set when a BO
was created won't directly affect its state WRT the flag when it's
reused from the cache).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED

[Marek Olšák]

On Tue, Jul 4, 2017 at 10:09 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 03/07/17 10:03 PM, Marek Olšák wrote:
>> On Mon, Jul 3, 2017 at 12:08 PM, Michel Dänzer <michel@daenzer.net> wrote:
>>> On 30/06/17 08:43 PM, Marek Olšák wrote:
>>>>
>>>> I don't know what is being talked about here anymore, but I wouldn't
>>>> like to use CPU_ACCESS_REQUIRED or CPU_ACCESS_REALLY_REQUIRED in
>>>> userspace. The reason is that userspace doesn't and can't know whether
>>>> CPU access will be required, and the frequency at which it will be
>>>> required. 3 heaps {no CPU access, no flag, CPU access required} are
>>>> too many. Userspace mostly doesn't use the "no flag" heap for VRAM. It
>>>> uses "CPU access required" for almost everything except tiled
>>>> textures, which use "no CPU access".
>>>
>>> FWIW, the difference between setting CPU_ACCESS_REQUIRED and not setting
>>> it for a BO created in VRAM will be: If it's set, the BO is initially
>>> created in CPU visible VRAM, otherwise it's most likely created in CPU
>>> invisible VRAM.
>>>
>>> If userspace knows that a BO will likely be accessed by the CPU first,
>>> setting the flag could save a move from CPU invisible to CPU visible
>>> VRAM when the CPU access happens. Conversely, if a BO will likely never
>>> be accessed by the CPU, not setting the flag may reduce pressure on CPU
>>> visible VRAM.
>>>
>>> Not sure radeonsi can make this distinction though.
>>
>> It can't.
>
> Not sure why, but anyway.
>
>
>> Either all mappable BOs set CPU_ACCESS_REQUIRED, or all mappable BOs
>> don't set it.
>
> FWIW, with kernels without John's fixes, the flag shouldn't be set for
> all such BOs, because it would prevent them from ever going to CPU
> invisible VRAM.
>
> With John's fixes, one possibility would be to set the flag based on the
> existing heuristics or some variant, and use it when a BO is newly
> created from the kernel, but to ignore the flag when reusing a BO from
> the cache (with John's fixes, whether or not the flag was set when a BO
> was created won't directly affect its state WRT the flag when it's
> reused from the cache).

Thanks. Based on your comments, I've come to the conclusion that the
userspace shouldn't set CPU_ACCESS_REQUIRED, because the only other
alternative (always setting the flag) is worse.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx