Re: [PATCH] drm/ttm: cleanup ttm_agp_backend

2021-04-26 Thread Christian König



Am 26.04.21 um 21:36 schrieb Ruhl, Michael J:

-Original Message-
From: dri-devel  On Behalf Of
Christian König
Sent: Monday, April 26, 2021 1:58 PM
To: dri-devel@lists.freedesktop.org
Cc: bske...@redhat.com
Subject: [PATCH] drm/ttm: cleanup ttm_agp_backend

Audit the includes and stop accessing the internal drm_mm_node.

The ttm_resource::start should be the same value as the
drm_mm_node::start.

"should be"?

Are you sure?  


Nope, that's why Ben is on CC. He explicitly changed that for nouveau 
about 10 years ago :)


But I'm pretty sure that this is completely outdated and not necessary 
any more.



If it isn't, is there an issue?

If they are the same (set in ttm_range_man_alloc?), maybe:

The ttm_resource::start is the same value as the drm_mm_node::start.


Yes, as far as I can see that is true for all current code paths. Going 
to update the commit message.




With that change:

Reviewed-by: Michael J. Ruhl 


Thanks,
Christian.



M


Signed-off-by: Christian König 
---
drivers/gpu/drm/ttm/ttm_agp_backend.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c
b/drivers/gpu/drm/ttm/ttm_agp_backend.c
index 0226ae69d3ab..6ddc16f0fe2b 100644
--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
@@ -32,8 +32,9 @@

#define pr_fmt(fmt) "[TTM] " fmt

-#include 
-#include 
+#include 
+#include 
+#include 
#include 
#include 
#include 
@@ -50,7 +51,6 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct
ttm_resource *bo_mem)
{
struct ttm_agp_backend *agp_be = container_of(ttm, struct
ttm_agp_backend, ttm);
struct page *dummy_read_page = ttm_glob.dummy_read_page;
-   struct drm_mm_node *node = bo_mem->mm_node;
struct agp_memory *mem;
int ret, cached = ttm->caching == ttm_cached;
unsigned i;
@@ -76,7 +76,7 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct
ttm_resource *bo_mem)
mem->is_flushed = 1;
mem->type = (cached) ? AGP_USER_CACHED_MEMORY :
AGP_USER_MEMORY;

-   ret = agp_bind_memory(mem, node->start);
+   ret = agp_bind_memory(mem, bo_mem->start);
if (ret)
pr_err("AGP Bind memory failed\n");

--
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: [PATCH] drm/ttm: cleanup ttm_agp_backend

2021-04-26 Thread Ruhl, Michael J
>-Original Message-
>From: dri-devel  On Behalf Of
>Christian König
>Sent: Monday, April 26, 2021 1:58 PM
>To: dri-devel@lists.freedesktop.org
>Cc: bske...@redhat.com
>Subject: [PATCH] drm/ttm: cleanup ttm_agp_backend
>
>Audit the includes and stop accessing the internal drm_mm_node.
>
>The ttm_resource::start should be the same value as the
>drm_mm_node::start.

"should be"?

Are you sure?  

If it isn't, is there an issue?

If they are the same (set in ttm_range_man_alloc?), maybe:

The ttm_resource::start is the same value as the drm_mm_node::start.

With that change:

Reviewed-by: Michael J. Ruhl 

M

>Signed-off-by: Christian König 
>---
> drivers/gpu/drm/ttm/ttm_agp_backend.c | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/ttm/ttm_agp_backend.c
>b/drivers/gpu/drm/ttm/ttm_agp_backend.c
>index 0226ae69d3ab..6ddc16f0fe2b 100644
>--- a/drivers/gpu/drm/ttm/ttm_agp_backend.c
>+++ b/drivers/gpu/drm/ttm/ttm_agp_backend.c
>@@ -32,8 +32,9 @@
>
> #define pr_fmt(fmt) "[TTM] " fmt
>
>-#include 
>-#include 
>+#include 
>+#include 
>+#include 
> #include 
> #include 
> #include 
>@@ -50,7 +51,6 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct
>ttm_resource *bo_mem)
> {
>   struct ttm_agp_backend *agp_be = container_of(ttm, struct
>ttm_agp_backend, ttm);
>   struct page *dummy_read_page = ttm_glob.dummy_read_page;
>-  struct drm_mm_node *node = bo_mem->mm_node;
>   struct agp_memory *mem;
>   int ret, cached = ttm->caching == ttm_cached;
>   unsigned i;
>@@ -76,7 +76,7 @@ int ttm_agp_bind(struct ttm_tt *ttm, struct
>ttm_resource *bo_mem)
>   mem->is_flushed = 1;
>   mem->type = (cached) ? AGP_USER_CACHED_MEMORY :
>AGP_USER_MEMORY;
>
>-  ret = agp_bind_memory(mem, node->start);
>+  ret = agp_bind_memory(mem, bo_mem->start);
>   if (ret)
>   pr_err("AGP Bind memory failed\n");
>
>--
>2.25.1
>
>___
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel