<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <p style="font-family:Arial;font-size:11pt;color:#0078D7;margin:5pt;" align="Left"> [AMD Official Use Only - Internal Distribution Only]<br> </p> <br> <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> yep, you are right...</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> I'll make those changes.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> David</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Christian König <ckoenig.leichtzumerken@gmail.com><br> <b>Sent:</b> Tuesday, May 11, 2021 11:56 PM<br> <b>To:</b> Nieto, David M <David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Subject:</b> Re: [PATCH 2/2] drm/amdgpu: fix fence calculation</font> <div> </div> </div> <div>And BTW <span style="">amdgpu_ctx_fence_time() should probably be static.<br> <br> Christian.<br> </span><br> <div class="x_moz-cite-prefix">Am 12.05.21 um 08:55 schrieb Christian König:<br> </div> <blockquote type="cite">In this case <span style="color:rgb(0,0,0); font-size:14.6667px; background-color:rgb(255,255,255); display:inline!important"> amdgpu_ctx_fence_time should probably be changed to initialize the variable itself.<br> <br> That is really bad coding style otherwise.<br> <br> Christian.<br> </span><br> <div class="x_moz-cite-prefix">Am 11.05.21 um 20:14 schrieb Nieto, David M:<br> </div> <blockquote type="cite"><style type="text/css" style="display:none">  </style> <p align="Left" style="font-family:Arial; font-size:11pt; color:#0078D7; margin:5pt"> [AMD Official Use Only - Internal Distribution Only]<br> </p> <br> <div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)"> The local variables need to be initialized to zero, since <span style="">amdgpu_ctx_fence_time accumulates and does not initialize</span></div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)"> <span style=""><br> </span></div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)"> <span style="">David</span></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> Christian König <a class="x_moz-txt-link-rfc2396E" href="mailto:ckoenig.leichtzumerken@gmail.com"> <ckoenig.leichtzumerken@gmail.com></a><br> <b>Sent:</b> Tuesday, May 11, 2021 12:53 AM<br> <b>To:</b> Nieto, David M <a class="x_moz-txt-link-rfc2396E" href="mailto:David.Nieto@amd.com"> <David.Nieto@amd.com></a>; <a class="x_moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> <a class="x_moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"> <amd-gfx@lists.freedesktop.org></a><br> <b>Subject:</b> Re: [PATCH 2/2] drm/amdgpu: fix fence calculation</font> <div> </div> </div> <div class="x_BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="x_PlainText">Am 10.05.21 um 22:29 schrieb David M Nieto:<br> > The proper metric for fence utilization over several<br> > contexts is an harmonic mean, but such calculation is<br> > prohibitive in kernel space, so the code approximates it.<br> ><br> > Because the approximation diverges when one context has a<br> > very small ratio compared with the other context, this change<br> > filter out ratios smaller that 0.01%<br> ><br> > Signed-off-by: David M Nieto <a class="x_moz-txt-link-rfc2396E" href="mailto:david.nieto@amd.com"> <david.nieto@amd.com></a><br> > Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b<br> > ---<br> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-<br> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +<br> > 2 files changed, 13 insertions(+), 1 deletion(-)<br> ><br> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c<br> > index 9036c93b4a0c..89ee464b9424 100644<br> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c<br> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c<br> > @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,<br> > struct amdgpu_ctx_entity *centity;<br> > ktime_t total = 0, max = 0;<br> > <br> > +<br> <br> Unrelated white space change.<br> <br> > if (idx >= AMDGPU_MAX_ENTITY_NUM)<br> > return 0;<br> > idp = &mgr->ctx_handles;<br> > mutex_lock(&mgr->lock);<br> > idr_for_each_entry(idp, ctx, id) {<br> > + ktime_t ttotal = tmax = ktime_set(0, 0);<br> <br> There should be a blank line between decleration and code and please <br> don't initialize local variables if it isn't necessary.<br> <br> Christian.<br> <br> > if (!ctx->entities[hwip][idx])<br> > continue;<br> > <br> > centity = ctx->entities[hwip][idx];<br> > - amdgpu_ctx_fence_time(ctx, centity, &total, &max);<br> > + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);<br> > +<br> > + /* Harmonic mean approximation diverges for very small<br> > + * values. If ratio < 0.01% ignore<br> > + */<br> > + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))<br> > + continue;<br> > +<br> > + total = ktime_add(total, ttotal);<br> > + max = ktime_after(tmax, max) ? tmax : max;<br> > }<br> > <br> > mutex_unlock(&mgr->lock);<br> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h<br> > index 10dcf59a5c6b..3541dfb059ec 100644<br> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h<br> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h<br> > @@ -30,6 +30,7 @@ struct drm_file;<br> > struct amdgpu_fpriv;<br> > <br> > #define AMDGPU_MAX_ENTITY_NUM 4<br> > +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)<br> > <br> > struct amdgpu_ctx_entity {<br> > uint64_t sequence;<br> <br> </div> </span></font></div> </div> </blockquote> <br> </blockquote> <br> </div> </div> </body> </html>