Changes for enabling ATS support from PTE

[Christian König]

Hi Yong,

looks pretty good to me, but still quite a few comments.

First of all please send the patches directly to the mailing list using 
"git send-email" and not as attachment.

Patch #1:
> +
> +	switch (word_size) {
> +	case 4:
> +		num_dw = num_loops * adev->mman.buffer_funcs->fill_num_dw;
> +		break;
> +	case 8: /* 10 double words for each SDMA_OP_PTEPDE cmd */
> +		num_dw = num_loops * 10;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
That is to complicated, we don't use the 32bit pattern during the fill 
anyway. So just change that to a 64bit pattern and always use the 
amdgpu_vm_set_pte_pde() function.

Patch #2:
> +/* Flag that supports ATS through PTE on GFX9 */
> +#define AMDGPU_GEM_CLEAR_PTE_WITH_ATS_SUPPORT	(1 << 6)
NAK to that approach, GEM flags are visible to userspace.

Instead add the pattern to use for the clear to 
amdgpu_bo_create()/amdgpu_bo_create_restricted().

>   	/* Flag to indicate if VM tables are updated by CPU or GPU (SDMA) */
>   	bool                    use_cpu_for_update;
>   
> -	/* Whether this is a Compute or GFX Context */
> -	int			vm_context;
> +	/* flags indicating the properties of VM context */
> +	int			vm_context_flags;
Either merge use_cpu_for_update into the flags as well or use a separate 
boolean for this (I prefer the later).

If you want to merge drop the "vm_context_" prefix in the name, just 
flags should be sufficient.

Regards,
Christian.

Am 26.07.2017 um 00:48 schrieb Yong Zhao:
> Hi there,
>
> Attached are two patches made to amdgpu in order to support ATS on 
> Raven. Please review them.
>
> Regards,
>
> Yong
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170726/93322b93/attachment.html>