On 2021-10-21 12:49, Russell, Kent
wrote:
[AMD Official Use Only]-----Original Message----- From: Tuikov, Luben <Luben.Tuikov@xxxxxxx> Sent: Thursday, October 21, 2021 12:47 PM To: Russell, Kent <Kent.Russell@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Joshi, Mukul <Mukul.Joshi@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold On 2021-10-21 12:42, Russell, Kent wrote:[AMD Official Use Only]-----Original Message----- From: Tuikov, Luben <Luben.Tuikov@xxxxxxx> Sent: Thursday, October 21, 2021 12:21 PM To: Russell, Kent <Kent.Russell@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Joshi, Mukul <Mukul.Joshi@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Tuikov, Luben <Luben.Tuikov@xxxxxxx> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring badpagethreshold On 2021-10-21 11:57, Kent Russell wrote:When a GPU hits the bad_page_threshold, it will not be initialized by the amdgpu driver. This means that the table cannot be cleared, nor can information gathering be performed (getting serial number, BDF, etc). If the bad_page_threshold kernel parameter is set to -2, continue to initialize the GPU, while printing a warning to dmesg that this action has been done Cc: Luben Tuikov <luben.tuikov@xxxxxxx> Cc: Mukul Joshi <Mukul.Joshi@xxxxxxx> Signed-off-by: Kent Russell <kent.russell@xxxxxxx> Acked-by: Felix Kuehling <Felix.Kuehling@xxxxxxx> Reviewed-by: Luben Tuikov <luben.tuikov@xxxxxxx> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.hb/drivers/gpu/drm/amd/amdgpu/amdgpu.hindex d58e37fd01f4..b85b67a88a3d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info; extern int amdgpu_ras_enable; extern uint amdgpu_ras_mask; extern int amdgpu_bad_page_threshold; +extern bool amdgpu_ignore_bad_page_threshold; extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer; extern int amdgpu_async_gfx_ring; extern int amdgpu_mcbp; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.cb/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.cindex 96bd63aeeddd..eee3cf874e7a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -877,7 +877,7 @@ module_param_named(reset_method,amdgpu_reset_method,int, 0444);* result in the GPU entering bad status when the number of total * faulty pages by ECC exceeds the threshold value. */ -MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(defaultvalue), 0 = disable bad page retirement)");+MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(defaultvalue), 0 = disable bad page retirement, -2 = ignore bad page threshold)");module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int,0444);MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want tosetup(8 if set to greater than 8 or less than 0, only affect gfx 8+)");diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.cb/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.cindex ce5089216474..bd6ed43b0df2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c @@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(structamdgpu_ras_eeprom_control *control,res = amdgpu_ras_eeprom_correct_header_tag(control, RAS_TABLE_HDR_VAL); } else { - *exceed_err_limit = true; - dev_err(adev->dev, - "RAS records:%d exceed threshold:%d, " - "GPU will not be initialized. Replace this GPU or increase thethreshold",+ dev_err(adev->dev, "RAS records:%d exceed threshold:%d", control->ras_num_recs, ras->bad_page_cnt_threshold);I thought this would all go in a single set of patches. I wasn't aware a singleton patchwentin already which changed just this line--this change was always a part of a patch set.Ah sorry. When you reviewed the original patch2 clarifying the message, I merged it andthen re-submitted the remaining 3 (which pared down to 2) for review. Sorry for the confusion, I was trying to minimize the number of moving parts. Admittedly, now you have 3 patches, one singleton and two coming in. Would've probably be best to submit only the current two. No worries for now--for the future.Thanks. For the most part, all of my previous multi-patch sets have been all-or-nothing. This is the first time that a patch in the middle got an RB before the rest, so I did the wrong thing and merged it while the rest was still moving. Thanks for bearing with me!
Yeah, sometimes we R-B some patches in a set, but we don't break off or submit them until they've all (the whole set has) gotten an R-B. We just apply the R-B to the patches which have gotten it and carry them around the set, and repost as you did with your v3 patch set.
No big deal at the moment, but having a single consistent set is preferable--for the future. :-)
Regards,
Luben
KentRegards, LubenKentRegards, Luben+ if (amdgpu_bad_page_threshold == -2) { + dev_warn(adev->dev, "GPU will be initialized due tobad_page_threshold = -2.");+ res = 0; + } else { + *exceed_err_limit = true; + dev_err(adev->dev, "GPU will not be initialized. Replace thisGPU or increase the threshold.");+ } } } else { DRM_INFO("Creating a new EEPROM table");
