Re: [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates
On 2023-08-04 14:20, Hamza Mahfooz wrote:
> We should be checking to see if async flips are supported in
> amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
> async flipping isn't supported if a plane's framebuffer changes memory
> domains during an atomic commit. So, move the check from
> dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
> the memory domain has changed in amdgpu_dm_atomic_check().
>
> Cc: sta...@vger.kernel.org
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2733
> Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast
> updates")
> Signed-off-by: Hamza Mahfooz
Reviewed-by: Harry Wentland
Harry
> ---
> v2: link issue and revert back to the old way of setting update_type.
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ---
> .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 --
> 2 files changed, 21 insertions(+), 15 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 32fb551862b0..1d3afab5bc85 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8086,10 +8086,12 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>* fast updates.
>*/
> if (crtc->state->async_flip &&
> - acrtc_state->update_type != UPDATE_TYPE_FAST)
> + (acrtc_state->update_type != UPDATE_TYPE_FAST ||
> + get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
> drm_warn_once(state->dev,
> "[PLANE:%d:%s] async flip with non-fast
> update\n",
> plane->base.id, plane->name);
> +
> bundle->flip_addrs[planes_count].flip_immediate =
> crtc->state->async_flip &&
> acrtc_state->update_type == UPDATE_TYPE_FAST &&
> @@ -10050,6 +10052,11 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
>
> /* Remove exiting planes if they are modified */
> for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
> new_plane_state, i) {
> + if (old_plane_state->fb && new_plane_state->fb &&
> + get_mem_type(old_plane_state->fb) !=
> + get_mem_type(new_plane_state->fb))
> + lock_and_validation_needed = true;
> +
> ret = dm_update_plane_state(dc, state, plane,
> old_plane_state,
> new_plane_state,
> @@ -10297,9 +10304,20 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> struct dm_crtc_state *dm_new_crtc_state =
> to_dm_crtc_state(new_crtc_state);
>
> + /*
> + * Only allow async flips for fast updates that don't change
> + * the FB pitch, the DCC state, rotation, etc.
> + */
> + if (new_crtc_state->async_flip && lock_and_validation_needed) {
> + drm_dbg_atomic(crtc->dev,
> +"[CRTC:%d:%s] async flips are only
> supported for fast updates\n",
> +crtc->base.id, crtc->name);
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> dm_new_crtc_state->update_type = lock_and_validation_needed ?
> - UPDATE_TYPE_FULL :
> - UPDATE_TYPE_FAST;
> + UPDATE_TYPE_FULL : UPDATE_TYPE_FAST;
> }
>
> /* Must be success */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 30d4c6fd95f5..440fc0869a34 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc
> *crtc,
> return -EINVAL;
> }
>
> - /*
> - * Only allow async flips for fast updates that don't change the FB
> - * pitch, the DCC state, rotation, etc.
> - */
> - if (crtc_state->async_flip &&
> - dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
> - drm_dbg_atomic(crtc->dev,
> -"[CRTC:%d:%s] async flips are only supported for
> fast updates\n",
> -crtc->base.id, crtc->name);
> - return -EINVAL;
> - }
> -
> /* In some use cases, like reset, no stream is attached */
> if (!dm_crtc_state->stream)
> return 0;
[PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates
We should be checking to see if async flips are supported in
amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
async flipping isn't supported if a plane's framebuffer changes memory
domains during an atomic commit. So, move the check from
dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
the memory domain has changed in amdgpu_dm_atomic_check().
Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2733
Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast
updates")
Signed-off-by: Hamza Mahfooz
---
v2: link issue and revert back to the old way of setting update_type.
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ---
.../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 --
2 files changed, 21 insertions(+), 15 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 32fb551862b0..1d3afab5bc85 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8086,10 +8086,12 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
* fast updates.
*/
if (crtc->state->async_flip &&
- acrtc_state->update_type != UPDATE_TYPE_FAST)
+ (acrtc_state->update_type != UPDATE_TYPE_FAST ||
+get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
drm_warn_once(state->dev,
"[PLANE:%d:%s] async flip with non-fast
update\n",
plane->base.id, plane->name);
+
bundle->flip_addrs[planes_count].flip_immediate =
crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST &&
@@ -10050,6 +10052,11 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
/* Remove exiting planes if they are modified */
for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
new_plane_state, i) {
+ if (old_plane_state->fb && new_plane_state->fb &&
+ get_mem_type(old_plane_state->fb) !=
+ get_mem_type(new_plane_state->fb))
+ lock_and_validation_needed = true;
+
ret = dm_update_plane_state(dc, state, plane,
old_plane_state,
new_plane_state,
@@ -10297,9 +10304,20 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(new_crtc_state);
+ /*
+* Only allow async flips for fast updates that don't change
+* the FB pitch, the DCC state, rotation, etc.
+*/
+ if (new_crtc_state->async_flip && lock_and_validation_needed) {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] async flips are only
supported for fast updates\n",
+ crtc->base.id, crtc->name);
+ ret = -EINVAL;
+ goto fail;
+ }
+
dm_new_crtc_state->update_type = lock_and_validation_needed ?
-UPDATE_TYPE_FULL :
-UPDATE_TYPE_FAST;
+ UPDATE_TYPE_FULL : UPDATE_TYPE_FAST;
}
/* Must be success */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 30d4c6fd95f5..440fc0869a34 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc
*crtc,
return -EINVAL;
}
- /*
-* Only allow async flips for fast updates that don't change the FB
-* pitch, the DCC state, rotation, etc.
-*/
- if (crtc_state->async_flip &&
- dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
- drm_dbg_atomic(crtc->dev,
- "[CRTC:%d:%s] async flips are only supported for
fast updates\n",
- crtc->base.id, crtc->name);
- return -EINVAL;
- }
-
/* In some use cases, like reset, no stream is attached */
if (!dm_crtc_state->stream)
return 0;
--
2.41.0
Re: [PATCH] drm/amd/display: ensure async flips are only accepted for fast updates
On 2023-08-04 11:56, Hamza Mahfooz wrote:
> We should be checking to see if async flips are supported in
> amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
> async flipping isn't supported if a plane's framebuffer changes memory
> domains during an atomic commit. So, move the check from
> dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
> the memory domain has changed in amdgpu_dm_atomic_check().
>
> Cc: sta...@vger.kernel.org
> Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast
> updates")
> Tested-by: Marcus Seyfarth
> Signed-off-by: Hamza Mahfooz
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 ---
> .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 -
> 2 files changed, 21 insertions(+), 16 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 32fb551862b0..e561d99b3f40 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8086,7 +8086,8 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>* fast updates.
>*/
> if (crtc->state->async_flip &&
> - acrtc_state->update_type != UPDATE_TYPE_FAST)
> + (acrtc_state->update_type != UPDATE_TYPE_FAST ||
> + get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
> drm_warn_once(state->dev,
> "[PLANE:%d:%s] async flip with non-fast
> update\n",
> plane->base.id, plane->name);
> @@ -10050,12 +10051,18 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
>
> /* Remove exiting planes if they are modified */
> for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
> new_plane_state, i) {
> + if (old_plane_state->fb && new_plane_state->fb &&
> + get_mem_type(old_plane_state->fb) !=
> + get_mem_type(new_plane_state->fb))
> + lock_and_validation_needed = true;
> +
> ret = dm_update_plane_state(dc, state, plane,
> old_plane_state,
> new_plane_state,
> false,
> &lock_and_validation_needed,
> &is_top_most_overlay);
> +
nit: extraneous newline
> if (ret) {
> DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
> goto fail;
> @@ -10069,6 +10076,7 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> new_crtc_state,
> false,
> &lock_and_validation_needed);
> +
nit: extraneous newline
> if (ret) {
> DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state()
> failed\n");
> goto fail;
> @@ -10297,9 +10305,18 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> struct dm_crtc_state *dm_new_crtc_state =
> to_dm_crtc_state(new_crtc_state);
>
> - dm_new_crtc_state->update_type = lock_and_validation_needed ?
> - UPDATE_TYPE_FULL :
> - UPDATE_TYPE_FAST;
> + /*
> + * Only allow async flips for fast updates that don't change
> + * the FB pitch, the DCC state, rotation, etc.
> + */
> + if (new_crtc_state->async_flip && lock_and_validation_needed) {
> + drm_dbg_atomic(crtc->dev,
> +"[CRTC:%d:%s] async flips are only
> supported for fast updates\n",
> +crtc->base.id, crtc->name);
> + ret = -EINVAL;
> + goto fail;
> + } else
nit: Add braces here as well
> + dm_new_crtc_state->update_type = UPDATE_TYPE_FAST;
If async_flip is false you'll be setting update_type to FAST here
uncoditionally.
You'll still need the lock_and_validation check here, i.e. this:
> dm_new_crtc_state->update_type = lock_and_validation_needed ?
>UPDATE_TYPE_FULL :
>UPDATE_TYPE_FAST;
Harry
> }
>
> /* Must be success */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 30d4c6fd95f5..440fc0869a34 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_c
Re: [PATCH] drm/amdkfd: wrap dynamic debug call with CONFIG_DYNAMIC_DEBUG_CORE
I just applied Arnd Bergmann's patch "drm/amdkfd: fix build failure
without CONFIG_DYNAMIC_DEBUG". This patch is no longer needed.
Regards,
Felix
On 2023-08-04 12:05, Alex Sierra wrote:
This causes error compilation if CONFIG_DYNAMIC_DEBUG_CORE is not
defined.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index a69994ff1c2f..cde4cc6afa83 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -824,6 +824,7 @@ svm_range_is_same_attrs(struct kfd_process *p, struct
svm_range *prange,
*
* Context: The caller must hold svms->lock
*/
+#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
static void svm_range_debug_dump(struct svm_range_list *svms)
{
struct interval_tree_node *node;
@@ -851,6 +852,7 @@ static void svm_range_debug_dump(struct svm_range_list
*svms)
node = interval_tree_iter_next(node, 0, ~0ULL);
}
}
+#endif
static void *
svm_range_copy_array(void *psrc, size_t size, uint64_t num_elements,
@@ -3594,7 +3596,9 @@ svm_range_set_attr(struct kfd_process *p, struct
mm_struct *mm,
break;
}
+#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
dynamic_svm_range_dump(svms);
+#endif
mutex_unlock(&svms->lock);
mmap_read_unlock(mm);
Re: [PATCH] drm/amdkfd: fix build failure without CONFIG_DYNAMIC_DEBUG
On 2023-08-04 9:29, Arnd Bergmann wrote:
From: Arnd Bergmann
When CONFIG_DYNAMIC_DEBUG is disabled altogether, calling
_dynamic_func_call_no_desc() does not work:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c: In function
'svm_range_set_attr':
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:52:9: error: implicit
declaration of function '_dynamic_func_call_no_desc'
[-Werror=implicit-function-declaration]
52 | _dynamic_func_call_no_desc("svm_range_dump",
svm_range_debug_dump, svms)
| ^~
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:3564:9: note: in expansion of
macro 'dynamic_svm_range_dump'
3564 | dynamic_svm_range_dump(svms);
| ^~
Add a compile-time conditional in addition to the runtime check.
Fixes: 8923137dbe4b2 ("drm/amdkfd: avoid svm dump when dynamic debug disabled")
Signed-off-by: Arnd Bergmann
The patch is
Reviewed-by: Felix Kuehling
I'm applying it to amd-staging-drm-next.
Thanks,
Felix
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 308384dbc502d..44e710821b6d9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -23,6 +23,7 @@
#include
#include
+#include
#include
#include
@@ -48,8 +49,13 @@
* page table is updated.
*/
#define AMDGPU_SVM_RANGE_RETRY_FAULT_PENDING (2UL * NSEC_PER_MSEC)
+#if IS_ENABLED(CONFIG_DYNAMIC_DEBUG)
#define dynamic_svm_range_dump(svms) \
_dynamic_func_call_no_desc("svm_range_dump", svm_range_debug_dump, svms)
+#else
+#define dynamic_svm_range_dump(svms) \
+ do { if (0) svm_range_debug_dump(svms); } while (0)
+#endif
/* Giant svm range split into smaller ranges based on this, it is decided using
* minimum of all dGPU/APU 1/32 VRAM size, between 2MB to 1GB and alignment to
[PATCH] drm/amdkfd: wrap dynamic debug call with CONFIG_DYNAMIC_DEBUG_CORE
This causes error compilation if CONFIG_DYNAMIC_DEBUG_CORE is not
defined.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index a69994ff1c2f..cde4cc6afa83 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -824,6 +824,7 @@ svm_range_is_same_attrs(struct kfd_process *p, struct
svm_range *prange,
*
* Context: The caller must hold svms->lock
*/
+#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
static void svm_range_debug_dump(struct svm_range_list *svms)
{
struct interval_tree_node *node;
@@ -851,6 +852,7 @@ static void svm_range_debug_dump(struct svm_range_list
*svms)
node = interval_tree_iter_next(node, 0, ~0ULL);
}
}
+#endif
static void *
svm_range_copy_array(void *psrc, size_t size, uint64_t num_elements,
@@ -3594,7 +3596,9 @@ svm_range_set_attr(struct kfd_process *p, struct
mm_struct *mm,
break;
}
+#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
dynamic_svm_range_dump(svms);
+#endif
mutex_unlock(&svms->lock);
mmap_read_unlock(mm);
--
2.32.0
[PATCH] drm/amd/display: ensure async flips are only accepted for fast updates
We should be checking to see if async flips are supported in
amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
async flipping isn't supported if a plane's framebuffer changes memory
domains during an atomic commit. So, move the check from
dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
the memory domain has changed in amdgpu_dm_atomic_check().
Cc: sta...@vger.kernel.org
Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast
updates")
Tested-by: Marcus Seyfarth
Signed-off-by: Hamza Mahfooz
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 ---
.../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 -
2 files changed, 21 insertions(+), 16 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 32fb551862b0..e561d99b3f40 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8086,7 +8086,8 @@ static void amdgpu_dm_commit_planes(struct
drm_atomic_state *state,
* fast updates.
*/
if (crtc->state->async_flip &&
- acrtc_state->update_type != UPDATE_TYPE_FAST)
+ (acrtc_state->update_type != UPDATE_TYPE_FAST ||
+get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
drm_warn_once(state->dev,
"[PLANE:%d:%s] async flip with non-fast
update\n",
plane->base.id, plane->name);
@@ -10050,12 +10051,18 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
/* Remove exiting planes if they are modified */
for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
new_plane_state, i) {
+ if (old_plane_state->fb && new_plane_state->fb &&
+ get_mem_type(old_plane_state->fb) !=
+ get_mem_type(new_plane_state->fb))
+ lock_and_validation_needed = true;
+
ret = dm_update_plane_state(dc, state, plane,
old_plane_state,
new_plane_state,
false,
&lock_and_validation_needed,
&is_top_most_overlay);
+
if (ret) {
DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
goto fail;
@@ -10069,6 +10076,7 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
new_crtc_state,
false,
&lock_and_validation_needed);
+
if (ret) {
DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state()
failed\n");
goto fail;
@@ -10297,9 +10305,18 @@ static int amdgpu_dm_atomic_check(struct drm_device
*dev,
struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(new_crtc_state);
- dm_new_crtc_state->update_type = lock_and_validation_needed ?
-UPDATE_TYPE_FULL :
-UPDATE_TYPE_FAST;
+ /*
+* Only allow async flips for fast updates that don't change
+* the FB pitch, the DCC state, rotation, etc.
+*/
+ if (new_crtc_state->async_flip && lock_and_validation_needed) {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] async flips are only
supported for fast updates\n",
+ crtc->base.id, crtc->name);
+ ret = -EINVAL;
+ goto fail;
+ } else
+ dm_new_crtc_state->update_type = UPDATE_TYPE_FAST;
}
/* Must be success */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 30d4c6fd95f5..440fc0869a34 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc
*crtc,
return -EINVAL;
}
- /*
-* Only allow async flips for fast updates that don't change the FB
-* pitch, the DCC state, rotation, etc.
-*/
- if (crtc_state->async_flip &&
- dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
- drm_dbg_atomic(crtc->dev,
- "[CRTC:%d:%s] async flips are only supported for
fast updates\n",
-
Re: [PATCH 3/3] drm/mst: adjust the function drm_dp_remove_payload_part2()
On Fri, Aug 04, 2023 at 02:20:29PM +0800, Wayne Lin wrote:
> [...]
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index e04f87ff755a..4270178f95f6 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3382,8 +3382,7 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part1);
> * drm_dp_remove_payload_part2() - Remove an MST payload locally
> * @mgr: Manager to use.
> * @mst_state: The MST atomic state
> - * @old_payload: The payload with its old state
> - * @new_payload: The payload with its latest state
> + * @payload: The payload with its latest state
> *
> * Updates the starting time slots of all other payloads which would have
> been shifted towards
> * the start of the payload ID table as a result of removing a payload.
> Driver should call this
> @@ -3392,25 +3391,36 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part1);
> */
> void drm_dp_remove_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
>struct drm_dp_mst_topology_state *mst_state,
> - const struct drm_dp_mst_atomic_payload
> *old_payload,
> - struct drm_dp_mst_atomic_payload *new_payload)
> + struct drm_dp_mst_atomic_payload *payload)
> {
> struct drm_dp_mst_atomic_payload *pos;
> + u8 time_slots_to_remove;
> + u8 next_payload_vc_start = mgr->next_start_slot;
> +
> + /* Find the current allocated time slot number of the payload */
> + list_for_each_entry(pos, &mst_state->payloads, next) {
> + if (pos != payload &&
> + pos->vc_start_slot > payload->vc_start_slot &&
> + pos->vc_start_slot < next_payload_vc_start)
> + next_payload_vc_start = pos->vc_start_slot;
> + }
> +
> + time_slots_to_remove = next_payload_vc_start - payload->vc_start_slot;
Imo, the intuitive way would be to pass the old payload state to this
function - which already contains the required time_slots param - and
refactor things instead moving vc_start_slot from the payload state to
mgr suggested by Ville earlier.
--Imre
> /* Remove local payload allocation */
> list_for_each_entry(pos, &mst_state->payloads, next) {
> - if (pos != new_payload && pos->vc_start_slot >
> new_payload->vc_start_slot)
> - pos->vc_start_slot -= old_payload->time_slots;
> + if (pos != payload && pos->vc_start_slot >
> payload->vc_start_slot)
> + pos->vc_start_slot -= time_slots_to_remove;
> }
> - new_payload->vc_start_slot = -1;
> + payload->vc_start_slot = -1;
>
> mgr->payload_count--;
> - mgr->next_start_slot -= old_payload->time_slots;
> + mgr->next_start_slot -= time_slots_to_remove;
>
> - if (new_payload->delete)
> - drm_dp_mst_put_port_malloc(new_payload->port);
> + if (payload->delete)
> + drm_dp_mst_put_port_malloc(payload->port);
>
> - new_payload->payload_allocation_status =
> DRM_DP_MST_PAYLOAD_ALLOCATION_NONE;
> + payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_NONE;
> }
Re: [PATCH] drm/amd/pm: avoid driver getting empty metrics table for the first time
[AMD Official Use Only - General]
Missed one thing - please replace msleep(1) with usleep_range.
Thanks,
Lijo
From: amd-gfx on behalf of Lazar, Lijo
Sent: Friday, August 4, 2023 8:07:11 PM
To: Wang, Yang(Kevin) ; amd-gfx@lists.freedesktop.org
Cc: Kamal, Asad
Subject: Re: [PATCH] drm/amd/pm: avoid driver getting empty metrics table for
the first time
On 8/4/2023 8:02 PM, Yang Wang wrote:
> From: Yang Wang
>
> add metrics.AccumulationCouter check to avoid driver getting an empty
> metrics data since metrics table not updated completely in pmfw side.
>
> Signed-off-by: Yang Wang
> Reviewed-by: Asad Kamal
> Tested-by: Asad Kamal
Reviewed-by: Lijo Lazar
Thanks,
Lijo
> ---
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 20 ++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 6253ad13833c..5adc6b92bc49 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -325,14 +325,24 @@ static int smu_v13_0_6_setup_driver_pptable(struct
> smu_context *smu)
>MetricsTable_t *metrics = (MetricsTable_t *)smu_table->metrics_table;
>struct PPTable_t *pptable =
>(struct PPTable_t *)smu_table->driver_pptable;
> - int ret;
> - int i;
> + int ret, i, retry = 100;
>
>/* Store one-time values in driver PPTable */
>if (!pptable->Init) {
> - ret = smu_v13_0_6_get_metrics_table(smu, NULL, false);
> - if (ret)
> - return ret;
> + while (retry--) {
> + ret = smu_v13_0_6_get_metrics_table(smu, NULL, true);
> + if (ret)
> + return ret;
> +
> + /* Ensure that metrics have been updated */
> + if (metrics->AccumulationCounter)
> + break;
> +
> + msleep(1);
> + }
> +
> + if (!retry)
> + return -ETIME;
>
>pptable->MaxSocketPowerLimit =
>SMUQ10_TO_UINT(metrics->MaxSocketPowerLimit);
Re: [PATCH 2/3] drm/mst: Refactor the flow for payload allocation/removement
Hi Wayne, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm-intel/for-linux-next-fixes drm/drm-next linus/master v6.5-rc4 next-20230804] [cannot apply to drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Wayne-Lin/drm-mst-delete-unnecessary-case-in-drm_dp_add_payload_part2/20230804-142405 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230804062029.5686-3-Wayne.Lin%40amd.com patch subject: [PATCH 2/3] drm/mst: Refactor the flow for payload allocation/removement config: s390-randconfig-r044-20230731 (https://download.01.org/0day-ci/archive/20230804/202308042253.lm5xmnna-...@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce: (https://download.01.org/0day-ci/archive/20230804/202308042253.lm5xmnna-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202308042253.lm5xmnna-...@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/nouveau/dispnv50/disp.c:24: In file included from drivers/gpu/drm/nouveau/dispnv50/disp.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/mem.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/mmu.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/object.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8: In file included from include/linux/pci.h:39: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 547 | val = __raw_readb(PCI_IOBASE + addr); | ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); | ~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' 37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) | ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' 102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) | ^ In file included from drivers/gpu/drm/nouveau/dispnv50/disp.c:24: In file included from drivers/gpu/drm/nouveau/dispnv50/disp.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/mem.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/mmu.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/object.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8: In file included from include/linux/pci.h:39: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); | ~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' 35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) | ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' 115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) | ^ In file included from drivers/gpu/drm/nouveau/dispnv50/disp.c:24: In file included from drivers/gpu/drm/nouveau/dispnv50/disp.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/mem.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/mmu.h:3: In file included from drivers/gpu/drm/nouveau/include/nvif/object.h:4: In file included from drivers/gpu/drm/nouveau/include/nvif/os.h:8: In file included from include/linux/pci.h:39: In file included from i
Re: [PATCH] drm/amd/pm: avoid driver getting empty metrics table for the first time
On 8/4/2023 8:02 PM, Yang Wang wrote:
From: Yang Wang
add metrics.AccumulationCouter check to avoid driver getting an empty
metrics data since metrics table not updated completely in pmfw side.
Signed-off-by: Yang Wang
Reviewed-by: Asad Kamal
Tested-by: Asad Kamal
Reviewed-by: Lijo Lazar
Thanks,
Lijo
---
.../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 20 ++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 6253ad13833c..5adc6b92bc49 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -325,14 +325,24 @@ static int smu_v13_0_6_setup_driver_pptable(struct
smu_context *smu)
MetricsTable_t *metrics = (MetricsTable_t *)smu_table->metrics_table;
struct PPTable_t *pptable =
(struct PPTable_t *)smu_table->driver_pptable;
- int ret;
- int i;
+ int ret, i, retry = 100;
/* Store one-time values in driver PPTable */
if (!pptable->Init) {
- ret = smu_v13_0_6_get_metrics_table(smu, NULL, false);
- if (ret)
- return ret;
+ while (retry--) {
+ ret = smu_v13_0_6_get_metrics_table(smu, NULL, true);
+ if (ret)
+ return ret;
+
+ /* Ensure that metrics have been updated */
+ if (metrics->AccumulationCounter)
+ break;
+
+ msleep(1);
+ }
+
+ if (!retry)
+ return -ETIME;
pptable->MaxSocketPowerLimit =
SMUQ10_TO_UINT(metrics->MaxSocketPowerLimit);
[PATCH] drm/amd/pm: avoid driver getting empty metrics table for the first time
From: Yang Wang
add metrics.AccumulationCouter check to avoid driver getting an empty
metrics data since metrics table not updated completely in pmfw side.
Signed-off-by: Yang Wang
Reviewed-by: Asad Kamal
Tested-by: Asad Kamal
---
.../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 20 ++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 6253ad13833c..5adc6b92bc49 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -325,14 +325,24 @@ static int smu_v13_0_6_setup_driver_pptable(struct
smu_context *smu)
MetricsTable_t *metrics = (MetricsTable_t *)smu_table->metrics_table;
struct PPTable_t *pptable =
(struct PPTable_t *)smu_table->driver_pptable;
- int ret;
- int i;
+ int ret, i, retry = 100;
/* Store one-time values in driver PPTable */
if (!pptable->Init) {
- ret = smu_v13_0_6_get_metrics_table(smu, NULL, false);
- if (ret)
- return ret;
+ while (retry--) {
+ ret = smu_v13_0_6_get_metrics_table(smu, NULL, true);
+ if (ret)
+ return ret;
+
+ /* Ensure that metrics have been updated */
+ if (metrics->AccumulationCounter)
+ break;
+
+ msleep(1);
+ }
+
+ if (!retry)
+ return -ETIME;
pptable->MaxSocketPowerLimit =
SMUQ10_TO_UINT(metrics->MaxSocketPowerLimit);
--
2.34.1
[PATCH] drm/amdkfd: fix build failure without CONFIG_DYNAMIC_DEBUG
From: Arnd Bergmann
When CONFIG_DYNAMIC_DEBUG is disabled altogether, calling
_dynamic_func_call_no_desc() does not work:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c: In function
'svm_range_set_attr':
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:52:9: error: implicit
declaration of function '_dynamic_func_call_no_desc'
[-Werror=implicit-function-declaration]
52 | _dynamic_func_call_no_desc("svm_range_dump",
svm_range_debug_dump, svms)
| ^~
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_svm.c:3564:9: note: in expansion of
macro 'dynamic_svm_range_dump'
3564 | dynamic_svm_range_dump(svms);
| ^~
Add a compile-time conditional in addition to the runtime check.
Fixes: 8923137dbe4b2 ("drm/amdkfd: avoid svm dump when dynamic debug disabled")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 308384dbc502d..44e710821b6d9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -23,6 +23,7 @@
#include
#include
+#include
#include
#include
@@ -48,8 +49,13 @@
* page table is updated.
*/
#define AMDGPU_SVM_RANGE_RETRY_FAULT_PENDING (2UL * NSEC_PER_MSEC)
+#if IS_ENABLED(CONFIG_DYNAMIC_DEBUG)
#define dynamic_svm_range_dump(svms) \
_dynamic_func_call_no_desc("svm_range_dump", svm_range_debug_dump, svms)
+#else
+#define dynamic_svm_range_dump(svms) \
+ do { if (0) svm_range_debug_dump(svms); } while (0)
+#endif
/* Giant svm range split into smaller ranges based on this, it is decided using
* minimum of all dGPU/APU 1/32 VRAM size, between 2MB to 1GB and alignment to
--
2.39.2
[PATCH] drm/amdgpu: Add FRU sysfs nodes only if needed
Create sysfs nodes for FRU data only if FRU data is available. Move the
logic to FRU specific file.
Signed-off-by: Lijo Lazar
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 70 +---
.../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 81 +++
.../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.h| 1 +
3 files changed, 84 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 0d602abd32ba..37bc6ebb7fad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -164,71 +164,6 @@ static DEVICE_ATTR(pcie_replay_count, 0444,
static void amdgpu_device_get_pcie_info(struct amdgpu_device *adev);
-/**
- * DOC: product_name
- *
- * The amdgpu driver provides a sysfs API for reporting the product name
- * for the device
- * The file product_name is used for this and returns the product name
- * as returned from the FRU.
- * NOTE: This is only available for certain server cards
- */
-
-static ssize_t amdgpu_device_get_product_name(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct drm_device *ddev = dev_get_drvdata(dev);
- struct amdgpu_device *adev = drm_to_adev(ddev);
-
- return sysfs_emit(buf, "%s\n", adev->product_name);
-}
-
-static DEVICE_ATTR(product_name, 0444,
- amdgpu_device_get_product_name, NULL);
-
-/**
- * DOC: product_number
- *
- * The amdgpu driver provides a sysfs API for reporting the part number
- * for the device
- * The file product_number is used for this and returns the part number
- * as returned from the FRU.
- * NOTE: This is only available for certain server cards
- */
-
-static ssize_t amdgpu_device_get_product_number(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct drm_device *ddev = dev_get_drvdata(dev);
- struct amdgpu_device *adev = drm_to_adev(ddev);
-
- return sysfs_emit(buf, "%s\n", adev->product_number);
-}
-
-static DEVICE_ATTR(product_number, 0444,
- amdgpu_device_get_product_number, NULL);
-
-/**
- * DOC: serial_number
- *
- * The amdgpu driver provides a sysfs API for reporting the serial number
- * for the device
- * The file serial_number is used for this and returns the serial number
- * as returned from the FRU.
- * NOTE: This is only available for certain server cards
- */
-
-static ssize_t amdgpu_device_get_serial_number(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct drm_device *ddev = dev_get_drvdata(dev);
- struct amdgpu_device *adev = drm_to_adev(ddev);
-
- return sysfs_emit(buf, "%s\n", adev->serial);
-}
-
-static DEVICE_ATTR(serial_number, 0444,
- amdgpu_device_get_serial_number, NULL);
/**
* amdgpu_device_supports_px - Is the device a dGPU with ATPX power control
@@ -3550,9 +3485,6 @@ static void amdgpu_device_check_iommu_direct_map(struct
amdgpu_device *adev)
}
static const struct attribute *amdgpu_dev_attributes[] = {
- &dev_attr_product_name.attr,
- &dev_attr_product_number.attr,
- &dev_attr_serial_number.attr,
&dev_attr_pcie_replay_count.attr,
NULL
};
@@ -3967,6 +3899,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
if (r)
dev_err(adev->dev, "Could not create amdgpu device attr\n");
+ amdgpu_fru_sysfs_init(adev);
+
if (IS_ENABLED(CONFIG_PERF_EVENTS))
r = amdgpu_pmu_init(adev);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
index 8c3ee042556a..bb1cc6830a12 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
@@ -212,3 +212,84 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
kfree(pia);
return 0;
}
+
+/**
+ * DOC: product_name
+ *
+ * The amdgpu driver provides a sysfs API for reporting the product name
+ * for the device
+ * The file product_name is used for this and returns the product name
+ * as returned from the FRU.
+ * NOTE: This is only available for certain server cards
+ */
+
+static ssize_t amdgpu_fru_get_product_name(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct amdgpu_device *adev = drm_to_adev(ddev);
+
+ return sysfs_emit(buf, "%s\n", adev->product_name);
+}
+
+static DEVICE_ATTR(product_name, 0444, amdgpu_fru_get_product_name, NULL);
+
+/**
+ * DOC: product_number
+ *
+ * The amdgpu driver provides a sysfs API for reporting the part number
+ * for the device
+ * The file product_number is used for this and returns the part number
+ * as returned from the FRU.
+ * NOTE: This is only available for certain server cards
+
Re: [PATCH v5 1/1] drm/doc: Document DRM device reset expectations
On Tue, Jun 27, 2023 at 10:23:23AM -0300, André Almeida wrote: > Create a section that specifies how to deal with DRM device resets for > kernel and userspace drivers. > > Acked-by: Pekka Paalanen > Signed-off-by: André Almeida > --- > > v4: > https://lore.kernel.org/lkml/20230626183347.55118-1-andrealm...@igalia.com/ > > Changes: > - Grammar fixes (Randy) > > Documentation/gpu/drm-uapi.rst | 68 ++ > 1 file changed, 68 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 65fb3036a580..3cbffa25ed93 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -285,6 +285,74 @@ for GPU1 and GPU2 from different vendors, and a third > handler for > mmapped regular files. Threads cause additional pain with signal > handling as well. > > +Device reset > + > + > +The GPU stack is really complex and is prone to errors, from hardware bugs, > +faulty applications and everything in between the many layers. Some errors > +require resetting the device in order to make the device usable again. This > +sections describes the expectations for DRM and usermode drivers when a > +device resets and how to propagate the reset status. > + > +Kernel Mode Driver > +-- > + > +The KMD is responsible for checking if the device needs a reset, and to > perform > +it as needed. Usually a hang is detected when a job gets stuck executing. KMD > +should keep track of resets, because userspace can query any time about the > +reset stats for an specific context. This is needed to propagate to the rest > of > +the stack that a reset has happened. Currently, this is implemented by each > +driver separately, with no common DRM interface. > + > +User Mode Driver > + > + > +The UMD should check before submitting new commands to the KMD if the device > has > +been reset, and this can be checked more often if the UMD requires it. After > +detecting a reset, UMD will then proceed to report it to the application > using > +the appropriate API error code, as explained in the section below about > +robustness. > + > +Robustness > +-- > + > +The only way to try to keep an application working after a reset is if it > +complies with the robustness aspects of the graphical API that it is using. > + > +Graphical APIs provide ways to applications to deal with device resets. > However, > +there is no guarantee that the app will use such features correctly, and the > +UMD can implement policies to close the app if it is a repeating offender, Not sure whether this one here is due to my input, but s/UMD/KMD. Repeat offender killing is more a policy where the kernel enforces policy, and no longer up to userspace to dtrt (because very clearly userspace is not really doing the right thing anymore when it's just hanging the gpu in an endless loop). Also maybe tune it down further to something like "the kernel driver may implemnent ..." In my opinion the umd shouldn't implement these kind of magic guesses, the entire point of robustness apis is to delegate responsibility for correctly recovering to the application. And the kernel is left with enforcing fair resource usage policies (which eventually might be a cgroups limit on how much gpu time you're allowed to waste with gpu resets). > +likely in a broken loop. This is done to ensure that it does not keep > blocking > +the user interface from being correctly displayed. This should be done even > if > +the app is correct but happens to trigger some bug in the hardware/driver. > + > +OpenGL > +~~ > + > +Apps using OpenGL should use the available robust interfaces, like the > +extension ``GL_ARB_robustness`` (or ``GL_EXT_robustness`` for OpenGL ES). > This > +interface tells if a reset has happened, and if so, all the context state is > +considered lost and the app proceeds by creating new ones. If it is possible > to > +determine that robustness is not in use, the UMD will terminate the app when > a > +reset is detected, giving that the contexts are lost and the app won't be > able > +to figure this out and recreate the contexts. > + > +Vulkan > +~~ > + > +Apps using Vulkan should check for ``VK_ERROR_DEVICE_LOST`` for submissions. > +This error code means, among other things, that a device reset has happened > and > +it needs to recreate the contexts to keep going. > + > +Reporting causes of resets > +-- > + > +Apart from propagating the reset through the stack so apps can recover, it's > +really useful for driver developers to learn more about what caused the > reset in > +first place. DRM devices should make use of devcoredump to store relevant > +information about the reset, so this information can be added to user bug > +reports. Since we do not seem to have a solid consensus in the community about non-robust userspace, maybe we could just document that lack of consensus to unblock this pa
Re: [PATCH] drm/amd/pm: update smu_v13_0_6 message vf flag
On 8/4/2023 3:51 PM, Yang Wang wrote:
v1:
Enable following message in vf mode.
- PPSMC_MSG_GetMinGfxclkFreqquency
- PPSMC_MSG_GetMaxGfxclkFreqquency
- PPSMC_MSG_GetMinDpmFreq
- PPSMC_MSG_GetMaxDpmFreq
these message will cause pp_dpm_* device node not work properly.
v2:
the following message is disabled in VF mode. (since pmfw 85.69.0)
- PPSMC_MSG_EnableAllSmuFeatures
Signed-off-by: Yang Wang
Acked-by: Hawking Zhang
Reviewed-by: Lijo Lazar
Thanks,
Lijo
---
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 00eba3f950c6..c53c370d2a3f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -89,8 +89,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(TestMessage, PPSMC_MSG_TestMessage,
0),
MSG_MAP(GetSmuVersion, PPSMC_MSG_GetSmuVersion,
1),
MSG_MAP(GetDriverIfVersion,
PPSMC_MSG_GetDriverIfVersion, 1),
- MSG_MAP(EnableAllSmuFeatures,
PPSMC_MSG_EnableAllSmuFeatures,1),
- MSG_MAP(DisableAllSmuFeatures,
PPSMC_MSG_DisableAllSmuFeatures, 1),
+ MSG_MAP(EnableAllSmuFeatures,
PPSMC_MSG_EnableAllSmuFeatures,0),
+ MSG_MAP(DisableAllSmuFeatures,
PPSMC_MSG_DisableAllSmuFeatures, 0),
MSG_MAP(RequestI2cTransaction,
PPSMC_MSG_RequestI2cTransaction, 0),
MSG_MAP(GetMetricsTable, PPSMC_MSG_GetMetricsTable,
1),
MSG_MAP(GetEnabledSmuFeaturesHigh,
PPSMC_MSG_GetEnabledSmuFeaturesHigh, 1),
@@ -101,8 +101,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(SetToolsDramAddrLow,
PPSMC_MSG_SetToolsDramAddrLow, 0),
MSG_MAP(SetSoftMinByFreq,
PPSMC_MSG_SetSoftMinByFreq,0),
MSG_MAP(SetSoftMaxByFreq,
PPSMC_MSG_SetSoftMaxByFreq,0),
- MSG_MAP(GetMinDpmFreq, PPSMC_MSG_GetMinDpmFreq,
0),
- MSG_MAP(GetMaxDpmFreq, PPSMC_MSG_GetMaxDpmFreq,
0),
+ MSG_MAP(GetMinDpmFreq, PPSMC_MSG_GetMinDpmFreq,
1),
+ MSG_MAP(GetMaxDpmFreq, PPSMC_MSG_GetMaxDpmFreq,
1),
MSG_MAP(GetDpmFreqByIndex,
PPSMC_MSG_GetDpmFreqByIndex, 1),
MSG_MAP(SetPptLimit, PPSMC_MSG_SetPptLimit,
0),
MSG_MAP(GetPptLimit, PPSMC_MSG_GetPptLimit,
1),
@@ -121,8 +121,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(EnableDeterminism,
PPSMC_MSG_EnableDeterminism, 0),
MSG_MAP(DisableDeterminism,
PPSMC_MSG_DisableDeterminism, 0),
MSG_MAP(GfxDriverResetRecovery,
PPSMC_MSG_GfxDriverResetRecovery, 0),
- MSG_MAP(GetMinGfxclkFrequency,
PPSMC_MSG_GetMinGfxDpmFreq,0),
- MSG_MAP(GetMaxGfxclkFrequency,
PPSMC_MSG_GetMaxGfxDpmFreq,0),
+ MSG_MAP(GetMinGfxclkFrequency,
PPSMC_MSG_GetMinGfxDpmFreq,1),
+ MSG_MAP(GetMaxGfxclkFrequency,
PPSMC_MSG_GetMaxGfxDpmFreq,1),
MSG_MAP(SetSoftMinGfxclk,
PPSMC_MSG_SetSoftMinGfxClk,0),
MSG_MAP(SetSoftMaxGfxClk,
PPSMC_MSG_SetSoftMaxGfxClk,0),
MSG_MAP(PrepareMp1ForUnload,
PPSMC_MSG_PrepareForDriverUnload, 0),
@@ -1386,6 +1386,9 @@ static int smu_v13_0_6_system_features_control(struct
smu_context *smu,
struct amdgpu_device *adev = smu->adev;
int ret = 0;
+ if (amdgpu_sriov_vf(adev))
+ return 0;
+
if (enable) {
if (!(adev->flags & AMD_IS_APU))
ret = smu_v13_0_system_features_control(smu, enable);
[PATCH] drm/amd/pm: update smu_v13_0_6 message vf flag
v1:
Enable following message in vf mode.
- PPSMC_MSG_GetMinGfxclkFreqquency
- PPSMC_MSG_GetMaxGfxclkFreqquency
- PPSMC_MSG_GetMinDpmFreq
- PPSMC_MSG_GetMaxDpmFreq
these message will cause pp_dpm_* device node not work properly.
v2:
the following message is disabled in VF mode. (since pmfw 85.69.0)
- PPSMC_MSG_EnableAllSmuFeatures
Signed-off-by: Yang Wang
Acked-by: Hawking Zhang
---
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 00eba3f950c6..c53c370d2a3f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -89,8 +89,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(TestMessage, PPSMC_MSG_TestMessage,
0),
MSG_MAP(GetSmuVersion, PPSMC_MSG_GetSmuVersion,
1),
MSG_MAP(GetDriverIfVersion,
PPSMC_MSG_GetDriverIfVersion, 1),
- MSG_MAP(EnableAllSmuFeatures,
PPSMC_MSG_EnableAllSmuFeatures,1),
- MSG_MAP(DisableAllSmuFeatures,
PPSMC_MSG_DisableAllSmuFeatures, 1),
+ MSG_MAP(EnableAllSmuFeatures,
PPSMC_MSG_EnableAllSmuFeatures,0),
+ MSG_MAP(DisableAllSmuFeatures,
PPSMC_MSG_DisableAllSmuFeatures, 0),
MSG_MAP(RequestI2cTransaction,
PPSMC_MSG_RequestI2cTransaction, 0),
MSG_MAP(GetMetricsTable, PPSMC_MSG_GetMetricsTable,
1),
MSG_MAP(GetEnabledSmuFeaturesHigh,
PPSMC_MSG_GetEnabledSmuFeaturesHigh, 1),
@@ -101,8 +101,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(SetToolsDramAddrLow,
PPSMC_MSG_SetToolsDramAddrLow, 0),
MSG_MAP(SetSoftMinByFreq,
PPSMC_MSG_SetSoftMinByFreq,0),
MSG_MAP(SetSoftMaxByFreq,
PPSMC_MSG_SetSoftMaxByFreq,0),
- MSG_MAP(GetMinDpmFreq, PPSMC_MSG_GetMinDpmFreq,
0),
- MSG_MAP(GetMaxDpmFreq, PPSMC_MSG_GetMaxDpmFreq,
0),
+ MSG_MAP(GetMinDpmFreq, PPSMC_MSG_GetMinDpmFreq,
1),
+ MSG_MAP(GetMaxDpmFreq, PPSMC_MSG_GetMaxDpmFreq,
1),
MSG_MAP(GetDpmFreqByIndex,
PPSMC_MSG_GetDpmFreqByIndex, 1),
MSG_MAP(SetPptLimit, PPSMC_MSG_SetPptLimit,
0),
MSG_MAP(GetPptLimit, PPSMC_MSG_GetPptLimit,
1),
@@ -121,8 +121,8 @@ static const struct cmn2asic_msg_mapping
smu_v13_0_6_message_map[SMU_MSG_MAX_COU
MSG_MAP(EnableDeterminism,
PPSMC_MSG_EnableDeterminism, 0),
MSG_MAP(DisableDeterminism,
PPSMC_MSG_DisableDeterminism, 0),
MSG_MAP(GfxDriverResetRecovery,
PPSMC_MSG_GfxDriverResetRecovery, 0),
- MSG_MAP(GetMinGfxclkFrequency,
PPSMC_MSG_GetMinGfxDpmFreq,0),
- MSG_MAP(GetMaxGfxclkFrequency,
PPSMC_MSG_GetMaxGfxDpmFreq,0),
+ MSG_MAP(GetMinGfxclkFrequency,
PPSMC_MSG_GetMinGfxDpmFreq,1),
+ MSG_MAP(GetMaxGfxclkFrequency,
PPSMC_MSG_GetMaxGfxDpmFreq,1),
MSG_MAP(SetSoftMinGfxclk,
PPSMC_MSG_SetSoftMinGfxClk,0),
MSG_MAP(SetSoftMaxGfxClk,
PPSMC_MSG_SetSoftMaxGfxClk,0),
MSG_MAP(PrepareMp1ForUnload,
PPSMC_MSG_PrepareForDriverUnload, 0),
@@ -1386,6 +1386,9 @@ static int smu_v13_0_6_system_features_control(struct
smu_context *smu,
struct amdgpu_device *adev = smu->adev;
int ret = 0;
+ if (amdgpu_sriov_vf(adev))
+ return 0;
+
if (enable) {
if (!(adev->flags & AMD_IS_APU))
ret = smu_v13_0_system_features_control(smu, enable);
--
2.34.1
Re: [PATCH 2/3] drm/mst: Refactor the flow for payload allocation/removement
Hi Wayne,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm-intel/for-linux-next-fixes drm/drm-next
linus/master v6.5-rc4 next-20230804]
[cannot apply to drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Wayne-Lin/drm-mst-delete-unnecessary-case-in-drm_dp_add_payload_part2/20230804-142405
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20230804062029.5686-3-Wayne.Lin%40amd.com
patch subject: [PATCH 2/3] drm/mst: Refactor the flow for payload
allocation/removement
config: microblaze-randconfig-r021-20230731
(https://download.01.org/0day-ci/archive/20230804/202308041635.wkgwwx5r-...@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 12.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20230804/202308041635.wkgwwx5r-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202308041635.wkgwwx5r-...@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_msto_prepare':
>> drivers/gpu/drm/nouveau/dispnv50/disp.c:919:53: warning: variable
>> 'old_payload' set but not used [-Wunused-but-set-variable]
919 | struct drm_dp_mst_atomic_payload *payload, *old_payload;
| ^~~
vim +/old_payload +919 drivers/gpu/drm/nouveau/dispnv50/disp.c
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
908
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
909 static void
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
910 nv50_msto_prepare(struct drm_atomic_state *state,
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
911 struct drm_dp_mst_topology_state *mst_state,
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
912 struct drm_dp_mst_topology_mgr *mgr,
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
913 struct nv50_msto *msto)
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
914 {
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
915 struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
916 struct nv50_mstc *mstc = msto->mstc;
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
917 struct nv50_mstm *mstm = mstc->mstm;
8fb3e25c3dd1a2 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2023-06-13
918 struct drm_dp_mst_topology_state *old_mst_state;
8fb3e25c3dd1a2 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2023-06-13
@919 struct drm_dp_mst_atomic_payload *payload, *old_payload;
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
920
f479c0ba4a170a drivers/gpu/drm/nouveau/nv50_display.c Ben Skeggs 2016-11-04
921 NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
922
8fb3e25c3dd1a2 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2023-06-13
923 old_mst_state = drm_atomic_get_old_mst_topology_state(state, mgr);
8fb3e25c3dd1a2 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2023-06-13
924
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
925 payload = drm_atomic_get_mst_payload_state(mst_state, mstc->port);
8fb3e25c3dd1a2 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2023-06-13
926 old_payload = drm_atomic_get_mst_payload_state(old_mst_state,
mstc->port);
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
927
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
928 // TODO: Figure out if we want to do a better job of handling VCPI
allocation failures here?
4d07b0bc403403 drivers/gpu/drm/nouveau/dispnv50/disp.c Lyude Paul 2022-08-17
929 if (msto->disabled) {
c9e8c7f37133c0 drivers/gpu/drm/nouveau/dispnv50/disp.c Wayne Lin 2023-08-04
930 drm_dp_remove_payload_part1(mgr, mst_state, payload);
8c7d980da9ba3e drivers/gpu/drm/nouveau/dispnv50/disp.c Ben Skeggs 2022-06-01
931
8c7d980da9ba3e drivers/gpu/drm/nouveau/dispnv50/disp.c Ben Skegg
Re: [pull] amdgpu, amdkfd, radeon drm-next-6.6
On Fri, Jul 28, 2023 at 05:42:28PM -0400, Alex Deucher wrote: > Hi Dave, Daniel, > > New stuff for 6.6. > > The following changes since commit 6725f33228077902ddac2a05e0ab361dee36e4ba: > > Merge tag 'drm-misc-next-fixes-2023-07-06' of > git://anongit.freedesktop.org/drm/drm-misc into drm-next (2023-07-07 11:05:16 > +1000) > > are available in the Git repository at: > > https://gitlab.freedesktop.org/agd5f/linux.git > tags/amd-drm-next-6.6-2023-07-28 > > for you to fetch changes up to 7ea1db28119e237d634c6f74ba52056939c009ad: > > drm/radeon: Prefer strscpy over strlcpy calls in radeon_atombios.c > (2023-07-27 15:05:32 -0400) Pulled to drm-next. There were a few conflicts, I've put notes about how I resolved them into the merge conflict. Please double-check that it's all good before I push more stuff on top. Cheers, Sima > > > amd-drm-next-6.6-2023-07-28: > > amdgpu: > - Lots of checkpatch cleanups > - GFX 9.4.3 updates > - Add USB PD and IFWI flashing documentation > - GPUVM updates > - RAS fixes > - DRR fixes > - FAMS fixes > - Virtual display fixes > - Soft IH fixes > - SMU13 fixes > - Rework PSP firmware loading for other IPs > - Kernel doc fixes > - DCN 3.0.1 fixes > - LTTPR fixes > - DP MST fixes > - DCN 3.1.6 fixes > - SubVP fixes > - Display bandwidth calculation fixes > - VCN4 secure submission fixes > - Allow building DC on RISC-V > - Add visible FB info to bo_print_info > - HBR3 fixes > - Add PSP 14.0 support > - GFX9 MCBP fix > - GMC10 vmhub index fix > - GMC11 vmhub index fix > - Create a new doorbell manager > - SR-IOV fixes > > amdkfd: > - Cleanup CRIU dma-buf handling > - Use KIQ to unmap HIQ > - GFX 9.4.3 debugger updates > - GFX 9.4.2 debugger fixes > - Enable cooperative groups fof gfx11 > - SVM fixes > > radeon: > - Lots of checkpatch cleanups > > > Alan Liu (2): > drm/amd/display: Hardcode vco_freq for dcn316 > drm/amd/display: Fix race condition when turning off an output alone > > Alex Deucher (5): > drm/amdgpu: return an error if query_video_caps is not set > drm/amdgpu/gfx9: move update_spm_vmid() out of rlc_init() > drm/amdgpu/gfx10: move update_spm_vmid() out of rlc_init() > drm/amdgpu/pm: make gfxclock consistent for sienna cichlid > drm/amdgpu/pm: make mclk consistent for smu 13.0.7 > > Alex Sierra (1): > drm/amdkfd: avoid svm dump when dynamic debug disabled > > Alvin Lee (3): > drm/amd/display: Update SW cursor fallback for subvp high refresh > drm/amd/display: Add missing triggers for full updates > drm/amd/display: Don't apply FIFO resync W/A if rdivider = 0 > > Anthony Koo (1): > drm/amd/display: Rearrange dmub_cmd defs order > > Aric Cyr (3): > drm/amd/display: Promote DAL to 3.2.242 > drm/amd/display: Promote DAL to 3.2.243 > drm/amd/display: 3.2.244 > > Arnd Bergmann (1): > drm/amdgpu: avoid integer overflow warning in > amdgpu_device_resize_fb_bar() > > Aurabindo Pillai (3): > drm/amd/display: export some optc function for reuse > drm/amd/display: add DCN301 specific logic for OTG programming > drm/amd/display: remove an unused file > > Bob Zhou (1): > drm/amdgpu: remove repeat code for mes_add_queue_pkt > > Candice Li (1): > drm/amdgpu: Allow the initramfs generator to include psp_13_0_6_ta > > Cruise Hung (1): > drm/amd/display: Add helpers to get DMUB FW boot options > > Dan Carpenter (1): > drm/amd/display: Unlock on error path in > dm_handle_mst_sideband_msg_ready_event() > > Daniel Miess (3): > drm/amd/display: Reenable all root clock gating options > drm/amd/display: Fix DP2 link training failure with RCO > drm/amd/display: Prevent vtotal from being set to 0 > > Eric Huang (2): > drm/amdkfd: add kfd2kgd debugger callbacks for GC v9.4.3 > drm/amdgpu: enable trap of each kfd vmid for gfx v9.4.3 > > Ethan Bitnun (1): > drm/amd/display: Prevent invalid pipe connections > > Evan Quan (1): > drm/amd/pm: share the code around SMU13 pcie parameters update > > George Shen (4): > drm/amd/display: Update 128b/132b downspread factor to 0.3% > drm/amd/display: Add stream overhead in BW calculations for 128b/132b > drm/amd/display: Add link encoding to timing BW calculation parameters > drm/amd/display: Guard DCN31 PHYD32CLK logic against chip family > > Guchun Chen (6): > drm/amdgpu/vkms: drop redundant set of fb_modifiers_not_supported > drm/amdgpu/vkms: relax timer deactivation by hrtimer_try_to_cancel > drm/amdgpu: Allocate root PD on correct partition > drm/amdgpu: fix slab-out-of-bounds issue in amdgpu_vm_pt_create > drm/amdgpu/vm: use the same xcp_id from root PD > drm/amdgpu: use a macro to define no xcp partition case > > Horace Chen (1): > drm/amdgpu: set
RE: [PATCH] Revert "drm/amdgpu: don't modify num_doorbells for mes"
[AMD Official Use Only - General]
Nack to the revert.
This is a whole series of doorbell changes, which has replacement for the
patches too.
https://patchwork.freedesktop.org/series/115802/
The whole series was pushed to staging branch, but looks like some of the
patches did not merge.
When the whole series is merged, functionality will be restored.
I will check if there is any problem due to which other patches are blocked
merge.
Regards
Shashank
-Original Message-
From: Zhang, Yifan
Sent: Friday, August 4, 2023 7:01 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
; Sharma, Shashank ; Zhang,
Yifan
Subject: [PATCH] Revert "drm/amdgpu: don't modify num_doorbells for mes"
This reverts commit f46644aa8de6d5efeff8d8c7fbf3ed58a89c765c.
THe doorbell index could go beyond the first page for mes queues, this patch
breaks the mes self test on gfx11.
[ 23.212740] [drm] ring gfx_32768.1.1 was added
[ 23.213147] [drm] ring compute_32768.2.2 was added
[ 23.213540] [drm] ring sdma_32768.3.3 was added
[ 23.213546] [drm:amdgpu_mm_wdoorbell64 [amdgpu]] *ERROR* writing beyond
doorbell aperture: 0x1000!
[ 23.214148] amdgpu :c2:00.0: amdgpu: gfx_v11_0_ring_set_wptr_gfx: 5168
0x402 0x1000 100
[ 23.560357] amdgpu :c2:00.0: [drm:amdgpu_ring_test_helper [amdgpu]]
*ERROR* ring gfx_32768.1.1 test failed (-110)
Signed-off-by: Yifan Zhang
---
.../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 34 +++
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
index 5c0d3cea817d..31db526d4921 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
@@ -140,21 +140,25 @@ int amdgpu_doorbell_init(struct amdgpu_device *adev)
adev->doorbell.base = pci_resource_start(adev->pdev, 2);
adev->doorbell.size = pci_resource_len(adev->pdev, 2);
- adev->doorbell.num_kernel_doorbells =
- min_t(u32, adev->doorbell.size / sizeof(u32),
- adev->doorbell_index.max_assignment + 1);
- if (adev->doorbell.num_kernel_doorbells == 0)
- return -EINVAL;
-
- /*
-* For Vega, reserve and map two pages on doorbell BAR since SDMA
-* paging queue doorbell use the second page. The
-* AMDGPU_DOORBELL64_MAX_ASSIGNMENT definition assumes all the
-* doorbells are in the first page. So with paging queue enabled,
-* the max num_kernel_doorbells should + 1 page (0x400 in dword)
-*/
- if (adev->asic_type >= CHIP_VEGA10)
- adev->doorbell.num_kernel_doorbells += 0x400;
+ if (adev->enable_mes) {
+ adev->doorbell.num_kernel_doorbells =
+ adev->doorbell.size / sizeof(u32);
+ } else {
+ adev->doorbell.num_kernel_doorbells =
+ min_t(u32, adev->doorbell.size / sizeof(u32),
+ adev->doorbell_index.max_assignment+1);
+ if (adev->doorbell.num_kernel_doorbells == 0)
+ return -EINVAL;
+
+ /* For Vega, reserve and map two pages on doorbell BAR since
SDMA
+* paging queue doorbell use the second page. The
+* AMDGPU_DOORBELL64_MAX_ASSIGNMENT definition assumes all the
+* doorbells are in the first page. So with paging queue
enabled,
+* the max num_kernel_doorbells should + 1 page (0x400 in dword)
+*/
+ if (adev->asic_type >= CHIP_VEGA10)
+ adev->doorbell.num_kernel_doorbells += 0x400;
+ }
adev->doorbell.ptr = ioremap(adev->doorbell.base,
adev->doorbell.num_kernel_doorbells *
--
2.37.3
Re: [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 18:31, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser wrote: > > > The KMS docs describe "subconnector" to be defined as "downstream port" for > > DP. > > Can USB-C (or USB) be seen as a DP downstream port? > > To expand on this a bit: I'm wondering if we're mixing apples and > oranges here. The current values of "subconnector" typically describe > the lower-level protocol tunneled inside DP. For instance, VGA can be > tunneled inside the DP cable when using DP → VGA adapter. My opinion hasn't changed: I think this should be the USB connector with proper DP / DVI / HDMI / etc. subconnector type (or lack of it). In the end, the physical connector on the side of laptop is USB-C. If we want to make it different from GUD, we might want to define a USB-DP connector type (which would also include SlimPort). > > However, in the USB-C case, DP itself is tunneled inside USB-C. And you > might use a USB-C → DP adapter. So it's not really *sub*connector, it's > more of a *super*connector, right? > > I think [1] is somewhat related, since it also allows user-space to > discover whether a connector uses USB-C. But relying on sysfs to figure > this out isn't super optimal perhaps. > > [1]: > https://lore.kernel.org/dri-devel/20221108185004.2263578-1-wonch...@google.com/ -- With best wishes Dmitry
Re: [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 23:47, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 22:44, Laurent Pinchart > wrote: > > > On Thu, Aug 03, 2023 at 03:31:16PM +, Simon Ser wrote: > > > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr > > > wrote: > > > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > > for DP. > > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > > oranges here. The current values of "subconnector" typically describe > > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > > tunneled inside the DP cable when using DP → VGA adapter. > > > > Doesn't this contradict the example use case you gave in your previous > > e-mail, with wlroots stating "DP-3 via DVI-D" ? I understand that as DP > > carried over a DVI-D physical connector, did I get it wrong ? > > No, this is DVI carried over DP. DP cannot be carried over VGA/DVI/HDMI, > but VGA/DVI/HDMI can be carried over DP. Well, not quite. It means that the sink (display) connected to this port identifies itself as an VGA / DVI / HDMI monitor. E.g. on if connect HDMI/DVI monitor through the DP-DVI dongle (native DP connector), AMD driver still identifies it as 'subconnector HDMI', despite dongle a DVI connector on the other side. -- With best wishes Dmitry
Re: [PATCH v5 05/11] drm/amdgpu: Use RMW accessors for changing LNKCTL
On Fri, 21 Jul 2023, Alex Deucher wrote:
> On Fri, Jul 21, 2023 at 4:18 AM Ilpo Järvinen
> wrote:
> >
> > On Thu, 20 Jul 2023, Bjorn Helgaas wrote:
> >
> > > On Mon, Jul 17, 2023 at 03:04:57PM +0300, Ilpo Järvinen wrote:
> > > > Don't assume that only the driver would be accessing LNKCTL. ASPM
> > > > policy changes can trigger write to LNKCTL outside of driver's control.
> > > > And in the case of upstream bridge, the driver does not even own the
> > > > device it's changing the registers for.
> > > >
> > > > Use RMW capability accessors which do proper locking to avoid losing
> > > > concurrent updates to the register value.
> > > >
> > > > Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts")
> > > > Fixes: 62a37553414a ("drm/amdgpu: add si implementation v10")
> > > > Suggested-by: Lukas Wunner
> > > > Signed-off-by: Ilpo Järvinen
> > > > Cc: sta...@vger.kernel.org
> > >
> > > Do we have any reports of problems that are fixed by this patch (or by
> > > others in the series)? If not, I'm not sure it really fits the usual
> > > stable kernel criteria:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?id=v6.4
> >
> > I was on the edge with this. The answer to your direct question is no,
> > there are no such reports so it would be okay to leave stable out I think.
> > This applies to all patches in this series.
> >
> > Basically, this series came to be after Lukas noted the potential
> > concurrency issues with how LNKCTL is unprotected when reviewing
> > (internally) my bandwidth controller series. Then I went to look around
> > all LNKCTL usage and realized existing things might alreary have similar
> > issues.
> >
> > Do you want me to send another version w/o cc stable or you'll take care
> > of that?
> >
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/cik.c | 36 +---
> > > > drivers/gpu/drm/amd/amdgpu/si.c | 36 +---
> > > > 2 files changed, 20 insertions(+), 52 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c
> > > > b/drivers/gpu/drm/amd/amdgpu/cik.c
> > > > index 5641cf05d856..e63abdf52b6c 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> > > > @@ -1574,17 +1574,8 @@ static void cik_pcie_gen3_enable(struct
> > > > amdgpu_device *adev)
> > > > u16 bridge_cfg2, gpu_cfg2;
> > > > u32 max_lw, current_lw, tmp;
> > > >
> > > > - pcie_capability_read_word(root, PCI_EXP_LNKCTL,
> > > > - &bridge_cfg);
> > > > - pcie_capability_read_word(adev->pdev,
> > > > PCI_EXP_LNKCTL,
> > > > - &gpu_cfg);
> > > > -
> > > > - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD;
> > > > - pcie_capability_write_word(root, PCI_EXP_LNKCTL,
> > > > tmp16);
> > > > -
> > > > - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD;
> > > > - pcie_capability_write_word(adev->pdev,
> > > > PCI_EXP_LNKCTL,
> > > > - tmp16);
> > > > + pcie_capability_set_word(root, PCI_EXP_LNKCTL,
> > > > PCI_EXP_LNKCTL_HAWD);
> > > > + pcie_capability_set_word(adev->pdev,
> > > > PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD);
> > > >
> > > > tmp = RREG32_PCIE(ixPCIE_LC_STATUS1);
> > > > max_lw = (tmp &
> > > > PCIE_LC_STATUS1__LC_DETECTED_LINK_WIDTH_MASK) >>
> > > > @@ -1637,21 +1628,14 @@ static void cik_pcie_gen3_enable(struct
> > > > amdgpu_device *adev)
> > > > msleep(100);
> > > >
> > > > /* linkctl */
> > > > - pcie_capability_read_word(root,
> > > > PCI_EXP_LNKCTL,
> > > > - &tmp16);
> > > > - tmp16 &= ~PCI_EXP_LNKCTL_HAWD;
> > > > - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD);
> > > > - pcie_capability_write_word(root,
> > > > PCI_EXP_LNKCTL,
> > > > - tmp16);
> > > > -
> > > > - pcie_capability_read_word(adev->pdev,
> > > > - PCI_EXP_LNKCTL,
> > > > - &tmp16);
> > > > - tmp16 &= ~PCI_EXP_LNKCTL_HAWD;
> > > > - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD);
> > > > - pcie_capability_write_word(adev->pdev,
> > > > - PCI_EXP_LNKCTL,
> > > > - tmp16);
> > > > + pcie_capability_clear_and_set_word(root,
Re: [PATCH 3/4] drm/uapi: document the USB subconnector type
On Thu, 3 Aug 2023 at 18:43, Simon Ser wrote: > > On Thursday, August 3rd, 2023 at 17:36, Dmitry Baryshkov > wrote: > > > On Thu, 3 Aug 2023 at 18:31, Simon Ser cont...@emersion.fr wrote: > > > > > On Thursday, August 3rd, 2023 at 17:22, Simon Ser cont...@emersion.fr > > > wrote: > > > > > > > The KMS docs describe "subconnector" to be defined as "downstream port" > > > > for DP. > > > > Can USB-C (or USB) be seen as a DP downstream port? > > > > > > To expand on this a bit: I'm wondering if we're mixing apples and > > > oranges here. The current values of "subconnector" typically describe > > > the lower-level protocol tunneled inside DP. For instance, VGA can be > > > tunneled inside the DP cable when using DP → VGA adapter. > > > > My opinion hasn't changed: I think this should be the USB connector > > with proper DP / DVI / HDMI / etc. subconnector type (or lack of it). > > In the end, the physical connector on the side of laptop is USB-C. > > - Even if the connector is USB-C, the protocol used for display is > still DP. There's also the case of Thunderbolt. Yes. But the connector type is not about the protocol. > - This is inconsistent with existing drivers. i915 and amdgpu expose > DP ports for their USB-C ports. Changing that isn't possible without > causing user-space regressions (compositor config files use the > connector type). Yes, I know. Consider my phrase as a personal opinion or minority report. I think that using DisplayPort for USB-C connectors was a mistake, which we now have to cope with somehow. -- With best wishes Dmitry
[PATCH -next v2] drm/amdgpu: Remove a lot of unnecessary ternary operators
There are many ternary operators, the true or false judgement
of which is unnecessary in C language semantics.
Signed-off-by: Ruan Jinjie
---
v2:
- add the wrong removed true or false judgement hunk code
- Update the commit message, Ther -> There
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 2 +-
drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c | 2 +-
drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c| 2 +-
drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c| 2 +-
drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c | 2 +-
drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c| 4 +---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_powertune.c | 2 +-
drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c| 9 +++--
.../gpu/drm/amd/pm/powerplay/smumgr/polaris10_smumgr.c | 2 +-
drivers/gpu/drm/amd/pm/powerplay/smumgr/vegam_smumgr.c | 7 +++
13 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index b582b83c4984..38ccec913f00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -460,7 +460,7 @@ bool amdgpu_get_bios(struct amdgpu_device *adev)
return false;
success:
- adev->is_atom_fw = (adev->asic_type >= CHIP_VEGA10) ? true : false;
+ adev->is_atom_fw = adev->asic_type >= CHIP_VEGA10;
return true;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
index 79791379fc2b..df4440c21bbf 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c
@@ -479,7 +479,7 @@ static int jpeg_v3_0_set_clockgating_state(void *handle,
enum amd_clockgating_state state)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
+ bool enable = state == AMD_CG_STATE_GATE;
if (enable) {
if (!jpeg_v3_0_is_idle(handle))
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index a707d407fbd0..3eb3dcd56b57 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -626,7 +626,7 @@ static int jpeg_v4_0_set_clockgating_state(void *handle,
enum amd_clockgating_state state)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
+ bool enable = state == AMD_CG_STATE_GATE;
if (enable) {
if (!jpeg_v4_0_is_idle(handle))
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
index ce2b22f7e4e4..153731d6ce8b 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c
@@ -785,7 +785,7 @@ static int jpeg_v4_0_3_set_clockgating_state(void *handle,
enum amd_clockgating_state state)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
+ bool enable = state == AMD_CG_STATE_GATE;
int i;
for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index b76ba21b5a89..9b662b105cc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -2095,7 +2095,7 @@ static int vcn_v3_0_set_clockgating_state(void *handle,
enum amd_clockgating_state state)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
+ bool enable = state == AMD_CG_STATE_GATE;
int i;
for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 6089c7deba8a..7c486745bece 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1918,7 +1918,7 @@ static int vcn_v4_0_wait_for_idle(void *handle)
static int vcn_v4_0_set_clockgating_state(void *handle, enum
amd_clockgating_state state)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
+ bool enable = state == AMD_CG_STATE_GATE;
int i;
for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
index 550ac040b4be..e62472e6e7b3 10
[PATCH] drm/amd/display: Return value of function
Added return value check hpd_enable Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Denis Arefev --- drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c index fa314493ffc5..bf2f620aeb66 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c @@ -1645,7 +1645,7 @@ void dce110_link_encoder_enable_hpd(struct link_encoder *enc) uint32_t hpd_enable = 0; uint32_t value = dm_read_reg(ctx, addr); - get_reg_field_value(hpd_enable, DC_HPD_CONTROL, DC_HPD_EN); + hpd_enable = get_reg_field_value(hpd_enable, DC_HPD_CONTROL, DC_HPD_EN); if (hpd_enable == 0) set_reg_field_value(value, 1, DC_HPD_CONTROL, DC_HPD_EN); -- 2.25.1
Re: [PATCH] drm/amdgpu: Remove volatile from 'wb' & from 'ptr' in amdgpu.h
On 8/3/2023 2:12 PM, Christian König wrote:
Am 03.08.23 um 07:23 schrieb SHANMUGAM, SRINIVASAN:
On 7/24/2023 10:43 PM, Alex Deucher wrote:
On Mon, Jul 24, 2023 at 11:54 AM Srinivasan Shanmugam
wrote:
Fixes the following from checkpatch.pl:
WARNING: Use of volatile is usually wrong: see
Documentation/process/volatile-considered-harmful.rst
+ volatile uint32_t *wb;
WARNING: Use of volatile is usually wrong: see
Documentation/process/volatile-considered-harmful.rst
+ volatile uint32_t *ptr;
'wb' field from 'amdgpu_wb' struct & 'ptr' field from
'amdgpu_mem_scratch', is not used to access h/w directly, neither they
are shared variables, so volatile is not necessary
How did you come to that determination? Both are GPU accessible
memory allocations. The writeback (wb) allocation happens to be in
GTT so it's system memory, but the the mem_scratch allocation can be
in device memory.
Alex
Hi Alex,
Thanks for your feedbacks!
Commit message is misleading, I presumed that this volatile modifiers
are used for monitoring HW status registers due to external events &
for some shared variables - may be volatile might be needed for *wb
pointer variable - as they may be used for caches in between (on
surface level info), can we split this patch into two, I felt
volatile for *ptr is unnecessary as it is type casted with void type
[(void **)&adev->mem_scratch.ptr); in amdgpu_device.c]- Any advises
onto this please?
Instead of declaring pointers we should use READ_ONCE()/WRITE_ONCE()
when accessing those values to make sure that the compiler doesn't do
any nasty things.
Regards,
Christian.
Thanks a lot Christian!
So both the variables needs to be changed to plain variables - (ie.,
"u32 wb" & "u32 ptr" without any volatile keyword or pointer variable) &
then protect this variables with "READ_ONCE()/WRITE_ONCE()", For ex: I
have proposed - https://patchwork.freedesktop.org/patch/551273/ am I
correct please? but may I know please is that volatile keyword, is that
doing the same job as "READ_ONCE()/WRITE_ONCE()", where compiler
optimizations is disabled? so that we can leave it as it is. & ignore
the checkpatch warning onto this please?
-Srini
Best regards,
Srini
Cc: Christian König
Cc: Alex Deucher
Signed-off-by: Srinivasan Shanmugam
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a046160b6a0e..06f79a84ff4b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -502,7 +502,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct
amdgpu_fpriv **fpriv);
struct amdgpu_wb {
struct amdgpu_bo*wb_obj;
- volatile uint32_t *wb;
+ u32 *wb;
uint64_tgpu_addr;
u32 num_wb; /* Number of wb slots actually
reserved for amdgpu. */
unsigned long used[DIV_ROUND_UP(AMDGPU_MAX_WB,
BITS_PER_LONG)];
@@ -621,7 +621,7 @@ int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev,
void *data,
/* VRAM scratch page for HDP bug, default vram page */
struct amdgpu_mem_scratch {
struct amdgpu_bo*robj;
- volatile uint32_t *ptr;
+ u32 *ptr;
u64 gpu_addr;
};
--
2.25.1
