<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Michal,<br>
</p>
<div class="moz-cite-prefix">On 6/4/2024 3:12 PM, Michal Wajdeczko
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e4a9c72c-ea14-4aad-9020-d7e503baa59e@intel.com">
<pre class="moz-quote-pre" wrap="">
On 04.06.2024 13:02, Nirmoy Das wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">xe_force_wake_get() increments the domain ref regardless of success
or failure so call xe_force_wake_put() even on failure to keep ref
count accurate.
Signed-off-by: Nirmoy Das <a class="moz-txt-link-rfc2396E" href="mailto:nirmoy.das@intel.com"><nirmoy.das@intel.com></a>
---
drivers/gpu/drm/xe/xe_reg_sr.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
index 440ac572f6e5..8fcc08658d89 100644
--- a/drivers/gpu/drm/xe/xe_reg_sr.c
+++ b/drivers/gpu/drm/xe/xe_reg_sr.c
@@ -201,13 +201,11 @@ void xe_reg_sr_apply_mmio(struct xe_reg_sr *sr, struct xe_gt *gt)
xa_for_each(&sr->xa, reg, entry)
apply_one_mmio(gt, entry);
- err = xe_force_wake_put(>->mmio.fw, XE_FORCEWAKE_ALL);
- XE_WARN_ON(err);
-
- return;
-
err_force_wake:
- xe_gt_err(gt, "Failed to apply, err=%d\n", err);
+ if (err)
+ xe_gt_err(gt, "Failed to whitelist %s registers, err=%d\n",
+ sr->name, err);
+ XE_WARN_ON(xe_force_wake_put(>->mmio.fw, XE_FORCEWAKE_ALL));
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
what's the rule whether to WARN about the failed xe_force_wake_put() or
not ? should we also WARN when xe_force_wake_get() failed ? does it make
sense to WARN about put() if get() already failed ?</pre>
</blockquote>
We don't have one yet. <br>
<blockquote type="cite"
cite="mid:e4a9c72c-ea14-4aad-9020-d7e503baa59e@intel.com">
<pre class="moz-quote-pre" wrap="">
maybe simpler solution would be make function xe_force_wake_put() void
as it almost nothing that caller can do and move WARN there if needed ?</pre>
</blockquote>
<p>We have code that does "return <span
style="white-space: pre-wrap">xe_force_wake_put</span>()" So
question is what shall we do <br>
</p>
<p>if <span style="white-space: pre-wrap">xe_force_wake_get() worked but subsequent </span><span
style="white-space: pre-wrap">xe_force_wake_put() fails ?</span></p>
<p><span style="white-space: pre-wrap">
</span></p>
<p><span style="white-space: pre-wrap">Regards,</span></p>
<p><span style="white-space: pre-wrap">Nirmoy
</span></p>
<blockquote type="cite"
cite="mid:e4a9c72c-ea14-4aad-9020-d7e503baa59e@intel.com">
<pre class="moz-quote-pre" wrap="">
what about making the flow more intuitive like this:
bool xe_force_wake_get(fw, d);
void xe_force_wake_put(fw, d);
if (xe_force_wake_get(fw, d)) {
...
xe_force_wake_put(fw, d);
}
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> }
void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
@@ -253,13 +251,11 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
xe_mmio_write32(gt, RING_FORCE_TO_NONPRIV(mmio_base, slot), addr);
}
- err = xe_force_wake_put(>->mmio.fw, XE_FORCEWAKE_ALL);
- XE_WARN_ON(err);
-
- return;
-
err_force_wake:
- drm_err(&xe->drm, "Failed to apply, err=%d\n", err);
+ if (err)
+ xe_gt_err(gt, "Failed to whitelist %s registers, err=%d\n",
+ sr->name, err);
+ XE_WARN_ON(xe_force_wake_put(>->mmio.fw, XE_FORCEWAKE_ALL));
}
/**
</pre>
</blockquote>
</blockquote>
</body>
</html>