<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 13, 2019 at 5:24 PM Niranjan Vishwanathapura <<a href="mailto:niranjana.vishwanathapura@intel.com">niranjana.vishwanathapura@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Dec 13, 2019 at 04:58:42PM -0600, Jason Ekstrand wrote:<br>
><br>
> +/**<br>
> + * struct drm_i915_gem_vm_bind<br>
> + *<br>
> + * Bind an object in a vm's page table.<br>
><br>
> First off, this is something I've wanted for a while for Vulkan, it's just<br>
> never made its way high enough up the priority list. However, it's going<br>
> to have to come one way or another soon. I'm glad to see kernel API for<br>
> this being proposed.<br>
> I do, however, have a few high-level comments/questions about the API:<br>
> 1. In order to be useful for sparse memory support, the API has to go the<br>
> other way around so that it binds a VA range to a range within the BO. It<br>
> also needs to be able to handle overlapping where two different VA ranges<br>
> may map to the same underlying bytes in the BO. This likely means that<br>
> unbind needs to also take a VA range and only unbind that range.<br>
> 2. If this is going to be useful for managing GL's address space where we<br>
> have lots of BOs, we probably want it to take a list of ranges so we<br>
> aren't making one ioctl for each thing we want to bind.<br>
<br>
Hi Jason,<br>
<br>
Yah, some of these requirements came up.<br></blockquote><div> </div><div>Yes, I have raised them every single time an API like this has come across my e-mail inbox for years and they continue to get ignored. Why are we landing an API that we know isn't the API we want especially when it's pretty obvious roughly what the API we want is? It may be less time in the short term, but long-term it means two ioctls and two implementations in i915, IGT tests for both code paths, and code in all UMDs to call one or the other depending on what kernel you're running on, and we have to maintain all that code going forward forever. Sure, that's a price we pay today for a variety of things but that's because they all seemed like the right thing at the time. Landing the wrong API when we know it's the wrong API seems foolish.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
They are not being done here due to time and effort involved in defining<br>
those requirements, implementing and validating.<br></blockquote><div><br></div><div>For #1, yes, it would require more effort but for #2, it really doesn't take any extra effort to make it take an array...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
However, this ioctl can be extended in a backward compatible way to handle<br>
those requirements if required.<br>
<br>
> 3. Why are there no ways to synchronize this with anything? For binding,<br>
> this probably isn't really needed as long as the VA range you're binding<br>
> is empty. However, if you want to move bindings around or unbind<br>
> something, the only option is to block in userspace and then call<br>
> bind/unbind. This can be done but it means even more threads in the UMD<br>
> which is unpleasant. One could argue that that's more or less what the<br>
> kernel is going to have to do so we may as well do it in userspace. <br>
> However, I'm not 100% convinced that's true.<br>
> --Jason<br>
><br>
<br>
Yah, that is the thought.<br>
But as SVM feature evolves, I think we can consider handling some such cases<br>
if hadling those in driver does make whole lot sense. <br></blockquote><div><br></div><div>Sparse binding exists as a feature. It's been in D3D for some time and it's in Vulkan. We pretty much know what the requirements are. If you go look at how it's supposed to work in Vulkan, you have a binding queue and it waits on semaphores before [un]binding and signals semaphores after [un]binding. The biggest problem from an API (as opposed to implementation) POV with doing that in i915 is that we have too many synchronization primitives to choose from. :-(</div><div><br></div><div>--Jason<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Thanks,<br>
Niranjana<br>
<br>
><br>
> + */<br>
> +struct drm_i915_gem_vm_bind {<br>
> + /** VA start to bind **/<br>
> + __u64 start;<br>
> +<br>
> + /** Type of memory to [un]bind **/<br>
> + __u32 type;<br>
> +#define I915_GEM_VM_BIND_SVM_OBJ 0<br>
> +<br>
> + /** Object handle to [un]bind for I915_GEM_VM_BIND_SVM_OBJ type<br>
> **/<br>
> + __u32 handle;<br>
> +<br>
> + /** vm to [un]bind **/<br>
> + __u32 vm_id;<br>
> +<br>
> + /** Flags **/<br>
> + __u32 flags;<br>
> +#define I915_GEM_VM_BIND_UNBIND (1 << 0)<br>
> +#define I915_GEM_VM_BIND_READONLY (1 << 1)<br>
> +};<br>
> +<br>
> #if defined(__cplusplus)<br>
> }<br>
> #endif<br>
> --<br>
> 2.21.0.rc0.32.g243a4c7e27<br>
><br>
> _______________________________________________<br>
> Intel-gfx mailing list<br>
> <a href="mailto:Intel-gfx@lists.freedesktop.org" target="_blank">Intel-gfx@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
</blockquote></div></div>