drm/ttm: ioremap buffer according to TTM mem caching setting

Commit Message

Zeng, Oak March 1, 2021, 10:43 p.m. UTC

If tbo.mem.bus.caching is cached, buffer is intended to be mapped
as cached from CPU. Map it with ioremap_cache.

This wasn't necessary before as device memory was never mapped
as cached from CPU side. It becomes necessary for aldebaran as
device memory is mapped cached from CPU.

Signed-off-by: Oak Zeng <[e-mail]>
Reviewed-by: Christian Konig <[e-mail]>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

kernel test robot March 2, 2021, 2:16 a.m. UTC | #1

Hi Oak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip linus/master v5.12-rc1 next-20210301]
[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: parisc-randconfig-r012-20210302 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
        git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
   drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
         |           ^~~~~~~~~~~~~
         |           ioremap_uc
>> drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
         |         ^
   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
   drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
         |                 ^
   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
   drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
         |                ^
   cc1: some warnings being treated as errors


vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c

    74	
    75	static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
    76				       struct ttm_resource *mem,
    77				       void **virtual)
    78	{
    79		int ret;
    80		void *addr;
    81	
    82		*virtual = NULL;
    83		ret = ttm_mem_io_reserve(bdev, mem);
    84		if (ret || !mem->bus.is_iomem)
    85			return ret;
    86	
    87		if (mem->bus.addr) {
    88			addr = mem->bus.addr;
    89		} else {
    90			size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
    91	
    92			if (mem->bus.caching == ttm_write_combined)
    93				addr = ioremap_wc(mem->bus.offset, bus_size);
    94			else if (mem->bus.caching == ttm_cached)
  > 95				addr = ioremap_cache(mem->bus.offset, bus_size);
    96			else
    97				addr = ioremap(mem->bus.offset, bus_size);
    98			if (!addr) {
    99				ttm_mem_io_free(bdev, mem);
   100				return -ENOMEM;
   101			}
   102		}
   103		*virtual = addr;
   104		return 0;
   105	}
   106	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

kernel test robot March 2, 2021, 4:12 a.m. UTC | #2

Hi Oak,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: parisc-randconfig-r012-20210302 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
        git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
>> drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
         |           ^~~~~~~~~~~~~
         |           ioremap_uc
   drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
         |         ^
   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
   drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
         |                 ^
   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
   drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
         |                ^
   cc1: some warnings being treated as errors


vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c

    74	
    75	static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
    76				       struct ttm_resource *mem,
    77				       void **virtual)
    78	{
    79		int ret;
    80		void *addr;
    81	
    82		*virtual = NULL;
    83		ret = ttm_mem_io_reserve(bdev, mem);
    84		if (ret || !mem->bus.is_iomem)
    85			return ret;
    86	
    87		if (mem->bus.addr) {
    88			addr = mem->bus.addr;
    89		} else {
    90			size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
    91	
    92			if (mem->bus.caching == ttm_write_combined)
    93				addr = ioremap_wc(mem->bus.offset, bus_size);
    94			else if (mem->bus.caching == ttm_cached)
  > 95				addr = ioremap_cache(mem->bus.offset, bus_size);
    96			else
    97				addr = ioremap(mem->bus.offset, bus_size);
    98			if (!addr) {
    99				ttm_mem_io_free(bdev, mem);
   100				return -ENOMEM;
   101			}
   102		}
   103		*virtual = addr;
   104		return 0;
   105	}
   106	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Christian König March 2, 2021, 11:31 a.m. UTC | #3

Hi guys,

adding the usual suspects direct. Does anybody of hand know how to check 
if an architecture supports ioremap_cache()?

For now we only need this on X86, but I would feel better if we don't 
use an #ifdef here.

Regards,
Christian.

Am 02.03.21 um 05:12 schrieb kernel test robot:
> Hi Oak,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on drm-intel/for-linux-next]
> [also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
> [cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
> base:   git://anongit.freedesktop.org/drm-intel for-linux-next
> config: parisc-randconfig-r012-20210302 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64
>          git remote add linux-review https://github.com/0day-ci/linux
>          git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
>          git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
>          # save the attached .config to linux build tree
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[e-mail]>
>
> All errors (new ones prefixed by >>):
>
>     drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
>>> drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
>        95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>           |           ^~~~~~~~~~~~~
>           |           ioremap_uc
>     drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>        95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>           |         ^
>     drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
>     drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>       379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
>           |                 ^
>     drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
>     drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>       500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
>           |                ^
>     cc1: some warnings being treated as errors
>
>
> vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c
>
>      74	
>      75	static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
>      76				       struct ttm_resource *mem,
>      77				       void **virtual)
>      78	{
>      79		int ret;
>      80		void *addr;
>      81	
>      82		*virtual = NULL;
>      83		ret = ttm_mem_io_reserve(bdev, mem);
>      84		if (ret || !mem->bus.is_iomem)
>      85			return ret;
>      86	
>      87		if (mem->bus.addr) {
>      88			addr = mem->bus.addr;
>      89		} else {
>      90			size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
>      91	
>      92			if (mem->bus.caching == ttm_write_combined)
>      93				addr = ioremap_wc(mem->bus.offset, bus_size);
>      94			else if (mem->bus.caching == ttm_cached)
>    > 95				addr = ioremap_cache(mem->bus.offset, bus_size);
>      96			else
>      97				addr = ioremap(mem->bus.offset, bus_size);
>      98			if (!addr) {
>      99				ttm_mem_io_free(bdev, mem);
>     100				return -ENOMEM;
>     101			}
>     102		}
>     103		*virtual = addr;
>     104		return 0;
>     105	}
>     106	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[e-mail]
>
> _______________________________________________
> amd-gfx mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Zeng, Oak March 2, 2021, 10:45 p.m. UTC | #4

[AMD Official Use Only - Internal Distribution Only]

Hi Daniel, Thomas, Dan,

Does below message mean the calling ioremap_cache failed intel's driver build? I can see both ioremap_cache and ioremap_wc are defined in arch/x86/mm/ioremap.c - why ioremap_wc doesn't break intel driver's build?

Are we supposed to use memremap (offset, size, MEMREMAP_WB) to replace ioremap_cache? When I read here https://lwn.net/Articles/653585/ I felt that ioremap_cache returns an address annotated with _iomem while memremap returns an address without __iomem annotation. In our use case, GPU memory is treated as UEFI SPM (specific purpose memory). I am not very sure whether memremap (thus no __iomem annotation) is the right thing to do. What I am sure is, we have tested ioremap_cache and it works fine on AMD system.

I will send out a test patch replacing ioremap_cache with ioremap_wc, to trigger Intel build robot to see whether it fails Intel build. I suppose it will not fail Intel build.

Regards,
Oak

From: Christian König <[e-mail]>
Sent: Tuesday, March 2, 2021 6:31 AM
To: [e-mail]; [e-mail]; Daniel Vetter <[e-mail]>; Dave Airlie <[e-mail]>; Thomas Hellström (Intel) <[e-mail]>
Cc: Zeng, Oak <[e-mail]>; [e-mail]; Kuehling, Felix <[e-mail]>; Kasiviswanathan, Harish <[e-mail]>; Deucher, Alexander <[e-mail]>; Huang, JinHuiEric <[e-mail]>; Koenig, Christian <[e-mail]>
Subject: Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

Hi guys,

adding the usual suspects direct. Does anybody of hand know how to check if an architecture supports ioremap_cache()?

For now we only need this on X86, but I would feel better if we don't use an #ifdef here.

Regards,
Christian.
Am 02.03.21 um 05:12 schrieb kernel test robot:

Hi Oak,



Thank you for the patch! Yet something to improve:



[auto build test ERROR on drm-intel/for-linux-next]

[also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]

[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]

[If your patch is applied to the wrong git tree, kindly drop us a note.

And when submitting patch, we suggest to use '--base' as documented in

https://git-scm.com/docs/git-format-patch<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793168696%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=p4iynMPvZGknfSGSyZnXV3kLwScMLbPDB8zVsmxhtk0%3D&reserved=0>]



url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FOak-Zeng%2Fdrm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting%2F20210302-064500&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793178689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2sc4jZR3bVRF0xDDqNOtUcNR9qiJMF2ATmCDAX%2BSWrQ%3D&reserved=0>

base:   git://anongit.freedesktop.org/drm-intel for-linux-next

config: parisc-randconfig-r012-20210302 (attached as .config)

compiler: hppa-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):

        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793178689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uILcLE%2F24bhSU%2Bo5GmWGAK6s6xDFivP6lrm6JgtM50Y%3D&reserved=0> -O ~/bin/make.cross

        chmod +x ~/bin/make.cross

        # https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2F225bb3711439ec559dd72ae5af8e62d34ea60a64&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793188685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2TOSPuKEMRcZjMfxO9lxgwFxgXwHqERCOgRednI7OE8%3D&reserved=0>

        git remote add linux-review https://github.com/0day-ci/linux<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793188685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TlXvs5mxH0RV9qQFaUF2B1LZisTWbnt4hfFd2OC7gGw%3D&reserved=0>

        git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500

        git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64

        # save the attached .config to linux build tree

        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc



If you fix the issue, kindly add following tag as appropriate

Reported-by: kernel test robot <[e-mail]><mailto:[e-mail]>



All errors (new ones prefixed by >>):



   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':

drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]

      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);

         |           ^~~~~~~~~~~~~

         |           ioremap_uc

   drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);

         |         ^

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':

   drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

     379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,

         |                 ^

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':

   drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

     500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,

         |                ^

   cc1: some warnings being treated as errors





vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c



    74

    75  static int ttm_resource_ioremap(struct ttm_bo_device *bdev,

    76                                struct ttm_resource *mem,

    77                                void **virtual)

    78  {

    79          int ret;

    80          void *addr;

    81

    82          *virtual = NULL;

    83          ret = ttm_mem_io_reserve(bdev, mem);

    84          if (ret || !mem->bus.is_iomem)

    85                  return ret;

    86

    87          if (mem->bus.addr) {

    88                  addr = mem->bus.addr;

    89          } else {

    90                  size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;

    91

    92                  if (mem->bus.caching == ttm_write_combined)

    93                         addr = ioremap_wc(mem->bus.offset, bus_size);

    94                  else if (mem->bus.caching == ttm_cached)

  > 95                         addr = ioremap_cache(mem->bus.offset, bus_size);

    96                  else

    97                         addr = ioremap(mem->bus.offset, bus_size);

    98                  if (!addr) {

    99                         ttm_mem_io_free(bdev, mem);

   100                         return -ENOMEM;

   101                  }

   102          }

   103          *virtual = addr;

   104          return 0;

   105  }

   106



---

0-DAY CI Kernel Test Service, Intel Corporation

https://lists.01.org/hyperkitty/list/[e-mail]<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793198680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8vaxBMiRPv5mgZyKjDnsNtzAz%2BDltnzlkUMDMg45%2BrI%3D&reserved=0>

Dave Airlie March 2, 2021, 10:53 p.m. UTC | #5

On Wed, 3 Mar 2021 at 08:45, Zeng, Oak <[e-mail]> wrote:
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
> Hi Daniel, Thomas, Dan,
>
>
>
> Does below message mean the calling ioremap_cache failed intel’s driver build? I can see both ioremap_cache and ioremap_wc are defined in arch/x86/mm/ioremap.c – why ioremap_wc doesn’t break intel driver’s build?

Just to clear up confusion here, the linux kernel robot is hosted by
Intel it does not test intel driver builds exclusively, it tests a lot
of different builds across lots of different architectures,.

If the robot complains it's because your patch breaks in the
configuration it describes, take the time to read that configuration
info and realise it's nothing to do with Intel at all.

Dave.

Thomas Zimmermann March 3, 2021, 8:49 a.m. UTC | #6

Hi

Am 01.03.21 um 23:43 schrieb Oak Zeng:
> If tbo.mem.bus.caching is cached, buffer is intended to be mapped
> as cached from CPU. Map it with ioremap_cache.

Just a question for my understanding: This is on-device memory? 
Accessing device memory is usually slow. If that memory can be mapped 
with CPU caching enabled, access will roughly be as fast as for system 
memory?

Best regards
Thomas

> 
> This wasn't necessary before as device memory was never mapped
> as cached from CPU side. It becomes necessary for aldebaran as
> device memory is mapped cached from CPU.
> 
> Signed-off-by: Oak Zeng <[e-mail]>
> Reviewed-by: Christian Konig <[e-mail]>
> ---
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 031e581..8c65a13 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -91,6 +91,8 @@ static int ttm_resource_ioremap(struct ttm_device *bdev,
>   
>   		if (mem->bus.caching == ttm_write_combined)
>   			addr = ioremap_wc(mem->bus.offset, bus_size);
> +		else if (mem->bus.caching == ttm_cached)
> +			addr = ioremap_cache(mem->bus.offset, bus_size);
>   		else
>   			addr = ioremap(mem->bus.offset, bus_size);
>   		if (!addr) {
> @@ -372,6 +374,9 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
>   		if (mem->bus.caching == ttm_write_combined)
>   			map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
>   						  size);
> +		else if (mem->bus.caching == ttm_cached)
> +			map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
> +						  size);
>   		else
>   			map->virtual = ioremap(bo->mem.bus.offset + offset,
>   					       size);
> @@ -490,6 +495,9 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
>   		else if (mem->bus.caching == ttm_write_combined)
>   			vaddr_iomem = ioremap_wc(mem->bus.offset,
>   						 bo->base.size);
> +		else if (mem->bus.caching == ttm_cached)
> +			vaddr_iomem = ioremap_cache(mem->bus.offset,
> +						  bo->base.size);
>   		else
>   			vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
>   
>

Christian König March 3, 2021, 10:37 a.m. UTC | #7

Hi Thomas,

Am 03.03.21 um 09:49 schrieb Thomas Zimmermann:
> Hi
>
> Am 01.03.21 um 23:43 schrieb Oak Zeng:
>> If tbo.mem.bus.caching is cached, buffer is intended to be mapped
>> as cached from CPU. Map it with ioremap_cache.
>
> Just a question for my understanding: This is on-device memory? 
> Accessing device memory is usually slow. If that memory can be mapped 
> with CPU caching enabled, access will roughly be as fast as for system 
> memory?

There is still a penalty associated with accessing it from the CPU, but 
it is much faster (both lower latency as well as throughput) as 
traditional device memory accessed over PCIe.

Regards,
Christian.

>
> Best regards
> Thomas
>
>>
>> This wasn't necessary before as device memory was never mapped
>> as cached from CPU side. It becomes necessary for aldebaran as
>> device memory is mapped cached from CPU.
>>
>> Signed-off-by: Oak Zeng <[e-mail]>
>> Reviewed-by: Christian Konig <[e-mail]>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo_util.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 031e581..8c65a13 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -91,6 +91,8 @@ static int ttm_resource_ioremap(struct ttm_device 
>> *bdev,
>>             if (mem->bus.caching == ttm_write_combined)
>>               addr = ioremap_wc(mem->bus.offset, bus_size);
>> +        else if (mem->bus.caching == ttm_cached)
>> +            addr = ioremap_cache(mem->bus.offset, bus_size);
>>           else
>>               addr = ioremap(mem->bus.offset, bus_size);
>>           if (!addr) {
>> @@ -372,6 +374,9 @@ static int ttm_bo_ioremap(struct 
>> ttm_buffer_object *bo,
>>           if (mem->bus.caching == ttm_write_combined)
>>               map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
>>                             size);
>> +        else if (mem->bus.caching == ttm_cached)
>> +            map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
>> +                          size);
>>           else
>>               map->virtual = ioremap(bo->mem.bus.offset + offset,
>>                              size);
>> @@ -490,6 +495,9 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, 
>> struct dma_buf_map *map)
>>           else if (mem->bus.caching == ttm_write_combined)
>>               vaddr_iomem = ioremap_wc(mem->bus.offset,
>>                            bo->base.size);
>> +        else if (mem->bus.caching == ttm_cached)
>> +            vaddr_iomem = ioremap_cache(mem->bus.offset,
>> +                          bo->base.size);
>>           else
>>               vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);
>>
>
>
> _______________________________________________
> amd-gfx mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Christian König March 3, 2021, 10:45 a.m. UTC | #8

Hi Oak,

> config: parisc-randconfig-r012-20210302 (attached as .config)

It's not the Intel driver build which fails here, but the build bot is 
just hosted by Intel.

The problem is that the parisc architecture doesn't defines the 
ioremap_cache() function.

I've looked at using memremap() instead of ioremap_cache(). The problem 
is that we do support architectures with the TTM as well as amndgpu code 
where the __iomem annotation is mandatory and correct.

Regards,
Christian.

Am 02.03.21 um 23:45 schrieb Zeng, Oak:
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
> Hi Daniel, Thomas, Dan,
>
> Does below message mean the calling ioremap_cache failed intel’s 
> driver build? I can see both ioremap_cache and ioremap_wc are defined 
> in arch/x86/mm/ioremap.c – why ioremap_wc doesn’t break intel driver’s 
> build?
>
> Are we supposed to use memremap (offset, size, MEMREMAP_WB) to replace 
> ioremap_cache? When I read here https://lwn.net/Articles/653585/ I 
> felt that ioremap_cache returns an address annotated with _iomem while 
> memremap returns an address without __iomem annotation. In our use 
> case, GPU memory is treated as UEFI SPM (specific purpose memory). I 
> am not very sure whether memremap (thus no __iomem annotation) is the 
> right thing to do. What I am sure is, we have tested ioremap_cache and 
> it works fine on AMD system.
>
> I will send out a test patch replacing ioremap_cache with ioremap_wc, 
> to trigger Intel build robot to see whether it fails Intel build. I 
> suppose it will not fail Intel build.
>
> Regards,
>
> Oak
>
> *From:* Christian König <[e-mail]>
> *Sent:* Tuesday, March 2, 2021 6:31 AM
> *To:* [e-mail]; [e-mail]; 
> Daniel Vetter <[e-mail]>; Dave Airlie <[e-mail]>; 
> Thomas Hellström (Intel) <[e-mail]>
> *Cc:* Zeng, Oak <[e-mail]>; [e-mail]; Kuehling, 
> Felix <[e-mail]>; Kasiviswanathan, Harish 
> <[e-mail]>; Deucher, Alexander 
> <[e-mail]>; Huang, JinHuiEric 
> <[e-mail]>; Koenig, Christian <[e-mail]>
> *Subject:* Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem 
> caching setting
>
> Hi guys,
>
> adding the usual suspects direct. Does anybody of hand know how to 
> check if an architecture supports ioremap_cache()?
>
> For now we only need this on X86, but I would feel better if we don't 
> use an #ifdef here.
>
> Regards,
> Christian.
>
> Am 02.03.21 um 05:12 schrieb kernel test robot:
>
>     Hi Oak,
>
>     Thank you for the patch! Yet something to improve:
>
>     [auto build test ERROR on drm-intel/for-linux-next]
>
>     [also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
>
>     [cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
>
>     [If your patch is applied to the wrong git tree, kindly drop us a note.
>
>     And when submitting patch, we suggest to use '--base' as documented in
>
>     https://git-scm.com/docs/git-format-patch  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793168696%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=p4iynMPvZGknfSGSyZnXV3kLwScMLbPDB8zVsmxhtk0%3D&reserved=0>]
>
>     url:https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FOak-Zeng%2Fdrm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting%2F20210302-064500&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793178689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2sc4jZR3bVRF0xDDqNOtUcNR9qiJMF2ATmCDAX%2BSWrQ%3D&reserved=0>
>
>     base:   git://anongit.freedesktop.org/drm-intel for-linux-next
>
>     config: parisc-randconfig-r012-20210302 (attached as .config)
>
>     compiler: hppa-linux-gcc (GCC) 9.3.0
>
>     reproduce (this is a W=1 build):
>
>              wgethttps://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793178689%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uILcLE%2F24bhSU%2Bo5GmWGAK6s6xDFivP6lrm6JgtM50Y%3D&reserved=0>  -O ~/bin/make.cross
>
>              chmod +x ~/bin/make.cross
>
>              #https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2F225bb3711439ec559dd72ae5af8e62d34ea60a64&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793188685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2TOSPuKEMRcZjMfxO9lxgwFxgXwHqERCOgRednI7OE8%3D&reserved=0>
>
>              git remote add linux-reviewhttps://github.com/0day-ci/linux  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793188685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TlXvs5mxH0RV9qQFaUF2B1LZisTWbnt4hfFd2OC7gGw%3D&reserved=0>
>
>              git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
>
>              git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
>
>              # save the attached .config to linux build tree
>
>              COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
>
>     If you fix the issue, kindly add following tag as appropriate
>
>     Reported-by: kernel test robot<[e-mail]>  <mailto:[e-mail]>
>
>     All errors (new ones prefixed by >>):
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
>
>            95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>
>               |           ^~~~~~~~~~~~~
>
>               |           ioremap_uc
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>            95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>
>               |         ^
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>           379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
>
>               |                 ^
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
>
>         drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>           500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
>
>               |                ^
>
>         cc1: some warnings being treated as errors
>
>     vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c
>
>          74
>
>          75  static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
>
>          76                                struct ttm_resource *mem,
>
>          77                                void **virtual)
>
>          78  {
>
>          79          int ret;
>
>          80          void *addr;
>
>          81
>
>          82          *virtual = NULL;
>
>          83          ret = ttm_mem_io_reserve(bdev, mem);
>
>          84          if (ret || !mem->bus.is_iomem)
>
>          85                  return ret;
>
>          86
>
>          87          if (mem->bus.addr) {
>
>          88                  addr = mem->bus.addr;
>
>          89          } else {
>
>          90                  size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
>
>          91
>
>          92                  if (mem->bus.caching == ttm_write_combined)
>
>          93                         addr = ioremap_wc(mem->bus.offset, bus_size);
>
>          94                  else if (mem->bus.caching == ttm_cached)
>
>        > 95                         addr = ioremap_cache(mem->bus.offset, bus_size);
>
>          96                  else
>
>          97                         addr = ioremap(mem->bus.offset, bus_size);
>
>          98                  if (!addr) {
>
>          99                         ttm_mem_io_free(bdev, mem);
>
>         100                         return -ENOMEM;
>
>         101                  }
>
>         102          }
>
>         103          *virtual = addr;
>
>         104          return 0;
>
>         105  }
>
>         106
>
>     ---
>
>     0-DAY CI Kernel Test Service, Intel Corporation
>
>     https://lists.01.org/hyperkitty/list/[e-mail]  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793198680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8vaxBMiRPv5mgZyKjDnsNtzAz%2BDltnzlkUMDMg45%2BrI%3D&reserved=0>
>
>
>
>     _______________________________________________
>
>     amd-gfx mailing list
>
>     [e-mail]  <mailto:[e-mail]>
>
>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7COak.Zeng%40amd.com%7C08f51e87e36c4de858bc08d8dd6eb16b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637502814793198680%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eRpwM7V33brBQtOxnkAfPTlp383tcA1x55zPPw1W0gA%3D&reserved=0>
>

Zeng, Oak March 3, 2021, 8:59 p.m. UTC | #9

[AMD Official Use Only - Internal Distribution Only]

Hi Christian,

Can you explain why __iomem annotation is mandatory for amdgpu driver? If this is the case, we can't switch to memremap. The only fix seems to me is add a #ifdef __x86_64__ to the ioremap_cache codes.

Regards,
Oak

From: Christian König <[e-mail]>
Sent: Wednesday, March 3, 2021 5:46 AM
To: Zeng, Oak <[e-mail]>; [e-mail]; [e-mail]; Daniel Vetter <[e-mail]>; Dave Airlie <[e-mail]>; Thomas Hellström (Intel) <[e-mail]>; [e-mail]
Cc: [e-mail]; Kuehling, Felix <[e-mail]>; Kasiviswanathan, Harish <[e-mail]>; Deucher, Alexander <[e-mail]>; Huang, JinHuiEric <[e-mail]>; Koenig, Christian <[e-mail]>
Subject: Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

Hi Oak,



config: parisc-randconfig-r012-20210302 (attached as .config)

It's not the Intel driver build which fails here, but the build bot is just hosted by Intel.

The problem is that the parisc architecture doesn't defines the ioremap_cache() function.

I've looked at using memremap() instead of ioremap_cache(). The problem is that we do support architectures with the TTM as well as amndgpu code where the __iomem annotation is mandatory and correct.

Regards,
Christian.
Am 02.03.21 um 23:45 schrieb Zeng, Oak:

[AMD Official Use Only - Internal Distribution Only]

Hi Daniel, Thomas, Dan,

Does below message mean the calling ioremap_cache failed intel's driver build? I can see both ioremap_cache and ioremap_wc are defined in arch/x86/mm/ioremap.c - why ioremap_wc doesn't break intel driver's build?

Are we supposed to use memremap (offset, size, MEMREMAP_WB) to replace ioremap_cache? When I read here https://lwn.net/Articles/653585/<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F653585%2F&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624296472%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ljCFrIfrYbb%2FXmKKS2TJ7dSQ7oCRNWoUhWS4gEBv%2FW4%3D&reserved=0> I felt that ioremap_cache returns an address annotated with _iomem while memremap returns an address without __iomem annotation. In our use case, GPU memory is treated as UEFI SPM (specific purpose memory). I am not very sure whether memremap (thus no __iomem annotation) is the right thing to do. What I am sure is, we have tested ioremap_cache and it works fine on AMD system.

I will send out a test patch replacing ioremap_cache with ioremap_wc, to trigger Intel build robot to see whether it fails Intel build. I suppose it will not fail Intel build.

Regards,
Oak

From: Christian König <[e-mail]><mailto:[e-mail]>
Sent: Tuesday, March 2, 2021 6:31 AM
To: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; Daniel Vetter <[e-mail]><mailto:[e-mail]>; Dave Airlie <[e-mail]><mailto:[e-mail]>; Thomas Hellström (Intel) <[e-mail]><mailto:[e-mail]>
Cc: Zeng, Oak <[e-mail]><mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; Kuehling, Felix <[e-mail]><mailto:[e-mail]>; Kasiviswanathan, Harish <[e-mail]><mailto:[e-mail]>; Deucher, Alexander <[e-mail]><mailto:[e-mail]>; Huang, JinHuiEric <[e-mail]><mailto:[e-mail]>; Koenig, Christian <[e-mail]><mailto:[e-mail]>
Subject: Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem caching setting

Hi guys,

adding the usual suspects direct. Does anybody of hand know how to check if an architecture supports ioremap_cache()?

For now we only need this on X86, but I would feel better if we don't use an #ifdef here.

Regards,
Christian.
Am 02.03.21 um 05:12 schrieb kernel test robot:

Hi Oak,



Thank you for the patch! Yet something to improve:



[auto build test ERROR on drm-intel/for-linux-next]

[also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]

[cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]

[If your patch is applied to the wrong git tree, kindly drop us a note.

And when submitting patch, we suggest to use '--base' as documented in

https://git-scm.com/docs/git-format-patch<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3f5ib%2FlZ6DXXF%2Bk1rXPKGu1IkOhXHdkUmX3obtuIRtA%3D&reserved=0>]



url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FOak-Zeng%2Fdrm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting%2F20210302-064500&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y9h9ZHM7d2Sqo3XQWr%2Frpb2DAk9SkHFIm5nD4ABFNbE%3D&reserved=0>

base:   git://anongit.freedesktop.org/drm-intel for-linux-next

config: parisc-randconfig-r012-20210302 (attached as .config)

compiler: hppa-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):

        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wGo3Nwk4mWZjb2W3E7qKf3FtKzLOxCVmQu1zd2rmzRU%3D&reserved=0> -O ~/bin/make.cross

        chmod +x ~/bin/make.cross

        # https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2F225bb3711439ec559dd72ae5af8e62d34ea60a64&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=kOz18UAawmA4SZ6IrCGRt4HISfsmmjfGN2TUgAtm0Ak%3D&reserved=0>

        git remote add linux-review https://github.com/0day-ci/linux<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=QnKs47gjzAz%2Fjv%2FAO6H%2BwYNQF6mdk1D8dCNveW0YHuM%3D&reserved=0>

        git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500

        git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64

        # save the attached .config to linux build tree

        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc



If you fix the issue, kindly add following tag as appropriate

Reported-by: kernel test robot <[e-mail]><mailto:[e-mail]>



All errors (new ones prefixed by >>):



   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':

drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]

      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);

         |           ^~~~~~~~~~~~~

         |           ioremap_uc

   drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

      95 |    addr = ioremap_cache(mem->bus.offset, bus_size);

         |         ^

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':

   drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

     379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,

         |                 ^

   drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':

   drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

     500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,

         |                ^

   cc1: some warnings being treated as errors





vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c



    74

    75  static int ttm_resource_ioremap(struct ttm_bo_device *bdev,

    76                                struct ttm_resource *mem,

    77                                void **virtual)

    78  {

    79          int ret;

    80          void *addr;

    81

    82          *virtual = NULL;

    83          ret = ttm_mem_io_reserve(bdev, mem);

    84          if (ret || !mem->bus.is_iomem)

    85                  return ret;

    86

    87          if (mem->bus.addr) {

    88                  addr = mem->bus.addr;

    89          } else {

    90                  size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;

    91

    92                  if (mem->bus.caching == ttm_write_combined)

    93                         addr = ioremap_wc(mem->bus.offset, bus_size);

    94                  else if (mem->bus.caching == ttm_cached)

  > 95                         addr = ioremap_cache(mem->bus.offset, bus_size);

    96                  else

    97                         addr = ioremap(mem->bus.offset, bus_size);

    98                  if (!addr) {

    99                         ttm_mem_io_free(bdev, mem);

   100                         return -ENOMEM;

   101                  }

   102          }

   103          *virtual = addr;

   104          return 0;

   105  }

   106



---

0-DAY CI Kernel Test Service, Intel Corporation

https://lists.01.org/hyperkitty/list/[e-mail]<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MY6tEGVY0X4VyS750dYmsfVoR71Zp78YdxuqqRljnsI%3D&reserved=0>

Christian König March 4, 2021, 7:46 a.m. UTC | #10

Hi Oak,

as far as I know some architectures like PowerPC/ARM/MIPS need that. And 
we at least officially support PowerPC and ARM and MIPS is best effort 
and shouldn't break if possible.

Thomas just recently had a whole bunch of DMA-buf patches to also fix 
that up for DMA-bufs vmap as well, previously we have just been casting 
away the __iomem and hoped that it always works.

Regards,
Christian.

Am 03.03.21 um 21:59 schrieb Zeng, Oak:
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
> Hi Christian,
>
> Can you explain why __iomem annotation is mandatory for amdgpu driver? 
> If this is the case, we can’t switch to memremap. The only fix seems 
> to me is add a #ifdef __x86_64__ to the ioremap_cache codes.
>
> Regards,
>
> Oak
>
> *From:* Christian König <[e-mail]>
> *Sent:* Wednesday, March 3, 2021 5:46 AM
> *To:* Zeng, Oak <[e-mail]>; [e-mail]; 
> [e-mail]; Daniel Vetter <[e-mail]>; Dave 
> Airlie <[e-mail]>; Thomas Hellström (Intel) 
> <[e-mail]>; [e-mail]
> *Cc:* [e-mail]; Kuehling, Felix 
> <[e-mail]>; Kasiviswanathan, Harish 
> <[e-mail]>; Deucher, Alexander 
> <[e-mail]>; Huang, JinHuiEric 
> <[e-mail]>; Koenig, Christian <[e-mail]>
> *Subject:* Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem 
> caching setting
>
> Hi Oak,
>
>
>     config: parisc-randconfig-r012-20210302 (attached as .config)
>
>
> It's not the Intel driver build which fails here, but the build bot is 
> just hosted by Intel.
>
> The problem is that the parisc architecture doesn't defines the 
> ioremap_cache() function.
>
> I've looked at using memremap() instead of ioremap_cache(). The 
> problem is that we do support architectures with the TTM as well as 
> amndgpu code where the __iomem annotation is mandatory and correct.
>
> Regards,
> Christian.
>
> Am 02.03.21 um 23:45 schrieb Zeng, Oak:
>
>     [AMD Official Use Only - Internal Distribution Only]
>
>     Hi Daniel, Thomas, Dan,
>
>     Does below message mean the calling ioremap_cache failed intel’s
>     driver build? I can see both ioremap_cache and ioremap_wc are
>     defined in arch/x86/mm/ioremap.c – why ioremap_wc doesn’t break
>     intel driver’s build?
>
>     Are we supposed to use memremap (offset, size, MEMREMAP_WB) to
>     replace ioremap_cache? When I read here
>     https://lwn.net/Articles/653585/
>     <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F653585%2F&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624296472%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ljCFrIfrYbb%2FXmKKS2TJ7dSQ7oCRNWoUhWS4gEBv%2FW4%3D&reserved=0>
>     I felt that ioremap_cache returns an address annotated with _iomem
>     while memremap returns an address without __iomem annotation. In
>     our use case, GPU memory is treated as UEFI SPM (specific purpose
>     memory). I am not very sure whether memremap (thus no __iomem
>     annotation) is the right thing to do. What I am sure is, we have
>     tested ioremap_cache and it works fine on AMD system.
>
>     I will send out a test patch replacing ioremap_cache with
>     ioremap_wc, to trigger Intel build robot to see whether it fails
>     Intel build. I suppose it will not fail Intel build.
>
>     Regards,
>
>     Oak
>
>     *From:* Christian König <[e-mail]>
>     <mailto:[e-mail]>
>     *Sent:* Tuesday, March 2, 2021 6:31 AM
>     *To:* [e-mail]
>     <mailto:[e-mail]>;
>     [e-mail]
>     <mailto:[e-mail]>; Daniel Vetter
>     <[e-mail]> <mailto:[e-mail]>; Dave Airlie
>     <[e-mail]> <mailto:[e-mail]>; Thomas Hellström
>     (Intel) <[e-mail]> <mailto:[e-mail]>
>     *Cc:* Zeng, Oak <[e-mail]> <mailto:[e-mail]>;
>     [e-mail] <mailto:[e-mail]>;
>     Kuehling, Felix <[e-mail]>
>     <mailto:[e-mail]>; Kasiviswanathan, Harish
>     <[e-mail]>
>     <mailto:[e-mail]>; Deucher, Alexander
>     <[e-mail]> <mailto:[e-mail]>;
>     Huang, JinHuiEric <[e-mail]>
>     <mailto:[e-mail]>; Koenig, Christian
>     <[e-mail]> <mailto:[e-mail]>
>     *Subject:* Re: [PATCH] drm/ttm: ioremap buffer according to TTM
>     mem caching setting
>
>     Hi guys,
>
>     adding the usual suspects direct. Does anybody of hand know how to
>     check if an architecture supports ioremap_cache()?
>
>     For now we only need this on X86, but I would feel better if we
>     don't use an #ifdef here.
>
>     Regards,
>     Christian.
>
>     Am 02.03.21 um 05:12 schrieb kernel test robot:
>
>         Hi Oak,
>
>           
>
>         Thank you for the patch! Yet something to improve:
>
>           
>
>         [auto build test ERROR on drm-intel/for-linux-next]
>
>         [also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
>
>         [cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
>
>         [If your patch is applied to the wrong git tree, kindly drop us a note.
>
>         And when submitting patch, we suggest to use '--base' as documented in
>
>         https://git-scm.com/docs/git-format-patch  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3f5ib%2FlZ6DXXF%2Bk1rXPKGu1IkOhXHdkUmX3obtuIRtA%3D&reserved=0>]
>
>           
>
>         url:https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FOak-Zeng%2Fdrm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting%2F20210302-064500&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y9h9ZHM7d2Sqo3XQWr%2Frpb2DAk9SkHFIm5nD4ABFNbE%3D&reserved=0>
>
>         base:   git://anongit.freedesktop.org/drm-intel for-linux-next
>
>         config: parisc-randconfig-r012-20210302 (attached as .config)
>
>         compiler: hppa-linux-gcc (GCC) 9.3.0
>
>         reproduce (this is a W=1 build):
>
>                  wgethttps://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wGo3Nwk4mWZjb2W3E7qKf3FtKzLOxCVmQu1zd2rmzRU%3D&reserved=0>  -O ~/bin/make.cross
>
>                  chmod +x ~/bin/make.cross
>
>                  #https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2F225bb3711439ec559dd72ae5af8e62d34ea60a64&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=kOz18UAawmA4SZ6IrCGRt4HISfsmmjfGN2TUgAtm0Ak%3D&reserved=0>
>
>                  git remote add linux-reviewhttps://github.com/0day-ci/linux  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=QnKs47gjzAz%2Fjv%2FAO6H%2BwYNQF6mdk1D8dCNveW0YHuM%3D&reserved=0>
>
>                  git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
>
>                  git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
>
>                  # save the attached .config to linux build tree
>
>                  COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
>
>           
>
>         If you fix the issue, kindly add following tag as appropriate
>
>         Reported-by: kernel test robot<[e-mail]>  <mailto:[e-mail]>
>
>           
>
>         All errors (new ones prefixed by >>):
>
>           
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
>
>                 drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
>
>                95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>
>                   |           ^~~~~~~~~~~~~
>
>                   |           ioremap_uc
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>                95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
>
>                   |         ^
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>               379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
>
>                   |                 ^
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
>
>             drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>
>               500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
>
>                   |                ^
>
>             cc1: some warnings being treated as errors
>
>           
>
>           
>
>         vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c
>
>           
>
>              74
>
>              75  static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
>
>              76                                struct ttm_resource *mem,
>
>              77                                void **virtual)
>
>              78  {
>
>              79          int ret;
>
>              80          void *addr;
>
>              81
>
>              82          *virtual = NULL;
>
>              83          ret = ttm_mem_io_reserve(bdev, mem);
>
>              84          if (ret || !mem->bus.is_iomem)
>
>              85                  return ret;
>
>              86
>
>              87          if (mem->bus.addr) {
>
>              88                  addr = mem->bus.addr;
>
>              89          } else {
>
>              90                  size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
>
>              91
>
>              92                  if (mem->bus.caching == ttm_write_combined)
>
>              93                         addr = ioremap_wc(mem->bus.offset, bus_size);
>
>              94                  else if (mem->bus.caching == ttm_cached)
>
>            > 95                         addr = ioremap_cache(mem->bus.offset, bus_size);
>
>              96                  else
>
>              97                         addr = ioremap(mem->bus.offset, bus_size);
>
>              98                  if (!addr) {
>
>              99                         ttm_mem_io_free(bdev, mem);
>
>             100                         return -ENOMEM;
>
>             101                  }
>
>             102          }
>
>             103          *virtual = addr;
>
>             104          return 0;
>
>             105  }
>
>             106
>
>           
>
>         ---
>
>         0-DAY CI Kernel Test Service, Intel Corporation
>
>         https://lists.01.org/hyperkitty/list/[e-mail]  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MY6tEGVY0X4VyS750dYmsfVoR71Zp78YdxuqqRljnsI%3D&reserved=0>
>
>
>
>
>         _______________________________________________
>
>         amd-gfx mailing list
>
>         [e-mail]  <mailto:[e-mail]>
>
>         https://lists.freedesktop.org/mailman/listinfo/amd-gfx  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624336455%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Or3tWmS5M6mXOaFP4FHsEbg0rqoizNGpTl6JEkD6fac%3D&reserved=0>
>

Daniel Vetter March 11, 2021, 1:06 p.m. UTC | #11

On Thu, Mar 04, 2021 at 08:46:26AM +0100, Christian König wrote:
> Hi Oak,
> 
> as far as I know some architectures like PowerPC/ARM/MIPS need that. And we
> at least officially support PowerPC and ARM and MIPS is best effort and
> shouldn't break if possible.
> 
> Thomas just recently had a whole bunch of DMA-buf patches to also fix that
> up for DMA-bufs vmap as well, previously we have just been casting away the
> __iomem and hoped that it always works.

On that, rolling out struct dma_buf_map to ttm and drivers I think would
clean up a lot of this __iomem handling rather nicely.
-Daniel

> 
> Regards,
> Christian.
> 
> Am 03.03.21 um 21:59 schrieb Zeng, Oak:
> > 
> > [AMD Official Use Only - Internal Distribution Only]
> > 
> > 
> > Hi Christian,
> > 
> > Can you explain why __iomem annotation is mandatory for amdgpu driver?
> > If this is the case, we can’t switch to memremap. The only fix seems to
> > me is add a #ifdef __x86_64__ to the ioremap_cache codes.
> > 
> > Regards,
> > 
> > Oak
> > 
> > *From:* Christian König <[e-mail]>
> > *Sent:* Wednesday, March 3, 2021 5:46 AM
> > *To:* Zeng, Oak <[e-mail]>; [e-mail];
> > [e-mail]; Daniel Vetter <[e-mail]>; Dave
> > Airlie <[e-mail]>; Thomas Hellström (Intel)
> > <[e-mail]>; [e-mail]
> > *Cc:* [e-mail]; Kuehling, Felix <[e-mail]>;
> > Kasiviswanathan, Harish <[e-mail]>; Deucher,
> > Alexander <[e-mail]>; Huang, JinHuiEric
> > <[e-mail]>; Koenig, Christian <[e-mail]>
> > *Subject:* Re: [PATCH] drm/ttm: ioremap buffer according to TTM mem
> > caching setting
> > 
> > Hi Oak,
> > 
> > 
> >     config: parisc-randconfig-r012-20210302 (attached as .config)
> > 
> > 
> > It's not the Intel driver build which fails here, but the build bot is
> > just hosted by Intel.
> > 
> > The problem is that the parisc architecture doesn't defines the
> > ioremap_cache() function.
> > 
> > I've looked at using memremap() instead of ioremap_cache(). The problem
> > is that we do support architectures with the TTM as well as amndgpu code
> > where the __iomem annotation is mandatory and correct.
> > 
> > Regards,
> > Christian.
> > 
> > Am 02.03.21 um 23:45 schrieb Zeng, Oak:
> > 
> >     [AMD Official Use Only - Internal Distribution Only]
> > 
> >     Hi Daniel, Thomas, Dan,
> > 
> >     Does below message mean the calling ioremap_cache failed intel’s
> >     driver build? I can see both ioremap_cache and ioremap_wc are
> >     defined in arch/x86/mm/ioremap.c – why ioremap_wc doesn’t break
> >     intel driver’s build?
> > 
> >     Are we supposed to use memremap (offset, size, MEMREMAP_WB) to
> >     replace ioremap_cache? When I read here
> >     https://lwn.net/Articles/653585/
> >     <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F653585%2F&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624296472%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ljCFrIfrYbb%2FXmKKS2TJ7dSQ7oCRNWoUhWS4gEBv%2FW4%3D&reserved=0>
> >     I felt that ioremap_cache returns an address annotated with _iomem
> >     while memremap returns an address without __iomem annotation. In
> >     our use case, GPU memory is treated as UEFI SPM (specific purpose
> >     memory). I am not very sure whether memremap (thus no __iomem
> >     annotation) is the right thing to do. What I am sure is, we have
> >     tested ioremap_cache and it works fine on AMD system.
> > 
> >     I will send out a test patch replacing ioremap_cache with
> >     ioremap_wc, to trigger Intel build robot to see whether it fails
> >     Intel build. I suppose it will not fail Intel build.
> > 
> >     Regards,
> > 
> >     Oak
> > 
> >     *From:* Christian König <[e-mail]>
> >     <mailto:[e-mail]>
> >     *Sent:* Tuesday, March 2, 2021 6:31 AM
> >     *To:* [e-mail]
> >     <mailto:[e-mail]>;
> >     [e-mail]
> >     <mailto:[e-mail]>; Daniel Vetter
> >     <[e-mail]> <mailto:[e-mail]>; Dave Airlie
> >     <[e-mail]> <mailto:[e-mail]>; Thomas Hellström
> >     (Intel) <[e-mail]> <mailto:[e-mail]>
> >     *Cc:* Zeng, Oak <[e-mail]> <mailto:[e-mail]>;
> >     [e-mail] <mailto:[e-mail]>;
> >     Kuehling, Felix <[e-mail]>
> >     <mailto:[e-mail]>; Kasiviswanathan, Harish
> >     <[e-mail]>
> >     <mailto:[e-mail]>; Deucher, Alexander
> >     <[e-mail]> <mailto:[e-mail]>;
> >     Huang, JinHuiEric <[e-mail]>
> >     <mailto:[e-mail]>; Koenig, Christian
> >     <[e-mail]> <mailto:[e-mail]>
> >     *Subject:* Re: [PATCH] drm/ttm: ioremap buffer according to TTM
> >     mem caching setting
> > 
> >     Hi guys,
> > 
> >     adding the usual suspects direct. Does anybody of hand know how to
> >     check if an architecture supports ioremap_cache()?
> > 
> >     For now we only need this on X86, but I would feel better if we
> >     don't use an #ifdef here.
> > 
> >     Regards,
> >     Christian.
> > 
> >     Am 02.03.21 um 05:12 schrieb kernel test robot:
> > 
> >         Hi Oak,
> > 
> > 
> >         Thank you for the patch! Yet something to improve:
> > 
> > 
> >         [auto build test ERROR on drm-intel/for-linux-next]
> > 
> >         [also build test ERROR on drm-tip/drm-tip linus/master v5.12-rc1 next-20210302]
> > 
> >         [cannot apply to tegra-drm/drm/tegra/for-next drm-exynos/exynos-drm-next drm/drm-next]
> > 
> >         [If your patch is applied to the wrong git tree, kindly drop us a note.
> > 
> >         And when submitting patch, we suggest to use '--base' as documented in
> > 
> >         https://git-scm.com/docs/git-format-patch  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3f5ib%2FlZ6DXXF%2Bk1rXPKGu1IkOhXHdkUmX3obtuIRtA%3D&reserved=0>]
> > 
> > 
> >         url:https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FOak-Zeng%2Fdrm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting%2F20210302-064500&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624306464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y9h9ZHM7d2Sqo3XQWr%2Frpb2DAk9SkHFIm5nD4ABFNbE%3D&reserved=0>
> > 
> >         base:   git://anongit.freedesktop.org/drm-intel for-linux-next
> > 
> >         config: parisc-randconfig-r012-20210302 (attached as .config)
> > 
> >         compiler: hppa-linux-gcc (GCC) 9.3.0
> > 
> >         reproduce (this is a W=1 build):
> > 
> >                  wgethttps://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wGo3Nwk4mWZjb2W3E7qKf3FtKzLOxCVmQu1zd2rmzRU%3D&reserved=0>  -O ~/bin/make.cross
> > 
> >                  chmod +x ~/bin/make.cross
> > 
> >                  #https://github.com/0day-ci/linux/commit/225bb3711439ec559dd72ae5af8e62d34ea60a64  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2F225bb3711439ec559dd72ae5af8e62d34ea60a64&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624316464%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=kOz18UAawmA4SZ6IrCGRt4HISfsmmjfGN2TUgAtm0Ak%3D&reserved=0>
> > 
> >                  git remote add linux-reviewhttps://github.com/0day-ci/linux  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=QnKs47gjzAz%2Fjv%2FAO6H%2BwYNQF6mdk1D8dCNveW0YHuM%3D&reserved=0>
> > 
> >                  git fetch --no-tags linux-review Oak-Zeng/drm-ttm-ioremap-buffer-according-to-TTM-mem-caching-setting/20210302-064500
> > 
> >                  git checkout 225bb3711439ec559dd72ae5af8e62d34ea60a64
> > 
> >                  # save the attached .config to linux build tree
> > 
> >                  COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
> > 
> > 
> >         If you fix the issue, kindly add following tag as appropriate
> > 
> >         Reported-by: kernel test robot<[e-mail]>  <mailto:[e-mail]>
> > 
> > 
> >         All errors (new ones prefixed by >>):
> > 
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_resource_ioremap':
> > 
> >                 drivers/gpu/drm/ttm/ttm_bo_util.c:95:11: error: implicit declaration of function 'ioremap_cache'; did you mean 'ioremap_uc'? [-Werror=implicit-function-declaration]
> > 
> >                95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
> > 
> >                   |           ^~~~~~~~~~~~~
> > 
> >                   |           ioremap_uc
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c:95:9: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> > 
> >                95 |    addr = ioremap_cache(mem->bus.offset, bus_size);
> > 
> >                   |         ^
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_ioremap':
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c:379:17: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> > 
> >               379 |    map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
> > 
> >                   |                 ^
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_vmap':
> > 
> >             drivers/gpu/drm/ttm/ttm_bo_util.c:500:16: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> > 
> >               500 |    vaddr_iomem = ioremap_cache(mem->bus.offset,
> > 
> >                   |                ^
> > 
> >             cc1: some warnings being treated as errors
> > 
> > 
> > 
> >         vim +95 drivers/gpu/drm/ttm/ttm_bo_util.c
> > 
> > 
> >              74
> > 
> >              75  static int ttm_resource_ioremap(struct ttm_bo_device *bdev,
> > 
> >              76                                struct ttm_resource *mem,
> > 
> >              77                                void **virtual)
> > 
> >              78  {
> > 
> >              79          int ret;
> > 
> >              80          void *addr;
> > 
> >              81
> > 
> >              82          *virtual = NULL;
> > 
> >              83          ret = ttm_mem_io_reserve(bdev, mem);
> > 
> >              84          if (ret || !mem->bus.is_iomem)
> > 
> >              85                  return ret;
> > 
> >              86
> > 
> >              87          if (mem->bus.addr) {
> > 
> >              88                  addr = mem->bus.addr;
> > 
> >              89          } else {
> > 
> >              90                  size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
> > 
> >              91
> > 
> >              92                  if (mem->bus.caching == ttm_write_combined)
> > 
> >              93                         addr = ioremap_wc(mem->bus.offset, bus_size);
> > 
> >              94                  else if (mem->bus.caching == ttm_cached)
> > 
> >            > 95                         addr = ioremap_cache(mem->bus.offset, bus_size);
> > 
> >              96                  else
> > 
> >              97                         addr = ioremap(mem->bus.offset, bus_size);
> > 
> >              98                  if (!addr) {
> > 
> >              99                         ttm_mem_io_free(bdev, mem);
> > 
> >             100                         return -ENOMEM;
> > 
> >             101                  }
> > 
> >             102          }
> > 
> >             103          *virtual = addr;
> > 
> >             104          return 0;
> > 
> >             105  }
> > 
> >             106
> > 
> > 
> >         ---
> > 
> >         0-DAY CI Kernel Test Service, Intel Corporation
> > 
> >         https://lists.01.org/hyperkitty/list/[e-mail]  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624326458%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MY6tEGVY0X4VyS750dYmsfVoR71Zp78YdxuqqRljnsI%3D&reserved=0>
> > 
> > 
> > 
> > 
> >         _______________________________________________
> > 
> >         amd-gfx mailing list
> > 
> >         [e-mail]  <mailto:[e-mail]>
> > 
> >         https://lists.freedesktop.org/mailman/listinfo/amd-gfx  <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7COak.Zeng%40amd.com%7Cc047ecb316df47cde7ed08d8de3188d4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503651624336455%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Or3tWmS5M6mXOaFP4FHsEbg0rqoizNGpTl6JEkD6fac%3D&reserved=0>
> > 
>

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 031e581..8c65a13 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -91,6 +91,8 @@  static int ttm_resource_ioremap(struct ttm_device *bdev,
 
 		if (mem->bus.caching == ttm_write_combined)
 			addr = ioremap_wc(mem->bus.offset, bus_size);
+		else if (mem->bus.caching == ttm_cached)
+			addr = ioremap_cache(mem->bus.offset, bus_size);
 		else
 			addr = ioremap(mem->bus.offset, bus_size);
 		if (!addr) {
@@ -372,6 +374,9 @@  static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
 		if (mem->bus.caching == ttm_write_combined)
 			map->virtual = ioremap_wc(bo->mem.bus.offset + offset,
 						  size);
+		else if (mem->bus.caching == ttm_cached)
+			map->virtual = ioremap_cache(bo->mem.bus.offset + offset,
+						  size);
 		else
 			map->virtual = ioremap(bo->mem.bus.offset + offset,
 					       size);
@@ -490,6 +495,9 @@  int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
 		else if (mem->bus.caching == ttm_write_combined)
 			vaddr_iomem = ioremap_wc(mem->bus.offset,
 						 bo->base.size);
+		else if (mem->bus.caching == ttm_cached)
+			vaddr_iomem = ioremap_cache(mem->bus.offset,
+						  bo->base.size);
 		else
 			vaddr_iomem = ioremap(mem->bus.offset, bo->base.size);