Re: [PATCH 2/8] drm/amdgpu: fix sdma v4 startup under SRIOV

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Another gentle ping? Ray, Monk can anybody give me an rb for this patch?

It's the last one in this series and I want to get it done.

Christian.

Am 12.10.2018 um 16:27 schrieb Koenig, Christian:
Great, can I get an rb or acked-by for the patch in this case?

Thanks,
Christian.

Am 10.10.2018 um 09:52 schrieb Liu, Monk:
Thanks Sigil

Hi Christian

Looks we can enable/disable ctx-switch for SDMA at will, no dependency or conflict on SRIOV

/Monk

-----Original Message-----
From: Ma, Sigil
Sent: Wednesday, October 10, 2018 3:25 PM
To: Liu, Monk <Monk.Liu@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; Min, Frank <Frank.Min@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: RE: [PATCH 2/8] drm/amdgpu: fix sdma v4 startup under SRIOV

Hi Monk,

AUTO_CTXSW_ENABLE is not relevant to worldswitch preemption. it only applies for ring buffer preemption. SDMA will do worldswitch whatever AUTO_CTXSW_ENABLE is 1 or 0.

-----Original Message-----
From: Liu, Monk
Sent: Wednesday, October 10, 2018 2:54 PM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; Min, Frank <Frank.Min@xxxxxxx>; Ma, Sigil <Sigil.Ma@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: RE: [PATCH 2/8] drm/amdgpu: fix sdma v4 startup under SRIOV

Oh, that mean I remember it reversed way, according to code looks we need to enable ctx_switch to support WORLD SWITCH for SDMA engine

But better let Sigil confirm it  ...

Hi @Ma, Sigil can you confirm it ? what's the relationship between ctx_swich and world swich for SDMA engines ?

Ctx_switch_enable() will set "SDMA0/1_CNTL's field: AUTO_CTXSW_ENABLE" to 1, can you tell us what's it for and how it go with SRIOV world switch ?

Thanks

/Monk

-----Original Message-----
From: Koenig, Christian
Sent: Tuesday, October 9, 2018 9:03 PM
To: Liu, Monk <Monk.Liu@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; Min, Frank <Frank.Min@xxxxxxx>; Ma, Sigil <Sigil.Ma@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/8] drm/amdgpu: fix sdma v4 startup under SRIOV

Hi Monk,

well that doesn't make much sense to me what you say here cause context switching certainly is already enabled under SRIOV:

-               if (amdgpu_sriov_vf(adev)) { /* bare-metal sequence
doesn't need below to lines */
-                       sdma_v4_0_ctx_switch_enable(adev, true);
-                       sdma_v4_0_enable(adev, true);
-               }
The problem is that context switching as well as the gfx ring is enabled for both SDMA0 and SDMA1 without initializing SDMA1.

That's most likely causing some unwanted consequences.

Christian.

Am 09.10.2018 um 13:45 schrieb Liu, Monk:
Context switch is for preemption across different queues (gfx, rlc0/1,
page) under bare-metal environment, For SRIOV we didn't need it and we didn't test it yet, so we just disable it to make life easier, besides since each VF share only 6 MS slice there is in fact no benefit to enable it for SRIOV ...

+ @Ma, Sigil to confirm

Hi Sigil

Do you think context switch could be enabled for SRIOV VF ?? I worry that the context switch have internal crush with preemption for world switch , thanks !

/Monk

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Tuesday, October 9, 2018 6:57 PM
To: Huang, Ray <Ray.Huang@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; Min,
Frank <Frank.Min@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/8] drm/amdgpu: fix sdma v4 startup under SRIOV

Am 09.10.2018 um 11:17 schrieb Huang Rui:
On Mon, Oct 08, 2018 at 03:35:15PM +0200, Christian König wrote:
[SNIP]
-	if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
-		r = sdma_v4_0_load_microcode(adev);
+	/* start the gfx rings and rlc compute queues */
+	for (i = 0; i < adev->sdma.num_instances; i++)
+		sdma_v4_0_gfx_resume(adev, i);
+
+	if (amdgpu_sriov_vf(adev)) {
+		sdma_v4_0_ctx_switch_enable(adev, true);
+		sdma_v4_0_enable(adev, true);
+	} else {
+		r = sdma_v4_0_rlc_resume(adev);
     		if (r)
     			return r;
     	}
+ Monk, Frank,

I probably cannot judge here, under SRIOV, I saw you disable ctx
switch before. Do you have any concern if we enabled it here.
The problem was that those calls where mixed into sdma_v4_0_gfx_resume() for the first SDMA instance.

What was happening is that SDMA0 was initialized and while doing so enabled both SDMA0 and SDMA1. So SDMA1 was starting up before the ring buffer was even set.

That this doesn't crashed was pure coincident and is most likely also the reason why we ran into problems when ring buffers weren't initialized.

Regards,
Christian.

Others, looks good for me. Christian, may we know which kind of jobs
will use sdma page queue(ring), you know, we just sdma gfx queue(ring) before?

Thanks,
Ray

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

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux