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
