Re: [Nouveau] [PATCH] drm/nouveau/pm: refactor deprecated strncpy

2023-09-14 Thread Kees Cook
On Thu, Sep 14, 2023 at 10:17:08PM +, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
> 
> We should prefer more robust and less ambiguous string interfaces.
> 
> A suitable replacement is `strscpy` [2] due to the fact that it guarantees
> NUL-termination on the destination buffer without unnecessarily NUL-padding.
> 
> Link: 
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
>  [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html 
> [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-harden...@vger.kernel.org
> Signed-off-by: Justin Stitt 

The "- 1" use in the original code is strong evidence for this being a
sane conversion. :)

Reviewed-by: Kees Cook 

-- 
Kees Cook


Re: [PATCH] drm/nouveau/core: refactor deprecated strncpy

2023-09-14 Thread Kees Cook
On Thu, Sep 14, 2023 at 09:40:49PM +, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
> 
> We should prefer more robust and less ambiguous string interfaces.
> 
> A suitable replacement is `strscpy` [2] due to the fact that it guarantees
> NUL-termination on the destination buffer without unnecessarily NUL-padding.
> 
> There is likely no bug in the current implementation due to the safeguard:
> | cname[sizeof(cname) - 1] = '\0';
> ... however we can provide simpler and easier to understand code using
> the newer (and recommended) `strscpy` api.
> 
> Link: 
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
>  [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html 
> [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-harden...@vger.kernel.org
> Signed-off-by: Justin Stitt 
> ---
>  drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c 
> b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> index 91fb494d4009..374212da9e95 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> @@ -79,8 +79,7 @@ nvkm_firmware_get(const struct nvkm_subdev *subdev, const 
> char *fwname, int ver,
>   int i;
>  
>   /* Convert device name to lowercase */
> - strncpy(cname, device->chip->name, sizeof(cname));
> - cname[sizeof(cname) - 1] = '\0';
> + strscpy(cname, device->chip->name, sizeof(cname));
>   i = strlen(cname);
>   while (i) {
>   --i;

Yup, consumed by strlen() and snprintf(). Looks like a standard
conversion. :)

Reviewed-by: Kees Cook 

-Kees

> 
> ---
> base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
> change-id: 
> 20230914-strncpy-drivers-gpu-drm-nouveau-nvkm-core-firmware-c-791223838b72
> 
> Best regards,
> --
> Justin Stitt 
> 

-- 
Kees Cook


Re: [PATCH] drm/nouveau/nvif: refactor deprecated strncpy

2023-09-14 Thread Kees Cook
On Thu, Sep 14, 2023 at 09:30:37PM +, Justin Stitt wrote:
> `strncpy` is deprecated and as such we should prefer more robust and
> less ambiguous string interfaces.
> 
> A suitable replacement is `strscpy_pad` due to the fact that it
> guarantees NUL-termination on the destination buffer whilst also
> maintaining the NUL-padding behavior that `strncpy` provides. I am not
> sure whether NUL-padding is strictly needed but I see in
> `nvif_object_ctor()` args is memcpy'd elsewhere so I figured we'd keep
> the same functionality.
> 
> Link: 
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
>  [1]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-harden...@vger.kernel.org
> Signed-off-by: Justin Stitt 
> ---
> Note: build-tested only.
> ---
>  drivers/gpu/drm/nouveau/nvif/client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvif/client.c 
> b/drivers/gpu/drm/nouveau/nvif/client.c
> index a3264a0e933a..3a27245f467f 100644
> --- a/drivers/gpu/drm/nouveau/nvif/client.c
> +++ b/drivers/gpu/drm/nouveau/nvif/client.c
> @@ -69,7 +69,7 @@ nvif_client_ctor(struct nvif_client *parent, const char 
> *name, u64 device,
>   } nop = {};
>   int ret;
>  
> - strncpy(args.name, name, sizeof(args.name));
> + strscpy_pad(args.name, name, sizeof(args.name));
>   ret = nvif_object_ctor(parent != client ? >object : NULL,
>  name ? name : "nvifClient", 0,
>  NVIF_CLASS_CLIENT, , sizeof(args),

Right, I see the memcpy too:

nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle,
 s32 oclass, void *data, u32 size, struct nvif_object *object)
{
...
memcpy(args->new.data, data, size);

However, "args" is already zeroed:

struct nvif_client_v0 args = { .device = device };

So there's no need for _pad(). However, I couldn't immediatley see
why args.name must be %NUL terminated. It's ultimately passed through
an ioctl:

return client->driver->ioctl(client->object.priv, data, size, hack);

But I did find it...

args is:

struct nvif_client_v0 {
...
char  name[32];
};

And "name" only ever comes through nvif_client_ctor(), which all end up
via here, using "cli":

struct nouveau_cli {
...
char name[32];
...


snprintf(cli->name, sizeof(cli->name), "%s", sname);
...
ret = nvif_client_ctor(>master.base, cli->name, device,
   >base);

So we'll always be %NUL terminated.

Therefore, yes, conversion looks good:

Reviewed-by: Kees Cook 

Thanks!

-Kees

> 
> ---
> base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
> change-id: 20230914-strncpy-drivers-gpu-drm-nouveau-nvif-client-c-82b023c36953
> 
> Best regards,
> --
> Justin Stitt 
> 

-- 
Kees Cook


[PATCH] drm/nouveau/pm: refactor deprecated strncpy

2023-09-14 Thread Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].

We should prefer more robust and less ambiguous string interfaces.

A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.

Link: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
 [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt 
---
 drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
index 8fe0444f761e..131db2645f84 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
@@ -462,7 +462,7 @@ nvkm_perfmon_mthd_query_domain(struct nvkm_perfmon *perfmon,
 
args->v0.id = di;
args->v0.signal_nr  = nvkm_perfdom_count_perfsig(dom);
-   strncpy(args->v0.name, dom->name, sizeof(args->v0.name) - 1);
+   strscpy(args->v0.name, dom->name, sizeof(args->v0.name));
 
/* Currently only global counters (PCOUNTER) are implemented
 * but this will be different for local counters (MP). */
@@ -513,8 +513,7 @@ nvkm_perfmon_mthd_query_signal(struct nvkm_perfmon *perfmon,
snprintf(args->v0.name, sizeof(args->v0.name),
 "/%s/%02x", dom->name, si);
} else {
-   strncpy(args->v0.name, sig->name,
-   sizeof(args->v0.name) - 1);
+   strscpy(args->v0.name, sig->name, 
sizeof(args->v0.name));
}
 
args->v0.signal = si;
@@ -572,7 +571,7 @@ nvkm_perfmon_mthd_query_source(struct nvkm_perfmon *perfmon,
 
args->v0.source = sig->source[si];
args->v0.mask   = src->mask;
-   strncpy(args->v0.name, src->name, sizeof(args->v0.name) - 1);
+   strscpy(args->v0.name, src->name, sizeof(args->v0.name));
}
 
if (++si < source_nr) {

---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 
20230914-strncpy-drivers-gpu-drm-nouveau-nvkm-engine-pm-base-c-38bf9c78bc0f

Best regards,
--
Justin Stitt 



[PATCH] drm/nouveau/core: refactor deprecated strncpy

2023-09-14 Thread Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings [1].

We should prefer more robust and less ambiguous string interfaces.

A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.

There is likely no bug in the current implementation due to the safeguard:
|   cname[sizeof(cname) - 1] = '\0';
... however we can provide simpler and easier to understand code using
the newer (and recommended) `strscpy` api.

Link: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
 [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt 
---
 drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c 
b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index 91fb494d4009..374212da9e95 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -79,8 +79,7 @@ nvkm_firmware_get(const struct nvkm_subdev *subdev, const 
char *fwname, int ver,
int i;
 
/* Convert device name to lowercase */
-   strncpy(cname, device->chip->name, sizeof(cname));
-   cname[sizeof(cname) - 1] = '\0';
+   strscpy(cname, device->chip->name, sizeof(cname));
i = strlen(cname);
while (i) {
--i;

---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 
20230914-strncpy-drivers-gpu-drm-nouveau-nvkm-core-firmware-c-791223838b72

Best regards,
--
Justin Stitt 



[PATCH] drm/nouveau/nvif: refactor deprecated strncpy

2023-09-14 Thread Justin Stitt
`strncpy` is deprecated and as such we should prefer more robust and
less ambiguous string interfaces.

A suitable replacement is `strscpy_pad` due to the fact that it
guarantees NUL-termination on the destination buffer whilst also
maintaining the NUL-padding behavior that `strncpy` provides. I am not
sure whether NUL-padding is strictly needed but I see in
`nvif_object_ctor()` args is memcpy'd elsewhere so I figured we'd keep
the same functionality.

Link: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
 [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt 
---
Note: build-tested only.
---
 drivers/gpu/drm/nouveau/nvif/client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/client.c 
b/drivers/gpu/drm/nouveau/nvif/client.c
index a3264a0e933a..3a27245f467f 100644
--- a/drivers/gpu/drm/nouveau/nvif/client.c
+++ b/drivers/gpu/drm/nouveau/nvif/client.c
@@ -69,7 +69,7 @@ nvif_client_ctor(struct nvif_client *parent, const char 
*name, u64 device,
} nop = {};
int ret;
 
-   strncpy(args.name, name, sizeof(args.name));
+   strscpy_pad(args.name, name, sizeof(args.name));
ret = nvif_object_ctor(parent != client ? >object : NULL,
   name ? name : "nvifClient", 0,
   NVIF_CLASS_CLIENT, , sizeof(args),

---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-gpu-drm-nouveau-nvif-client-c-82b023c36953

Best regards,
--
Justin Stitt 



Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström
On Thu, 2023-09-14 at 19:25 +0200, Danilo Krummrich wrote:
> On 9/14/23 19:21, Thomas Hellström wrote:
> > On Thu, 2023-09-14 at 18:36 +0200, Danilo Krummrich wrote:
> > > On 9/14/23 15:48, Thomas Hellström wrote:
> > > > Hi, Danilo
> > > > 
> > > > Some additional minor comments as xe conversion progresses.
> > > > 
> > > > On 9/9/23 17:31, Danilo Krummrich wrote:
> > > > > So far the DRM GPUVA manager offers common infrastructure to
> > > > > track GPU VA
> > > > > allocations and mappings, generically connect GPU VA mappings
> > > > > to
> > > > > their
> > > > > backing buffers and perform more complex mapping operations
> > > > > on
> > > > > the GPU VA
> > > > > space.
> > > > > 
> > > > > However, there are more design patterns commonly used by
> > > > > drivers,
> > > > > which
> > > > > can potentially be generalized in order to make the DRM GPUVA
> > > > > manager
> > > > > represent a basic GPU-VM implementation. In this context,
> > > > > this
> > > > > patch aims
> > > > > at generalizing the following elements.
> > > > > 
> > > > > 1) Provide a common dma-resv for GEM objects not being used
> > > > > outside of
> > > > >  this GPU-VM.
> > > > > 
> > > > > 2) Provide tracking of external GEM objects (GEM objects
> > > > > which
> > > > > are
> > > > >  shared with other GPU-VMs).
> > > > > 
> > > > > 3) Provide functions to efficiently lock all GEM objects dma-
> > > > > resv
> > > > > the
> > > > >  GPU-VM contains mappings of.
> > > > > 
> > > > > 4) Provide tracking of evicted GEM objects the GPU-VM
> > > > > contains
> > > > > mappings
> > > > >  of, such that validation of evicted GEM objects is
> > > > > accelerated.
> > > > > 
> > > > > 5) Provide some convinience functions for common patterns.
> > > > > 
> > > > > Rather than being designed as a "framework", the target is to
> > > > > make all
> > > > > features appear as a collection of optional helper functions,
> > > > > such that
> > > > > drivers are free to make use of the DRM GPUVA managers basic
> > > > > functionality and opt-in for other features without setting
> > > > > any
> > > > > feature
> > > > > flags, just by making use of the corresponding functions.
> > > > > 
> > > > > Big kudos to Boris Brezillon for his help to figure out
> > > > > locking
> > > > > for drivers
> > > > > updating the GPU VA space within the fence signalling path.
> > > > > 
> > > > > Suggested-by: Matthew Brost 
> > > > > Signed-off-by: Danilo Krummrich 
> > > > > ---
> > > > > 
> > > > > +/**
> > > > > + * drm_gpuvm_bo_evict() - add / remove a _gem_object to
> > > > > /
> > > > > from a
> > > > > + * _gpuvms evicted list
> > > > > + * @obj: the _gem_object to add or remove
> > > > > + * @evict: indicates whether the object is evicted
> > > > > + *
> > > > > + * Adds a _gem_object to or removes it from all
> > > > > _gpuvms
> > > > > evicted
> > > > > + * list containing a mapping of this _gem_object.
> > > > > + */
> > > > > +void
> > > > > +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
> > > > > +{
> > > > > +    struct drm_gpuvm_bo *vm_bo;
> > > > > +
> > > > > +    drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> > > > > +    if (evict)
> > > > > +    drm_gpuvm_bo_list_add(vm_bo, evict);
> > > > > +    else
> > > > > +    drm_gpuvm_bo_list_del(vm_bo, evict);
> > > > > +    }
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
> > > > > +
> > > > 
> > > > We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...)
> > > > that
> > > > puts a single gpuvm_bo on the list, the above function could
> > > > perhaps be renamed as drm_gpuvm_gem_obj_evict(obj, ).
> > > 
> > > Makes sense - gonna change that.
> > > 
> > > > 
> > > > Reason is some vm's are faulting vms which don't have an evict
> > > > list, but validate from the pagefault handler. Also evict ==
> > > > false
> > > > is dangerous because if called from within an exec, it might
> > > > remove
> > > > the obj from other vm's evict list before they've had a chance
> > > > to
> > > > rebind their VMAs.
> > > > 
> > > > >    static int
> > > > >    __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
> > > > >   struct drm_gpuva *va)
> > > > > diff --git a/include/drm/drm_gpuvm.h
> > > > > b/include/drm/drm_gpuvm.h
> > > > > index afa50b9059a2..834bb6d6617e 100644
> > > > > --- a/include/drm/drm_gpuvm.h
> > > > > +++ b/include/drm/drm_gpuvm.h
> > > > > @@ -26,10 +26,12 @@
> > > > >     */
> > > > >    #include 
> > > > > +#include 
> > > > >    #include 
> > > > >    #include 
> > > > >    #include 
> > > > > +#include 
> > > > >    struct drm_gpuvm;
> > > > >    struct drm_gpuvm_bo;
> > > > > @@ -259,6 +261,38 @@ struct drm_gpuvm {
> > > > >     * space
> > > > >     */
> > > > >    struct dma_resv *resv;
> > > > > +
> > > > > +    /**
> > > > > + * @extobj: structure holding the extobj list
> > > > > + */
> > > > > +    struct {
> > > > > +    /**
> > > > > + * @list: _head storing 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Danilo Krummrich

On 9/14/23 19:21, Thomas Hellström wrote:

On Thu, 2023-09-14 at 18:36 +0200, Danilo Krummrich wrote:

On 9/14/23 15:48, Thomas Hellström wrote:

Hi, Danilo

Some additional minor comments as xe conversion progresses.

On 9/9/23 17:31, Danilo Krummrich wrote:

So far the DRM GPUVA manager offers common infrastructure to
track GPU VA
allocations and mappings, generically connect GPU VA mappings to
their
backing buffers and perform more complex mapping operations on
the GPU VA
space.

However, there are more design patterns commonly used by drivers,
which
can potentially be generalized in order to make the DRM GPUVA
manager
represent a basic GPU-VM implementation. In this context, this
patch aims
at generalizing the following elements.

1) Provide a common dma-resv for GEM objects not being used
outside of
     this GPU-VM.

2) Provide tracking of external GEM objects (GEM objects which
are
     shared with other GPU-VMs).

3) Provide functions to efficiently lock all GEM objects dma-resv
the
     GPU-VM contains mappings of.

4) Provide tracking of evicted GEM objects the GPU-VM contains
mappings
     of, such that validation of evicted GEM objects is
accelerated.

5) Provide some convinience functions for common patterns.

Rather than being designed as a "framework", the target is to
make all
features appear as a collection of optional helper functions,
such that
drivers are free to make use of the DRM GPUVA managers basic
functionality and opt-in for other features without setting any
feature
flags, just by making use of the corresponding functions.

Big kudos to Boris Brezillon for his help to figure out locking
for drivers
updating the GPU VA space within the fence signalling path.

Suggested-by: Matthew Brost 
Signed-off-by: Danilo Krummrich 
---

+/**
+ * drm_gpuvm_bo_evict() - add / remove a _gem_object to /
from a
+ * _gpuvms evicted list
+ * @obj: the _gem_object to add or remove
+ * @evict: indicates whether the object is evicted
+ *
+ * Adds a _gem_object to or removes it from all _gpuvms
evicted
+ * list containing a mapping of this _gem_object.
+ */
+void
+drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
+{
+    struct drm_gpuvm_bo *vm_bo;
+
+    drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
+    if (evict)
+    drm_gpuvm_bo_list_add(vm_bo, evict);
+    else
+    drm_gpuvm_bo_list_del(vm_bo, evict);
+    }
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
+


We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that
puts a single gpuvm_bo on the list, the above function could
perhaps be renamed as drm_gpuvm_gem_obj_evict(obj, ).


Makes sense - gonna change that.



Reason is some vm's are faulting vms which don't have an evict
list, but validate from the pagefault handler. Also evict == false
is dangerous because if called from within an exec, it might remove
the obj from other vm's evict list before they've had a chance to
rebind their VMAs.


   static int
   __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
  struct drm_gpuva *va)
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index afa50b9059a2..834bb6d6617e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -26,10 +26,12 @@
    */
   #include 
+#include 
   #include 
   #include 
   #include 
+#include 
   struct drm_gpuvm;
   struct drm_gpuvm_bo;
@@ -259,6 +261,38 @@ struct drm_gpuvm {
    * space
    */
   struct dma_resv *resv;
+
+    /**
+ * @extobj: structure holding the extobj list
+ */
+    struct {
+    /**
+ * @list: _head storing _gpuvm_bos serving as
+ * external object
+ */
+    struct list_head list;
+
+    /**
+ * @lock: spinlock to protect the extobj list
+ */
+    spinlock_t lock;
+    } extobj;
+
+    /**
+ * @evict: structure holding the evict list and evict list
lock
+ */
+    struct {
+    /**
+ * @list: _head storing _gpuvm_bos currently
being
+ * evicted
+ */
+    struct list_head list;
+
+    /**
+ * @lock: spinlock to protect the evict list
+ */
+    spinlock_t lock;
+    } evict;
   };
   void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device
*drm,
@@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm,
struct drm_device *drm,
   const struct drm_gpuvm_ops *ops);
   void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm);
+/**
+ * drm_gpuvm_is_extobj() - indicates whether the given
_gem_object is an
+ * external object
+ * @gpuvm: the _gpuvm to check
+ * @obj: the _gem_object to check
+ *
+ * Returns: true if the _gem_object _resv differs from
the
+ * _gpuvms _resv, false otherwise
+ */
+static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
+   struct drm_gem_object *obj)
+{
+    return obj && obj->resv != gpuvm->resv;
+}
+
   static inline struct drm_gpuva *
   __drm_gpuva_next(struct drm_gpuva *va)
   {
@@ -346,6 +395,128 @@ __drm_gpuva_next(struct 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström
On Thu, 2023-09-14 at 18:36 +0200, Danilo Krummrich wrote:
> On 9/14/23 15:48, Thomas Hellström wrote:
> > Hi, Danilo
> > 
> > Some additional minor comments as xe conversion progresses.
> > 
> > On 9/9/23 17:31, Danilo Krummrich wrote:
> > > So far the DRM GPUVA manager offers common infrastructure to
> > > track GPU VA
> > > allocations and mappings, generically connect GPU VA mappings to
> > > their
> > > backing buffers and perform more complex mapping operations on
> > > the GPU VA
> > > space.
> > > 
> > > However, there are more design patterns commonly used by drivers,
> > > which
> > > can potentially be generalized in order to make the DRM GPUVA
> > > manager
> > > represent a basic GPU-VM implementation. In this context, this
> > > patch aims
> > > at generalizing the following elements.
> > > 
> > > 1) Provide a common dma-resv for GEM objects not being used
> > > outside of
> > >     this GPU-VM.
> > > 
> > > 2) Provide tracking of external GEM objects (GEM objects which
> > > are
> > >     shared with other GPU-VMs).
> > > 
> > > 3) Provide functions to efficiently lock all GEM objects dma-resv
> > > the
> > >     GPU-VM contains mappings of.
> > > 
> > > 4) Provide tracking of evicted GEM objects the GPU-VM contains
> > > mappings
> > >     of, such that validation of evicted GEM objects is
> > > accelerated.
> > > 
> > > 5) Provide some convinience functions for common patterns.
> > > 
> > > Rather than being designed as a "framework", the target is to
> > > make all
> > > features appear as a collection of optional helper functions,
> > > such that
> > > drivers are free to make use of the DRM GPUVA managers basic
> > > functionality and opt-in for other features without setting any
> > > feature
> > > flags, just by making use of the corresponding functions.
> > > 
> > > Big kudos to Boris Brezillon for his help to figure out locking
> > > for drivers
> > > updating the GPU VA space within the fence signalling path.
> > > 
> > > Suggested-by: Matthew Brost 
> > > Signed-off-by: Danilo Krummrich 
> > > ---
> > > 
> > > +/**
> > > + * drm_gpuvm_bo_evict() - add / remove a _gem_object to /
> > > from a
> > > + * _gpuvms evicted list
> > > + * @obj: the _gem_object to add or remove
> > > + * @evict: indicates whether the object is evicted
> > > + *
> > > + * Adds a _gem_object to or removes it from all _gpuvms
> > > evicted
> > > + * list containing a mapping of this _gem_object.
> > > + */
> > > +void
> > > +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
> > > +{
> > > +    struct drm_gpuvm_bo *vm_bo;
> > > +
> > > +    drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> > > +    if (evict)
> > > +    drm_gpuvm_bo_list_add(vm_bo, evict);
> > > +    else
> > > +    drm_gpuvm_bo_list_del(vm_bo, evict);
> > > +    }
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
> > > +
> > 
> > We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that
> > puts a single gpuvm_bo on the list, the above function could
> > perhaps be renamed as drm_gpuvm_gem_obj_evict(obj, ).
> 
> Makes sense - gonna change that.
> 
> > 
> > Reason is some vm's are faulting vms which don't have an evict
> > list, but validate from the pagefault handler. Also evict == false
> > is dangerous because if called from within an exec, it might remove
> > the obj from other vm's evict list before they've had a chance to
> > rebind their VMAs.
> > 
> > >   static int
> > >   __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
> > >  struct drm_gpuva *va)
> > > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> > > index afa50b9059a2..834bb6d6617e 100644
> > > --- a/include/drm/drm_gpuvm.h
> > > +++ b/include/drm/drm_gpuvm.h
> > > @@ -26,10 +26,12 @@
> > >    */
> > >   #include 
> > > +#include 
> > >   #include 
> > >   #include 
> > >   #include 
> > > +#include 
> > >   struct drm_gpuvm;
> > >   struct drm_gpuvm_bo;
> > > @@ -259,6 +261,38 @@ struct drm_gpuvm {
> > >    * space
> > >    */
> > >   struct dma_resv *resv;
> > > +
> > > +    /**
> > > + * @extobj: structure holding the extobj list
> > > + */
> > > +    struct {
> > > +    /**
> > > + * @list: _head storing _gpuvm_bos serving as
> > > + * external object
> > > + */
> > > +    struct list_head list;
> > > +
> > > +    /**
> > > + * @lock: spinlock to protect the extobj list
> > > + */
> > > +    spinlock_t lock;
> > > +    } extobj;
> > > +
> > > +    /**
> > > + * @evict: structure holding the evict list and evict list
> > > lock
> > > + */
> > > +    struct {
> > > +    /**
> > > + * @list: _head storing _gpuvm_bos currently
> > > being
> > > + * evicted
> > > + */
> > > +    struct list_head list;
> > > +
> > > +    /**
> > > + * @lock: spinlock to protect the evict list
> > > + */
> > > +    spinlock_t lock;
> > > +    } evict;
> > >   };
> > >   void 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Danilo Krummrich

On 9/14/23 19:13, Thomas Hellström wrote:

On Thu, 2023-09-14 at 17:27 +0200, Danilo Krummrich wrote:

On 9/14/23 13:32, Thomas Hellström wrote:


On 9/14/23 12:57, Danilo Krummrich wrote:

On 9/13/23 14:16, Danilo Krummrich wrote:




And validate() can remove it while still holding all dma-
resv locks,
neat!
However, what if two tasks are trying to lock the VA space
concurrently? What
do we do when the drm_gpuvm_bo's refcount drops to zero in
drm_gpuva_unlink()?
Are we guaranteed that at this point of time the
drm_gpuvm_bo is not
on the
evicted list? Because otherwise we would call
drm_gpuvm_bo_destroy()
with the
dma-resv lock held, which wouldn't be allowed, since
drm_gpuvm_bo_destroy()
might drop the last reference to the drm_gem_object and
hence we'd
potentially
free the dma-resv lock while holding it, at least if it's
an external
object.


Easiest way in this scheme is to think of the lists as being
protected
by the vm's resv lock. That means anybody calling unlink()
must also
hold the vm's resv lock. (Which is OK from an UAF point of
view, but
perhaps not from a locking inversion POW from an async list
update).


This would mean that on unlink() we'd need to hold the VM's
resv lock and the
corresponding GEM's resv lock (in case they're not the same
anyways) because the
VM's resv lock would protect the external / evicted object
lists and the GEM
objects resv lock protects the GEM's list of drm_gpuvm_bos and
the
drm_gpuvm_bo's list of drm_gpuvas.


As mentioned below the same applies for drm_gpuvm_bo_put() since
it might
destroy the vm_bo, which includes removing the vm_bo from
external / evicted
object lists and the GEMs list of vm_bos.

As mentioned, if the GEM's dma-resv is different from the VM's
dma-resv we need
to take both locks. Ultimately, this would mean we need a
drm_exec loop, because
we can't know the order in which to take these locks. Doing a
full drm_exec loop
just to put() a vm_bo doesn't sound reasonable to me.

Can we instead just have an internal mutex for locking the lists
such that we
avoid taking and dropping the spinlocks, which we use currently,
in a loop?


You'd have the same locking inversion problem with a mutex, right?
Since in the eviction path you have resv->mutex, from exec you have
resv->mutex->resv because validate would attempt to grab resv.


Both lists, evict and extobj, would need to have a separate mutex,
not a common one.
We'd also need a dedicated GEM gpuva lock. Then the only rule would
be that you can't
hold the dma-resv lock when calling put(). Which I admit is not that
nice.

With the current spinlock solution drivers wouldn't need to worry
about anything locking
related though. So maybe I come back to your proposal of having a
switch for external
locking with dma-resv locks entirely. Such that with external dma-
resv locking I skip
all the spinlocks and add lockdep checks instead.

I think that makes the most sense in terms of taking advantage of
external dma-resv locking
where possible and on the other hand having a self-contained solution
if not. This should
get all concerns out of the way, yours, Christian's and Boris'.


If we need additional locks yes, I'd prefer the opt-in/opt-out spinlock
solution, and check back after a while to see if we can remove either
option once most pitfalls are hit.


Sounds good, I'll prepare this for a V4.

- Danilo



Thanks,
/Thomas






That said, xe currently indeed does the vm+bo exec dance on vma
put.

One reason why that seemingly horrible construct is good, is that
when evicting an extobj and you need to access individual vmas to
Zap page table entries or TLB flush, those VMAs are not allowed to
go away (we're not refcounting them). Holding the bo resv on gpuva
put prevents that from happening. Possibly one could use another
mutex to protect the gem->vm_bo list to achieve the same, but we'd
need to hold it on gpuva put.

/Thomas




- Danilo









For extobjs an outer lock would be enough in case of
Xe, but I
really would not
like to add even more complexity just to get the
spinlock out of
the way in case
the driver already has an outer lock protecting this
path.


I must disagree here. These spinlocks and atomic
operations are
pretty
costly and as discussed earlier this type of locking was
the reason
(at
least according to the commit message) that made
Christian drop the
XArray
use in drm_exec for the same set of objects: "The locking
overhead
is
unecessary and measurable". IMHO the spinlock is the
added
complexity and a
single wide lock following the drm locking guidelines set
out by
Daniel and
David should really be the default choice with an opt-in
for a
spinlock if
needed for async and pushing out to a wq is not an
option.


For the external object list an outer lock would work as
long as it's
not the
dma-resv lock of the corresponding GEM object, since here
we actually
need to
remove the list entry from the external object list on
drm_gpuvm_bo_destroy().
It's just a bit weird design wise that drivers 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström
On Thu, 2023-09-14 at 17:27 +0200, Danilo Krummrich wrote:
> On 9/14/23 13:32, Thomas Hellström wrote:
> > 
> > On 9/14/23 12:57, Danilo Krummrich wrote:
> > > On 9/13/23 14:16, Danilo Krummrich wrote:
> > > 
> > > 
> > > 
> > > > > > And validate() can remove it while still holding all dma-
> > > > > > resv locks,
> > > > > > neat!
> > > > > > However, what if two tasks are trying to lock the VA space
> > > > > > concurrently? What
> > > > > > do we do when the drm_gpuvm_bo's refcount drops to zero in
> > > > > > drm_gpuva_unlink()?
> > > > > > Are we guaranteed that at this point of time the
> > > > > > drm_gpuvm_bo is not
> > > > > > on the
> > > > > > evicted list? Because otherwise we would call
> > > > > > drm_gpuvm_bo_destroy()
> > > > > > with the
> > > > > > dma-resv lock held, which wouldn't be allowed, since
> > > > > > drm_gpuvm_bo_destroy()
> > > > > > might drop the last reference to the drm_gem_object and
> > > > > > hence we'd
> > > > > > potentially
> > > > > > free the dma-resv lock while holding it, at least if it's
> > > > > > an external
> > > > > > object.
> > > > > 
> > > > > Easiest way in this scheme is to think of the lists as being
> > > > > protected
> > > > > by the vm's resv lock. That means anybody calling unlink()
> > > > > must also
> > > > > hold the vm's resv lock. (Which is OK from an UAF point of
> > > > > view, but
> > > > > perhaps not from a locking inversion POW from an async list
> > > > > update).
> > > > 
> > > > This would mean that on unlink() we'd need to hold the VM's
> > > > resv lock and the
> > > > corresponding GEM's resv lock (in case they're not the same
> > > > anyways) because the
> > > > VM's resv lock would protect the external / evicted object
> > > > lists and the GEM
> > > > objects resv lock protects the GEM's list of drm_gpuvm_bos and
> > > > the
> > > > drm_gpuvm_bo's list of drm_gpuvas.
> > > 
> > > As mentioned below the same applies for drm_gpuvm_bo_put() since
> > > it might
> > > destroy the vm_bo, which includes removing the vm_bo from
> > > external / evicted
> > > object lists and the GEMs list of vm_bos.
> > > 
> > > As mentioned, if the GEM's dma-resv is different from the VM's
> > > dma-resv we need
> > > to take both locks. Ultimately, this would mean we need a
> > > drm_exec loop, because
> > > we can't know the order in which to take these locks. Doing a
> > > full drm_exec loop
> > > just to put() a vm_bo doesn't sound reasonable to me.
> > > 
> > > Can we instead just have an internal mutex for locking the lists
> > > such that we
> > > avoid taking and dropping the spinlocks, which we use currently,
> > > in a loop?
> > 
> > You'd have the same locking inversion problem with a mutex, right?
> > Since in the eviction path you have resv->mutex, from exec you have
> > resv->mutex->resv because validate would attempt to grab resv.
> 
> Both lists, evict and extobj, would need to have a separate mutex,
> not a common one.
> We'd also need a dedicated GEM gpuva lock. Then the only rule would
> be that you can't
> hold the dma-resv lock when calling put(). Which I admit is not that
> nice.
> 
> With the current spinlock solution drivers wouldn't need to worry
> about anything locking
> related though. So maybe I come back to your proposal of having a
> switch for external
> locking with dma-resv locks entirely. Such that with external dma-
> resv locking I skip
> all the spinlocks and add lockdep checks instead.
> 
> I think that makes the most sense in terms of taking advantage of
> external dma-resv locking
> where possible and on the other hand having a self-contained solution
> if not. This should
> get all concerns out of the way, yours, Christian's and Boris'.

If we need additional locks yes, I'd prefer the opt-in/opt-out spinlock
solution, and check back after a while to see if we can remove either
option once most pitfalls are hit.

Thanks,
/Thomas


> 
> > 
> > That said, xe currently indeed does the vm+bo exec dance on vma
> > put.
> > 
> > One reason why that seemingly horrible construct is good, is that
> > when evicting an extobj and you need to access individual vmas to
> > Zap page table entries or TLB flush, those VMAs are not allowed to
> > go away (we're not refcounting them). Holding the bo resv on gpuva
> > put prevents that from happening. Possibly one could use another
> > mutex to protect the gem->vm_bo list to achieve the same, but we'd
> > need to hold it on gpuva put.
> > 
> > /Thomas
> > 
> > 
> > > 
> > > - Danilo
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > > > 
> > > > > > > > For extobjs an outer lock would be enough in case of
> > > > > > > > Xe, but I
> > > > > > > > really would not
> > > > > > > > like to add even more complexity just to get the
> > > > > > > > spinlock out of
> > > > > > > > the way in case
> > > > > > > > the driver already has an outer lock protecting this
> > > > > > > > path.
> > > > > > > 
> > > > > > > I must disagree here. These spinlocks and atomic
> > > > > > > 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Danilo Krummrich

On 9/14/23 15:48, Thomas Hellström wrote:

Hi, Danilo

Some additional minor comments as xe conversion progresses.

On 9/9/23 17:31, Danilo Krummrich wrote:

So far the DRM GPUVA manager offers common infrastructure to track GPU VA
allocations and mappings, generically connect GPU VA mappings to their
backing buffers and perform more complex mapping operations on the GPU VA
space.

However, there are more design patterns commonly used by drivers, which
can potentially be generalized in order to make the DRM GPUVA manager
represent a basic GPU-VM implementation. In this context, this patch aims
at generalizing the following elements.

1) Provide a common dma-resv for GEM objects not being used outside of
    this GPU-VM.

2) Provide tracking of external GEM objects (GEM objects which are
    shared with other GPU-VMs).

3) Provide functions to efficiently lock all GEM objects dma-resv the
    GPU-VM contains mappings of.

4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
    of, such that validation of evicted GEM objects is accelerated.

5) Provide some convinience functions for common patterns.

Rather than being designed as a "framework", the target is to make all
features appear as a collection of optional helper functions, such that
drivers are free to make use of the DRM GPUVA managers basic
functionality and opt-in for other features without setting any feature
flags, just by making use of the corresponding functions.

Big kudos to Boris Brezillon for his help to figure out locking for drivers
updating the GPU VA space within the fence signalling path.

Suggested-by: Matthew Brost 
Signed-off-by: Danilo Krummrich 
---

+/**
+ * drm_gpuvm_bo_evict() - add / remove a _gem_object to / from a
+ * _gpuvms evicted list
+ * @obj: the _gem_object to add or remove
+ * @evict: indicates whether the object is evicted
+ *
+ * Adds a _gem_object to or removes it from all _gpuvms evicted
+ * list containing a mapping of this _gem_object.
+ */
+void
+drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
+{
+    struct drm_gpuvm_bo *vm_bo;
+
+    drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
+    if (evict)
+    drm_gpuvm_bo_list_add(vm_bo, evict);
+    else
+    drm_gpuvm_bo_list_del(vm_bo, evict);
+    }
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
+


We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that puts a 
single gpuvm_bo on the list, the above function could perhaps be renamed as 
drm_gpuvm_gem_obj_evict(obj, ).


Makes sense - gonna change that.



Reason is some vm's are faulting vms which don't have an evict list, but 
validate from the pagefault handler. Also evict == false is dangerous because 
if called from within an exec, it might remove the obj from other vm's evict 
list before they've had a chance to rebind their VMAs.


  static int
  __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
 struct drm_gpuva *va)
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index afa50b9059a2..834bb6d6617e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -26,10 +26,12 @@
   */
  #include 
+#include 
  #include 
  #include 
  #include 
+#include 
  struct drm_gpuvm;
  struct drm_gpuvm_bo;
@@ -259,6 +261,38 @@ struct drm_gpuvm {
   * space
   */
  struct dma_resv *resv;
+
+    /**
+ * @extobj: structure holding the extobj list
+ */
+    struct {
+    /**
+ * @list: _head storing _gpuvm_bos serving as
+ * external object
+ */
+    struct list_head list;
+
+    /**
+ * @lock: spinlock to protect the extobj list
+ */
+    spinlock_t lock;
+    } extobj;
+
+    /**
+ * @evict: structure holding the evict list and evict list lock
+ */
+    struct {
+    /**
+ * @list: _head storing _gpuvm_bos currently being
+ * evicted
+ */
+    struct list_head list;
+
+    /**
+ * @lock: spinlock to protect the evict list
+ */
+    spinlock_t lock;
+    } evict;
  };
  void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
@@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct 
drm_device *drm,
  const struct drm_gpuvm_ops *ops);
  void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm);
+/**
+ * drm_gpuvm_is_extobj() - indicates whether the given _gem_object is an
+ * external object
+ * @gpuvm: the _gpuvm to check
+ * @obj: the _gem_object to check
+ *
+ * Returns: true if the _gem_object _resv differs from the
+ * _gpuvms _resv, false otherwise
+ */
+static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
+   struct drm_gem_object *obj)
+{
+    return obj && obj->resv != gpuvm->resv;
+}
+
  static inline struct drm_gpuva *
  __drm_gpuva_next(struct drm_gpuva *va)
  {
@@ -346,6 +395,128 @@ __drm_gpuva_next(struct drm_gpuva *va)
  #define drm_gpuvm_for_each_va_safe(va__, next__, gpuvm__) \
  list_for_each_entry_safe(va__, 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Danilo Krummrich

On 9/14/23 13:32, Thomas Hellström wrote:


On 9/14/23 12:57, Danilo Krummrich wrote:

On 9/13/23 14:16, Danilo Krummrich wrote:




And validate() can remove it while still holding all dma-resv locks,
neat!
However, what if two tasks are trying to lock the VA space
concurrently? What
do we do when the drm_gpuvm_bo's refcount drops to zero in
drm_gpuva_unlink()?
Are we guaranteed that at this point of time the drm_gpuvm_bo is not
on the
evicted list? Because otherwise we would call drm_gpuvm_bo_destroy()
with the
dma-resv lock held, which wouldn't be allowed, since
drm_gpuvm_bo_destroy()
might drop the last reference to the drm_gem_object and hence we'd
potentially
free the dma-resv lock while holding it, at least if it's an external
object.


Easiest way in this scheme is to think of the lists as being protected
by the vm's resv lock. That means anybody calling unlink() must also
hold the vm's resv lock. (Which is OK from an UAF point of view, but
perhaps not from a locking inversion POW from an async list update).


This would mean that on unlink() we'd need to hold the VM's resv lock and the
corresponding GEM's resv lock (in case they're not the same anyways) because the
VM's resv lock would protect the external / evicted object lists and the GEM
objects resv lock protects the GEM's list of drm_gpuvm_bos and the
drm_gpuvm_bo's list of drm_gpuvas.


As mentioned below the same applies for drm_gpuvm_bo_put() since it might
destroy the vm_bo, which includes removing the vm_bo from external / evicted
object lists and the GEMs list of vm_bos.

As mentioned, if the GEM's dma-resv is different from the VM's dma-resv we need
to take both locks. Ultimately, this would mean we need a drm_exec loop, because
we can't know the order in which to take these locks. Doing a full drm_exec loop
just to put() a vm_bo doesn't sound reasonable to me.

Can we instead just have an internal mutex for locking the lists such that we
avoid taking and dropping the spinlocks, which we use currently, in a loop?


You'd have the same locking inversion problem with a mutex, right? Since in the eviction 
path you have resv->mutex, from exec you have resv->mutex->resv because 
validate would attempt to grab resv.


Both lists, evict and extobj, would need to have a separate mutex, not a common 
one.
We'd also need a dedicated GEM gpuva lock. Then the only rule would be that you 
can't
hold the dma-resv lock when calling put(). Which I admit is not that nice.

With the current spinlock solution drivers wouldn't need to worry about 
anything locking
related though. So maybe I come back to your proposal of having a switch for 
external
locking with dma-resv locks entirely. Such that with external dma-resv locking 
I skip
all the spinlocks and add lockdep checks instead.

I think that makes the most sense in terms of taking advantage of external 
dma-resv locking
where possible and on the other hand having a self-contained solution if not. 
This should
get all concerns out of the way, yours, Christian's and Boris'.



That said, xe currently indeed does the vm+bo exec dance on vma put.

One reason why that seemingly horrible construct is good, is that when evicting an 
extobj and you need to access individual vmas to Zap page table entries or TLB 
flush, those VMAs are not allowed to go away (we're not refcounting them). Holding 
the bo resv on gpuva put prevents that from happening. Possibly one could use 
another mutex to protect the gem->vm_bo list to achieve the same, but we'd need 
to hold it on gpuva put.

/Thomas




- Danilo









For extobjs an outer lock would be enough in case of Xe, but I
really would not
like to add even more complexity just to get the spinlock out of
the way in case
the driver already has an outer lock protecting this path.


I must disagree here. These spinlocks and atomic operations are
pretty
costly and as discussed earlier this type of locking was the reason
(at
least according to the commit message) that made Christian drop the
XArray
use in drm_exec for the same set of objects: "The locking overhead
is
unecessary and measurable". IMHO the spinlock is the added
complexity and a
single wide lock following the drm locking guidelines set out by
Daniel and
David should really be the default choice with an opt-in for a
spinlock if
needed for async and pushing out to a wq is not an option.


For the external object list an outer lock would work as long as it's
not the
dma-resv lock of the corresponding GEM object, since here we actually
need to
remove the list entry from the external object list on
drm_gpuvm_bo_destroy().
It's just a bit weird design wise that drivers would need to take
this outer
lock on:

- drm_gpuvm_bo_extobj_add()
- drm_gpuvm_bo_destroy()(and hence also drm_gpuvm_bo_put())
- drm_gpuva_unlink()(because it needs to call
drm_gpuvm_bo_put())
- drm_gpuvm_exec_lock()
- drm_gpuvm_exec_lock_array()
- drm_gpuvm_prepare_range()

Given that it seems reasonable to do all the 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström

Hi,

On 9/14/23 13:54, Boris Brezillon wrote:

On Thu, 14 Sep 2023 12:45:44 +0200
Thomas Hellström  wrote:


On 9/14/23 10:20, Boris Brezillon wrote:

On Wed, 13 Sep 2023 15:22:56 +0200
Thomas Hellström  wrote:
  

On 9/13/23 13:33, Boris Brezillon wrote:

On Wed, 13 Sep 2023 12:39:01 +0200
Thomas Hellström  wrote:
 

Hi,

On 9/13/23 09:19, Boris Brezillon wrote:

On Wed, 13 Sep 2023 17:05:42 +1000
Dave Airlie  wrote:


On Wed, 13 Sept 2023 at 17:03, Boris Brezillon
 wrote:

On Tue, 12 Sep 2023 18:20:32 +0200
Thomas Hellström  wrote:
   

+/**
+ * get_next_vm_bo_from_list() - get the next vm_bo element
+ * @__gpuvm: The GPU VM
+ * @__list_name: The name of the list we're iterating on
+ * @__local_list: A pointer to the local list used to store already iterated 
items
+ * @__prev_vm_bo: The previous element we got from 
drm_gpuvm_get_next_cached_vm_bo()
+ *
+ * This helper is here to provide lockless list iteration. Lockless as in, the
+ * iterator releases the lock immediately after picking the first element from
+ * the list, so list insertion deletion can happen concurrently.

Are the list spinlocks needed for that async state update from within
the dma-fence critical section we've discussed previously?

Any driver calling _[un]link() from its drm_gpu_scheduler::run_job()
hook will be in this situation (Panthor at the moment, PowerVR soon). I
get that Xe and Nouveau don't need that because they update the VM
state early (in the ioctl path), but I keep thinking this will hurt us
if we don't think it through from the beginning, because once you've
set this logic to depend only on resv locks, it will be pretty hard to
get back to a solution which lets synchronous VM_BINDs take precedence
on asynchronous request, and, with vkQueueBindSparse() passing external
deps (plus the fact the VM_BIND queue might be pretty deep), it can
take a long time to get your synchronous VM_BIND executed...

So this would boil down to either (possibly opt-in) keeping the spinlock
approach or pushing the unlink out to a wq then?

Deferred _unlink() would not be an issue, since I already defer the
drm_gpuva destruction to a wq, it would just a be a matter of moving the
_unlink() call there as well. But _link() also takes the GEM gpuva list
lock, and that one is bit tricky, in that sm_map() can trigger 2 more
_link() calls for the prev/next mappings, which we can't guess until we
get to execute the VM update. If we mandate the use of the GEM resv
lock, that simply means async VM updates (AKA calling
drm_gpuvm_sm_[un]map()) are not an option. And if this is what everyone
agrees on, then I'd like the APIs that make this sort of async VM
update possible (drm_gpuvm_sm_[un]map(), the drm_gpuvm_ops::sm_step*
methods, and probably other things) to be dropped, so we don't make it
look like it's something we support.
 

BTW, as also asked in a reply to Danilo, how do you call unlink from
run_job() when it was requiring the obj->dma_resv lock, or was that a WIP?

_unlink() makes sure the GEM gpuva list lock is taken, but this can be
a custom lock (see drm_gem_gpuva_set_lock()). In panthor we have
panthor_gem_object::gpuva_list_lock that's dedicated the gpuva list
protection. We make sure we never take this lock while allocating
memory to guarantee the dma-signalling path can't deadlock.
 
   

btw what is the use case for this? do we have actual vulkan
applications we know will have problems here?

I don't, but I think that's a concern Faith raised at some point (dates
back from when I was reading threads describing how VM_BIND on i915
should work, and I was clearly discovering this whole VM_BIND thing at
that time, so maybe I misunderstood).


it feels like a bit of premature optimisation, but maybe we have use cases.

Might be, but that's the sort of thing that would put us in a corner if
we don't have a plan for when the needs arise. Besides, if we don't
want to support that case because it's too complicated, I'd recommend
dropping all the drm_gpuvm APIs that let people think this mode is
valid/supported (map/remap/unmap hooks in drm_gpuvm_ops,
drm_gpuvm_sm_[un]map helpers, etc). Keeping them around just adds to the
confusion.

Xe allows bypassing the bind-queue with another bind-queue, but to
completely avoid dependencies between queues the Operations may not
overlap.

So, you check the VM state with some VM lock held (would be the VM resv
in my case), and if the mapping is new (no overlaps with pre-existing
mappings), you queue it to the fast-track/sync-VM_BIND queue. What would
be missing I guess is a way to know if the mapping is active (MMU has
been updated) or pending (MMU update queued to the bind-queue), so I can
fast-track mapping/unmapping of active mappings.

Ok, so I started modifying the implementation, and quickly realized the
overlap test can't be done without your xe_range_fence tree because of
unmaps. Since we call drm_gpuva_unmap() early/in the IOCTL path (IOW,
before the 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström

Hi, Danilo

Some additional minor comments as xe conversion progresses.

On 9/9/23 17:31, Danilo Krummrich wrote:

So far the DRM GPUVA manager offers common infrastructure to track GPU VA
allocations and mappings, generically connect GPU VA mappings to their
backing buffers and perform more complex mapping operations on the GPU VA
space.

However, there are more design patterns commonly used by drivers, which
can potentially be generalized in order to make the DRM GPUVA manager
represent a basic GPU-VM implementation. In this context, this patch aims
at generalizing the following elements.

1) Provide a common dma-resv for GEM objects not being used outside of
this GPU-VM.

2) Provide tracking of external GEM objects (GEM objects which are
shared with other GPU-VMs).

3) Provide functions to efficiently lock all GEM objects dma-resv the
GPU-VM contains mappings of.

4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
of, such that validation of evicted GEM objects is accelerated.

5) Provide some convinience functions for common patterns.

Rather than being designed as a "framework", the target is to make all
features appear as a collection of optional helper functions, such that
drivers are free to make use of the DRM GPUVA managers basic
functionality and opt-in for other features without setting any feature
flags, just by making use of the corresponding functions.

Big kudos to Boris Brezillon for his help to figure out locking for drivers
updating the GPU VA space within the fence signalling path.

Suggested-by: Matthew Brost 
Signed-off-by: Danilo Krummrich 
---

+/**
+ * drm_gpuvm_bo_evict() - add / remove a _gem_object to / from a
+ * _gpuvms evicted list
+ * @obj: the _gem_object to add or remove
+ * @evict: indicates whether the object is evicted
+ *
+ * Adds a _gem_object to or removes it from all _gpuvms evicted
+ * list containing a mapping of this _gem_object.
+ */
+void
+drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
+{
+   struct drm_gpuvm_bo *vm_bo;
+
+   drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
+   if (evict)
+   drm_gpuvm_bo_list_add(vm_bo, evict);
+   else
+   drm_gpuvm_bo_list_del(vm_bo, evict);
+   }
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
+


We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that puts 
a single gpuvm_bo on the list, the above function could perhaps be 
renamed as drm_gpuvm_gem_obj_evict(obj, ).


Reason is some vm's are faulting vms which don't have an evict list, but 
validate from the pagefault handler. Also evict == false is dangerous 
because if called from within an exec, it might remove the obj from 
other vm's evict list before they've had a chance to rebind their VMAs.



  static int
  __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
   struct drm_gpuva *va)
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index afa50b9059a2..834bb6d6617e 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -26,10 +26,12 @@
   */
  
  #include 

+#include 
  #include 
  #include 
  
  #include 

+#include 
  
  struct drm_gpuvm;

  struct drm_gpuvm_bo;
@@ -259,6 +261,38 @@ struct drm_gpuvm {
 * space
 */
struct dma_resv *resv;
+
+   /**
+* @extobj: structure holding the extobj list
+*/
+   struct {
+   /**
+* @list: _head storing _gpuvm_bos serving as
+* external object
+*/
+   struct list_head list;
+
+   /**
+* @lock: spinlock to protect the extobj list
+*/
+   spinlock_t lock;
+   } extobj;
+
+   /**
+* @evict: structure holding the evict list and evict list lock
+*/
+   struct {
+   /**
+* @list: _head storing _gpuvm_bos currently being
+* evicted
+*/
+   struct list_head list;
+
+   /**
+* @lock: spinlock to protect the evict list
+*/
+   spinlock_t lock;
+   } evict;
  };
  
  void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,

@@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct 
drm_device *drm,
const struct drm_gpuvm_ops *ops);
  void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm);
  
+/**

+ * drm_gpuvm_is_extobj() - indicates whether the given _gem_object is an
+ * external object
+ * @gpuvm: the _gpuvm to check
+ * @obj: the _gem_object to check
+ *
+ * Returns: true if the _gem_object _resv differs from the
+ * _gpuvms _resv, false otherwise
+ */
+static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
+  struct drm_gem_object *obj)
+{
+   return obj && obj->resv != gpuvm->resv;
+}
+
  static inline struct drm_gpuva *
  __drm_gpuva_next(struct 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström



On 9/14/23 12:57, Danilo Krummrich wrote:

On 9/13/23 14:16, Danilo Krummrich wrote:




And validate() can remove it while still holding all dma-resv locks,
neat!
However, what if two tasks are trying to lock the VA space
concurrently? What
do we do when the drm_gpuvm_bo's refcount drops to zero in
drm_gpuva_unlink()?
Are we guaranteed that at this point of time the drm_gpuvm_bo is not
on the
evicted list? Because otherwise we would call drm_gpuvm_bo_destroy()
with the
dma-resv lock held, which wouldn't be allowed, since
drm_gpuvm_bo_destroy()
might drop the last reference to the drm_gem_object and hence we'd
potentially
free the dma-resv lock while holding it, at least if it's an external
object.


Easiest way in this scheme is to think of the lists as being protected
by the vm's resv lock. That means anybody calling unlink() must also
hold the vm's resv lock. (Which is OK from an UAF point of view, but
perhaps not from a locking inversion POW from an async list update).


This would mean that on unlink() we'd need to hold the VM's resv lock 
and the
corresponding GEM's resv lock (in case they're not the same anyways) 
because the
VM's resv lock would protect the external / evicted object lists and 
the GEM

objects resv lock protects the GEM's list of drm_gpuvm_bos and the
drm_gpuvm_bo's list of drm_gpuvas.


As mentioned below the same applies for drm_gpuvm_bo_put() since it might
destroy the vm_bo, which includes removing the vm_bo from external / 
evicted

object lists and the GEMs list of vm_bos.

As mentioned, if the GEM's dma-resv is different from the VM's 
dma-resv we need
to take both locks. Ultimately, this would mean we need a drm_exec 
loop, because
we can't know the order in which to take these locks. Doing a full 
drm_exec loop

just to put() a vm_bo doesn't sound reasonable to me.

Can we instead just have an internal mutex for locking the lists such 
that we
avoid taking and dropping the spinlocks, which we use currently, in a 
loop?


You'd have the same locking inversion problem with a mutex, right? Since 
in the eviction path you have resv->mutex, from exec you have 
resv->mutex->resv because validate would attempt to grab resv.


That said, xe currently indeed does the vm+bo exec dance on vma put.

One reason why that seemingly horrible construct is good, is that when 
evicting an extobj and you need to access individual vmas to Zap page 
table entries or TLB flush, those VMAs are not allowed to go away (we're 
not refcounting them). Holding the bo resv on gpuva put prevents that 
from happening. Possibly one could use another mutex to protect the 
gem->vm_bo list to achieve the same, but we'd need to hold it on gpuva put.


/Thomas




- Danilo









For extobjs an outer lock would be enough in case of Xe, but I
really would not
like to add even more complexity just to get the spinlock out of
the way in case
the driver already has an outer lock protecting this path.


I must disagree here. These spinlocks and atomic operations are
pretty
costly and as discussed earlier this type of locking was the reason
(at
least according to the commit message) that made Christian drop the
XArray
use in drm_exec for the same set of objects: "The locking overhead
is
unecessary and measurable". IMHO the spinlock is the added
complexity and a
single wide lock following the drm locking guidelines set out by
Daniel and
David should really be the default choice with an opt-in for a
spinlock if
needed for async and pushing out to a wq is not an option.


For the external object list an outer lock would work as long as it's
not the
dma-resv lock of the corresponding GEM object, since here we actually
need to
remove the list entry from the external object list on
drm_gpuvm_bo_destroy().
It's just a bit weird design wise that drivers would need to take
this outer
lock on:

- drm_gpuvm_bo_extobj_add()
- drm_gpuvm_bo_destroy()(and hence also drm_gpuvm_bo_put())
- drm_gpuva_unlink()(because it needs to call
drm_gpuvm_bo_put())
- drm_gpuvm_exec_lock()
- drm_gpuvm_exec_lock_array()
- drm_gpuvm_prepare_range()

Given that it seems reasonable to do all the required locking
internally.


 From a design POW, there has been a clear direction in XE to make
things similar to mmap() / munmap(), so this outer lock, which in Xe is
an rwsem, is used in a similar way as the mmap_lock. It's protecting
the page-table structures and vma rb tree, the userptr structures and
the extobj list. Basically it's taken early in the exec IOCTL, the
VM_BIND ioctl, the compute rebind worker and the pagefault handler, so
all of the above are just asserting that it is taken in the correct
mode.

But strictly with this scheme one could also use the vm's dma_resv for
the extobj list since with drm_exec, it's locked before traversing the
list.

The whole point of this scheme is to rely on locks that you already are
supposed to be holding for various reasons and is simple to comprehend.


I don't agree that we're 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Danilo Krummrich

On 9/13/23 14:16, Danilo Krummrich wrote:




And validate() can remove it while still holding all dma-resv locks,
neat!
However, what if two tasks are trying to lock the VA space
concurrently? What
do we do when the drm_gpuvm_bo's refcount drops to zero in
drm_gpuva_unlink()?
Are we guaranteed that at this point of time the drm_gpuvm_bo is not
on the
evicted list? Because otherwise we would call drm_gpuvm_bo_destroy()
with the
dma-resv lock held, which wouldn't be allowed, since
drm_gpuvm_bo_destroy()
might drop the last reference to the drm_gem_object and hence we'd
potentially
free the dma-resv lock while holding it, at least if it's an external
object.


Easiest way in this scheme is to think of the lists as being protected
by the vm's resv lock. That means anybody calling unlink() must also
hold the vm's resv lock. (Which is OK from an UAF point of view, but
perhaps not from a locking inversion POW from an async list update).


This would mean that on unlink() we'd need to hold the VM's resv lock and the
corresponding GEM's resv lock (in case they're not the same anyways) because the
VM's resv lock would protect the external / evicted object lists and the GEM
objects resv lock protects the GEM's list of drm_gpuvm_bos and the
drm_gpuvm_bo's list of drm_gpuvas.


As mentioned below the same applies for drm_gpuvm_bo_put() since it might
destroy the vm_bo, which includes removing the vm_bo from external / evicted
object lists and the GEMs list of vm_bos.

As mentioned, if the GEM's dma-resv is different from the VM's dma-resv we need
to take both locks. Ultimately, this would mean we need a drm_exec loop, because
we can't know the order in which to take these locks. Doing a full drm_exec loop
just to put() a vm_bo doesn't sound reasonable to me.

Can we instead just have an internal mutex for locking the lists such that we
avoid taking and dropping the spinlocks, which we use currently, in a loop?

- Danilo









For extobjs an outer lock would be enough in case of Xe, but I
really would not
like to add even more complexity just to get the spinlock out of
the way in case
the driver already has an outer lock protecting this path.


I must disagree here. These spinlocks and atomic operations are
pretty
costly and as discussed earlier this type of locking was the reason
(at
least according to the commit message) that made Christian drop the
XArray
use in drm_exec for the same set of objects: "The locking overhead
is
unecessary and measurable". IMHO the spinlock is the added
complexity and a
single wide lock following the drm locking guidelines set out by
Daniel and
David should really be the default choice with an opt-in for a
spinlock if
needed for async and pushing out to a wq is not an option.


For the external object list an outer lock would work as long as it's
not the
dma-resv lock of the corresponding GEM object, since here we actually
need to
remove the list entry from the external object list on
drm_gpuvm_bo_destroy().
It's just a bit weird design wise that drivers would need to take
this outer
lock on:

- drm_gpuvm_bo_extobj_add()
- drm_gpuvm_bo_destroy()(and hence also drm_gpuvm_bo_put())
- drm_gpuva_unlink()(because it needs to call
drm_gpuvm_bo_put())
- drm_gpuvm_exec_lock()
- drm_gpuvm_exec_lock_array()
- drm_gpuvm_prepare_range()

Given that it seems reasonable to do all the required locking
internally.


 From a design POW, there has been a clear direction in XE to make
things similar to mmap() / munmap(), so this outer lock, which in Xe is
an rwsem, is used in a similar way as the mmap_lock. It's protecting
the page-table structures and vma rb tree, the userptr structures and
the extobj list. Basically it's taken early in the exec IOCTL, the
VM_BIND ioctl, the compute rebind worker and the pagefault handler, so
all of the above are just asserting that it is taken in the correct
mode.

But strictly with this scheme one could also use the vm's dma_resv for
the extobj list since with drm_exec, it's locked before traversing the
list.

The whole point of this scheme is to rely on locks that you already are
supposed to be holding for various reasons and is simple to comprehend.


I don't agree that we're supposed to hold the VM's resv lock anyways for
functions like drm_gpuvm_bo_put() or drm_gpuva_unlink(), but I'm fine using it
for that purpose nevertheless.





In order to at least place lockdep checks, the driver would need to
supply the
corresponding lock's lockdep_map, because the GPUVM otherwise doesn't
know about
the lock.


Yes, that sounds reasonable. One lockdep map per list.


I'd really like to avoid that, especially now that everything got simpler. We
should define the actual locks to take instead.





Out of curiosity, what is the overhead of a spin_lock() that doesn't
need to
spin?


I guess it's hard to tell exactly, but it is much lower on modern x86
than what it used to be. Not sure about ARM, which is the other
architecture important to us. I figure 

Re: [Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-14 Thread Thomas Hellström



On 9/14/23 10:20, Boris Brezillon wrote:

On Wed, 13 Sep 2023 15:22:56 +0200
Thomas Hellström  wrote:


On 9/13/23 13:33, Boris Brezillon wrote:

On Wed, 13 Sep 2023 12:39:01 +0200
Thomas Hellström  wrote:
  

Hi,

On 9/13/23 09:19, Boris Brezillon wrote:

On Wed, 13 Sep 2023 17:05:42 +1000
Dave Airlie  wrote:
 

On Wed, 13 Sept 2023 at 17:03, Boris Brezillon
 wrote:

On Tue, 12 Sep 2023 18:20:32 +0200
Thomas Hellström  wrote:


+/**
+ * get_next_vm_bo_from_list() - get the next vm_bo element
+ * @__gpuvm: The GPU VM
+ * @__list_name: The name of the list we're iterating on
+ * @__local_list: A pointer to the local list used to store already iterated 
items
+ * @__prev_vm_bo: The previous element we got from 
drm_gpuvm_get_next_cached_vm_bo()
+ *
+ * This helper is here to provide lockless list iteration. Lockless as in, the
+ * iterator releases the lock immediately after picking the first element from
+ * the list, so list insertion deletion can happen concurrently.

Are the list spinlocks needed for that async state update from within
the dma-fence critical section we've discussed previously?

Any driver calling _[un]link() from its drm_gpu_scheduler::run_job()
hook will be in this situation (Panthor at the moment, PowerVR soon). I
get that Xe and Nouveau don't need that because they update the VM
state early (in the ioctl path), but I keep thinking this will hurt us
if we don't think it through from the beginning, because once you've
set this logic to depend only on resv locks, it will be pretty hard to
get back to a solution which lets synchronous VM_BINDs take precedence
on asynchronous request, and, with vkQueueBindSparse() passing external
deps (plus the fact the VM_BIND queue might be pretty deep), it can
take a long time to get your synchronous VM_BIND executed...

So this would boil down to either (possibly opt-in) keeping the spinlock
approach or pushing the unlink out to a wq then?

Deferred _unlink() would not be an issue, since I already defer the
drm_gpuva destruction to a wq, it would just a be a matter of moving the
_unlink() call there as well. But _link() also takes the GEM gpuva list
lock, and that one is bit tricky, in that sm_map() can trigger 2 more
_link() calls for the prev/next mappings, which we can't guess until we
get to execute the VM update. If we mandate the use of the GEM resv
lock, that simply means async VM updates (AKA calling
drm_gpuvm_sm_[un]map()) are not an option. And if this is what everyone
agrees on, then I'd like the APIs that make this sort of async VM
update possible (drm_gpuvm_sm_[un]map(), the drm_gpuvm_ops::sm_step*
methods, and probably other things) to be dropped, so we don't make it
look like it's something we support.
  

BTW, as also asked in a reply to Danilo, how do you call unlink from
run_job() when it was requiring the obj->dma_resv lock, or was that a WIP?

_unlink() makes sure the GEM gpuva list lock is taken, but this can be
a custom lock (see drm_gem_gpuva_set_lock()). In panthor we have
panthor_gem_object::gpuva_list_lock that's dedicated the gpuva list
protection. We make sure we never take this lock while allocating
memory to guarantee the dma-signalling path can't deadlock.
  


btw what is the use case for this? do we have actual vulkan
applications we know will have problems here?

I don't, but I think that's a concern Faith raised at some point (dates
back from when I was reading threads describing how VM_BIND on i915
should work, and I was clearly discovering this whole VM_BIND thing at
that time, so maybe I misunderstood).
 

it feels like a bit of premature optimisation, but maybe we have use cases.

Might be, but that's the sort of thing that would put us in a corner if
we don't have a plan for when the needs arise. Besides, if we don't
want to support that case because it's too complicated, I'd recommend
dropping all the drm_gpuvm APIs that let people think this mode is
valid/supported (map/remap/unmap hooks in drm_gpuvm_ops,
drm_gpuvm_sm_[un]map helpers, etc). Keeping them around just adds to the
confusion.

Xe allows bypassing the bind-queue with another bind-queue, but to
completely avoid dependencies between queues the Operations may not
overlap.

So, you check the VM state with some VM lock held (would be the VM resv
in my case), and if the mapping is new (no overlaps with pre-existing
mappings), you queue it to the fast-track/sync-VM_BIND queue. What would
be missing I guess is a way to know if the mapping is active (MMU has
been updated) or pending (MMU update queued to the bind-queue), so I can
fast-track mapping/unmapping of active mappings.

Ok, so I started modifying the implementation, and quickly realized the
overlap test can't be done without your xe_range_fence tree because of
unmaps. Since we call drm_gpuva_unmap() early/in the IOCTL path (IOW,
before the mapping teardown is effective), we lose track of this
yet-to-be-executed-unmap operation, and if we do our

[Nouveau] [PATCH 43/44] drm/nouveau/kms/nv50-: create outputs based on nvkm info

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 97 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.h   |  2 -
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 31 +-
 drivers/gpu/drm/nouveau/include/nvif/outp.h   | 40 
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  2 +-
 drivers/gpu/drm/nouveau/nvif/outp.c   | 44 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |  5 +-
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  2 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 50 ++
 9 files changed, 223 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 642b6bcde121..57b6864fb9d0 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -66,8 +66,6 @@
 #include "nouveau_fence.h"
 #include "nv50_display.h"
 
-#include 
-
 /**
  * EVO channel
  */
@@ -1702,7 +1700,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
}
 
if (head->func->display_id)
-   head->func->display_id(head, BIT(nv_encoder->dcb->id));
+   head->func->display_id(head, BIT(nv_encoder->outp.id));
 
nv_encoder->update(nv_encoder, nv_crtc->index, asyh, proto, depth);
 }
@@ -1734,16 +1732,6 @@ nv50_sor_func = {
.destroy = nv50_sor_destroy,
 };
 
-bool nv50_has_mst(struct nouveau_drm *drm)
-{
-   struct nvkm_bios *bios = nvxx_bios(>client.device);
-   u32 data;
-   u8 ver, hdr, cnt, len;
-
-   data = nvbios_dp_table(bios, , , , );
-   return data && ver >= 0x40 && (nvbios_rd08(bios, data + 0x08) & 0x04);
-}
-
 static int
 nv50_sor_create(struct nouveau_encoder *nv_encoder)
 {
@@ -1796,15 +1784,15 @@ nv50_sor_create(struct nouveau_encoder *nv_encoder)
nv_encoder->i2c = _connector->aux.ddc;
}
 
-   if (nv_connector->type != DCB_CONNECTOR_eDP &&
-   nv50_has_mst(drm)) {
+   if (nv_connector->type != DCB_CONNECTOR_eDP && 
nv_encoder->outp.info.dp.mst) {
ret = nv50_mstm_new(nv_encoder, _connector->aux,
16, nv_connector->base.base.id,
_encoder->dp.mstm);
if (ret)
return ret;
}
-   } else {
+   } else
+   if (nv_encoder->outp.info.ddc != NVIF_OUTP_DDC_INVALID) {
struct nvkm_i2c_bus *bus =
nvkm_i2c_bus_find(i2c, dcbe->i2c_index);
if (bus)
@@ -1927,12 +1915,12 @@ nv50_pior_create(struct nouveau_encoder *nv_encoder)
 
switch (dcbe->type) {
case DCB_OUTPUT_TMDS:
-   bus  = nvkm_i2c_bus_find(i2c, NVKM_I2C_BUS_EXT(dcbe->extdev));
+   bus  = nvkm_i2c_bus_find(i2c, nv_encoder->outp.info.ddc);
ddc  = bus ? >i2c : NULL;
type = DRM_MODE_ENCODER_TMDS;
break;
case DCB_OUTPUT_DP:
-   aux  = nvkm_i2c_aux_find(i2c, NVKM_I2C_AUX_EXT(dcbe->extdev));
+   aux  = nvkm_i2c_aux_find(i2c, nv_encoder->outp.info.dp.aux);
ddc  = aux ? >i2c : NULL;
type = DRM_MODE_ENCODER_TMDS;
break;
@@ -2695,12 +2683,10 @@ int
 nv50_display_create(struct drm_device *dev)
 {
struct nouveau_drm *drm = nouveau_drm(dev);
-   struct dcb_table *dcb = >vbios.dcb;
struct drm_connector *connector, *tmp;
struct nv50_disp *disp;
-   struct dcb_output *dcbe;
int ret, i;
-   bool has_mst = nv50_has_mst(drm);
+   bool has_mst = false;
 
disp = kzalloc(sizeof(*disp), GFP_KERNEL);
if (!disp)
@@ -2777,54 +2763,75 @@ nv50_display_create(struct drm_device *dev)
}
 
/* create encoder/connector objects based on VBIOS DCB table */
-   for (i = 0, dcbe = >entry[0]; i < dcb->entries; i++, dcbe++) {
+   for_each_set_bit(i, >disp->outp_mask, 
sizeof(disp->disp->outp_mask) * 8) {
struct nouveau_encoder *outp;
 
outp = kzalloc(sizeof(*outp), GFP_KERNEL);
if (!outp)
break;
 
-   ret = nvif_outp_ctor(disp->disp, "kmsOutp", dcbe->id, 
>outp);
+   ret = nvif_outp_ctor(disp->disp, "kmsOutp", i, >outp);
if (ret) {
kfree(outp);
continue;
}
 
-   connector = nouveau_connector_create(dev, dcbe->connector);
+   connector = nouveau_connector_create(dev, outp->outp.info.conn);
if (IS_ERR(connector)) {

[Nouveau] [PATCH 44/44] drm/nouveau/kms/nv50-: disable dcb parsing

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- nvkm should provide all this info now
- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_bios.c| 8 +---
 drivers/gpu/drm/nouveau/nouveau_display.c | 8 
 drivers/gpu/drm/nouveau/nvif/disp.c   | 2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c 
b/drivers/gpu/drm/nouveau/nouveau_bios.c
index 189903b65edc..9e878cdc8e38 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
@@ -2093,9 +2093,11 @@ nouveau_bios_init(struct drm_device *dev)
if (!NVInitVBIOS(dev))
return -ENODEV;
 
-   ret = parse_dcb_table(dev, bios);
-   if (ret)
-   return ret;
+   if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) {
+   ret = parse_dcb_table(dev, bios);
+   if (ret)
+   return ret;
+   }
 
if (!bios->major_version)   /* we don't run version 0 bios */
return 0;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 99977e5fe716..d8c92521226d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -724,10 +724,10 @@ nouveau_display_create(struct drm_device *dev)
drm_kms_helper_poll_init(dev);
drm_kms_helper_poll_disable(dev);
 
-   if (nouveau_modeset != 2 && drm->vbios.dcb.entries) {
-   ret = nvif_disp_ctor(>client.device, "kmsDisp", 0,
->disp);
-   if (ret == 0) {
+   if (nouveau_modeset != 2) {
+   ret = nvif_disp_ctor(>client.device, "kmsDisp", 0, 
>disp);
+
+   if (!ret && (disp->disp.outp_mask || drm->vbios.dcb.entries)) {
nouveau_display_create_properties(dev);
if (disp->disp.object.oclass < NV50_DISP) {
dev->mode_config.fb_modifiers_not_supported = 
true;
diff --git a/drivers/gpu/drm/nouveau/nvif/disp.c 
b/drivers/gpu/drm/nouveau/nvif/disp.c
index 09915f2715af..097246e10cdb 100644
--- a/drivers/gpu/drm/nouveau/nvif/disp.c
+++ b/drivers/gpu/drm/nouveau/nvif/disp.c
@@ -60,7 +60,7 @@ nvif_disp_ctor(struct nvif_device *device, const char *name, 
s32 oclass, struct
cid = nvif_sclass(>object, disps, oclass);
disp->object.client = NULL;
if (cid < 0) {
-   NVIF_ERRON(cid, >object, "[NEW disp%04x] not 
supported", oclass);
+   NVIF_DEBUG(>object, "[NEW disp%04x] not supported", 
oclass);
return cid;
}
 
-- 
2.41.0



[Nouveau] [PATCH 41/44] drm/nouveau/kms/nv50-: name aux channels after their connector

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- removes use of VBIOS data for naming
- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 25 -
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index c2929ad64b60..34f5ad0fdfd6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -400,10 +400,8 @@ nouveau_connector_destroy(struct drm_connector *connector)
kfree(nv_connector->edid);
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
-   if (nv_connector->aux.transfer) {
+   if (nv_connector->aux.transfer)
drm_dp_cec_unregister_connector(_connector->aux);
-   kfree(nv_connector->aux.name);
-   }
nvif_conn_dtor(_connector->conn);
kfree(connector);
 }
@@ -1280,13 +1278,11 @@ struct drm_connector *
 nouveau_connector_create(struct drm_device *dev,
 const struct dcb_output *dcbe)
 {
-   const struct drm_connector_funcs *funcs = _connector_funcs;
struct nouveau_drm *drm = nouveau_drm(dev);
struct nouveau_display *disp = nouveau_display(dev);
struct nouveau_connector *nv_connector = NULL;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
-   char aux_name[48] = {0};
int index = dcbe->connector;
int type, ret = 0;
bool dummy;
@@ -1376,7 +1372,13 @@ nouveau_connector_create(struct drm_device *dev,
}
}
 
-   switch ((type = drm_conntype_from_dcb(nv_connector->type))) {
+   type = drm_conntype_from_dcb(nv_connector->type);
+   if (type == DRM_MODE_CONNECTOR_LVDS)
+   drm_connector_init(dev, connector, 
_connector_funcs_lvds, type);
+   else
+   drm_connector_init(dev, connector, _connector_funcs, 
type);
+
+   switch (type) {
case DRM_MODE_CONNECTOR_LVDS:
ret = nouveau_bios_parse_lvds_table(dev, 0, , );
if (ret) {
@@ -1385,24 +1387,16 @@ nouveau_connector_create(struct drm_device *dev,
return ERR_PTR(ret);
}
 
-   funcs = _connector_funcs_lvds;
break;
case DRM_MODE_CONNECTOR_DisplayPort:
case DRM_MODE_CONNECTOR_eDP:
nv_connector->aux.dev = connector->kdev;
nv_connector->aux.drm_dev = dev;
nv_connector->aux.transfer = nouveau_connector_aux_xfer;
-   snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
-dcbe->hasht, dcbe->hashm);
-   nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
-   if (!nv_connector->aux.name) {
-   kfree(nv_connector);
-   return ERR_PTR(-ENOMEM);
-   }
+   nv_connector->aux.name = connector->name;
drm_dp_aux_init(_connector->aux);
break;
default:
-   funcs = _connector_funcs;
break;
}
 
@@ -1417,7 +1411,6 @@ nouveau_connector_create(struct drm_device *dev,
connector->interlace_allowed = false;
connector->doublescan_allowed = false;
 
-   drm_connector_init(dev, connector, funcs, type);
drm_connector_helper_add(connector, _connector_helper_funcs);
connector->polled = DRM_CONNECTOR_POLL_CONNECT;
 
-- 
2.41.0



[Nouveau] [PATCH 40/44] drm/nouveau/kms/nv50-: create heads after outps/conns

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- output info will be used later to determine MST support

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 64 -
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 3d9a312838bf..76d3fd1dec77 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2776,38 +2776,6 @@ nv50_display_create(struct drm_device *dev)
dev->mode_config.cursor_height = 64;
}
 
-   /* create crtc objects to represent the hw heads */
-   for_each_set_bit(i, >disp->head_mask, 
sizeof(disp->disp->head_mask) * 8) {
-   struct nv50_head *head;
-
-   head = nv50_head_create(dev, i);
-   if (IS_ERR(head)) {
-   ret = PTR_ERR(head);
-   goto out;
-   }
-
-   if (has_mst) {
-   head->msto = nv50_msto_new(dev, head, i);
-   if (IS_ERR(head->msto)) {
-   ret = PTR_ERR(head->msto);
-   head->msto = NULL;
-   goto out;
-   }
-
-   /*
-* FIXME: This is a hack to workaround the following
-* issues:
-*
-* https://gitlab.gnome.org/GNOME/mutter/issues/759
-* 
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/277
-*
-* Once these issues are closed, this should be
-* removed
-*/
-   head->msto->encoder.possible_crtcs = 
disp->disp->head_mask;
-   }
-   }
-
/* create encoder/connector objects based on VBIOS DCB table */
for (i = 0, dcbe = >entry[0]; i < dcb->entries; i++, dcbe++) {
struct nouveau_encoder *outp;
@@ -2870,6 +2838,38 @@ nv50_display_create(struct drm_device *dev)
connector->funcs->destroy(connector);
}
 
+   /* create crtc objects to represent the hw heads */
+   for_each_set_bit(i, >disp->head_mask, 
sizeof(disp->disp->head_mask) * 8) {
+   struct nv50_head *head;
+
+   head = nv50_head_create(dev, i);
+   if (IS_ERR(head)) {
+   ret = PTR_ERR(head);
+   goto out;
+   }
+
+   if (has_mst) {
+   head->msto = nv50_msto_new(dev, head, i);
+   if (IS_ERR(head->msto)) {
+   ret = PTR_ERR(head->msto);
+   head->msto = NULL;
+   goto out;
+   }
+
+   /*
+* FIXME: This is a hack to workaround the following
+* issues:
+*
+* https://gitlab.gnome.org/GNOME/mutter/issues/759
+* 
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/277
+*
+* Once these issues are closed, this should be
+* removed
+*/
+   head->msto->encoder.possible_crtcs = 
disp->disp->head_mask;
+   }
+   }
+
/* Disable vblank irqs aggressively for power-saving, safe on nv50+ */
dev->vblank_disable_immediate = true;
 
-- 
2.41.0



[Nouveau] [PATCH 39/44] drm/nouveau/kms/nv50-: create heads based on nvkm head mask

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

No need to go poking HW directly, and probably shouldn't on GSP-RM.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2c1dd7106518..3d9a312838bf 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2694,13 +2694,12 @@ nv50_display_destroy(struct drm_device *dev)
 int
 nv50_display_create(struct drm_device *dev)
 {
-   struct nvif_device *device = _drm(dev)->client.device;
struct nouveau_drm *drm = nouveau_drm(dev);
struct dcb_table *dcb = >vbios.dcb;
struct drm_connector *connector, *tmp;
struct nv50_disp *disp;
struct dcb_output *dcbe;
-   int crtcs, ret, i;
+   int ret, i;
bool has_mst = nv50_has_mst(drm);
 
disp = kzalloc(sizeof(*disp), GFP_KERNEL);
@@ -2778,20 +2777,9 @@ nv50_display_create(struct drm_device *dev)
}
 
/* create crtc objects to represent the hw heads */
-   if (disp->disp->object.oclass >= GV100_DISP)
-   crtcs = nvif_rd32(>object, 0x610060) & 0xff;
-   else
-   if (disp->disp->object.oclass >= GF110_DISP)
-   crtcs = nvif_rd32(>object, 0x612004) & 0xf;
-   else
-   crtcs = 0x3;
-
-   for (i = 0; i < fls(crtcs); i++) {
+   for_each_set_bit(i, >disp->head_mask, 
sizeof(disp->disp->head_mask) * 8) {
struct nv50_head *head;
 
-   if (!(crtcs & (1 << i)))
-   continue;
-
head = nv50_head_create(dev, i);
if (IS_ERR(head)) {
ret = PTR_ERR(head);
@@ -2816,7 +2804,7 @@ nv50_display_create(struct drm_device *dev)
 * Once these issues are closed, this should be
 * removed
 */
-   head->msto->encoder.possible_crtcs = crtcs;
+   head->msto->encoder.possible_crtcs = 
disp->disp->head_mask;
}
}
 
-- 
2.41.0



[Nouveau] [PATCH 36/44] drm/nouveau/disp: add dp mst id get/put methods

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 32 ---
 drivers/gpu/drm/nouveau/dispnv50/head.h   |  1 +
 drivers/gpu/drm/nouveau/dispnv50/headc57d.c   | 14 
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 18 +++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  2 ++
 drivers/gpu/drm/nouveau/nvif/outp.c   | 29 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 15 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  2 ++
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 28 
 9 files changed, 137 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9cce1323338b..2c1dd7106518 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -869,6 +869,8 @@ struct nv50_msto {
struct nv50_mstc *mstc;
bool disabled;
bool enabled;
+
+   u32 display_id;
 };
 
 struct nouveau_encoder *nv50_real_outp(struct drm_encoder *encoder)
@@ -893,10 +895,17 @@ nv50_msto_cleanup(struct drm_atomic_state *state,
struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
struct drm_dp_mst_atomic_payload *payload =
drm_atomic_get_mst_payload_state(mst_state, msto->mstc->port);
+   struct nv50_mstc *mstc = msto->mstc;
+   struct nv50_mstm *mstm = mstc->mstm;
 
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
 
if (msto->disabled) {
+   if (msto->head->func->display_id) {
+   nvif_outp_dp_mst_id_put(>outp->outp, 
msto->display_id);
+   msto->display_id = 0;
+   }
+
msto->mstc = NULL;
msto->disabled = false;
} else if (msto->enabled) {
@@ -1039,6 +1048,11 @@ nv50_msto_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
nouveau_dp_train(mstm->outp, true, 0, 0);
}
 
+   if (head->func->display_id) {
+   if (!WARN_ON(nvif_outp_dp_mst_id_get(>outp->outp, 
>display_id)))
+   head->func->display_id(head, msto->display_id);
+   }
+
if (mstm->outp->outp.or.link & 1)
proto = NV917D_SOR_SET_CONTROL_PROTOCOL_DP_A;
else
@@ -1059,6 +1073,9 @@ nv50_msto_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *s
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
 
+   if (msto->head->func->display_id)
+   msto->head->func->display_id(msto->head, 0);
+
mstm->outp->update(mstm->outp, msto->head->base.index, NULL, 0, 0);
mstm->modified = true;
if (!--mstm->links)
@@ -1542,7 +1559,7 @@ static void
 nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state 
*state)
 {
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
-   struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc);
+   struct nv50_head *head = nv50_head(nv_encoder->crtc);
struct nouveau_connector *nv_connector = 
nv50_outp_get_old_connector(state, nv_encoder);
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
@@ -1561,7 +1578,7 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
 #endif
 
if (nv_encoder->dcb->type == DCB_OUTPUT_TMDS && 
nv_encoder->hdmi.enabled) {
-   nvif_outp_hdmi(_encoder->outp, nv_crtc->index,
+   nvif_outp_hdmi(_encoder->outp, head->base.index,
   false, 0, 0, 0, false, false, false);
nv_encoder->hdmi.enabled = false;
}
@@ -1569,8 +1586,11 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
if (nv_encoder->dcb->type == DCB_OUTPUT_DP)
nouveau_dp_power_down(nv_encoder);
 
-   nv_encoder->update(nv_encoder, nv_crtc->index, NULL, 0, 0);
-   nv50_audio_disable(encoder, nv_crtc);
+   if (head->func->display_id)
+   head->func->display_id(head, 0);
+
+   nv_encoder->update(nv_encoder, head->base.index, NULL, 0, 0);
+   nv50_audio_disable(encoder, >base);
nv_encoder->crtc = NULL;
 }
 
@@ -1583,6 +1603,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
nv50_head_atom(drm_atomic_get_new_crtc_state(state, 
_crtc->base));
struct drm_display_mode *mode = >state.adjusted_mode;
struct nv50_disp *disp = nv50_disp(encoder->dev);
+   struct nv50_head *head = nv50_head(_crtc->base);
struct nvif_outp *outp = _encoder->outp;
struct drm_device *dev = encoder->dev;
struct nouveau_drm *drm = nouveau_drm(dev);
@@ -1680,6 +1701,9 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct 

[Nouveau] [PATCH 38/44] drm/nouveau/disp: move outp init/fini paths to chipset code

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- pre-nv5x doesn't use any of this
- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 .../gpu/drm/nouveau/nvkm/engine/disp/base.c   | 31 +++
 .../gpu/drm/nouveau/nvkm/engine/disp/conn.c   | 10 --
 .../gpu/drm/nouveau/nvkm/engine/disp/conn.h   |  2 --
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/nv50.c   | 14 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c   | 20 ++--
 6 files changed, 22 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
index 1dbe68f9a0e0..39f7e7ce9f4a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
@@ -102,18 +102,14 @@ static int
 nvkm_disp_fini(struct nvkm_engine *engine, bool suspend)
 {
struct nvkm_disp *disp = nvkm_disp(engine);
-   struct nvkm_conn *conn;
struct nvkm_outp *outp;
 
if (disp->func->fini)
disp->func->fini(disp);
 
list_for_each_entry(outp, >outps, head) {
-   nvkm_outp_fini(outp);
-   }
-
-   list_for_each_entry(conn, >conns, head) {
-   nvkm_conn_fini(conn);
+   if (outp->func->fini)
+   outp->func->fini(outp);
}
 
return 0;
@@ -123,16 +119,12 @@ static int
 nvkm_disp_init(struct nvkm_engine *engine)
 {
struct nvkm_disp *disp = nvkm_disp(engine);
-   struct nvkm_conn *conn;
struct nvkm_outp *outp;
struct nvkm_ior *ior;
 
-   list_for_each_entry(conn, >conns, head) {
-   nvkm_conn_init(conn);
-   }
-
list_for_each_entry(outp, >outps, head) {
-   nvkm_outp_init(outp);
+   if (outp->func->init)
+   outp->func->init(outp);
}
 
if (disp->func->init) {
@@ -156,9 +148,7 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
 {
struct nvkm_disp *disp = nvkm_disp(engine);
struct nvkm_subdev *subdev = >engine.subdev;
-   struct nvkm_outp *outp;
struct nvkm_head *head;
-   struct nvkm_ior *ior;
int ret, i;
 
if (disp->func->oneinit) {
@@ -167,19 +157,6 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
return ret;
}
 
-   /* Enforce identity-mapped SOR assignment for panels, which have
-* certain bits (ie. backlight controls) wired to a specific SOR.
-*/
-   list_for_each_entry(outp, >outps, head) {
-   if (outp->conn->info.type == DCB_CONNECTOR_LVDS ||
-   outp->conn->info.type == DCB_CONNECTOR_eDP) {
-   ior = nvkm_ior_find(disp, SOR, ffs(outp->info.or) - 1);
-   if (!WARN_ON(!ior))
-   ior->identity = true;
-   outp->identity = true;
-   }
-   }
-
i = 0;
list_for_each_entry(head, >heads, head)
i = max(i, head->id + 1);
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.c
index fbdae1137864..ff88a5a5253a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.c
@@ -29,16 +29,6 @@
 
 #include 
 
-void
-nvkm_conn_fini(struct nvkm_conn *conn)
-{
-}
-
-void
-nvkm_conn_init(struct nvkm_conn *conn)
-{
-}
-
 void
 nvkm_conn_del(struct nvkm_conn **pconn)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.h 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.h
index a0600e72b0ec..01c3146c7066 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/conn.h
@@ -19,8 +19,6 @@ struct nvkm_conn {
 int nvkm_conn_new(struct nvkm_disp *, int index, struct nvbios_connE *,
  struct nvkm_conn **);
 void nvkm_conn_del(struct nvkm_conn **);
-void nvkm_conn_init(struct nvkm_conn *);
-void nvkm_conn_fini(struct nvkm_conn *);
 
 #define CONN_MSG(c,l,f,a...) do {  
\
struct nvkm_conn *_conn = (c);\
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
index aaa7796946ce..b35fae96d855 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
@@ -603,6 +603,7 @@ nvkm_dp_fini(struct nvkm_outp *outp)
 static void
 nvkm_dp_init(struct nvkm_outp *outp)
 {
+   nvkm_outp_init(outp);
nvkm_dp_enable(outp, outp->dp.enabled);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
index 7343b24f10eb..4be09ec4fd5c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
@@ -1586,6 +1586,7 @@ nv50_disp_oneinit(struct nvkm_disp *disp)

[Nouveau] [PATCH 37/44] drm/nouveau/disp: move outp/conn construction to chipset code

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- pre-nv5x doesn't use any of this, has its own version DRM-side
- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 .../gpu/drm/nouveau/nvkm/engine/disp/base.c   | 117 +
 .../gpu/drm/nouveau/nvkm/engine/disp/nv50.c   | 122 +-
 2 files changed, 121 insertions(+), 118 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
index 73104b59f97f..1dbe68f9a0e0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
@@ -23,15 +23,12 @@
  */
 #include "priv.h"
 #include "conn.h"
-#include "dp.h"
 #include "head.h"
 #include "ior.h"
 #include "outp.h"
 
 #include 
 #include 
-#include 
-#include 
 
 #include 
 #include 
@@ -159,123 +156,11 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
 {
struct nvkm_disp *disp = nvkm_disp(engine);
struct nvkm_subdev *subdev = >engine.subdev;
-   struct nvkm_bios *bios = subdev->device->bios;
-   struct nvkm_outp *outp, *outt, *pair;
-   struct nvkm_conn *conn;
+   struct nvkm_outp *outp;
struct nvkm_head *head;
struct nvkm_ior *ior;
-   struct nvbios_connE connE;
-   struct dcb_output dcbE;
-   u8  hpd = 0, ver, hdr;
-   u32 data;
int ret, i;
 
-   /* Create output path objects for each VBIOS display path. */
-   i = -1;
-   while ((data = dcb_outp_parse(bios, ++i, , , ))) {
-   if (ver < 0x40) /* No support for chipsets prior to NV50. */
-   break;
-   if (dcbE.type == DCB_OUTPUT_UNUSED)
-   continue;
-   if (dcbE.type == DCB_OUTPUT_EOL)
-   break;
-   outp = NULL;
-
-   switch (dcbE.type) {
-   case DCB_OUTPUT_ANALOG:
-   case DCB_OUTPUT_TV:
-   case DCB_OUTPUT_TMDS:
-   case DCB_OUTPUT_LVDS:
-   ret = nvkm_outp_new(disp, i, , );
-   break;
-   case DCB_OUTPUT_DP:
-   ret = nvkm_dp_new(disp, i, , );
-   break;
-   case DCB_OUTPUT_WFD:
-   /* No support for WFD yet. */
-   ret = -ENODEV;
-   continue;
-   default:
-   nvkm_warn(subdev, "dcb %d type %d unknown\n",
- i, dcbE.type);
-   continue;
-   }
-
-   if (ret) {
-   if (outp) {
-   if (ret != -ENODEV)
-   OUTP_ERR(outp, "ctor failed: %d", ret);
-   else
-   OUTP_DBG(outp, "not supported");
-   nvkm_outp_del();
-   continue;
-   }
-   nvkm_error(subdev, "failed to create outp %d\n", i);
-   continue;
-   }
-
-   list_add_tail(>head, >outps);
-   hpd = max(hpd, (u8)(dcbE.connector + 1));
-   }
-
-   /* Create connector objects based on available output paths. */
-   list_for_each_entry_safe(outp, outt, >outps, head) {
-   /* VBIOS data *should* give us the most useful information. */
-   data = nvbios_connEp(bios, outp->info.connector, , ,
-);
-
-   /* No bios connector data... */
-   if (!data) {
-   /* Heuristic: anything with the same ccb index is
-* considered to be on the same connector, any
-* output path without an associated ccb entry will
-* be put on its own connector.
-*/
-   int ccb_index = outp->info.i2c_index;
-   if (ccb_index != 0xf) {
-   list_for_each_entry(pair, >outps, head) {
-   if (pair->info.i2c_index == ccb_index) {
-   outp->conn = pair->conn;
-   break;
-   }
-   }
-   }
-
-   /* Connector shared with another output path. */
-   if (outp->conn)
-   continue;
-
-   memset(, 0x00, sizeof(connE));
-   connE.type = DCB_CONNECTOR_NONE;
-   i = -1;
-   } else {
-   i = outp->info.connector;
-   }
-
-   /* Check that we haven't already created this connector. */
-   list_for_each_entry(conn, >conns, head) {
-   if 

[Nouveau] [PATCH 35/44] drm/nouveau/disp: add dp sst config method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

This is presently unused on HW, we read a bunch of regs and calculate
the watermark during the second supervisor interrupt.

I don't want to change this yet as I need to re-remember how older HW
works exactly, but RM wants this info via RPC.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 12 +++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  1 +
 drivers/gpu/drm/nouveau/nvif/outp.c   | 18 
 .../gpu/drm/nouveau/nvkm/engine/disp/ior.h|  2 ++
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 21 +++
 5 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 00ce0a46c152..6fb297b65ae8 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -36,6 +36,7 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_DP_RATES  0x72
 #define NVIF_OUTP_V0_DP_TRAIN  0x73
 #define NVIF_OUTP_V0_DP_DRIVE  0x74
+#define NVIF_OUTP_V0_DP_SST0x75
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
 union nvif_outp_detect_args {
@@ -222,6 +223,17 @@ union nvif_outp_dp_drive_args {
} v0;
 };
 
+union nvif_outp_dp_sst_args {
+   struct nvif_outp_dp_sst_v0 {
+   __u8  version;
+   __u8  head;
+   __u8  pad02[2];
+   __u32 watermark;
+   __u32 hblanksym;
+   __u32 vblanksym;
+   } v0;
+};
+
 union nvif_outp_dp_mst_vcpi_args {
struct nvif_outp_dp_mst_vcpi_v0 {
__u8  version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index b4f97fabecbd..881cbed5f0ee 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -68,6 +68,7 @@ int nvif_outp_dp_train(struct nvif_outp *, u8 
dpcd[DP_RECEIVER_CAP_SIZE],
   u8 lttprs, u8 link_nr, u32 link_bw, bool mst, bool 
post_lt_adj,
   bool retrain);
 int nvif_outp_dp_drive(struct nvif_outp *, u8 link_nr, u8 pe[4], u8 vs[4]);
+int nvif_outp_dp_sst(struct nvif_outp *, int head, u32 watermark, u32 
hblanksym, u32 vblanksym);
 int nvif_outp_dp_mst_vcpi(struct nvif_outp *, int head,
  u8 start_slot, u8 num_slots, u16 pbn, u16 
aligned_pbn);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 5fe5523587e6..952103aa93b7 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -46,6 +46,24 @@ nvif_outp_dp_mst_vcpi(struct nvif_outp *outp, int head,
return ret;
 }
 
+int
+nvif_outp_dp_sst(struct nvif_outp *outp, int head, u32 watermark, u32 
hblanksym, u32 vblanksym)
+{
+   struct nvif_outp_dp_sst_v0 args;
+   int ret;
+
+   args.version = 0;
+   args.head = head;
+   args.watermark = watermark;
+   args.hblanksym = hblanksym;
+   args.vblanksym = vblanksym;
+   ret = nvif_object_mthd(>object, NVIF_OUTP_V0_DP_SST, , 
sizeof(args));
+   NVIF_ERRON(ret, >object,
+  "[DP_SST head:%d watermark:%d hblanksym:%d vblanksym:%d]",
+  args.head, args.watermark, args.hblanksym, args.vblanksym);
+   return ret;
+}
+
 int
 nvif_outp_dp_drive(struct nvif_outp *outp, u8 link_nr, u8 pe[4], u8 vs[4])
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
index 8686e5c044a5..9beb9d1e8633 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
@@ -84,6 +84,8 @@ struct nvkm_ior_func {
void (*pattern)(struct nvkm_ior *, int pattern);
void (*drive)(struct nvkm_ior *, int ln, int pc,
  int dc, int pe, int tx_pu);
+   int (*sst)(struct nvkm_ior *, int head, bool ef,
+  u32 watermark, u32 hblanksym, u32 vblanksym);
void (*vcpi)(struct nvkm_ior *, int head, u8 slot,
 u8 slot_nr, u16 pbn, u16 aligned);
void (*audio)(struct nvkm_ior *, int head, bool enable);
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index b634e76c2a9b..225f88fbdae0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -45,6 +45,26 @@ nvkm_uoutp_mthd_dp_mst_vcpi(struct nvkm_outp *outp, void 
*argv, u32 argc)
return 0;
 }
 
+static int
+nvkm_uoutp_mthd_dp_sst(struct nvkm_outp *outp, void *argv, u32 argc)
+{
+   union nvif_outp_dp_sst_args *args = argv;
+   struct nvkm_disp *disp = outp->disp;
+   struct nvkm_ior *ior = outp->ior;
+
+   if (argc != sizeof(args->v0) || args->v0.version != 0)
+   return -ENOSYS;
+
+   if (!ior->func->dp || 

[Nouveau] [PATCH 34/44] drm/nouveau/disp: add support for post-LT adjust

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- fixes regression from previous commit

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 11 +++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  1 +
 drivers/gpu/drm/nouveau/nouveau_dp.c  | 72 ++-
 drivers/gpu/drm/nouveau/nvif/outp.c   | 16 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 17 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 14 
 7 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 14972b942be7..00ce0a46c152 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -35,6 +35,7 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_DP_AUX_XFER   0x71
 #define NVIF_OUTP_V0_DP_RATES  0x72
 #define NVIF_OUTP_V0_DP_TRAIN  0x73
+#define NVIF_OUTP_V0_DP_DRIVE  0x74
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
 union nvif_outp_detect_args {
@@ -211,6 +212,16 @@ union nvif_outp_dp_train_args {
} v0;
 };
 
+union nvif_outp_dp_drive_args {
+   struct nvif_outp_dp_drive_v0 {
+   __u8  version;
+   __u8  pad01[2];
+   __u8  lanes;
+   __u8  pe[4];
+   __u8  vs[4];
+   } v0;
+};
+
 union nvif_outp_dp_mst_vcpi_args {
struct nvif_outp_dp_mst_vcpi_v0 {
__u8  version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index 9a78483e0289..b4f97fabecbd 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -67,6 +67,7 @@ int nvif_outp_dp_rates(struct nvif_outp *, struct 
nvif_outp_dp_rate *rate, int r
 int nvif_outp_dp_train(struct nvif_outp *, u8 dpcd[DP_RECEIVER_CAP_SIZE],
   u8 lttprs, u8 link_nr, u32 link_bw, bool mst, bool 
post_lt_adj,
   bool retrain);
+int nvif_outp_dp_drive(struct nvif_outp *, u8 link_nr, u8 pe[4], u8 vs[4]);
 int nvif_outp_dp_mst_vcpi(struct nvif_outp *, int head,
  u8 start_slot, u8 num_slots, u16 pbn, u16 
aligned_pbn);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c 
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index cd03c29c1063..7de7707ec6a8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -320,15 +320,83 @@ nouveau_dp_power_down(struct nouveau_encoder *outp)
 static bool
 nouveau_dp_train_link(struct nouveau_encoder *outp, bool retrain)
 {
-   int ret;
+   struct drm_dp_aux *aux = >conn->aux;
+   bool post_lt = false;
+   int ret, retries = 0;
 
+   if ( (outp->dp.dpcd[DP_MAX_LANE_COUNT] & 0x20) &&
+   !(outp->dp.dpcd[DP_MAX_DOWNSPREAD] & DP_TPS4_SUPPORTED))
+   post_lt = true;
+
+retry:
ret = nvif_outp_dp_train(>outp, outp->dp.dpcd,
  outp->dp.lttpr.nr,
  outp->dp.lt.nr,
  outp->dp.lt.bw,
  outp->dp.lt.mst,
- false,
+ post_lt,
  retrain);
+   if (ret)
+   return false;
+
+   if (post_lt) {
+   u8 stat[DP_LINK_STATUS_SIZE];
+   u8 prev[2];
+   u8 time = 0, adjusts = 0, tmp;
+
+   ret = drm_dp_dpcd_read_phy_link_status(aux, DP_PHY_DPRX, stat);
+   if (ret)
+   return false;
+
+   for (;;) {
+   if (!drm_dp_channel_eq_ok(stat, outp->dp.lt.nr)) {
+   ret = 1;
+   break;
+   }
+
+   if (!(stat[2] & 0x02))
+   break;
+
+   msleep(5);
+   time += 5;
+
+   memcpy(prev, [4], sizeof(prev));
+   ret = drm_dp_dpcd_read_phy_link_status(aux, 
DP_PHY_DPRX, stat);
+   if (ret)
+   break;
+
+   if (!memcmp(prev, [4], sizeof(prev))) {
+   if (time > 200)
+   break;
+   } else {
+   u8 pe[4], vs[4];
+
+   if (adjusts++ == 6)
+   break;
+
+   for (int i = 0; i < outp->dp.lt.nr; i++) {
+   pe[i] = 
drm_dp_get_adjust_request_pre_emphasis(stat, i) >>
+   
DP_TRAIN_PRE_EMPHASIS_SHIFT;
+   vs[i] = 

[Nouveau] [PATCH 33/44] drm/nouveau/disp: move link training out of supervisor

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nouveau_dp.c  |  50 ++-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 137 +++---
 .../gpu/drm/nouveau/nvkm/engine/disp/nv50.c   |  13 --
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c   |  18 ++-
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |   7 +-
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  |  13 +-
 6 files changed, 91 insertions(+), 147 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c 
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 9280daf32534..cd03c29c1063 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -336,15 +336,44 @@ nouveau_dp_train_link(struct nouveau_encoder *outp, bool 
retrain)
 bool
 nouveau_dp_train(struct nouveau_encoder *outp, bool mst, u32 khz, u8 bpc)
 {
-   bool ret;
+   struct nouveau_drm *drm = nouveau_drm(outp->base.base.dev);
+   struct drm_dp_aux *aux = >conn->aux;
+   u32 min_rate;
+   u8 pwr;
+   bool ret = true;
+
+   if (mst)
+   min_rate = outp->dp.link_nr * outp->dp.rate[0].rate;
+   else
+   min_rate = DIV_ROUND_UP(khz * bpc * 3, 8);
+
+   NV_DEBUG(drm, "%s link training (mst:%d min_rate:%d)\n",
+outp->base.base.name, mst, min_rate);
 
mutex_lock(>dp.hpd_irq_lock);
 
-   outp->dp.lt.nr = outp->dp.link_nr;
-   outp->dp.lt.bw = 0;
-   outp->dp.lt.mst = mst;
-   ret = nouveau_dp_train_link(outp, false);
+   if (drm_dp_dpcd_readb(aux, DP_SET_POWER, ) == 1) {
+   if ((pwr & DP_SET_POWER_MASK) != DP_SET_POWER_D0) {
+   pwr &= ~DP_SET_POWER_MASK;
+   pwr |=  DP_SET_POWER_D0;
+   drm_dp_dpcd_writeb(aux, DP_SET_POWER, pwr);
+   }
+   }
+
+   for (int nr = outp->dp.link_nr; nr; nr >>= 1) {
+   for (int rate = 0; rate < outp->dp.rate_nr; rate++) {
+   if (outp->dp.rate[rate].rate * nr >= min_rate) {
+   outp->dp.lt.nr = nr;
+   outp->dp.lt.bw = outp->dp.rate[rate].rate;
+   outp->dp.lt.mst = mst;
+   if (nouveau_dp_train_link(outp, false))
+   goto done;
+   }
+   }
+   }
 
+   ret = false;
+done:
mutex_unlock(>dp.hpd_irq_lock);
return ret;
 }
@@ -352,6 +381,17 @@ nouveau_dp_train(struct nouveau_encoder *outp, bool mst, 
u32 khz, u8 bpc)
 static bool
 nouveau_dp_link_check_locked(struct nouveau_encoder *outp)
 {
+   u8 link_status[DP_LINK_STATUS_SIZE];
+
+   if (!outp || !outp->dp.lt.nr)
+   return true;
+
+   if (drm_dp_dpcd_read_phy_link_status(>conn->aux, DP_PHY_DPRX, 
link_status) < 0)
+   return false;
+
+   if (drm_dp_channel_eq_ok(link_status, outp->dp.lt.nr))
+   return true;
+
return nouveau_dp_train_link(outp, true);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
index b59cd2d4550f..6f08ff8b1fba 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
@@ -455,71 +455,44 @@ nvkm_dp_train_init(struct nvkm_outp *outp)
 }
 
 static int
-nvkm_dp_train_(struct nvkm_outp *outp, bool retrain)
+nvkm_dp_train(struct nvkm_outp *outp, bool retrain)
 {
-   if (retrain) {
-   if (!atomic_read(>dp.lt.done))
-   return 0;
+   struct nvkm_ior *ior = outp->ior;
+   int ret, rate;
 
-   return outp->func->acquire(outp);
+   for (rate = 0; rate < outp->dp.rates; rate++) {
+   if (outp->dp.rate[rate].rate == (retrain ? ior->dp.bw : 
outp->dp.lt.bw) * 27000)
+   break;
}
 
-   return 0;
-}
-
-static int
-nvkm_dp_train(struct nvkm_outp *outp, u32 dataKBps)
-{
-   struct nvkm_ior *ior = outp->ior;
-   int ret = -EINVAL, nr, rate;
-   u8  pwr;
+   if (WARN_ON(rate == outp->dp.rates))
+   return -EINVAL;
 
/* Retraining link?  Skip source configuration, it can mess up the 
active modeset. */
-   if (atomic_read(>dp.lt.done)) {
-   for (rate = 0; rate < outp->dp.rates; rate++) {
-   if (outp->dp.rate[rate].rate == ior->dp.bw * 27000)
-   return nvkm_dp_train_link(outp, ret);
-   }
-   WARN_ON(1);
-   return -EINVAL;
+   if (retrain) {
+   mutex_lock(>dp.mutex);
+   ret = nvkm_dp_train_link(outp, rate);
+   mutex_unlock(>dp.mutex);
+   return ret;
}
 
-   /* Ensure sink is not in a low-power state. */
-   if (!nvkm_rdaux(outp->dp.aux, DPCD_SC00, , 1)) {
-   if ((pwr & DPCD_SC00_SET_POWER) != 

[Nouveau] [PATCH 32/44] drm/nouveau/disp: add dp train method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- passes DPCD information from DRM to NVKM
- removes NVKM's own sink caps handling
- link still trained from supervisor, more patches to come

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |   6 +-
 drivers/gpu/drm/nouveau/include/nvif/if0012.h |  25 ++--
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |   7 +-
 drivers/gpu/drm/nouveau/nouveau_dp.c  |  75 +--
 drivers/gpu/drm/nouveau/nouveau_encoder.h |   7 ++
 drivers/gpu/drm/nouveau/nvif/outp.c   |  38 +++---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 118 +++---
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |   2 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  |  41 +++---
 9 files changed, 143 insertions(+), 176 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index be4c45fd3999..9cce1323338b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1036,7 +1036,7 @@ nv50_msto_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
 
if (!mstm->links++) {
nvif_outp_acquire_sor(>outp->outp, false /*TODO: MST 
audio... */);
-   nvif_outp_acquire_dp(>outp->outp, mstm->outp->dp.dpcd, 0, 
0, false, true);
+   nouveau_dp_train(mstm->outp, true, 0, 0);
}
 
if (mstm->outp->outp.or.link & 1)
@@ -1659,7 +1659,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
nvif_outp_lvds(_encoder->outp, lvds_dual, lvds_8bpc);
break;
case DCB_OUTPUT_DP:
-   nvif_outp_acquire_dp(_encoder->outp, nv_encoder->dp.dpcd, 0, 
0, hda, false);
+   nouveau_dp_train(nv_encoder, false, mode->clock, asyh->or.bpc);
depth = nv50_dp_bpc_to_depth(asyh->or.bpc);
 
if (nv_encoder->outp.or.link & 1)
@@ -1850,7 +1850,7 @@ nv50_pior_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
break;
case DCB_OUTPUT_DP:
ctrl |= NVDEF(NV507D, PIOR_SET_CONTROL, PROTOCOL, EXT_TMDS_ENC);
-   nvif_outp_acquire_dp(_encoder->outp, nv_encoder->dp.dpcd, 0, 
0, false, false);
+   nouveau_dp_train(nv_encoder, false, 
asyh->state.adjusted_mode.clock, 6);
break;
default:
BUG();
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index ddc8e3d85823..14972b942be7 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -34,7 +34,7 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_DP_AUX_PWR0x70
 #define NVIF_OUTP_V0_DP_AUX_XFER   0x71
 #define NVIF_OUTP_V0_DP_RATES  0x72
-#define NVIF_OUTP_V0_DP_RETRAIN0x73
+#define NVIF_OUTP_V0_DP_TRAIN  0x73
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
 union nvif_outp_detect_args {
@@ -71,7 +71,6 @@ union nvif_outp_acquire_args {
 #define NVIF_OUTP_ACQUIRE_V0_DAC  0x00
 #define NVIF_OUTP_ACQUIRE_V0_SOR  0x01
 #define NVIF_OUTP_ACQUIRE_V0_PIOR 0x02
-#define NVIF_OUTP_ACQUIRE_V0_DP  0x04
__u8 type;
__u8 or;
__u8 link;
@@ -80,14 +79,6 @@ union nvif_outp_acquire_args {
struct {
__u8 hda;
} sor;
-   struct {
-   __u8 link_nr; /* 0 = highest possible. */
-   __u8 link_bw; /* 0 = highest possible, DP BW 
code otherwise. */
-   __u8 hda;
-   __u8 mst;
-   __u8 pad04[4];
-   __u8 dpcd[DP_RECEIVER_CAP_SIZE];
-   } dp;
};
} v0;
 };
@@ -207,9 +198,17 @@ union nvif_outp_dp_rates_args {
} v0;
 };
 
-union nvif_outp_dp_retrain_args {
-   struct nvif_outp_dp_retrain_vn {
-   } vn;
+union nvif_outp_dp_train_args {
+   struct nvif_outp_dp_train_v0 {
+   __u8  version;
+   __u8  retrain;
+   __u8  mst;
+   __u8  lttprs;
+   __u8  post_lt_adj;
+   __u8  link_nr;
+   __u32 link_bw;
+   __u8 dpcd[DP_RECEIVER_CAP_SIZE];
+   } v0;
 };
 
 union nvif_outp_dp_mst_vcpi_args {
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index 596d543acd30..9a78483e0289 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -31,8 +31,6 @@ int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
 int nvif_outp_acquire_dac(struct nvif_outp *);
 int nvif_outp_acquire_sor(struct nvif_outp *, bool hda);
 int nvif_outp_acquire_pior(struct nvif_outp *);
-int nvif_outp_acquire_dp(struct nvif_outp 

[Nouveau] [PATCH 29/44] drm/nouveau/kms/nv50-: split DP disable+enable into two modesets

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Link training can finally be moved out of the supervisor sequence,
but first we need to split DP modesets into separate disable and
enable sequences to be able to perform link training between them
instead.

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d52735cd9d38..b7e9f951eee3 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2377,7 +2377,8 @@ nv50_disp_outp_atomic_check_clr(struct nv50_atom *atom,
if (IS_ERR(outp))
return PTR_ERR(outp);
 
-   if (outp->encoder->encoder_type == DRM_MODE_ENCODER_DPMST) {
+   if (outp->encoder->encoder_type == DRM_MODE_ENCODER_DPMST ||
+   nouveau_encoder(outp->encoder)->dcb->type == DCB_OUTPUT_DP) 
{
outp->flush_disable = true;
atom->flush_disable = true;
}
-- 
2.41.0



[Nouveau] [PATCH 31/44] drm/nouveau/kms/nv50-: fixup sink D3 before tearing down link

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- fixes bug preventing this on SST
- implement for MST

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 13 +++--
 drivers/gpu/drm/nouveau/nouveau_dp.c  | 15 +++
 drivers/gpu/drm/nouveau/nouveau_encoder.h |  1 +
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d52320965b50..be4c45fd3999 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1298,6 +1298,7 @@ nv50_mstm_cleanup(struct drm_atomic_state *state,
}
 
if (mstm->disabled) {
+   nouveau_dp_power_down(mstm->outp);
nvif_outp_release(>outp->outp);
mstm->disabled = false;
}
@@ -1549,7 +1550,6 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
 #endif
struct drm_dp_aux *aux = _connector->aux;
int ret;
-   u8 pwr;
 
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
if (backlight && backlight->uses_dpcd) {
@@ -1566,15 +1566,8 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
nv_encoder->hdmi.enabled = false;
}
 
-   if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
-   ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, );
-
-   if (ret == 0) {
-   pwr &= ~DP_SET_POWER_MASK;
-   pwr |=  DP_SET_POWER_D3;
-   drm_dp_dpcd_writeb(aux, DP_SET_POWER, pwr);
-   }
-   }
+   if (nv_encoder->dcb->type == DCB_OUTPUT_DP)
+   nouveau_dp_power_down(nv_encoder);
 
nv_encoder->update(nv_encoder, nv_crtc->index, NULL, 0, 0);
nv50_audio_disable(encoder, nv_crtc);
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c 
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index f26769bca195..1c0b992fe241 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -284,6 +284,21 @@ nouveau_dp_detect(struct nouveau_connector *nv_connector,
return ret;
 }
 
+void
+nouveau_dp_power_down(struct nouveau_encoder *outp)
+{
+   struct drm_dp_aux *aux = >conn->aux;
+   int ret;
+   u8 pwr;
+
+   ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, );
+   if (ret == 1) {
+   pwr &= ~DP_SET_POWER_MASK;
+   pwr |=  DP_SET_POWER_D3;
+   drm_dp_dpcd_writeb(aux, DP_SET_POWER, pwr);
+   }
+}
+
 bool
 nouveau_dp_link_check(struct nouveau_connector *nv_connector)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h 
b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 123d0ecf5f58..ed31db58176c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -155,6 +155,7 @@ enum nouveau_dp_status {
 };
 
 int nouveau_dp_detect(struct nouveau_connector *, struct nouveau_encoder *);
+void nouveau_dp_power_down(struct nouveau_encoder *);
 bool nouveau_dp_link_check(struct nouveau_connector *);
 void nouveau_dp_irq(struct work_struct *);
 enum drm_mode_status nv50_dp_mode_valid(struct nouveau_encoder *,
-- 
2.41.0



[Nouveau] [PATCH 30/44] drm/nouveau/kms/nv50-: flush mst disables together

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- fixes some issues tearing down modes on tiled displays

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 11 +--
 drivers/gpu/drm/nouveau/dispnv50/disp.h |  1 -
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index b7e9f951eee3..d52320965b50 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2086,13 +2086,6 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
help->atomic_disable(encoder, state);
outp->disabled = true;
interlock[NV50_DISP_INTERLOCK_CORE] |= 1;
-   if (outp->flush_disable) {
-   nv50_disp_atomic_commit_wndw(state, interlock);
-   nv50_disp_atomic_commit_core(state, interlock);
-   memset(interlock, 0x00, sizeof(interlock));
-
-   flushed = true;
-   }
}
}
 
@@ -2378,10 +2371,8 @@ nv50_disp_outp_atomic_check_clr(struct nv50_atom *atom,
return PTR_ERR(outp);
 
if (outp->encoder->encoder_type == DRM_MODE_ENCODER_DPMST ||
-   nouveau_encoder(outp->encoder)->dcb->type == DCB_OUTPUT_DP) 
{
-   outp->flush_disable = true;
+   nouveau_encoder(outp->encoder)->dcb->type == DCB_OUTPUT_DP)
atom->flush_disable = true;
-   }
outp->clr.ctrl = true;
atom->lock_core = true;
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h 
b/drivers/gpu/drm/nouveau/dispnv50/disp.h
index 42209f5b06f9..1e5601223c75 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h
@@ -83,7 +83,6 @@ struct nv50_outp_atom {
struct list_head head;
 
struct drm_encoder *encoder;
-   bool flush_disable;
 
bool disabled;
bool enabled;
-- 
2.41.0



[Nouveau] [PATCH 28/44] drm/nouveau/disp: add dp rates method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- moves building of link rates table from NVKM to DRM
- preparing to move link training out of supervisor

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h |  13 ++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |   8 +
 drivers/gpu/drm/nouveau/nouveau_dp.c  | 143 ++
 drivers/gpu/drm/nouveau/nouveau_encoder.h |  12 +-
 drivers/gpu/drm/nouveau/nvif/outp.c   |  21 +++
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |  66 
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |   1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  |  24 +++
 8 files changed, 189 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 94f1e55b0ce6..ddc8e3d85823 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -33,6 +33,7 @@ union nvif_outp_args {
 
 #define NVIF_OUTP_V0_DP_AUX_PWR0x70
 #define NVIF_OUTP_V0_DP_AUX_XFER   0x71
+#define NVIF_OUTP_V0_DP_RATES  0x72
 #define NVIF_OUTP_V0_DP_RETRAIN0x73
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
@@ -194,6 +195,18 @@ union nvif_outp_dp_aux_xfer_args {
} v0;
 };
 
+union nvif_outp_dp_rates_args {
+   struct nvif_outp_dp_rates_v0 {
+   __u8  version;
+   __u8  pad01[6];
+   __u8  rates;
+   struct {
+   __s8  dpcd;
+   __u32 rate;
+   } rate[8];
+   } v0;
+};
+
 union nvif_outp_dp_retrain_args {
struct nvif_outp_dp_retrain_vn {
} vn;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index dd4dd0e2a7a1..596d543acd30 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -59,6 +59,14 @@ int nvif_outp_hda_eld(struct nvif_outp *, int head, void 
*data, u32 size);
 
 int nvif_outp_dp_aux_pwr(struct nvif_outp *, bool enable);
 int nvif_outp_dp_aux_xfer(struct nvif_outp *, u8 type, u8 *size, u32 addr, u8 
*data);
+
+struct nvif_outp_dp_rate {
+   int dpcd; /* -1 for non-indexed rates */
+   u32 rate;
+};
+
+int nvif_outp_dp_rates(struct nvif_outp *, struct nvif_outp_dp_rate *rate, int 
rate_nr);
+
 int nvif_outp_dp_retrain(struct nvif_outp *);
 int nvif_outp_dp_mst_vcpi(struct nvif_outp *, int head,
  u8 start_slot, u8 num_slots, u16 pbn, u16 
aligned_pbn);
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c 
b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 01aa9b9c74a2..f26769bca195 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -42,6 +42,21 @@ nouveau_dp_has_sink_count(struct drm_connector *connector,
return drm_dp_read_sink_count_cap(connector, outp->dp.dpcd, 
>dp.desc);
 }
 
+static bool
+nouveau_dp_probe_lttpr(struct nouveau_encoder *outp)
+{
+   u8 rev, size = sizeof(rev);
+   int ret;
+
+   ret = nvif_outp_dp_aux_xfer(>outp, DP_AUX_NATIVE_READ, ,
+   
DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV,
+   );
+   if (ret || size < sizeof(rev) || rev < 0x14)
+   return false;
+
+   return true;
+}
+
 static enum drm_connector_status
 nouveau_dp_probe_dpcd(struct nouveau_connector *nv_connector,
  struct nouveau_encoder *outp)
@@ -53,10 +68,99 @@ nouveau_dp_probe_dpcd(struct nouveau_connector 
*nv_connector,
int ret;
u8 *dpcd = outp->dp.dpcd;
 
+   outp->dp.lttpr.nr = 0;
+   outp->dp.rate_nr  = 0;
+   outp->dp.link_nr  = 0;
+   outp->dp.link_bw  = 0;
+
+   if (connector->connector_type != DRM_MODE_CONNECTOR_eDP &&
+   nouveau_dp_probe_lttpr(outp) &&
+   !drm_dp_read_dpcd_caps(aux, dpcd) &&
+   !drm_dp_read_lttpr_common_caps(aux, dpcd, outp->dp.lttpr.caps)) {
+   int nr = drm_dp_lttpr_count(outp->dp.lttpr.caps);
+
+   if (nr > 0)
+   outp->dp.lttpr.nr = nr;
+   }
+
ret = drm_dp_read_dpcd_caps(aux, dpcd);
if (ret < 0)
goto out;
 
+   outp->dp.link_nr = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
+   if (outp->dcb->dpconf.link_nr < outp->dp.link_nr)
+   outp->dp.link_nr = outp->dcb->dpconf.link_nr;
+
+   if (outp->dp.lttpr.nr) {
+   int links = drm_dp_lttpr_max_lane_count(outp->dp.lttpr.caps);
+
+   if (links && links < outp->dp.link_nr)
+   outp->dp.link_nr = links;
+   }
+
+   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && 
dpcd[DP_DPCD_REV] >= 0x13) {
+   __le16 rates[DP_MAX_SUPPORTED_RATES];
+
+   ret = drm_dp_dpcd_read(aux, DP_SUPPORTED_LINK_RATES, rates, 
sizeof(rates));
+   if (ret == sizeof(rates)) {
+   for (int i = 0; 

[Nouveau] [PATCH 27/44] drm/nouveau/disp: add dp aux xfer method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 32 +--
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 12 +++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  2 ++
 drivers/gpu/drm/nouveau/nouveau_connector.c   | 12 ++-
 drivers/gpu/drm/nouveau/nouveau_encoder.h |  1 -
 drivers/gpu/drm/nouveau/nvif/outp.c   | 24 ++
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 14 
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 27 +---
 9 files changed, 94 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index a574b4315ac2..d52735cd9d38 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1702,14 +1702,13 @@ nv50_sor_destroy(struct drm_encoder *encoder)
 {
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
 
-   nvif_outp_dtor(_encoder->outp);
-
nv50_mstm_del(_encoder->dp.mstm);
drm_encoder_cleanup(encoder);
 
if (nv_encoder->dcb->type == DCB_OUTPUT_DP)
mutex_destroy(_encoder->dp.hpd_irq_lock);
 
+   nvif_outp_dtor(_encoder->outp);
kfree(encoder);
 }
 
@@ -1762,22 +1761,22 @@ nv50_sor_create(struct nouveau_encoder *nv_encoder)
nv50_outp_dump_caps(drm, nv_encoder);
 
if (dcbe->type == DCB_OUTPUT_DP) {
-   struct nvkm_i2c_aux *aux =
-   nvkm_i2c_aux_find(i2c, dcbe->i2c_index);
-
mutex_init(_encoder->dp.hpd_irq_lock);
 
-   if (aux) {
-   if (disp->disp->object.oclass < GF110_DISP) {
-   /* HW has no support for address-only
-* transactions, so we're required to
-* use custom I2C-over-AUX code.
-*/
-   nv_encoder->i2c = >i2c;
-   } else {
-   nv_encoder->i2c = _connector->aux.ddc;
-   }
-   nv_encoder->aux = aux;
+   if (disp->disp->object.oclass < GF110_DISP) {
+   /* HW has no support for address-only
+* transactions, so we're required to
+* use custom I2C-over-AUX code.
+*/
+   struct nvkm_i2c_aux *aux;
+
+   aux = nvkm_i2c_aux_find(i2c, dcbe->i2c_index);
+   if (!aux)
+   return -EINVAL;
+
+   nv_encoder->i2c = >i2c;
+   } else {
+   nv_encoder->i2c = _connector->aux.ddc;
}
 
if (nv_connector->type != DCB_CONNECTOR_eDP &&
@@ -1925,7 +1924,6 @@ nv50_pior_create(struct nouveau_encoder *nv_encoder)
}
 
nv_encoder->i2c = ddc;
-   nv_encoder->aux = aux;
 
mutex_init(_encoder->dp.hpd_irq_lock);
 
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index ee4cec541a90..94f1e55b0ce6 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -32,6 +32,7 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_HDA_ELD   0x61
 
 #define NVIF_OUTP_V0_DP_AUX_PWR0x70
+#define NVIF_OUTP_V0_DP_AUX_XFER   0x71
 #define NVIF_OUTP_V0_DP_RETRAIN0x73
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
@@ -182,6 +183,17 @@ union nvif_outp_dp_aux_pwr_args {
} v0;
 };
 
+union nvif_outp_dp_aux_xfer_args {
+   struct nvif_outp_dp_aux_xfer_v0 {
+   __u8  version;
+   __u8  pad01;
+   __u8  type;
+   __u8  size;
+   __u32 addr;
+   __u8  data[16];
+   } v0;
+};
+
 union nvif_outp_dp_retrain_args {
struct nvif_outp_dp_retrain_vn {
} vn;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index 0ddaec9416ee..dd4dd0e2a7a1 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -56,7 +56,9 @@ int nvif_outp_hdmi(struct nvif_outp *, int head, bool enable, 
u8 max_ac_packet,
 
 int nvif_outp_infoframe(struct nvif_outp *, u8 type, struct 
nvif_outp_infoframe_v0 *, u32 size);
 int nvif_outp_hda_eld(struct nvif_outp *, int head, void *data, u32 size);
+
 int nvif_outp_dp_aux_pwr(struct nvif_outp *, bool enable);
+int nvif_outp_dp_aux_xfer(struct nvif_outp *, u8 type, u8 *size, u32 addr, u8 
*data);
 int nvif_outp_dp_retrain(struct nvif_outp *);
 int nvif_outp_dp_mst_vcpi(struct nvif_outp *, int head,
  u8 start_slot, u8 num_slots, u16 pbn, u16 
aligned_pbn);
diff --git 

[Nouveau] [PATCH 26/44] drm/nouveau/disp: move dp aux pwr method to HAL

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c| 9 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h  | 4 
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 6 +++---
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
index 0e6e38800376..99fe7ef07a44 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c
@@ -41,6 +41,14 @@
  */
 #define AMPERE_IED_HACK(disp) ((disp)->engine.subdev.device->card_type >= 
GA100)
 
+static int
+nvkm_dp_aux_pwr(struct nvkm_outp *outp, bool pu)
+{
+   outp->dp.enabled = pu;
+   nvkm_dp_enable(outp, outp->dp.enabled);
+   return 0;
+}
+
 struct lt_state {
struct nvkm_outp *outp;
 
@@ -814,6 +822,7 @@ nvkm_dp_func = {
.disable = nvkm_dp_disable,
.bl.get = nvkm_outp_bl_get,
.bl.set = nvkm_outp_bl_set,
+   .dp.aux_pwr = nvkm_dp_aux_pwr,
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
index 38b6b43a9f20..513794a278a9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
@@ -104,6 +104,10 @@ struct nvkm_outp_func {
int (*get)(struct nvkm_outp *);
int (*set)(struct nvkm_outp *, int level);
} bl;
+
+   struct {
+   int (*aux_pwr)(struct nvkm_outp *, bool pu);
+   } dp;
 };
 
 #define OUTP_MSG(o,l,f,a...) do {  
\
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index 7574f2200644..6ca364e953bd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -75,10 +75,10 @@ nvkm_uoutp_mthd_dp_aux_pwr(struct nvkm_outp *outp, void 
*argv, u32 argc)
 
if (argc != sizeof(args->v0) || args->v0.version != 0)
return -ENOSYS;
+   if (!outp->func->dp.aux_pwr)
+   return -EINVAL;
 
-   outp->dp.enabled = !!args->v0.state;
-   nvkm_dp_enable(outp, outp->dp.enabled);
-   return 0;
+   return outp->func->dp.aux_pwr(outp, !!args->v0.state);
 }
 
 static int
-- 
2.41.0



[Nouveau] [PATCH 25/44] drm/nouveau/disp: add hdmi audio hal function

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

This just adds a hook for RM to use, HW paths remain untouched, but
should probably be cleaned up to use this too at some point.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h   |  1 +
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 10 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
index 6e750890bcc9..8686e5c044a5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ior.h
@@ -74,6 +74,7 @@ struct nvkm_ior_func {
 bool scrambling_low_rates);
void (*infoframe_avi)(struct nvkm_ior *, int head, void *data, 
u32 size);
void (*infoframe_vsi)(struct nvkm_ior *, int head, void *data, 
u32 size);
+   void (*audio)(struct nvkm_ior *, int head, bool enable);
} *hdmi;
 
const struct nvkm_ior_func_dp {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index ad75dc5c50cf..7574f2200644 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -99,12 +99,20 @@ nvkm_uoutp_mthd_hda_eld(struct nvkm_outp *outp, void *argv, 
u32 argc)
if (argc && args->v0.data[0]) {
if (outp->info.type == DCB_OUTPUT_DP)
ior->func->dp->audio(ior, args->v0.head, true);
+   else
+   if (ior->func->hdmi->audio)
+   ior->func->hdmi->audio(ior, args->v0.head, true);
+
ior->func->hda->hpd(ior, args->v0.head, true);
ior->func->hda->eld(ior, args->v0.head, args->v0.data, argc);
} else {
+   ior->func->hda->hpd(ior, args->v0.head, false);
+
if (outp->info.type == DCB_OUTPUT_DP)
ior->func->dp->audio(ior, args->v0.head, false);
-   ior->func->hda->hpd(ior, args->v0.head, false);
+   else
+   if (ior->func->hdmi->audio)
+   ior->func->hdmi->audio(ior, args->v0.head, false);
}
 
return 0;
-- 
2.41.0



[Nouveau] [PATCH 24/44] drm/nouveau/disp: add output lvds config method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- was previously part of acquire()

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  2 +-
 drivers/gpu/drm/nouveau/include/nvif/if0012.h| 16 ++--
 drivers/gpu/drm/nouveau/include/nvif/outp.h  |  3 ++-
 drivers/gpu/drm/nouveau/nvif/outp.c  | 15 +++
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 14 --
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 7b238f6599e2..a574b4315ac2 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1663,7 +1663,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
lvds_8bpc = true;
}
 
-   nvif_outp_acquire_lvds(_encoder->outp, lvds_dual, lvds_8bpc);
+   nvif_outp_lvds(_encoder->outp, lvds_dual, lvds_8bpc);
break;
case DCB_OUTPUT_DP:
nvif_outp_acquire_dp(_encoder->outp, nv_encoder->dp.dpcd, 0, 
0, hda, false);
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index f878784593b4..ee4cec541a90 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -24,6 +24,8 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_BL_GET0x30
 #define NVIF_OUTP_V0_BL_SET0x31
 
+#define NVIF_OUTP_V0_LVDS  0x40
+
 #define NVIF_OUTP_V0_HDMI  0x50
 
 #define NVIF_OUTP_V0_INFOFRAME 0x60
@@ -67,7 +69,6 @@ union nvif_outp_acquire_args {
 #define NVIF_OUTP_ACQUIRE_V0_DAC  0x00
 #define NVIF_OUTP_ACQUIRE_V0_SOR  0x01
 #define NVIF_OUTP_ACQUIRE_V0_PIOR 0x02
-#define NVIF_OUTP_ACQUIRE_V0_LVDS0x03
 #define NVIF_OUTP_ACQUIRE_V0_DP  0x04
__u8 type;
__u8 or;
@@ -77,11 +78,6 @@ union nvif_outp_acquire_args {
struct {
__u8 hda;
} sor;
-   struct {
-   __u8 dual;
-   __u8 bpc8;
-   __u8 pad02[6];
-   } lvds;
struct {
__u8 link_nr; /* 0 = highest possible. */
__u8 link_bw; /* 0 = highest possible, DP BW 
code otherwise. */
@@ -135,6 +131,14 @@ union nvif_outp_bl_set_args {
} v0;
 };
 
+union nvif_outp_lvds_args {
+   struct nvif_outp_lvds_v0 {
+   __u8  version;
+   __u8  dual;
+   __u8  bpc8;
+   } v0;
+};
+
 union nvif_outp_hdmi_args {
struct nvif_outp_hdmi_v0 {
__u8 version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index ef63d22b62f8..0ddaec9416ee 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -31,7 +31,6 @@ int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
 int nvif_outp_acquire_dac(struct nvif_outp *);
 int nvif_outp_acquire_sor(struct nvif_outp *, bool hda);
 int nvif_outp_acquire_pior(struct nvif_outp *);
-int nvif_outp_acquire_lvds(struct nvif_outp *, bool dual, bool bpc8);
 int nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
 int link_nr, int link_bw, bool hda, bool mst);
 int nvif_outp_inherit_rgb_crt(struct nvif_outp *outp, u8 *proto_out);
@@ -50,6 +49,8 @@ nvif_outp_acquired(struct nvif_outp *outp)
 int nvif_outp_bl_get(struct nvif_outp *);
 int nvif_outp_bl_set(struct nvif_outp *, int level);
 
+int nvif_outp_lvds(struct nvif_outp *, bool dual, bool bpc8);
+
 int nvif_outp_hdmi(struct nvif_outp *, int head, bool enable, u8 
max_ac_packet, u8 rekey, u32 khz,
   bool scdc, bool scdc_scrambling, bool scdc_low_rates);
 
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 5a3c0dd7d532..dbb0986f0555 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -150,18 +150,17 @@ nvif_outp_hdmi(struct nvif_outp *outp, int head, bool 
enable, u8 max_ac_packet,
 }
 
 int
-nvif_outp_acquire_lvds(struct nvif_outp *outp, bool dual, bool bpc8)
+nvif_outp_lvds(struct nvif_outp *outp, bool dual, bool bpc8)
 {
-   struct nvif_outp_acquire_v0 args;
+   struct nvif_outp_lvds_v0 args;
int ret;
 
-   args.lvds.dual = dual;
-   args.lvds.bpc8 = bpc8;
+   args.version = 0;
+   args.dual = dual;
+   args.bpc8 = bpc8;
 
-   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_LVDS, );
-   NVIF_ERRON(ret, >object,
-  "[ACQUIRE proto:LVDS dual:%d 8bpc:%d] or:%d link:%d",
-  args.lvds.dual, args.lvds.bpc8, args.or, args.link);
+   

[Nouveau] [PATCH 23/44] drm/nouveau/disp: add output backlight control methods

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 17 
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  3 +
 drivers/gpu/drm/nouveau/nouveau_backlight.c   | 90 ++-
 drivers/gpu/drm/nouveau/nvif/outp.c   | 27 ++
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |  2 +
 .../gpu/drm/nouveau/nvkm/engine/disp/g84.c|  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/g94.c|  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/ga102.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gf119.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gk104.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gm107.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gm200.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gp100.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gt215.c  | 38 
 .../gpu/drm/nouveau/nvkm/engine/disp/gv100.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/ior.h|  7 ++
 .../gpu/drm/nouveau/nvkm/engine/disp/mcp89.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/nv50.c   | 32 +++
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c   | 38 
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  8 ++
 .../gpu/drm/nouveau/nvkm/engine/disp/tu102.c  |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 41 +
 22 files changed, 233 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 230084d675ec..f878784593b4 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -21,6 +21,9 @@ union nvif_outp_args {
 
 #define NVIF_OUTP_V0_LOAD_DETECT   0x20
 
+#define NVIF_OUTP_V0_BL_GET0x30
+#define NVIF_OUTP_V0_BL_SET0x31
+
 #define NVIF_OUTP_V0_HDMI  0x50
 
 #define NVIF_OUTP_V0_INFOFRAME 0x60
@@ -118,6 +121,20 @@ union nvif_outp_release_args {
} vn;
 };
 
+union nvif_outp_bl_get_args {
+   struct nvif_outp_bl_get_v0 {
+   __u8  version;
+   __u8  level;
+   } v0;
+};
+
+union nvif_outp_bl_set_args {
+   struct nvif_outp_bl_set_v0 {
+   __u8  version;
+   __u8  level;
+   } v0;
+};
+
 union nvif_outp_hdmi_args {
struct nvif_outp_hdmi_v0 {
__u8 version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index ea60d418d7f0..ef63d22b62f8 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -47,6 +47,9 @@ nvif_outp_acquired(struct nvif_outp *outp)
return outp->or.id >= 0;
 }
 
+int nvif_outp_bl_get(struct nvif_outp *);
+int nvif_outp_bl_set(struct nvif_outp *, int level);
+
 int nvif_outp_hdmi(struct nvif_outp *, int head, bool enable, u8 
max_ac_packet, u8 rekey, u32 khz,
   bool scdc, bool scdc_scrambling, bool scdc_low_rates);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c 
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 91b5ecc57538..d47442125fa1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -109,42 +109,6 @@ nv40_backlight_init(struct nouveau_encoder *encoder,
return 0;
 }
 
-static int
-nv50_get_intensity(struct backlight_device *bd)
-{
-   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
-   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
-   struct nvif_object *device = >client.device.object;
-   int or = ffs(nv_encoder->dcb->or) - 1;
-   u32 div = 1025;
-   u32 val;
-
-   val  = nvif_rd32(device, NV50_PDISP_SOR_PWM_CTL(or));
-   val &= NV50_PDISP_SOR_PWM_CTL_VAL;
-   return ((val * 100) + (div / 2)) / div;
-}
-
-static int
-nv50_set_intensity(struct backlight_device *bd)
-{
-   struct nouveau_encoder *nv_encoder = bl_get_data(bd);
-   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
-   struct nvif_object *device = >client.device.object;
-   int or = ffs(nv_encoder->dcb->or) - 1;
-   u32 div = 1025;
-   u32 val = (bd->props.brightness * div) / 100;
-
-   nvif_wr32(device, NV50_PDISP_SOR_PWM_CTL(or),
- NV50_PDISP_SOR_PWM_CTL_NEW | val);
-   return 0;
-}
-
-static const struct backlight_ops nv50_bl_ops = {
-   .options = BL_CORE_SUSPENDRESUME,
-   .get_brightness = nv50_get_intensity,
-   .update_status = nv50_set_intensity,
-};
-
 /*
  * eDP brightness callbacks need to happen under lock, since we need to
  * enable/disable the backlight ourselves for modesets
@@ -238,53 +202,25 @@ static const struct backlight_ops nv50_edp_bl_ops = {
 };
 
 static int
-nva3_get_intensity(struct backlight_device *bd)
+nv50_get_intensity(struct backlight_device *bd)
 {
struct nouveau_encoder *nv_encoder = bl_get_data(bd);
-   struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
-   struct 

[Nouveau] [PATCH 22/44] drm/nouveau/disp: remove SOR routing updates from supervisor

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- these shouldn't be necessary now, and are done in acquire()/release()
- preparation for GSP-RM, where we don't control the supervisor

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/gf119.c | 1 -
 drivers/gpu/drm/nouveau/nvkm/engine/disp/gv100.c | 1 -
 drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c  | 1 -
 3 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gf119.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gf119.c
index a48e9bdf4cd0..937baae6a3eb 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gf119.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gf119.c
@@ -1038,7 +1038,6 @@ gf119_disp_super(struct work_struct *work)
continue;
nv50_disp_super_2_0(disp, head);
}
-   nvkm_outp_route(disp);
list_for_each_entry(head, >heads, head) {
if (!(mask[head->id] & 0x0001))
continue;
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gv100.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gv100.c
index 4ebc030e40d1..7ac59bab6309 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gv100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gv100.c
@@ -863,7 +863,6 @@ gv100_disp_super(struct work_struct *work)
continue;
nv50_disp_super_2_0(disp, head);
}
-   nvkm_outp_route(disp);
list_for_each_entry(head, >heads, head) {
if (!(mask[head->id] & 0x0001))
continue;
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
index be8116802960..a4ce60517789 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/nv50.c
@@ -1371,7 +1371,6 @@ nv50_disp_super(struct work_struct *work)
continue;
nv50_disp_super_2_0(disp, head);
}
-   nvkm_outp_route(disp);
list_for_each_entry(head, >heads, head) {
if (!(super & (0x0200 << head->id)))
continue;
-- 
2.41.0



[Nouveau] [PATCH 21/44] drm/nouveau/disp: release outputs post-modeset

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Prior to this commit, KMS would call release() prior to modeset, and the
second supervisor interrupt would update SOR routing if needed.

Now, KMS will call release() post-modeset and update routing immediately.

- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c| 18 --
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c|  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c   |  2 ++
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 15ddb92c31fb..7b238f6599e2 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -477,7 +477,6 @@ nv50_dac_atomic_disable(struct drm_encoder *encoder, struct 
drm_atomic_state *st
 
core->func->dac->ctrl(core, nv_encoder->outp.or.id, ctrl, NULL);
nv_encoder->crtc = NULL;
-   nvif_outp_release(_encoder->outp);
 }
 
 static void
@@ -1298,6 +1297,11 @@ nv50_mstm_cleanup(struct drm_atomic_state *state,
}
}
 
+   if (mstm->disabled) {
+   nvif_outp_release(>outp->outp);
+   mstm->disabled = false;
+   }
+
mstm->modified = false;
 }
 
@@ -1332,12 +1336,6 @@ nv50_mstm_prepare(struct drm_atomic_state *state,
nv50_msto_prepare(state, mst_state, >mgr, 
msto);
}
}
-
-   if (mstm->disabled) {
-   if (!mstm->links)
-   nvif_outp_release(>outp->outp);
-   mstm->disabled = false;
-   }
 }
 
 static struct drm_connector *
@@ -1580,7 +1578,6 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
 
nv_encoder->update(nv_encoder, nv_crtc->index, NULL, 0, 0);
nv50_audio_disable(encoder, nv_crtc);
-   nvif_outp_release(_encoder->outp);
nv_encoder->crtc = NULL;
 }
 
@@ -1825,7 +1822,6 @@ nv50_pior_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *s
 
core->func->pior->ctrl(core, nv_encoder->outp.or.id, ctrl, NULL);
nv_encoder->crtc = NULL;
-   nvif_outp_release(_encoder->outp);
 }
 
 static void
@@ -1992,8 +1988,10 @@ nv50_disp_atomic_commit_core(struct drm_atomic_state 
*state, u32 *interlock)
  nv_encoder->conn, NULL, NULL);
outp->enabled = outp->disabled = false;
} else {
-   if (outp->disabled)
+   if (outp->disabled) {
+   nvif_outp_release(_encoder->outp);
outp->disabled = false;
+   }
}
}
}
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
index b288ea6658da..20a013f1bbba 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
@@ -238,6 +238,7 @@ void
 nvkm_outp_release(struct nvkm_outp *outp)
 {
nvkm_outp_release_or(outp, NVKM_OUTP_USER);
+   nvkm_outp_route(outp->disp);
 }
 
 void
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index ffd174091454..40cbb4ddc037 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -188,6 +188,8 @@ nvkm_uoutp_mthd_release(struct nvkm_outp *outp, void *argv, 
u32 argc)
 
if (argc != sizeof(args->vn))
return -ENOSYS;
+   if (!outp->ior)
+   return -EINVAL;
 
nvkm_outp_release(outp);
return 0;
-- 
2.41.0



[Nouveau] [PATCH 20/44] drm/nouveau/disp: move hdmi disable out of release()

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- release() is being moved post-modeset, preserve hdmi behaviour for now

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  8 
 drivers/gpu/drm/nouveau/nouveau_encoder.h|  6 +-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 15 +++
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index a4e1525ac48e..15ddb92c31fb 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -842,6 +842,8 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
size = 0;
 
nvif_outp_infoframe(_encoder->outp, NVIF_OUTP_INFOFRAME_V0_VSI, 
, size);
+
+   nv_encoder->hdmi.enabled = true;
 }
 
 /**
@@ -1560,6 +1562,12 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
}
 #endif
 
+   if (nv_encoder->dcb->type == DCB_OUTPUT_TMDS && 
nv_encoder->hdmi.enabled) {
+   nvif_outp_hdmi(_encoder->outp, nv_crtc->index,
+  false, 0, 0, 0, false, false, false);
+   nv_encoder->hdmi.enabled = false;
+   }
+
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, );
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h 
b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index ea8ef10e71aa..b3a9415ba879 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -69,7 +69,11 @@ struct nouveau_encoder {
 
struct nv04_output_reg restore;
 
-   union {
+   struct {
+   struct {
+   bool enabled;
+   } hdmi;
+
struct {
struct nv50_mstm *mstm;
int link_nr;
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index 8ba96323e1de..ffd174091454 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -154,6 +154,13 @@ nvkm_uoutp_mthd_hdmi(struct nvkm_outp *outp, void *argv, 
u32 argc)
(args->v0.scdc && !ior->func->hdmi->scdc))
return -EINVAL;
 
+   if (!args->v0.enable) {
+   ior->func->hdmi->infoframe_avi(ior, args->v0.head, NULL, 0);
+   ior->func->hdmi->infoframe_vsi(ior, args->v0.head, NULL, 0);
+   ior->func->hdmi->ctrl(ior, args->v0.head, false, 0, 0);
+   return 0;
+   }
+
ior->func->hdmi->ctrl(ior, args->v0.head, args->v0.enable,
  args->v0.max_ac_packet, args->v0.rekey);
if (ior->func->hdmi->scdc)
@@ -177,19 +184,11 @@ nvkm_uoutp_mthd_acquire_lvds(struct nvkm_outp *outp, bool 
dual, bool bpc8)
 static int
 nvkm_uoutp_mthd_release(struct nvkm_outp *outp, void *argv, u32 argc)
 {
-   struct nvkm_head *head = outp->asy.head;
-   struct nvkm_ior *ior = outp->ior;
union nvif_outp_release_args *args = argv;
 
if (argc != sizeof(args->vn))
return -ENOSYS;
 
-   if (ior->func->hdmi && head) {
-   ior->func->hdmi->infoframe_avi(ior, head->id, NULL, 0);
-   ior->func->hdmi->infoframe_vsi(ior, head->id, NULL, 0);
-   ior->func->hdmi->ctrl(ior, head->id, false, 0, 0);
-   }
-
nvkm_outp_release(outp);
return 0;
 }
-- 
2.41.0



[Nouveau] [PATCH 19/44] drm/nouveau/disp: add output hdmi config method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- was previously part of acquire()
- preparation for GSP-RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 10 ++---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 28 --
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  5 ++-
 drivers/gpu/drm/nouveau/nvif/outp.c   | 33 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gm200.c  | 16 ++--
 .../gpu/drm/nouveau/nvkm/engine/disp/ior.h|  5 ++-
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 37 +--
 7 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index ec63a2413455..a4e1525ac48e 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -778,7 +778,6 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
struct drm_hdmi_info *hdmi = _connector->base.display_info.hdmi;
union hdmi_infoframe infoframe = { 0 };
const u8 rekey = 56; /* binary driver, and tegra, constant */
-   u8 scdc = 0;
u32 max_ac_packet;
struct {
struct nvif_outp_infoframe_v0 infoframe;
@@ -791,8 +790,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
max_ac_packet -= 18; /* constant from tegra */
max_ac_packet /= 32;
 
-   if (hdmi->scdc.scrambling.supported) {
+   if (nv_encoder->i2c && hdmi->scdc.scrambling.supported) {
const bool high_tmds_clock_ratio = mode->clock > 34;
+   u8 scdc;
 
ret = drm_scdc_readb(nv_encoder->i2c, SCDC_TMDS_CONFIG, );
if (ret < 0) {
@@ -812,8 +812,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
 scdc, ret);
}
 
-   ret = nvif_outp_acquire_tmds(_encoder->outp, nv_crtc->index, true,
-max_ac_packet, rekey, scdc, hda);
+   ret = nvif_outp_hdmi(_encoder->outp, nv_crtc->index, true, 
max_ac_packet, rekey,
+mode->clock, hdmi->scdc.supported, 
hdmi->scdc.scrambling.supported,
+hdmi->scdc.scrambling.low_rates);
if (ret)
return;
 
@@ -1850,7 +1851,6 @@ nv50_pior_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
switch (nv_encoder->dcb->type) {
case DCB_OUTPUT_TMDS:
ctrl |= NVDEF(NV507D, PIOR_SET_CONTROL, PROTOCOL, EXT_TMDS_ENC);
-   nvif_outp_acquire_tmds(_encoder->outp, false, false, 0, 0, 
0, false);
break;
case DCB_OUTPUT_DP:
ctrl |= NVDEF(NV507D, PIOR_SET_CONTROL, PROTOCOL, EXT_TMDS_ENC);
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 57bc4b2f2b17..230084d675ec 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -21,6 +21,8 @@ union nvif_outp_args {
 
 #define NVIF_OUTP_V0_LOAD_DETECT   0x20
 
+#define NVIF_OUTP_V0_HDMI  0x50
+
 #define NVIF_OUTP_V0_INFOFRAME 0x60
 #define NVIF_OUTP_V0_HDA_ELD   0x61
 
@@ -62,7 +64,6 @@ union nvif_outp_acquire_args {
 #define NVIF_OUTP_ACQUIRE_V0_DAC  0x00
 #define NVIF_OUTP_ACQUIRE_V0_SOR  0x01
 #define NVIF_OUTP_ACQUIRE_V0_PIOR 0x02
-#define NVIF_OUTP_ACQUIRE_V0_TMDS0x05
 #define NVIF_OUTP_ACQUIRE_V0_LVDS0x03
 #define NVIF_OUTP_ACQUIRE_V0_DP  0x04
__u8 type;
@@ -73,17 +74,6 @@ union nvif_outp_acquire_args {
struct {
__u8 hda;
} sor;
-   struct {
-   __u8 head;
-   __u8 hdmi;
-   __u8 hdmi_max_ac_packet;
-   __u8 hdmi_rekey;
-#define NVIF_OUTP_ACQUIRE_V0_TMDS_HDMI_SCDC_SCRAMBLE (1 << 0)
-#define NVIF_OUTP_ACQUIRE_V0_TMDS_HDMI_SCDC_DIV_BY_4 (1 << 1)
-   __u8 hdmi_scdc;
-   __u8 hdmi_hda;
-   __u8 pad06[2];
-   } tmds;
struct {
__u8 dual;
__u8 bpc8;
@@ -128,6 +118,20 @@ union nvif_outp_release_args {
} vn;
 };
 
+union nvif_outp_hdmi_args {
+   struct nvif_outp_hdmi_v0 {
+   __u8 version;
+   __u8 head;
+   __u8 enable;
+   __u8 max_ac_packet;
+   __u8 rekey;
+   __u8 scdc;
+   __u8 scdc_scrambling;
+   __u8 scdc_low_rates;
+   __u32 khz;
+   } v0;
+};
+
 union nvif_outp_infoframe_args {
struct nvif_outp_infoframe_v0 {
__u8 version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 

[Nouveau] [PATCH 18/44] drm/nouveau/kms/nv50-: move audio enable post-modeset

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- adds tracking for post-UPDATE modeset operations, similar to mst[mo]'s
- audio won't work on RM without this
- we should probably have been doing this anyway

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 35 ++---
 drivers/gpu/drm/nouveau/dispnv50/disp.h |  3 +++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 11790ced2b7d..ec63a2413455 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -707,6 +707,18 @@ nv50_audio_supported(struct drm_encoder *encoder)
disp->disp->object.oclass == GT206_DISP)
return false;
 
+   if (encoder->encoder_type != DRM_MODE_ENCODER_DPMST) {
+   struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
+
+   switch (nv_encoder->dcb->type) {
+   case DCB_OUTPUT_TMDS:
+   case DCB_OUTPUT_DP:
+   break;
+   default:
+   return false;
+   }
+   }
+
return true;
 }
 
@@ -829,8 +841,6 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
size = 0;
 
nvif_outp_infoframe(_encoder->outp, NVIF_OUTP_INFOFRAME_V0_VSI, 
, size);
-
-   nv50_audio_enable(encoder, nv_crtc, nv_connector, state, mode);
 }
 
 /**
@@ -1658,8 +1668,6 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
else
proto = NV887D_SOR_SET_CONTROL_PROTOCOL_DP_B;
 
-   nv50_audio_enable(encoder, nv_crtc, nv_connector, state, mode);
-
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
backlight = nv_connector->backlight;
if (backlight && backlight->uses_dpcd)
@@ -1941,7 +1949,9 @@ nv50_disp_atomic_commit_core(struct drm_atomic_state 
*state, u32 *interlock)
struct drm_dp_mst_topology_state *mst_state;
struct nouveau_drm *drm = nouveau_drm(state->dev);
struct nv50_disp *disp = nv50_disp(drm->dev);
+   struct nv50_atom *atom = nv50_atom(state);
struct nv50_core *core = disp->core;
+   struct nv50_outp_atom *outp;
struct nv50_mstm *mstm;
int i;
 
@@ -1964,6 +1974,21 @@ nv50_disp_atomic_commit_core(struct drm_atomic_state 
*state, u32 *interlock)
if (mstm->modified)
nv50_mstm_cleanup(state, mst_state, mstm);
}
+
+   list_for_each_entry(outp, >outp, head) {
+   if (outp->encoder->encoder_type != DRM_MODE_ENCODER_DPMST) {
+   struct nouveau_encoder *nv_encoder = 
nouveau_encoder(outp->encoder);
+
+   if (outp->enabled) {
+   nv50_audio_enable(outp->encoder, 
nouveau_crtc(nv_encoder->crtc),
+ nv_encoder->conn, NULL, NULL);
+   outp->enabled = outp->disabled = false;
+   } else {
+   if (outp->disabled)
+   outp->disabled = false;
+   }
+   }
+   }
 }
 
 static void
@@ -2055,6 +2080,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
 
if (outp->clr.mask) {
help->atomic_disable(encoder, state);
+   outp->disabled = true;
interlock[NV50_DISP_INTERLOCK_CORE] |= 1;
if (outp->flush_disable) {
nv50_disp_atomic_commit_wndw(state, interlock);
@@ -2094,6 +2120,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
 
if (outp->set.mask) {
help->atomic_enable(encoder, state);
+   outp->enabled = true;
interlock[NV50_DISP_INTERLOCK_CORE] = 1;
}
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h 
b/drivers/gpu/drm/nouveau/dispnv50/disp.h
index 9d66c9c726c3..42209f5b06f9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h
@@ -85,6 +85,9 @@ struct nv50_outp_atom {
struct drm_encoder *encoder;
bool flush_disable;
 
+   bool disabled;
+   bool enabled;
+
union nv50_outp_atom_mask {
struct {
bool ctrl:1;
-- 
2.41.0



[Nouveau] [PATCH 17/44] drm/nouveau/kms/nv50-: keep output state around until modeset complete

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- we'll want this info post-UPDATE for later patches

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 165bc6a0d563..11790ced2b7d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2082,7 +2082,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
nv50_crc_atomic_init_notifier_contexts(state);
 
/* Update output path(s). */
-   list_for_each_entry_safe(outp, outt, >outp, head) {
+   list_for_each_entry(outp, >outp, head) {
const struct drm_encoder_helper_funcs *help;
struct drm_encoder *encoder;
 
@@ -2096,9 +2096,6 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
help->atomic_enable(encoder, state);
interlock[NV50_DISP_INTERLOCK_CORE] = 1;
}
-
-   list_del(>head);
-   kfree(outp);
}
 
/* Update head(s). */
@@ -2196,6 +2193,11 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state 
*state)
if (atom->lock_core)
mutex_unlock(>mutex);
 
+   list_for_each_entry_safe(outp, outt, >outp, head) {
+   list_del(>head);
+   kfree(outp);
+   }
+
/* Wait for HW to signal completion. */
for_each_new_plane_in_state(state, plane, new_plane_state, i) {
struct nv50_wndw_atom *asyw = nv50_wndw_atom(new_plane_state);
-- 
2.41.0



[Nouveau] [PATCH 16/44] drm/nouveau/kms/nv50-: remove nv_encoder.audio.connector

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- use nv_encoder.conn instead, outp->conn never changes

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 4 +---
 drivers/gpu/drm/nouveau/nouveau_encoder.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 1123d25be77d..165bc6a0d563 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -611,7 +611,7 @@ nv50_audio_component_get_eld(struct device *kdev, int port, 
int dev_id,
continue; /* TODO */
 
nv_encoder = nouveau_encoder(encoder);
-   nv_connector = nouveau_connector(nv_encoder->audio.connector);
+   nv_connector = nv_encoder->conn;
nv_crtc = nouveau_crtc(nv_encoder->crtc);
 
if (!nv_crtc || nv_encoder->outp.or.id != port || 
nv_crtc->index != dev_id)
@@ -723,7 +723,6 @@ nv50_audio_disable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc)
mutex_lock(>audio.lock);
if (nv_encoder->audio.enabled) {
nv_encoder->audio.enabled = false;
-   nv_encoder->audio.connector = NULL;
nvif_outp_hda_eld(_encoder->outp, nv_crtc->index, NULL, 0);
}
mutex_unlock(>audio.lock);
@@ -748,7 +747,6 @@ nv50_audio_enable(struct drm_encoder *encoder, struct 
nouveau_crtc *nv_crtc,
nvif_outp_hda_eld(_encoder->outp, nv_crtc->index, 
nv_connector->base.eld,
  drm_eld_size(nv_connector->base.eld));
nv_encoder->audio.enabled = true;
-   nv_encoder->audio.connector = _connector->base;
 
mutex_unlock(>audio.lock);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h 
b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index b1554ad9d929..ea8ef10e71aa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -62,7 +62,6 @@ struct nouveau_encoder {
/* Protected by nouveau_drm.audio.lock */
struct {
bool enabled;
-   struct drm_connector *connector;
} audio;
 
struct drm_display_mode mode;
-- 
2.41.0



[Nouveau] [PATCH 13/44] drm/nouveau/disp: add acquire_sor/pior()

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparing to move protocol-specific args out of acquire() again
- avoid re-acquiring acquired output, will matter when enforced later
- sor/pior done at same time due to shared tmds/dp handling

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 15 ++
 drivers/gpu/drm/nouveau/include/nvif/if0012.h |  7 -
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  2 ++
 drivers/gpu/drm/nouveau/nvif/outp.c   | 24 
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 28 ++-
 5 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9339971aa90b..4c54c46bb451 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1032,7 +1032,7 @@ nv50_msto_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
return;
 
if (!mstm->links++) {
-   /*XXX: MST audio. */
+   nvif_outp_acquire_sor(>outp->outp, false /*TODO: MST 
audio... */);
nvif_outp_acquire_dp(>outp->outp, mstm->outp->dp.dpcd, 0, 
0, false, true);
}
 
@@ -1600,15 +1600,17 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *sta
 
if ((disp->disp->object.oclass == GT214_DISP ||
 disp->disp->object.oclass >= GF110_DISP) &&
+   nv_encoder->dcb->type != DCB_OUTPUT_LVDS &&
drm_detect_monitor_audio(nv_connector->edid))
hda = true;
 
+   if (!nvif_outp_acquired(outp))
+   nvif_outp_acquire_sor(outp, hda);
+
switch (nv_encoder->dcb->type) {
case DCB_OUTPUT_TMDS:
-   if (disp->disp->object.oclass == NV50_DISP ||
-   !drm_detect_hdmi_monitor(nv_connector->edid))
-   nvif_outp_acquire_tmds(outp, nv_crtc->index, false, 0, 
0, 0, false);
-   else
+   if (disp->disp->object.oclass != NV50_DISP &&
+   drm_detect_hdmi_monitor(nv_connector->edid))
nv50_hdmi_enable(encoder, nv_crtc, nv_connector, state, 
mode, hda);
 
if (nv_encoder->outp.or.link & 1) {
@@ -1848,6 +1850,9 @@ nv50_pior_atomic_enable(struct drm_encoder *encoder, 
struct drm_atomic_state *st
default: asyh->or.depth = NV837D_PIOR_SET_CONTROL_PIXEL_DEPTH_DEFAULT; 
break;
}
 
+   if (!nvif_outp_acquired(_encoder->outp))
+   nvif_outp_acquire_pior(_encoder->outp);
+
switch (nv_encoder->dcb->type) {
case DCB_OUTPUT_TMDS:
ctrl |= NVDEF(NV507D, PIOR_SET_CONTROL, PROTOCOL, EXT_TMDS_ENC);
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index d139d070c0bc..57bc4b2f2b17 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -60,7 +60,9 @@ union nvif_outp_acquire_args {
struct nvif_outp_acquire_v0 {
__u8 version;
 #define NVIF_OUTP_ACQUIRE_V0_DAC  0x00
-#define NVIF_OUTP_ACQUIRE_V0_TMDS0x02
+#define NVIF_OUTP_ACQUIRE_V0_SOR  0x01
+#define NVIF_OUTP_ACQUIRE_V0_PIOR 0x02
+#define NVIF_OUTP_ACQUIRE_V0_TMDS0x05
 #define NVIF_OUTP_ACQUIRE_V0_LVDS0x03
 #define NVIF_OUTP_ACQUIRE_V0_DP  0x04
__u8 type;
@@ -68,6 +70,9 @@ union nvif_outp_acquire_args {
__u8 link;
__u8 pad04[4];
union {
+   struct {
+   __u8 hda;
+   } sor;
struct {
__u8 head;
__u8 hdmi;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index c6d8823ef782..a9090424dbf7 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -29,6 +29,8 @@ int nvif_outp_edid_get(struct nvif_outp *, u8 **pedid);
 
 int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
 int nvif_outp_acquire_dac(struct nvif_outp *);
+int nvif_outp_acquire_sor(struct nvif_outp *, bool hda);
+int nvif_outp_acquire_pior(struct nvif_outp *);
 int nvif_outp_acquire_tmds(struct nvif_outp *, int head,
   bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool 
hda);
 int nvif_outp_acquire_lvds(struct nvif_outp *, bool dual, bool bpc8);
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 41c4de40895f..81dbda52117e 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -187,6 +187,30 @@ nvif_outp_acquire(struct nvif_outp *outp, u8 type, struct 
nvif_outp_acquire_v0 *
return 0;
 }
 
+int
+nvif_outp_acquire_pior(struct nvif_outp *outp)
+{
+   struct nvif_outp_acquire_v0 args;
+   int ret;
+
+   ret 

[Nouveau] [PATCH 15/44] drm/nouveau/kms/nv50-: pull some common init out of OR-specific code

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- cleanup before additional changes

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 69 ---
 drivers/gpu/drm/nouveau/nouveau_encoder.h |  2 +
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 4c54c46bb451..1123d25be77d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -554,34 +554,27 @@ nv50_dac_func = {
 };
 
 static int
-nv50_dac_create(struct drm_connector *connector, struct dcb_output *dcbe)
+nv50_dac_create(struct nouveau_encoder *nv_encoder)
 {
+   struct drm_connector *connector = _encoder->conn->base;
struct nouveau_drm *drm = nouveau_drm(connector->dev);
-   struct nv50_disp *disp = nv50_disp(connector->dev);
struct nvkm_i2c *i2c = nvxx_i2c(>client.device);
struct nvkm_i2c_bus *bus;
-   struct nouveau_encoder *nv_encoder;
struct drm_encoder *encoder;
+   struct dcb_output *dcbe = nv_encoder->dcb;
int type = DRM_MODE_ENCODER_DAC;
 
-   nv_encoder = kzalloc(sizeof(*nv_encoder), GFP_KERNEL);
-   if (!nv_encoder)
-   return -ENOMEM;
-   nv_encoder->dcb = dcbe;
-
bus = nvkm_i2c_bus_find(i2c, dcbe->i2c_index);
if (bus)
nv_encoder->i2c = >i2c;
 
encoder = to_drm_encoder(nv_encoder);
-   encoder->possible_crtcs = dcbe->heads;
-   encoder->possible_clones = 0;
drm_encoder_init(connector->dev, encoder, _dac_func, type,
 "dac-%04x-%04x", dcbe->hasht, dcbe->hashm);
drm_encoder_helper_add(encoder, _dac_help);
 
drm_connector_attach_encoder(connector, encoder);
-   return nvif_outp_ctor(disp->disp, nv_encoder->base.base.name, dcbe->id, 
_encoder->outp);
+   return 0;
 }
 
 /*
@@ -1724,13 +1717,14 @@ bool nv50_has_mst(struct nouveau_drm *drm)
 }
 
 static int
-nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe)
+nv50_sor_create(struct nouveau_encoder *nv_encoder)
 {
+   struct drm_connector *connector = _encoder->conn->base;
struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_drm *drm = nouveau_drm(connector->dev);
struct nvkm_i2c *i2c = nvxx_i2c(>client.device);
-   struct nouveau_encoder *nv_encoder;
struct drm_encoder *encoder;
+   struct dcb_output *dcbe = nv_encoder->dcb;
struct nv50_disp *disp = nv50_disp(connector->dev);
int type, ret;
 
@@ -1743,15 +1737,9 @@ nv50_sor_create(struct drm_connector *connector, struct 
dcb_output *dcbe)
break;
}
 
-   nv_encoder = kzalloc(sizeof(*nv_encoder), GFP_KERNEL);
-   if (!nv_encoder)
-   return -ENOMEM;
-   nv_encoder->dcb = dcbe;
nv_encoder->update = nv50_sor_update;
 
encoder = to_drm_encoder(nv_encoder);
-   encoder->possible_crtcs = dcbe->heads;
-   encoder->possible_clones = 0;
drm_encoder_init(connector->dev, encoder, _sor_func, type,
 "sor-%04x-%04x", dcbe->hasht, dcbe->hashm);
drm_encoder_helper_add(encoder, _sor_help);
@@ -1795,7 +1783,7 @@ nv50_sor_create(struct drm_connector *connector, struct 
dcb_output *dcbe)
nv_encoder->i2c = >i2c;
}
 
-   return nvif_outp_ctor(disp->disp, nv_encoder->base.base.name, dcbe->id, 
_encoder->outp);
+   return 0;
 }
 
 /**
@@ -1897,8 +1885,9 @@ nv50_pior_func = {
 };
 
 static int
-nv50_pior_create(struct drm_connector *connector, struct dcb_output *dcbe)
+nv50_pior_create(struct nouveau_encoder *nv_encoder)
 {
+   struct drm_connector *connector = _encoder->conn->base;
struct drm_device *dev = connector->dev;
struct nouveau_drm *drm = nouveau_drm(dev);
struct nv50_disp *disp = nv50_disp(dev);
@@ -1906,8 +1895,8 @@ nv50_pior_create(struct drm_connector *connector, struct 
dcb_output *dcbe)
struct nvkm_i2c_bus *bus = NULL;
struct nvkm_i2c_aux *aux = NULL;
struct i2c_adapter *ddc;
-   struct nouveau_encoder *nv_encoder;
struct drm_encoder *encoder;
+   struct dcb_output *dcbe = nv_encoder->dcb;
int type;
 
switch (dcbe->type) {
@@ -1925,18 +1914,12 @@ nv50_pior_create(struct drm_connector *connector, 
struct dcb_output *dcbe)
return -ENODEV;
}
 
-   nv_encoder = kzalloc(sizeof(*nv_encoder), GFP_KERNEL);
-   if (!nv_encoder)
-   return -ENOMEM;
-   nv_encoder->dcb = dcbe;
nv_encoder->i2c = ddc;
nv_encoder->aux = aux;
 
mutex_init(_encoder->dp.hpd_irq_lock);
 
encoder = to_drm_encoder(nv_encoder);
-   encoder->possible_crtcs = dcbe->heads;
-   encoder->possible_clones = 0;
drm_encoder_init(connector->dev, 

[Nouveau] [PATCH 14/44] drm/nouveau/disp: update SOR routing immediately on acquire()

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- was previously delayed until second supervisor interrupt

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c  | 2 +-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
index 5b55598e09c8..b288ea6658da 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
@@ -48,8 +48,8 @@ nvkm_outp_route(struct nvkm_disp *disp)
 
list_for_each_entry(ior, >iors, head) {
if ((outp = ior->asy.outp)) {
-   OUTP_DBG(outp, "acquire %s", ior->name);
if (ior->asy.outp != ior->arm.outp) {
+   OUTP_DBG(outp, "acquire %s", ior->name);
if (ior->func->route.set)
ior->func->route.set(outp, ior);
ior->arm.outp = ior->asy.outp;
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index d71bc188047e..042a43c22061 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -235,6 +235,8 @@ nvkm_uoutp_mthd_acquire(struct nvkm_outp *outp, void *argv, 
u32 argc)
if (ret)
return ret;
 
+   nvkm_outp_route(outp->disp);
+
args->v0.or = outp->ior->id;
args->v0.link = outp->ior->asy.link;
return 0;
-- 
2.41.0



[Nouveau] [PATCH 12/44] drm/nouveau/disp: add acquire_dac()

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparing to move protocol-specific args out of acquire() again
- avoid re-acquiring acquired output, will matter when enforced later
- this one is basically just a rename

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  3 ++-
 drivers/gpu/drm/nouveau/include/nvif/if0012.h|  5 ++---
 drivers/gpu/drm/nouveau/include/nvif/outp.h  |  9 -
 drivers/gpu/drm/nouveau/nvif/outp.c  | 10 +-
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c |  4 ++--
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 32b40229fd18..9339971aa90b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -502,7 +502,8 @@ nv50_dac_atomic_enable(struct drm_encoder *encoder, struct 
drm_atomic_state *sta
 
ctrl |= NVDEF(NV507D, DAC_SET_CONTROL, PROTOCOL, RGB_CRT);
 
-   nvif_outp_acquire_rgb_crt(_encoder->outp);
+   if (!nvif_outp_acquired(_encoder->outp))
+   nvif_outp_acquire_dac(_encoder->outp);
 
core->func->dac->ctrl(core, nv_encoder->outp.or.id, ctrl, asyh);
asyh->or.depth = 0;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 6cfc885e0aa9..d139d070c0bc 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -59,12 +59,11 @@ union nvif_outp_load_detect_args {
 union nvif_outp_acquire_args {
struct nvif_outp_acquire_v0 {
__u8 version;
-#define NVIF_OUTP_ACQUIRE_V0_RGB_CRT 0x00
-#define NVIF_OUTP_ACQUIRE_V0_TV  0x01
+#define NVIF_OUTP_ACQUIRE_V0_DAC  0x00
 #define NVIF_OUTP_ACQUIRE_V0_TMDS0x02
 #define NVIF_OUTP_ACQUIRE_V0_LVDS0x03
 #define NVIF_OUTP_ACQUIRE_V0_DP  0x04
-   __u8 proto;
+   __u8 type;
__u8 or;
__u8 link;
__u8 pad04[4];
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index 23776057bfea..c6d8823ef782 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -28,7 +28,7 @@ enum nvif_outp_detect_status nvif_outp_detect(struct 
nvif_outp *);
 int nvif_outp_edid_get(struct nvif_outp *, u8 **pedid);
 
 int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
-int nvif_outp_acquire_rgb_crt(struct nvif_outp *);
+int nvif_outp_acquire_dac(struct nvif_outp *);
 int nvif_outp_acquire_tmds(struct nvif_outp *, int head,
   bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool 
hda);
 int nvif_outp_acquire_lvds(struct nvif_outp *, bool dual, bool bpc8);
@@ -40,6 +40,13 @@ int nvif_outp_inherit_tmds(struct nvif_outp *outp, u8 
*proto_out);
 int nvif_outp_inherit_dp(struct nvif_outp *outp, u8 *proto_out);
 
 void nvif_outp_release(struct nvif_outp *);
+
+static inline bool
+nvif_outp_acquired(struct nvif_outp *outp)
+{
+   return outp->or.id >= 0;
+}
+
 int nvif_outp_infoframe(struct nvif_outp *, u8 type, struct 
nvif_outp_infoframe_v0 *, u32 size);
 int nvif_outp_hda_eld(struct nvif_outp *, int head, void *data, u32 size);
 int nvif_outp_dp_aux_pwr(struct nvif_outp *, bool enable);
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index eecccfc17c1c..41c4de40895f 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -171,12 +171,12 @@ nvif_outp_release(struct nvif_outp *outp)
 }
 
 static inline int
-nvif_outp_acquire(struct nvif_outp *outp, u8 proto, struct 
nvif_outp_acquire_v0 *args)
+nvif_outp_acquire(struct nvif_outp *outp, u8 type, struct nvif_outp_acquire_v0 
*args)
 {
int ret;
 
args->version = 0;
-   args->proto = proto;
+   args->type = type;
 
ret = nvif_mthd(>object, NVIF_OUTP_V0_ACQUIRE, args, 
sizeof(*args));
if (ret)
@@ -188,13 +188,13 @@ nvif_outp_acquire(struct nvif_outp *outp, u8 proto, 
struct nvif_outp_acquire_v0
 }
 
 int
-nvif_outp_acquire_rgb_crt(struct nvif_outp *outp)
+nvif_outp_acquire_dac(struct nvif_outp *outp)
 {
struct nvif_outp_acquire_v0 args;
int ret;
 
-   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_RGB_CRT, );
-   NVIF_ERRON(ret, >object, "[ACQUIRE proto:RGB_CRT] or:%d", 
args.or);
+   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_DAC, );
+   NVIF_ERRON(ret, >object, "[ACQUIRE DAC] or:%d", args.or);
return ret;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index d56a87ae5b26..73c6227446fb 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -217,8 +217,8 @@ nvkm_uoutp_mthd_acquire(struct nvkm_outp *outp, void *argv, 
u32 argc)
if (outp->ior)
 

[Nouveau] [PATCH 11/44] drm/nouveau/disp: shuffle to make upcoming diffs prettier

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparing to move protocol-specific args out of acquire() again
- no code changes

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvif/outp.c   | 106 +-
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  |  74 ++--
 2 files changed, 91 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 795658f0c920..eecccfc17c1c 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -54,6 +54,28 @@ nvif_outp_dp_retrain(struct nvif_outp *outp)
return ret;
 }
 
+static inline int nvif_outp_acquire(struct nvif_outp *, u8, struct 
nvif_outp_acquire_v0 *);
+
+int
+nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
+int link_nr, int link_bw, bool hda, bool mst)
+{
+   struct nvif_outp_acquire_v0 args;
+   int ret;
+
+   args.dp.link_nr = link_nr;
+   args.dp.link_bw = link_bw;
+   args.dp.hda = hda;
+   args.dp.mst = mst;
+   memcpy(args.dp.dpcd, dpcd, sizeof(args.dp.dpcd));
+
+   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_DP, );
+   NVIF_ERRON(ret, >object,
+  "[ACQUIRE proto:DP link_nr:%d link_bw:%02x hda:%d mst:%d] 
or:%d link:%d",
+  args.dp.link_nr, args.dp.link_bw, args.dp.hda, args.dp.mst, 
args.or, args.link);
+   return ret;
+}
+
 int
 nvif_outp_dp_aux_pwr(struct nvif_outp *outp, bool enable)
 {
@@ -101,48 +123,26 @@ nvif_outp_infoframe(struct nvif_outp *outp, u8 type, 
struct nvif_outp_infoframe_
return ret;
 }
 
-void
-nvif_outp_release(struct nvif_outp *outp)
-{
-   int ret = nvif_mthd(>object, NVIF_OUTP_V0_RELEASE, NULL, 0);
-   NVIF_ERRON(ret, >object, "[RELEASE]");
-   outp->or.id = -1;
-}
-
-static inline int
-nvif_outp_acquire(struct nvif_outp *outp, u8 proto, struct 
nvif_outp_acquire_v0 *args)
-{
-   int ret;
-
-   args->version = 0;
-   args->proto = proto;
-
-   ret = nvif_mthd(>object, NVIF_OUTP_V0_ACQUIRE, args, 
sizeof(*args));
-   if (ret)
-   return ret;
-
-   outp->or.id = args->or;
-   outp->or.link = args->link;
-   return 0;
-}
-
 int
-nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE],
-int link_nr, int link_bw, bool hda, bool mst)
+nvif_outp_acquire_tmds(struct nvif_outp *outp, int head,
+  bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool hda)
 {
struct nvif_outp_acquire_v0 args;
int ret;
 
-   args.dp.link_nr = link_nr;
-   args.dp.link_bw = link_bw;
-   args.dp.hda = hda;
-   args.dp.mst = mst;
-   memcpy(args.dp.dpcd, dpcd, sizeof(args.dp.dpcd));
+   args.tmds.head = head;
+   args.tmds.hdmi = hdmi;
+   args.tmds.hdmi_max_ac_packet = max_ac_packet;
+   args.tmds.hdmi_rekey = rekey;
+   args.tmds.hdmi_scdc = scdc;
+   args.tmds.hdmi_hda = hda;
 
-   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_DP, );
+   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_TMDS, );
NVIF_ERRON(ret, >object,
-  "[ACQUIRE proto:DP link_nr:%d link_bw:%02x hda:%d mst:%d] 
or:%d link:%d",
-  args.dp.link_nr, args.dp.link_bw, args.dp.hda, args.dp.mst, 
args.or, args.link);
+  "[ACQUIRE proto:TMDS head:%d hdmi:%d max_ac_packet:%d 
rekey:%d scdc:%d hda:%d]"
+  " or:%d link:%d", args.tmds.head, args.tmds.hdmi, 
args.tmds.hdmi_max_ac_packet,
+  args.tmds.hdmi_rekey, args.tmds.hdmi_scdc, 
args.tmds.hdmi_hda,
+  args.or, args.link);
return ret;
 }
 
@@ -162,27 +162,29 @@ nvif_outp_acquire_lvds(struct nvif_outp *outp, bool dual, 
bool bpc8)
return ret;
 }
 
-int
-nvif_outp_acquire_tmds(struct nvif_outp *outp, int head,
-  bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool hda)
+void
+nvif_outp_release(struct nvif_outp *outp)
+{
+   int ret = nvif_mthd(>object, NVIF_OUTP_V0_RELEASE, NULL, 0);
+   NVIF_ERRON(ret, >object, "[RELEASE]");
+   outp->or.id = -1;
+}
+
+static inline int
+nvif_outp_acquire(struct nvif_outp *outp, u8 proto, struct 
nvif_outp_acquire_v0 *args)
 {
-   struct nvif_outp_acquire_v0 args;
int ret;
 
-   args.tmds.head = head;
-   args.tmds.hdmi = hdmi;
-   args.tmds.hdmi_max_ac_packet = max_ac_packet;
-   args.tmds.hdmi_rekey = rekey;
-   args.tmds.hdmi_scdc = scdc;
-   args.tmds.hdmi_hda = hda;
+   args->version = 0;
+   args->proto = proto;
 
-   ret = nvif_outp_acquire(outp, NVIF_OUTP_ACQUIRE_V0_TMDS, );
-   NVIF_ERRON(ret, >object,
-  "[ACQUIRE proto:TMDS head:%d hdmi:%d max_ac_packet:%d 
rekey:%d scdc:%d hda:%d]"
-  " or:%d link:%d", args.tmds.head, args.tmds.hdmi, 
args.tmds.hdmi_max_ac_packet,
-  args.tmds.hdmi_rekey, args.tmds.hdmi_scdc, 

[Nouveau] [PATCH 10/44] drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state

2023-09-14 Thread Ben Skeggs
From: Lyude Paul 

Now that we're supporting things like Ada and the GSP, there's situations
where we really need to actually know the display state that we're starting
with when loading the driver in order to prevent breaking GSP expectations.
The first step in doing this is making it so that we can read the current
state of IORs from nvkm in DRM, so that we can fill in said into into the
atomic state.

We do this by introducing an INHERIT ioctl to nvkm/nvif. This is basically
another form of ACQUIRE, except that it will only acquire the given output
path for userspace if it's already set up in hardware. This way, we can go
through and probe each outp object we have in DRM in order to figure out
the current hardware state of each one. If the outp isn't in use, it simply
returns -ENODEV.

This is also part of the work that will be required for implementing GSP
support for display. While the GSP should mostly work without this commit,
this commit should fix some edge case bugs that can occur on initial driver
load. This also paves the way for some of the initial groundwork for
fastboot support.

Signed-off-by: Lyude Paul 
Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 101 ++
 drivers/gpu/drm/nouveau/include/nvif/if0012.h |  23 
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |   5 +
 drivers/gpu/drm/nouveau/nvif/outp.c   |  68 
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |   1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c   |  39 ---
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |   3 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  |  64 +++
 8 files changed, 291 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2911167bf22a..32b40229fd18 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2521,6 +2521,104 @@ nv50_display_fini(struct drm_device *dev, bool runtime, 
bool suspend)
cancel_work_sync(>hpd_work);
 }
 
+static inline void
+nv50_display_read_hw_or_state(struct drm_device *dev, struct nv50_disp *disp,
+ struct nouveau_encoder *outp)
+{
+   struct drm_crtc *crtc;
+   struct drm_connector_list_iter conn_iter;
+   struct drm_connector *conn;
+   struct nv50_head_atom *armh;
+   const u32 encoder_mask = drm_encoder_mask(>base.base);
+   bool found_conn = false, found_head = false;
+   u8 proto;
+   int head_idx;
+   int ret;
+
+   switch (outp->dcb->type) {
+   case DCB_OUTPUT_TMDS:
+   ret = nvif_outp_inherit_tmds(>outp, );
+   break;
+   case DCB_OUTPUT_DP:
+   ret = nvif_outp_inherit_dp(>outp, );
+   break;
+   case DCB_OUTPUT_LVDS:
+   ret = nvif_outp_inherit_lvds(>outp, );
+   break;
+   case DCB_OUTPUT_ANALOG:
+   ret = nvif_outp_inherit_rgb_crt(>outp, );
+   break;
+   default:
+   drm_dbg_kms(dev, "Readback for %s not implemented yet, 
skipping\n",
+   outp->base.base.name);
+   drm_WARN_ON(dev, true);
+   return;
+   }
+
+   if (ret < 0)
+   return;
+
+   head_idx = ret;
+
+   drm_for_each_crtc(crtc, dev) {
+   if (crtc->index != head_idx)
+   continue;
+
+   armh = nv50_head_atom(crtc->state);
+   found_head = true;
+   break;
+   }
+   if (drm_WARN_ON(dev, !found_head))
+   return;
+
+   /* Figure out which connector is being used by this encoder */
+   drm_connector_list_iter_begin(dev, _iter);
+   nouveau_for_each_non_mst_connector_iter(conn, _iter) {
+   if (nouveau_connector(conn)->index == outp->dcb->connector) {
+   found_conn = true;
+   break;
+   }
+   }
+   drm_connector_list_iter_end(_iter);
+   if (drm_WARN_ON(dev, !found_conn))
+   return;
+
+   armh->state.encoder_mask = encoder_mask;
+   armh->state.connector_mask = drm_connector_mask(conn);
+   armh->state.active = true;
+   armh->state.enable = true;
+   pm_runtime_get_noresume(dev->dev);
+
+   outp->crtc = crtc;
+   outp->ctrl = NVVAL(NV507D, SOR_SET_CONTROL, PROTOCOL, proto) | 
BIT(crtc->index);
+
+   drm_connector_get(conn);
+   conn->state->crtc = crtc;
+   conn->state->best_encoder = >base.base;
+}
+
+/* Read back the currently programmed display state */
+static void
+nv50_display_read_hw_state(struct nouveau_drm *drm)
+{
+   struct drm_device *dev = drm->dev;
+   struct drm_encoder *encoder;
+   struct drm_modeset_acquire_ctx ctx;
+   struct nv50_disp *disp = nv50_disp(dev);
+   int ret;
+
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
+
+   

[Nouveau] [PATCH 06/44] drm/nouveau/disp: rearrange output methods

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- preparation for a bunch of API changes, to make diffs prettier

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 19 +++
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 12 ++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 16d4ad5023a3..7c56f653070c 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -12,14 +12,17 @@ union nvif_outp_args {
} v0;
 };
 
-#define NVIF_OUTP_V0_LOAD_DETECT 0x00
-#define NVIF_OUTP_V0_ACQUIRE 0x01
-#define NVIF_OUTP_V0_RELEASE 0x02
-#define NVIF_OUTP_V0_INFOFRAME   0x03
-#define NVIF_OUTP_V0_HDA_ELD 0x04
-#define NVIF_OUTP_V0_DP_AUX_PWR  0x05
-#define NVIF_OUTP_V0_DP_RETRAIN  0x06
-#define NVIF_OUTP_V0_DP_MST_VCPI 0x07
+#define NVIF_OUTP_V0_ACQUIRE   0x11
+#define NVIF_OUTP_V0_RELEASE   0x12
+
+#define NVIF_OUTP_V0_LOAD_DETECT   0x20
+
+#define NVIF_OUTP_V0_INFOFRAME 0x60
+#define NVIF_OUTP_V0_HDA_ELD   0x61
+
+#define NVIF_OUTP_V0_DP_AUX_PWR0x70
+#define NVIF_OUTP_V0_DP_RETRAIN0x73
+#define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
 union nvif_outp_load_detect_args {
struct nvif_outp_load_detect_v0 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index fc283a4a1522..440ea52cc7d2 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -279,11 +279,11 @@ static int
 nvkm_uoutp_mthd_acquired(struct nvkm_outp *outp, u32 mthd, void *argv, u32 
argc)
 {
switch (mthd) {
-   case NVIF_OUTP_V0_RELEASE: return nvkm_uoutp_mthd_release(outp, 
argv, argc);
-   case NVIF_OUTP_V0_INFOFRAME  : return nvkm_uoutp_mthd_infoframe  (outp, 
argv, argc);
-   case NVIF_OUTP_V0_HDA_ELD: return nvkm_uoutp_mthd_hda_eld(outp, 
argv, argc);
-   case NVIF_OUTP_V0_DP_RETRAIN : return nvkm_uoutp_mthd_dp_retrain (outp, 
argv, argc);
-   case NVIF_OUTP_V0_DP_MST_VCPI: return nvkm_uoutp_mthd_dp_mst_vcpi(outp, 
argv, argc);
+   case NVIF_OUTP_V0_RELEASE  : return nvkm_uoutp_mthd_release  
(outp, argv, argc);
+   case NVIF_OUTP_V0_INFOFRAME: return nvkm_uoutp_mthd_infoframe
(outp, argv, argc);
+   case NVIF_OUTP_V0_HDA_ELD  : return nvkm_uoutp_mthd_hda_eld  
(outp, argv, argc);
+   case NVIF_OUTP_V0_DP_RETRAIN   : return nvkm_uoutp_mthd_dp_retrain   
(outp, argv, argc);
+   case NVIF_OUTP_V0_DP_MST_VCPI  : return nvkm_uoutp_mthd_dp_mst_vcpi  
(outp, argv, argc);
default:
break;
}
@@ -295,8 +295,8 @@ static int
 nvkm_uoutp_mthd_noacquire(struct nvkm_outp *outp, u32 mthd, void *argv, u32 
argc)
 {
switch (mthd) {
-   case NVIF_OUTP_V0_LOAD_DETECT: return nvkm_uoutp_mthd_load_detect(outp, 
argv, argc);
case NVIF_OUTP_V0_ACQUIRE: return nvkm_uoutp_mthd_acquire(outp, 
argv, argc);
+   case NVIF_OUTP_V0_LOAD_DETECT: return nvkm_uoutp_mthd_load_detect(outp, 
argv, argc);
case NVIF_OUTP_V0_DP_AUX_PWR : return nvkm_uoutp_mthd_dp_aux_pwr (outp, 
argv, argc);
default:
break;
-- 
2.41.0



[Nouveau] [PATCH 09/44] drm/nouveau/disp: rename internal output acquire/release functions

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

These will be made static later in the patch series, after the code that
uses them has been cleaned up in preparation for GSP-RM support.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c  | 10 --
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h  |  5 +++--
 drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 16 
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
index fb061144438d..3ed93df475fc 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
@@ -89,7 +89,7 @@ nvkm_outp_xlat(struct nvkm_outp *outp, enum nvkm_ior_type 
*type)
 }
 
 void
-nvkm_outp_release(struct nvkm_outp *outp, u8 user)
+nvkm_outp_release_or(struct nvkm_outp *outp, u8 user)
 {
struct nvkm_ior *ior = outp->ior;
OUTP_TRACE(outp, "release %02x &= %02x %p", outp->acquired, ~user, ior);
@@ -142,7 +142,7 @@ nvkm_outp_acquire_hda(struct nvkm_outp *outp, enum 
nvkm_ior_type type,
 }
 
 int
-nvkm_outp_acquire(struct nvkm_outp *outp, u8 user, bool hda)
+nvkm_outp_acquire_or(struct nvkm_outp *outp, u8 user, bool hda)
 {
struct nvkm_ior *ior = outp->ior;
enum nvkm_ior_proto proto;
@@ -234,6 +234,12 @@ nvkm_outp_detect(struct nvkm_outp *outp)
return ret;
 }
 
+void
+nvkm_outp_release(struct nvkm_outp *outp)
+{
+   nvkm_outp_release_or(outp, NVKM_OUTP_USER);
+}
+
 void
 nvkm_outp_fini(struct nvkm_outp *outp)
 {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
index 1cd70868f225..76d83fb9c6e5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.h
@@ -77,8 +77,9 @@ void nvkm_outp_fini(struct nvkm_outp *);
 
 int nvkm_outp_detect(struct nvkm_outp *);
 
-int nvkm_outp_acquire(struct nvkm_outp *, u8 user, bool hda);
-void nvkm_outp_release(struct nvkm_outp *, u8 user);
+int nvkm_outp_acquire_or(struct nvkm_outp *, u8 user, bool hda);
+void nvkm_outp_release(struct nvkm_outp *);
+void nvkm_outp_release_or(struct nvkm_outp *, u8 user);
 void nvkm_outp_route(struct nvkm_disp *);
 
 struct nvkm_outp_func {
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index 0c4ffa3ffb28..828db77af242 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -141,7 +141,7 @@ nvkm_uoutp_mthd_release(struct nvkm_outp *outp, void *argv, 
u32 argc)
ior->func->hdmi->ctrl(ior, head->id, false, 0, 0);
}
 
-   nvkm_outp_release(outp, NVKM_OUTP_USER);
+   nvkm_outp_release(outp);
return 0;
 }
 
@@ -151,7 +151,7 @@ nvkm_uoutp_mthd_acquire_dp(struct nvkm_outp *outp, u8 
dpcd[DP_RECEIVER_CAP_SIZE]
 {
int ret;
 
-   ret = nvkm_outp_acquire(outp, NVKM_OUTP_USER, hda);
+   ret = nvkm_outp_acquire_or(outp, NVKM_OUTP_USER, hda);
if (ret)
return ret;
 
@@ -172,7 +172,7 @@ nvkm_uoutp_mthd_acquire_tmds(struct nvkm_outp *outp, u8 
head, u8 hdmi, u8 hdmi_m
if (!(outp->asy.head = nvkm_head_find(outp->disp, head)))
return -EINVAL;
 
-   ret = nvkm_outp_acquire(outp, NVKM_OUTP_USER, hdmi && hdmi_hda);
+   ret = nvkm_outp_acquire_or(outp, NVKM_OUTP_USER, hdmi && hdmi_hda);
if (ret)
return ret;
 
@@ -182,7 +182,7 @@ nvkm_uoutp_mthd_acquire_tmds(struct nvkm_outp *outp, u8 
head, u8 hdmi, u8 hdmi_m
if (!ior->func->hdmi ||
hdmi_max_ac_packet > 0x1f || hdmi_rekey > 0x7f ||
(hdmi_scdc && !ior->func->hdmi->scdc)) {
-   nvkm_outp_release(outp, NVKM_OUTP_USER);
+   nvkm_outp_release_or(outp, NVKM_OUTP_USER);
return -EINVAL;
}
 
@@ -203,7 +203,7 @@ nvkm_uoutp_mthd_acquire_lvds(struct nvkm_outp *outp, bool 
dual, bool bpc8)
outp->lvds.dual = dual;
outp->lvds.bpc8 = bpc8;
 
-   return nvkm_outp_acquire(outp, NVKM_OUTP_USER, false);
+   return nvkm_outp_acquire_or(outp, NVKM_OUTP_USER, false);
 }
 
 static int
@@ -219,7 +219,7 @@ nvkm_uoutp_mthd_acquire(struct nvkm_outp *outp, void *argv, 
u32 argc)
 
switch (args->v0.proto) {
case NVIF_OUTP_ACQUIRE_V0_RGB_CRT:
-   ret = nvkm_outp_acquire(outp, NVKM_OUTP_USER, false);
+   ret = nvkm_outp_acquire_or(outp, NVKM_OUTP_USER, false);
break;
case NVIF_OUTP_ACQUIRE_V0_TMDS:
ret = nvkm_uoutp_mthd_acquire_tmds(outp, args->v0.tmds.head,
@@ -261,7 +261,7 @@ nvkm_uoutp_mthd_load_detect(struct nvkm_outp *outp, void 
*argv, u32 argc)
if (argc != sizeof(args->v0) || args->v0.version != 0)
return -ENOSYS;
 
-   ret = nvkm_outp_acquire(outp, 

[Nouveau] [PATCH 05/44] drm/nouveau/kms/nv50-: fix mst payload alloc fail crashing evo

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Programming -1 (vc_start_slot, if alloc fails) into HW probably isn't
the best idea.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 4e7c9c353c51..2911167bf22a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -912,6 +912,7 @@ nv50_msto_prepare(struct drm_atomic_state *state,
struct nv50_mstm *mstm = mstc->mstm;
struct drm_dp_mst_topology_state *old_mst_state;
struct drm_dp_mst_atomic_payload *payload, *old_payload;
+   int ret = 0;
 
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
 
@@ -920,18 +921,20 @@ nv50_msto_prepare(struct drm_atomic_state *state,
payload = drm_atomic_get_mst_payload_state(mst_state, mstc->port);
old_payload = drm_atomic_get_mst_payload_state(old_mst_state, 
mstc->port);
 
-   // TODO: Figure out if we want to do a better job of handling VCPI 
allocation failures here?
if (msto->disabled) {
drm_dp_remove_payload(mgr, mst_state, old_payload, payload);
-
-   nvif_outp_dp_mst_vcpi(>outp->outp, 
msto->head->base.index, 0, 0, 0, 0);
+   ret = 1;
} else {
if (msto->enabled)
-   drm_dp_add_payload_part1(mgr, mst_state, payload);
+   ret = drm_dp_add_payload_part1(mgr, mst_state, payload);
+   }
 
+   if (ret == 0) {
nvif_outp_dp_mst_vcpi(>outp->outp, msto->head->base.index,
  payload->vc_start_slot, 
payload->time_slots,
  payload->pbn, payload->time_slots * 
mst_state->pbn_div);
+   } else {
+   nvif_outp_dp_mst_vcpi(>outp->outp, 
msto->head->base.index, 0, 0, 0, 0);
}
 }
 
-- 
2.41.0



[Nouveau] [PATCH 07/44] drm/nouveau/disp: add output detect method

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

This will check the relevant hotplug pin and skip the DDC probe we
currently do if a display is present.

- preparation for GSP-RM.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/conn.h   |  5 --
 drivers/gpu/drm/nouveau/include/nvif/if0011.h | 11 
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 12 
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  9 +++
 drivers/gpu/drm/nouveau/nouveau_connector.c   | 60 ---
 drivers/gpu/drm/nouveau/nouveau_dp.c  | 10 +---
 drivers/gpu/drm/nouveau/nvif/conn.c   | 14 -
 drivers/gpu/drm/nouveau/nvif/outp.c   | 25 
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.c   | 28 +
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  6 ++
 .../gpu/drm/nouveau/nvkm/engine/disp/uconn.c  | 41 -
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 24 
 13 files changed, 145 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/conn.h 
b/drivers/gpu/drm/nouveau/include/nvif/conn.h
index dc355e1dfafa..8a6017a35897 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/conn.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/conn.h
@@ -18,11 +18,6 @@ nvif_conn_id(struct nvif_conn *conn)
return conn->object.handle;
 }
 
-#define NVIF_CONN_HPD_STATUS_UNSUPPORTED 0 /* negative if query fails */
-#define NVIF_CONN_HPD_STATUS_NOT_PRESENT 1
-#define NVIF_CONN_HPD_STATUS_PRESENT 2
-int nvif_conn_hpd_status(struct nvif_conn *);
-
 int nvif_conn_event_ctor(struct nvif_conn *, const char *name, 
nvif_event_func, u8 types,
 struct nvif_event *);
 #endif
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0011.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0011.h
index 69b0b779f942..0c25288a5a78 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0011.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0011.h
@@ -20,15 +20,4 @@ union nvif_conn_event_args {
__u8 pad02[6];
} v0;
 };
-
-#define NVIF_CONN_V0_HPD_STATUS 0x
-
-union nvif_conn_hpd_status_args {
-   struct nvif_conn_hpd_status_v0 {
-   __u8 version;
-   __u8 support;
-   __u8 present;
-   __u8 pad03[5];
-   } v0;
-};
 #endif
diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 7c56f653070c..923bc30af2a9 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -12,6 +12,8 @@ union nvif_outp_args {
} v0;
 };
 
+#define NVIF_OUTP_V0_DETECT0x00
+
 #define NVIF_OUTP_V0_ACQUIRE   0x11
 #define NVIF_OUTP_V0_RELEASE   0x12
 
@@ -24,6 +26,16 @@ union nvif_outp_args {
 #define NVIF_OUTP_V0_DP_RETRAIN0x73
 #define NVIF_OUTP_V0_DP_MST_VCPI   0x78
 
+union nvif_outp_detect_args {
+   struct nvif_outp_detect_v0 {
+   __u8 version;
+#define NVIF_OUTP_DETECT_V0_NOT_PRESENT 0x00
+#define NVIF_OUTP_DETECT_V0_PRESENT 0x01
+#define NVIF_OUTP_DETECT_V0_UNKNOWN 0x02
+   __u8 status;
+   } v0;
+};
+
 union nvif_outp_load_detect_args {
struct nvif_outp_load_detect_v0 {
__u8  version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index fa76a7b5e4b3..c3e1e4d2f1a1 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -17,6 +17,15 @@ struct nvif_outp {
 
 int nvif_outp_ctor(struct nvif_disp *, const char *name, int id, struct 
nvif_outp *);
 void nvif_outp_dtor(struct nvif_outp *);
+
+enum nvif_outp_detect_status {
+   NOT_PRESENT,
+   PRESENT,
+   UNKNOWN,
+};
+
+enum nvif_outp_detect_status nvif_outp_detect(struct nvif_outp *);
+
 int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
 int nvif_outp_acquire_rgb_crt(struct nvif_outp *);
 int nvif_outp_acquire_tmds(struct nvif_outp *, int head,
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 79ea30aac31f..ff57466c8d28 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -413,6 +413,7 @@ nouveau_connector_ddc_detect(struct drm_connector 
*connector)
 {
struct drm_device *dev = connector->dev;
struct pci_dev *pdev = to_pci_dev(dev->dev);
+   struct nouveau_connector *conn = nouveau_connector(connector);
struct nouveau_encoder *nv_encoder = NULL, *found = NULL;
struct drm_encoder *encoder;
int ret;
@@ -421,33 +422,48 @@ nouveau_connector_ddc_detect(struct drm_connector 
*connector)
drm_connector_for_each_possible_encoder(connector, encoder) {
nv_encoder = nouveau_encoder(encoder);
 
-   switch (nv_encoder->dcb->type) {
-   case DCB_OUTPUT_DP:
-   ret = 

[Nouveau] [PATCH 08/44] drm/nouveau/disp: add output method to fetch edid

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

- needed to support TMDS EDID on RM

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 10 +++
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  1 +
 drivers/gpu/drm/nouveau/nouveau_connector.c   | 22 --
 drivers/gpu/drm/nouveau/nvif/outp.c   | 30 +++
 .../gpu/drm/nouveau/nvkm/engine/disp/outp.h   |  1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/uoutp.c  | 15 ++
 6 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvif/if0012.h 
b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
index 923bc30af2a9..725d6e8e3d2d 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/if0012.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/if0012.h
@@ -13,6 +13,7 @@ union nvif_outp_args {
 };
 
 #define NVIF_OUTP_V0_DETECT0x00
+#define NVIF_OUTP_V0_EDID_GET  0x01
 
 #define NVIF_OUTP_V0_ACQUIRE   0x11
 #define NVIF_OUTP_V0_RELEASE   0x12
@@ -36,6 +37,15 @@ union nvif_outp_detect_args {
} v0;
 };
 
+union nvif_outp_edid_get_args {
+   struct nvif_outp_edid_get_v0 {
+   __u8  version;
+   __u8  pad01;
+   __u16 size;
+   __u8  data[2048];
+   } v0;
+};
+
 union nvif_outp_load_detect_args {
struct nvif_outp_load_detect_v0 {
__u8  version;
diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h 
b/drivers/gpu/drm/nouveau/include/nvif/outp.h
index c3e1e4d2f1a1..7c2c34a84fbd 100644
--- a/drivers/gpu/drm/nouveau/include/nvif/outp.h
+++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h
@@ -25,6 +25,7 @@ enum nvif_outp_detect_status {
 };
 
 enum nvif_outp_detect_status nvif_outp_detect(struct nvif_outp *);
+int nvif_outp_edid_get(struct nvif_outp *, u8 **pedid);
 
 int nvif_outp_load_detect(struct nvif_outp *, u32 loadval);
 int nvif_outp_acquire_rgb_crt(struct nvif_outp *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index ff57466c8d28..b95ec49935fe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -570,7 +570,6 @@ nouveau_connector_detect(struct drm_connector *connector, 
bool force)
struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_encoder *nv_encoder = NULL;
struct nouveau_encoder *nv_partner;
-   struct i2c_adapter *i2c;
int type;
int ret;
enum drm_connector_status conn_status = connector_status_disconnected;
@@ -593,15 +592,20 @@ nouveau_connector_detect(struct drm_connector *connector, 
bool force)
}
 
nv_encoder = nouveau_connector_ddc_detect(connector);
-   if (nv_encoder && (i2c = nv_encoder->i2c) != NULL) {
-   struct edid *new_edid;
+   if (nv_encoder) {
+   struct edid *new_edid = NULL;
 
-   if ((vga_switcheroo_handler_flags() &
-VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
-   nv_connector->type == DCB_CONNECTOR_LVDS)
-   new_edid = drm_get_edid_switcheroo(connector, i2c);
-   else
-   new_edid = drm_get_edid(connector, i2c);
+   if (nv_encoder->i2c) {
+   if ((vga_switcheroo_handler_flags() & 
VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
+   nv_connector->type == DCB_CONNECTOR_LVDS)
+   new_edid = drm_get_edid_switcheroo(connector, 
nv_encoder->i2c);
+   else
+   new_edid = drm_get_edid(connector, 
nv_encoder->i2c);
+   } else {
+   ret = nvif_outp_edid_get(_encoder->outp, (u8 
**)_edid);
+   if (ret < 0)
+   return connector_status_disconnected;
+   }
 
nouveau_connector_set_edid(nv_connector, new_edid);
if (!nv_connector->edid) {
diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 7f1daab35a0d..10480142eea5 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -210,6 +210,36 @@ nvif_outp_load_detect(struct nvif_outp *outp, u32 loadval)
return ret < 0 ? ret : args.load;
 }
 
+int
+nvif_outp_edid_get(struct nvif_outp *outp, u8 **pedid)
+{
+   struct nvif_outp_edid_get_v0 *args;
+   int ret;
+
+   args = kmalloc(sizeof(*args), GFP_KERNEL);
+   if (!args)
+   return -ENOMEM;
+
+   args->version = 0;
+
+   ret = nvif_mthd(>object, NVIF_OUTP_V0_EDID_GET, args, 
sizeof(*args));
+   NVIF_ERRON(ret, >object, "[EDID_GET] size:%d", args->size);
+   if (ret)
+   goto done;
+
+   *pedid = kmalloc(args->size, GFP_KERNEL);
+   if (!*pedid) {
+   ret = -ENOMEM;
+   goto done;
+   }
+
+   memcpy(*pedid, args->data, args->size);
+   ret = 

[Nouveau] [PATCH 04/44] drm/nouveau/mmu/gp100-: always invalidate TLBs at CACHE_LEVEL_ALL

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Fixes some issues when running on top of RM.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c | 2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmtu102.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
index f3630d0e0d55..bddac77f48f0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
@@ -558,7 +558,7 @@ gp100_vmm_invalidate_pdb(struct nvkm_vmm *vmm, u64 addr)
 void
 gp100_vmm_flush(struct nvkm_vmm *vmm, int depth)
 {
-   u32 type = (5 /* CACHE_LEVEL_UP_TO_PDE3 */ - depth) << 24;
+   u32 type = 0;
if (atomic_read(>engref[NVKM_SUBDEV_BAR]))
type |= 0x0004; /* HUB_ONLY */
type |= 0x0001; /* PAGE_ALL */
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmtu102.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmtu102.c
index 6cb5eefa45e9..0095d58d4d9a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmtu102.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmtu102.c
@@ -27,7 +27,7 @@ static void
 tu102_vmm_flush(struct nvkm_vmm *vmm, int depth)
 {
struct nvkm_device *device = vmm->mmu->subdev.device;
-   u32 type = (5 /* CACHE_LEVEL_UP_TO_PDE3 */ - depth) << 24;
+   u32 type = 0;
 
type |= 0x0001; /* PAGE_ALL */
if (atomic_read(>engref[NVKM_SUBDEV_BAR]))
-- 
2.41.0



[Nouveau] [PATCH 03/44] drm/nouveau/gr/gf100-: lose contents of global ctxbufs across suspend

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Some of these buffers are quite large, and there's no need to preserve
them across suspend.

Mark the contents as lost to speedup suspend/resume.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c
index 3648868bb9fc..c494a1ff2d57 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c
@@ -2032,18 +2032,18 @@ gf100_gr_oneinit(struct nvkm_gr *base)
}
 
/* Allocate global context buffers. */
-   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 
gr->func->grctx->pagepool_size,
- 0x100, false, >pagepool);
+   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST_SR_LOST,
+ gr->func->grctx->pagepool_size, 0x100, false, 
>pagepool);
if (ret)
return ret;
 
-   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 
gr->func->grctx->bundle_size,
+   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST_SR_LOST, 
gr->func->grctx->bundle_size,
  0x100, false, >bundle_cb);
if (ret)
return ret;
 
-   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 
gr->func->grctx->attrib_cb_size(gr),
- 0x1000, false, >attrib_cb);
+   ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST_SR_LOST,
+ gr->func->grctx->attrib_cb_size(gr), 0x1000, 
false, >attrib_cb);
if (ret)
return ret;
 
-- 
2.41.0



[Nouveau] [PATCH 02/44] drm/nouveau/imem: support allocations not preserved across suspend

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

Will initially be used to tag some large grctx allocations which don't
need to be saved, to speedup suspend/resume.

Signed-off-by: Ben Skeggs 
---
 .../drm/nouveau/include/nvkm/core/memory.h|  1 +
 .../drm/nouveau/include/nvkm/subdev/instmem.h |  2 +-
 drivers/gpu/drm/nouveau/nvkm/core/memory.c| 15 +--
 .../drm/nouveau/nvkm/subdev/instmem/base.c| 19 ++-
 .../drm/nouveau/nvkm/subdev/instmem/priv.h|  1 +
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/memory.h 
b/drivers/gpu/drm/nouveau/include/nvkm/core/memory.h
index d3b6a68ddda3..fc0f38981391 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/core/memory.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/core/memory.h
@@ -12,6 +12,7 @@ struct nvkm_tags {
 };
 
 enum nvkm_memory_target {
+   NVKM_MEM_TARGET_INST_SR_LOST, /* instance memory - not preserved across 
suspend */
NVKM_MEM_TARGET_INST, /* instance memory */
NVKM_MEM_TARGET_VRAM, /* video memory */
NVKM_MEM_TARGET_HOST, /* coherent system memory */
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h 
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h
index fcdaefc99fe8..92a36ddfc29f 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/instmem.h
@@ -26,7 +26,7 @@ struct nvkm_instmem {
 
 u32 nvkm_instmem_rd32(struct nvkm_instmem *, u32 addr);
 void nvkm_instmem_wr32(struct nvkm_instmem *, u32 addr, u32 data);
-int nvkm_instobj_new(struct nvkm_instmem *, u32 size, u32 align, bool zero,
+int nvkm_instobj_new(struct nvkm_instmem *, u32 size, u32 align, bool zero, 
bool preserve,
 struct nvkm_memory **);
 int nvkm_instobj_wrap(struct nvkm_device *, struct nvkm_memory *, struct 
nvkm_memory **);
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/memory.c 
b/drivers/gpu/drm/nouveau/nvkm/core/memory.c
index c69daac9bac7..a705c2dfca80 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/memory.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/memory.c
@@ -140,12 +140,23 @@ nvkm_memory_new(struct nvkm_device *device, enum 
nvkm_memory_target target,
 {
struct nvkm_instmem *imem = device->imem;
struct nvkm_memory *memory;
+   bool preserve = true;
int ret;
 
-   if (unlikely(target != NVKM_MEM_TARGET_INST || !imem))
+   if (unlikely(!imem))
return -ENOSYS;
 
-   ret = nvkm_instobj_new(imem, size, align, zero, );
+   switch (target) {
+   case NVKM_MEM_TARGET_INST_SR_LOST:
+   preserve = false;
+   break;
+   case NVKM_MEM_TARGET_INST:
+   break;
+   default:
+   return -ENOSYS;
+   }
+
+   ret = nvkm_instobj_new(imem, size, align, zero, preserve, );
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/base.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/base.c
index e0e4f97be029..24886eabe8dc 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/base.c
@@ -94,15 +94,21 @@ nvkm_instobj_wrap(struct nvkm_device *device,
  struct nvkm_memory *memory, struct nvkm_memory **pmemory)
 {
struct nvkm_instmem *imem = device->imem;
+   int ret;
 
if (!imem->func->memory_wrap)
return -ENOSYS;
 
-   return imem->func->memory_wrap(imem, memory, pmemory);
+   ret = imem->func->memory_wrap(imem, memory, pmemory);
+   if (ret)
+   return ret;
+
+   container_of(*pmemory, struct nvkm_instobj, memory)->preserve = true;
+   return 0;
 }
 
 int
-nvkm_instobj_new(struct nvkm_instmem *imem, u32 size, u32 align, bool zero,
+nvkm_instobj_new(struct nvkm_instmem *imem, u32 size, u32 align, bool zero, 
bool preserve,
 struct nvkm_memory **pmemory)
 {
struct nvkm_subdev *subdev = >subdev;
@@ -130,6 +136,7 @@ nvkm_instobj_new(struct nvkm_instmem *imem, u32 size, u32 
align, bool zero,
nvkm_done(memory);
}
 
+   container_of(memory, struct nvkm_instobj, memory)->preserve = preserve;
 done:
if (ret)
nvkm_memory_unref();
@@ -176,9 +183,11 @@ nvkm_instmem_fini(struct nvkm_subdev *subdev, bool suspend)
 
if (suspend) {
list_for_each_entry(iobj, >list, head) {
-   int ret = nvkm_instobj_save(iobj);
-   if (ret)
-   return ret;
+   if (iobj->preserve) {
+   int ret = nvkm_instobj_save(iobj);
+   if (ret)
+   return ret;
+   }
}
 
nvkm_bar_bar2_fini(subdev->device);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/priv.h 

[Nouveau] [PATCH 00/44] drm/nouveau: display rework to support GSP-RM

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

The primary issue being tackled here is that, for historical reasons (we
didn't know any better / couldn't make it work reliably otherwise), some
operations (SOR routing, DP link training) were performed during the 2nd
HW supervisor interrupt.

We don't have control of the display supervisor when running on top of
RM, so this needed to be untangled and fixed - which, is one of the main
aims of this patch series.

The ordering of this series is pretty important, so take care if/when
backporting patches from it.

Beyond that main goal, various other interfaces have been added or
extended to provide the information that RM will need for its version of
similar interfaces.

Ben Skeggs (43):
  drm/nouveau/devinit/tu102-: remove attempt at loading PreOS
  drm/nouveau/imem: support allocations not preserved across suspend
  drm/nouveau/gr/gf100-: lose contents of global ctxbufs across suspend
  drm/nouveau/mmu/gp100-: always invalidate TLBs at CACHE_LEVEL_ALL
  drm/nouveau/kms/nv50-: fix mst payload alloc fail crashing evo
  drm/nouveau/disp: rearrange output methods
  drm/nouveau/disp: add output detect method
  drm/nouveau/disp: add output method to fetch edid
  drm/nouveau/disp: rename internal output acquire/release functions
  drm/nouveau/disp: shuffle to make upcoming diffs prettier
  drm/nouveau/disp: add acquire_dac()
  drm/nouveau/disp: add acquire_sor/pior()
  drm/nouveau/disp: update SOR routing immediately on acquire()
  drm/nouveau/kms/nv50-: pull some common init out of OR-specific code
  drm/nouveau/kms/nv50-: remove nv_encoder.audio.connector
  drm/nouveau/kms/nv50-: keep output state around until modeset complete
  drm/nouveau/kms/nv50-: move audio enable post-modeset
  drm/nouveau/disp: add output hdmi config method
  drm/nouveau/disp: move hdmi disable out of release()
  drm/nouveau/disp: release outputs post-modeset
  drm/nouveau/disp: remove SOR routing updates from supervisor
  drm/nouveau/disp: add output backlight control methods
  drm/nouveau/disp: add output lvds config method
  drm/nouveau/disp: add hdmi audio hal function
  drm/nouveau/disp: move dp aux pwr method to HAL
  drm/nouveau/disp: add dp aux xfer method
  drm/nouveau/disp: add dp rates method
  drm/nouveau/kms/nv50-: split DP disable+enable into two modesets
  drm/nouveau/kms/nv50-: flush mst disables together
  drm/nouveau/kms/nv50-: fixup sink D3 before tearing down link
  drm/nouveau/disp: add dp train method
  drm/nouveau/disp: move link training out of supervisor
  drm/nouveau/disp: add support for post-LT adjust
  drm/nouveau/disp: add dp sst config method
  drm/nouveau/disp: add dp mst id get/put methods
  drm/nouveau/disp: move outp/conn construction to chipset code
  drm/nouveau/disp: move outp init/fini paths to chipset code
  drm/nouveau/kms/nv50-: create heads based on nvkm head mask
  drm/nouveau/kms/nv50-: create heads after outps/conns
  drm/nouveau/kms/nv50-: name aux channels after their connector
  drm/nouveau/kms/nv50-: create connectors based on nvkm info
  drm/nouveau/kms/nv50-: create outputs based on nvkm info
  drm/nouveau/kms/nv50-: disable dcb parsing

Lyude Paul (1):
  drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR state

 drivers/gpu/drm/nouveau/dispnv04/disp.c   |   2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 512 +++---
 drivers/gpu/drm/nouveau/dispnv50/disp.h   |   6 +-
 drivers/gpu/drm/nouveau/dispnv50/head.h   |   1 +
 drivers/gpu/drm/nouveau/dispnv50/headc57d.c   |  14 +
 drivers/gpu/drm/nouveau/include/nvif/conn.h   |  20 +-
 drivers/gpu/drm/nouveau/include/nvif/if0011.h |  21 +-
 drivers/gpu/drm/nouveau/include/nvif/if0012.h | 249 +++--
 drivers/gpu/drm/nouveau/include/nvif/outp.h   |  96 +++-
 .../drm/nouveau/include/nvkm/core/memory.h|   1 +
 .../drm/nouveau/include/nvkm/subdev/instmem.h |   2 +-
 drivers/gpu/drm/nouveau/nouveau_backlight.c   |  90 +--
 drivers/gpu/drm/nouveau/nouveau_bios.c|   8 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   | 251 -
 drivers/gpu/drm/nouveau/nouveau_connector.h   |   3 +-
 drivers/gpu/drm/nouveau/nouveau_display.c |   8 +-
 drivers/gpu/drm/nouveau/nouveau_dp.c  | 345 ++--
 drivers/gpu/drm/nouveau/nouveau_encoder.h |  30 +-
 drivers/gpu/drm/nouveau/nvif/conn.c   |  36 +-
 drivers/gpu/drm/nouveau/nvif/disp.c   |   2 +-
 drivers/gpu/drm/nouveau/nvif/outp.c   | 412 --
 drivers/gpu/drm/nouveau/nvkm/core/memory.c|  15 +-
 .../gpu/drm/nouveau/nvkm/engine/disp/base.c   | 146 +
 .../gpu/drm/nouveau/nvkm/engine/disp/conn.c   |  10 -
 .../gpu/drm/nouveau/nvkm/engine/disp/conn.h   |   2 -
 drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c | 362 -
 .../gpu/drm/nouveau/nvkm/engine/disp/g84.c|   1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/g94.c|   1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/ga102.c  |   1 +
 .../gpu/drm/nouveau/nvkm/engine/disp/gf119.c  |   2 +-
 

[Nouveau] [PATCH 01/44] drm/nouveau/devinit/tu102-: remove attempt at loading PreOS

2023-09-14 Thread Ben Skeggs
From: Ben Skeggs 

>From Turing, HW will already have handled this and locked-down the
falcon before we get control.  So this *should* be a no-op.

Signed-off-by: Ben Skeggs 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/devinit/tu102.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/tu102.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/tu102.c
index 81a1ad2c88a7..40997ad1d101 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/tu102.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/tu102.c
@@ -83,17 +83,9 @@ tu102_devinit_wait(struct nvkm_device *device)
 }
 
 int
-tu102_devinit_post(struct nvkm_devinit *base, bool post)
+tu102_devinit_post(struct nvkm_devinit *init, bool post)
 {
-   struct nv50_devinit *init = nv50_devinit(base);
-   int ret;
-
-   ret = tu102_devinit_wait(init->base.subdev.device);
-   if (ret)
-   return ret;
-
-   gm200_devinit_preos(init, post);
-   return 0;
+   return tu102_devinit_wait(init->subdev.device);
 }
 
 static const struct nvkm_devinit_func
-- 
2.41.0