2017-12-24 05:50:18

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 00/12] drm: add check if io_mem_pfn is NULL and cleanup

I found an OOPS when I used the mainline kernel for graphical tests in Hisilicon
D05, I do not know how to solve this problem until I saw your discussion on this
issue a month ago:

https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html

And my problem can be solved perfectly by your solution.

This is important for me, I want to solve this problem as soon as possible. So
I follow the result of your discussion, make and send these patches below.

If anything is not good, please point it out, thanks.

Michal Srb (1):
drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

Tan Xiaojun (11):
drm/ast: remove the default io_mem_pfn set
drm/bochs: remove the default io_mem_pfn set
drm/cirrus: remove the default io_mem_pfn set
drm/mgag200: remove the default io_mem_pfn set
drm/nouveau: remove the default io_mem_pfn set
drm/qxl: remove the default io_mem_pfn set
drm/radeon: remove the default io_mem_pfn set
drm/virtio: remove the default io_mem_pfn set
drm/vmwgfx: remove the default io_mem_pfn set
staging: remove the default io_mem_pfn set
drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

drivers/gpu/drm/ast/ast_ttm.c | 1 -
drivers/gpu/drm/bochs/bochs_mm.c | 1 -
drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
drivers/gpu/drm/ttm/ttm_bo_vm.c | 28 +++++++++++++++++++---------
drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
drivers/staging/vboxvideo/vbox_ttm.c | 1 -
include/drm/ttm/ttm_bo_api.h | 11 -----------
12 files changed, 19 insertions(+), 30 deletions(-)

--
2.7.4


2017-12-24 05:50:05

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 02/12] drm/ast: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/ast/ast_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 696a15d..fdd521d 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver ast_bo_driver = {
.verify_access = ast_bo_verify_access,
.io_mem_reserve = &ast_ttm_io_mem_reserve,
.io_mem_free = &ast_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int ast_mm_init(struct ast_private *ast)
--
2.7.4

2017-12-24 05:50:08

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 04/12] drm/cirrus: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1ff1838..2c652af 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
.verify_access = cirrus_bo_verify_access,
.io_mem_reserve = &cirrus_ttm_io_mem_reserve,
.io_mem_free = &cirrus_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int cirrus_mm_init(struct cirrus_device *cirrus)
--
2.7.4

2017-12-24 05:50:26

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 05/12] drm/mgag200: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd..89b550f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
.verify_access = mgag200_bo_verify_access,
.io_mem_reserve = &mgag200_ttm_io_mem_reserve,
.io_mem_free = &mgag200_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int mgag200_mm_init(struct mga_device *mdev)
--
2.7.4

2017-12-24 05:50:42

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 09/12] drm/virtio: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index cd389c5..4a12434 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -431,7 +431,6 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
.verify_access = &virtio_gpu_verify_access,
.io_mem_reserve = &virtio_gpu_ttm_io_mem_reserve,
.io_mem_free = &virtio_gpu_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = &virtio_gpu_bo_move_notify,
.swap_notify = &virtio_gpu_bo_swap_notify,
};
--
2.7.4

2017-12-24 05:50:23

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

From: Michal Srb <[email protected]>

The io_mem_pfn field was added in commit ea642c3216cb2a60d1c0e760ae47ee85c9c16447
and is called unconditionally. However, not all drivers were updated to set it.

Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.

Signed-off-by: Michal Srb <[email protected]>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index c8ebb75..e25a99b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (bo->mem.bus.is_iomem) {
/* Iomem should not be marked encrypted */
cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
- pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+ if (bdev->driver->io_mem_pfn)
+ pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+ else
+ pfn = ttm_bo_default_io_mem_pfn(bo, page_offset);
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
--
2.7.4

2017-12-24 05:50:33

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 06/12] drm/nouveau: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 435ff86..8de82a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1667,5 +1667,4 @@ struct ttm_bo_driver nouveau_bo_driver = {
.fault_reserve_notify = &nouveau_ttm_fault_reserve_notify,
.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
.io_mem_free = &nouveau_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};
--
2.7.4

2017-12-24 05:50:38

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 07/12] drm/qxl: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ab48238..a86eaf9 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -393,7 +393,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.verify_access = &qxl_verify_access,
.io_mem_reserve = &qxl_ttm_io_mem_reserve,
.io_mem_free = &qxl_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = &qxl_bo_move_notify,
};

--
2.7.4

2017-12-24 05:50:30

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 08/12] drm/radeon: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6ada64d..8595c76 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -844,7 +844,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
.io_mem_reserve = &radeon_ttm_io_mem_reserve,
.io_mem_free = &radeon_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int radeon_ttm_init(struct radeon_device *rdev)
--
2.7.4

2017-12-24 05:50:24

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 03/12] drm/bochs: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/bochs/bochs_mm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index c4cadb6..857755a 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -205,7 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
.verify_access = bochs_bo_verify_access,
.io_mem_reserve = &bochs_ttm_io_mem_reserve,
.io_mem_free = &bochs_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int bochs_mm_init(struct bochs_device *bochs)
--
2.7.4

2017-12-24 05:51:38

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 10/12] drm/vmwgfx: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c705632..828dd59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -859,5 +859,4 @@ struct ttm_bo_driver vmw_bo_driver = {
.fault_reserve_notify = &vmw_ttm_fault_reserve_notify,
.io_mem_reserve = &vmw_ttm_io_mem_reserve,
.io_mem_free = &vmw_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};
--
2.7.4

2017-12-24 05:51:53

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 12/12] drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

No one will use this function except in ttm_bo_vm.c now. So unexport it
and make it static.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/gpu/drm/ttm/ttm_bo_vm.c | 23 +++++++++++++++--------
include/drm/ttm/ttm_bo_api.h | 11 -----------
2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index e25a99b..ff70fc96 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -92,6 +92,21 @@ static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
return ret;
}

+/**
+ * ttm_bo_default_iomem_pfn - get a pfn for a page offset
+ *
+ * @bo: the BO we need to look up the pfn for
+ * @page_offset: offset inside the BO to look up.
+ *
+ * Calculate the PFN for iomem based mappings during page fault
+ */
+static unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
+ unsigned long page_offset)
+{
+ return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
+ + page_offset;
+}
+
static int ttm_bo_vm_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
@@ -407,14 +422,6 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_bo_device *bdev,
return bo;
}

-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
- unsigned long page_offset)
-{
- return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
- + page_offset;
-}
-EXPORT_SYMBOL(ttm_bo_default_io_mem_pfn);
-
int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
{
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index fa07be1..0b1ce05 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -711,17 +711,6 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
struct ttm_buffer_object *bo);

/**
- * ttm_bo_default_iomem_pfn - get a pfn for a page offset
- *
- * @bo: the BO we need to look up the pfn for
- * @page_offset: offset inside the BO to look up.
- *
- * Calculate the PFN for iomem based mappings during page fault
- */
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
- unsigned long page_offset);
-
-/**
* ttm_bo_mmap - mmap out of the ttm device address space.
*
* @filp: filp as input from the mmap method.
--
2.7.4

2017-12-24 05:51:55

by Tan Xiaojun

[permalink] [raw]
Subject: [PATCH 11/12] staging: remove the default io_mem_pfn set

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <[email protected]>
---
drivers/staging/vboxvideo/vbox_ttm.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_ttm.c b/drivers/staging/vboxvideo/vbox_ttm.c
index 4eb410a..4da1723 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -241,7 +241,6 @@ static struct ttm_bo_driver vbox_bo_driver = {
.verify_access = vbox_bo_verify_access,
.io_mem_reserve = &vbox_ttm_io_mem_reserve,
.io_mem_free = &vbox_ttm_io_mem_free,
- .io_mem_pfn = ttm_bo_default_io_mem_pfn,
};

int vbox_mm_init(struct vbox_private *vbox)
--
2.7.4

2017-12-24 09:27:14

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

Am 24.12.2017 um 07:14 schrieb Tan Xiaojun:
> From: Michal Srb <[email protected]>
>
> The io_mem_pfn field was added in commit ea642c3216cb2a60d1c0e760ae47ee85c9c16447
> and is called unconditionally. However, not all drivers were updated to set it.
>
> Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.
>
> Signed-off-by: Michal Srb <[email protected]>
> ---
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index c8ebb75..e25a99b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
> if (bo->mem.bus.is_iomem) {
> /* Iomem should not be marked encrypted */
> cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
> - pfn = bdev->driver->io_mem_pfn(bo, page_offset);
> + if (bdev->driver->io_mem_pfn)
> + pfn = bdev->driver->io_mem_pfn(bo, page_offset);
> + else
> + pfn = ttm_bo_default_io_mem_pfn(bo, page_offset);

Please move this check into a new function ttm_bo_io_mem_pfn().

You can then move the calculation of ttm_bo_default_io_mem_pfn() into
this new function in patch #12 as well.

Regards,
Christian.

> } else {
> page = ttm->pages[page_offset];
> if (unlikely(!page && i == 0)) {

2017-12-25 01:24:52

by Tan Xiaojun

[permalink] [raw]
Subject: Re: [PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

On 2017/12/24 17:27, Christian König wrote:
> Am 24.12.2017 um 07:14 schrieb Tan Xiaojun:
>> From: Michal Srb <[email protected]>
>>
>> The io_mem_pfn field was added in commit ea642c3216cb2a60d1c0e760ae47ee85c9c16447
>> and is called unconditionally. However, not all drivers were updated to set it.
>>
>> Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.
>>
>> Signed-off-by: Michal Srb <[email protected]>
>> ---
>> drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index c8ebb75..e25a99b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
>> if (bo->mem.bus.is_iomem) {
>> /* Iomem should not be marked encrypted */
>> cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
>> - pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> + if (bdev->driver->io_mem_pfn)
>> + pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> + else
>> + pfn = ttm_bo_default_io_mem_pfn(bo, page_offset);
>
> Please move this check into a new function ttm_bo_io_mem_pfn().
>
> You can then move the calculation of ttm_bo_default_io_mem_pfn() into this new function in patch #12 as well.
>
> Regards,
> Christian.
>

OK. Thank you for your reply. I will modify it and send v2.

Thanks.
Xiaojun.

>> } else {
>> page = ttm->pages[page_offset];
>> if (unlikely(!page && i == 0)) {
>
>
> .
>