<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: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=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:"Consolas",serif;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal">This looks reasonable to me.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Reviewed-by: Michael J. Ruhl <a href="mailto:michael.j.ruhl@intel.com">
michael.j.ruhl@intel.com</a><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">M<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Dani Liberman <dliberman@habana.ai> <br>
<b>Sent:</b> Tuesday, October 31, 2023 5:10 PM<br>
<b>To:</b> Ruhl, Michael J <michael.j.ruhl@intel.com>; intel-xe@lists.freedesktop.org<br>
<b>Subject:</b> Re: [Intel-xe] [PATCH v2] drm/xe: use variable instead of multiple function calls<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 31/10/2023 22:09, Ruhl, Michael J wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>-----Original Message-----<o:p></o:p></pre>
<pre>From: Intel-xe <a href="mailto:intel-xe-bounces@lists.freedesktop.org"><intel-xe-bounces@lists.freedesktop.org></a> On Behalf Of Dani<o:p></o:p></pre>
<pre>Liberman<o:p></o:p></pre>
<pre>Sent: Tuesday, October 31, 2023 1:12 PM<o:p></o:p></pre>
<pre>To: <a href="mailto:intel-xe@lists.freedesktop.org">intel-xe@lists.freedesktop.org</a><o:p></o:p></pre>
<pre>Subject: [Intel-xe] [PATCH v2] drm/xe: use variable instead of multiple function<o:p></o:p></pre>
<pre>calls<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Using function calls with negative logic was a bit confusing,<o:p></o:p></pre>
<pre>positive logic is more readable.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>v2:<o:p></o:p></pre>
<pre>- Update commit message<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Cc: Matthew Brost <a href="mailto:matthew.brost@intel.com"><matthew.brost@intel.com></a><o:p></o:p></pre>
<pre>Cc: Ville Syrjala <a href="mailto:ville.syrjala@linux.intel.com"><ville.syrjala@linux.intel.com></a><o:p></o:p></pre>
<pre>Signed-off-by: Dani Liberman <a href="mailto:dliberman@habana.ai"><dliberman@habana.ai></a><o:p></o:p></pre>
<pre>Reviewed-by: Matthew Brost <a href="mailto:matthew.brost@intel.com"><matthew.brost@intel.com></a><o:p></o:p></pre>
<pre>---<o:p></o:p></pre>
<pre>drivers/gpu/drm/xe/xe_exec.c | 15 ++++++++-------<o:p></o:p></pre>
<pre>1 file changed, 8 insertions(+), 7 deletions(-)<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c<o:p></o:p></pre>
<pre>index 28e84a0bbeb0..2de6c2c05078 100644<o:p></o:p></pre>
<pre>--- a/drivers/gpu/drm/xe/xe_exec.c<o:p></o:p></pre>
<pre>+++ b/drivers/gpu/drm/xe/xe_exec.c<o:p></o:p></pre>
<pre>@@ -145,7 +145,7 @@ int xe_exec_ioctl(struct drm_device *dev, void *data,<o:p></o:p></pre>
<pre>struct drm_file *file)<o:p></o:p></pre>
<pre> struct xe_sched_job *job;<o:p></o:p></pre>
<pre> struct dma_fence *rebind_fence;<o:p></o:p></pre>
<pre> struct xe_vm *vm;<o:p></o:p></pre>
<pre>- bool write_locked;<o:p></o:p></pre>
<pre>+ bool write_locked, vm_with_dma_fences;<o:p></o:p></pre>
<pre> ktime_t end = 0;<o:p></o:p></pre>
<pre> int err = 0;<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>@@ -196,8 +196,9 @@ int xe_exec_ioctl(struct drm_device *dev, void *data,<o:p></o:p></pre>
<pre>struct drm_file *file)<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>+ vm_with_dma_fences = !xe_vm_no_dma_fences(vm);<o:p></o:p></pre>
<pre>retry:<o:p></o:p></pre>
</blockquote>
<pre><o:p> </o:p></pre>
<pre>Are you 100% certain that this value will never change on retry?<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>M<o:p></o:p></pre>
</blockquote>
<pre>It depends on vm flags that are getting set on vm create, so yes.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Dani<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>- if (!xe_vm_no_dma_fences(vm) && xe_vm_userptr_check_repin(vm)) {<o:p></o:p></pre>
<pre>+ if (vm_with_dma_fences && xe_vm_userptr_check_repin(vm)) {<o:p></o:p></pre>
<pre> err = down_write_killable(&vm->lock);<o:p></o:p></pre>
<pre> write_locked = true;<o:p></o:p></pre>
<pre> } else {<o:p></o:p></pre>
<pre>@@ -279,7 +280,7 @@ int xe_exec_ioctl(struct drm_device *dev, void *data,<o:p></o:p></pre>
<pre>struct drm_file *file)<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> /* Wait behind munmap style rebinds */<o:p></o:p></pre>
<pre>- if (!xe_vm_no_dma_fences(vm)) {<o:p></o:p></pre>
<pre>+ if (vm_with_dma_fences) {<o:p></o:p></pre>
<pre> err = drm_sched_job_add_resv_dependencies(&job->drm,<o:p></o:p></pre>
<pre> &vm->resv,<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>DMA_RESV_USAGE_KERNEL);<o:p></o:p></pre>
<pre>@@ -292,7 +293,7 @@ int xe_exec_ioctl(struct drm_device *dev, void *data,<o:p></o:p></pre>
<pre>struct drm_file *file)<o:p></o:p></pre>
<pre> if (err)<o:p></o:p></pre>
<pre> goto err_put_job;<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>- if (!xe_vm_no_dma_fences(vm)) {<o:p></o:p></pre>
<pre>+ if (vm_with_dma_fences) {<o:p></o:p></pre>
<pre> err = down_read_interruptible(&vm->userptr.notifier_lock);<o:p></o:p></pre>
<pre> if (err)<o:p></o:p></pre>
<pre> goto err_put_job;<o:p></o:p></pre>
<pre>@@ -307,7 +308,7 @@ int xe_exec_ioctl(struct drm_device *dev, void *data,<o:p></o:p></pre>
<pre>struct drm_file *file)<o:p></o:p></pre>
<pre> * the job and let the DRM scheduler / backend clean up the job.<o:p></o:p></pre>
<pre> */<o:p></o:p></pre>
<pre> xe_sched_job_arm(job);<o:p></o:p></pre>
<pre>- if (!xe_vm_no_dma_fences(vm)) {<o:p></o:p></pre>
<pre>+ if (vm_with_dma_fences) {<o:p></o:p></pre>
<pre> /* Block userptr invalidations / BO eviction */<o:p></o:p></pre>
<pre> dma_resv_add_fence(&vm->resv,<o:p></o:p></pre>
<pre> &job->drm.s_fence->finished,<o:p></o:p></pre>
<pre>@@ -330,14 +331,14 @@ int xe_exec_ioctl(struct drm_device *dev, void<o:p></o:p></pre>
<pre>*data, struct drm_file *file)<o:p></o:p></pre>
<pre> xe_sched_job_push(job);<o:p></o:p></pre>
<pre> xe_vm_reactivate_rebind(vm);<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>- if (!err && !xe_vm_no_dma_fences(vm)) {<o:p></o:p></pre>
<pre>+ if (!err && vm_with_dma_fences) {<o:p></o:p></pre>
<pre> spin_lock(&xe->ttm.lru_lock);<o:p></o:p></pre>
<pre> ttm_lru_bulk_move_tail(&vm->lru_bulk_move);<o:p></o:p></pre>
<pre> spin_unlock(&xe->ttm.lru_lock);<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>err_repin:<o:p></o:p></pre>
<pre>- if (!xe_vm_no_dma_fences(vm))<o:p></o:p></pre>
<pre>+ if (vm_with_dma_fences)<o:p></o:p></pre>
<pre> up_read(&vm->userptr.notifier_lock);<o:p></o:p></pre>
<pre>err_put_job:<o:p></o:p></pre>
<pre> if (err)<o:p></o:p></pre>
<pre>--<o:p></o:p></pre>
<pre>2.34.1<o:p></o:p></pre>
</blockquote>
<pre><o:p> </o:p></pre>
</blockquote>
<p><o:p> </o:p></p>
</div>
</div>
</body>
</html>