Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()

[Tom Lendacky]

On 8/13/21 12:08 PM, Tom Lendacky wrote:

On 8/12/21 5:07 AM, Kirill A. Shutemov wrote:

On Wed, Aug 11, 2021 at 10:52:55AM -0500, Tom Lendacky wrote:

On 8/11/21 7:19 AM, Kirill A. Shutemov wrote:

On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:

On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:

...

Looking at code agains, now I *think* the reason is accessing a global
variable from __startup_64() inside TDX version of prot_guest_has().

__startup_64() is special. If you access any global variable you need to
use fixup_pointer(). See comment before __startup_64().

I'm not sure how you get away with accessing sme_me_mask directly from
there. Any clues? Maybe just a luck and complier generates code just 
right

for your case, I donno.


Hmm... yeah, could be that the compiler is using rip-relative addressing
for it because it lives in the .data section?


I guess. It has to be fixed. It may break with complier upgrade or any
random change around the code.


I'll look at doing that separate from this series.



BTW, does it work with clang for you?


I haven't tried with clang, I'll check on that.


Just as an fyi, clang also uses rip relative addressing for those 
variables. No issues booting SME and SEV guests built with clang.


Thanks,
Tom



Thanks,
Tom

RE: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301

[Liu, Zhan]

[Public]

> -Original Message-
> From: Liu, Zhan 
> Sent: 2021/August/13, Friday 3:21 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Cornij, Nikola ; Liu, Zhan
> ; Logush, Oliver 
> Subject: [PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301
>
>
> [why]
> dcn301_calculate_wm_and_dl() causes flickering when external monitor is
> connected.
>
> This issue has been fixed before by commit 0e4c0ae59d7e
> ("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"),
> however part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
> 'amd-drm-next-5.13-2021-03-23' of
> https://gitlab.freedesktop.org/agd5f/linux into drm-next").
>
> [how]
> Use dcn30_calculate_wm_and_dlg() instead as in the original fix.
>
> Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of
> https://gitlab.freedesktop.org/agd5f/linux into drm-next")
> Signed-off-by: Nikola Cornij 

Reviewed-by: Zhan Liu 
Tested-by: Zhan Liu 
Tested-by: Oliver Logush 

> ---
>  .../amd/display/dc/dcn301/dcn301_resource.c   | 96 +--
>  1 file changed, 1 insertion(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> index 9776d1737818..912285fdce18 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
> @@ -1622,106 +1622,12 @@ static void
> dcn301_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
> dml_init_instance(>dml, _01_soc, _01_ip,
> DML_PROJECT_DCN30);  }
>
> -static void calculate_wm_set_for_vlevel(
> -   int vlevel,
> -   struct wm_range_table_entry *table_entry,
> -   struct dcn_watermarks *wm_set,
> -   struct display_mode_lib *dml,
> -   display_e2e_pipe_params_st *pipes,
> -   int pipe_cnt)
> -{
> -   double dram_clock_change_latency_cached = dml-
> >soc.dram_clock_change_latency_us;
> -
> -   ASSERT(vlevel < dml->soc.num_states);
> -   /* only pipe 0 is read for voltage and dcf/soc clocks */
> -   pipes[0].clks_cfg.voltage = vlevel;
> -   pipes[0].clks_cfg.dcfclk_mhz = 
> dml->soc.clock_limits[vlevel].dcfclk_mhz;
> -   pipes[0].clks_cfg.socclk_mhz = 
> dml->soc.clock_limits[vlevel].socclk_mhz;
> -
> -   dml->soc.dram_clock_change_latency_us = table_entry-
> >pstate_latency_us;
> -   dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
> -   dml->soc.sr_enter_plus_exit_time_us = table_entry-
> >sr_enter_plus_exit_time_us;
> -
> -   wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
> -   wm_set->cstate_pstate.cstate_enter_plus_exit_ns =
> get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
> -   wm_set->cstate_pstate.cstate_exit_ns = get_wm_stutter_exit(dml, pipes,
> pipe_cnt) * 1000;
> -   wm_set->cstate_pstate.pstate_change_ns =
> get_wm_dram_clock_change(dml, pipes, pipe_cnt) * 1000;
> -   wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes,
> pipe_cnt) * 1000;
> -   wm_set->frac_urg_bw_nom = get_fraction_of_urgent_bandwidth(dml,
> pipes, pipe_cnt) * 1000;
> -   wm_set->frac_urg_bw_flip =
> get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
> -   wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes, pipe_cnt)
> * 1000;
> -   dml->soc.dram_clock_change_latency_us =
> dram_clock_change_latency_cached;
> -
> -}
> -
> -static void dcn301_calculate_wm_and_dlg(
> -   struct dc *dc, struct dc_state *context,
> -   display_e2e_pipe_params_st *pipes,
> -   int pipe_cnt,
> -   int vlevel_req)
> -{
> -   int i, pipe_idx;
> -   int vlevel, vlevel_max;
> -   struct wm_range_table_entry *table_entry;
> -   struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
> -
> -   ASSERT(bw_params);
> -
> -   vlevel_max = bw_params->clk_table.num_entries - 1;
> -
> -   /* WM Set D */
> -   table_entry = _params->wm_table.entries[WM_D];
> -   if (table_entry->wm_type == WM_TYPE_RETRAINING)
> -   vlevel = 0;
> -   else
> -   vlevel = vlevel_max;
> -   calculate_wm_set_for_vlevel(vlevel, table_entry, 
> >bw_ctx.bw.dcn.watermarks.d,
> -   >bw_ctx.dml, pipes, 
> pipe_cnt);
> -   /* WM Set C */
> -   table_entry = _params->wm_table.entries[WM_C];
> -   vlevel = min(max(vlevel_req, 2), vlevel_max);
> -   calculate_wm_set_for_vlevel(vlevel, table_entry, 
> >bw_ctx.bw.dcn.watermarks.c,
> -   >bw_ctx.dml, pipes, 
> pipe_cnt);
> -   /* WM Set B */
> -   table_entry = _params->wm_table.entries[WM_B];
> -   vlevel = min(max(vlevel_req, 1), vlevel_max);
> -   calculate_wm_set_for_vlevel(vlevel, table_entry, 
> >bw_ctx.bw.dcn.watermarks.b,
> -   >bw_ctx.dml, 

[PATCH] drm/amd/display: Use DCN30 watermark calc for DCN301

[Liu, Zhan]

[AMD Official Use Only]

[why]
dcn301_calculate_wm_and_dl() causes flickering when external monitor is
connected.

This issue has been fixed before by commit 0e4c0ae59d7e
("drm/amdgpu/display: drop dcn301_calculate_wm_and_dl for now"), however
part of the fix was gone after commit 2cbcb78c9ee5 ("Merge tag
'amd-drm-next-5.13-2021-03-23' of
https://gitlab.freedesktop.org/agd5f/linux into drm-next").

[how]
Use dcn30_calculate_wm_and_dlg() instead as in the original fix.

Fixes: 2cbcb78c9ee5 ("Merge tag 'amd-drm-next-5.13-2021-03-23' of 
https://gitlab.freedesktop.org/agd5f/linux into drm-next")
Signed-off-by: Nikola Cornij mailto:nikola.cor...@amd.com
---
 .../amd/display/dc/dcn301/dcn301_resource.c   | 96 +--
 1 file changed, 1 insertion(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
index 9776d1737818..912285fdce18 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
@@ -1622,106 +1622,12 @@ static void dcn301_update_bw_bounding_box(struct dc 
*dc, struct clk_bw_params *b
dml_init_instance(>dml, _01_soc, _01_ip, 
DML_PROJECT_DCN30);
 }

-static void calculate_wm_set_for_vlevel(
-   int vlevel,
-   struct wm_range_table_entry *table_entry,
-   struct dcn_watermarks *wm_set,
-   struct display_mode_lib *dml,
-   display_e2e_pipe_params_st *pipes,
-   int pipe_cnt)
-{
-   double dram_clock_change_latency_cached = 
dml->soc.dram_clock_change_latency_us;
-
-   ASSERT(vlevel < dml->soc.num_states);
-   /* only pipe 0 is read for voltage and dcf/soc clocks */
-   pipes[0].clks_cfg.voltage = vlevel;
-   pipes[0].clks_cfg.dcfclk_mhz = dml->soc.clock_limits[vlevel].dcfclk_mhz;
-   pipes[0].clks_cfg.socclk_mhz = dml->soc.clock_limits[vlevel].socclk_mhz;
-
-   dml->soc.dram_clock_change_latency_us = table_entry->pstate_latency_us;
-   dml->soc.sr_exit_time_us = table_entry->sr_exit_time_us;
-   dml->soc.sr_enter_plus_exit_time_us = 
table_entry->sr_enter_plus_exit_time_us;
-
-   wm_set->urgent_ns = get_wm_urgent(dml, pipes, pipe_cnt) * 1000;
-   wm_set->cstate_pstate.cstate_enter_plus_exit_ns = 
get_wm_stutter_enter_exit(dml, pipes, pipe_cnt) * 1000;
-   wm_set->cstate_pstate.cstate_exit_ns = get_wm_stutter_exit(dml, pipes, 
pipe_cnt) * 1000;
-   wm_set->cstate_pstate.pstate_change_ns = get_wm_dram_clock_change(dml, 
pipes, pipe_cnt) * 1000;
-   wm_set->pte_meta_urgent_ns = get_wm_memory_trip(dml, pipes, pipe_cnt) * 
1000;
-   wm_set->frac_urg_bw_nom = get_fraction_of_urgent_bandwidth(dml, pipes, 
pipe_cnt) * 1000;
-   wm_set->frac_urg_bw_flip = 
get_fraction_of_urgent_bandwidth_imm_flip(dml, pipes, pipe_cnt) * 1000;
-   wm_set->urgent_latency_ns = get_urgent_latency(dml, pipes, pipe_cnt) * 
1000;
-   dml->soc.dram_clock_change_latency_us = 
dram_clock_change_latency_cached;
-
-}
-
-static void dcn301_calculate_wm_and_dlg(
-   struct dc *dc, struct dc_state *context,
-   display_e2e_pipe_params_st *pipes,
-   int pipe_cnt,
-   int vlevel_req)
-{
-   int i, pipe_idx;
-   int vlevel, vlevel_max;
-   struct wm_range_table_entry *table_entry;
-   struct clk_bw_params *bw_params = dc->clk_mgr->bw_params;
-
-   ASSERT(bw_params);
-
-   vlevel_max = bw_params->clk_table.num_entries - 1;
-
-   /* WM Set D */
-   table_entry = _params->wm_table.entries[WM_D];
-   if (table_entry->wm_type == WM_TYPE_RETRAINING)
-   vlevel = 0;
-   else
-   vlevel = vlevel_max;
-   calculate_wm_set_for_vlevel(vlevel, table_entry, 
>bw_ctx.bw.dcn.watermarks.d,
-   >bw_ctx.dml, pipes, 
pipe_cnt);
-   /* WM Set C */
-   table_entry = _params->wm_table.entries[WM_C];
-   vlevel = min(max(vlevel_req, 2), vlevel_max);
-   calculate_wm_set_for_vlevel(vlevel, table_entry, 
>bw_ctx.bw.dcn.watermarks.c,
-   >bw_ctx.dml, pipes, 
pipe_cnt);
-   /* WM Set B */
-   table_entry = _params->wm_table.entries[WM_B];
-   vlevel = min(max(vlevel_req, 1), vlevel_max);
-   calculate_wm_set_for_vlevel(vlevel, table_entry, 
>bw_ctx.bw.dcn.watermarks.b,
-   >bw_ctx.dml, pipes, 
pipe_cnt);
-
-   /* WM Set A */
-   table_entry = _params->wm_table.entries[WM_A];
-   vlevel = min(vlevel_req, vlevel_max);
-   calculate_wm_set_for_vlevel(vlevel, table_entry, 
>bw_ctx.bw.dcn.watermarks.a,
-   >bw_ctx.dml, pipes, 
pipe_cnt);
-
-   for (i = 0, pipe_idx = 0; i < dc->res_pool->pipe_count; i++) {
-   if (!context->res_ctx.pipe_ctx[i].stream)
-   continue;
-
-  

Re: [PATCH v2 00/12] Implement generic prot_guest_has() helper function

[Tom Lendacky]

On 8/13/21 11:59 AM, Tom Lendacky wrote:

This patch series provides a generic helper function, prot_guest_has(),
to replace the sme_active(), sev_active(), sev_es_active() and
mem_encrypt_active() functions.

It is expected that as new protected virtualization technologies are
added to the kernel, they can all be covered by a single function call
instead of a collection of specific function calls all called from the
same locations.

The powerpc and s390 patches have been compile tested only. Can the
folks copied on this series verify that nothing breaks for them.


There are some patches related to PPC that added new calls to the 
mem_encrypt_active() function that are not yet in the tip tree. After the 
merge window, I'll need to send a v3 with those additional changes before 
this series can be applied.


Thanks,
Tom

Re: [PATCH v5 2/9] moduleparam: add data member to struct kernel_param

[jim . cromie]

On Fri, Aug 13, 2021 at 9:44 AM Andy Shevchenko
 wrote:
>
> On Fri, Aug 13, 2021 at 09:17:10AM -0600, Jim Cromie wrote:
> > Add a const void* data member to the struct, to allow attaching
> > private data that will be used soon by a setter method (via kp->data)
> > to perform more elaborate actions.
> >
> > To attach the data at compile time, add new macros:
> >
> > module_param_cbd() derives from module_param_cb(), adding data param,
> > and latter is redefined to use former.
> >
> > It calls __module_param_call_wdata(), which accepts a new data param
> > and inits .data with it. Re-define __module_param_call() to use it.
> >
> > Use of this new data member will be rare, it might be worth redoing
> > this as a separate/sub-type to de-bloat the base case.
>
> ...
>
> > +#define module_param_cbd(name, ops, arg, perm, data)   
> >   \
> > + __module_param_call_wdata(MODULE_PARAM_PREFIX, name, ops, arg, perm, 
> > -1, 0, data)
>
> Cryptic name. Moreover, inconsistent with the rest.
> What about module_param_cb_data() ?
>
> >  #define module_param_cb_unsafe(name, ops, arg, perm)   
> > \
> >   __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1,
> > \
> >   KERNEL_PARAM_FL_UNSAFE)
>
> (above left for the above comment)
>
> ...
>
> > +#define __module_param_call_wdata(prefix, name, ops, arg, perm, level, 
> > flags, data) \
>
> Similar __module_param_call_with_data()
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

yes to all renames, revised.
thanks

Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()

[Tom Lendacky]

On 8/12/21 5:07 AM, Kirill A. Shutemov wrote:

On Wed, Aug 11, 2021 at 10:52:55AM -0500, Tom Lendacky wrote:

On 8/11/21 7:19 AM, Kirill A. Shutemov wrote:

On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:

On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:

...

Looking at code agains, now I *think* the reason is accessing a global
variable from __startup_64() inside TDX version of prot_guest_has().

__startup_64() is special. If you access any global variable you need to
use fixup_pointer(). See comment before __startup_64().

I'm not sure how you get away with accessing sme_me_mask directly from
there. Any clues? Maybe just a luck and complier generates code just right
for your case, I donno.


Hmm... yeah, could be that the compiler is using rip-relative addressing
for it because it lives in the .data section?


I guess. It has to be fixed. It may break with complier upgrade or any
random change around the code.


I'll look at doing that separate from this series.



BTW, does it work with clang for you?


I haven't tried with clang, I'll check on that.

Thanks,
Tom

[PATCH v2 12/12] s390/mm: Remove the now unused mem_encrypt_active() function

[Tom Lendacky]

The mem_encrypt_active() function has been replaced by prot_guest_has(),
so remove the implementation. Since the default implementation of the
prot_guest_has() matches the s390 implementation of mem_encrypt_active(),
prot_guest_has() does not need to be implemented in s390 (the config
option ARCH_HAS_PROTECTED_GUEST is not set).

Cc: Heiko Carstens 
Cc: Vasily Gorbik 
Cc: Christian Borntraeger 
Signed-off-by: Tom Lendacky 
---
 arch/s390/include/asm/mem_encrypt.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/s390/include/asm/mem_encrypt.h 
b/arch/s390/include/asm/mem_encrypt.h
index 2542cbf7e2d1..08a8b96606d7 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -4,8 +4,6 @@
 
 #ifndef __ASSEMBLY__
 
-static inline bool mem_encrypt_active(void) { return false; }
-
 int set_memory_encrypted(unsigned long addr, int numpages);
 int set_memory_decrypted(unsigned long addr, int numpages);
 
-- 
2.32.0

[PATCH v2 11/12] powerpc/pseries/svm: Remove the now unused mem_encrypt_active() function

[Tom Lendacky]

The mem_encrypt_active() function has been replaced by prot_guest_has(),
so remove the implementation.

Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Signed-off-by: Tom Lendacky 
---
 arch/powerpc/include/asm/mem_encrypt.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/powerpc/include/asm/mem_encrypt.h 
b/arch/powerpc/include/asm/mem_encrypt.h
index ba9dab07c1be..2f26b8fc8d29 100644
--- a/arch/powerpc/include/asm/mem_encrypt.h
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -10,11 +10,6 @@
 
 #include 
 
-static inline bool mem_encrypt_active(void)
-{
-   return is_secure_guest();
-}
-
 static inline bool force_dma_unencrypted(struct device *dev)
 {
return is_secure_guest();
-- 
2.32.0

[PATCH v2 10/12] x86/sev: Remove the now unused mem_encrypt_active() function

[Tom Lendacky]

The mem_encrypt_active() function has been replaced by prot_guest_has(),
so remove the implementation.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Reviewed-by: Joerg Roedel 
Signed-off-by: Tom Lendacky 
---
 arch/x86/include/asm/mem_encrypt.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h 
b/arch/x86/include/asm/mem_encrypt.h
index 797146e0cd6b..94c089e9ea69 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -97,11 +97,6 @@ static inline void mem_encrypt_free_decrypted_mem(void) { }
 
 extern char __start_bss_decrypted[], __end_bss_decrypted[], 
__start_bss_decrypted_unused[];
 
-static inline bool mem_encrypt_active(void)
-{
-   return sme_me_mask;
-}
-
 static inline u64 sme_get_me_mask(void)
 {
return sme_me_mask;
-- 
2.32.0

[PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function

[Tom Lendacky]

The mem_encrypt_active() function has been replaced by prot_guest_has(),
so remove the implementation.

Reviewed-by: Joerg Roedel 
Signed-off-by: Tom Lendacky 
---
 include/linux/mem_encrypt.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
index 5c4a18a91f89..ae4526389261 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -16,10 +16,6 @@
 
 #include 
 
-#else  /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
-
-static inline bool mem_encrypt_active(void) { return false; }
-
 #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT
-- 
2.32.0

[PATCH v2 08/12] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()

[Tom Lendacky]

Replace occurrences of mem_encrypt_active() with calls to prot_guest_has()
with the PATTR_MEM_ENCRYPT attribute.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: VMware Graphics 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Dave Young 
Cc: Baoquan He 
Signed-off-by: Tom Lendacky 
---
 arch/x86/kernel/head64.c| 4 ++--
 arch/x86/mm/ioremap.c   | 4 ++--
 arch/x86/mm/mem_encrypt.c   | 5 ++---
 arch/x86/mm/pat/set_memory.c| 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++-
 drivers/gpu/drm/drm_cache.c | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
 drivers/iommu/amd/iommu.c   | 3 ++-
 drivers/iommu/amd/iommu_v2.c| 3 ++-
 drivers/iommu/iommu.c   | 3 ++-
 fs/proc/vmcore.c| 6 +++---
 kernel/dma/swiotlb.c| 4 ++--
 13 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index de01903c3735..cafed6456d45 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #include 
@@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
 * there is no need to zero it after changing the memory encryption
 * attribute.
 */
-   if (mem_encrypt_active()) {
+   if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
vaddr = (unsigned long)__start_bss_decrypted;
vaddr_end = (unsigned long)__end_bss_decrypted;
for (; vaddr < vaddr_end; vaddr += PMD_SIZE) {
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 3ed0f28f12af..7f012fc1b600 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -694,7 +694,7 @@ static bool __init 
early_memremap_is_setup_data(resource_size_t phys_addr,
 bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size,
 unsigned long flags)
 {
-   if (!mem_encrypt_active())
+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return true;
 
if (flags & MEMREMAP_ENC)
@@ -724,7 +724,7 @@ pgprot_t __init 
early_memremap_pgprot_adjust(resource_size_t phys_addr,
 {
bool encrypted_prot;
 
-   if (!mem_encrypt_active())
+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return prot;
 
encrypted_prot = true;
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 38dfa84b77a1..69aed9935b5e 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -364,8 +364,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, 
unsigned long size)
 /*
  * SME and SEV are very similar but they are not the same, so there are
  * times that the kernel will need to distinguish between SME and SEV. The
- * sme_active() and sev_active() functions are used for this.  When a
- * distinction isn't needed, the mem_encrypt_active() function can be used.
+ * sme_active() and sev_active() functions are used for this.
  *
  * The trampoline code is a good example for this requirement.  Before
  * paging is activated, SME will access all memory as decrypted, but SEV
@@ -451,7 +450,7 @@ void __init mem_encrypt_free_decrypted_mem(void)
 * The unused memory range was mapped decrypted, change the encryption
 * attribute from decrypted to encrypted before freeing it.
 */
-   if (mem_encrypt_active()) {
+   if (amd_prot_guest_has(PATTR_MEM_ENCRYPT)) {
r = set_memory_encrypted(vaddr, npages);
if (r) {
pr_warn("failed to free unused decrypted pages\n");
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index ad8a5c586a35..6925f2bb4be1 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1986,7 +1987,7 @@ static int __set_memory_enc_dec(unsigned long addr, int 
numpages, bool enc)
int ret;
 
/* Nothing to do if memory encryption is not active */
-   if (!mem_encrypt_active())
+   if (!prot_guest_has(PATTR_MEM_ENCRYPT))
return 0;
 
/* Should not be working on unaligned addresses */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 971c5b8e75dc..21c1e3056070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "amdgpu.h"
 #include "amdgpu_irq.h"
@@ -1250,7 +1251,8 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 * 

[PATCH v2 07/12] x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()

[Tom Lendacky]

Replace occurrences of sev_es_active() with the more generic
prot_guest_has() using PATTR_GUEST_PROT_STATE, except for in
arch/x86/kernel/sev*.c and arch/x86/mm/mem_encrypt*.c where PATTR_SEV_ES
will be used. If future support is added for other memory encyrption
techonologies, the use of PATTR_GUEST_PROT_STATE can be updated, as
required, to specifically use PATTR_SEV_ES.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Signed-off-by: Tom Lendacky 
---
 arch/x86/include/asm/mem_encrypt.h | 2 --
 arch/x86/kernel/sev.c  | 6 +++---
 arch/x86/mm/mem_encrypt.c  | 7 +++
 arch/x86/realmode/init.c   | 3 +--
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h 
b/arch/x86/include/asm/mem_encrypt.h
index 7e25de37c148..797146e0cd6b 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -50,7 +50,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
 void __init mem_encrypt_init(void);
 
 void __init sev_es_init_vc_handling(void);
-bool sev_es_active(void);
 bool amd_prot_guest_has(unsigned int attr);
 
 #define __bss_decrypted __section(".bss..decrypted")
@@ -74,7 +73,6 @@ static inline void __init sme_encrypt_kernel(struct 
boot_params *bp) { }
 static inline void __init sme_enable(struct boot_params *bp) { }
 
 static inline void sev_es_init_vc_handling(void) { }
-static inline bool sev_es_active(void) { return false; }
 static inline bool amd_prot_guest_has(unsigned int attr) { return false; }
 
 static inline int __init
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index a6895e440bc3..66a4ab9d95d7 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -11,7 +11,7 @@
 
 #include  /* For show_regs() */
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -615,7 +615,7 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
int cpu;
u64 pfn;
 
-   if (!sev_es_active())
+   if (!prot_guest_has(PATTR_SEV_ES))
return 0;
 
pflags = _PAGE_NX | _PAGE_RW;
@@ -774,7 +774,7 @@ void __init sev_es_init_vc_handling(void)
 
BUILD_BUG_ON(offsetof(struct sev_es_runtime_data, ghcb_page) % 
PAGE_SIZE);
 
-   if (!sev_es_active())
+   if (!prot_guest_has(PATTR_SEV_ES))
return;
 
if (!sev_es_check_cpu_features())
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 83bc928f529e..38dfa84b77a1 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -383,8 +383,7 @@ static bool sme_active(void)
return sme_me_mask && !sev_active();
 }
 
-/* Needs to be called from non-instrumentable code */
-bool noinstr sev_es_active(void)
+static bool sev_es_active(void)
 {
return sev_status & MSR_AMD64_SEV_ES_ENABLED;
 }
@@ -482,7 +481,7 @@ static void print_mem_encrypt_feature_info(void)
pr_cont(" SEV");
 
/* Encrypted Register State */
-   if (sev_es_active())
+   if (amd_prot_guest_has(PATTR_SEV_ES))
pr_cont(" SEV-ES");
 
pr_cont("\n");
@@ -501,7 +500,7 @@ void __init mem_encrypt_init(void)
 * With SEV, we need to unroll the rep string I/O instructions,
 * but SEV-ES supports them through the #VC handler.
 */
-   if (amd_prot_guest_has(PATTR_SEV) && !sev_es_active())
+   if (amd_prot_guest_has(PATTR_SEV) && !amd_prot_guest_has(PATTR_SEV_ES))
static_branch_enable(_enable_key);
 
print_mem_encrypt_feature_info();
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index 2109ae569c67..7711d0071f41 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -2,7 +2,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct trampoline_header 
*th)
if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
th->flags |= TH_FLAGS_SME_ACTIVE;
 
-   if (sev_es_active()) {
+   if (prot_guest_has(PATTR_GUEST_PROT_STATE)) {
/*
 * Skip the call to verify_cpu() in secondary_startup_64 as it
 * will cause #VC exceptions when the AP can't handle them yet.
-- 
2.32.0

[PATCH v2 06/12] x86/sev: Replace occurrences of sev_active() with prot_guest_has()

[Tom Lendacky]

Replace occurrences of sev_active() with the more generic prot_guest_has()
using PATTR_GUEST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c
where PATTR_SEV will be used. If future support is added for other memory
encryption technologies, the use of PATTR_GUEST_MEM_ENCRYPT can be
updated, as required, to use PATTR_SEV.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Ard Biesheuvel 
Reviewed-by: Joerg Roedel 
Signed-off-by: Tom Lendacky 
---
 arch/x86/include/asm/mem_encrypt.h |  2 --
 arch/x86/kernel/crash_dump_64.c|  4 +++-
 arch/x86/kernel/kvm.c  |  3 ++-
 arch/x86/kernel/kvmclock.c |  4 ++--
 arch/x86/kernel/machine_kexec_64.c | 16 
 arch/x86/kvm/svm/svm.c |  3 ++-
 arch/x86/mm/ioremap.c  |  6 +++---
 arch/x86/mm/mem_encrypt.c  | 15 +++
 arch/x86/platform/efi/efi_64.c |  9 +
 9 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h 
b/arch/x86/include/asm/mem_encrypt.h
index 956338406cec..7e25de37c148 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -50,7 +50,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
 void __init mem_encrypt_init(void);
 
 void __init sev_es_init_vc_handling(void);
-bool sev_active(void);
 bool sev_es_active(void);
 bool amd_prot_guest_has(unsigned int attr);
 
@@ -75,7 +74,6 @@ static inline void __init sme_encrypt_kernel(struct 
boot_params *bp) { }
 static inline void __init sme_enable(struct boot_params *bp) { }
 
 static inline void sev_es_init_vc_handling(void) { }
-static inline bool sev_active(void) { return false; }
 static inline bool sev_es_active(void) { return false; }
 static inline bool amd_prot_guest_has(unsigned int attr) { return false; }
 
diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 045e82e8945b..0cfe35f03e67 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
  unsigned long offset, int userbuf,
@@ -73,5 +74,6 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char 
*buf, size_t csize,
 
 ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
 {
-   return read_from_oldmem(buf, count, ppos, 0, sev_active());
+   return read_from_oldmem(buf, count, ppos, 0,
+   prot_guest_has(PATTR_GUEST_MEM_ENCRYPT));
 }
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index a26643dc6bd6..9d08ad2f3faa 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -418,7 +419,7 @@ static void __init sev_map_percpu_data(void)
 {
int cpu;
 
-   if (!sev_active())
+   if (!prot_guest_has(PATTR_GUEST_MEM_ENCRYPT))
return;
 
for_each_possible_cpu(cpu) {
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index ad273e5861c1..f7ba78a23dcd 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -16,9 +16,9 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
-#include 
 #include 
 #include 
 
@@ -232,7 +232,7 @@ static void __init kvmclock_init_mem(void)
 * hvclock is shared between the guest and the hypervisor, must
 * be mapped decrypted.
 */
-   if (sev_active()) {
+   if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT)) {
r = set_memory_decrypted((unsigned long) hvclock_mem,
 1UL << order);
if (r) {
diff --git a/arch/x86/kernel/machine_kexec_64.c 
b/arch/x86/kernel/machine_kexec_64.c
index 8e7b517ad738..66ff788b79c9 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -167,7 +167,7 @@ static int init_transition_pgtable(struct kimage *image, 
pgd_t *pgd)
}
pte = pte_offset_kernel(pmd, vaddr);
 
-   if (sev_active())
+   if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT))
prot = PAGE_KERNEL_EXEC;
 
set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
@@ -207,7 +207,7 @@ static int init_pgtable(struct kimage *image, unsigned long 
start_pgtable)
level4p = (pgd_t *)__va(start_pgtable);
clear_page(level4p);
 
-   if (sev_active()) {
+   if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT)) {
info.page_flag   |= _PAGE_ENC;
info.kernpg_flag |= _PAGE_ENC;
}
@@ -570,12 +570,12 @@ void arch_kexec_unprotect_crashkres(void)
  */
 int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
 {
-   if (sev_active())
+   if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
return 0;
 
/*
- 

[PATCH v2 05/12] x86/sme: Replace occurrences of sme_active() with prot_guest_has()

[Tom Lendacky]

Replace occurrences of sme_active() with the more generic prot_guest_has()
using PATTR_HOST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c
where PATTR_SME will be used. If future support is added for other memory
encryption technologies, the use of PATTR_HOST_MEM_ENCRYPT can be
updated, as required, to use PATTR_SME.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Joerg Roedel 
Cc: Will Deacon 
Reviewed-by: Joerg Roedel 
Signed-off-by: Tom Lendacky 
---
 arch/x86/include/asm/kexec.h |  2 +-
 arch/x86/include/asm/mem_encrypt.h   |  2 --
 arch/x86/kernel/machine_kexec_64.c   |  3 ++-
 arch/x86/kernel/pci-swiotlb.c|  9 -
 arch/x86/kernel/relocate_kernel_64.S |  2 +-
 arch/x86/mm/ioremap.c|  6 +++---
 arch/x86/mm/mem_encrypt.c| 10 +-
 arch/x86/mm/mem_encrypt_identity.c   |  3 ++-
 arch/x86/realmode/init.c |  5 +++--
 drivers/iommu/amd/init.c |  7 ---
 10 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 0a6e34b07017..11b7c06e2828 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -129,7 +129,7 @@ relocate_kernel(unsigned long indirection_page,
unsigned long page_list,
unsigned long start_address,
unsigned int preserve_context,
-   unsigned int sme_active);
+   unsigned int host_mem_enc_active);
 #endif
 
 #define ARCH_HAS_KIMAGE_ARCH
diff --git a/arch/x86/include/asm/mem_encrypt.h 
b/arch/x86/include/asm/mem_encrypt.h
index a46d47662772..956338406cec 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -50,7 +50,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
 void __init mem_encrypt_init(void);
 
 void __init sev_es_init_vc_handling(void);
-bool sme_active(void);
 bool sev_active(void);
 bool sev_es_active(void);
 bool amd_prot_guest_has(unsigned int attr);
@@ -76,7 +75,6 @@ static inline void __init sme_encrypt_kernel(struct 
boot_params *bp) { }
 static inline void __init sme_enable(struct boot_params *bp) { }
 
 static inline void sev_es_init_vc_handling(void) { }
-static inline bool sme_active(void) { return false; }
 static inline bool sev_active(void) { return false; }
 static inline bool sev_es_active(void) { return false; }
 static inline bool amd_prot_guest_has(unsigned int attr) { return false; }
diff --git a/arch/x86/kernel/machine_kexec_64.c 
b/arch/x86/kernel/machine_kexec_64.c
index 131f30fdcfbd..8e7b517ad738 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -358,7 +359,7 @@ void machine_kexec(struct kimage *image)
   (unsigned long)page_list,
   image->start,
   image->preserve_context,
-  sme_active());
+  prot_guest_has(PATTR_HOST_MEM_ENCRYPT));
 
 #ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index c2cfa5e7c152..bd9a9cfbc9a2 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -6,7 +6,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 #include 
@@ -45,11 +45,10 @@ int __init pci_swiotlb_detect_4gb(void)
swiotlb = 1;
 
/*
-* If SME is active then swiotlb will be set to 1 so that bounce
-* buffers are allocated and used for devices that do not support
-* the addressing range required for the encryption mask.
+* Set swiotlb to 1 so that bounce buffers are allocated and used for
+* devices that can't support DMA to encrypted memory.
 */
-   if (sme_active())
+   if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
swiotlb = 1;
 
return swiotlb;
diff --git a/arch/x86/kernel/relocate_kernel_64.S 
b/arch/x86/kernel/relocate_kernel_64.S
index c53271aebb64..c8fe74a28143 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -47,7 +47,7 @@ SYM_CODE_START_NOALIGN(relocate_kernel)
 * %rsi page_list
 * %rdx start address
 * %rcx preserve_context
-* %r8  sme_active
+* %r8  host_mem_enc_active
 */
 
/* Save the CPU context, used for jumping back */
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index ccff76cedd8f..583afd54c7e1 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -14,7 +14,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -703,7 +703,7 @@ bool arch_memremap_can_ram_remap(resource_size_t phys_addr, 
unsigned 

[PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()

[Tom Lendacky]

Introduce a powerpc version of the prot_guest_has() function. This will
be used to replace the powerpc mem_encrypt_active() implementation, so
the implementation will initially only support the PATTR_MEM_ENCRYPT
attribute.

Cc: Michael Ellerman 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Signed-off-by: Tom Lendacky 
---
 arch/powerpc/include/asm/protected_guest.h | 30 ++
 arch/powerpc/platforms/pseries/Kconfig |  1 +
 2 files changed, 31 insertions(+)
 create mode 100644 arch/powerpc/include/asm/protected_guest.h

diff --git a/arch/powerpc/include/asm/protected_guest.h 
b/arch/powerpc/include/asm/protected_guest.h
new file mode 100644
index ..ce55c2c7e534
--- /dev/null
+++ b/arch/powerpc/include/asm/protected_guest.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Protected Guest (and Host) Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky 
+ */
+
+#ifndef _POWERPC_PROTECTED_GUEST_H
+#define _POWERPC_PROTECTED_GUEST_H
+
+#include 
+
+#ifndef __ASSEMBLY__
+
+static inline bool prot_guest_has(unsigned int attr)
+{
+   switch (attr) {
+   case PATTR_MEM_ENCRYPT:
+   return is_secure_guest();
+
+   default:
+   return false;
+   }
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _POWERPC_PROTECTED_GUEST_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig 
b/arch/powerpc/platforms/pseries/Kconfig
index 5e037df2a3a1..8ce5417d6feb 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -159,6 +159,7 @@ config PPC_SVM
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+   select ARCH_HAS_PROTECTED_GUEST
help
 There are certain POWER platforms which support secure guests using
 the Protected Execution Facility, with the help of an Ultravisor
-- 
2.32.0

[PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()

[Tom Lendacky]

Introduce an x86 version of the prot_guest_has() function. This will be
used in the more generic x86 code to replace vendor specific calls like
sev_active(), etc.

While the name suggests this is intended mainly for guests, it will
also be used for host memory encryption checks in place of sme_active().

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Reviewed-by: Joerg Roedel 
Co-developed-by: Andi Kleen 
Signed-off-by: Andi Kleen 
Co-developed-by: Kuppuswamy Sathyanarayanan 

Signed-off-by: Kuppuswamy Sathyanarayanan 

Signed-off-by: Tom Lendacky 
---
 arch/x86/Kconfig   |  1 +
 arch/x86/include/asm/mem_encrypt.h |  2 ++
 arch/x86/include/asm/protected_guest.h | 29 ++
 arch/x86/mm/mem_encrypt.c  | 25 ++
 include/linux/protected_guest.h|  5 +
 5 files changed, 62 insertions(+)
 create mode 100644 arch/x86/include/asm/protected_guest.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 421fa9e38c60..82e5fb713261 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1514,6 +1514,7 @@ config AMD_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select INSTRUCTION_DECODER
select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+   select ARCH_HAS_PROTECTED_GUEST
help
  Say yes to enable support for the encryption of system memory.
  This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/include/asm/mem_encrypt.h 
b/arch/x86/include/asm/mem_encrypt.h
index 9c80c68d75b5..a46d47662772 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -53,6 +53,7 @@ void __init sev_es_init_vc_handling(void);
 bool sme_active(void);
 bool sev_active(void);
 bool sev_es_active(void);
+bool amd_prot_guest_has(unsigned int attr);
 
 #define __bss_decrypted __section(".bss..decrypted")
 
@@ -78,6 +79,7 @@ static inline void sev_es_init_vc_handling(void) { }
 static inline bool sme_active(void) { return false; }
 static inline bool sev_active(void) { return false; }
 static inline bool sev_es_active(void) { return false; }
+static inline bool amd_prot_guest_has(unsigned int attr) { return false; }
 
 static inline int __init
 early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return 
0; }
diff --git a/arch/x86/include/asm/protected_guest.h 
b/arch/x86/include/asm/protected_guest.h
new file mode 100644
index ..51e4eefd9542
--- /dev/null
+++ b/arch/x86/include/asm/protected_guest.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Protected Guest (and Host) Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky 
+ */
+
+#ifndef _X86_PROTECTED_GUEST_H
+#define _X86_PROTECTED_GUEST_H
+
+#include 
+
+#ifndef __ASSEMBLY__
+
+static inline bool prot_guest_has(unsigned int attr)
+{
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+   if (sme_me_mask)
+   return amd_prot_guest_has(attr);
+#endif
+
+   return false;
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _X86_PROTECTED_GUEST_H */
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index ff08dc463634..edc67ddf065d 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -389,6 +390,30 @@ bool noinstr sev_es_active(void)
return sev_status & MSR_AMD64_SEV_ES_ENABLED;
 }
 
+bool amd_prot_guest_has(unsigned int attr)
+{
+   switch (attr) {
+   case PATTR_MEM_ENCRYPT:
+   return sme_me_mask != 0;
+
+   case PATTR_SME:
+   case PATTR_HOST_MEM_ENCRYPT:
+   return sme_active();
+
+   case PATTR_SEV:
+   case PATTR_GUEST_MEM_ENCRYPT:
+   return sev_active();
+
+   case PATTR_SEV_ES:
+   case PATTR_GUEST_PROT_STATE:
+   return sev_es_active();
+
+   default:
+   return false;
+   }
+}
+EXPORT_SYMBOL_GPL(amd_prot_guest_has);
+
 /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
 bool force_dma_unencrypted(struct device *dev)
 {
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
index 43d4dde94793..5ddef1b6a2ea 100644
--- a/include/linux/protected_guest.h
+++ b/include/linux/protected_guest.h
@@ -20,6 +20,11 @@
 #define PATTR_GUEST_MEM_ENCRYPT2   /* Guest encrypted 
memory */
 #define PATTR_GUEST_PROT_STATE 3   /* Guest encrypted state */
 
+/* 0x800 - 0x8ff reserved for AMD */
+#define PATTR_SME  0x800
+#define PATTR_SEV  0x801
+#define PATTR_SEV_ES   0x802
+
 #ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST
 
 #include 
-- 
2.32.0

[PATCH v2 02/12] mm: Introduce a function to check for virtualization protection features

[Tom Lendacky]

In prep for other protected virtualization technologies, introduce a
generic helper function, prot_guest_has(), that can be used to check
for specific protection attributes, like memory encryption. This is
intended to eliminate having to add multiple technology-specific checks
to the code (e.g. if (sev_active() || tdx_active())).

Reviewed-by: Joerg Roedel 
Co-developed-by: Andi Kleen 
Signed-off-by: Andi Kleen 
Co-developed-by: Kuppuswamy Sathyanarayanan 

Signed-off-by: Kuppuswamy Sathyanarayanan 

Signed-off-by: Tom Lendacky 
---
 arch/Kconfig|  3 +++
 include/linux/protected_guest.h | 35 +
 2 files changed, 38 insertions(+)
 create mode 100644 include/linux/protected_guest.h

diff --git a/arch/Kconfig b/arch/Kconfig
index 98db63496bab..bd4f60c581f1 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1231,6 +1231,9 @@ config RELR
 config ARCH_HAS_MEM_ENCRYPT
bool
 
+config ARCH_HAS_PROTECTED_GUEST
+   bool
+
 config HAVE_SPARSE_SYSCALL_NR
bool
help
diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h
new file mode 100644
index ..43d4dde94793
--- /dev/null
+++ b/include/linux/protected_guest.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Protected Guest (and Host) Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky 
+ */
+
+#ifndef _PROTECTED_GUEST_H
+#define _PROTECTED_GUEST_H
+
+#ifndef __ASSEMBLY__
+
+#include 
+#include 
+
+#define PATTR_MEM_ENCRYPT  0   /* Encrypted memory */
+#define PATTR_HOST_MEM_ENCRYPT 1   /* Host encrypted memory */
+#define PATTR_GUEST_MEM_ENCRYPT2   /* Guest encrypted 
memory */
+#define PATTR_GUEST_PROT_STATE 3   /* Guest encrypted state */
+
+#ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST
+
+#include 
+
+#else  /* !CONFIG_ARCH_HAS_PROTECTED_GUEST */
+
+static inline bool prot_guest_has(unsigned int attr) { return false; }
+
+#endif /* CONFIG_ARCH_HAS_PROTECTED_GUEST */
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _PROTECTED_GUEST_H */
-- 
2.32.0

[PATCH v2 01/12] x86/ioremap: Selectively build arch override encryption functions

[Tom Lendacky]

In prep for other uses of the prot_guest_has() function besides AMD's
memory encryption support, selectively build the AMD memory encryption
architecture override functions only when CONFIG_AMD_MEM_ENCRYPT=y. These
functions are:
- early_memremap_pgprot_adjust()
- arch_memremap_can_ram_remap()

Additionally, routines that are only invoked by these architecture
override functions can also be conditionally built. These functions are:
- memremap_should_map_decrypted()
- memremap_is_efi_data()
- memremap_is_setup_data()
- early_memremap_is_setup_data()

And finally, phys_mem_access_encrypted() is conditionally built as well,
but requires a static inline version of it when CONFIG_AMD_MEM_ENCRYPT is
not set.

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Signed-off-by: Tom Lendacky 
---
 arch/x86/include/asm/io.h | 8 
 arch/x86/mm/ioremap.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 841a5d104afa..5c6a4af0b911 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -391,6 +391,7 @@ extern void arch_io_free_memtype_wc(resource_size_t start, 
resource_size_t size)
 #define arch_io_reserve_memtype_wc arch_io_reserve_memtype_wc
 #endif
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
 extern bool arch_memremap_can_ram_remap(resource_size_t offset,
unsigned long size,
unsigned long flags);
@@ -398,6 +399,13 @@ extern bool arch_memremap_can_ram_remap(resource_size_t 
offset,
 
 extern bool phys_mem_access_encrypted(unsigned long phys_addr,
  unsigned long size);
+#else
+static inline bool phys_mem_access_encrypted(unsigned long phys_addr,
+unsigned long size)
+{
+   return true;
+}
+#endif
 
 /**
  * iosubmit_cmds512 - copy data to single MMIO location, in 512-bit units
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 60ade7dd71bd..ccff76cedd8f 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -508,6 +508,7 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
memunmap((void *)((unsigned long)addr & PAGE_MASK));
 }
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
 /*
  * Examine the physical address to determine if it is an area of memory
  * that should be mapped decrypted.  If the memory is not part of the
@@ -746,7 +747,6 @@ bool phys_mem_access_encrypted(unsigned long phys_addr, 
unsigned long size)
return arch_memremap_can_ram_remap(phys_addr, size, 0);
 }
 
-#ifdef CONFIG_AMD_MEM_ENCRYPT
 /* Remap memory with encryption */
 void __init *early_memremap_encrypted(resource_size_t phys_addr,
  unsigned long size)
-- 
2.32.0

[PATCH v2 00/12] Implement generic prot_guest_has() helper function

[Tom Lendacky]

This patch series provides a generic helper function, prot_guest_has(),
to replace the sme_active(), sev_active(), sev_es_active() and
mem_encrypt_active() functions.

It is expected that as new protected virtualization technologies are
added to the kernel, they can all be covered by a single function call
instead of a collection of specific function calls all called from the
same locations.

The powerpc and s390 patches have been compile tested only. Can the
folks copied on this series verify that nothing breaks for them.

Cc: Andi Kleen 
Cc: Andy Lutomirski 
Cc: Ard Biesheuvel 
Cc: Baoquan He 
Cc: Benjamin Herrenschmidt 
Cc: Borislav Petkov 
Cc: Christian Borntraeger 
Cc: Daniel Vetter 
Cc: Dave Hansen 
Cc: Dave Young 
Cc: David Airlie 
Cc: Heiko Carstens 
Cc: Ingo Molnar 
Cc: Joerg Roedel 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Michael Ellerman 
Cc: Paul Mackerras 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Thomas Zimmermann 
Cc: Vasily Gorbik 
Cc: VMware Graphics 
Cc: Will Deacon 

---

Patches based on:
  https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
  0b52902cd2d9 ("Merge branch 'efi/urgent'")

Changes since v1:
- Move some arch ioremap functions within #ifdef CONFIG_AMD_MEM_ENCRYPT
  in prep for use of prot_guest_has() by TDX.
- Add type includes to the the protected_guest.h header file to prevent
  build errors outside of x86.
- Make amd_prot_guest_has() EXPORT_SYMBOL_GPL
- Use amd_prot_guest_has() in place of checking sme_me_mask in the
  arch/x86/mm/mem_encrypt.c file.

Tom Lendacky (12):
  x86/ioremap: Selectively build arch override encryption functions
  mm: Introduce a function to check for virtualization protection
features
  x86/sev: Add an x86 version of prot_guest_has()
  powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
  x86/sme: Replace occurrences of sme_active() with prot_guest_has()
  x86/sev: Replace occurrences of sev_active() with prot_guest_has()
  x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
  treewide: Replace the use of mem_encrypt_active() with
prot_guest_has()
  mm: Remove the now unused mem_encrypt_active() function
  x86/sev: Remove the now unused mem_encrypt_active() function
  powerpc/pseries/svm: Remove the now unused mem_encrypt_active()
function
  s390/mm: Remove the now unused mem_encrypt_active() function

 arch/Kconfig   |  3 ++
 arch/powerpc/include/asm/mem_encrypt.h |  5 --
 arch/powerpc/include/asm/protected_guest.h | 30 +++
 arch/powerpc/platforms/pseries/Kconfig |  1 +
 arch/s390/include/asm/mem_encrypt.h|  2 -
 arch/x86/Kconfig   |  1 +
 arch/x86/include/asm/io.h  |  8 +++
 arch/x86/include/asm/kexec.h   |  2 +-
 arch/x86/include/asm/mem_encrypt.h | 13 +
 arch/x86/include/asm/protected_guest.h | 29 +++
 arch/x86/kernel/crash_dump_64.c|  4 +-
 arch/x86/kernel/head64.c   |  4 +-
 arch/x86/kernel/kvm.c  |  3 +-
 arch/x86/kernel/kvmclock.c |  4 +-
 arch/x86/kernel/machine_kexec_64.c | 19 +++
 arch/x86/kernel/pci-swiotlb.c  |  9 ++--
 arch/x86/kernel/relocate_kernel_64.S   |  2 +-
 arch/x86/kernel/sev.c  |  6 +--
 arch/x86/kvm/svm/svm.c |  3 +-
 arch/x86/mm/ioremap.c  | 18 +++
 arch/x86/mm/mem_encrypt.c  | 60 +++---
 arch/x86/mm/mem_encrypt_identity.c |  3 +-
 arch/x86/mm/pat/set_memory.c   |  3 +-
 arch/x86/platform/efi/efi_64.c |  9 ++--
 arch/x86/realmode/init.c   |  8 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 +-
 drivers/gpu/drm/drm_cache.c|  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c|  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c|  6 +--
 drivers/iommu/amd/init.c   |  7 +--
 drivers/iommu/amd/iommu.c  |  3 +-
 drivers/iommu/amd/iommu_v2.c   |  3 +-
 drivers/iommu/iommu.c  |  3 +-
 fs/proc/vmcore.c   |  6 +--
 include/linux/mem_encrypt.h|  4 --
 include/linux/protected_guest.h| 40 +++
 kernel/dma/swiotlb.c   |  4 +-
 37 files changed, 232 insertions(+), 105 deletions(-)
 create mode 100644 arch/powerpc/include/asm/protected_guest.h
 create mode 100644 arch/x86/include/asm/protected_guest.h
 create mode 100644 include/linux/protected_guest.h

-- 
2.32.0

[pull] amdgpu, amdkfd drm-next-5.15

[Alex Deucher]

Hi Dave, Daniel,

Updates for 5.15.  Mostly bug fixes and cleanups.

The following changes since commit a43e2a0e11491b73e2acaa27ee74d6c3b86deac0:

  drm/amdkfd: Allow querying SVM attributes that are clear (2021-08-06 16:12:32 
-0400)

are available in the Git repository at:

  https://gitlab.freedesktop.org/agd5f/linux.git 
tags/amd-drm-next-5.15-2021-08-13

for you to fetch changes up to 554594567b1fa3da74f88ec7b2dc83d000c58e98:

  drm/display: fix possible null-pointer dereference in dcn10_set_clock() 
(2021-08-11 17:19:54 -0400)


amd-drm-next-5.15-2021-08-13:

amdgpu:
- Improve aux i2c tracing
- Misc display updates
- Misc code cleanups
- sprintf to sysfs_emit updates
- Fix some fan control corner cases with suspend

amdkfd:
- Enable CWSR with software scheduling


Alex Deucher (1):
  drm/amdgpu: handle VCN instances when harvesting (v2)

Anson Jacob (1):
  drm/amd/display: use GFP_ATOMIC in amdgpu_dm_irq_schedule_work

Anthony Koo (2):
  drm/amd/display: [FW Promotion] Release 0.0.78
  drm/amd/display: 3.2.148

Ashley Thomas (1):
  drm/amd/display: Add AUX I2C tracing.

Darren Powell (7):
  amdgpu/pm: Replace navi10 usage of sprintf with sysfs_emit
  amdgpu/pm: Replace smu11 usage of sprintf with sysfs_emit
  amdgpu/pm: Replace smu12/13 usage of sprintf with sysfs_emit
  amdgpu/pm: Replace vega10 usage of sprintf with sysfs_emit
  amdgpu/pm: Replace vega12,20 usage of sprintf with sysfs_emit
  amdgpu/pm: Replace hwmgr smu usage of sprintf with sysfs_emit
  amdgpu/pm: Replace amdgpu_pm usage of sprintf with sysfs_emit

Eric Bernstein (1):
  drm/amd/display: Remove invalid assert for ODM + MPC case

Mukul Joshi (1):
  drm/amdkfd: CWSR with software scheduler

Nicholas Kazlauskas (2):
  drm/amd/display: Clear GPINT after DMCUB has reset
  drm/amd/display: Increase timeout threshold for DMCUB reset

Philip Yang (1):
  drm/amdkfd: AIP mGPUs best prefetch location for xnack on

Randy Dunlap (2):
  drm/amd/display: use do-while-0 for DC_TRACE_LEVEL_MESSAGE()
  drm/amdgpu: fix kernel-doc warnings on non-kernel-doc comments

Roy Chan (5):
  drm/amd/display: fix missing writeback disablement if plane is removed
  drm/amd/display: refactor the codes to centralize the stream/pipe 
checking logic
  drm/amd/display: refactor the cursor programing codes
  drm/amd/display: fix incorrect CM/TF programming sequence in dwb
  drm/amd/display: Correct comment style

Ryan Taylor (2):
  drm/amd/pm: restore fan_mode AMD_FAN_CTRL_NONE on resume (v2)
  drm/amd/pm: graceful exit on restore fan mode failure (v2)

Sergio Miguéns Iglesias (1):
  drm/amdgpu: Removed unnecessary if statement

Tuo Li (2):
  gpu: drm: amd: amdgpu: amdgpu_i2c: fix possible uninitialized-variable 
access in amdgpu_i2c_router_select_ddc_port()
  drm/display: fix possible null-pointer dereference in dcn10_set_clock()

Victor Zhao (1):
  drm/amdgpu: Extend full access wait time in guest

Wenjing Liu (1):
  drm/amd/display: add authentication_complete in hdcp output

YuBiao Wang (1):
  drm/amd/amdgpu: skip locking delayed work if not initialized.

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c |  31 
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10_3.c   |  31 
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c  |  33 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c  |  12 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |   3 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c|   2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_4_2.c|   6 +-
 drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c  |  16 +-
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  |  21 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c   |  35 ++--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c  |   2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  62 ---
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 106 +++-
 drivers/gpu/drm/amd/display/dc/dc.h|   2 +-
 drivers/gpu/drm/amd/display/dc/dce/dce_aux.c   | 192 -
 drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c  |   2 +-
 .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c  |  11 +-
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c |  14 +-
 .../gpu/drm/amd/display/dc/dcn30/dcn30_dwb_cm.c|  90 +++---
 drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c |  12 +-
 .../gpu/drm/amd/display/dc/dcn30/dcn30_resource.c  |   1 -
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h|   6 +-
 drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c  |  18 +-
 drivers/gpu/drm/amd/display/modules/hdcp/hdcp.c|   5 +-
 drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h|   8 +
 .../amd/display/modules/hdcp/hdcp1_transition.c|   

Re: [PATCH v5 3/9] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks

[Matthew Wilcox]

On Fri, Aug 13, 2021 at 06:51:05PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 13, 2021 at 09:17:11AM -0600, Jim Cromie wrote:
> > +int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
> > +{
> > +   unsigned long inbits;
> > +   int rc, i, chgct = 0, totct = 0;
> > +   char query[OUR_QUERY_SIZE];
> > +   struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data;
> 
> So you need space after ')' ?

More importantly, if ->data is of type 'void *', it is bad style to
cast the pointer at all.  I can't tell what type 'data' has; if it
is added to kernel_param as part of this series, I wasn't cc'd on
the patch that did that.

Re: [PATCH v5 3/9] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks

[Andy Shevchenko]

On Fri, Aug 13, 2021 at 09:17:11AM -0600, Jim Cromie wrote:
> DEFINE_DYNAMIC_DEBUG_CATEGORIES(name, var, bitmap_desc, @bit_descs)
> allows users to define a drm.debug style (bitmap) sysfs interface, and
> to specify the desired mapping from bits[0-N] to the format-prefix'd
> pr_debug()s to be controlled.
> 
> DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
>   "i915/gvt bitmap desc",
>   /**
>* search-prefixes, passed to dd-exec_queries
>* defines bits 0-N in order.
>* leading ^ is tacitly inserted (by callback currently)
>* trailing space used here excludes subcats.
>* helper macro needs more work
>* macro to autogen ++$i, 0x%x$i ?
>*/
>   _DD_cat_("gvt:cmd: "),
>   _DD_cat_("gvt:core: "),
>   _DD_cat_("gvt:dpy: "),
>   _DD_cat_("gvt:el: "),
>   _DD_cat_("gvt:irq: "),
>   _DD_cat_("gvt:mm: "),
>   _DD_cat_("gvt:mmio: "),
>   _DD_cat_("gvt:render: "),
>   _DD_cat_("gvt:sched: "));
> 
> dynamic_debug.c: add 3 new elements:
> 
>  - int param_set_dyndbg()
>  - int param_get_dyndbg()
>  - struct kernel_param_ops param_ops_dyndbg
> 
> Following the model of kernel/params.c STANDARD_PARAM_DEFS, All 3 are
> non-static and exported.
> 
> dynamic_debug.h:
> 
> Add DEFINE_DYNAMIC_DEBUG_CATEGORIES() described above, and a do-nothing stub.
> 
> Note that it also calls MODULE_PARM_DESC for the user, but expects the
> user to catenate all the bit-descriptions together (as is done in
> drm.debug), and in the following uses in amdgpu, i915.
> 
> This in the hope that someone can offer an auto-incrementing
> label-generating macro, producing "\tbit-4 0x10\t" etc, and can show
> how to apply it to __VA_ARGS__.
> 
> Also extern the struct kernel_param param_ops_dyndbg symbol, as is
> done in moduleparams.h for all the STANDARD params.
> 
> USAGE NOTES:
> 
> Using dyndbg to query on "format ^$prefix" requires that the prefix be
> present in the compiled-in format string; where run-time prefixing is
> used, that format would be "%s...", which is not usefully selectable.
> 
> Adding structural query terms (func,file,lineno) could help (module is
> already done), but DEFINE_DYNAMIC_DEBUG_CATEGORIES can't do that now,
> adding it needs a better reason imo.
> 
> Dyndbg is completely agnostic wrt the categorization scheme used, to
> play well with any prefix convention already in use.  Ad-hoc
> categories and sub-categories are implicitly allowed, author
> discipline and review is expected.
> 
> Here are some examples:
> 
> "1","2","3"   2 doesnt imply 1.
>   otherwize, sorta like printk levels
> "1:","2:","3:"are better, avoiding [1-9]\d+ ambiguity
> "hi:","mid:","low:"   are reasonable, and imply independence
> "todo:","rfc:"might be handy
> "A:".."Z:"uhm, yeah
> 
> Hierarchical classes/categories are natural:
> 
> "drm::"  is used in later commit
> "drm:::"is a natural extension.
> "drm:atomic:fail:"has been proposed, sounds directly useful
> 
> Some properties of a hierarchical category deserve explication:
> 
> Trailing spaces matter !
> 
> With 1..3-space ("drm: ", "drm:atomic: ", "drm:atomic:fail: "), the
> ":" doesnt terminate the search-space, the trailing space does.
> So a "drm:" search specification will match all DRM categories &
> subcategories, and will not be useful in an interface where all
> categories are controlled together.  That said, "drm:atomic:" &
> "drm:atomic: " are different, and both are useful in cases.
> 
> Ad-Hoc sub-categories:
> 
> These have a caveat wrt wrapper macros adding prefixes like
> "drm:atomic: "; the trailing space in the prefix means that
> drm_dbg("fail: ...") renders as "drm:atomic: fail: ", which obviously
> isn't ideal wrt clear and simple bitmaps.
> 
> A possible solution is to have a FOO_() version of every FOO() macro
> which (anti-mnemonically) elides the trailing space, which is normally
> inserted by a modified FOO().  Doing this would enforce a policy
> decision that "debug categories will be space terminated", with an
> pressure-relief valve.
> 
> Summarizing:
> 
>  - "drm:kms: " & "drm:kms:" are different
>  - "drm:kms"  also different - includes drm:kms2:
>  - "drm:kms:\t"   also different
>  - "drm:kms:*"doesnt work, no wildcard on format atm.
> 
> Order matters in DEFINE_DYNAMIC_DEBUG_CATEGORIES(... @bit_descs)
> 
> @bit_descs (array) position determines the bit mapping to the prefix,
> so to keep a stable map, new categories or 3rd level categories must
> be added to the end.
> 
> Since bits are/will-stay applied 0-N, the later bits can countermand
> the earlier ones, but its tricky - consider;
> 
> DD_CATs(... "drm:atomic:", ""drm:atomic:fail:" ) // misleading
> 
> The 1st search-term is misleading, because it includes (modifies)
> subcategories, but then 2nd overrides it.  So don't do that.
> 
> There is still plenty of bikeshedding 

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Michel Dänzer]

On 2021-08-13 5:07 p.m., Lazar, Lijo wrote:
> 
> 
> On 8/13/2021 8:10 PM, Michel Dänzer wrote:
>> On 2021-08-13 4:14 p.m., Lazar, Lijo wrote:
>>> On 8/13/2021 7:04 PM, Michel Dänzer wrote:
 On 2021-08-13 1:50 p.m., Lazar, Lijo wrote:
> On 8/13/2021 3:59 PM, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> schedule_delayed_work does not push back the work if it was already
>> scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
>> after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
>> was disabled and re-enabled again during those 100 ms.
>>
>> This resulted in frame drops / stutter with the upcoming mutter 41
>> release on Navi 14, due to constantly enabling GFXOFF in the HW and
>> disabling it again (for getting the GPU clock counter).
>>
>> To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
>> enabled to disabled. This makes sure the delayed work will be scheduled
>> as intended in the reverse case.
>>
>> In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
>> to use mutex_trylock instead of mutex_lock.
>>
>> v2:
>> * Use cancel_delayed_work_sync & mutex_trylock instead of
>>  mod_delayed_work.
>>
>> Signed-off-by: Michel Dänzer 
>> ---
>>     drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++--
>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
>>     3 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index f3fd5ec710b6..8b025f70706c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2777,7 +2777,16 @@ static void 
>> amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
>>     struct amdgpu_device *adev =
>>     container_of(work, struct amdgpu_device, 
>> gfx.gfx_off_delay_work.work);
>>     -    mutex_lock(>gfx.gfx_off_mutex);
>> +    /* mutex_lock could deadlock with cancel_delayed_work_sync in 
>> amdgpu_gfx_off_ctrl. */
>> +    if (!mutex_trylock(>gfx.gfx_off_mutex)) {
>> +    /* If there's a bug which causes amdgpu_gfx_off_ctrl to be 
>> called with enable=true
>> + * when adev->gfx.gfx_off_req_count is already 0, we might race 
>> with that.
>> + * Re-schedule to make sure gfx off will be re-enabled in the 
>> HW eventually.
>> + */
>> +    schedule_delayed_work(>gfx.gfx_off_delay_work, 
>> AMDGPU_GFX_OFF_DELAY_ENABLE);
>> +    return;
>
> This is not needed and is just creating another thread to contend for 
> mutex.

 Still not sure what you mean by that. What other thread?
>>>
>>> Sorry, I meant it schedules another workitem and delays GFXOFF enablement 
>>> further. For ex: if it was another function like gfx_off_status holding the 
>>> lock at the time of check.
>>>

> The checks below take care of enabling gfxoff correctly. If it's already 
> in gfx_off state, it doesn't do anything. So I don't see why this change 
> is needed.

 mutex_trylock is needed to prevent the deadlock discussed before and below.

 schedule_delayed_work is needed due to this scenario hinted at by the 
 comment:

 1. amdgpu_gfx_off_ctrl locks mutex, calls schedule_delayed_work
 2. amdgpu_device_delay_enable_gfx_off runs, calls mutex_trylock, which 
 fails

 GFXOFF would never get re-enabled in HW in this case (until 
 amdgpu_gfx_off_ctrl calls schedule_delayed_work again).

 (cancel_delayed_work_sync guarantees there's no pending delayed work when 
 it returns, even if amdgpu_device_delay_enable_gfx_off calls 
 schedule_delayed_work)

>>>
>>> I think we need to explain based on the original code before. There is an 
>>> asssumption here that the only other contention of this mutex is with the 
>>> gfx_off_ctrl function.
>>
>> Not really.
>>
>>
>>> As far as I understand if the work has already started running when 
>>> schedule_delayed_work is called, it will insert another in the work queue 
>>> after delay. Based on that understanding I didn't find a problem with the 
>>> original code.
>>
>> Original code as in without this patch or the mod_delayed_work patch? If so, 
>> the problem is not when the work has already started running. It's that when 
>> it hasn't started running yet, schedule_delayed_work doesn't change the 
>> timeout for the already scheduled work, so it ends up enabling GFXOFF 
>> earlier than intended (and thus at all in scenarios when it's not supposed 
>> to).
>>
> 
> I meant the original implementation of amdgpu_device_delay_enable_gfx_off().
> 
> 
> If you indeed want to use _sync, 

Re: [PATCH v5 2/9] moduleparam: add data member to struct kernel_param

[Andy Shevchenko]

On Fri, Aug 13, 2021 at 09:17:10AM -0600, Jim Cromie wrote:
> Add a const void* data member to the struct, to allow attaching
> private data that will be used soon by a setter method (via kp->data)
> to perform more elaborate actions.
> 
> To attach the data at compile time, add new macros:
> 
> module_param_cbd() derives from module_param_cb(), adding data param,
> and latter is redefined to use former.
> 
> It calls __module_param_call_wdata(), which accepts a new data param
> and inits .data with it. Re-define __module_param_call() to use it.
> 
> Use of this new data member will be rare, it might be worth redoing
> this as a separate/sub-type to de-bloat the base case.

...

> +#define module_param_cbd(name, ops, arg, perm, data) 
> \
> + __module_param_call_wdata(MODULE_PARAM_PREFIX, name, ops, arg, perm, 
> -1, 0, data)

Cryptic name. Moreover, inconsistent with the rest.
What about module_param_cb_data() ?

>  #define module_param_cb_unsafe(name, ops, arg, perm)   \
>   __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1,\
>   KERNEL_PARAM_FL_UNSAFE)

(above left for the above comment)

...

> +#define __module_param_call_wdata(prefix, name, ops, arg, perm, level, 
> flags, data) \

Similar __module_param_call_with_data()

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 9/9] dyndbg: RFC add tracer facility RFC

[Jim Cromie]

Sean Paul seanp...@chromium.org proposed, in
https://patchwork.freedesktop.org/series/78133/
drm/trace: Mirror DRM debug logs to tracefs

That patchset's goal is to be able to duplicate the debug stream to a
tracing destination, by splitting drm_debug_enabled() into syslog &
trace flavors, and enabling them separately.  That clashes rather
deeply with this patchset's goal; to avoid drm_debug_enabled() using
dyndbg.

Instead, this puts the print-to-trace decision in dyndbg, after the
is-it-enabled test (which is a noop), so it has near zero additional
cost (other than memory increase); the print-to-trace test is only
done on enabled callsites.

The basic elements:

 - add a new struct _ddebug member: (*tracer)(char *format, ...)
 - add a new T flag to enable tracer
 - adjust the static-key-enable/disable condition for (p|T)
 - if (p) before printk, since T enables too.
 - if (T) call tracer if its true

 = int dynamic_debug_register_tracer(query, modname, tracer);
 = int dynamic_debug_unregister_tracer(query, modname, cookie);

This new interface lets clients set/unset a tracer function on each
callsite matching the query, for example: "drm:atomic:fail:".

Clients are expected to unregister the same callsites they register (a
cookie), allowing protection of each client's dyndbg-state setup
against overwrites by others.

Intended Behavior: (things are in flux, RFC)

- register sets empty slot, doesnt overwrite
  the query selects callsites, and sets +T (grammar requires +-action)

- register allows same-tracer over-set wo warn
  2nd register can then enable superset, subset, disjoint set

- unregister clears slot if it matches cookie/tracer
  query selects set, -T (as tested)
  tolerates over-clears

- dd-exec-queries(+/-T) can modify the enablements
  not sure its needed, but it falls out..

The code is currently in-line in ddebug_change(), to be moved to
separate fn, rc determines flow, may also veto/alter changes by
altering flag-settings - tbd.

TBD: Im not sure what happens when exec-queries(+T) hits a site wo a
tracer (silence I think. maybe not ideal).

internal call-chain gets a tracer param:
New arg:
public: dynamic_debug_exec_queries
ro-string copy moved ...
1   ddebug_exec_queries tracer=NULL
... to here
2   ddebug_exec_query   tracer=NULL

call-chain gets (re)used: with !NULL

public: dynamic_debug_register_tracer   tracer=client's
w ro-string
1   ddebug_exec_queries tracer
...

SELFTEST: test_dynamic_debug.ko:

Uses the tracer facility to do a selftest:

- A custom tracer counts the number of calls (of T-enabled pr_debugs),
- do_debugging(x) calls a set of categorized pr_debugs x times

- test registers the tracer on the function,
  then iteratively:
  manipulates dyndbg states via query-cmds
  runs do_debugging()
  counts enabled callsite executions
  reports mismatches

- modprobe test_dynamic_debug use_bad_tracer=1
  attaches a bad/recursive tracer
  Bad Things Happen.
  has thrown me interesting panics.

NOTES:

This needs more work. RFC.

ERRORS (or WARNINGS):

It should be an error to +T a callsite which has no aux_print set (ie
already registered with a query that selected that callsite).  This
tacitly enforces registration.

Then +T,-T can toggle those aux_print callsites (or subsets of them)
to tailor the debug-stream for the purpose.  Controlling flow is the
best work limiter.

---
v4+: (this patch sent after (on top of) v4)

. fix "too many arguments to function", and name the args:
  int (*aux_print)(const char *fmt, char *prefix, char *label, void *);
   prefix : is a slot for dynamic_emit_prefix, or for custom buffer insert
   label  : for builtin-caller used by drm-trace-print
   void*  : vaf, add type constraint later.

. fix printk (to syslog) needs if (+p), since +T also enables
. add prototypes for un/register_aux_print
. change iface names: s/aux_print/tracer/
. also s/trace_print/tracer/
. struct va_format *vaf - tighten further ?

Signed-off-by: Jim Cromie 
---
 include/linux/dynamic_debug.h |  32 -
 lib/Kconfig.debug |  10 ++
 lib/Makefile  |   1 +
 lib/dynamic_debug.c   | 109 +++
 lib/test_dynamic_debug.c  | 247 ++
 5 files changed, 372 insertions(+), 27 deletions(-)
 create mode 100644 lib/test_dynamic_debug.c

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 42cfb37d4870..cbcb1c94cec3 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -20,6 +20,7 @@ struct _ddebug {
const char *function;
const char *filename;
const char *format;
+   int (*tracer)(const char *fmt, char *prefix, char *label, struct 
va_format *vaf);
unsigned int lineno:18;
/*
 * The flags field controls the behaviour at the callsite.
@@ -27,7 +28,11 @@ struct _ddebug {
 * writes commands 

[PATCH v5 8/9] amdgpu_ucode: reduce number of pr_debug calls

[Jim Cromie]

There are blocks of DRM_DEBUG calls, consolidate their args into
single calls.  With dynamic-debug in use, each callsite consumes 56
bytes of ro callsite data, and this patch removes about 65 calls, so
it saves ~3.5kb.

no functional changes.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 293 --
 1 file changed, 158 insertions(+), 135 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 2834981f8c08..14a9fef1f4c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -30,17 +30,26 @@
 
 static void amdgpu_ucode_print_common_hdr(const struct common_firmware_header 
*hdr)
 {
-   DRM_DEBUG("size_bytes: %u\n", le32_to_cpu(hdr->size_bytes));
-   DRM_DEBUG("header_size_bytes: %u\n", 
le32_to_cpu(hdr->header_size_bytes));
-   DRM_DEBUG("header_version_major: %u\n", 
le16_to_cpu(hdr->header_version_major));
-   DRM_DEBUG("header_version_minor: %u\n", 
le16_to_cpu(hdr->header_version_minor));
-   DRM_DEBUG("ip_version_major: %u\n", le16_to_cpu(hdr->ip_version_major));
-   DRM_DEBUG("ip_version_minor: %u\n", le16_to_cpu(hdr->ip_version_minor));
-   DRM_DEBUG("ucode_version: 0x%08x\n", le32_to_cpu(hdr->ucode_version));
-   DRM_DEBUG("ucode_size_bytes: %u\n", le32_to_cpu(hdr->ucode_size_bytes));
-   DRM_DEBUG("ucode_array_offset_bytes: %u\n",
- le32_to_cpu(hdr->ucode_array_offset_bytes));
-   DRM_DEBUG("crc32: 0x%08x\n", le32_to_cpu(hdr->crc32));
+   DRM_DEBUG("size_bytes: %u\n"
+ "header_size_bytes: %u\n"
+ "header_version_major: %u\n"
+ "header_version_minor: %u\n"
+ "ip_version_major: %u\n"
+ "ip_version_minor: %u\n"
+ "ucode_version: 0x%08x\n"
+ "ucode_size_bytes: %u\n"
+ "ucode_array_offset_bytes: %u\n"
+ "crc32: 0x%08x\n",
+ le32_to_cpu(hdr->size_bytes),
+ le32_to_cpu(hdr->header_size_bytes),
+ le16_to_cpu(hdr->header_version_major),
+ le16_to_cpu(hdr->header_version_minor),
+ le16_to_cpu(hdr->ip_version_major),
+ le16_to_cpu(hdr->ip_version_minor),
+ le32_to_cpu(hdr->ucode_version),
+ le32_to_cpu(hdr->ucode_size_bytes),
+ le32_to_cpu(hdr->ucode_array_offset_bytes),
+ le32_to_cpu(hdr->crc32));
 }
 
 void amdgpu_ucode_print_mc_hdr(const struct common_firmware_header *hdr)
@@ -55,9 +64,9 @@ void amdgpu_ucode_print_mc_hdr(const struct 
common_firmware_header *hdr)
const struct mc_firmware_header_v1_0 *mc_hdr =
container_of(hdr, struct mc_firmware_header_v1_0, 
header);
 
-   DRM_DEBUG("io_debug_size_bytes: %u\n",
- le32_to_cpu(mc_hdr->io_debug_size_bytes));
-   DRM_DEBUG("io_debug_array_offset_bytes: %u\n",
+   DRM_DEBUG("io_debug_size_bytes: %u\n"
+ "io_debug_array_offset_bytes: %u\n",
+ le32_to_cpu(mc_hdr->io_debug_size_bytes),
  le32_to_cpu(mc_hdr->io_debug_array_offset_bytes));
} else {
DRM_ERROR("Unknown MC ucode version: %u.%u\n", version_major, 
version_minor);
@@ -82,13 +91,17 @@ void amdgpu_ucode_print_smc_hdr(const struct 
common_firmware_header *hdr)
switch (version_minor) {
case 0:
v2_0_hdr = container_of(hdr, struct 
smc_firmware_header_v2_0, v1_0.header);
-   DRM_DEBUG("ppt_offset_bytes: %u\n", 
le32_to_cpu(v2_0_hdr->ppt_offset_bytes));
-   DRM_DEBUG("ppt_size_bytes: %u\n", 
le32_to_cpu(v2_0_hdr->ppt_size_bytes));
+   DRM_DEBUG("ppt_offset_bytes: %u\n"
+ "ppt_size_bytes: %u\n",
+ le32_to_cpu(v2_0_hdr->ppt_offset_bytes),
+ le32_to_cpu(v2_0_hdr->ppt_size_bytes));
break;
case 1:
v2_1_hdr = container_of(hdr, struct 
smc_firmware_header_v2_1, v1_0.header);
-   DRM_DEBUG("pptable_count: %u\n", 
le32_to_cpu(v2_1_hdr->pptable_count));
-   DRM_DEBUG("pptable_entry_offset: %u\n", 
le32_to_cpu(v2_1_hdr->pptable_entry_offset));
+   DRM_DEBUG("pptable_count: %u\n"
+ "pptable_entry_offset: %u\n",
+ le32_to_cpu(v2_1_hdr->pptable_count),
+ le32_to_cpu(v2_1_hdr->pptable_entry_offset));
break;
default:
break;
@@ -111,10 +124,12 @@ void amdgpu_ucode_print_gfx_hdr(const struct 
common_firmware_header *hdr)
  

[PATCH v5 7/9] drm_print: add choice to use dynamic debug in drm-debug

[Jim Cromie]

drm's debug system writes 10 distinct categories of messages to syslog
using a small API[1]: drm_dbg*(10 names), DRM_DEBUG*(8 names),
DRM_DEV_DEBUG*(3 names).  There are thousands of these callsites, each
categorized by their authors.

These callsites can be enabled at runtime by their category, each
controlled by a bit in drm.debug (/sys/modules/drm/parameter/debug).
In the current "basic" implementation, drm_debug_enabled() tests these
bits in __drm_debug each time an API[1] call is executed; while cheap
individually, the costs accumulate.

This patch uses dynamic-debug with jump-label to patch enabled calls
onto their respective NOOP slots, avoiding all runtime bit-checks of
__drm_debug.

Dynamic debug has no concept of category, but we can emulate one by
replacing enum categories with a set of prefix-strings; "drm:core:",
"drm:kms:" "drm:driver:" etc, and prepend them (at compile time) to
the given formats.

Then we can use:
  `echo module drm format "^drm:core: " +p > control`

to enable the whole category with one query.

This conversion yields ~2100 new callsites on my i7/i915 laptop:

  dyndbg: 195 debug prints in module drm_kms_helper
  dyndbg: 298 debug prints in module drm
  dyndbg: 1630 debug prints in module i915

CONFIG_DRM_USE_DYNAMIC_DEBUG enables this, and is available if
CONFIG_DYNAMIC_DEBUG or CONFIG_DYNAMIC_DEBUG_CORE is chosen, and if
CONFIG_JUMP_LABEL is enabled; this because its required to get the
promised optimizations.

The "basic" -> "dyndbg" switchover is layered into the macro scheme

A. use DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
"DRM debug category-per-bit control",
{ "drm:core:", "enable CORE debug messages" },
{ "drm:kms:", "enable KMS debug messages" }, ...);

B. A "classy" version of DRM_UT_ map, named DRM_DBG_CAT_

   DRM_DBG_CLASS_ was proposed, I had agreed, but reconsidered;
   CATEGORY is already DRM's term-of-art, and adding a near-synonym
   'CLASS' only adds ambiguity.

   "basic":  DRM_DBG_CAT_  <===  DRM_UT_.  Identity map.
   "dyndbg":
 #define DRM_DBG_CAT_KMS"drm:kms: "
 #define DRM_DBG_CAT_PRIME  "drm:prime: "
 #define DRM_DBG_CAT_ATOMIC "drm:atomic: "

   DRM_UT_* are preserved, since theyre used elsewhere.
   We can probably reduce its use further, but thats a separate thing.

C. drm_dev_dbg() & drm_debug() are interposed with macros

 basic: forward to renamed fn, with args preserved
 enabled:   redirect to pr_debug, dev_dbg, with CATEGORY # format

   this is where drm_debug_enabled() is avoided.
   prefix is prepended at compile-time, no category at runtime.

D. API[1] uses DRM_DBG_CAT_s
   these already use (C), now they use (B) too,
   to get the correct token type for "basic" and "dyndbg" configs.

NOTES:

Code Review is expected to catch lack of correspondence between
bit=>prefix definitions (the selector) and the prefixes used in the
API[1] layer above pr_debug()

I've coded the search-prefixes/categories with a trailing space, which
excludes any sub-categories added later.  This convention protects any
"drm:atomic:fail:" callsites from getting stomped on by `echo 0 > debug`.
Other categories could differ, but we need some default.

Dyndbg requires that the prefix be in the compiled-in format string;
run-time prefixing evades callsite selection by category.

pr_debug("%s: ...", __func__, ...) // not ideal

With "lineno X" in a query, its possible to enable single callsites,
but it is tedious, and useless in a category context.

Unfortunately __func__ is not a macro, and cannot be catenated at
preprocess/compile time.

pr_debug("Entry: ...")  // +fml gives useful log-info
pr_debug("Exit: ...")   // hard to catch them all

But "func foo" added to query-command would work, should it be useful
enough to justify extending the declarative interface.

---
v4+:

. use DEFINE_DYNAMIC_DEBUG_CATEGORIES in drm_print.c
. s/DRM_DBG_CLASS_/DRM_DBG_CAT_/ - dont need another term
. default=y in KBuild entry - per @DanVet
. move some commit-log prose to dyndbg commit
. add-prototyes to (param_get/set)_dyndbg
. more wrinkles found by 
. relocate ratelimit chunk from elsewhere
. add kernel doc

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/Kconfig |  13 
 drivers/gpu/drm/drm_print.c |  49 +
 include/drm/drm_print.h | 141 
 3 files changed, 159 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 7ff89690a976..97e38d86fd27 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -57,6 +57,19 @@ config DRM_DEBUG_MM
 
  If in doubt, say "N".
 
+config DRM_USE_DYNAMIC_DEBUG
+   bool "use dynamic debug to implement drm.debug"
+   default y
+   depends on DRM
+   depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+   depends on JUMP_LABEL
+   help
+ The "basic" drm.debug facility does a lot 

[PATCH v5 6/9] amdgpu: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to control categorized pr_debugs

[Jim Cromie]

logger_types.h defines many DC_LOG_*() categorized debug wrappers.
Most of these use DRM debug API, so are controllable using drm.debug,
but others use bare pr_debug("$prefix: .."), each with a different
class-prefix matching "^\[\w+\]:"

Use DEFINE_DYNAMIC_DEBUG_CATEGORIES to create a /sys debug_dc
parameter, modinfos, and to specify a map from bits -> categorized
pr_debugs to be controlled.

Signed-off-by: Jim Cromie 
---
 .../gpu/drm/amd/display/dc/core/dc_debug.c| 44 ++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
index 21be2a684393..69e68d721512 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
@@ -36,8 +36,50 @@
 
 #include "resource.h"
 
-#define DC_LOGGER_INIT(logger)
+#ifdef DRM_USE_DYNAMIC_DEBUG
+/* define a drm.debug style dyndbg pr-debug control point */
+#include 
+
+unsigned long __debug_dc;
+EXPORT_SYMBOL(__debug_dc);
+
+#define _help_(key)"\t   " key "\t- help for " key "\n"
+
+/* Id like to do these inside DEFINE_DYNAMIC_DEBUG_CATEGORIES, if possible */
+#define DC_DYNDBG_BITMAP_DESC(name)\
+   "Control pr_debugs via /sys/module/amdgpu/parameters/" #name\
+   ", where each bit controls a debug category.\n" \
+   _help_("[SURFACE]:")\
+   _help_("[CURSOR]:") \
+   _help_("[PFLIP]:")  \
+   _help_("[VBLANK]:") \
+   _help_("[HW_LINK_TRAINING]:")   \
+   _help_("[HW_AUDIO]:")   \
+   _help_("[SCALER]:") \
+   _help_("[BIOS]:")   \
+   _help_("[BANDWIDTH_CALCS]:")\
+   _help_("[DML]:")\
+   _help_("[IF_TRACE]:")   \
+   _help_("[GAMMA]:")  \
+   _help_("[SMU_MSG]:")
+
+DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_dc, __debug_dc,
+   DC_DYNDBG_BITMAP_DESC(debug_dc),
+   _DD_cat_("[CURSOR]:"),
+   _DD_cat_("[PFLIP]:"),
+   _DD_cat_("[VBLANK]:"),
+   _DD_cat_("[HW_LINK_TRAINING]:"),
+   _DD_cat_("[HW_AUDIO]:"),
+   _DD_cat_("[SCALER]:"),
+   _DD_cat_("[BIOS]:"),
+   _DD_cat_("[BANDWIDTH_CALCS]:"),
+   _DD_cat_("[DML]:"),
+   _DD_cat_("[IF_TRACE]:"),
+   _DD_cat_("[GAMMA]:"),
+   _DD_cat_("[SMU_MSG]:"));
+#endif
 
+#define DC_LOGGER_INIT(logger)
 
 #define SURFACE_TRACE(...) do {\
if (dc->debug.surface_trace) \
-- 
2.31.1

[PATCH v5 5/9] i915/gvt: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to create "gvt:core:" etc categories

[Jim Cromie]

The gvt component of this driver has ~120 pr_debugs, in 9 categories
quite similar to those in DRM.  Following the interface model of
drm.debug, add a parameter to map bits to these categorizations.

DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
"dyndbg bitmap desc",
{ "gvt:cmd: ",  "command processing" },
{ "gvt:core: ", "core help" },
{ "gvt:dpy: ",  "display help" },
{ "gvt:el: ",   "help" },
{ "gvt:irq: ",  "help" },
{ "gvt:mm: ",   "help" },
{ "gvt:mmio: ", "help" },
{ "gvt:render: ", "help" },
{ "gvt:sched: " "help" });

The actual patch has a few details different, cmd_help() macro emits
the initialization construct.

if CONFIG_DRM_USE_DYNAMIC_DEBUG, then -DDYNAMIC_DEBUG_MODULE is added
cflags, by gvt/Makefile.

---
v4+:
. static decl of vector of bit->class descriptors - Emil.V
. relocate gvt-makefile chunk from elsewhere

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/i915/gvt/Makefile  |  4 
 drivers/gpu/drm/i915/i915_params.c | 35 ++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
b/drivers/gpu/drm/i915/gvt/Makefile
index ea8324abc784..846ba73b8de6 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -7,3 +7,7 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o 
trace_points.o firmware.o \
 
 ccflags-y  += -I $(srctree)/$(src) -I 
$(srctree)/$(src)/$(GVT_DIR)/
 i915-y += $(addprefix $(GVT_DIR)/, 
$(GVT_SOURCE))
+
+#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
+ccflags-y  += -DDYNAMIC_DEBUG_MODULE
+#endif
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..683e942a074e 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -265,3 +265,38 @@ void i915_params_free(struct i915_params *params)
I915_PARAMS_FOR_EACH(FREE);
 #undef FREE
 }
+
+#ifdef DRM_USE_DYNAMIC_DEBUG
+/* todo: needs DYNAMIC_DEBUG_MODULE in some cases */
+
+unsigned long __gvt_debug;
+EXPORT_SYMBOL(__gvt_debug);
+
+#define _help(key) "\t\"" key "\"\t: help for " key "\n"
+
+#define I915_GVT_CATEGORIES(name) \
+   " Enable debug output via /sys/module/i915/parameters/" #name   \
+   ", where each bit enables a debug category.\n"  \
+   _help("gvt:cmd:")   \
+   _help("gvt:core:")  \
+   _help("gvt:dpy:")   \
+   _help("gvt:el:")\
+   _help("gvt:irq:")   \
+   _help("gvt:mm:")\
+   _help("gvt:mmio:")  \
+   _help("gvt:render:")\
+   _help("gvt:sched:")
+
+DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
+   I915_GVT_CATEGORIES(debug_gvt),
+   _DD_cat_("gvt:cmd:"),
+   _DD_cat_("gvt:core:"),
+   _DD_cat_("gvt:dpy:"),
+   _DD_cat_("gvt:el:"),
+   _DD_cat_("gvt:irq:"),
+   _DD_cat_("gvt:mm:"),
+   _DD_cat_("gvt:mmio:"),
+   _DD_cat_("gvt:render:"),
+   _DD_cat_("gvt:sched:"));
+
+#endif
-- 
2.31.1

[PATCH v5 4/9] i915/gvt: remove spaces in pr_debug "gvt: core:" etc prefixes

[Jim Cromie]

Taking embedded spaces out of existing prefixes makes them better
class-prefixes; simplifying the nested quoting needed otherwise:

  $> echo "format '^gvt: core:' +p" >control

Dropping the internal spaces means any trailing space in a query will
more clearly terminate the prefix being searched for.

Consider a generic drm-debug example:

  # turn off ATOMIC reports
  echo format "^drm:atomic: " -p > control

  # turn off all ATOMIC:* reports, including any sub-categories
  echo format "^drm:atomic:" -p > control

  # turn on ATOMIC:FAIL: reports
  echo format "^drm:atomic:fail: " +p > control

Removing embedded spaces in the class-prefixes simplifies the
corresponding match-prefix.  This means that "quoted" match-prefixes
are only needed when the trailing space is desired, in order to
exclude explicitly sub-categorized pr-debugs; in this example,
"drm:atomic:fail:".

RFC: maybe the prefix catenation should paste in the " " class-prefix
terminator explicitly.  A pr_debug_() flavor could exclude the " ",
allowing ad-hoc sub-categorization by appending for example, "fail:"
to "drm:atomic:".

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/i915/gvt/debug.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
index c6027125c1ec..b4021f41c546 100644
--- a/drivers/gpu/drm/i915/gvt/debug.h
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -36,30 +36,30 @@ do {
\
 } while (0)
 
 #define gvt_dbg_core(fmt, args...) \
-   pr_debug("gvt: core: "fmt, ##args)
+   pr_debug("gvt:core: "fmt, ##args)
 
 #define gvt_dbg_irq(fmt, args...) \
-   pr_debug("gvt: irq: "fmt, ##args)
+   pr_debug("gvt:irq: "fmt, ##args)
 
 #define gvt_dbg_mm(fmt, args...) \
-   pr_debug("gvt: mm: "fmt, ##args)
+   pr_debug("gvt:mm: "fmt, ##args)
 
 #define gvt_dbg_mmio(fmt, args...) \
-   pr_debug("gvt: mmio: "fmt, ##args)
+   pr_debug("gvt:mmio: "fmt, ##args)
 
 #define gvt_dbg_dpy(fmt, args...) \
-   pr_debug("gvt: dpy: "fmt, ##args)
+   pr_debug("gvt:dpy: "fmt, ##args)
 
 #define gvt_dbg_el(fmt, args...) \
-   pr_debug("gvt: el: "fmt, ##args)
+   pr_debug("gvt:el: "fmt, ##args)
 
 #define gvt_dbg_sched(fmt, args...) \
-   pr_debug("gvt: sched: "fmt, ##args)
+   pr_debug("gvt:sched: "fmt, ##args)
 
 #define gvt_dbg_render(fmt, args...) \
-   pr_debug("gvt: render: "fmt, ##args)
+   pr_debug("gvt:render: "fmt, ##args)
 
 #define gvt_dbg_cmd(fmt, args...) \
-   pr_debug("gvt: cmd: "fmt, ##args)
+   pr_debug("gvt:cmd: "fmt, ##args)
 
 #endif
-- 
2.31.1

[PATCH v5 3/9] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks

[Jim Cromie]

DEFINE_DYNAMIC_DEBUG_CATEGORIES(name, var, bitmap_desc, @bit_descs)
allows users to define a drm.debug style (bitmap) sysfs interface, and
to specify the desired mapping from bits[0-N] to the format-prefix'd
pr_debug()s to be controlled.

DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
"i915/gvt bitmap desc",
/**
 * search-prefixes, passed to dd-exec_queries
 * defines bits 0-N in order.
 * leading ^ is tacitly inserted (by callback currently)
 * trailing space used here excludes subcats.
 * helper macro needs more work
 * macro to autogen ++$i, 0x%x$i ?
 */
_DD_cat_("gvt:cmd: "),
_DD_cat_("gvt:core: "),
_DD_cat_("gvt:dpy: "),
_DD_cat_("gvt:el: "),
_DD_cat_("gvt:irq: "),
_DD_cat_("gvt:mm: "),
_DD_cat_("gvt:mmio: "),
_DD_cat_("gvt:render: "),
_DD_cat_("gvt:sched: "));

dynamic_debug.c: add 3 new elements:

 - int param_set_dyndbg()
 - int param_get_dyndbg()
 - struct kernel_param_ops param_ops_dyndbg

Following the model of kernel/params.c STANDARD_PARAM_DEFS, All 3 are
non-static and exported.

dynamic_debug.h:

Add DEFINE_DYNAMIC_DEBUG_CATEGORIES() described above, and a do-nothing stub.

Note that it also calls MODULE_PARM_DESC for the user, but expects the
user to catenate all the bit-descriptions together (as is done in
drm.debug), and in the following uses in amdgpu, i915.

This in the hope that someone can offer an auto-incrementing
label-generating macro, producing "\tbit-4 0x10\t" etc, and can show
how to apply it to __VA_ARGS__.

Also extern the struct kernel_param param_ops_dyndbg symbol, as is
done in moduleparams.h for all the STANDARD params.

USAGE NOTES:

Using dyndbg to query on "format ^$prefix" requires that the prefix be
present in the compiled-in format string; where run-time prefixing is
used, that format would be "%s...", which is not usefully selectable.

Adding structural query terms (func,file,lineno) could help (module is
already done), but DEFINE_DYNAMIC_DEBUG_CATEGORIES can't do that now,
adding it needs a better reason imo.

Dyndbg is completely agnostic wrt the categorization scheme used, to
play well with any prefix convention already in use.  Ad-hoc
categories and sub-categories are implicitly allowed, author
discipline and review is expected.

Here are some examples:

"1","2","3" 2 doesnt imply 1.
otherwize, sorta like printk levels
"1:","2:","3:"  are better, avoiding [1-9]\d+ ambiguity
"hi:","mid:","low:" are reasonable, and imply independence
"todo:","rfc:"  might be handy
"A:".."Z:"  uhm, yeah

Hierarchical classes/categories are natural:

"drm::"is used in later commit
"drm:::"  is a natural extension.
"drm:atomic:fail:"  has been proposed, sounds directly useful

Some properties of a hierarchical category deserve explication:

Trailing spaces matter !

With 1..3-space ("drm: ", "drm:atomic: ", "drm:atomic:fail: "), the
":" doesnt terminate the search-space, the trailing space does.
So a "drm:" search specification will match all DRM categories &
subcategories, and will not be useful in an interface where all
categories are controlled together.  That said, "drm:atomic:" &
"drm:atomic: " are different, and both are useful in cases.

Ad-Hoc sub-categories:

These have a caveat wrt wrapper macros adding prefixes like
"drm:atomic: "; the trailing space in the prefix means that
drm_dbg("fail: ...") renders as "drm:atomic: fail: ", which obviously
isn't ideal wrt clear and simple bitmaps.

A possible solution is to have a FOO_() version of every FOO() macro
which (anti-mnemonically) elides the trailing space, which is normally
inserted by a modified FOO().  Doing this would enforce a policy
decision that "debug categories will be space terminated", with an
pressure-relief valve.

Summarizing:

 - "drm:kms: " & "drm:kms:" are different
 - "drm:kms"also different - includes drm:kms2:
 - "drm:kms:\t" also different
 - "drm:kms:*"  doesnt work, no wildcard on format atm.

Order matters in DEFINE_DYNAMIC_DEBUG_CATEGORIES(... @bit_descs)

@bit_descs (array) position determines the bit mapping to the prefix,
so to keep a stable map, new categories or 3rd level categories must
be added to the end.

Since bits are/will-stay applied 0-N, the later bits can countermand
the earlier ones, but its tricky - consider;

DD_CATs(... "drm:atomic:", ""drm:atomic:fail:" ) // misleading

The 1st search-term is misleading, because it includes (modifies)
subcategories, but then 2nd overrides it.  So don't do that.

There is still plenty of bikeshedding to do.

---
v4+:

. rename to DEFINE_DYNAMIC_DEBUG_CATEGORIES from DEFINE_DYNDBG_BITMAP
. in query, replace hardcoded "i915" w kp->mod->name
. static inline the stubs
. const *str in structs, const array. -Emil
. dyndbg: add do-nothing DEFINE_DYNAMIC_DEBUG_CATEGORIES if !DD_CORE
. 

[PATCH v5 2/9] moduleparam: add data member to struct kernel_param

[Jim Cromie]

Add a const void* data member to the struct, to allow attaching
private data that will be used soon by a setter method (via kp->data)
to perform more elaborate actions.

To attach the data at compile time, add new macros:

module_param_cbd() derives from module_param_cb(), adding data param,
and latter is redefined to use former.

It calls __module_param_call_wdata(), which accepts a new data param
and inits .data with it. Re-define __module_param_call() to use it.

Use of this new data member will be rare, it might be worth redoing
this as a separate/sub-type to de-bloat the base case.

---
v4+:
. const void* data - 

Signed-off-by: Jim Cromie 
---
 include/linux/moduleparam.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index eed280fae433..878387e0b2d9 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -78,6 +78,7 @@ struct kernel_param {
const struct kparam_string *str;
const struct kparam_array *arr;
};
+   const void *data;
 };
 
 extern const struct kernel_param __start___param[], __stop___param[];
@@ -175,6 +176,9 @@ struct kparam_array
 #define module_param_cb(name, ops, arg, perm)\
__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
 
+#define module_param_cbd(name, ops, arg, perm, data)   
\
+   __module_param_call_wdata(MODULE_PARAM_PREFIX, name, ops, arg, perm, 
-1, 0, data)
+
 #define module_param_cb_unsafe(name, ops, arg, perm) \
__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1,\
KERNEL_PARAM_FL_UNSAFE)
@@ -284,14 +288,17 @@ struct kparam_array
 
 /* This is the fundamental function for registering boot/module
parameters. */
-#define __module_param_call(prefix, name, ops, arg, perm, level, flags)
\
+#define __module_param_call(prefix, name, ops, arg, perm, level, flags) \
+   __module_param_call_wdata(prefix, name, ops, arg, perm, level, flags, 
NULL)
+
+#define __module_param_call_wdata(prefix, name, ops, arg, perm, level, flags, 
data) \
/* Default value instead of permissions? */ \
static const char __param_str_##name[] = prefix #name;  \
static struct kernel_param __moduleparam_const __param_##name   \
__used __section("__param") \
__aligned(__alignof__(struct kernel_param)) \
= { __param_str_##name, THIS_MODULE, ops,   \
-   VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
+   VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg }, data }
 
 /* Obsolete - use module_param_cb() */
 #define module_param_call(name, _set, _get, arg, perm) \
-- 
2.31.1

[PATCH v5 1/9] drm/print: fixup spelling in a comment

[Jim Cromie]

s/prink/printk/ - no functional changes

Signed-off-by: Jim Cromie 
---
 include/drm/drm_print.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 9b66be54dd16..15a089a87c22 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -327,7 +327,7 @@ static inline bool drm_debug_enabled(enum 
drm_debug_category category)
 /*
  * struct device based logging
  *
- * Prefer drm_device based logging over device or prink based logging.
+ * Prefer drm_device based logging over device or printk based logging.
  */
 
 __printf(3, 4)
-- 
2.31.1

[PATCH v5 0/9] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and use in DRM

[Jim Cromie]

hi Jason, Greg, Daniel, dri-everyone,

drm_debug_enabled() is called a lot (by drm-debug api) to do unlikely
bit-tests to selectively enable debug printing; this is a good job for
DYNAMIC_DEBUG, IFF it is built with JUMP_LABEL.
 
This patchset enables the use of dynamic-debug to avoid those
drm_debug_enabled() overheads, if CONFIG_DRM_USE_DYNAMIC_DEBUG=y.

v5: much rework

- based on Daniel Vetter's feedback, not RFC anymore. (except last one)

- move POC bit_map callback code into dynamic_debug
  add .data to struct kernel_param
  add DEFINE_DYNAMIC_DEBUG_CATEGORIES :
  a declarative interface for bits => control-queries
  this is all new functionality.

- use DEFINE_DYNAMIC_DEBUG_CATEGORIES in i915, amdgpu
  adds selectivity/control to existing categorizations

- DRM_USE_DYNAMIC_DEBUG
  replace DRM_UT_ (an enum)
  with DRM_CAT_   (a prefix string, cpp-prepended to format)
  _UT_ still present, drm_debug_enabled() still used
  todo:
  change __drm_debug param-var to read DDD_CATEGORIES's param-var
  might suffice to keep parallel schemes coherent.
  
- RFC add tracer func as syslog alternate
  test_dynamic_debug.ko: uses tracer for observability, does selftest
  has some misuse risk; calling pr_debug recursively.

v4: (brown-bagger, various fixes after snips)
v3: fixes missed SOB, && on BOL, commit-log tweaks
v2: https://lore.kernel.org/lkml/20210711055003.528167-1-jim.cro...@gmail.com/
v1: https://lore.kernel.org/lkml/20201204035318.332419-1-jim.cro...@gmail.com/

Doing so creates many new pr_debug callsites,
otherwise i915 has ~120 prdbgs, and drm has just 1;

  bash-5.1# modprobe i915
  dyndbg:   8 debug prints in module video
  dyndbg: 305 debug prints in module drm
  dyndbg: 207 debug prints in module drm_kms_helper
  dyndbg:   2 debug prints in module ttm
  dyndbg: 1720 debug prints in module i915

On amdgpu, enabling it adds ~3200 prdbgs, currently at 56 bytes each.
So CONFIG_DRM_USE_DYNAMIC_DEBUG=y affects resource requirements.
Im working on a diet-plan.

Im running this patchset bare-metal on an i7/i915 laptop & an
r9/amdgpu desktop (both as loadable modules).  I booted the amdgpu box
with:

BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.14.0-rc4-d7a-9-g5db471cba844 \
 root=UUID=mumble ro \
 rootflags=subvol=root00 rhgb \
 dynamic_debug.verbose=3 main.dyndbg=+p \
 amdgpu.debug=1 amdgpu.test=1 \
 "amdgpu.dyndbg=format ^[ +p"

That last line enables ~1700 prdbg callsites with a format like '[DML'
etc at boot, and amdgpu.test=1 triggers 90 seconds of tests, yielding
~76k prdbgs in 409 seconds, before I turned them off with:

  echo module amdgpu -p > /proc/dynamic_debug/control

Its worth noting, this changes the dyndbg-state underneath settings
applied with `echo > parameters/debug`; the latter is qualitatively
writeonly, maybe a param_get should return "NA" "-1"

this merged cleanly, on top of
commit d65ef4634e5c795a6a4df1d198992c70e9692fb3 (drm-tip/drm-tip)

Jim Cromie (9):
  drm/print: fixup spelling in a comment
  moduleparam: add data member to struct kernel_param
  dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks
  i915/gvt: remove spaces in pr_debug "gvt: core:" etc prefixes
  i915/gvt: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to create "gvt:core:"
etc categories
  amdgpu: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to control categorized
pr_debugs
  drm_print: add choice to use dynamic debug in drm-debug
  amdgpu_ucode: reduce number of pr_debug calls
  dyndbg: RFC add tracer facility RFC

 drivers/gpu/drm/Kconfig   |  13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 293 ++
 .../gpu/drm/amd/display/dc/core/dc_debug.c|  44 ++-
 drivers/gpu/drm/drm_print.c   |  49 ++-
 drivers/gpu/drm/i915/gvt/Makefile |   4 +
 drivers/gpu/drm/i915/gvt/debug.h  |  18 +-
 drivers/gpu/drm/i915/i915_params.c|  35 +++
 include/drm/drm_print.h   | 143 +++--
 include/linux/dynamic_debug.h |  82 -
 include/linux/moduleparam.h   |  11 +-
 lib/Kconfig.debug |  10 +
 lib/Makefile  |   1 +
 lib/dynamic_debug.c   | 171 --
 lib/test_dynamic_debug.c  | 247 +++
 14 files changed, 901 insertions(+), 220 deletions(-)
 create mode 100644 lib/test_dynamic_debug.c

-- 
2.31.1

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Lazar, Lijo]

On 8/13/2021 8:10 PM, Michel Dänzer wrote:

On 2021-08-13 4:14 p.m., Lazar, Lijo wrote:

On 8/13/2021 7:04 PM, Michel Dänzer wrote:

On 2021-08-13 1:50 p.m., Lazar, Lijo wrote:

On 8/13/2021 3:59 PM, Michel Dänzer wrote:

From: Michel Dänzer 

schedule_delayed_work does not push back the work if it was already
scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
was disabled and re-enabled again during those 100 ms.

This resulted in frame drops / stutter with the upcoming mutter 41
release on Navi 14, due to constantly enabling GFXOFF in the HW and
disabling it again (for getting the GPU clock counter).

To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
enabled to disabled. This makes sure the delayed work will be scheduled
as intended in the reverse case.

In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
to use mutex_trylock instead of mutex_lock.

v2:
* Use cancel_delayed_work_sync & mutex_trylock instead of
     mod_delayed_work.

Signed-off-by: Michel Dänzer 
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++--
    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
    3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f3fd5ec710b6..8b025f70706c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct 
work_struct *work)
    struct amdgpu_device *adev =
    container_of(work, struct amdgpu_device, 
gfx.gfx_off_delay_work.work);
    -    mutex_lock(>gfx.gfx_off_mutex);
+    /* mutex_lock could deadlock with cancel_delayed_work_sync in 
amdgpu_gfx_off_ctrl. */
+    if (!mutex_trylock(>gfx.gfx_off_mutex)) {
+    /* If there's a bug which causes amdgpu_gfx_off_ctrl to be called with 
enable=true
+ * when adev->gfx.gfx_off_req_count is already 0, we might race with 
that.
+ * Re-schedule to make sure gfx off will be re-enabled in the HW 
eventually.
+ */
+    schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+    return;


This is not needed and is just creating another thread to contend for mutex.


Still not sure what you mean by that. What other thread?


Sorry, I meant it schedules another workitem and delays GFXOFF enablement 
further. For ex: if it was another function like gfx_off_status holding the 
lock at the time of check.




The checks below take care of enabling gfxoff correctly. If it's already in 
gfx_off state, it doesn't do anything. So I don't see why this change is needed.


mutex_trylock is needed to prevent the deadlock discussed before and below.

schedule_delayed_work is needed due to this scenario hinted at by the comment:

1. amdgpu_gfx_off_ctrl locks mutex, calls schedule_delayed_work
2. amdgpu_device_delay_enable_gfx_off runs, calls mutex_trylock, which fails

GFXOFF would never get re-enabled in HW in this case (until amdgpu_gfx_off_ctrl 
calls schedule_delayed_work again).

(cancel_delayed_work_sync guarantees there's no pending delayed work when it 
returns, even if amdgpu_device_delay_enable_gfx_off calls schedule_delayed_work)



I think we need to explain based on the original code before. There is an 
asssumption here that the only other contention of this mutex is with the 
gfx_off_ctrl function.


Not really.



As far as I understand if the work has already started running when 
schedule_delayed_work is called, it will insert another in the work queue after 
delay. Based on that understanding I didn't find a problem with the original 
code.


Original code as in without this patch or the mod_delayed_work patch? If so, 
the problem is not when the work has already started running. It's that when it 
hasn't started running yet, schedule_delayed_work doesn't change the timeout 
for the already scheduled work, so it ends up enabling GFXOFF earlier than 
intended (and thus at all in scenarios when it's not supposed to).



I meant the original implementation of 
amdgpu_device_delay_enable_gfx_off().



If you indeed want to use _sync, there is a small problem with this 
implementation also which is roughly equivalent to the original problem 
you faced.


amdgpu_gfx_off_ctrl(disable) locks mutex
calls cancel_delayed_work_sync
amdgpu_device_delay_enable_gfx_off already started running
mutex_trylock fails and schedules another one
amdgpu_gfx_off_ctrl(enable)
	schedules_delayed_work() - Delay is not extended, it's the same as when 
it's rearmed from work item.


Probably, overthinking about the solution. Looking back, mod_ version is 
simpler :). May be just delay it further everytime there is a call with 
enable instead of doing it only for req_cnt==0?

Re: [PATCH] drm: radeon: r600_dma: Replace cpu_to_le32() by lower_32_bits()

[Michel Dänzer]

On 2021-08-13 10:54 a.m., zhaoxiao wrote:
> This patch fixes the following sparse errors:
> drivers/gpu/drm/radeon/r600_dma.c:247:30: warning: incorrect type in 
> assignment (different base types)
> drivers/gpu/drm/radeon/r600_dma.c:247:30:expected unsigned int volatile 
> [usertype]
> drivers/gpu/drm/radeon/r600_dma.c:247:30:got restricted __le32 [usertype]
> 
> Signed-off-by: zhaoxiao 
> ---
>  drivers/gpu/drm/radeon/r600_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/radeon/r600_dma.c 
> b/drivers/gpu/drm/radeon/r600_dma.c
> index fb65e6fb5c4f..a2d0b1edcd22 100644
> --- a/drivers/gpu/drm/radeon/r600_dma.c
> +++ b/drivers/gpu/drm/radeon/r600_dma.c
> @@ -244,7 +244,7 @@ int r600_dma_ring_test(struct radeon_device *rdev,
>   gpu_addr = rdev->wb.gpu_addr + index;
>  
>   tmp = 0xCAFEDEAD;
> - rdev->wb.wb[index/4] = cpu_to_le32(tmp);
> + rdev->wb.wb[index/4] = lower_32_bits(tmp);
>  
>   r = radeon_ring_lock(rdev, ring, 4);
>   if (r) {
> 

Seems better to mark rdev->wb.wb as little endian instead. It's read with 
le32_to_cpu (with some exceptions which look like bugs), which would result in 
0xADEDFECA like this.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Michel Dänzer]

On 2021-08-13 4:14 p.m., Lazar, Lijo wrote:
> On 8/13/2021 7:04 PM, Michel Dänzer wrote:
>> On 2021-08-13 1:50 p.m., Lazar, Lijo wrote:
>>> On 8/13/2021 3:59 PM, Michel Dänzer wrote:
 From: Michel Dänzer 

 schedule_delayed_work does not push back the work if it was already
 scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
 after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
 was disabled and re-enabled again during those 100 ms.

 This resulted in frame drops / stutter with the upcoming mutter 41
 release on Navi 14, due to constantly enabling GFXOFF in the HW and
 disabling it again (for getting the GPU clock counter).

 To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
 enabled to disabled. This makes sure the delayed work will be scheduled
 as intended in the reverse case.

 In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
 to use mutex_trylock instead of mutex_lock.

 v2:
 * Use cancel_delayed_work_sync & mutex_trylock instead of
     mod_delayed_work.

 Signed-off-by: Michel Dänzer 
 ---
    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++--
    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
    3 files changed, 20 insertions(+), 7 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index f3fd5ec710b6..8b025f70706c 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -2777,7 +2777,16 @@ static void 
 amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
    struct amdgpu_device *adev =
    container_of(work, struct amdgpu_device, 
 gfx.gfx_off_delay_work.work);
    -    mutex_lock(>gfx.gfx_off_mutex);
 +    /* mutex_lock could deadlock with cancel_delayed_work_sync in 
 amdgpu_gfx_off_ctrl. */
 +    if (!mutex_trylock(>gfx.gfx_off_mutex)) {
 +    /* If there's a bug which causes amdgpu_gfx_off_ctrl to be called 
 with enable=true
 + * when adev->gfx.gfx_off_req_count is already 0, we might race 
 with that.
 + * Re-schedule to make sure gfx off will be re-enabled in the HW 
 eventually.
 + */
 +    schedule_delayed_work(>gfx.gfx_off_delay_work, 
 AMDGPU_GFX_OFF_DELAY_ENABLE);
 +    return;
>>>
>>> This is not needed and is just creating another thread to contend for mutex.
>>
>> Still not sure what you mean by that. What other thread?
> 
> Sorry, I meant it schedules another workitem and delays GFXOFF enablement 
> further. For ex: if it was another function like gfx_off_status holding the 
> lock at the time of check.
> 
>>
>>> The checks below take care of enabling gfxoff correctly. If it's already in 
>>> gfx_off state, it doesn't do anything. So I don't see why this change is 
>>> needed.
>>
>> mutex_trylock is needed to prevent the deadlock discussed before and below.
>>
>> schedule_delayed_work is needed due to this scenario hinted at by the 
>> comment:
>>
>> 1. amdgpu_gfx_off_ctrl locks mutex, calls schedule_delayed_work
>> 2. amdgpu_device_delay_enable_gfx_off runs, calls mutex_trylock, which fails
>>
>> GFXOFF would never get re-enabled in HW in this case (until 
>> amdgpu_gfx_off_ctrl calls schedule_delayed_work again).
>>
>> (cancel_delayed_work_sync guarantees there's no pending delayed work when it 
>> returns, even if amdgpu_device_delay_enable_gfx_off calls 
>> schedule_delayed_work)
>>
> 
> I think we need to explain based on the original code before. There is an 
> asssumption here that the only other contention of this mutex is with the 
> gfx_off_ctrl function.

Not really.


> As far as I understand if the work has already started running when 
> schedule_delayed_work is called, it will insert another in the work queue 
> after delay. Based on that understanding I didn't find a problem with the 
> original code.

Original code as in without this patch or the mod_delayed_work patch? If so, 
the problem is not when the work has already started running. It's that when it 
hasn't started running yet, schedule_delayed_work doesn't change the timeout 
for the already scheduled work, so it ends up enabling GFXOFF earlier than 
intended (and thus at all in scenarios when it's not supposed to).


> [...], there could be cases where it could have gone to gfxoff right after 
> gfx_off_status releases the lock, but it doesn't delaying it further. That 
> would be the case if some other function is also introduced which takes this 
> mutex.

I really don't think we need to worry about amdgpu_get_gfx_off_status, since 
it's only called from debugfs (and should be very short). If something hits 
that debugfs file and it causes higher energy 

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Lazar, Lijo]

On 8/13/2021 7:04 PM, Michel Dänzer wrote:

On 2021-08-13 1:50 p.m., Lazar, Lijo wrote:



On 8/13/2021 3:59 PM, Michel Dänzer wrote:

From: Michel Dänzer 

schedule_delayed_work does not push back the work if it was already
scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
was disabled and re-enabled again during those 100 ms.

This resulted in frame drops / stutter with the upcoming mutter 41
release on Navi 14, due to constantly enabling GFXOFF in the HW and
disabling it again (for getting the GPU clock counter).

To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
enabled to disabled. This makes sure the delayed work will be scheduled
as intended in the reverse case.

In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
to use mutex_trylock instead of mutex_lock.

v2:
* Use cancel_delayed_work_sync & mutex_trylock instead of
    mod_delayed_work.

Signed-off-by: Michel Dänzer 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++--
   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
   3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f3fd5ec710b6..8b025f70706c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct 
work_struct *work)
   struct amdgpu_device *adev =
   container_of(work, struct amdgpu_device, 
gfx.gfx_off_delay_work.work);
   -    mutex_lock(>gfx.gfx_off_mutex);
+    /* mutex_lock could deadlock with cancel_delayed_work_sync in 
amdgpu_gfx_off_ctrl. */
+    if (!mutex_trylock(>gfx.gfx_off_mutex)) {
+    /* If there's a bug which causes amdgpu_gfx_off_ctrl to be called with 
enable=true
+ * when adev->gfx.gfx_off_req_count is already 0, we might race with 
that.
+ * Re-schedule to make sure gfx off will be re-enabled in the HW 
eventually.
+ */
+    schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+    return;


This is not needed and is just creating another thread to contend for mutex.


Still not sure what you mean by that. What other thread?


Sorry, I meant it schedules another workitem and delays GFXOFF 
enablement further. For ex: if it was another function like 
gfx_off_status holding the lock at the time of check.





The checks below take care of enabling gfxoff correctly. If it's already in 
gfx_off state, it doesn't do anything. So I don't see why this change is needed.


mutex_trylock is needed to prevent the deadlock discussed before and below.

schedule_delayed_work is needed due to this scenario hinted at by the comment:

1. amdgpu_gfx_off_ctrl locks mutex, calls schedule_delayed_work
2. amdgpu_device_delay_enable_gfx_off runs, calls mutex_trylock, which fails

GFXOFF would never get re-enabled in HW in this case (until amdgpu_gfx_off_ctrl 
calls schedule_delayed_work again).

(cancel_delayed_work_sync guarantees there's no pending delayed work when it 
returns, even if amdgpu_device_delay_enable_gfx_off calls schedule_delayed_work)



I think we need to explain based on the original code before. There is 
an asssumption here that the only other contention of this mutex is with 
the gfx_off_ctrl function. That is not true, so this is not the only 
case where mutex_trylock can fail. It could be because gfx_off_status is 
holding the lock.


As far as I understand if the work has already started running when 
schedule_delayed_work is called, it will insert another in the work 
queue after delay. Based on that understanding I didn't find a problem 
with the original code. Maybe, mutex_trylock is added to call _sync to 
make sure work is cancelled or not running but that breaks other 
assumptions.



The other problem is amdgpu_get_gfx_off_status() also uses the same mutex.


Not sure what for TBH. AFAICT there's only one implementation of this for 
Renoir, which just reads a register. (It's only called from debugfs)



I'm not sure either :) But as long as there are other functions that 
contend for the same lock, it's not good to implement based on 
assumptions only about a particular scenario.



So it won't be knowing which thread it would be contending against and blindly 
creates more work items.


There is only ever at most one instance of the delayed work at any time. 
amdgpu_device_delay_enable_gfx_off doesn't care whether amdgpu_gfx_off_ctrl or 
amdgpu_get_gfx_off_status is holding the mutex, it just keeps re-scheduling 
itself 100 ms later until it succeeds.



Yes, that is the problem, there could be cases where it could have gone 
to gfxoff right after gfx_off_status releases the lock, but it doesn't 
delaying it further. That would be the case 

Re: [PATCH] drm/amdgpu: disable BACO support for 699F:C7 polaris12 SKU temporarily

[Alex Deucher]

On Fri, Aug 13, 2021 at 9:54 AM Alex Deucher  wrote:
>
> Acked-by: Alex Deucher 
>
> On Fri, Aug 13, 2021 at 4:09 AM Evan Quan  wrote:
> >
> > We have a S3 issue on that SKU with BACO enabled. Will bring back this
> > when that root caused.
> >

Actually it might be worth checking the SSIDs as well assuming this is
platform specific.

Alex

> > Change-Id: I56d4830e6275e20a415808896eecbadfe944070b
> > Signed-off-by: Evan Quan 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/vi.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c 
> > b/drivers/gpu/drm/amd/amdgpu/vi.c
> > index fe9a7cc8d9eb..7210f80815b8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> > @@ -904,7 +904,12 @@ static bool vi_asic_supports_baco(struct amdgpu_device 
> > *adev)
> > case CHIP_POLARIS11:
> > case CHIP_POLARIS12:
> > case CHIP_TOPAZ:
> > -   return amdgpu_dpm_is_baco_supported(adev);
> > +   /* Disable BACO support for the specific polaris12 SKU 
> > temporarily */
> > +   if ((adev->pdev->device == 0x699F) &&
> > +   (adev->pdev->revision == 0xC7))
> > +   return false;
> > +   else
> > +   return amdgpu_dpm_is_baco_supported(adev);
> > default:
> > return false;
> > }
> > --
> > 2.29.0
> >

Re: [PATCH] drm/amdgpu: disable BACO support for 699F:C7 polaris12 SKU temporarily

[Alex Deucher]

Acked-by: Alex Deucher 

On Fri, Aug 13, 2021 at 4:09 AM Evan Quan  wrote:
>
> We have a S3 issue on that SKU with BACO enabled. Will bring back this
> when that root caused.
>
> Change-Id: I56d4830e6275e20a415808896eecbadfe944070b
> Signed-off-by: Evan Quan 
> ---
>  drivers/gpu/drm/amd/amdgpu/vi.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
> index fe9a7cc8d9eb..7210f80815b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -904,7 +904,12 @@ static bool vi_asic_supports_baco(struct amdgpu_device 
> *adev)
> case CHIP_POLARIS11:
> case CHIP_POLARIS12:
> case CHIP_TOPAZ:
> -   return amdgpu_dpm_is_baco_supported(adev);
> +   /* Disable BACO support for the specific polaris12 SKU 
> temporarily */
> +   if ((adev->pdev->device == 0x699F) &&
> +   (adev->pdev->revision == 0xC7))
> +   return false;
> +   else
> +   return amdgpu_dpm_is_baco_supported(adev);
> default:
> return false;
> }
> --
> 2.29.0
>

[PATCH] drm: radeon: r600_dma: Replace cpu_to_le32() by lower_32_bits()

[zhaoxiao]

This patch fixes the following sparse errors:
drivers/gpu/drm/radeon/r600_dma.c:247:30: warning: incorrect type in assignment 
(different base types)
drivers/gpu/drm/radeon/r600_dma.c:247:30:expected unsigned int volatile 
[usertype]
drivers/gpu/drm/radeon/r600_dma.c:247:30:got restricted __le32 [usertype]

Signed-off-by: zhaoxiao 
---
 drivers/gpu/drm/radeon/r600_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/r600_dma.c 
b/drivers/gpu/drm/radeon/r600_dma.c
index fb65e6fb5c4f..a2d0b1edcd22 100644
--- a/drivers/gpu/drm/radeon/r600_dma.c
+++ b/drivers/gpu/drm/radeon/r600_dma.c
@@ -244,7 +244,7 @@ int r600_dma_ring_test(struct radeon_device *rdev,
gpu_addr = rdev->wb.gpu_addr + index;
 
tmp = 0xCAFEDEAD;
-   rdev->wb.wb[index/4] = cpu_to_le32(tmp);
+   rdev->wb.wb[index/4] = lower_32_bits(tmp);
 
r = radeon_ring_lock(rdev, ring, 4);
if (r) {
-- 
2.20.1


F

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Michel Dänzer]

On 2021-08-13 1:50 p.m., Lazar, Lijo wrote:
> 
> 
> On 8/13/2021 3:59 PM, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> schedule_delayed_work does not push back the work if it was already
>> scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
>> after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
>> was disabled and re-enabled again during those 100 ms.
>>
>> This resulted in frame drops / stutter with the upcoming mutter 41
>> release on Navi 14, due to constantly enabling GFXOFF in the HW and
>> disabling it again (for getting the GPU clock counter).
>>
>> To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
>> enabled to disabled. This makes sure the delayed work will be scheduled
>> as intended in the reverse case.
>>
>> In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
>> to use mutex_trylock instead of mutex_lock.
>>
>> v2:
>> * Use cancel_delayed_work_sync & mutex_trylock instead of
>>    mod_delayed_work.
>>
>> Signed-off-by: Michel Dänzer 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c    | 13 +++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  3 +++
>>   3 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index f3fd5ec710b6..8b025f70706c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct 
>> work_struct *work)
>>   struct amdgpu_device *adev =
>>   container_of(work, struct amdgpu_device, 
>> gfx.gfx_off_delay_work.work);
>>   -    mutex_lock(>gfx.gfx_off_mutex);
>> +    /* mutex_lock could deadlock with cancel_delayed_work_sync in 
>> amdgpu_gfx_off_ctrl. */
>> +    if (!mutex_trylock(>gfx.gfx_off_mutex)) {
>> +    /* If there's a bug which causes amdgpu_gfx_off_ctrl to be called 
>> with enable=true
>> + * when adev->gfx.gfx_off_req_count is already 0, we might race 
>> with that.
>> + * Re-schedule to make sure gfx off will be re-enabled in the HW 
>> eventually.
>> + */
>> +    schedule_delayed_work(>gfx.gfx_off_delay_work, 
>> AMDGPU_GFX_OFF_DELAY_ENABLE);
>> +    return;
> 
> This is not needed and is just creating another thread to contend for mutex.

Still not sure what you mean by that. What other thread?

> The checks below take care of enabling gfxoff correctly. If it's already in 
> gfx_off state, it doesn't do anything. So I don't see why this change is 
> needed.

mutex_trylock is needed to prevent the deadlock discussed before and below.

schedule_delayed_work is needed due to this scenario hinted at by the comment:

1. amdgpu_gfx_off_ctrl locks mutex, calls schedule_delayed_work
2. amdgpu_device_delay_enable_gfx_off runs, calls mutex_trylock, which fails

GFXOFF would never get re-enabled in HW in this case (until amdgpu_gfx_off_ctrl 
calls schedule_delayed_work again).

(cancel_delayed_work_sync guarantees there's no pending delayed work when it 
returns, even if amdgpu_device_delay_enable_gfx_off calls schedule_delayed_work)


> The other problem is amdgpu_get_gfx_off_status() also uses the same mutex.

Not sure what for TBH. AFAICT there's only one implementation of this for 
Renoir, which just reads a register. (It's only called from debugfs)

> So it won't be knowing which thread it would be contending against and 
> blindly creates more work items. 

There is only ever at most one instance of the delayed work at any time. 
amdgpu_device_delay_enable_gfx_off doesn't care whether amdgpu_gfx_off_ctrl or 
amdgpu_get_gfx_off_status is holding the mutex, it just keeps re-scheduling 
itself 100 ms later until it succeeds.


>> @@ -569,9 +566,13 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, 
>> bool enable)
>>   adev->gfx.gfx_off_req_count--;
>>     if (enable && !adev->gfx.gfx_off_state && 
>> !adev->gfx.gfx_off_req_count) {
>> -    schedule_delayed_work(>gfx.gfx_off_delay_work, 
>> GFX_OFF_DELAY_ENABLE);
>> -    } else if (!enable && adev->gfx.gfx_off_state) {
>> -    if (!amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, 
>> false)) {
>> +    schedule_delayed_work(>gfx.gfx_off_delay_work, 
>> AMDGPU_GFX_OFF_DELAY_ENABLE);
>> +    } else if (!enable) {
>> +    if (adev->gfx.gfx_off_req_count == 1 && !adev->gfx.gfx_off_state)
>> +    cancel_delayed_work_sync(>gfx.gfx_off_delay_work);
> 
> This has the deadlock problem as discussed in the other thread.

It does not. If amdgpu_device_delay_enable_gfx_off runs while 
amdgpu_gfx_off_ctrl holds the mutex, 
mutex_trylock fails and the former bails.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer

RE: [PATCH] drm/amd/amdgpu: remove unnecessary RAS context field

[Clements, John]

[Public]

Reviewed-by: John Clements 

From: Li, Candice 
Sent: Friday, August 13, 2021 7:46 PM
To: amd-gfx@lists.freedesktop.org
Cc: Clements, John ; Li, Candice 
Subject: [PATCH] drm/amd/amdgpu: remove unnecessary RAS context field


[Public]

Delete ras_if->name in the RAS ctx structure and remove related lines.

Signed-off-by: Candice Li candice...@amd.com
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c  | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   | 4 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h   | 5 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c  | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c  | 1 -
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c| 4 ++--
10 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 9cfef56b2aee..5beaa7c1bd11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -615,7 +615,6 @@ int amdgpu_gfx_ras_late_init(struct amdgpu_device *adev)
adev->gfx.ras_if->block = AMDGPU_RAS_BLOCK__GFX;
adev->gfx.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->gfx.ras_if->sub_block_index = 0;
-   strcpy(adev->gfx.ras_if->name, "gfx");
 }
 fs_info.head = ih_info.head = *adev->gfx.ras_if;
 r = amdgpu_ras_late_init(adev, adev->gfx.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
index 1d50d534d77c..a766e1aad2b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
@@ -41,7 +41,6 @@ int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev)
adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP;
adev->hdp.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->hdp.ras_if->sub_block_index = 0;
-   strcpy(adev->hdp.ras_if->name, "hdp");
 }
 ih_info.head = fs_info.head = *adev->hdp.ras_if;
 r = amdgpu_ras_late_init(adev, adev->hdp.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
index ead3dc572ec5..24297dc51434 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
@@ -41,7 +41,6 @@ int amdgpu_mmhub_ras_late_init(struct amdgpu_device *adev)
adev->mmhub.ras_if->block = AMDGPU_RAS_BLOCK__MMHUB;
adev->mmhub.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->mmhub.ras_if->sub_block_index = 0;
-   strcpy(adev->mmhub.ras_if->name, "mmhub");
 }
 ih_info.head = fs_info.head = *adev->mmhub.ras_if;
 r = amdgpu_ras_late_init(adev, adev->mmhub.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
index 6201a5f4b4fa..6afb02fef8cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
@@ -39,7 +39,6 @@ int amdgpu_nbio_ras_late_init(struct amdgpu_device *adev)
adev->nbio.ras_if->block = 
AMDGPU_RAS_BLOCK__PCIE_BIF;
adev->nbio.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->nbio.ras_if->sub_block_index = 0;
-   strcpy(adev->nbio.ras_if->name, "pcie_bif");
 }
 ih_info.head = fs_info.head = *adev->nbio.ras_if;
 r = amdgpu_ras_late_init(adev, adev->nbio.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 3811b6b6a192..96a8fd0ca1df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -64,7 +64,6 @@ const char *ras_block_string[] = {
};

 #define ras_err_str(i) (ras_error_string[ffs(i)])
-#define ras_block_str(i) (ras_block_string[i])

 #define RAS_DEFAULT_FLAGS (AMDGPU_RAS_FLAG_INIT_BY_VBIOS)

@@ -530,7 +529,7 @@ static inline void put_obj(struct ras_manager *obj)
 if (obj && (--obj->use == 0))
list_del(>node);
 if (obj && (obj->use < 0))
-   DRM_ERROR("RAS ERROR: Unbalance obj(%s) use\n", 
obj->head.name);
+   DRM_ERROR("RAS ERROR: Unbalance obj(%s) use\n", 
ras_block_str(obj->head.block));
}

 /* make one obj and return it. */
@@ -793,7 +792,6 @@ static int amdgpu_ras_enable_all_features(struct 
amdgpu_device *adev,
  .type = 

Re: [PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Lazar, Lijo]

On 8/13/2021 3:59 PM, Michel Dänzer wrote:

From: Michel Dänzer 

schedule_delayed_work does not push back the work if it was already
scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
was disabled and re-enabled again during those 100 ms.

This resulted in frame drops / stutter with the upcoming mutter 41
release on Navi 14, due to constantly enabling GFXOFF in the HW and
disabling it again (for getting the GPU clock counter).

To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
enabled to disabled. This makes sure the delayed work will be scheduled
as intended in the reverse case.

In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
to use mutex_trylock instead of mutex_lock.

v2:
* Use cancel_delayed_work_sync & mutex_trylock instead of
   mod_delayed_work.

Signed-off-by: Michel Dänzer 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c| 13 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h|  3 +++
  3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f3fd5ec710b6..8b025f70706c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct 
work_struct *work)
struct amdgpu_device *adev =
container_of(work, struct amdgpu_device, 
gfx.gfx_off_delay_work.work);
  
-	mutex_lock(>gfx.gfx_off_mutex);

+   /* mutex_lock could deadlock with cancel_delayed_work_sync in 
amdgpu_gfx_off_ctrl. */
+   if (!mutex_trylock(>gfx.gfx_off_mutex)) {
+   /* If there's a bug which causes amdgpu_gfx_off_ctrl to be 
called with enable=true
+* when adev->gfx.gfx_off_req_count is already 0, we might race 
with that.
+* Re-schedule to make sure gfx off will be re-enabled in the 
HW eventually.
+*/
+   schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+   return;


This is not needed and is just creating another thread to contend for 
mutex. The checks below take care of enabling gfxoff correctly. If it's 
already in gfx_off state, it doesn't do anything. So I don't see why 
this change is needed.


The other problem is amdgpu_get_gfx_off_status() also uses the same 
mutex. So it won't be knowing which thread it would be contending 
against and blindly creates more work items.



+   }
+
if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
if (!amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, true))
adev->gfx.gfx_off_state = true;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index a0be0772c8b3..da4c46db3093 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -28,9 +28,6 @@
  #include "amdgpu_rlc.h"
  #include "amdgpu_ras.h"
  
-/* delay 0.1 second to enable gfx off feature */

-#define GFX_OFF_DELAY_ENABLE msecs_to_jiffies(100)
-
  /*
   * GPU GFX IP block helpers function.
   */
@@ -569,9 +566,13 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool 
enable)
adev->gfx.gfx_off_req_count--;
  
  	if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {

-   schedule_delayed_work(>gfx.gfx_off_delay_work, 
GFX_OFF_DELAY_ENABLE);
-   } else if (!enable && adev->gfx.gfx_off_state) {
-   if (!amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, false)) {
+   schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+   } else if (!enable) {
+   if (adev->gfx.gfx_off_req_count == 1 && 
!adev->gfx.gfx_off_state)
+   cancel_delayed_work_sync(>gfx.gfx_off_delay_work);


This has the deadlock problem as discussed in the other thread.

Thanks,
Lijo


+   if (adev->gfx.gfx_off_state &&
+   !amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, false)) {
adev->gfx.gfx_off_state = false;
  
  			if (adev->gfx.funcs->init_spm_golden) {

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index d43fe2ed8116..dcdb505bb7f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -32,6 +32,9 @@
  #include "amdgpu_rlc.h"
  #include "soc15.h"
  
+/* delay 0.1 second to enable gfx off feature */

+#define AMDGPU_GFX_OFF_DELAY_ENABLE msecs_to_jiffies(100)
+
  /* GFX current status */
  #define AMDGPU_GFX_NORMAL_MODE0xL
  #define AMDGPU_GFX_SAFE_MODE  0x0001L

[PATCH] drm/amd/amdgpu: remove unnecessary RAS context field

[Li, Candice]

[Public]

Delete ras_if->name in the RAS ctx structure and remove related lines.

Signed-off-by: Candice Li candice...@amd.com
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c  | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   | 4 +---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h   | 5 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c  | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_umc.c   | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c  | 1 -
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c| 4 ++--
10 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 9cfef56b2aee..5beaa7c1bd11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -615,7 +615,6 @@ int amdgpu_gfx_ras_late_init(struct amdgpu_device *adev)
adev->gfx.ras_if->block = AMDGPU_RAS_BLOCK__GFX;
adev->gfx.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->gfx.ras_if->sub_block_index = 0;
-   strcpy(adev->gfx.ras_if->name, "gfx");
 }
 fs_info.head = ih_info.head = *adev->gfx.ras_if;
 r = amdgpu_ras_late_init(adev, adev->gfx.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
index 1d50d534d77c..a766e1aad2b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.c
@@ -41,7 +41,6 @@ int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev)
adev->hdp.ras_if->block = AMDGPU_RAS_BLOCK__HDP;
adev->hdp.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->hdp.ras_if->sub_block_index = 0;
-   strcpy(adev->hdp.ras_if->name, "hdp");
 }
 ih_info.head = fs_info.head = *adev->hdp.ras_if;
 r = amdgpu_ras_late_init(adev, adev->hdp.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
index ead3dc572ec5..24297dc51434 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mmhub.c
@@ -41,7 +41,6 @@ int amdgpu_mmhub_ras_late_init(struct amdgpu_device *adev)
adev->mmhub.ras_if->block = AMDGPU_RAS_BLOCK__MMHUB;
adev->mmhub.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->mmhub.ras_if->sub_block_index = 0;
-   strcpy(adev->mmhub.ras_if->name, "mmhub");
 }
 ih_info.head = fs_info.head = *adev->mmhub.ras_if;
 r = amdgpu_ras_late_init(adev, adev->mmhub.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
index 6201a5f4b4fa..6afb02fef8cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.c
@@ -39,7 +39,6 @@ int amdgpu_nbio_ras_late_init(struct amdgpu_device *adev)
adev->nbio.ras_if->block = 
AMDGPU_RAS_BLOCK__PCIE_BIF;
adev->nbio.ras_if->type = 
AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE;
adev->nbio.ras_if->sub_block_index = 0;
-   strcpy(adev->nbio.ras_if->name, "pcie_bif");
 }
 ih_info.head = fs_info.head = *adev->nbio.ras_if;
 r = amdgpu_ras_late_init(adev, adev->nbio.ras_if,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 3811b6b6a192..96a8fd0ca1df 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -64,7 +64,6 @@ const char *ras_block_string[] = {
};
 #define ras_err_str(i) (ras_error_string[ffs(i)])
-#define ras_block_str(i) (ras_block_string[i])
 #define RAS_DEFAULT_FLAGS (AMDGPU_RAS_FLAG_INIT_BY_VBIOS)
@@ -530,7 +529,7 @@ static inline void put_obj(struct ras_manager *obj)
 if (obj && (--obj->use == 0))
list_del(>node);
 if (obj && (obj->use < 0))
-   DRM_ERROR("RAS ERROR: Unbalance obj(%s) use\n", 
obj->head.name);
+   DRM_ERROR("RAS ERROR: Unbalance obj(%s) use\n", 
ras_block_str(obj->head.block));
}
 /* make one obj and return it. */
@@ -793,7 +792,6 @@ static int amdgpu_ras_enable_all_features(struct 
amdgpu_device *adev,
  .type = default_ras_type,
  .sub_block_index = 0,
};
-   strcpy(head.name, ras_block_str(i));
if (bypass) {
 

Re: [PATCH 2/2] drm/amdgpu: Use mod_delayed_work in JPEG/UVD/VCE/VCN ring_end_use hooks

[Lazar, Lijo]

On 8/13/2021 4:01 PM, Michel Dänzer wrote:

On 2021-08-13 6:23 a.m., Lazar, Lijo wrote:



On 8/12/2021 10:24 PM, Michel Dänzer wrote:

On 2021-08-12 1:33 p.m., Lazar, Lijo wrote:

On 8/12/2021 1:41 PM, Michel Dänzer wrote:

On 2021-08-12 7:55 a.m., Koenig, Christian wrote:

Hi James,

Evan seems to have understood how this all works together.

See while any begin/end use critical section is active the work should not be 
active.

When you handle only one ring you can just call cancel in begin use and 
schedule in end use. But when you have more than one ring you need a lock or 
counter to prevent concurrent work items to be started.

Michelle's idea to use mod_delayed_work is a bad one because it assumes that 
the delayed work is still running.


It merely assumes that the work may already have been scheduled before.

Admittedly, I missed the cancel_delayed_work_sync calls for patch 2. While I 
think it can still have some effect when there's a single work item for 
multiple rings, as described by James, it's probably negligible, since 
presumably the time intervals between ring_begin_use and ring_end_use are 
normally much shorter than a second.

So, while patch 2 is at worst a no-op (since mod_delayed_work is the same as 
schedule_delayed_work if the work hasn't been scheduled yet), I'm fine with 
dropping it.



Something similar applies to the first patch I think,


There are no cancel work calls in that case, so the commit log is accurate 
TTBOMK.


Curious -

For patch 1, does it make a difference if any delayed work scheduled is 
cancelled in the else part before proceeding?

} else if (!enable && adev->gfx.gfx_off_state) {
cancel_delayed_work();


I tried the patch below.

While this does seem to fix the problem as well, I see a potential issue:

1. amdgpu_gfx_off_ctrl locks adev->gfx.gfx_off_mutex
2. amdgpu_device_delay_enable_gfx_off runs, blocks in mutex_lock
3. amdgpu_gfx_off_ctrl calls cancel_delayed_work_sync

I'm afraid this would deadlock? (CONFIG_PROVE_LOCKING doesn't complain though)


Should use the cancel_delayed_work instead of the _sync version.


The thing is, it's not clear to me from cancel_delayed_work's description that 
it's guaranteed not to wait for amdgpu_device_delay_enable_gfx_off to finish if 
it's already running. If that's not guaranteed, it's prone to the same deadlock.


From what I understood from the the description, cancel initiates a 
cancel. If the work has already started, it returns false saying it 
couldn't succeed otherwise cancels out the scheduled work and returns 
true. In the note below, it asks to specifically use the _sync version 
if we need to wait for an already started work and that definitely has 
the problem of deadlock you mentioned above.


 * Note:
 * The work callback function may still be running on return, unless
 * it returns %true and the work doesn't re-arm itself.  Explicitly 
flush or

 * use cancel_delayed_work_sync() to wait on it.





As you mentioned - at best work is not scheduled yet and cancelled 
successfully, or at worst it's waiting for the mutex. In the worst case, if 
amdgpu_device_delay_enable_gfx_off gets the mutex after amdgpu_gfx_off_ctrl 
unlocks it, there is an extra check as below.

if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count)

The count wouldn't be 0 and hence it won't enable GFXOFF.


I'm not sure, but it might also be possible for amdgpu_device_delay_enable_gfx_off 
to get the mutex only after amdgpu_gfx_off_ctrl was called again and set 
adev->gfx.gfx_off_req_count back to 0.



Yes, this is a case we can't avoid in either case. If the work has 
already started, then mod_delayed_ also doesn't have any impact. Another 
case is work thread already got the mutex and a disable request comes 
just at that time. It needs to wait till mutex is released by work, that 
could mean enable gfxoff immediately followed by disable.





Maybe it's possible to fix it with cancel_delayed_work_sync somehow, but I'm 
not sure how offhand. (With cancel_delayed_work instead, I'm worried 
amdgpu_device_delay_enable_gfx_off might still enable GFXOFF in the HW 
immediately after amdgpu_gfx_off_ctrl unlocks the mutex. Then again, that might 
happen with mod_delayed_work as well...)


As mentioned earlier, cancel_delayed_work won't cause this issue.

In the mod_delayed_ patch, mod_ version is called only when req_count is 0. 
While that is a good thing, it keeps alive one more contender for the mutex.


Not sure what you mean. It leaves the possibility of 
amdgpu_device_delay_enable_gfx_off running just after amdgpu_gfx_off_ctrl tried 
to postpone it. As discussed above, something similar might be possible with 
cancel_delayed_work as well.



The mod_delayed is called only req_count gets back to 0. If there is 
another disable request comes after that, it doesn't cancel out the work

scheduled nor does it adjust the delay.

Ex:
Disable gfxoff -> Enable gfxoff (now the work is scheduled) -> Disable 
gfxoff (within 

Re: [PATCH 2/2] drm/amdgpu: Use mod_delayed_work in JPEG/UVD/VCE/VCN ring_end_use hooks

[Michel Dänzer]

On 2021-08-13 6:23 a.m., Lazar, Lijo wrote:
> 
> 
> On 8/12/2021 10:24 PM, Michel Dänzer wrote:
>> On 2021-08-12 1:33 p.m., Lazar, Lijo wrote:
>>> On 8/12/2021 1:41 PM, Michel Dänzer wrote:
 On 2021-08-12 7:55 a.m., Koenig, Christian wrote:
> Hi James,
>
> Evan seems to have understood how this all works together.
>
> See while any begin/end use critical section is active the work should 
> not be active.
>
> When you handle only one ring you can just call cancel in begin use and 
> schedule in end use. But when you have more than one ring you need a lock 
> or counter to prevent concurrent work items to be started.
>
> Michelle's idea to use mod_delayed_work is a bad one because it assumes 
> that the delayed work is still running.

 It merely assumes that the work may already have been scheduled before.

 Admittedly, I missed the cancel_delayed_work_sync calls for patch 2. While 
 I think it can still have some effect when there's a single work item for 
 multiple rings, as described by James, it's probably negligible, since 
 presumably the time intervals between ring_begin_use and ring_end_use are 
 normally much shorter than a second.

 So, while patch 2 is at worst a no-op (since mod_delayed_work is the same 
 as schedule_delayed_work if the work hasn't been scheduled yet), I'm fine 
 with dropping it.


> Something similar applies to the first patch I think,

 There are no cancel work calls in that case, so the commit log is accurate 
 TTBOMK.
>>>
>>> Curious -
>>>
>>> For patch 1, does it make a difference if any delayed work scheduled is 
>>> cancelled in the else part before proceeding?
>>>
>>> } else if (!enable && adev->gfx.gfx_off_state) {
>>> cancel_delayed_work();
>>
>> I tried the patch below.
>>
>> While this does seem to fix the problem as well, I see a potential issue:
>>
>> 1. amdgpu_gfx_off_ctrl locks adev->gfx.gfx_off_mutex
>> 2. amdgpu_device_delay_enable_gfx_off runs, blocks in mutex_lock
>> 3. amdgpu_gfx_off_ctrl calls cancel_delayed_work_sync
>>
>> I'm afraid this would deadlock? (CONFIG_PROVE_LOCKING doesn't complain 
>> though)
> 
> Should use the cancel_delayed_work instead of the _sync version.

The thing is, it's not clear to me from cancel_delayed_work's description that 
it's guaranteed not to wait for amdgpu_device_delay_enable_gfx_off to finish if 
it's already running. If that's not guaranteed, it's prone to the same deadlock.

> As you mentioned - at best work is not scheduled yet and cancelled 
> successfully, or at worst it's waiting for the mutex. In the worst case, if 
> amdgpu_device_delay_enable_gfx_off gets the mutex after amdgpu_gfx_off_ctrl 
> unlocks it, there is an extra check as below.
> 
> if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count)
> 
> The count wouldn't be 0 and hence it won't enable GFXOFF.

I'm not sure, but it might also be possible for 
amdgpu_device_delay_enable_gfx_off to get the mutex only after 
amdgpu_gfx_off_ctrl was called again and set adev->gfx.gfx_off_req_count back 
to 0.


>> Maybe it's possible to fix it with cancel_delayed_work_sync somehow, but I'm 
>> not sure how offhand. (With cancel_delayed_work instead, I'm worried 
>> amdgpu_device_delay_enable_gfx_off might still enable GFXOFF in the HW 
>> immediately after amdgpu_gfx_off_ctrl unlocks the mutex. Then again, that 
>> might happen with mod_delayed_work as well...)
> 
> As mentioned earlier, cancel_delayed_work won't cause this issue.
> 
> In the mod_delayed_ patch, mod_ version is called only when req_count is 0. 
> While that is a good thing, it keeps alive one more contender for the mutex.

Not sure what you mean. It leaves the possibility of 
amdgpu_device_delay_enable_gfx_off running just after amdgpu_gfx_off_ctrl tried 
to postpone it. As discussed above, something similar might be possible with 
cancel_delayed_work as well.

> The cancel_ version eliminates that contender if happens to be called at the 
> right time (more likely if there are multiple requests to disable gfxoff). On 
> the other hand, don't know how costly it is to call cancel_ every time on the 
> else part (or maybe call only once when count increments to 1?).

Sure, why not, though I doubt it matters much — I expect 
adev->gfx.gfx_off_req_count transitioning between 0 <-> 1 to be the most common 
case by far.


I sent out a v2 patch which should address all these issues.


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer

[PATCH] drm/amdgpu: Cancel delayed work when GFXOFF is disabled

[Michel Dänzer]

From: Michel Dänzer 

schedule_delayed_work does not push back the work if it was already
scheduled before, so amdgpu_device_delay_enable_gfx_off ran ~100 ms
after the first time GFXOFF was disabled and re-enabled, even if GFXOFF
was disabled and re-enabled again during those 100 ms.

This resulted in frame drops / stutter with the upcoming mutter 41
release on Navi 14, due to constantly enabling GFXOFF in the HW and
disabling it again (for getting the GPU clock counter).

To fix this, call cancel_delayed_work_sync when GFXOFF transitions from
enabled to disabled. This makes sure the delayed work will be scheduled
as intended in the reverse case.

In order to avoid a deadlock, amdgpu_device_delay_enable_gfx_off needs
to use mutex_trylock instead of mutex_lock.

v2:
* Use cancel_delayed_work_sync & mutex_trylock instead of
  mod_delayed_work.

Signed-off-by: Michel Dänzer 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c| 13 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h|  3 +++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f3fd5ec710b6..8b025f70706c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2777,7 +2777,16 @@ static void amdgpu_device_delay_enable_gfx_off(struct 
work_struct *work)
struct amdgpu_device *adev =
container_of(work, struct amdgpu_device, 
gfx.gfx_off_delay_work.work);
 
-   mutex_lock(>gfx.gfx_off_mutex);
+   /* mutex_lock could deadlock with cancel_delayed_work_sync in 
amdgpu_gfx_off_ctrl. */
+   if (!mutex_trylock(>gfx.gfx_off_mutex)) {
+   /* If there's a bug which causes amdgpu_gfx_off_ctrl to be 
called with enable=true
+* when adev->gfx.gfx_off_req_count is already 0, we might race 
with that.
+* Re-schedule to make sure gfx off will be re-enabled in the 
HW eventually.
+*/
+   schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+   return;
+   }
+
if (!adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) {
if (!amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, true))
adev->gfx.gfx_off_state = true;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index a0be0772c8b3..da4c46db3093 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -28,9 +28,6 @@
 #include "amdgpu_rlc.h"
 #include "amdgpu_ras.h"
 
-/* delay 0.1 second to enable gfx off feature */
-#define GFX_OFF_DELAY_ENABLE msecs_to_jiffies(100)
-
 /*
  * GPU GFX IP block helpers function.
  */
@@ -569,9 +566,13 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool 
enable)
adev->gfx.gfx_off_req_count--;
 
if (enable && !adev->gfx.gfx_off_state && !adev->gfx.gfx_off_req_count) 
{
-   schedule_delayed_work(>gfx.gfx_off_delay_work, 
GFX_OFF_DELAY_ENABLE);
-   } else if (!enable && adev->gfx.gfx_off_state) {
-   if (!amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, false)) {
+   schedule_delayed_work(>gfx.gfx_off_delay_work, 
AMDGPU_GFX_OFF_DELAY_ENABLE);
+   } else if (!enable) {
+   if (adev->gfx.gfx_off_req_count == 1 && 
!adev->gfx.gfx_off_state)
+   cancel_delayed_work_sync(>gfx.gfx_off_delay_work);
+
+   if (adev->gfx.gfx_off_state &&
+   !amdgpu_dpm_set_powergating_by_smu(adev, 
AMD_IP_BLOCK_TYPE_GFX, false)) {
adev->gfx.gfx_off_state = false;
 
if (adev->gfx.funcs->init_spm_golden) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index d43fe2ed8116..dcdb505bb7f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -32,6 +32,9 @@
 #include "amdgpu_rlc.h"
 #include "soc15.h"
 
+/* delay 0.1 second to enable gfx off feature */
+#define AMDGPU_GFX_OFF_DELAY_ENABLE msecs_to_jiffies(100)
+
 /* GFX current status */
 #define AMDGPU_GFX_NORMAL_MODE 0xL
 #define AMDGPU_GFX_SAFE_MODE   0x0001L
-- 
2.32.0

RE: [PATCH 2/2] drm/amd/pm: change the workload type for some cards

[Zhang, Hawking]

[AMD Official Use Only]

Series is

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Kenneth Feng  
Sent: Friday, August 13, 2021 16:48
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
Subject: [PATCH 2/2] drm/amd/pm: change the workload type for some cards

change the workload type for some cards as it is needed.

Signed-off-by: Kenneth Feng 
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index a9bd8265b508..f3cd397cb56b 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -5127,6 +5127,13 @@ static int vega10_get_power_profile_mode(struct pp_hwmgr 
*hwmgr, char *buf)
return size;
 }
 
+static bool vega10_get_power_profile_mode_quirks(struct pp_hwmgr 
+*hwmgr) {
+   struct amdgpu_device *adev = hwmgr->adev;
+
+   return (adev->pdev->device == 0x6860); }
+
 static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, 
uint32_t size)  {
struct vega10_hwmgr *data = hwmgr->backend; @@ -5163,9 +5170,15 @@ 
static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui
}
 
 out:
-   smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask,
+   if (vega10_get_power_profile_mode_quirks(hwmgr))
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_SetWorkloadMask,
1 << power_profile_mode,
NULL);
+   else
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_SetWorkloadMask,
+   (!power_profile_mode) ? 0 : 1 
<< (power_profile_mode - 1),
+   NULL);
+
hwmgr->power_profile_mode = power_profile_mode;
 
return 0;
--
2.17.1

[PATCH 2/2] drm/amd/pm: change the workload type for some cards

[Kenneth Feng]

change the workload type for some cards as it is needed.

Signed-off-by: Kenneth Feng 
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index a9bd8265b508..f3cd397cb56b 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -5127,6 +5127,13 @@ static int vega10_get_power_profile_mode(struct pp_hwmgr 
*hwmgr, char *buf)
return size;
 }
 
+static bool vega10_get_power_profile_mode_quirks(struct pp_hwmgr *hwmgr)
+{
+   struct amdgpu_device *adev = hwmgr->adev;
+
+   return (adev->pdev->device == 0x6860);
+}
+
 static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, 
uint32_t size)
 {
struct vega10_hwmgr *data = hwmgr->backend;
@@ -5163,9 +5170,15 @@ static int vega10_set_power_profile_mode(struct pp_hwmgr 
*hwmgr, long *input, ui
}
 
 out:
-   smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask,
+   if (vega10_get_power_profile_mode_quirks(hwmgr))
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_SetWorkloadMask,
1 << power_profile_mode,
NULL);
+   else
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_SetWorkloadMask,
+   (!power_profile_mode) ? 0 : 1 
<< (power_profile_mode - 1),
+   NULL);
+
hwmgr->power_profile_mode = power_profile_mode;
 
return 0;
-- 
2.17.1

[PATCH 1/2] Revert "drm/amd/pm: fix workload mismatch on vega10"

[Kenneth Feng]

This reverts commit 0979d43259e13846d86ba17e451e17fec185d240.
Revert this because it does not apply to all the cards.

Signed-off-by: Kenneth Feng 
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 621a49f86b0c..a9bd8265b508 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -5164,7 +5164,7 @@ static int vega10_set_power_profile_mode(struct pp_hwmgr 
*hwmgr, long *input, ui
 
 out:
smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask,
-   (!power_profile_mode) ? 0 : 1 
<< (power_profile_mode - 1),
+   1 << power_profile_mode,
NULL);
hwmgr->power_profile_mode = power_profile_mode;
 
-- 
2.17.1

[PATCH] drm/amdgpu: disable BACO support for 699F:C7 polaris12 SKU temporarily

[Evan Quan]

We have a S3 issue on that SKU with BACO enabled. Will bring back this
when that root caused.

Change-Id: I56d4830e6275e20a415808896eecbadfe944070b
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index fe9a7cc8d9eb..7210f80815b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -904,7 +904,12 @@ static bool vi_asic_supports_baco(struct amdgpu_device 
*adev)
case CHIP_POLARIS11:
case CHIP_POLARIS12:
case CHIP_TOPAZ:
-   return amdgpu_dpm_is_baco_supported(adev);
+   /* Disable BACO support for the specific polaris12 SKU 
temporarily */
+   if ((adev->pdev->device == 0x699F) &&
+   (adev->pdev->revision == 0xC7))
+   return false;
+   else
+   return amdgpu_dpm_is_baco_supported(adev);
default:
return false;
}
-- 
2.29.0

Re: [Mesa-dev] Requests For Proposals for hosting XDC 2022 are now open

[Samuel Iglesias Gonsálvez]

Deadline is at the end of this month. Do not forget to submit your XDC
2022 hosting proposal!

Sam

On Thu, 2021-07-29 at 21:01 +0200, Samuel Iglesias Gonsálvez wrote:
> Remember before enjoying your holiday that the deadline for XDC 2022
> proposals is *September 1st, 2021* :-)
> 
> Feel free to submit your proposal before, so we can give you early
> feedback on it!
> 
> Sam
> 
> On Thu, 2021-07-01 at 18:14 +0200, Samuel Iglesias Gonsálvez wrote:
> > This is a reminder that the call for proposals for hosting XDC 2022
> > period finishes in two months.
> > 
> > Be sure to prepare your submission before you leave on holiday!
> > 
> > Sam
> > 
> > On Thu, 2021-05-20 at 12:15 +0200, Samuel Iglesias Gonsálvez wrote:
> > > Hello everyone!
> > > 
> > > The X.org board is soliciting proposals to host XDC in 2022.
> > > Since
> > > XDC 2021 is being held in Europe this year (although virtually),
> > > we've
> > > decided to host in North America. However, the board is open to
> > > other
> > > locations, especially if there's an interesting co-location with
> > > another conference.
> > > 
> > > Of course though, due to the ongoing COVID-19 pandemic it's not
> > > yet
> > > clear whether or not it will be possible to host XDC 2022 in
> > > person,
> > > although is seems very likely. Because of this, we would like to
> > > make it clear that sponsors should prepare for both the
> > > possibility
> > > of an in person conference, and the possibility of a virtual
> > > conference. We will work with organizers on coming up with a
> > > deadline for deciding whether or not we'll be going virtual,
> > > likely
> > > sometime around July 2022.
> > > 
> > > If you're considering hosting XDC, we've assembled a wiki page
> > > with
> > > what's generally expected and needed:
> > > 
> > > https://www.x.org/wiki/Events/RFP/
> > > 
> > > When submitting your proposal, please make sure to include at
> > > least
> > > the
> > > key information about the potential location in question,
> > > possible
> > > dates along with estimated costs. Proposals can be submitted to
> > > board
> > > at foundation.x.org until the deadline of *September 1st, 2021*. 
> > > 
> > > Additionally, an quirk early heads-up to the board if you're
> > > considering hosting would be appreciated, in case we need to
> > > adjust
> > > the
> > > schedule a bit. Also, earlier is better since there generally
> > > will
> > > be
> > > a
> > > bit of Q with organizers.
> > > 
> > > And if you just have some questions about what organizing XDC
> > > entails,
> > > please feel free to chat with previous organizers, or someone
> > > from
> > > the
> > > board.
> > > 
> > > Thanks,
> > > 
> > > Sam
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> > ___
> > mesa-dev mailing list
> > mesa-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



signature.asc
Description: This is a digitally signed message part

[PATCH] drivers:gpu:drm:amd:amdgpu:fix a potential use-after-free

[lwt105]

in line 1503, "dma_fence_put(fence);" drop the reference to fence and may
cause fence to be released. However, fence is used subsequently in line
1510 "fence->error". This may result in an use-after-free bug.

It can be fixed by recording fence->error in an variable before dropping
the reference to fence and referencing it after dropping.

Signed-off-by: lwt105 <3061522...@qq.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 30fa1f61e0e5..99d03180e113 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1486,7 +1486,7 @@ static int amdgpu_cs_wait_all_fences(struct amdgpu_device 
*adev,
 struct drm_amdgpu_fence *fences)
 {
uint32_t fence_count = wait->in.fence_count;
-   unsigned int i;
+   unsigned int i, error;
long r = 1;
 
for (i = 0; i < fence_count; i++) {
@@ -1500,6 +1500,7 @@ static int amdgpu_cs_wait_all_fences(struct amdgpu_device 
*adev,
continue;
 
r = dma_fence_wait_timeout(fence, true, timeout);
+   error = fence->error;
dma_fence_put(fence);
if (r < 0)
return r;
@@ -1507,8 +1508,8 @@ static int amdgpu_cs_wait_all_fences(struct amdgpu_device 
*adev,
if (r == 0)
break;
 
-   if (fence->error)
-   return fence->error;
+   if (error)
+   return error;
}
 
memset(wait, 0, sizeof(*wait));
-- 
2.25.1

[PATCH 7/7] drm/amd/display: 3.2.149

[Wayne Lin]

From: Aric Cyr 

This version brings along following fixes:
* Ensure DCN save init registers after VM setup
* Fix multi-display support for idle opt workqueue
* Use vblank control events for PSR enable/disable
* Create default dc_sink when fail reading EDID under MST

Reviewed-by: Aric Cyr 
Acked-by: Wayne Lin 
Signed-off-by: Aric Cyr 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 0494e6dcf4dc..3ab52d9a82cf 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -45,7 +45,7 @@
 /* forward declaration */
 struct aux_payload;
 
-#define DC_VER "3.2.148"
+#define DC_VER "3.2.149"
 
 #define MAX_SURFACES 3
 #define MAX_PLANES 6
-- 
2.25.1

[PATCH 6/7] drm/amd/display: [FW Promotion] Release 0.0.79

[Wayne Lin]

From: Anthony Koo 

Reviewed-by: Anthony Koo 
Acked-by: Wayne Lin 
Signed-off-by: Anthony Koo 
---
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h 
b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
index a6f3d58f82c6..7b684e7f60df 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -47,10 +47,10 @@
 
 /* Firmware versioning. */
 #ifdef DMUB_EXPOSE_VERSION
-#define DMUB_FW_VERSION_GIT_HASH 0x9b7fa7783
+#define DMUB_FW_VERSION_GIT_HASH 0x7383caadc
 #define DMUB_FW_VERSION_MAJOR 0
 #define DMUB_FW_VERSION_MINOR 0
-#define DMUB_FW_VERSION_REVISION 78
+#define DMUB_FW_VERSION_REVISION 79
 #define DMUB_FW_VERSION_TEST 0
 #define DMUB_FW_VERSION_VBIOS 0
 #define DMUB_FW_VERSION_HOTFIX 0
@@ -322,6 +322,10 @@ union dmub_fw_boot_status {
uint32_t mailbox_rdy : 1; /**< 1 if mailbox ready */
uint32_t optimized_init_done : 1; /**< 1 if optimized init done 
*/
uint32_t restore_required : 1; /**< 1 if driver should call 
restore */
+   uint32_t defer_load : 1; /**< 1 if VBIOS data is deferred 
programmed */
+   uint32_t reserved : 1;
+   uint32_t detection_required: 1; /**<  if detection need to be 
triggered by driver */
+
} bits; /**< status bits */
uint32_t all; /**< 32-bit access to status bits */
 };
@@ -335,6 +339,7 @@ enum dmub_fw_boot_status_bit {
DMUB_FW_BOOT_STATUS_BIT_OPTIMIZED_INIT_DONE = (1 << 2), /**< 1 if init 
done */
DMUB_FW_BOOT_STATUS_BIT_RESTORE_REQUIRED = (1 << 3), /**< 1 if driver 
should call restore */
DMUB_FW_BOOT_STATUS_BIT_DEFERRED_LOADED = (1 << 4), /**< 1 if VBIOS 
data is deferred programmed */
+   DMUB_FW_BOOT_STATUS_BIT_DETECTION_REQUIRED = (1 << 6), /**< 1 if 
detection need to be triggered by driver*/
 };
 
 /* Register bit definition for SCRATCH5 */
@@ -489,6 +494,11 @@ enum dmub_gpint_command {
 * RETURN: PSR residency in milli-percent.
 */
DMUB_GPINT__PSR_RESIDENCY = 9,
+
+   /**
+* DESC: Notifies DMCUB detection is done so detection required can be 
cleared.
+*/
+   DMUB_GPINT__NOTIFY_DETECTION_DONE = 12,
 };
 
 /**
-- 
2.25.1

[PATCH 5/7] drm/amd/display: Guard vblank wq flush with DCN guards

[Wayne Lin]

From: Nicholas Kazlauskas 

[Why]
Compilation of the workqueue fails if not building with the DCN config
option set.

[How]
Guard calls to the flush with the DCN config option to fix the build.

Reviewed-by: Roman Li 
Acked-by: Wayne Lin 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
 1 file changed, 4 insertions(+)

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 cebd663b6708..816723691d51 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8643,11 +8643,13 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
/* Update the planes if changed or disable if we don't have any. */
if ((planes_count || acrtc_state->active_planes == 0) &&
acrtc_state->stream) {
+#if defined(CONFIG_DRM_AMD_DC_DCN)
/*
 * If PSR or idle optimizations are enabled then flush out
 * any pending work before hardware programming.
 */
flush_workqueue(dm->vblank_control_workqueue);
+#endif
 
bundle->stream_update.stream = acrtc_state->stream;
if (new_pcrtc_state->mode_changed) {
@@ -8980,7 +8982,9 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
if (dc_state) {
/* if there mode set or reset, disable eDP PSR */
if (mode_set_reset_required) {
+#if defined(CONFIG_DRM_AMD_DC_DCN)
flush_workqueue(dm->vblank_control_workqueue);
+#endif
amdgpu_dm_psr_disable_all(dm);
}
 
-- 
2.25.1

[PATCH 4/7] drm/amd/display: Ensure DCN save after VM setup

[Wayne Lin]

From: Jake Wang 

[Why]
DM initializes VM context after DMCUB initialization.
This results in loss of DCN_VM_CONTEXT registers after z10.

[How]
Notify DMCUB when VM setup is complete, and have DMCUB
save init registers.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Wayne Lin 
Signed-off-by: Jake Wang 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  6 ++
 drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c |  3 +++
 drivers/gpu/drm/amd/display/dc/dc.h|  1 +
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c | 12 
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h |  1 +
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c  |  1 +
 drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h  |  1 +
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h|  5 +
 8 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 7a442fcfa6ac..c798c65d4276 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1549,6 +1549,12 @@ void dc_z10_restore(struct dc *dc)
if (dc->hwss.z10_restore)
dc->hwss.z10_restore(dc);
 }
+
+void dc_z10_save_init(struct dc *dc)
+{
+   if (dc->hwss.z10_save_init)
+   dc->hwss.z10_save_init(dc);
+}
 #endif
 /*
  * Applies given context to HW and copy it into current context.
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
index f2b39ec35c89..80c752ca4a5a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
@@ -47,6 +47,9 @@ int dc_setup_system_context(struct dc *dc, struct 
dc_phy_addr_space_config *pa_c
 */
memcpy(>vm_pa_config, pa_config, sizeof(struct 
dc_phy_addr_space_config));
dc->vm_pa_config.valid = true;
+#if defined(CONFIG_DRM_AMD_DC_DCN3_1)
+   dc_z10_save_init(dc);
+#endif
}
 
return num_vmids;
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 03b81e5c5d67..0494e6dcf4dc 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1338,6 +1338,7 @@ void dc_hardware_release(struct dc *dc);
 bool dc_set_psr_allow_active(struct dc *dc, bool enable);
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 void dc_z10_restore(struct dc *dc);
+void dc_z10_save_init(struct dc *dc);
 #endif
 
 bool dc_enable_dmub_notifications(struct dc *dc);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
index 6399e8acd093..3f2333ec67e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
@@ -407,6 +407,18 @@ void dcn31_update_info_frame(struct pipe_ctx *pipe_ctx)
_ctx->stream_res.encoder_info_frame);
}
 }
+void dcn31_z10_save_init(struct dc *dc)
+{
+   union dmub_rb_cmd cmd;
+
+   memset(, 0, sizeof(cmd));
+   cmd.dcn_restore.header.type = DMUB_CMD__IDLE_OPT;
+   cmd.dcn_restore.header.sub_type = DMUB_CMD__IDLE_OPT_DCN_SAVE_INIT;
+
+   dc_dmub_srv_cmd_queue(dc->ctx->dmub_srv, );
+   dc_dmub_srv_cmd_execute(dc->ctx->dmub_srv);
+   dc_dmub_srv_wait_idle(dc->ctx->dmub_srv);
+}
 
 void dcn31_z10_restore(struct dc *dc)
 {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
index 40dfebe78fdd..140435e4f7ff 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
@@ -44,6 +44,7 @@ void dcn31_enable_power_gating_plane(
 void dcn31_update_info_frame(struct pipe_ctx *pipe_ctx);
 
 void dcn31_z10_restore(struct dc *dc);
+void dcn31_z10_save_init(struct dc *dc);
 
 void dcn31_hubp_pg_control(struct dce_hwseq *hws, unsigned int hubp_inst, bool 
power_on);
 int dcn31_init_sys_ctx(struct dce_hwseq *hws, struct dc *dc, struct 
dc_phy_addr_space_config *pa_config);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
index 05954045c332..40011cd3c8ef 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
@@ -97,6 +97,7 @@ static const struct hw_sequencer_funcs dcn31_funcs = {
.set_abm_immediate_disable = dcn21_set_abm_immediate_disable,
.set_pipe = dcn21_set_pipe,
.z10_restore = dcn31_z10_restore,
+   .z10_save_init = dcn31_z10_save_init,
.is_abm_supported = dcn31_is_abm_supported,
.set_disp_pattern_generator = dcn30_set_disp_pattern_generator,
.update_visual_confirm_color = dcn20_update_visual_confirm_color,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
index 5ab008e62b82..ad5f2adcc40d 

[PATCH 3/7] drm/amd/display: Use vblank control events for PSR enable/disable

[Wayne Lin]

From: Nicholas Kazlauskas 

[Why]
PSR can disable the HUBP along with the OTG when PSR is active.

We'll hit a pageflip timeout when the OTG is disable because we're no
longer updating the CRTC vblank counter and the pflip high IRQ will
not fire on the flip.

In order to flip the page flip timeout occur we should modify the
enter/exit conditions to match DRM requirements.

[How]
Use our deferred handlers for DRM vblank control to notify DMCU(B)
when it can enable or disable PSR based on whether vblank is disabled or
enabled respectively.

We'll need to pass along the stream with the notification now because
we want to access the CRTC state while the CRTC is locked to get the
stream state prior to the commit.

Retain a reference to the stream so it remains safe to continue to
access and release that reference once we're done with it.

Enable/disable logic follows what we were previously doing in
update_planes.

The workqueue has to be flushed before programming streams or planes
to ensure that we exit out of idle optimizations and PSR before
these events occur if necessary.

To keep the skip count logic the same to avoid FBCON PSR enablement
requires copying the allow condition onto the DM IRQ parameters - a
field that we can actually access from the worker.

Reviewed-by: Roman Li 
Acked-by: Wayne Lin 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |  1 +
 3 files changed, 43 insertions(+), 8 deletions(-)

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 f88b6c5b83cd..cebd663b6708 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1061,7 +1061,22 @@ static void vblank_control_worker(struct work_struct 
*work)
 
DRM_DEBUG_KMS("Allow idle optimizations (MALL): %d\n", 
dm->active_vblank_irq_count == 0);
 
+   /* Control PSR based on vblank requirements from OS */
+   if (vblank_work->stream && vblank_work->stream->link) {
+   if (vblank_work->enable) {
+   if 
(vblank_work->stream->link->psr_settings.psr_allow_active)
+   amdgpu_dm_psr_disable(vblank_work->stream);
+   } else if 
(vblank_work->stream->link->psr_settings.psr_feature_enabled &&
+  
!vblank_work->stream->link->psr_settings.psr_allow_active &&
+  vblank_work->acrtc->dm_irq_params.allow_psr_entry) {
+   amdgpu_dm_psr_enable(vblank_work->stream);
+   }
+   }
+
mutex_unlock(>dc_lock);
+
+   dc_stream_release(vblank_work->stream);
+
kfree(vblank_work);
 }
 
@@ -6018,6 +6033,11 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
work->acrtc = acrtc;
work->enable = enable;
 
+   if (acrtc_state->stream) {
+   dc_stream_retain(acrtc_state->stream);
+   work->stream = acrtc_state->stream;
+   }
+
queue_work(dm->vblank_control_workqueue, >work);
 #endif
 
@@ -8623,6 +8643,12 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
/* Update the planes if changed or disable if we don't have any. */
if ((planes_count || acrtc_state->active_planes == 0) &&
acrtc_state->stream) {
+   /*
+* If PSR or idle optimizations are enabled then flush out
+* any pending work before hardware programming.
+*/
+   flush_workqueue(dm->vblank_control_workqueue);
+
bundle->stream_update.stream = acrtc_state->stream;
if (new_pcrtc_state->mode_changed) {
bundle->stream_update.src = acrtc_state->stream->src;
@@ -8691,16 +8717,20 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,

acrtc_state->stream->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED &&

!acrtc_state->stream->link->psr_settings.psr_feature_enabled)
amdgpu_dm_link_setup_psr(acrtc_state->stream);
-   else if ((acrtc_state->update_type == UPDATE_TYPE_FAST) &&
-   
acrtc_state->stream->link->psr_settings.psr_feature_enabled &&
-   
!acrtc_state->stream->link->psr_settings.psr_allow_active) {
-   struct amdgpu_dm_connector *aconn = (struct 
amdgpu_dm_connector *)
-   acrtc_state->stream->dm_stream_context;
+
+   /* Decrement skip count when PSR is enabled and we're doing 
fast updates. */
+   if (acrtc_state->update_type == UPDATE_TYPE_FAST &&
+   

[PATCH 2/7] drm/amd/display: Fix multi-display support for idle opt workqueue

[Wayne Lin]

From: Nicholas Kazlauskas 

[Why]
The current implementation for idle optimization support only has a
single work item that gets reshuffled into the system workqueue
whenever we receive an enable or disable event.

We can have mismatched events if the work hasn't been processed or if
we're getting control events from multiple displays at once.

This fixes this issue and also makes the implementation usable for
PSR control - which will be addressed in another patch.

[How]
We need to be able to flush remaining work out on demand for driver stop
and psr disable so create a driver specific workqueue instead of using
the system one. The workqueue will be single threaded to guarantee the
ordering of enable/disable events.

Refactor the queue to allocate the control work and deallocate it
after processing it.

Pass the acrtc directly to make it easier to handle psr enable/disable
in a later patch.

Rename things to indicate that it's not just MALL specific.

Reviewed-by: Roman Li 
Acked-by: Wayne Lin 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 62 ---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 21 ---
 2 files changed, 36 insertions(+), 47 deletions(-)

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 3e28f17c84fa..f88b6c5b83cd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1044,10 +1044,10 @@ static void mmhub_read_system_context(struct 
amdgpu_device *adev, struct dc_phy_
 }
 #endif
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-static void event_mall_stutter(struct work_struct *work)
+static void vblank_control_worker(struct work_struct *work)
 {
-
-   struct vblank_workqueue *vblank_work = container_of(work, struct 
vblank_workqueue, mall_work);
+   struct vblank_control_work *vblank_work =
+   container_of(work, struct vblank_control_work, work);
struct amdgpu_display_manager *dm = vblank_work->dm;
 
mutex_lock(>dc_lock);
@@ -1062,22 +1062,9 @@ static void event_mall_stutter(struct work_struct *work)
DRM_DEBUG_KMS("Allow idle optimizations (MALL): %d\n", 
dm->active_vblank_irq_count == 0);
 
mutex_unlock(>dc_lock);
+   kfree(vblank_work);
 }
 
-static struct vblank_workqueue *vblank_create_workqueue(struct amdgpu_device 
*adev, struct dc *dc)
-{
-   struct vblank_workqueue *vblank_work;
-
-   vblank_work = kzalloc(sizeof(*vblank_work), GFP_KERNEL);
-   if (ZERO_OR_NULL_PTR(vblank_work)) {
-   kfree(vblank_work);
-   return NULL;
-   }
-
-   INIT_WORK(_work->mall_work, event_mall_stutter);
-
-   return vblank_work;
-}
 #endif
 static int amdgpu_dm_init(struct amdgpu_device *adev)
 {
@@ -1220,12 +1207,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
 #if defined(CONFIG_DRM_AMD_DC_DCN)
if (adev->dm.dc->caps.max_links > 0) {
-   adev->dm.vblank_workqueue = vblank_create_workqueue(adev, 
adev->dm.dc);
-
-   if (!adev->dm.vblank_workqueue)
+   adev->dm.vblank_control_workqueue =
+   
create_singlethread_workqueue("dm_vblank_control_workqueue");
+   if (!adev->dm.vblank_control_workqueue)
DRM_ERROR("amdgpu: failed to initialize 
vblank_workqueue.\n");
-   else
-   DRM_DEBUG_DRIVER("amdgpu: vblank_workqueue init done 
%p.\n", adev->dm.vblank_workqueue);
}
 #endif
 
@@ -1298,6 +1283,13 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
 {
int i;
 
+#if defined(CONFIG_DRM_AMD_DC_DCN)
+   if (adev->dm.vblank_control_workqueue) {
+   destroy_workqueue(adev->dm.vblank_control_workqueue);
+   adev->dm.vblank_control_workqueue = NULL;
+   }
+#endif
+
for (i = 0; i < adev->dm.display_indexes_num; i++) {
drm_encoder_cleanup(>dm.mst_encoders[i].base);
}
@@ -1321,14 +1313,6 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
dc_deinit_callbacks(adev->dm.dc);
 #endif
 
-#if defined(CONFIG_DRM_AMD_DC_DCN)
-   if (adev->dm.vblank_workqueue) {
-   adev->dm.vblank_workqueue->dm = NULL;
-   kfree(adev->dm.vblank_workqueue);
-   adev->dm.vblank_workqueue = NULL;
-   }
-#endif
-
dc_dmub_srv_destroy(>dm.dc->ctx->dmub_srv);
 
if (dc_enable_dmub_notifications(adev->dm.dc)) {
@@ -6000,7 +5984,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
 #if defined(CONFIG_DRM_AMD_DC_DCN)
struct amdgpu_display_manager *dm = >dm;
-   unsigned long flags;
+   struct vblank_control_work *work;
 #endif
int rc = 0;
 
@@ -6025,12 +6009,16 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
  

[PATCH 1/7] drm/amd/display: Create dc_sink when EDID fail

[Wayne Lin]

[Why]
While reading remote EDID via Startech 1-to-4 hub, occasionally we
won't get response in time and won't light up corresponding monitor.

Ideally, we can still add generic modes for userspace to choose to try
to light up the monitor and which is done in
drm_helper_probe_single_connector_modes(). So the main problem here is
that we fail .mode_valid since we don't create remote dc_sink for this
case.

[How]
Also add default dc_sink if we can't get the EDID.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Wayne Lin 
Signed-off-by: Wayne Lin 
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5568d4e518e6..1bcba6943fd7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -213,6 +213,29 @@ static int dm_dp_mst_get_modes(struct drm_connector 
*connector)
drm_connector_update_edid_property(
>base,
NULL);
+
+   DRM_DEBUG_KMS("Can't get EDID of %s. Add default remote 
sink.", connector->name);
+   if (!aconnector->dc_sink) {
+   struct dc_sink *dc_sink;
+   struct dc_sink_init_data init_params = {
+   .link = aconnector->dc_link,
+   .sink_signal = 
SIGNAL_TYPE_DISPLAY_PORT_MST };
+
+   dc_sink = dc_link_add_remote_sink(
+   aconnector->dc_link,
+   NULL,
+   0,
+   _params);
+
+   if (!dc_sink) {
+   DRM_ERROR("Unable to add a remote 
sink\n");
+   return 0;
+   }
+
+   dc_sink->priv = aconnector;
+   aconnector->dc_sink = dc_sink;
+   }
+
return ret;
}
 
-- 
2.25.1

[PATCH 0/7] DC Patches August 13, 2021

[Wayne Lin]

This DC patchset brings improvements in multiple areas. In summary, we 
highlight:

* Ensure DCN save init registers after VM setup
* Fix multi-display support for idle opt workqueue
* Use vblank control events for PSR enable/disable
* Create default dc_sink when fail reading EDID under MST

---

Anthony Koo (1):
  drm/amd/display: [FW Promotion] Release 0.0.79

Aric Cyr (1):
  drm/amd/display: 3.2.149

Jake Wang (1):
  drm/amd/display: Ensure DCN save after VM setup

Nicholas Kazlauskas (3):
  drm/amd/display: Fix multi-display support for idle opt workqueue
  drm/amd/display: Use vblank control events for PSR enable/disable
  drm/amd/display: Guard vblank wq flush with DCN guards

Wayne Lin (1):
  drm/amd/display: Create dc_sink when EDID fail

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 112 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  23 ++--
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |   1 +
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  23 
 drivers/gpu/drm/amd/display/dc/core/dc.c  |   6 +
 .../drm/amd/display/dc/core/dc_vm_helper.c|   3 +
 drivers/gpu/drm/amd/display/dc/dc.h   |   3 +-
 .../drm/amd/display/dc/dcn31/dcn31_hwseq.c|  12 ++
 .../drm/amd/display/dc/dcn31/dcn31_hwseq.h|   1 +
 .../gpu/drm/amd/display/dc/dcn31/dcn31_init.c |   1 +
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |   1 +
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   |  19 ++-
 12 files changed, 148 insertions(+), 57 deletions(-)

-- 
2.25.1

[PATCH v6 12/13] tools: update hmm-test to support device generic type

[Alex Sierra]

Test cases such as migrate_fault and migrate_multiple,
were modified to explicit migrate from device to sys memory
without the need of page faults, when using device generic
type.

Snapshot test case updated to read memory device type
first and based on that, get the proper returned results
migrate_ping_pong test case added to test explicit migration
from device to sys memory for both private and generic
zone types.

Helpers to migrate from device to sys memory and vicerversa
were also added.

Signed-off-by: Alex Sierra 
---
 tools/testing/selftests/vm/hmm-tests.c | 142 +
 1 file changed, 124 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/vm/hmm-tests.c 
b/tools/testing/selftests/vm/hmm-tests.c
index 5d1ac691b9f4..70632b195497 100644
--- a/tools/testing/selftests/vm/hmm-tests.c
+++ b/tools/testing/selftests/vm/hmm-tests.c
@@ -44,6 +44,8 @@ struct hmm_buffer {
int fd;
uint64_tcpages;
uint64_tfaults;
+   int zone_device_type;
+   boolalloc_to_devmem;
 };
 
 #define TWOMEG (1 << 21)
@@ -133,6 +135,7 @@ static int hmm_dmirror_cmd(int fd,
cmd.addr = (__u64)buffer->ptr;
cmd.ptr = (__u64)buffer->mirror;
cmd.npages = npages;
+   cmd.alloc_to_devmem = buffer->alloc_to_devmem;
 
for (;;) {
ret = ioctl(fd, request, );
@@ -144,6 +147,7 @@ static int hmm_dmirror_cmd(int fd,
}
buffer->cpages = cmd.cpages;
buffer->faults = cmd.faults;
+   buffer->zone_device_type = cmd.zone_device_type;
 
return 0;
 }
@@ -211,6 +215,34 @@ static void hmm_nanosleep(unsigned int n)
nanosleep(, NULL);
 }
 
+static int hmm_migrate_sys_to_dev(int fd,
+  struct hmm_buffer *buffer,
+  unsigned long npages)
+{
+   buffer->alloc_to_devmem = true;
+   return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+}
+
+static int hmm_migrate_dev_to_sys(int fd,
+  struct hmm_buffer *buffer,
+  unsigned long npages)
+{
+   buffer->alloc_to_devmem = false;
+   return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+}
+
+static int hmm_is_private_device(int fd, bool *res)
+{
+   struct hmm_buffer buffer;
+   int ret;
+
+   buffer.ptr = 0;
+   ret = hmm_dmirror_cmd(fd, HMM_DMIRROR_GET_MEM_DEV_TYPE, , 1);
+   *res = (buffer.zone_device_type == HMM_DMIRROR_MEMORY_DEVICE_PRIVATE);
+
+   return ret;
+}
+
 /*
  * Simple NULL test of device open/close.
  */
@@ -875,7 +907,7 @@ TEST_F(hmm, migrate)
ptr[i] = i;
 
/* Migrate memory to device. */
-   ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+   ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages, npages);
 
@@ -923,7 +955,7 @@ TEST_F(hmm, migrate_fault)
ptr[i] = i;
 
/* Migrate memory to device. */
-   ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+   ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages, npages);
 
@@ -936,7 +968,7 @@ TEST_F(hmm, migrate_fault)
ASSERT_EQ(ptr[i], i);
 
/* Migrate memory to the device again. */
-   ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+   ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages, npages);
 
@@ -976,7 +1008,7 @@ TEST_F(hmm, migrate_shared)
ASSERT_NE(buffer->ptr, MAP_FAILED);
 
/* Migrate memory to device. */
-   ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+   ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, -ENOENT);
 
hmm_buffer_free(buffer);
@@ -1015,7 +1047,7 @@ TEST_F(hmm2, migrate_mixed)
p = buffer->ptr;
 
/* Migrating a protected area should be an error. */
-   ret = hmm_dmirror_cmd(self->fd1, HMM_DMIRROR_MIGRATE, buffer, npages);
+   ret = hmm_migrate_sys_to_dev(self->fd1, buffer, npages);
ASSERT_EQ(ret, -EINVAL);
 
/* Punch a hole after the first page address. */
@@ -1023,7 +1055,7 @@ TEST_F(hmm2, migrate_mixed)
ASSERT_EQ(ret, 0);
 
/* We expect an error if the vma doesn't cover the range. */
-   ret = hmm_dmirror_cmd(self->fd1, HMM_DMIRROR_MIGRATE, buffer, 3);
+   ret = hmm_migrate_sys_to_dev(self->fd1, buffer, 3);
ASSERT_EQ(ret, -EINVAL);
 
/* Page 2 will be a read-only zero page. */
@@ -1055,13 +1087,13 @@ TEST_F(hmm2, migrate_mixed)
 
/* Now try to migrate pages 2-5 to device 1. */
buffer->ptr = p + 2 * self->page_size;
-   ret = hmm_dmirror_cmd(self->fd1, HMM_DMIRROR_MIGRATE, buffer, 4);
+   ret = 

[PATCH v6 11/13] lib: add support for device generic type in test_hmm

[Alex Sierra]

Device Generic type uses device memory that is coherently
accesible by the CPU. Usually, this is shown as SP
(special purpose) memory range at the BIOS-e820 memory
enumeration. If no SP memory is supported in system,
this could be faked by setting CONFIG_EFI_FAKE_MEMMAP.

Currently, test_hmm only supports two different SP ranges
of at least 256MB size. This could be specified in the
kernel parameter variable efi_fake_mem. Ex. Two SP ranges
of 1GB starting at 0x1 & 0x14000 physical address.
efi_fake_mem=1G@0x1:0x4,1G@0x14000:0x4

Signed-off-by: Alex Sierra 
---
 lib/test_hmm.c  | 166 +++-
 lib/test_hmm_uapi.h |  10 ++-
 2 files changed, 113 insertions(+), 63 deletions(-)

diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index b4f885c6c6ae..42edcc8eaad2 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -469,6 +469,7 @@ static int dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
unsigned long pfn_first;
unsigned long pfn_last;
void *ptr;
+   int ret = -ENOMEM;
 
devmem = kzalloc(sizeof(*devmem), GFP_KERNEL);
if (!devmem)
@@ -550,7 +551,7 @@ static int dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
}
spin_unlock(>lock);
 
-   return true;
+   return 0;
 
 err_release:
mutex_unlock(>devmem_lock);
@@ -558,7 +559,7 @@ static int dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
 err_devmem:
kfree(devmem);
 
-   return false;
+   return ret;
 }
 
 static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice)
@@ -567,8 +568,10 @@ static struct page *dmirror_devmem_alloc_page(struct 
dmirror_device *mdevice)
struct page *rpage;
 
/*
-* This is a fake device so we alloc real system memory to store
-* our device memory.
+* For ZONE_DEVICE private type, this is a fake device so we alloc real
+* system memory to store our device memory.
+* For ZONE_DEVICE generic type we use the actual dpage to store the 
data
+* and ignore rpage.
 */
rpage = alloc_page(GFP_HIGHUSER);
if (!rpage)
@@ -601,7 +604,7 @@ static void dmirror_migrate_alloc_and_copy(struct 
migrate_vma *args,
   struct dmirror *dmirror)
 {
struct dmirror_device *mdevice = dmirror->mdevice;
-   const unsigned long *src = args->src;
+   unsigned long *src = args->src;
unsigned long *dst = args->dst;
unsigned long addr;
 
@@ -619,12 +622,18 @@ static void dmirror_migrate_alloc_and_copy(struct 
migrate_vma *args,
 * unallocated pte_none() or read-only zero page.
 */
spage = migrate_pfn_to_page(*src);
-
+   if (spage && is_zone_device_page(spage)) {
+   pr_debug("page already in device spage pfn: 0x%lx\n",
+ page_to_pfn(spage));
+   *src &= ~MIGRATE_PFN_MIGRATE;
+   continue;
+   }
dpage = dmirror_devmem_alloc_page(mdevice);
if (!dpage)
continue;
 
-   rpage = dpage->zone_device_data;
+   rpage = is_device_private_page(dpage) ? dpage->zone_device_data 
:
+   dpage;
if (spage)
copy_highpage(rpage, spage);
else
@@ -636,8 +645,10 @@ static void dmirror_migrate_alloc_and_copy(struct 
migrate_vma *args,
 * the simulated device memory and that page holds the pointer
 * to the mirror.
 */
+   rpage = dpage->zone_device_data;
rpage->zone_device_data = dmirror;
-
+   pr_debug("migrating from sys to dev pfn src: 0x%lx pfn dst: 
0x%lx\n",
+page_to_pfn(spage), page_to_pfn(dpage));
*dst = migrate_pfn(page_to_pfn(dpage)) |
MIGRATE_PFN_LOCKED;
if ((*src & MIGRATE_PFN_WRITE) ||
@@ -671,10 +682,13 @@ static int dmirror_migrate_finalize_and_map(struct 
migrate_vma *args,
continue;
 
/*
-* Store the page that holds the data so the page table
-* doesn't have to deal with ZONE_DEVICE private pages.
+* For ZONE_DEVICE private pages we store the page that
+* holds the data so the page table doesn't have to deal it.
+* For ZONE_DEVICE generic pages we store the actual page, since
+* the CPU has coherent access to the page.
 */
-   entry = dpage->zone_device_data;
+   entry = is_device_private_page(dpage) ? dpage->zone_device_data 
:
+   dpage;
if (*dst & MIGRATE_PFN_WRITE)
  

[PATCH v6 13/13] tools: update test_hmm script to support SP config

[Alex Sierra]

Add two more parameters to set spm_addr_dev0 & spm_addr_dev1
addresses. These two parameters configure the start SP
addresses for each device in test_hmm driver.
Consequently, this configures zone device type as generic.

Signed-off-by: Alex Sierra 
---
 tools/testing/selftests/vm/test_hmm.sh | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vm/test_hmm.sh 
b/tools/testing/selftests/vm/test_hmm.sh
index 0647b525a625..3eeabe94399f 100755
--- a/tools/testing/selftests/vm/test_hmm.sh
+++ b/tools/testing/selftests/vm/test_hmm.sh
@@ -40,7 +40,18 @@ check_test_requirements()
 
 load_driver()
 {
-   modprobe $DRIVER > /dev/null 2>&1
+   if [ $# -eq 0 ]; then
+   modprobe $DRIVER > /dev/null 2>&1
+   else
+   if [ $# -eq 2 ]; then
+   modprobe $DRIVER spm_addr_dev0=$1 spm_addr_dev1=$2
+   > /dev/null 2>&1
+   else
+   echo "Missing module parameters. Make sure pass"\
+   "spm_addr_dev0 and spm_addr_dev1"
+   usage
+   fi
+   fi
if [ $? == 0 ]; then
major=$(awk "\$2==\"HMM_DMIRROR\" {print \$1}" /proc/devices)
mknod /dev/hmm_dmirror0 c $major 0
@@ -58,7 +69,7 @@ run_smoke()
 {
echo "Running smoke test. Note, this test provides basic coverage."
 
-   load_driver
+   load_driver $1 $2
$(dirname "${BASH_SOURCE[0]}")/hmm-tests
unload_driver
 }
@@ -75,6 +86,9 @@ usage()
echo "# Smoke testing"
echo "./${TEST_NAME}.sh smoke"
echo
+   echo "# Smoke testing with SPM enabled"
+   echo "./${TEST_NAME}.sh smoke  "
+   echo
exit 0
 }
 
@@ -84,7 +98,7 @@ function run_test()
usage
else
if [ "$1" = "smoke" ]; then
-   run_smoke
+   run_smoke $2 $3
else
usage
fi
-- 
2.32.0

[PATCH v6 09/13] lib: test_hmm add ioctl to get zone device type

[Alex Sierra]

new ioctl cmd added to query zone device type. This will be
used once the test_hmm adds zone device generic type.

Signed-off-by: Alex Sierra 
---
 lib/test_hmm.c  | 15 ++-
 lib/test_hmm_uapi.h |  7 +++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 6998f10350ea..3cd91ca31dd7 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -82,6 +82,7 @@ struct dmirror_chunk {
 struct dmirror_device {
struct cdev cdevice;
struct hmm_devmem   *devmem;
+   unsigned intzone_device_type;
 
unsigned intdevmem_capacity;
unsigned intdevmem_count;
@@ -468,6 +469,7 @@ static bool dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
if (IS_ERR(res))
goto err_devmem;
 
+   mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
devmem->pagemap.range.start = res->start;
devmem->pagemap.range.end = res->end;
@@ -912,6 +914,15 @@ static int dmirror_snapshot(struct dmirror *dmirror,
return ret;
 }
 
+static int dmirror_get_device_type(struct dmirror *dmirror,
+   struct hmm_dmirror_cmd *cmd)
+{
+   mutex_lock(>mutex);
+   cmd->zone_device_type = dmirror->mdevice->zone_device_type;
+   mutex_unlock(>mutex);
+
+   return 0;
+}
 static long dmirror_fops_unlocked_ioctl(struct file *filp,
unsigned int command,
unsigned long arg)
@@ -952,7 +963,9 @@ static long dmirror_fops_unlocked_ioctl(struct file *filp,
case HMM_DMIRROR_SNAPSHOT:
ret = dmirror_snapshot(dmirror, );
break;
-
+   case HMM_DMIRROR_GET_MEM_DEV_TYPE:
+   ret = dmirror_get_device_type(dmirror, );
+   break;
default:
return -EINVAL;
}
diff --git a/lib/test_hmm_uapi.h b/lib/test_hmm_uapi.h
index 670b4ef2a5b6..ee88701793d5 100644
--- a/lib/test_hmm_uapi.h
+++ b/lib/test_hmm_uapi.h
@@ -26,6 +26,7 @@ struct hmm_dmirror_cmd {
__u64   npages;
__u64   cpages;
__u64   faults;
+   __u64   zone_device_type;
 };
 
 /* Expose the address space of the calling process through hmm device file */
@@ -33,6 +34,7 @@ struct hmm_dmirror_cmd {
 #define HMM_DMIRROR_WRITE  _IOWR('H', 0x01, struct hmm_dmirror_cmd)
 #define HMM_DMIRROR_MIGRATE_IOWR('H', 0x02, struct hmm_dmirror_cmd)
 #define HMM_DMIRROR_SNAPSHOT   _IOWR('H', 0x03, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_GET_MEM_DEV_TYPE   _IOWR('H', 0x04, struct hmm_dmirror_cmd)
 
 /*
  * Values returned in hmm_dmirror_cmd.ptr for HMM_DMIRROR_SNAPSHOT.
@@ -60,4 +62,9 @@ enum {
HMM_DMIRROR_PROT_DEV_PRIVATE_REMOTE = 0x30,
 };
 
+enum {
+   /* 0 is reserved to catch uninitialized type fields */
+   HMM_DMIRROR_MEMORY_DEVICE_PRIVATE = 1,
+};
+
 #endif /* _LIB_TEST_HMM_UAPI_H */
-- 
2.32.0

[PATCH v6 08/13] mm: call pgmap->ops->page_free for DEVICE_GENERIC pages

[Alex Sierra]

Add MEMORY_DEVICE_GENERIC case to free_zone_device_page callback.
Device generic type memory case is now able to free its pages properly.

Signed-off-by: Alex Sierra 
---
 mm/memremap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/memremap.c b/mm/memremap.c
index 5aa8163fd948..5773e15b6ac9 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -459,7 +459,7 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
 EXPORT_SYMBOL_GPL(get_dev_pagemap);
 
 #ifdef CONFIG_DEV_PAGEMAP_OPS
-static void free_device_private_page(struct page *page)
+static void free_device_page(struct page *page)
 {
 
__ClearPageWaiters(page);
@@ -498,7 +498,8 @@ void free_zone_device_page(struct page *page)
wake_up_var(>_refcount);
return;
case MEMORY_DEVICE_PRIVATE:
-   free_device_private_page(page);
+   case MEMORY_DEVICE_GENERIC:
+   free_device_page(page);
return;
default:
return;
-- 
2.32.0

[PATCH v6 07/13] mm: add generic type support to migrate_vma helpers

[Alex Sierra]

Device generic type case added for migrate_vma_pages and
migrate_vma_check_page helpers.
Both, generic and private device types have the same
conditions to decide to migrate pages from/to device
memory.

Signed-off-by: Alex Sierra 
---
 mm/migrate.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index e3a10e2a1bb3..f9e6bfa2867c 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2565,7 +2565,7 @@ static bool migrate_vma_check_page(struct page *page)
 * FIXME proper solution is to rework migration_entry_wait() so
 * it does not need to take a reference on page.
 */
-   return is_device_private_page(page);
+   return is_device_page(page);
}
 
/* For file back page */
@@ -2854,7 +2854,7 @@ EXPORT_SYMBOL(migrate_vma_setup);
  * handle_pte_fault()
  *   do_anonymous_page()
  * to map in an anonymous zero page but the struct page will be a ZONE_DEVICE
- * private page.
+ * private or generic page.
  */
 static void migrate_vma_insert_page(struct migrate_vma *migrate,
unsigned long addr,
@@ -2925,10 +2925,14 @@ static void migrate_vma_insert_page(struct migrate_vma 
*migrate,
 
swp_entry = make_device_private_entry(page, 
vma->vm_flags & VM_WRITE);
entry = swp_entry_to_pte(swp_entry);
+   } else if (is_device_page(page)) {
+   entry = mk_pte(page, vma->vm_page_prot);
+   if (vma->vm_flags & VM_WRITE)
+   entry = pte_mkwrite(pte_mkdirty(entry));
} else {
/*
-* For now we only support migrating to un-addressable
-* device memory.
+* We support migrating to private and generic types 
for device
+* zone memory.
 */
pr_warn_once("Unsupported ZONE_DEVICE page type.\n");
goto abort;
@@ -3034,10 +3038,10 @@ void migrate_vma_pages(struct migrate_vma *migrate)
mapping = page_mapping(page);
 
if (is_zone_device_page(newpage)) {
-   if (is_device_private_page(newpage)) {
+   if (is_device_page(newpage)) {
/*
-* For now only support private anonymous when
-* migrating to un-addressable device memory.
+* For now only support private and generic
+* anonymous when migrating to device memory.
 */
if (mapping) {
migrate->src[i] &= ~MIGRATE_PFN_MIGRATE;
-- 
2.32.0

[PATCH v6 06/13] include/linux/mm.h: helpers to check zone device generic type

[Alex Sierra]

Two helpers added. One checks if zone device page is generic
type. The other if page is either private or generic type.

Signed-off-by: Alex Sierra 
---
 include/linux/mm.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index d48a1f0889d1..c25cdb92038f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1187,6 +1187,14 @@ static inline bool is_device_private_page(const struct 
page *page)
page->pgmap->type == MEMORY_DEVICE_PRIVATE;
 }
 
+static inline bool is_device_page(const struct page *page)
+{
+   return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
+   is_zone_device_page(page) &&
+   (page->pgmap->type == MEMORY_DEVICE_PRIVATE ||
+page->pgmap->type == MEMORY_DEVICE_GENERIC);
+}
+
 static inline bool is_pci_p2pdma_page(const struct page *page)
 {
return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
-- 
2.32.0

[PATCH v6 10/13] lib: test_hmm add module param for zone device type

[Alex Sierra]

In order to configure device generic in test_hmm, two
module parameters should be passed, which correspon to the
SP start address of each device (2) spm_addr_dev0 &
spm_addr_dev1. If no parameters are passed, private device
type is configured.

v5:
Remove devmem->pagemap.type = MEMORY_DEVICE_PRIVATE at
dmirror_allocate_chunk that was forcing to configure pagemap.type
to MEMORY_DEVICE_PRIVATE

v6:
Check for null pointers for resource and memremap references
at dmirror_allocate_chunk

Signed-off-by: Alex Sierra 
---
 lib/test_hmm.c  | 56 -
 lib/test_hmm_uapi.h |  1 +
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 3cd91ca31dd7..b4f885c6c6ae 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -33,6 +33,16 @@
 #define DEVMEM_CHUNK_SIZE  (256 * 1024 * 1024U)
 #define DEVMEM_CHUNKS_RESERVE  16
 
+static unsigned long spm_addr_dev0;
+module_param(spm_addr_dev0, long, 0644);
+MODULE_PARM_DESC(spm_addr_dev0,
+   "Specify start address for SPM (special purpose memory) used 
for device 0. By setting this Generic device type will be used. Make sure 
spm_addr_dev1 is set too");
+
+static unsigned long spm_addr_dev1;
+module_param(spm_addr_dev1, long, 0644);
+MODULE_PARM_DESC(spm_addr_dev1,
+   "Specify start address for SPM (special purpose memory) used 
for device 1. By setting this Generic device type will be used. Make sure 
spm_addr_dev0 is set too");
+
 static const struct dev_pagemap_ops dmirror_devmem_ops;
 static const struct mmu_interval_notifier_ops dmirror_min_ops;
 static dev_t dmirror_dev;
@@ -450,11 +460,11 @@ static int dmirror_write(struct dmirror *dmirror, struct 
hmm_dmirror_cmd *cmd)
return ret;
 }
 
-static bool dmirror_allocate_chunk(struct dmirror_device *mdevice,
+static int dmirror_allocate_chunk(struct dmirror_device *mdevice,
   struct page **ppage)
 {
struct dmirror_chunk *devmem;
-   struct resource *res;
+   struct resource *res = NULL;
unsigned long pfn;
unsigned long pfn_first;
unsigned long pfn_last;
@@ -462,15 +472,26 @@ static bool dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
 
devmem = kzalloc(sizeof(*devmem), GFP_KERNEL);
if (!devmem)
-   return false;
+   return -ENOMEM;
+
+   if (!spm_addr_dev0 && !spm_addr_dev1) {
+   res = request_free_mem_region(_resource, 
DEVMEM_CHUNK_SIZE,
+ "hmm_dmirror");
+   devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
+   mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
+   } else if (spm_addr_dev0 && spm_addr_dev1) {
+   res = lookup_resource(_resource, 
MINOR(mdevice->cdevice.dev) ?
+   spm_addr_dev0 :
+   spm_addr_dev1);
+   devmem->pagemap.type = MEMORY_DEVICE_GENERIC;
+   mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_GENERIC;
+   } else {
+   pr_err("Both spm_addr_dev parameters should be set\n");
+   }
 
-   res = request_free_mem_region(_resource, DEVMEM_CHUNK_SIZE,
- "hmm_dmirror");
-   if (IS_ERR(res))
+   if (IS_ERR_OR_NULL(res))
goto err_devmem;
 
-   mdevice->zone_device_type = HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
-   devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
devmem->pagemap.range.start = res->start;
devmem->pagemap.range.end = res->end;
devmem->pagemap.nr_range = 1;
@@ -493,10 +514,14 @@ static bool dmirror_allocate_chunk(struct dmirror_device 
*mdevice,
mdevice->devmem_capacity = new_capacity;
mdevice->devmem_chunks = new_chunks;
}
-
ptr = memremap_pages(>pagemap, numa_node_id());
-   if (IS_ERR(ptr))
+   if (IS_ERR_OR_NULL(ptr)) {
+   if (ptr)
+   ret = PTR_ERR(ptr);
+   else
+   ret = -EFAULT;
goto err_release;
+   }
 
devmem->mdevice = mdevice;
pfn_first = devmem->pagemap.range.start >> PAGE_SHIFT;
@@ -1097,10 +1122,8 @@ static int dmirror_device_init(struct dmirror_device 
*mdevice, int id)
if (ret)
return ret;
 
-   /* Build a list of free ZONE_DEVICE private struct pages */
-   dmirror_allocate_chunk(mdevice, NULL);
-
-   return 0;
+   /* Build a list of free ZONE_DEVICE struct pages */
+   return dmirror_allocate_chunk(mdevice, NULL);
 }
 
 static void dmirror_device_remove(struct dmirror_device *mdevice)
@@ -1113,8 +1136,9 @@ static void dmirror_device_remove(struct dmirror_device 
*mdevice)
mdevice->devmem_chunks[i];
 
memunmap_pages(>pagemap);

[PATCH v6 05/13] drm/amdkfd: generic type as sys mem on migration to ram

[Alex Sierra]

Generic device type memory on VRAM to RAM migration,
has similar access as System RAM from the CPU. This flag sets
the source from the sender. Which in Generic type case,
should be set as SYSTEM.

Signed-off-by: Alex Sierra 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 24a8b6d4f947..e5b10de83a5f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -616,9 +616,12 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct 
svm_range *prange,
migrate.vma = vma;
migrate.start = start;
migrate.end = end;
-   migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
 
+   if (adev->gmc.xgmi.connected_to_cpu)
+   migrate.flags = MIGRATE_VMA_SELECT_SYSTEM;
+   else
+   migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
size *= npages;
buf = kvmalloc(size, GFP_KERNEL | __GFP_ZERO);
-- 
2.32.0

[PATCH v6 02/13] mm: remove extra ZONE_DEVICE struct page refcount

[Alex Sierra]

From: Ralph Campbell 

ZONE_DEVICE struct pages have an extra reference count that complicates the
code for put_page() and several places in the kernel that need to check the
reference count to see that a page is not being used (gup, compaction,
migration, etc.). Clean up the code so the reference count doesn't need to
be treated specially for ZONE_DEVICE.

v2:
AS: merged this patch in linux 5.11 version

v5:
AS: add condition at try_grab_page to check for the zone device type, while
page ref counter is checked less/equal to zero. In case of device zone, pages
ref counter are initialized to zero.

Signed-off-by: Ralph Campbell 
Signed-off-by: Alex Sierra 
---
 arch/powerpc/kvm/book3s_hv_uvmem.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.c |  2 +-
 fs/dax.c   |  4 +-
 include/linux/dax.h|  2 +-
 include/linux/memremap.h   |  7 +--
 include/linux/mm.h | 13 +
 lib/test_hmm.c |  2 +-
 mm/internal.h  |  8 +++
 mm/memremap.c  | 68 +++---
 mm/migrate.c   |  5 --
 mm/page_alloc.c|  3 ++
 mm/swap.c  | 45 ++---
 12 files changed, 46 insertions(+), 115 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 84e5a2dc8be5..acee67710620 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -711,7 +711,7 @@ static struct page *kvmppc_uvmem_get_page(unsigned long 
gpa, struct kvm *kvm)
 
dpage = pfn_to_page(uvmem_pfn);
dpage->zone_device_data = pvt;
-   get_page(dpage);
+   init_page_count(dpage);
lock_page(dpage);
return dpage;
 out_clear:
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 92987daa5e17..8bc7120e1216 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -324,7 +324,7 @@ nouveau_dmem_page_alloc_locked(struct nouveau_drm *drm)
return NULL;
}
 
-   get_page(page);
+   init_page_count(page);
lock_page(page);
return page;
 }
diff --git a/fs/dax.c b/fs/dax.c
index c387d09e3e5a..1166630b7190 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -571,14 +571,14 @@ static void *grab_mapping_entry(struct xa_state *xas,
 
 /**
  * dax_layout_busy_page_range - find first pinned page in @mapping
- * @mapping: address space to scan for a page with ref count > 1
+ * @mapping: address space to scan for a page with ref count > 0
  * @start: Starting offset. Page containing 'start' is included.
  * @end: End offset. Page containing 'end' is included. If 'end' is LLONG_MAX,
  *   pages from 'start' till the end of file are included.
  *
  * DAX requires ZONE_DEVICE mapped pages. These pages are never
  * 'onlined' to the page allocator so they are considered idle when
- * page->count == 1. A filesystem uses this interface to determine if
+ * page->count == 0. A filesystem uses this interface to determine if
  * any page in the mapping is busy, i.e. for DMA, or other
  * get_user_pages() usages.
  *
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 8b5da1d60dbc..05fc982ce153 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -245,7 +245,7 @@ static inline bool dax_mapping(struct address_space 
*mapping)
 
 static inline bool dax_page_unused(struct page *page)
 {
-   return page_ref_count(page) == 1;
+   return page_ref_count(page) == 0;
 }
 
 #define dax_wait_page(_inode, _page, _wait_cb) \
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 45a79da89c5f..77ff5fd0685f 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -66,9 +66,10 @@ enum memory_type {
 
 struct dev_pagemap_ops {
/*
-* Called once the page refcount reaches 1.  (ZONE_DEVICE pages never
-* reach 0 refcount unless there is a refcount bug. This allows the
-* device driver to implement its own memory management.)
+* Called once the page refcount reaches 0. The reference count
+* should be reset to one with init_page_count(page) before reusing
+* the page. This allows the device driver to implement its own
+* memory management.
 */
void (*page_free)(struct page *page);
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8ae31622deef..d48a1f0889d1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1218,7 +1218,7 @@ __maybe_unused struct page *try_grab_compound_head(struct 
page *page, int refs,
 static inline __must_check bool try_get_page(struct page *page)
 {
page = compound_head(page);
-   if (WARN_ON_ONCE(page_ref_count(page) <= 0))
+   if (WARN_ON_ONCE(page_ref_count(page) < 
(int)!is_zone_device_page(page)))

[PATCH v6 03/13] kernel: resource: lookup_resource as exported symbol

[Alex Sierra]

The AMD architecture for the Frontier supercomputer will
have device memory which can be coherently accessed by
the CPU. The system BIOS advertises this memory as SPM
(special purpose memory) in the UEFI system address map.

The AMDGPU driver needs to be able to lookup this resource
in order to claim it as MEMORY_DEVICE_GENERIC using
devm_memremap_pages.

Signed-off-by: Alex Sierra 
---
 kernel/resource.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/resource.c b/kernel/resource.c
index ca9f5198a01f..227fc9fab573 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -772,6 +772,7 @@ struct resource *lookup_resource(struct resource *root, 
resource_size_t start)
 
return res;
 }
+EXPORT_SYMBOL_GPL(lookup_resource);
 
 /*
  * Insert a resource into the resource tree. If successful, return NULL,
-- 
2.32.0

[PATCH v6 04/13] drm/amdkfd: add SPM support for SVM

[Alex Sierra]

When CPU is connected throug XGMI, it has coherent
access to VRAM resource. In this case that resource
is taken from a table in the device gmc aperture base.
This resource is used along with the device type, which could
be DEVICE_PRIVATE or DEVICE_GENERIC to create the device
page map region.

Signed-off-by: Alex Sierra 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index dab290a4d19d..24a8b6d4f947 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -868,6 +868,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
struct resource *res;
unsigned long size;
void *r;
+   bool xgmi_connected_to_cpu = adev->gmc.xgmi.connected_to_cpu;
 
/* Page migration works on Vega10 or newer */
if (kfddev->device_info->asic_family < CHIP_VEGA10)
@@ -880,17 +881,22 @@ int svm_migrate_init(struct amdgpu_device *adev)
 * should remove reserved size
 */
size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
-   res = devm_request_free_mem_region(adev->dev, _resource, size);
+   if (xgmi_connected_to_cpu)
+   res = lookup_resource(_resource, adev->gmc.aper_base);
+   else
+   res = devm_request_free_mem_region(adev->dev, _resource, 
size);
+
if (IS_ERR(res))
return -ENOMEM;
 
-   pgmap->type = MEMORY_DEVICE_PRIVATE;
pgmap->nr_range = 1;
pgmap->range.start = res->start;
pgmap->range.end = res->end;
+   pgmap->type = xgmi_connected_to_cpu ?
+   MEMORY_DEVICE_GENERIC : MEMORY_DEVICE_PRIVATE;
pgmap->ops = _migrate_pgmap_ops;
pgmap->owner = SVM_ADEV_PGMAP_OWNER(adev);
-   pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
+   pgmap->flags = 0;
r = devm_memremap_pages(adev->dev, pgmap);
if (IS_ERR(r)) {
pr_err("failed to register HMM device memory\n");
@@ -914,6 +920,7 @@ void svm_migrate_fini(struct amdgpu_device *adev)
struct dev_pagemap *pgmap = >kfd.dev->pgmap;
 
devm_memunmap_pages(adev->dev, pgmap);
-   devm_release_mem_region(adev->dev, pgmap->range.start,
-   pgmap->range.end - pgmap->range.start + 1);
+   if (pgmap->type == MEMORY_DEVICE_PRIVATE)
+   devm_release_mem_region(adev->dev, pgmap->range.start,
+   pgmap->range.end - pgmap->range.start + 
1);
 }
-- 
2.32.0

[PATCH v6 01/13] ext4/xfs: add page refcount helper

[Alex Sierra]

From: Ralph Campbell 

There are several places where ZONE_DEVICE struct pages assume a reference
count == 1 means the page is idle and free. Instead of open coding this,
add a helper function to hide this detail.

v3:
[AS]: rename dax_layout_is_idle_page func to dax_page_unused

v4:
[AS]: This ref count functionality was missing on fuse/dax.c.

Signed-off-by: Ralph Campbell 
Signed-off-by: Alex Sierra 
---
 fs/dax.c|  4 ++--
 fs/ext4/inode.c |  5 +
 fs/fuse/dax.c   |  4 +---
 fs/xfs/xfs_file.c   |  4 +---
 include/linux/dax.h | 10 ++
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 62352cbcf0f4..c387d09e3e5a 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -369,7 +369,7 @@ static void dax_disassociate_entry(void *entry, struct 
address_space *mapping,
for_each_mapped_pfn(entry, pfn) {
struct page *page = pfn_to_page(pfn);
 
-   WARN_ON_ONCE(trunc && page_ref_count(page) > 1);
+   WARN_ON_ONCE(trunc && !dax_page_unused(page));
WARN_ON_ONCE(page->mapping && page->mapping != mapping);
page->mapping = NULL;
page->index = 0;
@@ -383,7 +383,7 @@ static struct page *dax_busy_page(void *entry)
for_each_mapped_pfn(entry, pfn) {
struct page *page = pfn_to_page(pfn);
 
-   if (page_ref_count(page) > 1)
+   if (!dax_page_unused(page))
return page;
}
return NULL;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index fe6045a46599..05ffe6875cb1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3971,10 +3971,7 @@ int ext4_break_layouts(struct inode *inode)
if (!page)
return 0;
 
-   error = ___wait_var_event(>_refcount,
-   atomic_read(>_refcount) == 1,
-   TASK_INTERRUPTIBLE, 0, 0,
-   ext4_wait_dax_page(ei));
+   error = dax_wait_page(ei, page, ext4_wait_dax_page);
} while (error == 0);
 
return error;
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index ff99ab2a3c43..2b1f190ba78a 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -677,9 +677,7 @@ static int __fuse_dax_break_layouts(struct inode *inode, 
bool *retry,
return 0;
 
*retry = true;
-   return ___wait_var_event(>_refcount,
-   atomic_read(>_refcount) == 1, TASK_INTERRUPTIBLE,
-   0, 0, fuse_wait_dax_page(inode));
+   return dax_wait_page(inode, page, fuse_wait_dax_page);
 }
 
 /* dmap_end == 0 leads to unmapping of whole file */
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 396ef36dcd0a..182057281086 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -840,9 +840,7 @@ xfs_break_dax_layouts(
return 0;
 
*retry = true;
-   return ___wait_var_event(>_refcount,
-   atomic_read(>_refcount) == 1, TASK_INTERRUPTIBLE,
-   0, 0, xfs_wait_dax_page(inode));
+   return dax_wait_page(inode, page, xfs_wait_dax_page);
 }
 
 int
diff --git a/include/linux/dax.h b/include/linux/dax.h
index b52f084aa643..8b5da1d60dbc 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -243,6 +243,16 @@ static inline bool dax_mapping(struct address_space 
*mapping)
return mapping->host && IS_DAX(mapping->host);
 }
 
+static inline bool dax_page_unused(struct page *page)
+{
+   return page_ref_count(page) == 1;
+}
+
+#define dax_wait_page(_inode, _page, _wait_cb) \
+   ___wait_var_event(&(_page)->_refcount,  \
+   dax_page_unused(_page), \
+   TASK_INTERRUPTIBLE, 0, 0, _wait_cb(_inode))
+
 #ifdef CONFIG_DEV_DAX_HMEM_DEVICES
 void hmem_register_device(int target_nid, struct resource *r);
 #else
-- 
2.32.0

[PATCH v6 00/13] Support DEVICE_GENERIC memory in migrate_vma_*

[Alex Sierra]

v1:
AMD is building a system architecture for the Frontier supercomputer with a
coherent interconnect between CPUs and GPUs. This hardware architecture allows
the CPUs to coherently access GPU device memory. We have hardware in our labs
and we are working with our partner HPE on the BIOS, firmware and software
for delivery to the DOE.

The system BIOS advertises the GPU device memory (aka VRAM) as SPM
(special purpose memory) in the UEFI system address map. The amdgpu driver looks
it up with lookup_resource and registers it with devmap as MEMORY_DEVICE_GENERIC
using devm_memremap_pages.

Now we're trying to migrate data to and from that memory using the migrate_vma_*
helpers so we can support page-based migration in our unified memory 
allocations,
while also supporting CPU access to those pages.

This patch series makes a few changes to make MEMORY_DEVICE_GENERIC pages behave
correctly in the migrate_vma_* helpers. We are looking for feedback about this
approach. If we're close, what's needed to make our patches acceptable upstream?
If we're not close, any suggestions how else to achieve what we are trying to do
(i.e. page migration and coherent CPU access to VRAM)?

This work is based on HMM and our SVM memory manager that was recently 
upstreamed
to Dave Airlie's drm-next branch
https://cgit.freedesktop.org/drm/drm/log/?h=drm-next
On top of that we did some rework of our VRAM management for migrations to 
remove
some incorrect assumptions, allow partially successful migrations and GPU memory
mappings that mix pages in VRAM and system memory.
https://lore.kernel.org/dri-devel/20210527205606.2660-6-felix.kuehl...@amd.com/T/#r996356015e295780eb50453e7dbd5d0d68b47cbc

v2:
This patch series version has merged "[RFC PATCH v3 0/2]
mm: remove extra ZONE_DEVICE struct page refcount" patch series made by
Ralph Campbell. It also applies at the top of these series, our changes
to support device generic type in migration_vma helpers.
This has been tested in systems with device memory that has coherent
access by CPU.

Also addresses the following feedback made in v1:
- Isolate in one patch kernel/resource.c modification, based
on Christoph's feedback.
- Add helpers check for generic and private type to avoid
duplicated long lines.

v3:
- Include cover letter from v1.
- Rename dax_layout_is_idle_page func to dax_page_unused in patch
ext4/xfs: add page refcount helper.

v4:
- Add support for zone device generic type in lib/test_hmm and
tool/testing/selftest/vm/hmm-tests.
- Add missing page refcount helper to fuse/dax.c. This was included in
one of Ralph Campbell's patches.

v5:
- Cosmetic changes on patches 3, 5 and 13
- A bug was found while running one of the xfstest (generic/413) used to
validate fs_dax device type. This was first introduced by patch: "mm: remove
extra ZONE_DEVICE struct page refcount" whic is part of these patch series.
The bug was showed as WARNING message at try_grab_page function call, due to
a page refcounter equal to zero. Part of "mm: remove extra ZONE_DEVICE struct
page refcount" changes, was to initialize page refcounter to zero. Therefore,
a special condition was added to try_grab_page on this v5, were it checks for
device zone pages too. It is included in the same patch.

This is how mm changes from these patch series have been validated:
- hmm-tests were run using device private and device generic types. This last,
just added in these patch series. efi_fake_mem was used to mimic SPM memory
for device generic.
- xfstests tool was used to validate fs-dax device type and page refcounter
changes. DAX configuration was used along with emulated Persisten Memory set as
memmap=4G!4G memmap=4G!9G. xfstests were run from ext4 and generic lists. Some
of them, did not run due to limitations in configuration. Ex. test not
supporting specific file system or DAX mode.
Only three tests failed, generic/356/357 and ext4/049. However, these failures
were consistent before and after applying these patch series.
xfstest configuration:
TEST_DEV=/dev/pmem0
TEST_DIR=/mnt/ram0
SCRATCH_DEV=/dev/pmem1
SCRATCH_MNT=/mnt/ram1
TEST_FS_MOUNT_OPTS="-o dax"
EXT_MOUNT_OPTIONS="-o dax"
MKFS_OPTIONS="-b4096"
xfstest passed list:
Ext4:
001,003,005,021,022,023,025,026,030,031,032,036,037,038,042,043,044,271,306
Generic:
1,2,3,4,5,6,7,8,9,11,12,13,14,15,16,20,21,22,23,24,25,28,29,30,31,32,33,35,37,
50,52,53,58,60,61,62,63,64,67,69,70,71,75,76,78,79,80,82,84,86,87,88,91,92,94,
96,97,98,99,103,105,112,113,114,117,120,124,126,129,130,131,135,141,169,184,
198,207,210,211,212,213,214,215,221,223,225,228,236,237,240,244,245,246,247,
248,249,255,257,258,263,277,286,294,306,307,308,309,313,315,316,318,319,337,
346,360,361,371,375,377,379,380,383,384,385,386,389,391,392,393,394,400,401,
403,404,406,409,410,411,412,413,417,420,422,423,424,425,426,427,428

v6:
- These patch series was rebased on amd-staging-drm-next, which in turn is
based on v5.13:
https://gitlab.freedesktop.org/agd5f/linux/-/tree/amd-staging-drm-next
- Handle null 

[PATCH 4/7] drm/amd/display: Ensure DCN save after VM setup

[Wayne Lin]

From: Jake Wang 

[Why]
DM initializes VM context after DMCUB initialization.
This results in loss of DCN_VM_CONTEXT registers after z10.

[How]
Notify DMCUB when VM setup is complete, and have DMCUB
save init registers.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Wayne Lin 
Signed-off-by: Jake Wang 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  6 ++
 drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c |  3 +++
 drivers/gpu/drm/amd/display/dc/dc.h|  1 +
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c | 12 
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h |  1 +
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c  |  1 +
 drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h  |  1 +
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h|  5 +
 8 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 7a442fcfa6ac..c798c65d4276 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1549,6 +1549,12 @@ void dc_z10_restore(struct dc *dc)
if (dc->hwss.z10_restore)
dc->hwss.z10_restore(dc);
 }
+
+void dc_z10_save_init(struct dc *dc)
+{
+   if (dc->hwss.z10_save_init)
+   dc->hwss.z10_save_init(dc);
+}
 #endif
 /*
  * Applies given context to HW and copy it into current context.
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
index f2b39ec35c89..80c752ca4a5a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_vm_helper.c
@@ -47,6 +47,9 @@ int dc_setup_system_context(struct dc *dc, struct 
dc_phy_addr_space_config *pa_c
 */
memcpy(>vm_pa_config, pa_config, sizeof(struct 
dc_phy_addr_space_config));
dc->vm_pa_config.valid = true;
+#if defined(CONFIG_DRM_AMD_DC_DCN3_1)
+   dc_z10_save_init(dc);
+#endif
}
 
return num_vmids;
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 03b81e5c5d67..0494e6dcf4dc 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1338,6 +1338,7 @@ void dc_hardware_release(struct dc *dc);
 bool dc_set_psr_allow_active(struct dc *dc, bool enable);
 #if defined(CONFIG_DRM_AMD_DC_DCN)
 void dc_z10_restore(struct dc *dc);
+void dc_z10_save_init(struct dc *dc);
 #endif
 
 bool dc_enable_dmub_notifications(struct dc *dc);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
index 6399e8acd093..3f2333ec67e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
@@ -407,6 +407,18 @@ void dcn31_update_info_frame(struct pipe_ctx *pipe_ctx)
_ctx->stream_res.encoder_info_frame);
}
 }
+void dcn31_z10_save_init(struct dc *dc)
+{
+   union dmub_rb_cmd cmd;
+
+   memset(, 0, sizeof(cmd));
+   cmd.dcn_restore.header.type = DMUB_CMD__IDLE_OPT;
+   cmd.dcn_restore.header.sub_type = DMUB_CMD__IDLE_OPT_DCN_SAVE_INIT;
+
+   dc_dmub_srv_cmd_queue(dc->ctx->dmub_srv, );
+   dc_dmub_srv_cmd_execute(dc->ctx->dmub_srv);
+   dc_dmub_srv_wait_idle(dc->ctx->dmub_srv);
+}
 
 void dcn31_z10_restore(struct dc *dc)
 {
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
index 40dfebe78fdd..140435e4f7ff 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.h
@@ -44,6 +44,7 @@ void dcn31_enable_power_gating_plane(
 void dcn31_update_info_frame(struct pipe_ctx *pipe_ctx);
 
 void dcn31_z10_restore(struct dc *dc);
+void dcn31_z10_save_init(struct dc *dc);
 
 void dcn31_hubp_pg_control(struct dce_hwseq *hws, unsigned int hubp_inst, bool 
power_on);
 int dcn31_init_sys_ctx(struct dce_hwseq *hws, struct dc *dc, struct 
dc_phy_addr_space_config *pa_config);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
index 05954045c332..40011cd3c8ef 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c
@@ -97,6 +97,7 @@ static const struct hw_sequencer_funcs dcn31_funcs = {
.set_abm_immediate_disable = dcn21_set_abm_immediate_disable,
.set_pipe = dcn21_set_pipe,
.z10_restore = dcn31_z10_restore,
+   .z10_save_init = dcn31_z10_save_init,
.is_abm_supported = dcn31_is_abm_supported,
.set_disp_pattern_generator = dcn30_set_disp_pattern_generator,
.update_visual_confirm_color = dcn20_update_visual_confirm_color,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
index 5ab008e62b82..ad5f2adcc40d 

[PATCH 3/7] drm/amd/display: Use vblank control events for PSR enable/disable

[Wayne Lin]

From: Nicholas Kazlauskas 

[Why]
PSR can disable the HUBP along with the OTG when PSR is active.

We'll hit a pageflip timeout when the OTG is disable because we're no
longer updating the CRTC vblank counter and the pflip high IRQ will
not fire on the flip.

In order to flip the page flip timeout occur we should modify the
enter/exit conditions to match DRM requirements.

[How]
Use our deferred handlers for DRM vblank control to notify DMCU(B)
when it can enable or disable PSR based on whether vblank is disabled or
enabled respectively.

We'll need to pass along the stream with the notification now because
we want to access the CRTC state while the CRTC is locked to get the
stream state prior to the commit.

Retain a reference to the stream so it remains safe to continue to
access and release that reference once we're done with it.

Enable/disable logic follows what we were previously doing in
update_planes.

The workqueue has to be flushed before programming streams or planes
to ensure that we exit out of idle optimizations and PSR before
these events occur if necessary.

To keep the skip count logic the same to avoid FBCON PSR enablement
requires copying the allow condition onto the DM IRQ parameters - a
field that we can actually access from the worker.

Reviewed-by: Roman Li 
Acked-by: Wayne Lin 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |  1 +
 3 files changed, 43 insertions(+), 8 deletions(-)

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 f88b6c5b83cd..cebd663b6708 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1061,7 +1061,22 @@ static void vblank_control_worker(struct work_struct 
*work)
 
DRM_DEBUG_KMS("Allow idle optimizations (MALL): %d\n", 
dm->active_vblank_irq_count == 0);
 
+   /* Control PSR based on vblank requirements from OS */
+   if (vblank_work->stream && vblank_work->stream->link) {
+   if (vblank_work->enable) {
+   if 
(vblank_work->stream->link->psr_settings.psr_allow_active)
+   amdgpu_dm_psr_disable(vblank_work->stream);
+   } else if 
(vblank_work->stream->link->psr_settings.psr_feature_enabled &&
+  
!vblank_work->stream->link->psr_settings.psr_allow_active &&
+  vblank_work->acrtc->dm_irq_params.allow_psr_entry) {
+   amdgpu_dm_psr_enable(vblank_work->stream);
+   }
+   }
+
mutex_unlock(>dc_lock);
+
+   dc_stream_release(vblank_work->stream);
+
kfree(vblank_work);
 }
 
@@ -6018,6 +6033,11 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
work->acrtc = acrtc;
work->enable = enable;
 
+   if (acrtc_state->stream) {
+   dc_stream_retain(acrtc_state->stream);
+   work->stream = acrtc_state->stream;
+   }
+
queue_work(dm->vblank_control_workqueue, >work);
 #endif
 
@@ -8623,6 +8643,12 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
/* Update the planes if changed or disable if we don't have any. */
if ((planes_count || acrtc_state->active_planes == 0) &&
acrtc_state->stream) {
+   /*
+* If PSR or idle optimizations are enabled then flush out
+* any pending work before hardware programming.
+*/
+   flush_workqueue(dm->vblank_control_workqueue);
+
bundle->stream_update.stream = acrtc_state->stream;
if (new_pcrtc_state->mode_changed) {
bundle->stream_update.src = acrtc_state->stream->src;
@@ -8691,16 +8717,20 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,

acrtc_state->stream->link->psr_settings.psr_version != 
DC_PSR_VERSION_UNSUPPORTED &&

!acrtc_state->stream->link->psr_settings.psr_feature_enabled)
amdgpu_dm_link_setup_psr(acrtc_state->stream);
-   else if ((acrtc_state->update_type == UPDATE_TYPE_FAST) &&
-   
acrtc_state->stream->link->psr_settings.psr_feature_enabled &&
-   
!acrtc_state->stream->link->psr_settings.psr_allow_active) {
-   struct amdgpu_dm_connector *aconn = (struct 
amdgpu_dm_connector *)
-   acrtc_state->stream->dm_stream_context;
+
+   /* Decrement skip count when PSR is enabled and we're doing 
fast updates. */
+   if (acrtc_state->update_type == UPDATE_TYPE_FAST &&
+   

[PATCH 2/7] drm/amd/display: Fix multi-display support for idle opt workqueue

[Wayne Lin]

From: Nicholas Kazlauskas 

[Why]
The current implementation for idle optimization support only has a
single work item that gets reshuffled into the system workqueue
whenever we receive an enable or disable event.

We can have mismatched events if the work hasn't been processed or if
we're getting control events from multiple displays at once.

This fixes this issue and also makes the implementation usable for
PSR control - which will be addressed in another patch.

[How]
We need to be able to flush remaining work out on demand for driver stop
and psr disable so create a driver specific workqueue instead of using
the system one. The workqueue will be single threaded to guarantee the
ordering of enable/disable events.

Refactor the queue to allocate the control work and deallocate it
after processing it.

Pass the acrtc directly to make it easier to handle psr enable/disable
in a later patch.

Rename things to indicate that it's not just MALL specific.

Reviewed-by: Roman Li 
Acked-by: Wayne Lin 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 62 ---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 21 ---
 2 files changed, 36 insertions(+), 47 deletions(-)

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 3e28f17c84fa..f88b6c5b83cd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1044,10 +1044,10 @@ static void mmhub_read_system_context(struct 
amdgpu_device *adev, struct dc_phy_
 }
 #endif
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-static void event_mall_stutter(struct work_struct *work)
+static void vblank_control_worker(struct work_struct *work)
 {
-
-   struct vblank_workqueue *vblank_work = container_of(work, struct 
vblank_workqueue, mall_work);
+   struct vblank_control_work *vblank_work =
+   container_of(work, struct vblank_control_work, work);
struct amdgpu_display_manager *dm = vblank_work->dm;
 
mutex_lock(>dc_lock);
@@ -1062,22 +1062,9 @@ static void event_mall_stutter(struct work_struct *work)
DRM_DEBUG_KMS("Allow idle optimizations (MALL): %d\n", 
dm->active_vblank_irq_count == 0);
 
mutex_unlock(>dc_lock);
+   kfree(vblank_work);
 }
 
-static struct vblank_workqueue *vblank_create_workqueue(struct amdgpu_device 
*adev, struct dc *dc)
-{
-   struct vblank_workqueue *vblank_work;
-
-   vblank_work = kzalloc(sizeof(*vblank_work), GFP_KERNEL);
-   if (ZERO_OR_NULL_PTR(vblank_work)) {
-   kfree(vblank_work);
-   return NULL;
-   }
-
-   INIT_WORK(_work->mall_work, event_mall_stutter);
-
-   return vblank_work;
-}
 #endif
 static int amdgpu_dm_init(struct amdgpu_device *adev)
 {
@@ -1220,12 +1207,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
 #if defined(CONFIG_DRM_AMD_DC_DCN)
if (adev->dm.dc->caps.max_links > 0) {
-   adev->dm.vblank_workqueue = vblank_create_workqueue(adev, 
adev->dm.dc);
-
-   if (!adev->dm.vblank_workqueue)
+   adev->dm.vblank_control_workqueue =
+   
create_singlethread_workqueue("dm_vblank_control_workqueue");
+   if (!adev->dm.vblank_control_workqueue)
DRM_ERROR("amdgpu: failed to initialize 
vblank_workqueue.\n");
-   else
-   DRM_DEBUG_DRIVER("amdgpu: vblank_workqueue init done 
%p.\n", adev->dm.vblank_workqueue);
}
 #endif
 
@@ -1298,6 +1283,13 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
 {
int i;
 
+#if defined(CONFIG_DRM_AMD_DC_DCN)
+   if (adev->dm.vblank_control_workqueue) {
+   destroy_workqueue(adev->dm.vblank_control_workqueue);
+   adev->dm.vblank_control_workqueue = NULL;
+   }
+#endif
+
for (i = 0; i < adev->dm.display_indexes_num; i++) {
drm_encoder_cleanup(>dm.mst_encoders[i].base);
}
@@ -1321,14 +1313,6 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
dc_deinit_callbacks(adev->dm.dc);
 #endif
 
-#if defined(CONFIG_DRM_AMD_DC_DCN)
-   if (adev->dm.vblank_workqueue) {
-   adev->dm.vblank_workqueue->dm = NULL;
-   kfree(adev->dm.vblank_workqueue);
-   adev->dm.vblank_workqueue = NULL;
-   }
-#endif
-
dc_dmub_srv_destroy(>dm.dc->ctx->dmub_srv);
 
if (dc_enable_dmub_notifications(adev->dm.dc)) {
@@ -6000,7 +5984,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
 #if defined(CONFIG_DRM_AMD_DC_DCN)
struct amdgpu_display_manager *dm = >dm;
-   unsigned long flags;
+   struct vblank_control_work *work;
 #endif
int rc = 0;
 
@@ -6025,12 +6009,16 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, 
bool enable)
  

[PATCH 1/7] drm/amd/display: Create dc_sink when EDID fail

[Wayne Lin]

[Why]
While reading remote EDID via Startech 1-to-4 hub, occasionally we
won't get response in time and won't light up corresponding monitor.

Ideally, we can still add generic modes for userspace to choose to try
to light up the monitor and which is done in
drm_helper_probe_single_connector_modes(). So the main problem here is
that we fail .mode_valid since we don't create remote dc_sink for this
case.

[How]
Also add default dc_sink if we can't get the EDID.

Reviewed-by: Nicholas Kazlauskas 
Acked-by: Wayne Lin 
Signed-off-by: Wayne Lin 
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 5568d4e518e6..1bcba6943fd7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -213,6 +213,29 @@ static int dm_dp_mst_get_modes(struct drm_connector 
*connector)
drm_connector_update_edid_property(
>base,
NULL);
+
+   DRM_DEBUG_KMS("Can't get EDID of %s. Add default remote 
sink.", connector->name);
+   if (!aconnector->dc_sink) {
+   struct dc_sink *dc_sink;
+   struct dc_sink_init_data init_params = {
+   .link = aconnector->dc_link,
+   .sink_signal = 
SIGNAL_TYPE_DISPLAY_PORT_MST };
+
+   dc_sink = dc_link_add_remote_sink(
+   aconnector->dc_link,
+   NULL,
+   0,
+   _params);
+
+   if (!dc_sink) {
+   DRM_ERROR("Unable to add a remote 
sink\n");
+   return 0;
+   }
+
+   dc_sink->priv = aconnector;
+   aconnector->dc_sink = dc_sink;
+   }
+
return ret;
}
 
-- 
2.25.1

[PATCH 0/7] DC Patches Aug 13, 2021

[Wayne Lin]

This DC patchset brings improvements in multiple areas. In summary, we 
highlight:

* Ensure DCN save init registers after VM setup
* Fix multi-display support for idle opt workqueue
* Use vblank control events for PSR enable/disable
* Create default dc_sink when fail reading EDID under MST

---

Anthony Koo (1):
  drm/amd/display: [FW Promotion] Release 0.0.79

Aric Cyr (1):
  drm/amd/display: 3.2.149

Jake Wang (1):
  drm/amd/display: Ensure DCN save after VM setup

Nicholas Kazlauskas (3):
  drm/amd/display: Fix multi-display support for idle opt workqueue
  drm/amd/display: Use vblank control events for PSR enable/disable
  drm/amd/display: Guard vblank wq flush with DCN guards

Wayne Lin (1):
  drm/amd/display: Create dc_sink when EDID fail

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 112 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  23 ++--
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |   1 +
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  23 
 drivers/gpu/drm/amd/display/dc/core/dc.c  |   6 +
 .../drm/amd/display/dc/core/dc_vm_helper.c|   3 +
 drivers/gpu/drm/amd/display/dc/dc.h   |   3 +-
 .../drm/amd/display/dc/dcn31/dcn31_hwseq.c|  12 ++
 .../drm/amd/display/dc/dcn31/dcn31_hwseq.h|   1 +
 .../gpu/drm/amd/display/dc/dcn31/dcn31_init.c |   1 +
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |   1 +
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   |  19 ++-
 12 files changed, 148 insertions(+), 57 deletions(-)

-- 
2.25.1