[AMD Official Use Only - AMD Internal Distribution Only]
Tested-by: George Zhang <George.zhang@xxxxxxx>
Thanks,
George
-----Original Message-----
From: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Sent: Tuesday, June 4, 2024 11:50 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Mahfooz, Hamza <Hamza.Mahfooz@xxxxxxx>; Zhang, George <George.Zhang@xxxxxxx>; Arnd Bergmann <arnd@xxxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>; Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx>; Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>
Subject: [PATCH] drm/amd/display: use pre-allocated temp structure for bounding box
This mirrors what the driver does for older DCN generations.
Should fix:
BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 449, name: kworker/u64:8
preempt_count: 2, expected: 0
RCU nest depth: 0, expected: 0
Preemption disabled at:
ffffffffc0ce1580>] dc_fpu_begin+0x30/0xd0 [amdgpu]
CPU: 5 PID: 449 Comm: kworker/u64:8 Tainted: G W 6.8.0+ #35
Hardware name: System manufacturer System Product Name/ROG STRIX X570-E GAMING WIFI II, BIOS 4204 02/24/2022
Workqueue: events_unbound async_run_entry_fn
Fixes: 88c61827cedc ("drm/amd/display: dynamically allocate dml2_configuration_options structures")
Suggested-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: George Zhang <george.zhang@xxxxxxx>
Cc: Arnd Bergmann <arnd@xxxxxxxx>
Cc: harry.wentland@xxxxxxx
Cc: sunpeng.li@xxxxxxx
Cc: Rodrigo.Siqueira@xxxxxxx
---
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../drm/amd/display/dc/resource/dcn32/dcn32_resource.c | 8 +++-----
.../drm/amd/display/dc/resource/dcn321/dcn321_resource.c | 8 +++-----
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index d0ed01ac460d..d843933ad731 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1444,6 +1444,7 @@ struct dc {
} scratch;
struct dml2_configuration_options dml2_options;
+ struct dml2_configuration_options dml2_tmp;
enum dc_acpi_cm_power_state power_state;
};
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index 0f11d7c8791c..7a8aa9396dea 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -2007,11 +2007,9 @@ void dcn32_calculate_wm_and_dlg(struct dc *dc, struct dc_state *context,
static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw_params)
{
- struct dml2_configuration_options *dml2_opt;
+ struct dml2_configuration_options *dml2_opt = &dc->dml2_tmp;
- dml2_opt = kmemdup(&dc->dml2_options, sizeof(dc->dml2_options), GFP_KERNEL);
- if (!dml2_opt)
- return;
+ memcpy(dml2_opt, &dc->dml2_options, sizeof(dc->dml2_options));
DC_FP_START();
@@ -2027,7 +2025,7 @@ static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw
DC_FP_END();
- kfree(dml2_opt);
+ memcpy(&dc->dml2_options, dml2_opt, sizeof(dc->dml2_options));
}
static struct resource_funcs dcn32_res_pool_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
index 07ca6f58447d..ef30e8632607 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
@@ -1581,11 +1581,9 @@ static struct dc_cap_funcs cap_funcs = {
static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *bw_params)
{
- struct dml2_configuration_options *dml2_opt;
+ struct dml2_configuration_options *dml2_opt = &dc->dml2_tmp;
- dml2_opt = kmemdup(&dc->dml2_options, sizeof(dc->dml2_options), GFP_KERNEL);
- if (!dml2_opt)
- return;
+ memcpy(dml2_opt, &dc->dml2_options, sizeof(dc->dml2_options));
DC_FP_START();
@@ -1601,7 +1599,7 @@ static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params *b
DC_FP_END();
- kfree(dml2_opt);
+ memcpy(&dc->dml2_options, dml2_opt, sizeof(dc->dml2_options));
}
static struct resource_funcs dcn321_res_pool_funcs = {
--
2.45.1
--- Begin Message ---
- To: "Koenig, Christian" <Christian.Koenig@xxxxxxx>, "amd-gfx@xxxxxxxxxxxxxxxxxxxxx" <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>, "Huang, Tim" <Tim.Huang@xxxxxxx>, "Zhang, Jesse(Jie)" <Jesse.Zhang@xxxxxxx>
- Subject: RE: [PATCH] drm/amdgpu: replace int with unsigned int for imu_v12_0.c
- From: "Zhou, Bob" <Bob.Zhou@xxxxxxx>
- Date: Tue, 4 Jun 2024 06:52:10 +0000
- Accept-language: en-US, zh-CN
- Cc: "Deucher, Alexander" <Alexander.Deucher@xxxxxxx>
- In-reply-to: <faa146e9-c2cb-4831-af0a-539dac51d912@amd.com>
- Msip_labels: MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_ActionId=5f9a48c3-e5da-4a48-b6aa-cc938407c8e3; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_ContentBits=0; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_Enabled=true; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_Method=Standard; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_Name=AMD Internal Distribution Only; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_SetDate=2024-06-04T02:43:48Z; MSIP_Label_dce362fe-1558-4fb5-9f64-8a6240d76441_SiteId=3dd8961f-e488-4e60-8e11-a82d994e183d;
- References: <20240603055925.353172-1-bob.zhou@amd.com> <643687ee-4f93-4448-8d09-c841e1ce2746@amd.com> <CO6PR12MB5394B8D4CCD80B551FFE43EA94FF2@CO6PR12MB5394.namprd12.prod.outlook.com> <faa146e9-c2cb-4831-af0a-539dac51d912@amd.com>
[AMD Official Use Only - AMD Internal Distribution Only]
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Christian,
Thanks for your comments, the RREG32_SOC15() macro should return the 32bit wide value so it couldn't need the mask.
And I will send out v2 patch to modify the commit message and use uint32_t or u32 instead .
Regards,
Bob
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: 2024年6月3日 19:11
To: Zhou, Bob <Bob.Zhou@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Huang, Tim <Tim.Huang@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: replace int with unsigned int for imu_v12_0.c
Am 03.06.24 um 10:53 schrieb Zhou, Bob:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Hi Christian,
>
> It fixes a potential Overflowed constant (INTEGER_OVERFLOW) warning reported by Coverity.
You need to mention that in the commit message.
And I haven't checked the hardware docs, but it can be that this isn't the right solution for the warning. Instead you should probably use the
RREG32_SOC15() macro with a mask which only leaves the valid bits around.
Only when the register is really 32bit wide you need an unsigned datatype and if that is the case I suggest to use either uint32_t or u32 instead.
Regards,
Christian.
>
> Regards,
> Bob
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@xxxxxxx>
> Sent: 2024年6月3日 15:56
> To: Zhou, Bob <Bob.Zhou@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx;
> Huang, Tim <Tim.Huang@xxxxxxx>; Zhang, Jesse(Jie)
> <Jesse.Zhang@xxxxxxx>
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> Subject: Re: [PATCH] drm/amdgpu: replace int with unsigned int for
> imu_v12_0.c
>
> Am 03.06.24 um 07:59 schrieb Bob Zhou:
>> The return value of RREG32_SOC15 is unsigned int, so modify variable to unsigned.
> And why is that an improvement?
>
> Regards,
> Christian.
>
>> Signed-off-by: Bob Zhou <bob.zhou@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/amdgpu/imu_v12_0.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> b/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> index 0c8ef908d112..2d6f7549c2af 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/imu_v12_0.c
>> @@ -119,7 +119,7 @@ static int imu_v12_0_load_microcode(struct
>> amdgpu_device *adev)
>>
>> static int imu_v12_0_wait_for_reset_status(struct amdgpu_device *adev)
>> {
>> - int i, imu_reg_val = 0;
>> + unsigned int i, imu_reg_val = 0;
>>
>> for (i = 0; i < adev->usec_timeout; i++) {
>> imu_reg_val = RREG32_SOC15(GC, 0,
>> regGFX_IMU_GFX_RESET_CTRL); @@
>> -138,7 +138,7 @@ static int imu_v12_0_wait_for_reset_status(struct
>> amdgpu_device *adev)
>>
>> static void imu_v12_0_setup(struct amdgpu_device *adev)
>> {
>> - int imu_reg_val;
>> + unsigned int imu_reg_val;
>>
>> WREG32_SOC15(GC, 0, regGFX_IMU_C2PMSG_ACCESS_CTRL0, 0xffffff);
>> WREG32_SOC15(GC, 0, regGFX_IMU_C2PMSG_ACCESS_CTRL1, 0xffff);
>> @@
>> -157,7 +157,7 @@ static void imu_v12_0_setup(struct amdgpu_device
>> *adev)
>>
>> static int imu_v12_0_start(struct amdgpu_device *adev)
>> {
>> - int imu_reg_val;
>> + unsigned int imu_reg_val;
>>
>> imu_reg_val = RREG32_SOC15(GC, 0, regGFX_IMU_CORE_CTRL);
>> imu_reg_val &= 0xfffffffe;
--- End Message ---
