Re: [bug report] drm/amdgpu: add ring buffer information in devcoredump
The static checker is just complaining about NULL checking that doesn't make sense. It raises the question, can the pointer be NULL or not? Based on your comments and from reviewing the code, I do not think it can be NULL. Thus the correct thing is to remove the unnecessary NULL check. regards, dan carpenter
[PATCH] drm/amd/display: delete unnecessary check in dcn35_set_long_vblank()
"timing" is "&pipe_ctx[i]->stream->timing" where ->timing is not the first struct member of ->stream. So it's the address which points into the middle of a struct. It can't be NULL so delete the NULL check. Signed-off-by: Dan Carpenter --- drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c index 2e8ec58a16eb..87cfd9f1cec9 100644 --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c @@ -1411,10 +1411,7 @@ void dcn35_set_long_vblank(struct pipe_ctx **pipe_ctx, if (pipe_ctx[i]->stream) { struct dc_crtc_timing *timing = &pipe_ctx[i]->stream->timing; - if (timing) - params.vertical_blank_start = timing->v_total - timing->v_front_porch; - else - params.vertical_blank_start = 0; + params.vertical_blank_start = timing->v_total - timing->v_front_porch; if ((pipe_ctx[i]->stream_res.tg != NULL) && pipe_ctx[i]->stream_res.tg->funcs && pipe_ctx[i]->stream_res.tg->funcs->set_long_vtotal) -- 2.43.0
[PATCH] drm/amdgpu: Fix truncation issues in smu_v13_0_init_microcode
Fixes the below with gcc W=1: drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0.c: In function ‘smu_v13_0_init_microcode’: drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0.c:108:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] 108 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix); |^~ drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0.c:108:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 30 108 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix); | ^ Cc: Alex Deucher Cc: Christian König Signed-off-by: Srinivasan Shanmugam Suggested-by: Lijo Lazar --- drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c index 48170bb5112e..ce16f2a08a47 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -93,7 +93,7 @@ int smu_v13_0_init_microcode(struct smu_context *smu) { struct amdgpu_device *adev = smu->adev; char fw_name[30]; - char ucode_prefix[30]; + char ucode_prefix[15]; int err = 0; const struct smc_firmware_header_v1_0 *hdr; const struct common_firmware_header *header; -- 2.34.1
RE: [PATCH] drm/amdgpu: trigger flr_work if reading pf2vf data failed
[AMD Official Use Only - General] > -Original Message- > From: amd-gfx On Behalf Of Lazar, > Lijo > Sent: Friday, March 15, 2024 6:47 AM > To: Luo, Zhigang ; amd-gfx@lists.freedesktop.org > Cc: Zhang, Hawking ; Saye, Sashank > ; Chan, Hing Pong > Subject: Re: [PATCH] drm/amdgpu: trigger flr_work if reading pf2vf data failed > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > On 3/14/2024 10:24 PM, Zhigang Luo wrote: > > if reading pf2vf data failed 5 times continuously, it means something > > is wrong. Need to trigger flr_work to recover the issue. > > > > also use dev_err to print the error message to get which device has > > issue and add warning message if waiting IDH_FLR_NOTIFICATION_CMPL > > timeout. > > > > Signed-off-by: Zhigang Luo > > Change-Id: Ia7ce934d0c3068ad3934715c14bbffdfcbafc4c2 > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++ > > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 29 ++- > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 1 + > > drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 2 ++ > > drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c | 2 ++ > > 5 files changed, 39 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index 1e9454e6e4cb..9bb04a56d020 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -143,6 +143,8 @@ const char *amdgpu_asic_name[] = { > > "LAST", > > }; > > > > +static inline void amdgpu_device_stop_pending_resets(struct > > +amdgpu_device *adev); > > + > > /** > > * DOC: pcie_replay_count > > * > > @@ -4972,6 +4974,8 @@ static int amdgpu_device_reset_sriov(struct > > amdgpu_device *adev, > > retry: > > amdgpu_amdkfd_pre_reset(adev); > > > > + amdgpu_device_stop_pending_resets(adev); > > + > > if (from_hypervisor) > > r = amdgpu_virt_request_full_gpu(adev, true); > > else > > @@ -5689,11 +5693,12 @@ int amdgpu_device_gpu_recover(struct > amdgpu_device *adev, > > tmp_adev->asic_reset_res = r; > > } > > > > - /* > > - * Drop all pending non scheduler resets. Scheduler resets > > - * were already dropped during drm_sched_stop > > - */ > > - amdgpu_device_stop_pending_resets(tmp_adev); > > + if (!amdgpu_sriov_vf(tmp_adev)) > > + /* > > + * Drop all pending non scheduler resets. Scheduler > > resets > > + * were already dropped during drm_sched_stop > > + */ > > + amdgpu_device_stop_pending_resets(tmp_adev); > > } > > > > For better consistency - you may make this path common for both VF and > bare metal. Call this right before "skip_hw_reset" after a successful reset. > All > pending reset jobs submitted may be cancelled at that point once a succesful > reset of the device/hive is completed. > > Thanks, > Lijo > > > /* Actual ASIC resets if needed.*/ diff --git > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > > index 7a4eae36778a..4e8364a46d4e 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > > @@ -32,6 +32,7 @@ > > > > #include "amdgpu.h" > > #include "amdgpu_ras.h" > > +#include "amdgpu_reset.h" > > #include "vi.h" > > #include "soc15.h" > > #include "nv.h" > > @@ -424,7 +425,7 @@ static int amdgpu_virt_read_pf2vf_data(struct > amdgpu_device *adev) > > return -EINVAL; > > > > if (pf2vf_info->size > 1024) { > > - DRM_ERROR("invalid pf2vf message size\n"); > > + dev_err(adev->dev, "invalid pf2vf message size: 0x%x\n", > > + pf2vf_info->size); > > return -EINVAL; > > } > > > > @@ -435,7 +436,9 @@ static int amdgpu_virt_read_pf2vf_data(struct > amdgpu_device *adev) > > adev->virt.fw_reserve.p_pf2vf, pf2vf_info->size, > > adev->virt.fw_reserve.checksum_key, checksum); > > if (checksum != checkval) { > > - DRM_ERROR("invalid pf2vf message\n"); > > + dev_err(adev->dev, > > + "invalid pf2vf message: header checksum=0x%x > > calculated > checksum=0x%x\n", > > + checksum, checkval); > > return -EINVAL; > > } > > > > @@ -449,7 +452,9 @@ static int amdgpu_virt_read_pf2vf_data(struct > amdgpu_device *adev) > > adev->virt.fw_reserve.p_pf2vf, pf2vf_info->size, > > 0, checksum); > > if (checksum != checkval) { > > - DRM_ERROR("invalid pf2vf message\n"); > > +
Re: [PATCH 1/2] drm/amd/display: Introduce overlay cursor mode
Hi, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.8 next-20240315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/sunpeng-li-amd-com/drm-amd-display-Introduce-overlay-cursor-mode/20240316-011404 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20240315170959.165505-2-sunpeng.li%40amd.com patch subject: [PATCH 1/2] drm/amd/display: Introduce overlay cursor mode config: loongarch-defconfig (https://download.01.org/0day-ci/archive/20240316/202403161600.6kspdesj-...@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403161600.6kspdesj-...@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 | Closes: https://lore.kernel.org/oe-kbuild-all/202403161600.6kspdesj-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:10639: warning: >> This comment starts with '/**', but isn't a kernel-doc comment. Refer >> Documentation/doc-guide/kernel-doc.rst * Set whether the cursor should be enabled in native mode, or overlay mode, on vim +10639 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c 10637 10638 /** 10639 * Set whether the cursor should be enabled in native mode, or overlay mode, on 10640 * the dm_crtc_state. 10641 * 10642 * The cursor should be enabled in overlay mode if the immediate underlying 10643 * plane contains a video format. 10644 * 10645 * Since zpos info is required, drm_atomic_normalize_zpos must be called before 10646 * calling this function. 10647 */ 10648 static int dm_crtc_set_cursor_mode(struct drm_atomic_state *state, 10649 struct dm_crtc_state *dm_crtc_state) 10650 { 10651 struct drm_plane_state *plane_state, *old_plane_state, *target_plane_state; 10652 struct drm_crtc_state *crtc_state = &dm_crtc_state->base; 10653 struct drm_plane *plane; 10654 bool consider_mode_change = false; 10655 bool cursor_changed = false; 10656 unsigned int target_zpos; 10657 unsigned int cursor_zpos; 10658 int i; 10659 10660 /* 10661 * Cursor mode can change if a plane's format changes, is 10662 * enabled/disabled, or z-order changes. 10663 */ 10664 for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) { 10665 10666 /* Only care about planes on this CRTC */ 10667 if ((drm_plane_mask(plane) & crtc_state->plane_mask) == 0) 10668 continue; 10669 10670 if (plane->type == DRM_PLANE_TYPE_CURSOR) 10671 cursor_changed = true; 10672 10673 if (drm_atomic_plane_enabling(old_plane_state, plane_state) || 10674 drm_atomic_plane_disabling(old_plane_state, plane_state) || 10675 old_plane_state->fb->format != plane_state->fb->format) { 10676 consider_mode_change = true; 10677 break; 10678 } 10679 } 10680 10681 if (!consider_mode_change && !crtc_state->zpos_changed) { 10682 return 0; 10683 } 10684 10685 /* 10686 * If no cursor change on this CRTC, and not enabled on this CRTC, then 10687 * no need to set cursor mode. This avoids needlessly locking the cursor 10688 * state. 10689 */ 10690 if (!cursor_changed && 10691 !(drm_plane_mask(crtc_state->crtc->cursor) & crtc_state->plane_mask)) { 10692 return 0; 10693 } 10694 10695 plane_state = drm_atomic_get_plane_state(state, 10696 crtc_state->crtc->cursor); 10697 if (IS_ERR(plane_state)) 10698 return PTR_ERR(plane_state); 10699 10700 /* Cursor is disabled */ 10701 if (!plane_state->fb) 10702 return 0; 10703 10704 cursor_zpos = plane_state->normalized_zpos; 10705 10706