[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
[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(>reset_domain->sem);
return -EINVAL;
}
queue_input.queue_type = (uint32_t)queue_type;
--
2.43.0
[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
[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
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: 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 = >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
[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(>page_retirement_dwork, 0);
+ schedule_delayed_work(>page_retirement_dwork, 0);
}
static int amdgpu_ras_poison_consumption_handler(struct amdgpu_device *adev,
--
2.43.0
[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
[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,
);
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 );
8409 update->cursor_position =
_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(, 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 ))
8438 DRM_ERROR("DC failed to set cursor
attributes\n");
8439
8440 update->cursor_attributes =
_state->stream->cursor_attributes;
8441
8442 if (!dc_stream_set_cursor_position(crtc_state->stream,
8443))
8444 DRM_ERROR("DC failed to set cursor
position\n");
8445
8446 update->cursor_position =
_state->stream->cursor_position;
8447 }
8448 }
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
[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 ? >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
[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(>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
- REG_SET(I
[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 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
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
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
[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
[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 = _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/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
[PATCH] drm/amd/display: delete unnecessary check in dcn35_set_long_vblank()
"timing" is "_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 =
_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
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
[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();
186
187 drm_printf(, " AMDGPU Device Coredump \n");
188 drm_printf(, "version: " AMDGPU_COREDUMP_VERSION "\n");
189 drm_printf(, "kernel: " UTS_RELEASE "\n");
190 drm_printf(, "module: " KBUILD_MODNAME "\n");
191 drm_printf(, "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(, "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(, "\nRing timed out details\n");
201 drm_printf(, "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 >adev->vm_manager.fault_info;
209
210 drm_printf(, "\n[%s] Page fault observed\n",
211fault_info->vmhub ? "mmhub" : "gfxhub");
212 drm_printf(, "Faulty page starting at address:
0x%016llx\n",
213fault_info->addr);
214 drm_printf(, "Protection fault status register:
0x%x\n\n",
215fault_info->status);
216 }
217
218 drm_printf(, "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(, "ring name: %s\n", ring->name);
224 drm_printf(, "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(, "Ring size in dwords: %d\n",
229ring->ring_size / 4);
230 drm_printf(, "Ring contents\n");
231 drm_printf(, "Offset \t Value\n");
232
233 while (j < ring->ring_size) {
234 drm_printf(, "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(, "VRAM is lost due to GPU reset!\n");
241 if (coredump->adev->reset_info.num_regs) {
^^
Here too
242 drm_printf(, "AMDGPU register dumps:\nOffset:
Value:\n");
243
244 for (i = 0; i < coredump->adev->reset_info.num_regs;
i++)
245 drm_printf(, "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
[bug report] drm/amd/display: Add debug counters to IPS exit prints
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/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 = >dm_dp_aux.aux.ddc;
6665 else
ddc = >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(_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
[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
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
[bug report] drm/amd/display: Simplify the per-CPU usage.
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, >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, >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, >base,
2572 _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(_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
[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
[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(>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
[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 = _hwid_mcatypes[type];
if (!hwip->hwid)
--
2.43.0
[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
[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(>hive->kobj);
601 return adev->hive;
602 }
603
--> 604 mutex_lock(_mutex);
^^^
Shhh The mutexes are sleeping.
605
606 list_for_each_entry(hive, _hive_list, node) {
The caller is amdgpu_gfx_disable_kcq():
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
516 spin_lock(>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(>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 >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(>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(>in_recovery) ||
hive_ras_recovery)) {
543 spin_unlock(>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(>ring_lock);
regards,
dan carpenter
[bug report] drm/amd/display: Add interface to enable DPIA trace
Hello Stylon Wang,
The patch 71ba6b577a35: "drm/amd/display: Add interface to enable
DPIA trace" from Jun 30, 2023 (linux-next), leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:1041
dc_dmub_srv_enable_dpia_trace()
error: we previously assumed 'dc_dmub_srv' could be null (see line 1040)
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c
1033 void dc_dmub_srv_enable_dpia_trace(const struct dc *dc)
1034 {
1035 struct dc_dmub_srv *dc_dmub_srv = dc->ctx->dmub_srv;
1036 struct dmub_srv *dmub;
1037 enum dmub_status status;
1038 static const uint32_t timeout_us = 30;
1039
1040 if (!dc_dmub_srv || !dc_dmub_srv->dmub) {
--> 1041 DC_LOG_ERROR("%s: invalid parameters.", __func__);
^
This macro dereferences dc_dmub_srv.
1042 return;
1043 }
1044
1045 dmub = dc_dmub_srv->dmub;
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:513 amdgpu_get_xgmi_hive()
warn: sleeping in atomic context
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
500 struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device
*adev)
501 {
502 struct amdgpu_hive_info *hive = NULL;
503 int ret;
504
505 if (!adev->gmc.xgmi.hive_id)
506 return NULL;
507
508 if (adev->hive) {
509 kobject_get(>hive->kobj);
510 return adev->hive;
511 }
512
--> 513 mutex_lock(_mutex);
The patch adds a new caller amdgpu_gfx_disable_kcq() which is holding
spin_lock(>ring_lock). And we can't take a mutex if we're already
holding a spin_lock. Turn on CONFIG_DEBUG_ATOMIC_SLEEP to see the
warning.
regards,
dan carpenter
[bug report] drm/amd/display: Introduce DML2
Hello Qingqing Zhuo,
The patch 7966f319c66d: "drm/amd/display: Introduce DML2" from Jul
28, 2023 (linux-next), leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:77
map_hw_resources()
error: buffer overflow
'dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_stream_id' 6 <= 7
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c:79
map_hw_resources()
error: buffer overflow
'dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_plane_id' 6 <= 7
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_wrapper.c
59 static void map_hw_resources(struct dml2_context *dml2,
60 struct dml_display_cfg_st *in_out_display_cfg, struct
dml_mode_support_info_st *mode_support_info)
61 {
62 unsigned int num_pipes = 0;
63 int i, j;
64
65 for (i = 0; i < __DML_NUM_PLANES__; i++) {
^^
__DML_NUM_PLANES__ is 8. This loops 0-7.
66 in_out_display_cfg->hw.ODMMode[i] =
mode_support_info->ODMMode[i];
67 in_out_display_cfg->hw.DPPPerSurface[i] =
mode_support_info->DPPPerSurface[i];
68 in_out_display_cfg->hw.DSCEnabled[i] =
mode_support_info->DSCEnabled[i];
69 in_out_display_cfg->hw.NumberOfDSCSlices[i] =
mode_support_info->NumberOfDSCSlices[i];
70 in_out_display_cfg->hw.DLGRefClkFreqMHz = 24;
71 if (dml2->v20.dml_core_ctx.project != dml_project_dcn35
&&
72 dml2->v20.dml_core_ctx.project !=
dml_project_dcn351) {
73 /*dGPU default as 50Mhz*/
74 in_out_display_cfg->hw.DLGRefClkFreqMHz = 50;
75 }
76 for (j = 0; j < mode_support_info->DPPPerSurface[i];
j++) {
--> 77
dml2->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id[num_pipes] =
dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_stream_id[i];
Both of these arrays have 6 elements.
78
dml2->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_stream_id_valid[num_pipes]
= true;
79
dml2->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_plane_id[num_pipes] =
dml2->v20.scratch.dml_to_dc_pipe_mapping.disp_cfg_to_plane_id[i];
80
dml2->v20.scratch.dml_to_dc_pipe_mapping.dml_pipe_idx_to_plane_id_valid[num_pipes]
= true;
81 num_pipes++;
82 }
83 }
84 }
regards,
dan carpenter
[bug report] drm/amd/display: Introduce DML2
Hello Qingqing Zhuo,
The patch 7966f319c66d: "drm/amd/display: Introduce DML2" from Jul
28, 2023 (linux-next), leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:2748
TruncToValidBPP()
warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c
2736 MinDSCBPP = 8;
2737 MaxDSCBPP = 3 * DSCInputBitPerComponent - 1.0 / 16;
2738 } else {
2739 if (Output == dml_hdmi) {
2740 NonDSCBPP0 = 24;
2741 NonDSCBPP1 = 24;
2742 NonDSCBPP2 = 24;
2743 } else {
2744 NonDSCBPP0 = 16;
2745 NonDSCBPP1 = 20;
2746 NonDSCBPP2 = 24;
2747 }
--> 2748 if (Format == dml_n422) {
This code should be indented another tab.
2749 MinDSCBPP = 7;
2750 MaxDSCBPP = 2 * DSCInputBitPerComponent - 1.0
/ 16.0;
2751 } else {
2752 MinDSCBPP = 8;
2753 MaxDSCBPP = 3 * DSCInputBitPerComponent - 1.0
/ 16.0;
2754 }
2755 }
2756
2757 if (Output == dml_dp2p0) {
2758 MaxLinkBPP = LinkBitRate * Lanes / PixelClock * 128.0
/ 132.0 * 383.0 / 384.0 * 65536.0 / 65540.0;
There are a bunch of other warnings as well. Too many to review.
regards,
dan carpenter
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:2903 dm_resume()
warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dce_calcs.c:914
calculate_bandwidth() error: uninitialized symbol 'max_chunks_fbc_mode'.
drivers/gpu/drm/amd/amdgpu/../display/dc/basics/dce_calcs.c:917
calculate_bandwidth() error: uninitialized symbol 'max_chunks_fbc_mode'.
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:702
dcn35_clk_mgr_helper_populate_bw_params() warn: if statement not indented
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:1207
CalculatePrefetchSchedule() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:1288
CalculatePrefetchSchedule() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:1455
CalculatePrefetchSchedule() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:1558
CalculatePrefetchSchedule() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:2645
CalculateVMAndRowBytes() warn: previously used '*PixelPTEReqWidth' as divisor
(see line 2634)
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:2748
TruncToValidBPP() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:2750
TruncToValidBPP() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:2812
TruncToValidBPP() warn: ignoring unreachable code.
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:3171
CalculateDCFCLKDeepSleep() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:3749
CalculateVMGroupAndRequestTimes() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:4051
CalculateStutterEfficiency() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:4201
CalculateSwathAndDETConfiguration() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:4247
CalculateSwathAndDETConfiguration() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:4784
CalculateSurfaceSizeInMall() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:4789
CalculateSurfaceSizeInMall() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:5196
CalculateVMRowAndSwath() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6013
CalculatePrefetchBandwithSupport() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6195
CalculateMaxVStartup() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6916
dml_core_mode_support() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:7206
dml_core_mode_support() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:7455
dml_core_mode_support() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:7869
dml_core_mode_support() warn:
[bug report] drm/amd/display: add DMUB registers to crash dump diagnostic data.
Hello Ashley Thomas,
The patch 2631ac1ac328: "drm/amd/display: add DMUB registers to crash
dump diagnostic data." from May 17, 2021 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:800
dc_dmub_srv_log_diagnostic_data()
error: we previously assumed 'dc_dmub_srv' could be null (see line 799)
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c
795 void dc_dmub_srv_log_diagnostic_data(struct dc_dmub_srv *dc_dmub_srv)
796 {
797 struct dmub_diagnostic_data diag_data = {0};
798
799 if (!dc_dmub_srv || !dc_dmub_srv->dmub) {
^^^
Check for NULL.
--> 800 DC_LOG_ERROR("%s: invalid parameters.", __func__);
The logging will dereference dc_dmub_srv.
801 return;
802 }
803
804 if (!dc_dmub_srv_get_diagnostic_data(dc_dmub_srv, _data)) {
regards,
dan carpenter
[bug report] drm/amd/display: switch DC over to the new DRM logging macros
Hello Hamza Mahfooz,
The patch 5d72e247e58c: "drm/amd/display: switch DC over to the new
DRM logging macros" from Sep 20, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:542
dm_helpers_dp_read_dpcd()
error: we previously assumed 'aconnector' could be null (see line 541)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c
531 bool dm_helpers_dp_read_dpcd(
532 struct dc_context *ctx,
533 const struct dc_link *link,
534 uint32_t address,
535 uint8_t *data,
536 uint32_t size)
537 {
538
539 struct amdgpu_dm_connector *aconnector = link->priv;
540
541 if (!aconnector) {
^^
Check for NULL
--> 542 drm_dbg_dp(aconnector->base.dev,
NULL dereference.
543"Failed to find connector for link!\n");
544 return false;
545 }
546
547 return drm_dp_dpcd_read(>dm_dp_aux.aux, address,
data,
548 size) == size;
549 }
regards,
dan carpenter
[PATCH] drm/amd/pm: delete dead code
"ret" was checked earlier inside the loop, so we know it is zero here. No need to check a second time. Signed-off-by: Dan Carpenter --- drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c index 11a6cd96c601..0ffe55e713f3 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c @@ -2346,9 +2346,6 @@ static int mca_get_mca_entry(struct amdgpu_device *adev, enum amdgpu_mca_error_t return ret; } - if (ret) - return ret; - entry->idx = idx; entry->type = type; -- 2.39.2
[bug report] drm/amd/display: Add DCN35 CLK_MGR
973
"smu_dpm_clks.dpm_clks->DfPstateTable[%d].MemClk= %d\n"
974
"smu_dpm_clks.dpm_clks->DfPstateTable[%d].Voltage = %d\n",
975 i,
smu_dpm_clks.dpm_clks->DfPstateTable[i].FClk,
976 i,
smu_dpm_clks.dpm_clks->DfPstateTable[i].MemClk,
977 i,
smu_dpm_clks.dpm_clks->DfPstateTable[i].Voltage);
978 }
979
980 if (ctx->dc_bios && ctx->dc_bios->integrated_info &&
ctx->dc->config.use_default_clock_table == false) {
This NULL check is too late. It will already have crashed.
981 dcn35_clk_mgr_helper_populate_bw_params(
982 _mgr->base,
regards,
dan carpenter
[bug report] Mass report of new Smatch warnings
Here is the list of new warning which were introduced while I was out of office. The line numbers are from linux-next next-20230905. regards, dan carpenter drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:292 dcn35_update_clocks() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:919 dcn35_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:921 dcn35_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:980 dcn35_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios->integrated_info' (see line 913) drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c:980 dcn35_clk_mgr_construct() warn: variable dereferenced before check 'ctx->dc_bios' (see line 913) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hwseq.c:1751 dcn20_program_pipe() error: we previously assumed 'pipe_ctx->plane_state' could be null (see line 1710) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:159 dcn35_init_hw() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:159 dcn35_init_hw() warn: variable dereferenced before check 'res_pool->dccg' (see line 150) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:206 dcn35_init_hw() error: we previously assumed 'res_pool->hubbub' could be null (see line 159) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:285 dcn35_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see line 136) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:977 dcn35_calc_blocks_to_gate() error: we previously assumed 'pipe_ctx->plane_res.hubp' could be null (see line 973) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_hwseq.c:980 dcn35_calc_blocks_to_gate() warn: always true condition '(pipe_ctx->plane_res.mpcc_inst >= 0) => (0-255 >= 0)' drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_pg_cntl.c:203 pg_cntl35_hubp_dpp_pg_control() warn: duplicate check 'power_on' (previous on line 193) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_pg_cntl.c:318 pg_cntl35_io_clk_pg_control() warn: duplicate check 'power_on' (previous on line 312) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_pg_cntl.c:404 pg_cntl35_plane_otg_pg_control() warn: duplicate check 'power_on' (previous on line 398) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn35/dcn35_resource.c:1877 dcn35_resource_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn35/dcn35_fpu.c:260 dcn35_update_bw_bounding_box_fpu() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn35/dcn35_fpu.c:351 dcn35_update_bw_bounding_box_fpu() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_srv.c:355 dmub_srv_hw_setup() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/nbio_v7_11.c:34 nbio_v7_11_get_rev_id() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_6_ppt.c:351 smu_v13_0_6_setup_driver_pptable() warn: should this be 'retry == -1'
[PATCH] drm/amdgpu: fix retry loop test
This loop will exit with "retry" set to -1 if it fails but the code
checks for if "retry" is zero. Fix this by changing post-op to a
pre-op. --retry vs retry--.
Fixes: e01eeffc3f86 ("drm/amd/pm: avoid driver getting empty metrics table for
the first time")
Signed-off-by: Dan Carpenter
---
Obviously this only loop 99 times now instead of a hundred but that's
fine, this is an approximation.
drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index ff58ee14a68f..20163a9b2a66 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -336,7 +336,7 @@ static int smu_v13_0_6_setup_driver_pptable(struct
smu_context *smu)
/* Store one-time values in driver PPTable */
if (!pptable->Init) {
- while (retry--) {
+ while (--retry) {
ret = smu_v13_0_6_get_metrics_table(smu, NULL, true);
if (ret)
return ret;
--
2.39.2
Re: [bug report] drm/amdgpu: add selftest framework for UMSCH
On Wed, Sep 06, 2023 at 07:07:32PM +0800, Lang Yu wrote:
> On 09/06/ , Dan Carpenter wrote:
>
> Thanks for reporting this bug. Can you give a link to this bug report? Commit
> message requests it.
> ("Reported-by: should be immediately followed by Link: with a URL to the
> report")
>
My email hasn't hit lore.kernel.org yet... Presumably it will in a bit.
We could link to yours or swap out the the message-id.
https://lore.kernel.org/all/ZPhddADtKmOuVyDq@lang-desktop/
regards,
dan carpenter
[bug report] drm/amdgpu: add selftest framework for UMSCH
Hello Lang Yu,
The patch 5d5eac7e8303: "drm/amdgpu: add selftest framework for
UMSCH" from Jun 21, 2023 (linux-next), leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c:338 setup_umsch_mm_test()
warn: unsigned error codes 'test->pasid'
drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
319 static int setup_umsch_mm_test(struct amdgpu_device *adev,
320 struct umsch_mm_test *test)
321 {
322 struct amdgpu_vmhub *hub = >vmhub[AMDGPU_MMHUB0(0)];
323 int r;
324
325 test->vm_cntx_cntl = hub->vm_cntx_cntl;
326
327 test->vm = kzalloc(sizeof(*test->vm), GFP_KERNEL);
328 if (!test->vm) {
329 r = -ENOMEM;
330 return r;
331 }
332
333 r = amdgpu_vm_init(adev, test->vm, -1);
334 if (r)
335 goto error_free_vm;
336
337 test->pasid = amdgpu_pasid_alloc(16);
--> 338 if (test->pasid < 0) {
^^^
Unsigned can't be less than zero.
339 r = test->pasid;
340 goto error_fini_vm;
341 }
342
343 r = amdgpu_bo_create_kernel(adev, sizeof(struct
umsch_mm_test_ctx_data),
regards,
dan carpenter
[bug report] drm/amd/display: Add Functions to enable Freesync Panel Replay
Hello Bhawanpreet Lakha,
The patch c7ddc0a800bc: "drm/amd/display: Add Functions to enable
Freesync Panel Replay" from May 12, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:849
edp_set_replay_allow_active()
error: we previously assumed 'replay' could be null (see line 841)
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:932
edp_setup_replay()
warn: duplicate check 'replay' (previous on line 904)
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c
834 bool edp_set_replay_allow_active(struct dc_link *link, const bool
*allow_active,
835 bool wait, bool force_static, const unsigned int *power_opts)
836 {
837 struct dc *dc = link->ctx->dc;
838 struct dmub_replay *replay = dc->res_pool->replay;
839 unsigned int panel_inst;
840
841 if (replay == NULL && force_static)
replay is allow to be NULL if force_static is false.
842 return false;
843
844 if (!dc_get_edp_link_panel_inst(dc, link, _inst))
845 return false;
846
847 /* Set power optimization flag */
848 if (power_opts && link->replay_settings.replay_power_opt_active
!= *power_opts) {
--> 849 if (link->replay_settings.replay_feature_enabled &&
replay->funcs->replay_set_power_opt) {
Unchecked dereference.
850 replay->funcs->replay_set_power_opt(replay,
*power_opts, panel_inst);
851 link->replay_settings.replay_power_opt_active =
*power_opts;
852 }
853 }
854
855 /* Activate or deactivate Replay */
856 if (allow_active && link->replay_settings.replay_allow_active
!= *allow_active) {
857 // TODO: Handle mux change case if force_static is set
858 // If force_static is set, just change the
replay_allow_active state directly
859 if (replay != NULL &&
link->replay_settings.replay_feature_enabled)
860 replay->funcs->replay_enable(replay,
*allow_active, wait, panel_inst);
861 link->replay_settings.replay_allow_active =
*allow_active;
862 }
863
864 return true;
865 }
regards,
dan carpenter
[bug report] drm/amdgpu: optimize amdgpu device attribute code
Hello Kevin Wang, This is a semi-automatic email about new static checker warnings. The patch 4e01847c38f7: "drm/amdgpu: optimize amdgpu device attribute code" from Apr 27, 2020, leads to the following Smatch complaint: ./drivers/gpu/drm/amd/pm/amdgpu_pm.c:2182 amdgpu_device_attr_create() warn: variable dereferenced before check 'attr' (see line 2175) ./drivers/gpu/drm/amd/pm/amdgpu_pm.c 2174 struct device_attribute *dev_attr = >dev_attr; 2175 const char *name = dev_attr->attr.name; ^^ Dereferenced. 2176 enum amdgpu_device_attr_states attr_states = ATTR_STATE_SUPPORTED; 2177 struct amdgpu_device_attr_entry *attr_entry; 2178 2179 int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr, 2180 uint32_t mask, enum amdgpu_device_attr_states *states) = default_attr_update; 2181 2182 BUG_ON(!attr); Checked to late. Also doesn't checkpatch warn about this? Calling BUG_ON() here is not correct. 2183 2184 attr_update = attr->attr_update ? attr->attr_update : default_attr_update; regards, dan carpenter
[PATCH] drm/amd/display: Unlock on error path in dm_handle_mst_sideband_msg_ready_event()
This error path needs to unlock the "aconnector->handle_mst_msg_ready"
mutex before returning.
Fixes: bb4fa525f327 ("drm/amd/display: Add polling method to handle MST reply
packet")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 1abdec14344e..943959012d04 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
@@ -707,7 +707,7 @@ void dm_handle_mst_sideband_msg_ready_event(
if (retry == 3) {
DRM_ERROR("Failed to ack MST event.\n");
- return;
+ break;
}
drm_dp_mst_hpd_irq_send_new_request(>mst_mgr);
--
2.39.2
[bug report] drm/amd/display: Reduce stack size
Hello Rodrigo Siqueira, The patch 135fd1b35690: "drm/amd/display: Reduce stack size" from Jun 21, 2023 (linux-next), leads to the following Smatch static checker warning: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:8785 amdgpu_dm_atomic_commit_tail() error: potential null dereference 'dummy_updates'. (kzalloc returns null) drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c 8771 status = dc_stream_get_status(dm_new_crtc_state->stream); 8772 8773 if (WARN_ON(!status)) 8774 continue; 8775 8776 WARN_ON(!status->plane_count); 8777 8778 /* 8779 * TODO: DC refuses to perform stream updates without a dc_surface_update. 8780 * Here we create an empty update on each plane. 8781 * To fix this, DC should permit updating only stream properties. 8782 */ 8783 dummy_updates = kzalloc(sizeof(struct dc_surface_update) * MAX_SURFACES, GFP_ATOMIC); This needs a NULL check. 8784 for (j = 0; j < status->plane_count; j++) --> 8785 dummy_updates[j].surface = status->plane_states[0]; Kaplooey! 8786 8787 8788 mutex_lock(>dc_lock); 8789 dc_update_planes_and_stream(dm->dc, 8790 dummy_updates, 8791 status->plane_count, 8792 dm_new_crtc_state->stream, 8793 _update); 8794 mutex_unlock(>dc_lock); 8795 kfree(dummy_updates); regards, dan carpenter
Re: [PATCH v3 00/18] fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags
On Fri, Jul 14, 2023 at 12:24:05PM +0200, Thomas Zimmermann wrote: > > > > >fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers > > >fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers > > >fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers > > >fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers > > > I wasn't happy about this either. But I could not come up with a description > that fits into the 74-char limit for each commit. They only differ in the > method of memory allocation. Do you have any ideas? fbdev: Remove FBINFO_DEFAULT from static structs fbdev: Remove FBINFO_DEFAULT from kzalloc() structs fbdev: Remove FBINFO_DEFAULT from devm_kzalloc() structs regards, dan carpenter
[bug report] drm/amdgpu/gfx11: add aggregated doorbell support
Hello Jack Xiao,
The patch af019bef6d6f: "drm/amdgpu/gfx11: add aggregated doorbell
support" from Jul 11, 2022, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:5193
gfx_v11_0_ring_set_wptr_gfx()
warn: duplicate check '*is_queue_unmap'
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
5166 static void gfx_v11_0_ring_set_wptr_gfx(struct amdgpu_ring *ring)
5167 {
5168 struct amdgpu_device *adev = ring->adev;
5169 uint32_t *wptr_saved;
5170 uint32_t *is_queue_unmap;
5171 uint64_t aggregated_db_index;
5172 uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_GFX].mqd_size;
5173 uint64_t wptr_tmp;
5174
5175 if (ring->is_mes_queue) {
5176 wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
5177 is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size
+
5178 sizeof(uint32_t));
5179 aggregated_db_index =
5180 amdgpu_mes_get_aggregated_doorbell_index(adev,
5181
ring->hw_prio);
5182
5183 wptr_tmp = ring->wptr & ring->buf_mask;
5184 atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
wptr_tmp);
5185 *wptr_saved = wptr_tmp;
5186 /* assume doorbell always being used by mes mapped
queue */
5187 if (*is_queue_unmap) {
^^^
Non-zero here.
5188 WDOORBELL64(aggregated_db_index, wptr_tmp);
5189 WDOORBELL64(ring->doorbell_index, wptr_tmp);
5190 } else {
5191 WDOORBELL64(ring->doorbell_index, wptr_tmp);
5192
--> 5193 if (*is_queue_unmap)
^^^
You would think it's zero here. Although, possibly this value is
getting set in another thread?
5194 WDOORBELL64(aggregated_db_index,
wptr_tmp);
5195 }
5196 } else {
5197 if (ring->use_doorbell) {
5198 /* XXX check if swapping is necessary on BE */
5199 atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
5200 ring->wptr);
5201 WDOORBELL64(ring->doorbell_index, ring->wptr);
5202 } else {
5203 WREG32_SOC15(GC, 0, regCP_RB0_WPTR,
5204 lower_32_bits(ring->wptr));
5205 WREG32_SOC15(GC, 0, regCP_RB0_WPTR_HI,
5206 upper_32_bits(ring->wptr));
5207 }
5208 }
5209 }
regards,
dan carpenter
Re: [PATCH] drm/amd/amdgpu: Properly tune the size of struct
On Tue, Jun 20, 2023 at 12:59:19PM +0800, Su Hui wrote: > Smatch error: > gpu/drm/amd/amdgpu/amdgv_sriovmsg.h:316:49: error: > static assertion failed: "amd_sriov_msg_pf2vf_info must be 1 KB" > static assertion failed: "amd_sriov_msg_vf2pf_info must be 1 KB" > I doubt that moving the struct members around is safe. This looks like it's in a very specific order. So I don't think this patch is correct. The reason for this false positive this code uses a #pragma to pack the struct. #pragma pack(push, 1) // PF2VF / VF2PF data areas are byte packed Sparse does not support this and Smatch uses Sparse as a parser. The main reason why Sparse doesn't support this pragma is because Linus thinks it's gross. You probably didn't even see the #pragma did you? And anything you can't see is unreadable by definition. "Mark the associated types properly packed individually, rather than use the disgusting "pragma pack()" that should never be used." https://lore.kernel.org/linux-sparse/CAHk-=wi7jgz+bvbt-ufxokpeqdhzf3z2hbjkgdjh8q4dvpp...@mail.gmail.com/ regards, dan carpenter
Re: [PATCH] drm/amd/amdgpu: Use “__packed“ instead of "pragma pack()"
When there was a #pragma then Sparse just turned off. The Sparse
warnings are places where people forgot to put the __user in their casts
or didn't annotate endianness correctly. It's not a "bug" to forget
to annotate endianness or user pointers. That's how we used to do it
prior to 2003. But these days it feels strange and dangerous to see
these sorts of warnings.
Smatch also disabled some uninitialized variable checks. These are
mostly false positives where we have a loop:
int r;
while (something) {
r = frob();
}
return r;
Smatch complains that we don't necessarily enter the loop. I think
I'm going to disable this type of "enter the loop" warning when you
don't have the cross function database available. That will silence
these for the kbuild bot.
regards,
dan carpenter
Hi Su,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Su-Hui/drm-amd-amdgpu-Use-__packed-instead-of-pragma-pack/20230620-165739
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20230620085543.576733-1-suhui%40nfschina.com
patch subject: [PATCH] drm/amd/amdgpu: Use “__packed“ instead of "pragma pack()"
config: mips-randconfig-m031-20230620
(https://download.01.org/0day-ci/archive/20230621/202306210527.oxvd2jlz-...@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.3.0
reproduce:
(https://download.01.org/0day-ci/archive/20230621/202306210527.oxvd2jlz-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Closes: https://lore.kernel.org/r/202306210527.oxvd2jlz-...@intel.com/
New smatch warnings:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:5914 amdgpu_pci_slot_reset() error:
uninitialized symbol 'memsize'.
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:2244 amdgpu_fill_buffer() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c:55 amdgpu_benchmark_do_move()
error: uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:534 amdgpu_vram_mgr_new() error:
uninitialized symbol 'cur_size'.
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:534 amdgpu_vram_mgr_new() error:
uninitialized symbol 'size'.
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1274 amdgpu_ras_query_error_count()
error: uninitialized symbol 'ret'.
drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c:141
amdgpu_securedisplay_debugfs_write() error: uninitialized symbol 'phy_id'.
drivers/gpu/drm/amd/amdgpu/aldebaran.c:103 aldebaran_mode2_suspend_ip() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/aldebaran.c:385 aldebaran_mode2_restore_hwcontext()
error: uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c:96
sienna_cichlid_mode2_suspend_ip() error: uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c:95 smu_v13_0_10_mode2_suspend_ip()
error: uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c:612
mmhub_v2_0_update_medium_grain_clock_gating() error: uninitialized symbol 'def'.
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:1183 amdgpu_gfx_cp_init_microcode()
error: uninitialized symbol 'ucode_fw'.
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c:1184 amdgpu_gfx_cp_init_microcode()
error: uninitialized symbol 'fw_size'.
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c:594 sdma_v4_0_init_microcode() error:
uninitialized symbol 'ret'.
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c:251 sdma_v5_0_init_microcode() error:
uninitialized symbol 'ret'.
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c:1021 sdma_v5_0_ring_test_ring() error:
uninitialized symbol 'index'.
drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c:905 sdma_v6_0_ring_test_ring() error:
uninitialized symbol 'index'.
drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c:145 sdma_v4_4_2_init_microcode()
error: uninitialized symbol 'ret'.
drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c:1390 sdma_v4_4_2_sw_init() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c:855 sdma_v5_2_ring_test_ring() error:
uninitialized symbol 'index'.
drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c:361 vcn_v3_0_hw_init() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c:222 vcn_v4_0_3_hw_init() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c:281 vcn_v4_0_hw_init() error:
uninitialized symbol 'r'.
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:592
amdgpu_amdkfd_get_xgmi_bandwidth_mbytes() error: uninitialized symbol
'peer_adev'.
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/vangogh_ppt.c:1011
vangogh_get_dpm_ultimate_freq() error: uninitialized symbol 'mclk_mask'.
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/vangogh_ppt.c:1016
vangogh_get_dpm_ultimate_freq() error: uninitialized symbol 'soc_mask'.
drivers/gpu/dr
Re: [PATCH] drm/amd/amdgpu: Properly tune the size of struct
On Tue, Jun 20, 2023 at 10:37:59AM +0300, Dan Carpenter wrote:
> "Mark the associated types properly packed individually, rather than
> use the disgusting "pragma pack()" that should never be used."
>
> https://lore.kernel.org/linux-sparse/CAHk-=wi7jgz+bvbt-ufxokpeqdhzf3z2hbjkgdjh8q4dvpp...@mail.gmail.com/
Marking the structs packed could be very simple.
regards,
dan carpenter
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index 104a5ad8397d..e29dae04f7e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -209,7 +209,7 @@ struct amd_sriov_msg_pf2vf_info {
uint32_t pcie_atomic_ops_support_flags;
/* reserved */
uint32_t reserved[256 - AMD_SRIOV_MSG_PF2VF_INFO_FILLED_SIZE];
-};
+} __packed;
struct amd_sriov_msg_vf2pf_info_header {
/* the total structure size in byte */
@@ -262,7 +262,7 @@ struct amd_sriov_msg_vf2pf_info {
struct {
uint8_t id;
uint32_t version;
- } ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
+ } __packed ucode_info[AMD_SRIOV_MSG_RESERVE_UCODE];
uint64_t dummy_page_addr;
/* reserved */
[PATCH] drm/amdkfd: potential error pointer dereference in ioctl
The "target" either comes from kfd_create_process() which returns error
pointers on error or kfd_lookup_process_by_pid() which returns NULL on
error. So we need to check for both types of errors.
Fixes: a42e42c4e3b1 ("drm/amdkfd: prepare per-process debug enable and disable")
Signed-off-by: Dan Carpenter
---
I'm not sure how to compile this code or why I'm seeing this warning
again after two years... Very strange.
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index fc385000c007..6a27b000a246 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2920,9 +2920,9 @@ static int kfd_ioctl_set_debug_trap(struct file *filep,
struct kfd_process *p, v
target = kfd_lookup_process_by_pid(pid);
}
- if (!target) {
+ if (IS_ERR_OR_NULL(target)) {
pr_debug("Cannot find process PID %i to debug\n", args->pid);
- r = -ESRCH;
+ r = target ? PTR_ERR(target) : -ESRCH;
goto out;
}
--
2.39.2
[PATCH] drm/amd/pm: Fix memory some memory corruption
The "od_table" is a pointer to a large struct, but this code is doing
pointer math as if it were pointing to bytes. It results in writing
far outside the struct.
Fixes: f0a0c659fb96 ("drm/amd/pm: fulfill the OD support for SMU13.0.0")
Fixes: e3afa4f988b3 ("drm/amd/pm: fulfill the OD support for SMU13.0.7")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 4 ++--
drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index 5ac5ea770c1c..413e592f0ed6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -1535,7 +1535,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
smu_context *smu,
* settings. Thus we do not cache it.
*/
offset_of_featurectrlmask = offsetof(OverDriveTable_t,
FeatureCtrlMask);
- if (memcmp(od_table + offset_of_featurectrlmask,
+ if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
table_context->user_overdrive_table +
offset_of_featurectrlmask,
sizeof(OverDriveTableExternal_t) -
offset_of_featurectrlmask)) {
smu_v13_0_0_dump_od_table(smu, od_table);
@@ -1548,7 +1548,7 @@ static int smu_v13_0_0_od_edit_dpm_table(struct
smu_context *smu,
od_table->OverDriveTable.FeatureCtrlMask = 0;
memcpy(table_context->user_overdrive_table +
offset_of_featurectrlmask,
- od_table + offset_of_featurectrlmask,
+ (u8 *)od_table + offset_of_featurectrlmask,
sizeof(OverDriveTableExternal_t) -
offset_of_featurectrlmask);
if (!memcmp(table_context->user_overdrive_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
index 0bd086360efa..cda4e818aab7 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
@@ -1524,7 +1524,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
smu_context *smu,
* settings. Thus we do not cache it.
*/
offset_of_featurectrlmask = offsetof(OverDriveTable_t,
FeatureCtrlMask);
- if (memcmp(od_table + offset_of_featurectrlmask,
+ if (memcmp((u8 *)od_table + offset_of_featurectrlmask,
table_context->user_overdrive_table +
offset_of_featurectrlmask,
sizeof(OverDriveTableExternal_t) -
offset_of_featurectrlmask)) {
smu_v13_0_7_dump_od_table(smu, od_table);
@@ -1537,7 +1537,7 @@ static int smu_v13_0_7_od_edit_dpm_table(struct
smu_context *smu,
od_table->OverDriveTable.FeatureCtrlMask = 0;
memcpy(table_context->user_overdrive_table +
offset_of_featurectrlmask,
- od_table + offset_of_featurectrlmask,
+ (u8 *)od_table + offset_of_featurectrlmask,
sizeof(OverDriveTableExternal_t) -
offset_of_featurectrlmask);
if (!memcmp(table_context->user_overdrive_table,
--
2.39.2
[bug report] drm: Track drm_mm allocators and show leaks on shutdown
Hello Chris Wilson,
The patch 5705670d0463: "drm: Track drm_mm allocators and show leaks
on shutdown" from Oct 31, 2016, leads to the following Smatch static
checker warning:
drivers/gpu/drm/drm_mm.c:1001 drm_mm_takedown()
warn: sleeping in atomic context
drivers/gpu/drm/drm_mm.c
991 * drm_mm_takedown - clean up a drm_mm allocator
992 * @mm: drm_mm allocator to clean up
993 *
994 * Note that it is a bug to call this function on an allocator which is
not
995 * clean.
996 */
997 void drm_mm_takedown(struct drm_mm *mm)
998 {
999 if (WARN(!drm_mm_clean(mm),
1000 "Memory manager not clean during takedown.\n"))
--> 1001 show_leaks(mm);
^^^
The show_leaks() function does a GFP_KERNEL allocation but a couple
of the callers for drm_mm_takedown() are holding spinlocks so it's a
sleeping in atomic bug.
The problematic callers are ttm_range_man_fini_nocheck() and
amdgpu_gtt_mgr_fini().
1002 }
regards,
dan carpenter
[bug report] drm/amd/display: Check clock table return
Hello Rodrigo Siqueira,
This is a semi-automatic email about new static checker warnings.
The patch 4b4f21ff7f5d: "drm/amd/display: Check clock table return"
from Aug 20, 2020, leads to the following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn21/rn_clk_mgr.c:775
rn_clk_mgr_construct()
warn: variable dereferenced before check 'ctx->dc_bios->integrated_info'
(see line 743)
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn21/rn_clk_mgr.c
742
743 if (ctx->dc_bios->integrated_info->memory_type ==
LpDdr4MemType) {
^^
Not sure why Smatch is complaining about this after three years.
744 if (clk_mgr->periodic_retraining_disabled) {
745 rn_bw_params.wm_table =
lpddr4_wm_table_with_disabled_ppt;
746 } else {
747 if (is_green_sardine)
748 rn_bw_params.wm_table =
lpddr4_wm_table_gs;
749 else
750 rn_bw_params.wm_table =
lpddr4_wm_table_rn;
751 }
752 } else {
753 if (is_green_sardine)
754 rn_bw_params.wm_table = ddr4_wm_table_gs;
755 else {
756 if (ctx->dc->config.is_single_rank_dimm)
757 rn_bw_params.wm_table =
ddr4_1R_wm_table_rn;
758 else
759 rn_bw_params.wm_table =
ddr4_wm_table_rn;
760 }
761 }
762 /* Saved clocks configured at boot for debug purposes */
763 rn_dump_clk_registers(_mgr->base.boot_snapshot,
_mgr->base, _info);
764
765 clk_mgr->base.dprefclk_khz = 60;
766 dce_clock_read_ss_info(clk_mgr);
767
768
769 clk_mgr->base.bw_params = _bw_params;
770
771 if (pp_smu && pp_smu->rn_funcs.get_dpm_clock_table) {
772 status =
pp_smu->rn_funcs.get_dpm_clock_table(_smu->rn_funcs.pp_smu, _table);
773
774 if (status == PP_SMU_RESULT_OK &&
775 ctx->dc_bios && ctx->dc_bios->integrated_info) {
Check for NULL is too late.
776 rn_clk_mgr_helper_populate_bw_params
(clk_mgr->base.bw_params, _table, ctx->dc_bios->integrated_info);
777 /* treat memory config as single channel if
memory is asymmetrics. */
regards,
dan carpenter
[PATCH] drm/amd/amdgpu: Fix up locking etc in amdgpu_debugfs_gprwave_ioctl()
There are two bugs here.
1) Drop the lock if copy_from_user() fails.
2) If the copy fails then the correct error code is -EFAULT instead of
-EINVAL.
I also broke up the long line and changed "sizeof rd->id" to
"sizeof(rd->id)".
Fixes: 164fb2940933 ("drm/amd/amdgpu: Update debugfs for XCC support (v3)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index c657bed350ac..56e89e76ff17 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -478,15 +478,16 @@ static ssize_t amdgpu_debugfs_gprwave_read(struct file
*f, char __user *buf, siz
static long amdgpu_debugfs_gprwave_ioctl(struct file *f, unsigned int cmd,
unsigned long data)
{
struct amdgpu_debugfs_gprwave_data *rd = f->private_data;
- int r;
+ int r = 0;
mutex_lock(>lock);
switch (cmd) {
case AMDGPU_DEBUGFS_GPRWAVE_IOC_SET_STATE:
- r = copy_from_user(>id, (struct
amdgpu_debugfs_gprwave_iocdata *)data, sizeof rd->id);
- if (r)
- return r ? -EINVAL : 0;
+ if (copy_from_user(>id,
+ (struct amdgpu_debugfs_gprwave_iocdata
*)data,
+ sizeof(rd->id)))
+ r = -EFAULT;
goto done;
default:
r = -EINVAL;
--
2.39.2
[bug report] drm/amd/display: Clean FPGA code in dc
_clock_table) {
772 status =
pp_smu->rn_funcs.get_dpm_clock_table(_smu->rn_funcs.pp_smu, _table);
773
774 if (status == PP_SMU_RESULT_OK &&
--> 775 ctx->dc_bios && ctx->dc_bios->integrated_info) {
Checking for NULL after a dereference doesn't work.
776 rn_clk_mgr_helper_populate_bw_params
(clk_mgr->base.bw_params, _table, ctx->dc_bios->integrated_info);
777 /* treat memory config as single channel if
memory is asymmetrics. */
778 if (ctx->dc->config.is_asymmetric_memory)
779 clk_mgr->base.bw_params->num_channels =
1;
780 }
781 }
782
783 /* enable powerfeatures when displaycount goes to 0 */
784 if (clk_mgr->smu_ver >= 0x00371500)
785 rn_vbios_smu_enable_48mhz_tmdp_refclk_pwrdwn(clk_mgr,
!debug->disable_48mhz_pwrdwn);
786 }
regards,
dan carpenter
[bug report] drm/amd/display: Add connector HPD trigger debugfs entry
Hello Eryk Brol,
This is a semi-automatic email about new static checker warnings.
The patch 6f77b2ac6280: "drm/amd/display: Add connector HPD trigger
debugfs entry" from Aug 10, 2020, leads to the following Smatch
complaint:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c:1220
trigger_hotplug()
warn: variable dereferenced before check 'aconnector' (see line 1210)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c
1204 static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
1205 size_t size,
loff_t *pos)
1206 {
1207 struct amdgpu_dm_connector *aconnector =
file_inode(f)->i_private;
1208 struct drm_connector *connector = >base;
Not a dereference.
1209 struct dc_link *link = NULL;
1210 struct drm_device *dev = connector->dev;
^^
Argh... dereference.
1211 struct amdgpu_device *adev = drm_to_adev(dev);
1212 enum dc_connection_type new_connection_type =
dc_connection_none;
1213 char *wr_buf = NULL;
1214 uint32_t wr_buf_size = 42;
1215 int max_param_num = 1;
1216 long param[1] = {0};
1217 uint8_t param_nums = 0;
1218 bool ret = false;
1219
1220 if (!aconnector || !aconnector->dc_link)
^^^
Too late. Dead already.
1221 return -EINVAL;
1222
1223 if (size == 0)
regards,
dan carpenter
Re: [PATCH] drm/amdgpu: remove unnecessary (void*) conversions
On Mon, May 15, 2023 at 10:11:39AM -0400, Alex Deucher wrote:
> On Mon, May 15, 2023 at 3:17 AM Dan Carpenter
> wrote:
> >
> > On Mon, May 15, 2023 at 09:34:28AM +0800, Su Hui wrote:
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > index f60753f97ac5..c837e0bf2cfc 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > > @@ -1470,7 +1470,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device
> > > *adev)
> > >
> > > static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
> > > {
> > > - struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
> > > + struct amdgpu_device *adev = m->private;
> > > struct drm_device *dev = adev_to_drm(adev);
> > > int r = 0, i;
> > >
> >
> > This declaration block was originally written in reverse Christmas tree
> > order:
> >
> > long long long variable name;
> > medium length name;
> > short name;
> >
> > So you probably want to change the order now that the lengths have
> > changed. Same in the other places as well.
>
> I don't think it's possible due to the variable dependencies unless
> you separate the declarations and assignments which doesn't seem like
> a net win to me.
Gar. I'm dumb. Sorry for the noise.
regards,
dan carpenter
Re: [PATCH] drm/amdgpu: remove unnecessary (void*) conversions
On Mon, May 15, 2023 at 09:34:28AM +0800, Su Hui wrote:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index f60753f97ac5..c837e0bf2cfc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1470,7 +1470,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
>
> static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
> {
> - struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
> + struct amdgpu_device *adev = m->private;
> struct drm_device *dev = adev_to_drm(adev);
> int r = 0, i;
>
This declaration block was originally written in reverse Christmas tree
order:
long long long variable name;
medium length name;
short name;
So you probably want to change the order now that the lengths have
changed. Same in the other places as well.
regards,
dan carpenter
[PATCH] drm/amdgpu: release correct lock in amdgpu_gfx_enable_kgq()
This function was releasing the incorrect lock on the error path.
Reported-by: kernel test robot
Fixes: 9bfa241d1289 ("drm/amdgpu: add [en/dis]able_kgq() functions")
Signed-off-by: Dan Carpenter
---
The LKP robot sent me an email about this after I had already written
the patch. (I review LKP Smatch emails and hit forward).
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 969f256aa003..7d2f119d9223 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -644,7 +644,7 @@ int amdgpu_gfx_enable_kgq(struct amdgpu_device *adev, int
xcc_id)
adev->gfx.num_gfx_rings);
if (r) {
DRM_ERROR("Failed to lock KIQ (%d).\n", r);
- spin_unlock(>gfx.kiq[0].ring_lock);
+ spin_unlock(>ring_lock);
return r;
}
--
2.39.2
[PATCH] drm/amdgpu: unlock on error in gfx_v9_4_3_kiq_resume()
Smatch complains that we need to drop this lock before returning.
drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c:1838 gfx_v9_4_3_kiq_resume()
warn: inconsistent returns 'ring->mqd_obj->tbo.base.resv'.
Fixes: 86301129698b ("drm/amdgpu: split gc v9_4_3 functionality from gc v9_0")
Signed-off-by: Dan Carpenter
---
The Fixes tag is weird, but I think it's correct?
drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
index 56a415e151d4..552729a514d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
@@ -1827,8 +1827,10 @@ static int gfx_v9_4_3_kiq_resume(struct amdgpu_device
*adev, int xcc_id)
return r;
r = amdgpu_bo_kmap(ring->mqd_obj, (void **)>mqd_ptr);
- if (unlikely(r != 0))
+ if (unlikely(r != 0)) {
+ amdgpu_bo_unreserve(ring->mqd_obj);
return r;
+ }
gfx_v9_4_3_kiq_init_queue(ring, xcc_id);
amdgpu_bo_kunmap(ring->mqd_obj);
--
2.39.2
[PATCH] drm/amdgpu: unlock the correct lock in amdgpu_gfx_enable_kcq()
We changed which lock we are supposed to take but this error path
was accidentally over looked so it still drops the old lock.
Fixes: def799c6596d ("drm/amdgpu: add multi-xcc support to amdgpu_gfx
interfaces (v4)")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 60bb4bba1994..1de3fffae9d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -566,7 +566,7 @@ int amdgpu_gfx_enable_kcq(struct amdgpu_device *adev, int
xcc_id)
kiq->pmf->set_resources_size);
if (r) {
DRM_ERROR("Failed to lock KIQ (%d).\n", r);
- spin_unlock(>gfx.kiq[0].ring_lock);
+ spin_unlock(>ring_lock);
return r;
}
--
2.39.2
[bug report] drm/amd/display: Use per pipe P-State force for FPO
Hello Alvin Lee, This is a semi-automatic email about new static checker warnings. The patch 4ed793083afc: "drm/amd/display: Use per pipe P-State force for FPO" from Mar 15, 2023, leads to the following Smatch complaint: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hwseq.c:2009 dcn20_post_unlock_program_front_end() error: we previously assumed 'hwseq' could be null (see line 2003) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hwseq.c 2002 */ 2003 if (hwseq && hwseq->funcs.update_force_pstate) ^ The patch adds this NULL check but hopefully it can be deleted. Otherwise we are screwed. 2004 dc->hwseq->funcs.update_force_pstate(dc, context); 2005 2006 /* Only program the MALL registers after all the main and phantom pipes 2007 * are done programming. 2008 */ 2009 if (hwseq->funcs.program_mall_pipe_config) 2010 hwseq->funcs.program_mall_pipe_config(dc, context); 2011 regards, dan carpenter
[PATCH] drm/amdgpu: fix AMDGPU_RAS_BLOCK__DF check
There is a mixup where AMDGPU_RAS_BLOCK__DF is used as a mask instead of
a shifter. It means that this condition will be true for
AMDGPU_RAS_BLOCK__MMHUB instead of for AMDGPU_RAS_BLOCK__DF.
Fixes: b6f512168478 ("drm/amdgpu: Add fatal error handling in nbio v4_3")
Signed-off-by: Dan Carpenter
---
>From static analysis. Not tested at all.
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index fac45f98145d..4069bce9479f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2564,7 +2564,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev)
adev->nbio.ras = _v7_4_ras;
break;
case IP_VERSION(4, 3, 0):
- if (adev->ras_hw_enabled & AMDGPU_RAS_BLOCK__DF)
+ if (adev->ras_hw_enabled & (1 << AMDGPU_RAS_BLOCK__DF))
/* unlike other generation of nbio ras,
* nbio v4_3 only support fatal error interrupt
* to inform software that DF is freezed due to
--
2.39.1
[bug report] drm/amd/display: move eDP panel control logic to link_edp_panel_control
The recent function renames made these warnings show up as new again:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:358
edp_receiver_ready_T9() warn: potential negative cast to bool 'result'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:393
edp_receiver_ready_T7() warn: potential negative cast to bool 'result'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c
336 bool edp_receiver_ready_T9(struct dc_link *link)
337 {
338 unsigned int tries = 0;
339 unsigned char sinkstatus = 0;
340 unsigned char edpRev = 0;
341 enum dc_status result = DC_OK;
342
343 result = core_link_read_dpcd(link, DP_EDP_DPCD_REV, ,
sizeof(edpRev));
344
345 /* start from eDP version 1.2, SINK_STAUS indicate the sink is
ready.*/
346 if (result == DC_OK && edpRev >= DP_EDP_12) {
347 do {
348 sinkstatus = 1;
349 result = core_link_read_dpcd(link,
DP_SINK_STATUS, , sizeof(sinkstatus));
350 if (sinkstatus == 0)
351 break;
352 if (result != DC_OK)
353 break;
354 udelay(100); //MAx T9
355 } while (++tries < 50);
356 }
357
--> 358 return result;
^^
result is a non-zero enum so this always returns true. Which is fine
because the caller doesn't check.
359 }
regards,
dan carpenter
[bug report] drm/amd/display: Various logs added
Hello Leo Chen,
This is a semi-automatic email about new static checker warnings.
The patch 7ef414375fcc: "drm/amd/display: Various logs added" from
Aug 29, 2022, leads to the following Smatch complaint:
./drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c:1862
get_firmware_info_v3_2()
warn: variable dereferenced before check 'smu_info_v3_2' (see line 1861)
./drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
1860
DATA_TABLES(smu_info));
1861 DC_LOG_BIOS("gpuclk_ss_percentage (unit of 0.001
percent): %d\n", smu_info_v3_2->gpuclk_ss_percentage);
^^^
Log adds a crash.
1862 if (!smu_info_v3_2)
^
Too late.
1863 return BP_RESULT_BADBIOSTABLE;
1864
regards,
dan carpenter
[bug report] drm/amd/display: break down dc_link.c
Hello Wenjing Liu,
This is a semi-automatic email about new static checker warnings.
The patch 5461d1ea: "drm/amd/display: break down dc_link.c" from
Jan 18, 2023, leads to the following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c:365
dc_link_construct_phy()
warn: variable dereferenced before check 'link->link_enc' (see line 362)
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c
356
357 enc_init_data.transmitter =
358 translate_encoder_to_transmitter(enc_init_data.encoder);
359 link->link_enc =
360 link->dc->res_pool->funcs->link_enc_create(dc_ctx,
_init_data);
361
362 DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d",
link->link_enc->features.flags.bits.DP_IS_USB_C);
Dereference
363 DC_LOG_DC("BIOS object table - IS_DP2_CAPABLE: %d",
link->link_enc->features.flags.bits.IS_DP2_CAPABLE);
364
365 if (!link->link_enc) {
^^
Checked too late
366 DC_ERROR("Failed to create link encoder!\n");
367 goto link_enc_create_fail;
368 }
369
regards,
dan carpenter
[bug report] drm/amd/display: Disable MALL SS and messages for PSR supported configs
Hello Dillon Varone,
This is a semi-automatic email about new static checker warnings.
The patch 0bed85e48af1: "drm/amd/display: Disable MALL SS and
messages for PSR supported configs" from Jan 4, 2023, leads to the
following Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:257
dcn32_apply_idle_power_optimizations()
warn: variable dereferenced before check 'dc->current_state' (see line 249)
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c
248
249 for (i = 0; i < dc->current_state->stream_count; i++) {
^^^
Patch adds unchecked dereference
250 /* MALL SS messaging is not supported with PSR at this
time */
251 if (dc->current_state->streams[i] != NULL &&
252
dc->current_state->streams[i]->link->psr_settings.psr_version !=
DC_PSR_VERSION_UNSUPPORTED)
253 return false;
254 }
255
256 if (enable) {
257 if (dc->current_state) {
Checked too late.
258
259 /* 1. Check no memory request case for CAB.
regards,
dan carpenter
[bug report] drm/amd/display: move eDP panel control logic to link_edp_panel_control
Hello Wenjing Liu,
The patch 0078c924e733: "drm/amd/display: move eDP panel control
logic to link_edp_panel_control" from Dec 19, 2022, leads to the
following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:353
link_edp_receiver_ready_T9() warn: potential negative cast to bool 'result'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:388
link_edp_receiver_ready_T7() warn: potential negative cast to bool 'result'
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c
331 bool link_edp_receiver_ready_T9(struct dc_link *link)
This function returns DC_OK (1) on success or positive on error or -1
on unknown error. So casting it to bool means it always returns true.
332 {
333 unsigned int tries = 0;
334 unsigned char sinkstatus = 0;
335 unsigned char edpRev = 0;
336 enum dc_status result = DC_OK;
337
338 result = core_link_read_dpcd(link, DP_EDP_DPCD_REV, ,
sizeof(edpRev));
339
340 /* start from eDP version 1.2, SINK_STAUS indicate the sink is
ready.*/
341 if (result == DC_OK && edpRev >= DP_EDP_12) {
342 do {
343 sinkstatus = 1;
344 result = core_link_read_dpcd(link,
DP_SINK_STATUS, , sizeof(sinkstatus));
345 if (sinkstatus == 0)
346 break;
347 if (result != DC_OK)
348 break;
349 udelay(100); //MAx T9
350 } while (++tries < 50);
351 }
352
--> 353 return result;
354 }
regards,
dan carpenter
[PATCH] drm/amdgpu: Add a missing tab
This tab was deleted accidentally and triggers a Smatch warning:
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:1006 gfx_v8_0_init_microcode()
warn: inconsistent indenting
Add it back.
Fixes: 0aaafb7359d2 ("drm/amd: Use `amdgpu_ucode_*` helpers for GFX8")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 4fb577d047fd..b1f2684d854a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1003,7 +1003,7 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device
*adev)
err = amdgpu_ucode_request(adev, >gfx.me_fw, fw_name);
if (err == -ENODEV) {
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me.bin",
chip_name);
- err = amdgpu_ucode_request(adev, >gfx.me_fw, fw_name);
+ err = amdgpu_ucode_request(adev, >gfx.me_fw,
fw_name);
}
} else {
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me.bin",
chip_name);
--
2.35.1
Re: [PATCH v6] drm: Optimise for continuous memory allocation
Hi xinhui,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/xinhui-pan/drm-Optimise-for-continuous-memory-allocation/20221218-145922
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20221218065708.93332-1-xinhui.pan%40amd.com
patch subject: [PATCH v6] drm: Optimise for continuous memory allocation
config: s390-randconfig-m041-20221218
compiler: s390-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
smatch warnings:
drivers/gpu/drm/drm_buddy.c:501 find_continuous_blocks() error: uninitialized
symbol 'block'.
vim +/block +501 drivers/gpu/drm/drm_buddy.c
8a257b57bc11a2 xinhui pan 2022-12-18 472 static struct drm_buddy_block *
8a257b57bc11a2 xinhui pan 2022-12-18 473 find_continuous_blocks(struct
drm_buddy *mm,
8a257b57bc11a2 xinhui pan 2022-12-18 474 int order,
8a257b57bc11a2 xinhui pan 2022-12-18 475 unsigned long
flags,
8a257b57bc11a2 xinhui pan 2022-12-18 476 struct
drm_buddy_block **lb)
8a257b57bc11a2 xinhui pan 2022-12-18 477 {
8a257b57bc11a2 xinhui pan 2022-12-18 478 struct list_head *head =
>free_list[order - 1];
8a257b57bc11a2 xinhui pan 2022-12-18 479 struct drm_buddy_block
*free_block, *first = NULL, *last = NULL;
8a257b57bc11a2 xinhui pan 2022-12-18 480
8a257b57bc11a2 xinhui pan 2022-12-18 481 /*
8a257b57bc11a2 xinhui pan 2022-12-18 482* Look for continuous free
memory in buddy and buddy-in-law.
8a257b57bc11a2 xinhui pan 2022-12-18 483* IOW, the most left blocks at
right of free block and the most right
8a257b57bc11a2 xinhui pan 2022-12-18 484* blocks at left of free block.
8a257b57bc11a2 xinhui pan 2022-12-18 485*/
8a257b57bc11a2 xinhui pan 2022-12-18 486
8a257b57bc11a2 xinhui pan 2022-12-18 487 list_for_each_entry(free_block,
head, link) {
8a257b57bc11a2 xinhui pan 2022-12-18 488 struct drm_buddy_block
*buddy, *parent, *block;
8a257b57bc11a2 xinhui pan 2022-12-18 489 int left, min_order = 0;
8a257b57bc11a2 xinhui pan 2022-12-18 490 LIST_HEAD(fbl);
8a257b57bc11a2 xinhui pan 2022-12-18 491
8a257b57bc11a2 xinhui pan 2022-12-18 492 parent =
free_block->parent;
8a257b57bc11a2 xinhui pan 2022-12-18 493 if (!parent)
8a257b57bc11a2 xinhui pan 2022-12-18 494 continue;
8a257b57bc11a2 xinhui pan 2022-12-18 495
8a257b57bc11a2 xinhui pan 2022-12-18 496 left = parent->left ==
free_block;
8a257b57bc11a2 xinhui pan 2022-12-18 497
list_add(_block->tmp_link, );
8a257b57bc11a2 xinhui pan 2022-12-18 498 buddy =
__get_buddy(free_block);
8a257b57bc11a2 xinhui pan 2022-12-18 499
__continuous_block_in_tree(buddy, , left, min_order);
8a257b57bc11a2 xinhui pan 2022-12-18 500
8a257b57bc11a2 xinhui pan 2022-12-18 @501 while (parent &&
!((parent->left == block) ^ left)) {
^
Not initialized on first iteration.
8a257b57bc11a2 xinhui pan 2022-12-18 502 block = parent;
8a257b57bc11a2 xinhui pan 2022-12-18 503 parent =
parent->parent;
8a257b57bc11a2 xinhui pan 2022-12-18 504 }
8a257b57bc11a2 xinhui pan 2022-12-18 505
8a257b57bc11a2 xinhui pan 2022-12-18 506 if (!parent)
8a257b57bc11a2 xinhui pan 2022-12-18 507 continue;
8a257b57bc11a2 xinhui pan 2022-12-18 508
8a257b57bc11a2 xinhui pan 2022-12-18 509 buddy =
__get_buddy(block);
8a257b57bc11a2 xinhui pan 2022-12-18 510
__continuous_block_in_tree(buddy, , !left, min_order);
8a257b57bc11a2 xinhui pan 2022-12-18 511
8a257b57bc11a2 xinhui pan 2022-12-18 512 /* list head of fbl is
invalid outside.
8a257b57bc11a2 xinhui pan 2022-12-18 513* Walk through list
from first fo last only.
8a257b57bc11a2 xinhui pan 2022-12-18 514*/
8a257b57bc11a2 xinhui pan 2022-12-18 515 if
(__free_block_in_order(, free_block, order, , ))
8a257b57bc11a2 xinhui pan 2022-12-18 516 break;
8a257b57bc11a2 xinhui pan 2022-12-18 517 }
8a257b57bc11a2 xinhui pan 2022-12-18 518
8a257b57bc11a2 xinhui pan 2022-12-18 519 *lb = last;
8a257b57bc11a2 xinhui pan 2022-12-18 520 return first;
8a257b57bc11a2 xinhui pan 2022-12-18 521 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
[PATCH] drm/amdkfd: Remove unnecessary condition in kfd_topology_add_device()
We re-arranged this code recently so "ret" is always zero at this point. Signed-off-by: Dan Carpenter --- drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index 6f01ebc8557b..bceb1a5b2518 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -2012,10 +2012,9 @@ int kfd_topology_add_device(struct kfd_dev *gpu) kfd_debug_print_topology(); - if (!res) - kfd_notify_gpu_change(gpu_id, 1); + kfd_notify_gpu_change(gpu_id, 1); - return res; + return 0; } /** -- 2.35.1
Re: [PATCH] drm/amdkfd: Release the topology_lock in error case
On Wed, Nov 16, 2022 at 03:49:18PM -0500, Felix Kuehling wrote: > Am 2022-11-16 um 03:04 schrieb Ma Jun: > > Release the topology_lock in error case > > > > Signed-off-by: Ma Jun > > Reported-by: Dan Carpenter > Dan, did you change your email address, is this one correct? > Yep. I'm still around doing Smatch stuff though: https://lore.kernel.org/all/Y1qf7w%2Fjo8FH5I8G@kadam/ regards, dan carpenter
[bug report] drm/amdkfd: Fix the warning of array-index-out-of-bounds
[ Ugh... My email messed up and I have to Resend all my emails for the
past two weeks. -dan ]
Hello Ma Jun,
The patch c0cc999f3c32: "drm/amdkfd: Fix the warning of
array-index-out-of-bounds" from Nov 2, 2022, leads to the following
Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2008
kfd_topology_add_device()
warn: inconsistent returns '_lock'.
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c
1808 int kfd_topology_add_device(struct kfd_dev *gpu)
1809 {
1810 uint32_t gpu_id;
1811 struct kfd_topology_device *dev;
1812 struct kfd_cu_info cu_info;
1813 int res = 0;
1814 struct list_head temp_topology_device_list;
1815 void *crat_image = NULL;
1816 size_t image_size = 0;
1817 int proximity_domain;
1818 int i;
1819 const char *asic_name = amdgpu_asic_name[gpu->adev->asic_type];
1820
1821 INIT_LIST_HEAD(_topology_device_list);
1822
1823 gpu_id = kfd_generate_gpu_id(gpu);
1824 pr_debug("Adding new GPU (ID: 0x%x) to topology\n", gpu_id);
1825
1826 /* Check to see if this gpu device exists in the
topology_device_list.
1827 * If so, assign the gpu to that device,
1828 * else create a Virtual CRAT for this gpu device and then
parse that
1829 * CRAT to create a new topology device. Once created assign
the gpu to
1830 * that topology device
1831 */
1832 down_write(_lock);
^^
Takes the lock.
1833 dev = kfd_assign_gpu(gpu);
1834 if (!dev) {
1835 proximity_domain = ++topology_crat_proximity_domain;
1836
1837 res = kfd_create_crat_image_virtual(_image,
_size,
1838 COMPUTE_UNIT_GPU,
gpu,
1839 proximity_domain);
1840 if (res) {
1841 pr_err("Error creating VCRAT for GPU (ID:
0x%x)\n",
1842gpu_id);
1843 topology_crat_proximity_domain--;
1844 return res;
Does not drop the lock.
1845 }
1846
1847 res = kfd_parse_crat_table(crat_image,
1848_topology_device_list,
1849proximity_domain);
1850 if (res) {
1851 pr_err("Error parsing VCRAT for GPU (ID:
0x%x)\n",
1852gpu_id);
1853 topology_crat_proximity_domain--;
1854 goto err;
Does not drop the lock.
1855 }
1856
1857
kfd_topology_update_device_list(_topology_device_list,
1858 _device_list);
1859
1860 dev = kfd_assign_gpu(gpu);
1861 if (WARN_ON(!dev)) {
1862 res = -ENODEV;
1863 goto err;
Does not drop the lock.
1864 }
1865
1866 /* Fill the cache affinity information here for the
GPUs
1867 * using VCRAT
1868 */
1869 kfd_fill_cache_non_crat_info(dev, gpu);
1870
1871 /* Update the SYSFS tree, since we added another
topology
1872 * device
1873 */
1874 res = kfd_topology_update_sysfs();
1875 if (!res)
1876 sys_props.generation_count++;
1877 else
1878 pr_err("Failed to update GPU (ID: 0x%x) to
sysfs topology. res=%d\n",
1879 gpu_id, res);
1880 }
1881 up_write(_lock);
^
Drops the lock. The patch has some other locking changes which are
not explained in the commit message and seem unrelated to the out of
bounds issue.
1882
1883 dev->gpu_id = gpu_id;
1884 gpu->id = gpu_id;
regards,
dan carpenter
[PATCH] amdgpu/pm: prevent array underflow in vega20_odn_edit_dpm_table()
In the PP_OD_EDIT_VDDC_CURVE case the "input_index" variable is capped at
2 but not checked for negative values so it results in an out of bounds
read. This value comes from the user via sysfs.
Fixes: d5bf26539494 ("drm/amd/powerplay: added vega20 overdrive support V3")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
index 97b3ad369046..b30684c84e20 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
@@ -2961,7 +2961,8 @@ static int vega20_odn_edit_dpm_table(struct pp_hwmgr
*hwmgr,
data->od8_settings.od8_settings_array;
OverDriveTable_t *od_table =
&(data->smc_state_table.overdrive_table);
- int32_t input_index, input_clk, input_vol, i;
+ int32_t input_clk, input_vol, i;
+ uint32_t input_index;
int od8_id;
int ret;
--
2.35.1
[bug report] drm/amdgpu/mes: use ring for kernel queue submission
Hello Jack Xiao,
The patch d0c423b64765: "drm/amdgpu/mes: use ring for kernel queue
submission" from Mar 27, 2020, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1056 amdgpu_mes_add_ring()
error: format string overflow. buf_size: 16 length: 38 [user data]
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
980 int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
981 int queue_type, int idx,
982 struct amdgpu_mes_ctx_data *ctx_data,
983 struct amdgpu_ring **out)
984 {
985 struct amdgpu_ring *ring;
986 struct amdgpu_mes_gang *gang;
987 struct amdgpu_mes_queue_properties qprops = {0};
988 int r, queue_id, pasid;
989
990 /*
991 * Avoid taking any other locks under MES lock to avoid circular
992 * lock dependencies.
993 */
994 amdgpu_mes_lock(>mes);
995 gang = idr_find(>mes.gang_id_idr, gang_id);
996 if (!gang) {
997 DRM_ERROR("gang id %d doesn't exist\n", gang_id);
998 amdgpu_mes_unlock(>mes);
999 return -EINVAL;
1000 }
1001 pasid = gang->process->pasid;
1002
1003 ring = kzalloc(sizeof(struct amdgpu_ring), GFP_KERNEL);
1004 if (!ring) {
1005 amdgpu_mes_unlock(>mes);
1006 return -ENOMEM;
1007 }
1008
1009 ring->ring_obj = NULL;
1010 ring->use_doorbell = true;
1011 ring->is_mes_queue = true;
1012 ring->mes_ctx = ctx_data;
1013 ring->idx = idx;
1014 ring->no_scheduler = true;
1015
1016 if (queue_type == AMDGPU_RING_TYPE_COMPUTE) {
1017 int offset = offsetof(struct amdgpu_mes_ctx_meta_data,
1018 compute[ring->idx].mec_hpd);
1019 ring->eop_gpu_addr =
1020 amdgpu_mes_ctx_get_offs_gpu_addr(ring, offset);
1021 }
1022
1023 switch (queue_type) {
1024 case AMDGPU_RING_TYPE_GFX:
1025 ring->funcs = adev->gfx.gfx_ring[0].funcs;
1026 break;
1027 case AMDGPU_RING_TYPE_COMPUTE:
1028 ring->funcs = adev->gfx.compute_ring[0].funcs;
1029 break;
1030 case AMDGPU_RING_TYPE_SDMA:
1031 ring->funcs = adev->sdma.instance[0].ring.funcs;
1032 break;
1033 default:
1034 BUG();
1035 }
1036
1037 r = amdgpu_ring_init(adev, ring, 1024, NULL, 0,
1038 AMDGPU_RING_PRIO_DEFAULT, NULL);
1039 if (r)
1040 goto clean_up_memory;
1041
1042 amdgpu_mes_ring_to_queue_props(adev, ring, );
1043
1044 dma_fence_wait(gang->process->vm->last_update, false);
1045 dma_fence_wait(ctx_data->meta_data_va->last_pt_update, false);
1046 amdgpu_mes_unlock(>mes);
1047
1048 r = amdgpu_mes_add_hw_queue(adev, gang_id, , _id);
1049 if (r)
1050 goto clean_up_ring;
1051
1052 ring->hw_queue_id = queue_id;
1053 ring->doorbell_index = qprops.doorbell_off;
1054
1055 if (queue_type == AMDGPU_RING_TYPE_GFX)
--> 1056 sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id,
queue_id);
I'm not sure why this is warning now instead of in 2020. But the bug is
definitely real. "gang_id" is capped at INT_MAX so that can overflow
already even if the values of "pasid" and "queue_id" are zero.
Using snprintf() is safer but also probably the buffer should be larger.
1057 else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
1058 sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
1059 queue_id);
1060 else if (queue_type == AMDGPU_RING_TYPE_SDMA)
1061 sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
1062 queue_id);
1063 else
1064 BUG();
1065
1066 *out = ring;
1067 return 0;
1068
1069 clean_up_ring:
1070 amdgpu_ring_fini(ring);
1071 clean_up_memory:
1072 kfree(ring);
1073 amdgpu_mes_unlock(>mes);
1074 return r;
1075 }
regards,
dan carpenter
[bug report] drm/amd/display: Update MALL SS NumWays calculation
Hello Alvin Lee,
The patch 525a65c77db5: "drm/amd/display: Update MALL SS NumWays
calculation" from Sep 14, 2022, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:282
dcn32_calculate_cab_allocation()
warn: if statement not indented
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c
276
277 // Include cursor size for CAB allocation
278 for (j = 0; j < dc->res_pool->pipe_count; j++) {
279 struct pipe_ctx *pipe = >res_ctx.pipe_ctx[j];
280 struct hubp *hubp = pipe->plane_res.hubp;
281
--> 282 if (pipe->stream && pipe->plane_state && hubp)
283 /* Find the cursor plane and use the exact size
instead of
284 using the max for calculation */
The code for this if statement is missing so it runs into the next if
statement.
285
286 if (hubp->curs_attr.width > 0) {
287 // Round cursor width to next multiple
of 64
288 cursor_size = (((hubp->curs_attr.width
+ 63) / 64) * 64) * hubp->curs_attr.height;
289
290 switch
(pipe->stream->cursor_attributes.color_format) {
291 case CURSOR_MODE_MONO:
292 cursor_size /= 2;
293 cursor_bpp = 4;
294 break;
295 case CURSOR_MODE_COLOR_1BIT_AND:
296 case
CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA:
297 case
CURSOR_MODE_COLOR_UN_PRE_MULTIPLIED_ALPHA:
298 cursor_size *= 4;
299 cursor_bpp = 4;
300 break;
301
regards,
dan carpenter
[bug report] drm/amd/display: Reverted DSC programming sequence change
Hello Nagulendran, Iswara,
The patch c7783a6ed4fc: "drm/amd/display: Reverted DSC programming
sequence change" from Aug 23, 2022, leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:4310
core_link_enable_stream()
warn: if statement not indented
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c
4299
4300 if (pipe_ctx->stream->dpms_off)
4301 return;
4302
4303 /* Have to setup DSC before DIG FE and BE are
connected (which happens before the
4304 * link training). This is to make sure the bandwidth
sent to DIG BE won't be
4305 * bigger than what the link and/or DIG BE can handle.
VBID[6]/CompressedStream_flag
4306 * will be automatically set at a later time when the
video is enabled
4307 * (DP_VID_STREAM_EN = 1).
4308 */
4309 if (pipe_ctx->stream->timing.flags.DSC) {
--> 4310 if (dc_is_dp_signal(pipe_ctx->stream->signal)
||
4311
dc_is_virtual_signal(pipe_ctx->stream->signal))
4312 dp_set_dsc_enable(pipe_ctx, true);
4313
4314 }
This seems likes a bug? Like a line was deleted. Or should it be:
if (pipe_ctx->stream->timing.flags.DSC &&
(dc_is_dp_signal(pipe_ctx->stream->signal) ||
dc_is_virtual_signal(pipe_ctx->stream->signal)))
dp_set_dsc_enable(pipe_ctx, true);
4315
4316 status = enable_link(state, pipe_ctx);
4317
4318 if (status != DC_OK) {
4319 DC_LOG_WARNING("enabling link %u failed: %d\n",
4320 pipe_ctx->stream->link->link_index,
4321 status);
4322
4323 /* Abort stream enable *unless* the failure
was due to
4324 * DP link training - some DP monitors will
recover and
4325 * show the stream anyway. But MST displays
can't proceed
regards,
dan carpenter
Re: [PATCH] drm/amdkfd: potential crash in kfd_create_indirect_link_prop()
On Fri, Aug 12, 2022 at 05:10:51PM -0400, Felix Kuehling wrote:
> On 2022-08-12 02:20, Dan Carpenter wrote:
> > This code has two bugs. If kfd_topology_device_by_proximity_domain()
> > failed on the first iteration through the loop then "cpu_link" is
> > uninitialized and should not be dereferenced.
> >
> > The second bug is that we cannot dereference a list iterator when it
> > points to the list head. In other words, if we exit the
> > list_for_each_entry() loop exits without hitting a break then "cpu_link"
> > is not a valid pointer and should not be dereferenced.
> >
> > Fix both of these problems by setting "cpu_link" to NULL when it is invalid
> > and non-NULL when it is valid. That makes it easier to test for
> > valid vs invalid.
> >
> > Fixes: 0f28cca87e9a ("drm/amdkfd: Extend KFD device topology to surface
> > peer-to-peer links")
> > Signed-off-by: Dan Carpenter
> > ---
> > I reported these in June but never heard back.
>
> I thought Ramesh implemented a fix for this:
> https://lore.kernel.org/all/20220706183302.1719795-1-ramesh.errab...@amd.com/
>
> You commented on a version of his patch:
> https://lore.kernel.org/all/20220629161241.GM11460@kadam/
Oh, Sorry! I appologize for forgetting that.
>
> Did this get lost somehow? Anyway, your patch looks good to me and I'm going
> to apply it to amd-staging-drm-next now.
>
> Reviewed-by: Felix Kuehling
Looking at Ramesh's patch now, he added a continue if
kfd_topology_device_by_proximity_domain() failed. That is a behavior
change, but it might also be a bug fix.
My patch does not change the behavior except for eliminating the crash
so I stand by my patch, but adding the continue might be a good thing to
do as a separate step. I don't know the code well enough to say one way
or the other.
regards,
dan carpenter
[PATCH] drm/amdkfd: potential crash in kfd_create_indirect_link_prop()
This code has two bugs. If kfd_topology_device_by_proximity_domain()
failed on the first iteration through the loop then "cpu_link" is
uninitialized and should not be dereferenced.
The second bug is that we cannot dereference a list iterator when it
points to the list head. In other words, if we exit the
list_for_each_entry() loop exits without hitting a break then "cpu_link"
is not a valid pointer and should not be dereferenced.
Fix both of these problems by setting "cpu_link" to NULL when it is invalid
and non-NULL when it is valid. That makes it easier to test for
valid vs invalid.
Fixes: 0f28cca87e9a ("drm/amdkfd: Extend KFD device topology to surface
peer-to-peer links")
Signed-off-by: Dan Carpenter
---
I reported these in June but never heard back.
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 25990bec600d..3f0a4a415907 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1392,8 +1392,8 @@ static int kfd_build_p2p_node_entry(struct
kfd_topology_device *dev,
static int kfd_create_indirect_link_prop(struct kfd_topology_device *kdev, int
gpu_node)
{
+ struct kfd_iolink_properties *gpu_link, *tmp_link, *cpu_link;
struct kfd_iolink_properties *props = NULL, *props2 = NULL;
- struct kfd_iolink_properties *gpu_link, *cpu_link;
struct kfd_topology_device *cpu_dev;
int ret = 0;
int i, num_cpu;
@@ -1416,16 +1416,19 @@ static int kfd_create_indirect_link_prop(struct
kfd_topology_device *kdev, int g
continue;
/* find CPU <--> CPU links */
+ cpu_link = NULL;
cpu_dev = kfd_topology_device_by_proximity_domain(i);
if (cpu_dev) {
- list_for_each_entry(cpu_link,
+ list_for_each_entry(tmp_link,
_dev->io_link_props, list) {
- if (cpu_link->node_to == gpu_link->node_to)
+ if (tmp_link->node_to == gpu_link->node_to) {
+ cpu_link = tmp_link;
break;
+ }
}
}
- if (cpu_link->node_to != gpu_link->node_to)
+ if (!cpu_link)
return -ENOMEM;
/* CPU <--> CPU <--> GPU, GPU node*/
--
2.35.1
[bug report] drm/amd/display: Add SubVP required code
Hello Alvin Lee,
The patch 85f4bc0c333c: "drm/amd/display: Add SubVP required code"
from May 2, 2022, leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn32/dcn32_clk_mgr_smu_msg.c:103
dcn32_smu_send_cab_for_uclk_message()
warn: was shift intended here '(num_ways > 0)'
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn32/dcn32_clk_mgr_smu_msg.c
101 void dcn32_smu_send_cab_for_uclk_message(struct clk_mgr_internal
*clk_mgr, unsigned int num_ways)
102 {
--> 103 uint32_t param = (num_ways << 1) | (num_ways > 0);
What is happening here? It might be more readable as?
uint32_t param = (num_ways << 1) | (num_ways != 0);
Still confusing actually...
104
105 dcn32_smu_send_msg_with_param(clk_mgr,
DALSMC_MSG_SetCabForUclkPstate, param, NULL);
106 smu_print("Numways for SubVP : %d\n", num_ways);
107 }
regards,
dan carpenter
amdgpu: misc ugliness and inconsistency
Hi AMD devs, I guess there was a big merge in linux-next the other day? Anyway, it generated a lot of Smatch warnings. I reported some of them but there is just a lot so I'm mass reporting these. Inconsistent NULL checking can be harmless. Part of the function assumes that the pointer can be NULL and part assumes that it cannot. It does not make sense, but it is often harmless and the correct response is just to remove the NULL check. The other class of reports is inconsistent indenting. There were two bugs but I already fixed one and reported the other so these should be harmless. regards, dan carpenter Inconsistent NULL checks: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:1271 handle_cursor_update() error: we previously assumed 'afb' could be null (see line 1230) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn201/dcn201_resource.c:1017 dcn201_acquire_idle_pipe_for_layer() error: we previously assumed 'head_pipe' could be null (see line 1011) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_hwseq.c:222 dcn21_set_backlight_level() error: we previously assumed 'panel_cntl' could be null (see line 213) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:765 dcn30_apply_idle_power_optimizations() error: we previously assumed 'stream' could be null (see line 749) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:767 dcn30_apply_idle_power_optimizations() error: we previously assumed 'plane' could be null (see line 749) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_hwseq.c:284 dcn31_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see line 117) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:896 dcn32_init_hw() error: we previously assumed 'dc->clk_mgr' could be null (see line 738) drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:777 subvp_drr_schedulable() error: we previously assumed 'pipe->stream' could be null (see line 768) drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:868 subvp_vblank_schedulable() error: we previously assumed 'subvp_pipe' could be null (see line 860) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn201/dcn201_hwseq.c:250 dcn201_init_hw() warn: variable dereferenced before check 'res_pool->dccg' (see line 227) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hwseq.c:782 dcn20_enable_stream_timing() warn: variable dereferenced before check 'pipe_ctx->stream_res.tg' (see line 698) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_hwseq.c:157 dcn31_init_hw() warn: variable dereferenced before check 'res_pool->dccg' (see line 147) drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:765 dcn32_init_hw() warn: variable dereferenced before check 'res_pool->dccg' (see line 742) Bad indenting: drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn201/dcn201_clk_mgr.c:107 dcn201_update_clocks() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn314/dcn314_clk_mgr.c:716 dcn314_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c:655 dcn315_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn316/dcn316_clk_mgr.c:683 dcn316_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn31/dcn31_clk_mgr.c:726 dcn31_clk_mgr_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2152 dcn10_align_pixel_clocks() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn301/dcn301_resource.c:854 dcn301_hubbub_create() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:618 dcn32_set_output_transfer_func() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hwseq.c:910 dcn32_init_hw() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_mpc.c:306 mpc32_get_shaper_current() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_resource.c:1979 dcn32_resource_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_resource.c:2319 dcn32_resource_construct() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn303/dcn303_fpu.c:205 dcn303_fpu_update_bw_bounding_box() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn303/dcn303_fpu.c:355 dcn303_fpu_init_soc_bounding_box() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/dcn30_fpu.c:185 optc3_fpu_set_vrr_m_const() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/dcn30_fpu.c:355 dcn30_fpu_set_mcif_arb_params() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/dcn30_fpu.c:384 dcn30_fpu_calculate_wm_and_dlg() warn: inconsist
Re: [bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes
On Tue, Jul 26, 2022 at 03:27:54PM +, Simon Ser wrote: > Hi, > > plane->state->fb is NULL iff afb is NULL. There is an early return to > make sure the dereferences don't cause a segfault. > Are you talking about this: if (!plane->state->fb && !old_plane_state->fb) return; Should the && be ||? regards, dan carpenter
[bug report] drm/amd/display: DML changes for DCN32/321
P == 0) {
1623 if (DSCEnable) {
1624 if (MaxLinkBPP < MinDSCBPP)
1625 return BPP_INVALID;
1626 else if (MaxLinkBPP >= MaxDSCBPP)
1627 return MaxDSCBPP;
1628 else
1629 return dml_floor(16.0 * MaxLinkBPP,
1.0) / 16.0;
1630 } else {
--> 1631 if (MaxLinkBPP >= NonDSCBPP3)
1632 return NonDSCBPP3;
1633 else if (MaxLinkBPP >= NonDSCBPP2)
1634 return NonDSCBPP2;
1635 else if (MaxLinkBPP >= NonDSCBPP1)
1636 return NonDSCBPP1;
1637 else if (MaxLinkBPP >= NonDSCBPP0)
1638 return 16.0;
1639 else
1640 return BPP_INVALID;
1641 }
1642 } else {
1643 if (!((DSCEnable == false && (DesiredBPP == NonDSCBPP2
|| DesiredBPP == NonDSCBPP1 ||
1644 DesiredBPP == NonDSCBPP0 || DesiredBPP
== NonDSCBPP3)) ||
1645 (DSCEnable && DesiredBPP >= MinDSCBPP
&& DesiredBPP <= MaxDSCBPP)))
1646 return BPP_INVALID;
1647 else
1648 return DesiredBPP;
1649 }
1650
1651 *RequiredSlots = dml_ceil(DesiredBPP / MaxLinkBPP * 64, 1);
1652
1653 return BPP_INVALID;
1654 } // TruncToValidBPP
regards,
dan carpenter
[PATCH] drm/amd/display: Fix apply_synaptics_fifo_reset_wa()
There is a stray return which accidentally turns the last part of the
function into dead code.
Fixes: 2ca97adccdc9 ("drm/amd/display: Add Synaptics Fifo Reset Workaround")
Signed-off-by: Dan Carpenter
---
>From static analysis. Untested.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index a0154a5f7183..0d200e276e67 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -671,7 +671,6 @@ static void apply_synaptics_fifo_reset_wa(struct drm_dp_aux
*aux)
return;
data[0] |= (1 << 1); // set bit 1 to 1
- return;
if (!execute_synaptics_rc_command(aux, false, 0x31, 4, 0x221198, data))
return;
--
2.35.1
[bug report] drm/amd/display: Synchronize displays with different timings
Hello Vladimir Stempen,
The patch 77a2b7265f20: "drm/amd/display: Synchronize displays with
different timings" from Dec 29, 2020, leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2199
dcn10_enable_vblanks_synchronization() warn: if statement not indented
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c:2207
dcn10_enable_vblanks_synchronization() warn: inconsistent indenting
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.c
2191 DC_SYNC_INFO("Aligning DP DTOs\n");
2192
2193 master = dcn10_align_pixel_clocks(dc, group_size,
grouped_pipes);
2194
2195 DC_SYNC_INFO("Synchronizing VBlanks\n");
2196
2197 if (master >= 0) {
2198 for (i = 0; i < group_size; i++) {
2199 if (i != master &&
!grouped_pipes[i]->stream->has_non_synchronizable_pclk)
2200
grouped_pipes[i]->stream_res.tg->funcs->align_vblanks(
2201 grouped_pipes[master]->stream_res.tg,
2202 grouped_pipes[i]->stream_res.tg,
2203
grouped_pipes[master]->stream->timing.pix_clk_100hz,
2204
grouped_pipes[i]->stream->timing.pix_clk_100hz,
2205
get_clock_divider(grouped_pipes[master], false),
2206 get_clock_divider(grouped_pipes[i],
false));
--> 2207
grouped_pipes[i]->stream->vblank_synchronized = true;
It looks like this is supposed to have curly braces (this code is buggy).
2208 }
2209 grouped_pipes[master]->stream->vblank_synchronized =
true;
2210 DC_SYNC_INFO("Sync complete\n");
2211 }
2212
2213 for (i = 1; i < group_size; i++) {
2214 opp = grouped_pipes[i]->stream_res.opp;
2215 tg = grouped_pipes[i]->stream_res.tg;
2216 tg->funcs->get_otg_active_size(tg, , );
2217 if (opp->funcs->opp_program_dpg_dimensions)
2218 opp->funcs->opp_program_dpg_dimensions(opp,
width, height);
2219 }
2220 }
regards,
dan carpenter
[bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes
Hello Simon Ser,
The patch 03a663673063: "drm/amd/display: use FB pitch to fill
dc_cursor_attributes" from Dec 2, 2020, leads to the following Smatch
static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:1271
handle_cursor_update()
error: we previously assumed 'afb' could be null (see line 1230)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c
1222 void handle_cursor_update(struct drm_plane *plane,
1223 struct drm_plane_state
*old_plane_state)
1224 {
1225 struct amdgpu_device *adev = drm_to_adev(plane->dev);
1226 struct amdgpu_framebuffer *afb =
to_amdgpu_framebuffer(plane->state->fb);
1227 struct drm_crtc *crtc = afb ? plane->state->crtc :
old_plane_state->crtc;
afb is container_of() but it's basically a cast in this case. I would
always prefer to check "plane->state->fb" for NULL instead of the
container_of(), but that's sort of a style debate, I guess. Some people
really like checking the returned pointer and add build time asserts to
ensure that the container_of() is a no-op.
1228 struct dm_crtc_state *crtc_state = crtc ?
to_dm_crtc_state(crtc->state) : NULL;
1229 struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
1230 uint64_t address = afb ? afb->address : 0;
1231 struct dc_cursor_position position = {0};
1232 struct dc_cursor_attributes attributes;
1233 int ret;
1234
1235 if (!plane->state->fb && !old_plane_state->fb)
1236 return;
1237
1238 DC_LOG_CURSOR("%s: crtc_id=%d with size %d to %d\n",
1239 __func__,
1240 amdgpu_crtc->crtc_id,
1241 plane->state->crtc_w,
1242 plane->state->crtc_h);
1243
1244 ret = get_cursor_position(plane, crtc, );
1245 if (ret)
1246 return;
1247
1248 if (!position.enable) {
1249 /* turn off cursor */
1250 if (crtc_state && crtc_state->stream) {
1251 mutex_lock(>dm.dc_lock);
1252
dc_stream_set_cursor_position(crtc_state->stream,
1253 );
1254 mutex_unlock(>dm.dc_lock);
1255 }
1256 return;
1257 }
1258
1259 amdgpu_crtc->cursor_width = plane->state->crtc_w;
1260 amdgpu_crtc->cursor_height = plane->state->crtc_h;
1261
1262 memset(, 0, sizeof(attributes));
1263 attributes.address.high_part = upper_32_bits(address);
1264 attributes.address.low_part = lower_32_bits(address);
1265 attributes.width = plane->state->crtc_w;
1266 attributes.height= plane->state->crtc_h;
1267 attributes.color_format =
CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA;
1268 attributes.rotation_angle= 0;
1269 attributes.attribute_flags.value = 0;
1270
--> 1271 attributes.pitch = afb->base.pitches[0] /
afb->base.format->cpp[0];
^ ^
Unchecked dereferences.
1272
1273 if (crtc_state->stream) {
1274 mutex_lock(>dm.dc_lock);
1275 if
(!dc_stream_set_cursor_attributes(crtc_state->stream,
1276 ))
1277 DRM_ERROR("DC failed to set cursor
attributes\n");
1278
1279 if (!dc_stream_set_cursor_position(crtc_state->stream,
1280))
1281 DRM_ERROR("DC failed to set cursor
position\n");
1282 mutex_unlock(>dm.dc_lock);
1283 }
1284 }
regards,
dan carpenter
[bug report] drm/amd/display: Add dcn3.01 support to DC (v2)
[ It's not clear to me why Smatch is complaining about 2 year old code
but that seems like the buggy commit? ]
Hello Roman Li,
The patch 3a83e4e64bb1: "drm/amd/display: Add dcn3.01 support to DC
(v2)" from Sep 29, 2020, leads to the following Smatch static checker
warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn301/vg_clk_mgr.c:539
find_dcfclk_for_voltage()
error: buffer overflow 'clock_table->DcfClocks' 7 <= 7
drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn301/vg_clk_mgr.c
532 static unsigned int find_dcfclk_for_voltage(const struct vg_dpm_clocks
*clock_table,
533 unsigned int voltage)
534 {
535 int i;
536
537 for (i = 0; i < VG_NUM_SOC_VOLTAGE_LEVELS; i++) {
538 if (clock_table->SocVoltage[i] == voltage)
--> 539 return clock_table->DcfClocks[i];
^
The ->SocVoltage[] array has 8 elements but the ->DcfClocks[] array
only has 7 and the mismatch leads to an out of bounds.
540 }
541
542 ASSERT(0);
543 return 0;
544 }
regards,
dan carpenter
[bug report] drm/amd/display: Initial DC support for Beige Goby
Hello Aurabindo Pillai,
The patch cd6d421e3d1a: "drm/amd/display: Initial DC support for
Beige Goby" from Mar 15, 2021, leads to the following Smatch static
checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn303/dcn303_resource.c:392
dcn303_stream_encoder_create()
error: buffer overflow 'stream_enc_regs' 2 <= 4
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn301/dcn301_resource.c:1024
dcn301_stream_encoder_create() error: buffer overflow 'stream_enc_regs' 4
<= 5
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn303/dcn303_resource.c
366 static struct stream_encoder *dcn303_stream_encoder_create(enum
engine_id eng_id, struct dc_context *ctx)
367 {
368 struct dcn10_stream_encoder *enc1;
369 struct vpg *vpg;
370 struct afmt *afmt;
371 int vpg_inst;
372 int afmt_inst;
373
374 /* Mapping of VPG, AFMT, DME register blocks to DIO block
instance */
375 if (eng_id <= ENGINE_ID_DIGE) {
This tells us that eng_id can be <= 4.
376 vpg_inst = eng_id;
377 afmt_inst = eng_id;
378 } else
379 return NULL;
380
381 enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL);
382 vpg = dcn303_vpg_create(ctx, vpg_inst);
383 afmt = dcn303_afmt_create(ctx, afmt_inst);
384
385 if (!enc1 || !vpg || !afmt) {
386 kfree(enc1);
387 kfree(vpg);
388 kfree(afmt);
389 return NULL;
390 }
391
--> 392 dcn30_dio_stream_encoder_construct(enc1, ctx, ctx->dc_bios,
eng_id, vpg, afmt, _enc_regs[eng_id],
^^^
But anything more than 1 is out of bounds. The dcn301 code is basically
the same.
393 _shift, _mask);
394
395 return >base;
396 }
regards,
dan carpenter
[bug report] drm/amdgpu: create I2S platform devices for Jadeite platform
LUE;
485
486 while (true) {
487 val = cgs_read_register(adev->acp.cgs_device,
mmACP_STATUS);
488 if (val & (u32) 0x1)
489 break;
490 if (--count == 0) {
491 dev_err(>pdev->dev, "Failed to reset
ACP\n");
492 r = -ETIMEDOUT;
493 goto failure;
494 }
495 udelay(100);
496 }
497 /* Deassert the SOFT RESET flags */
498 val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
499 val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
500 cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
501 return 0;
502
503 failure:
504 kfree(i2s_pdata);
505 kfree(adev->acp.acp_res);
506 kfree(adev->acp.acp_cell);
507 kfree(adev->acp.acp_genpd);
508 return r;
509 }
regards,
dan carpenter
[PATCH] drm/amd/display: fix signedness bug in execute_synaptics_rc_command()
The "ret" variable needs to be signed for the error handling to work.
Fixes: 2ca97adccdc9 ("drm/amd/display: Add Synaptics Fifo Reset Workaround")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d66e3cd64ebd..a0154a5f7183 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -569,7 +569,7 @@ static bool execute_synaptics_rc_command(struct drm_dp_aux
*aux,
unsigned char rc_cmd = 0;
unsigned char rc_result = 0xFF;
unsigned char i = 0;
- uint8_t ret = 0;
+ int ret;
if (is_write_cmd) {
// write rc data
--
2.35.1
[bug report] drm/amd/display: Add basic infrastructure for enabling FAMS
Hello Rodrigo Siqueira, This is a semi-automatic email about new static checker warnings. The patch 00fa7f031dd4: "drm/amd/display: Add basic infrastructure for enabling FAMS" from Jun 16, 2022, leads to the following Smatch complaint: drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:311 dc_dmub_srv_p_state_delegate() warn: variable dereferenced before check 'dc' (see line 309) drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c 308 int ramp_up_num_steps = 1; // TODO: Ramp is currently disabled. Reenable it. 309 uint8_t visual_confirm_enabled = dc->debug.visual_confirm == VISUAL_CONFIRM_FAMS; Dereference 310 311 if (dc == NULL) ^^ NULL check is too late 312 return false; 313 regards, dan carpenter
[bug report] drm/amdgpu/mes: ring aggregatged doorbell when mes queue is unmapped
Hello Le Ma,
The patch 5fdbff096edb: "drm/amdgpu/mes: ring aggregatged doorbell
when mes queue is unmapped" from Oct 30, 2020, leads to the following
unpublished Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:8552
gfx_v10_0_ring_set_wptr_gfx()
warn: duplicate check '*is_queue_unmap' (previous on line 8546)
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
8525 static void gfx_v10_0_ring_set_wptr_gfx(struct amdgpu_ring *ring)
8526 {
8527 struct amdgpu_device *adev = ring->adev;
8528 uint32_t *wptr_saved;
8529 uint32_t *is_queue_unmap;
8530 uint64_t aggregated_db_index;
8531 uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_GFX].mqd_size;
8532 uint64_t wptr_tmp;
8533
8534 if (ring->is_mes_queue) {
8535 wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
8536 is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size
+
8537 sizeof(uint32_t));
8538 aggregated_db_index =
8539 amdgpu_mes_get_aggregated_doorbell_index(adev,
8540 AMDGPU_MES_PRIORITY_LEVEL_NORMAL);
8541
8542 wptr_tmp = ring->wptr & ring->buf_mask;
8543 atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
wptr_tmp);
8544 *wptr_saved = wptr_tmp;
8545 /* assume doorbell always being used by mes mapped
queue */
8546 if (*is_queue_unmap) {
^^^
Checked
8547 WDOORBELL64(aggregated_db_index, wptr_tmp);
8548 WDOORBELL64(ring->doorbell_index, wptr_tmp);
8549 } else {
8550 WDOORBELL64(ring->doorbell_index, wptr_tmp);
8551
--> 8552 if (*is_queue_unmap)
^^^
It's possible this is changed inside a function, but it looks suspicious.
8553 WDOORBELL64(aggregated_db_index,
wptr_tmp);
8554 }
8555 } else {
8556 if (ring->use_doorbell) {
8557 /* XXX check if swapping is necessary on BE */
8558 atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
8559 ring->wptr);
8560 WDOORBELL64(ring->doorbell_index, ring->wptr);
8561 } else {
8562 WREG32_SOC15(GC, 0, mmCP_RB0_WPTR,
8563 lower_32_bits(ring->wptr));
8564 WREG32_SOC15(GC, 0, mmCP_RB0_WPTR_HI,
8565 upper_32_bits(ring->wptr));
8566 }
8567 }
8568 }
regards,
dan carpenter
[PATCH] drm/amd/display: Remove unnecessary NULL check in commit_planes_for_stream()
Smatch complains that:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3369
commit_planes_for_stream()
warn: variable dereferenced before check 'stream' (see line 3114)
The 'stream' pointer cannot be NULL and the check can be removed.
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 dc2c59995a19..76f9af2c5e19 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3366,7 +3366,7 @@ static void commit_planes_for_stream(struct dc *dc,
top_pipe_to_program->stream_res.tg,
CRTC_STATE_VACTIVE);
- 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.35.1
Re: [linux-next:master] BUILD REGRESSION 088b9c375534d905a4d337c78db3b3bfbb52c4a0
On Thu, Jul 07, 2022 at 07:02:58AM -0700, Guenter Roeck wrote: > and the NULL > dereferences in the binder driver are at the very least suspicious. The NULL dereferences in binder are just nonsense Sparse annotations. They don't affect runtime. drivers/android/binder.c:1481:19-23: ERROR: from is NULL but dereferenced. drivers/android/binder.c:2920:29-33: ERROR: target_thread is NULL but dereferenced. drivers/android/binder.c:353:25-35: ERROR: node -> proc is NULL but dereferenced. drivers/android/binder.c:4888:16-20: ERROR: t is NULL but dereferenced. regards, dan carpenter
Re: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch
On Tue, Jun 28, 2022 at 06:25:38PM -0500, Ramesh Errabolu wrote: > The patch fixes couple of warnings, as reported by Smatch > a static analyzer > I had forgotten what the warnings were. Could you put that in the commit message or describe the bug somehow? > Signed-off-by: Ramesh Errabolu > Reported-by: Dan Carpenter This needs a Fixes tag as well. regards, dan carpenter
