All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib
@ 2021-05-13  3:21 Jiansong Chen
  2021-05-13  3:28 ` Zhang, Hawking
  2021-05-13  7:54 ` Christian König
  0 siblings, 2 replies; 3+ messages in thread
From: Jiansong Chen @ 2021-05-13  3:21 UTC (permalink / raw)
  To: amd-gfx; +Cc: Jiansong Chen, christian.koenig, Hawking.Zhang

Take the situation with gfxoff, the optimization may cause
corrupt CE ram contents. In addition emit_cntxcntl callback
has similar optimization which firmware can handle properly
even for power feature.

Signed-off-by: Jiansong Chen <Jiansong.Chen@amd.com>
Change-Id: I962946557108bb0575f8b2afc25b18a6dcf0d838
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 2e6789a7dc46..77baf9b48d67 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -130,7 +130,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	struct amdgpu_device *adev = ring->adev;
 	struct amdgpu_ib *ib = &ibs[0];
 	struct dma_fence *tmp = NULL;
-	bool skip_preamble, need_ctx_switch;
+	bool need_ctx_switch;
 	unsigned patch_offset = ~0;
 	struct amdgpu_vm *vm;
 	uint64_t fence_ctx;
@@ -227,7 +227,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	if (need_ctx_switch)
 		status |= AMDGPU_HAVE_CTX_SWITCH;
 
-	skip_preamble = ring->current_ctx == fence_ctx;
 	if (job && ring->funcs->emit_cntxcntl) {
 		status |= job->preamble_status;
 		status |= job->preemption_status;
@@ -245,14 +244,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	for (i = 0; i < num_ibs; ++i) {
 		ib = &ibs[i];
 
-		/* drop preamble IBs if we don't have a context switch */
-		if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
-		    skip_preamble &&
-		    !(status & AMDGPU_PREAMBLE_IB_PRESENT_FIRST) &&
-		    !amdgpu_mcbp &&
-		    !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
-			continue;
-
 		if (job && ring->funcs->emit_frame_cntl) {
 			if (secure != !!(ib->flags & AMDGPU_IB_FLAGS_SECURE)) {
 				amdgpu_ring_emit_frame_cntl(ring, false, secure);
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* RE: [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib
  2021-05-13  3:21 [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib Jiansong Chen
@ 2021-05-13  3:28 ` Zhang, Hawking
  2021-05-13  7:54 ` Christian König
  1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Hawking @ 2021-05-13  3:28 UTC (permalink / raw)
  To: Chen, Jiansong (Simon), amd-gfx; +Cc: Koenig, Christian, Chen, Jiansong (Simon)

[AMD Public Use]

Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>

Regards,
Hawking
-----Original Message-----
From: Jiansong Chen <Jiansong.Chen@amd.com> 
Sent: Thursday, May 13, 2021 11:22
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Chen, Jiansong (Simon) <Jiansong.Chen@amd.com>
Subject: [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib

Take the situation with gfxoff, the optimization may cause corrupt CE ram contents. In addition emit_cntxcntl callback has similar optimization which firmware can handle properly even for power feature.

Signed-off-by: Jiansong Chen <Jiansong.Chen@amd.com>
Change-Id: I962946557108bb0575f8b2afc25b18a6dcf0d838
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 2e6789a7dc46..77baf9b48d67 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -130,7 +130,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	struct amdgpu_device *adev = ring->adev;
 	struct amdgpu_ib *ib = &ibs[0];
 	struct dma_fence *tmp = NULL;
-	bool skip_preamble, need_ctx_switch;
+	bool need_ctx_switch;
 	unsigned patch_offset = ~0;
 	struct amdgpu_vm *vm;
 	uint64_t fence_ctx;
@@ -227,7 +227,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	if (need_ctx_switch)
 		status |= AMDGPU_HAVE_CTX_SWITCH;
 
-	skip_preamble = ring->current_ctx == fence_ctx;
 	if (job && ring->funcs->emit_cntxcntl) {
 		status |= job->preamble_status;
 		status |= job->preemption_status;
@@ -245,14 +244,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	for (i = 0; i < num_ibs; ++i) {
 		ib = &ibs[i];
 
-		/* drop preamble IBs if we don't have a context switch */
-		if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
-		    skip_preamble &&
-		    !(status & AMDGPU_PREAMBLE_IB_PRESENT_FIRST) &&
-		    !amdgpu_mcbp &&
-		    !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
-			continue;
-
 		if (job && ring->funcs->emit_frame_cntl) {
 			if (secure != !!(ib->flags & AMDGPU_IB_FLAGS_SECURE)) {
 				amdgpu_ring_emit_frame_cntl(ring, false, secure);
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib
  2021-05-13  3:21 [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib Jiansong Chen
  2021-05-13  3:28 ` Zhang, Hawking
@ 2021-05-13  7:54 ` Christian König
  1 sibling, 0 replies; 3+ messages in thread
From: Christian König @ 2021-05-13  7:54 UTC (permalink / raw)
  To: Jiansong Chen, amd-gfx; +Cc: Hawking.Zhang

Am 13.05.21 um 05:21 schrieb Jiansong Chen:
> Take the situation with gfxoff, the optimization may cause
> corrupt CE ram contents. In addition emit_cntxcntl callback
> has similar optimization which firmware can handle properly
> even for power feature.

NAK, it is the whole purpose of the preamble IB to be dropped if there 
isn't a context switch.

You are completely disabling this feature with that.

Christian.

>
> Signed-off-by: Jiansong Chen <Jiansong.Chen@amd.com>
> Change-Id: I962946557108bb0575f8b2afc25b18a6dcf0d838
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 11 +----------
>   1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 2e6789a7dc46..77baf9b48d67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -130,7 +130,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   	struct amdgpu_device *adev = ring->adev;
>   	struct amdgpu_ib *ib = &ibs[0];
>   	struct dma_fence *tmp = NULL;
> -	bool skip_preamble, need_ctx_switch;
> +	bool need_ctx_switch;
>   	unsigned patch_offset = ~0;
>   	struct amdgpu_vm *vm;
>   	uint64_t fence_ctx;
> @@ -227,7 +227,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   	if (need_ctx_switch)
>   		status |= AMDGPU_HAVE_CTX_SWITCH;
>   
> -	skip_preamble = ring->current_ctx == fence_ctx;
>   	if (job && ring->funcs->emit_cntxcntl) {
>   		status |= job->preamble_status;
>   		status |= job->preemption_status;
> @@ -245,14 +244,6 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   	for (i = 0; i < num_ibs; ++i) {
>   		ib = &ibs[i];
>   
> -		/* drop preamble IBs if we don't have a context switch */
> -		if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
> -		    skip_preamble &&
> -		    !(status & AMDGPU_PREAMBLE_IB_PRESENT_FIRST) &&
> -		    !amdgpu_mcbp &&
> -		    !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
> -			continue;
> -
>   		if (job && ring->funcs->emit_frame_cntl) {
>   			if (secure != !!(ib->flags & AMDGPU_IB_FLAGS_SECURE)) {
>   				amdgpu_ring_emit_frame_cntl(ring, false, secure);

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-13  7:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-13  3:21 [PATCH] drm/amdgpu: remove unsafe optimization to drop preamble ib Jiansong Chen
2021-05-13  3:28 ` Zhang, Hawking
2021-05-13  7:54 ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.