[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