Re: [PATCH v2 4/4] drm/i915/uapi: document behaviour for DG2 64K support

2022-01-19 Thread Jordan Justen
Robert Beckett  writes:

> From: Matthew Auld 
>
> On discrete platforms like DG2, we need to support a minimum page size
> of 64K when dealing with device local-memory. This is quite tricky for
> various reasons, so try to document the new implicit uapi for this.
>
> v2: Fixed suggestions on formatting [Daniel]
>
> Signed-off-by: Matthew Auld 
> Signed-off-by: Ramalingam C 
> Signed-off-by: Robert Beckett 
> cc: Simon Ser 
> cc: Pekka Paalanen 
> Cc: Jordan Justen 
> Cc: Kenneth Graunke 
> Cc: mesa-dev@lists.freedesktop.org
> Cc: Tony Ye 
> Cc: Slawomir Milczarek 
> ---
>  include/uapi/drm/i915_drm.h | 44 -
>  1 file changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5e678917da70..486b7b96291e 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 {
>   /**
>* When the EXEC_OBJECT_PINNED flag is specified this is populated by
>* the user with the GTT offset at which this object will be pinned.
> +  *
>* When the I915_EXEC_NO_RELOC flag is specified this must contain the
>* presumed_offset of the object.
> +  *
>* During execbuffer2 the kernel populates it with the value of the
>* current GTT offset of the object, for future presumed_offset writes.
> +  *
> +  * See struct drm_i915_gem_create_ext for the rules when dealing with
> +  * alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with
> +  * minimum page sizes, like DG2.
>*/
>   __u64 offset;
>  
> @@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext {
>*
>* The (page-aligned) allocated size for the object will be returned.
>*
> -  * Note that for some devices we have might have further minimum
> -  * page-size restrictions(larger than 4K), like for device local-memory.
> -  * However in general the final size here should always reflect any
> -  * rounding up, if for example using the 
> I915_GEM_CREATE_EXT_MEMORY_REGIONS
> -  * extension to place the object in device local-memory.
> +  *
> +  * **DG2 64K min page size implications:**

Long term, I'm not sure that the "**" (for emphasis) is needed here or
below. It's interesting at the moment, but will be just another thing
baked into the kernel/user code in a month from now. :)

> +  *
> +  * On discrete platforms, starting from DG2, we have to contend with GTT
> +  * page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE
> +  * objects.  Specifically the hardware only supports 64K or larger GTT
> +  * page sizes for such memory. The kernel will already ensure that all
> +  * I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page
> +  * sizes underneath.
> +  *
> +  * Note that the returned size here will always reflect any required
> +  * rounding up done by the kernel, i.e 4K will now become 64K on devices
> +  * such as DG2.
> +  *
> +  * **Special DG2 GTT address alignment requirement:**
> +  *
> +  * The GTT alignment will also need be at least 2M for  such objects.
> +  *
> +  * Note that due to how the hardware implements 64K GTT page support, we
> +  * have some further complications:
> +  *
> +  *   1) The entire PDE(which covers a 2MB virtual address range), must
> +  *   contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same
> +  *   PDE is forbidden by the hardware.
> +  *
> +  *   2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM
> +  *   objects.
> +  *
> +  * To keep things simple for userland, we mandate that any GTT mappings
> +  * must be aligned to and rounded up to 2MB. As this only wastes virtual
> +  * address space and avoids userland having to copy any needlessly
> +  * complicated PDE sharing scheme (coloring) and only affects GD2, this
> +  * id deemed to be a good compromise.

typos: GD2, id

Isn't much of this more relavent to the vma offset at exec time? Is
there actually any new restriction on the size field during buffer
creation?

I see Matthew references these notes from the offset comments, so if the
kernel devs prefer it here, then you can add my Acked-by on this patch.

-Jordan

>*/
>   __u64 size;
>   /**
> -- 
> 2.25.1


Re: [PATCH v2 4/4] drm/i915/uapi: document behaviour for DG2 64K support

2022-01-19 Thread Robert Beckett




On 19/01/2022 18:36, Jordan Justen wrote:

Robert Beckett  writes:


From: Matthew Auld 

On discrete platforms like DG2, we need to support a minimum page size
of 64K when dealing with device local-memory. This is quite tricky for
various reasons, so try to document the new implicit uapi for this.

v2: Fixed suggestions on formatting [Daniel]

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
cc: Simon Ser 
cc: Pekka Paalanen 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: mesa-dev@lists.freedesktop.org
Cc: Tony Ye 
Cc: Slawomir Milczarek 
---
  include/uapi/drm/i915_drm.h | 44 -
  1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5e678917da70..486b7b96291e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 {
/**
 * When the EXEC_OBJECT_PINNED flag is specified this is populated by
 * the user with the GTT offset at which this object will be pinned.
+*
 * When the I915_EXEC_NO_RELOC flag is specified this must contain the
 * presumed_offset of the object.
+*
 * During execbuffer2 the kernel populates it with the value of the
 * current GTT offset of the object, for future presumed_offset writes.
+*
+* See struct drm_i915_gem_create_ext for the rules when dealing with
+* alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with
+* minimum page sizes, like DG2.
 */
__u64 offset;
  
@@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext {

 *
 * The (page-aligned) allocated size for the object will be returned.
 *
-* Note that for some devices we have might have further minimum
-* page-size restrictions(larger than 4K), like for device local-memory.
-* However in general the final size here should always reflect any
-* rounding up, if for example using the 
I915_GEM_CREATE_EXT_MEMORY_REGIONS
-* extension to place the object in device local-memory.
+*
+* **DG2 64K min page size implications:**


Long term, I'm not sure that the "**" (for emphasis) is needed here or
below. It's interesting at the moment, but will be just another thing
baked into the kernel/user code in a month from now. :)


fair point, I'll make it less shouty




+*
+* On discrete platforms, starting from DG2, we have to contend with GTT
+* page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE
+* objects.  Specifically the hardware only supports 64K or larger GTT
+* page sizes for such memory. The kernel will already ensure that all
+* I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page
+* sizes underneath.
+*
+* Note that the returned size here will always reflect any required
+* rounding up done by the kernel, i.e 4K will now become 64K on devices
+* such as DG2.
+*
+* **Special DG2 GTT address alignment requirement:**
+*
+* The GTT alignment will also need be at least 2M for  such objects.
+*
+* Note that due to how the hardware implements 64K GTT page support, we
+* have some further complications:
+*
+*   1) The entire PDE(which covers a 2MB virtual address range), must
+*   contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same
+*   PDE is forbidden by the hardware.
+*
+*   2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM
+*   objects.
+*
+* To keep things simple for userland, we mandate that any GTT mappings
+* must be aligned to and rounded up to 2MB. As this only wastes virtual
+* address space and avoids userland having to copy any needlessly
+* complicated PDE sharing scheme (coloring) and only affects GD2, this
+* id deemed to be a good compromise.


typos: GD2, id


thanks



Isn't much of this more relavent to the vma offset at exec time? Is
there actually any new restriction on the size field during buffer
creation?


No new restriction on size, just placement, which mesa is already doing.
The request for ack was just to get an ack from mesa folks that they are 
happy with the mandatory 2MB alignment for DG2 vma.




I see Matthew references these notes from the offset comments, so if the
kernel devs prefer it here, then you can add my Acked-by on this patch.


thanks!



-Jordan


 */
__u64 size;
/**
--
2.25.1


Replacing NIR with SPIR-V?

2022-01-19 Thread Abel Bernabeu
Hi,

My name Abel Bernabeu and I currently chair the Graphics and ML Special
Interest Group within RISC-V.

As part of my work for RISC-V I am currently looking at what is needed for
supporting a graphics product that uses a (potentially extended) RISC-V ISA
for its shading cores. My initial focus has been on analyzing the
functional gap between RISC-V and SPIR-V, assuming that whatever is needed
for a modern graphics accelerator is inevitably present on SPIR-V.

Now, the thing is that most of the potential adopters on our committee will
likely be interested in using mesa for developing their drivers and that
means using NIR as intermediate representation. Thus, I also need to
consider NIR when looking at the functional gap, doubling the amount of
work during the analysis.

Why is mesa using NIR as intermediate representation rather than SPIR-V? It
would make my life easier if mesa used SPIR-V rather than NIR for
communicating the front-end and the backends.

I know it is a lot of work to migrate to SPIR-V, but I am interested in
knowing what is the opinion of the mesa developers:

- My understanding is that when mesa adopted NIR, there was no SPIR-V. Was
a comparison made after the SPIR-V ratification?

- Does it make sense to move to SPIR-V?

- Is it feasible in terms of functionality supported by SPIR-V?

- Is the cost worth the potential advantage of using a more commonly
adopted standard?

Thanks in advance for your time and thoughts.

Regards.


Re: Replacing NIR with SPIR-V?

2022-01-19 Thread Jason Ekstrand
> - Does it make sense to move to SPIR-V?

None whatsoever.  SPIR-V is an interchange format, not a set of
manipulatable data structures suitable for compiler lowering and
optimization.

You also don't want to build hardware around consuming SPIR-V.  There are
lots of things that the SPIR-V has which you wouldn't want to support
natively in hardware such as structures and arrays in SSA values or complex
trig ops like atan2().  Part of the purpose of NIR is to lower these things
to simpler constructs which are supported in native hardware.

--Jason

On Wed, Jan 19, 2022 at 7:17 PM Abel Bernabeu <
abel.berna...@esperantotech.com> wrote:

> Hi,
>
> My name Abel Bernabeu and I currently chair the Graphics and ML Special
> Interest Group within RISC-V.
>
> As part of my work for RISC-V I am currently looking at what is needed for
> supporting a graphics product that uses a (potentially extended) RISC-V ISA
> for its shading cores. My initial focus has been on analyzing the
> functional gap between RISC-V and SPIR-V, assuming that whatever is needed
> for a modern graphics accelerator is inevitably present on SPIR-V.
>
> Now, the thing is that most of the potential adopters on our committee
> will likely be interested in using mesa for developing their drivers and
> that means using NIR as intermediate representation. Thus, I also need to
> consider NIR when looking at the functional gap, doubling the amount of
> work during the analysis.
>
> Why is mesa using NIR as intermediate representation rather than SPIR-V?
> It would make my life easier if mesa used SPIR-V rather than NIR for
> communicating the front-end and the backends.
>
> I know it is a lot of work to migrate to SPIR-V, but I am interested in
> knowing what is the opinion of the mesa developers:
>
> - My understanding is that when mesa adopted NIR, there was no SPIR-V. Was
> a comparison made after the SPIR-V ratification?
>
> - Does it make sense to move to SPIR-V?
>
> - Is it feasible in terms of functionality supported by SPIR-V?
>
> - Is the cost worth the potential advantage of using a more commonly
> adopted standard?
>
> Thanks in advance for your time and thoughts.
>
> Regards.
>