[PATCH v2] drm/amdgpu: Fix realloc of ptr
From: Tom Rix
Clang static analysis reports this error
amdgpu_debugfs.c:1690:9: warning: 1st function call
argument is an uninitialized value
tmp = krealloc_array(tmp, i + 1,
^~~
realloc uses tmp, so tmp can not be garbage.
And the return needs to be checked.
Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
Signed-off-by: Tom Rix
---
v2:
use 'new' to hold/check the ralloc return
fix commit log mistake on ralloc freeing to using input ptr
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 9eb9b440bd438..2f4f8c5618d81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1676,7 +1676,7 @@ static ssize_t
amdgpu_reset_dump_register_list_write(struct file *f,
{
struct amdgpu_device *adev = (struct amdgpu_device
*)file_inode(f)->i_private;
char reg_offset[11];
- uint32_t *tmp;
+ uint32_t *new, *tmp = NULL;
int ret, i = 0, len = 0;
do {
@@ -1687,7 +1687,12 @@ static ssize_t
amdgpu_reset_dump_register_list_write(struct file *f,
goto error_free;
}
- tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
+ new = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
+ if (!new) {
+ ret = -ENOMEM;
+ goto error_free;
+ }
+ tmp = new;
if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
ret = -EINVAL;
goto error_free;
--
2.26.3
[PATCH] drm/amdgpu: Fix realloc of ptr
From: Tom Rix
Clang static analysis reports this error
amdgpu_debugfs.c:1690:9: warning: 1st function call
argument is an uninitialized value
tmp = krealloc_array(tmp, i + 1,
^~~
realloc will free tmp, so tmp can not be garbage.
And the return needs to be checked.
Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 9eb9b440bd438..159b97c0b4ebc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1676,7 +1676,7 @@ static ssize_t
amdgpu_reset_dump_register_list_write(struct file *f,
{
struct amdgpu_device *adev = (struct amdgpu_device
*)file_inode(f)->i_private;
char reg_offset[11];
- uint32_t *tmp;
+ uint32_t *tmp = NULL;
int ret, i = 0, len = 0;
do {
@@ -1688,6 +1688,10 @@ static ssize_t
amdgpu_reset_dump_register_list_write(struct file *f,
}
tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
+ if (!tmp) {
+ ret = -ENOMEM;
+ goto error_free;
+ }
if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
ret = -EINVAL;
goto error_free;
--
2.26.3
[PATCH] drm/amdgpu: fix printk format for size_t variable
From: Tom Rix
On mips64 allyesconfig, there is this build break
amdgpu_discovery.c:671:35: error: format '%ld' expects
argument of type 'long int', but argument 4 has
type 'size_t' {aka 'unsigned int'}
DRM_DEBUG("match:%d @ ip_offset:%ld", ii, ip_offset);
For size_t, use %zu.
Fixes: a6c40b178092 ("drm/amdgpu: Show IP discovery in sysfs")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 7c7e28fd912e..58238f67b1d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -668,7 +668,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device
*adev,
le16_to_cpu(ip->hw_id) != ii)
goto next_ip;
- DRM_DEBUG("match:%d @ ip_offset:%ld", ii, ip_offset);
+ DRM_DEBUG("match:%d @ ip_offset:%zu", ii, ip_offset);
/* We have a hw_id match; register the hw
* block if not yet registered.
--
2.26.3
[PATCH] drm/amdkfd: rework criu_restore_bos error handling
From: Tom Rix
Clang static analysis reports this problem
kfd_chardev.c:2327:2: warning: 1st function call argument
is an uninitialized value
kvfree(bo_privs);
^~~~
If the copy_from_users(bo_buckets, ...) fails, there is a jump to
the generic error handler at exit:. The freeing of bo_privs and
unwinding of the dmabuf_fd loop do not need to be done.
Add some specific labels for the early failures.
Reorder the frees to be the reverse of their allocs.
Move the initialize of 'i' back to the loop.
The problem with the early frees predates the loop
unwinding problem.
Fixes: 73fa13b6a511 ("drm/amdkfd: CRIU Implement KFD restore ioctl")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 965af2a08bc0..1d5f41ac3832 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2102,7 +2102,7 @@ static int criu_restore_bos(struct kfd_process *p,
const bool criu_resume = true;
bool flush_tlbs = false;
int ret = 0, j = 0;
- uint32_t i = 0;
+ uint32_t i;
if (*priv_offset + (args->num_bos * sizeof(*bo_privs)) >
max_priv_data_size)
return -EINVAL;
@@ -2119,13 +2119,13 @@ static int criu_restore_bos(struct kfd_process *p,
if (ret) {
pr_err("Failed to copy BOs information from user\n");
ret = -EFAULT;
- goto exit;
+ goto free_buckets;
}
bo_privs = kvmalloc_array(args->num_bos, sizeof(*bo_privs), GFP_KERNEL);
if (!bo_privs) {
ret = -ENOMEM;
- goto exit;
+ goto free_buckets;
}
ret = copy_from_user(bo_privs, (void __user *)args->priv_data +
*priv_offset,
@@ -2133,12 +2133,12 @@ static int criu_restore_bos(struct kfd_process *p,
if (ret) {
pr_err("Failed to copy BOs information from user\n");
ret = -EFAULT;
- goto exit;
+ goto free_privs;
}
*priv_offset += args->num_bos * sizeof(*bo_privs);
/* Create and map new BOs */
- for (; i < args->num_bos; i++) {
+ for (i = 0; i < args->num_bos; i++) {
struct kfd_criu_bo_bucket *bo_bucket;
struct kfd_criu_bo_priv_data *bo_priv;
struct kfd_dev *dev;
@@ -2323,8 +2323,11 @@ static int criu_restore_bos(struct kfd_process *p,
if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
close_fd(bo_buckets[i].dmabuf_fd);
}
- kvfree(bo_buckets);
+free_privs:
kvfree(bo_privs);
+free_buckets:
+ kvfree(bo_buckets);
+
return ret;
}
--
2.26.3
[PATCH] drm/amdkfd: fix typo in setting enum value
From: Tom Rix
Clang build fails with
kfd_packet_manager_v9.c:267:3: error: implicit conversion
from enumeration type 'enum mes_map_queues_extended_engine_sel_enum'
to different enumeration type
'enum mes_unmap_queues_extended_engine_sel_enum'
extended_engine_sel__mes_map_queues__sdma0_to_7_sel :
^~~
This looks like a typo, the function is _unmap_, the enum
extended_engine_sel__mes_map_queues__sdma0_to_7_sel is _map_.
To match the packet->bitfields2.extended_engine_set type
it should be extended_engine_sel__mes_unmap_queues__sdma0_to_7_sel.
Fixes: 009e9a158505 ("drm/amdkfd: navi2x requires extended engines to map and
unmap sdma queues")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
index 806a03566a24..18250845a989 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
@@ -264,7 +264,7 @@ static int pm_unmap_queues_v9(struct packet_manager *pm,
uint32_t *buffer,
sizeof(struct pm4_mes_unmap_queues));
packet->bitfields2.extended_engine_sel = pm_use_ext_eng(pm->dqm->dev) ?
- extended_engine_sel__mes_map_queues__sdma0_to_7_sel :
+ extended_engine_sel__mes_unmap_queues__sdma0_to_7_sel :
extended_engine_sel__mes_unmap_queues__legacy_engine_sel;
packet->bitfields2.engine_sel =
--
2.26.3
[PATCH] drm/amdgpu: fix amdgpu_ras_block_late_init error handler
From: Tom Rix
Clang build fails with
amdgpu_ras.c:2416:7: error: variable 'ras_obj' is used uninitialized
whenever 'if' condition is true
if (adev->in_suspend || amdgpu_in_reset(adev)) {
^
amdgpu_ras.c:2453:6: note: uninitialized use occurs here
if (ras_obj->ras_cb)
^~~
There is a logic error in the error handler's labels.
ex/ The sysfs: is the last goto label in the normal code but
is the middle of error handler. Rework the error handler.
cleanup: is the first error, so it's handler should be last.
interrupt: is the second error, it's handler is next. interrupt:
handles the failure of amdgpu_ras_interrupt_add_hander() by
calling amdgpu_ras_interrupt_remove_handler(). This is wrong,
remove() assumes the interrupt has been setup, not torn down by
add(). Change the goto label to cleanup.
sysfs is the last error, it's handler should be first. sysfs:
handles the failure of amdgpu_ras_sysfs_create() by calling
amdgpu_ras_sysfs_remove(). But when the create() fails there
is nothing added so there is nothing to remove. This error
handler is not needed. Remove the error handler and change
goto label to interrupt.
Fixes: b293e891b057 ("drm/amdgpu: add helper function to do common
ras_late_init/fini (v3)")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index b5cd21cb6e58..c5c8a666110f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2432,12 +2432,12 @@ int amdgpu_ras_block_late_init(struct amdgpu_device
*adev,
if (ras_obj->ras_cb) {
r = amdgpu_ras_interrupt_add_handler(adev, ras_block);
if (r)
- goto interrupt;
+ goto cleanup;
}
r = amdgpu_ras_sysfs_create(adev, ras_block);
if (r)
- goto sysfs;
+ goto interrupt;
/* Those are the cached values at init.
*/
@@ -2447,12 +2447,11 @@ int amdgpu_ras_block_late_init(struct amdgpu_device
*adev,
}
return 0;
-cleanup:
- amdgpu_ras_sysfs_remove(adev, ras_block);
-sysfs:
+
+interrupt:
if (ras_obj->ras_cb)
amdgpu_ras_interrupt_remove_handler(adev, ras_block);
-interrupt:
+cleanup:
amdgpu_ras_feature_enable(adev, ras_block, 0);
return r;
}
--
2.26.3
[PATCH] drm/amdgpu: check return status before using stable_pstate
From: Tom Rix
Clang static analysis reports this problem
amdgpu_ctx.c:616:26: warning: Assigned value is garbage
or undefined
args->out.pstate.flags = stable_pstate;
^ ~
amdgpu_ctx_stable_pstate can fail without setting
stable_pstate. So check.
Fixes: 8cda7a4f96e4 ("drm/amdgpu/UAPI: add new CTX OP to get/set stable
pstates")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 1c72f6095f08..f522b52725e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -613,7 +613,8 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
if (args->in.flags)
return -EINVAL;
r = amdgpu_ctx_stable_pstate(adev, fpriv, id, false,
&stable_pstate);
- args->out.pstate.flags = stable_pstate;
+ if (!r)
+ args->out.pstate.flags = stable_pstate;
break;
case AMDGPU_CTX_OP_SET_STABLE_PSTATE:
if (args->in.flags & ~AMDGPU_CTX_STABLE_PSTATE_FLAGS_MASK)
--
2.26.3
[PATCH] drm/amdkfd: fix loop error handling
From: Tom Rix
Clang static analysis reports this problem
kfd_chardev.c:2594:16: warning: The expression is an uninitialized value.
The computed value will also be garbage
while (ret && i--) {
^~~
i is a loop variable and this block unwinds a problem in the loop.
When the error happens before the loop, this value is garbage.
Move the initialization of i to its decalaration.
Fixes: be072b06c739 ("drm/amdkfd: CRIU export BOs as prime dmabuf objects")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 636391c61cafb..4310ca07af130 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2374,7 +2374,7 @@ static int criu_restore_bos(struct kfd_process *p,
const bool criu_resume = true;
bool flush_tlbs = false;
int ret = 0, j = 0;
- uint32_t i;
+ uint32_t i = 0;
if (*priv_offset + (args->num_bos * sizeof(*bo_privs)) >
max_priv_data_size)
return -EINVAL;
@@ -2410,7 +2410,7 @@ static int criu_restore_bos(struct kfd_process *p,
*priv_offset += args->num_bos * sizeof(*bo_privs);
/* Create and map new BOs */
- for (i = 0; i < args->num_bos; i++) {
+ for (; i < args->num_bos; i++) {
struct kfd_criu_bo_bucket *bo_bucket;
struct kfd_criu_bo_priv_data *bo_priv;
struct kfd_dev *dev;
--
2.26.3
[PATCH] drm/amdkfd: fix freeing an unset pointer
From: Tom Rix
clang static analysis reports this problem
kfd_chardev.c:2092:2: warning: 1st function call argument
is an uninitialized value
kvfree(bo_privs);
^~~~
When bo_buckets alloc fails, it jumps to an error handler
that frees the yet to be allocated bo_privs. Because
bo_buckets is the first error, return directly.
Fixes: 5ccbb057c0a1 ("drm/amdkfd: CRIU Implement KFD checkpoint ioctl")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 64e3b4e3a7126..636391c61cafb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1982,10 +1982,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
void *mem;
bo_buckets = kvzalloc(num_bos * sizeof(*bo_buckets), GFP_KERNEL);
- if (!bo_buckets) {
- ret = -ENOMEM;
- goto exit;
- }
+ if (!bo_buckets)
+ return -ENOMEM;
bo_privs = kvzalloc(num_bos * sizeof(*bo_privs), GFP_KERNEL);
if (!bo_privs) {
--
2.26.3
[PATCH] drm/amd/pm: fix error handling
From: Tom Rix
clang static analysis reports this error
amdgpu_smu.c:2289:9: warning: Called function pointer
is null (null dereference)
return smu->ppt_funcs->emit_clk_levels(
^~~~
There is a logic error in the earlier check of
emit_clk_levels. The error value is set to
the ret variable but ret is never used. Return
directly and remove the unneeded ret variable.
Fixes: 5d64f9bbb628 ("amdgpu/pm: Implement new API function "emit" that accepts
buffer base and write offset")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index af368aa1fd0ae..5f3b3745a9b7a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2274,7 +2274,6 @@ static int smu_emit_ppclk_levels(void *handle, enum
pp_clock_type type, char *bu
{
struct smu_context *smu = handle;
enum smu_clk_type clk_type;
- int ret = 0;
clk_type = smu_convert_to_smuclk(type);
if (clk_type == SMU_CLK_COUNT)
@@ -2284,7 +2283,7 @@ static int smu_emit_ppclk_levels(void *handle, enum
pp_clock_type type, char *bu
return -EOPNOTSUPP;
if (!smu->ppt_funcs->emit_clk_levels)
- ret = -ENOENT;
+ return -ENOENT;
return smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
--
2.26.3
[PATCH] drm/amdgpu: initialize reg_access_ctrl
From: Tom Rix
clang build fails with
amdgpu_virt.c:878:51: error: variable 'reg_access_ctrl' is
uninitialized when used here
... + 4 * reg_access_ctrl->scratch_reg0;
^~~
The reg_access_ctrl ptr is never initialized, so
initialize once we know it is supported.
Fixes: 5d447e296701 ("drm/amdgpu: add helper for rlcg indirect reg access")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 80c25176c9932..c137652189190 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -875,6 +875,7 @@ static u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device
*adev, u32 offset, u32 v
return 0;
}
+ reg_access_ctrl = &adev->gfx.rlc.reg_access_ctrl;
scratch_reg0 = (void __iomem *)adev->rmmio + 4 *
reg_access_ctrl->scratch_reg0;
scratch_reg1 = (void __iomem *)adev->rmmio + 4 *
reg_access_ctrl->scratch_reg1;
scratch_reg2 = (void __iomem *)adev->rmmio + 4 *
reg_access_ctrl->scratch_reg2;
--
2.26.3
[PATCH v2] drm/amd/pm: return -ENOTSUPP if there is no get_dpm_ultimate_freq function
From: Tom Rix
clang static analysis reports this represenative problem
amdgpu_smu.c:144:18: warning: The left operand of '*' is a garbage value
return clk_freq * 100;
^
If there is no get_dpm_ultimate_freq function,
smu_get_dpm_freq_range returns success without setting the
output min,max parameters. So return an -ENOTSUPP error.
Fixes: e5ef784b1e17 ("drm/amd/powerplay: revise calling chain on retrieving
frequency range")
Signed-off-by: Tom Rix
---
v2: return error instead of initializing min/max
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5ace30434e603..264eb09ccfd51 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -116,7 +116,7 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
uint32_t *min,
uint32_t *max)
{
- int ret = 0;
+ int ret = -ENOTSUPP;
if (!min && !max)
return -EINVAL;
--
2.26.3
[PATCH] drm/amd/pm: set min, max to 0 if there is no get_dpm_ultimate_freq function
From: Tom Rix
clang static analysis reports this represenative problem
amdgpu_smu.c:144:18: warning: The left operand of '*' is a garbage value
return clk_freq * 100;
^
If there is no get_dpm_ultimate_freq function,
smu_get_dpm_freq_range returns success without setting the
output min,max parameters. Because this is an extern function,
set the min,max to 0 when there is no get_dpm_ultimate_freq.
Fixes: e5ef784b1e17 ("drm/amd/powerplay: revise calling chain on retrieving
frequency range")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5ace30434e603..35fbe51f52eaa 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -121,11 +121,17 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
if (!min && !max)
return -EINVAL;
- if (smu->ppt_funcs->get_dpm_ultimate_freq)
+ if (smu->ppt_funcs->get_dpm_ultimate_freq) {
ret = smu->ppt_funcs->get_dpm_ultimate_freq(smu,
clk_type,
min,
max);
+ } else {
+ if (min)
+ *min = 0;
+ if (max)
+ *max = 0;
+ }
return ret;
}
--
2.26.3
[PATCH] drm/amdkfd: simplify else if to else check of MIGRATION_COPY_DIR
From: Tom Rix
The enum MIGRATION_COPY_DIR type has 2 values. So
the else-if can be converted to an else.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index d986f9ee0e1f4..c06c44f37b00e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -146,7 +146,8 @@ svm_migrate_copy_memory_gart(struct amdgpu_device *adev,
dma_addr_t *sys,
gart_s = svm_migrate_direct_mapping_addr(adev, *vram);
r = svm_migrate_gart_map(ring, size, sys, &gart_d, 0);
- } else if (direction == FROM_RAM_TO_VRAM) {
+ } else {
+ /* direction == FROM_RAM_TO_VRAM */
r = svm_migrate_gart_map(ring, size, sys, &gart_s,
KFD_IOCTL_SVM_FLAG_GPU_RO);
gart_d = svm_migrate_direct_mapping_addr(adev, *vram);
--
2.26.3
[PATCH] drm/amdkfd: match the signatures of the real and stub kgd2kfd_probe()
From: Tom Rix
When CONFIG_HSA_AMD=n this there is this error
amdgpu_amdkfd.c:75:56: error: incompatible type for
argument 2 of ‘kgd2kfd_probe’
75 | adev->kfd.dev = kgd2kfd_probe((struct kgd_dev *)adev, vf);
amdgpu_amdkfd.h:349:17: note: declared here
349 | struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd,
struct pci_dev *pdev,
The signature of the stub kgd2kfd_probe() does not match the real one.
So change the stub to match.
Fixes: 920f37e6a3fc ("drm/amdkfd: clean up parameters in kgd2kfd_probe")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 38d883dffc20..69de31754907 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -346,8 +346,7 @@ static inline void kgd2kfd_exit(void)
}
static inline
-struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd, struct pci_dev *pdev,
- unsigned int asic_type, bool vf)
+struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd, bool vf)
{
return NULL;
}
--
2.26.3
[PATCH] drm/amdgpu: initialize amdgpu_ras_query_error_count() error count parameters
From: Tom Rix
Static analysis reports this problem
amdgpu_ras.c:2324:2: warning: 2nd function call argument is an
uninitialized value
atomic_set(&con->ras_ce_count, ce_count);
^~~~
ce_count is normally set by the earlier call to
amdgpu_ras_query_error_count(). But amdgpu_ras_query_error_count()
can return early without setting, leaving its error count parameters
in a garbage state.
Initialize the error count parameters earlier.
Fixes: a46751fbcde5 ("drm/amdgpu: Fix RAS function interface")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 875874ea745ec..c80fa545aa2b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1056,6 +1056,12 @@ void amdgpu_ras_query_error_count(struct amdgpu_device
*adev,
struct ras_manager *obj;
unsigned long ce, ue;
+ if (ce_count)
+ *ce_count = 0;
+
+ if (ue_count)
+ *ue_count = 0;
+
if (!adev->ras_enabled || !con)
return;
--
2.26.3
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/pm: initialize variable
From: Tom Rix
Static analysis reports this problem
amdgpu_pm.c:478:16: warning: The right operand of '<' is a garbage value
for (i = 0; i < data.nums; i++) {
^ ~
In some cases data is not set. Initialize to 0 and flag not setting
data as an error with the existing check.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 4e459ef632ef..9a54066ec0af 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -451,7 +451,7 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev,
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- struct pp_states_info data;
+ struct pp_states_info data = {0};
enum amd_pm_state_type pm = 0;
int i = 0, ret = 0;
--
2.26.3
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/radeon: remove h from printk format specifier
From: Tom Rix
See Documentation/core-api/printk-formats.rst.
h should no longer be used in the format specifier for printk.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/radeon/radeon_uvd.c | 2 +-
drivers/gpu/drm/radeon/radeon_vce.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 57fb3eb3a4b4..39c1c339be7b 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -155,7 +155,7 @@ int radeon_uvd_init(struct radeon_device *rdev)
family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
version_major = (le32_to_cpu(hdr->ucode_version) >> 24)
& 0xff;
version_minor = (le32_to_cpu(hdr->ucode_version) >> 8)
& 0xff;
- DRM_INFO("Found UVD firmware Version: %hu.%hu Family
ID: %hu\n",
+ DRM_INFO("Found UVD firmware Version: %u.%u Family ID:
%u\n",
version_major, version_minor, family_id);
/*
diff --git a/drivers/gpu/drm/radeon/radeon_vce.c
b/drivers/gpu/drm/radeon/radeon_vce.c
index 5e8006444704..a450497368b2 100644
--- a/drivers/gpu/drm/radeon/radeon_vce.c
+++ b/drivers/gpu/drm/radeon/radeon_vce.c
@@ -122,7 +122,7 @@ int radeon_vce_init(struct radeon_device *rdev)
if (sscanf(c, "%2u]", &rdev->vce.fb_version) != 1)
return -EINVAL;
- DRM_INFO("Found VCE firmware/feedback version %hhd.%hhd.%hhd / %d!\n",
+ DRM_INFO("Found VCE firmware/feedback version %d.%d.%d / %d!\n",
start, mid, end, rdev->vce.fb_version);
rdev->vce.fw_version = (start << 24) | (mid << 16) | (end << 8);
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: remove h from printk format specifier
From: Tom Rix
See Documentation/core-api/printk-formats.rst.
h should no longer be used in the format specifier for printk.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 7c5b60e53482..8b989670ed66 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -240,7 +240,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
- DRM_INFO("Found UVD firmware Version: %hu.%hu Family ID: %hu\n",
+ DRM_INFO("Found UVD firmware Version: %u.%u Family ID: %u\n",
version_major, version_minor, family_id);
/*
@@ -267,7 +267,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
dec_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
enc_minor = (le32_to_cpu(hdr->ucode_version) >> 24) & 0x3f;
enc_major = (le32_to_cpu(hdr->ucode_version) >> 30) & 0x3;
- DRM_INFO("Found UVD firmware ENC: %hu.%hu DEC: .%hu Family ID:
%hu\n",
+ DRM_INFO("Found UVD firmware ENC: %u.%u DEC: .%u Family ID:
%u\n",
enc_major, enc_minor, dec_minor, family_id);
adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 4861f8ddc1b5..ea6a62f67e38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -179,7 +179,7 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned
long size)
version_major = (ucode_version >> 20) & 0xfff;
version_minor = (ucode_version >> 8) & 0xfff;
binary_id = ucode_version & 0xff;
- DRM_INFO("Found VCE firmware Version: %hhd.%hhd Binary ID: %hhd\n",
+ DRM_INFO("Found VCE firmware Version: %d.%d Binary ID: %d\n",
version_major, version_minor, binary_id);
adev->vce.fw_version = ((version_major << 24) | (version_minor << 16) |
(binary_id << 8));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 1e756186e3f8..99b82f3c2617 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -181,7 +181,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
enc_major = fw_check;
dec_ver = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xf;
vep = (le32_to_cpu(hdr->ucode_version) >> 28) & 0xf;
- DRM_INFO("Found VCN firmware Version ENC: %hu.%hu DEC: %hu VEP:
%hu Revision: %hu\n",
+ DRM_INFO("Found VCN firmware Version ENC: %u.%u DEC: %u VEP: %u
Revision: %u\n",
enc_major, enc_minor, dec_ver, vep, fw_rev);
} else {
unsigned int version_major, version_minor, family_id;
@@ -189,7 +189,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
- DRM_INFO("Found VCN firmware Version: %hu.%hu Family ID: %hu\n",
+ DRM_INFO("Found VCN firmware Version: %u.%u Family ID: %u\n",
version_major, version_minor, family_id);
}
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/display: remove trailing semicolon in macro definition
From: Tom Rix The macro use will already have a semicolon. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index f9c81bc21ba4..301e93c9e72a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1213,7 +1213,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev); #define amdgpu_asic_update_umd_stable_pstate(adev, enter) \ ((adev)->asic_funcs->update_umd_stable_pstate ? (adev)->asic_funcs->update_umd_stable_pstate((adev), (enter)) : 0) -#define amdgpu_inc_vram_lost(adev) atomic_inc(&((adev)->vram_lost_counter)); +#define amdgpu_inc_vram_lost(adev) atomic_inc(&((adev)->vram_lost_counter)) /* Common functions */ bool amdgpu_device_has_job_running(struct amdgpu_device *adev); -- 2.18.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC] MAINTAINERS tag for cleanup robot
A difficult part of automating commits is composing the subsystem
preamble in the commit log. For the ongoing effort of a fixer producing
one or two fixes a release the use of 'treewide:' does not seem appropriate.
It would be better if the normal prefix was used. Unfortunately normal is
not consistent across the tree.
So I am looking for comments for adding a new tag to the MAINTAINERS file
D: Commit subsystem prefix
ex/ for FPGA DFL DRIVERS
D: fpga: dfl:
Continuing with cleaning up clang's -Wextra-semi-stmt
A significant number of warnings are caused by function like macros with
a trailing semicolon. For example.
#define FOO(a) a++; <-- extra, unneeded semicolon
void bar() {
int v = 0;
FOO(a);
}
Clang will warn at the FOO(a); expansion location. Instead of removing
the semicolon there, the fixer removes semicolon from the macro
definition. After the fixer, the code will be:
#define FOO(a) a++
void bar() {
int v = 0;
FOO(a);
}
The fixer review is
https://reviews.llvm.org/D91789
A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
The false positives are caused by macros passed to other macros and by
some macro expansions that did not have an extra semicolon.
This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
warnings in linux-next.
An update to [RFC] clang tooling cleanup
This change adds the clang-tidy-fix as a top level target and
uses it to do the cleaning. The next iteration will do a loop of
cleaners. This will mean breaking clang-tidy-fix out into its own
processing function 'run_fixers'.
Makefile: Add toplevel target clang-tidy-fix to makefile
Calls clang-tidy with -fix option for a set of checkers that
programatically fixes the kernel source in place, treewide.
Signed-off-by: Tom Rix
---
Makefile | 7 ---
scripts/clang-tools/run-clang-tools.py | 20 +---
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 47a8add4dd28..57756dbb767b 100644
--- a/Makefile
+++ b/Makefile
@@ -1567,20 +1567,21 @@ help:
echo ''
@echo 'Static analysers:'
@echo ' checkstack - Generate a list of stack hogs'
@echo ' versioncheck- Sanity check on version.h usage'
@echo ' includecheck- Check for duplicate included header files'
@echo ' export_report - List the usages of all exported symbols'
@echo ' headerdep - Detect inclusion cycles in headers'
@echo ' coccicheck - Check with Coccinelle'
@echo ' clang-analyzer - Check with clang static analyzer'
@echo ' clang-tidy - Check with clang-tidy'
+ @echo ' clang-tidy-fix - Check and fix with clang-tidy'
@echo ''
@echo 'Tools:'
@echo ' nsdeps - Generate missing symbol namespace
dependencies'
@echo ''
@echo 'Kernel selftest:'
@echo ' kselftest - Build and run kernel selftest'
@echo ' Build, install, and boot kernel before'
@echo ' running kselftest on it'
@echo ' Run as root for full coverage'
@echo ' kselftest-all - Build kernel selftest'
@@ -1842,30 +1843,30 @@ nsdeps: modules
quiet_cmd_gen_compile_commands = GEN $@
cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<,
$(real-prereqs))
$(extmod-prefix)compile_commands.json:
scripts/clang-tools/gen_compile_commands.py \
$(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
$(if $(CONFIG_MODULES), $(MODORDER)) FORCE
$(call if_changed,gen_compile_commands)
targets += $(extmod-prefix)compile_commands.json
-PHONY += clang-tidy clang-analyzer
+PHONY += clang-tidy-fix clang-tidy clang-analyzer
ifdef CONFIG_CC_IS_CLANG
quiet_cmd_clang_tools = CHECK $<
cmd_clang_tools = $(PYTHON3)
$(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<
-clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
+clang-tidy-fix clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
$(call cmd,clang_tools)
else
-clang-tidy clang-analyzer:
+clang-tidy-fix clang-tidy clang-analyzer:
@echo "$@ requires CC=clang" >&2
@false
endif
# Scripts to check various things for consistency
# ---
PHONY += includecheck versioncheck coccicheck export_report
includecheck:
diff --git a/scripts/clang-tools/run-clang-tools.py
b/scripts/clang-tools/run-clang-tools.py
index fa7655c7cec0..c177ca822c56 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -22,43 +22,57 @@ def parse_arguments():
Returns:
args: Dict of parsed args
Has keys: [path, type]
"""
usage = """Run clang-
[PATCH] drm/amd/display: remove unneeded semicolon
From: Tom Rix A semicolon is not needed after a switch statement. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 2 +- drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c index 7b4b2304bbff..5feb804af4be 100644 --- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c @@ -858,7 +858,7 @@ static struct clock_source *find_matching_pll( return pool->clock_sources[DCE112_CLK_SRC_PLL5]; default: return NULL; - }; + } return 0; } diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c index fb6a19d020f9..ee5230ccf3c4 100644 --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c @@ -280,6 +280,6 @@ char *mod_hdcp_state_id_to_str(int32_t id) return "D2_A9_VALIDATE_STREAM_READY"; default: return "UNKNOWN_STATE_ID"; - }; + } } -- 2.18.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: remove unneeded semicolon
From: Tom Rix A semicolon is not needed after a switch statement. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 1b213c4ddfcb..19c0a3655228 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -654,7 +654,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) default: return 0; - }; + } return ret; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 8bf6a7c056bc..a61cf8cfbfc3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -953,7 +953,7 @@ static char *amdgpu_ras_badpage_flags_str(unsigned int flags) case AMDGPU_RAS_RETIRE_PAGE_FAULT: default: return "F"; - }; + } } /** -- 2.18.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Subject: [RFC] clang tooling cleanups
This rfc will describe
An upcoming treewide cleanup.
How clang tooling was used to programatically do the clean up.
Solicit opinions on how to generally use clang tooling.
The clang warning -Wextra-semi-stmt produces about 10k warnings.
Reviewing these, a subset of semicolon after a switch looks safe to
fix all the time. An example problem
void foo(int a) {
switch(a) {
case 1:
...
}; <--- extra semicolon
}
Treewide, there are about 100 problems in 50 files for x86_64 allyesconfig.
These fixes will be the upcoming cleanup.
clang already supports fixing this problem. Add to your command line
clang -c -Wextra-semi-stmt -Xclang -fixit foo.c
foo.c:8:3: warning: empty expression statement has no effect;
remove unnecessary ';' to silence this warning [-Wextra-semi-stmt]
};
^
foo.c:8:3: note: FIX-IT applied suggested code changes
1 warning generated.
The big problem is using this treewide is it will fix all 10k problems.
10k changes to analyze and upstream is not practical.
Another problem is the generic fixer only removes the semicolon.
So empty lines with some tabs need to be manually cleaned.
What is needed is a more precise fixer.
Enter clang-tidy.
https://clang.llvm.org/extra/clang-tidy/
Already part of the static checker infrastructure, invoke on the clang
build with
make clang-tidy
It is only a matter of coding up a specific checker for the cleanup.
Upstream this is review is happening here
https://reviews.llvm.org/D90180
The development of a checker/fixer is
Start with a reproducer
void foo (int a) {
switch (a) {};
}
Generate the abstract syntax tree (AST)
clang -Xclang -ast-dump foo.c
`-FunctionDecl
|-ParmVarDecl
`-CompoundStmt
|-SwitchStmt
| |-ImplicitCastExpr
| | `-DeclRefExpr
| `-CompoundStmt
`-NullStmt
Write a matcher to get you most of the way
void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this);
}
The 'bind' method is important, it allows a string to be associated
with a node in the AST. In this case these are
`-FunctionDecl
|-ParmVarDecl
`-CompoundStmt < comp
|-SwitchStmt < switch
| |-ImplicitCastExpr
| | `-DeclRefExpr
| `-CompoundStmt
`-NullStmt
When a match is made the 'check' method will be called.
void SwitchSemiCheck::check(const MatchFinder::MatchResult &Result) {
auto *C = Result.Nodes.getNodeAs("comp");
auto *S = Result.Nodes.getNodeAs("switch");
This is where the string in the bind calls are changed to nodes
`-FunctionDecl
|-ParmVarDecl
`-CompoundStmt < comp, C
|-SwitchStmt < switch, S
| |-ImplicitCastExpr
| | `-DeclRefExpr
| `-CompoundStmt
`-NullStmt <-- looking for N
And then more logic to find the NullStmt
auto Current = C->body_begin();
auto Next = Current;
Next++;
while (Next != C->body_end()) {
if (*Current == S) {
if (const auto *N = dyn_cast(*Next)) {
When it is found, a warning is printed and a FixItHint is proposed.
auto H = FixItHint::CreateReplacement(
SourceRange(S->getBody()->getEndLoc(), N->getSemiLoc()), "}");
diag(N->getSemiLoc(), "unneeded semicolon") << H;
This fixit replaces from the end of switch to the semicolon with a
'}'. Because the end of the switch is '}' this has the effect of
removing all the whitespace as well as the semicolon.
Because of the checker's placement in clang-tidy existing linuxkernel
checkers, all that was needed to fix the tree was to add a '-fix'to the
build's clang-tidy call.
I am looking for opinions on what we want to do specifically with
cleanups and generally about other source-to-source programmatic
changes to the code base.
For cleanups, I think we need a new toplevel target
clang-tidy-fix
And an explicit list of fixers that have a very high (100%?) fix rate.
Ideally a bot should make the changes, but a bot could also nag folks.
Is there interest in a bot making the changes? Does one already exist?
The general source-to-source is a bit blue sky. Ex/ could automagicly
refactor api, outline similar cut-n-pasted functions etc. Anything on
someone's wishlist you want to try out ?
Signed-off-by: Tom Rix
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: remove unneeded break
From: Tom Rix
A break is not needed if it is preceded by a return or break
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/display/dc/dce/dce_transform.c | 1 -
drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 7 ---
drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 7 ---
drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 7 ---
drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c | 7 ---
drivers/gpu/drm/amd/display/dc/dce60/dce60_resource.c | 7 ---
drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c | 7 ---
7 files changed, 43 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
index 2a32b66959ba..130a0a0c8332 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c
@@ -1330,7 +1330,6 @@ static bool configure_graphics_mode(
REG_SET(OUTPUT_CSC_CONTROL, 0,
OUTPUT_CSC_GRPH_MODE, 0);
break;
- break;
case COLOR_SPACE_SRGB_LIMITED:
/* TV RGB */
REG_SET(OUTPUT_CSC_CONTROL, 0,
diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
index d741787f75dc..42c7d157da32 100644
--- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
@@ -418,25 +418,18 @@ static int map_transmitter_id_to_phy_instance(
switch (transmitter) {
case TRANSMITTER_UNIPHY_A:
return 0;
- break;
case TRANSMITTER_UNIPHY_B:
return 1;
- break;
case TRANSMITTER_UNIPHY_C:
return 2;
- break;
case TRANSMITTER_UNIPHY_D:
return 3;
- break;
case TRANSMITTER_UNIPHY_E:
return 4;
- break;
case TRANSMITTER_UNIPHY_F:
return 5;
- break;
case TRANSMITTER_UNIPHY_G:
return 6;
- break;
default:
ASSERT(0);
return 0;
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
index 2bbfa2e176a9..382581c4a674 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
@@ -471,25 +471,18 @@ static int map_transmitter_id_to_phy_instance(
switch (transmitter) {
case TRANSMITTER_UNIPHY_A:
return 0;
- break;
case TRANSMITTER_UNIPHY_B:
return 1;
- break;
case TRANSMITTER_UNIPHY_C:
return 2;
- break;
case TRANSMITTER_UNIPHY_D:
return 3;
- break;
case TRANSMITTER_UNIPHY_E:
return 4;
- break;
case TRANSMITTER_UNIPHY_F:
return 5;
- break;
case TRANSMITTER_UNIPHY_G:
return 6;
- break;
default:
ASSERT(0);
return 0;
diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
index b622b4b1dac3..7b4b2304bbff 100644
--- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
@@ -446,25 +446,18 @@ static int map_transmitter_id_to_phy_instance(
switch (transmitter) {
case TRANSMITTER_UNIPHY_A:
return 0;
- break;
case TRANSMITTER_UNIPHY_B:
return 1;
- break;
case TRANSMITTER_UNIPHY_C:
return 2;
- break;
case TRANSMITTER_UNIPHY_D:
return 3;
- break;
case TRANSMITTER_UNIPHY_E:
return 4;
- break;
case TRANSMITTER_UNIPHY_F:
return 5;
- break;
case TRANSMITTER_UNIPHY_G:
return 6;
- break;
default:
ASSERT(0);
return 0;
diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
index 16fe7344702f..3d782b7c86cb 100644
--- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
@@ -383,25 +383,18 @@ static int map_transmitter_id_to_phy_instance(
switch (transmitter) {
case TRANSMITTER_UNIPHY_A:
return 0;
- break;
case TRANSMITTER_UNIPHY_B:
return 1;
- break;
case TRANSMITTER_UNIPHY_C:
return 2;
- break;
case TRANSMITTER_UNIPHY_D:
return 3;
- break;
case TRANSMITTER_UNIPHY_E:
return
[RFC] treewide: cleanup unreachable breaks
From: Tom Rix
This is a upcoming change to clean up a new warning treewide.
I am wondering if the change could be one mega patch (see below) or
normal patch per file about 100 patches or somewhere half way by collecting
early acks.
clang has a number of useful, new warnings see
https://clang.llvm.org/docs/DiagnosticsReference.html
This change cleans up -Wunreachable-code-break
https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
The method of fixing was to look for warnings where the preceding statement
was a simple statement and by inspection made the subsequent break unneeded.
In order of frequency these look like
return and break
switch (c->x86_vendor) {
case X86_VENDOR_INTEL:
intel_p5_mcheck_init(c);
return 1;
- break;
goto and break
default:
operation = 0; /* make gcc happy */
goto fail_response;
- break;
break and break
case COLOR_SPACE_SRGB:
/* by pass */
REG_SET(OUTPUT_CSC_CONTROL, 0,
OUTPUT_CSC_GRPH_MODE, 0);
break;
- break;
The exception to the simple statement, is a switch case with a block
and the end of block is a return
struct obj_buffer *buff = r->ptr;
return scnprintf(str, PRIV_STR_SIZE,
"size=%u\naddr=0x%X\n", buff->size,
buff->addr);
}
- break;
Not considered obvious and excluded, breaks after
multi level switches
complicated if-else if-else blocks
panic() or similar calls
And there is an odd addition of a 'fallthrough' in drivers/tty/nozomi.c
Tom
---
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 1c08cb9eb9f6..16ce86aed8e2 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1809,15 +1809,13 @@ static int __mcheck_cpu_ancient_init(struct cpuinfo_x86
*c)
switch (c->x86_vendor) {
case X86_VENDOR_INTEL:
intel_p5_mcheck_init(c);
return 1;
- break;
case X86_VENDOR_CENTAUR:
winchip_mcheck_init(c);
return 1;
- break;
default:
return 0;
}
return 0;
diff --git a/arch/x86/kernel/cpu/microcode/amd.c
b/arch/x86/kernel/cpu/microcode/amd.c
index 3f6b137ef4e6..3d4a48336084 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -213,11 +213,10 @@ static unsigned int __verify_patch_size(u8 family, u32
sh_psize, size_t buf_size
max_size = F14H_MPB_MAX_SIZE;
break;
default:
WARN(1, "%s: WTF family: 0x%x\n", __func__, family);
return 0;
- break;
}
if (sh_psize > min_t(u32, buf_size, max_size))
return 0;
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 838b719ec7ce..d5411a166685 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -102,11 +102,10 @@ acpi_extract_package(union acpi_object *package,
printk(KERN_WARNING PREFIX "Invalid package
element"
" [%d]: got number, expecting"
" [%c]\n",
i, format_string[i]);
return AE_BAD_DATA;
- break;
}
break;
case ACPI_TYPE_STRING:
case ACPI_TYPE_BUFFER:
@@ -127,11 +126,10 @@ acpi_extract_package(union acpi_object *package,
printk(KERN_WARNING PREFIX "Invalid package
element"
" [%d] got string/buffer,"
" expecting [%c]\n",
i, format_string[i]);
return AE_BAD_DATA;
- break;
}
break;
case ACPI_TYPE_LOCAL_REFERENCE:
switch (format_string[i]) {
case 'R':
@@ -142,22 +140,20 @@ acpi_extract_package(union acpi_object *package,
printk(KERN_WARNING PREFIX "Invalid package
element"
" [%d] got reference,"
" expecting [%c]\n",
i, format_string[i]);
return AE_BAD_DATA;
- br
[PATCH] drm/amdgpu: add missing newline at eof
From: Tom Rix Representative checkpatch.pl warning WARNING: adding a line without newline at end of file 30: FILE: drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h:30: +#endif Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/include/asic_reg/gc/gc_9_4_1_sh_mask.h | 2 +- drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/include/asic_reg/gc/gc_9_4_1_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/gc/gc_9_4_1_sh_mask.h index f26246a600c6..4089cfa081f5 100644 --- a/drivers/gpu/drm/amd/include/asic_reg/gc/gc_9_4_1_sh_mask.h +++ b/drivers/gpu/drm/amd/include/asic_reg/gc/gc_9_4_1_sh_mask.h @@ -745,4 +745,4 @@ #define RLC_EDC_CNT2__RLC_SPM_SE7_SCRATCH_RAM_SEC_COUNT_MASK 0x3000L #define RLC_EDC_CNT2__RLC_SPM_SE7_SCRATCH_RAM_DED_COUNT_MASK 0xC000L -#endif \ No newline at end of file +#endif diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h index 29929b360db8..d8696e2274c4 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.h @@ -27,4 +27,4 @@ extern void vangogh_set_ppt_funcs(struct smu_context *smu); -#endif \ No newline at end of file +#endif -- 2.18.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/radeon: fix double free
From: Tom Rix
clang static analysis flags this error
drivers/gpu/drm/radeon/ci_dpm.c:5652:9: warning: Use of memory after it is
freed [unix.Malloc]
kfree(rdev->pm.dpm.ps[i].ps_priv);
^~
drivers/gpu/drm/radeon/ci_dpm.c:5654:2: warning: Attempt to free released
memory [unix.Malloc]
kfree(rdev->pm.dpm.ps);
^~
problem is reported in ci_dpm_fini, with these code blocks.
for (i = 0; i < rdev->pm.dpm.num_ps; i++) {
kfree(rdev->pm.dpm.ps[i].ps_priv);
}
kfree(rdev->pm.dpm.ps);
The first free happens in ci_parse_power_table where it cleans up locally
on a failure. ci_dpm_fini also does a cleanup.
ret = ci_parse_power_table(rdev);
if (ret) {
ci_dpm_fini(rdev);
return ret;
}
So remove the cleanup in ci_parse_power_table and
move the num_ps calculation to inside the loop so ci_dpm_fini
will know how many array elements to free.
Fixes: cc8dbbb4f62a ("drm/radeon: add dpm support for CI dGPUs (v2)")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/radeon/ci_dpm.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/ci_dpm.c b/drivers/gpu/drm/radeon/ci_dpm.c
index 86ac032275bb..ba20c6f03719 100644
--- a/drivers/gpu/drm/radeon/ci_dpm.c
+++ b/drivers/gpu/drm/radeon/ci_dpm.c
@@ -5563,6 +5563,7 @@ static int ci_parse_power_table(struct radeon_device
*rdev)
if (!rdev->pm.dpm.ps)
return -ENOMEM;
power_state_offset = (u8 *)state_array->states;
+ rdev->pm.dpm.num_ps = 0;
for (i = 0; i < state_array->ucNumEntries; i++) {
u8 *idx;
power_state = (union pplib_power_state *)power_state_offset;
@@ -5572,10 +5573,8 @@ static int ci_parse_power_table(struct radeon_device
*rdev)
if (!rdev->pm.power_state[i].clock_info)
return -EINVAL;
ps = kzalloc(sizeof(struct ci_ps), GFP_KERNEL);
- if (ps == NULL) {
- kfree(rdev->pm.dpm.ps);
+ if (ps == NULL)
return -ENOMEM;
- }
rdev->pm.dpm.ps[i].ps_priv = ps;
ci_parse_pplib_non_clock_info(rdev, &rdev->pm.dpm.ps[i],
non_clock_info,
@@ -5597,8 +5596,8 @@ static int ci_parse_power_table(struct radeon_device
*rdev)
k++;
}
power_state_offset += 2 + power_state->v2.ucNumDPMLevels;
+ rdev->pm.dpm.num_ps = i + 1;
}
- rdev->pm.dpm.num_ps = state_array->ucNumEntries;
/* fill in the vce power states */
for (i = 0; i < RADEON_MAX_VCE_LEVELS; i++) {
--
2.18.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
