<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;" dir="ltr"> Ok. I will add the v2 next time. I did not add the v2 in case this email list does not put the two patches under same thread... -Alex Bin <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Christian König <deathsimple@vodafone.de> Sent: Thursday, June 1, 2017 9:26 AM To: Xie, AlexBin; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c <div> </div> </div> </div> <div class="PlainText">Am 01.06.2017 um 01:08 schrieb Alex Xie: > In review, Christian would like to keep the logic > inside amdgpu_vm.c with a cost of slightly slower. > The loop is still optimized out with this patch. > > v2: remove the if statement. Now it is not slower. > > Signed-off-by: Alex Xie <AlexBin.Xie@amd.com> When you create a v2 of a patch please note that in the subject line as well. Either by using the "-v2" option with "git send-email" or by adding the v2 manually on the subject line. I sometimes get mails twice because they are send to multiple lists I'm registered on and that helps allot seeing that a patch changed and is not just a duplicated mail. Anyway this version is Reviewed-by: Christian König <christian.koeng@amd.com> as well. Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 32 ------------------------- > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 ---- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 38 ++++++++++++++++++++++++++++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 + > 5 files changed, 39 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 89bc34a..2f1a4e9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2227,6 +2227,8 @@ int amdgpu_device_init(struct amdgpu_device *adev, > > adev->accel_working = true; > > + amdgpu_vm_check_compute_bug(adev); > + > /* Initialize the buffer migration limit. */ > if (amdgpu_moverate >= 0) > max_MBps = amdgpu_moverate; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > index 7d95435..31aa51d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > @@ -153,36 +153,6 @@ void amdgpu_ring_undo(struct amdgpu_ring *ring) > } > > /** > - * amdgpu_ring_check_compute_vm_bug - check whether this ring has compute vm bug > - * > - * @adev: amdgpu_device pointer > - * @ring: amdgpu_ring structure holding ring information > - */ > -static void amdgpu_ring_check_compute_vm_bug(struct amdgpu_device *adev, > - struct amdgpu_ring *ring) > -{ > - const struct amdgpu_ip_block *ip_block; > - > - ring->has_compute_vm_bug = false; > - > - if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE) > - /* only compute rings */ > - return; > - > - ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX); > - if (!ip_block) > - return; > - > - /* Compute ring has a VM bug for GFX version < 7. > - And compute ring has a VM bug for GFX 8 MEC firmware version < 673.*/ > - if (ip_block->version->major <= 7) { > - ring->has_compute_vm_bug = true; > - } else if (ip_block->version->major == 8) > - if (adev->gfx.mec_fw_version < 673) > - ring->has_compute_vm_bug = true; > -} > - > -/** > * amdgpu_ring_init - init driver ring struct. > * > * @adev: amdgpu_device pointer > @@ -288,8 +258,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, > DRM_ERROR("Failed to register debugfs file for rings !\n"); > } > > - amdgpu_ring_check_compute_vm_bug(adev, ring); > - > return 0; > } > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > index 334307e..ad399c8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > @@ -208,9 +208,4 @@ static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring) > > } > > -static inline bool amdgpu_ring_has_compute_vm_bug(struct amdgpu_ring *ring) > -{ > - return ring->has_compute_vm_bug; > -} > - > #endif > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index d4d05a8..6e32748 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -656,6 +656,41 @@ static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev, > return r; > } > > +/** > + * amdgpu_vm_check_compute_bug - check whether asic has compute vm bug > + * > + * @adev: amdgpu_device pointer > + */ > +void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev) > +{ > + const struct amdgpu_ip_block *ip_block; > + bool has_compute_vm_bug; > + struct amdgpu_ring *ring; > + int i; > + > + has_compute_vm_bug = false; > + > + ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX); > + if (ip_block) { > + /* Compute has a VM bug for GFX version < 7. > + Compute has a VM bug for GFX 8 MEC firmware version < 673.*/ > + if (ip_block->version->major <= 7) > + has_compute_vm_bug = true; > + else if (ip_block->version->major == 8) > + if (adev->gfx.mec_fw_version < 673) > + has_compute_vm_bug = true; > + } > + > + for (i = 0; i < adev->num_rings; i++) { > + ring = adev->rings[i]; > + if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE) > + /* only compute rings */ > + ring->has_compute_vm_bug = has_compute_vm_bug; > + else > + ring->has_compute_vm_bug = false; > + } > +} > + > bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring, > struct amdgpu_job *job) > { > @@ -664,8 +699,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring, > struct amdgpu_vm_id_manager *id_mgr = &adev->vm_manager.id_mgr[vmhub]; > struct amdgpu_vm_id *id; > bool gds_switch_needed; > - bool vm_flush_needed = job->vm_needs_flush || > - amdgpu_ring_has_compute_vm_bug(ring); > + bool vm_flush_needed = job->vm_needs_flush || ring->has_compute_vm_bug; > > if (job->vm_id == 0) > return false; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > index 8309bc7..f5dba9c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > @@ -245,5 +245,6 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size); > int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); > bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring, > struct amdgpu_job *job); > +void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev); > > #endif </div> </div> </div> </body> </html>