Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
> Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
> amdgpu_bo_do_create into standalone helper so it can be reused
> in other functions.
>
> v4:
> Switch to return bool.
>
> v5: Fix typos.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61 +++++++++++++++++-------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
>   2 files changed, 37 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 989b7b5..8702062 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
>   	return false;
>   }
>   
> +bool amdgpu_bo_support_uswc(u64 bo_flags)
> +{
> +
> +#ifdef CONFIG_X86_32
> +	/* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> +	 * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
> +	 */
> +	return false;
> +#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
> +	/* Don't try to enable write-combining when it can't work, or things
> +	 * may be slow
> +	 * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
> +	 */
> +
> +#ifndef CONFIG_COMPILE_TEST
> +#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
> +	 thanks to write-combining
> +#endif
> +
> +	if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
> +		DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for "
> +			      "better performance thanks to write-combining\n");

I don't think this message belongs here.

> +	return false;
> +#else
> +	/* For architectures that don't support WC memory,
> +	 * mask out the WC flag from the BO
> +	 */
> +	if (!drm_arch_can_wc_memory())
> +		return false;
> +
> +	return true;
> +#endif
> +}
> +
>   static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   			       struct amdgpu_bo_param *bp,
>   			       struct amdgpu_bo **bo_ptr)
> @@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   
>   	bo->flags = bp->flags;
>   
> -#ifdef CONFIG_X86_32
> -	/* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> -	 * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
> -	 */
> -	bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
> -	/* Don't try to enable write-combining when it can't work, or things
> -	 * may be slow
> -	 * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
> -	 */
> -
> -#ifndef CONFIG_COMPILE_TEST
> -#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
> -	 thanks to write-combining
> -#endif
> -
> -	if (bo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
> -		DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for "
> -			      "better performance thanks to write-combining\n");
> -	bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -#else
> -	/* For architectures that don't support WC memory,
> -	 * mask out the WC flag from the BO
> -	 */
> -	if (!drm_arch_can_wc_memory())
> +	if (!amdgpu_bo_support_uswc(bo->flags))
>   		bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;

Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC 
&& !amdgpu_bo_support_uswc())" and then clear the flag and also print 
the warning.

Apart from that the series looks good to me,
Christian.

> -#endif
>   
>   	bo->tbo.bdev = &adev->mman.bdev;
>   	if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index d60593c..dc44cf3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -308,5 +308,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
>   					 struct seq_file *m);
>   #endif
>   
> +bool amdgpu_bo_support_uswc(u64 bo_flags);
> +
>   
>   #endif

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