[PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[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.

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 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   |  6 -----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 35 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  3 +++
 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..a9223a8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -185,7 +185,6 @@ struct amdgpu_ring {
 	u64			cond_exe_gpu_addr;
 	volatile u32		*cond_exe_cpu_addr;
 	unsigned		vm_inv_eng;
-	bool			has_compute_vm_bug;
 #if defined(CONFIG_DEBUG_FS)
 	struct dentry *ent;
 #endif
@@ -208,9 +207,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 90392a1..f8af695 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -656,6 +656,39 @@ static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev,
 	return r;
 }
 
+static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
+{
+	if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE)
+		/* only compute rings */
+		return ring->adev->vm_manager.has_compute_vm_bug;
+	else
+		return false;
+}
+
+/**
+ * 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;
+
+	adev->vm_manager.has_compute_vm_bug = false;
+
+	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) {
+		adev->vm_manager.has_compute_vm_bug = true;
+	} else if (ip_block->version->major == 8)
+		if (adev->gfx.mec_fw_version < 673)
+			adev->vm_manager.has_compute_vm_bug = true;
+}
+
 bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 				  struct amdgpu_job *job)
 {
@@ -665,7 +698,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 	struct amdgpu_vm_id *id;
 	bool gds_switch_needed;
 	bool vm_flush_needed = job->vm_needs_flush ||
-		amdgpu_ring_has_compute_vm_bug(ring);
+		amdgpu_vm_ring_has_compute_vm_bug(ring);
 
 	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..10f5675 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -184,6 +184,8 @@ struct amdgpu_vm_manager {
 	/* partial resident texture handling */
 	spinlock_t				prt_lock;
 	atomic_t				num_prt_users;
+
+	bool					has_compute_vm_bug;
 };
 
 void amdgpu_vm_manager_init(struct amdgpu_device *adev);
@@ -245,5 +247,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
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[Christian König]

Am 31.05.2017 um 20:09 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.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   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   |  6 -----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 35 +++++++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     |  3 +++
>   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..a9223a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,7 +185,6 @@ struct amdgpu_ring {
>   	u64			cond_exe_gpu_addr;
>   	volatile u32		*cond_exe_cpu_addr;
>   	unsigned		vm_inv_eng;
> -	bool			has_compute_vm_bug;
>   #if defined(CONFIG_DEBUG_FS)
>   	struct dentry *ent;
>   #endif
> @@ -208,9 +207,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 90392a1..f8af695 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,6 +656,39 @@ static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> +static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
> +{
> +	if (ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE)
> +		/* only compute rings */
> +		return ring->adev->vm_manager.has_compute_vm_bug;
> +	else
> +		return false;
> +}
> +
> +/**
> + * 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;
> +
> +	adev->vm_manager.has_compute_vm_bug = false;
> +
> +	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) {
> +		adev->vm_manager.has_compute_vm_bug = true;
> +	} else if (ip_block->version->major == 8)
> +		if (adev->gfx.mec_fw_version < 673)
> +			adev->vm_manager.has_compute_vm_bug = true;
> +}
> +
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   				  struct amdgpu_job *job)
>   {
> @@ -665,7 +698,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   	struct amdgpu_vm_id *id;
>   	bool gds_switch_needed;
>   	bool vm_flush_needed = job->vm_needs_flush ||
> -		amdgpu_ring_has_compute_vm_bug(ring);
> +		amdgpu_vm_ring_has_compute_vm_bug(ring);
>   
>   	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..10f5675 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -184,6 +184,8 @@ struct amdgpu_vm_manager {
>   	/* partial resident texture handling */
>   	spinlock_t				prt_lock;
>   	atomic_t				num_prt_users;
> +
> +	bool					has_compute_vm_bug;
>   };
>   
>   void amdgpu_vm_manager_init(struct amdgpu_device *adev);
> @@ -245,5 +247,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


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[Leo Liu]

[-- Attachment #1.1: Type: text/plain, Size: 8193 bytes --]

When you git email-send first time, there is a message-id, when you 
would send follow up patches, using --in-reply-to="message-id" in your 
comment line, then that will be in the same thread.


Cheers,

Leo



On 06/01/2017 09:40 AM, Xie, AlexBin wrote:
>
> 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
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *Sent:* Thursday, June 1, 2017 9:26 AM
> *To:* Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH] drm/amdgpu: Move compute vm bug logic to 
> amdgpu_vm.c
> 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-5C7GfCeVMHo@public.gmane.org>
>
> 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-5C7GfCeVMHo@public.gmane.org> 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
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 15870 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[Xie, AlexBin]

[-- Attachment #1.1: Type: text/plain, Size: 7446 bytes --]

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

________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Thursday, June 1, 2017 9:26 AM
To: Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

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-5C7GfCeVMHo@public.gmane.org>

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-5C7GfCeVMHo@public.gmane.org> 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



[-- Attachment #1.2: Type: text/html, Size: 13536 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[Christian König]

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


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[Xie, AlexBin]

[-- Attachment #1.1: Type: text/plain, Size: 6596 bytes --]

Hi Christian,


Please review this v2 patch. It is slightly faster.


Thanks,

Alex Bin

________________________________
From: Xie, AlexBin
Sent: Wednesday, May 31, 2017 7:08 PM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Xie, AlexBin
Subject: [PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

  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-5C7GfCeVMHo@public.gmane.org>
---
 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
--
2.7.4


[-- Attachment #1.2: Type: text/html, Size: 12433 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Move compute vm bug logic to amdgpu_vm.c

[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>
---
 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
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx