[PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Alex Xie]

  Move several if statements and a loop statment from
  run time to initialization time.

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 6a85db0..7d95435 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -153,6 +153,36 @@ 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
@@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
 	if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -185,6 +185,7 @@ 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
@@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -656,32 +656,6 @@ 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)
-{
-	struct amdgpu_device *adev = ring->adev;
-	const struct amdgpu_ip_block *ip_block;
-
-	if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
-		/* only compute rings */
-		return false;
-
-	ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
-	if (!ip_block)
-		return false;
-
-	if (ip_block->version->major <= 7) {
-		/* gfx7 has no workaround */
-		return true;
-	} else if (ip_block->version->major == 8) {
-		if (adev->gfx.mec_fw_version >= 673)
-			/* gfx8 is fixed in MEC firmware 673 */
-			return false;
-		else
-			return true;
-	}
-	return false;
-}
-
 bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 				  struct amdgpu_job *job)
 {
@@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
+		amdgpu_ring_has_compute_vm_bug(ring);
 
 	if (job->vm_id == 0)
 		return false;
-- 
2.7.4

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

[PATCH 2/3] drm/amdgpu: Remove two ! operations in an if condition

[Alex Xie]

 Make the code easier to understand.

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 7a323f9..9fdeb82 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -680,9 +680,11 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 
 	if (amdgpu_vm_had_gpu_reset(adev, id))
 		return true;
-	if (!vm_flush_needed && !gds_switch_needed)
+
+	if (vm_flush_needed || gds_switch_needed)
+		return true;
+	else
 		return false;
-	return true;
 }
 
 /**
-- 
2.7.4

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

[PATCH 3/3] drm/amdgpu: clean up code to make it easier to understand

[Alex Xie]

 The original code convert pointer from amdgpu_crtc to
 drm_crtc then later convert the pointer back.
 It is difficult to understand why it was written
 that way.

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 4dd83a3..7317fc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -258,8 +258,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 		for (i = 0, found = 0; i < adev->mode_info.num_crtc; i++) {
 			crtc = (struct drm_crtc *)minfo->crtcs[i];
 			if (crtc && crtc->base.id == info->mode_crtc.id) {
-				struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-				ui32 = amdgpu_crtc->crtc_id;
+				ui32 = minfo->crtcs[i]->crtc_id;
 				found = 1;
 				break;
 			}
-- 
2.7.4

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

Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[zhoucm1]

On 2017年05月31日 05:47, Alex Xie wrote:
>    Move several if statements and a loop statment from
>    run time to initialization time.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
nice, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
>   3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 6a85db0..7d95435 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -153,6 +153,36 @@ 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
> @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>   	if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,6 +185,7 @@ 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
> @@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,32 +656,6 @@ 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)
> -{
> -	struct amdgpu_device *adev = ring->adev;
> -	const struct amdgpu_ip_block *ip_block;
> -
> -	if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> -		/* only compute rings */
> -		return false;
> -
> -	ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> -	if (!ip_block)
> -		return false;
> -
> -	if (ip_block->version->major <= 7) {
> -		/* gfx7 has no workaround */
> -		return true;
> -	} else if (ip_block->version->major == 8) {
> -		if (adev->gfx.mec_fw_version >= 673)
> -			/* gfx8 is fixed in MEC firmware 673 */
> -			return false;
> -		else
> -			return true;
> -	}
> -	return false;
> -}
> -
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   				  struct amdgpu_job *job)
>   {
> @@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
> +		amdgpu_ring_has_compute_vm_bug(ring);
>   
>   	if (job->vm_id == 0)
>   		return false;

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

Re: [PATCH 2/3] drm/amdgpu: Remove two ! operations in an if condition

[zhoucm1]

On 2017年05月31日 05:47, Alex Xie wrote:
>   Make the code easier to understand.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
ok to me, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7a323f9..9fdeb82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -680,9 +680,11 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   
>   	if (amdgpu_vm_had_gpu_reset(adev, id))
>   		return true;
> -	if (!vm_flush_needed && !gds_switch_needed)
> +
> +	if (vm_flush_needed || gds_switch_needed)
> +		return true;
> +	else
>   		return false;
> -	return true;
>   }
>   
>   /**

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

Re: [PATCH 2/3] drm/amdgpu: Remove two ! operations in an if condition

[Michel Dänzer]

On 31/05/17 06:47 AM, Alex Xie wrote:
>  Make the code easier to understand.
> 
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7a323f9..9fdeb82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -680,9 +680,11 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>  
>  	if (amdgpu_vm_had_gpu_reset(adev, id))
>  		return true;
> -	if (!vm_flush_needed && !gds_switch_needed)
> +
> +	if (vm_flush_needed || gds_switch_needed)
> +		return true;
> +	else
>  		return false;

This can be further simplified to

	return vm_flush_needed || gds_switch_needed;

With that,

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu: clean up code to make it easier to understand

[zhoucm1]

On 2017年05月31日 05:47, Alex Xie wrote:
>   The original code convert pointer from amdgpu_crtc to
>   drm_crtc then later convert the pointer back.
>   It is difficult to understand why it was written
>   that way.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 4dd83a3..7317fc9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -258,8 +258,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>   		for (i = 0, found = 0; i < adev->mode_info.num_crtc; i++) {
>   			crtc = (struct drm_crtc *)minfo->crtcs[i];
>   			if (crtc && crtc->base.id == info->mode_crtc.id) {
> -				struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -				ui32 = amdgpu_crtc->crtc_id;
> +				ui32 = minfo->crtcs[i]->crtc_id;
I feel previous code is more clear, it differentiates drm_crtc and 
amdgpu_crtc, which makes more clear for accessing their member.
If totally not convert, then we will see the below code, when we read 
the code, we will need to check struct amdgpu_mode_info, struct 
amdgpu_crtc and struct drm_crtc for condition line.
if (minfo->crtcs[i] && minfo->crtcs[i]->base.base.id == 
info->mode_crtc.id) {
     ui32 = minfo->crtcs[i]->crtc_id;


Regards,
David Zhou
>   				found = 1;
>   				break;
>   			}

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

Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Christian König]

Am 30.05.2017 um 23:47 schrieb Alex Xie:
>    Move several if statements and a loop statment from
>    run time to initialization time.

Yeah, that's exactly what I've suggested before as well.

Just keep the code inside amdgpu_vm.c (and the variable inside 
amdgpu_vm_manager), since this isn't related to ring management at all.

Regards,
Christian.

>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
>   3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 6a85db0..7d95435 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -153,6 +153,36 @@ 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
> @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>   	if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,6 +185,7 @@ 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
> @@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,32 +656,6 @@ 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)
> -{
> -	struct amdgpu_device *adev = ring->adev;
> -	const struct amdgpu_ip_block *ip_block;
> -
> -	if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> -		/* only compute rings */
> -		return false;
> -
> -	ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> -	if (!ip_block)
> -		return false;
> -
> -	if (ip_block->version->major <= 7) {
> -		/* gfx7 has no workaround */
> -		return true;
> -	} else if (ip_block->version->major == 8) {
> -		if (adev->gfx.mec_fw_version >= 673)
> -			/* gfx8 is fixed in MEC firmware 673 */
> -			return false;
> -		else
> -			return true;
> -	}
> -	return false;
> -}
> -
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   				  struct amdgpu_job *job)
>   {
> @@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
> +		amdgpu_ring_has_compute_vm_bug(ring);
>   
>   	if (job->vm_id == 0)
>   		return false;


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

Re: [PATCH 2/3] drm/amdgpu: Remove two ! operations in an if condition

[Christian König]

Am 30.05.2017 um 23:47 schrieb Alex Xie:
>   Make the code easier to understand.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7a323f9..9fdeb82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -680,9 +680,11 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   
>   	if (amdgpu_vm_had_gpu_reset(adev, id))
>   		return true;
> -	if (!vm_flush_needed && !gds_switch_needed)
> +
> +	if (vm_flush_needed || gds_switch_needed)
> +		return true;
> +	else
>   		return false;
> -	return true;

Actually that should even use the canonical form, e.g. something like 
"return vm_flush_needed || gds_switch_needed;".

Apart from that looks good to me,
Christian.

>   }
>   
>   /**


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

Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Xie, AlexBin]

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

HI Christian,


Too late. The code has been committed.


I don't have strong opinion about where the code should be. But if we put this code in VM, there will be one extra array index operation because the VM bug is ring related. VM manager need to maintain an array to manage this information.


In the amdgpu_ring structure, there is already information like vm_inv_eng and vmhub. Those are VM related information too. So this one extra information is not new.


Thanks,

Alex Bin

________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Wednesday, May 31, 2017 2:57 AM
To: Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

Am 30.05.2017 um 23:47 schrieb Alex Xie:
>    Move several if statements and a loop statment from
>    run time to initialization time.

Yeah, that's exactly what I've suggested before as well.

Just keep the code inside amdgpu_vm.c (and the variable inside
amdgpu_vm_manager), since this isn't related to ring management at all.

Regards,
Christian.

>
> Signed-off-by: Alex Xie <AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
>   3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 6a85db0..7d95435 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -153,6 +153,36 @@ 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
> @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>        if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,6 +185,7 @@ 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
> @@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,32 +656,6 @@ 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)
> -{
> -     struct amdgpu_device *adev = ring->adev;
> -     const struct amdgpu_ip_block *ip_block;
> -
> -     if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> -             /* only compute rings */
> -             return false;
> -
> -     ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> -     if (!ip_block)
> -             return false;
> -
> -     if (ip_block->version->major <= 7) {
> -             /* gfx7 has no workaround */
> -             return true;
> -     } else if (ip_block->version->major == 8) {
> -             if (adev->gfx.mec_fw_version >= 673)
> -                     /* gfx8 is fixed in MEC firmware 673 */
> -                     return false;
> -             else
> -                     return true;
> -     }
> -     return false;
> -}
> -
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>                                  struct amdgpu_job *job)
>   {
> @@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
> +             amdgpu_ring_has_compute_vm_bug(ring);
>
>        if (job->vm_id == 0)
>                return false;



[-- Attachment #1.2: Type: text/html, Size: 11529 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 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Christian König]

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

> I don't have strong opinion about where the code should be. But if we 
> put this code in VM, there will be one extra array index operation 
> because the VM bug is ring related. VM manager need to maintain an 
> array to manage this information.
>
Yeah, but array index suck when you access cache cold stuff as well.

My concern is not so much where to put the field, but rather where to 
put the code to detect this condition. That is a bug very deeply related 
to how the CP manages VMs and VMIDs and actually not specific to the ring.

Just send out a patch which uses the ring type again to check if that 
workaround applies or not. The heavy stuff was calling 
amdgpu_get_ip_block() on every command submission, one additional if 
shouldn't hurt us here.

Regards,
Christian.

Am 31.05.2017 um 14:53 schrieb Xie, AlexBin:
>
> HI Christian,
>
>
> Too late. The code has been committed.
>
>
> I don't have strong opinion about where the code should be. But if we 
> put this code in VM, there will be one extra array index operation 
> because the VM bug is ring related. VM manager need to maintain an 
> array to manage this information.
>
>
> In the amdgpu_ring structure, there is already information like 
> vm_inv_eng and vmhub. Those are VM related information too. So this 
> one extra information is not new.
>
>
> Thanks,
>
> Alex Bin
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *Sent:* Wednesday, May 31, 2017 2:57 AM
> *To:* Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by 
> every IB sheduling
> Am 30.05.2017 um 23:47 schrieb Alex Xie:
> >    Move several if statements and a loop statment from
> >    run time to initialization time.
>
> Yeah, that's exactly what I've suggested before as well.
>
> Just keep the code inside amdgpu_vm.c (and the variable inside
> amdgpu_vm_manager), since this isn't related to ring management at all.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Alex Xie <AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 
> ++++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 
> +--------------------------
> >   3 files changed, 40 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > index 6a85db0..7d95435 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > @@ -153,6 +153,36 @@ 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
> > @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, 
> struct amdgpu_ring *ring,
> >        if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -185,6 +185,7 @@ 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
> > @@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -656,32 +656,6 @@ 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)
> > -{
> > -     struct amdgpu_device *adev = ring->adev;
> > -     const struct amdgpu_ip_block *ip_block;
> > -
> > -     if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> > -             /* only compute rings */
> > -             return false;
> > -
> > -     ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> > -     if (!ip_block)
> > -             return false;
> > -
> > -     if (ip_block->version->major <= 7) {
> > -             /* gfx7 has no workaround */
> > -             return true;
> > -     } else if (ip_block->version->major == 8) {
> > -             if (adev->gfx.mec_fw_version >= 673)
> > -                     /* gfx8 is fixed in MEC firmware 673 */
> > -                     return false;
> > -             else
> > -                     return true;
> > -     }
> > -     return false;
> > -}
> > -
> >   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
> >                                  struct amdgpu_job *job)
> >   {
> > @@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
> > +             amdgpu_ring_has_compute_vm_bug(ring);
> >
> >        if (job->vm_id == 0)
> >                return false;
>
>


[-- Attachment #1.2: Type: text/html, Size: 12888 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 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Xie, AlexBin]

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

Hi Christian,


I have asked for code review of a new patch.


It turns out that the index operation is not needed. But there is one extra if statement in every IB submission, which will be a slight performance hit.


Function amdgpu_vm_check_compute_bug is called in amdgpu_device_init.

Function amdgpu_vm_check_compute_bug cannot be called in amdgpu_vm_manager_init, in which GFX is not initialized yet. MEC firmware version information is not available yet.


Thanks,

Alex Bin Xie

________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Wednesday, May 31, 2017 9:07 AM
To: Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling


I don't have strong opinion about where the code should be. But if we put this code in VM, there will be one extra array index operation because the VM bug is ring related. VM manager need to maintain an array to manage this information.

Yeah, but array index suck when you access cache cold stuff as well.

My concern is not so much where to put the field, but rather where to put the code to detect this condition. That is a bug very deeply related to how the CP manages VMs and VMIDs and actually not specific to the ring.

Just send out a patch which uses the ring type again to check if that workaround applies or not. The heavy stuff was calling amdgpu_get_ip_block() on every command submission, one additional if shouldn't hurt us here.

Regards,
Christian.

Am 31.05.2017 um 14:53 schrieb Xie, AlexBin:

HI Christian,


Too late. The code has been committed.


I don't have strong opinion about where the code should be. But if we put this code in VM, there will be one extra array index operation because the VM bug is ring related. VM manager need to maintain an array to manage this information.


In the amdgpu_ring structure, there is already information like vm_inv_eng and vmhub. Those are VM related information too. So this one extra information is not new.


Thanks,

Alex Bin

________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org><mailto:deathsimple@vodafone.de>
Sent: Wednesday, May 31, 2017 2:57 AM
To: Xie, AlexBin; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>
Subject: Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

Am 30.05.2017 um 23:47 schrieb Alex Xie:
>    Move several if statements and a loop statment from
>    run time to initialization time.

Yeah, that's exactly what I've suggested before as well.

Just keep the code inside amdgpu_vm.c (and the variable inside
amdgpu_vm_manager), since this isn't related to ring management at all.

Regards,
Christian.

>
> Signed-off-by: Alex Xie <AlexBin.Xie-5C7GfCeVMHo@public.gmane.org><mailto:AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
>   3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 6a85db0..7d95435 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -153,6 +153,36 @@ 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
> @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>        if (amdgpu_debugfs_ring_init(adev, 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 a9223a8..334307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,6 +185,7 @@ 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
> @@ -207,4 +208,9 @@ 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 b2384b8..7a323f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,32 +656,6 @@ 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)
> -{
> -     struct amdgpu_device *adev = ring->adev;
> -     const struct amdgpu_ip_block *ip_block;
> -
> -     if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> -             /* only compute rings */
> -             return false;
> -
> -     ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> -     if (!ip_block)
> -             return false;
> -
> -     if (ip_block->version->major <= 7) {
> -             /* gfx7 has no workaround */
> -             return true;
> -     } else if (ip_block->version->major == 8) {
> -             if (adev->gfx.mec_fw_version >= 673)
> -                     /* gfx8 is fixed in MEC firmware 673 */
> -                     return false;
> -             else
> -                     return true;
> -     }
> -     return false;
> -}
> -
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>                                  struct amdgpu_job *job)
>   {
> @@ -691,7 +665,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_vm_ring_has_compute_vm_bug(ring);
> +             amdgpu_ring_has_compute_vm_bug(ring);
>
>        if (job->vm_id == 0)
>                return false;




[-- Attachment #1.2: Type: text/html, Size: 14551 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