Re: PRT support for amdgpu v3

[Christian König]

Hi Nicolai,

that one should be fixed by "drm/amdgpu: fix PRT cleanup order in the 
VM". Please test and/or review.


Thanks,
Christian.

Am 12.02.2017 um 12:36 schrieb Nicolai Hähnle:

Hi,

Some more testing uncovered a bug in cleanup paths. When the 
application segfaults while PRT mappings exist, I get a WARN_ON (which 
seems fairly straightforward) and occasionally also an RCU error 
warning -- see the attached dmesg logs.


Regular application shutdown works fine, though.

Cheers,
Nicolai

On 08.02.2017 16:04, Christian König wrote:

Hi guys,

ok I finally found time to write an unit test for this and hammered 
out the last few bugs.


Seems to work fine on my Tonga now. Please note that this set is 
based on "fix race in GEM VA map IOCTL v2", without that patch you 
will run into a NULL pointer dereference during PRT mapping.


Going to send out the unit test in a minute.

Regards,
Christian.

___
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: PRT support for amdgpu v3

[Nicolai Hähnle]

Hi,

Some more testing uncovered a bug in cleanup paths. When the application 
segfaults while PRT mappings exist, I get a WARN_ON (which seems fairly 
straightforward) and occasionally also an RCU error warning -- see the 
attached dmesg logs.


Regular application shutdown works fine, though.

Cheers,
Nicolai

On 08.02.2017 16:04, Christian König wrote:

Hi guys,

ok I finally found time to write an unit test for this and hammered out the 
last few bugs.

Seems to work fine on my Tonga now. Please note that this set is based on "fix race 
in GEM VA map IOCTL v2", without that patch you will run into a NULL pointer 
dereference during PRT mapping.

Going to send out the unit test in a minute.

Regards,
Christian.

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



Feb 12 12:07:46 cassiopeia kernel: [36172.512767] arb_sparse_buff[15881]: 
segfault at 10 ip 7f295fdda14a sp 7ffce6dd2860 error 4 in 
radeonsi_dri.so[7f295f57e000+b54000]
Feb 12 12:07:46 cassiopeia kernel: [36172.688430] [ cut here 
]
Feb 12 12:07:46 cassiopeia kernel: [36172.688689] WARNING: CPU: 3 PID: 15886 at 
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:1000 amdgpu_bo_gpu_offset+0xe0/0x1d0 
[amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.688692] Modules linked in: 
snd_usb_audio snd_usbmidi_lib btrfs xor raid6_pq binfmt_misc edac_mce_amd 
edac_core nls_iso8859_1 dm_crypt kvm_amd kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel input_leds joydev aesni_intel aes_x86_64 
glue_helper lrw gf128mul ablk_helper cryptd snd_hda_codec_realtek 
snd_hda_codec_generic serio_raw snd_hda_codec_hdmi fam15h_power snd_hda_intel 
snd_hda_codec k10temp snd_hda_core snd_hwdep snd_pcm i2c_piix4 snd_seq_midi 
snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore 
tpm_infineon eeepc_wmi asus_wmi video mac_hid sparse_keymap mxm_wmi shpchp wmi 
parport_pc ppdev lp parport autofs4 amdkfd amd_iommu_v2 amdgpu i2c_algo_bit 
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse ttm drm 
ahci r8169 libahci mii fjes
Feb 12 12:07:46 cassiopeia kernel: [36172.688896]  hid_generic usbhid hid
Feb 12 12:07:46 cassiopeia kernel: [36172.688915] CPU: 3 PID: 15886 Comm: 
si_shader:3 Tainted: GB   4.9.0-amd-staging-4.9-prt #133
Feb 12 12:07:46 cassiopeia kernel: [36172.688920] Hardware name: To be filled 
by O.E.M. To be filled by O.E.M./M5A97 LE R2.0, BIOS 2601 03/24/2015
Feb 12 12:07:46 cassiopeia kernel: [36172.688925]  8804d952f3e0 
a06b93a7  
Feb 12 12:07:46 cassiopeia kernel: [36172.688940]  8804d952f428 
a00d25b1 880560a06440 03e8
Feb 12 12:07:46 cassiopeia kernel: [36172.688953]  88065d4dd468 
88065d4dd51c 88065d4dcd28 1fff
Feb 12 12:07:46 cassiopeia kernel: [36172.688966] Call Trace:
Feb 12 12:07:46 cassiopeia kernel: [36172.688980]  [] 
dump_stack+0x86/0xcf
Feb 12 12:07:46 cassiopeia kernel: [36172.688991]  [] 
__warn+0x111/0x130
Feb 12 12:07:46 cassiopeia kernel: [36172.689002]  [] 
warn_slowpath_null+0x1d/0x20
Feb 12 12:07:46 cassiopeia kernel: [36172.689234]  [] 
amdgpu_bo_gpu_offset+0xe0/0x1d0 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.689474]  [] 
amdgpu_vm_update_ptes.isra.9+0x103/0x2c0 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.689484]  [] ? 
kmem_cache_alloc+0x195/0x270
Feb 12 12:07:46 cassiopeia kernel: [36172.689723]  [] 
amdgpu_vm_frag_ptes+0x121/0x140 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.689963]  [] 
amdgpu_vm_bo_split_mapping+0x7d6/0xa00 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.689974]  [] ? 
do_group_exit+0x98/0x160
Feb 12 12:07:46 cassiopeia kernel: [36172.690213]  [] ? 
amdgpu_vm_free_mapping.isra.10+0xb0/0xb0 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.690221]  [] ? 
get_signal+0x3a9/0xb90
Feb 12 12:07:46 cassiopeia kernel: [36172.690230]  [] ? 
trace_hardirqs_on_caller+0x16/0x280
Feb 12 12:07:46 cassiopeia kernel: [36172.690238]  [] ? 
trace_hardirqs_on+0xd/0x10
Feb 12 12:07:46 cassiopeia kernel: [36172.690487]  [] ? 
amdgpu_vm_do_copy_ptes+0x1c0/0x1c0 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.690496]  [] ? 
kfree+0xea/0x2a0
Feb 12 12:07:46 cassiopeia kernel: [36172.690736]  [] 
amdgpu_vm_clear_freed+0x11a/0x200 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.690976]  [] ? 
amdgpu_vm_bo_update+0x750/0x750 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.690986]  [] ? 
kasan_slab_free+0x89/0xc0
Feb 12 12:07:46 cassiopeia kernel: [36172.691225]  [] ? 
amdgpu_vm_fini+0x259/0x430 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.691464]  [] 
amdgpu_vm_fini+0x28c/0x430 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.691716]  [] ? 
amdgpu_vm_init+0x4d0/0x4d0 [amdgpu]
Feb 12 12:07:46 cassiopeia kernel: [36172.691975]  [] 
amdgpu_driver_postclose_kms+0x233/0x3b0 [amdgpu]
Feb 12 12:07:46 cassiopeia 

Re: PRT support for amdgpu v3

[Bas Nieuwenhuizen]

Tested on amd-staging-4.9 + these patches, and it works for me too.

Thanks,
Bas Nieuwenhuizen

On Wed, Feb 8, 2017, at 16:04, Christian König wrote:
> Hi guys,
> 
> ok I finally found time to write an unit test for this and hammered out
> the last few bugs.
> 
> Seems to work fine on my Tonga now. Please note that this set is based on
> "fix race in GEM VA map IOCTL v2", without that patch you will run into a
> NULL pointer dereference during PRT mapping.
> 
> Going to send out the unit test in a minute.
> 
> Regards,
> Christian.
> 
> ___
> 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: PRT support for amdgpu v3

[Christian König]

Am 09.02.2017 um 11:11 schrieb Nicolai Hähnle:

On 08.02.2017 16:04, Christian König wrote:

Hi guys,

ok I finally found time to write an unit test for this and hammered 
out the last few bugs.


Seems to work fine on my Tonga now. Please note that this set is 
based on "fix race in GEM VA map IOCTL v2", without that patch you 
will run into a NULL pointer dereference during PRT mapping.


I can confirm that it works with my Mesa series as well: I get the 
"Disabling VM faults" warning message, and indeed accessing outside 
the committed region does not cause VM faults.


Great! Anybody who wants to give me some RBs on the patches?

I would like to commit those to the internal branches.

Regards,
Christian.



Cheers,
Nicolai



Going to send out the unit test in a minute.

Regards,
Christian.

___
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: PRT support for amdgpu v3

[Nicolai Hähnle]

On 08.02.2017 16:04, Christian König wrote:

Hi guys,

ok I finally found time to write an unit test for this and hammered out the 
last few bugs.

Seems to work fine on my Tonga now. Please note that this set is based on "fix race 
in GEM VA map IOCTL v2", without that patch you will run into a NULL pointer 
dereference during PRT mapping.


I can confirm that it works with my Mesa series as well: I get the 
"Disabling VM faults" warning message, and indeed accessing outside the 
committed region does not cause VM faults.


Cheers,
Nicolai



Going to send out the unit test in a minute.

Regards,
Christian.

___
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: PRT support for amdgpu v2

[Bas Nieuwenhuizen]

What branch is this based on? It fails to apply for me on
drm-next-4.11-wip of Alex' git repo.

On Thu, Feb 2, 2017, at 11:25, Christian König wrote:
> Hi guys,
> 
> a bunch of bug fixes, but still completely untested since I'm on sick
> leave.
> 
> Bas maybe you could give it a try with radv.
> 
> Regards,
> Christian.
> 
> ___
> 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: PRT support for amdgpu

[Christian König]

Am 02.02.2017 um 10:33 schrieb Nicolai Hähnle:

On 02.02.2017 10:29, Bas Nieuwenhuizen wrote:

On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:

On 02.02.2017 02:49, Dave Airlie wrote:
I think we would require a fully open source user for this sort of 
thing,
there are way to many corner cases for us to fall down here, 
prematurely
pushing the API without a proven test suite running on it would be 
bad.


We'd want to get radeonsi or radv up and running and have a complete
run of the conformance suite to make sure the kernel API was sane,
design is good, proving the design is the hard bit.


I think we can start with just GL_ARB_sparse_buffer. That extension
isn't as useful in comparison, but it exercises all the memory
management bits without having to worry about texture layout
considerations, and doing that one first seems like a reasonable
development strategy anyway.


Yeah, I noticed that vulkan had a similar extension that can be done
pretty easily, trying to get that done before the weekend.


That would be cool. I thought of doing this for radeonsi, but I 
seriously doubt that I'll get to it any time soon.


Thanks, that would be great. I'm going to send out my latest bug fixes 
for the kernel side in a few minutes.






What would be a plan for upstreaming all this? In an earlier case (the
wait on multiple fences ioctl), AFAIU the problem for upstreaming was
that there was no open-source user.  However I then wrote a branch
(which is probably outdated by now ...), but would like to wait with
upstreaming it till I know which libdrm and kernel driver version to use
for the feature tests. As a result all three the parts (kernel, libdrm,
radv) haven't been upstreamed yet. How can we avoid having the same
problem with this feature?


Once all the parts are there, the sequence should be upstream kernel, 
upstream libdrm, upstream radv.


Maybe you can ping the corresponding threads or re-send the patches 
for review?


Alex did that internally just last week, but nobody volunteered to take 
care of pushing it.


Basically everybody is just to busy to look into it.

Regards,
Christian.



Nicolai



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

Re: PRT support for amdgpu

[Nicolai Hähnle]

On 02.02.2017 10:29, Bas Nieuwenhuizen wrote:

On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:

On 02.02.2017 02:49, Dave Airlie wrote:

I think we would require a fully open source user for this sort of thing,
there are way to many corner cases for us to fall down here, prematurely
pushing the API without a proven test suite running on it would be bad.

We'd want to get radeonsi or radv up and running and have a complete
run of the conformance suite to make sure the kernel API was sane,
design is good, proving the design is the hard bit.


I think we can start with just GL_ARB_sparse_buffer. That extension
isn't as useful in comparison, but it exercises all the memory
management bits without having to worry about texture layout
considerations, and doing that one first seems like a reasonable
development strategy anyway.


Yeah, I noticed that vulkan had a similar extension that can be done
pretty easily, trying to get that done before the weekend.


That would be cool. I thought of doing this for radeonsi, but I 
seriously doubt that I'll get to it any time soon.




What would be a plan for upstreaming all this? In an earlier case (the
wait on multiple fences ioctl), AFAIU the problem for upstreaming was
that there was no open-source user.  However I then wrote a branch
(which is probably outdated by now ...), but would like to wait with
upstreaming it till I know which libdrm and kernel driver version to use
for the feature tests. As a result all three the parts (kernel, libdrm,
radv) haven't been upstreamed yet. How can we avoid having the same
problem with this feature?


Once all the parts are there, the sequence should be upstream kernel, 
upstream libdrm, upstream radv.


Maybe you can ping the corresponding threads or re-send the patches for 
review?


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

Re: PRT support for amdgpu

[Bas Nieuwenhuizen]

On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:
> [ ceterum censeo, + John for addrlib :P ]
> 
> On 02.02.2017 02:49, Dave Airlie wrote:
> >>> answered better by Dave.
> >>
> >>
> >> Yeah, though so as well. Dave can you comment?
> >
> > I think we would require a fully open source user for this sort of thing,
> > there are way to many corner cases for us to fall down here, prematurely
> > pushing the API without a proven test suite running on it would be bad.
> >
> > We'd want to get radeonsi or radv up and running and have a complete
> > run of the conformance suite to make sure the kernel API was sane,
> > design is good, proving the design is the hard bit.
> 
> I think we can start with just GL_ARB_sparse_buffer. That extension 
> isn't as useful in comparison, but it exercises all the memory 
> management bits without having to worry about texture layout 
> considerations, and doing that one first seems like a reasonable 
> development strategy anyway.

Yeah, I noticed that vulkan had a similar extension that can be done
pretty easily, trying to get that done before the weekend.

What would be a plan for upstreaming all this? In an earlier case (the
wait on multiple fences ioctl), AFAIU the problem for upstreaming was
that there was no open-source user.  However I then wrote a branch
(which is probably outdated by now ...), but would like to wait with
upstreaming it till I know which libdrm and kernel driver version to use
for the feature tests. As a result all three the parts (kernel, libdrm,
radv) haven't been upstreamed yet. How can we avoid having the same
problem with this feature?

> 
> 
> > And uggh on addrlib, just stick the whole thing on github already.
> 
> I wish :/
> 
> Nicolai
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: PRT support for amdgpu

[Nicolai Hähnle]

[ ceterum censeo, + John for addrlib :P ]

On 02.02.2017 02:49, Dave Airlie wrote:

answered better by Dave.



Yeah, though so as well. Dave can you comment?


I think we would require a fully open source user for this sort of thing,
there are way to many corner cases for us to fall down here, prematurely
pushing the API without a proven test suite running on it would be bad.

We'd want to get radeonsi or radv up and running and have a complete
run of the conformance suite to make sure the kernel API was sane,
design is good, proving the design is the hard bit.


I think we can start with just GL_ARB_sparse_buffer. That extension 
isn't as useful in comparison, but it exercises all the memory 
management bits without having to worry about texture layout 
considerations, and doing that one first seems like a reasonable 
development strategy anyway.




And uggh on addrlib, just stick the whole thing on github already.


I wish :/

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

Re: PRT support for amdgpu

[Dave Airlie]

>> answered better by Dave.
>
>
> Yeah, though so as well. Dave can you comment?

I think we would require a fully open source user for this sort of thing,
there are way to many corner cases for us to fall down here, prematurely
pushing the API without a proven test suite running on it would be bad.

We'd want to get radeonsi or radv up and running and have a complete
run of the conformance suite to make sure the kernel API was sane,
design is good, proving the design is the hard bit.

And uggh on addrlib, just stick the whole thing on github already.

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

Re: PRT support for amdgpu

[Nicolai Hähnle]

On 31.01.2017 17:28, Christian König wrote:

Am 31.01.2017 um 14:06 schrieb Bas Nieuwenhuizen:

So this API seems usable, and I think this is something we can use for
radv. However, I'm not sure how much time it takes for us to implement,
as the TFE variants are not in LLVM yet and I haven't figured out what
values the NACKs get.


Actually this is also useful without the special NACK handling. E.g.
when you sample from a texture part which isn't present you always get
zero and writes are ignored.

The TFE bit and the extra signaling to for special handling in shader
code are only optional if I'm not completely mistaken.


I think it's needed for ARB_sparse_texture2 / whatever the Vulkan 
equivalent of that is. But yeah, I don't think ARB_sparse_texture needs 
TFE support in LLVM.




Furthermore, if addrlib is missing stuff like Nicolai suggests then that
could result in complications. I can try if I can get something working
over the weekend, but no promises.


Not sure what concern Nicolai has about addrlib here. In general we
should know where the different parts of a texture start (LODs, layers
etc...) and as far as I can see that's all you need to know.


Well, you're right that it's probably possible to work with the open 
addrlib already.


In particular, you need address info not just for layers and levels, but 
also for blocks within each 2D image, to be able to compute 
VIRTUAL_PAGE_SIZE_X/Y, and to map them to the corresponding addresses.


There are AddrComputeSurfaceAddrFromCoord and 
AddrComputeSurfaceCoordFromAddr functions that can be tricked into 
providing the necessary info.


It may be more natural with some additional functions that aren't open yet.

Also, you might possibly want different tilings for sparse textures, but 
I don't think that's really needed for an initial implementation.


Cheers,
Nicolai



As far as an unit test being sufficient, I assume you mean as open
source user for inclusion into the kernel?


Yes.


I think that'd be a question
answered better by Dave.


Yeah, though so as well. Dave can you comment?

Thanks for the comments,
Christian.




Best regards,
Christian.

___
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



___
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: PRT support for amdgpu

[Christian König]

Am 31.01.2017 um 17:09 schrieb Alex Deucher:

On Mon, Jan 30, 2017 at 7:57 AM, Christian König
 wrote:

Hi Dave and Bas,

Hi Dave and Bas,

the following set of patches is a proposal for adding support for partial 
resident textures (PRT) to the amdgpu kernel module.

The basic idea behind PRT support is that you turn of VM fault reporting and 
only map parts of a texture into your virtual address space.

When a shader now tries to sample from a not present page it gets a 
notification instead of a VM fault and can react gracefully by switch to a 
different LOD for example.

Do we actually need to disable faults?  I guess the shader hw probably
requires it to get the proper result in the shader?


Yes, you need to disable the VM faults for the PRT range otherwise the 
interrupt ring will just be flooded with faults.


Keep in mind that faults (or rather NACKs from the MC) are considered 
normal with that approach. This is also the reason why we need to change 
the TLB rules and also keep invalid entries in there.


Regards,
Christian.



Alex


On our current available hardware generation you can unfortunately only turn of 
VM faults globally, but on future generation you can do this on a per page 
basis. So my proposal is to have a consistent interface over all generations 
with a per mapping PRT flag, but enable/disable it globally on current hardware 
when the first/last mapping is made/destroyed.

An open problem with the proposal is that we don't know when or if we want to 
add the userspace implementation into radeonsi.

So price question could you guys use this for radv as well? Or is it sufficient 
to just write an unit test for it?

Best regards,
Christian.

___
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



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

Re: PRT support for amdgpu

[Christian König]

Am 31.01.2017 um 14:06 schrieb Bas Nieuwenhuizen:


On Mon, Jan 30, 2017, at 13:57, Christian König wrote:

Hi Dave and Bas,

Hi Dave and Bas,

the following set of patches is a proposal for adding support for partial
resident textures (PRT) to the amdgpu kernel module.

The basic idea behind PRT support is that you turn of VM fault reporting
and only map parts of a texture into your virtual address space.

If we add some backing to a range, do we need to unmap the PRT range,
split and map two PRt ranges? Or will this be handled like mmap and a
new map just overrides the earlier maps for that range?


Currently the idea is to unmap the PRT range first and then map the new 
stuff. But I've already discussed internally with Nicolai a couple of 
alternatives.


The problem is that IOCTL are supposed to be transactional, e.g. they 
either fail completely or they success completely. But that is rather 
tricky when you need to split mappings like you suggested as well.


So at least for the initial implementation I would like to stick to 
manual unmap calls we can still add the ability to split mappings later 
on if we find performance problems with that approach



When a shader now tries to sample from a not present page it gets a
notification instead of a VM fault and can react gracefully by switch to
a different LOD for example.

So to confirm this is just using texture instruction with the TFE bit
enabled, no trap handlers or such?


I'm not so deeply into the shader instructions, but I think so yes.


On our current available hardware generation you can unfortunately only
turn of VM faults globally, but on future generation you can do this on a
per page basis. So my proposal is to have a consistent interface over all
generations with a per mapping PRT flag, but enable/disable it globally
on current hardware when the first/last mapping is made/destroyed.

An open problem with the proposal is that we don't know when or if we
want to add the userspace implementation into radeonsi.

So price question could you guys use this for radv as well? Or is it
sufficient to just write an unit test for it?

So this API seems usable, and I think this is something we can use for
radv. However, I'm not sure how much time it takes for us to implement,
as the TFE variants are not in LLVM yet and I haven't figured out what
values the NACKs get.


Actually this is also useful without the special NACK handling. E.g. 
when you sample from a texture part which isn't present you always get 
zero and writes are ignored.


The TFE bit and the extra signaling to for special handling in shader 
code are only optional if I'm not completely mistaken.



Furthermore, if addrlib is missing stuff like Nicolai suggests then that
could result in complications. I can try if I can get something working
over the weekend, but no promises.


Not sure what concern Nicolai has about addrlib here. In general we 
should know where the different parts of a texture start (LODs, layers 
etc...) and as far as I can see that's all you need to know.



As far as an unit test being sufficient, I assume you mean as open
source user for inclusion into the kernel?


Yes.


I think that'd be a question
answered better by Dave.


Yeah, though so as well. Dave can you comment?

Thanks for the comments,
Christian.




Best regards,
Christian.

___
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



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

Re: PRT support for amdgpu

[Alex Deucher]

On Mon, Jan 30, 2017 at 7:57 AM, Christian König
 wrote:
> Hi Dave and Bas,
>
> Hi Dave and Bas,
>
> the following set of patches is a proposal for adding support for partial 
> resident textures (PRT) to the amdgpu kernel module.
>
> The basic idea behind PRT support is that you turn of VM fault reporting and 
> only map parts of a texture into your virtual address space.
>
> When a shader now tries to sample from a not present page it gets a 
> notification instead of a VM fault and can react gracefully by switch to a 
> different LOD for example.

Do we actually need to disable faults?  I guess the shader hw probably
requires it to get the proper result in the shader?

Alex

>
> On our current available hardware generation you can unfortunately only turn 
> of VM faults globally, but on future generation you can do this on a per page 
> basis. So my proposal is to have a consistent interface over all generations 
> with a per mapping PRT flag, but enable/disable it globally on current 
> hardware when the first/last mapping is made/destroyed.
>
> An open problem with the proposal is that we don't know when or if we want to 
> add the userspace implementation into radeonsi.
>
> So price question could you guys use this for radv as well? Or is it 
> sufficient to just write an unit test for it?
>
> Best regards,
> Christian.
>
> ___
> 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: PRT support for amdgpu

[Bas Nieuwenhuizen]

On Mon, Jan 30, 2017, at 13:57, Christian König wrote:
> Hi Dave and Bas,
> 
> Hi Dave and Bas,
> 
> the following set of patches is a proposal for adding support for partial
> resident textures (PRT) to the amdgpu kernel module.
> 
> The basic idea behind PRT support is that you turn of VM fault reporting
> and only map parts of a texture into your virtual address space.

If we add some backing to a range, do we need to unmap the PRT range,
split and map two PRt ranges? Or will this be handled like mmap and a
new map just overrides the earlier maps for that range?

> 
> When a shader now tries to sample from a not present page it gets a
> notification instead of a VM fault and can react gracefully by switch to
> a different LOD for example.

So to confirm this is just using texture instruction with the TFE bit
enabled, no trap handlers or such?

> 
> On our current available hardware generation you can unfortunately only
> turn of VM faults globally, but on future generation you can do this on a
> per page basis. So my proposal is to have a consistent interface over all
> generations with a per mapping PRT flag, but enable/disable it globally
> on current hardware when the first/last mapping is made/destroyed.
> 
> An open problem with the proposal is that we don't know when or if we
> want to add the userspace implementation into radeonsi.
> 
> So price question could you guys use this for radv as well? Or is it
> sufficient to just write an unit test for it?

So this API seems usable, and I think this is something we can use for
radv. However, I'm not sure how much time it takes for us to implement,
as the TFE variants are not in LLVM yet and I haven't figured out what
values the NACKs get. 

Furthermore, if addrlib is missing stuff like Nicolai suggests then that
could result in complications. I can try if I can get something working
over the weekend, but no promises.

As far as an unit test being sufficient, I assume you mean as open
source user for inclusion into the kernel? I think that'd be a question
answered better by Dave.

> 
> Best regards,
> Christian.
> 
> ___
> 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: PRT support for amdgpu

[Nicolai Hähnle]

[ Cc John for addrlib ]

On 30.01.2017 13:57, Christian König wrote:

An open problem with the proposal is that we don't know when or if we want to 
add the userspace implementation into radeonsi.

So price question could you guys use this for radv as well? Or is it sufficient 
to just write an unit test for it?


For radeonsi, I think the question is more "when", not "if". At least 
PRT/sparse textures are necessarily immutable, so the associated Mesa 
headache should be bounded, but still...


As you know, I think the ioctl interface is good.

One thing to keep in mind is that I don't know how well PRT layout is 
supported by the open addrlib. There have been a bunch of PRT-related 
fixes in the internal copy of addrlib, but opening them has been stuck 
in review limbo for a while now...


Cheers,
Nicolai



Best regards,
Christian.

___
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