Re: [PATCH] drm/amdgpu: remove unused variable

[Christian König]

Am 19.01.22 um 07:42 schrieb mziya:

Remove set but unused variable.
warning: variable 'umc_reg_offset' set but not used

Signed-off-by: mziya 


You should use your full written name here.

Apart from that nit the patch looks good to me.

Christian.


Reported-by: kernel test robot 
Reviewed-by: Hawking Zhang 
---
  drivers/gpu/drm/amd/amdgpu/umc_v8_7.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
index 291b37f6db4e..05f79eea307c 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
@@ -94,16 +94,12 @@ static void umc_v8_7_ecc_info_query_ras_error_count(struct 
amdgpu_device *adev,
  
  	uint32_t umc_inst= 0;

uint32_t ch_inst = 0;
-   uint32_t umc_reg_offset  = 0;
uint32_t channel_index   = 0;
  
  	/* TODO: driver needs to toggle DF Cstate to ensure

 * safe access of UMC registers. Will add the protection
 */
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
-   umc_reg_offset = get_umc_v8_7_reg_offset(adev,
-   umc_inst,
-   ch_inst);
channel_index = get_umc_v8_7_channel_index(adev,
umc_inst,
ch_inst);

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Christian König]

Am 18.01.22 um 21:13 schrieb Eric Huang:
I understand Alex's concern. I think usually we only check the version 
when a feature is only available in a specific version, and other 
version or newer version doesn't have.


In case of FW fix, we assume the driver and FWs have to be 
synchronous. If we have driver backward compatibility for FWs, there 
must be a lot of redundant codes for FW version check. So this patch 
and SDMA fix will be pushed into ROCm 5.1 release branch at the same 
time.


Others replied as well, but just to make it clear this approach is an 
absolutely clear NAK.


Driver backward compatibility is a documented mandatory feature.

So if you already pushed this to ROCm 5.1 then please revert that ASAP.

Regards,
Christian.



Regards,
Eric

[PATCH 3/3] drm: Convert open yes/no strings to yesno()

[Lucas De Marchi]

linux/string_helpers.h provides a helper to return "yes"/"no"
strings. Replace the open coded versions with yesno(). The places were
identified with the following semantic patch:

@@
expression b;
@@

- b ? "yes" : "no"
+ yesno(b)

Then the includes were added, so we include-what-we-use, and parenthesis
adjusted in drivers/gpu/drm/v3d/v3d_debugfs.c. After the conversion we
still see the same binary sizes:

   textdata bss dec hex filename
1442171   60344 800 1503315  16f053 ./drivers/gpu/drm/radeon/radeon.ko
1442171   60344 800 1503315  16f053 ./drivers/gpu/drm/radeon/radeon.ko.old
5985991  324439   33808 6344238  60ce2e ./drivers/gpu/drm/amd/amdgpu/amdgpu.ko
5985991  324439   33808 6344238  60ce2e 
./drivers/gpu/drm/amd/amdgpu/amdgpu.ko.old
 411986   104906176  428652   68a6c ./drivers/gpu/drm/drm.ko
 411986   104906176  428652   68a6c ./drivers/gpu/drm/drm.ko.old
1970292  1095152352 2082159  1fc56f ./drivers/gpu/drm/nouveau/nouveau.ko
1970292  1095152352 2082159  1fc56f ./drivers/gpu/drm/nouveau/nouveau.ko.old

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/amd/amdgpu/atom.c |  3 ++-
 drivers/gpu/drm/drm_client_modeset.c  |  3 ++-
 drivers/gpu/drm/drm_dp_helper.c   |  3 ++-
 drivers/gpu/drm/drm_gem.c |  3 ++-
 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c |  4 +++-
 drivers/gpu/drm/radeon/atom.c |  3 ++-
 drivers/gpu/drm/v3d/v3d_debugfs.c | 11 ++-
 drivers/gpu/drm/virtio/virtgpu_debugfs.c  |  3 ++-
 8 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c 
b/drivers/gpu/drm/amd/amdgpu/atom.c
index 6fa2229b7229..3d7d0f4cfc05 100644
--- a/drivers/gpu/drm/amd/amdgpu/atom.c
+++ b/drivers/gpu/drm/amd/amdgpu/atom.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -740,7 +741,7 @@ static void atom_op_jump(atom_exec_context *ctx, int *ptr, 
int arg)
break;
}
if (arg != ATOM_COND_ALWAYS)
-   SDEBUG("   taken: %s\n", execute ? "yes" : "no");
+   SDEBUG("   taken: %s\n", yesno(execute));
SDEBUG("   target: 0x%04X\n", target);
if (execute) {
if (ctx->last_jump == (ctx->start + target)) {
diff --git a/drivers/gpu/drm/drm_client_modeset.c 
b/drivers/gpu/drm/drm_client_modeset.c
index ced09c7c06f9..3c55156a75fa 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -241,7 +242,7 @@ static void drm_client_connectors_enabled(struct 
drm_connector **connectors,
connector = connectors[i];
enabled[i] = drm_connector_enabled(connector, true);
DRM_DEBUG_KMS("connector %d enabled? %s\n", connector->base.id,
- connector->display_info.non_desktop ? "non 
desktop" : enabled[i] ? "yes" : "no");
+ connector->display_info.non_desktop ? "non 
desktop" : yesno(enabled[i]));
 
any_enabled |= enabled[i];
}
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 4d0d1e8e51fa..f600616839f3 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1122,7 +1123,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
bool branch_device = drm_dp_is_branch(dpcd);
 
seq_printf(m, "\tDP branch device present: %s\n",
-  branch_device ? "yes" : "no");
+  yesno(branch_device));
 
if (!branch_device)
return;
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4dcdec6487bb..6436876341bb 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1145,7 +1146,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned 
int indent,
  drm_vma_node_start(&obj->vma_node));
drm_printf_indent(p, indent, "size=%zu\n", obj->size);
drm_printf_indent(p, indent, "imported=%s\n",
- obj->import_attach ? "yes" : "no");
+ yesno(obj->import_attach));
 
if (obj->funcs->print_info)
obj->funcs->print_info(p, indent, obj);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c
index a11637b0f6cc..d39a9c1a2a6e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c
@@ -21,6 +21,8 @@
  *
  * Authors: Ben Skeggs
  */
+#include 
+
 #include "aux.h"
 #include "pad.h"
 
@@ -94,7 +96,7 @@ void
 nvkm_i2c_aux_monitor(struct nvkm_i2c_aux *aux,

[PATCH 2/3] lib/string_helpers: Add helpers for enable[d]/disable[d]

[Lucas De Marchi]

Follow the yes/no logic and add helpers for enabled/disabled and
enable/disable - those are not so common throughout the kernel,
but they give a nice way to reuse the strings to log things as
enabled/disabled or enable/disable.

Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_utils.h | 10 --
 include/linux/string_helpers.h|  2 ++
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_utils.h 
b/drivers/gpu/drm/i915/i915_utils.h
index 2a8781cc648b..cbec79bae0d2 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -419,16 +419,6 @@ static inline const char *onoff(bool v)
return v ? "on" : "off";
 }
 
-static inline const char *enabledisable(bool v)
-{
-   return v ? "enable" : "disable";
-}
-
-static inline const char *enableddisabled(bool v)
-{
-   return v ? "enabled" : "disabled";
-}
-
 void add_taint_for_CI(struct drm_i915_private *i915, unsigned int taint);
 static inline void __add_taint_for_CI(unsigned int taint)
 {
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index e980dec05d31..e4b82f364ee1 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -103,5 +103,7 @@ char *kstrdup_quotable_file(struct file *file, gfp_t gfp);
 void kfree_strarray(char **array, size_t n);
 
 static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
+static inline const char *enabledisable(bool v) { return v ? "enable" : 
"disable"; }
+static inline const char *enableddisabled(bool v) { return v ? "enabled" : 
"disabled"; }
 
 #endif
-- 
2.34.1

[PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation

[Lucas De Marchi]

There are a few implementations of yesno() in the tree. Consolidate them
in include/linux/string_helpers.h.  Quite a few users of open coded
yesno() could later be converted to the new function:

$ git grep '?\s*"yes"\s*' | wc -l
286
$ git grep '?\s*"no"\s*' | wc -l
20

The inlined function should keep the const strings local to each
compilation unit, the same way it's now, thus not changing the current
behavior.

Signed-off-by: Lucas De Marchi 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  |  6 +-
 drivers/gpu/drm/i915/i915_utils.h  |  5 -
 .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ---
 include/linux/string_helpers.h |  2 ++
 security/tomoyo/audit.c|  2 +-
 security/tomoyo/common.c   | 18 --
 security/tomoyo/common.h   |  1 -
 7 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 9d43ecb1f692..b59760f91bf6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -23,6 +23,7 @@
  *
  */
 
+#include 
 #include 
 
 #include "dc.h"
@@ -49,11 +50,6 @@ struct dmub_debugfs_trace_entry {
uint32_t param1;
 };
 
-static inline const char *yesno(bool v)
-{
-   return v ? "yes" : "no";
-}
-
 /* parse_write_buffer_into_params - Helper function to parse debugfs write 
buffer into an array
  *
  * Function takes in attributes passed to debugfs write entry
diff --git a/drivers/gpu/drm/i915/i915_utils.h 
b/drivers/gpu/drm/i915/i915_utils.h
index 7a5925072466..2a8781cc648b 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -414,11 +414,6 @@ wait_remaining_ms_from_jiffies(unsigned long 
timestamp_jiffies, int to_wait_ms)
 #define MBps(x) KBps(1000 * (x))
 #define GBps(x) ((u64)1000 * MBps((x)))
 
-static inline const char *yesno(bool v)
-{
-   return v ? "yes" : "no";
-}
-
 static inline const char *onoff(bool v)
 {
return v ? "on" : "off";
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 7d49fd4edc9e..61a04d7abc1f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -2015,17 +2015,6 @@ static const struct file_operations rss_debugfs_fops = {
 /* RSS Configuration.
  */
 
-/* Small utility function to return the strings "yes" or "no" if the supplied
- * argument is non-zero.
- */
-static const char *yesno(int x)
-{
-   static const char *yes = "yes";
-   static const char *no = "no";
-
-   return x ? yes : no;
-}
-
 static int rss_config_show(struct seq_file *seq, void *v)
 {
struct adapter *adapter = seq->private;
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 4ba39e1403b2..e980dec05d31 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -102,4 +102,6 @@ char *kstrdup_quotable_file(struct file *file, gfp_t gfp);
 
 void kfree_strarray(char **array, size_t n);
 
+static inline const char *yesno(bool v) { return v ? "yes" : "no"; }
+
 #endif
diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c
index d79bf07e16be..1458e27361e8 100644
--- a/security/tomoyo/audit.c
+++ b/security/tomoyo/audit.c
@@ -166,7 +166,7 @@ static char *tomoyo_print_header(struct tomoyo_request_info 
*r)
   "#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s 
granted=%s (global-pid=%u) task={ pid=%u ppid=%u uid=%u gid=%u euid=%u egid=%u 
suid=%u sgid=%u fsuid=%u fsgid=%u }",
   stamp.year, stamp.month, stamp.day, stamp.hour,
   stamp.min, stamp.sec, r->profile, tomoyo_mode[r->mode],
-  tomoyo_yesno(r->granted), gpid, tomoyo_sys_getpid(),
+  yesno(r->granted), gpid, tomoyo_sys_getpid(),
   tomoyo_sys_getppid(),
   from_kuid(&init_user_ns, current_uid()),
   from_kgid(&init_user_ns, current_gid()),
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 5c64927bf2b3..304ed0f426dd 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "common.h"
 
 /* String table for operation mode. */
@@ -174,16 +175,6 @@ static bool tomoyo_manage_by_non_root;
 
 /* Utility functions. */
 
-/**
- * tomoyo_yesno - Return "yes" or "no".
- *
- * @value: Bool value.
- */
-const char *tomoyo_yesno(const unsigned int value)
-{
-   return value ? "yes" : "no";
-}
-
 /**
  * tomoyo_addprintf - strncat()-like-snprintf().
  *
@@ -730,8 +721,8 @@ static void tomoyo_print_config(struct tomoyo_io_buffer 
*head, const u8 config)
 {
tomoyo_io_printf(head, "={ mode=%s gra

[PATCH 0/3] lib/string_helpers: Add a few string helpers

[Lucas De Marchi]

Add some helpers under lib/string_helpers.h so they can be used
throughout the kernel. When I started doing this there were 2 other
previous attempts I know of, not counting the iterations each of them
had:

1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nik...@intel.com/
2) 
https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevche...@linux.intel.com/#t

Going through the comments I tried to find some common ground and
justification for what is in here, addressing some of the concerns
raised.

a. This version should be a drop-in replacement for what is currently in
   the tree, with no change in behavior or binary size. For binary
   size what I checked wat that the linked objects in the end have the
   same size (gcc 11). From comments in the previous attempts this seems
   also the case for earlier compiler versions

b. I didn't change the function name to choice_* as suggested by Andrew
   Morton in 20191023155619.43e0013f0c8c673a5c508...@linux-foundation.org
   because other people argumented in favor of shorter names for these
   simple helpers - if they are long and people simply not use due to
   that, we failed

c. Use string_helper.h for these helpers - pulling string.h in the
   compilations units was one of the concerns and I think re-using this
   already existing header is better than creating a new string-choice.h

d. This doesn't bring onoff() helper as there are some places in the
   kernel with onoff as variable - another name is probably needed for
   this function in order not to shadow the variable, or those variables
   could be renamed.  Or if people wanting  
   try to find a short one

e. One alternative to all of this suggested by Christian König
   (43456ba7-c372-84cc-4949-dcb817188...@amd.com) would be to add a
   printk format. But besides the comment, he also seemed to like
   the common function. This brought the argument from others that the
   simple yesno()/enabledisable() already used in the code is easier to
   remember and use than e.g. %py[DOY]

Last patch also has some additional conversion of open coded cases. I
preferred starting with drm/ since this is "closer to home".

I hope this is a good summary of the previous attempts and a way we can
move forward.

Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my
proposal is to take first 2 patches either through mm tree or maybe
vsprintf. Last patch can be taken later through drm.

thanks
Lucas De Marchi

Cc: Alex Deucher 
Cc: Andrew Morton 
Cc: Andy Shevchenko 
Cc: Andy Shevchenko 
Cc: Ben Skeggs 
Cc: Christian König 
Cc: Chris Wilson 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: David S. Miller 
Cc: Emma Anholt 
Cc: Eryk Brol 
Cc: Francis Laniel 
Cc: Greg Kroah-Hartman 
Cc: Harry Wentland 
Cc: Jakub Kicinski 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Julia Lawall 
Cc: Kentaro Takeda 
Cc: Leo Li 
Cc: Mikita Lipski 
Cc: Petr Mladek 
Cc: Rahul Lakkireddy 
Cc: Raju Rangoju 
Cc: Rasmus Villemoes 
Cc: Rodrigo Vivi 
Cc: Sakari Ailus 
Cc: Sergey Senozhatsky 
Cc: Steven Rostedt 
Cc: Vishal Kulkarni 

Lucas De Marchi (3):
  lib/string_helpers: Consolidate yesno() implementation
  lib/string_helpers: Add helpers for enable[d]/disable[d]
  drm: Convert open yes/no strings to yesno()

 drivers/gpu/drm/amd/amdgpu/atom.c  |  3 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c  |  6 +-
 drivers/gpu/drm/drm_client_modeset.c   |  3 ++-
 drivers/gpu/drm/drm_dp_helper.c|  3 ++-
 drivers/gpu/drm/drm_gem.c  |  3 ++-
 drivers/gpu/drm/i915/i915_utils.h  | 15 ---
 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/aux.c  |  4 +++-
 drivers/gpu/drm/radeon/atom.c  |  3 ++-
 drivers/gpu/drm/v3d/v3d_debugfs.c  | 11 ++-
 drivers/gpu/drm/virtio/virtgpu_debugfs.c   |  3 ++-
 .../net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 11 ---
 include/linux/string_helpers.h |  4 
 security/tomoyo/audit.c|  2 +-
 security/tomoyo/common.c   | 18 --
 security/tomoyo/common.h   |  1 -
 15 files changed, 31 insertions(+), 59 deletions(-)

-- 
2.34.1

Re: [PATCH v9 1/6] drm: move the buddy allocator from i915 into common drm

[Christian König]

Am 18.01.22 um 11:44 schrieb Arunpravin:

Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
   will be moved to drm selftest folder

cleanup i915 buddy references in i915 driver module
and replace with drm buddy

v2:
   - include header file in alphabetical order(Thomas)
   - merged changes listed in the body section into a single patch
 to keep the build intact(Christian, Jani)

v3:
   - make drm buddy a separate module(Thomas, Christian)

v4:
   - Fix build error reported by kernel test robot 
   - removed i915 buddy selftest from i915_mock_selftests.h to
 avoid build error
   - removed selftests/i915_buddy.c file as we create a new set of
 buddy test cases in drm/selftests folder

v5:
   - Fix merge conflict issue

v6:
   - replace drm_buddy_mm structure name as drm_buddy(Thomas, Christian)
   - replace drm_buddy_alloc() function name as drm_buddy_alloc_blocks()
 (Thomas)
   - replace drm_buddy_free() function name as drm_buddy_free_block()
 (Thomas)
   - export drm_buddy_free_block() function
   - fix multiple instances of KMEM_CACHE() entry

v7:
   - fix warnings reported by kernel test robot 
   - modify the license(Christian)

v8:
   - fix warnings reported by kernel test robot 

Signed-off-by: Arunpravin 


I've just gone ahead and pushed this version here to drm-misc-next.

That should at least reduce the amount of mails send back and forth.

Let me know when there are more rbs on the rest and I will push that as 
well.


Thanks,
Christian.


---
  drivers/gpu/drm/Kconfig   |   6 +
  drivers/gpu/drm/Makefile  |   2 +
  drivers/gpu/drm/drm_buddy.c   | 535 
  drivers/gpu/drm/i915/Kconfig  |   1 +
  drivers/gpu/drm/i915/Makefile |   1 -
  drivers/gpu/drm/i915/i915_buddy.c | 466 ---
  drivers/gpu/drm/i915/i915_buddy.h | 143 
  drivers/gpu/drm/i915/i915_module.c|   3 -
  drivers/gpu/drm/i915/i915_scatterlist.c   |  11 +-
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  33 +-
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |   4 +-
  drivers/gpu/drm/i915/selftests/i915_buddy.c   | 787 --
  .../drm/i915/selftests/i915_mock_selftests.h  |   1 -
  .../drm/i915/selftests/intel_memory_region.c  |  13 +-
  include/drm/drm_buddy.h   | 150 
  15 files changed, 725 insertions(+), 1431 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_buddy.c
  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c
  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h
  delete mode 100644 drivers/gpu/drm/i915/selftests/i915_buddy.c
  create mode 100644 include/drm/drm_buddy.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 91f54aeb0b7c..cc3e979c9c9d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -204,6 +204,12 @@ config DRM_TTM
  GPU memory types. Will be enabled automatically if a device driver
  uses it.
  
+config DRM_BUDDY

+   tristate
+   depends on DRM
+   help
+ A page based buddy allocator
+
  config DRM_VRAM_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 700abeb4945e..8675c2af7ae1 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -40,6 +40,8 @@ obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
  drm_shmem_helper-y := drm_gem_shmem_helper.o
  obj-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_shmem_helper.o
  
+obj-$(CONFIG_DRM_BUDDY) += drm_buddy.o

+
  drm_vram_helper-y := drm_gem_vram_helper.o
  obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
  
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c

new file mode 100644
index ..d60878bc9c20
--- /dev/null
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -0,0 +1,535 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+static struct kmem_cache *slab_blocks;
+
+static struct drm_buddy_block *drm_block_alloc(struct drm_buddy *mm,
+  struct drm_buddy_block *parent,
+  unsigned int order,
+  u64 offset)
+{
+   struct drm_buddy_block *block;
+
+   BUG_ON(order > DRM_BUDDY_MAX_ORDER);
+
+   block = kmem_cache_zalloc(slab_blocks, GFP_KERNEL);
+   if (!block)
+   return NULL;
+
+   block->header = offset;
+   block->header |= order;
+   b

RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init

[Chai, Thomas]

-Original Message-
From: Zhou1, Tao  
Sent: Wednesday, January 19, 2022 2:11 PM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Clements, John 

Subject: RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from 
.late_init to .early_init

[AMD Official Use Only]



> -Original Message-
> From: Chai, Thomas 
> Sent: Wednesday, January 19, 2022 10:56 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Chai, Thomas ; Zhang, Hawking 
> ; Zhou1, Tao ; Clements, 
> John ; Chai, Thomas 
> Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from 
> .late_init to .early_init
> 
> Move xgmi ras initialization from .late_init to .early_init, which let 
> xgmi ras be initialized only once.
> 
> Signed-off-by: yipechai 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 - 
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 1 -
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 9 +
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 3483a82f5734..d83eee1984c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device
> *adev)
>   return r;
>   }
> 
> - if (!adev->gmc.xgmi.connected_to_cpu) {
> - adev->gmc.xgmi.ras = &xgmi_ras;
> - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras-
> >ras_block);
> - }
> -
>   if (adev->gmc.xgmi.ras && 
> adev->gmc.xgmi.ras->ras_block.ras_late_init)
> {
>   r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL);
>   if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 4f8d356f8432..5f9f82091000 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct 
> amdgpu_device *adev)
>   }
>  }
> 
> -
>[Tao]: Please don't introduce irrelevant change.

[Thomas] OK

>  static int gmc_v10_0_early_init(void *handle)  {
>   struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff 
> --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index c76ffd1a70cd..8d1b11368a7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct 
> amdgpu_device *adev)
>   amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block);  }
> 
> +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) {
> + if (!adev->gmc.xgmi.connected_to_cpu) {
> + adev->gmc.xgmi.ras = &xgmi_ras;
> + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras-
> >ras_block);
> + }
> +}
>[Tao]: Since the initialization of xgmi.ras is common for all versions of IP, 
>I recommend to create a generic ras_early_init func for it.
>BTW, I think we can remove the check for block_obj's existence in 
>register_ras_block now.
 
[Thomas] I will update the patch.

> +
>  static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev)  {
>   /* is UMC the right IP to check for MCA?  Maybe DF? */ @@ -1339,6
> +1347,7 @@ static int gmc_v9_0_early_init(void *handle)
>   gmc_v9_0_set_gfxhub_funcs(adev);
>   gmc_v9_0_set_hdp_ras_funcs(adev);
>   gmc_v9_0_set_mca_funcs(adev);
> + gmc_v9_0_set_xgmi_ras_funcs(adev);
> 
>   adev->gmc.shared_aperture_start = 0x2000ULL;
>   adev->gmc.shared_aperture_end =
> --
> 2.25.1

[PATCH] drm/amdgpu: remove unused variable

[mziya]

Remove set but unused variable.
warning: variable 'umc_reg_offset' set but not used

Signed-off-by: mziya 
Reported-by: kernel test robot 
Reviewed-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/umc_v8_7.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
index 291b37f6db4e..05f79eea307c 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
@@ -94,16 +94,12 @@ static void umc_v8_7_ecc_info_query_ras_error_count(struct 
amdgpu_device *adev,
 
uint32_t umc_inst= 0;
uint32_t ch_inst = 0;
-   uint32_t umc_reg_offset  = 0;
uint32_t channel_index   = 0;
 
/* TODO: driver needs to toggle DF Cstate to ensure
 * safe access of UMC registers. Will add the protection
 */
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
-   umc_reg_offset = get_umc_v8_7_reg_offset(adev,
-   umc_inst,
-   ch_inst);
channel_index = get_umc_v8_7_channel_index(adev,
umc_inst,
ch_inst);
-- 
2.25.1

RE: [PATCH] drm/amdgpu: remove unused variable

[Zhang, Hawking]

[AMD Official Use Only]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Ziya, Mohammad zafar  
Sent: Wednesday, January 19, 2022 14:23
To: Zhang, Hawking ; Lazar, Lijo ; 
Clements, John 
Cc: amd-gfx@lists.freedesktop.org; Ziya, Mohammad zafar 
; kernel test robot 
Subject: [PATCH] drm/amdgpu: remove unused variable

Remove set but unused variable.
warning: variable 'umc_reg_offset' set but not used

Signed-off-by: mziya 
Reported-by: kernel test robot 
---
 drivers/gpu/drm/amd/amdgpu/umc_v8_7.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
index 291b37f6db4e..05f79eea307c 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
@@ -94,16 +94,12 @@ static void umc_v8_7_ecc_info_query_ras_error_count(struct 
amdgpu_device *adev,
 
uint32_t umc_inst= 0;
uint32_t ch_inst = 0;
-   uint32_t umc_reg_offset  = 0;
uint32_t channel_index   = 0;
 
/* TODO: driver needs to toggle DF Cstate to ensure
 * safe access of UMC registers. Will add the protection
 */
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
-   umc_reg_offset = get_umc_v8_7_reg_offset(adev,
-   umc_inst,
-   ch_inst);
channel_index = get_umc_v8_7_channel_index(adev,
umc_inst,
ch_inst);
-- 
2.25.1

RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init

[Zhang, Hawking]

[AMD Official Use Only]

Patch is
Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Chai, Thomas  
Sent: Wednesday, January 19, 2022 10:56
To: amd-gfx@lists.freedesktop.org
Cc: Chai, Thomas ; Zhang, Hawking ; 
Zhou1, Tao ; Clements, John ; Chai, 
Thomas 
Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init 
to .early_init

Move xgmi ras initialization from .late_init to .early_init, which let xgmi ras 
be initialized only once.

Signed-off-by: yipechai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 -  
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 9 +
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 3483a82f5734..d83eee1984c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev)
return r;
}
 
-   if (!adev->gmc.xgmi.connected_to_cpu) {
-   adev->gmc.xgmi.ras = &xgmi_ras;
-   amdgpu_ras_register_ras_block(adev, 
&adev->gmc.xgmi.ras->ras_block);
-   }
-
if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init) {
r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4f8d356f8432..5f9f82091000 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct amdgpu_device 
*adev)
}
 }
 
-
 static int gmc_v10_0_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff --git 
a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index c76ffd1a70cd..8d1b11368a7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct 
amdgpu_device *adev)
amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block);  }
 
+static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) {
+   if (!adev->gmc.xgmi.connected_to_cpu) {
+   adev->gmc.xgmi.ras = &xgmi_ras;
+   amdgpu_ras_register_ras_block(adev, 
&adev->gmc.xgmi.ras->ras_block);
+   }
+}
+
 static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev)  {
/* is UMC the right IP to check for MCA?  Maybe DF? */ @@ -1339,6 
+1347,7 @@ static int gmc_v9_0_early_init(void *handle)
gmc_v9_0_set_gfxhub_funcs(adev);
gmc_v9_0_set_hdp_ras_funcs(adev);
gmc_v9_0_set_mca_funcs(adev);
+   gmc_v9_0_set_xgmi_ras_funcs(adev);
 
adev->gmc.shared_aperture_start = 0x2000ULL;
adev->gmc.shared_aperture_end =
--
2.25.1

[PATCH] drm/amdgpu: remove unused variable

[mziya]

Remove set but unused variable.
warning: variable 'umc_reg_offset' set but not used

Signed-off-by: mziya 
Reported-by: kernel test robot 
---
 drivers/gpu/drm/amd/amdgpu/umc_v8_7.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c 
b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
index 291b37f6db4e..05f79eea307c 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v8_7.c
@@ -94,16 +94,12 @@ static void umc_v8_7_ecc_info_query_ras_error_count(struct 
amdgpu_device *adev,
 
uint32_t umc_inst= 0;
uint32_t ch_inst = 0;
-   uint32_t umc_reg_offset  = 0;
uint32_t channel_index   = 0;
 
/* TODO: driver needs to toggle DF Cstate to ensure
 * safe access of UMC registers. Will add the protection
 */
LOOP_UMC_INST_AND_CH(umc_inst, ch_inst) {
-   umc_reg_offset = get_umc_v8_7_reg_offset(adev,
-   umc_inst,
-   ch_inst);
channel_index = get_umc_v8_7_channel_index(adev,
umc_inst,
ch_inst);
-- 
2.25.1

RE: [PATCH 1/3] drm/amdgpu: Remove repeated calls

[Zhou1, Tao]

[AMD Official Use Only]

Reviewed-by: Tao Zhou 

> -Original Message-
> From: Chai, Thomas 
> Sent: Wednesday, January 19, 2022 10:56 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Chai, Thomas ; Zhang, Hawking
> ; Zhou1, Tao ; Clements,
> John ; Chai, Thomas 
> Subject: [PATCH 1/3] drm/amdgpu: Remove repeated calls
> 
> Remove repeated calls.
> 
> Signed-off-by: yipechai 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 7a1d2bac698e..4992bc554c0c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1704,9 +1704,7 @@ static void amdgpu_ras_log_on_err_counter(struct
> amdgpu_device *adev)  static void amdgpu_ras_error_status_query(struct
> amdgpu_device *adev,
> struct ras_query_if *info)
>  {
> - struct amdgpu_ras_block_object *block_obj =
> amdgpu_ras_get_ras_block(adev,
> - info-
> >head.block,
> - info-
> >head.sub_block_index);
> + struct amdgpu_ras_block_object *block_obj;
>   /*
>* Only two block need to query read/write
>* RspStatus at current state
> --
> 2.25.1

RE: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init

[Zhou1, Tao]

[AMD Official Use Only]



> -Original Message-
> From: Chai, Thomas 
> Sent: Wednesday, January 19, 2022 10:56 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Chai, Thomas ; Zhang, Hawking
> ; Zhou1, Tao ; Clements,
> John ; Chai, Thomas 
> Subject: [PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init
> to .early_init
> 
> Move xgmi ras initialization from .late_init to .early_init, which let xgmi 
> ras be
> initialized only once.
> 
> Signed-off-by: yipechai 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 -
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 1 -
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 9 +
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 3483a82f5734..d83eee1984c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device
> *adev)
>   return r;
>   }
> 
> - if (!adev->gmc.xgmi.connected_to_cpu) {
> - adev->gmc.xgmi.ras = &xgmi_ras;
> - amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras-
> >ras_block);
> - }
> -
>   if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init)
> {
>   r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL);
>   if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 4f8d356f8432..5f9f82091000 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct
> amdgpu_device *adev)
>   }
>  }
> 
> -
[Tao]: Please don't introduce irrelevant change.

>  static int gmc_v10_0_early_init(void *handle)  {
>   struct amdgpu_device *adev = (struct amdgpu_device *)handle; diff --git
> a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index c76ffd1a70cd..8d1b11368a7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct
> amdgpu_device *adev)
>   amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block);  }
> 
> +static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev) {
> + if (!adev->gmc.xgmi.connected_to_cpu) {
> + adev->gmc.xgmi.ras = &xgmi_ras;
> + amdgpu_ras_register_ras_block(adev, &adev->gmc.xgmi.ras-
> >ras_block);
> + }
> +}
[Tao]: Since the initialization of xgmi.ras is common for all versions of IP, I 
recommend to create a generic ras_early_init func for it.
BTW, I think we can remove the check for block_obj's existence in 
register_ras_block now.

> +
>  static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev)  {
>   /* is UMC the right IP to check for MCA?  Maybe DF? */ @@ -1339,6
> +1347,7 @@ static int gmc_v9_0_early_init(void *handle)
>   gmc_v9_0_set_gfxhub_funcs(adev);
>   gmc_v9_0_set_hdp_ras_funcs(adev);
>   gmc_v9_0_set_mca_funcs(adev);
> + gmc_v9_0_set_xgmi_ras_funcs(adev);
> 
>   adev->gmc.shared_aperture_start = 0x2000ULL;
>   adev->gmc.shared_aperture_end =
> --
> 2.25.1

RE: [PATCH 1/3] drm/amdgpu: Remove repeated calls

[Zhang, Hawking]

[AMD Official Use Only]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Chai, Thomas  
Sent: Wednesday, January 19, 2022 10:56
To: amd-gfx@lists.freedesktop.org
Cc: Chai, Thomas ; Zhang, Hawking ; 
Zhou1, Tao ; Clements, John ; Chai, 
Thomas 
Subject: [PATCH 1/3] drm/amdgpu: Remove repeated calls

Remove repeated calls.

Signed-off-by: yipechai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 7a1d2bac698e..4992bc554c0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1704,9 +1704,7 @@ static void amdgpu_ras_log_on_err_counter(struct 
amdgpu_device *adev)  static void amdgpu_ras_error_status_query(struct 
amdgpu_device *adev,
  struct ras_query_if *info)
 {
-   struct amdgpu_ras_block_object *block_obj = 
amdgpu_ras_get_ras_block(adev,
-   
info->head.block,
-   
info->head.sub_block_index);
+   struct amdgpu_ras_block_object *block_obj;
/*
 * Only two block need to query read/write
 * RspStatus at current state
--
2.25.1

[PATCH v4] drm/amd: Warn users about potential s0ix problems

[Mario Limonciello]

On some OEM setups users can configure the BIOS for S3 or S2idle.
When configured to S3 users can still choose 's2idle' in the kernel by
using `/sys/power/mem_sleep`.  Before commit 6dc8265f9803 ("drm/amdgpu:
always reset the asic in suspend (v2)"), the GPU would crash.  Now when
configured this way, the system should resume but will use more power.

As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
potential power consumption issues during their first attempt at
suspending.

Reported-by: Bjoren Dasse 
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
Signed-off-by: Mario Limonciello 
---
v3->v4:
 * Add back in CONFIG_SUSPEND check
v2->v3:
 * Better direct users how to recover in the bad cases
v1->v2:
 * Only show messages in s2idle cases

 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 4811b0faafd9..2531da6cbec3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1040,11 +1040,20 @@ void amdgpu_acpi_detect(void)
  */
 bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
 {
-#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
-   if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
-   if (adev->flags & AMD_IS_APU)
-   return pm_suspend_target_state == PM_SUSPEND_TO_IDLE;
-   }
-#endif
+#if IS_ENABLED(CONFIG_SUSPEND)
+   if (!(adev->flags & AMD_IS_APU) ||
+   pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
+   return false;
+#else
return false;
+#endif
+   if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+   dev_warn_once(adev->dev,
+ "Power consumption will be higher as BIOS has not 
been configured for suspend-to-idle.\n"
+ "To use suspend-to-idle change the sleep mode in 
BIOS setup.\n");
+#if !IS_ENABLED(CONFIG_AMD_PMC)
+   dev_warn_once(adev->dev,
+ "Power consumption will be higher as the kernel has not 
been compiled with CONFIG_AMD_PMC.\n");
+#endif
+   return true;
 }
-- 
2.25.1

[PATCH] drm/amdgpu: force using sdma to update vm page table when mmio is blocked

[Yang Wang]

when mmio protection feature is enabled in hypervisor,
it will cause guest OS can't R/W HDP regiters,
and using cpu to update page table is not working well.

force using sdma to update page table when mmio is blocked.

Signed-off-by: Yang Wang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b23cb463b106..0f86f0b2e183 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2959,6 +2959,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
AMDGPU_VM_USE_CPU_FOR_GFX);
 
+   if (vm->use_cpu_for_update && amdgpu_sriov_vf(adev) && 
amdgpu_virt_mmio_blocked(adev))
+   vm->use_cpu_for_update = false;
+
DRM_DEBUG_DRIVER("VM update mode is %s\n",
 vm->use_cpu_for_update ? "CPU" : "SDMA");
WARN_ONCE((vm->use_cpu_for_update &&
@@ -3094,6 +3097,10 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, 
struct amdgpu_vm *vm)
/* Update VM state */
vm->use_cpu_for_update = !!(adev->vm_manager.vm_update_mode &
AMDGPU_VM_USE_CPU_FOR_COMPUTE);
+
+   if (vm->use_cpu_for_update && amdgpu_sriov_vf(adev) && 
amdgpu_virt_mmio_blocked(adev))
+   vm->use_cpu_for_update = false;
+
DRM_DEBUG_DRIVER("VM update mode is %s\n",
 vm->use_cpu_for_update ? "CPU" : "SDMA");
WARN_ONCE((vm->use_cpu_for_update &&
-- 
2.25.1

[PATCH] drm/amd: Fix MSB of SMU version printing

[Mario Limonciello]

Yellow carp has been outputting versions like `1093.24.0`, but this
is supposed to be 69.24.0. That is the MSB is being interpreted
incorrectly.

The MSB is not part of the major version, but has generally been
treated that way thus far.  It's actually the program, and used to
distinguish between two programs from a similar family but different
codebase.

Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 10 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 10 +-
 drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c | 10 +-
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 14 +++---
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f3c864223033..ae793c648f5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1431,8 +1431,7 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
struct drm_amdgpu_info_firmware fw_info;
struct drm_amdgpu_query_fw query_fw;
struct atom_context *ctx = adev->mode_info.atom_context;
-   uint8_t smu_minor, smu_debug;
-   uint16_t smu_major;
+   uint8_t smu_program, smu_major, smu_minor, smu_debug;
int ret, i;
 
static const char *ta_fw_name[TA_FW_TYPE_MAX_INDEX] = {
@@ -1578,11 +1577,12 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
ret = amdgpu_firmware_info(&fw_info, &query_fw, adev);
if (ret)
return ret;
-   smu_major = (fw_info.ver >> 16) & 0x;
+   smu_program = (fw_info.ver >> 24) & 0xff;
+   smu_major = (fw_info.ver >> 16) & 0xff;
smu_minor = (fw_info.ver >> 8) & 0xff;
smu_debug = (fw_info.ver >> 0) & 0xff;
-   seq_printf(m, "SMC feature version: %u, firmware version: 0x%08x 
(%d.%d.%d)\n",
-  fw_info.feature, fw_info.ver, smu_major, smu_minor, 
smu_debug);
+   seq_printf(m, "SMC feature version: %u, program: %d, firmware version: 
0x%08x (%d.%d.%d)\n",
+  fw_info.feature, smu_program, fw_info.ver, smu_major, 
smu_minor, smu_debug);
 
/* SDMA */
query_fw.fw_type = AMDGPU_INFO_FW_SDMA;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 7029e5deb6b3..e94a400db669 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -225,15 +225,15 @@ int smu_v11_0_check_fw_version(struct smu_context *smu)
 {
struct amdgpu_device *adev = smu->adev;
uint32_t if_version = 0xff, smu_version = 0xff;
-   uint16_t smu_major;
-   uint8_t smu_minor, smu_debug;
+   uint8_t smu_program, smu_major, smu_minor, smu_debug;
int ret = 0;
 
ret = smu_cmn_get_smc_version(smu, &if_version, &smu_version);
if (ret)
return ret;
 
-   smu_major = (smu_version >> 16) & 0x;
+   smu_program = (smu_version >> 24) & 0xff;
+   smu_major = (smu_version >> 16) & 0xff;
smu_minor = (smu_version >> 8) & 0xff;
smu_debug = (smu_version >> 0) & 0xff;
if (smu->is_apu)
@@ -287,9 +287,9 @@ int smu_v11_0_check_fw_version(struct smu_context *smu)
 */
if (if_version != smu->smc_driver_if_version) {
dev_info(smu->adev->dev, "smu driver if version = 0x%08x, smu 
fw if version = 0x%08x, "
-   "smu fw version = 0x%08x (%d.%d.%d)\n",
+   "smu fw program = %d, version = 0x%08x (%d.%d.%d)\n",
smu->smc_driver_if_version, if_version,
-   smu_version, smu_major, smu_minor, smu_debug);
+   smu_program, smu_version, smu_major, smu_minor, 
smu_debug);
dev_warn(smu->adev->dev, "SMU driver if version not matched\n");
}
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
index 9c91e79c955f..56a02bc60cee 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
@@ -74,15 +74,15 @@ int smu_v12_0_check_fw_version(struct smu_context *smu)
 {
struct amdgpu_device *adev = smu->adev;
uint32_t if_version = 0xff, smu_version = 0xff;
-   uint16_t smu_major;
-   uint8_t smu_minor, smu_debug;
+   uint8_t smu_program, smu_major, smu_minor, smu_debug;
int ret = 0;
 
ret = smu_cmn_get_smc_version(smu, &if_version, &smu_version);
if (ret)
return ret;
 
-   smu_major = (smu_version >> 16) & 0x;
+   smu_program = (smu_version >> 24) & 0xff;
+   smu_major = (smu_version >> 16) & 0xff;
smu_minor = (smu_version >> 8) & 0xff;
smu_debug = (smu_version >> 0) & 0xff;
if (smu->is_apu)
@@ -98,9 +98,9 @@ int smu_v12_0_chec

[PATCH 2/3] drm/amdgpu: Move xgmi ras initialization from .late_init to .early_init

[yipechai]

Move xgmi ras initialization from .late_init to .early_init, which let
xgmi ras be initialized only once.

Signed-off-by: yipechai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 5 -
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 9 +
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 3483a82f5734..d83eee1984c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -452,11 +452,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev)
return r;
}
 
-   if (!adev->gmc.xgmi.connected_to_cpu) {
-   adev->gmc.xgmi.ras = &xgmi_ras;
-   amdgpu_ras_register_ras_block(adev, 
&adev->gmc.xgmi.ras->ras_block);
-   }
-
if (adev->gmc.xgmi.ras && adev->gmc.xgmi.ras->ras_block.ras_late_init) {
r = adev->gmc.xgmi.ras->ras_block.ras_late_init(adev, NULL);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4f8d356f8432..5f9f82091000 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -716,7 +716,6 @@ static void gmc_v10_0_set_gfxhub_funcs(struct amdgpu_device 
*adev)
}
 }
 
-
 static int gmc_v10_0_early_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index c76ffd1a70cd..8d1b11368a7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1303,6 +1303,14 @@ static void gmc_v9_0_set_hdp_ras_funcs(struct 
amdgpu_device *adev)
amdgpu_ras_register_ras_block(adev, &adev->hdp.ras->ras_block);
 }
 
+static void gmc_v9_0_set_xgmi_ras_funcs(struct amdgpu_device *adev)
+{
+   if (!adev->gmc.xgmi.connected_to_cpu) {
+   adev->gmc.xgmi.ras = &xgmi_ras;
+   amdgpu_ras_register_ras_block(adev, 
&adev->gmc.xgmi.ras->ras_block);
+   }
+}
+
 static void gmc_v9_0_set_mca_funcs(struct amdgpu_device *adev)
 {
/* is UMC the right IP to check for MCA?  Maybe DF? */
@@ -1339,6 +1347,7 @@ static int gmc_v9_0_early_init(void *handle)
gmc_v9_0_set_gfxhub_funcs(adev);
gmc_v9_0_set_hdp_ras_funcs(adev);
gmc_v9_0_set_mca_funcs(adev);
+   gmc_v9_0_set_xgmi_ras_funcs(adev);
 
adev->gmc.shared_aperture_start = 0x2000ULL;
adev->gmc.shared_aperture_end =
-- 
2.25.1

[PATCH 3/3] drm/amdgpu: Remove redundant code in gmc v10

[yipechai]

Gmc v10 doesn't support ras function, remove redundant code in it.

Signed-off-by: yipechai 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 5f9f82091000..a833ef130495 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -664,25 +664,10 @@ static void gmc_v10_0_set_umc_funcs(struct amdgpu_device 
*adev)
adev->umc.umc_inst_num = UMC_V8_7_UMC_INSTANCE_NUM;
adev->umc.channel_offs = UMC_V8_7_PER_CHANNEL_OFFSET_SIENNA;
adev->umc.channel_idx_tbl = &umc_v8_7_channel_idx_tbl[0][0];
-   adev->umc.ras = &umc_v8_7_ras;
break;
default:
break;
}
-   if (adev->umc.ras) {
-   amdgpu_ras_register_ras_block(adev, &adev->umc.ras->ras_block);
-
-   strcpy(adev->umc.ras->ras_block.name, "umc");
-   adev->umc.ras->ras_block.block = AMDGPU_RAS_BLOCK__UMC;
-
-   /* If don't define special ras_late_init function, use default 
ras_late_init */
-   if (!adev->umc.ras->ras_block.ras_late_init)
-   adev->umc.ras->ras_block.ras_late_init = 
amdgpu_umc_ras_late_init;
-
-   /* If don't define special ras_fini function, use default 
ras_fini */
-   if (!adev->umc.ras->ras_block.ras_fini)
-   adev->umc.ras->ras_block.ras_fini = 
amdgpu_umc_ras_fini;
-   }
 }
 
 
@@ -745,10 +730,6 @@ static int gmc_v10_0_late_init(void *handle)
if (r)
return r;
 
-   r = amdgpu_gmc_ras_late_init(adev);
-   if (r)
-   return r;
-
return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 }
 
-- 
2.25.1

[PATCH 1/3] drm/amdgpu: Remove repeated calls

[yipechai]

Remove repeated calls.

Signed-off-by: yipechai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 7a1d2bac698e..4992bc554c0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1704,9 +1704,7 @@ static void amdgpu_ras_log_on_err_counter(struct 
amdgpu_device *adev)
 static void amdgpu_ras_error_status_query(struct amdgpu_device *adev,
  struct ras_query_if *info)
 {
-   struct amdgpu_ras_block_object *block_obj = 
amdgpu_ras_get_ras_block(adev,
-   
info->head.block,
-   
info->head.sub_block_index);
+   struct amdgpu_ras_block_object *block_obj;
/*
 * Only two block need to query read/write
 * RspStatus at current state
-- 
2.25.1

Re: [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout

[Esaki Tomohito]

On 2022/01/18 18:53, Andy Shevchenko wrote:

On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote:

On 2022/01/14 23:16, Andy Shevchenko wrote:

On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote:

The LINEAR modifier is advertised as default if a driver doesn't specify
modifiers.


...


+   const uint64_t default_modifiers[] = {
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID


+ Comma?


There is no mention in the coding style about adding/removing a comma to the
last element of an array. Is there a policy in drm driver?

I think the advantage of adding a comma to the last element of an array is
that diff is only one line when an element is added to the end.
However since INVALID is always the last element in the modifiers array, I
think it can be either in this case.
If there is a policy, I will match it.


Indeed, but there is a common sense. The idea behind (multi-line) definitions
that when next time somebody will add an element in the array, there are will
be:

a) no additional churn (like in case of this patch, if the item will be added
at the bottom;

b) an element that may not be added behind the terminator, which will look
weird.

That said, the question is if the element is terminator one or not, if not,
comma is better than no comma and vise versa.



Ah I see. In this case, DRM_FORMAT_MOD_INVALID is terminator, so it
should not have a comma.

Thanks
Tomohito Esaki

Re: [PATCH v3] drm/amd: Warn users about potential s0ix problems

[Lazar, Lijo]

[Public]

IS_ENABLED(CONFIG_SUSPEND) will be required for using  pm_suspend_target_state.

Thanks,
Lijo

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Felix Kuehling]

Am 2022-01-18 um 7:08 p.m. schrieb Russell, Kent:
> One question inline
>
>
> *KENT RUSSELL***  
>
> Sr. Software Engineer | Linux Compute Kernel
>
> 1 Commerce Valley Drive East
>
> Markham, ON L3T 7X6
>
> *O*+(1) 289-695-2122**| Ext 72122
>
>
> 
> *From:* amd-gfx  on behalf of
> Felix Kuehling 
> *Sent:* Tuesday, January 18, 2022 6:36 PM
> *To:* Huang, JinHuiEric ;
> amd-gfx@lists.freedesktop.org 
> *Subject:* Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on
> Arcturus
>  
> Am 2022-01-18 um 5:45 p.m. schrieb Eric Huang:
> > SDMA FW fixes the hang issue for adding heavy-weight TLB
> > flush on Arcturus, so we can enable it.
> >
> > Signed-off-by: Eric Huang 
>
> Reviewed-by: Felix Kuehling 
>
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  6 --
> >  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index a64cbbd943ba..acb4fd973e60 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1892,12 +1892,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> >    true);
> >    ret = unreserve_bo_and_vms(&ctx, false, false);
> > 
> > - /* Only apply no TLB flush on Aldebaran to
> > -  * workaround regressions on other Asics.
> > -  */
> > - if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> > - *table_freed = true;
> > -
> >    goto out;
> > 
> >  out_unreserve:
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index b570c0454ce9..485d4c52c7de 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1596,6 +1596,12 @@ static int
> kfd_ioctl_free_memory_of_gpu(struct file *filep,
> >    return ret;
> >  }
> > 
> > +static bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev) {
> > + return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)
>
> Do we need to add a check for sdma ver >=8 here?

What's the significance of version 8 for Aldebaran? This code was
working on Aldebaran without a version check before. Did we ever
publicly release an SDMA firmware older than version 8 that for Aldebaran?

Regards,
  Felix


>  
> ||
> > +    (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
> > + dev->adev->sdma.instance[0].fw_version >= 18);
> > +}
> > +
> >  static int kfd_ioctl_map_memory_to_gpu(struct file *filep,
> >    struct kfd_process *p, void
> *data)
> >  {
> > @@ -1692,7 +1698,7 @@ static int kfd_ioctl_map_memory_to_gpu(struct
> file *filep,
> >    }
> > 
> >    /* Flush TLBs after waiting for the page table updates to
> complete */
> > - if (table_freed) {
> > + if (table_freed || !kfd_flush_tlb_after_unmap(dev)) {
> >    for (i = 0; i < args->n_devices; i++) {
> >    peer = kfd_device_by_id(devices_arr[i]);
> >    if (WARN_ON_ONCE(!peer))
> > @@ -1806,7 +1812,7 @@ static int
> kfd_ioctl_unmap_memory_from_gpu(struct file *filep,
> >    }
> >    mutex_unlock(&p->mutex);
> > 
> > - if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> > + if (kfd_flush_tlb_after_unmap(dev)) {
> >    err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> >    (struct kgd_mem *) mem, true);
> >    if (err) {

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Russell, Kent]

One question inline


KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122


From: amd-gfx  on behalf of Felix 
Kuehling 
Sent: Tuesday, January 18, 2022 6:36 PM
To: Huang, JinHuiEric ; amd-gfx@lists.freedesktop.org 

Subject: Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

Am 2022-01-18 um 5:45 p.m. schrieb Eric Huang:
> SDMA FW fixes the hang issue for adding heavy-weight TLB
> flush on Arcturus, so we can enable it.
>
> Signed-off-by: Eric Huang 

Reviewed-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  6 --
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a64cbbd943ba..acb4fd973e60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1892,12 +1892,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>true);
>ret = unreserve_bo_and_vms(&ctx, false, false);
>
> - /* Only apply no TLB flush on Aldebaran to
> -  * workaround regressions on other Asics.
> -  */
> - if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> - *table_freed = true;
> -
>goto out;
>
>  out_unreserve:
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b570c0454ce9..485d4c52c7de 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1596,6 +1596,12 @@ static int kfd_ioctl_free_memory_of_gpu(struct file 
> *filep,
>return ret;
>  }
>
> +static bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev) {
> + return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)

Do we need to add a check for sdma ver >=8 here?

||
> +(KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
> + dev->adev->sdma.instance[0].fw_version >= 18);
> +}
> +
>  static int kfd_ioctl_map_memory_to_gpu(struct file *filep,
>struct kfd_process *p, void *data)
>  {
> @@ -1692,7 +1698,7 @@ static int kfd_ioctl_map_memory_to_gpu(struct file 
> *filep,
>}
>
>/* Flush TLBs after waiting for the page table updates to complete */
> - if (table_freed) {
> + if (table_freed || !kfd_flush_tlb_after_unmap(dev)) {
>for (i = 0; i < args->n_devices; i++) {
>peer = kfd_device_by_id(devices_arr[i]);
>if (WARN_ON_ONCE(!peer))
> @@ -1806,7 +1812,7 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
> *filep,
>}
>mutex_unlock(&p->mutex);
>
> - if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> + if (kfd_flush_tlb_after_unmap(dev)) {
>err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
>(struct kgd_mem *) mem, true);
>if (err) {

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Felix Kuehling]

Am 2022-01-18 um 5:45 p.m. schrieb Eric Huang:
> SDMA FW fixes the hang issue for adding heavy-weight TLB
> flush on Arcturus, so we can enable it.
>
> Signed-off-by: Eric Huang 

Reviewed-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  6 --
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a64cbbd943ba..acb4fd973e60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1892,12 +1892,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   true);
>   ret = unreserve_bo_and_vms(&ctx, false, false);
>  
> - /* Only apply no TLB flush on Aldebaran to
> -  * workaround regressions on other Asics.
> -  */
> - if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> - *table_freed = true;
> -
>   goto out;
>  
>  out_unreserve:
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b570c0454ce9..485d4c52c7de 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1596,6 +1596,12 @@ static int kfd_ioctl_free_memory_of_gpu(struct file 
> *filep,
>   return ret;
>  }
>  
> +static bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev) {
> + return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> +(KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
> + dev->adev->sdma.instance[0].fw_version >= 18);
> +}
> +
>  static int kfd_ioctl_map_memory_to_gpu(struct file *filep,
>   struct kfd_process *p, void *data)
>  {
> @@ -1692,7 +1698,7 @@ static int kfd_ioctl_map_memory_to_gpu(struct file 
> *filep,
>   }
>  
>   /* Flush TLBs after waiting for the page table updates to complete */
> - if (table_freed) {
> + if (table_freed || !kfd_flush_tlb_after_unmap(dev)) {
>   for (i = 0; i < args->n_devices; i++) {
>   peer = kfd_device_by_id(devices_arr[i]);
>   if (WARN_ON_ONCE(!peer))
> @@ -1806,7 +1812,7 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
> *filep,
>   }
>   mutex_unlock(&p->mutex);
>  
> - if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> + if (kfd_flush_tlb_after_unmap(dev)) {
>   err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
>   (struct kgd_mem *) mem, true);
>   if (err) {

[PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  6 --
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 10 --
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a64cbbd943ba..acb4fd973e60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,12 +1892,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
true);
ret = unreserve_bo_and_vms(&ctx, false, false);
 
-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
-*/
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
-   *table_freed = true;
-
goto out;
 
 out_unreserve:
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b570c0454ce9..485d4c52c7de 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1596,6 +1596,12 @@ static int kfd_ioctl_free_memory_of_gpu(struct file 
*filep,
return ret;
 }
 
+static bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev) {
+   return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+  (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
+   dev->adev->sdma.instance[0].fw_version >= 18);
+}
+
 static int kfd_ioctl_map_memory_to_gpu(struct file *filep,
struct kfd_process *p, void *data)
 {
@@ -1692,7 +1698,7 @@ static int kfd_ioctl_map_memory_to_gpu(struct file *filep,
}
 
/* Flush TLBs after waiting for the page table updates to complete */
-   if (table_freed) {
+   if (table_freed || !kfd_flush_tlb_after_unmap(dev)) {
for (i = 0; i < args->n_devices; i++) {
peer = kfd_device_by_id(devices_arr[i]);
if (WARN_ON_ONCE(!peer))
@@ -1806,7 +1812,7 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
*filep,
}
mutex_unlock(&p->mutex);
 
-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (kfd_flush_tlb_after_unmap(dev)) {
err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
(struct kgd_mem *) mem, true);
if (err) {
-- 
2.25.1

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Felix Kuehling]

Am 2022-01-18 um 4:28 p.m. schrieb Eric Huang:
> SDMA FW fixes the hang issue for adding heavy-weight TLB
> flush on Arcturus, so we can enable it.
>
> Signed-off-by: Eric Huang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 +++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a64cbbd943ba..f1fed0fc31d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1892,10 +1892,13 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
>   true);
>   ret = unreserve_bo_and_vms(&ctx, false, false);
>  
> - /* Only apply no TLB flush on Aldebaran to
> -  * workaround regressions on other Asics.
> + /* Only apply no TLB flush on Aldebaran and Arcturus
> +  * to workaround regressions on other Asics.
>*/
> - if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> + if (table_freed &&
> + (adev->asic_type != CHIP_ALDEBARAN) &&
> + (adev->asic_type != CHIP_ARCTURUS ||
> +  adev->sdma.instance[0].fw_version < 18))
>   *table_freed = true;

Can we move this check into the caller in kfd_chardev.c? That avoids
spreading around these conditions in several places.


>  
>   goto out;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b570c0454ce9..0e4a76dca809 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1806,7 +1806,9 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
> *filep,
>   }
>   mutex_unlock(&p->mutex);
>  
> - if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> + if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> + (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
> +  dev->adev->sdma.instance[0].fw_version >= 18)) {

Maybe put this into a helper function that can be used here and in
kfd_ioctl_map_memory_to_gpu. I also saw this being duplicated in the
upcoming CRIU patches. And we may want to adopt this in the SVM code as
well. Having one common helper makes sure we'll keep the TLB flushing
strategy consistent everywhere. Something like:

bool kfd_flush_tlb_after_unmap(struct kfd_dev *dev) {
return KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
  (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
   dev->adev->sdma.instance[0].fw_version >= 18);
}

Regards,
  Felix


>   err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
>   (struct kgd_mem *) mem, true);
>   if (err) {

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Alex Deucher]

On Tue, Jan 18, 2022 at 4:28 PM Eric Huang  wrote:
>
> SDMA FW fixes the hang issue for adding heavy-weight TLB
> flush on Arcturus, so we can enable it.
>
> Signed-off-by: Eric Huang 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 +++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a64cbbd943ba..f1fed0fc31d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1892,10 +1892,13 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> true);
> ret = unreserve_bo_and_vms(&ctx, false, false);
>
> -   /* Only apply no TLB flush on Aldebaran to
> -* workaround regressions on other Asics.
> +   /* Only apply no TLB flush on Aldebaran and Arcturus
> +* to workaround regressions on other Asics.
>  */
> -   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> +   if (table_freed &&
> +   (adev->asic_type != CHIP_ALDEBARAN) &&
> +   (adev->asic_type != CHIP_ARCTURUS ||
> +adev->sdma.instance[0].fw_version < 18))
> *table_freed = true;
>
> goto out;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b570c0454ce9..0e4a76dca809 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1806,7 +1806,9 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
> *filep,
> }
> mutex_unlock(&p->mutex);
>
> -   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> +   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> +   (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
> +dev->adev->sdma.instance[0].fw_version >= 18)) {
> err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> (struct kgd_mem *) mem, true);
> if (err) {
> --
> 2.25.1
>

[PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 +++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a64cbbd943ba..f1fed0fc31d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,10 +1892,13 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
true);
ret = unreserve_bo_and_vms(&ctx, false, false);
 
-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
+   /* Only apply no TLB flush on Aldebaran and Arcturus
+* to workaround regressions on other Asics.
 */
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
+   if (table_freed &&
+   (adev->asic_type != CHIP_ALDEBARAN) &&
+   (adev->asic_type != CHIP_ARCTURUS ||
+adev->sdma.instance[0].fw_version < 18))
*table_freed = true;
 
goto out;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b570c0454ce9..0e4a76dca809 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1806,7 +1806,9 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
*filep,
}
mutex_unlock(&p->mutex);
 
-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+   (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1) &&
+dev->adev->sdma.instance[0].fw_version >= 18)) {
err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
(struct kgd_mem *) mem, true);
if (err) {
-- 
2.25.1

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Dave Airlie]

On Wed, 19 Jan 2022 at 06:14, Eric Huang  wrote:
>
> I understand Alex's concern. I think usually we only check the version
> when a feature is only available in a specific version, and other
> version or newer version doesn't have.
>
> In case of FW fix, we assume the driver and FWs have to be synchronous.
> If we have driver backward compatibility for FWs, there must be a lot of
> redundant codes for FW version check. So this patch and SDMA fix will be
> pushed into ROCm 5.1 release branch at the same time.
>

Please remove that assumption from everwhere.

If you have released a fw into linux-firmware then you need to support
it going forward, even if it means printing something in dmesg
recommending people upgrade for features.

Dave.

Re: [PATCH] amdgpu/amdgpu_psp: remove unneeded ret variable

[Alex Deucher]

Applied.  Thanks!

Alex

On Tue, Jan 18, 2022 at 2:57 AM  wrote:
>
> From: Minghao Chi 
>
> Return value from amdgpu_bo_create_kernel() directly instead
> of taking this in another redundant variable.
>
> Reported-by: Zeal Robot 
> Signed-off-by: Minghao Chi 
> Signed-off-by: CGEL ZTE 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index dee17a0e1187..ac2b87f81ef9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -914,19 +914,15 @@ static void psp_prep_ta_load_cmd_buf(struct 
> psp_gfx_cmd_resp *cmd,
>  static int psp_ta_init_shared_buf(struct psp_context *psp,
>   struct ta_mem_context *mem_ctx)
>  {
> -   int ret;
> -
> /*
> * Allocate 16k memory aligned to 4k from Frame Buffer (local
> * physical) for ta to host memory
> */
> -   ret = amdgpu_bo_create_kernel(psp->adev, mem_ctx->shared_mem_size,
> +   return amdgpu_bo_create_kernel(psp->adev, mem_ctx->shared_mem_size,
>   PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
>   &mem_ctx->shared_bo,
>   &mem_ctx->shared_mc_addr,
>   &mem_ctx->shared_buf);
> -
> -   return ret;
>  }
>
>  static void psp_ta_free_shared_buf(struct ta_mem_context *mem_ctx)
> --
> 2.25.1
>

Re: [PATCH] drm/radeon: fix UVD suspend error

[Alex Deucher]

Applied.  Thanks!

On Mon, Jan 17, 2022 at 3:05 PM Leo Liu  wrote:
>
>
> On 2022-01-17 2:47 a.m., Qiang Ma wrote:
> > I met a bug recently and the kernel log:
> >
> > [  330.171875] radeon :03:00.0: couldn't schedule ib
> > [  330.175781] [drm:radeon_uvd_suspend [radeon]] *ERROR* Error destroying 
> > UVD (-22)!
> >
> > In radeon drivers, using UVD suspend is as follows:
> >
> > if (rdev->has_uvd) {
> >  uvd_v1_0_fini(rdev);
> >  radeon_uvd_suspend(rdev);
> > }
> >
> > In radeon_ib_schedule function, we check the 'ring->ready' state,
> > but in uvd_v1_0_fini funciton, we've cleared the ready state.
> > So, just modify the suspend code flow to fix error.
>
> It seems reasonable to me. The suspend sends the destroy message if
> there is still incomplete job, so it should be before the fini which
> stops the hardware.
>
> The series are:
>
> Reviewed-by: Leo Liu 
>
>
> >
> > Signed-off-by: Qiang Ma 
> > ---
> >   drivers/gpu/drm/radeon/cik.c   | 2 +-
> >   drivers/gpu/drm/radeon/evergreen.c | 2 +-
> >   drivers/gpu/drm/radeon/ni.c| 2 +-
> >   drivers/gpu/drm/radeon/r600.c  | 2 +-
> >   drivers/gpu/drm/radeon/rv770.c | 2 +-
> >   drivers/gpu/drm/radeon/si.c| 2 +-
> >   6 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> > index 81b4de7be9f2..5819737c21c6 100644
> > --- a/drivers/gpu/drm/radeon/cik.c
> > +++ b/drivers/gpu/drm/radeon/cik.c
> > @@ -8517,8 +8517,8 @@ int cik_suspend(struct radeon_device *rdev)
> >   cik_cp_enable(rdev, false);
> >   cik_sdma_enable(rdev, false);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   if (rdev->has_vce)
> >   radeon_vce_suspend(rdev);
> > diff --git a/drivers/gpu/drm/radeon/evergreen.c 
> > b/drivers/gpu/drm/radeon/evergreen.c
> > index eeb590d2dec2..455f8036aa54 100644
> > --- a/drivers/gpu/drm/radeon/evergreen.c
> > +++ b/drivers/gpu/drm/radeon/evergreen.c
> > @@ -5156,8 +5156,8 @@ int evergreen_suspend(struct radeon_device *rdev)
> >   radeon_pm_suspend(rdev);
> >   radeon_audio_fini(rdev);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   r700_cp_stop(rdev);
> >   r600_dma_stop(rdev);
> > diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> > index 4a364ca7a1be..927e5f42e97d 100644
> > --- a/drivers/gpu/drm/radeon/ni.c
> > +++ b/drivers/gpu/drm/radeon/ni.c
> > @@ -2323,8 +2323,8 @@ int cayman_suspend(struct radeon_device *rdev)
> >   cayman_cp_enable(rdev, false);
> >   cayman_dma_stop(rdev);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   evergreen_irq_suspend(rdev);
> >   radeon_wb_disable(rdev);
> > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> > index ca3fcae2adb5..dd78fc499402 100644
> > --- a/drivers/gpu/drm/radeon/r600.c
> > +++ b/drivers/gpu/drm/radeon/r600.c
> > @@ -3232,8 +3232,8 @@ int r600_suspend(struct radeon_device *rdev)
> >   radeon_audio_fini(rdev);
> >   r600_cp_stop(rdev);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   r600_irq_suspend(rdev);
> >   radeon_wb_disable(rdev);
> > diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
> > index e592e57be1bb..38796af4fadd 100644
> > --- a/drivers/gpu/drm/radeon/rv770.c
> > +++ b/drivers/gpu/drm/radeon/rv770.c
> > @@ -1894,8 +1894,8 @@ int rv770_suspend(struct radeon_device *rdev)
> >   radeon_pm_suspend(rdev);
> >   radeon_audio_fini(rdev);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   r700_cp_stop(rdev);
> >   r600_dma_stop(rdev);
> > diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> > index 013e44ed0f39..8d5e4b25609d 100644
> > --- a/drivers/gpu/drm/radeon/si.c
> > +++ b/drivers/gpu/drm/radeon/si.c
> > @@ -6800,8 +6800,8 @@ int si_suspend(struct radeon_device *rdev)
> >   si_cp_enable(rdev, false);
> >   cayman_dma_stop(rdev);
> >   if (rdev->has_uvd) {
> > - uvd_v1_0_fini(rdev);
> >   radeon_uvd_suspend(rdev);
> > + uvd_v1_0_fini(rdev);
> >   }
> >   if (rdev->has_vce)
> >   radeon_vce_suspend(rdev);

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Alex Deucher]

On Tue, Jan 18, 2022 at 3:13 PM Eric Huang  wrote:
>
> I understand Alex's concern. I think usually we only check the version
> when a feature is only available in a specific version, and other
> version or newer version doesn't have.
>
> In case of FW fix, we assume the driver and FWs have to be synchronous.
> If we have driver backward compatibility for FWs, there must be a lot of
> redundant codes for FW version check. So this patch and SDMA fix will be
> pushed into ROCm 5.1 release branch at the same time.

We need to check.  The firmwares are not distributed in lock step with
the driver.

Alex


>
> Regards,
> Eric
>
> On 2022-01-18 14:35, Alex Deucher wrote:
> > On Tue, Jan 18, 2022 at 2:27 PM Russell, Kent  wrote:
> >> [AMD Official Use Only]
> >>
> >> I think what he means is that if we are using SDMA v17, this will cause 
> >> issues, won't it? Should we check that SDMA version is >=18 before 
> >> enabling it? Or am I misunderstanding the fix?
> > Yes, that was my concern.
> >
> > Alex
> >
> >>   Kent
> >>
> >>> -Original Message-
> >>> From: amd-gfx  On Behalf Of Eric 
> >>> Huang
> >>> Sent: Tuesday, January 18, 2022 2:09 PM
> >>> To: Alex Deucher 
> >>> Cc: amd-gfx list 
> >>> Subject: Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus
> >>>
> >>> The SDMA fix is generic and not in a specific version of FW, so we don't
> >>> have to check.
> >>>
> >>> Thanks,
> >>> Eric
> >>>
> >>> On 2022-01-18 11:35, Alex Deucher wrote:
>  On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  
>  wrote:
> > SDMA FW fixes the hang issue for adding heavy-weight TLB
> > flush on Arcturus, so we can enable it.
>  Do we need to check for a specific fw version?
> 
>  Alex
> 
> > Signed-off-by: Eric Huang 
> > ---
> >drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
> >drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
> >2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index a64cbbd943ba..7b24a920c12e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> >   true);
> >   ret = unreserve_bo_and_vms(&ctx, false, false);
> >
> > -   /* Only apply no TLB flush on Aldebaran to
> > -* workaround regressions on other Asics.
> > +   /* Only apply no TLB flush on Aldebaran and Arcturus
> > +* to workaround regressions on other Asics.
> >*/
> > -   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> > +   if (table_freed &&
> > +   (adev->asic_type != CHIP_ALDEBARAN) &&
> > +   (adev->asic_type != CHIP_ARCTURUS))
> >   *table_freed = true;
> >
> >   goto out;
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index b570c0454ce9..ef4d676cc71c 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct 
> > file
> >>> *filep,
> >   }
> >   mutex_unlock(&p->mutex);
> >
> > -   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> > +   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> > +   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
> >   err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> >   (struct kgd_mem *) mem, true);
> >   if (err) {
> > --
> > 2.25.1
> >
>

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

I understand Alex's concern. I think usually we only check the version 
when a feature is only available in a specific version, and other 
version or newer version doesn't have.


In case of FW fix, we assume the driver and FWs have to be synchronous. 
If we have driver backward compatibility for FWs, there must be a lot of 
redundant codes for FW version check. So this patch and SDMA fix will be 
pushed into ROCm 5.1 release branch at the same time.


Regards,
Eric

On 2022-01-18 14:35, Alex Deucher wrote:

On Tue, Jan 18, 2022 at 2:27 PM Russell, Kent  wrote:

[AMD Official Use Only]

I think what he means is that if we are using SDMA v17, this will cause issues, 
won't it? Should we check that SDMA version is >=18 before enabling it? Or am I 
misunderstanding the fix?

Yes, that was my concern.

Alex


  Kent


-Original Message-
From: amd-gfx  On Behalf Of Eric Huang
Sent: Tuesday, January 18, 2022 2:09 PM
To: Alex Deucher 
Cc: amd-gfx list 
Subject: Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

The SDMA fix is generic and not in a specific version of FW, so we don't
have to check.

Thanks,
Eric

On 2022-01-18 11:35, Alex Deucher wrote:

On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  wrote:

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Do we need to check for a specific fw version?

Alex


Signed-off-by: Eric Huang 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
   2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c

b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c

index a64cbbd943ba..7b24a920c12e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
  true);
  ret = unreserve_bo_and_vms(&ctx, false, false);

-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
+   /* Only apply no TLB flush on Aldebaran and Arcturus
+* to workaround regressions on other Asics.
   */
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
+   if (table_freed &&
+   (adev->asic_type != CHIP_ALDEBARAN) &&
+   (adev->asic_type != CHIP_ARCTURUS))
  *table_freed = true;

  goto out;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c

b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c

index b570c0454ce9..ef4d676cc71c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file

*filep,

  }
  mutex_unlock(&p->mutex);

-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
  err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
  (struct kgd_mem *) mem, true);
  if (err) {
--
2.25.1

Re: [PATCH] drm/amdgpu: Add missing pm_runtime_put_autosuspend

[Alex Deucher]

Applied.  Strangely I can't seem to find this patch in my inbox or in
the dri-devel or amd-gfx archives.

Alex

On Tue, Jan 18, 2022 at 9:03 AM Lazar, Lijo  wrote:
>
>
>
> On 1/18/2022 5:31 PM, Yongzhi Liu wrote:
> > pm_runtime_get_sync() increments the runtime PM usage counter even
> > when it returns an error code, thus a matching decrement is needed
> > on the error handling path to keep the counter balanced.
> >
> > Signed-off-by: Yongzhi Liu 
>
> Thanks!
>
> Reviewed-by: Lijo Lazar 
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index 9aea1cc..4b950de 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -1120,8 +1120,10 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct 
> > file *f, char __user *buf,
> >   return -EINVAL;
> >
> >   r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> > - if (r < 0)
> > + if (r < 0) {
> > + pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> >   return r;
> > + }
> >
> >   while (size) {
> >   uint32_t value;
> >

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Alex Deucher]

On Tue, Jan 18, 2022 at 2:27 PM Russell, Kent  wrote:
>
> [AMD Official Use Only]
>
> I think what he means is that if we are using SDMA v17, this will cause 
> issues, won't it? Should we check that SDMA version is >=18 before enabling 
> it? Or am I misunderstanding the fix?

Yes, that was my concern.

Alex

>
>  Kent
>
> > -Original Message-
> > From: amd-gfx  On Behalf Of Eric 
> > Huang
> > Sent: Tuesday, January 18, 2022 2:09 PM
> > To: Alex Deucher 
> > Cc: amd-gfx list 
> > Subject: Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus
> >
> > The SDMA fix is generic and not in a specific version of FW, so we don't
> > have to check.
> >
> > Thanks,
> > Eric
> >
> > On 2022-01-18 11:35, Alex Deucher wrote:
> > > On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  
> > > wrote:
> > >> SDMA FW fixes the hang issue for adding heavy-weight TLB
> > >> flush on Arcturus, so we can enable it.
> > > Do we need to check for a specific fw version?
> > >
> > > Alex
> > >
> > >> Signed-off-by: Eric Huang 
> > >> ---
> > >>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
> > >>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
> > >>   2 files changed, 7 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > >> index a64cbbd943ba..7b24a920c12e 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > >> @@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> > >>  true);
> > >>  ret = unreserve_bo_and_vms(&ctx, false, false);
> > >>
> > >> -   /* Only apply no TLB flush on Aldebaran to
> > >> -* workaround regressions on other Asics.
> > >> +   /* Only apply no TLB flush on Aldebaran and Arcturus
> > >> +* to workaround regressions on other Asics.
> > >>   */
> > >> -   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> > >> +   if (table_freed &&
> > >> +   (adev->asic_type != CHIP_ALDEBARAN) &&
> > >> +   (adev->asic_type != CHIP_ARCTURUS))
> > >>  *table_freed = true;
> > >>
> > >>  goto out;
> > >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > >> index b570c0454ce9..ef4d676cc71c 100644
> > >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > >> @@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct 
> > >> file
> > *filep,
> > >>  }
> > >>  mutex_unlock(&p->mutex);
> > >>
> > >> -   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> > >> +   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> > >> +   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
> > >>  err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> > >>  (struct kgd_mem *) mem, true);
> > >>  if (err) {
> > >> --
> > >> 2.25.1
> > >>
>

RE: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Russell, Kent]

[AMD Official Use Only]

I think what he means is that if we are using SDMA v17, this will cause issues, 
won't it? Should we check that SDMA version is >=18 before enabling it? Or am I 
misunderstanding the fix?

 Kent

> -Original Message-
> From: amd-gfx  On Behalf Of Eric Huang
> Sent: Tuesday, January 18, 2022 2:09 PM
> To: Alex Deucher 
> Cc: amd-gfx list 
> Subject: Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus
>
> The SDMA fix is generic and not in a specific version of FW, so we don't
> have to check.
>
> Thanks,
> Eric
>
> On 2022-01-18 11:35, Alex Deucher wrote:
> > On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  
> > wrote:
> >> SDMA FW fixes the hang issue for adding heavy-weight TLB
> >> flush on Arcturus, so we can enable it.
> > Do we need to check for a specific fw version?
> >
> > Alex
> >
> >> Signed-off-by: Eric Huang 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
> >>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
> >>   2 files changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> index a64cbbd943ba..7b24a920c12e 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> @@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> >>  true);
> >>  ret = unreserve_bo_and_vms(&ctx, false, false);
> >>
> >> -   /* Only apply no TLB flush on Aldebaran to
> >> -* workaround regressions on other Asics.
> >> +   /* Only apply no TLB flush on Aldebaran and Arcturus
> >> +* to workaround regressions on other Asics.
> >>   */
> >> -   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> >> +   if (table_freed &&
> >> +   (adev->asic_type != CHIP_ALDEBARAN) &&
> >> +   (adev->asic_type != CHIP_ARCTURUS))
> >>  *table_freed = true;
> >>
> >>  goto out;
> >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> index b570c0454ce9..ef4d676cc71c 100644
> >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> >> @@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct 
> >> file
> *filep,
> >>  }
> >>  mutex_unlock(&p->mutex);
> >>
> >> -   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> >> +   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> >> +   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
> >>  err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> >>  (struct kgd_mem *) mem, true);
> >>  if (err) {
> >> --
> >> 2.25.1
> >>

Re: [PATCH v11 03/19] dyndbg: add write-to-tracefs code

[jim . cromie]

On Fri, Jan 14, 2022 at 4:46 AM Vincent Whitchurch
 wrote:
>
> On Fri, Jan 07, 2022 at 06:29:26AM +0100, Jim Cromie wrote:
> >

> > Enabling debug-to-tracefs is 2 steps:
> >
> >   # event enable
> >   echo 1 > /sys/kernel/tracing/events/dyndbg/enable
> >   # callsite enable
> >   echo module foo +T > /proc/dynamic_debug/control
> >
> > This patch,~1,~2 are based upon:
> >   
> > https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu...@axis.com/
> >
> > .. with simplification of temporarily reusing trace_console() rather
> > than adding a new printk:dyndbg event.  Soon, add 2 new events
> > capturing the pr_debug & dev_dbg() args.
>
> The example above does not match the code in this patch since the
> dyndbg:* events are only added in a later patch.  Perhaps you could
> reorder this patch stack so that you don't use trace_console() in this
> patch just to replace it with the new events in the next patch?
>

good catch, thanks.
Ive just dropped the example, it seemed the simplest fix.
It seemed proper to commit your code as pristine as practical,
so that subsequent mistakes receive the blame.

and Ive fixed the spurious whitespace change you noted.

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

The SDMA fix is generic and not in a specific version of FW, so we don't 
have to check.


Thanks,
Eric

On 2022-01-18 11:35, Alex Deucher wrote:

On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  wrote:

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Do we need to check for a specific fw version?

Alex


Signed-off-by: Eric Huang 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
  2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a64cbbd943ba..7b24a920c12e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
 true);
 ret = unreserve_bo_and_vms(&ctx, false, false);

-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
+   /* Only apply no TLB flush on Aldebaran and Arcturus
+* to workaround regressions on other Asics.
  */
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
+   if (table_freed &&
+   (adev->asic_type != CHIP_ALDEBARAN) &&
+   (adev->asic_type != CHIP_ARCTURUS))
 *table_freed = true;

 goto out;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b570c0454ce9..ef4d676cc71c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
*filep,
 }
 mutex_unlock(&p->mutex);

-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
 err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
 (struct kgd_mem *) mem, true);
 if (err) {
--
2.25.1

Re: [PATCH] drm/amd/display: Copy crc_skip_count when duplicating CRTC state

[Rodrigo Siqueira Jordao]

On 2022-01-18 11:49 a.m., Kazlauskas, Nicholas wrote:

On 1/18/2022 11:40 AM, Rodrigo Siqueira wrote:

From: Leo Li 

[Why]
crc_skip_count is used to track how many frames to skip to allow the OTG
CRC engine to "warm up" before it outputs correct CRC values.
Experimentally, this seems to be 2 frames.

When duplicating CRTC states, this value was not copied to the
duplicated state. Therefore, when this state is committed, we will
needlessly wait 2 frames before outputing CRC values. Even if the CRC
engine is already warmed up. >
[How]
Copy the crc_skip_count as part of dm_crtc_duplicate_state.


This likely introduces regressions.

Here's an example case where it can take two frames even after the CRTC 
is enabled:


1. VUPDATE is before line 0, in the front porch, counter=0
2. Flip arrives before VUPDATE is signaled, but does not finish 
programming until after VUPDATE point, counter=0.

3. Vblank counter increments, counter=1.
4. Flip programming finishes, counter=1.
5. OS delay happens, cursor programming is delayed, counter=1.
6. Cursor programming starts, counter=1.
7. VUPDATE fires, updating frame but missing cursor, counter=1.
8. Cursor programming finishes, counter=2.
9. Cursor programming pending for counter=2.

This is a little contrived, but I've seen something similar happen 
during IGT testing before.


Hi Nick,

I'm wondering if we have a test that can reproduce the issue that you 
described. For this specific patch, I ran it through our pre-submission 
CI, and everything worked as expected, except the subtest 
pipe-a-primary-size-* in the kms_plane_cursor. However, I submitted the 
below IGT patch that fixed the issue:


 https://patchwork.freedesktop.org/patch/469919/?series=98994&rev=1
 (Note that the IGT fix is not related to the scenario you described.)

In summary, I do not see regressions in other tests, whereas this patch 
fixes multiple CRC mismatches when running IGT (e.g., kms_plane and 
plane_scaling).


Do you recommend running some specific test to reproduce the issue? 
Maybe some particular set of steps that enable me to see the issue?


Ps.: Some IGT tests already compensate for the two extra frames (see 
link [1]), this behavior also happens on other drivers such as i915, and 
we still fail in those tests because we are skipping four frames without 
this patch.
1. 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_plane.c#L612


Thanks
Siqueira

This is because cursor update happens independent of the rest of plane 
programming and is tied to a separate lock. That lock part can't change 
due to potential for stuttering, but the first part could be fixed.


Regards,
Nicholas Kazlauskas



Cc: Mark Yacoub 
Cc: Hayden Goodfellow 
Cc: Harry Wentland 
Cc: Nicholas Choi 

Signed-off-by: Leo Li 
Signed-off-by: Rodrigo Siqueira 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

index 87299e62fe12..5482b0925396 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6568,6 +6568,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
  state->freesync_config = cur->freesync_config;
  state->cm_has_degamma = cur->cm_has_degamma;
  state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
+    state->crc_skip_count = cur->crc_skip_count;
  state->force_dpms_off = cur->force_dpms_off;
  /* TODO Duplicate dc_stream after objects are stream object is 
flattened */

Re: [PATCH v3] drm/amd/display: move calcs folder into DML

[Rodrigo Siqueira Jordao]

On 2022-01-07 4:33 p.m., Isabella Basso wrote:

The calcs folder has FPU code on it, which should be isolated inside the
DML folder as per https://patchwork.freedesktop.org/series/93042/.

This commit aims single-handedly to correct the location of such FPU
code and does not refactor any functions.

Changes since v2:
- Corrected problems to compile when DCN was disabled.

Signed-off-by: Isabella Basso 
---
  drivers/gpu/drm/amd/display/dc/Makefile   |  4 +-
  drivers/gpu/drm/amd/display/dc/calcs/Makefile | 68 ---
  drivers/gpu/drm/amd/display/dc/dml/Makefile   | 10 ++-
  .../amd/display/dc/{ => dml}/calcs/bw_fixed.c |  0
  .../display/dc/{ => dml}/calcs/calcs_logger.h |  0
  .../display/dc/{ => dml}/calcs/custom_float.c |  0
  .../display/dc/{ => dml}/calcs/dce_calcs.c|  0
  .../dc/{ => dml}/calcs/dcn_calc_auto.c|  0
  .../dc/{ => dml}/calcs/dcn_calc_auto.h|  0
  .../dc/{ => dml}/calcs/dcn_calc_math.c|  0
  .../display/dc/{ => dml}/calcs/dcn_calcs.c|  0
  11 files changed, 11 insertions(+), 71 deletions(-)
  delete mode 100644 drivers/gpu/drm/amd/display/dc/calcs/Makefile
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/bw_fixed.c (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/calcs_logger.h (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/custom_float.c (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dce_calcs.c (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dcn_calc_auto.c (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dcn_calc_auto.h (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dcn_calc_math.c (100%)
  rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dcn_calcs.c (100%)

diff --git a/drivers/gpu/drm/amd/display/dc/Makefile 
b/drivers/gpu/drm/amd/display/dc/Makefile
index b1f0d6260226..a4ef8f314307 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -23,12 +23,12 @@
  # Makefile for Display Core (dc) component.
  #
  
-DC_LIBS = basics bios calcs clk_mgr dce gpio irq virtual

+DC_LIBS = basics bios clk_mgr dce dml gpio irq virtual
  
  ifdef CONFIG_DRM_AMD_DC_DCN

  DC_LIBS += dcn20
  DC_LIBS += dsc
-DC_LIBS += dcn10 dml
+DC_LIBS += dcn10
  DC_LIBS += dcn21
  DC_LIBS += dcn201
  DC_LIBS += dcn30
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/Makefile 
b/drivers/gpu/drm/amd/display/dc/calcs/Makefile
deleted file mode 100644
index f3c00f479e1c..
--- a/drivers/gpu/drm/amd/display/dc/calcs/Makefile
+++ /dev/null
@@ -1,68 +0,0 @@
-#
-# Copyright 2017 Advanced Micro Devices, Inc.
-# Copyright 2019 Raptor Engineering, LLC
-#
-# Permission is hereby granted, free of charge, to any person obtaining a
-# copy of this software and associated documentation files (the "Software"),
-# to deal in the Software without restriction, including without limitation
-# the rights to use, copy, modify, merge, publish, distribute, sublicense,
-# and/or sell copies of the Software, and to permit persons to whom the
-# Software is furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice shall be included in
-# all copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
-# THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
-# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
-# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
-# OTHER DEALINGS IN THE SOFTWARE.
-#
-#
-# Makefile for the 'calcs' sub-component of DAL.
-# It calculates Bandwidth and Watermarks values for HW programming
-#
-
-ifdef CONFIG_X86
-calcs_ccflags := -mhard-float -msse
-endif
-
-ifdef CONFIG_PPC64
-calcs_ccflags := -mhard-float -maltivec
-endif
-
-ifdef CONFIG_CC_IS_GCC
-ifeq ($(call cc-ifversion, -lt, 0701, y), y)
-IS_OLD_GCC = 1
-endif
-endif
-
-ifdef CONFIG_X86
-ifdef IS_OLD_GCC
-# Stack alignment mismatch, proceed with caution.
-# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3
-# (8B stack alignment).
-calcs_ccflags += -mpreferred-stack-boundary=4
-else
-calcs_ccflags += -msse2
-endif
-endif
-
-CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_ccflags)
-CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_ccflags) 
-Wno-tautological-compare
-CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_rcflags)
-CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_rcflags)
-CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_rcflags)
-
-BW_CALCS = dce_calcs.o bw_fixed.o custom_float.o
-
-ifdef CONFIG_DRM_AMD_DC_DCN
-BW_CALCS += dcn_calcs.o dcn_calc_math.o dcn_calc_auto

[PATCH v3] drm/amd: Warn users about potential s0ix problems

[Mario Limonciello]

On some OEM setups users can configure the BIOS for S3 or S2idle.
When configured to S3 users can still choose 's2idle' in the kernel by
using `/sys/power/mem_sleep`.  Before commit 6dc8265f9803 ("drm/amdgpu:
always reset the asic in suspend (v2)"), the GPU would crash.  Now when
configured this way, the system should resume but will use more power.

As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
potential power consumption issues during their first attempt at
suspending.

Cc: Liang Prike 
Cc: Lazar Lijo 
Reported-by: Bjoren Dasse 
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1824
Signed-off-by: Mario Limonciello 
---
v2->v3:
 * Better direct users how to recover in the bad cases
v1->v2:
 * Only show messages in s2idle cases

 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 4811b0faafd9..1ad379a46883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1040,11 +1040,16 @@ void amdgpu_acpi_detect(void)
  */
 bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
 {
-#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
-   if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
-   if (adev->flags & AMD_IS_APU)
-   return pm_suspend_target_state == PM_SUSPEND_TO_IDLE;
-   }
+   if (!(adev->flags & AMD_IS_APU) ||
+   pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
+   return false;
+   if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
+   dev_warn_once(adev->dev,
+ "Power consumption will be higher as BIOS has not 
been configured for suspend-to-idle.\n"
+ "To use suspend-to-idle change the sleep mode in 
BIOS setup.\n");
+#if !IS_ENABLED(CONFIG_AMD_PMC)
+   dev_warn_once(adev->dev,
+ "Power consumption will be higher as the kernel has not 
been compiled with CONFIG_AMD_PMC.\n");
 #endif
-   return false;
+   return true;
 }
-- 
2.25.1

RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

[Limonciello, Mario]

[Public]

That is what I was hoping to convey with the warning messages.  I'll take a 
stab at rewording them and send out an updated revision.

From: Liang, Prike 
Sent: Tuesday, January 18, 2022 02:28
To: Limonciello, Mario ; Lazar, Lijo 
; amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse 
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

If the flag ACPI_FADT_LOW_POWER_S0 not set or AMDPMC driver not build, then 
that seems will mess up the suspend entry and unable to enter either S3 nor 
S2idle properly. In this S2idle configuration issue case, how about add some 
message to notify end user how to configure S2idle correctly?

Thanks,
Prike
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Limonciello, Mario
Sent: Tuesday, January 18, 2022 1:26 AM
To: Lazar, Lijo mailto:lijo.la...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse mailto:bjoern.da...@gmail.com>>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems


[Public]

Yes, that's part of why I want to make sure there are explicit warnings to 
users about using this flow.
When not enabled in ACPI then also the LPS0 device is not exported and AMD_PMC 
won't load or be used.

I think from amdgpu perspective it should behave relatively similar to an 
aborted suspend.

From: Lazar, Lijo mailto:lijo.la...@amd.com>>
Sent: Monday, January 17, 2022 11:20
To: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse mailto:bjoern.da...@gmail.com>>
Subject: Re: [PATCH v2] drm/amd: Warn users about potential s0ix problems

Any problem with PMFW sequence in the way Linux handles s2idle when it's not 
enabled in ACPI?

Thanks,
Lijo

From: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>
Sent: Monday, January 17, 2022 10:45:44 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>; Lazar, 
Lijo mailto:lijo.la...@amd.com>>
Cc: Bjoren Dasse mailto:bjoern.da...@gmail.com>>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

[Public]

This has been sitting a week or so.
Bump on review for this patch.

> -Original Message-
> From: Limonciello, Mario 
> mailto:mario.limoncie...@amd.com>>
> Sent: Tuesday, January 11, 2022 14:00
> To: amd-gfx@lists.freedesktop.org
> Cc: Limonciello, Mario 
> mailto:mario.limoncie...@amd.com>>; Bjoren Dasse
> mailto:bjoern.da...@gmail.com>>
> Subject: [PATCH v2] drm/amd: Warn users about potential s0ix problems
>
> On some OEM setups users can configure the BIOS for S3 or S2idle.
> When configured to S3 users can still choose 's2idle' in the kernel by
> using `/sys/power/mem_sleep`.  Before commit 6dc8265f9803
> ("drm/amdgpu:
> always reset the asic in suspend (v2)"), the GPU would crash.  Now when
> configured this way, the system should resume but will use more power.
>
> As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
> potential power consumption issues during their first attempt at
> suspending.
>
> Reported-by: Bjoren Dasse 
> mailto:bjoern.da...@gmail.com>>
> Link: 
> https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> Signed-off-by: Mario Limonciello 
> mailto:mario.limoncie...@amd.com>>
> ---
> v1->v2:
>  * Only show messages in s2idle cases
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 4811b0faafd9..1295de6d6c30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1040,11 +1040,15 @@ void amdgpu_acpi_detect(void)
>   */
>  bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
>  {
> -#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
> - if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
> - if (adev->flags & AMD_IS_APU)
> - return pm_suspend_target_state ==
> PM_SUSPEND_TO_IDLE;
> - }
> + if (!(adev->flags & AMD_IS_APU) ||
> + pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
> + return false;
> + if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
> + dev_warn_once(adev->dev,
> +   "BIOS is not configured for suspend-to-idle, power
> consumption will be higher\n");
> +#if 

Re: [PATCH] drm/amd/display: Copy crc_skip_count when duplicating CRTC state

[Kazlauskas, Nicholas]

On 1/18/2022 11:40 AM, Rodrigo Siqueira wrote:

From: Leo Li 

[Why]
crc_skip_count is used to track how many frames to skip to allow the OTG
CRC engine to "warm up" before it outputs correct CRC values.
Experimentally, this seems to be 2 frames.

When duplicating CRTC states, this value was not copied to the
duplicated state. Therefore, when this state is committed, we will
needlessly wait 2 frames before outputing CRC values. Even if the CRC
engine is already warmed up. >
[How]
Copy the crc_skip_count as part of dm_crtc_duplicate_state.


This likely introduces regressions.

Here's an example case where it can take two frames even after the CRTC 
is enabled:


1. VUPDATE is before line 0, in the front porch, counter=0
2. Flip arrives before VUPDATE is signaled, but does not finish 
programming until after VUPDATE point, counter=0.

3. Vblank counter increments, counter=1.
4. Flip programming finishes, counter=1.
5. OS delay happens, cursor programming is delayed, counter=1.
6. Cursor programming starts, counter=1.
7. VUPDATE fires, updating frame but missing cursor, counter=1.
8. Cursor programming finishes, counter=2.
9. Cursor programming pending for counter=2.

This is a little contrived, but I've seen something similar happen 
during IGT testing before.


This is because cursor update happens independent of the rest of plane 
programming and is tied to a separate lock. That lock part can't change 
due to potential for stuttering, but the first part could be fixed.


Regards,
Nicholas Kazlauskas



Cc: Mark Yacoub 
Cc: Hayden Goodfellow 
Cc: Harry Wentland 
Cc: Nicholas Choi 

Signed-off-by: Leo Li 
Signed-off-by: Rodrigo Siqueira 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 87299e62fe12..5482b0925396 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6568,6 +6568,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->freesync_config = cur->freesync_config;
state->cm_has_degamma = cur->cm_has_degamma;
state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
+   state->crc_skip_count = cur->crc_skip_count;
state->force_dpms_off = cur->force_dpms_off;
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
  

[PATCH] drm/amd/display: Copy crc_skip_count when duplicating CRTC state

[Rodrigo Siqueira]

From: Leo Li 

[Why]
crc_skip_count is used to track how many frames to skip to allow the OTG
CRC engine to "warm up" before it outputs correct CRC values.
Experimentally, this seems to be 2 frames.

When duplicating CRTC states, this value was not copied to the
duplicated state. Therefore, when this state is committed, we will
needlessly wait 2 frames before outputing CRC values. Even if the CRC
engine is already warmed up.

[How]
Copy the crc_skip_count as part of dm_crtc_duplicate_state.

Cc: Mark Yacoub 
Cc: Hayden Goodfellow 
Cc: Harry Wentland 
Cc: Nicholas Choi 

Signed-off-by: Leo Li 
Signed-off-by: Rodrigo Siqueira 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 87299e62fe12..5482b0925396 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6568,6 +6568,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->freesync_config = cur->freesync_config;
state->cm_has_degamma = cur->cm_has_degamma;
state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
+   state->crc_skip_count = cur->crc_skip_count;
state->force_dpms_off = cur->force_dpms_off;
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
 
-- 
2.25.1

Re: [PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Alex Deucher]

On Tue, Jan 18, 2022 at 11:16 AM Eric Huang  wrote:
>
> SDMA FW fixes the hang issue for adding heavy-weight TLB
> flush on Arcturus, so we can enable it.

Do we need to check for a specific fw version?

Alex

>
> Signed-off-by: Eric Huang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index a64cbbd943ba..7b24a920c12e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> true);
> ret = unreserve_bo_and_vms(&ctx, false, false);
>
> -   /* Only apply no TLB flush on Aldebaran to
> -* workaround regressions on other Asics.
> +   /* Only apply no TLB flush on Aldebaran and Arcturus
> +* to workaround regressions on other Asics.
>  */
> -   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
> +   if (table_freed &&
> +   (adev->asic_type != CHIP_ALDEBARAN) &&
> +   (adev->asic_type != CHIP_ARCTURUS))
> *table_freed = true;
>
> goto out;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index b570c0454ce9..ef4d676cc71c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
> *filep,
> }
> mutex_unlock(&p->mutex);
>
> -   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
> +   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
> +   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
> err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
> (struct kgd_mem *) mem, true);
> if (err) {
> --
> 2.25.1
>

[PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a64cbbd943ba..7b24a920c12e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
true);
ret = unreserve_bo_and_vms(&ctx, false, false);
 
-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
+   /* Only apply no TLB flush on Aldebaran and Arcturus
+* to workaround regressions on other Asics.
 */
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
+   if (table_freed &&
+   (adev->asic_type != CHIP_ALDEBARAN) &&
+   (adev->asic_type != CHIP_ARCTURUS))
*table_freed = true;
 
goto out;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b570c0454ce9..ef4d676cc71c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
*filep,
}
mutex_unlock(&p->mutex);
 
-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
(struct kgd_mem *) mem, true);
if (err) {
-- 
2.25.1

[PATCH] drm/amdkfd: enable heavy-weight TLB flush on Arcturus

[Eric Huang]

SDMA FW fixes the hang issue for adding heavy-weight TLB
flush on Arcturus, so we can enable it.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 8 +---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a64cbbd943ba..e1d90643731c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1892,10 +1892,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
true);
ret = unreserve_bo_and_vms(&ctx, false, false);
 
-   /* Only apply no TLB flush on Aldebaran to
-* workaround regressions on other Asics.
+   /* Only apply no TLB flush on Aldebaran and Arcturus
+* to workaround regressions on other Asics.
 */
-   if (table_freed && (adev->asic_type != CHIP_ALDEBARAN))
+   if (table_freed &&
+   ((adev->asic_type != CHIP_ALDEBARAN) ||
+(adev->asic_type != CHIP_ARCTURUS))
*table_freed = true;
 
goto out;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b570c0454ce9..ef4d676cc71c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1806,7 +1806,8 @@ static int kfd_ioctl_unmap_memory_from_gpu(struct file 
*filep,
}
mutex_unlock(&p->mutex);
 
-   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2)) {
+   if (KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 2) ||
+   KFD_GC_VERSION(dev) == IP_VERSION(9, 4, 1)) {
err = amdgpu_amdkfd_gpuvm_sync_memory(dev->adev,
(struct kgd_mem *) mem, true);
if (err) {
-- 
2.25.1

Re: [PATCH] drm/amdgpu: modify a pair of functions for the pcie port wreg/rreg

[Huang Rui]

On Tue, Jan 18, 2022 at 07:26:18PM +0800, Du, Xiaojian wrote:
> This patch will modify a pair of functions for pcie port wreg/rreg.
> AMD GPU have had an independent NBIO block from SOC15 arch.
> If the dirver wants to read/write the address space of the pcie devices,
> it has to go through the NBIO block.
> This patch will move the pcie port wreg/rreg functions to
> "amdgpu_device.c", so that to make the functions can be used on the
> future GPU ASICs.
> 
> Signed-off-by: Xiaojian Du 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +
>  drivers/gpu/drm/amd/amdgpu/nv.c| 34 ++
>  3 files changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index b2da840f4718..691d7868d64d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1421,6 +1421,10 @@ void amdgpu_device_invalidate_hdp(struct amdgpu_device 
> *adev,
>   struct amdgpu_ring *ring);
>  
>  void amdgpu_device_halt(struct amdgpu_device *adev);
> +u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
> + u32 reg);
> +void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
> + u32 reg, u32 v);
>  
>  /* atpx handler */
>  #if defined(CONFIG_VGA_SWITCHEROO)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ff4cf0e2a01f..10f2b7cbb49d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6023,3 +6023,36 @@ void amdgpu_device_halt(struct amdgpu_device *adev)
>   pci_disable_device(pdev);
>   pci_wait_for_pending_transaction(pdev);
>  }
> +
> +u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
> + u32 reg)
> +{
> + unsigned long flags, address, data;
> + u32 r;
> +
> + address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> + data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
> +
> + spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + WREG32(address, reg * 4);
> + (void)RREG32(address);
> + r = RREG32(data);
> + spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> + return r;
> +}
> +
> +void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
> + u32 reg, u32 v)
> +{
> + unsigned long flags, address, data;
> +
> + address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> + data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
> +
> + spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> + WREG32(address, reg * 4);
> + (void)RREG32(address);
> + WREG32(data, v);
> + (void)RREG32(data);
> + spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index e52d1114501c..17480c1eeae8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -256,21 +256,6 @@ static u64 nv_pcie_rreg64(struct amdgpu_device *adev, 
> u32 reg)
>   return amdgpu_device_indirect_rreg64(adev, address, data, reg);
>  }
>  
> -static u32 nv_pcie_port_rreg(struct amdgpu_device *adev, u32 reg)
> -{
> - unsigned long flags, address, data;
> - u32 r;
> - address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> - data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
> -
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> - WREG32(address, reg * 4);
> - (void)RREG32(address);
> - r = RREG32(data);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> - return r;
> -}
> -
>  static void nv_pcie_wreg64(struct amdgpu_device *adev, u32 reg, u64 v)
>  {
>   unsigned long address, data;
> @@ -281,21 +266,6 @@ static void nv_pcie_wreg64(struct amdgpu_device *adev, 
> u32 reg, u64 v)
>   amdgpu_device_indirect_wreg64(adev, address, data, reg, v);
>  }
>  
> -static void nv_pcie_port_wreg(struct amdgpu_device *adev, u32 reg, u32 v)
> -{
> - unsigned long flags, address, data;
> -
> - address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
> - data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
> -
> - spin_lock_irqsave(&adev->pcie_idx_lock, flags);
> - WREG32(address, reg * 4);
> - (void)RREG32(address);
> - WREG32(data, v);
> - (void)RREG32(data);
> - spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> -}
> -
>  static u32 nv_didt_rreg(struct amdgpu_device *adev, u32 reg)
>  {
>   unsigned long flags, address, data;
> @@ -709,8 +679,8 @@ static int nv_common_early_init(void *handle)
>   adev->pcie_wreg = &nv_pcie_wreg;
>   adev->pcie_rreg64 = &nv_pcie_rreg64;
>   adev->pcie_wreg64 = &nv_pcie_wreg64;
> - a

RE: [PATCH] drm/amdgpu: remove gart.ready flag

[Chen, Guchun]

[Public]

Thanks for the clarification. The patch is:
Reviewed-by: Guchun Chen 

Regards,
Guchun

-Original Message-
From: Christian König  
Sent: Tuesday, January 18, 2022 10:10 PM
To: Chen, Guchun ; Kim, Jonathan ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: remove gart.ready flag

Am 18.01.22 um 14:28 schrieb Chen, Guchun:
> [Public]
>
> - if (amdgpu_sriov_vf(adev) && amdgpu_in_reset(adev))
> - goto skip_pin_bo;
> -
> - r = amdgpu_gtt_mgr_recover(&adev->mman.gtt_mgr);
> - if (r)
> - return r;
> -
> -skip_pin_bo:
>
> Does deleting skip_pin_bo path cause bo redundant pin in SRIOV case?

Pinning/unpinning the BO was already removed as well.

See Nirmoy's patches in the git log.

Regards,
Christian.

>
> Regards,
> Guchun
>
> -Original Message-
> From: Christian König 
> Sent: Tuesday, January 18, 2022 8:02 PM
> To: Chen, Guchun ; Kim, Jonathan 
> ; amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: remove gart.ready flag
>
> That's just a leftover from old radeon days and was preventing CS and GART 
> bindings before the hardware was initialized. But nowdays that is perfectly 
> valid.
>
> The only thing we need to warn about are GART binding before the table is 
> even allocated.
>
> Signed-off-by: Christian König 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c| 35 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h| 15 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 77 ++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 11 +--
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  7 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |  8 +--
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  8 +--
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 10 +--
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c|  5 +-
>   11 files changed, 52 insertions(+), 137 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 645950a653a0..53cc844346f0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -150,7 +150,7 @@ void amdgpu_gart_table_vram_free(struct amdgpu_device 
> *adev)
>* replaces them with the dummy page (all asics).
>* Returns 0 for success, -EINVAL for failure.
>*/
> -int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
> +void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>   int pages)
>   {
>   unsigned t;
> @@ -161,13 +161,11 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, 
> uint64_t offset,
>   uint64_t flags = 0;
>   int idx;
>   
> - if (!adev->gart.ready) {
> - WARN(1, "trying to unbind memory from uninitialized GART !\n");
> - return -EINVAL;
> - }
> + if (WARN_ON(!adev->gart.ptr))
> + return;
>   
>   if (!drm_dev_enter(adev_to_drm(adev), &idx))
> - return 0;
> + return;
>   
>   t = offset / AMDGPU_GPU_PAGE_SIZE;
>   p = t / AMDGPU_GPU_PAGES_IN_CPU_PAGE; @@ -188,7 +186,6 @@ int 
> amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>   amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
>   
>   drm_dev_exit(idx);
> - return 0;
>   }
>   
>   /**
> @@ -204,7 +201,7 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, 
> uint64_t offset,
>* Map the dma_addresses into GART entries (all asics).
>* Returns 0 for success, -EINVAL for failure.
>*/
> -int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
> +void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
>   int pages, dma_addr_t *dma_addr, uint64_t flags,
>   void *dst)
>   {
> @@ -212,13 +209,8 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
> offset,
>   unsigned i, j, t;
>   int idx;
>   
> - if (!adev->gart.ready) {
> - WARN(1, "trying to bind memory to uninitialized GART !\n");
> - return -EINVAL;
> - }
> -
>   if (!drm_dev_enter(adev_to_drm(adev), &idx))
> - return 0;
> + return;
>   
>   t = offset / AMDGPU_GPU_PAGE_SIZE;
>   
> @@ -230,7 +222,6 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
> offset,
>   }
>   }
>   drm_dev_exit(idx);
> - return 0;
>   }
>   
>   /**
> @@ -246,20 +237,14 @@ int amdgpu_gart_map(struct amdgpu_device *adev, 
> uint64_t offset,
>* (all asics).
>* Returns 0 for success, -EINVAL for failure.
>*/
> -int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
> +void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
>int pages, dma_addr_t *dma_addr,
>uint64_t flags)
>   {
> - if (!adev->gart.ready) {
> -  

Re: [RFC PATCH v3 2/3] drm: add support modifiers for drivers whose planes only support linear layout

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 02:15:48PM +0900, Esaki Tomohito wrote:
> On 2022/01/14 23:16, Andy Shevchenko wrote:
> > On Fri, Jan 14, 2022 at 07:17:52PM +0900, Tomohito Esaki wrote:
> > > The LINEAR modifier is advertised as default if a driver doesn't specify
> > > modifiers.
> > 
> > ...
> > 
> > > + const uint64_t default_modifiers[] = {
> > > + DRM_FORMAT_MOD_LINEAR,
> > > + DRM_FORMAT_MOD_INVALID
> > 
> > + Comma?
> 
> There is no mention in the coding style about adding/removing a comma to the
> last element of an array. Is there a policy in drm driver?
> 
> I think the advantage of adding a comma to the last element of an array is
> that diff is only one line when an element is added to the end.
> However since INVALID is always the last element in the modifiers array, I
> think it can be either in this case.
> If there is a policy, I will match it.

Indeed, but there is a common sense. The idea behind (multi-line) definitions
that when next time somebody will add an element in the array, there are will
be:

a) no additional churn (like in case of this patch, if the item will be added
   at the bottom;

b) an element that may not be added behind the terminator, which will look
   weird.

That said, the question is if the element is terminator one or not, if not,
comma is better than no comma and vise versa.

-- 
With Best Regards,
Andy Shevchenko

[PATCH] amdgpu/amdgpu_psp: remove unneeded ret variable

[cgel . zte]

From: Minghao Chi 

Return value from amdgpu_bo_create_kernel() directly instead
of taking this in another redundant variable.

Reported-by: Zeal Robot 
Signed-off-by: Minghao Chi 
Signed-off-by: CGEL ZTE 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index dee17a0e1187..ac2b87f81ef9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -914,19 +914,15 @@ static void psp_prep_ta_load_cmd_buf(struct 
psp_gfx_cmd_resp *cmd,
 static int psp_ta_init_shared_buf(struct psp_context *psp,
  struct ta_mem_context *mem_ctx)
 {
-   int ret;
-
/*
* Allocate 16k memory aligned to 4k from Frame Buffer (local
* physical) for ta to host memory
*/
-   ret = amdgpu_bo_create_kernel(psp->adev, mem_ctx->shared_mem_size,
+   return amdgpu_bo_create_kernel(psp->adev, mem_ctx->shared_mem_size,
  PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
  &mem_ctx->shared_bo,
  &mem_ctx->shared_mc_addr,
  &mem_ctx->shared_buf);
-
-   return ret;
 }
 
 static void psp_ta_free_shared_buf(struct ta_mem_context *mem_ctx)
-- 
2.25.1

Re: [RFC PATCH v4 0/3] Add support modifiers for drivers whose planes only support linear layout

[Andy Shevchenko]

On Tue, Jan 18, 2022 at 05:36:49PM +0900, Tomohito Esaki wrote:
> Some drivers whose planes only support linear layout fb do not support format
> modifiers.
> These drivers should support modifiers, however the DRM core should handle 
> this
> rather than open-coding in every driver.
> 
> In this patch series, these drivers expose format modifiers based on the
> following suggestion[1].
> 
> On Thu, Nov 18, 2021 at 01:02:11PM +, Daniel Stone wrote:
> > I think the best way forward here is:
> >   - add a new mode_config.cannot_support_modifiers flag, and enable
> > this in radeon (plus any other drivers in the same boat)
> >   - change drm_universal_plane_init() to advertise the LINEAR modifier
> > when NULL is passed as the modifier list (including installing a
> > default .format_mod_supported hook)
> >   - remove the mode_config.allow_fb_modifiers hook and always
> > advertise modifier support, unless
> > mode_config.cannot_support_modifiers is set

FWIW,
Reviewed-by: Andy Shevchenko 

> [1] 
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20190509054518.10781-1-e...@igel.co.jp/#24602575
> 
> v4:
> * modify documentation for fb_modifiers_not_supported flag in kerneldoc
> 
> v3: https://www.spinics.net/lists/dri-devel/msg329102.html
> * change the order as follows:
>1. add fb_modifiers_not_supported flag
>2. add default modifiers
>3. remove allow_fb_modifiers flag
> * add a conditional disable in amdgpu_dm_plane_init()
> 
> v2: https://www.spinics.net/lists/dri-devel/msg328939.html
> * rebase to the latest master branch (5.16.0+)
>   + "drm/plane: Make format_mod_supported truly optional" patch [2]
>   [2] https://patchwork.freedesktop.org/patch/467940/?series=98255&rev=3
> 
> v1: https://www.spinics.net/lists/dri-devel/msg327352.html
> * The initial patch set
> 
> Tomohito Esaki (3):
>   drm: introduce fb_modifiers_not_supported flag in mode_config
>   drm: add support modifiers for drivers whose planes only support
> linear layout
>   drm: remove allow_fb_modifiers
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  6 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +++
>  drivers/gpu/drm/drm_framebuffer.c |  6 ++---
>  drivers/gpu/drm/drm_ioctl.c   |  2 +-
>  drivers/gpu/drm/drm_plane.c   | 22 +--
>  drivers/gpu/drm/nouveau/nouveau_display.c |  6 +++--
>  drivers/gpu/drm/radeon/radeon_display.c   |  2 ++
>  .../gpu/drm/selftests/test-drm_framebuffer.c  |  1 -
>  include/drm/drm_mode_config.h | 18 +--
>  include/drm/drm_plane.h   |  3 +++
>  14 files changed, 43 insertions(+), 33 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] drm/amdgpu: remove gart.ready flag

[Christian König]

Am 18.01.22 um 14:28 schrieb Chen, Guchun:

[Public]

-   if (amdgpu_sriov_vf(adev) && amdgpu_in_reset(adev))
-   goto skip_pin_bo;
-
-   r = amdgpu_gtt_mgr_recover(&adev->mman.gtt_mgr);
-   if (r)
-   return r;
-
-skip_pin_bo:

Does deleting skip_pin_bo path cause bo redundant pin in SRIOV case?


Pinning/unpinning the BO was already removed as well.

See Nirmoy's patches in the git log.

Regards,
Christian.



Regards,
Guchun

-Original Message-
From: Christian König 
Sent: Tuesday, January 18, 2022 8:02 PM
To: Chen, Guchun ; Kim, Jonathan ; 
amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: remove gart.ready flag

That's just a leftover from old radeon days and was preventing CS and GART 
bindings before the hardware was initialized. But nowdays that is perfectly 
valid.

The only thing we need to warn about are GART binding before the table is even 
allocated.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c| 35 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h| 15 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 77 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 11 +--
  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  7 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |  8 +--
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  8 +--
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 10 +--
  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c|  5 +-
  11 files changed, 52 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 645950a653a0..53cc844346f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -150,7 +150,7 @@ void amdgpu_gart_table_vram_free(struct amdgpu_device *adev)
   * replaces them with the dummy page (all asics).
   * Returns 0 for success, -EINVAL for failure.
   */
-int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
int pages)
  {
unsigned t;
@@ -161,13 +161,11 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, 
uint64_t offset,
uint64_t flags = 0;
int idx;
  
-	if (!adev->gart.ready) {

-   WARN(1, "trying to unbind memory from uninitialized GART !\n");
-   return -EINVAL;
-   }
+   if (WARN_ON(!adev->gart.ptr))
+   return;
  
  	if (!drm_dev_enter(adev_to_drm(adev), &idx))

-   return 0;
+   return;
  
  	t = offset / AMDGPU_GPU_PAGE_SIZE;

p = t / AMDGPU_GPU_PAGES_IN_CPU_PAGE;
@@ -188,7 +186,6 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
  
  	drm_dev_exit(idx);

-   return 0;
  }
  
  /**

@@ -204,7 +201,7 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
   * Map the dma_addresses into GART entries (all asics).
   * Returns 0 for success, -EINVAL for failure.
   */
-int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
int pages, dma_addr_t *dma_addr, uint64_t flags,
void *dst)
  {
@@ -212,13 +209,8 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
unsigned i, j, t;
int idx;
  
-	if (!adev->gart.ready) {

-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
if (!drm_dev_enter(adev_to_drm(adev), &idx))
-   return 0;
+   return;
  
  	t = offset / AMDGPU_GPU_PAGE_SIZE;
  
@@ -230,7 +222,6 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,

}
}
drm_dev_exit(idx);
-   return 0;
  }
  
  /**

@@ -246,20 +237,14 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
   * (all asics).
   * Returns 0 for success, -EINVAL for failure.
   */
-int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 int pages, dma_addr_t *dma_addr,
 uint64_t flags)
  {
-   if (!adev->gart.ready) {
-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
-   if (!adev->gart.ptr)
-   return 0;
+   if (WARN_ON(!adev->gart.ptr))
+   return;
  
-	return amdgpu_gart_map(adev, offset, pages, dma_addr, flags,

-  adev->gart.ptr);
+   amdgpu_gart_map(adev, offset, pages, dma_addr, flags, adev->gart.ptr);
  }
  
  /**

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h 
b/drivers/gpu/drm/am

Re: [PATCH] drm/amdgpu: Add missing pm_runtime_put_autosuspend

[Lazar, Lijo]

On 1/18/2022 5:31 PM, Yongzhi Liu wrote:

pm_runtime_get_sync() increments the runtime PM usage counter even
when it returns an error code, thus a matching decrement is needed
on the error handling path to keep the counter balanced.

Signed-off-by: Yongzhi Liu 


Thanks!

Reviewed-by: Lijo Lazar 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 9aea1cc..4b950de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1120,8 +1120,10 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct file 
*f, char __user *buf,
return -EINVAL;
  
  	r = pm_runtime_get_sync(adev_to_drm(adev)->dev);

-   if (r < 0)
+   if (r < 0) {
+   pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
return r;
+   }
  
  	while (size) {

uint32_t value;

Re: [PATCH] drm/amdgpu: modify a pair of functions for the pcie port wreg/rreg

[Lazar, Lijo]

On 1/18/2022 4:56 PM, Xiaojian Du wrote:

This patch will modify a pair of functions for pcie port wreg/rreg.
AMD GPU have had an independent NBIO block from SOC15 arch.
If the dirver wants to read/write the address space of the pcie devices,
it has to go through the NBIO block.
This patch will move the pcie port wreg/rreg functions to
"amdgpu_device.c", so that to make the functions can be used on the
future GPU ASICs.

Signed-off-by: Xiaojian Du 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +
  drivers/gpu/drm/amd/amdgpu/nv.c| 34 ++
  3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b2da840f4718..691d7868d64d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1421,6 +1421,10 @@ void amdgpu_device_invalidate_hdp(struct amdgpu_device 
*adev,
struct amdgpu_ring *ring);
  
  void amdgpu_device_halt(struct amdgpu_device *adev);

+u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
+   u32 reg);
+void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
+   u32 reg, u32 v);
  
  /* atpx handler */

  #if defined(CONFIG_VGA_SWITCHEROO)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ff4cf0e2a01f..10f2b7cbb49d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6023,3 +6023,36 @@ void amdgpu_device_halt(struct amdgpu_device *adev)
pci_disable_device(pdev);
pci_wait_for_pending_transaction(pdev);
  }
+
+u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
+   u32 reg)
+{
+   unsigned long flags, address, data;
+   u32 r;
+
+   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
+   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
+
+   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+   WREG32(address, reg * 4);
+   (void)RREG32(address);
+   r = RREG32(data);
+   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+   return r;
+}
+
+void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
+   u32 reg, u32 v)
+{
+   unsigned long flags, address, data;
+
+   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
+   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
+
+   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+   WREG32(address, reg * 4);
+   (void)RREG32(address);
+   WREG32(data, v);
+   (void)RREG32(data);
+   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index e52d1114501c..17480c1eeae8 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -256,21 +256,6 @@ static u64 nv_pcie_rreg64(struct amdgpu_device *adev, u32 
reg)
return amdgpu_device_indirect_rreg64(adev, address, data, reg);
  }
  
-static u32 nv_pcie_port_rreg(struct amdgpu_device *adev, u32 reg)

-{
-   unsigned long flags, address, data;
-   u32 r;
-   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
-   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
-
-   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
-   WREG32(address, reg * 4);
-   (void)RREG32(address);
-   r = RREG32(data);
-   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-   return r;
-}
-
  static void nv_pcie_wreg64(struct amdgpu_device *adev, u32 reg, u64 v)
  {
unsigned long address, data;
@@ -281,21 +266,6 @@ static void nv_pcie_wreg64(struct amdgpu_device *adev, u32 
reg, u64 v)
amdgpu_device_indirect_wreg64(adev, address, data, reg, v);
  }
  
-static void nv_pcie_port_wreg(struct amdgpu_device *adev, u32 reg, u32 v)

-{
-   unsigned long flags, address, data;
-
-   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
-   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
-
-   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
-   WREG32(address, reg * 4);
-   (void)RREG32(address);
-   WREG32(data, v);
-   (void)RREG32(data);
-   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-}
-
  static u32 nv_didt_rreg(struct amdgpu_device *adev, u32 reg)
  {
unsigned long flags, address, data;
@@ -709,8 +679,8 @@ static int nv_common_early_init(void *handle)
adev->pcie_wreg = &nv_pcie_wreg;
adev->pcie_rreg64 = &nv_pcie_rreg64;
adev->pcie_wreg64 = &nv_pcie_wreg64;


Looks good, would be better if the above ones also are changed to common 
implementation.


Thanks,
Lijo


-   adev->pciep_rreg = &nv_pcie_port_rreg;
-   adev->pciep_wreg 

RE: [PATCH] drm/amdgpu: remove gart.ready flag

[Chen, Guchun]

[Public]

-   if (amdgpu_sriov_vf(adev) && amdgpu_in_reset(adev))
-   goto skip_pin_bo;
-
-   r = amdgpu_gtt_mgr_recover(&adev->mman.gtt_mgr);
-   if (r)
-   return r;
-
-skip_pin_bo:

Does deleting skip_pin_bo path cause bo redundant pin in SRIOV case?

Regards,
Guchun

-Original Message-
From: Christian König  
Sent: Tuesday, January 18, 2022 8:02 PM
To: Chen, Guchun ; Kim, Jonathan ; 
amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: remove gart.ready flag

That's just a leftover from old radeon days and was preventing CS and GART 
bindings before the hardware was initialized. But nowdays that is perfectly 
valid.

The only thing we need to warn about are GART binding before the table is even 
allocated.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c| 35 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h| 15 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 77 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 11 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 10 +--
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c|  5 +-
 11 files changed, 52 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 645950a653a0..53cc844346f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -150,7 +150,7 @@ void amdgpu_gart_table_vram_free(struct amdgpu_device *adev)
  * replaces them with the dummy page (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
int pages)
 {
unsigned t;
@@ -161,13 +161,11 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, 
uint64_t offset,
uint64_t flags = 0;
int idx;
 
-   if (!adev->gart.ready) {
-   WARN(1, "trying to unbind memory from uninitialized GART !\n");
-   return -EINVAL;
-   }
+   if (WARN_ON(!adev->gart.ptr))
+   return;
 
if (!drm_dev_enter(adev_to_drm(adev), &idx))
-   return 0;
+   return;
 
t = offset / AMDGPU_GPU_PAGE_SIZE;
p = t / AMDGPU_GPU_PAGES_IN_CPU_PAGE;
@@ -188,7 +186,6 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
 
drm_dev_exit(idx);
-   return 0;
 }
 
 /**
@@ -204,7 +201,7 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
  * Map the dma_addresses into GART entries (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
int pages, dma_addr_t *dma_addr, uint64_t flags,
void *dst)
 {
@@ -212,13 +209,8 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
unsigned i, j, t;
int idx;
 
-   if (!adev->gart.ready) {
-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
if (!drm_dev_enter(adev_to_drm(adev), &idx))
-   return 0;
+   return;
 
t = offset / AMDGPU_GPU_PAGE_SIZE;
 
@@ -230,7 +222,6 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
}
}
drm_dev_exit(idx);
-   return 0;
 }
 
 /**
@@ -246,20 +237,14 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
  * (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 int pages, dma_addr_t *dma_addr,
 uint64_t flags)
 {
-   if (!adev->gart.ready) {
-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
-   if (!adev->gart.ptr)
-   return 0;
+   if (WARN_ON(!adev->gart.ptr))
+   return;
 
-   return amdgpu_gart_map(adev, offset, pages, dma_addr, flags,
-  adev->gart.ptr);
+   amdgpu_gart_map(adev, offset, pages, dma_addr, flags, adev->gart.ptr);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
index 78895413cf9f..8fea3e04e411 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
@@ -46,7 +46,6 @

[PATCH] drm/amdgpu: remove gart.ready flag

[Christian König]

That's just a leftover from old radeon days and was preventing CS and GART
bindings before the hardware was initialized. But nowdays that is
perfectly valid.

The only thing we need to warn about are GART binding before the table
is even allocated.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c| 35 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h| 15 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  9 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 77 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 11 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 10 +--
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c|  5 +-
 11 files changed, 52 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 645950a653a0..53cc844346f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -150,7 +150,7 @@ void amdgpu_gart_table_vram_free(struct amdgpu_device *adev)
  * replaces them with the dummy page (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
int pages)
 {
unsigned t;
@@ -161,13 +161,11 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, 
uint64_t offset,
uint64_t flags = 0;
int idx;
 
-   if (!adev->gart.ready) {
-   WARN(1, "trying to unbind memory from uninitialized GART !\n");
-   return -EINVAL;
-   }
+   if (WARN_ON(!adev->gart.ptr))
+   return;
 
if (!drm_dev_enter(adev_to_drm(adev), &idx))
-   return 0;
+   return;
 
t = offset / AMDGPU_GPU_PAGE_SIZE;
p = t / AMDGPU_GPU_PAGES_IN_CPU_PAGE;
@@ -188,7 +186,6 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
 
drm_dev_exit(idx);
-   return 0;
 }
 
 /**
@@ -204,7 +201,7 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t 
offset,
  * Map the dma_addresses into GART entries (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_map(struct amdgpu_device *adev, uint64_t offset,
int pages, dma_addr_t *dma_addr, uint64_t flags,
void *dst)
 {
@@ -212,13 +209,8 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
unsigned i, j, t;
int idx;
 
-   if (!adev->gart.ready) {
-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
if (!drm_dev_enter(adev_to_drm(adev), &idx))
-   return 0;
+   return;
 
t = offset / AMDGPU_GPU_PAGE_SIZE;
 
@@ -230,7 +222,6 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
}
}
drm_dev_exit(idx);
-   return 0;
 }
 
 /**
@@ -246,20 +237,14 @@ int amdgpu_gart_map(struct amdgpu_device *adev, uint64_t 
offset,
  * (all asics).
  * Returns 0 for success, -EINVAL for failure.
  */
-int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
+void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 int pages, dma_addr_t *dma_addr,
 uint64_t flags)
 {
-   if (!adev->gart.ready) {
-   WARN(1, "trying to bind memory to uninitialized GART !\n");
-   return -EINVAL;
-   }
-
-   if (!adev->gart.ptr)
-   return 0;
+   if (WARN_ON(!adev->gart.ptr))
+   return;
 
-   return amdgpu_gart_map(adev, offset, pages, dma_addr, flags,
-  adev->gart.ptr);
+   amdgpu_gart_map(adev, offset, pages, dma_addr, flags, adev->gart.ptr);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
index 78895413cf9f..8fea3e04e411 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
@@ -46,7 +46,6 @@ struct amdgpu_gart {
unsignednum_gpu_pages;
unsignednum_cpu_pages;
unsignedtable_size;
-   boolready;
 
/* Asic default pte flags */
uint64_tgart_pte_flags;
@@ -58,12 +57,12 @@ int amdgpu_gart_table_vram_pin(struct amdgpu_device *adev);
 void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev);
 int amdgpu_gart_init(struct amdgpu_device *adev);

[PATCH] drm/amdgpu: modify a pair of functions for the pcie port wreg/rreg

[Xiaojian Du]

This patch will modify a pair of functions for pcie port wreg/rreg.
AMD GPU have had an independent NBIO block from SOC15 arch.
If the dirver wants to read/write the address space of the pcie devices,
it has to go through the NBIO block.
This patch will move the pcie port wreg/rreg functions to
"amdgpu_device.c", so that to make the functions can be used on the
future GPU ASICs.

Signed-off-by: Xiaojian Du 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +
 drivers/gpu/drm/amd/amdgpu/nv.c| 34 ++
 3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b2da840f4718..691d7868d64d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1421,6 +1421,10 @@ void amdgpu_device_invalidate_hdp(struct amdgpu_device 
*adev,
struct amdgpu_ring *ring);
 
 void amdgpu_device_halt(struct amdgpu_device *adev);
+u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
+   u32 reg);
+void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
+   u32 reg, u32 v);
 
 /* atpx handler */
 #if defined(CONFIG_VGA_SWITCHEROO)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ff4cf0e2a01f..10f2b7cbb49d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6023,3 +6023,36 @@ void amdgpu_device_halt(struct amdgpu_device *adev)
pci_disable_device(pdev);
pci_wait_for_pending_transaction(pdev);
 }
+
+u32 amdgpu_device_pcie_port_rreg(struct amdgpu_device *adev,
+   u32 reg)
+{
+   unsigned long flags, address, data;
+   u32 r;
+
+   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
+   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
+
+   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+   WREG32(address, reg * 4);
+   (void)RREG32(address);
+   r = RREG32(data);
+   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+   return r;
+}
+
+void amdgpu_device_pcie_port_wreg(struct amdgpu_device *adev,
+   u32 reg, u32 v)
+{
+   unsigned long flags, address, data;
+
+   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
+   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
+
+   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
+   WREG32(address, reg * 4);
+   (void)RREG32(address);
+   WREG32(data, v);
+   (void)RREG32(data);
+   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index e52d1114501c..17480c1eeae8 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -256,21 +256,6 @@ static u64 nv_pcie_rreg64(struct amdgpu_device *adev, u32 
reg)
return amdgpu_device_indirect_rreg64(adev, address, data, reg);
 }
 
-static u32 nv_pcie_port_rreg(struct amdgpu_device *adev, u32 reg)
-{
-   unsigned long flags, address, data;
-   u32 r;
-   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
-   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
-
-   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
-   WREG32(address, reg * 4);
-   (void)RREG32(address);
-   r = RREG32(data);
-   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-   return r;
-}
-
 static void nv_pcie_wreg64(struct amdgpu_device *adev, u32 reg, u64 v)
 {
unsigned long address, data;
@@ -281,21 +266,6 @@ static void nv_pcie_wreg64(struct amdgpu_device *adev, u32 
reg, u64 v)
amdgpu_device_indirect_wreg64(adev, address, data, reg, v);
 }
 
-static void nv_pcie_port_wreg(struct amdgpu_device *adev, u32 reg, u32 v)
-{
-   unsigned long flags, address, data;
-
-   address = adev->nbio.funcs->get_pcie_port_index_offset(adev);
-   data = adev->nbio.funcs->get_pcie_port_data_offset(adev);
-
-   spin_lock_irqsave(&adev->pcie_idx_lock, flags);
-   WREG32(address, reg * 4);
-   (void)RREG32(address);
-   WREG32(data, v);
-   (void)RREG32(data);
-   spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-}
-
 static u32 nv_didt_rreg(struct amdgpu_device *adev, u32 reg)
 {
unsigned long flags, address, data;
@@ -709,8 +679,8 @@ static int nv_common_early_init(void *handle)
adev->pcie_wreg = &nv_pcie_wreg;
adev->pcie_rreg64 = &nv_pcie_rreg64;
adev->pcie_wreg64 = &nv_pcie_wreg64;
-   adev->pciep_rreg = &nv_pcie_port_rreg;
-   adev->pciep_wreg = &nv_pcie_port_wreg;
+   adev->pciep_rreg = amdgpu_device_pcie_port_rreg;
+   adev->pciep_wreg = amdgpu_device_pcie_port_wreg;
 
/* TODO: will add them during V

Re: [PATCH 3/3] drm/amdgpu: add AMDGPURESET uevent on AMD GPU reset

[Christian König]

Am 18.01.22 um 11:40 schrieb S, Shirish:

Hi Shashank,


On 1/12/2022 6:30 PM, Sharma, Shashank wrote:



On 1/11/2022 12:26 PM, Christian König wrote:

Am 11.01.22 um 08:12 schrieb Somalapuram Amaranath:

AMDGPURESET uevent added to notify userspace,
collect dump_stack and amdgpu_reset_reg_dumps

Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/amd/amdgpu/nv.c | 31 +++
  1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
b/drivers/gpu/drm/amd/amdgpu/nv.c

index 2ec1ffb36b1f..41a2c37e825f 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -529,10 +529,41 @@ nv_asic_reset_method(struct amdgpu_device *adev)
  }
  }
+/**
+ * drm_sysfs_reset_event - generate a DRM uevent
+ * @dev: DRM device
+ *
+ * Send a uevent for the DRM device specified by @dev. Currently 
we only
+ * set AMDGPURESET=1 in the uevent environment, but this could be 
expanded to

+ * deal with other types of events.
+ *
+ * Any new uapi should be using the 
drm_sysfs_connector_status_event()

+ * for uevents on connector status change.
+ */
+void drm_sysfs_reset_event(struct drm_device *dev)
+{
+    char *event_string = "AMDGPURESET=1";
+    char *envp[2] = { event_string, NULL };
+
+ kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);


That won't work like this.

kobject_uevent_env() needs to allocate memory to send the event to 
userspace and that is not allowed while we do an reset. The Intel 
guys felt into the same trap already.


What we could maybe do is to teach kobject_uevent_env() gfp flags 
and make all allocations from the atomic pool.


Regards,
Christian.


Hi Amar,

I see another problem here,

We are sending the event at the GPU reset, but we are collecting the 
register values only when the corresponding userspace agent calls a 
read() on the respective sysfs entry.


Is the presumption here that gpu reset is always triggered within 
kernel & user space has to be made aware of it?


Yes, pretty much.



From what I know OS'/apps use GL extensions like robustness and other 
ways to detect hangs/gpu resets and flush out guilty contexts or take 
approp next steps.


BTW, is there any userspace infra already in place that have a 
task/thread listening  for reset events implemented, similar to hpd?


No, it's also completely different to HPD since we need to gather and 
save prerecorded data of the crash.




I believe there are several ways to make user space aware of reset via 
gpu_reset_counter etc, also if the objective is the have call trace 
upon reset or dump registers you can do it in the 
amdgpu_device_gpu_recover() but guard it with a proper CONFIG


that can be enabled in kernel's debug builds only, like tag along with 
KASAN etc.,


This way there will be lesser dependency on userspace.


That's a really bad idea. Gathering crash dump data should work on 
production kernels as well and is nothing we really need a compiler 
switch for.


Regards,
Christian.




Regards,

Shirish S



There is a very fair possibility that the register values are reset 
by the HW by then, and we are reading re-programmed values. At least 
there will be a race().


I think we should change this design in such a way:
1. Get into gpu_reset()
2. collect the register values and save this context into a separate 
file/node. Probably sending a trace_event here would be easiest way.

3. Send the drm event to the userspace client
4. The client reads from the trace file, and gets the data.

- Shashank




+}
+
+void amdgpu_reset_dumps(struct amdgpu_device *adev)
+{
+    struct drm_device *ddev = adev_to_drm(adev);
+    /* original raven doesn't have full asic reset */
+    if ((adev->apu_flags & AMD_APU_IS_RAVEN) &&
+    !(adev->apu_flags & AMD_APU_IS_RAVEN2))
+    return;
+    drm_sysfs_reset_event(ddev);
+    dump_stack();
+}
+
  static int nv_asic_reset(struct amdgpu_device *adev)
  {
  int ret = 0;
+    amdgpu_reset_dumps(adev);
  switch (nv_asic_reset_method(adev)) {
  case AMD_RESET_METHOD_PCI:
  dev_info(adev->dev, "PCI reset\n");

[PATCH v9 6/6] drm/amdgpu: add drm buddy support to amdgpu

[Arunpravin]

- Remove drm_mm references and replace with drm buddy functionalities
- Add res cursor support for drm buddy

v2(Matthew Auld):
  - replace spinlock with mutex as we call kmem_cache_zalloc
(..., GFP_KERNEL) in drm_buddy_alloc() function

  - lock drm_buddy_block_trim() function as it calls
mark_free/mark_split are all globally visible

v3(Matthew Auld):
  - remove trim method error handling as we address the failure case
at drm_buddy_block_trim() function

v4:
  - fix warnings reported by kernel test robot 

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/Kconfig   |   1 +
 .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h|  97 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h   |   7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  | 259 ++
 4 files changed, 231 insertions(+), 133 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cc3e979c9c9d..ff5628c477b4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -278,6 +278,7 @@ config DRM_AMDGPU
select HWMON
select BACKLIGHT_CLASS_DEVICE
select INTERVAL_TREE
+   select DRM_BUDDY
help
  Choose this option if you have a recent AMD Radeon graphics card.
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index acfa207cf970..da12b4ff2e45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -30,12 +30,15 @@
 #include 
 #include 
 
+#include "amdgpu_vram_mgr.h"
+
 /* state back for walking over vram_mgr and gtt_mgr allocations */
 struct amdgpu_res_cursor {
uint64_tstart;
uint64_tsize;
uint64_tremaining;
-   struct drm_mm_node  *node;
+   void*node;
+   uint32_tmem_type;
 };
 
 /**
@@ -52,27 +55,63 @@ static inline void amdgpu_res_first(struct ttm_resource 
*res,
uint64_t start, uint64_t size,
struct amdgpu_res_cursor *cur)
 {
+   struct drm_buddy_block *block;
+   struct list_head *head, *next;
struct drm_mm_node *node;
 
-   if (!res || res->mem_type == TTM_PL_SYSTEM) {
-   cur->start = start;
-   cur->size = size;
-   cur->remaining = size;
-   cur->node = NULL;
-   WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
-   return;
-   }
+   if (!res)
+   goto err_out;
 
BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
 
-   node = to_ttm_range_mgr_node(res)->mm_nodes;
-   while (start >= node->size << PAGE_SHIFT)
-   start -= node++->size << PAGE_SHIFT;
+   cur->mem_type = res->mem_type;
+
+   switch (cur->mem_type) {
+   case TTM_PL_VRAM:
+   head = &to_amdgpu_vram_mgr_node(res)->blocks;
+
+   block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+   if (!block)
+   goto err_out;
+
+   while (start >= amdgpu_node_size(block)) {
+   start -= amdgpu_node_size(block);
+
+   next = block->link.next;
+   if (next != head)
+   block = list_entry(next, struct 
drm_buddy_block, link);
+   }
+
+   cur->start = amdgpu_node_start(block) + start;
+   cur->size = min(amdgpu_node_size(block) - start, size);
+   cur->remaining = size;
+   cur->node = block;
+   break;
+   case TTM_PL_TT:
+   node = to_ttm_range_mgr_node(res)->mm_nodes;
+   while (start >= node->size << PAGE_SHIFT)
+   start -= node++->size << PAGE_SHIFT;
+
+   cur->start = (node->start << PAGE_SHIFT) + start;
+   cur->size = min((node->size << PAGE_SHIFT) - start, size);
+   cur->remaining = size;
+   cur->node = node;
+   break;
+   default:
+   goto err_out;
+   }
 
-   cur->start = (node->start << PAGE_SHIFT) + start;
-   cur->size = min((node->size << PAGE_SHIFT) - start, size);
+   return;
+
+err_out:
+   cur->start = start;
+   cur->size = size;
cur->remaining = size;
-   cur->node = node;
+   cur->node = NULL;
+   WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
+   return;
 }
 
 /**
@@ -85,7 +124,9 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
  */
 static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t 
size)
 {
-   struct drm_mm_node *node = cur->node;
+   struct drm_buddy_block *block;
+   struct drm_mm_node *node;
+  

[PATCH v9 4/6] drm: implement a method to free unused pages

[Arunpravin]

On contiguous allocation, we round up the size
to the *next* power of 2, implement a function
to free the unused pages after the newly allocate block.

v2(Matthew Auld):
  - replace function name 'drm_buddy_free_unused_pages' with
drm_buddy_block_trim
  - replace input argument name 'actual_size' with 'new_size'
  - add more validation checks for input arguments
  - add overlaps check to avoid needless searching and splitting
  - merged the below patch to see the feature in action
 - add free unused pages support to i915 driver
  - lock drm_buddy_block_trim() function as it calls mark_free/mark_split
are all globally visible

v3(Matthew Auld):
  - remove trim method error handling as we address the failure case
at drm_buddy_block_trim() function

v4:
  - in case of trim, at __alloc_range() split_block failure path
marks the block as free and removes it from the original list,
potentially also freeing it, to overcome this problem, we turn
the drm_buddy_block_trim() input node into a temporary node to
prevent recursively freeing itself, but still retain the
un-splitting/freeing of the other nodes(Matthew Auld)

  - modify the drm_buddy_block_trim() function return type

v5(Matthew Auld):
  - revert drm_buddy_block_trim() function return type changes in v4
  - modify drm_buddy_block_trim() passing argument n_pages to original_size
as n_pages has already been rounded up to the next power-of-two and
passing n_pages results noop

v6:
  - fix warnings reported by kernel test robot 

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/drm_buddy.c   | 65 +++
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 10 +++
 include/drm/drm_buddy.h   |  4 ++
 3 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 6aa5c1ce25bf..c5902a81b8c5 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -546,6 +546,71 @@ static int __drm_buddy_alloc_range(struct drm_buddy *mm,
return __alloc_range(mm, &dfs, start, size, blocks);
 }
 
+/**
+ * drm_buddy_block_trim - free unused pages
+ *
+ * @mm: DRM buddy manager
+ * @new_size: original size requested
+ * @blocks: output list head to add allocated blocks
+ *
+ * For contiguous allocation, we round up the size to the nearest
+ * power of two value, drivers consume *actual* size, so remaining
+ * portions are unused and it can be freed.
+ *
+ * Returns:
+ * 0 on success, error code on failure.
+ */
+int drm_buddy_block_trim(struct drm_buddy *mm,
+u64 new_size,
+struct list_head *blocks)
+{
+   struct drm_buddy_block *parent;
+   struct drm_buddy_block *block;
+   LIST_HEAD(dfs);
+   u64 new_start;
+   int err;
+
+   if (!list_is_singular(blocks))
+   return -EINVAL;
+
+   block = list_first_entry(blocks,
+struct drm_buddy_block,
+link);
+
+   if (!drm_buddy_block_is_allocated(block))
+   return -EINVAL;
+
+   if (new_size > drm_buddy_block_size(mm, block))
+   return -EINVAL;
+
+   if (!new_size && !IS_ALIGNED(new_size, mm->chunk_size))
+   return -EINVAL;
+
+   if (new_size == drm_buddy_block_size(mm, block))
+   return 0;
+
+   list_del(&block->link);
+   mark_free(mm, block);
+   mm->avail += drm_buddy_block_size(mm, block);
+
+   /* Prevent recursively freeing this node */
+   parent = block->parent;
+   block->parent = NULL;
+
+   new_start = drm_buddy_block_offset(block);
+   list_add(&block->tmp_link, &dfs);
+   err =  __alloc_range(mm, &dfs, new_start, new_size, blocks);
+   if (err) {
+   mark_allocated(block);
+   mm->avail -= drm_buddy_block_size(mm, block);
+   list_add(&block->link, blocks);
+   }
+
+   block->parent = parent;
+   return err;
+}
+EXPORT_SYMBOL(drm_buddy_block_trim);
+
 /**
  * drm_buddy_alloc_blocks - allocate power-of-two blocks
  *
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 3662434b64bb..53eb100688a6 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -97,6 +97,16 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
if (unlikely(err))
goto err_free_blocks;
 
+   if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+   u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
+
+   mutex_lock(&bman->lock);
+   drm_buddy_block_trim(mm,
+original_size,
+&bman_res->blocks);
+   mutex_unlock(&bman->lock);
+   }
+
*res = &bman_res->base;
return 0;
 
diff --git a/include/dr

[PATCH v9 5/6] drm/amdgpu: move vram inline functions into a header

[Arunpravin]

Move shared vram inline functions and structs
into a header file

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 51 
 1 file changed, 51 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
new file mode 100644
index ..59983464cce5
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright 2021 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __AMDGPU_VRAM_MGR_H__
+#define __AMDGPU_VRAM_MGR_H__
+
+#include 
+
+struct amdgpu_vram_mgr_node {
+   struct ttm_resource base;
+   struct list_head blocks;
+   unsigned long flags;
+};
+
+static inline u64 amdgpu_node_start(struct drm_buddy_block *block)
+{
+   return drm_buddy_block_offset(block);
+}
+
+static inline u64 amdgpu_node_size(struct drm_buddy_block *block)
+{
+   return PAGE_SIZE << drm_buddy_block_order(block);
+}
+
+static inline struct amdgpu_vram_mgr_node *
+to_amdgpu_vram_mgr_node(struct ttm_resource *res)
+{
+   return container_of(res, struct amdgpu_vram_mgr_node, base);
+}
+
+#endif
-- 
2.25.1

[PATCH v9 3/6] drm: implement top-down allocation method

[Arunpravin]

Implemented a function which walk through the order list,
compares the offset and returns the maximum offset block,
this method is unpredictable in obtaining the high range
address blocks which depends on allocation and deallocation.
for instance, if driver requests address at a low specific
range, allocator traverses from the root block and splits
the larger blocks until it reaches the specific block and
in the process of splitting, lower orders in the freelist
are occupied with low range address blocks and for the
subsequent TOPDOWN memory request we may return the low
range blocks.To overcome this issue, we may go with the
below approach.

The other approach, sorting each order list entries in
ascending order and compares the last entry of each
order list in the freelist and return the max block.
This creates sorting overhead on every drm_buddy_free()
request and split up of larger blocks for a single page
request.

v2:
  - Fix alignment issues(Matthew Auld)
  - Remove unnecessary list_empty check(Matthew Auld)
  - merged the below patch to see the feature in action
 - add top-down alloc support to i915 driver

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/drm_buddy.c   | 36 ---
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  3 ++
 include/drm/drm_buddy.h   |  1 +
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 954e31962c74..6aa5c1ce25bf 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -371,6 +371,26 @@ alloc_range_bias(struct drm_buddy *mm,
return ERR_PTR(err);
 }
 
+static struct drm_buddy_block *
+get_maxblock(struct list_head *head)
+{
+   struct drm_buddy_block *max_block = NULL, *node;
+
+   max_block = list_first_entry_or_null(head,
+struct drm_buddy_block,
+link);
+   if (!max_block)
+   return NULL;
+
+   list_for_each_entry(node, head, link) {
+   if (drm_buddy_block_offset(node) >
+   drm_buddy_block_offset(max_block))
+   max_block = node;
+   }
+
+   return max_block;
+}
+
 static struct drm_buddy_block *
 alloc_from_freelist(struct drm_buddy *mm,
unsigned int order,
@@ -381,11 +401,17 @@ alloc_from_freelist(struct drm_buddy *mm,
int err;
 
for (i = order; i <= mm->max_order; ++i) {
-   block = list_first_entry_or_null(&mm->free_list[i],
-struct drm_buddy_block,
-link);
-   if (block)
-   break;
+   if (flags & DRM_BUDDY_TOPDOWN_ALLOCATION) {
+   block = get_maxblock(&mm->free_list[i]);
+   if (block)
+   break;
+   } else {
+   block = list_first_entry_or_null(&mm->free_list[i],
+struct drm_buddy_block,
+link);
+   if (block)
+   break;
+   }
}
 
if (!block)
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 1411f4cf1f21..3662434b64bb 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -53,6 +53,9 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
INIT_LIST_HEAD(&bman_res->blocks);
bman_res->mm = mm;
 
+   if (place->flags & TTM_PL_FLAG_TOPDOWN)
+   bman_res->flags |= DRM_BUDDY_TOPDOWN_ALLOCATION;
+
if (place->fpfn || lpfn != man->size)
bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION;
 
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index 865664b90a8a..424fc443115e 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -28,6 +28,7 @@
 })
 
 #define DRM_BUDDY_RANGE_ALLOCATION (1 << 0)
+#define DRM_BUDDY_TOPDOWN_ALLOCATION (1 << 1)
 
 struct drm_buddy_block {
 #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
-- 
2.25.1

[PATCH v9 2/6] drm: improve drm_buddy_alloc function

[Arunpravin]

- Make drm_buddy_alloc a single function to handle
  range allocation and non-range allocation demands

- Implemented a new function alloc_range() which allocates
  the requested power-of-two block comply with range limitations

- Moved order computation and memory alignment logic from
  i915 driver to drm buddy

v2:
  merged below changes to keep the build unbroken
   - drm_buddy_alloc_range() becomes obsolete and may be removed
   - enable ttm range allocation (fpfn / lpfn) support in i915 driver
   - apply enhanced drm_buddy_alloc() function to i915 driver

v3(Matthew Auld):
  - Fix alignment issues and remove unnecessary list_empty check
  - add more validation checks for input arguments
  - make alloc_range() block allocations as bottom-up
  - optimize order computation logic
  - replace uint64_t with u64, which is preferred in the kernel

v4(Matthew Auld):
  - keep drm_buddy_alloc_range() function implementation for generic
actual range allocations
  - keep alloc_range() implementation for end bias allocations

v5(Matthew Auld):
  - modify drm_buddy_alloc() passing argument place->lpfn to lpfn
as place->lpfn will currently always be zero for i915

v6(Matthew Auld):
  - fixup potential uaf - If we are unlucky and can't allocate
enough memory when splitting blocks, where we temporarily
end up with the given block and its buddy on the respective
free list, then we need to ensure we delete both blocks,
and no just the buddy, before potentially freeing them

  - fix warnings reported by kernel test robot 

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/drm_buddy.c   | 326 +-
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  67 ++--
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |   2 +
 include/drm/drm_buddy.h   |  22 +-
 4 files changed, 293 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index d60878bc9c20..954e31962c74 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -282,23 +282,99 @@ void drm_buddy_free_list(struct drm_buddy *mm, struct 
list_head *objects)
 }
 EXPORT_SYMBOL(drm_buddy_free_list);
 
-/**
- * drm_buddy_alloc_blocks - allocate power-of-two blocks
- *
- * @mm: DRM buddy manager to allocate from
- * @order: size of the allocation
- *
- * The order value here translates to:
- *
- * 0 = 2^0 * mm->chunk_size
- * 1 = 2^1 * mm->chunk_size
- * 2 = 2^2 * mm->chunk_size
- *
- * Returns:
- * allocated ptr to the &drm_buddy_block on success
- */
-struct drm_buddy_block *
-drm_buddy_alloc_blocks(struct drm_buddy *mm, unsigned int order)
+static inline bool overlaps(u64 s1, u64 e1, u64 s2, u64 e2)
+{
+   return s1 <= e2 && e1 >= s2;
+}
+
+static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2)
+{
+   return s1 <= s2 && e1 >= e2;
+}
+
+static struct drm_buddy_block *
+alloc_range_bias(struct drm_buddy *mm,
+u64 start, u64 end,
+unsigned int order)
+{
+   struct drm_buddy_block *block;
+   struct drm_buddy_block *buddy;
+   LIST_HEAD(dfs);
+   int err;
+   int i;
+
+   end = end - 1;
+
+   for (i = 0; i < mm->n_roots; ++i)
+   list_add_tail(&mm->roots[i]->tmp_link, &dfs);
+
+   do {
+   u64 block_start;
+   u64 block_end;
+
+   block = list_first_entry_or_null(&dfs,
+struct drm_buddy_block,
+tmp_link);
+   if (!block)
+   break;
+
+   list_del(&block->tmp_link);
+
+   if (drm_buddy_block_order(block) < order)
+   continue;
+
+   block_start = drm_buddy_block_offset(block);
+   block_end = block_start + drm_buddy_block_size(mm, block) - 1;
+
+   if (!overlaps(start, end, block_start, block_end))
+   continue;
+
+   if (drm_buddy_block_is_allocated(block))
+   continue;
+
+   if (contains(start, end, block_start, block_end) &&
+   order == drm_buddy_block_order(block)) {
+   /*
+* Find the free block within the range.
+*/
+   if (drm_buddy_block_is_free(block))
+   return block;
+
+   continue;
+   }
+
+   if (!drm_buddy_block_is_split(block)) {
+   err = split_block(mm, block);
+   if (unlikely(err))
+   goto err_undo;
+   }
+
+   list_add(&block->right->tmp_link, &dfs);
+   list_add(&block->left->tmp_link, &dfs);
+   } while (1);
+
+   return ERR_PTR(-ENOSPC);
+
+err_undo:
+   /*
+* We really don't want to leave around a bunch of split blocks, since
+* 

[PATCH v9 1/6] drm: move the buddy allocator from i915 into common drm

[Arunpravin]

Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
  will be moved to drm selftest folder

cleanup i915 buddy references in i915 driver module
and replace with drm buddy

v2:
  - include header file in alphabetical order(Thomas)
  - merged changes listed in the body section into a single patch
to keep the build intact(Christian, Jani)

v3:
  - make drm buddy a separate module(Thomas, Christian)

v4:
  - Fix build error reported by kernel test robot 
  - removed i915 buddy selftest from i915_mock_selftests.h to
avoid build error
  - removed selftests/i915_buddy.c file as we create a new set of
buddy test cases in drm/selftests folder

v5:
  - Fix merge conflict issue

v6:
  - replace drm_buddy_mm structure name as drm_buddy(Thomas, Christian)
  - replace drm_buddy_alloc() function name as drm_buddy_alloc_blocks()
(Thomas)
  - replace drm_buddy_free() function name as drm_buddy_free_block()
(Thomas)
  - export drm_buddy_free_block() function
  - fix multiple instances of KMEM_CACHE() entry

v7:
  - fix warnings reported by kernel test robot 
  - modify the license(Christian)

v8:
  - fix warnings reported by kernel test robot 

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/Kconfig   |   6 +
 drivers/gpu/drm/Makefile  |   2 +
 drivers/gpu/drm/drm_buddy.c   | 535 
 drivers/gpu/drm/i915/Kconfig  |   1 +
 drivers/gpu/drm/i915/Makefile |   1 -
 drivers/gpu/drm/i915/i915_buddy.c | 466 ---
 drivers/gpu/drm/i915/i915_buddy.h | 143 
 drivers/gpu/drm/i915/i915_module.c|   3 -
 drivers/gpu/drm/i915/i915_scatterlist.c   |  11 +-
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  33 +-
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |   4 +-
 drivers/gpu/drm/i915/selftests/i915_buddy.c   | 787 --
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 -
 .../drm/i915/selftests/intel_memory_region.c  |  13 +-
 include/drm/drm_buddy.h   | 150 
 15 files changed, 725 insertions(+), 1431 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_buddy.c
 delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c
 delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h
 delete mode 100644 drivers/gpu/drm/i915/selftests/i915_buddy.c
 create mode 100644 include/drm/drm_buddy.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 91f54aeb0b7c..cc3e979c9c9d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -204,6 +204,12 @@ config DRM_TTM
  GPU memory types. Will be enabled automatically if a device driver
  uses it.
 
+config DRM_BUDDY
+   tristate
+   depends on DRM
+   help
+ A page based buddy allocator
+
 config DRM_VRAM_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 700abeb4945e..8675c2af7ae1 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -40,6 +40,8 @@ obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
 drm_shmem_helper-y := drm_gem_shmem_helper.o
 obj-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_shmem_helper.o
 
+obj-$(CONFIG_DRM_BUDDY) += drm_buddy.o
+
 drm_vram_helper-y := drm_gem_vram_helper.o
 obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
 
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
new file mode 100644
index ..d60878bc9c20
--- /dev/null
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -0,0 +1,535 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+static struct kmem_cache *slab_blocks;
+
+static struct drm_buddy_block *drm_block_alloc(struct drm_buddy *mm,
+  struct drm_buddy_block *parent,
+  unsigned int order,
+  u64 offset)
+{
+   struct drm_buddy_block *block;
+
+   BUG_ON(order > DRM_BUDDY_MAX_ORDER);
+
+   block = kmem_cache_zalloc(slab_blocks, GFP_KERNEL);
+   if (!block)
+   return NULL;
+
+   block->header = offset;
+   block->header |= order;
+   block->parent = parent;
+
+   BUG_ON(block->header & DRM_BUDDY_HEADER_UNUSED);
+   return block;
+}
+
+static void drm_block_free(struct drm_buddy *mm,
+  struct drm_buddy_block *block)
+{
+   kmem_cache_free(slab_blocks, block);
+}
+
+static void mark_allocated(struct drm_buddy_block *block)
+{
+  

Re: [PATCH 3/3] drm/amdgpu: add AMDGPURESET uevent on AMD GPU reset

[S, Shirish]

Hi Shashank,


On 1/12/2022 6:30 PM, Sharma, Shashank wrote:



On 1/11/2022 12:26 PM, Christian König wrote:

Am 11.01.22 um 08:12 schrieb Somalapuram Amaranath:

AMDGPURESET uevent added to notify userspace,
collect dump_stack and amdgpu_reset_reg_dumps

Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/amd/amdgpu/nv.c | 31 +++
  1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
b/drivers/gpu/drm/amd/amdgpu/nv.c

index 2ec1ffb36b1f..41a2c37e825f 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -529,10 +529,41 @@ nv_asic_reset_method(struct amdgpu_device *adev)
  }
  }
+/**
+ * drm_sysfs_reset_event - generate a DRM uevent
+ * @dev: DRM device
+ *
+ * Send a uevent for the DRM device specified by @dev. Currently we 
only
+ * set AMDGPURESET=1 in the uevent environment, but this could be 
expanded to

+ * deal with other types of events.
+ *
+ * Any new uapi should be using the drm_sysfs_connector_status_event()
+ * for uevents on connector status change.
+ */
+void drm_sysfs_reset_event(struct drm_device *dev)
+{
+    char *event_string = "AMDGPURESET=1";
+    char *envp[2] = { event_string, NULL };
+
+ kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);


That won't work like this.

kobject_uevent_env() needs to allocate memory to send the event to 
userspace and that is not allowed while we do an reset. The Intel 
guys felt into the same trap already.


What we could maybe do is to teach kobject_uevent_env() gfp flags and 
make all allocations from the atomic pool.


Regards,
Christian.


Hi Amar,

I see another problem here,

We are sending the event at the GPU reset, but we are collecting the 
register values only when the corresponding userspace agent calls a 
read() on the respective sysfs entry.


Is the presumption here that gpu reset is always triggered within kernel 
& user space has to be made aware of it?


From what I know OS'/apps use GL extensions like robustness and other 
ways to detect hangs/gpu resets and flush out guilty contexts or take 
approp next steps.


BTW, is there any userspace infra already in place that have a 
task/thread listening  for reset events implemented, similar to hpd?


I believe there are several ways to make user space aware of reset via 
gpu_reset_counter etc, also if the objective is the have call trace upon 
reset or dump registers you can do it in the amdgpu_device_gpu_recover() 
but guard it with a proper CONFIG


that can be enabled in kernel's debug builds only, like tag along with 
KASAN etc.,


This way there will be lesser dependency on userspace.


Regards,

Shirish S



There is a very fair possibility that the register values are reset by 
the HW by then, and we are reading re-programmed values. At least 
there will be a race().


I think we should change this design in such a way:
1. Get into gpu_reset()
2. collect the register values and save this context into a separate 
file/node. Probably sending a trace_event here would be easiest way.

3. Send the drm event to the userspace client
4. The client reads from the trace file, and gets the data.

- Shashank




+}
+
+void amdgpu_reset_dumps(struct amdgpu_device *adev)
+{
+    struct drm_device *ddev = adev_to_drm(adev);
+    /* original raven doesn't have full asic reset */
+    if ((adev->apu_flags & AMD_APU_IS_RAVEN) &&
+    !(adev->apu_flags & AMD_APU_IS_RAVEN2))
+    return;
+    drm_sysfs_reset_event(ddev);
+    dump_stack();
+}
+
  static int nv_asic_reset(struct amdgpu_device *adev)
  {
  int ret = 0;
+    amdgpu_reset_dumps(adev);
  switch (nv_asic_reset_method(adev)) {
  case AMD_RESET_METHOD_PCI:
  dev_info(adev->dev, "PCI reset\n");

[RFC PATCH v4 3/3] drm: remove allow_fb_modifiers

[Tomohito Esaki]

The allow_fb_modifiers flag is unnecessary since it has been replaced
with cannot_support_modifiers flag.

Signed-off-by: Tomohito Esaki 
---
 drivers/gpu/drm/drm_plane.c  |  9 -
 drivers/gpu/drm/selftests/test-drm_framebuffer.c |  1 -
 include/drm/drm_mode_config.h| 16 
 3 files changed, 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 5aa7e241971e..89a3d044ab59 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -288,15 +288,6 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
}
}
 
-   /* autoset the cap and check for consistency across all planes */
-   if (format_modifier_count) {
-   drm_WARN_ON(dev, !config->allow_fb_modifiers &&
-   !list_empty(&config->plane_list));
-   config->allow_fb_modifiers = true;
-   } else {
-   drm_WARN_ON(dev, config->allow_fb_modifiers);
-   }
-
plane->modifier_count = format_modifier_count;
plane->modifiers = kmalloc_array(format_modifier_count,
 sizeof(format_modifiers[0]),
diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c 
b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
index 61b44d3a6a61..f6d66285c5fc 100644
--- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
+++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
@@ -323,7 +323,6 @@ static struct drm_device mock_drm_device = {
.max_width = MAX_WIDTH,
.min_height = MIN_HEIGHT,
.max_height = MAX_HEIGHT,
-   .allow_fb_modifiers = true,
.funcs = &mock_config_funcs,
},
 };
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index b73af96e2c96..c41df5932a6b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -917,22 +917,6 @@ struct drm_mode_config {
 */
bool async_page_flip;
 
-   /**
-* @allow_fb_modifiers:
-*
-* Whether the driver supports fb modifiers in the ADDFB2.1 ioctl call.
-* Note that drivers should not set this directly, it is automatically
-* set in drm_universal_plane_init().
-*
-* IMPORTANT:
-*
-* If this is set the driver must fill out the full implicit modifier
-* information in their &drm_mode_config_funcs.fb_create hook for legacy
-* userspace which does not set modifiers. Otherwise the GETFB2 ioctl is
-* broken for modifier aware userspace.
-*/
-   bool allow_fb_modifiers;
-
/**
 * @fb_modifiers_not_supported:
 *
-- 
2.25.1

[RFC PATCH v4 2/3] drm: add support modifiers for drivers whose planes only support linear layout

[Tomohito Esaki]

The LINEAR modifier is advertised as default if a driver doesn't specify
modifiers.

Signed-off-by: Tomohito Esaki 
---
 drivers/gpu/drm/drm_plane.c | 15 ---
 include/drm/drm_plane.h |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index deeec60a3315..5aa7e241971e 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -237,6 +237,10 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
  const char *name, va_list ap)
 {
struct drm_mode_config *config = &dev->mode_config;
+   const uint64_t default_modifiers[] = {
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+   };
unsigned int format_modifier_count = 0;
int ret;
 
@@ -277,6 +281,11 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
 
while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
format_modifier_count++;
+   } else {
+   if (!dev->mode_config.fb_modifiers_not_supported) {
+   format_modifiers = default_modifiers;
+   format_modifier_count = 1;
+   }
}
 
/* autoset the cap and check for consistency across all planes */
@@ -341,7 +350,7 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
drm_object_attach_property(&plane->base, config->prop_src_h, 0);
}
 
-   if (config->allow_fb_modifiers)
+   if (format_modifier_count)
create_in_format_blob(dev, plane);
 
return 0;
@@ -368,8 +377,8 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
  * drm_universal_plane_init() to let the DRM managed resource infrastructure
  * take care of cleanup and deallocation.
  *
- * Drivers supporting modifiers must set @format_modifiers on all their planes,
- * even those that only support DRM_FORMAT_MOD_LINEAR.
+ * For drivers supporting modifiers, all planes will advertise
+ * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set.
  *
  * Returns:
  * Zero on success, error code on failure.
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 0c1102dc4d88..cad641b1f797 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -803,6 +803,9 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
  *
  * The @drm_plane_funcs.destroy hook must be NULL.
  *
+ * For drivers supporting modifiers, all planes will advertise
+ * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set.
+ *
  * Returns:
  * Pointer to new plane, or ERR_PTR on failure.
  */
-- 
2.25.1

[RFC PATCH v4 1/3] drm: introduce fb_modifiers_not_supported flag in mode_config

[Tomohito Esaki]

If only linear modifier is advertised, since there are many drivers that
only linear supported, the DRM core should handle this rather than
open-coding in every driver. However, there are legacy drivers such as
radeon that do not support modifiers but infer the actual layout of the
underlying buffer. Therefore, a new flag fb_modifiers_not_supported is
introduced for these legacy drivers, and allow_fb_modifiers is replaced
with this new flag.

Signed-off-by: Tomohito Esaki 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  6 +++---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 ++
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 ++
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  1 +
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 ++
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +++
 drivers/gpu/drm/drm_framebuffer.c |  6 +++---
 drivers/gpu/drm/drm_ioctl.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_display.c |  6 --
 drivers/gpu/drm/radeon/radeon_display.c   |  2 ++
 include/drm/drm_mode_config.h | 10 ++
 11 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 82011e75ed85..edbb30d47b8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -954,7 +954,7 @@ static int amdgpu_display_verify_sizes(struct 
amdgpu_framebuffer *rfb)
int ret;
unsigned int i, block_width, block_height, block_size_log2;
 
-   if (!rfb->base.dev->mode_config.allow_fb_modifiers)
+   if (rfb->base.dev->mode_config.fb_modifiers_not_supported)
return 0;
 
for (i = 0; i < format_info->num_planes; ++i) {
@@ -1141,7 +1141,7 @@ int amdgpu_display_framebuffer_init(struct drm_device 
*dev,
if (ret)
return ret;
 
-   if (!dev->mode_config.allow_fb_modifiers) {
+   if (dev->mode_config.fb_modifiers_not_supported) {
drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI,
  "GFX9+ requires FB check based on format 
modifier\n");
ret = check_tiling_flags_gfx6(rfb);
@@ -1149,7 +1149,7 @@ int amdgpu_display_framebuffer_init(struct drm_device 
*dev,
return ret;
}
 
-   if (dev->mode_config.allow_fb_modifiers &&
+   if (!dev->mode_config.fb_modifiers_not_supported &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
ret = convert_tiling_flags_to_modifier(rfb);
if (ret) {
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index d1570a462a51..fb61c0814115 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2798,6 +2798,8 @@ static int dce_v10_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
 
+   adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
 
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 18a7b3bd633b..17942a11366d 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2916,6 +2916,8 @@ static int dce_v11_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
 
+   adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
+
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
 
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index c7803dc2b2d5..2ec99ec8e1a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2674,6 +2674,7 @@ static int dce_v6_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.max_height = 16384;
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+   adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;
adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
 
r = amdgpu_display_modeset_create_props(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 8318ee8339f1..de11fbe5aba2 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2695,6 +2695,8 @@ static int dce_v8_0_sw_init(void *handle)
adev_to_drm(adev)->mode_config.preferred_depth = 24;
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
 
+   adev_to_drm(adev)->mode

[RFC PATCH v4 0/3] Add support modifiers for drivers whose planes only support linear layout

[Tomohito Esaki]

Some drivers whose planes only support linear layout fb do not support format
modifiers.
These drivers should support modifiers, however the DRM core should handle this
rather than open-coding in every driver.

In this patch series, these drivers expose format modifiers based on the
following suggestion[1].

On Thu, Nov 18, 2021 at 01:02:11PM +, Daniel Stone wrote:
> I think the best way forward here is:
>   - add a new mode_config.cannot_support_modifiers flag, and enable
> this in radeon (plus any other drivers in the same boat)
>   - change drm_universal_plane_init() to advertise the LINEAR modifier
> when NULL is passed as the modifier list (including installing a
> default .format_mod_supported hook)
>   - remove the mode_config.allow_fb_modifiers hook and always
> advertise modifier support, unless
> mode_config.cannot_support_modifiers is set


[1] 
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20190509054518.10781-1-e...@igel.co.jp/#24602575

v4:
* modify documentation for fb_modifiers_not_supported flag in kerneldoc

v3: https://www.spinics.net/lists/dri-devel/msg329102.html
* change the order as follows:
   1. add fb_modifiers_not_supported flag
   2. add default modifiers
   3. remove allow_fb_modifiers flag
* add a conditional disable in amdgpu_dm_plane_init()

v2: https://www.spinics.net/lists/dri-devel/msg328939.html
* rebase to the latest master branch (5.16.0+)
  + "drm/plane: Make format_mod_supported truly optional" patch [2]
  [2] https://patchwork.freedesktop.org/patch/467940/?series=98255&rev=3

v1: https://www.spinics.net/lists/dri-devel/msg327352.html
* The initial patch set

Tomohito Esaki (3):
  drm: introduce fb_modifiers_not_supported flag in mode_config
  drm: add support modifiers for drivers whose planes only support
linear layout
  drm: remove allow_fb_modifiers

 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  6 ++---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 ++
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 ++
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |  1 +
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +++
 drivers/gpu/drm/drm_framebuffer.c |  6 ++---
 drivers/gpu/drm/drm_ioctl.c   |  2 +-
 drivers/gpu/drm/drm_plane.c   | 22 +--
 drivers/gpu/drm/nouveau/nouveau_display.c |  6 +++--
 drivers/gpu/drm/radeon/radeon_display.c   |  2 ++
 .../gpu/drm/selftests/test-drm_framebuffer.c  |  1 -
 include/drm/drm_mode_config.h | 18 +--
 include/drm/drm_plane.h   |  3 +++
 14 files changed, 43 insertions(+), 33 deletions(-)

-- 
2.25.1

RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

[Liang, Prike]

If the flag ACPI_FADT_LOW_POWER_S0 not set or AMDPMC driver not build, then 
that seems will mess up the suspend entry and unable to enter either S3 nor 
S2idle properly. In this S2idle configuration issue case, how about add some 
message to notify end user how to configure S2idle correctly?

Thanks,
Prike
From: amd-gfx  On Behalf Of Limonciello, 
Mario
Sent: Tuesday, January 18, 2022 1:26 AM
To: Lazar, Lijo ; amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse 
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems


[Public]

Yes, that's part of why I want to make sure there are explicit warnings to 
users about using this flow.
When not enabled in ACPI then also the LPS0 device is not exported and AMD_PMC 
won't load or be used.

I think from amdgpu perspective it should behave relatively similar to an 
aborted suspend.

From: Lazar, Lijo mailto:lijo.la...@amd.com>>
Sent: Monday, January 17, 2022 11:20
To: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Cc: Bjoren Dasse mailto:bjoern.da...@gmail.com>>
Subject: Re: [PATCH v2] drm/amd: Warn users about potential s0ix problems

Any problem with PMFW sequence in the way Linux handles s2idle when it's not 
enabled in ACPI?

Thanks,
Lijo

From: Limonciello, Mario 
mailto:mario.limoncie...@amd.com>>
Sent: Monday, January 17, 2022 10:45:44 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>; Lazar, 
Lijo mailto:lijo.la...@amd.com>>
Cc: Bjoren Dasse mailto:bjoern.da...@gmail.com>>
Subject: RE: [PATCH v2] drm/amd: Warn users about potential s0ix problems

[Public]

This has been sitting a week or so.
Bump on review for this patch.

> -Original Message-
> From: Limonciello, Mario 
> mailto:mario.limoncie...@amd.com>>
> Sent: Tuesday, January 11, 2022 14:00
> To: amd-gfx@lists.freedesktop.org
> Cc: Limonciello, Mario 
> mailto:mario.limoncie...@amd.com>>; Bjoren Dasse
> mailto:bjoern.da...@gmail.com>>
> Subject: [PATCH v2] drm/amd: Warn users about potential s0ix problems
>
> On some OEM setups users can configure the BIOS for S3 or S2idle.
> When configured to S3 users can still choose 's2idle' in the kernel by
> using `/sys/power/mem_sleep`.  Before commit 6dc8265f9803
> ("drm/amdgpu:
> always reset the asic in suspend (v2)"), the GPU would crash.  Now when
> configured this way, the system should resume but will use more power.
>
> As such, adjust the `amdpu_acpi_is_s0ix function` to warn users about
> potential power consumption issues during their first attempt at
> suspending.
>
> Reported-by: Bjoren Dasse 
> mailto:bjoern.da...@gmail.com>>
> Link: 
> https://gitlab.freedesktop.org/drm/amd/-/issues/1824
> Signed-off-by: Mario Limonciello 
> mailto:mario.limoncie...@amd.com>>
> ---
> v1->v2:
>  * Only show messages in s2idle cases
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index 4811b0faafd9..1295de6d6c30 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1040,11 +1040,15 @@ void amdgpu_acpi_detect(void)
>   */
>  bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev)
>  {
> -#if IS_ENABLED(CONFIG_AMD_PMC) && IS_ENABLED(CONFIG_SUSPEND)
> - if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) {
> - if (adev->flags & AMD_IS_APU)
> - return pm_suspend_target_state ==
> PM_SUSPEND_TO_IDLE;
> - }
> + if (!(adev->flags & AMD_IS_APU) ||
> + pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
> + return false;
> + if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0))
> + dev_warn_once(adev->dev,
> +   "BIOS is not configured for suspend-to-idle, power
> consumption will be higher\n");
> +#if !IS_ENABLED(CONFIG_AMD_PMC)
> + dev_warn_once(adev->dev,
> +   "amd-pmc is not enabled in the kernel, power
> consumption will be higher\n");
>  #endif
> - return false;
> + return true;
>  }
> --
> 2.25.1

RE: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

[Chen, Guchun]

[Public]

Okay... in amdgpu_gart_bind, the check of gart.ptr is also present, so it's 
safe.

Regards,
Guchun

-Original Message-
From: Koenig, Christian  
Sent: Tuesday, January 18, 2022 4:14 PM
To: Chen, Guchun ; Kim, Jonathan ; 
amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix 
Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram 
access bounce buffer

That check is utterly nonsense and should probably be removed.

What needs to be checked is if the GART ptr is available and that should 
certainly be the case at this point.

Christian.

Am 18.01.22 um 09:09 schrieb Chen, Guchun:
> [Public]
>
> Hi Christian,
>
> Re: Well that doesn't seem to make sense the GART is initialized by the code 
> around the allocation so that should work fine.
>
> Below is the calltrace during driver probe. When binding the page(SDMA bo) 
> into gart table, there is a check by gart.ready, that will be set to be true 
> later on in gmc_v10_0_hw_init. So a calltrace is observed.
>
> [3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
> [3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
> [3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
> [3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
> [3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
> [3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
> [3.381951]  ? vprintk_default+0x1d/0x20
> [3.381955]  ? printk+0x58/0x6f
> [3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
> [3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]
>
> Regards,
> Guchun
>
> -Original Message-
> From: Koenig, Christian 
> Sent: Tuesday, January 18, 2022 3:30 PM
> To: Kim, Jonathan ; 
> amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix ; Chen, Guchun 
> 
> Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of 
> debug vram access bounce buffer
>
> Am 18.01.22 um 00:43 schrieb Jonathan Kim:
>> Move the debug sdma vram bounce buffer GART map on device init after 
>> when GART is ready to avoid warnings and non-access to SDMA.
> Well that doesn't seem to make sense the GART is initialized by the code 
> around the allocation so that should work fine.
>
> Freeing the BO indeed needs to be moved up, but that can still be in the same 
> function.
>
> Also please don't move TTM related code outside of the TTM code in amdgpu.
>
> Regards,
> Christian.
>
>> Also move bounce buffer tear down after the memory manager has 
>> flushed queued work for safety.
>>
>> Signed-off-by: Jonathan Kim 
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++
>>drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  8 
>>2 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index da3348fa7b0e..099460d15258 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
>> *adev)
>>  if (r)
>>  goto init_failed;
>>
>> +/* GTT bounce buffer for debug vram access over sdma. */
>> +if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> +AMDGPU_GEM_DOMAIN_GTT,
>> +&adev->mman.sdma_access_bo, NULL,
>> +&adev->mman.sdma_access_ptr))
>> +DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> +
>>  /*
>>   * retired pages will be loaded from eeprom and reserved here,
>>   * it should be called after amdgpu_device_ip_hw_init_phase2  
>> since @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device 
>> *adev)
>>  }
>>  adev->shutdown = true;
>>
>> +/* remove debug vram sdma access bounce buffer. */
>> +amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>> +&adev->mman.sdma_access_ptr);
>> +
>>  /* make sure IB test finished before entering exclusive mode
>>   * to avoid preemption on IB test
>>   * */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index b489cd8abe31..6178ae7ba624 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>  return r;
>>  }
>>
>> -if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
>> -AMDGPU_GEM_DOMAIN_GTT,
>> -&adev->mman.sdma_access_bo, NULL,
>> -adev->mman.sdma_access_ptr))
>> -DRM_WARN("Debug VRAM access will use slowpath MM access\n");
>> -
>>  return 0;
>>}
>>
>> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>  ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_

Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

[Christian König]

That check is utterly nonsense and should probably be removed.

What needs to be checked is if the GART ptr is available and that should 
certainly be the case at this point.


Christian.

Am 18.01.22 um 09:09 schrieb Chen, Guchun:

[Public]

Hi Christian,

Re: Well that doesn't seem to make sense the GART is initialized by the code 
around the allocation so that should work fine.

Below is the calltrace during driver probe. When binding the page(SDMA bo) into 
gart table, there is a check by gart.ready, that will be set to be true later 
on in gmc_v10_0_hw_init. So a calltrace is observed.

[3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
[3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
[3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
[3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
[3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
[3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
[3.381951]  ? vprintk_default+0x1d/0x20
[3.381955]  ? printk+0x58/0x6f
[3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
[3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]

Regards,
Guchun

-Original Message-
From: Koenig, Christian 
Sent: Tuesday, January 18, 2022 3:30 PM
To: Kim, Jonathan ; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix ; Chen, Guchun 
Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram 
access bounce buffer

Am 18.01.22 um 00:43 schrieb Jonathan Kim:

Move the debug sdma vram bounce buffer GART map on device init after
when GART is ready to avoid warnings and non-access to SDMA.

Well that doesn't seem to make sense the GART is initialized by the code around 
the allocation so that should work fine.

Freeing the BO indeed needs to be moved up, but that can still be in the same 
function.

Also please don't move TTM related code outside of the TTM code in amdgpu.

Regards,
Christian.


Also move bounce buffer tear down after the memory manager has flushed
queued work for safety.

Signed-off-by: Jonathan Kim 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++
   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  8 
   2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index da3348fa7b0e..099460d15258 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
*adev)
if (r)
goto init_failed;
   
+	/* GTT bounce buffer for debug vram access over sdma. */

+   if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
+   AMDGPU_GEM_DOMAIN_GTT,
+   &adev->mman.sdma_access_bo, NULL,
+   &adev->mman.sdma_access_ptr))
+   DRM_WARN("Debug VRAM access will use slowpath MM access\n");
+
/*
 * retired pages will be loaded from eeprom and reserved here,
 * it should be called after amdgpu_device_ip_hw_init_phase2  since
@@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
}
adev->shutdown = true;
   
+	/* remove debug vram sdma access bounce buffer. */

+   amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
+   &adev->mman.sdma_access_ptr);
+
/* make sure IB test finished before entering exclusive mode
 * to avoid preemption on IB test
 * */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b489cd8abe31..6178ae7ba624 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
return r;
}
   
-	if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,

-   AMDGPU_GEM_DOMAIN_GTT,
-   &adev->mman.sdma_access_bo, NULL,
-   adev->mman.sdma_access_ptr))
-   DRM_WARN("Debug VRAM access will use slowpath MM access\n");
-
return 0;
   }
   
@@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)

ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
ttm_device_fini(&adev->mman.bdev);
adev->mman.initialized = false;
-   amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
-   &adev->mman.sdma_access_ptr);
DRM_INFO("amdgpu: ttm finalized\n");
   }
   

RE: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram access bounce buffer

[Chen, Guchun]

[Public]

Hi Christian,

Re: Well that doesn't seem to make sense the GART is initialized by the code 
around the allocation so that should work fine.

Below is the calltrace during driver probe. When binding the page(SDMA bo) into 
gart table, there is a check by gart.ready, that will be set to be true later 
on in gmc_v10_0_hw_init. So a calltrace is observed.

[3.381510]  amdgpu_ttm_gart_bind+0x80/0xc0 [amdgpu]
[3.381580]  amdgpu_ttm_alloc_gart+0x158/0x1c0 [amdgpu]
[3.381647]  amdgpu_bo_create_reserved+0x136/0x1e0 [amdgpu]
[3.381714]  ? amdgpu_ttm_debugfs_init+0x120/0x120 [amdgpu]
[3.381782]  amdgpu_bo_create_kernel+0x17/0x80 [amdgpu]
[3.381849]  amdgpu_ttm_init.cold+0x174/0x18e [amdgpu]
[3.381951]  ? vprintk_default+0x1d/0x20
[3.381955]  ? printk+0x58/0x6f
[3.381957]  amdgpu_bo_init.cold+0x4b/0x53 [amdgpu]
[3.382052]  gmc_v10_0_sw_init+0x304/0x490 [amdgpu]

Regards,
Guchun

-Original Message-
From: Koenig, Christian  
Sent: Tuesday, January 18, 2022 3:30 PM
To: Kim, Jonathan ; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix ; Chen, Guchun 
Subject: Re: [PATCH 1/2] drm/amdgpu: fix build up and tear down of debug vram 
access bounce buffer

Am 18.01.22 um 00:43 schrieb Jonathan Kim:
> Move the debug sdma vram bounce buffer GART map on device init after 
> when GART is ready to avoid warnings and non-access to SDMA.

Well that doesn't seem to make sense the GART is initialized by the code around 
the allocation so that should work fine.

Freeing the BO indeed needs to be moved up, but that can still be in the same 
function.

Also please don't move TTM related code outside of the TTM code in amdgpu.

Regards,
Christian.

>
> Also move bounce buffer tear down after the memory manager has flushed 
> queued work for safety.
>
> Signed-off-by: Jonathan Kim 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  8 
>   2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index da3348fa7b0e..099460d15258 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2378,6 +2378,13 @@ static int amdgpu_device_ip_init(struct amdgpu_device 
> *adev)
>   if (r)
>   goto init_failed;
>   
> + /* GTT bounce buffer for debug vram access over sdma. */
> + if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_GTT,
> + &adev->mman.sdma_access_bo, NULL,
> + &adev->mman.sdma_access_ptr))
> + DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> +
>   /*
>* retired pages will be loaded from eeprom and reserved here,
>* it should be called after amdgpu_device_ip_hw_init_phase2  since 
> @@ -3872,6 +3879,10 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
>   }
>   adev->shutdown = true;
>   
> + /* remove debug vram sdma access bounce buffer. */
> + amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> + &adev->mman.sdma_access_ptr);
> +
>   /* make sure IB test finished before entering exclusive mode
>* to avoid preemption on IB test
>* */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index b489cd8abe31..6178ae7ba624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1855,12 +1855,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   return r;
>   }
>   
> - if (amdgpu_bo_create_kernel(adev, PAGE_SIZE, PAGE_SIZE,
> - AMDGPU_GEM_DOMAIN_GTT,
> - &adev->mman.sdma_access_bo, NULL,
> - adev->mman.sdma_access_ptr))
> - DRM_WARN("Debug VRAM access will use slowpath MM access\n");
> -
>   return 0;
>   }
>   
> @@ -1901,8 +1895,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
>   ttm_device_fini(&adev->mman.bdev);
>   adev->mman.initialized = false;
> - amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
> - &adev->mman.sdma_access_ptr);
>   DRM_INFO("amdgpu: ttm finalized\n");
>   }
>