<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">Hi Yong, looks pretty good to me, but still quite a few comments. First of all please send the patches directly to the mailing list using "git send-email" and not as attachment. Patch #1: <blockquote type="cite"> <pre wrap="">+ + switch (word_size) { + case 4: + num_dw = num_loops * adev->mman.buffer_funcs->fill_num_dw; + break; + case 8: /* 10 double words for each SDMA_OP_PTEPDE cmd */ + num_dw = num_loops * 10; + break; + default: + return -EINVAL; + }</pre> </blockquote> That is to complicated, we don't use the 32bit pattern during the fill anyway. So just change that to a 64bit pattern and always use the amdgpu_vm_set_pte_pde() function. Patch #2: <blockquote type="cite"> <pre wrap="">+/* Flag that supports ATS through PTE on GFX9 */ +#define AMDGPU_GEM_CLEAR_PTE_WITH_ATS_SUPPORT (1 << 6)</pre> </blockquote> NAK to that approach, GEM flags are visible to userspace. Instead add the pattern to use for the clear to amdgpu_bo_create()/amdgpu_bo_create_restricted(). <blockquote type="cite"> <pre wrap=""> /* Flag to indicate if VM tables are updated by CPU or GPU (SDMA) */ bool use_cpu_for_update; - /* Whether this is a Compute or GFX Context */ - int vm_context; + /* flags indicating the properties of VM context */ + int vm_context_flags;</pre> </blockquote> Either merge use_cpu_for_update into the flags as well or use a separate boolean for this (I prefer the later). If you want to merge drop the "vm_context_" prefix in the name, just flags should be sufficient. Regards, Christian. Am 26.07.2017 um 00:48 schrieb Yong Zhao: </div> <blockquote type="cite" cite="mid:69a3e55f-a311-5827-10a2-1cac8b0a1ee3@amd.com">Hi there, Attached are two patches made to amdgpu in order to support ATS on Raven. Please review them. Regards, Yong <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </body> </html>