[Intel-xe] [PATCH 1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Intel-xe] [PATCH 1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Rodrigo Vivi]

Also the uapi should be reviewed and scrutinized before xe
is accepted upstream and we shouldn't cause regression.

Link: https://lore.kernel.org/all/20230630100059.122881-1-thomas.hellstrom@linux.intel.com
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/gpu/rfc/xe.rst | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
index 2516fe141db6..3d2181bf3dad 100644
--- a/Documentation/gpu/rfc/xe.rst
+++ b/Documentation/gpu/rfc/xe.rst
@@ -67,12 +67,6 @@ platforms.
 
 When the time comes for Xe, the protection will be lifted on Xe and kept in i915.
 
-Xe driver will be protected with both STAGING Kconfig and force_probe. Changes in
-the uAPI are expected while the driver is behind these protections. STAGING will
-be removed when the driver uAPI gets to a mature state where we can guarantee the
-‘no regression’ rule. Then force_probe will be lifted only for future platforms
-that will be productized with Xe driver, but not with i915.
-
 Xe – Pre-Merge Goals
 ====================
 
-- 
2.41.0

[Intel-xe] [PATCH 2/4] drm/doc/rfc: Mark Dev_coredump as completed.

[Rodrigo Vivi]

Xe is already using devcoredump infrastructure as the primary
error state and all the changes needed for user space error
replay and other useful logs are getting added into xe_devcoredump.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_devcoredump.c
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/gpu/rfc/xe.rst | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
index 3d2181bf3dad..bf60c5c82d0e 100644
--- a/Documentation/gpu/rfc/xe.rst
+++ b/Documentation/gpu/rfc/xe.rst
@@ -67,8 +67,8 @@ platforms.
 
 When the time comes for Xe, the protection will be lifted on Xe and kept in i915.
 
-Xe – Pre-Merge Goals
-====================
+Xe – Pre-Merge Goals - Work-in-Progress
+=======================================
 
 Drm_scheduler
 -------------
@@ -206,6 +206,14 @@ This item ties into the GPUVA, VM_BIND, and even long-running compute support.
 As a key measurable result, we need to have a community consensus documented in
 this document and the Xe driver prepared for the changes, if necessary.
 
+Xe – uAPI high level overview
+=============================
+
+...Warning: To be done in follow up patches after/when/where the main consensus in various items are individually reached.
+
+Xe – Pre-Merge Goals - Completed
+================================
+
 Dev_coredump
 ------------
 
@@ -222,8 +230,3 @@ Later, when we are in-tree, the goal is to collaborate with devcoredump
 infrastructure with overall possible improvements, like multiple file support
 for better organization of the dumps, snapshot support, dmesg extra print,
 and whatever may make sense and help the overall infrastructure.
-
-Xe – uAPI high level overview
-=============================
-
-...Warning: To be done in follow up patches after/when/where the main consensus in various items are individually reached.
-- 
2.41.0

[Intel-xe] [PATCH 3/4] drm/doc/rfc: Mark DRM_VM_BIND as complete.

[Rodrigo Vivi]

The consensus is for individual drivers VM_BIND uapis with
the GPUVA helpers that are already implemented and merged
upstream.

The merged GPUVA documentation also establish some overall
rules for the locking to be followed by the drivers.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/gpu/rfc/xe.rst | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
index bf60c5c82d0e..a115526c03e0 100644
--- a/Documentation/gpu/rfc/xe.rst
+++ b/Documentation/gpu/rfc/xe.rst
@@ -106,23 +106,6 @@ our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
 related patch should be independent and present on dri-devel or acked by
 maintainers to go along with the first Xe pull request towards drm-next.
 
-DRM_VM_BIND
------------
-Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
-fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
-development of a common new drm_infrastructure. However, the Xe team needs to
-engage with the community to explore the options of a common API.
-
-As a key measurable result, the DRM_VM_BIND needs to be documented in this file
-below, or this entire block deleted if the consensus is for independent drivers
-vm_bind ioctls.
-
-Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
-Xe merged, it is mandatory to enforce the overall locking scheme for all major
-structs and list (so vm and vma). So, a consensus is needed, and possibly some
-common helpers. If helpers are needed, they should be also documented in this
-document.
-
 ASYNC VM_BIND
 -------------
 Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
@@ -230,3 +213,20 @@ Later, when we are in-tree, the goal is to collaborate with devcoredump
 infrastructure with overall possible improvements, like multiple file support
 for better organization of the dumps, snapshot support, dmesg extra print,
 and whatever may make sense and help the overall infrastructure.
+
+DRM_VM_BIND
+-----------
+Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
+fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
+development of a common new drm_infrastructure. However, the Xe team needs to
+engage with the community to explore the options of a common API.
+
+As a key measurable result, the DRM_VM_BIND needs to be documented in this file
+below, or this entire block deleted if the consensus is for independent drivers
+vm_bind ioctls.
+
+Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
+Xe merged, it is mandatory to enforce the overall locking scheme for all major
+structs and list (so vm and vma). So, a consensus is needed, and possibly some
+common helpers. If helpers are needed, they should be also documented in this
+document.
-- 
2.41.0

[Intel-xe] [PATCH 4/4] drm/doc/rfc: Mark GPU VA as complete.

[Rodrigo Vivi]

Nouveau has landed the GPU VA helpers, support and documentation
already and Xe is already using the upstream GPU VA.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 Documentation/gpu/rfc/xe.rst | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
index a115526c03e0..b67f8e6a1825 100644
--- a/Documentation/gpu/rfc/xe.rst
+++ b/Documentation/gpu/rfc/xe.rst
@@ -88,24 +88,6 @@ depend on any other patch touching drm_scheduler itself that was not yet merged
 through drm-misc. This, by itself, already includes the reach of an agreement for
 uniform 1 to 1 relationship implementation / usage across drivers.
 
-GPU VA
-------
-Two main goals of Xe are meeting together here:
-
-1) Have an uAPI that aligns with modern UMD needs.
-
-2) Early upstream engagement.
-
-RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
-track of GPU virtual address mappings. This is still not merged upstream, but
-this aligns very well with our goals and with our VM_BIND. The engagement with
-upstream and the port of Xe towards GPUVA is already ongoing.
-
-As a key measurable result, Xe needs to be aligned with the GPU VA and working in
-our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
-related patch should be independent and present on dri-devel or acked by
-maintainers to go along with the first Xe pull request towards drm-next.
-
 ASYNC VM_BIND
 -------------
 Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
@@ -230,3 +212,21 @@ Xe merged, it is mandatory to enforce the overall locking scheme for all major
 structs and list (so vm and vma). So, a consensus is needed, and possibly some
 common helpers. If helpers are needed, they should be also documented in this
 document.
+
+GPU VA
+------
+Two main goals of Xe are meeting together here:
+
+1) Have an uAPI that aligns with modern UMD needs.
+
+2) Early upstream engagement.
+
+RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
+track of GPU virtual address mappings. This is still not merged upstream, but
+this aligns very well with our goals and with our VM_BIND. The engagement with
+upstream and the port of Xe towards GPUVA is already ongoing.
+
+As a key measurable result, Xe needs to be aligned with the GPU VA and working in
+our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
+related patch should be independent and present on dri-devel or acked by
+maintainers to go along with the first Xe pull request towards drm-next.
-- 
2.41.0

[Intel-xe] ✗ CI.Patch_applied: failure for series starting with [1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/doc/rfc: No STAGING out of drivers/staging.
URL   : https://patchwork.freedesktop.org/series/123028/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: d8c08057a drm/xe: Add patch version on guc firmware init
=== git am output follows ===
Applying: drm/doc/rfc: No STAGING out of drivers/staging.
Applying: drm/doc/rfc: Mark Dev_coredump as completed.
Applying: drm/doc/rfc: Mark DRM_VM_BIND as complete.
Applying: drm/doc/rfc: Mark GPU VA as complete.

Re: [Intel-xe] [PATCH 1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Lucas De Marchi]

On Tue, Aug 29, 2023 at 12:30:01PM -0400, Rodrigo Vivi wrote:
>Also the uapi should be reviewed and scrutinized before xe
>is accepted upstream and we shouldn't cause regression.
>
>Link: https://lore.kernel.org/all/20230630100059.122881-1-thomas.hellstrom@linux.intel.com
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Agreed. The STAGING config is something specifically for drivers/staging
as noted in the previous discussion. If other subsystems want to re-use
it, then there would be more to do to cause a taint.  It could also be
confusing to cause that taint for drivers outside drivers/staging.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> Documentation/gpu/rfc/xe.rst | 6 ------
> 1 file changed, 6 deletions(-)
>
>diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
>index 2516fe141db6..3d2181bf3dad 100644
>--- a/Documentation/gpu/rfc/xe.rst
>+++ b/Documentation/gpu/rfc/xe.rst
>@@ -67,12 +67,6 @@ platforms.
>
> When the time comes for Xe, the protection will be lifted on Xe and kept in i915.
>
>-Xe driver will be protected with both STAGING Kconfig and force_probe. Changes in
>-the uAPI are expected while the driver is behind these protections. STAGING will
>-be removed when the driver uAPI gets to a mature state where we can guarantee the
>-‘no regression’ rule. Then force_probe will be lifted only for future platforms
>-that will be productized with Xe driver, but not with i915.
>-
> Xe – Pre-Merge Goals
> ====================
>
>-- 
>2.41.0
>

Re: [Intel-xe] [PATCH 1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Daniel Vetter]

On Tue, Aug 29, 2023 at 12:30:01PM -0400, Rodrigo Vivi wrote:
> Also the uapi should be reviewed and scrutinized before xe
> is accepted upstream and we shouldn't cause regression.
> 
> Link: https://lore.kernel.org/all/20230630100059.122881-1-thomas.hellstrom@linux.intel.com
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On the series:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

But also, I really don't want to be the gatekeeper for "is xe ready for
merging", so at least for the last two patches I think an ack from Danilo
that there's indeed a rough consensus/plan is much more important than my
own. The first two don't need that, because there was no "build dri-devel
consensus" aspect to those.

And for the sched side I guess an ack from Christian or maybe some of the
other in-flight drivers (Lina or Boris?), with maybe an ack from Danilo
for the long running compute stuff (or whoever cares about that, I don't
think amd is working on extracting the amdkfd trickery, but maybe they
need the sched support when they add real compute to the render driver
too) is again much more important than me dropping an ack from the
sidelines.

Cheers, Sima

> ---
>  Documentation/gpu/rfc/xe.rst | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> index 2516fe141db6..3d2181bf3dad 100644
> --- a/Documentation/gpu/rfc/xe.rst
> +++ b/Documentation/gpu/rfc/xe.rst
> @@ -67,12 +67,6 @@ platforms.
>  
>  When the time comes for Xe, the protection will be lifted on Xe and kept in i915.
>  
> -Xe driver will be protected with both STAGING Kconfig and force_probe. Changes in
> -the uAPI are expected while the driver is behind these protections. STAGING will
> -be removed when the driver uAPI gets to a mature state where we can guarantee the
> -‘no regression’ rule. Then force_probe will be lifted only for future platforms
> -that will be productized with Xe driver, but not with i915.
> -
>  Xe – Pre-Merge Goals
>  ====================
>  
> -- 
> 2.41.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-xe] [PATCH 1/4] drm/doc/rfc: No STAGING out of drivers/staging.

[Rodrigo Vivi]

On Thu, Aug 31, 2023 at 10:31:07AM +0200, Daniel Vetter wrote:
> On Tue, Aug 29, 2023 at 12:30:01PM -0400, Rodrigo Vivi wrote:
> > Also the uapi should be reviewed and scrutinized before xe
> > is accepted upstream and we shouldn't cause regression.
> > 
> > Link: https://lore.kernel.org/all/20230630100059.122881-1-thomas.hellstrom@linux.intel.com
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> On the series:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thank you

> 
> But also, I really don't want to be the gatekeeper for "is xe ready for
> merging",

It makes sense. I'm also cc'ing the folks you mentioned here so they
are aware on why I would be pinging them asking for acks on the follow-up
patches.

Also I'd like to mention that we have already addressed almost all of the
critical items found by Oded, and we will soon open gitlab issues for
the items he agreed that will be nice to have.

Another front of getting Xe really ready is the uAPI review. We have
identified the items that we need to change before we are ready for our
first pull-request:
https://gitlab.freedesktop.org/drm/xe/kernel/-/issues?label_name=Fix-for-upstream

now back to this xe.rst updates:

> so at least for the last two patches I think an ack from Danilo
> that there's indeed a rough consensus/plan is much more important than my
> own. The first two don't need that, because there was no "build dri-devel
> consensus" aspect to those.

Agreed.

> 
> And for the sched side I guess an ack from Christian or maybe some of the
> other in-flight drivers (Lina or Boris?), with maybe an ack from Danilo
> for the long running compute stuff (or whoever cares about that, I don't
> think amd is working on extracting the amdkfd trickery, but maybe they
> need the sched support when they add real compute to the render driver
> too) is again much more important than me dropping an ack from the
> sidelines.

Yeap, that long-running patch is out of this series for now. Let's first
get these ones in so we start to mark little by little what is completed.

On that long-running one, I have chatted with Alex and the goal for the
amd compute is to really use the userspace queue for any kind of compute
and long-running.

And the only thing that I could spot on their code that could be similar
is the their eviction_fences vs our preempt_fences. But I don't see much
value on having another layer for that instead of using the dma_fences
directly.

But as I told, let's first get these 4 updates in and next I will send
only the long-running one cc'ing all the mentioned folks so we can agree
on a consensus around the long-running.

Thanks,
Rodrigo

> 
> Cheers, Sima
> 
> > ---
> >  Documentation/gpu/rfc/xe.rst | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> > index 2516fe141db6..3d2181bf3dad 100644
> > --- a/Documentation/gpu/rfc/xe.rst
> > +++ b/Documentation/gpu/rfc/xe.rst
> > @@ -67,12 +67,6 @@ platforms.
> >  
> >  When the time comes for Xe, the protection will be lifted on Xe and kept in i915.
> >  
> > -Xe driver will be protected with both STAGING Kconfig and force_probe. Changes in
> > -the uAPI are expected while the driver is behind these protections. STAGING will
> > -be removed when the driver uAPI gets to a mature state where we can guarantee the
> > -‘no regression’ rule. Then force_probe will be lifted only for future platforms
> > -that will be productized with Xe driver, but not with i915.
> > -
> >  Xe – Pre-Merge Goals
> >  ====================
> >  
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [Intel-xe] [PATCH 3/4] drm/doc/rfc: Mark DRM_VM_BIND as complete.

[Rodrigo Vivi]

On Tue, Aug 29, 2023 at 12:30:03PM -0400, Rodrigo Vivi wrote:
> The consensus is for individual drivers VM_BIND uapis with
> the GPUVA helpers that are already implemented and merged
> upstream.
> 
> The merged GPUVA documentation also establish some overall
> rules for the locking to be followed by the drivers.

Danilo, do you agree with this?
if nothing is missing on that front, could you please ack this patch?

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  Documentation/gpu/rfc/xe.rst | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> index bf60c5c82d0e..a115526c03e0 100644
> --- a/Documentation/gpu/rfc/xe.rst
> +++ b/Documentation/gpu/rfc/xe.rst
> @@ -106,23 +106,6 @@ our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
>  related patch should be independent and present on dri-devel or acked by
>  maintainers to go along with the first Xe pull request towards drm-next.
>  
> -DRM_VM_BIND
> ------------
> -Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
> -fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
> -development of a common new drm_infrastructure. However, the Xe team needs to
> -engage with the community to explore the options of a common API.
> -
> -As a key measurable result, the DRM_VM_BIND needs to be documented in this file
> -below, or this entire block deleted if the consensus is for independent drivers
> -vm_bind ioctls.
> -
> -Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> -Xe merged, it is mandatory to enforce the overall locking scheme for all major
> -structs and list (so vm and vma). So, a consensus is needed, and possibly some
> -common helpers. If helpers are needed, they should be also documented in this
> -document.
> -
>  ASYNC VM_BIND
>  -------------
>  Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> @@ -230,3 +213,20 @@ Later, when we are in-tree, the goal is to collaborate with devcoredump
>  infrastructure with overall possible improvements, like multiple file support
>  for better organization of the dumps, snapshot support, dmesg extra print,
>  and whatever may make sense and help the overall infrastructure.
> +
> +DRM_VM_BIND
> +-----------
> +Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
> +fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
> +development of a common new drm_infrastructure. However, the Xe team needs to
> +engage with the community to explore the options of a common API.
> +
> +As a key measurable result, the DRM_VM_BIND needs to be documented in this file
> +below, or this entire block deleted if the consensus is for independent drivers
> +vm_bind ioctls.
> +
> +Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> +Xe merged, it is mandatory to enforce the overall locking scheme for all major
> +structs and list (so vm and vma). So, a consensus is needed, and possibly some
> +common helpers. If helpers are needed, they should be also documented in this
> +document.
> -- 
> 2.41.0
> 

Re: [Intel-xe] [PATCH 4/4] drm/doc/rfc: Mark GPU VA as complete.

[Rodrigo Vivi]

On Tue, Aug 29, 2023 at 12:30:04PM -0400, Rodrigo Vivi wrote:
> Nouveau has landed the GPU VA helpers, support and documentation
> already and Xe is already using the upstream GPU VA.

Danilo, although this is more on the Xe side and I wouldn't ask you
to review our code entirely, I'd like to get your ack here as Daniel
recommended. Meaning that we are aligned there and not creating any
change on top of GPU VA. Xe is currently using GPU VA directly without
any customization.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commit/ea4ae69e66b2940107e74f240ecb9dae87bf1ff1
Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commits/drm-xe-next?ref_type=heads

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  Documentation/gpu/rfc/xe.rst | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> index a115526c03e0..b67f8e6a1825 100644
> --- a/Documentation/gpu/rfc/xe.rst
> +++ b/Documentation/gpu/rfc/xe.rst
> @@ -88,24 +88,6 @@ depend on any other patch touching drm_scheduler itself that was not yet merged
>  through drm-misc. This, by itself, already includes the reach of an agreement for
>  uniform 1 to 1 relationship implementation / usage across drivers.
>  
> -GPU VA
> -------
> -Two main goals of Xe are meeting together here:
> -
> -1) Have an uAPI that aligns with modern UMD needs.
> -
> -2) Early upstream engagement.
> -
> -RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
> -track of GPU virtual address mappings. This is still not merged upstream, but
> -this aligns very well with our goals and with our VM_BIND. The engagement with
> -upstream and the port of Xe towards GPUVA is already ongoing.
> -
> -As a key measurable result, Xe needs to be aligned with the GPU VA and working in
> -our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
> -related patch should be independent and present on dri-devel or acked by
> -maintainers to go along with the first Xe pull request towards drm-next.
> -
>  ASYNC VM_BIND
>  -------------
>  Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> @@ -230,3 +212,21 @@ Xe merged, it is mandatory to enforce the overall locking scheme for all major
>  structs and list (so vm and vma). So, a consensus is needed, and possibly some
>  common helpers. If helpers are needed, they should be also documented in this
>  document.
> +
> +GPU VA
> +------
> +Two main goals of Xe are meeting together here:
> +
> +1) Have an uAPI that aligns with modern UMD needs.
> +
> +2) Early upstream engagement.
> +
> +RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
> +track of GPU virtual address mappings. This is still not merged upstream, but
> +this aligns very well with our goals and with our VM_BIND. The engagement with
> +upstream and the port of Xe towards GPUVA is already ongoing.
> +
> +As a key measurable result, Xe needs to be aligned with the GPU VA and working in
> +our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
> +related patch should be independent and present on dri-devel or acked by
> +maintainers to go along with the first Xe pull request towards drm-next.
> -- 
> 2.41.0
> 

Re: [Intel-xe] [PATCH 3/4] drm/doc/rfc: Mark DRM_VM_BIND as complete.

[Danilo Krummrich]

Hi Rodrigo,

On 8/31/23 21:10, Rodrigo Vivi wrote:
> On Tue, Aug 29, 2023 at 12:30:03PM -0400, Rodrigo Vivi wrote:
>> The consensus is for individual drivers VM_BIND uapis with
>> the GPUVA helpers that are already implemented and merged
>> upstream.
>>
>> The merged GPUVA documentation also establish some overall
>> rules for the locking to be followed by the drivers.
> 
> Danilo, do you agree with this?
> if nothing is missing on that front, could you please ack this patch?

I agree with the above, hence:

Acked-by: Danilo Krummrich <dakr@redhat.com>

However, the documentation below seems to be more about a common DRM_VM_BIND IOCTL?
I guess your commit refers to the end where it talks about common VM_BIND helpers.

Otherwise the patch is moving the "DRM_VM_BIND" paragraph somewhere below the
"Dev_coredump"paragraph. Is there some kind of "Done-Section" I'm missing?

- Danilo

> 
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   Documentation/gpu/rfc/xe.rst | 34 +++++++++++++++++-----------------
>>   1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
>> index bf60c5c82d0e..a115526c03e0 100644
>> --- a/Documentation/gpu/rfc/xe.rst
>> +++ b/Documentation/gpu/rfc/xe.rst
>> @@ -106,23 +106,6 @@ our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
>>   related patch should be independent and present on dri-devel or acked by
>>   maintainers to go along with the first Xe pull request towards drm-next.
>>   
>> -DRM_VM_BIND
>> ------------
>> -Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
>> -fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
>> -development of a common new drm_infrastructure. However, the Xe team needs to
>> -engage with the community to explore the options of a common API.
>> -
>> -As a key measurable result, the DRM_VM_BIND needs to be documented in this file
>> -below, or this entire block deleted if the consensus is for independent drivers
>> -vm_bind ioctls.
>> -
>> -Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
>> -Xe merged, it is mandatory to enforce the overall locking scheme for all major
>> -structs and list (so vm and vma). So, a consensus is needed, and possibly some
>> -common helpers. If helpers are needed, they should be also documented in this
>> -document.
>> -
>>   ASYNC VM_BIND
>>   -------------
>>   Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
>> @@ -230,3 +213,20 @@ Later, when we are in-tree, the goal is to collaborate with devcoredump
>>   infrastructure with overall possible improvements, like multiple file support
>>   for better organization of the dumps, snapshot support, dmesg extra print,
>>   and whatever may make sense and help the overall infrastructure.
>> +
>> +DRM_VM_BIND
>> +-----------
>> +Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
>> +fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
>> +development of a common new drm_infrastructure. However, the Xe team needs to
>> +engage with the community to explore the options of a common API.
>> +
>> +As a key measurable result, the DRM_VM_BIND needs to be documented in this file
>> +below, or this entire block deleted if the consensus is for independent drivers
>> +vm_bind ioctls.
>> +
>> +Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
>> +Xe merged, it is mandatory to enforce the overall locking scheme for all major
>> +structs and list (so vm and vma). So, a consensus is needed, and possibly some
>> +common helpers. If helpers are needed, they should be also documented in this
>> +document.
>> -- 
>> 2.41.0
>>
> 

Re: [Intel-xe] [PATCH 4/4] drm/doc/rfc: Mark GPU VA as complete.

[Danilo Krummrich]

On 8/31/23 21:17, Rodrigo Vivi wrote:
> On Tue, Aug 29, 2023 at 12:30:04PM -0400, Rodrigo Vivi wrote:
>> Nouveau has landed the GPU VA helpers, support and documentation
>> already and Xe is already using the upstream GPU VA.
> 
> Danilo, although this is more on the Xe side and I wouldn't ask you
> to review our code entirely, I'd like to get your ack here as Daniel
> recommended. Meaning that we are aligned there and not creating any
> change on top of GPU VA. Xe is currently using GPU VA directly without
> any customization.
> 
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commit/ea4ae69e66b2940107e74f240ecb9dae87bf1ff1
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commits/drm-xe-next?ref_type=heads

Acked-by: Danilo Krummrich <dakr@redhat.com>

Just one note: If we end up to agree on [1] few more adjustments are needed.

Otherwise, same as the other commit, where is the paragraph going?

- Danilo

[1] https://lore.kernel.org/dri-devel/202308221050.kTj8uFMA-lkp@intel.com/T/#m7f3b5a7ff70723332adeea32671578cb95c62f7c

> 
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   Documentation/gpu/rfc/xe.rst | 36 ++++++++++++++++++------------------
>>   1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
>> index a115526c03e0..b67f8e6a1825 100644
>> --- a/Documentation/gpu/rfc/xe.rst
>> +++ b/Documentation/gpu/rfc/xe.rst
>> @@ -88,24 +88,6 @@ depend on any other patch touching drm_scheduler itself that was not yet merged
>>   through drm-misc. This, by itself, already includes the reach of an agreement for
>>   uniform 1 to 1 relationship implementation / usage across drivers.
>>   
>> -GPU VA
>> -------
>> -Two main goals of Xe are meeting together here:
>> -
>> -1) Have an uAPI that aligns with modern UMD needs.
>> -
>> -2) Early upstream engagement.
>> -
>> -RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
>> -track of GPU virtual address mappings. This is still not merged upstream, but
>> -this aligns very well with our goals and with our VM_BIND. The engagement with
>> -upstream and the port of Xe towards GPUVA is already ongoing.
>> -
>> -As a key measurable result, Xe needs to be aligned with the GPU VA and working in
>> -our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
>> -related patch should be independent and present on dri-devel or acked by
>> -maintainers to go along with the first Xe pull request towards drm-next.
>> -
>>   ASYNC VM_BIND
>>   -------------
>>   Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
>> @@ -230,3 +212,21 @@ Xe merged, it is mandatory to enforce the overall locking scheme for all major
>>   structs and list (so vm and vma). So, a consensus is needed, and possibly some
>>   common helpers. If helpers are needed, they should be also documented in this
>>   document.
>> +
>> +GPU VA
>> +------
>> +Two main goals of Xe are meeting together here:
>> +
>> +1) Have an uAPI that aligns with modern UMD needs.
>> +
>> +2) Early upstream engagement.
>> +
>> +RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
>> +track of GPU virtual address mappings. This is still not merged upstream, but
>> +this aligns very well with our goals and with our VM_BIND. The engagement with
>> +upstream and the port of Xe towards GPUVA is already ongoing.
>> +
>> +As a key measurable result, Xe needs to be aligned with the GPU VA and working in
>> +our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
>> +related patch should be independent and present on dri-devel or acked by
>> +maintainers to go along with the first Xe pull request towards drm-next.
>> -- 
>> 2.41.0
>>
> 

Re: [Intel-xe] [PATCH 3/4] drm/doc/rfc: Mark DRM_VM_BIND as complete.

[Rodrigo Vivi]

On Mon, Sep 04, 2023 at 11:32:30PM +0200, Danilo Krummrich wrote:
> Hi Rodrigo,
> 
> On 8/31/23 21:10, Rodrigo Vivi wrote:
> > On Tue, Aug 29, 2023 at 12:30:03PM -0400, Rodrigo Vivi wrote:
> > > The consensus is for individual drivers VM_BIND uapis with
> > > the GPUVA helpers that are already implemented and merged
> > > upstream.
> > > 
> > > The merged GPUVA documentation also establish some overall
> > > rules for the locking to be followed by the drivers.
> > 
> > Danilo, do you agree with this?
> > if nothing is missing on that front, could you please ack this patch?
> 
> I agree with the above, hence:
> 
> Acked-by: Danilo Krummrich <dakr@redhat.com>
> 
> However, the documentation below seems to be more about a common DRM_VM_BIND IOCTL?
> I guess your commit refers to the end where it talks about common VM_BIND helpers.

Yes, it was about a common vm_bind helpers. But if the consensus is to go with
the individual drivers vm_bind uapis with common gpu_va, this common drm_vm_bind
talk makes no sense anymore. So we can end the talks about it.

> 
> Otherwise the patch is moving the "DRM_VM_BIND" paragraph somewhere below the
> "Dev_coredump"paragraph. Is there some kind of "Done-Section" I'm missing?

Yes, it moves to a new
+Xe – Pre-Merge Goals - Completed
+================================

added on patch 2 with devcoredump:

https://lore.kernel.org/all/20230829163005.54067-2-rodrigo.vivi@intel.com/


> 
> - Danilo
> 
> > 
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >   Documentation/gpu/rfc/xe.rst | 34 +++++++++++++++++-----------------
> > >   1 file changed, 17 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> > > index bf60c5c82d0e..a115526c03e0 100644
> > > --- a/Documentation/gpu/rfc/xe.rst
> > > +++ b/Documentation/gpu/rfc/xe.rst
> > > @@ -106,23 +106,6 @@ our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
> > >   related patch should be independent and present on dri-devel or acked by
> > >   maintainers to go along with the first Xe pull request towards drm-next.
> > > -DRM_VM_BIND
> > > ------------
> > > -Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
> > > -fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
> > > -development of a common new drm_infrastructure. However, the Xe team needs to
> > > -engage with the community to explore the options of a common API.
> > > -
> > > -As a key measurable result, the DRM_VM_BIND needs to be documented in this file
> > > -below, or this entire block deleted if the consensus is for independent drivers
> > > -vm_bind ioctls.
> > > -
> > > -Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> > > -Xe merged, it is mandatory to enforce the overall locking scheme for all major
> > > -structs and list (so vm and vma). So, a consensus is needed, and possibly some
> > > -common helpers. If helpers are needed, they should be also documented in this
> > > -document.
> > > -
> > >   ASYNC VM_BIND
> > >   -------------
> > >   Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> > > @@ -230,3 +213,20 @@ Later, when we are in-tree, the goal is to collaborate with devcoredump
> > >   infrastructure with overall possible improvements, like multiple file support
> > >   for better organization of the dumps, snapshot support, dmesg extra print,
> > >   and whatever may make sense and help the overall infrastructure.
> > > +
> > > +DRM_VM_BIND
> > > +-----------
> > > +Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in order to
> > > +fulfill the needs of the modern uAPI. Xe merge should *not* be blocked on the
> > > +development of a common new drm_infrastructure. However, the Xe team needs to
> > > +engage with the community to explore the options of a common API.
> > > +
> > > +As a key measurable result, the DRM_VM_BIND needs to be documented in this file
> > > +below, or this entire block deleted if the consensus is for independent drivers
> > > +vm_bind ioctls.
> > > +
> > > +Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> > > +Xe merged, it is mandatory to enforce the overall locking scheme for all major
> > > +structs and list (so vm and vma). So, a consensus is needed, and possibly some
> > > +common helpers. If helpers are needed, they should be also documented in this
> > > +document.
> > > -- 
> > > 2.41.0
> > > 
> > 
> 

Re: [Intel-xe] [PATCH 4/4] drm/doc/rfc: Mark GPU VA as complete.

[Rodrigo Vivi]

On Mon, Sep 04, 2023 at 11:39:55PM +0200, Danilo Krummrich wrote:
> On 8/31/23 21:17, Rodrigo Vivi wrote:
> > On Tue, Aug 29, 2023 at 12:30:04PM -0400, Rodrigo Vivi wrote:
> > > Nouveau has landed the GPU VA helpers, support and documentation
> > > already and Xe is already using the upstream GPU VA.
> > 
> > Danilo, although this is more on the Xe side and I wouldn't ask you
> > to review our code entirely, I'd like to get your ack here as Daniel
> > recommended. Meaning that we are aligned there and not creating any
> > change on top of GPU VA. Xe is currently using GPU VA directly without
> > any customization.
> > 
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commit/ea4ae69e66b2940107e74f240ecb9dae87bf1ff1
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/commits/drm-xe-next?ref_type=heads
> 
> Acked-by: Danilo Krummrich <dakr@redhat.com>
> 
> Just one note: If we end up to agree on [1] few more adjustments are needed.

Thanks. I see Thomas is already engaged there and I believe it will be easier
to change that and align when we are merged. Since that appears to not
break any uapi.

> 
> Otherwise, same as the other commit, where is the paragraph going?

to the new
+Xe – Pre-Merge Goals - Completed
+================================
https://lore.kernel.org/all/20230829163005.54067-2-rodrigo.vivi@intel.com/

> 
> - Danilo
> 
> [1] https://lore.kernel.org/dri-devel/202308221050.kTj8uFMA-lkp@intel.com/T/#m7f3b5a7ff70723332adeea32671578cb95c62f7c
> 
> > 
> > > 
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >   Documentation/gpu/rfc/xe.rst | 36 ++++++++++++++++++------------------
> > >   1 file changed, 18 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> > > index a115526c03e0..b67f8e6a1825 100644
> > > --- a/Documentation/gpu/rfc/xe.rst
> > > +++ b/Documentation/gpu/rfc/xe.rst
> > > @@ -88,24 +88,6 @@ depend on any other patch touching drm_scheduler itself that was not yet merged
> > >   through drm-misc. This, by itself, already includes the reach of an agreement for
> > >   uniform 1 to 1 relationship implementation / usage across drivers.
> > > -GPU VA
> > > -------
> > > -Two main goals of Xe are meeting together here:
> > > -
> > > -1) Have an uAPI that aligns with modern UMD needs.
> > > -
> > > -2) Early upstream engagement.
> > > -
> > > -RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
> > > -track of GPU virtual address mappings. This is still not merged upstream, but
> > > -this aligns very well with our goals and with our VM_BIND. The engagement with
> > > -upstream and the port of Xe towards GPUVA is already ongoing.
> > > -
> > > -As a key measurable result, Xe needs to be aligned with the GPU VA and working in
> > > -our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
> > > -related patch should be independent and present on dri-devel or acked by
> > > -maintainers to go along with the first Xe pull request towards drm-next.
> > > -
> > >   ASYNC VM_BIND
> > >   -------------
> > >   Although having a common DRM level IOCTL for VM_BIND is not a requirement to get
> > > @@ -230,3 +212,21 @@ Xe merged, it is mandatory to enforce the overall locking scheme for all major
> > >   structs and list (so vm and vma). So, a consensus is needed, and possibly some
> > >   common helpers. If helpers are needed, they should be also documented in this
> > >   document.
> > > +
> > > +GPU VA
> > > +------
> > > +Two main goals of Xe are meeting together here:
> > > +
> > > +1) Have an uAPI that aligns with modern UMD needs.
> > > +
> > > +2) Early upstream engagement.
> > > +
> > > +RedHat engineers working on Nouveau proposed a new DRM feature to handle keeping
> > > +track of GPU virtual address mappings. This is still not merged upstream, but
> > > +this aligns very well with our goals and with our VM_BIND. The engagement with
> > > +upstream and the port of Xe towards GPUVA is already ongoing.
> > > +
> > > +As a key measurable result, Xe needs to be aligned with the GPU VA and working in
> > > +our tree. Missing Nouveau patches should *not* block Xe and any needed GPUVA
> > > +related patch should be independent and present on dri-devel or acked by
> > > +maintainers to go along with the first Xe pull request towards drm-next.
> > > -- 
> > > 2.41.0
> > > 
> > 
>