[PATCH] drm/amdgpu: Fix unused variable ‘idx’ in ‘amdgpu_atom_parse’

2023-07-18 Thread Srinivasan Shanmugam
drivers/gpu/drm/amd/amdgpu/atom.c: In function ‘amdgpu_atom_parse’:
drivers/gpu/drm/amd/amdgpu/atom.c:1468:6: warning: unused variable ‘idx’ 
[-Wunused-variable]
 1468 |  u16 idx;
  |  ^~~

Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/amdgpu/atom.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c 
b/drivers/gpu/drm/amd/amdgpu/atom.c
index edbb862c1025..9f63ddb89b75 100644
--- a/drivers/gpu/drm/amd/amdgpu/atom.c
+++ b/drivers/gpu/drm/amd/amdgpu/atom.c
@@ -1465,7 +1465,6 @@ struct atom_context *amdgpu_atom_parse(struct card_info 
*card, void *bios)
struct _ATOM_ROM_HEADER *atom_rom_header;
struct _ATOM_MASTER_DATA_TABLE *master_table;
struct _ATOM_FIRMWARE_INFO *atom_fw_info;
-   u16 idx;
 
if (!ctx)
return NULL;
-- 
2.25.1



[PATCH 1/2] drm/amd/display: Convert macros to functions in amdgpu_display.c & amdgpu_display.h

2023-07-18 Thread Srinivasan Shanmugam
Convert macros to functions to fix the following & for better readability:

drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:26: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:32: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:34: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:36: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:38: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:40: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:42: Macro argument reuse 'adev' - 
possible side-effects?
drivers/gpu/drm/amd/amdgpu/amdgpu_display.h:44: Macro argument reuse 'adev' - 
possible side-effects?

And other warnings:

WARNING: Block comments use * on subsequent lines
WARNING: Block comments use a trailing */ on a separate line
WARNING: suspect code indent for conditional statements (8, 12)
WARNING: braces {} are not necessary for single statement blocks

Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 118 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h |  46 ++--
 2 files changed, 136 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index b702f499f5fb..6eea92cef97c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -45,6 +45,82 @@
 #include 
 #include 
 
+u32 amdgpu_display_vblank_get_counter(struct amdgpu_device *adev, int crtc)
+{
+   return (adev)->mode_info.funcs->vblank_get_counter((adev), (crtc));
+}
+
+void amdgpu_display_backlight_set_level(struct amdgpu_device *adev,
+   struct amdgpu_encoder *amdgpu_encoder,
+   u8 level)
+{
+   (adev)->mode_info.funcs->backlight_set_level((amdgpu_encoder), (level));
+}
+
+u8 amdgpu_display_backlight_get_level(struct amdgpu_device *adev,
+ struct amdgpu_encoder *amdgpu_encoder)
+{
+   return (adev)->mode_info.funcs->backlight_get_level(amdgpu_encoder);
+}
+
+bool amdgpu_display_hpd_sense(struct amdgpu_device *adev,
+ enum amdgpu_hpd_id hpd)
+{
+   return (adev)->mode_info.funcs->hpd_sense((adev), (hpd));
+}
+
+void amdgpu_display_hpd_set_polarity(struct amdgpu_device *adev,
+enum amdgpu_hpd_id hpd)
+{
+   (adev)->mode_info.funcs->hpd_set_polarity((adev), (hpd));
+}
+
+u32 amdgpu_display_hpd_get_gpio_reg(struct amdgpu_device *adev)
+{
+   return (adev)->mode_info.funcs->hpd_get_gpio_reg(adev);
+}
+
+void amdgpu_display_bandwidth_update(struct amdgpu_device *adev)
+{
+   (adev)->mode_info.funcs->bandwidth_update(adev);
+}
+
+void amdgpu_display_page_flip(struct amdgpu_device *adev,
+ int crtc_id, u64 crtc_base,
+ bool async)
+{
+   (adev)->mode_info.funcs->page_flip((adev), (crtc_id), (crtc_base), 
(async));
+}
+
+int amdgpu_display_page_flip_get_scanoutpos(struct amdgpu_device *adev, int 
crtc,
+   u32 *vbl, u32 *pos)
+{
+   return (adev)->mode_info.funcs->page_flip_get_scanoutpos((adev), 
(crtc), (vbl), (pos));
+}
+
+void amdgpu_display_add_encoder(struct amdgpu_device *adev,
+   u32 encoder_enum,
+   u32 supported_device,
+   u16 caps)
+{
+   (adev)->mode_info.funcs->add_encoder((adev), (encoder_enum), 
(supported_device), (caps));
+}
+
+void amdgpu_display_add_connector(struct amdgpu_device *adev,
+ u32 connector_id,
+ u32 supported_device,
+ int connector_type,
+ struct amdgpu_i2c_bus_rec *i2c_bus,
+ u16 connector_object_id,
+ struct amdgpu_hpd *hpd,
+ struct amdgpu_router *router)
+{
+   (adev)->mode_info.funcs->add_connector((adev), (connector_id),
+  (supported_device), 
(connector_type),
+  (i2c_bus), (connector_object_id),
+  (hpd), (router));
+}
+
 /**
  * amdgpu_display_hotplug_work_func - work handler for display hotplug event
  *
@@ -124,7 +200,7 @@ static void amdgpu_display_flip_work_func(struct 
work_struct *__work)
 
struct drm_crtc *crtc = _crtc->base;
unsigned long flags;
-   unsigned i;
+   unsigned int i;
int vpos, hpos;
 
for (i = 0; i < 

[PATCH 2/2] drm/radeon: Prefer dev_warn over printk

2023-07-18 Thread Srinivasan Shanmugam
From: Srinivasan Shanmugam 

Fixes the following checkpatch.pl:

WARNING: printk() should include KERN_ facility level

Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/radeon/radeon_atpx_handler.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c 
b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index d0b450a06506..875a995fff66 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -115,7 +115,7 @@ static union acpi_object *radeon_atpx_call(acpi_handle 
handle, int function,
 
/* Fail only if calling the method fails and ATPX is supported */
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
-   printk("failed to evaluate ATPX got %s\n",
+   pr_err("failed to evaluate ATPX got %s\n",
   acpi_format_exception(status));
kfree(buffer.pointer);
return NULL;
@@ -171,7 +171,7 @@ static int radeon_atpx_validate(struct radeon_atpx *atpx)
 
size = *(u16 *) info->buffer.pointer;
if (size < 10) {
-   printk("ATPX buffer is too small: %zu\n", size);
+   pr_err("ATPX buffer is too small: %zu\n", size);
kfree(info);
return -EINVAL;
}
@@ -202,7 +202,7 @@ static int radeon_atpx_validate(struct radeon_atpx *atpx)
 
atpx->is_hybrid = false;
if (valid_bits & ATPX_MS_HYBRID_GFX_SUPPORTED) {
-   printk("ATPX Hybrid Graphics\n");
+   pr_info("ATPX Hybrid Graphics\n");
/*
 * Disable legacy PM methods only when pcie port PM is usable,
 * otherwise the device might fail to power off or power on.
@@ -239,7 +239,7 @@ static int radeon_atpx_verify_interface(struct radeon_atpx 
*atpx)
 
size = *(u16 *) info->buffer.pointer;
if (size < 8) {
-   printk("ATPX buffer is too small: %zu\n", size);
+   pr_err("ATPX buffer is too small: %zu\n", size);
err = -EINVAL;
goto out;
}
@@ -248,7 +248,7 @@ static int radeon_atpx_verify_interface(struct radeon_atpx 
*atpx)
memcpy(, info->buffer.pointer, size);
 
/* TODO: check version? */
-   printk("ATPX version %u, functions 0x%08x\n",
+   pr_info("ATPX version %u, functions 0x%08x\n",
   output.version, output.function_bits);
 
radeon_atpx_parse_functions(>functions, output.function_bits);
-- 
2.25.1



[PATCH 0/2] Some more Cleanup fixes in radeon & amdgpu/amdgpu_display.c

2023-07-18 Thread Srinivasan Shanmugam
Srinivasan Shanmugam (2):
  drm/amd/display: Convert macros to functions in amdgpu_display.c &
amdgpu_display.h
  drm/radeon: Prefer dev_warn over printk

 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c  | 118 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h  |  46 ++--
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |  10 +-
 3 files changed, 141 insertions(+), 33 deletions(-)

-- 
2.25.1



RE: [PATCH] drm/amdgpu: update sched.ready within kernel drm scheduler

2023-07-18 Thread Chen, Guchun
[Public]

Please ignore this patch, will send another one after more rework is done.

Regards,
Guchun

> -Original Message-
> From: Chen, Guchun 
> Sent: Wednesday, July 19, 2023 12:52 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Zhang, Hawking
> ; Koenig, Christian
> ; Lazar, Lijo 
> Cc: Chen, Guchun 
> Subject: [PATCH] drm/amdgpu: update sched.ready within kernel drm
> scheduler
>
> amdgpu_test_ring_helper will set sched.ready unconditionally based on ring
> test result, this will lead value mismatch between
> ring->sched.ready and no_scheduler for those rings without a kernel
> scheluer, after they perform ring test. This will be confused as kernel ring
> no_scheduler is true, but ring->sched.ready is true as well.
>
> Signed-off-by: Guchun Chen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 80d6e132e409..afa76d069d94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -630,7 +630,9 @@ int amdgpu_ring_test_helper(struct amdgpu_ring
> *ring)
>   DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
> ring->name);
>
> - ring->sched.ready = !r;
> + /* Only set sched.ready on top of kernel scheduler. */
> + if (!ring->no_scheduler)
> + ring->sched.ready = !r;
>   return r;
>  }
>
> --
> 2.25.1



RE: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Chen, Guchun
[Public]

> -Original Message-
> From: Lazar, Lijo 
> Sent: Wednesday, July 19, 2023 12:53 PM
> To: Chen, Guchun ; Zhu, James
> ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Kamal, Asad
> ; Zhang, Hawking 
> Subject: Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed
>
>
>
> On 7/19/2023 10:12 AM, Chen, Guchun wrote:
> > [Public]
> >
> >> -Original Message-
> >> From: amd-gfx  On Behalf Of
> >> Lazar, Lijo
> >> Sent: Wednesday, July 19, 2023 12:13 AM
> >> To: Zhu, James ; amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander ; Zhu, James
> >> ; Kamal, Asad ; Zhang,
> Hawking
> >> 
> >> Subject: Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed
> >>
> >>
> >>
> >> On 7/18/2023 9:39 PM, James Zhu wrote:
> >>>
> >>> On 2023-07-18 11:54, Lazar, Lijo wrote:
> 
> 
>  On 7/18/2023 8:39 PM, James Zhu wrote:
> >
> > On 2023-07-18 10:19, Lazar, Lijo wrote:
> >>
> >>
> >> On 7/18/2023 7:30 PM, James Zhu wrote:
> >>>
> >>> On 2023-07-18 08:21, Lijo Lazar wrote:
>  Not all rings have scheduler associated. Only update scheduler
>  data for rings with scheduler. It could result in out of bound
>  access as total rings are more than those associated with
>  particular IPs.
> 
>  Signed-off-by: Lijo Lazar 
>  ---
> drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
>  diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>  b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>  index 72b629a78c62..d0fc62784e82 100644
>  --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>  +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>  @@ -134,7 +134,7 @@ static int
>  aqua_vanjaram_xcp_sched_list_update(
> for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> ring = adev->rings[i];
>  -if (!ring || !ring->sched.ready)
>  +if (!ring || !ring->sched.ready || ring->no_scheduler)
> >>> [JZ] any case for ring->no_scheduler = true, but
> >>> ring->sched.ready = true?
> >>
> >> amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings.
> >> amdgpu_device_init_schedulers inits schedulers only for rings
> >> which doesn't have no_scheduler. However in
> >> gfx_v9_4_3_xcc_kiq_resume, the ring is marked with
> >> ring->sched.ready = true;
> >>
> >> I'm not sure why it's done this way, but this is the case even
> >> for gfxv9.
> >
> > Driver so far still has some concept abuse on sched.ready. In
> amdgpu_device_init_schedulers , it's set to be true once setting up sw
> scheduler for the ring requirement, however, after driver is up, this flag
> works like a hint to tell the corresponding ring is ready for HW submission
> after ring test succeeds.
> >
> > For this case, it's probably caused by amdgpu_gfx_enable_kcq  calling
> amdgpu_ring_test_helper, which sets sched.ready unconditionally based on
> ring test result. This will lead value mismatch between ring->no_scheduler
> and ring->sched.ready. If my understanding is correct, I think adding a check
> in this helper function which only sets sched.ready when no_scheduler is
> false will help.  So I will provide a patch soon to prevent this mismatch in
> future.
> >
> > +if (!ring->no_scheduler)
> > +   ring->sched.ready != r;
>
> The ring.ready(ring->sched.ready) flag is used in gmcv9 code as well.This will
> need more rework.

Exactly, let me double check.

Regards,
Guchun

> Thanks,
> Lijo
> >
> > Regards,
> > Guchun
> >
> > [JZ] I think the sched.ready confuses people. here just means ring
> > is ready be used.
> 
>  Thanks for the clarification. One question is then do we need to
>  check ring ready status for assigning xcp ids or just check if the
>  ring is associated with a scheduler?
> 
>  Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids
>  and leave the ring ready status to the logic which really accepts
>  jobs on the ring?
> >>> [JZ] I think keeping ring->sched.ready will reduce schedule list
> >>> which will save a little scheduling time.
> >>
> >> Fine, will just push this one.
> >>
> >> Thanks,
> >> Lijo
> >>
> 
>  Thanks,
>  Lijo
> 
> >> This patch is Reviewed-by: James Zhu 
> >
> >> Thanks,
> >> Lijo
> >>
> continue;
> aqua_vanjaram_xcp_gpu_sched_update(adev, ring,
>  ring->xcp_id);


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Lazar, Lijo




On 7/19/2023 10:12 AM, Chen, Guchun wrote:

[Public]


-Original Message-
From: amd-gfx  On Behalf Of Lazar,
Lijo
Sent: Wednesday, July 19, 2023 12:13 AM
To: Zhu, James ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Zhu, James
; Kamal, Asad ; Zhang,
Hawking 
Subject: Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed



On 7/18/2023 9:39 PM, James Zhu wrote:


On 2023-07-18 11:54, Lazar, Lijo wrote:



On 7/18/2023 8:39 PM, James Zhu wrote:


On 2023-07-18 10:19, Lazar, Lijo wrote:



On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:

Not all rings have scheduler associated. Only update scheduler
data for rings with scheduler. It could result in out of bound
access as total rings are more than those associated with
particular IPs.

Signed-off-by: Lijo Lazar 
---
   drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int
aqua_vanjaram_xcp_sched_list_update(
   for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
   ring = adev->rings[i];
-if (!ring || !ring->sched.ready)
+if (!ring || !ring->sched.ready || ring->no_scheduler)

[JZ] any case for ring->no_scheduler = true, but ring->sched.ready
= true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings.
amdgpu_device_init_schedulers inits schedulers only for rings which
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume,
the ring is marked with ring->sched.ready = true;

I'm not sure why it's done this way, but this is the case even for
gfxv9.


Driver so far still has some concept abuse on sched.ready. In 
amdgpu_device_init_schedulers , it's set to be true once setting up sw 
scheduler for the ring requirement, however, after driver is up, this flag 
works like a hint to tell the corresponding ring is ready for HW submission 
after ring test succeeds.

For this case, it's probably caused by amdgpu_gfx_enable_kcq  calling 
amdgpu_ring_test_helper, which sets sched.ready unconditionally based on ring test 
result. This will lead value mismatch between ring->no_scheduler and 
ring->sched.ready. If my understanding is correct, I think adding a check in this 
helper function which only sets sched.ready when no_scheduler is false will help.  So 
I will provide a patch soon to prevent this mismatch in future.

+if (!ring->no_scheduler)
+   ring->sched.ready != r;


The ring.ready(ring->sched.ready) flag is used in gmcv9 code as 
well.This will need more rework.


Thanks,
Lijo


Regards,
Guchun


[JZ] I think the sched.ready confuses people. here just means ring
is ready be used.


Thanks for the clarification. One question is then do we need to
check ring ready status for assigning xcp ids or just check if the
ring is associated with a scheduler?

Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids
and leave the ring ready status to the logic which really accepts
jobs on the ring?

[JZ] I think keeping ring->sched.ready will reduce schedule list which
will save a little scheduling time.


Fine, will just push this one.

Thanks,
Lijo



Thanks,
Lijo


This patch is Reviewed-by: James Zhu 



Thanks,
Lijo


   continue;
   aqua_vanjaram_xcp_gpu_sched_update(adev, ring,
ring->xcp_id);


[PATCH] drm/amdgpu: update sched.ready within kernel drm scheduler

2023-07-18 Thread Guchun Chen
amdgpu_test_ring_helper will set sched.ready unconditionally
based on ring test result, this will lead value mismatch between
ring->sched.ready and no_scheduler for those rings without a kernel
scheluer, after they perform ring test. This will be confused as
kernel ring no_scheduler is true, but ring->sched.ready is true as well.

Signed-off-by: Guchun Chen 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 80d6e132e409..afa76d069d94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -630,7 +630,9 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
  ring->name);
 
-   ring->sched.ready = !r;
+   /* Only set sched.ready on top of kernel scheduler. */
+   if (!ring->no_scheduler)
+   ring->sched.ready = !r;
return r;
 }
 
-- 
2.25.1



RE: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Chen, Guchun
[Public]

> -Original Message-
> From: amd-gfx  On Behalf Of Lazar,
> Lijo
> Sent: Wednesday, July 19, 2023 12:13 AM
> To: Zhu, James ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Zhu, James
> ; Kamal, Asad ; Zhang,
> Hawking 
> Subject: Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed
>
>
>
> On 7/18/2023 9:39 PM, James Zhu wrote:
> >
> > On 2023-07-18 11:54, Lazar, Lijo wrote:
> >>
> >>
> >> On 7/18/2023 8:39 PM, James Zhu wrote:
> >>>
> >>> On 2023-07-18 10:19, Lazar, Lijo wrote:
> 
> 
>  On 7/18/2023 7:30 PM, James Zhu wrote:
> >
> > On 2023-07-18 08:21, Lijo Lazar wrote:
> >> Not all rings have scheduler associated. Only update scheduler
> >> data for rings with scheduler. It could result in out of bound
> >> access as total rings are more than those associated with
> >> particular IPs.
> >>
> >> Signed-off-by: Lijo Lazar 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> >> b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> >> index 72b629a78c62..d0fc62784e82 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
> >> @@ -134,7 +134,7 @@ static int
> >> aqua_vanjaram_xcp_sched_list_update(
> >>   for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> >>   ring = adev->rings[i];
> >> -if (!ring || !ring->sched.ready)
> >> +if (!ring || !ring->sched.ready || ring->no_scheduler)
> > [JZ] any case for ring->no_scheduler = true, but ring->sched.ready
> > = true?
> 
>  amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings.
>  amdgpu_device_init_schedulers inits schedulers only for rings which
>  doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume,
>  the ring is marked with ring->sched.ready = true;
> 
>  I'm not sure why it's done this way, but this is the case even for
>  gfxv9.

Driver so far still has some concept abuse on sched.ready. In 
amdgpu_device_init_schedulers , it's set to be true once setting up sw 
scheduler for the ring requirement, however, after driver is up, this flag 
works like a hint to tell the corresponding ring is ready for HW submission 
after ring test succeeds.

For this case, it's probably caused by amdgpu_gfx_enable_kcq  calling 
amdgpu_ring_test_helper, which sets sched.ready unconditionally based on ring 
test result. This will lead value mismatch between ring->no_scheduler and 
ring->sched.ready. If my understanding is correct, I think adding a check in 
this helper function which only sets sched.ready when no_scheduler is false 
will help.  So I will provide a patch soon to prevent this mismatch in future.

+if (!ring->no_scheduler)
+   ring->sched.ready != r;

Regards,
Guchun

> >>> [JZ] I think the sched.ready confuses people. here just means ring
> >>> is ready be used.
> >>
> >> Thanks for the clarification. One question is then do we need to
> >> check ring ready status for assigning xcp ids or just check if the
> >> ring is associated with a scheduler?
> >>
> >> Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids
> >> and leave the ring ready status to the logic which really accepts
> >> jobs on the ring?
> > [JZ] I think keeping ring->sched.ready will reduce schedule list which
> > will save a little scheduling time.
>
> Fine, will just push this one.
>
> Thanks,
> Lijo
>
> >>
> >> Thanks,
> >> Lijo
> >>
>  This patch is Reviewed-by: James Zhu 
> >>>
>  Thanks,
>  Lijo
> 
> >>   continue;
> >>   aqua_vanjaram_xcp_gpu_sched_update(adev, ring,
> >> ring->xcp_id);


RE: [PATCH V6 1/9] drivers core: Add support for Wifi band RF mitigations

2023-07-18 Thread Quan, Evan
[AMD Official Use Only - General]

> -Original Message-
> From: Andrew Lunn 
> Sent: Tuesday, July 18, 2023 10:16 PM
> To: Quan, Evan 
> Cc: raf...@kernel.org; l...@kernel.org; Deucher, Alexander
> ; Koenig, Christian
> ; Pan, Xinhui ;
> airl...@gmail.com; dan...@ffwll.ch; johan...@sipsolutions.net;
> da...@davemloft.net; eduma...@google.com; k...@kernel.org;
> pab...@redhat.com; Limonciello, Mario ;
> mdaen...@redhat.com; maarten.lankho...@linux.intel.com;
> tzimmerm...@suse.de; hdego...@redhat.com; jingyuwang_...@163.com;
> Lazar, Lijo ; jim.cro...@gmail.com;
> bellosili...@gmail.com; andrealm...@igalia.com; t...@redhat.com;
> j...@jsg.id.au; a...@arndb.de; linux-ker...@vger.kernel.org; linux-
> a...@vger.kernel.org; amd-gfx@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; linux-wirel...@vger.kernel.org;
> net...@vger.kernel.org
> Subject: Re: [PATCH V6 1/9] drivers core: Add support for Wifi band RF
> mitigations
>
> > The wbrf_supported_producer and wbrf_supported_consumer APIs seem
> > unnecessary for the generic implementation.
>
> I'm happy with these, once the description is corrected. As i said in another
> comment, 'can' should be replaced with 'should'. The device itself knows if it
> can, only the core knows if it should, based on the policy of if actions need 
> to
> be taken, and there are both providers and consumers registered with the
> core.
Sure, will update that in V7.

Evan
>
>Andrew


RE: [PATCH 1/2] drm/amdkfd: fix trap handling work around for debugging

2023-07-18 Thread Ji, Ruili
[Public]

Hi  Jonathan,

Your change looks fine.
Acknowledged-by: Ji, Ruili 

Thanks,
Ruili
-Original Message-
From: Kim, Jonathan 
Sent: Wednesday, July 19, 2023 6:13 AM
To: amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix ; Ji, Ruili 
Subject: RE: [PATCH 1/2] drm/amdkfd: fix trap handling work around for debugging

[Public]

+ Ruiji Li as this is a follow up to

commit 52223c7e74d124bea47beec467e59fdfc77559fc
Author: Ruili Ji 
Date:   Tue Jun 6 14:06:01 2023 +0800

drm/amdkfd: To enable traps for GC_11_0_4 and up

Flag trap_en should be enabled for trap handler.

Signed-off-by: Ruili Ji 
Signed-off-by: Aaron Liu 
Reviewed-by: Alex Deucher 

To ensure debugger is consistent with other checks.

Thanks,

Jon

> -Original Message-
> From: Kim, Jonathan 
> Sent: Friday, July 14, 2023 5:38 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix ; Kim, Jonathan
> 
> Subject: [PATCH 1/2] drm/amdkfd: fix trap handling work around for
> debugging
>
> Update the list of devices that require the cwsr trap handling
> workaround for debugging use cases.
>
> Signed-off-by: Jonathan Kim 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.c| 5 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.h| 6 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 190b03efe5ff..ccfc81f085ce 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -302,8 +302,7 @@ static int kfd_dbg_set_queue_workaround(struct
> queue *q, bool enable)
>   if (!q)
>   return 0;
>
> - if (KFD_GC_VERSION(q->device) < IP_VERSION(11, 0, 0) ||
> - KFD_GC_VERSION(q->device) >= IP_VERSION(12, 0, 0))
> + if (!kfd_dbg_has_cwsr_workaround(q->device))
>   return 0;
>
>   if (enable && q->properties.is_user_cu_masked) @@ -349,7 +348,7
> @@ int kfd_dbg_set_mes_debug_mode(struct kfd_process_device *pdd)  {
>   uint32_t spi_dbg_cntl = pdd->spi_dbg_override | pdd-
> >spi_dbg_launch_mode;
>   uint32_t flags = pdd->process->dbg_flags;
> - bool sq_trap_en = !!spi_dbg_cntl;
> + bool sq_trap_en = !!spi_dbg_cntl ||
> !kfd_dbg_has_cwsr_workaround(pdd->dev);
>
>   if (!kfd_dbg_is_per_vmid_supported(pdd->dev))
>   return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index ba616ed17dee..586d7f886712 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -101,6 +101,12 @@ static inline bool
> kfd_dbg_is_rlc_restore_supported(struct kfd_node *dev)
>KFD_GC_VERSION(dev) == IP_VERSION(10, 1, 1));  }
>
> +static inline bool kfd_dbg_has_cwsr_workaround(struct kfd_node *dev)
> +{
> + return KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0) &&
> +KFD_GC_VERSION(dev) <= IP_VERSION(11, 0, 3); }
> +
>  static inline bool kfd_dbg_has_gws_support(struct kfd_node *dev)  {
>   if ((KFD_GC_VERSION(dev) == IP_VERSION(9, 0, 1) diff --git
> a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 31cac1fd0d58..761963ad6154 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -226,8 +226,7 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
>   queue_input.paging = false;
>   queue_input.tba_addr = qpd->tba_addr;
>   queue_input.tma_addr = qpd->tma_addr;
> - queue_input.trap_en = KFD_GC_VERSION(q->device) <
> IP_VERSION(11, 0, 0) ||
> -   KFD_GC_VERSION(q->device) > IP_VERSION(11, 0,
> 3);
> + queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
>   queue_input.skip_process_ctx_clear = qpd->pqm->process-
> >debug_trap_enabled;
>
>   queue_type = convert_to_mes_queue_type(q->properties.type);
> @@ -1827,8 +1826,7 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
>*/
>   q->properties.is_evicted = !!qpd->evicted;
>   q->properties.is_dbg_wa = qpd->pqm->process-
> >debug_trap_enabled &&
> - KFD_GC_VERSION(q->device) >= IP_VERSION(11, 0, 0)
> &&
> - KFD_GC_VERSION(q->device) <= IP_VERSION(11, 0, 3);
> +
> + kfd_dbg_has_cwsr_workaround(q->device);
>
>   if (qd)
>   mqd_mgr->restore_mqd(mqd_mgr, >mqd, q-
> >mqd_mem_obj, >gart_mqd_addr,
> --
> 2.25.1




RE: [bug/bisected] commit a2848d08742c8e8494675892c02c0d22acbe3cf8 cause general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI

2023-07-18 Thread Chen, Guchun
[Public]

Thank for your test, Mike. After getting RB for this patch, I will ask 
Christian to apply it to corresponding branches like drm-misc-next.

Regards,
Guchun

> -Original Message-
> From: Mikhail Gavrilov 
> Sent: Tuesday, July 18, 2023 5:17 PM
> To: Chen, Guchun 
> Cc: Koenig, Christian ; Pelloux-Prayer, Pierre-
> Eric ; Deucher, Alexander
> ; amd-gfx list  g...@lists.freedesktop.org>; Linux List Kernel Mailing  ker...@vger.kernel.org>
> Subject: Re: [bug/bisected] commit
> a2848d08742c8e8494675892c02c0d22acbe3cf8 cause general protection
> fault, probably for non-canonical address 0xdc00:  [#1]
> PREEMPT SMP KASAN NOPTI
>
> On Tue, Jul 18, 2023 at 7:13 AM Chen, Guchun 
> wrote:
> >
> > [Public]
> >
> > Hello Mike,
> >
> > I guess this patch can resolve your problem.
> > https://patchwork.freedesktop.org/patch/547897/
> >
> > Regards,
> > Guchun
> >
>
> Tested-by: Mikhail Gavrilov  Thanks, the
> issue was gone with this patch.
>
> I didn't say anything above about how to reproduce this problem.
> Case was like this:
> On a dual GPU laptop, I ran Google Chrome on a discrete graphics card.
> I used for it this command:
> $ DRI_PRIME=1 google-chrome-unstable --disable-features=Vulkan
>
> --
> Best Regards,
> Mike Gavrilov.


RE: [PATCH 1/2] drm/amdkfd: fix trap handling work around for debugging

2023-07-18 Thread Kim, Jonathan
[Public]

+ Ruiji Li as this is a follow up to

commit 52223c7e74d124bea47beec467e59fdfc77559fc
Author: Ruili Ji 
Date:   Tue Jun 6 14:06:01 2023 +0800

drm/amdkfd: To enable traps for GC_11_0_4 and up

Flag trap_en should be enabled for trap handler.

Signed-off-by: Ruili Ji 
Signed-off-by: Aaron Liu 
Reviewed-by: Alex Deucher 

To ensure debugger is consistent with other checks.

Thanks,

Jon

> -Original Message-
> From: Kim, Jonathan 
> Sent: Friday, July 14, 2023 5:38 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix ; Kim, Jonathan
> 
> Subject: [PATCH 1/2] drm/amdkfd: fix trap handling work around for
> debugging
>
> Update the list of devices that require the cwsr trap handling
> workaround for debugging use cases.
>
> Signed-off-by: Jonathan Kim 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.c| 5 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_debug.h| 6 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 190b03efe5ff..ccfc81f085ce 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -302,8 +302,7 @@ static int kfd_dbg_set_queue_workaround(struct
> queue *q, bool enable)
>   if (!q)
>   return 0;
>
> - if (KFD_GC_VERSION(q->device) < IP_VERSION(11, 0, 0) ||
> - KFD_GC_VERSION(q->device) >= IP_VERSION(12, 0, 0))
> + if (!kfd_dbg_has_cwsr_workaround(q->device))
>   return 0;
>
>   if (enable && q->properties.is_user_cu_masked)
> @@ -349,7 +348,7 @@ int kfd_dbg_set_mes_debug_mode(struct
> kfd_process_device *pdd)
>  {
>   uint32_t spi_dbg_cntl = pdd->spi_dbg_override | pdd-
> >spi_dbg_launch_mode;
>   uint32_t flags = pdd->process->dbg_flags;
> - bool sq_trap_en = !!spi_dbg_cntl;
> + bool sq_trap_en = !!spi_dbg_cntl ||
> !kfd_dbg_has_cwsr_workaround(pdd->dev);
>
>   if (!kfd_dbg_is_per_vmid_supported(pdd->dev))
>   return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index ba616ed17dee..586d7f886712 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -101,6 +101,12 @@ static inline bool
> kfd_dbg_is_rlc_restore_supported(struct kfd_node *dev)
>KFD_GC_VERSION(dev) == IP_VERSION(10, 1, 1));
>  }
>
> +static inline bool kfd_dbg_has_cwsr_workaround(struct kfd_node *dev)
> +{
> + return KFD_GC_VERSION(dev) >= IP_VERSION(11, 0, 0) &&
> +KFD_GC_VERSION(dev) <= IP_VERSION(11, 0, 3);
> +}
> +
>  static inline bool kfd_dbg_has_gws_support(struct kfd_node *dev)
>  {
>   if ((KFD_GC_VERSION(dev) == IP_VERSION(9, 0, 1)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 31cac1fd0d58..761963ad6154 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -226,8 +226,7 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
>   queue_input.paging = false;
>   queue_input.tba_addr = qpd->tba_addr;
>   queue_input.tma_addr = qpd->tma_addr;
> - queue_input.trap_en = KFD_GC_VERSION(q->device) <
> IP_VERSION(11, 0, 0) ||
> -   KFD_GC_VERSION(q->device) > IP_VERSION(11, 0,
> 3);
> + queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
>   queue_input.skip_process_ctx_clear = qpd->pqm->process-
> >debug_trap_enabled;
>
>   queue_type = convert_to_mes_queue_type(q->properties.type);
> @@ -1827,8 +1826,7 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
>*/
>   q->properties.is_evicted = !!qpd->evicted;
>   q->properties.is_dbg_wa = qpd->pqm->process-
> >debug_trap_enabled &&
> - KFD_GC_VERSION(q->device) >= IP_VERSION(11, 0, 0)
> &&
> - KFD_GC_VERSION(q->device) <= IP_VERSION(11, 0, 3);
> +   kfd_dbg_has_cwsr_workaround(q->device);
>
>   if (qd)
>   mqd_mgr->restore_mqd(mqd_mgr, >mqd, q-
> >mqd_mem_obj, >gart_mqd_addr,
> --
> 2.25.1



Re: [PATCH 2/2] drm/amdkfd: enable cooperative groups for gfx11

2023-07-18 Thread Felix Kuehling



Am 2023-07-14 um 05:37 schrieb Jonathan Kim:

MES can concurrently schedule queues on the device that require
exclusive device access if marked exclusively_scheduled without the
requirement of GWS.  Similar to the F32 HWS, MES will manage
quality of service for these queues.
Use this for cooperative groups since cooperative groups are device
occupancy limited.

Since some GFX11 devices can only be debugged with partial CUs, do not
allow the debugging of cooperative groups on these devices as the CU
occupancy limit will change on attach.

In addition, zero initialize the MES add queue submission vector for MES
initialization tests as we do not want these to be cooperative
dispatches.

NOTE: FIXME MES FW enablement checks are a placeholder at the moment and
will be updated when the binary revision number is finalized.

Signed-off-by: Jonathan Kim 

Some nit-picks inline. Looks good to me otherwise.



---
  drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c   |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h   |  1 +
  drivers/gpu/drm/amd/amdgpu/mes_v11_0.c|  2 ++
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c  |  3 ++-
  drivers/gpu/drm/amd/amdkfd/kfd_debug.c|  3 ++-
  drivers/gpu/drm/amd/amdkfd/kfd_device.c   |  6 +-
  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c |  9 -
  .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c| 11 +++
  drivers/gpu/drm/amd/include/mes_v11_api_def.h |  4 +++-
  9 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index e9091ebfe230..8d13623389d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -638,7 +638,7 @@ int amdgpu_mes_add_hw_queue(struct amdgpu_device *adev, int 
gang_id,
  {
struct amdgpu_mes_queue *queue;
struct amdgpu_mes_gang *gang;
-   struct mes_add_queue_input queue_input;
+   struct mes_add_queue_input queue_input = {0};


 Generally, it is preferred to use memset to initialize structures on 
the stack because that also initializes padding.




unsigned long flags;
int r;
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h

index 2d6ac30b7135..2053954a235c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
@@ -224,6 +224,7 @@ struct mes_add_queue_input {
uint32_tis_kfd_process;
uint32_tis_aql_queue;
uint32_tqueue_size;
+   uint32_texclusively_scheduled;
  };
  
  struct mes_remove_queue_input {

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 1bdaa00c0b46..8e67e965f7ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -214,6 +214,8 @@ static int mes_v11_0_add_hw_queue(struct amdgpu_mes *mes,
mes_add_queue_pkt.is_aql_queue = input->is_aql_queue;
mes_add_queue_pkt.gds_size = input->queue_size;
  
+	mes_add_queue_pkt.exclusively_scheduled = input->exclusively_scheduled;

+
return mes_v11_0_submit_pkt_and_poll_completion(mes,
_add_queue_pkt, sizeof(mes_add_queue_pkt),
offsetof(union MESAPI__ADD_QUEUE, api_status));
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 40ac093b5035..e18401811956 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1487,7 +1487,8 @@ static int kfd_ioctl_alloc_queue_gws(struct file *filep,
goto out_unlock;
}
  
-	if (!kfd_dbg_has_gws_support(dev) && p->debug_trap_enabled) {

+   if (p->debug_trap_enabled && (!kfd_dbg_has_gws_support(dev) ||
+  kfd_dbg_has_cwsr_workaround(dev))) {


Indentation looks off. kfd_dbg_has_cwsr_workaround should be indented 
one less space. Otherwise you may be incorrectly implying that the ! 
applies to it.




retval = -EBUSY;
goto out_unlock;
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index ccfc81f085ce..895e7f690fd0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -753,7 +753,8 @@ int kfd_dbg_trap_enable(struct kfd_process *target, 
uint32_t fd,
if (!KFD_IS_SOC15(pdd->dev))
return -ENODEV;
  
-		if (!kfd_dbg_has_gws_support(pdd->dev) && pdd->qpd.num_gws)

+   if (pdd->qpd.num_gws && (!kfd_dbg_has_gws_support(pdd->dev) ||
+ 
kfd_dbg_has_cwsr_workaround(pdd->dev)))


Same as above.



return -EBUSY;
}
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 

RE: [PATCH] drm/amd: Fix an error handling mistake in psp_sw_init()

2023-07-18 Thread Limonciello, Mario
[Public]

> -Original Message-
> From: Limonciello, Mario 
> Sent: Thursday, July 13, 2023 00:15
> To: amd-gfx@lists.freedesktop.org
> Cc: Limonciello, Mario 
> Subject: [PATCH] drm/amd: Fix an error handling mistake in psp_sw_init()
>
> If the second call to amdgpu_bo_create_kernel() fails, the memory
> allocated from the first call should be cleared.  If the third call
> fails, the memory from the second call should be cleared.
>
> Fixes: b95b5391684b ("drm/amdgpu/psp: move PSP memory alloc from
> hw_init to sw_init")
> Signed-off-by: Mario Limonciello 

Ping on this one.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 1b4d5f04d968..6ffc1a640d2d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -491,11 +491,11 @@ static int psp_sw_init(void *handle)
>   return 0;
>
>  failed2:
> - amdgpu_bo_free_kernel(>fw_pri_bo,
> -   >fw_pri_mc_addr, >fw_pri_buf);
> -failed1:
>   amdgpu_bo_free_kernel(>fence_buf_bo,
> >fence_buf_mc_addr, >fence_buf);
> +failed1:
> + amdgpu_bo_free_kernel(>fw_pri_bo,
> +   >fw_pri_mc_addr, >fw_pri_buf);
>   return ret;
>  }
>
> --
> 2.34.1



Re: [PATCH] drm/amd: Avoid reading the VBIOS part number twice

2023-07-18 Thread Alex Deucher
On Tue, Jul 18, 2023 at 2:03 PM Mario Limonciello
 wrote:
>
> The VBIOS part number is read both in amdgpu_atom_parse() as well
> as in atom_get_vbios_pn() and stored twice in the `struct atom_context`
> structure. Remove the first unnecessary read and move the `pr_info`
> line from that read into the second.
>
> Signed-off-by: Mario Limonciello 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c  |  2 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 20 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   |  8 
>  .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c|  8 
>  drivers/gpu/drm/amd/amdgpu/atom.c | 13 ++--
>  drivers/gpu/drm/amd/amdgpu/atom.h |  2 --
>  7 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index f4e3c133a16ca..dce9e7d5e4ec6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -1776,7 +1776,7 @@ static ssize_t amdgpu_atombios_get_vbios_version(struct 
> device *dev,
> struct amdgpu_device *adev = drm_to_adev(ddev);
> struct atom_context *ctx = adev->mode_info.atom_context;
>
> -   return sysfs_emit(buf, "%s\n", ctx->vbios_version);
> +   return sysfs_emit(buf, "%s\n", ctx->vbios_pn);
>  }
>
>  static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 4620c4712ce32..c9f16eab0f3d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -60,10 +60,10 @@ static bool is_fru_eeprom_supported(struct amdgpu_device 
> *adev, u32 *fru_addr)
> switch (adev->asic_type) {
> case CHIP_VEGA20:
> /* D161 and D163 are the VG20 server SKUs */
> -   if (strnstr(atom_ctx->vbios_version, "D161",
> -   sizeof(atom_ctx->vbios_version)) ||
> -   strnstr(atom_ctx->vbios_version, "D163",
> -   sizeof(atom_ctx->vbios_version))) {
> +   if (strnstr(atom_ctx->vbios_pn, "D161",
> +   sizeof(atom_ctx->vbios_pn)) ||
> +   strnstr(atom_ctx->vbios_pn, "D163",
> +   sizeof(atom_ctx->vbios_pn))) {
> if (fru_addr)
> *fru_addr = FRU_EEPROM_MADDR_6;
> return true;
> @@ -72,16 +72,16 @@ static bool is_fru_eeprom_supported(struct amdgpu_device 
> *adev, u32 *fru_addr)
> }
> case CHIP_ALDEBARAN:
> /* All Aldebaran SKUs have an FRU */
> -   if (!strnstr(atom_ctx->vbios_version, "D673",
> -sizeof(atom_ctx->vbios_version)))
> +   if (!strnstr(atom_ctx->vbios_pn, "D673",
> +sizeof(atom_ctx->vbios_pn)))
> if (fru_addr)
> *fru_addr = FRU_EEPROM_MADDR_6;
> return true;
> case CHIP_SIENNA_CICHLID:
> -   if (strnstr(atom_ctx->vbios_version, "D603",
> -   sizeof(atom_ctx->vbios_version))) {
> -   if (strnstr(atom_ctx->vbios_version, "D603GLXE",
> -   sizeof(atom_ctx->vbios_version))) {
> +   if (strnstr(atom_ctx->vbios_pn, "D603",
> +   sizeof(atom_ctx->vbios_pn))) {
> +   if (strnstr(atom_ctx->vbios_pn, "D603GLXE",
> +   sizeof(atom_ctx->vbios_pn))) {
> return false;
> } else {
> if (fru_addr)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index cca5a495611f3..46844082762a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1719,7 +1719,7 @@ static int amdgpu_debugfs_firmware_info_show(struct 
> seq_file *m, void *unused)
> seq_printf(m, "MES feature version: %u, firmware version: 0x%08x\n",
>fw_info.feature, fw_info.ver);
>
> -   seq_printf(m, "VBIOS version: %s\n", ctx->vbios_version);
> +   seq_printf(m, "VBIOS version: %s\n", ctx->vbios_pn);
>
> return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 8aaa427f8c0f6..5055c14d6b426 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -2440,10 +2440,10 @@ static void amdgpu_ras_get_quirks(struct 
> amdgpu_device *adev)
> if (!ctx)
>  

[PATCH] drm/amdgpu: Fix infinite loop in gfxhub_v1_2_xcc_gart_enable

2023-07-18 Thread Victor Lu
An instance of for_each_inst() was not changed to match its new
behaviour and is causing a loop.

Fixes: 50c1d81d6365 ("drm/amdgpu: Modify for_each_inst macro")
Signed-off-by: Victor Lu 
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
index 47f95ec218a3..b74df0e9fb08 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_2.c
@@ -413,7 +413,6 @@ static int gfxhub_v1_2_xcc_gart_enable(struct amdgpu_device 
*adev,
 */
if (amdgpu_sriov_vf(adev)) {
for_each_inst(i, tmp_mask) {
-   i = ffs(tmp_mask) - 1;
WREG32_SOC15_RLC(GC, GET_INST(GC, i), 
regMC_VM_FB_LOCATION_BASE,
 adev->gmc.vram_start >> 24);
WREG32_SOC15_RLC(GC, GET_INST(GC, i), 
regMC_VM_FB_LOCATION_TOP,
-- 
2.34.1



[PATCH] drm/amd: Avoid reading the VBIOS part number twice

2023-07-18 Thread Mario Limonciello
The VBIOS part number is read both in amdgpu_atom_parse() as well
as in atom_get_vbios_pn() and stored twice in the `struct atom_context`
structure. Remove the first unnecessary read and move the `pr_info`
line from that read into the second.

Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c  |  2 +-
 .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 20 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   |  8 
 .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c|  8 
 drivers/gpu/drm/amd/amdgpu/atom.c | 13 ++--
 drivers/gpu/drm/amd/amdgpu/atom.h |  2 --
 7 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index f4e3c133a16ca..dce9e7d5e4ec6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -1776,7 +1776,7 @@ static ssize_t amdgpu_atombios_get_vbios_version(struct 
device *dev,
struct amdgpu_device *adev = drm_to_adev(ddev);
struct atom_context *ctx = adev->mode_info.atom_context;
 
-   return sysfs_emit(buf, "%s\n", ctx->vbios_version);
+   return sysfs_emit(buf, "%s\n", ctx->vbios_pn);
 }
 
 static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
index 4620c4712ce32..c9f16eab0f3d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
@@ -60,10 +60,10 @@ static bool is_fru_eeprom_supported(struct amdgpu_device 
*adev, u32 *fru_addr)
switch (adev->asic_type) {
case CHIP_VEGA20:
/* D161 and D163 are the VG20 server SKUs */
-   if (strnstr(atom_ctx->vbios_version, "D161",
-   sizeof(atom_ctx->vbios_version)) ||
-   strnstr(atom_ctx->vbios_version, "D163",
-   sizeof(atom_ctx->vbios_version))) {
+   if (strnstr(atom_ctx->vbios_pn, "D161",
+   sizeof(atom_ctx->vbios_pn)) ||
+   strnstr(atom_ctx->vbios_pn, "D163",
+   sizeof(atom_ctx->vbios_pn))) {
if (fru_addr)
*fru_addr = FRU_EEPROM_MADDR_6;
return true;
@@ -72,16 +72,16 @@ static bool is_fru_eeprom_supported(struct amdgpu_device 
*adev, u32 *fru_addr)
}
case CHIP_ALDEBARAN:
/* All Aldebaran SKUs have an FRU */
-   if (!strnstr(atom_ctx->vbios_version, "D673",
-sizeof(atom_ctx->vbios_version)))
+   if (!strnstr(atom_ctx->vbios_pn, "D673",
+sizeof(atom_ctx->vbios_pn)))
if (fru_addr)
*fru_addr = FRU_EEPROM_MADDR_6;
return true;
case CHIP_SIENNA_CICHLID:
-   if (strnstr(atom_ctx->vbios_version, "D603",
-   sizeof(atom_ctx->vbios_version))) {
-   if (strnstr(atom_ctx->vbios_version, "D603GLXE",
-   sizeof(atom_ctx->vbios_version))) {
+   if (strnstr(atom_ctx->vbios_pn, "D603",
+   sizeof(atom_ctx->vbios_pn))) {
+   if (strnstr(atom_ctx->vbios_pn, "D603GLXE",
+   sizeof(atom_ctx->vbios_pn))) {
return false;
} else {
if (fru_addr)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index cca5a495611f3..46844082762a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1719,7 +1719,7 @@ static int amdgpu_debugfs_firmware_info_show(struct 
seq_file *m, void *unused)
seq_printf(m, "MES feature version: %u, firmware version: 0x%08x\n",
   fw_info.feature, fw_info.ver);
 
-   seq_printf(m, "VBIOS version: %s\n", ctx->vbios_version);
+   seq_printf(m, "VBIOS version: %s\n", ctx->vbios_pn);
 
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 8aaa427f8c0f6..5055c14d6b426 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2440,10 +2440,10 @@ static void amdgpu_ras_get_quirks(struct amdgpu_device 
*adev)
if (!ctx)
return;
 
-   if (strnstr(ctx->vbios_version, "D16406",
-   sizeof(ctx->vbios_version)) ||
-   strnstr(ctx->vbios_version, "D36002",
-   sizeof(ctx->vbios_version)))
+   if (strnstr(ctx->vbios_pn, "D16406",
+

Re: [PATCH] drm/amd/display: Allow building DC with clang on RISC-V

2023-07-18 Thread Nathan Chancellor
On Mon, Jul 17, 2023 at 03:29:23PM -0700, Samuel Holland wrote:
> clang on RISC-V appears to be unaffected by the bug causing excessive
> stack usage in calculate_bandwidth(). clang 16 with -fstack-usage
> reports a 304 byte stack frame size with CONFIG_ARCH_RV32I, and 512
> bytes with CONFIG_ARCH_RV64I.
> 
> Signed-off-by: Samuel Holland 

I built ARCH=riscv allmodconfig drivers/gpu/drm/amd/amdgpu/ (confirming
that CONFIG_DRM_AMD_DC gets enabled) with LLVM 11 through 17 with and
without CONFIG_KASAN=y and I never saw the -Wframe-larger-than instance
that this was disabled for, so I agree.

Reviewed-by: Nathan Chancellor 
Tested-by: Nathan Chancellor 

> 
>  drivers/gpu/drm/amd/display/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/Kconfig 
> b/drivers/gpu/drm/amd/display/Kconfig
> index bf0a655d009e..901d1961b739 100644
> --- a/drivers/gpu/drm/amd/display/Kconfig
> +++ b/drivers/gpu/drm/amd/display/Kconfig
> @@ -5,7 +5,7 @@ menu "Display Engine Configuration"
>  config DRM_AMD_DC
>   bool "AMD DC - Enable new display engine"
>   default y
> - depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
> + depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
>   select SND_HDA_COMPONENT if SND_HDA_CORE
>   # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
>   select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || 
> (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG))
> -- 
> 2.40.1
> 


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Lazar, Lijo




On 7/18/2023 9:39 PM, James Zhu wrote:


On 2023-07-18 11:54, Lazar, Lijo wrote:



On 7/18/2023 8:39 PM, James Zhu wrote:


On 2023-07-18 10:19, Lazar, Lijo wrote:



On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:
Not all rings have scheduler associated. Only update scheduler 
data for

rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
  ring = adev->rings[i];
-    if (!ring || !ring->sched.ready)
+    if (!ring || !ring->sched.ready || ring->no_scheduler)
[JZ] any case for ring->no_scheduler = true, but ring->sched.ready 
= true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
amdgpu_device_init_schedulers inits schedulers only for rings which 
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
ring is marked with ring->sched.ready = true;


I'm not sure why it's done this way, but this is the case even for 
gfxv9.
[JZ] I think the sched.ready confuses people. here just means ring is 
ready be used.


Thanks for the clarification. One question is then do we need to check 
ring ready status for assigning xcp ids or just check if the ring is 
associated with a scheduler?


Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids 
and leave the ring ready status to the logic which really accepts jobs 
on the ring?
[JZ] I think keeping ring->sched.ready will reduce schedule list which 
will save a little scheduling time.


Fine, will just push this one.

Thanks,
Lijo



Thanks,
Lijo


This patch is Reviewed-by: James Zhu 



Thanks,
Lijo


  continue;
  aqua_vanjaram_xcp_gpu_sched_update(adev, ring, 
ring->xcp_id);


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread James Zhu



On 2023-07-18 11:54, Lazar, Lijo wrote:



On 7/18/2023 8:39 PM, James Zhu wrote:


On 2023-07-18 10:19, Lazar, Lijo wrote:



On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:
Not all rings have scheduler associated. Only update scheduler 
data for

rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
  ring = adev->rings[i];
-    if (!ring || !ring->sched.ready)
+    if (!ring || !ring->sched.ready || ring->no_scheduler)
[JZ] any case for ring->no_scheduler = true, but ring->sched.ready 
= true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
amdgpu_device_init_schedulers inits schedulers only for rings which 
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
ring is marked with ring->sched.ready = true;


I'm not sure why it's done this way, but this is the case even for 
gfxv9.
[JZ] I think the sched.ready confuses people. here just means ring is 
ready be used.


Thanks for the clarification. One question is then do we need to check 
ring ready status for assigning xcp ids or just check if the ring is 
associated with a scheduler?


Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids 
and leave the ring ready status to the logic which really accepts jobs 
on the ring?
[JZ] I think keeping ring->sched.ready will reduce schedule list which 
will save a little scheduling time.


Thanks,
Lijo


This patch is Reviewed-by: James Zhu 



Thanks,
Lijo


  continue;
  aqua_vanjaram_xcp_gpu_sched_update(adev, ring, 
ring->xcp_id);


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Lazar, Lijo




On 7/18/2023 8:39 PM, James Zhu wrote:


On 2023-07-18 10:19, Lazar, Lijo wrote:



On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:

Not all rings have scheduler associated. Only update scheduler data for
rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
  ring = adev->rings[i];
-    if (!ring || !ring->sched.ready)
+    if (!ring || !ring->sched.ready || ring->no_scheduler)
[JZ] any case for ring->no_scheduler = true, but ring->sched.ready = 
true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
amdgpu_device_init_schedulers inits schedulers only for rings which 
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
ring is marked with ring->sched.ready = true;


I'm not sure why it's done this way, but this is the case even for gfxv9.
[JZ] I think the sched.ready confuses people. here just means ring is 
ready be used.


Thanks for the clarification. One question is then do we need to check 
ring ready status for assigning xcp ids or just check if the ring is 
associated with a scheduler?


Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids and 
leave the ring ready status to the logic which really accepts jobs on 
the ring?


Thanks,
Lijo


This patch is Reviewed-by: James Zhu 



Thanks,
Lijo


  continue;
  aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);


Re: [PATCH 1/5] mm: introduce vma_is_stack() and vma_is_heap()

2023-07-18 Thread Christian Göttsche
On Wed, 12 Jul 2023 at 16:25, Kefeng Wang  wrote:
>
> Introduce the two helpers for general use.
>
> Signed-off-by: Kefeng Wang 
> ---
>  include/linux/mm.h | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1462cf15badf..0bbeb31ac750 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -926,6 +926,18 @@ static inline bool vma_is_anonymous(struct 
> vm_area_struct *vma)
> return !vma->vm_ops;
>  }
>
> +static inline bool vma_is_heap(struct vm_area_struct *vma)

What about declaring the parameters const to document in code these
functions do not modify any state, and allow callers to pass pointers
to const?

> +{
> +   return vma->vm_start <= vma->vm_mm->brk &&
> +   vma->vm_end >= vma->vm_mm->start_brk;
> +}
> +
> +static inline bool vma_is_stack(struct vm_area_struct *vma)
> +{
> +   return vma->vm_start <= vma->vm_mm->start_stack &&
> +  vma->vm_end >= vma->vm_mm->start_stack;
> +}
> +
>  static inline bool vma_is_temporary_stack(struct vm_area_struct *vma)
>  {
> int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);
> --
> 2.41.0
>


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread James Zhu



On 2023-07-18 10:19, Lazar, Lijo wrote:



On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:

Not all rings have scheduler associated. Only update scheduler data for
rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
  ring = adev->rings[i];
-    if (!ring || !ring->sched.ready)
+    if (!ring || !ring->sched.ready || ring->no_scheduler)
[JZ] any case for ring->no_scheduler = true, but ring->sched.ready = 
true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
amdgpu_device_init_schedulers inits schedulers only for rings which 
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
ring is marked with ring->sched.ready = true;


I'm not sure why it's done this way, but this is the case even for gfxv9.
[JZ] I think the sched.ready confuses people. here just means ring is 
ready be used.

This patch is Reviewed-by: James Zhu 



Thanks,
Lijo


  continue;
  aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);


Re: [PATCH] drm/radeon/dpm: ERROR: open brace '{' following enum go on the same line

2023-07-18 Thread Alex Deucher
I'm happy to apply these patches, but please check your mailer.  The
formatting is messed up and they don't apply cleanly.  Please use
git-format-patch and git-send-email to generate and send the patches.

Thanks!

Alex

On Fri, Jul 14, 2023 at 3:15 AM  wrote:
>
> Fix four occurrences of the checkpatch.pl error:
> ERROR: open brace '{' following enum go on the same line
>
> Signed-off-by: Jie Shi 
> ---
>   drivers/gpu/drm/radeon/ni_dpm.h | 12 
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ni_dpm.h
> b/drivers/gpu/drm/radeon/ni_dpm.h
> index 74e301936906..4e3e7303e035 100644
> --- a/drivers/gpu/drm/radeon/ni_dpm.h
> +++ b/drivers/gpu/drm/radeon/ni_dpm.h
> @@ -59,8 +59,7 @@ struct ni_mc_reg_table {
>
>   #define NISLANDS_MCREGISTERTABLE_FIRST_DRIVERSTATE_SLOT 2
>
> -enum ni_dc_cac_level
> -{
> +enum ni_dc_cac_level {
>   NISLANDS_DCCAC_LEVEL_0 = 0,
>   NISLANDS_DCCAC_LEVEL_1,
>   NISLANDS_DCCAC_LEVEL_2,
> @@ -72,8 +71,7 @@ enum ni_dc_cac_level
>   NISLANDS_DCCAC_MAX_LEVELS
>   };
>
> -struct ni_leakage_coeffients
> -{
> +struct ni_leakage_coeffients {
>   u32 at;
>   u32 bt;
>   u32 av;
> @@ -83,8 +81,7 @@ struct ni_leakage_coeffients
>   u32 t_ref;
>   };
>
> -struct ni_cac_data
> -{
> +struct ni_cac_data {
>   struct ni_leakage_coeffients leakage_coefficients;
>   u32 i_leakage;
>   s32 leakage_minimum_temperature;
> @@ -100,8 +97,7 @@ struct ni_cac_data
>   u8 lts_truncate_n;
>   };
>
> -struct ni_cac_weights
> -{
> +struct ni_cac_weights {
>   u32 weight_tcp_sig0;
>   u32 weight_tcp_sig1;
>   u32 weight_ta_sig;


Re: [PATCH] drm/amd: open brace '{' following struct go on the same line

2023-07-18 Thread Alex Deucher
I'm happy to apply these patches, but please check your mailer.  The
formatting is messed up and they don't apply cleanly.  Please use
git-format-patch and git-send-email to generate and send the patches.

Thanks!

Alex

On Fri, Jul 14, 2023 at 2:04 AM  wrote:
>
> Fix the checkpatch error as open brace '{' following struct should
> go on the same line.
>
> Signed-off-by: Ran Sun 
> ---
>   drivers/gpu/drm/amd/include/yellow_carp_offset.h | 6 ++
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/yellow_carp_offset.h
> b/drivers/gpu/drm/amd/include/yellow_carp_offset.h
> index 0fea6a746611..a2c8dca2425e 100644
> --- a/drivers/gpu/drm/amd/include/yellow_carp_offset.h
> +++ b/drivers/gpu/drm/amd/include/yellow_carp_offset.h
> @@ -7,13 +7,11 @@
>   #define MAX_SEGMENT 6
>
> -struct IP_BASE_INSTANCE
> -{
> +struct IP_BASE_INSTANCE {
>   unsigned int segment[MAX_SEGMENT];
>   } __maybe_unused;
>
> -struct IP_BASE
> -{
> +struct IP_BASE {
>   struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
>   } __maybe_unused;


Re: [PATCH V6 1/9] drivers core: Add support for Wifi band RF mitigations

2023-07-18 Thread Andrew Lunn
> The wbrf_supported_producer and wbrf_supported_consumer APIs seem
> unnecessary for the generic implementation.

I'm happy with these, once the description is corrected. As i said in
another comment, 'can' should be replaced with 'should'. The device
itself knows if it can, only the core knows if it should, based on the
policy of if actions need to be taken, and there are both providers
and consumers registered with the core.

   Andrew


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Lazar, Lijo




On 7/18/2023 7:30 PM, James Zhu wrote:


On 2023-07-18 08:21, Lijo Lazar wrote:

Not all rings have scheduler associated. Only update scheduler data for
rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
  ring = adev->rings[i];
-    if (!ring || !ring->sched.ready)
+    if (!ring || !ring->sched.ready || ring->no_scheduler)

[JZ] any case for ring->no_scheduler = true, but ring->sched.ready = true?


amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
amdgpu_device_init_schedulers inits schedulers only for rings which 
doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
ring is marked with ring->sched.ready = true;


I'm not sure why it's done this way, but this is the case even for gfxv9.

Thanks,
Lijo


  continue;
  aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);


Re: [PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread James Zhu



On 2023-07-18 08:21, Lijo Lazar wrote:

Not all rings have scheduler associated. Only update scheduler data for
rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c 
b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
  
  	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {

ring = adev->rings[i];
-   if (!ring || !ring->sched.ready)
+   if (!ring || !ring->sched.ready || ring->no_scheduler)

[JZ] any case for ring->no_scheduler = true, but ring->sched.ready = true?

continue;
  
  		aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);


Re: [PATCH] drm/amdgpu: Enabling FW workaround through shared memory for VCN4_0_2

2023-07-18 Thread Leo Liu

Reviewed-by: Leo Liu 

On 2023-07-17 23:20, sguttula wrote:

This patch will enable VCN FW workaround using
DRM KEY INJECT WORKAROUND method,
which is helping in fixing the secure playback.

Signed-off-by: sguttula 

---

Changes in v2:
-updated commit message as per veera's feedback

Changes in v3:
-updated commit message as enabling for 402
-updated the logic as per leo, feedback
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 9 +
  drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c   | 6 ++
  2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 1f1d7dc94f90..a3eed90b6af0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -161,6 +161,7 @@
} while (0)
  
  #define AMDGPU_FW_SHARED_FLAG_0_UNIFIED_QUEUE (1 << 2)

+#define AMDGPU_FW_SHARED_FLAG_0_DRM_KEY_INJECT (1 << 4)
  #define AMDGPU_VCN_FW_SHARED_FLAG_0_RB(1 << 6)
  #define AMDGPU_VCN_MULTI_QUEUE_FLAG   (1 << 8)
  #define AMDGPU_VCN_SW_RING_FLAG   (1 << 9)
@@ -180,6 +181,8 @@
  #define AMDGPU_VCN_SMU_DPM_INTERFACE_DGPU (0)
  #define AMDGPU_VCN_SMU_DPM_INTERFACE_APU (1)
  
+#define AMDGPU_DRM_KEY_INJECT_WORKAROUND_VCNFW_ASD_HANDSHAKING 2

+
  enum fw_queue_mode {
FW_QUEUE_RING_RESET = 1,
FW_QUEUE_DPG_HOLD_OFF = 2,
@@ -343,6 +346,11 @@ struct amdgpu_fw_shared_rb_setup {
uint32_t  reserved[6];
  };
  
+struct amdgpu_fw_shared_drm_key_wa {

+   uint8_t  method;
+   uint8_t  reserved[3];
+};
+
  struct amdgpu_vcn4_fw_shared {
uint32_t present_flag_0;
uint8_t pad[12];
@@ -352,6 +360,7 @@ struct amdgpu_vcn4_fw_shared {
uint8_t pad2[20];
struct amdgpu_fw_shared_rb_setup rb_setup;
struct amdgpu_fw_shared_smu_interface_info smu_dpm_interface;
+   struct amdgpu_fw_shared_drm_key_wa drm_key_wa;
  };
  
  struct amdgpu_vcn_fwlog {

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index e8c02ae10163..16ee73cfc3a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -169,6 +169,12 @@ static int vcn_v4_0_sw_init(void *handle)
fw_shared->smu_dpm_interface.smu_interface_type = (adev->flags 
& AMD_IS_APU) ?
AMDGPU_VCN_SMU_DPM_INTERFACE_APU : 
AMDGPU_VCN_SMU_DPM_INTERFACE_DGPU;
  
+		if (adev->ip_versions[VCN_HWIP][0] == IP_VERSION(4, 0, 2)) {

+   fw_shared->present_flag_0 |= 
AMDGPU_FW_SHARED_FLAG_0_DRM_KEY_INJECT;
+   fw_shared->drm_key_wa.method =
+   
AMDGPU_DRM_KEY_INJECT_WORKAROUND_VCNFW_ASD_HANDSHAKING;
+   }
+
if (amdgpu_sriov_vf(adev))
fw_shared->present_flag_0 |= 
cpu_to_le32(AMDGPU_VCN_VF_RB_SETUP_FLAG);
  


Re: [PATCH] drm/amdgpu: allow secure submission on VCN4 ring

2023-07-18 Thread Leo Liu

Reviewed-by: Leo Liu 

On 2023-07-17 13:27, sguttula wrote:

This patch will enable secure decode playback on VCN4_0_2

Signed-off-by: sguttula 

---
Changes in v2:
-updated commit message only enabling for VCN402
-updated the logic as per Leo's feedback
---
  drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index e8c02ae10163..d2d89bb711b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1801,7 +1801,7 @@ static int vcn_v4_0_ring_patch_cs_in_place(struct 
amdgpu_cs_parser *p,
return 0;
  }
  
-static const struct amdgpu_ring_funcs vcn_v4_0_unified_ring_vm_funcs = {

+static struct amdgpu_ring_funcs vcn_v4_0_unified_ring_vm_funcs = {
.type = AMDGPU_RING_TYPE_VCN_ENC,
.align_mask = 0x3f,
.nop = VCN_ENC_CMD_NO_OP,
@@ -1846,7 +1846,11 @@ static void vcn_v4_0_set_unified_ring_funcs(struct 
amdgpu_device *adev)
if (adev->vcn.harvest_config & (1 << i))
continue;
  
-		adev->vcn.inst[i].ring_enc[0].funcs = _v4_0_unified_ring_vm_funcs;

+   if (adev->ip_versions[VCN_HWIP][0] == IP_VERSION(4, 0, 2))
+   
vcn_v4_0_unified_ring_vm_funcs.secure_submission_supported = true;
+
+   adev->vcn.inst[i].ring_enc[0].funcs =
+  (const struct amdgpu_ring_funcs 
*)_v4_0_unified_ring_vm_funcs;
adev->vcn.inst[i].ring_enc[0].me = i;
  
  		DRM_INFO("VCN(%d) encode/decode are enabled in VM mode\n", i);


[PATCH] drm/amdgpu: Update ring scheduler info as needed

2023-07-18 Thread Lijo Lazar
Not all rings have scheduler associated. Only update scheduler data for
rings with scheduler. It could result in out of bound access as total
rings are more than those associated with particular IPs.

Signed-off-by: Lijo Lazar 
---
 drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c 
b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
index 72b629a78c62..d0fc62784e82 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
 
for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
ring = adev->rings[i];
-   if (!ring || !ring->sched.ready)
+   if (!ring || !ring->sched.ready || ring->no_scheduler)
continue;
 
aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);
-- 
2.25.1



Re: [PATCH v4 11/18] media: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers

2023-07-18 Thread Hans Verkuil
Hi Thomas,

On 15/07/2023 20:51, Thomas Zimmermann wrote:
> The flag FBINFO_FLAG_DEFAULT is 0 and has no effect, as struct
> fbinfo.flags has been allocated to zero by kzalloc(). So do not
> set it.
> 
> Flags should signal differences from the default values. After cleaning
> up all occurrences of FBINFO_DEFAULT, the token will be removed.
> 
> v2:
>   * fix commit message (Miguel)
> 
> Signed-off-by: Thomas Zimmermann 
> Acked-by: Sam Ravnborg 
> Cc: Andy Walls 
> Cc: Mauro Carvalho Chehab 
> Cc: Hans Verkuil 
> ---
>  drivers/media/pci/ivtv/ivtvfb.c  | 1 -
>  drivers/media/test-drivers/vivid/vivid-osd.c | 1 -
>  2 files changed, 2 deletions(-)

I can take this patches for 6.6, unless you prefer to have this whole series
merged in one go?

In that case you can use my:

Reviewed-by: Hans Verkuil 

Regards,

Hans

> 
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 0aeb9daaee4c..23c8c094e791 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -1048,7 +1048,6 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>   /* Generate valid fb_info */
>  
>   oi->ivtvfb_info.node = -1;
> - oi->ivtvfb_info.flags = FBINFO_FLAG_DEFAULT;
>   oi->ivtvfb_info.par = itv;
>   oi->ivtvfb_info.var = oi->ivtvfb_defined;
>   oi->ivtvfb_info.fix = oi->ivtvfb_fix;
> diff --git a/drivers/media/test-drivers/vivid/vivid-osd.c 
> b/drivers/media/test-drivers/vivid/vivid-osd.c
> index ec25edc679b3..051f1805a16d 100644
> --- a/drivers/media/test-drivers/vivid/vivid-osd.c
> +++ b/drivers/media/test-drivers/vivid/vivid-osd.c
> @@ -310,7 +310,6 @@ static int vivid_fb_init_vidmode(struct vivid_dev *dev)
>   /* Generate valid fb_info */
>  
>   dev->fb_info.node = -1;
> - dev->fb_info.flags = FBINFO_FLAG_DEFAULT;
>   dev->fb_info.par = dev;
>   dev->fb_info.var = dev->fb_defined;
>   dev->fb_info.fix = dev->fb_fix;



RE: [PATCH V6 1/9] drivers core: Add support for Wifi band RF mitigations

2023-07-18 Thread Quan, Evan
[AMD Official Use Only - General]

Personally I would like to treat the wbrf core as a water pool. Any stream can 
flow in. Also any needed can drain water from it at any time.
The way to allow producers to report only when there is consumer existing does 
not work. Since the consumer might come after the producer.
Just considering the scenario below:
Wifi core/driver started --> wifi driver reports its frequency in-use  --> 
proper action taken by wbrf core --> amdgpu driver(consumer) started
What should be the proper action taken by wbrf core then? Stop the producer to 
report its frequency in-use? That might lead consumer to never have a chance to 
know that.

The wbrf_supported_producer and wbrf_supported_consumer APIs seem unnecessary 
for the generic implementation.
But to support AMD ACPI implementation(or future device tree implementation), 
they are needed. The wbrf core needs to check whether the necessary AML codes 
are there.
It needs those information to judge whether a producer can report(will be 
accepted) or a consumer can retrieve needed information.

> > +struct wbrf_ranges_out {
> > +   u32 num_of_ranges;
> > +   struct exclusion_range  band_list[MAX_NUM_OF_WBRF_RANGES];
> > +} __packed;
>
> Seems odd using packed here. It is the only structure which is
> packed. I would also move the u32 after the struct so it is naturally
> aligned on 64 bit systems.
This is to align with the AMD ACPI implementation.
But I can make this AMD ACPI specific and bring a more generic version here.

Evan
> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, July 13, 2023 7:12 AM
> To: Quan, Evan 
> Cc: raf...@kernel.org; l...@kernel.org; Deucher, Alexander
> ; Koenig, Christian
> ; Pan, Xinhui ;
> airl...@gmail.com; dan...@ffwll.ch; johan...@sipsolutions.net;
> da...@davemloft.net; eduma...@google.com; k...@kernel.org;
> pab...@redhat.com; Limonciello, Mario ;
> mdaen...@redhat.com; maarten.lankho...@linux.intel.com;
> tzimmerm...@suse.de; hdego...@redhat.com; jingyuwang_...@163.com;
> Lazar, Lijo ; jim.cro...@gmail.com;
> bellosili...@gmail.com; andrealm...@igalia.com; t...@redhat.com;
> j...@jsg.id.au; a...@arndb.de; linux-ker...@vger.kernel.org; linux-
> a...@vger.kernel.org; amd-gfx@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; linux-wirel...@vger.kernel.org;
> net...@vger.kernel.org
> Subject: Re: [PATCH V6 1/9] drivers core: Add support for Wifi band RF
> mitigations
>
> > +/**
> > + * wbrf_supported_producer - Determine if the device can report
> frequencies
> > + *
> > + * @dev: device pointer
> > + *
> > + * WBRF is used to mitigate devices that cause harmonic interference.
> > + * This function will determine if this device needs to report such
> frequencies.
>
> How is the WBRF core supposed to answer this question? That it knows
> there is at least one device which has registered with WBRF saying it
> can change its behaviour to avoid causing interference?
>
> Rather than "Determine if the device can report frequencies" should it be
> "Determine if the device should report frequencies"
>
> A WiFi device can always report frequencies, since it knows what
> frequency is it currently using. However, it is pointless making such
> reports if there is no device which can actually make use of the
> information.
>
> > +bool wbrf_supported_producer(struct device *dev)
> > +{
> > +   return true;
> > +}
>
> I found the default implementation of true being odd. It makes me
> wounder, what is the point of this call. I would expect this to see if
> a linked list is empty or not.
>
> > +/**
> > + * wbrf_supported_consumer - Determine if the device can react to
> frequencies
>
> This again seems odd. A device should know if it can react to
> frequencies or not. WBRF core should not need to tell it. What makes
> more sense to me is that this call is about a device telling the WBRF
> core it is able to react to frequencies. The WBRF core then can give a
> good answer to wbrf_supported_producer(), yes, i know of some other
> device who might be able to do something to avoid causing interference
> to you, so please do tell me about frequencies you want to use.
>
> What is missing here in this API is policy information. The WBRF core
> knows it has zero or more devices which can report what frequencies
> they are using, and it has zero or more devices which maybe can do
> something. But then you need policy to say this particular board needs
> any registered devices to actually do something because of poor
> shielding. Should this policy be as simple as a bool, or should it
> actually say the board has shielding issues for a list of frequencies?
> I think the answer to what will depend on the cost of taking action
> when no action is actually required.
>
> > + * wbrf_register_notifier - Register for notifications of frequency changes
> > + *
> > + * @nb: driver notifier block
> > + *
> > + * WBRF is used to mitigate devices that cause harmonic interference.
> > + * 

Re: [bug/bisected] commit a2848d08742c8e8494675892c02c0d22acbe3cf8 cause general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI

2023-07-18 Thread Mikhail Gavrilov
On Tue, Jul 18, 2023 at 7:13 AM Chen, Guchun  wrote:
>
> [Public]
>
> Hello Mike,
>
> I guess this patch can resolve your problem.
> https://patchwork.freedesktop.org/patch/547897/
>
> Regards,
> Guchun
>

Tested-by: Mikhail Gavrilov 
Thanks, the issue was gone with this patch.

I didn't say anything above about how to reproduce this problem.
Case was like this:
On a dual GPU laptop, I ran Google Chrome on a discrete graphics card.
I used for it this command:
$ DRI_PRIME=1 google-chrome-unstable --disable-features=Vulkan

-- 
Best Regards,
Mike Gavrilov.


[PATCH v3] drm/amdgpu: load sdma ucode in the guest machine

2023-07-18 Thread YuanShang
[why]
User mode driver need to check the sdma ucode version to
see whether the sdma engine supports a new type of PM4 packet.
In SRIOV, sdma is loaded by the host. And, there is no way
to check the sdma ucode version of CHIP_NAVI12 and
CHIP_SIENNA_CICHLID of the host in the guest machine.

[how]
Load the sdma ucode for CHIP_NAVI12 and CHIP_SIENNA_CICHLID
in the guest machine.

Signed-off-by: YuanShang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c |  3 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 11 +++
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   |  8 +++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index dacf281d2b21..e2b9392d7f0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -239,9 +239,6 @@ int amdgpu_sdma_init_microcode(struct amdgpu_device *adev,
   sizeof(struct amdgpu_sdma_instance));
}
 
-   if (amdgpu_sriov_vf(adev))
-   return 0;
-
DRM_DEBUG("psp_load == '%s'\n",
  adev->firmware.load_type == AMDGPU_FW_LOAD_PSP ? "true" : 
"false");
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 41aa853a07d2..3365fe04275a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -845,6 +845,17 @@ bool amdgpu_virt_fw_load_skip_check(struct amdgpu_device 
*adev, uint32_t ucode_i
return false;
else
return true;
+   case IP_VERSION(11, 0, 9):
+   case IP_VERSION(11, 0, 7):
+   /* black list for CHIP_NAVI12 and CHIP_SIENNA_CICHLID */
+   if (ucode_id == AMDGPU_UCODE_ID_RLC_G
+   || ucode_id == AMDGPU_UCODE_ID_RLC_RESTORE_LIST_CNTL
+   || ucode_id == AMDGPU_UCODE_ID_RLC_RESTORE_LIST_GPM_MEM
+   || ucode_id == AMDGPU_UCODE_ID_RLC_RESTORE_LIST_SRM_MEM
+   || ucode_id == AMDGPU_UCODE_ID_SMC)
+   return true;
+   else
+   return false;
case IP_VERSION(13, 0, 10):
/* white list */
if (ucode_id == AMDGPU_UCODE_ID_CAP
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 5c4d4df9cf94..1cc34efb455b 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -237,17 +237,15 @@ static void sdma_v5_0_init_golden_registers(struct 
amdgpu_device *adev)
 // emulation only, won't work on real chip
 // navi10 real chip need to use PSP to load firmware
 static int sdma_v5_0_init_microcode(struct amdgpu_device *adev)
-{  int ret, i;
-
-   if (amdgpu_sriov_vf(adev) && (adev->ip_versions[SDMA0_HWIP][0] == 
IP_VERSION(5, 0, 5)))
-   return 0;
+{
+   int ret, i;
 
for (i = 0; i < adev->sdma.num_instances; i++) {
ret = amdgpu_sdma_init_microcode(adev, i, false);
if (ret)
return ret;
}
-   
+
return ret;
 }
 
-- 
2.25.1



Re: [PATCH 2/5] mm: use vma_is_stack() and vma_is_heap()

2023-07-18 Thread Kefeng Wang




On 2023/7/17 18:25, David Hildenbrand wrote:

On 12.07.23 16:38, Kefeng Wang wrote:

Use the helpers to simplify code.

Signed-off-by: Kefeng Wang 
---
  fs/proc/task_mmu.c   | 24 
  fs/proc/task_nommu.c | 15 +--
  2 files changed, 5 insertions(+), 34 deletions(-)



Please squash patch #1 and this patch and call it something like

"mm: factor out VMA stack and heap checks"

And then, maybe also keep the comments in these functions, they sound 
reasonable to have.


Thanks, will re-post them.




[PATCH] drm/amd/display: Allow building DC with clang on RISC-V

2023-07-18 Thread Samuel Holland
clang on RISC-V appears to be unaffected by the bug causing excessive
stack usage in calculate_bandwidth(). clang 16 with -fstack-usage
reports a 304 byte stack frame size with CONFIG_ARCH_RV32I, and 512
bytes with CONFIG_ARCH_RV64I.

Signed-off-by: Samuel Holland 
---

 drivers/gpu/drm/amd/display/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index bf0a655d009e..901d1961b739 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -5,7 +5,7 @@ menu "Display Engine Configuration"
 config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
-   depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
+   depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || 
(ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG))
-- 
2.40.1