<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif;"> <p>The broadcast read was required to attempt to debug a pro-stack problem which I believe the "cache gca config" patches are meant to mitigate. The broadcast write support is required to select CUs for wave readings. <span style="font-size: 12pt;">libdrm performs a broadcast read when reading raster config data so apparently that's a thing.</span></p> <div><br> </div> Since it's a debugfs interface any stability problems a broadcast read may or may not cause isn't really a concern for day to day operations. FWIW I've never yet seen an issue with broadcast reading raster config registers. <div><br> </div> <div>But to put it simply, the patch was part of a series that was blocking other work and there wasn't sufficient cause to NAK it.</div> <div><br> </div> <div>Tom<br> <br> <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Michel Dänzer <michel@daenzer.net><br> <b>Sent:</b> Tuesday, October 18, 2016 23:49<br> <b>To:</b> StDenis, Tom; Christian König<br> <b>Cc:</b> amd-gfx@lists.freedesktop.org<br> <b>Subject:</b> Re: [PATCH] drm/amd/amdgpu: Allow broadcast on debugfs read (v2)</font> <div> </div> </div> </div> <font size="2"><span style="font-size:10pt;"> <div class="PlainText">On 13/10/16 04:20 PM, Michel Dänzer wrote:<br> > On 13/10/16 12:39 AM, StDenis, Tom wrote:<br> >> It comes from amdgpu_query_gpu_info_init()<br> >><br> >><br> >> for (i = 0; i < (int)dev->info.num_shader_engines; i++) {<br> >> unsigned instance = (i << AMDGPU_INFO_MMR_SE_INDEX_SHIFT) |<br> >> (*AMDGPU_INFO_MMR_SH_INDEX_MASK*<<<br> >> AMDGPU_INFO_MMR_SH_INDEX_SHIFT);<br> >><br> >> r = amdgpu_read_mm_registers(dev, 0x263d, 1, instance, 0,<br> >> &dev->info.backend_disable[i]);<br> >><br> >> This effectively reads from 0/* where the kernel adds the instance of *<br> >> so it's 0/*/*. That line was last changed by Alex<br> >><br> >> *0936139536380* (Alex Deucher 2015-04-20 12:04:22 -0400 174) <br> >> (AMDGPU_INFO_MMR_SH_INDEX_MASK <<<br> > <br> > As a side note, following that to the end in the kernel code, I noticed<br> > an interesting minor difference between the AMDGPU_INFO_READ_MMR_REG<br> > functionality used by this code and the debugfs interface:<br> > <br> > With AMDGPU_INFO_READ_MMR_REG, the effect is that<br> > amdgpu_asic_read_register() doesn't call amdgpu_gfx_select_se_sh() at<br> > all before reading the register, so the read is performed from whichever<br> > SH instance is currently selected.<br> > <br> > Whereas with this patch, amdgpu_debugfs_regs_read() calls<br> > amdgpu_gfx_select_se_sh(adev, se_bank, 0xFFFFFFFF, instance_bank) before<br> > the register read, which translates to only the SH_BROADCAST_WRITES bit<br> > being set for the SH instance index.<br> > <br> > The end result should be the same though, since<br> > amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff) is<br> > normally called after every register read.<br> > <br> > <br> >> I still don't get why this is a reason to hit pause on the patch(es)<br> >> though.<br> > <br> > At the very least, it should be documented in an appropriate place<br> > (commit log and/or code, or any other place where the debugfs interface<br> > semantics are documented) what actually happens when passing all ones<br> > for the SE/SH index. Does the hardware ignore the *_BROADCAST_WRITES bit<br> > for reads, so they're performed from instance 0, or does it combine the<br> > values from all instances with logical and/or?<br> <br> I'm not sure how to interpret the fact that this patch has landed<br> without any changes or followups.<br> <br> FWIW, I'm still interested in (pointers to) information about what the<br> libdrm_amdgpu code above expects and what the hardware does for reads<br> with the broadcast bit enabled, from anyone.<br> <br> <br> -- <br> Earthling Michel Dänzer | <a href="http://www.amd.com" id="LPlnk758282" previewremoved="true"> http://www.amd.com</a> <div id="LPBorder_GT_14768734048770.8775835774995799" style="margin-bottom: 20px; overflow: auto; width: 100%; text-indent: 0px;"> <table id="LPContainer_14768734048740.6196712470071157" cellspacing="0" style="width: 90%; background-color: rgb(255, 255, 255); position: relative; overflow: auto; padding-top: 20px; padding-bottom: 20px; margin-top: 20px; border-top: 1px dotted rgb(200, 200, 200); border-bottom: 1px dotted rgb(200, 200, 200);"> <tbody> <tr valign="top" style="border-spacing: 0px;"> <td id="TextCell_14768734048750.8107831176369602" colspan="2" style="vertical-align: top; position: relative; padding: 0px; display: table-cell;"> <div id="LPRemovePreviewContainer_14768734048760.36270012816875474"></div> <div id="LPTitle_14768734048760.2595286131757666" style="top: 0px; color: rgb(59, 87, 119); font-weight: normal; font-size: 21px; font-family: wf_segoe-ui_light, "Segoe UI Light", "Segoe WP Light", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; line-height: 21px;"> <a id="LPUrlAnchor_14768734048760.7459485059600515" href="http://www.amd.com/" target="_blank" style="text-decoration: none;">Graphics, Processors and Immersive VR Solutions | AMD </a></div> <div id="LPMetadata_14768734048760.8916323305065981" style="margin: 10px 0px 16px; color: rgb(102, 102, 102); font-weight: normal; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 14px;"> www.amd.com</div> <div id="LPDescription_14768734048760.8199608896286328" style="display: block; color: rgb(102, 102, 102); font-weight: normal; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 20px; max-height: 100px; overflow: hidden;"> Explore a wide range of innovative next generation computing processors, graphics, and Immersive VR solutions by Advanced Micro Devices (AMD). Visit AMD.com now!</div> </td> </tr> </tbody> </table> </div> <br> <br> Libre software enthusiast | Mesa and X developer<br> </div> </span></font></div> </div> </div> </body> </html>