[PATCH] drm/amdgpu: reduce the time of reading VBIOS

[Yu, Xiangliang]

Ok, thanks!

-----Original Message-----
From: Christian König [mailto:deathsimple at vodafone.de] 
Sent: Friday, July 21, 2017 10:12 PM
To: Yu, Xiangliang <Xiangliang.Yu at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: reduce the time of reading VBIOS

Am 21.07.2017 um 04:52 schrieb Xiangliang.Yu:
> From: Xiangliang Yu <Xiangliang.Yu at amd.com>
>
> Change ioremap mode from noncache to write combine and it will reduce 
> the read vbios time from 188ms to 8ms.

Nice improvement, but I think you need to improve the commit message a bit.

Changing the ioremap mode from noncache to write combine should actually not improve the read performance (reads are uncached in both modes).

I think this patch makes a difference because VRAM is usually marked write combined and so ioremap needs to change that for no good reason and that in turn takes quite some time.

I think you should write something like "Avoid changing the mapping mode of VRAM while reading the vbios from it, this reduce the time from....".

>
> Signed-off-by: Xiangliang Yu <Xiangliang.Yu at amd.com>
> Signed-off-by: Xiangliang.Yu <Xiangliang.Yu at amd.com>
You added the signed-off-by line twice, please fix this.

With that fixed and the commit message improved a bit the patch is
Reviewed-by: Christian König <christian.koenig at amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index ea3a250..c21adf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -104,7 +104,7 @@ static bool igp_read_bios_from_vram(struct 
> amdgpu_device *adev)
>   
>   	adev->bios = NULL;
>   	vram_base = pci_resource_start(adev->pdev, 0);
> -	bios = ioremap(vram_base, size);
> +	bios = ioremap_wc(vram_base, size);
>   	if (!bios) {
>   		return false;
>   	}