[PATCH] drm/amdgpu:implement CONTEXT_CONTROL (v3)

Wed Sep 7 07:07:50 UTC 2016

> What do you guys think
Sounds exactly like what I had in mind as well.

> So your #2 seems not correct, because if we only insert CONTEXT_CONTROL when doing context switch
Sorry for not being 100% clear on that. I meant that we should always 
insert CONTEXT_CONTROL as well, but set it's flags based on if a switch 
occurred or not.

Regards,
Christian.

Am 07.09.2016 um 07:30 schrieb Liu, Monk:
> 1. Don't touch the preamble flag at all, just keep that handling as it is for now.
>
> 2. Add the CONTEXT_CONTROL with the appropriate handling on context switch and bump the version number to signal that this is done.
>
> 3. When #2 is upstream we hack on Mesa to drop the CONTEXT_CONTROL packets from its IBs when it sees the new kernel version.
>
> 4. When we release SR-IOV we add some logic to the kernel driver to ignore the preamble flag.
>
>
> [ml]
> Let's simplify the problem and don't involve SR-IOV currently, the object is that:
> A) We change dma frame to make it compatible with standard/documented scheme, which is aligned with d3d and close OGL, and meanwhile
> B) We keep current MESA still work.
>
> For A), we need CONTEXT_CONTROL always inserted in ring buffer, because the load_xxx of close OGL  is dynamically skipped/kept by CONTEXT_CONTROL in ring buffer.
> So your #2 seems not correct, because if we only insert CONTEXT_CONTROL when doing context switch, that means the load_xxx from CEIB/DEIB of close OGL is always kept ( and only Preamble is skipped) even there is no context switch, and this harms performance.
>
> Since MESA use CONTEXT_CONTROL in IB, so the CONTEXT_CONTROL in ring buffer will be replaced by MESA, that means kmd can always insert CONTEXT_CONTROL.
>
> With above concerns, I think the step could follow below steps:
>
> 1. keep original preamble_flag logic: skip the preamble IB if no context switch occurs, so that old MESA doesn't break.
> 	Note: I remembered  @Bas mentioned that even no context switch, preamble IB should be kept because MESA rely on Preamble IB to do some L2 update like shader uniforms. I think that's wrong, because original 		logic is we always skip Preamble IB when no context switch occurs.
> 2.always Insert CONTEXT_CONTROL before ALL IB in ring buffer, and this CONTEXT_CONTROL skips load_xxx when no context switch ( keep load_xxx when context switch), so the close OGL can gain performance and supported by kmd with right logic.
> 	Note: like I said, MESA shouldn't get trouble with #2 because current MESA's CONTEXT_CONROL in IB will override the one of kmd.
>
> 3.Bump up KMS version to notify MESA that KMD already use CONTEXT_CONTROL,  for MESA we change it and let it not insert CONTEXT_CONTROL when talking with new version KMS. Also Mesa should make Preamble CEIb, CEIB, DEIB behave the same way as close OGL.
>
>
> I think above steps can satisfy both current MESA an close OGL logic, as well as new MESA logic.
> after sr-iov get upstreamed, kmd can remove the logic of "skip Preamble IB when no context switch".
>
>
> What do you guys think
>
> BR monk
>
>
>
>
>
>
> -----Original Message-----
> From: Christian König [mailto:deathsimple at vodafone.de]
> Sent: Tuesday, September 06, 2016 5:39 PM
> To: Liu, Monk <Monk.Liu at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> Cc: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu:implement CONTEXT_CONTROL (v3)
>
>> 1) Is that my patch must work with current MESA driver ? (even MESA doesn't change any bit of its logic) ?
> Yes and it must work with all old versions of Mesa. This is usual the tricky part to get right and most of my concern right now.
>
>> 2) is that my patch can let kmd go to a new path (using CONTEXT_CONTROL) with bump up the KMS version ?
> No, not necessary. Bumping the KMS version can only be used to signal to Mesa that it can use a new feature. E.g. Mesa can then stop to use CONTEXT_CONTROL in it's IBs, but there isn't any guarantee that it does.
>
>> 3) will MESA change its logic (align with close OGL driver) and bump up its version so that new version MESA can work with new version KMS/kmd?
> No, usually Mesa is changed in a way so that it works with both old and new kernel drivers. Only when there is a really good reason (usually critical bugs in the kernel driver) Mesa will drop support for older kernel versions.
>
> As I said in my internal mail let's do it like this:
>
> 1. Don't touch the preamble flag at all, just keep that handling as it is for now.
>
> 2. Add the CONTEXT_CONTROL with the appropriate handling on context switch and bump the version number to signal that this is done.
>
> 3. When #2 is upstream we hack on Mesa to drop the CONTEXT_CONTROL packets from it's IBs when it sees the new kernel version.
>
> 4. When we release SR-IOV we add some logic to the kernel driver to ignore the preamble flag.
>
> Regards,
> Christian.
>
> Am 06.09.2016 um 11:28 schrieb Liu, Monk:
>> Hi Bas & Christian
>>
>> I'm not familiar with the policy of upstream kernel driver, so I
>> cannot say if your proposal is doable or not,
>>
>> I have questions:
>>
>> 1) Is that my patch must work with current MESA driver ? (even MESA doesn't change any bit of its logic) ?
>> 2) is that my patch can let kmd go to a new path (using CONTEXT_CONTROL) with bump up the KMS version ?
>> 3) will MESA change its logic (align with close OGL driver) and bump up its version so that new version MESA can work with new version KMS/kmd?
>>
>> With above question addressed, we can together discuss how to modify
>> CONTEXT_CONTROL patch
>>
>> BR Monk
>>
>>
>>
>>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Monday, September 05, 2016 7:57 PM
>> To: Liu, Monk <Monk.Liu at amd.com>
>> Cc: brahma_hybrid_dev <brahma_hybrid_dev at amd.com>
>> Subject: Re: [PATCH] drm/amdgpu:implement CONTEXT_CONTROL (v3)
>>
>> Another possible solution which just came to my mind: Completely ignore the preamble flag on the IB on keep the existing preamble handling as it is.
>>
>> Just insert a CONTEXT_CONTROL package at the beginning of the command submission controlled by if we have seen a context switch or not and then raise the driver version number.
>>
>> Then we can fix Mesa to not emit the CONTEXT_CONTROL commands from the UMD any more and when SR-IOV comes out we add a handling to ignore the preamble flag in the kernel when it is activated.
>>
>> Does that sounds like it should work?
>>
>> Regards,
>> Christian.
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx