[bug report] drm/amdgpu/gfx11: add a mutex for the gfx semaphore
Hello Alex Deucher,
Commit 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx
semaphore") from Jul 12, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:4778
gfx_v11_0_request_gfx_index_mutex()
warn: inconsistent returns '&adev->gfx.reset_sem_mutex'.
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
4745 int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev,
4746 bool req)
4747 {
4748 u32 i, tmp, val;
4749
4750 if (req)
4751 mutex_lock(&adev->gfx.reset_sem_mutex);
Could we move this lock into the caller?
4752 for (i = 0; i < adev->usec_timeout; i++) {
4753 /* Request with MeId=2, PipeId=0 */
4754 tmp = REG_SET_FIELD(0, CP_GFX_INDEX_MUTEX, REQUEST,
req);
4755 tmp = REG_SET_FIELD(tmp, CP_GFX_INDEX_MUTEX, CLIENTID,
4);
4756 WREG32_SOC15(GC, 0, regCP_GFX_INDEX_MUTEX, tmp);
4757
4758 val = RREG32_SOC15(GC, 0, regCP_GFX_INDEX_MUTEX);
4759 if (req) {
4760 if (val == tmp)
4761 break;
4762 } else {
4763 tmp = REG_SET_FIELD(tmp, CP_GFX_INDEX_MUTEX,
4764 REQUEST, 1);
4765
4766 /* unlocked or locked by firmware */
4767 if (val != tmp)
4768 break;
4769 }
4770 udelay(1);
4771 }
4772 if (!req)
4773 mutex_unlock(&adev->gfx.reset_sem_mutex);
4774
4775 if (i >= adev->usec_timeout)
4776 return -EINVAL;
Either way this error path needs an unlock.
if (i >= adev->usec_timeout) {
if (req)
mutex_unlock(&adev->gfx.reset_sem_mutex);
return -EINVAL;
}
But I really think it would be nicer in the caller.
4777
--> 4778 return 0;
4779 }
regards,
dan carpenter
Re: [PATCH] drm/amd/gfx11: move the gfx mutex into the caller
On Tue, Aug 20, 2024 at 10:39:09AM -0400, Alex Deucher wrote:
> Otherwise we can fail to drop the software mutex when
> we fail to take the hardware mutex.
>
> Fixes: 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx semaphore")
> Reported-by: Dan Carpenter
> Signed-off-by: Alex Deucher
Thanks so much!
Reviewed-by: Dan Carpenter
regards,
dan carpenter
[PATCH] drm/amd/display: fix some indenting in CalculatePrefetchSchedule()
These tabs were deleted accidentally in commit d07722e1fc74 ("drm/amd/display:
DML2.1 Reintegration for Various Fixes"). Add them back.
Signed-off-by: Dan Carpenter
---
.../dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.c| 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
index e2c45e498664..7d287b07108f 100644
---
a/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
+++
b/drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
@@ -5612,8 +5612,8 @@ static bool CalculatePrefetchSchedule(struct
dml2_core_internal_scratch *scratch
s->TimeForFetchingVM = s->Tvm_equ;
s->TimeForFetchingRowInVBlank = s->Tr0_equ;
- *p->dst_y_per_vm_vblank = math_ceil2(4.0 * s->TimeForFetchingVM
/ s->LineTime, 1.0) / 4.0;
- *p->dst_y_per_row_vblank = math_ceil2(4.0 *
s->TimeForFetchingRowInVBlank / s->LineTime, 1.0) / 4.0;
+ *p->dst_y_per_vm_vblank = math_ceil2(4.0 *
s->TimeForFetchingVM / s->LineTime, 1.0) / 4.0;
+ *p->dst_y_per_row_vblank = math_ceil2(4.0 *
s->TimeForFetchingRowInVBlank / s->LineTime, 1.0) / 4.0;
#ifdef __DML_VBA_DEBUG__
dml2_printf("DML::%s: Using equ bw scheduling for
prefetch\n", __func__);
--
2.43.0
[PATCH] drm/amdgpu/mes11: Indent an if statment
Indent the "break" statement one more tab.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 0f055d1b1da6..ee91ff9e52a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -415,7 +415,7 @@ static int mes_v11_0_reset_queue_mmio(struct amdgpu_mes
*mes, uint32_t queue_typ
/* wait till dequeue take effects */
for (i = 0; i < adev->usec_timeout; i++) {
if (!(RREG32_SOC15(GC, 0, regCP_HQD_ACTIVE) & 1))
- break;
+ break;
udelay(1);
}
if (i >= adev->usec_timeout) {
--
2.45.2
[bug report] drm/amd/display: Simplify the per-CPU usage.
pool->base.dscs[i] = dcn30_dsc_create(ctx, i);
2531 if (pool->base.dscs[i] == NULL) {
2532 BREAK_TO_DEBUGGER();
2533 dm_error("DC: failed to create display stream
compressor %d!\n", i);
2534 goto create_fail;
2535 }
2536 }
2537
2538 /* DWB and MMHUBBUB */
2539 if (!dcn30_dwbc_create(ctx, &pool->base)) {
2540 BREAK_TO_DEBUGGER();
2541 dm_error("DC: failed to create dwbc!\n");
2542 goto create_fail;
2543 }
2544
2545 if (!dcn30_mmhubbub_create(ctx, &pool->base)) {
2546 BREAK_TO_DEBUGGER();
2547 dm_error("DC: failed to create mcif_wb!\n");
2548 goto create_fail;
2549 }
2550
2551 /* AUX and I2C */
2552 for (i = 0; i < pool->base.res_cap->num_ddc; i++) {
2553 pool->base.engines[i] = dcn30_aux_engine_create(ctx,
i);
2554 if (pool->base.engines[i] == NULL) {
2555 BREAK_TO_DEBUGGER();
2556 dm_error(
2557 "DC:failed to create aux engine!!\n");
2558 goto create_fail;
2559 }
2560 pool->base.hw_i2cs[i] = dcn30_i2c_hw_create(ctx, i);
2561 if (pool->base.hw_i2cs[i] == NULL) {
2562 BREAK_TO_DEBUGGER();
2563 dm_error(
2564 "DC:failed to create hw i2c!!\n");
2565 goto create_fail;
2566 }
2567 pool->base.sw_i2cs[i] = NULL;
2568 }
2569
2570 /* Audio, Stream Encoders including DIG and virtual, MPC 3D
LUTs */
2571 if (!resource_construct(num_virtual_links, dc, &pool->base,
2572 &res_create_funcs))
2573 goto create_fail;
2574
2575 /* HW Sequencer and Plane caps */
2576 dcn30_hw_sequencer_construct(dc);
2577
2578 dc->caps.max_planes = pool->base.pipe_count;
2579
2580 for (i = 0; i < dc->caps.max_planes; ++i)
2581 dc->caps.planes[i] = plane_cap;
2582
2583 dc->cap_funcs = cap_funcs;
2584
2585 if (dc->ctx->dc_bios->fw_info.oem_i2c_present) {
2586 ddc_init_data.ctx = dc->ctx;
2587 ddc_init_data.link = NULL;
2588 ddc_init_data.id.id =
dc->ctx->dc_bios->fw_info.oem_i2c_obj_id;
2589 ddc_init_data.id.enum_id = 0;
2590 ddc_init_data.id.type = OBJECT_TYPE_GENERIC;
2591 pool->base.oem_device =
dc->link_srv->create_ddc_service(&ddc_init_data);
2592 } else {
2593 pool->base.oem_device = NULL;
2594 }
2595
2596 DC_FP_END();
2597
2598 return true;
2599
2600 create_fail:
2601
2602 DC_FP_END();
2603 dcn30_resource_destruct(pool);
2604
2605 return false;
2606 }
regards,
dan carpenter
Re: [bug report] drm/amd/display: Simplify the per-CPU usage.
On Thu, Feb 01, 2024 at 02:53:42PM +0100, Sebastian Andrzej Siewior wrote: > On 2024-02-01 15:18:04 [+0300], Dan Carpenter wrote: > > Hello Sebastian Andrzej Siewior, > Hi Dan, > > > The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU > > usage." from Sep 21, 2023 (linux-next), leads to the following Smatch > > static checker warning: > > Did I introduce that or has it been made visible? > That change adds preempt_disable() to DC_FP_START() but this was there > already, just hidden. For x86 it is done within kernel_fpu_begin(). > Sorry, yeah, the bug was there before. I don't know why this shows up as a new warning. Probably it's because AMD driver files were renamed... Smatch parses kernel_fpu_begin() correctly and sees the preempt_disable() but I didn't know it disables preemption so it's likely human error on my part. regards, dan carpenter
[PATCH] drm/amd/display: Fix && vs || typos
These ANDs should be ORs or it will lead to a NULL dereference.
Fixes: fb5a3d037082 ("drm/amd/display: Add NULL test for 'timing generator' in
'dcn21_set_pipe()'")
Fixes: 886571d217d7 ("drm/amd/display: Fix 'panel_cntl' could be null in
'dcn21_set_backlight_level()'")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 5c7f380a84f9..7252f5f781f0 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -211,7 +211,7 @@ void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
uint32_t otg_inst;
- if (!abm && !tg && !panel_cntl)
+ if (!abm || !tg || !panel_cntl)
return;
otg_inst = tg->inst;
@@ -245,7 +245,7 @@ bool dcn21_set_backlight_level(struct pipe_ctx *pipe_ctx,
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
uint32_t otg_inst;
- if (!abm && !tg && !panel_cntl)
+ if (!abm || !tg || !panel_cntl)
return false;
otg_inst = tg->inst;
--
2.43.0
[bug report] drm/amd/display: fix null-pointer dereference on edid reading
Hello Melissa Wen,
This is a semi-automatic email about new static checker warnings.
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6683
amdgpu_dm_connector_funcs_force()
warn: variable dereferenced before check 'dc_link' (see line 6663)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
6662
6663 if (dc_link->aux_mode)
^
The patch adds an unchecked dereference
6664 ddc = &aconnector->dm_dp_aux.aux.ddc;
6665 else
ddc = &aconnector->i2c->base;
6667
6668 /*
6669 * Note: drm_get_edid gets edid in the following order:
6670 * 1) override EDID if set via edid_override debugfs,
6671 * 2) firmware EDID if set via edid_firmware module parameter
6672 * 3) regular DDC read.
6673 */
6674 edid = drm_get_edid(connector, ddc);
6675 if (!edid) {
6676 DRM_ERROR("No EDID found on connector: %s.\n",
connector->name);
6677 return;
6678 }
6679
6680 aconnector->edid = edid;
6681
6682 /* Update emulated (virtual) sink's EDID */
6683 if (dc_em_sink && dc_link) {
^^^
The existing code assumed dc_link could be NULL? Can it? If not then
let's delete this check.
6684 memset(&dc_em_sink->edid_caps, 0, sizeof(struct
dc_edid_caps));
6685 memmove(dc_em_sink->dc_edid.raw_edid, edid,
(edid->extensions + 1) * EDID_LENGTH);
regards,
dan carpenter
[bug report] drm/amd/display: Add debug counters to IPS exit prints
.ips1_commit,
1349
ips_fw->signals.bits.ips2_commit);
1350
1351
dc->clk_mgr->funcs->exit_low_power_state(dc->clk_mgr);
1352
1353 DC_LOG_IPS(
1354 "wait IPS2 commit clear
(ips1_commit=%d ips2_commit=%d)",
1355
ips_fw->signals.bits.ips1_commit,
1356
ips_fw->signals.bits.ips2_commit);
1357
1358 while
(ips_fw->signals.bits.ips2_commit)
1359 udelay(1);
1360
1361 DC_LOG_IPS(
1362 "wait hw_pwr_up
(ips1_commit=%d ips2_commit=%d)",
1363
ips_fw->signals.bits.ips1_commit,
1364
ips_fw->signals.bits.ips2_commit);
1365
1366 if
(!dc_dmub_srv_is_hw_pwr_up(dc->ctx->dmub_srv, true))
1367 ASSERT(0);
1368
1369 DC_LOG_IPS(
1370 "resync inbox1 (ips1_commit=%d
ips2_commit=%d)",
1371
ips_fw->signals.bits.ips1_commit,
1372
ips_fw->signals.bits.ips2_commit);
1373
1374
dmub_srv_sync_inbox1(dc->ctx->dmub_srv->dmub);
1375 }
1376 }
1377
1378 dc_dmub_srv_notify_idle(dc, false);
1379 if (prev_driver_signals.bits.allow_ips1) {
1380 DC_LOG_IPS(
1381 "wait for IPS1 commit clear
(ips1_commit=%d ips2_commit=%d)",
1382 ips_fw->signals.bits.ips1_commit,
1383 ips_fw->signals.bits.ips2_commit);
1384
1385 while (ips_fw->signals.bits.ips1_commit)
1386 udelay(1);
1387
1388 DC_LOG_IPS(
1389 "wait for IPS1 commit clear done
(ips1_commit=%d ips2_commit=%d)",
1390 ips_fw->signals.bits.ips1_commit,
1391 ips_fw->signals.bits.ips2_commit);
1392 }
1393 }
variables uninitialized on else path
1394
1395 if (!dc_dmub_srv_is_hw_pwr_up(dc->ctx->dmub_srv, true))
1396 ASSERT(0);
1397
--> 1398 DC_LOG_IPS("%s exit (count rcg=%d ips1=%d ips2=%d)",
1399 __func__,
1400 rcg_exit_count,
1401 ips1_exit_count,
1402 ips2_exit_count);
used here
1403 }
regards,
dan carpenter
[bug report] drm/amdgpu: add ring buffer information in devcoredump
Hello Sunil Khatri,
Commit 42742cc541bb ("drm/amdgpu: add ring buffer information in
devcoredump") from Mar 11, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c:219 amdgpu_devcoredump_read()
error: we previously assumed 'coredump->adev' could be null (see line
206)
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.c
171 static ssize_t
172 amdgpu_devcoredump_read(char *buffer, loff_t offset, size_t count,
173 void *data, size_t datalen)
174 {
175 struct drm_printer p;
176 struct amdgpu_coredump_info *coredump = data;
177 struct drm_print_iterator iter;
178 int i;
179
180 iter.data = buffer;
181 iter.offset = 0;
182 iter.start = offset;
183 iter.remain = count;
184
185 p = drm_coredump_printer(&iter);
186
187 drm_printf(&p, " AMDGPU Device Coredump \n");
188 drm_printf(&p, "version: " AMDGPU_COREDUMP_VERSION "\n");
189 drm_printf(&p, "kernel: " UTS_RELEASE "\n");
190 drm_printf(&p, "module: " KBUILD_MODNAME "\n");
191 drm_printf(&p, "time: %lld.%09ld\n",
coredump->reset_time.tv_sec,
192 coredump->reset_time.tv_nsec);
193
194 if (coredump->reset_task_info.pid)
195 drm_printf(&p, "process_name: %s PID: %d\n",
196coredump->reset_task_info.process_name,
197coredump->reset_task_info.pid);
198
199 if (coredump->ring) {
200 drm_printf(&p, "\nRing timed out details\n");
201 drm_printf(&p, "IP Type: %d Ring Name: %s\n",
202coredump->ring->funcs->type,
203coredump->ring->name);
204 }
205
206 if (coredump->adev) {
^^
Check for NULL
207 struct amdgpu_vm_fault_info *fault_info =
208 &coredump->adev->vm_manager.fault_info;
209
210 drm_printf(&p, "\n[%s] Page fault observed\n",
211fault_info->vmhub ? "mmhub" : "gfxhub");
212 drm_printf(&p, "Faulty page starting at address:
0x%016llx\n",
213fault_info->addr);
214 drm_printf(&p, "Protection fault status register:
0x%x\n\n",
215fault_info->status);
216 }
217
218 drm_printf(&p, "Ring buffer information\n");
--> 219 for (int i = 0; i < coredump->adev->num_rings; i++) {
^^
Unchecked dereference
220 int j = 0;
221 struct amdgpu_ring *ring = coredump->adev->rings[i];
222
223 drm_printf(&p, "ring name: %s\n", ring->name);
224 drm_printf(&p, "Rptr: 0x%llx Wptr: 0x%llx RB mask:
%x\n",
225amdgpu_ring_get_rptr(ring),
226amdgpu_ring_get_wptr(ring),
227ring->buf_mask);
228 drm_printf(&p, "Ring size in dwords: %d\n",
229ring->ring_size / 4);
230 drm_printf(&p, "Ring contents\n");
231 drm_printf(&p, "Offset \t Value\n");
232
233 while (j < ring->ring_size) {
234 drm_printf(&p, "0x%x \t 0x%x\n", j,
ring->ring[j/4]);
235 j += 4;
236 }
237 }
238
239 if (coredump->reset_vram_lost)
240 drm_printf(&p, "VRAM is lost due to GPU reset!\n");
241 if (coredump->adev->reset_info.num_regs) {
^^
Here too
242 drm_printf(&p, "AMDGPU register dumps:\nOffset:
Value:\n");
243
244 for (i = 0; i < coredump->adev->reset_info.num_regs;
i++)
245 drm_printf(&p, "0x%08x: 0x%08x\n",
246
coredump->adev->reset_info.reset_dump_reg_list[i],
247
coredump->adev->reset_info.reset_dump_reg_value[i]);
248 }
249
250 return count - iter.remain;
251 }
regards,
dan carpenter
Re: [bug report] drm/amdgpu: add ring buffer information in devcoredump
The static checker is just complaining about NULL checking that doesn't make sense. It raises the question, can the pointer be NULL or not? Based on your comments and from reviewing the code, I do not think it can be NULL. Thus the correct thing is to remove the unnecessary NULL check. regards, dan carpenter
[PATCH] drm/amd/display: delete unnecessary check in dcn35_set_long_vblank()
"timing" is "&pipe_ctx[i]->stream->timing" where ->timing is not the
first struct member of ->stream. So it's the address which points into
the middle of a struct. It can't be NULL so delete the NULL check.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
index 2e8ec58a16eb..87cfd9f1cec9 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
@@ -1411,10 +1411,7 @@ void dcn35_set_long_vblank(struct pipe_ctx **pipe_ctx,
if (pipe_ctx[i]->stream) {
struct dc_crtc_timing *timing =
&pipe_ctx[i]->stream->timing;
- if (timing)
- params.vertical_blank_start = timing->v_total -
timing->v_front_porch;
- else
- params.vertical_blank_start = 0;
+ params.vertical_blank_start = timing->v_total -
timing->v_front_porch;
if ((pipe_ctx[i]->stream_res.tg != NULL) &&
pipe_ctx[i]->stream_res.tg->funcs &&
pipe_ctx[i]->stream_res.tg->funcs->set_long_vtotal)
--
2.43.0
[bug report] drm/amd/display: Allow Z8 when stutter threshold is not met
Hello Bhawanpreet Lakha,
This is a semi-automatic email about new static checker warnings.
Commit e9a09a198bfe ("drm/amd/display: Allow Z8 when stutter
threshold is not met") from Mar 13, 2024, leads to the following
Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:1088
decide_zstate_support()
warn: variable dereferenced before check 'link' (see line 1087)
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c
1086 bool allow_z8 = context->bw_ctx.dml.vba.StutterPeriod >
(double)minmum_z8_residency;
1087 bool is_pwrseq0 = link->link_index == 0;
The existing code assumes link isn't NULL
1088 bool is_psr = (link && (link->psr_settings.psr_version
== DC_PSR_VERSION_1 ||
1089
link->psr_settings.psr_version == DC_PSR_VERSION_SU_1) &&
!link->panel_config.psr.disable_psr);
1090 bool is_replay = link &&
link->replay_settings.replay_feature_enabled;
but the patch assumes link can be NULL. Somebody is wrong.
regards,
dan carpenter
[bug report] drm/amd/display: Remove plane and stream pointers from dc scratch
Hello Alvin Lee,
Commit 285a7054bf81 ("drm/amd/display: Remove plane and stream
pointers from dc scratch") from Mar 15, 2024 (linux-next), leads to
the following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c:1112
dcn20_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn10/dcn10_hwseq.c:1839
dcn10_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dce110/dce110_hwseq.c:301
dce110_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c
1094 bool dcn20_set_input_transfer_func(struct dc *dc,
1095 struct pipe_ctx *pipe_ctx,
1096 const struct dc_plane_state
*plane_state)
1097 {
1098 struct dce_hwseq *hws = dc->hwseq;
1099 struct dpp *dpp_base = pipe_ctx->plane_res.dpp;
1100 const struct dc_transfer_func *tf = NULL;
^
This assignment is not necessary now.
1101 bool result = true;
1102 bool use_degamma_ram = false;
1103
1104 if (dpp_base == NULL || plane_state == NULL)
1105 return false;
1106
1107 hws->funcs.set_shaper_3dlut(pipe_ctx, plane_state);
1108 hws->funcs.set_blend_lut(pipe_ctx, plane_state);
1109
1110 tf = &plane_state->in_transfer_func;
^
Before there was an if statement but now tf is assigned unconditionally
--> 1112 if (tf == NULL) {
^
so these conditions are impossible.
1113 dpp_base->funcs->dpp_set_degamma(dpp_base,
1114 IPP_DEGAMMA_MODE_BYPASS);
1115 return true;
1116 }
1117
1118 if (tf->type == TF_TYPE_HWPWL || tf->type ==
TF_TYPE_DISTRIBUTED_POINTS)
1119 use_degamma_ram = true;
1120
1121 if (use_degamma_ram == true) {
1122 if (tf->type == TF_TYPE_HWPWL)
1123
dpp_base->funcs->dpp_program_degamma_pwl(dpp_base,
regards,
dan carpenter
[bug report] drm/amdgpu: add init support for GFX11 (v2)
Hello Hawking Zhang,
Commit 3d879e81f0f9 ("drm/amdgpu: add init support for GFX11 (v2)")
from Apr 13, 2022 (linux-next), leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:4503 gfx_v11_0_hw_init()
error: we previously assumed 'adev->gfx.imu.funcs' could be null (see
line 4497)
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
4491 static int gfx_v11_0_hw_init(void *handle)
4492 {
4493 int r;
4494 struct amdgpu_device *adev = (struct amdgpu_device *)handle;
4495
4496 if (adev->firmware.load_type ==
AMDGPU_FW_LOAD_RLC_BACKDOOR_AUTO) {
4497 if (adev->gfx.imu.funcs) {
^^^
Check for NULL
4498 /* RLC autoload sequence 1: Program rlc ram */
4499 if (adev->gfx.imu.funcs->program_rlc_ram)
4500
adev->gfx.imu.funcs->program_rlc_ram(adev);
4501 }
4502 /* rlc autoload firmware */
--> 4503 r = gfx_v11_0_rlc_backdoor_autoload_enable(adev);
Unchecked dereference inside the function. (Probably just delete the
NULL check?)
4504 if (r)
4505 return r;
4506 } else {
regards,
dan carpenter
[PATCH] drm/amdgpu/kfd: Add unlock() on error path to add_queue_mes()
We recently added locking to add_queue_mes() but this error path was
overlooked. Add an unlock to the error path.
Fixes: 1802b042a343 ("drm/amdgpu/kfd: remove is_hws_hang and is_resetting")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index d2fceb6f9802..4f48507418d2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -230,6 +230,7 @@ static int add_queue_mes(struct device_queue_manager *dqm,
struct queue *q,
if (queue_type < 0) {
dev_err(adev->dev, "Queue type not supported with MES,
queue:%d\n",
q->properties.type);
+ up_read(&adev->reset_domain->sem);
return -EINVAL;
}
queue_input.queue_type = (uint32_t)queue_type;
--
2.43.0
[PATCH] drm/amd/display: Clean up indenting in dm_dp_mst_is_port_support_mode()
This code works, but it's not aligned correctly. Add a couple missing
tabs.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 48118447c8d9..5d4f831b1e55 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -1691,7 +1691,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
if (aconnector->mst_output_port->passthrough_aux) {
if (bw_range.min_kbps > end_to_end_bw_in_kbps) {
DRM_DEBUG_DRIVER("DSC passthrough. Max dsc
compression can't fit into end-to-end bw\n");
- return DC_FAIL_BANDWIDTH_VALIDATE;
+ return DC_FAIL_BANDWIDTH_VALIDATE;
}
} else {
/*dsc bitstream decoded at the dp last link*/
@@ -1756,7 +1756,7 @@ enum dc_status dm_dp_mst_is_port_support_mode(
if (branch_max_throughput_mps != 0 &&
((stream->timing.pix_clk_100hz / 10) >
branch_max_throughput_mps * 1000)) {
DRM_DEBUG_DRIVER("DSC is required but max throughput
mps fails");
- return DC_FAIL_BANDWIDTH_VALIDATE;
+ return DC_FAIL_BANDWIDTH_VALIDATE;
}
} else {
DRM_DEBUG_DRIVER("DSC is required but can't find common dsc
config.");
--
2.43.0
[bug report] drm/amd/display: Add DCN3 HWSEQ
Hello Bhawanpreet Lakha,
Commit d99f13878d6f ("drm/amd/display: Add DCN3 HWSEQ") from May 21,
2020 (linux-next), leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn30/dcn30_hwseq.c:401
dcn30_set_output_transfer_func()
error: we previously assumed 'mpc->funcs->set_output_gamma' could be
null (see line 386)
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn30/dcn30_hwseq.c
373 bool dcn30_set_output_transfer_func(struct dc *dc,
374 struct pipe_ctx *pipe_ctx,
375 const struct dc_stream_state *stream)
376 {
377 int mpcc_id = pipe_ctx->plane_res.hubp->inst;
378 struct mpc *mpc =
pipe_ctx->stream_res.opp->ctx->dc->res_pool->mpc;
379 const struct pwl_params *params = NULL;
380 bool ret = false;
381
382 /* program OGAM or 3DLUT only for the top pipe*/
383 if (pipe_ctx->top_pipe == NULL) {
384 /*program rmu shaper and 3dlut in MPC*/
385 ret = dcn30_set_mpc_shaper_3dlut(pipe_ctx, stream);
386 if (ret == false && mpc->funcs->set_output_gamma) {
If this is NULL
387 if (stream->out_transfer_func.type ==
TF_TYPE_HWPWL)
388 params = &stream->out_transfer_func.pwl;
389 else if
(pipe_ctx->stream->out_transfer_func.type ==
390 TF_TYPE_DISTRIBUTED_POINTS &&
391
cm3_helper_translate_curve_to_hw_format(
392 &stream->out_transfer_func,
393 &mpc->blender_params, false))
394 params = &mpc->blender_params;
395 /* there are no ROM LUTs in OUTGAM */
396 if (stream->out_transfer_func.type ==
TF_TYPE_PREDEFINED)
397 BREAK_TO_DEBUGGER();
398 }
399 }
400
--> 401 mpc->funcs->set_output_gamma(mpc, mpcc_id, params);
Then it will crash
402 return ret;
403 }
regards,
dan carpenter
[bug report] drm/amd/display: Fix ineffective setting of max bpc property
Hello Stylon Wang,
Commit fa7041d9d2fc ("drm/amd/display: Fix ineffective setting of max
bpc property") from Jun 12, 2020 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7201
create_validate_stream_for_sink()
error: we previously assumed 'dm_state' could be null (see line 7194)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
7185 struct dc_stream_state *
7186 create_validate_stream_for_sink(struct amdgpu_dm_connector *aconnector,
7187 const struct drm_display_mode
*drm_mode,
7188 const struct dm_connector_state
*dm_state,
7189 const struct dc_stream_state
*old_stream)
7190 {
7191 struct drm_connector *connector = &aconnector->base;
7192 struct amdgpu_device *adev = drm_to_adev(connector->dev);
7193 struct dc_stream_state *stream;
7194 const struct drm_connector_state *drm_state = dm_state ?
&dm_state->base : NULL;
^
This used check connector->state but then we changed it to dm_state instead
7195 int requested_bpc = drm_state ? drm_state->max_requested_bpc :
8;
7196 enum dc_status dc_result = DC_OK;
7197
7198 do {
7199 stream = create_stream_for_sink(connector, drm_mode,
7200 dm_state, old_stream,
But dm_state is dereferenced on the next line without checking. (Presumably
the NULL check can be removed).
--> 7201 requested_bpc);
7202 if (stream == NULL) {
7203 DRM_ERROR("Failed to create stream for
sink!\n");
7204 break;
7205 }
7206
7207 if (aconnector->base.connector_type ==
DRM_MODE_CONNECTOR_WRITEBACK)
regards,
dan carpenter
[bug report] drm/amdkfd: Validate queue cwsr area and eop buffer size
Hello Philip Yang,
Commit 629568d25fea ("drm/amdkfd: Validate queue cwsr area and eop
buffer size") from Jun 26, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_queue.c:265
kfd_queue_acquire_buffers()
warn: missing error code 'err'
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_queue.c
226 int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties)
227 {
228 struct kfd_topology_device *topo_dev;
229 struct amdgpu_vm *vm;
230 u32 total_cwsr_size;
231 int err;
232
233 topo_dev = kfd_topology_device_by_id(pdd->dev->id);
234 if (!topo_dev)
235 return -EINVAL;
236
237 vm = drm_priv_to_vm(pdd->drm_priv);
238 err = amdgpu_bo_reserve(vm->root.bo, false);
239 if (err)
240 return err;
241
242 err = kfd_queue_buffer_get(vm, properties->write_ptr,
&properties->wptr_bo, PAGE_SIZE);
243 if (err)
244 goto out_err_unreserve;
245
246 err = kfd_queue_buffer_get(vm, properties->read_ptr,
&properties->rptr_bo, PAGE_SIZE);
247 if (err)
248 goto out_err_unreserve;
249
250 err = kfd_queue_buffer_get(vm, (void
*)properties->queue_address,
251&properties->ring_bo,
properties->queue_size);
252 if (err)
253 goto out_err_unreserve;
254
255 /* only compute queue requires EOP buffer and CWSR area */
256 if (properties->type != KFD_QUEUE_TYPE_COMPUTE)
257 goto out_unreserve;
This is clearly a success path.
258
259 /* EOP buffer is not required for all ASICs */
260 if (properties->eop_ring_buffer_address) {
261 if (properties->eop_ring_buffer_size !=
topo_dev->node_props.eop_buffer_size) {
262 pr_debug("queue eop bo size 0x%lx not equal to
node eop buf size 0x%x\n",
263 properties->eop_buf_bo->tbo.base.size,
264 topo_dev->node_props.eop_buffer_size);
--> 265 goto out_err_unreserve;
This has err in the label name. err = -EINVAL?
266 }
267 err = kfd_queue_buffer_get(vm, (void
*)properties->eop_ring_buffer_address,
268&properties->eop_buf_bo,
269
properties->eop_ring_buffer_size);
270 if (err)
271 goto out_err_unreserve;
272 }
273
274 if (properties->ctl_stack_size !=
topo_dev->node_props.ctl_stack_size) {
275 pr_debug("queue ctl stack size 0x%x not equal to node
ctl stack size 0x%x\n",
276 properties->ctl_stack_size,
277 topo_dev->node_props.ctl_stack_size);
278 goto out_err_unreserve;
err?
279 }
280
281 if (properties->ctx_save_restore_area_size !=
topo_dev->node_props.cwsr_size) {
282 pr_debug("queue cwsr size 0x%x not equal to node cwsr
size 0x%x\n",
283 properties->ctx_save_restore_area_size,
284 topo_dev->node_props.cwsr_size);
285 goto out_err_unreserve;
err? Not sure.
286 }
287
288 total_cwsr_size = (topo_dev->node_props.cwsr_size +
topo_dev->node_props.debug_memory_size)
289 * NUM_XCC(pdd->dev->xcc_mask);
290 total_cwsr_size = ALIGN(total_cwsr_size, PAGE_SIZE);
291
292 err = kfd_queue_buffer_get(vm, (void
*)properties->ctx_save_restore_area_address,
293&properties->cwsr_bo,
total_cwsr_size);
294 if (!err)
295 goto out_unreserve;
296
297 amdgpu_bo_unreserve(vm->root.bo);
298
299 err = kfd_queue_buffer_svm_get(pdd,
properties->ctx_save_restore_area_address,
300total_cwsr_size);
301 if (err)
302 goto out_err_release;
303
304 return 0;
305
306 out_unreserve:
307 amdgpu_bo_unreserve(vm->root.bo);
308 return 0;
309
310 out_err_unreserve:
311 amdgpu_bo_unreserve(vm->root.bo);
312 out_err_release:
313 kfd_queue_release_buffers(pdd, properties);
314 return err;
315 }
regards,
dan carpenter
[bug report] drm/amd/display: DML2.1 resynchronization
p->DETBufferSizeC[k] =
p->DETBufferSizeInKByte[k] * 1024 / 2;
3816 } else {
3817 #ifdef __DML_VBA_DEBUG__
3818 dml2_printf("DML::%s: k=%u, 2/3 DET will be
used for plane0, and 1/3 for plane1\n", __func__, k);
3819 #endif
3820 p->DETBufferSizeY[k] = (unsigned
int)(math_floor2(p->DETBufferSizeInKByte[k] * 1024 * 2 / 3, 1024));
3821 p->DETBufferSizeC[k] =
p->DETBufferSizeInKByte[k] * 1024 - p->DETBufferSizeY[k];
3822 }
3823
3824 #ifdef __DML_VBA_DEBUG__
3825 dml2_printf("DML::%s: k=%u SwathHeightY = %u\n",
__func__, k, p->SwathHeightY[k]);
3826 dml2_printf("DML::%s: k=%u SwathHeightC = %u\n",
__func__, k, p->SwathHeightC[k]);
3827 dml2_printf("DML::%s: k=%u full_swath_bytes_l = %u\n",
__func__, k, p->full_swath_bytes_l[k]);
3828 dml2_printf("DML::%s: k=%u full_swath_bytes_c = %u\n",
__func__, k, p->full_swath_bytes_c[k]);
3829 dml2_printf("DML::%s: k=%u RoundedUpSwathSizeBytesY =
%u\n", __func__, k, RoundedUpSwathSizeBytesY[k]);
3830 dml2_printf("DML::%s: k=%u RoundedUpSwathSizeBytesC =
%u\n", __func__, k, RoundedUpSwathSizeBytesC[k]);
3831 dml2_printf("DML::%s: k=%u DETBufferSizeInKByte =
%u\n", __func__, k, p->DETBufferSizeInKByte[k]);
3832 dml2_printf("DML::%s: k=%u DETBufferSizeY = %u\n",
__func__, k, p->DETBufferSizeY[k]);
3833 dml2_printf("DML::%s: k=%u DETBufferSizeC = %u\n",
__func__, k, p->DETBufferSizeC[k]);
3834 dml2_printf("DML::%s: k=%u
ViewportSizeSupportPerSurface = %u\n", __func__, k,
p->ViewportSizeSupportPerSurface[k]);
3835 #endif
3836
3837 }
3838
3839 *p->compbuf_reserved_space_64b = 2 * p->pixel_chunk_size_kbytes
* 1024 / 64;
3840 if (*p->UnboundedRequestEnabled) {
^^^
Everything check if enabled is true
3841 *p->compbuf_reserved_space_64b = (unsigned
int)math_ceil2(math_max2(*p->compbuf_reserved_space_64b,
3842 (double)(p->rob_buffer_size_kbytes * 1024 / 64)
- (double)(RoundedUpSwathSizeBytesY[SurfaceDoingUnboundedRequest] *
TTUFIFODEPTH / (p->mrq_present ? MAXIMUMCOMPRESSION : 1) / 64)), 1.0);
3843 #ifdef __DML_VBA_DEBUG__
3844 dml2_printf("DML::%s: RoundedUpSwathSizeBytesY[%d] =
%u\n", __func__, SurfaceDoingUnboundedRequest,
RoundedUpSwathSizeBytesY[SurfaceDoingUnboundedRequest]);
3845 dml2_printf("DML::%s: rob_buffer_size_kbytes = %u\n",
__func__, p->rob_buffer_size_kbytes);
3846 #endif
3847 }
3848 #ifdef __DML_VBA_DEBUG__
3849 dml2_printf("DML::%s: compbuf_reserved_space_64b = %u\n",
__func__, *p->compbuf_reserved_space_64b);
3850 #endif
3851
3852 *p->hw_debug5 = false;
3853 for (unsigned int k = 0; k < p->NumberOfActiveSurfaces; ++k) {
3854 if (!(p->mrq_present) && (!p->UnboundedRequestEnabled)
&& (TotalActiveDPP == 1)
^^
This checks if the pointer to enabled is NULL. Probably *p was intended.
3855 &&
p->display_cfg->plane_descriptors[k].surface.dcc.enable
3856 && ((p->rob_buffer_size_kbytes * 1024 *
(p->mrq_present ? MAXIMUMCOMPRESSION : 1)
regards,
dan carpenter
Re: [PATCH v2] drm/amd/amdgpu: Properly tune the size of struct
On Mon, Jul 29, 2024 at 10:00:00PM +0800, WangYuli wrote:
> The struct assertion is failed because sparse cannot parse
> `#pragma pack(push, 1)` and `#pragma pack(pop)` correctly.
> GCC's output is still 1-byte-aligned. No harm to memory layout.
>
> The error can be filtered out by sparse-diff, but sometimes
> multiple lines queezed into one, making the sparse-diff thinks
> its a new error. I'm trying to aviod this by fixing errors.
>
> Link: https://lore.kernel.org/all/20230620045919.492128-1-su...@nfschina.com/
> Fixes: 1721bc1b2afa ("drm/amdgpu: Update VF2PF interface")
> Signed-off-by: Su Hui
> Signed-off-by: Dan Carpenter
> Signed-off-by: wenlunpeng
> Signed-off-by: WangYuli
> ---
Thanks for doing this, but these Signed-off-by tags aren't correct.
Signed-off-by is like signing a legal document. It came out of the SCO
lawsuits. SCO was a scam where SCO claimed that Linux stole their
source code and tried to get Linux users to pay licensing fees. (No one
stole anything and SCO didn't even own the copyrights to Unix, those
belonged to IBM).
You could maybe use the Reported-by: or Suggested-by: tags for Su Hui.
But the rest of us could just be Cc:
Cc: Dan Carpenter
Cc: wenlunpeng
Reported-by: Su Hui
Signed-off-by: WangYuli
regards,
dan carpenter
[PATCH] drm/amd/display: re-indent dc_power_down_on_boot()
These lines are indented too far. Clean the whitespace.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 8eefba757da4..f64d7229eb6c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -5043,11 +5043,10 @@ void dc_interrupt_ack(struct dc *dc, enum dc_irq_source
src)
void dc_power_down_on_boot(struct dc *dc)
{
if (dc->ctx->dce_environment != DCE_ENV_VIRTUAL_HW &&
- dc->hwss.power_down_on_boot) {
-
- if (dc->caps.ips_support)
- dc_exit_ips_for_hw_access(dc);
+ dc->hwss.power_down_on_boot) {
+ if (dc->caps.ips_support)
+ dc_exit_ips_for_hw_access(dc);
dc->hwss.power_down_on_boot(dc);
}
}
--
2.43.0
Re: [PATCH] drm/amd/display: re-indent dc_power_down_on_boot()
On Wed, Apr 24, 2024 at 03:33:11PM +0200, Christian König wrote:
> Am 24.04.24 um 15:20 schrieb Dan Carpenter:
> > On Wed, Apr 24, 2024 at 03:11:08PM +0200, Christian König wrote:
> > > Am 24.04.24 um 13:41 schrieb Dan Carpenter:
> > > > These lines are indented too far. Clean the whitespace.
> > > >
> > > > Signed-off-by: Dan Carpenter
> > > > ---
> > > >drivers/gpu/drm/amd/display/dc/core/dc.c | 7 +++
> > > >1 file changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> > > > b/drivers/gpu/drm/amd/display/dc/core/dc.c
> > > > index 8eefba757da4..f64d7229eb6c 100644
> > > > --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> > > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> > > > @@ -5043,11 +5043,10 @@ void dc_interrupt_ack(struct dc *dc, enum
> > > > dc_irq_source src)
> > > >void dc_power_down_on_boot(struct dc *dc)
> > > >{
> > > > if (dc->ctx->dce_environment != DCE_ENV_VIRTUAL_HW &&
> > > > - dc->hwss.power_down_on_boot) {
> > > > -
> > > > - if (dc->caps.ips_support)
> > > > - dc_exit_ips_for_hw_access(dc);
> > > > + dc->hwss.power_down_on_boot) {
> > > > + if (dc->caps.ips_support)
> > > > + dc_exit_ips_for_hw_access(dc);
> > > Well while at it can't the two ifs be merged here?
> > >
> > > (I don't know this code to well, but it looks like it).
> > >
> > I'm sorry, I don't see what you're saying.
>
> The indentation was so messed up that I though the call to
> power_down_on_boot() was after both ifs, but it is still inside the first.
>
> So your patch is actually right, sorry for the noise.
Okay, but let me send a v2 anyway to delete the extra blank line.
regards,
dan carpenter
Re: [PATCH] drm/amd/display: re-indent dc_power_down_on_boot()
On Wed, Apr 24, 2024 at 03:11:08PM +0200, Christian König wrote:
> Am 24.04.24 um 13:41 schrieb Dan Carpenter:
> > These lines are indented too far. Clean the whitespace.
> >
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/gpu/drm/amd/display/dc/core/dc.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc.c
> > index 8eefba757da4..f64d7229eb6c 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> > @@ -5043,11 +5043,10 @@ void dc_interrupt_ack(struct dc *dc, enum
> > dc_irq_source src)
> > void dc_power_down_on_boot(struct dc *dc)
> > {
> > if (dc->ctx->dce_environment != DCE_ENV_VIRTUAL_HW &&
> > - dc->hwss.power_down_on_boot) {
> > -
> > - if (dc->caps.ips_support)
> > - dc_exit_ips_for_hw_access(dc);
> > + dc->hwss.power_down_on_boot) {
> > + if (dc->caps.ips_support)
> > + dc_exit_ips_for_hw_access(dc);
>
> Well while at it can't the two ifs be merged here?
>
> (I don't know this code to well, but it looks like it).
>
I'm sorry, I don't see what you're saying.
I probably should have deleted the other blank line as well, though.
It introduces a checkpatch.pl --strict warning.
regards,
dan carpenter
[PATCH v2] drm/amd/display: re-indent dc_power_down_on_boot()
These lines are indented too far. Clean the whitespace.
Signed-off-by: Dan Carpenter
---
v2: Delete another blank line (checkpatch.pl --strict).
drivers/gpu/drm/amd/display/dc/core/dc.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 3e16041bf4f9..5a0835f884a8 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -5192,11 +5192,9 @@ void dc_interrupt_ack(struct dc *dc, enum dc_irq_source
src)
void dc_power_down_on_boot(struct dc *dc)
{
if (dc->ctx->dce_environment != DCE_ENV_VIRTUAL_HW &&
- dc->hwss.power_down_on_boot) {
-
- if (dc->caps.ips_support)
- dc_exit_ips_for_hw_access(dc);
-
+ dc->hwss.power_down_on_boot) {
+ if (dc->caps.ips_support)
+ dc_exit_ips_for_hw_access(dc);
dc->hwss.power_down_on_boot(dc);
}
}
--
2.43.0
[PATCH] drm/amdgpu: Fix signedness bug in sdma_v4_0_process_trap_irq()
The "instance" variable needs to be signed for the error handling to work.
Fixes: b34ddc71267a ("drm/amdgpu: add error handle to avoid out-of-bounds")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 101038395c3b..772604feb6ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2017,7 +2017,7 @@ static int sdma_v4_0_process_trap_irq(struct
amdgpu_device *adev,
struct amdgpu_irq_src *source,
struct amdgpu_iv_entry *entry)
{
- uint32_t instance;
+ int instance;
DRM_DEBUG("IH: SDMA trap\n");
instance = sdma_v4_0_irq_id_to_seq(entry->client_id);
--
2.43.0
[PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()
Smatch complains because some lines are indented more than they should
be. I went a bit crazy re-indenting this. ;)
The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.
I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit. It's more readable without the line breaks.
I also tried to align it so you can see what is changing on each line.
Signed-off-by: Dan Carpenter
---
.../display/dc/dpp/dcn401/dcn401_dpp_dscl.c | 93 ++-
1 file changed, 30 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct dpp
*dpp_base,
const struct scaler_data *scl_data)
{
struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+ const struct dscl_prog_data *data;
if (memcmp(&dpp->scl_data, scl_data, sizeof(*scl_data)) == 0)
return;
PERF_TRACE();
dpp->scl_data = *scl_data;
- // ISHARP_EN
- REG_SET(ISHARP_MODE, 0,
- ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
- // ISHARP_NOISEDET_EN
- REG_SET(ISHARP_MODE, 0,
- ISHARP_NOISEDET_EN,
scl_data->dscl_prog_data.isharp_noise_det.enable);
- // ISHARP_NOISEDET_MODE
- REG_SET(ISHARP_MODE, 0,
- ISHARP_NOISEDET_MODE,
scl_data->dscl_prog_data.isharp_noise_det.mode);
- // ISHARP_NOISEDET_UTHRE
- REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
- ISHARP_NOISEDET_UTHRE,
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
- // ISHARP_NOISEDET_DTHRE
- REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
- ISHARP_NOISEDET_DTHRE,
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
- REG_SET(ISHARP_MODE, 0,
- ISHARP_NOISEDET_MODE,
scl_data->dscl_prog_data.isharp_noise_det.mode);
- // ISHARP_NOISEDET_UTHRE
- REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
- ISHARP_NOISEDET_UTHRE,
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
- // ISHARP_NOISEDET_DTHRE
- REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
- ISHARP_NOISEDET_DTHRE,
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
- // ISHARP_NOISEDET_PWL_START_IN
- REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
- ISHARP_NOISEDET_PWL_START_IN,
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);
- // ISHARP_NOISEDET_PWL_END_IN
- REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
- ISHARP_NOISEDET_PWL_END_IN,
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);
- // ISHARP_NOISEDET_PWL_SLOPE
- REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
- ISHARP_NOISEDET_PWL_SLOPE,
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);
- // ISHARP_LBA_MODE
- REG_SET(ISHARP_MODE, 0,
- ISHARP_LBA_MODE,
scl_data->dscl_prog_data.isharp_lba.mode);
- // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
- // ISHARP_FMT_MODE
- REG_SET(ISHARP_MODE, 0,
- ISHARP_FMT_MODE,
scl_data->dscl_prog_data.isharp_fmt.mode);
- // ISHARP_FMT_NORM
- REG_SET(ISHARP_MODE, 0,
- ISHARP_FMT_NORM,
scl_data->dscl_prog_data.isharp_fmt.norm);
- // ISHARP_DELTA_LUT
- dpp401_dscl_set_isharp_filter(dpp,
scl_data->dscl_prog_data.isharp_delta);
- // ISHARP_NLDELTA_SCLIP_EN_P
- REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
- ISHARP_NLDELTA_SCLIP_EN_P,
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);
- // ISHARP_NLDELTA_SCLIP_PIVOT_P
- REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
- ISHARP_NLDELTA_SCLIP_PIVOT_P,
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);
- // ISHARP_NLDELTA_SCLIP_SLOPE_P
- REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
- ISHARP_NLDELTA_SCLIP_SLOPE_P,
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);
- // ISHARP_NLDELTA_SCLIP_EN_N
-
[PATCH] drm/amd/pm: Fix error code in vega10_hwmgr_backend_init()
Return -EINVAL on error instead of success. Also on the success path,
return a literal zero instead of "return result;"
Fixes: e098bc9612c2 ("drm/amd/pm: optimize the power related source code
layout")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 37c915d7723c..9b9f8615070a 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -924,7 +924,7 @@ static int vega10_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
data->total_active_cus = adev->gfx.cu_info.number;
if (!hwmgr->not_vf)
- return result;
+ return -EINVAL;
/* Setup default Overdrive Fan control settings */
data->odn_fan_table.target_fan_speed =
@@ -947,7 +947,7 @@ static int vega10_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
"Mem Channel Index Exceeded maximum!",
return -EINVAL);
- return result;
+ return 0;
}
static int vega10_init_sclk_threshold(struct pp_hwmgr *hwmgr)
--
2.43.0
[bug report] drm/amd/display: Separate setting and programming of cursor
Hello Harry Wentland,
Commit f63f86b5affc ("drm/amd/display: Separate setting and
programming of cursor") from Mar 15, 2024 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:398
dc_stream_program_cursor_position()
error: we previously assumed 'stream' could be null (see line 397)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c
389 bool dc_stream_program_cursor_position(
390 struct dc_stream_state *stream,
391 const struct dc_cursor_position *position)
392 {
393 struct dc *dc;
394 bool reset_idle_optimizations = false;
395 const struct dc_cursor_position *old_position;
396
397 old_position = stream ? &stream->cursor_position : NULL;
The patch adds a NULL check
--> 398 dc = stream->ctx->dc;
The old code didn't check
399
400 if (dc_stream_set_cursor_position(stream, position)) {
401 dc_z10_restore(dc);
402
403 /* disable idle optimizations if enabling cursor */
404 if (dc->idle_optimizations_allowed &&
405 (!old_position->enable ||
dc->debug.exit_idle_opt_for_cursor_updates) &&
406 position->enable) {
407 dc_allow_idle_optimizations(dc, false);
regards,
dan carpenter
[bug report] drm/amdgpu: Add sdma v7_0 ip block support (v7)
Hello Likun Gao,
Commit b412351e91bd ("drm/amdgpu: Add sdma v7_0 ip block support
(v7)") from Jul 4, 2023 (linux-next), leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c:171 sdma_v7_0_ring_set_wptr()
warn: duplicate check '*is_queue_unmap' (previous on line 161)
drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
140 static void sdma_v7_0_ring_set_wptr(struct amdgpu_ring *ring)
141 {
142 struct amdgpu_device *adev = ring->adev;
143 uint32_t *wptr_saved;
144 uint32_t *is_queue_unmap;
145 uint64_t aggregated_db_index;
146 uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_DMA].mqd_size;
147
148 DRM_DEBUG("Setting write pointer\n");
149
150 if (ring->is_mes_queue) {
151 wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
152 is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size +
Set here
153 sizeof(uint32_t));
154 aggregated_db_index =
155 amdgpu_mes_get_aggregated_doorbell_index(adev,
156 ring->hw_prio);
157
158 atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
159 ring->wptr << 2);
160 *wptr_saved = ring->wptr << 2;
161 if (*is_queue_unmap) {
^^^
Checked here
162 WDOORBELL64(aggregated_db_index, ring->wptr <<
2);
163 DRM_DEBUG("calling WDOORBELL64(0x%08x,
0x%016llx)\n",
164 ring->doorbell_index,
ring->wptr << 2);
165 WDOORBELL64(ring->doorbell_index, ring->wptr <<
2);
166 } else {
167 DRM_DEBUG("calling WDOORBELL64(0x%08x,
0x%016llx)\n",
168 ring->doorbell_index,
ring->wptr << 2);
169 WDOORBELL64(ring->doorbell_index, ring->wptr <<
2);
170
--> 171 if (*is_queue_unmap)
^^^
This is dead code. We know it's false.
172 WDOORBELL64(aggregated_db_index,
173 ring->wptr << 2);
174 }
175 } else {
176 if (ring->use_doorbell) {
177 DRM_DEBUG("Using doorbell -- "
178 "wptr_offs == 0x%08x "
regards,
dan carpenter
[bug report] drm/amd/display: Do cursor programming with rest of pipe
Hello Harry Wentland,
Commit 66eba12a5482 ("drm/amd/display: Do cursor programming with
rest of pipe") from Mar 15, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:8433
amdgpu_dm_update_cursor()
error: we previously assumed 'afb' could be null (see line 8388)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
8379 static void amdgpu_dm_update_cursor(struct drm_plane *plane,
8380 struct drm_plane_state
*old_plane_state,
8381 struct dc_stream_update *update)
8382 {
8383 struct amdgpu_device *adev = drm_to_adev(plane->dev);
8384 struct amdgpu_framebuffer *afb =
to_amdgpu_framebuffer(plane->state->fb);
8385 struct drm_crtc *crtc = afb ? plane->state->crtc :
old_plane_state->crtc;
^
8386 struct dm_crtc_state *crtc_state = crtc ?
to_dm_crtc_state(crtc->state) : NULL;
8387 struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
8388 uint64_t address = afb ? afb->address : 0;
^
Checks for NULL
8389 struct dc_cursor_position position = {0};
8390 struct dc_cursor_attributes attributes;
8391 int ret;
8392
8393 if (!plane->state->fb && !old_plane_state->fb)
8394 return;
8395
8396 drm_dbg_atomic(plane->dev, "crtc_id=%d with size %d to %d\n",
8397amdgpu_crtc->crtc_id, plane->state->crtc_w,
8398plane->state->crtc_h);
8399
8400 ret = amdgpu_dm_plane_get_cursor_position(plane, crtc,
&position);
8401 if (ret)
8402 return;
8403
8404 if (!position.enable) {
8405 /* turn off cursor */
8406 if (crtc_state && crtc_state->stream) {
8407
dc_stream_set_cursor_position(crtc_state->stream,
8408 &position);
8409 update->cursor_position =
&crtc_state->stream->cursor_position;
8410 }
8411 return;
8412 }
8413
8414 amdgpu_crtc->cursor_width = plane->state->crtc_w;
8415 amdgpu_crtc->cursor_height = plane->state->crtc_h;
8416
8417 memset(&attributes, 0, sizeof(attributes));
8418 attributes.address.high_part = upper_32_bits(address);
8419 attributes.address.low_part = lower_32_bits(address);
8420 attributes.width = plane->state->crtc_w;
8421 attributes.height= plane->state->crtc_h;
8422 attributes.color_format =
CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA;
8423 attributes.rotation_angle= 0;
8424 attributes.attribute_flags.value = 0;
8425
8426 /* Enable cursor degamma ROM on DCN3+ for implicit sRGB
degamma in DRM
8427 * legacy gamma setup.
8428 */
8429 if (crtc_state->cm_is_degamma_srgb &&
8430 adev->dm.dc->caps.color.dpp.gamma_corr)
8431 attributes.attribute_flags.bits.ENABLE_CURSOR_DEGAMMA
= 1;
8432
--> 8433 attributes.pitch = afb->base.pitches[0] /
afb->base.format->cpp[0];
^ ^
Unchecked dereferences
8434
8435 if (crtc_state->stream) {
8436 if
(!dc_stream_set_cursor_attributes(crtc_state->stream,
8437 &attributes))
8438 DRM_ERROR("DC failed to set cursor
attributes\n");
8439
8440 update->cursor_attributes =
&crtc_state->stream->cursor_attributes;
8441
8442 if (!dc_stream_set_cursor_position(crtc_state->stream,
8443&position))
8444 DRM_ERROR("DC failed to set cursor
position\n");
8445
8446 update->cursor_position =
&crtc_state->stream->cursor_position;
8447 }
8448 }
regards,
dan carpenter
[bug report] drm/amdkfd: mark GFX12 system and peer GPU memory mappings as MTYPE_NC
Hello Sreekant Somasekharan,
This is a semi-automatic email about new static checker warnings.
Commit 628e1ace2379 ("drm/amdkfd: mark GFX12 system and peer GPU
memory mappings as MTYPE_NC") from Mar 26, 2024, leads to the
following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c:518 gmc_v12_0_get_vm_pte()
warn: variable dereferenced before check 'bo' (see line 500)
drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
499 struct amdgpu_bo *bo = mapping->bo_va->base.bo;
500 struct amdgpu_device *bo_adev = amdgpu_ttm_adev(bo->tbo.bdev);
501 bool coherent = bo->flags & AMDGPU_GEM_CREATE_COHERENT;
^
502 bool is_system = bo->tbo.resource->mem_type == TTM_PL_SYSTEM;
^^^
The patch adds unchecked dereferences.
503
504
505 *flags &= ~AMDGPU_PTE_EXECUTABLE;
506 *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
507
508 *flags &= ~AMDGPU_PTE_MTYPE_GFX12_MASK;
509 *flags |= (mapping->flags & AMDGPU_PTE_MTYPE_GFX12_MASK);
510
511 if (mapping->flags & AMDGPU_PTE_PRT_GFX12) {
512 *flags |= AMDGPU_PTE_PRT_GFX12;
513 *flags |= AMDGPU_PTE_SNOOPED;
514 *flags |= AMDGPU_PTE_SYSTEM;
515 *flags &= ~AMDGPU_PTE_VALID;
516 }
517
518 if (bo && bo->flags & (AMDGPU_GEM_CREATE_COHERENT |
^^
But previously we assumed bo could be NULL.
519 AMDGPU_GEM_CREATE_UNCACHED))
520 *flags = (*flags & ~AMDGPU_PTE_MTYPE_GFX12_MASK) |
regards,
dan carpenter
[PATCH] drm/amdgpu: delete unnecessary check
The "ret" variable is zero. No need to check.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index a037e8fba29f..4d50fb039509 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2807,7 +2807,7 @@ static void amdgpu_ras_do_page_retirement(struct
work_struct *work)
static void amdgpu_ras_poison_creation_handler(struct amdgpu_device *adev,
uint32_t timeout_ms)
{
- int ret = 0;
+ int ret;
struct ras_ecc_log_info *ecc_log;
struct ras_query_if info;
uint32_t timeout = timeout_ms;
@@ -2836,8 +2836,7 @@ static void amdgpu_ras_poison_creation_handler(struct
amdgpu_device *adev,
return;
}
- if (!ret)
- schedule_delayed_work(&ras->page_retirement_dwork, 0);
+ schedule_delayed_work(&ras->page_retirement_dwork, 0);
}
static int amdgpu_ras_poison_consumption_handler(struct amdgpu_device *adev,
--
2.43.0
[bug report] drm/amd/display: Introduce DML2
Hello Qingqing Zhuo,
This is a semi-automatic email about new static checker warnings.
Commit 7966f319c66d ("drm/amd/display: Introduce DML2") from Jul 28,
2023, leads to the following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:576
dml2_validate_and_build_resource()
warn: variable dereferenced before check 'context' (see line 569)
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:665
dml2_validate_only()
warn: variable dereferenced before check 'context' (see line 662)
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c
568 {
569 struct dml2_context *dml2 = context->bw_ctx.dml2;
^
Dereferenced
570 struct dml2_wrapper_scratch *s = &dml2->v20.scratch;
571 struct dml2_dcn_clocks out_clks;
572 unsigned int result = 0;
573 bool need_recalculation = false;
574 uint32_t cstate_enter_plus_exit_z8_ns;
575
576 if (!context || context->stream_count == 0)
Checked too late
577 return true;
578
regards,
dan carpenter
Re: [PATCH v3] drm/amdkfd: Remove bo NULL check in gmc_v12_0_get_vm_pte() function
On Tue, May 14, 2024 at 08:31:03PM -0400, Sreekant Somasekharan wrote: > Remove bo NULL check in amdgpu/gmc_v12_0.c:gmc_v12_0_get_vm_pte() function > to fix smatch warning: > > 'drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c:518 gmc_v12_0_get_vm_pte() > warn: variable dereferenced before check 'bo' (see line 500)' > > Signed-off-by: Sreekant Somasekharan > Suggested-by: Dan Carpenter > --- This is fine, but you're overthinking these... The v1 patch was also fine. regards, dan carpenter
[bug report] drm/amd/display: Find max flickerless instant vtotal delta
Hello Ethan Bitnun,
Commit bd051aa2fcfb ("drm/amd/display: Find max flickerless instant
vtotal delta") from Apr 1, 2024 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:1006
dc_stream_get_max_flickerless_instant_vtotal_delta()
warn: always true condition '((stream->timing.v_total - safe_refresh_v_total)
>= 0) => (0-u32max >= 0)'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c:1008
dc_stream_get_max_flickerless_instant_vtotal_delta()
warn: always true condition '((safe_refresh_v_total - stream->timing.v_total)
>= 0) => (0-u32max >= 0)'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.c
990 static unsigned int
dc_stream_get_max_flickerless_instant_vtotal_delta(struct dc_stream_state
*stream, bool is_gaming, bool increase)
991 {
992 if (stream->timing.v_total * stream->timing.h_total == 0)
993 return 0;
994
995 int current_refresh_hz = (int)div64_s64((long
long)stream->timing.pix_clk_100hz*100,
stream->timing.v_total*stream->timing.h_total);
996
997 int safe_refresh_hz =
dc_stream_calculate_flickerless_refresh_rate(stream,
998
dc_stream_get_brightness_millinits_from_refresh(stream, current_refresh_hz),
999
current_refresh_hz,
1000 is_gaming,
1001 increase);
1002
1003 int safe_refresh_v_total = (int)div64_s64((long
long)stream->timing.pix_clk_100hz*100, safe_refresh_hz*stream->timing.h_total);
1004
1005 if (increase)
--> 1006 return ((stream->timing.v_total -
safe_refresh_v_total) >= 0) ? (stream->timing.v_total - safe_refresh_v_total) :
0;
^
stream->timing.v_total is u32 so it makes the subtract u32 thus it's
always >= 0.
1007
1008 return ((safe_refresh_v_total - stream->timing.v_total) >= 0)
? (safe_refresh_v_total - stream->timing.v_total) : 0;
^^^
Same.
1009 }
regards,
dan carpenter
[bug report] drm/amdgpu: Workaround to skip kiq ring test during ras gpu recovery
Hello Stanley.Yang,
The patch b1338a8e71ac: "drm/amdgpu: Workaround to skip kiq ring test
during ras gpu recovery" from Oct 17, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:604 amdgpu_get_xgmi_hive()
warn: sleeping in atomic context
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
591 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device
*adev)
592 {
593 struct amdgpu_hive_info *hive = NULL;
594 int ret;
595
596 if (!adev->gmc.xgmi.hive_id)
597 return NULL;
598
599 if (adev->hive) {
600 kobject_get(&adev->hive->kobj);
601 return adev->hive;
602 }
603
--> 604 mutex_lock(&xgmi_mutex);
^^^
Shhh The mutexes are sleeping.
605
606 list_for_each_entry(hive, &xgmi_hive_list, node) {
The caller is amdgpu_gfx_disable_kcq():
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
516 spin_lock(&kiq->ring_lock);
^^^
Holding a spin lock.
517 if (amdgpu_ring_alloc(kiq_ring, kiq->pmf->unmap_queues_size *
518 adev->gfx.num_compute_rings)) {
519 spin_unlock(&kiq->ring_lock);
520 return -ENOMEM;
521 }
522
523 for (i = 0; i < adev->gfx.num_compute_rings; i++) {
524 j = i + xcc_id * adev->gfx.num_compute_rings;
525 kiq->pmf->kiq_unmap_queues(kiq_ring,
526 &adev->gfx.compute_ring[j],
527 RESET_QUEUES, 0, 0);
528 }
529
530 /**
531 * This is workaround: only skip kiq_ring test
532 * during ras recovery in suspend stage for gfx9.4.3
533 */
534 hive = amdgpu_get_xgmi_hive(adev);
^^
Can't call a sleeping function when holding a spin_lock.
535 if (hive) {
536 hive_ras_recovery = atomic_read(&hive->ras_recovery);
537 amdgpu_put_xgmi_hive(hive);
538 }
539
540 ras = amdgpu_ras_get_context(adev);
541 if ((amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4,
3)) &&
542 ras && (atomic_read(&ras->in_recovery) ||
hive_ras_recovery)) {
543 spin_unlock(&kiq->ring_lock);
544 return 0;
545 }
546
547 if (kiq_ring->sched.ready && !adev->job_hang)
548 r = amdgpu_ring_test_helper(kiq_ring);
549 spin_unlock(&kiq->ring_lock);
regards,
dan carpenter
[bug report] drm/amdgpu/vpe: enable vpe dpm
Hello Peyton Lee,
The patch 5f82a0c90cca: "drm/amdgpu/vpe: enable vpe dpm" from Dec 12,
2023 (linux-next), leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c:62 vpe_u1_8_from_fraction() warn:
unsigned 'numerator' is never less than zero.
drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c:63 vpe_u1_8_from_fraction() warn:
unsigned 'denominator' is never less than zero.
drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
60 static uint16_t vpe_u1_8_from_fraction(uint16_t numerator, uint16_t
denominator)
61 {
--> 62 bool arg1_negative = numerator < 0;
63 bool arg2_negative = denominator < 0;
uint16_t can't be negative.
64
65 uint16_t arg1_value = (uint16_t)(arg1_negative ? -numerator :
numerator);
66 uint16_t arg2_value = (uint16_t)(arg2_negative ? -denominator :
denominator);
67
68 uint16_t remainder;
69
regards,
dan carpenter
[PATCH] drm/amdgpu: fix return value in aca_bank_hwip_is_matched()
The aca_bank_hwip_is_matched() function is type bool. This error path
return -EINVAL which is cast to true, but it should return false
instead.
Fixes: 22a4fa4709e3 ("drm/amdgpu: implement RAS ACA driver framework")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
index 6074a529caf7..1d3ae7c241e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
@@ -182,7 +182,7 @@ static bool aca_bank_hwip_is_matched(struct aca_bank *bank,
enum aca_hwip_type t
u64 ipid;
if (!bank || type == ACA_HWIP_TYPE_UNKNOW)
- return -EINVAL;
+ return false;
hwip = &aca_hwid_mcatypes[type];
if (!hwip->hwid)
--
2.43.0
[bug report] drm/amdgpu: Auto-validate DMABuf imports in compute VMs
Hello Felix Kuehling,
This is a semi-automatic email about new static checker warnings.
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1480 amdgpu_vm_handle_moved()
warn: variable dereferenced before check 'bo_va->base.bo' (see line 1453)
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
1452 base.vm_status);
1453 resv = bo_va->base.bo->tbo.base.resv;
Unchecked dereference
1454 spin_unlock(&vm->status_lock);
1455
1456 /* Try to reserve the BO to avoid clearing its ptes */
1457 if (!adev->debug_vm && dma_resv_trylock(resv)) {
1458 clear = false;
1459 unlock = true;
1460 /* The caller is already holding the reservation lock */
1461 } else if (ticket && dma_resv_locking_ctx(resv) ==
ticket) {
1462 clear = false;
1463 unlock = false;
1464 /* Somebody else is using the BO right now */
1465 } else {
1466 clear = true;
1467 unlock = false;
1468 }
1469
1470 r = amdgpu_vm_bo_update(adev, bo_va, clear);
1471
1472 if (unlock)
1473 dma_resv_unlock(resv);
1474 if (r)
1475 return r;
1476
1477 /* Remember evicted DMABuf imports in compute VMs for
later
1478 * validation
1479 */
1480 if (vm->is_compute_context && bo_va->base.bo &&
^^
The patch adds this NULL check but hopefully it's not required.
1481 bo_va->base.bo->tbo.base.import_attach &&
1482 (!bo_va->base.bo->tbo.resource ||
regards,
dan carpenter
[bug report] drm/amd/display: Add dpia display mode validation logic
Hello Meenakshikumar Somasundaram,
The patch 59f1622a5f05: "drm/amd/display: Add dpia display mode
validation logic" from Dec 5, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_dpia_bw.c:208
get_host_router_total_dp_tunnel_bw()
error: buffer overflow 'dc->links' 12 <= 12
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_dpia_bw.c
192 static int get_host_router_total_dp_tunnel_bw(const struct dc *dc,
uint8_t hr_index)
193 {
194 uint8_t lowest_dpia_index = get_lowest_dpia_index(dc->links[0]);
195 uint8_t hr_index_temp = 0;
196 struct dc_link *link_dpia_primary, *link_dpia_secondary;
197 int total_bw = 0;
198
199 for (uint8_t i = 0; i < MAX_PIPES * 2; ++i) {
200
201 if (!dc->links[i] || dc->links[i]->ep_type !=
DISPLAY_ENDPOINT_USB4_DPIA)
202 continue;
203
204 hr_index_temp = (dc->links[i]->link_index -
lowest_dpia_index) / 2;
205
206 if (hr_index_temp == hr_index) {
207 link_dpia_primary = dc->links[i];
--> 208 link_dpia_secondary = dc->links[i + 1];
Imagine "i = MAX_PIPES * 2 - 1" then that means [i + 1] is out of
bounds.
209
210 /**
211 * If BW allocation enabled on both DPIAs, then
212 * HR BW = Estimated(dpia_primary) +
Allocated(dpia_secondary)
213 * otherwise HR BW = Estimated(bw alloc enabled
dpia)
214 */
215 if ((link_dpia_primary->hpd_status &&
216
link_dpia_primary->dpia_bw_alloc_config.bw_alloc_enabled) &&
217 (link_dpia_secondary->hpd_status &&
218
link_dpia_secondary->dpia_bw_alloc_config.bw_alloc_enabled)) {
219 total_bw +=
link_dpia_primary->dpia_bw_alloc_config.estimated_bw +
220
link_dpia_secondary->dpia_bw_alloc_config.allocated_bw;
221 } else if (link_dpia_primary->hpd_status &&
222
link_dpia_primary->dpia_bw_alloc_config.bw_alloc_enabled) {
223 total_bw =
link_dpia_primary->dpia_bw_alloc_config.estimated_bw;
224 } else if (link_dpia_secondary->hpd_status &&
225
link_dpia_secondary->dpia_bw_alloc_config.bw_alloc_enabled) {
226 total_bw +=
link_dpia_secondary->dpia_bw_alloc_config.estimated_bw;
227 }
228 break;
229 }
230 }
231
232 return total_bw;
233 }
regards,
dan carpenter
[PATCH] drm/amdgpu: check for allocation failure in amdgpu_vkms_sw_init()
Check whether the kcalloc() fails and return -ENOMEM if it does.
Fixes: eeba0b9046fc ("drm/amdgpu: create amdgpu_vkms (v4)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 50bdc39733aa..ce982afeff91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -482,6 +482,8 @@ static int amdgpu_vkms_sw_init(void *handle)
return r;
adev->amdgpu_vkms_output = kcalloc(adev->mode_info.num_crtc,
sizeof(struct amdgpu_vkms_output), GFP_KERNEL);
+ if (!adev->amdgpu_vkms_output)
+ return -ENOMEM;
/* allocate crtcs, encoders, connectors */
for (i = 0; i < adev->mode_info.num_crtc; i++) {
--
2.20.1
[bug report] drm/amd/display: check top_pipe_to_program pointer
dc->hwss.update_plane_addr(dc,
pipe_ctx);
3053 }
3054 }
3055
3056 }
3057
3058 if (should_lock_all_pipes &&
dc->hwss.interdependent_update_lock)
3059 dc->hwss.interdependent_update_lock(dc, context, false);
3060 else
3061 dc->hwss.pipe_control_lock(dc, top_pipe_to_program,
false);
3062
3063 if ((update_type != UPDATE_TYPE_FAST) &&
stream->update_flags.bits.dsc_changed)
^^
This condition is exactly the same as the one where we added a NULL
check at the start of the function.
3064 if
(top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
^^^
Unchecked dereference.
Ideally someone would know if "top_pipe_to_program" can really be NULL
or not. Adding NULL checks will make the static checkers happy but it
isn't necessarily the correct fix.
3065
top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
3066
top_pipe_to_program->stream_res.tg,
regards,
dan carpenter
[bug report] drm/amdkfd: process exit and retry fault race
different pages of same range */
2657 if (timestamp < AMDGPU_SVM_RANGE_RETRY_FAULT_PENDING) {
2658 pr_debug("svms 0x%p [0x%lx %lx] already restored\n",
2659 svms, prange->start, prange->last);
2660 goto out_unlock_range;
Same.
2661 }
2662
2663 /* __do_munmap removed VMA, return success as we are handling
stale
2664 * retry fault.
2665 */
2666 vma = find_vma(mm, addr << PAGE_SHIFT);
2667 if (!vma || (addr << PAGE_SHIFT) < vma->vm_start) {
2668 pr_debug("address 0x%llx VMA is removed\n", addr);
2669 r = 0;
2670 goto out_unlock_range;
This success path is very clear. Good!
2671 }
2672
2673 if (!svm_fault_allowed(vma, write_fault)) {
2674 pr_debug("fault addr 0x%llx no %s permission\n", addr,
2675 write_fault ? "write" : "read");
2676 r = -EPERM;
2677 goto out_unlock_range;
2678 }
2679
2680 best_loc = svm_range_best_restore_location(prange, adev,
&gpuidx);
2681 if (best_loc == -1) {
2682 pr_debug("svms %p failed get best restore loc [0x%lx
0x%lx]\n",
2683 svms, prange->start, prange->last);
2684 r = -EACCES;
2685 goto out_unlock_range;
2686 }
2687
2688 pr_debug("svms %p [0x%lx 0x%lx] best restore 0x%x, actual loc
0x%x\n",
2689 svms, prange->start, prange->last, best_loc,
2690 prange->actual_loc);
2691
2692 if (prange->actual_loc != best_loc) {
2693 if (best_loc) {
2694 r = svm_migrate_to_vram(prange, best_loc, mm);
2695 if (r) {
2696 pr_debug("svm_migrate_to_vram failed
(%d) at %llx, falling back to system memory\n",
2697 r, addr);
2698 /* Fallback to system memory if
migration to
2699 * VRAM failed
2700 */
2701 if (prange->actual_loc)
2702 r =
svm_migrate_vram_to_ram(prange, mm);
2703 else
2704 r = 0;
2705 }
2706 } else {
2707 r = svm_migrate_vram_to_ram(prange, mm);
2708 }
2709 if (r) {
2710 pr_debug("failed %d to migrate svms %p [0x%lx
0x%lx]\n",
2711 r, svms, prange->start, prange->last);
2712 goto out_unlock_range;
2713 }
2714 }
2715
2716 r = svm_range_validate_and_map(mm, prange, gpuidx, false,
false);
2717 if (r)
2718 pr_debug("failed %d to map svms 0x%p [0x%lx 0x%lx] to
gpus\n",
2719 r, svms, prange->start, prange->last);
2720
2721 out_unlock_range:
2722 mutex_unlock(&prange->migrate_mutex);
2723 out_unlock_svms:
2724 mutex_unlock(&svms->lock);
2725 mmap_read_unlock(mm);
2726
2727 svm_range_count_fault(adev, p, gpuidx);
2728
2729 mmput(mm);
2730 out:
2731 kfd_unref_process(p);
2732
2733 if (r == -EAGAIN) {
2734 pr_debug("recover vm fault later\n");
2735 amdgpu_gmc_filter_faults_remove(adev, addr, pasid);
2736 r = 0;
2737 }
2738 return r;
2739 }
regards,
dan carpenter
[bug report] drm/amd/display: Reuse parsing code of debugfs write buffer
dex++;
132 }
133
134 return 0;
135 }
This whole function could probably be written like (not compiled):
wr_buf = memdup_user(buf, wr_buf_size + 1);
if (IS_ERR(wr_buf))
return PTR_ERR(wr_buf);
wr_buf[wr_buf_size] = '\0';
p = wr_buf;
while (p && *p) {
if (*param_nums >= max_param_num)
goto done;
while (*p && isspace(*p))
p++;
sub_str = strsep(&p, delimiter);
ret = kstrtol(sub_str, 16, &(param[param_index]));
if (ret)
goto done;
(*param_nums)++;
}
done:
kfree(wr_buf);
return 0;
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amdgpu: page retire over debugfs mechanism
Hello John Clements,
The patch cbb8f989d5a0: "drm/amdgpu: page retire over debugfs
mechanism" from Apr 9, 2021, leads to the following static checker
warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:377
amdgpu_ras_debugfs_ctrl_write()
info: return a literal instead of 'ret'
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
366 ret = amdgpu_ras_debugfs_ctrl_parse_data(f, buf, size, pos,
&data);
367 if (ret)
368 return -EINVAL;
369
370 if (data.op == 3)
371 {
^
Please use scripts/checkpatch.pl... :( Bonus points for replacing the
magic number 3 with a define?
372 ret = amdgpu_reserve_page_direct(adev,
data.inject.address);
373
374 if (ret)
375 return size;
376 else
377 return ret;
This static checker warning is disguised as a style warning, but it's
really to detect code like this where the if statements are reversed as
appears to be the case here.
ret = amdgpu_reserve_page_direct(adev, data.inject.address);
if (ret)
return ret;
return size;
378 }
379
380 if (!amdgpu_ras_is_supported(adev, data.head.block))
381 return -EINVAL;
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amdgpu: fix an error code in init_pmu_entry_by_type_and_add()
If the kmemdup() fails then this should return a negative error code
but it currently returns success
Fixes: b4a7db71ea06 ("drm/amdgpu: add per device user friendly xgmi events for
vega20")
Signed-off-by: Dan Carpenter
---
v2: I sent this patch in Feb but I accidentally added an unrelated
hunk from nouveau to the commit. Now both hunks are have been sent to
the correct lists.
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 19c0a3655228..82e9ecf84352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -519,8 +519,10 @@ static int init_pmu_entry_by_type_and_add(struct
amdgpu_pmu_entry *pmu_entry,
pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
GFP_KERNEL);
- if (!pmu_entry->pmu.attr_groups)
+ if (!pmu_entry->pmu.attr_groups) {
+ ret = -ENOMEM;
goto err_attr_group;
+ }
snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_entry->pmu_file_prefix,
adev_to_drm(pmu_entry->adev)->primary->index);
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/pm: fix error code in smu_set_power_limit()
We should return -EINVAL instead of success if the "limit" is too high.
Fixes: e098bc9612c2 ("drm/amd/pm: optimize the power related source code
layout")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index e0eb7ca112e2..c29d8b3131b7 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2221,6 +2221,7 @@ static int smu_set_power_limit(void *handle, uint32_t
limit)
dev_err(smu->adev->dev,
"New power limit (%d) is over the max allowed %d\n",
limit, smu->max_power_limit);
+ ret = -EINVAL;
goto out;
}
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] new kvmalloc() WARN() triggered by DRM ioctls tracking
Hi DRM Devs,
In commit 7661809d493b ("mm: don't allow oversized kvmalloc() calls")
from July, Linus added a WARN_ONCE() for "crazy" allocations over 2GB.
I have a static checker warning for this and most of the warnings are
from DRM ioctls.
drivers/gpu/drm/lima/lima_drv.c:124 lima_ioctl_gem_submit() warn: uncapped user
size for kvmalloc() will WARN
drivers/gpu/drm/radeon/radeon_cs.c:291 radeon_cs_parser_init() warn: uncapped
user size for kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:311 v3d_lookup_bos() warn: uncapped user size for
kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:319 v3d_lookup_bos() warn: uncapped user size for
kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:601 v3d_get_multisync_post_deps() warn: uncapped
user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:476 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:477 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:478 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:479 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/virtio/virtgpu_ioctl.c:186 virtio_gpu_execbuffer_ioctl() warn:
uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/panfrost/panfrost_drv.c:198 panfrost_copy_in_sync() warn:
uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:120 amdgpu_cs_parser_init() warn:
uncapped user size for kvmalloc() will WARN
These ioctls can all trigger the stack dump. The line numbers are from
linux next (next-20211214).
I feel like ideally if this could be fixed in a central way, but if not
then hopefully I've added the relevant lists to the CC.
regards,
dan carpenter
[bug report] drm/amd/display: define link res and make it accessible to all link interfaces
Hello Wenjing Liu,
The patch ef30f441f6ac: "drm/amd/display: define link res and make it
accessible to all link interfaces" from Nov 25, 2021, leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:4586
dc_link_set_drive_settings()
warn: address of 'dc->current_state->res_ctx.pipe_ctx[i]' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:4587
dc_link_set_drive_settings()
error: we previously assumed 'pipe->stream' could be null (see line 4581)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c
4568 void dc_link_set_drive_settings(struct dc *dc,
4569 struct link_training_settings
*lt_settings,
4570 const struct dc_link *link)
4571 {
4572
4573 int i;
4574 struct pipe_ctx *pipe = NULL;
4575 const struct link_resource *link_res;
4576
4577 link_res = dc_link_get_cur_link_res(link);
4578
4579 for (i = 0; i < MAX_PIPES; i++) {
4580 pipe = &dc->current_state->res_ctx.pipe_ctx[i];
4581 if (pipe->stream && pipe->stream->link) {
This checks for "pipe->stream"
4582 if (pipe->stream->link == link)
4583 break;
4584 }
4585 }
4586 if (pipe && link_res)
"pipe" can't possibly be NULL
--> 4587 dc_link_dp_set_drive_settings(pipe->stream->link,
link_res, lt_settings);
This dereferences "pipe->stream" without checking
4588 else
4589 ASSERT_CRITICAL(false);
4590 }
regards,
dan carpenter
[bug report] drm/amdgpu: Modify gfx block to fit for the unified ras block data and ops
Hello yipechai,
The patch d51ce4db0747: "drm/amdgpu: Modify gfx block to fit for the
unified ras block data and ops" from Jan 4, 2022, leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1058 amdgpu_ras_error_inject()
warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
1047 int amdgpu_ras_error_inject(struct amdgpu_device *adev,
1048 struct ras_inject_if *info)
1049 {
1050 struct ras_manager *obj = amdgpu_ras_find_obj(adev,
&info->head);
1051 struct ta_ras_trigger_error_input block_info = {
1052 .block_id = amdgpu_ras_block_to_ta(info->head.block),
1053 .inject_error_type =
amdgpu_ras_error_to_ta(info->head.type),
1054 .sub_block_index = info->head.sub_block_index,
1055 .address = info->address,
1056 .value = info->value,
1057 };
--> 1058 int ret = -EINVAL;
1059 struct amdgpu_ras_block_object* block_obj =
amdgpu_ras_get_ras_block(adev, info->head.block, info->head.sub_block_index);
Really? You can't be bothered to run checkpatch on your code? AMD drm
code is uniquely bad in this regards. It's the only place outside of
drivers/staging/ where you see stuff like this.
In theory, it's admirable to be this informal and free from bureaucracy
and rules. But in another way, this kind of code is like plumber crack.
You might be a good plumber but it's not attractive. And we might not
point it out, but we all see it.
1060
1061 if (!obj)
1062 return -EINVAL;
1063
1064 if (!block_obj || !block_obj->hw_ops){
1065 dev_info(adev->dev, "%s doesn't config ras function
\n", get_ras_block_str(&info->head));
1066 return -EINVAL;
1067 }
regards,
dan carpenter
[PATCH] drm/amdgpu: Indent some if statements
These if statements need to be indented.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index d4d9b9ea8bbd..777def770dc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1714,8 +1714,7 @@ static void amdgpu_ras_error_status_query(struct
amdgpu_device *adev,
}
if (block_obj->hw_ops->query_ras_error_status)
- block_obj->hw_ops->query_ras_error_status(adev);
-
+ block_obj->hw_ops->query_ras_error_status(adev);
}
static void amdgpu_ras_query_err_status(struct amdgpu_device *adev)
@@ -2722,7 +2721,7 @@ struct amdgpu_ras* amdgpu_ras_get_context(struct
amdgpu_device *adev)
int amdgpu_ras_set_context(struct amdgpu_device *adev, struct amdgpu_ras*
ras_con)
{
if (!adev)
- return -EINVAL;;
+ return -EINVAL;
adev->psp.ras_context.ras = ras_con;
return 0;
--
2.20.1
[bug report] drm/amd/display: access hpo dp link encoder only through link resource
Hello Wenjing Liu,
The patch 3d38a5839ea8: "drm/amd/display: access hpo dp link encoder
only through link resource" from Nov 26, 2021, leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:2961
dc_link_dp_sync_lt_end()
error: NULL dereference inside function dp_disable_link_phy()
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c
2954 bool dc_link_dp_sync_lt_end(struct dc_link *link, bool link_down)
2955 {
2956 /* If input parameter is set, shut down phy.
2957 * Still shouldn't turn off dp_receiver (DPCD:600h)
2958 */
2959 if (link_down == true) {
2960 struct dc_link_settings link_settings =
link->cur_link_settings;
--> 2961 dp_disable_link_phy(link, NULL,
link->connector_signal);
This NULL will lead to an Oops.
2962 if (dp_get_link_encoding_format(&link_settings) ==
DP_8b_10b_ENCODING)
2963 dp_set_fec_ready(link, NULL, false);
2964 }
2965
2966 link->sync_lt_in_progress = false;
2967 return true;
2968 }
regards,
dan carpenter
[bug report] drm/amd/display: refactor destructive verify link cap sequence
Hello Wenjing Liu,
The patch 1a206273c322: "drm/amd/display: refactor destructive verify
link cap sequence" from Jan 28, 2022, leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3248
dp_verify_link_cap()
error: uninitialized symbol 'status'.
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c
3192 static bool dp_verify_link_cap(
3193 struct dc_link *link,
3194 struct dc_link_settings *known_limit_link_setting,
3195 int *fail_count)
3196 {
3197 struct dc_link_settings cur_link_settings = {0};
3198 struct dc_link_settings initial_link_settings =
*known_limit_link_setting;
3199 bool success = false;
3200 bool skip_video_pattern;
3201 enum clock_source_id dp_cs_id = get_clock_source_id(link);
3202 enum link_training_result status;
3203 union hpd_irq_data irq_data;
3204 struct link_resource link_res;
3205
3206 memset(&irq_data, 0, sizeof(irq_data));
3207 cur_link_settings = initial_link_settings;
3208
3209 /* Grant extended timeout request */
3210 if ((link->lttpr_mode == LTTPR_MODE_NON_TRANSPARENT) &&
(link->dpcd_caps.lttpr_caps.max_ext_timeout > 0)) {
3211 uint8_t grant =
link->dpcd_caps.lttpr_caps.max_ext_timeout & 0x80;
3212
3213 core_link_write_dpcd(link,
DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT, &grant, sizeof(grant));
3214 }
3215
3216 do {
3217 if (!get_temp_dp_link_res(link, &link_res,
&cur_link_settings))
3218 continue;
"status" is not set on this continure path.
3219
3220 skip_video_pattern = cur_link_settings.link_rate !=
LINK_RATE_LOW;
3221 dp_enable_link_phy(
3222 link,
3223 &link_res,
3224 link->connector_signal,
3225 dp_cs_id,
3226 &cur_link_settings);
3227
3228 status = dc_link_dp_perform_link_training(
3229 link,
3230 &link_res,
3231 &cur_link_settings,
3232 skip_video_pattern);
3233
3234 if (status == LINK_TRAINING_SUCCESS) {
3235 success = true;
3236 udelay(1000);
3237 if (read_hpd_rx_irq_data(link, &irq_data) ==
DC_OK &&
3238
hpd_rx_irq_check_link_loss_status(
3239 link,
3240 &irq_data))
3241 (*fail_count)++;
3242
3243 } else {
3244 (*fail_count)++;
3245 }
3246 dp_disable_link_phy(link, &link_res,
link->connector_signal);
3247 } while (!success && decide_fallback_link_setting(link,
--> 3248 initial_link_settings, &cur_link_settings,
status));
^^
Uninitialized variable
3249
3250 link->verified_link_cap = success ?
3251 cur_link_settings : fail_safe_link_settings;
3252 return success;
3253 }
regards,
dan carpenter
[PATCH] drm/amdkfd: CRIU fix a NULL vs IS_ERR() check
The kfd_process_device_data_by_id() does not return error pointers,
it returns NULL.
Fixes: bef153b70c6e ("drm/amdkfd: CRIU implement gpu_id remapping")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 64e3b4e3a712..75aca99f6ece 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2526,7 +2526,7 @@ static int criu_restore_bos(struct kfd_process *p,
break;
peer_pdd = kfd_process_device_data_by_id(p,
bo_priv->mapped_gpuids[j]);
- if (IS_ERR(peer_pdd)) {
+ if (!peer_pdd) {
ret = -EINVAL;
goto exit;
}
--
2.20.1
[PATCH] drm/amdkfd: CRIU return -EFAULT for copy_to_user() failure
If copy_to_user() fails, it returns the number of bytes remaining to
be copied but we want to return a negative error code (-EFAULT) to the
user.
Fixes: 9d5dabfeff3c ("drm/amdkfd: CRIU Save Shared Virtual Memory ranges")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 41f03d165bad..38a056fd0966 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -3775,10 +3775,10 @@ int kfd_criu_checkpoint_svm(struct kfd_process *p,
goto exit_priv;
}
- ret = copy_to_user(user_priv_data + *priv_data_offset,
- svm_priv, svm_priv_data_size);
- if (ret) {
+ if (copy_to_user(user_priv_data + *priv_data_offset, svm_priv,
+svm_priv_data_size)) {
pr_err("Failed to copy svm priv to user\n");
+ ret = -EFAULT;
goto exit_priv;
}
--
2.20.1
[PATCH 1/2] drm/amdgpu: add missing unreserve on error
The amdgpu_bo_unreserve() has to be done on the error path as well.
Fixes: b4f0f97b8f5f ("drm/amdgpu: Move kfd_mem_attach outside reservation")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 928e8d57cd08..68109908a869 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -624,11 +624,10 @@ kfd_mem_attach_userptr(struct amdgpu_device *adev, struct
kgd_mem *mem,
0, ttm_bo_type_sg,
mem->bo->tbo.base.resv,
&gobj);
+ amdgpu_bo_unreserve(mem->bo);
if (ret)
return ret;
- amdgpu_bo_unreserve(mem->bo);
-
*bo = gem_to_amdgpu_bo(gobj);
(*bo)->parent = amdgpu_bo_ref(mem->bo);
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: Fix an error code in kfd_mem_attach_dmabuf()
If amdgpu_gem_prime_export() fails, then this code accidentally
returns zero/success instead of a negative error code.
Fixes: 190f2d7696c8 ("drm/amdgpu: Add DMA mapping of GTT BOs")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 68109908a869..9b7a3f849a16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -639,14 +639,16 @@ kfd_mem_attach_dmabuf(struct amdgpu_device *adev, struct
kgd_mem *mem,
struct amdgpu_bo **bo)
{
struct drm_gem_object *gobj;
+ int ret;
if (!mem->dmabuf) {
mem->dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base,
mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ?
DRM_RDWR : 0);
if (IS_ERR(mem->dmabuf)) {
+ ret = PTR_ERR(mem->dmabuf);
mem->dmabuf = NULL;
- return PTR_ERR(mem->dmabuf);
+ return ret;
}
}
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix amdgpu_preempt_mgr_new()
There is a reversed if statement in amdgpu_preempt_mgr_new() so it
always returns -ENOMEM.
Fixes: 09b020bb05a5 ("Merge tag 'drm-misc-next-2021-06-09' of
git://anongit.freedesktop.org/drm/drm-misc into drm-next")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
index f6aff7ce5160..d02c8637f909 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
@@ -71,7 +71,7 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager
*man,
struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
*res = kzalloc(sizeof(**res), GFP_KERNEL);
- if (*res)
+ if (!*res)
return -ENOMEM;
ttm_resource_init(tbo, place, *res);
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Add DCN3.1 HDCP support
Hello Nicholas Kazlauskas, This is a semi-automatic email about new static checker warnings. The patch bf62221e9d0e: "drm/amd/display: Add DCN3.1 HDCP support" from May 19, 2021, leads to the following Smatch complaint: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:474 update_config() error: we previously assumed 'aconnector->dc_sink' could be null (see line 463) drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c 462 463 if (aconnector->dc_sink != NULL) ^^^ Check for NULL 464 link->mode = mod_hdcp_signal_type_to_operation_mode(aconnector->dc_sink->sink_signal); 465 466 display->controller = CONTROLLER_ID_D0 + config->otg_inst; 467 display->dig_fe = config->dig_fe; 468 link->dig_be = config->dig_be; 469 link->ddc_line = aconnector->dc_link->ddc_hw_inst + 1; 470 display->stream_enc_idx = config->stream_enc_idx; 471 link->link_enc_idx = config->link_enc_idx; 472 link->phy_idx = config->phy_idx; 473 link->hdcp_supported_informational = dc_link_is_hdcp14(aconnector->dc_link, 474 aconnector->dc_sink->sink_signal) ? 1 : 0; ^^^ Unchecked dereference. Originally there were some #ifdefs here and that maybe ensured that ->dc_sink was non-NULL? I'm not sure. 475 link->dp.rev = aconnector->dc_link->dpcd_caps.dpcd_rev.raw; 476 link->dp.assr_enabled = config->assr_enabled; regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/4] drm/amdgpu: fix a signedness bug in __verify_ras_table_checksum()
If amdgpu_eeprom_read() returns a negative error code then the error
handling checks:
if (res < buf_size) {
The problem is that "buf_size" is a u32 so negative values are type
promoted to a high positive values and the condition is false. Fix
this by changing the type of "buf_size" to int.
Fixes: 79beb6114014 ("drm/amdgpu: Optimize EEPROM RAS table I/O")
Signed-off-by: Dan Carpenter
---
It's hard for me to tell the exact upper bound that "buf_size" can be,
but if it's over USHRT_MAX then we are well toasted.
drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index fc70620369e4..f07a456506ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -978,9 +978,8 @@ const struct file_operations
amdgpu_ras_debugfs_eeprom_table_ops = {
static int __verify_ras_table_checksum(struct amdgpu_ras_eeprom_control
*control)
{
struct amdgpu_device *adev = to_amdgpu_device(control);
- int res;
+ int buf_size, res;
u8 csum, *buf, *pp;
- u32 buf_size;
buf_size = RAS_TABLE_HEADER_SIZE +
control->ras_num_recs * RAS_TABLE_RECORD_SIZE;
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/4] drm/amdgpu: Fix signedness bug in __amdgpu_eeprom_xfer()
The i2c_transfer() function returns negatives or else the number of
messages transferred. This code does not work because ARRAY_SIZE()
is type size_t and so that means negative values of "r" are type
promoted to high positive values which are greater than the ARRAY_SIZE().
Fix this by changing the < to != which works regardless of type
promotion.
Fixes: 6cda0af81a50 ("drm/amdgpu: Fixes to the AMDGPU EEPROM driver")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
index 4c3c65a5acae..4d9eb0137f8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
@@ -147,7 +147,7 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter
*i2c_adap, u32 eeprom_addr,
/* This constitutes a START-STOP transaction.
*/
r = i2c_transfer(i2c_adap, msgs, ARRAY_SIZE(msgs));
- if (r < ARRAY_SIZE(msgs))
+ if (r != ARRAY_SIZE(msgs))
break;
if (!read) {
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/4] drm/amdgpu: unlock on error in amdgpu_ras_debugfs_table_read()
This error path needs to unlock before returning. While we're at it,
the correct error code from copy_to_user() failure is -EFAULT, not
-EINVAL.
Fixes: 9b790694a031 (""drm/amdgpu: RAS EEPROM table is now in debugfs)
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index f07a456506ef..3e33e85d8dbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -821,7 +821,7 @@ static ssize_t amdgpu_ras_debugfs_table_read(struct file
*f, char __user *buf,
struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
struct amdgpu_ras_eeprom_control *control = &ras->eeprom_control;
const size_t orig_size = size;
- int res = -EINVAL;
+ int res = -EFAULT;
size_t data_len;
mutex_lock(&control->ras_tbl_mutex);
@@ -912,8 +912,10 @@ static ssize_t amdgpu_ras_debugfs_table_read(struct file
*f, char __user *buf,
record.retired_page);
data_len = min_t(size_t, rec_hdr_fmt_size - r, size);
- if (copy_to_user(buf, &data[r], data_len))
- return -EINVAL;
+ if (copy_to_user(buf, &data[r], data_len)) {
+ res = -EFAULT;
+ goto Out;
+ }
buf += data_len;
size -= data_len;
*pos += data_len;
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/4] drm/amdgpu: return -EFAULT if copy_to_user() fails
If copy_to_user() fails then this should return -EFAULT instead of
-EINVAL.
Fixes: 9b790694a031 ("drm/amdgpu: RAS EEPROM table is now in debugfs")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index 3e33e85d8dbc..d2e5b2567bc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -771,7 +771,7 @@ amdgpu_ras_debugfs_eeprom_size_read(struct file *f, char
__user *buf,
res = min_t(size_t, res, size);
if (copy_to_user(buf, &data[*pos], res))
- return -EINVAL;
+ return -EFAULT;
*pos += res;
@@ -950,7 +950,7 @@ amdgpu_ras_debugfs_eeprom_table_read(struct file *f, char
__user *buf,
res = min_t(size_t, res, size);
if (copy_to_user(buf, &data[*pos], res))
- return -EINVAL;
+ return -EFAULT;
*pos += res;
--
2.30.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amdgpu/acpi: unify ATCS handling (v3)
Hello Alex Deucher,
The patch 77bf762f8b30: "drm/amdgpu/acpi: unify ATCS handling (v3)"
from May 19, 2021, leads to the following static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:857 amdgpu_acpi_init()
warn: this array is probably non-NULL. 'dm->backlight_dev'
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
848 int amdgpu_acpi_init(struct amdgpu_device *adev)
849 {
850 struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
851
852 #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE)
853 if (atif->notifications.brightness_change) {
854 if (amdgpu_device_has_dc_support(adev)) {
855 #if defined(CONFIG_DRM_AMD_DC)
856 struct amdgpu_display_manager *dm = &adev->dm;
857 if (dm->backlight_dev)
^
The commit adds an unnecessary NULL check.
858 atif->bd = dm->backlight_dev[0];
859 #endif
860 } else {
861 struct drm_encoder *tmp;
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: Indent an if statement
The if statement wasn't indented so it's confusing. Signed-off-by: Dan Carpenter --- drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c index ca26714c800e..c6b737dd8425 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c @@ -71,7 +71,7 @@ enum dce_version resource_parse_asic_id(struct hw_asic_id asic_id) if (ASIC_REV_IS_TAHITI_P(asic_id.hw_internal_rev) || ASIC_REV_IS_PITCAIRN_PM(asic_id.hw_internal_rev) || ASIC_REV_IS_CAPEVERDE_M(asic_id.hw_internal_rev)) - dc_version = DCE_VERSION_6_0; + dc_version = DCE_VERSION_6_0; else if (ASIC_REV_IS_OLAND_M(asic_id.hw_internal_rev)) dc_version = DCE_VERSION_6_4; else -- 2.27.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm: amdgpu: Use the correct size when allocating memory
On Sun, Aug 09, 2020 at 10:34:06PM +0200, Christophe JAILLET wrote:
> When '*sgt' is allocated, we must allocated 'sizeof(**sgt)' bytes instead
> of 'sizeof(*sg)'. 'sg' (i.e. struct scatterlist) is smaller than
> 'sgt' (i.e struct sg_table), so this could lead to memory corruption.
The sizeof(*sg) is bigger than sizeof(**sgt) so this wastes memory but
it won't lead to corruption.
11 struct scatterlist {
12 unsigned long page_link;
13 unsigned intoffset;
14 unsigned intlength;
15 dma_addr_t dma_address;
16 #ifdef CONFIG_NEED_SG_DMA_LENGTH
17 unsigned intdma_length;
18 #endif
19 };
42 struct sg_table {
43 struct scatterlist *sgl;/* the list */
44 unsigned int nents; /* number of mapped entries */
45 unsigned int orig_nents; /* original size of list */
46 };
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm: amdgpu: Use the correct size when allocating memory
On Mon, Aug 10, 2020 at 08:41:14PM +0200, Marion & Christophe JAILLET wrote:
>
> Le 10/08/2020 à 17:42, Dan Carpenter a écrit :
> > On Sun, Aug 09, 2020 at 10:34:06PM +0200, Christophe JAILLET wrote:
> > > When '*sgt' is allocated, we must allocated 'sizeof(**sgt)' bytes instead
> > > of 'sizeof(*sg)'. 'sg' (i.e. struct scatterlist) is smaller than
> > > 'sgt' (i.e struct sg_table), so this could lead to memory corruption.
> > The sizeof(*sg) is bigger than sizeof(**sgt) so this wastes memory but
> > it won't lead to corruption.
> >
> > 11 struct scatterlist {
> > 12 unsigned long page_link;
> > 13 unsigned intoffset;
> > 14 unsigned intlength;
> > 15 dma_addr_t dma_address;
> > 16 #ifdef CONFIG_NEED_SG_DMA_LENGTH
> > 17 unsigned intdma_length;
> > 18 #endif
> > 19 };
> >
> > 42 struct sg_table {
> > 43 struct scatterlist *sgl;/* the list */
> > 44 unsigned int nents; /* number of mapped
> > entries */
> > 45 unsigned int orig_nents;/* original size of list */
> > 46 };
> >
> > regards,
> > dan carpenter
>
>
> My bad. I read 'struct scatterlist sgl' (without the *)
> Thanks for the follow-up, Dan.
>
> Doesn't smatch catch such mismatch?
> (I've not run smatch for a while, so it is maybe reported)
That's why I was investigating it, because Smatch didn't catch it.
Smatch would have warned if it led to memory corruption. Smatch also
tries to detect struct mismatches as a separate check but for some
reason it missed it. I'm not totally sure why yet. I suspect that it's
a complicated internal reason where Sparse is the sizeof to a normal
number... It's a known issue and hard to fix.
>
> Well, the proposal is still valid, even if it has less impact as initially
> thought.
Yep.
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm: amdgpu: Use the correct size when allocating memory
On Tue, Aug 11, 2020 at 10:57:02AM +0300, Dan Carpenter wrote:
> On Mon, Aug 10, 2020 at 08:41:14PM +0200, Marion & Christophe JAILLET wrote:
> >
> > Le 10/08/2020 à 17:42, Dan Carpenter a écrit :
> > > On Sun, Aug 09, 2020 at 10:34:06PM +0200, Christophe JAILLET wrote:
> > > > When '*sgt' is allocated, we must allocated 'sizeof(**sgt)' bytes
> > > > instead
> > > > of 'sizeof(*sg)'. 'sg' (i.e. struct scatterlist) is smaller than
> > > > 'sgt' (i.e struct sg_table), so this could lead to memory corruption.
> > > The sizeof(*sg) is bigger than sizeof(**sgt) so this wastes memory but
> > > it won't lead to corruption.
> > >
> > > 11 struct scatterlist {
> > > 12 unsigned long page_link;
> > > 13 unsigned intoffset;
> > > 14 unsigned intlength;
> > > 15 dma_addr_t dma_address;
> > > 16 #ifdef CONFIG_NEED_SG_DMA_LENGTH
> > > 17 unsigned intdma_length;
> > > 18 #endif
> > > 19 };
> > >
> > > 42 struct sg_table {
> > > 43 struct scatterlist *sgl;/* the list */
> > > 44 unsigned int nents; /* number of mapped
> > > entries */
> > > 45 unsigned int orig_nents;/* original size of list
> > > */
> > > 46 };
> > >
> > > regards,
> > > dan carpenter
> >
> >
> > My bad. I read 'struct scatterlist sgl' (without the *)
> > Thanks for the follow-up, Dan.
> >
> > Doesn't smatch catch such mismatch?
> > (I've not run smatch for a while, so it is maybe reported)
>
> That's why I was investigating it, because Smatch didn't catch it.
>
> Smatch would have warned if it led to memory corruption. Smatch also
> tries to detect struct mismatches as a separate check but for some
> reason it missed it. I'm not totally sure why yet. I suspect that it's
> a complicated internal reason where Sparse is the sizeof to a normal
s/is/changes/
> number... It's a known issue and hard to fix.
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/powerplay: add point check to avoid NULL point hang.
Hello Rex Zhu,
The patch 88b8dcbe21fd: "drm/amd/powerplay: add point check to avoid
NULL point hang." from Dec 11, 2015, leads to the following static
checker warning:
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c:274
phm_check_smc_update_required_for_display_configuration()
warn: signedness bug returning '(-22)'
drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/hardwaremanager.c
272 bool phm_check_smc_update_required_for_display_configuration(struct
pp_hwmgr *hwmgr)
273 {
274 PHM_FUNC_CHECK(hwmgr);
PHM_FUNC_CHECK() has a hiddren -EINVAL return that becomes true when
casted to bool.
275 if (hwmgr->pp_one_vf)
276 return false;
277
278 if
(hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration == NULL)
279 return false;
280
281 return
hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration(hwmgr);
282 }
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] drm/amdgpu/cik: fix buffer overflow in cik_get_register_value()
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.
I split this function into to two to make the error handling simpler.
Fixes: aca31681b1a5 ("drm/amdgpu: used cached gca values for cik_read_register")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/cik.c | 195 ---
1 file changed, 103 insertions(+), 92 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index 7e71ffbca93d..91ebfa485333 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1042,100 +1042,108 @@ static const struct amdgpu_allowed_register_entry
cik_allowed_read_registers[] =
{mmPA_SC_RASTER_CONFIG_1, true},
};
+static int cik_get_register_value_indexed(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num,
+ u32 reg_offset, u32 *value)
+{
+ unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+ unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-static uint32_t cik_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- if (indexed) {
- uint32_t val;
- unsigned se_idx = (se_num == 0x) ? 0 : se_num;
- unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
- switch (reg_offset) {
- case mmCC_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
- case mmGC_USER_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
- case mmPA_SC_RASTER_CONFIG:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
- case mmPA_SC_RASTER_CONFIG_1:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
- }
+ if (se_idx >= AMDGPU_GFX_MAX_SE ||
+ sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+ return -EINVAL;
+
+ switch (reg_offset) {
+ case mmCC_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+ return 0;
+ case mmGC_USER_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG_1:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
+ return 0;
+ }
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num,
0x);
+ mutex_lock(&adev->grbm_idx_mutex);
+ if (se_num != 0x || sh_num != 0x)
+ amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0x);
- val = RREG32(reg_offset);
+ *value = RREG32(reg_offset);
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
- } else {
- unsigned idx;
-
- switch (reg_offset) {
- case mmGB_ADDR_CONFIG:
- return adev->gfx.config.gb_addr_config;
- case mmMC_ARB_RAMCFG:
- return adev->gfx.config.mc_arb_ramcfg;
- case mmGB_TILE_MODE0:
- case mmGB_TILE_MODE1:
- case mmGB_TILE_MODE2:
- case mmGB_TILE_MODE3:
- case mmGB_TILE_MODE4:
- case mmGB_TILE_MODE5:
- case mmGB_TILE_MODE6:
- case mmGB_TILE_MODE7:
- case mmGB_TILE_MODE8:
- case mmGB_TILE_MODE9:
- case mmGB_TILE_MODE10:
- case mmGB_TILE_MODE11:
- case mmGB_TILE_MODE12:
- case mmGB_TILE_MODE13:
- case mmGB_TILE_MODE14:
- case mmGB_TILE_MODE15:
- case mmGB_TILE_MODE16:
- case mmGB_TILE_MODE17:
- case mmGB_TILE_MODE18:
- case mmGB_TILE_MODE19:
- case mmGB_TILE_MODE20:
- case mmGB_TILE_MODE21:
- case mm
[PATCH 1/3] drm/amdgpu/si: Fix buffer overflow in si_get_register_value()
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.
I split this function into to two to make the error handling simpler.
Fixes: dd5dfa61b4ff ("drm/amdgpu: refine si_read_register")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/si.c | 157 +---
1 file changed, 85 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
index e330884edd19..ccf39a6932ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/si.c
+++ b/drivers/gpu/drm/amd/amdgpu/si.c
@@ -1051,81 +1051,90 @@ static struct amdgpu_allowed_register_entry
si_allowed_read_registers[] = {
{PA_SC_RASTER_CONFIG, true},
};
-static uint32_t si_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- if (indexed) {
- uint32_t val;
- unsigned se_idx = (se_num == 0x) ? 0 : se_num;
- unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
- switch (reg_offset) {
- case mmCC_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
- case mmGC_USER_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
- case mmPA_SC_RASTER_CONFIG:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
- }
+static int si_get_register_value_indexed(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num,
+ u32 reg_offset, u32 *value)
+{
+ unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+ unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num,
0x);
+ if (se_idx >= AMDGPU_GFX_MAX_SE ||
+ sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+ return -EINVAL;
- val = RREG32(reg_offset);
+ switch (reg_offset) {
+ case mmCC_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+ return 0;
+ case mmGC_USER_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+ return 0;
+ }
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
- } else {
- unsigned idx;
-
- switch (reg_offset) {
- case mmGB_ADDR_CONFIG:
- return adev->gfx.config.gb_addr_config;
- case mmMC_ARB_RAMCFG:
- return adev->gfx.config.mc_arb_ramcfg;
- case mmGB_TILE_MODE0:
- case mmGB_TILE_MODE1:
- case mmGB_TILE_MODE2:
- case mmGB_TILE_MODE3:
- case mmGB_TILE_MODE4:
- case mmGB_TILE_MODE5:
- case mmGB_TILE_MODE6:
- case mmGB_TILE_MODE7:
- case mmGB_TILE_MODE8:
- case mmGB_TILE_MODE9:
- case mmGB_TILE_MODE10:
- case mmGB_TILE_MODE11:
- case mmGB_TILE_MODE12:
- case mmGB_TILE_MODE13:
- case mmGB_TILE_MODE14:
- case mmGB_TILE_MODE15:
- case mmGB_TILE_MODE16:
- case mmGB_TILE_MODE17:
- case mmGB_TILE_MODE18:
- case mmGB_TILE_MODE19:
- case mmGB_TILE_MODE20:
- case mmGB_TILE_MODE21:
- case mmGB_TILE_MODE22:
- case mmGB_TILE_MODE23:
- case mmGB_TILE_MODE24:
- case mmGB_TILE_MODE25:
- case mmGB_TILE_MODE26:
- case mmGB_TILE_MODE27:
- case mmGB_TILE_MODE28:
- case mmGB_TILE_MODE29:
- case mmGB_TILE_MODE30:
- case mmGB_TILE_MODE31:
- idx = (reg_offset - mmGB_TILE_MODE0);
- return adev->gfx.config.tile_mode_array[idx];
- default:
- return RREG32(reg_offset
[PATCH 3/3] drm/amdgpu/vi: fix buffer overflow in vi_get_register_value()
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.
I split this function into to two to make the error handling simpler.
Fixes: db9635cc14f3 ("drm/amdgpu: used cached gca values for vi_read_register
(v2)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/vi.c | 197 +---
1 file changed, 105 insertions(+), 92 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index f6f2ed0830b1..fd623ad9d51f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -527,99 +527,108 @@ static const struct amdgpu_allowed_register_entry
vi_allowed_read_registers[] =
{mmPA_SC_RASTER_CONFIG_1, true},
};
-static uint32_t vi_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- if (indexed) {
- uint32_t val;
- unsigned se_idx = (se_num == 0x) ? 0 : se_num;
- unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
-
- switch (reg_offset) {
- case mmCC_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
- case mmGC_USER_RB_BACKEND_DISABLE:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
- case mmPA_SC_RASTER_CONFIG:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
- case mmPA_SC_RASTER_CONFIG_1:
- return
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
- }
+static int vi_get_register_value_indexed(struct amdgpu_device *adev,
+u32 se_num, u32 sh_num,
+u32 reg_offset, u32 *value)
+{
+ unsigned se_idx = (se_num == 0x) ? 0 : se_num;
+ unsigned sh_idx = (sh_num == 0x) ? 0 : sh_num;
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num,
0x);
+ if (se_idx >= AMDGPU_GFX_MAX_SE ||
+ sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+ return -EINVAL;
- val = RREG32(reg_offset);
+ switch (reg_offset) {
+ case mmCC_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+ return 0;
+ case mmGC_USER_RB_BACKEND_DISABLE:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG_1:
+ *value =
adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
+ return 0;
+ }
- if (se_num != 0x || sh_num != 0x)
- amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
- } else {
- unsigned idx;
-
- switch (reg_offset) {
- case mmGB_ADDR_CONFIG:
- return adev->gfx.config.gb_addr_config;
- case mmMC_ARB_RAMCFG:
- return adev->gfx.config.mc_arb_ramcfg;
- case mmGB_TILE_MODE0:
- case mmGB_TILE_MODE1:
- case mmGB_TILE_MODE2:
- case mmGB_TILE_MODE3:
- case mmGB_TILE_MODE4:
- case mmGB_TILE_MODE5:
- case mmGB_TILE_MODE6:
- case mmGB_TILE_MODE7:
- case mmGB_TILE_MODE8:
- case mmGB_TILE_MODE9:
- case mmGB_TILE_MODE10:
- case mmGB_TILE_MODE11:
- case mmGB_TILE_MODE12:
- case mmGB_TILE_MODE13:
- case mmGB_TILE_MODE14:
- case mmGB_TILE_MODE15:
- case mmGB_TILE_MODE16:
- case mmGB_TILE_MODE17:
- case mmGB_TILE_MODE18:
- case mmGB_TILE_MODE19:
- case mmGB_TILE_MODE20:
- case mmGB_TILE_MODE21:
- case mmGB_TILE_MODE22:
- case mmGB_TILE_MODE23:
- case mmGB_TILE_MODE24:
- case mmGB_TILE_MODE25:
- case mmGB_TILE_MODE26:
- case mmGB_TILE_MODE27:
- case mmGB_TILE
Re: [PATCH 1/3] drm/amdgpu/si: Fix buffer overflow in si_get_register_value()
On Tue, Aug 25, 2020 at 11:53:25AM -0400, Alex Deucher wrote:
> On Tue, Aug 25, 2020 at 7:21 AM Dan Carpenter
> wrote:
> >
> > The values for "se_num" and "sh_num" come from the user in the ioctl.
> > They can be in the 0-255 range but if they're more than
> > AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
> > an out of bounds read.
> >
> > I split this function into to two to make the error handling simpler.
> >
> > Fixes: dd5dfa61b4ff ("drm/amdgpu: refine si_read_register")
> > Signed-off-by: Dan Carpenter
>
> Good catch. This is more defensive, but It's a much simpler check to
> validate these in the caller. See the attached patch.
>
That works too.
Acked-by: Dan Carpenter
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Add connector to the state if DSC debugfs is set
Hello Eryk Brol,
The patch c44a22b3128d: "drm/amd/display: Add connector to the state
if DSC debugfs is set" from Aug 27, 2020, leads to the following
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:917
amdgpu_check_debugfs_connector_property_change()
error: 'crtc_state' dereferencing possible ERR_PTR()
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
883 static void amdgpu_check_debugfs_connector_property_change(struct
amdgpu_device *adev,
884 struct
drm_atomic_state *state)
885 {
886 struct drm_connector *connector;
887 struct drm_crtc *crtc;
888 struct amdgpu_dm_connector *amdgpu_dm_connector;
889 struct drm_connector_state *conn_state;
890 struct dm_crtc_state *acrtc_state;
891 struct drm_crtc_state *crtc_state;
892 struct dc_stream_state *stream;
893 struct drm_device *dev = adev_to_drm(adev);
894
895 list_for_each_entry(connector,
&dev->mode_config.connector_list, head) {
896
897 amdgpu_dm_connector = to_amdgpu_dm_connector(connector);
898 conn_state = connector->state;
899
900 if (!(conn_state && conn_state->crtc))
901 continue;
902
903 crtc = conn_state->crtc;
904 acrtc_state = to_dm_crtc_state(crtc->state);
905
906 if (!(acrtc_state && acrtc_state->stream))
907 continue;
908
909 stream = acrtc_state->stream;
910
911 if (amdgpu_dm_connector->dsc_settings.dsc_force_enable
||
912 amdgpu_dm_connector->dsc_settings.dsc_num_slices_v
||
913 amdgpu_dm_connector->dsc_settings.dsc_num_slices_h
||
914
amdgpu_dm_connector->dsc_settings.dsc_bits_per_pixel) {
915 conn_state =
drm_atomic_get_connector_state(state, connector);
916 crtc_state = drm_atomic_get_crtc_state(state,
crtc);
^^^
Check for errors?
917 crtc_state->mode_changed = true;
918 }
919 }
920 }
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/pm: add the parameters of power profiles for vangogh
Hello Xiaojian Du,
The patch 307f049bfcad: "drm/amd/pm: add the parameters of power
profiles for vangogh" from Jan 7, 2021, leads to the following static
checker warning:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/vangogh_ppt.c:801
vangogh_get_power_profile_mode()
error: buffer overflow 'profile_name' 5 <= 6
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/vangogh_ppt.c
773 static int vangogh_get_power_profile_mode(struct smu_context *smu,
774 char *buf)
775 {
776 static const char *profile_name[] = {
777 "FULL_SCREEN_3D",
778 "VIDEO",
779 "VR",
780 "COMPUTE",
781 "CUSTOM"};
This list leaves out DEFAULT and POWERSAVING profiles, so I don't think
it's correct.
782 uint32_t i, size = 0;
783 int16_t workload_type = 0;
784
785 if (!buf)
786 return -EINVAL;
787
788 for (i = 0; i <= PP_SMC_POWER_PROFILE_CUSTOM; i++) {
789 /*
790 * Conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT
791 * Not all profile modes are supported on vangogh.
792 */
793 workload_type = smu_cmn_to_asic_specific_index(smu,
794
CMN2ASIC_MAPPING_WORKLOAD,
795 i);
796
797 if (workload_type < 0)
798 continue;
799
800 size += sprintf(buf + size, "%2d %14s%s\n",
801 i, profile_name[i], (i ==
smu->power_profile_mode) ? "*" : " ");
^^^^^^^
Also it leads to an array overflow.
802 }
803
804 return size;
805 }
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH][next] drm/amdgpu: Add missing BOOTUP_DEFAULT to profile_name[]
On Mon, Jan 11, 2021 at 11:46:38AM +, Colin King wrote: > From: Colin Ian King > > A recent change added a new BOOTUP_DEFAULT power profile mode > to the PP_SMC_POWER_PROFILE enum but omitted updating the > corresponding profile_name array. Fix this by adding in the > missing BOOTUP_DEFAULT to profile_name[]. > Still not enough to prevent the array overflow. It needs POWERSAVE as well. regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix bitwise vs logical negate
There was a mixup between logical and bitwise negate so it just sets
"data1" and "data2" to zero.
Fixes: 3c9a7b7d6e75 ("drm/amdgpu: update mmhub mgcg&ls for mmhub_v2_3")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
index 1961745e89c7..ab9be5ad5a5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
@@ -531,12 +531,12 @@ mmhub_v2_3_update_medium_grain_light_sleep(struct
amdgpu_device *adev,
if (enable && (adev->cg_flags & AMD_CG_SUPPORT_MC_LS)) {
data &= ~MM_ATC_L2_CGTT_CLK_CTRL__MGLS_OVERRIDE_MASK;
- data1 &= !(DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_MASK |
+ data1 &= ~(DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_MASK |
DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_WRITE_MASK |
DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_READ_MASK |
DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_RETURN_MASK |
DAGB0_WR_CGTT_CLK_CTRL__LS_OVERRIDE_REGISTER_MASK);
- data2 &= !(DAGB0_RD_CGTT_CLK_CTRL__LS_OVERRIDE_MASK |
+ data2 &= ~(DAGB0_RD_CGTT_CLK_CTRL__LS_OVERRIDE_MASK |
DAGB0_RD_CGTT_CLK_CTRL__LS_OVERRIDE_WRITE_MASK |
DAGB0_RD_CGTT_CLK_CTRL__LS_OVERRIDE_READ_MASK |
DAGB0_RD_CGTT_CLK_CTRL__LS_OVERRIDE_RETURN_MASK |
--
2.29.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: Fix a potential NULL dereference
The debug printk dereferences "link->link_enc" before we have ensured
that it is non-NULL. Fix this potential NULL derefence by moving the
printk after the check.
Fixes: 1975b95ad4e7 ("drm/amd/display: Log link/connector info provided in BIOS
object table")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index c16af3983fdb..4d31b2fae1f9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1525,13 +1525,13 @@ static bool dc_link_construct(struct dc_link *link,
link->link_enc =
link->dc->res_pool->funcs->link_enc_create(&enc_init_data);
- DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d",
link->link_enc->features.flags.bits.DP_IS_USB_C);
-
if (!link->link_enc) {
DC_ERROR("Failed to create link encoder!\n");
goto link_enc_create_fail;
}
+ DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d",
link->link_enc->features.flags.bits.DP_IS_USB_C);
+
link->link_enc_hw_inst = link->link_enc->transmitter;
for (i = 0; i < 4; i++) {
--
2.29.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Prevent shift wrapping in amdgpu_read_mask()
If the user passes a "level" value which is higher than 31 then that
leads to shift wrapping. The undefined behavior will lead to a
syzkaller stack dump.
Fixes: 5632708f4452 ("drm/amd/powerplay: add dpm force multiple levels on
cz/tonga/fiji/polaris (v2)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index e9b569b76716..1a4010fcf561 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1094,7 +1094,7 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev,
static ssize_t amdgpu_read_mask(const char *buf, size_t count, uint32_t *mask)
{
int ret;
- long level;
+ unsigned long level;
char *sub_str = NULL;
char *tmp;
char buf_cpy[AMDGPU_MASK_BUF_MAX + 1];
@@ -1109,9 +1109,9 @@ static ssize_t amdgpu_read_mask(const char *buf, size_t
count, uint32_t *mask)
tmp = buf_cpy;
while (tmp[0]) {
sub_str = strsep(&tmp, delimiter);
- if (strlen(sub_str)) {
- ret = kstrtol(sub_str, 0, &level);
- if (ret)
+ if (sub_str[0]) {
+ ret = kstrtoul(sub_str, 0, &level);
+ if (ret || level > 31)
return -EINVAL;
*mask |= 1 << level;
} else
--
2.29.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix an error code in init_pmu_entry_by_type_and_add()
If the kmemdup() fails then this should return a negative error code
but it currently returns success.
Fixes: b4a7db71ea06 ("drm/amdgpu: add per device user friendly xgmi events for
vega20")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +++-
drivers/gpu/drm/nouveau/nouveau_backlight.c | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 19c0a3655228..82e9ecf84352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -519,8 +519,10 @@ static int init_pmu_entry_by_type_and_add(struct
amdgpu_pmu_entry *pmu_entry,
pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
GFP_KERNEL);
- if (!pmu_entry->pmu.attr_groups)
+ if (!pmu_entry->pmu.attr_groups) {
+ ret = -ENOMEM;
goto err_attr_group;
+ }
snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_entry->pmu_file_prefix,
adev_to_drm(pmu_entry->adev)->primary->index);
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 72f35a2babcb..3786b1c85182 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -274,6 +274,7 @@ nouveau_backlight_init(struct drm_connector *connector)
if (!nouveau_get_backlight_name(backlight_name, bl)) {
NV_ERROR(drm, "Failed to retrieve a unique name for the
backlight interface\n");
+ ret = -ENOMEM;
goto fail_alloc;
}
--
2.30.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix an error code in init_pmu_entry_by_type_and_add()
On Tue, Feb 16, 2021 at 02:52:55PM -0500, Alex Deucher wrote:
> On Tue, Feb 16, 2021 at 2:35 PM Dan Carpenter
> wrote:
> >
> > If the kmemdup() fails then this should return a negative error code
> > but it currently returns success.
> >
> > Fixes: b4a7db71ea06 ("drm/amdgpu: add per device user friendly xgmi events
> > for vega20")
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +++-
> > drivers/gpu/drm/nouveau/nouveau_backlight.c | 1 +
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > index 19c0a3655228..82e9ecf84352 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > @@ -519,8 +519,10 @@ static int init_pmu_entry_by_type_and_add(struct
> > amdgpu_pmu_entry *pmu_entry,
> > pmu_entry->pmu.attr_groups = kmemdup(attr_groups,
> > sizeof(attr_groups),
> > GFP_KERNEL);
> >
> > - if (!pmu_entry->pmu.attr_groups)
> > + if (!pmu_entry->pmu.attr_groups) {
> > + ret = -ENOMEM;
> > goto err_attr_group;
> > + }
> >
> > snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
> > pmu_entry->pmu_file_prefix,
> >
> > adev_to_drm(pmu_entry->adev)->primary->index);
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > index 72f35a2babcb..3786b1c85182 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> > @@ -274,6 +274,7 @@ nouveau_backlight_init(struct drm_connector *connector)
> >
> > if (!nouveau_get_backlight_name(backlight_name, bl)) {
> > NV_ERROR(drm, "Failed to retrieve a unique name for the
> > backlight interface\n");
> > + ret = -ENOMEM;
> > goto fail_alloc;
> > }
>
> This hunk looks unrelated?
>
That's so weird... I don't even have any clue how that would happen at
all.. Weee wooo weee w ALIENS!
Anyway, I'll resend. Thanks!
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: Fix off by one in hdmi_14_process_transaction()
The hdcp_i2c_offsets[] array did not have an entry for
HDCP_MESSAGE_ID_WRITE_CONTENT_STREAM_TYPE so it led to an off by one
read overflow. I added an entry and copied the 0x0 value for the offset
from similar code in drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c.
I also declared several of these arrays as having HDCP_MESSAGE_ID_MAX
entries. This doesn't change the code, but it's just a belt and
suspenders approach to try future proof the code.
Fixes: 4c283fdac08a ("drm/amd/display: Add HDCP module")
Signed-off-by: Dan Carpenter
---
>From static analysis, as mentioned in the commit message the offset
is basically an educated guess.
I reported this bug on Apr 16, 2020 but I guess we lost take of it.
drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
b/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
index 5e384a8a83dc..51855a2624cf 100644
--- a/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
+++ b/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
@@ -39,7 +39,7 @@
#define HDCP14_KSV_SIZE 5
#define HDCP14_MAX_KSV_FIFO_SIZE 127*HDCP14_KSV_SIZE
-static const bool hdcp_cmd_is_read[] = {
+static const bool hdcp_cmd_is_read[HDCP_MESSAGE_ID_MAX] = {
[HDCP_MESSAGE_ID_READ_BKSV] = true,
[HDCP_MESSAGE_ID_READ_RI_R0] = true,
[HDCP_MESSAGE_ID_READ_PJ] = true,
@@ -75,7 +75,7 @@ static const bool hdcp_cmd_is_read[] = {
[HDCP_MESSAGE_ID_WRITE_CONTENT_STREAM_TYPE] = false
};
-static const uint8_t hdcp_i2c_offsets[] = {
+static const uint8_t hdcp_i2c_offsets[HDCP_MESSAGE_ID_MAX] = {
[HDCP_MESSAGE_ID_READ_BKSV] = 0x0,
[HDCP_MESSAGE_ID_READ_RI_R0] = 0x8,
[HDCP_MESSAGE_ID_READ_PJ] = 0xA,
@@ -106,7 +106,8 @@ static const uint8_t hdcp_i2c_offsets[] = {
[HDCP_MESSAGE_ID_WRITE_REPEATER_AUTH_SEND_ACK] = 0x60,
[HDCP_MESSAGE_ID_WRITE_REPEATER_AUTH_STREAM_MANAGE] = 0x60,
[HDCP_MESSAGE_ID_READ_REPEATER_AUTH_STREAM_READY] = 0x80,
- [HDCP_MESSAGE_ID_READ_RXSTATUS] = 0x70
+ [HDCP_MESSAGE_ID_READ_RXSTATUS] = 0x70,
+ [HDCP_MESSAGE_ID_WRITE_CONTENT_STREAM_TYPE] = 0x0,
};
struct protection_properties {
@@ -184,7 +185,7 @@ static const struct protection_properties
hdmi_14_protection = {
.process_transaction = hdmi_14_process_transaction
};
-static const uint32_t hdcp_dpcd_addrs[] = {
+static const uint32_t hdcp_dpcd_addrs[HDCP_MESSAGE_ID_MAX] = {
[HDCP_MESSAGE_ID_READ_BKSV] = 0x68000,
[HDCP_MESSAGE_ID_READ_RI_R0] = 0x68005,
[HDCP_MESSAGE_ID_READ_PJ] = 0x,
--
2.30.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Implement dmub trace event
Hello Yongqiang Sun,
This is a semi-automatic email about new static checker warnings.
The patch 70732504c53b: "drm/amd/display: Implement dmub trace event"
from Feb 19, 2021, leads to the following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_srv.c:520
dmub_srv_hw_init()
error: we previously assumed 'tracebuff_fb' could be null (see line 447)
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_srv.c
446
447 if (inst_fb && data_fb && bios_fb && mail_fb && tracebuff_fb &&
This old code assumes "tracebuff_fb" can be NULL.
448 fw_state_fb && scratch_mem_fb) {
449 cw2.offset.quad_part = data_fb->gpu_addr;
450 cw2.region.base = DMUB_CW0_BASE + inst_fb->size;
451 cw2.region.top = cw2.region.base + data_fb->size;
452
453 cw3.offset.quad_part = bios_fb->gpu_addr;
454 cw3.region.base = DMUB_CW3_BASE;
455 cw3.region.top = cw3.region.base + bios_fb->size;
456
457 cw4.offset.quad_part = mail_fb->gpu_addr;
458 cw4.region.base = DMUB_CW4_BASE;
459 cw4.region.top = cw4.region.base + mail_fb->size;
460
461 /**
462 * Doubled the mailbox region to accomodate inbox and
outbox.
463 * Note: Currently, currently total mailbox size is
16KB. It is split
464 * equally into 8KB between inbox and outbox. If this
config is
465 * changed, then uncached base address configuration of
outbox1
466 * has to be updated in funcs->setup_out_mailbox.
467 */
468 inbox1.base = cw4.region.base;
469 inbox1.top = cw4.region.base + DMUB_RB_SIZE;
470 outbox1.base = inbox1.top;
471 outbox1.top = cw4.region.top;
472
473 cw5.offset.quad_part = tracebuff_fb->gpu_addr;
474 cw5.region.base = DMUB_CW5_BASE;
475 cw5.region.top = cw5.region.base + tracebuff_fb->size;
476
477 outbox0.base = DMUB_REGION5_BASE +
TRACE_BUFFER_ENTRY_OFFSET;
478 outbox0.top = outbox0.base + sizeof(struct
dmcub_trace_buf_entry) * PERF_TRACE_MAX_ENTRY;
479
480
481 cw6.offset.quad_part = fw_state_fb->gpu_addr;
482 cw6.region.base = DMUB_CW6_BASE;
483 cw6.region.top = cw6.region.base + fw_state_fb->size;
484
485 dmub->fw_state = fw_state_fb->cpu_addr;
486
487 dmub->scratch_mem_fb = *scratch_mem_fb;
488
489 if (dmub->hw_funcs.setup_windows)
490 dmub->hw_funcs.setup_windows(dmub, &cw2, &cw3,
&cw4,
491 &cw5, &cw6);
492
493 if (dmub->hw_funcs.setup_outbox0)
494 dmub->hw_funcs.setup_outbox0(dmub, &outbox0);
495
496 if (dmub->hw_funcs.setup_mailbox)
497 dmub->hw_funcs.setup_mailbox(dmub, &inbox1);
498 if (dmub->hw_funcs.setup_out_mailbox)
499 dmub->hw_funcs.setup_out_mailbox(dmub,
&outbox1);
500 }
501
502 if (mail_fb) {
503 dmub_memset(&rb_params, 0, sizeof(rb_params));
504 rb_params.ctx = dmub;
505 rb_params.base_address = mail_fb->cpu_addr;
506 rb_params.capacity = DMUB_RB_SIZE;
507
508 dmub_rb_init(&dmub->inbox1_rb, &rb_params);
509
510 // Initialize outbox1 ring buffer
511 rb_params.ctx = dmub;
512 rb_params.base_address = (void *) ((uint64_t)
(mail_fb->cpu_addr) + DMUB_RB_SIZE);
513 rb_params.capacity = DMUB_RB_SIZE;
514 dmub_rb_init(&dmub->outbox1_rb, &rb_params);
515
516 }
517
518 dmub_memset(&outbox0_rb_params, 0, sizeof(outbox0_rb_params));
519 outbox0_rb_params.ctx = dmub;
520 outbox0_rb_params.base_address = (void
*)((uint64_t)(tracebuff_fb->cpu_addr) + TRACE_BUFFER_ENTRY_OFFSET);
^^
This new code just dereferences "tracebuff_fb" without checking.
521 outbox0_rb_params.capacity = sizeof(struct
dmcub_trac
[PATCH] drm/amdgpu: off by on in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at
"i == 0". Currently it starts a "i" and ends at "i == 1". The result
is that it removes one attribute that wasn't created yet, and leaks the
zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b75362bf0742..ee4a8e44fbeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -1931,7 +1931,7 @@ static int amdgpu_device_attr_create_groups(struct
amdgpu_device *adev,
uint32_t mask)
{
int ret = 0;
- uint32_t i = 0;
+ int i;
for (i = 0; i < counts; i++) {
ret = amdgpu_device_attr_create(adev, &attrs[i], mask);
@@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct
amdgpu_device *adev,
return 0;
failed:
- for (; i > 0; i--) {
+ while (--i >= 0)
amdgpu_device_attr_remove(adev, &attrs[i]);
- }
return ret;
}
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: off by on in amdgpu_device_attr_create_groups() error handling
On Wed, May 20, 2020 at 02:05:19PM +0200, Christian König wrote:
> Am 20.05.20 um 14:00 schrieb Dan Carpenter:
> > This loop in the error handling code should start a "i - 1" and end at
> > "i == 0". Currently it starts a "i" and ends at "i == 1". The result
> > is that it removes one attribute that wasn't created yet, and leaks the
> > zeroeth attribute.
> >
> > Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code")
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > index b75362bf0742..ee4a8e44fbeb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > @@ -1931,7 +1931,7 @@ static int amdgpu_device_attr_create_groups(struct
> > amdgpu_device *adev,
> > uint32_t mask)
> > {
> > int ret = 0;
> > - uint32_t i = 0;
> > + int i;
> > for (i = 0; i < counts; i++) {
> > ret = amdgpu_device_attr_create(adev, &attrs[i], mask);
> > @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct
> > amdgpu_device *adev,
> > return 0;
> > failed:
> > - for (; i > 0; i--) {
> > + while (--i >= 0)
>
> As far as I know the common idiom for this is while (i--) which even works
> without changing the type of i to signed.
It's about 50/50, one way or the other. To me --i >= 0 seems far more
readable.
I've been trying to figure out which tool tells people to make iterators
unsigned so I can help them avoid it. :/ I understand how in theory
iterators could go above INT_MAX but if we're going above INT_MAX then
probably we should use a 64 bit type. There are very few times where 2
billion iterations is not enough but in those situations probably 4
billion is not enough either. So unsigned int iterators never or seldom
solve real life bugs but they regularly cause them.
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amdgpu: off by on in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at
"i == 0". Currently it starts a "i" and ends at "i == 1". The result
is that it removes one attribute that wasn't created yet, and leaks the
zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code")
Signed-off-by: Dan Carpenter
---
v2: style change
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b75362bf0742..e809534fabd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct
amdgpu_device *adev,
return 0;
failed:
- for (; i > 0; i--) {
+ while (i--)
amdgpu_device_attr_remove(adev, &attrs[i]);
- }
return ret;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3] drm/amdgpu: off by one in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at
"i == 0". Currently it starts a "i" and ends at "i == 1". The result
is that it removes one attribute that wasn't created yet, and leaks the
zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code")
Signed-off-by: Dan Carpenter
---
v2: style change
v3: Fix embarrassing typo in the subject
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b75362bf0742..e809534fabd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct
amdgpu_device *adev,
return 0;
failed:
- for (; i > 0; i--) {
+ while (i--)
amdgpu_device_attr_remove(adev, &attrs[i]);
- }
return ret;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amdkfd: Track SDMA utilization per process
Hello Mukul Joshi, This is a semi-automatic email about new static checker warnings. The patch 522b89c63370: "drm/amdkfd: Track SDMA utilization per process" from May 26, 2020, leads to the following Smatch complaint: drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c:109 kfd_sdma_activity_worker() warn: variable dereferenced before check 'pdd' (see line 106) drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c:109 kfd_sdma_activity_worker() warn: address of 'pdd->qpd' is non-NULL drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c 105 pdd = workarea->pdd; 106 dqm = pdd->dev->dqm; Dereference. 107 qpd = &pdd->qpd; 108 109 if (!pdd || !dqm || !qpd) ^^^ ^^^ pdd is checked too late and qpd can't possibly be NULL. 110 return; 111 regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Add DCN3 HWSEQ
Hello Bhawanpreet Lakha,
This is a semi-automatic email about new static checker warnings.
The patch 581b9589487e: "drm/amd/display: Add DCN3 HWSEQ" from May
21, 2020, leads to the following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:463
dcn30_init_hw()
warn: variable dereferenced before check 'res_pool->dccg' (see line 437)
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c
436 // Initialize the dccg
437 if (res_pool->dccg->funcs->dccg_init)
^^
The patch adds a dereference.
438 res_pool->dccg->funcs->dccg_init(res_pool->dccg);
439
440 //Enable ability to power gate / don't force power on
permanently
441 hws->funcs.enable_power_gating_plane(dc->hwseq, true);
442
443 if (IS_FPGA_MAXIMUS_DC(dc->ctx->dce_environment)) {
444 REG_WRITE(RBBMIF_TIMEOUT_DIS, 0x);
445 REG_WRITE(RBBMIF_TIMEOUT_DIS_2, 0x);
446
447 hws->funcs.dccg_init(hws);
448
449 REG_UPDATE(DCHUBBUB_GLOBAL_TIMER_CNTL,
DCHUBBUB_GLOBAL_TIMER_REFDIV, 2);
450 REG_UPDATE(DCHUBBUB_GLOBAL_TIMER_CNTL,
DCHUBBUB_GLOBAL_TIMER_ENABLE, 1);
451 REG_WRITE(REFCLK_CNTL, 0);
452 } else {
453 if (!dcb->funcs->is_accelerated_mode(dcb)) {
454 hws->funcs.bios_golden_init(dc);
455 hws->funcs.disable_vga(dc->hwseq);
456 }
457
458 if (dc->ctx->dc_bios->fw_info_valid) {
459 res_pool->ref_clocks.xtalin_clock_inKhz =
460
dc->ctx->dc_bios->fw_info.pll_info.crystal_frequency;
461
462 if
(!IS_FPGA_MAXIMUS_DC(dc->ctx->dce_environment)) {
463 if (res_pool->dccg && res_pool->hubbub)
{
^^
This check is too late.
464
465
(res_pool->dccg->funcs->get_dccg_ref_freq)(res_pool->dccg,
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Add DCN3 HWSEQ
606 /* power AFMT HDMI memory TODO: may move to dis/en output save
power*/
607 REG_WRITE(DIO_MEM_PWR_CTRL, 0);
608
609 if (!dc->debug.disable_clock_gate) {
610 /* enable all DCN clock gating */
611 REG_WRITE(DCCG_GATE_DISABLE_CNTL, 0);
612
613 REG_WRITE(DCCG_GATE_DISABLE_CNTL2, 0);
614
615 REG_UPDATE(DCFCLK_CNTL, DCFCLK_GATE_DIS, 0);
616 }
617
618 if (dc->clk_mgr->funcs->notify_wm_ranges)
^^^
This code dereferences it without checking for NULL.
619 dc->clk_mgr->funcs->notify_wm_ranges(dc->clk_mgr);
620
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: Fix indenting in dcn30_set_output_transfer_func()
These lines are a part of the if statement and they are supposed to be indented one more tab. Signed-off-by: Dan Carpenter --- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c index ab20320ebc994..37c310dbb3665 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c @@ -203,9 +203,9 @@ bool dcn30_set_output_transfer_func(struct dc *dc, stream->out_transfer_func, &mpc->blender_params, false)) params = &mpc->blender_params; -/* there are no ROM LUTs in OUTGAM */ - if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED) - BREAK_TO_DEBUGGER(); +/* there are no ROM LUTs in OUTGAM */ + if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED) + BREAK_TO_DEBUGGER(); } } -- 2.26.2 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: Fix buffer overflow in arcturus_get_unique_id()
The comments say that the "sn" buffer is used to hold a 16-digit HEX
string so the buffer needs to be at least 17 characters to hold the
NUL terminator.
Fixes: 81a16241114b ("drm/amdgpu: Add unique_id and serial_number for Arcturus
v3")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index df7b408319f76..a575cb9d1574c 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2266,7 +2266,7 @@ static void arcturus_get_unique_id(struct smu_context
*smu)
{
struct amdgpu_device *adev = smu->adev;
uint32_t top32, bottom32, smu_version, size;
- char sn[16];
+ char sn[20];
uint64_t id;
if (smu_get_smc_version(smu, NULL, &smu_version)) {
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Fix indenting in dcn30_set_output_transfer_func()
On Mon, Jun 08, 2020 at 10:16:27AM -0700, Joe Perches wrote: > On Mon, 2020-06-08 at 17:16 +0300, Dan Carpenter wrote: > > These lines are a part of the if statement and they are supposed to > > be indented one more tab. > > > > Signed-off-by: Dan Carpenter > > --- > > drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c > > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c > > index ab20320ebc994..37c310dbb3665 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c > > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c > > @@ -203,9 +203,9 @@ bool dcn30_set_output_transfer_func(struct dc *dc, > > stream->out_transfer_func, > > &mpc->blender_params, false)) > > params = &mpc->blender_params; > > -/* there are no ROM LUTs in OUTGAM */ > > - if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED) > > - BREAK_TO_DEBUGGER(); > > +/* there are no ROM LUTs in OUTGAM */ > > + if (stream->out_transfer_func->type == > > TF_TYPE_PREDEFINED) > > + BREAK_TO_DEBUGGER(); > > } > > } > > > > Maybe the if is at the right indentation but the > close brace below the if is misplaced instead? > Yeah. I considered that, but the code is correct, it's just the indenting is wrong. I normally leave drm/amd/ code alone but this indenting was so confusing that I though it was worth fixing. There are lots of ugly stuff which is not confusing like this: (The line numbers are from next-20200605). drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c:1530 pp_asic_reset_mode_2() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3387 bw_calcs() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dwb.c:104 dwb2_enable() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dpp_cm.c:450 dpp20_get_blndgam_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dpp_cm.c:543 dpp20_get_shaper_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_mpc.c:306 mpc20_get_ogam_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:1519 dc_link_dp_perform_link_training() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:3137 core_link_enable_stream() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:207 dcn30_set_output_transfer_func() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_dpp.c:650 dpp3_get_blndgam_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_dpp.c:747 dpp3_get_shaper_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_dpp_cm.c:67 dpp30_get_gamcor_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_mpc.c:116 mpc3_get_ogam_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_mpc.c:432 mpc3_get_shaper_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_resource.c:2351 dcn30_update_bw_bounding_box() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_optc.c:178 optc3_set_dsc_config() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:2704 dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.c:2777 dml20v2_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:2633 DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:5031 dml21_ModeSupportAndSystemConfigurationFull() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:5036 dml21_ModeSupportAndSystemConfigurationFull() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:5056 dml21_ModeSupportAndSystemConfigurationFull() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../
[PATCH v2] drm/amdgpu: Fix a buffer overflow handling the serial number
The comments say that the serial number is a 16-digit HEX string so the
buffer needs to be at least 17 characters to hold the NUL terminator.
The other issue is that "size" returned from sprintf() is the number of
characters before the NUL terminator so the memcpy() wasn't copying the
terminator. The serial number needs to be NUL terminated so that it
doesn't lead to a read overflow in amdgpu_device_get_serial_number().
Also it's just cleaner and faster to sprintf() directly to adev->serial[]
instead of using a temporary buffer.
Fixes: 81a16241114b ("drm/amdgpu: Add unique_id and serial_number for Arcturus
v3")
Signed-off-by: Dan Carpenter
---
v2: Change adev->serial. The original patch would have just moved the
overflow until amdgpu_device_get_serial_number() is called.
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 6 ++
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 135530286f34f..905cf0bac100c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -986,7 +986,7 @@ struct amdgpu_device {
/* Chip product information */
charproduct_number[16];
charproduct_name[32];
- charserial[16];
+ charserial[20];
struct amdgpu_autodump autodump;
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index df7b408319f76..d58146a5fa21d 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -2265,8 +2265,7 @@ static void arcturus_i2c_eeprom_control_fini(struct
i2c_adapter *control)
static void arcturus_get_unique_id(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- uint32_t top32, bottom32, smu_version, size;
- char sn[16];
+ uint32_t top32, bottom32, smu_version;
uint64_t id;
if (smu_get_smc_version(smu, NULL, &smu_version)) {
@@ -2289,8 +2288,7 @@ static void arcturus_get_unique_id(struct smu_context
*smu)
/* For Arcturus-and-later, unique_id == serial_number, so convert it to
a
* 16-digit HEX string for convenience and backwards-compatibility
*/
- size = sprintf(sn, "%llx", id);
- memcpy(adev->serial, &sn, size);
+ sprintf(adev->serial, "%llx", id);
}
static bool arcturus_is_baco_supported(struct smu_context *smu)
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display: Use dmub fw to lock pipe, cursor, dig
Hello Wyatt Wood,
This is a semi-automatic email about new static checker warnings.
The patch 4dc079787b23: "drm/amd/display: Use dmub fw to lock pipe,
cursor, dig" from May 27, 2020, leads to the following Smatch
complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:4091
dc_link_dp_set_test_pattern()
warn: variable dereferenced before check 'pipe_ctx->stream' (see line 4068)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c
4067 color_space,
4068
pipe_ctx->stream->use_vsc_sdp_for_colorimetry,
4069
link->dpcd_caps.dprx_feature.bits.SST_SPLIT_SDP_CAP);
4070
4071 if (pipe_ctx->stream->use_vsc_sdp_for_colorimetry) {
^^
This is dereferenced in the original code.
4072 if (test_pattern ==
DP_TEST_PATTERN_COLOR_SQUARES_CEA)
4073 pipe_ctx->stream->vsc_infopacket.sb[17]
|= (1 << 7); // sb17 bit 7 Dynamic Range: 0 = VESA range, 1 = CTA range
4074 else
4075 pipe_ctx->stream->vsc_infopacket.sb[17]
&= ~(1 << 7);
4076 resource_build_info_frame(pipe_ctx);
4077 link->dc->hwss.update_info_frame(pipe_ctx);
4078 }
4079
4080 /* CRTC Patterns */
4081 set_crtc_test_pattern(link, pipe_ctx, test_pattern,
test_pattern_color_space);
4082
pipe_ctx->stream_res.tg->funcs->unlock(pipe_ctx->stream_res.tg);
4083
pipe_ctx->stream_res.tg->funcs->wait_for_state(pipe_ctx->stream_res.tg,
4084 CRTC_STATE_VACTIVE);
4085
pipe_ctx->stream_res.tg->funcs->wait_for_state(pipe_ctx->stream_res.tg,
4086 CRTC_STATE_VBLANK);
4087
pipe_ctx->stream_res.tg->funcs->wait_for_state(pipe_ctx->stream_res.tg,
4088 CRTC_STATE_VACTIVE);
4089
4090 if
(pipe_ctx->stream_res.tg->funcs->lock_doublebuffer_disable) {
4091 if (pipe_ctx->stream &&
should_use_dmub_lock(pipe_ctx->stream->link)) {
The patch adds a new dereference, is it really required?
4092 union dmub_hw_lock_flags hw_locks = { 0
};
4093 struct dmub_hw_lock_inst_flags
inst_flags = { 0 };
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: remove an unnecessary NULL check
The "stream" pointer can't be NULL and it's dereferenced on the line
before so the check doesn't make sense.
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 4f97329d9daf..7e58f242dab9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2324,7 +2324,7 @@ static void commit_planes_for_stream(struct dc *dc,
if ((update_type != UPDATE_TYPE_FAST) &&
stream->update_flags.bits.dsc_changed)
if
(top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
- if (stream && should_use_dmub_lock(stream->link)) {
+ if (should_use_dmub_lock(stream->link)) {
union dmub_hw_lock_flags hw_locks = { 0 };
struct dmub_hw_lock_inst_flags inst_flags = { 0
};
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: off by one bugs in smu_cmn_to_asic_specific_index()
These tables have _COUNT number of elements so the comparisons should be
>= instead of > to prevent reading one element beyond the end of the
array.
Fixes: 8264ee69f0d8 ("drm/amd/powerplay: drop unused code")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/powerplay/smu_cmn.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/smu_cmn.c
b/drivers/gpu/drm/amd/powerplay/smu_cmn.c
index be4b678d0e60..5c23c44c33bd 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_cmn.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_cmn.c
@@ -166,7 +166,7 @@ int smu_cmn_to_asic_specific_index(struct smu_context *smu,
switch (type) {
case CMN2ASIC_MAPPING_MSG:
- if (index > SMU_MSG_MAX_COUNT ||
+ if (index >= SMU_MSG_MAX_COUNT ||
!smu->message_map)
return -EINVAL;
@@ -181,7 +181,7 @@ int smu_cmn_to_asic_specific_index(struct smu_context *smu,
return msg_mapping.map_to;
case CMN2ASIC_MAPPING_CLK:
- if (index > SMU_CLK_COUNT ||
+ if (index >= SMU_CLK_COUNT ||
!smu->clock_map)
return -EINVAL;
@@ -192,7 +192,7 @@ int smu_cmn_to_asic_specific_index(struct smu_context *smu,
return mapping.map_to;
case CMN2ASIC_MAPPING_FEATURE:
- if (index > SMU_FEATURE_COUNT ||
+ if (index >= SMU_FEATURE_COUNT ||
!smu->feature_map)
return -EINVAL;
@@ -203,7 +203,7 @@ int smu_cmn_to_asic_specific_index(struct smu_context *smu,
return mapping.map_to;
case CMN2ASIC_MAPPING_TABLE:
- if (index > SMU_TABLE_COUNT ||
+ if (index >= SMU_TABLE_COUNT ||
!smu->table_map)
return -EINVAL;
@@ -214,7 +214,7 @@ int smu_cmn_to_asic_specific_index(struct smu_context *smu,
return mapping.map_to;
case CMN2ASIC_MAPPING_PWR:
- if (index > SMU_POWER_SOURCE_COUNT ||
+ if (index >= SMU_POWER_SOURCE_COUNT ||
!smu->pwr_src_map)
return -EINVAL;
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: fix dp_dsc_clock_en_read() debugfs function
There are problems with the dp_dsc_clock_en_read() function. Only one
of the memory leak is a runtime bug.
1) It leaks memory on the -ENXIO and -EFAULT error paths.
2) There is a discrepency between rd_buf_size (10) and str_len (30).
Static analysis complain that this could lead to a buffer overflow,
but actually the buffer overflow is prevented by other factors.
3) The "rd_buf_ptr" is assigned "+= str_len" but the result is not used.
This leads to static checker warnings as well. Also the "str_len"
is misleading because it's not the strlen() and in fact is beyond
the end of the buffer.
4) This code re-implements the simple_read_from_buffer() function.
This code can be cleaned up by removing the allocation and using the
simple_read_from_buffer() function.
Fixes: c06e09b76639 ("drm/amd/display: Add DSC parameters logging to debugfs")
Signed-off-by: Dan Carpenter
---
.../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 38 +++
1 file changed, 5 insertions(+), 33 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 e5a6d9115949..114962922ff3 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
@@ -983,22 +983,13 @@ static ssize_t dp_dpcd_data_read(struct file *f, char
__user *buf,
static ssize_t dp_dsc_clock_en_read(struct file *f, char __user *buf,
size_t size, loff_t *pos)
{
- char *rd_buf = NULL;
- char *rd_buf_ptr = NULL;
struct amdgpu_dm_connector *aconnector = file_inode(f)->i_private;
struct display_stream_compressor *dsc;
struct dcn_dsc_state dsc_state = {0};
- const uint32_t rd_buf_size = 10;
struct pipe_ctx *pipe_ctx;
- ssize_t result = 0;
- int i, r, str_len = 30;
-
- rd_buf = kcalloc(rd_buf_size, sizeof(char), GFP_KERNEL);
-
- if (!rd_buf)
- return -ENOMEM;
-
- rd_buf_ptr = rd_buf;
+ char rd_buf[10];
+ int len;
+ int i;
for (i = 0; i < MAX_PIPES; i++) {
pipe_ctx =
&aconnector->dc_link->dc->current_state->res_ctx.pipe_ctx[i];
@@ -1014,27 +1005,8 @@ static ssize_t dp_dsc_clock_en_read(struct file *f, char
__user *buf,
if (dsc)
dsc->funcs->dsc_read_state(dsc, &dsc_state);
- snprintf(rd_buf_ptr, str_len,
- "%d\n",
- dsc_state.dsc_clock_en);
- rd_buf_ptr += str_len;
-
- while (size) {
- if (*pos >= rd_buf_size)
- break;
-
- r = put_user(*(rd_buf + result), buf);
- if (r)
- return r; /* r = -EFAULT */
-
- buf += 1;
- size -= 1;
- *pos += 1;
- result += 1;
- }
-
- kfree(rd_buf);
- return result;
+ len = snprintf(rd_buf, sizeof(rd_buf), "%d\n", dsc_state.dsc_clock_en);
+ return simple_read_from_buffer(buf, size, pos, rd_buf, len);
}
static ssize_t dp_dsc_slice_width_read(struct file *f, char __user *buf,
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
