[PATCH AUTOSEL 6.6 8/8] nouveau: fix disp disabling with GSP

2024-01-08 Thread Sasha Levin
From: Dave Airlie 

[ Upstream commit 7854ea0e408d7f2e8faaada1773f3ddf9cb538f5 ]

This func ptr here is normally static allocation, but gsp r535
uses a dynamic pointer, so we need to handle that better.

This fixes a crash with GSP when you use config=disp=0 to avoid
disp problems.

Signed-off-by: Dave Airlie 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20231222043308.3090089-4-airl...@gmail.com
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
index 73104b59f97fe..54a3017f4756a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
@@ -276,7 +276,7 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
list_add_tail(&outp->conn->head, &disp->conns);
}
 
-   if (disp->func->oneinit) {
+   if (disp->func && disp->func->oneinit) {
ret = disp->func->oneinit(disp);
if (ret)
return ret;
@@ -377,8 +377,10 @@ nvkm_disp_new_(const struct nvkm_disp_func *func, struct 
nvkm_device *device,
spin_lock_init(&disp->client.lock);
 
ret = nvkm_engine_ctor(&nvkm_disp, device, type, inst, true, 
&disp->engine);
-   if (ret)
+   if (ret) {
+   disp->func = NULL;
return ret;
+   }
 
if (func->super) {
disp->super.wq = create_singlethread_workqueue("nvkm-disp");
-- 
2.43.0



[PATCH AUTOSEL 6.1 5/5] nouveau: fix disp disabling with GSP

2024-01-08 Thread Sasha Levin
From: Dave Airlie 

[ Upstream commit 7854ea0e408d7f2e8faaada1773f3ddf9cb538f5 ]

This func ptr here is normally static allocation, but gsp r535
uses a dynamic pointer, so we need to handle that better.

This fixes a crash with GSP when you use config=disp=0 to avoid
disp problems.

Signed-off-by: Dave Airlie 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20231222043308.3090089-4-airl...@gmail.com
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
index 65c99d948b686..ae47eabd5d0bd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
@@ -359,7 +359,7 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
if (ret)
return ret;
 
-   if (disp->func->oneinit) {
+   if (disp->func && disp->func->oneinit) {
ret = disp->func->oneinit(disp);
if (ret)
return ret;
@@ -461,8 +461,10 @@ nvkm_disp_new_(const struct nvkm_disp_func *func, struct 
nvkm_device *device,
spin_lock_init(&disp->client.lock);
 
ret = nvkm_engine_ctor(&nvkm_disp, device, type, inst, true, 
&disp->engine);
-   if (ret)
+   if (ret) {
+   disp->func = NULL;
return ret;
+   }
 
if (func->super) {
disp->super.wq = create_singlethread_workqueue("nvkm-disp");
-- 
2.43.0



Re: [PATCH linux-next] drm/nouveau/disp: switch to use kmemdup() helper

2024-01-08 Thread Danilo Krummrich

Hi Yang,

On 12/14/23 13:03, yang.gua...@zte.com.cn wrote:

From: Yang Guang 

Use kmemdup() helper instead of open-coding to
simplify the code.

Signed-off-by: Chen Haonan 


Please add your "Signed-off-by" tag to this patch. If the above is intended to 
indicate
that Chan was involved in creating this patch (e.g. as co-author, reporter, 
etc.) please
use the corresponding tag instead. See also [1].

Once this is fixed, I'll apply the patch.

- Danilo

[1] 
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin


---
  drivers/gpu/drm/nouveau/nvif/outp.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c 
b/drivers/gpu/drm/nouveau/nvif/outp.c
index 5d3190c05250..6daeb7f0b09b 100644
--- a/drivers/gpu/drm/nouveau/nvif/outp.c
+++ b/drivers/gpu/drm/nouveau/nvif/outp.c
@@ -452,13 +452,12 @@ nvif_outp_edid_get(struct nvif_outp *outp, u8 **pedid)
if (ret)
goto done;

-   *pedid = kmalloc(args->size, GFP_KERNEL);
+   *pedid = kmemdup(args->data, args->size, GFP_KERNEL);
if (!*pedid) {
ret = -ENOMEM;
goto done;
}

-   memcpy(*pedid, args->data, args->size);
ret = args->size;
  done:
kfree(args);




Re: [PATCH] drm/nouveau/fifo: remove duplicated including

2024-01-08 Thread Danilo Krummrich

Hi Wang,

there is another patch [1] to fix this, which already made it upstream.

- Danilo

[1] 
https://patchwork.freedesktop.org/patch/msgid/20231122004926.84933-1-yang@linux.alibaba.com

On 12/15/23 11:02, Wang Jinchao wrote:

rm second including of chid.h

Signed-off-by: Wang Jinchao 
---
  drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c
index 87a62d4ff4bd..7d4716dcd512 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chan.c
@@ -24,7 +24,6 @@
  #include "chan.h"
  #include "chid.h"
  #include "cgrp.h"
-#include "chid.h"
  #include "runl.h"
  #include "priv.h"
  




Re: [PATCH] drm/nouveau/bios/init: drop kernel-doc notation

2024-01-08 Thread Danilo Krummrich

On 12/16/23 21:11, Randy Dunlap wrote:

The "/**" comments in this file are not kernel-doc comments. They are
used on static functions which can have kernel-doc comments, but that
is not the primary focus of kernel-doc comments.
Since these comments are incomplete for kernel-doc notation, remove
the kernel-doc "/**" markers and make them common comments.

This prevents scripts/kernel-doc from issuing 68 warnings:

init.c:584: warning: Function parameter or member 'init' not described in 
'init_reserved'

and 67 warnings like this one:
init.c:611: warning: expecting prototype for INIT_DONE(). Prototype was for 
init_done() instead

Signed-off-by: Randy Dunlap 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: dri-de...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 


Applied to drm-misc-next, thanks!


---
  drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c |  136 +++---
  1 file changed, 68 insertions(+), 68 deletions(-)

diff -- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c
@@ -575,7 +575,7 @@ init_tmds_reg(struct nvbios_init *init,
   * init opcode handlers
   
*/
  
-/**

+/*
   * init_reserved - stub for various unknown/unused single-byte opcodes
   *
   */
@@ -602,7 +602,7 @@ init_reserved(struct nvbios_init *init)
init->offset += length;
  }
  
-/**

+/*
   * INIT_DONE - opcode 0x71
   *
   */
@@ -613,7 +613,7 @@ init_done(struct nvbios_init *init)
init->offset = 0x;
  }
  
-/**

+/*
   * INIT_IO_RESTRICT_PROG - opcode 0x32
   *
   */
@@ -650,7 +650,7 @@ init_io_restrict_prog(struct nvbios_init
trace("}]\n");
  }
  
-/**

+/*
   * INIT_REPEAT - opcode 0x33
   *
   */
@@ -676,7 +676,7 @@ init_repeat(struct nvbios_init *init)
init->repeat = repeat;
  }
  
-/**

+/*
   * INIT_IO_RESTRICT_PLL - opcode 0x34
   *
   */
@@ -716,7 +716,7 @@ init_io_restrict_pll(struct nvbios_init
trace("}]\n");
  }
  
-/**

+/*
   * INIT_END_REPEAT - opcode 0x36
   *
   */
@@ -732,7 +732,7 @@ init_end_repeat(struct nvbios_init *init
}
  }
  
-/**

+/*
   * INIT_COPY - opcode 0x37
   *
   */
@@ -759,7 +759,7 @@ init_copy(struct nvbios_init *init)
init_wrvgai(init, port, index, data);
  }
  
-/**

+/*
   * INIT_NOT - opcode 0x38
   *
   */
@@ -771,7 +771,7 @@ init_not(struct nvbios_init *init)
init_exec_inv(init);
  }
  
-/**

+/*
   * INIT_IO_FLAG_CONDITION - opcode 0x39
   *
   */
@@ -788,7 +788,7 @@ init_io_flag_condition(struct nvbios_ini
init_exec_set(init, false);
  }
  
-/**

+/*
   * INIT_GENERIC_CONDITION - opcode 0x3a
   *
   */
@@ -840,7 +840,7 @@ init_generic_condition(struct nvbios_ini
}
  }
  
-/**

+/*
   * INIT_IO_MASK_OR - opcode 0x3b
   *
   */
@@ -859,7 +859,7 @@ init_io_mask_or(struct nvbios_init *init
init_wrvgai(init, 0x03d4, index, data &= ~(1 << or));
  }
  
-/**

+/*
   * INIT_IO_OR - opcode 0x3c
   *
   */
@@ -878,7 +878,7 @@ init_io_or(struct nvbios_init *init)
init_wrvgai(init, 0x03d4, index, data | (1 << or));
  }
  
-/**

+/*
   * INIT_ANDN_REG - opcode 0x47
   *
   */
@@ -895,7 +895,7 @@ init_andn_reg(struct nvbios_init *init)
init_mask(init, reg, mask, 0);
  }
  
-/**

+/*
   * INIT_OR_REG - opcode 0x48
   *
   */
@@ -912,7 +912,7 @@ init_or_reg(struct nvbios_init *init)
init_mask(init, reg, 0, mask);
  }
  
-/**

+/*
   * INIT_INDEX_ADDRESS_LATCHED - opcode 0x49
   *
   */
@@ -942,7 +942,7 @@ init_idx_addr_latched(struct nvbios_init
}
  }
  
-/**

+/*
   * INIT_IO_RESTRICT_PLL2 - opcode 0x4a
   *
   */
@@ -977,7 +977,7 @@ init_io_restrict_pll2(struct nvbios_init
trace("}]\n");
  }
  
-/**

+/*
   * INIT_PLL2 - opcode 0x4b
   *
   */
@@ -994,7 +994,7 @@ init_pll2(struct nvbios_init *init)
init_prog_pll(init, reg, freq);
  }
  
-/**

+/*
   * INIT_I2C_BYTE - opcode 0x4c
   *
   */
@@ -1025,7 +1025,7 @@ init_i2c_byte(struct nvbios_init *init)
}
  }
  
-/**

+/*
   * INIT_ZM_I2C_BYTE - opcode 0x4d
   *
   */
@@ -1051,7 +1051,7 @@ init_zm_i2c_byte(struct nvbios_init *ini
}
  }
  
-/**

+/*
   * INIT_ZM_I2C - opcode 0x4e
   *
   */
@@ -1085,7 +1085,7 @@ init_zm_i2c(struct nvbios_init *init)
}
  }
  
-/**

+/*
   * INIT_TMDS - opcode 0x4f
   *
   */
@@ -,7 +,7 @@ init_tmds(struct nvbios_init *init)
init_wr32(init, reg + 0, addr);
  }
  
-/**

+/*
   * INIT_ZM_TMDS_GROUP - opcode 0x50
   *
   */
@@ -1138,7 +1138,7 @@ init_zm_tmds_group(struct nvbios_init *i
}
  }
  
-/**

+/*
   * INIT_CR_INDEX_ADDRESS_LATCHED - opcode 0x51
   *
   */
@@ -1168,7 +1168,7 @@ init_cr_idx_adr_latch(struct nvbios_init
init_wrvgai(init, 0x03d4, addr0, save0);
  }
 

Re: [PATCH 1/4] drm/nouveau/disp: don't misuse kernel-doc comments

2024-01-08 Thread Danilo Krummrich

On 1/1/24 00:36, Randy Dunlap wrote:

Change kernel-doc "/**" comments to common "/*" comments to prevent
kernel-doc warnings:

crtc.c:453: warning: This comment starts with '/**', but isn't a kernel-doc 
comment. Refer Documentation/doc-guide/kernel-doc.rst
  * Sets up registers for the given mode/adjusted_mode pair.
crtc.c:453: warning: missing initial short description on line:
  * Sets up registers for the given mode/adjusted_mode pair.
crtc.c:629: warning: This comment starts with '/**', but isn't a kernel-doc 
comment. Refer Documentation/doc-guide/kernel-doc.rst
  * Sets up registers for the given mode/adjusted_mode pair.
crtc.c:629: warning: missing initial short description on line:
  * Sets up registers for the given mode/adjusted_mode pair.

Signed-off-by: Randy Dunlap 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: nouveau@lists.freedesktop.org
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 


Series applied to drm-misc-next, thanks!


---
  drivers/gpu/drm/nouveau/dispnv04/crtc.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff -- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -449,7 +449,7 @@ nv_crtc_mode_set_vga(struct drm_crtc *cr
regp->Attribute[NV_CIO_AR_CSEL_INDEX] = 0x00;
  }
  
-/**

+/*
   * Sets up registers for the given mode/adjusted_mode pair.
   *
   * The clocks, CRTCs and outputs attached to this CRTC must be off.
@@ -625,7 +625,7 @@ nv_crtc_swap_fbs(struct drm_crtc *crtc,
return ret;
  }
  
-/**

+/*
   * Sets up registers for the given mode/adjusted_mode pair.
   *
   * The clocks, CRTCs and outputs attached to this CRTC must be off.





Re: [PATCH 1/3] drm/nouveau: include drm/drm_edid.h only where needed

2024-01-08 Thread Danilo Krummrich

On 1/4/24 21:16, Jani Nikula wrote:

Including drm_edid.h from nouveau_connector.h causes the rebuild of 15
files when drm_edid.h is modified, while there are only a few files that
actually need to include drm_edid.h.

Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Jani Nikula 


Reviewed-by: Danilo Krummrich 


---
  drivers/gpu/drm/nouveau/dispnv50/head.c | 1 +
  drivers/gpu/drm/nouveau/nouveau_connector.h | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c 
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 5f490fbf1877..83355dbc15ee 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -32,6 +32,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include "nouveau_connector.h"
  
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h

index a2df4918340c..0608cabed058 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -35,7 +35,6 @@
  
  #include 

  #include 
-#include 
  #include 
  #include 
  
@@ -44,6 +43,7 @@
  
  struct nvkm_i2c_port;

  struct dcb_output;
+struct edid;
  
  #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT

  struct nouveau_backlight {




Re: [PATCH -next] drm/nouveau: uapi: fix kerneldoc warnings

2024-01-08 Thread Danilo Krummrich

On 12/25/23 07:51, Vegard Nossum wrote:

As of commit b77fdd6a48e6 ("scripts/kernel-doc: restore warning for
Excess struct/union"), we see the following warnings when running 'make
htmldocs':

   ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_OP_MAP' description in 'drm_nouveau_vm_bind_op'
   ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_OP_UNMAP' description in 'drm_nouveau_vm_bind_op'
   ./include/uapi/drm/nouveau_drm.h:292: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_SPARSE' description in 'drm_nouveau_vm_bind_op'
   ./include/uapi/drm/nouveau_drm.h:336: warning: Excess struct member 
'DRM_NOUVEAU_VM_BIND_RUN_ASYNC' description in 'drm_nouveau_vm_bind'

The problem is that these values are #define constants, but had kerneldoc
comments attached to them as if they were actual struct members.

There are a number of ways we could fix this, but I chose to draw
inspiration from include/uapi/drm/i915_drm.h, which pulls them into the
corresponding kerneldoc comment for the struct member that they are
intended to be used with.

To keep the diff readable, there are a number of things I _didn't_ do in
this patch, but which we should also consider:

- This is pretty good documentation, but it ends up in gpu/driver-uapi,
   which is part of subsystem-apis/ when it really ought to display under
   userspace-api/ (the "Linux kernel user-space API guide" book of the
   documentation).


I agree, it indeed looks like this would make sense, same goes for
gpu/drm-uapi.rst.

@Jani, Sima: Was this intentional? Or can we change it?



- More generally, we might want a warning if include/uapi/ files are
   kerneldoc'd outside userspace-api/.

- I'd consider it cleaner if the #defines appeared between the kerneldoc
   for the member and the member itself (which is something other DRM-
   related UAPI docs do).

- The %IDENTIFIER kerneldoc syntax is intended for "constants", and is
   more appropriate in this context than ``IDENTIFIER`` or &IDENTIFIER.
   The DRM docs aren't very consistent on this.

Cc: Randy Dunlap 
Cc: Jonathan Corbet 
Signed-off-by: Vegard Nossum 


Applied to drm-misc-next, thanks!


---
  include/uapi/drm/nouveau_drm.h | 56 --
  1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index 0bade1592f34..c95ef8a4d94a 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -238,34 +238,32 @@ struct drm_nouveau_vm_init {
  struct drm_nouveau_vm_bind_op {
/**
 * @op: the operation type
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_OP_MAP - Map a GEM object to the GPU's VA
+* space. Optionally, the &DRM_NOUVEAU_VM_BIND_SPARSE flag can be
+* passed to instruct the kernel to create sparse mappings for the
+* given range.
+*
+* %DRM_NOUVEAU_VM_BIND_OP_UNMAP - Unmap an existing mapping in the
+* GPU's VA space. If the region the mapping is located in is a
+* sparse region, new sparse mappings are created where the unmapped
+* (memory backed) mapping was mapped previously. To remove a sparse
+* region the &DRM_NOUVEAU_VM_BIND_SPARSE must be set.
 */
__u32 op;
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_MAP:
- *
- * Map a GEM object to the GPU's VA space. Optionally, the
- * &DRM_NOUVEAU_VM_BIND_SPARSE flag can be passed to instruct the kernel to
- * create sparse mappings for the given range.
- */
  #define DRM_NOUVEAU_VM_BIND_OP_MAP 0x0
-/**
- * @DRM_NOUVEAU_VM_BIND_OP_UNMAP:
- *
- * Unmap an existing mapping in the GPU's VA space. If the region the mapping
- * is located in is a sparse region, new sparse mappings are created where the
- * unmapped (memory backed) mapping was mapped previously. To remove a sparse
- * region the &DRM_NOUVEAU_VM_BIND_SPARSE must be set.
- */
  #define DRM_NOUVEAU_VM_BIND_OP_UNMAP 0x1
/**
 * @flags: the flags for a &drm_nouveau_vm_bind_op
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_SPARSE - Indicates that an allocated VA
+* space region should be sparse.
 */
__u32 flags;
-/**
- * @DRM_NOUVEAU_VM_BIND_SPARSE:
- *
- * Indicates that an allocated VA space region should be sparse.
- */
  #define DRM_NOUVEAU_VM_BIND_SPARSE (1 << 8)
/**
 * @handle: the handle of the DRM GEM object to map
@@ -301,17 +299,17 @@ struct drm_nouveau_vm_bind {
__u32 op_count;
/**
 * @flags: the flags for a &drm_nouveau_vm_bind ioctl
+*
+* Supported values:
+*
+* %DRM_NOUVEAU_VM_BIND_RUN_ASYNC - Indicates that the given VM_BIND
+* operation should be executed asynchronously by the kernel.
+*
+* If this flag is not supplied the kernel executes the associated
+* operations synchronous

Re: [PATCH AUTOSEL 6.1 5/5] nouveau: fix disp disabling with GSP

2024-01-08 Thread David Airlie
NAK for backporting this to anything, it is just a fix for 6.7


On Mon, Jan 8, 2024 at 10:28 PM Sasha Levin  wrote:
>
> From: Dave Airlie 
>
> [ Upstream commit 7854ea0e408d7f2e8faaada1773f3ddf9cb538f5 ]
>
> This func ptr here is normally static allocation, but gsp r535
> uses a dynamic pointer, so we need to handle that better.
>
> This fixes a crash with GSP when you use config=disp=0 to avoid
> disp problems.
>
> Signed-off-by: Dave Airlie 
> Link: 
> https://patchwork.freedesktop.org/patch/msgid/20231222043308.3090089-4-airl...@gmail.com
> Signed-off-by: Sasha Levin 
> ---
>  drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c 
> b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
> index 65c99d948b686..ae47eabd5d0bd 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/base.c
> @@ -359,7 +359,7 @@ nvkm_disp_oneinit(struct nvkm_engine *engine)
> if (ret)
> return ret;
>
> -   if (disp->func->oneinit) {
> +   if (disp->func && disp->func->oneinit) {
> ret = disp->func->oneinit(disp);
> if (ret)
> return ret;
> @@ -461,8 +461,10 @@ nvkm_disp_new_(const struct nvkm_disp_func *func, struct 
> nvkm_device *device,
> spin_lock_init(&disp->client.lock);
>
> ret = nvkm_engine_ctor(&nvkm_disp, device, type, inst, true, 
> &disp->engine);
> -   if (ret)
> +   if (ret) {
> +   disp->func = NULL;
> return ret;
> +   }
>
> if (func->super) {
> disp->super.wq = create_singlethread_workqueue("nvkm-disp");
> --
> 2.43.0
>



[PATCH 1/5] drm/vmwgfx: remove vmw_vram_gmr_placement

2024-01-08 Thread Christian König
Seems to be unused.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 28 --
 2 files changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 3cd5090dedfc..12efecc17df6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -942,7 +942,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private 
*dev_priv)
 
 extern const size_t vmw_tt_size;
 extern struct ttm_placement vmw_vram_placement;
-extern struct ttm_placement vmw_vram_gmr_placement;
 extern struct ttm_placement vmw_sys_placement;
 extern struct ttm_device_funcs vmw_bo_driver;
 extern const struct vmw_sg_table *
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index af8562c95cc3..a84fffcef8e1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -43,13 +43,6 @@ static const struct ttm_place sys_placement_flags = {
.flags = 0
 };
 
-static const struct ttm_place gmr_placement_flags = {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = VMW_PL_GMR,
-   .flags = 0
-};
-
 struct ttm_placement vmw_vram_placement = {
.num_placement = 1,
.placement = &vram_placement_flags,
@@ -57,27 +50,6 @@ struct ttm_placement vmw_vram_placement = {
.busy_placement = &vram_placement_flags
 };
 
-static const struct ttm_place vram_gmr_placement_flags[] = {
-   {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = TTM_PL_VRAM,
-   .flags = 0
-   }, {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = VMW_PL_GMR,
-   .flags = 0
-   }
-};
-
-struct ttm_placement vmw_vram_gmr_placement = {
-   .num_placement = 2,
-   .placement = vram_gmr_placement_flags,
-   .num_busy_placement = 1,
-   .busy_placement = &gmr_placement_flags
-};
-
 struct ttm_placement vmw_sys_placement = {
.num_placement = 1,
.placement = &sys_placement_flags,
-- 
2.34.1



Rework TTMs busy handling

2024-01-08 Thread Christian König
Hi guys,

I'm trying to make this functionality a bit more useful for years now
since we multiple reports that behavior of drivers can be suboptimal
when multiple placements be given.

So basically instead of hacking around the TTM behavior in the driver
once more I've gone ahead and changed the idle/busy placement list
into idle/busy placement flags. This not only saves a bunch of code,
but also allows setting some placements as fallback which are used if
allocating from the preferred ones didn't worked.

Zack pointed out that some removed VMWGFX code was brought back because
of rebasing, fixed in this version.

Intel CI seems to be happy with those patches, so any more comments?

Regards,
Christian.




[PATCH 3/5] drm/ttm: replace busy placement with flags v5

2024-01-08 Thread Christian König
From: Somalapuram Amaranath 

Instead of a list of separate busy placement add flags which indicate
that a placement should only be used when there is room or if we need to
evict.

v2: add missing TTM_PL_FLAG_IDLE for i915
v3: fix auto build test ERROR on drm-tip/drm-tip
v4: fix some typos pointed out by checkpatch
v5: cleanup some rebase problems with VMWGFX

Signed-off-by: Christian König 
Signed-off-by: Somalapuram Amaranath 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 11 +---
 drivers/gpu/drm/drm_gem_vram_helper.c  |  2 -
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 37 +--
 drivers/gpu/drm/loongson/lsdc_ttm.c|  2 -
 drivers/gpu/drm/nouveau/nouveau_bo.c   | 59 +++--
 drivers/gpu/drm/nouveau/nouveau_bo.h   |  1 -
 drivers/gpu/drm/qxl/qxl_object.c   |  2 -
 drivers/gpu/drm/qxl/qxl_ttm.c  |  2 -
 drivers/gpu/drm/radeon/radeon_object.c |  2 -
 drivers/gpu/drm/radeon/radeon_ttm.c|  8 +--
 drivers/gpu/drm/radeon/radeon_uvd.c|  1 -
 drivers/gpu/drm/ttm/ttm_bo.c   | 21 ---
 drivers/gpu/drm/ttm/ttm_resource.c | 73 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  2 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  4 --
 include/drm/ttm/ttm_placement.h| 10 +--
 include/drm/ttm/ttm_resource.h |  8 +--
 18 files changed, 79 insertions(+), 172 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index cef920a93924..aa0dd6dad068 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -220,9 +220,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, 
u32 domain)
 
placement->num_placement = c;
placement->placement = places;
-
-   placement->num_busy_placement = c;
-   placement->busy_placement = places;
 }
 
 /**
@@ -1406,8 +1403,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct 
ttm_buffer_object *bo)
AMDGPU_GEM_DOMAIN_GTT);
 
/* Avoid costly evictions; only set GTT as a busy placement */
-   abo->placement.num_busy_placement = 1;
-   abo->placement.busy_placement = &abo->placements[1];
+   abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
 
r = ttm_bo_validate(bo, &abo->placement, &ctx);
if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 05991c5c8ddb..9a6a00b1af40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -102,23 +102,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
/* Don't handle scatter gather BOs */
if (bo->type == ttm_bo_type_sg) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
 
/* Object isn't an AMDGPU object so ignore */
if (!amdgpu_bo_is_amdgpu_bo(bo)) {
placement->placement = &placements;
-   placement->busy_placement = &placements;
placement->num_placement = 1;
-   placement->num_busy_placement = 1;
return;
}
 
abo = ttm_to_amdgpu_bo(bo);
if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
}
 
@@ -128,13 +124,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
case AMDGPU_PL_OA:
case AMDGPU_PL_DOORBELL:
placement->num_placement = 0;
-   placement->num_busy_placement = 0;
return;
 
case TTM_PL_VRAM:
if (!adev->mman.buffer_funcs_enabled) {
/* Move to system memory */
amdgpu_bo_placement_from_domain(abo, 
AMDGPU_GEM_DOMAIN_CPU);
+
} else if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
   !(abo->flags & 
AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&
   amdgpu_bo_in_cpu_visible_vram(abo)) {
@@ -149,8 +145,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
AMDGPU_GEM_DOMAIN_CPU);
abo->placements[0].fpfn = adev->gmc.visible_vram_size 
>> PAGE_SHIFT;
abo->placements[0].lpfn = 0;
-   abo->placement.busy_placement = &abo->placements[1];
-   abo->placement.num_busy_placement = 1;
+   abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
} else {
/* Move to GTT memory */
amdgpu_bo_placement_from_domain(abo, 
AMDGPU_GEM_DOMAIN_GTT |
@@ -967,8 +962,6 @

[PATCH 4/5] drm/ttm: improve idle/busy handling v2

2024-01-08 Thread Christian König
Previously we would never try to move a BO into the preferred placements
when it ever landed in a busy placement since those were considered
compatible.

Rework the whole handling and finally unify the idle and busy handling.
ttm_bo_validate() is now responsible to try idle placement first and then
use the busy placement if that didn't worked.

Drawback is that we now always try the idle placement first for each
validation which might cause some additional CPU overhead on overcommit.

v2: fix kerneldoc warning and coding style

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|   2 +-
 drivers/gpu/drm/ttm/ttm_bo.c   | 131 -
 drivers/gpu/drm/ttm/ttm_resource.c |  15 ++-
 include/drm/ttm/ttm_bo.h   |   3 +-
 include/drm/ttm/ttm_resource.h |   3 +-
 6 files changed, 65 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index aa0dd6dad068..f110dfdc4feb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -404,7 +404,7 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
(*bo_ptr)->placements[i].lpfn = (offset + size) >> PAGE_SHIFT;
}
r = ttm_bo_mem_space(&(*bo_ptr)->tbo, &(*bo_ptr)->placement,
-&(*bo_ptr)->tbo.resource, &ctx);
+&(*bo_ptr)->tbo.resource, &ctx, false);
if (r)
goto error;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9a6a00b1af40..00da9a81cf6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -967,7 +967,7 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
placements.mem_type = TTM_PL_TT;
placements.flags = bo->resource->placement;
 
-   r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
+   r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx, true);
if (unlikely(r))
return r;
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index aa12bd5cfd17..17bfc252f76d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -414,7 +414,7 @@ static int ttm_bo_bounce_temp_buffer(struct 
ttm_buffer_object *bo,
hop_placement.placement = hop;
 
/* find space in the bounce domain */
-   ret = ttm_bo_mem_space(bo, &hop_placement, &hop_mem, ctx);
+   ret = ttm_bo_mem_space(bo, &hop_placement, &hop_mem, ctx, true);
if (ret)
return ret;
/* move to the bounce domain */
@@ -454,7 +454,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
return ttm_bo_pipeline_gutting(bo);
}
 
-   ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
+   ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx, true);
if (ret) {
if (ret != -ERESTARTSYS) {
pr_err("Failed to find memory space for buffer 0x%p 
eviction\n",
@@ -724,37 +724,6 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object 
*bo,
return ret;
 }
 
-/*
- * Repeatedly evict memory from the LRU for @mem_type until we create enough
- * space, or we've evicted everything and there isn't enough space.
- */
-static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
- const struct ttm_place *place,
- struct ttm_resource **mem,
- struct ttm_operation_ctx *ctx)
-{
-   struct ttm_device *bdev = bo->bdev;
-   struct ttm_resource_manager *man;
-   struct ww_acquire_ctx *ticket;
-   int ret;
-
-   man = ttm_manager_type(bdev, place->mem_type);
-   ticket = dma_resv_locking_ctx(bo->base.resv);
-   do {
-   ret = ttm_resource_alloc(bo, place, mem);
-   if (likely(!ret))
-   break;
-   if (unlikely(ret != -ENOSPC))
-   return ret;
-   ret = ttm_mem_evict_first(bdev, man, place, ctx,
- ticket);
-   if (unlikely(ret != 0))
-   return ret;
-   } while (1);
-
-   return ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
-}
-
 /**
  * ttm_bo_mem_space
  *
@@ -763,6 +732,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object 
*bo,
  * @placement: Proposed new placement for the buffer object.
  * @mem: A struct ttm_resource.
  * @ctx: if and how to sleep, lock buffers and alloc memory
+ * @force_space: If we should evict buffers to force space
  *
  * Allocate memory space for the buffer object pointed to by @bo, using
  * the placement flags in @placement, potentially evicting other idle buffer 
objects.
@@ -776,12 +746,14 

[PATCH 2/5] drm/ttm: return ENOSPC from ttm_bo_mem_space

2024-01-08 Thread Christian König
Only convert it to ENOMEM in ttm_bo_validate.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index edf10618fe2b..8c1eaa74fa21 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -830,7 +830,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
goto error;
}
 
-   ret = -ENOMEM;
+   ret = -ENOSPC;
if (!type_found) {
pr_err(TTM_PFX "No compatible memory type found\n");
ret = -EINVAL;
@@ -916,6 +916,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
return -EINVAL;
 
ret = ttm_bo_move_buffer(bo, placement, ctx);
+   /* For backward compatibility with userspace */
+   if (ret == -ENOSPC)
+   return -ENOMEM;
if (ret)
return ret;
 
-- 
2.34.1



[PATCH 5/5] drm/amdgpu: use GTT only as fallback for VRAM|GTT

2024-01-08 Thread Christian König
Try to fill up VRAM as well by setting the busy flag on GTT allocations.

This fixes the issue that when VRAM was evacuated for suspend it's never
filled up again unless the application is restarted.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index f110dfdc4feb..979cecf18f17 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo 
*abo, u32 domain)
abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ?
AMDGPU_PL_PREEMPT : TTM_PL_TT;
places[c].flags = 0;
+   /*
+* When GTT is just an alternative to VRAM make sure that we
+* only use it as fallback and still try to fill up VRAM first.
+*/
+   if (domain & abo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)
+   places[c].flags |= TTM_PL_FLAG_BUSY;
c++;
}
 
-- 
2.34.1