<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72"> <div class="WordSection1"> [AMD Public Use]<o:p></o:p> <o:p> </o:p> Yes, John, that concern is cleared after I look into the code.<o:p></o:p> <o:p> </o:p> One more issue is, it’s better that function get_umc_reg_offset is one static inline function? With this problem fixed, the patch is: Reviewed-by: Guchun Chen <guchun.chen@amd.com><o:p></o:p> <o:p> </o:p> uint32_t get_umc_reg_offset(struct amdgpu_device *adev,<o:p></o:p> + uint32_t umc_inst,<o:p></o:p> + uint32_t ch_inst)<o:p></o:p> <div> <o:p> </o:p> Regards,<o:p></o:p> Guchun<o:p></o:p> </div> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Clements, John <John.Clements@amd.com> Sent: Friday, January 3, 2020 10:58 AM To: Chen, Guchun <Guchun.Chen@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; amd-gfx@lists.freedesktop.org; Zhou1, Tao <Tao.Zhou1@amd.com> Subject: RE: [PATCH] drm/amdgpu: resolved bug in UMC 6 error counter query<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Public Use]<o:p></o:p> <o:p> </o:p> Hello GuChun/Hawking,<o:p></o:p> <o:p> </o:p> Thank you for your feedback, I have updated the patch with the following amendments:<o:p></o:p> <ul style="margin-top:0in" type="disc"> <li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3">Remove +#define UMC_REG_OFFSET (I forgot to remove this in original patch, I prefer the function over the macro)<o:p></o:p></li><li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3">Updated the coding style of the braces in the for loops to have the starting brace on the same line as the for loop declaration<o:p></o:p></li></ul> <o:p> </o:p> GuChun,<o:p></o:p> For your concern about the umc_v6_1_query_ras_error_count, in the UE/CE error counter register reading, the local SW error counters can only be incremented and not cleared throughout the iteration over the UMC error counter registers.<o:p></o:p> <o:p> </o:p> Thank you,<o:p></o:p> John Clements<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>> Sent: Friday, January 3, 2020 9:07 AM To: Zhang, Hawking <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>>; Clements, John <<a href="mailto:John.Clements@amd.com">John.Clements@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Zhou1, Tao <<a href="mailto:Tao.Zhou1@amd.com">Tao.Zhou1@amd.com</a>> Subject: RE: [PATCH] drm/amdgpu: resolved bug in UMC 6 error counter query<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Public Use]<o:p></o:p> <o:p> </o:p> +#define UMC_REG_OFFSET(adev, ch_inst, umc_inst) ((adev)->umc.channel_offs * (ch_inst) + UMC_6_INST_DIST*(umc_inst))<o:p></o:p> Coding style problem, miss blank space around last “*”.<o:p></o:p> <o:p> </o:p> <div> + for (umc_inst = 0; umc_inst < adev->umc.umc_inst_num; umc_inst++)<o:p></o:p> + {<o:p></o:p> Another coding style problem. “{” should follow closely at the same line, not starting at one new line.<o:p></o:p> <o:p> </o:p> Thirdly, in umc_v6_1_query_ras_error_count, we use dual loops for query error counter for all UMC channels. But we always use the same variable to do the query. So the value will be overwritten by new one? Then we will miss former error counters if there are. Correct?<o:p></o:p> <o:p> </o:p> Regards,<o:p></o:p> Guchun<o:p></o:p> </div> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: amd-gfx <<a href="mailto:amd-gfx-bounces@lists.freedesktop.org">amd-gfx-bounces@lists.freedesktop.org</a>> On Behalf Of Zhang, Hawking Sent: Thursday, January 2, 2020 8:38 PM To: Clements, John <<a href="mailto:John.Clements@amd.com">John.Clements@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Zhou1, Tao <<a href="mailto:Tao.Zhou1@amd.com">Tao.Zhou1@amd.com</a>> Subject: RE: [PATCH] drm/amdgpu: resolved bug in UMC 6 error counter query<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only - Internal Distribution Only]<o:p></o:p> <o:p> </o:p> UMC_REG_OFFSET(adev, ch_inst, umc_inst) and the function get_umc_reg_offset actually do the same thing? I guess you just want to keep either of them, right?<o:p></o:p> <o:p> </o:p> Regards, Hawking<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Clements, John <<a href="mailto:John.Clements@amd.com">John.Clements@amd.com</a>> Sent: Thursday, January 2, 2020 18:31 To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; Zhang, Hawking <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>>; Zhou1, Tao <<a href="mailto:Tao.Zhou1@amd.com">Tao.Zhou1@amd.com</a>> Subject: [PATCH] drm/amdgpu: resolved bug in UMC 6 error counter query<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only - Internal Distribution Only]<o:p></o:p> <o:p> </o:p> Added patch to resolve following issue where error counter detection was not iterating over all UMC instances/channels.<o:p></o:p> Removed support for accessing UMC error counters via MMIO.<o:p></o:p> <o:p> </o:p> Thank you,<o:p></o:p> John Clements<o:p></o:p> </div> </body> </html>