[PATCH] drm/nouveau/disp: fix an error problem in nvkm_uconn_new()
Clang Static Checker(scan-build) Warning:
drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c:line 215, column 4
Value stored to 'ret' is never read.
Return the error code rather than zero when 'conn->info.type' has an
unknown type.
Fixes: 8b7d92cad953 ("drm/nouveau/kms/nv50-: create connectors based on nvkm
info")
Signed-off-by: Su Hui
---
drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
index 2dab6612c4fc..6a67d307bf79 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
@@ -213,13 +213,14 @@ nvkm_uconn_new(const struct nvkm_oclass *oclass, void
*argv, u32 argc, struct nv
default:
WARN_ON(1);
ret = -EINVAL;
- break;
+ goto err;
}
nvkm_object_ctor(&nvkm_uconn, oclass, &conn->object);
*pobject = &conn->object;
ret = 0;
}
+err:
spin_unlock(&disp->client.lock);
return ret;
}
--
2.30.2
Re: [PATCH] drm/amdkfd: return negative error code in svm_ioctl()
On 2024/3/25 22:09, Philip Yang wrote: On 2024-03-25 02:31, Su Hui wrote: Good catch, ioctl should return -errno. I will apply it to drm-next. Reviewed-by: Philip Yang --- Ps: When I try to compile this file, there is a error : drivers/gpu/drm/amd/amdkfd/kfd_migrate.c:28:10: fatal error: amdgpu_sync.h: No such file or directory. Maybe there are some steps I missed or this place need to be corrected? Don't know how you compile the driver, amdgpu_sync.h is located under amdgpu folder, amdkfd/Makefile is included from amdgpu/Makefile, which set ccflag-y -I correctly. Got it, I should using 'make drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_migrate.o' rather than 'make drivers/gpu/drm/amd/amdkfd/kfd_migrate.o'. Thanks a lot! Regards, Su Hui
[PATCH] drm/amdkfd: return negative error code in svm_ioctl()
svm_ioctl() should return negative error code in default case.
Fixes: 42de677f7999 ("drm/amdkfd: register svm range")
Signed-off-by: Su Hui
---
Ps: When I try to compile this file, there is a error :
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c:28:10: fatal error: amdgpu_sync.h:
No such file or directory.
Maybe there are some steps I missed or this place need to be corrected?
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index f0f7f48af413..41c376f3fd27 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -4147,7 +4147,7 @@ svm_ioctl(struct kfd_process *p, enum kfd_ioctl_svm_op
op, uint64_t start,
r = svm_range_get_attr(p, mm, start, size, nattrs, attrs);
break;
default:
- r = EINVAL;
+ r = -EINVAL;
break;
}
--
2.30.2
[PATCH] backlight: ili922x: add an error code check in ili922x_write
Clang static analyzer complains that value stored to 'ret' is never read.
Return the error code when spi_sync() failed.
Signed-off-by: Su Hui
---
drivers/video/backlight/ili922x.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/video/backlight/ili922x.c
b/drivers/video/backlight/ili922x.c
index e7b6bd827986..47b872ac64a7 100644
--- a/drivers/video/backlight/ili922x.c
+++ b/drivers/video/backlight/ili922x.c
@@ -269,6 +269,10 @@ static int ili922x_write(struct spi_device *spi, u8 reg,
u16 value)
spi_message_add_tail(&xfer_regindex, &msg);
ret = spi_sync(spi, &msg);
+ if (ret < 0) {
+ dev_err(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
spi_message_init(&msg);
tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
--
2.30.2
[PATCH] drm/radeon/ni_dpm: add an error code check in ni_dpm_init
ni_patch_single_dependency_table_based_on_leakage() return zero or
negative error code. But ni_patch_dependency_tables_based_on_leakage()
doesn't check the return value of the first function call. It's same for
ni_dpm_init(). It's better to add this error code check.
Signed-off-by: Su Hui
---
drivers/gpu/drm/radeon/ni_dpm.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
index 3e1c1a392fb7..f521dc929a06 100644
--- a/drivers/gpu/drm/radeon/ni_dpm.c
+++ b/drivers/gpu/drm/radeon/ni_dpm.c
@@ -1010,6 +1010,8 @@ static int
ni_patch_dependency_tables_based_on_leakage(struct radeon_device *rde
ret = ni_patch_single_dependency_table_based_on_leakage(rdev,
&rdev->pm.dpm.dyn_state.vddc_dependency_on_sclk);
+ if (ret)
+ return ret;
ret = ni_patch_single_dependency_table_based_on_leakage(rdev,
&rdev->pm.dpm.dyn_state.vddc_dependency_on_mclk);
@@ -4098,7 +4100,12 @@ int ni_dpm_init(struct radeon_device *rdev)
rdev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries[3].clk =
72000;
rdev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries[3].v = 900;
- ni_patch_dependency_tables_based_on_leakage(rdev);
+ ret = ni_patch_dependency_tables_based_on_leakage(rdev);
+ if (ret) {
+
kfree(rdev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries);
+ r600_free_extended_power_table(rdev);
+ return ret;
+ }
if (rdev->pm.dpm.voltage_response_time == 0)
rdev->pm.dpm.voltage_response_time =
R600_VOLTAGERESPONSETIME_DFLT;
--
2.30.2
[PATCH v2] vga_switcheroo: Fix impossible judgment condition
'id' is enum type like unsigned int, so it will never be less than zero.
It's better to check VGA_SWITCHEROO_UNKNOWN_ID too.
Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound
GPU id")
Signed-off-by: Su Hui
---
v2:
- add check of VGA_SWITCHEROO_UNKNOWN_ID(Dan's suggestion).
By the way, all functions of 'get_client_id' will never return error code
or VGA_SWITCHEROO_UNKNOWN_ID，should we remove this check or keep it for
future.
drivers/gpu/vga/vga_switcheroo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 365e6ddbe90f..cf530094f929 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct pci_dev
*pdev,
mutex_lock(&vgasr_mutex);
if (vgasr_priv.active) {
id = vgasr_priv.handler->get_client_id(vga_dev);
- if (id < 0) {
+ if ((int)id < 0 || id == VGA_SWITCHEROO_UNKNOWN_ID) {
mutex_unlock(&vgasr_mutex);
return -EINVAL;
}
--
2.30.2
Re: [PATCH] vga_switcheroo: Fix impossible judgment condition
On 2023/10/26 12:44, Dan Carpenter wrote:
On Thu, Oct 26, 2023 at 10:10:57AM +0800, Su Hui wrote:
'id' is enum type like unsigned int, so it will never be less than zero.
Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound GPU
id")
Signed-off-by: Su Hui
---
drivers/gpu/vga/vga_switcheroo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 365e6ddbe90f..d3064466fd3a 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct pci_dev
*pdev,
mutex_lock(&vgasr_mutex);
if (vgasr_priv.active) {
id = vgasr_priv.handler->get_client_id(vga_dev);
- if (id < 0) {
+ if ((int)id < 0) {
Hi,
I feel like you're using Smatch? Which is great! Fantastic!
Yep, Smatch helps me a lot to find these bugs! I really like this
excellent tool!
Have you built the cross function database? If you have there is a
command that's useful.
Not yet, bu I want to build this.
$ ~/smatch/smatch_db/smdb.py functions vga_switcheroo_handler get_client_id |
tee where
drivers/gpu/drm/nouveau/nouveau_acpi.c | (struct
vga_switcheroo_handler)->get_client_id | nouveau_dsm_get_client_id | 1
drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | (struct
vga_switcheroo_handler)->get_client_id | amdgpu_atpx_get_client_id | 1
drivers/gpu/drm/radeon/radeon_atpx_handler.c | (struct
vga_switcheroo_handler)->get_client_id | radeon_atpx_get_client_id | 1
drivers/platform/x86/apple-gmux.c | (struct
vga_switcheroo_handler)->get_client_id | gmux_get_client_id | 1
$ make cscope
$ vim where
Use cscope to jump to each of those four functions. Move the cursor to
the nouveau_dsm_get_client_id and hit CTRL-].
Sounds great! I must try this!
They never return negatives. The enum vga_switcheroo_client_id has a
VGA_SWITCHEROO_UNKNOWN_ID define which I guess these functions are
supposed to return on error. They never do return that, but I bet
that's what we are supposed to check for. It honestly might be good
to check for both...
if ((int)id < 0 || id == VGA_SWITCHEROO_UNKNOWN_ID) {
mutex_unlock(&vgasr_mutex);
return -EINVAL;
}
Agreed, I will send v2 patch soon.
Really thanks for your wonderful suggestion! :)
Su Hui
regards,
dan carpenter
[PATCH] vga_switcheroo: Fix impossible judgment condition
'id' is enum type like unsigned int, so it will never be less than zero.
Fixes: 4aaf448fa975 ("vga_switcheroo: set audio client id according to bound
GPU id")
Signed-off-by: Su Hui
---
drivers/gpu/vga/vga_switcheroo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 365e6ddbe90f..d3064466fd3a 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -375,7 +375,7 @@ int vga_switcheroo_register_audio_client(struct pci_dev
*pdev,
mutex_lock(&vgasr_mutex);
if (vgasr_priv.active) {
id = vgasr_priv.handler->get_client_id(vga_dev);
- if (id < 0) {
+ if ((int)id < 0) {
mutex_unlock(&vgasr_mutex);
return -EINVAL;
}
--
2.30.2
[PATCH] drm/nouveau/nvif: avoid possible memory leak of 'args'
Use kfree() to free 'args' before return '-EINVAL'. Signed-off-by: Su Hui --- drivers/gpu/drm/nouveau/nvif/vmm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/nouveau/nvif/vmm.c b/drivers/gpu/drm/nouveau/nvif/vmm.c index 99296f03371a..a0afe3bf0d78 100644 --- a/drivers/gpu/drm/nouveau/nvif/vmm.c +++ b/drivers/gpu/drm/nouveau/nvif/vmm.c @@ -219,6 +219,7 @@ nvif_vmm_ctor(struct nvif_mmu *mmu, const char *name, s32 oclass, case RAW: args->type = NVIF_VMM_V0_TYPE_RAW; break; default: WARN_ON(1); + kfree(args); return -EINVAL; } -- 2.30.2
Re: [PATCH v2] drm/amdgpu: Avoid possible buffer overflow
On 2023/8/21 17:31, Christian König wrote:
Am 21.08.23 um 09:37 schrieb Su Hui:
smatch error:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257
amdgpu_discovery_reg_base_init() error:
testing array offset 'adev->vcn.num_vcn_inst' after use.
change the assignment order to avoid buffer overflow.
Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV")
Signed-off-by: Su Hui
---
changes in v2:
- fix the error about ip->revision (thanks to Christophe JAILLET).
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 8e1cfc87122d..b07bfd106a9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1250,11 +1250,10 @@ static int
amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
* 0b10 : encode is disabled
* 0b01 : decode is disabled
*/
- adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
- ip->revision & 0xc0;
- ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+ adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+ ip->revision & 0xc0;
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
@@ -1265,6 +1264,7 @@ static int
amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
adev->vcn.num_vcn_inst + 1,
AMDGPU_MAX_VCN_INSTANCES);
}
+ ip->revision &= ~0xc0;
That doesn't looks correct either. The assignment is intentionally
outside of the "if".
See "adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = ip->revision &
0xc0;" is always valid.
Hi,
if "adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = ip->revision & 0xc0;"
is always valid, then
"adev->vcn.num_vcn_inst< AMDGPU_MAX_VCN_INSTANCES " is always true. So
the below judgement has
no sense.
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
On the contrary, if we need this judgement, then
"adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = ip->revision & 0xc0;"is not
always valid, because "adev->vcn.num_vcn_inst >=
AMDGPU_MAX_VCN_INSTANCES" can be true, which cause buffer overflow.
So I think this patch has some sense if I don't make some mistakes.
Su Hui
We just avoid incrementing num_vcn_inst when we already have to many.
Regards,
Christian.
}
if (le16_to_cpu(ip->hw_id) == SDMA0_HWID ||
le16_to_cpu(ip->hw_id) == SDMA1_HWID ||
[PATCH v2] drm/amdgpu: Avoid possible buffer overflow
smatch error:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257
amdgpu_discovery_reg_base_init() error:
testing array offset 'adev->vcn.num_vcn_inst' after use.
change the assignment order to avoid buffer overflow.
Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV")
Signed-off-by: Su Hui
---
changes in v2:
- fix the error about ip->revision (thanks to Christophe JAILLET).
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 8e1cfc87122d..b07bfd106a9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1250,11 +1250,10 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
* 0b10 : encode is disabled
* 0b01 : decode is disabled
*/
- adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
- ip->revision & 0xc0;
- ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+
adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+ ip->revision & 0xc0;
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
@@ -1265,6 +1264,7 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
adev->vcn.num_vcn_inst + 1,
AMDGPU_MAX_VCN_INSTANCES);
}
+ ip->revision &= ~0xc0;
}
if (le16_to_cpu(ip->hw_id) == SDMA0_HWID ||
le16_to_cpu(ip->hw_id) == SDMA1_HWID ||
--
2.30.2
Re: [PATCH] drm/ast: Avoid possible NULL dereference
On 2023/8/21 15:04, Christophe JAILLET wrote:
Le 21/08/2023 à 08:22, Su Hui a écrit :
smatch error:
drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error:
we previously assumed 'ast->dp501_fw' could be null (see line 223)
when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true,
there will be a NULL dereference of 'ast->dp501_fw'.
Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when
required")
Signed-off-by: Su Hui
---
drivers/gpu/drm/ast/ast_dp501.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c
b/drivers/gpu/drm/ast/ast_dp501.c
index 1bc35a992369..d9f3a0786a6f 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev)
ast_load_dp501_microcode(dev) < 0)
return false;
- fw_addr = (u8 *)ast->dp501_fw->data;
- len = ast->dp501_fw->size;
+ if (ast->dp501_fw) {
+ fw_addr = (u8 *)ast->dp501_fw->data;
+ len = ast->dp501_fw->size;
+ }
}
/* Get BootAddress */
ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
Hi,
this looks like a false positive.
If "!ast->dp501_fw", and ast_load_dp501_microcode()>=0, then
ast_load_dp501_microcode() will set a valid value in ast->dp501_fw.
Hi,
Sorry for the noise, you are right, this is a false positive.
I will take more careful next time.
Su Hui
CJ
Re: [PATCH] drm/amdgpu: Avoid possible buffer overflow
On 2023/8/21 14:47, Christophe JAILLET wrote:
Le 21/08/2023 à 08:19, Su Hui a écrit :
smatch error:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257
amdgpu_discovery_reg_base_init() error:
testing array offset 'adev->vcn.num_vcn_inst' after use.
change the assignment order to avoid buffer overflow.
Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV")
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 8e1cfc87122d..ba95526c3d45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1250,11 +1250,12 @@ static int
amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
* 0b10 : encode is disabled
* 0b01 : decode is disabled
*/
- adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
- ip->revision & 0xc0;
+
ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+ adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+ ip->revision & 0xc0;
Hi,
I don't think that the patch is correct.
Because of the "ip->revision &= ~0xc0" which is now above it,
"ip->revision & 0xc0" should now always be 0.
Maybe both lines should be moved within the "if"?
CJ
Hi,
Oh, really sorry for this, I just missed this line.
you are right, I will send v2 soon.
Thanks for your reminder!
Su Hui
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
[PATCH] drm/ast: Avoid possible NULL dereference
smatch error:
drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error:
we previously assumed 'ast->dp501_fw' could be null (see line 223)
when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true,
there will be a NULL dereference of 'ast->dp501_fw'.
Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when required")
Signed-off-by: Su Hui
---
drivers/gpu/drm/ast/ast_dp501.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 1bc35a992369..d9f3a0786a6f 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev)
ast_load_dp501_microcode(dev) < 0)
return false;
- fw_addr = (u8 *)ast->dp501_fw->data;
- len = ast->dp501_fw->size;
+ if (ast->dp501_fw) {
+ fw_addr = (u8 *)ast->dp501_fw->data;
+ len = ast->dp501_fw->size;
+ }
}
/* Get BootAddress */
ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
--
2.30.2
[PATCH] drm/amdgpu: Avoid possible buffer overflow
smatch error:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257
amdgpu_discovery_reg_base_init() error:
testing array offset 'adev->vcn.num_vcn_inst' after use.
change the assignment order to avoid buffer overflow.
Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV")
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 8e1cfc87122d..ba95526c3d45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1250,11 +1250,12 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
* 0b10 : encode is disabled
* 0b01 : decode is disabled
*/
- adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
- ip->revision & 0xc0;
+
ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+
adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+ ip->revision & 0xc0;
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
--
2.30.2
Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
On 2023/7/25 13:51, Dan Carpenter wrote:
The reason why the first five attempts had bugs is because we are
trying to write it in the most complicated way possible, shifting by
logical not what?
Wonderful! Should I add your name as signed-of-by?
I will send a v3 patch later.
Really thanks for your help!
Su Hui
regards,
dan carpenter
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..6997b6cb1df2 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,13 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ int div = tv_mode->oversample;
+
+ if (!tv_mode->progressive)
+ div >>= 1;
+ if (div == 0)
+ div = 1;
+ mode->clock = clock / div;
/*
* tv_mode horizontal timings:
@@ -1135,6 +1141,8 @@ intel_tv_get_config(struct intel_encoder *encoder,
break;
default:
tv_mode.oversample = 1;
+ WARN_ON_ONCE(!tv_mode.progressive);
+ tv_mode.progressive = true;
break;
}
Re: [Intel-gfx] [PATCH v2] drm/i915/tv: avoid possible division by zero
On 2023/7/25 01:35, Andi Shyti wrote:
On Tue, Jul 18, 2023 at 09:32:17AM +0800, Su Hui wrote:
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..f59553f7c132 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,7 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
but this does not provide the same value. Try with:
8 / (2 >> 1)
and
8 / 2 << 1
They are definitely different.
The real check could be:
if (!(tv_mode->oversample >> 1))
return ...
But first I would check if that's actually possible.
Oh, I have a v3 patch, like this:
- mode->clock = clock / (tv_mode->oversample >>
!tv_mode->progressive);
+ mode->clock = clock;
+ if (tv_mode->oversample >> !tv_mode->progressive)
+ mode->clock /= tv_mode->oversample >> 1;
But I'm not sure does it need to print some error messages or do some
things when
"tv_mode->oversample << !tv_mode->progressive" is zero?
If all right , I will send this v3 patch.
Su Hui
Andi
Re: [PATCH] drm/i915/tv: avoid possible division by zero
On 2023/7/18 19:28, Andrzej Hajda wrote:
On 18.07.2023 12:10, Su Hui wrote:
On 2023/7/18 13:39, Dan Carpenter wrote:
On Mon, Jul 17, 2023 at 04:52:51PM +0200, Andrzej Hajda wrote:
On 17.07.2023 08:22, Su Hui wrote:
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..82b54af51f23 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,8 @@ intel_tv_mode_to_mode(struct drm_display_mode
*mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >>
!tv_mode->progressive);
+ mode->clock = clock / (tv_mode->oversample != 1 ?
+ tv_mode->oversample >> !tv_mode->progressive : 1);
Seems too smart to me, why not just:
mode->clock = clock / tv_mode->oversample;
if (!tv_mode->progressive)
mode->clock <<= 1;
This is nice.
mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
But I think this one is much better, it has less code and run faster.
Should I resend v3 to add some explanation or follow Dan's advice?
Speed gain here is irrelevant here, and disputable.
One thing which could be problematic is that we could loose the least
significant bit in mode->clock,
in case non-progressive, but I am not sure if it really matters, as
mode->clock is not precise value anyway.
Alternatively we could 1st shift, then divide, but in this case
overflow can occur, at least in theory - I suspect there are no such
big clocks (in kHz).
Finally I would agree with Dan, readability is better with conditional.
How about this one?
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock;
+ if (tv_mode->oversample >> !tv_mode->progressive)
+ mode->clock /= tv_mode->oversample >> 1;
Prevent loss of accuracy and also make it more readable.
If it's OK, I will send v3 patch.
By the way, do we need to print some error messages or do some things when
"tv_mode->oversample << !tv_mode->progressive" is zero? I'm not sure about
this.
Su Hui
Regards
Andrzej
Re: [PATCH] drm/i915/tv: avoid possible division by zero
On 2023/7/18 13:39, Dan Carpenter wrote:
On Mon, Jul 17, 2023 at 04:52:51PM +0200, Andrzej Hajda wrote:
On 17.07.2023 08:22, Su Hui wrote:
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..82b54af51f23 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,8 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock / (tv_mode->oversample != 1 ?
+ tv_mode->oversample >> !tv_mode->progressive : 1);
Seems too smart to me, why not just:
mode->clock = clock / tv_mode->oversample;
if (!tv_mode->progressive)
mode->clock <<= 1;
This is nice.
mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
But I think this one is much better, it has less code and run faster.
Should I resend v3 to add some explanation or follow Dan's advice?
Su Hui
regards,
dan carpenter
[PATCH v2] drm/i915/tv: avoid possible division by zero
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..f59553f7c132 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,7 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
/*
* tv_mode horizontal timings:
--
2.30.2
Re: [PATCH] drm/i915/tv: avoid possible division by zero
On 2023/7/17 22:52, Andrzej Hajda wrote:
On 17.07.2023 08:22, Su Hui wrote:
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..82b54af51f23 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,8 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >>
!tv_mode->progressive);
+ mode->clock = clock / (tv_mode->oversample != 1 ?
+ tv_mode->oversample >> !tv_mode->progressive : 1);
Seems too smart to me, why not just:
mode->clock = clock / tv_mode->oversample;
if (!tv_mode->progressive)
mode->clock <<= 1;
Or trying being smart:
mode->clock = clock / tv_mode->oversample << !tv_mode->progressive;
Hi,
Yes, this is more readable and clear.
Thanks four your advice. I will send v2 soon.
Su Hui
Btw in both cases there is assumption tv_mode->oversample != 0, I
guess it is true.
Regards
Andrzej
/*
* tv_mode horizontal timings:
[PATCH] drm/i915/tv: avoid possible division by zero
Clang warning: drivers/gpu/drm/i915/display/intel_tv.c:
line 991, column 22 Division by zero.
Assuming tv_mode->oversample=1 and (!tv_mode->progressive)=1,
then division by zero will happen.
Fixes: 1bba5543e4fe ("drm/i915: Fix TV encoder clock computation")
Signed-off-by: Su Hui
---
drivers/gpu/drm/i915/display/intel_tv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c
b/drivers/gpu/drm/i915/display/intel_tv.c
index 36b479b46b60..82b54af51f23 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -988,7 +988,8 @@ intel_tv_mode_to_mode(struct drm_display_mode *mode,
const struct tv_mode *tv_mode,
int clock)
{
- mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
+ mode->clock = clock / (tv_mode->oversample != 1 ?
+ tv_mode->oversample >> !tv_mode->progressive : 1);
/*
* tv_mode horizontal timings:
--
2.30.2
Re: [PATCH] drm/virtio: remove some redundant code
On 2023/7/12 14:36, Dan Carpenter wrote: On Wed, Jul 12, 2023 at 09:18:42AM +0800, Su Hui wrote: On 2023/7/11 19:13, Dan Carpenter wrote: On Tue, Jul 11, 2023 at 05:00:31PM +0800, Su Hui wrote: virtio_gpu_get_vbuf always be successful, so remove the error judgment. No, just ignore the static checker false positive in this case. The intent of the code is clear that if it did have an error it should return an error pointer. Hi, Dan, Function "virtio_gpu_get_vbuf" call "kmem_cache_zalloc (vgdev->vbufs, GFP_KERNEL | __GFP_NOFAIL)" to allocate memory. Adding the " __GFP_NOFAIL”flag make sure it won't fail. And "virtio_gpu_get_vbuf" never return an error code, so I think this is not a false positive. We all see this and agree. However the check for if (IS_ERR()) is written deliberately because we might change the code to return error pointers in the future. Static checkers are looking for code that does something unintentional but in this case the code was written that way deliberately. Got it , I shouldn't remove it because the check may be useful in the future. Thanks for your explanation. Su Hui regards, dan carpenter
Re: [PATCH] drm/virtio: remove some redundant code
On 2023/7/11 17:33, Markus Elfring wrote: virtio_gpu_get_vbuf always be successful, so remove the error judgment. How do you think about to improve this change description any more? Hi, virtio_gpu_get_vbuf use "__GFP_NOFAIL" flag to allocate memory, this make sure it won't fail, and virtio_gpu_get_vbuf never return error code, so remove the error judgment. How about this one? Thanks for your advice. Su Hui Regards, Markus
Re: [PATCH] drm/virtio: remove some redundant code
On 2023/7/11 19:13, Dan Carpenter wrote: On Tue, Jul 11, 2023 at 05:00:31PM +0800, Su Hui wrote: virtio_gpu_get_vbuf always be successful, so remove the error judgment. No, just ignore the static checker false positive in this case. The intent of the code is clear that if it did have an error it should return an error pointer. Hi, Dan, Function "virtio_gpu_get_vbuf" call "kmem_cache_zalloc (vgdev->vbufs, GFP_KERNEL | __GFP_NOFAIL)" to allocate memory. Adding the " __GFP_NOFAIL”flag make sure it won't fail. And "virtio_gpu_get_vbuf" never return an error code, so I think this is not a false positive. Su Hui regards, dan carpenter
[PATCH] drm/virtio: remove some redundant code
virtio_gpu_get_vbuf always be successful,
so remove the error judgment.
Signed-off-by: Su Hui
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index b1a00c0c25a7..7a2680b3f1a7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -129,10 +129,6 @@ virtio_gpu_alloc_cursor(struct virtio_gpu_device *vgdev,
vbuf = virtio_gpu_get_vbuf
(vgdev, sizeof(struct virtio_gpu_update_cursor),
0, NULL, NULL);
- if (IS_ERR(vbuf)) {
- *vbuffer_p = NULL;
- return ERR_CAST(vbuf);
- }
*vbuffer_p = vbuf;
return (struct virtio_gpu_update_cursor *)vbuf->buf;
}
--
2.30.2
Re: [PATCH] drm/amd/amdgpu: Use “__packed“ instead of "pragma pack()"
On 2023/6/21 14:11, Dan Carpenter wrote:
When there was a #pragma then Sparse just turned off. The Sparse
warnings are places where people forgot to put the __user in their casts
or didn't annotate endianness correctly. It's not a "bug" to forget
to annotate endianness or user pointers. That's how we used to do it
prior to 2003. But these days it feels strange and dangerous to see
these sorts of warnings.
Got it. And it is really strange when I first saw these warnings.
Thanks for your explanation!
Su Hui
Smatch also disabled some uninitialized variable checks. These are
mostly false positives where we have a loop:
int r;
while (something) {
r = frob();
}
return r;
Smatch complains that we don't necessarily enter the loop. I think
I'm going to disable this type of "enter the loop" warning when you
don't have the cross function database available. That will silence
these for the kbuild bot.
regards,
dan carpenter
[PATCH] drm/amd/amdgpu: Use “__packed“ instead of "pragma pack()"
use "__packed" is clearer amd better than “pragma pack()”.
Signed-off-by: Su Hui
---
As Dan Carpenter mentioned:
'"Mark the associated types properly packed individually, rather than
use the disgusting "pragma pack()" that should never be used."
https://lore.kernel.org/linux-sparse/CAHk-=wi7jgz+bvbt-ufxokpeqdhzf3z2hbjkgdjh8q4dvpp...@mail.gmail.com/'
use "__packed" is better.
the previous wrong patch's address:
https://lore.kernel.org/kernel-janitors/c12c4031-52fb-25a2-b411-e668eb9ba...@tom.com/T/#t
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index 24d42d24e6a0..025adc950026 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -83,8 +83,6 @@ enum amd_sriov_ucode_engine_id {
AMD_SRIOV_UCODE_ID__MAX
};
-#pragma pack(push, 1) // PF2VF / VF2PF data areas are byte packed
-
union amd_sriov_msg_feature_flags {
struct {
uint32_t error_log_collect : 1;
@@ -210,7 +208,7 @@ struct amd_sriov_msg_pf2vf_info {
uint32_t pcie_atomic_ops_support_flags;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_PF2VF_INFO_FILLED_SIZE];
-};
+} __packed;
struct amd_sriov_msg_vf2pf_info_header {
/* the total structure size in byte */
@@ -263,7 +261,7 @@ struct amd_sriov_msg_vf2pf_info {
struct {
uint8_t id;
uint32_t version;
- } ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
+ } __packed ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
uint64_t dummy_page_addr;
/* reserved */
@@ -301,8 +299,6 @@ enum amd_sriov_gpu_init_data_version {
GPU_INIT_DATA_READY_V1 = 1,
};
-#pragma pack(pop) // Restore previous packing option
-
/* checksum function between host and guest */
unsigned int amd_sriov_msg_checksum(void *obj, unsigned long obj_size,
unsigned int key,
unsigned int checksum);
--
2.30.2
[PATCH] drm/amd/amdgpu: Properly tune the size of struct
Smatch error:
gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:316:49: error:
static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB"
static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB"
Fixes: 1721bc1b2afa ("drm/amdgpu: Update VF2PF interface")
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index 24d42d24e6a0..a482b422fed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -177,10 +177,10 @@ struct amd_sriov_msg_pf2vf_info {
uint64_t mecfw_offset;
/* MEC FW size in BYTE */
uint32_t mecfw_size;
- /* UVD FW position in BYTE from the start of VF visible frame buffer */
- uint64_t uvdfw_offset;
/* UVD FW size in BYTE */
uint32_t uvdfw_size;
+ /* UVD FW position in BYTE from the start of VF visible frame buffer */
+ uint64_t uvdfw_offset;
/* VCE FW position in BYTE from the start of VF visible frame buffer */
uint64_t vcefw_offset;
/* VCE FW size in BYTE */
@@ -193,8 +193,8 @@ struct amd_sriov_msg_pf2vf_info {
/* frequency for VF to update the VF2PF area in msec, 0 = manual */
uint32_t vf2pf_update_interval_ms;
/* identification in ROCm SMI */
- uint64_t uuid;
uint32_t fcn_idx;
+ uint64_t uuid;
/* flags to indicate which register access method VF should use */
union amd_sriov_reg_access_flags reg_access_flags;
/* MM BW management */
@@ -263,7 +263,7 @@ struct amd_sriov_msg_vf2pf_info {
struct {
uint8_t id;
uint32_t version;
- } ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
+ } __packed ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
uint64_t dummy_page_addr;
/* reserved */
--
2.30.2
[PATCH v3] drm/bridge: ti-sn65dsi86: Avoid possible buffer overflow
Smatch error:buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5.
Fixes: cea86c5bb442 ("drm/bridge: ti-sn65dsi86: Implement the pwm_chip")
Signed-off-by: Su Hui
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 7a748785c545..4676cf2900df 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -298,6 +298,10 @@ static void ti_sn_bridge_set_refclk_freq(struct
ti_sn65dsi86 *pdata)
if (refclk_lut[i] == refclk_rate)
break;
+ /* avoid buffer overflow and "1" is the default rate in the datasheet.
*/
+ if (i >= refclk_lut_size)
+ i = 1;
+
regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
REFCLK_FREQ(i));
--
2.30.2
Re: [PATCH v2] drm/bridge: ti-sn65dsi86: Avoid possible buffer overflow
On 2023/6/7 22:03, Doug Anderson wrote:
Hi,
On Tue, Jun 6, 2023 at 6:25 PM Su Hui wrote:
Smatch error:buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5.
Fixes: cea86c5bb442 ("drm/bridge: ti-sn65dsi86: Implement the pwm_chip")
Signed-off-by: Su Hui
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 7a748785c545..bb88406495e9 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -305,7 +305,7 @@ static void ti_sn_bridge_set_refclk_freq(struct
ti_sn65dsi86 *pdata)
* The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
* regardless of its actual sourcing.
*/
- pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
+ pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i < refclk_lut_size ?
i : 1];
This looks more correct, but it really needs a comment since it's
totally not obviously what you're doing here. IMO the best solution
here is to update "i" right after the for loop and have a comment
about the datasheet saying that "1" is the default rate so we'll fall
back to that if we couldn't find a match. Moving it to right after the
for loop will change the value written into the registers, but that's
fine and makes it clearer what's happening.
Got it. Add some comment and move the code up.
I will send patch v3 soon.
Thanks for your suggestion again :) .
Su Hui
-Doug
[PATCH v2] drm/bridge: ti-sn65dsi86: Avoid possible buffer overflow
Smatch error:buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5.
Fixes: cea86c5bb442 ("drm/bridge: ti-sn65dsi86: Implement the pwm_chip")
Signed-off-by: Su Hui
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 7a748785c545..bb88406495e9 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -305,7 +305,7 @@ static void ti_sn_bridge_set_refclk_freq(struct
ti_sn65dsi86 *pdata)
* The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
* regardless of its actual sourcing.
*/
- pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
+ pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i < refclk_lut_size ?
i : 1];
}
static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata)
--
2.30.2
Re: [PATCH] drm/bridge: ti-sn65dsi86: Avoid possible buffer overflow
Hi,
On 2023/6/6 23:28, Doug Anderson wrote:
Hi,
On Tue, Jun 6, 2023 at 12:56 AM Su Hui wrote:
Smatch error:buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5.
Fixes: cea86c5bb442 ("drm/bridge: ti-sn65dsi86: Implement the pwm_chip")
Signed-off-by: Su Hui
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 7a748785c545..952aae4221e7 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -305,7 +305,8 @@ static void ti_sn_bridge_set_refclk_freq(struct
ti_sn65dsi86 *pdata)
* The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
* regardless of its actual sourcing.
*/
- pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
+ if (i < refclk_lut_size)
+ pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
I don't think this is quite the right fix. I don't think we can just
skip assigning "pdata->pwm_refclk_freq". In general I think we're in
pretty bad shape if we ever fail to match a refclk from the table and
I'm not quite sure how the bridge chip could work at all in this case.
Probably that at least deserves a warning message in the logs. There's
no place to return an error though, so I guess the warning is the best
we can do and then we can do our best to do something reasonable.
In this case, I think "reasonable" might be that if the for loop exits
and "i == refclk_lut_size" that we should set "i" to 1. According to
the datasheet [1] setting a value of 5 (which the existing code does)
is the same as setting a value of 1 (the default) and if it's 1 then
we'll be able to look this up in the table.
I think you are right, set i to 1 if i >= refclk_lut_size. I will resend
this patch soon.
Thanks for your reply!
Su Hui
[1] https://www.ti.com/lit/gpn/sn65dsi86
-Doug
[PATCH] drm/bridge: ti-sn65dsi86: Avoid possible buffer overflow
Smatch error:buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5.
Fixes: cea86c5bb442 ("drm/bridge: ti-sn65dsi86: Implement the pwm_chip")
Signed-off-by: Su Hui
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 7a748785c545..952aae4221e7 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -305,7 +305,8 @@ static void ti_sn_bridge_set_refclk_freq(struct
ti_sn65dsi86 *pdata)
* The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
* regardless of its actual sourcing.
*/
- pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
+ if (i < refclk_lut_size)
+ pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
}
static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata)
--
2.30.2
Re: [PATCH] drm/nouveau/nvif: use struct_size()
On 2023/5/31 16:31, Dan Carpenter wrote:
On Wed, May 31, 2023 at 12:38:26PM +0800, Su Hui wrote:
Use struct_size() instead of hand writing it.
This is less verbose and more informative.
Signed-off-by: Su Hui
---
drivers/gpu/drm/nouveau/nvif/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..4bd693aa4ee0 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -65,7 +65,7 @@ nvif_object_sclass_get(struct nvif_object *object, struct
nvif_sclass **psclass)
u32 size;
while (1) {
- size = sizeof(*args) + cnt * sizeof(args->sclass.oclass[0]);
+ size = struct_size(args, sclass.oclass, cnt);
This is from the original code, but now that you are using the
struct_size() macro static checkers will complain about it. (Maybe they
don't yet?). size is a u32. Never save struct_size() returns to
anything except unsigned long or size_t. (ssize_t is also fine, I
suppose). Otherwise, you do not benefit from the integer overflow
checking.
Sorry, I don't notice the issue caused by type size.
You are right, this patch is wrong because of the type mismatch.
Thanks for your reply!
Su Hui
if (!(args = kmalloc(size, GFP_KERNEL)))
return -ENOMEM;
args->ioctl.version = 0;
regards,
dan carpenter
[PATCH] drm/nouveau/nvif: use struct_size()
Use struct_size() instead of hand writing it.
This is less verbose and more informative.
Signed-off-by: Su Hui
---
drivers/gpu/drm/nouveau/nvif/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..4bd693aa4ee0 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -65,7 +65,7 @@ nvif_object_sclass_get(struct nvif_object *object, struct
nvif_sclass **psclass)
u32 size;
while (1) {
- size = sizeof(*args) + cnt * sizeof(args->sclass.oclass[0]);
+ size = struct_size(args, sclass.oclass, cnt);
if (!(args = kmalloc(size, GFP_KERNEL)))
return -ENOMEM;
args->ioctl.version = 0;
--
2.30.2
Re: [PATCH] drm: Remove unnecessary (void*) conversions
On 2023/5/26 15:27, Christian König wrote:
Am 26.05.23 um 05:32 schrieb Su Hui:
Pointer variables of (void*) type do not require type cast.
Please split that up by subsystem/driver. Taking it through the misc
tree might just cause merge conflicts.
Sorry for that, I will split it and send again.
Thanks for your reply!
Su Hui
Christian.
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +-
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_debugfs.c | 2 +-
drivers/gpu/drm/omapdrm/omap_debugfs.c | 6 +++---
drivers/gpu/drm/pl111/pl111_debugfs.c | 2 +-
drivers/gpu/drm/qxl/qxl_debugfs.c | 4 ++--
drivers/gpu/drm/tiny/arcpgu.c | 2 +-
drivers/gpu/drm/ttm/ttm_resource.c | 3 +--
drivers/gpu/drm/virtio/virtgpu_debugfs.c | 6 +++---
drivers/gpu/drm/vmwgfx/ttm_object.c | 5 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 +-
12 files changed, 19 insertions(+), 21 deletions(-)
diff --git
a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 827fcb4fb3b3..8a2c39927167 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -3312,7 +3312,7 @@ static ssize_t dtn_log_write(
static int mst_topo_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 58c2246918fd..e6c870bd307b 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -3671,7 +3671,7 @@ static void amdgpu_parse_cg_state(struct
seq_file *m, u64 flags)
static int amdgpu_debugfs_pm_info_show(struct seq_file *m, void
*unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
u64 flags = 0;
int r;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 31a7f59ccb49..dd57f7164e9a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -198,7 +198,7 @@ static int etnaviv_ring_show(struct etnaviv_gpu
*gpu, struct seq_file *m)
static int show_unlocked(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
int (*show)(struct drm_device *dev, struct seq_file *m) =
node->info_ent->data;
@@ -208,7 +208,7 @@ static int show_unlocked(struct seq_file *m, void
*arg)
static int show_each_gpu(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct etnaviv_drm_private *priv = dev->dev_private;
struct etnaviv_gpu *gpu;
diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 2a36d1ca8fda..96b59d5d68ed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -37,7 +37,7 @@
static int
nouveau_debugfs_vbios_image(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct nouveau_drm *drm = nouveau_drm(node->minor->dev);
int i;
diff --git a/drivers/gpu/drm/omapdrm/omap_debugfs.c
b/drivers/gpu/drm/omapdrm/omap_debugfs.c
index a3d470468e5b..a94ce502e152 100644
--- a/drivers/gpu/drm/omapdrm/omap_debugfs.c
+++ b/drivers/gpu/drm/omapdrm/omap_debugfs.c
@@ -19,7 +19,7 @@
static int gem_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct omap_drm_private *priv = dev->dev_private;
@@ -33,7 +33,7 @@ static int gem_show(struct seq_file *m, void *arg)
static int mm_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
st
[PATCH] drm: Remove unnecessary (void*) conversions
Pointer variables of (void*) type do not require type cast.
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +-
drivers/gpu/drm/amd/pm/amdgpu_pm.c| 2 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_debugfs.c | 2 +-
drivers/gpu/drm/omapdrm/omap_debugfs.c| 6 +++---
drivers/gpu/drm/pl111/pl111_debugfs.c | 2 +-
drivers/gpu/drm/qxl/qxl_debugfs.c | 4 ++--
drivers/gpu/drm/tiny/arcpgu.c | 2 +-
drivers/gpu/drm/ttm/ttm_resource.c| 3 +--
drivers/gpu/drm/virtio/virtgpu_debugfs.c | 6 +++---
drivers/gpu/drm/vmwgfx/ttm_object.c | 5 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 +-
12 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 827fcb4fb3b3..8a2c39927167 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -3312,7 +3312,7 @@ static ssize_t dtn_log_write(
static int mst_topo_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 58c2246918fd..e6c870bd307b 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -3671,7 +3671,7 @@ static void amdgpu_parse_cg_state(struct seq_file *m, u64
flags)
static int amdgpu_debugfs_pm_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
u64 flags = 0;
int r;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 31a7f59ccb49..dd57f7164e9a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -198,7 +198,7 @@ static int etnaviv_ring_show(struct etnaviv_gpu *gpu,
struct seq_file *m)
static int show_unlocked(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
int (*show)(struct drm_device *dev, struct seq_file *m) =
node->info_ent->data;
@@ -208,7 +208,7 @@ static int show_unlocked(struct seq_file *m, void *arg)
static int show_each_gpu(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct etnaviv_drm_private *priv = dev->dev_private;
struct etnaviv_gpu *gpu;
diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 2a36d1ca8fda..96b59d5d68ed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -37,7 +37,7 @@
static int
nouveau_debugfs_vbios_image(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct nouveau_drm *drm = nouveau_drm(node->minor->dev);
int i;
diff --git a/drivers/gpu/drm/omapdrm/omap_debugfs.c
b/drivers/gpu/drm/omapdrm/omap_debugfs.c
index a3d470468e5b..a94ce502e152 100644
--- a/drivers/gpu/drm/omapdrm/omap_debugfs.c
+++ b/drivers/gpu/drm/omapdrm/omap_debugfs.c
@@ -19,7 +19,7 @@
static int gem_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct omap_drm_private *priv = dev->dev_private;
@@ -33,7 +33,7 @@ static int gem_show(struct seq_file *m, void *arg)
static int mm_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_printer p = drm_seq_file_printer(m);
@@ -45,7 +45,7 @@ static int mm_show(struct seq_file *m, void *arg)
#ifdef CONFIG_DRM_FBDEV_EMULATION
static int fb_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node =
[PATCH] drm/msm: Remove unnecessary (void*) conversions
Pointer variables of (void*) type do not require type cast.
Signed-off-by: Su Hui
---
drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
drivers/gpu/drm/msm/msm_debugfs.c | 6 +++---
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
index 6bd397a85834..169b8fe688f8 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
@@ -69,7 +69,7 @@ static void roq_print(struct msm_gpu *gpu, struct drm_printer
*p)
static int show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
struct drm_printer p = drm_seq_file_printer(m);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index cc66ddffe672..6e684a7b49a1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1392,7 +1392,7 @@ DEFINE_SHOW_ATTRIBUTE(_dpu_debugfs_status);
static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
{
- struct drm_crtc *crtc = (struct drm_crtc *) s->private;
+ struct drm_crtc *crtc = s->private;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e..3b307ce637a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -57,8 +57,8 @@ static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
static int _dpu_danger_signal_status(struct seq_file *s,
bool danger_status)
{
- struct dpu_kms *kms = (struct dpu_kms *)s->private;
struct dpu_danger_safe_status status;
+ struct dpu_kms *kms = s->private;
int i;
if (!kms->hw_mdp) {
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 29ae5c9613f3..323079cfd698 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -229,7 +229,7 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
#ifdef CONFIG_DEBUG_FS
static int smp_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c
b/drivers/gpu/drm/msm/msm_debugfs.c
index 9c0e633a3a61..a0a936f80ae3 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -211,7 +211,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(shrink_fops,
static int msm_gem_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
int ret;
@@ -229,7 +229,7 @@ static int msm_gem_show(struct seq_file *m, void *arg)
static int msm_mm_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_printer p = drm_seq_file_printer(m);
@@ -240,7 +240,7 @@ static int msm_mm_show(struct seq_file *m, void *arg)
static int msm_fb_show(struct seq_file *m, void *arg)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_framebuffer *fb, *fbdev_fb = NULL;
--
2.30.2
[PATCH] drm/radeon: Remove unnecessary (void*) conversions
No need cast (void*) to (struct radeon_device *)
or (struct radeon_ring *).
Signed-off-by: Su Hui
---
drivers/gpu/drm/radeon/r100.c | 8
drivers/gpu/drm/radeon/r300.c | 2 +-
drivers/gpu/drm/radeon/r420.c | 2 +-
drivers/gpu/drm/radeon/r600.c | 2 +-
drivers/gpu/drm/radeon/radeon_fence.c | 2 +-
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_ib.c| 2 +-
drivers/gpu/drm/radeon/radeon_pm.c| 2 +-
drivers/gpu/drm/radeon/radeon_ring.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/radeon/rs400.c| 2 +-
drivers/gpu/drm/radeon/rv515.c| 4 ++--
12 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index d4f09ecc3d22..affa9e0309b2 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2929,7 +2929,7 @@ static void r100_set_safe_registers(struct radeon_device
*rdev)
#if defined(CONFIG_DEBUG_FS)
static int r100_debugfs_rbbm_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
uint32_t reg, value;
unsigned i;
@@ -2948,7 +2948,7 @@ static int r100_debugfs_rbbm_info_show(struct seq_file
*m, void *unused)
static int r100_debugfs_cp_ring_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
uint32_t rdp, wdp;
unsigned count, i, j;
@@ -2974,7 +2974,7 @@ static int r100_debugfs_cp_ring_info_show(struct seq_file
*m, void *unused)
static int r100_debugfs_cp_csq_fifo_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
uint32_t csq_stat, csq2_stat, tmp;
unsigned r_rptr, r_wptr, ib1_rptr, ib1_wptr, ib2_rptr, ib2_wptr;
unsigned i;
@@ -3022,7 +3022,7 @@ static int r100_debugfs_cp_csq_fifo_show(struct seq_file
*m, void *unused)
static int r100_debugfs_mc_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
uint32_t tmp;
tmp = RREG32(RADEON_CONFIG_MEMSIZE);
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 7b0cfeaddcec..9c1a92fa2af6 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -589,7 +589,7 @@ int rv370_get_pcie_lanes(struct radeon_device *rdev)
#if defined(CONFIG_DEBUG_FS)
static int rv370_debugfs_pcie_gart_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
uint32_t tmp;
tmp = RREG32_PCIE(RADEON_PCIE_TX_GART_CNTL);
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index 7e6320e8c6a0..eae8a6389f5e 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -474,7 +474,7 @@ int r420_init(struct radeon_device *rdev)
#if defined(CONFIG_DEBUG_FS)
static int r420_debugfs_pipes_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
uint32_t tmp;
tmp = RREG32(R400_GB_PIPE_SELECT);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index dd78fc499402..382795a8b3c0 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -4345,7 +4345,7 @@ int r600_irq_process(struct radeon_device *rdev)
static int r600_debugfs_mc_info_show(struct seq_file *m, void *unused)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
DREG32_SYS(m, rdev, R_000E50_SRBM_STATUS);
DREG32_SYS(m, rdev, VM_L2_STATUS);
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index 73e3117420bf..2749dde5838f 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -955,7 +955,7 @@ void radeon_fence_driver_force_completion(struct
radeon_device *rdev, int ring)
#if defined(CONFIG_DEBUG_FS)
static int radeon_debugfs_fence_info_show(struct seq_file *m, void *data)
{
- struct radeon_device *rdev = (struct radeon_device *)m->private;
+ struct radeon_device *rdev = m->private;
int i, j;
for (i = 0; i < RADEON_NUM_RINGS; ++i) {
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
b/drivers/gpu/drm/radeon/radeon_gem.c
index bdc5af23f005..5de99ffa072f 100644
--- a/driver
[PATCH] drm/amdgpu: remove unnecessary (void*) conversions
No need cast (void*) to (struct amdgpu_device *).
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f60753f97ac5..c837e0bf2cfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1470,7 +1470,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
int r = 0, i;
@@ -1581,7 +1581,7 @@ static int amdgpu_debugfs_benchmark(void *data, u64 val)
static int amdgpu_debugfs_vm_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
struct drm_file *file;
int r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index f52d0ba91a77..f0615a43b3cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -835,7 +835,7 @@ static const struct dma_fence_ops amdgpu_job_fence_ops = {
#if defined(CONFIG_DEBUG_FS)
static int amdgpu_debugfs_fence_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
int i;
for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 863cb668e000..28f79cf8c3fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -948,7 +948,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
#if defined(CONFIG_DEBUG_FS)
static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_device *dev = adev_to_drm(adev);
struct drm_file *file;
int r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 4ff348e10e4d..49a4238a120e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -436,7 +436,7 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
static int amdgpu_debugfs_sa_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
seq_printf(m, "- DELAYED - \n");
amdgpu_sa_bo_dump_debug_info(&adev->ib_pools[AMDGPU_IB_POOL_DELAYED],
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 0efb38539d70..9f9274249b57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1441,7 +1441,7 @@ void amdgpu_disable_vblank_kms(struct drm_crtc *crtc)
static int amdgpu_debugfs_firmware_info_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
struct drm_amdgpu_info_firmware fw_info;
struct drm_amdgpu_query_fw query_fw;
struct atom_context *ctx = adev->mode_info.atom_context;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2cd081cbf706..21f340ed4cca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2164,7 +2164,7 @@ int amdgpu_ttm_evict_resources(struct amdgpu_device
*adev, int mem_type)
static int amdgpu_ttm_page_pool_show(struct seq_file *m, void *unused)
{
- struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
+ struct amdgpu_device *adev = m->private;
return ttm_pool_debugfs(&adev->mman.bdev.pool, m);
}
--
2.30.2
