On Tue, Oct 16, 2018 at 08:34:37PM +0800, Christian König wrote: > 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. >
Feel free to add Reviewed-by: Huang Rui <[email protected]> > 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 <[email protected]>; Koenig, Christian > >> <[email protected]>; Huang, Ray <[email protected]>; Min, Frank > >> <[email protected]> > >> Cc: [email protected] > >> 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 <[email protected]>; Huang, Ray > >> <[email protected]>; Min, Frank <[email protected]>; Ma, Sigil > >> <[email protected]> > >> Cc: [email protected] > >> 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 <[email protected]>; Huang, Ray <[email protected]>; Min, > >> Frank <[email protected]>; Ma, Sigil <[email protected]> > >> Cc: [email protected] > >> 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 <[email protected]> > >>> Sent: Tuesday, October 9, 2018 6:57 PM > >>> To: Huang, Ray <[email protected]>; Liu, Monk <[email protected]>; Min, > >>> Frank <[email protected]> > >>> Cc: [email protected] > >>> 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 > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
