All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/ttm: remove unused placement flags
@ 2016-09-08 13:41 Christian König
  2016-09-08 13:41 ` [PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h Christian König
       [not found] ` <1473342102-324-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 2 replies; 14+ messages in thread
From: Christian König @ 2016-09-08 13:41 UTC (permalink / raw)
  To: dri-devel

From: Christian König <christian.koenig@amd.com>

Either never used or not used in quite a while.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
 include/drm/ttm/ttm_placement.h | 19 -------------------
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index bc37f02..4d2e8f2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
 {
 	int i;
 
-	for (i = 0; i <= TTM_PL_PRIV5; i++)
+	for (i = 0; i <= TTM_PL_PRIV2; i++)
 		if (place->flags & (1 << i)) {
 			*mem_type = i;
 			return 0;
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 8ed44f9..20219d9 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -40,10 +40,6 @@
 #define TTM_PL_PRIV0            3
 #define TTM_PL_PRIV1            4
 #define TTM_PL_PRIV2            5
-#define TTM_PL_PRIV3            6
-#define TTM_PL_PRIV4            7
-#define TTM_PL_PRIV5            8
-#define TTM_PL_SWAPPED          15
 
 #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
 #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
@@ -51,10 +47,6 @@
 #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
 #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
 #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
-#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
-#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
-#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
-#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
 #define TTM_PL_MASK_MEM         0x0000FFFF
 
 /*
@@ -72,7 +64,6 @@
 #define TTM_PL_FLAG_CACHED      (1 << 16)
 #define TTM_PL_FLAG_UNCACHED    (1 << 17)
 #define TTM_PL_FLAG_WC          (1 << 18)
-#define TTM_PL_FLAG_SHARED      (1 << 20)
 #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
 #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
 
@@ -82,14 +73,4 @@
 
 #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
 
-/*
- * Access flags to be used for CPU- and GPU- mappings.
- * The idea is that the TTM synchronization mechanism will
- * allow concurrent READ access and exclusive write access.
- * Currently GPU- and CPU accesses are exclusive.
- */
-
-#define TTM_ACCESS_READ         (1 << 0)
-#define TTM_ACCESS_WRITE        (1 << 1)
-
 #endif
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h
  2016-09-08 13:41 [PATCH 1/2] drm/ttm: remove unused placement flags Christian König
@ 2016-09-08 13:41 ` Christian König
  2016-09-09  6:42   ` zhoucm1
       [not found] ` <1473342102-324-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  1 sibling, 1 reply; 14+ messages in thread
From: Christian König @ 2016-09-08 13:41 UTC (permalink / raw)
  To: dri-devel

From: Christian König <christian.koenig@amd.com>

Makes more sense to keep that together.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 include/drm/ttm/ttm_bo_api.h    | 32 +-------------------------------
 include/drm/ttm/ttm_placement.h | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 97aaf5c..d73c7c2 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -45,37 +45,7 @@ struct ttm_bo_device;
 
 struct drm_mm_node;
 
-/**
- * struct ttm_place
- *
- * @fpfn:	first valid page frame number to put the object
- * @lpfn:	last valid page frame number to put the object
- * @flags:	memory domain and caching flags for the object
- *
- * Structure indicating a possible place to put an object.
- */
-struct ttm_place {
-	unsigned	fpfn;
-	unsigned	lpfn;
-	uint32_t	flags;
-};
-
-/**
- * struct ttm_placement
- *
- * @num_placement:	number of preferred placements
- * @placement:		preferred placements
- * @num_busy_placement:	number of preferred placements when need to evict buffer
- * @busy_placement:	preferred placements when need to evict buffer
- *
- * Structure indicating the placement you request for an object.
- */
-struct ttm_placement {
-	unsigned		num_placement;
-	const struct ttm_place	*placement;
-	unsigned		num_busy_placement;
-	const struct ttm_place	*busy_placement;
-};
+struct ttm_placement;
 
 /**
  * struct ttm_bus_placement
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 20219d9..ff5195c 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -30,6 +30,9 @@
 
 #ifndef _TTM_PLACEMENT_H_
 #define _TTM_PLACEMENT_H_
+
+#include <linux/types.h>
+
 /*
  * Memory regions for data placement.
  */
@@ -73,4 +76,36 @@
 
 #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
 
+/**
+ * struct ttm_place
+ *
+ * @fpfn:	first valid page frame number to put the object
+ * @lpfn:	last valid page frame number to put the object
+ * @flags:	memory domain and caching flags for the object
+ *
+ * Structure indicating a possible place to put an object.
+ */
+struct ttm_place {
+	unsigned	fpfn;
+	unsigned	lpfn;
+	uint32_t	flags;
+};
+
+/**
+ * struct ttm_placement
+ *
+ * @num_placement:	number of preferred placements
+ * @placement:		preferred placements
+ * @num_busy_placement:	number of preferred placements when need to evict buffer
+ * @busy_placement:	preferred placements when need to evict buffer
+ *
+ * Structure indicating the placement you request for an object.
+ */
+struct ttm_placement {
+	unsigned		num_placement;
+	const struct ttm_place	*placement;
+	unsigned		num_busy_placement;
+	const struct ttm_place	*busy_placement;
+};
+
 #endif
-- 
2.5.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found] ` <1473342102-324-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2016-09-09  6:41   ` zhoucm1
       [not found]     ` <57D2598C.1070903-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: zhoucm1 @ 2016-09-09  6:41 UTC (permalink / raw)
  To: Christian König, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Cui, Flora



On 2016年09月08日 21:41, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Either never used or not used in quite a while.
No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2. 
And you cannot make sure there isn't no one using them in other closed 
projects, right?
If you removed now, that obviously will break her implementation and 
brings  her many troubles.


Regards,
David Zhou
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>   include/drm/ttm/ttm_placement.h | 19 -------------------
>   2 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index bc37f02..4d2e8f2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>   {
>   	int i;
>   
> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>   		if (place->flags & (1 << i)) {
>   			*mem_type = i;
>   			return 0;
> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> index 8ed44f9..20219d9 100644
> --- a/include/drm/ttm/ttm_placement.h
> +++ b/include/drm/ttm/ttm_placement.h
> @@ -40,10 +40,6 @@
>   #define TTM_PL_PRIV0            3
>   #define TTM_PL_PRIV1            4
>   #define TTM_PL_PRIV2            5
> -#define TTM_PL_PRIV3            6
> -#define TTM_PL_PRIV4            7
> -#define TTM_PL_PRIV5            8
> -#define TTM_PL_SWAPPED          15
>   
>   #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>   #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
> @@ -51,10 +47,6 @@
>   #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>   #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>   #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>   #define TTM_PL_MASK_MEM         0x0000FFFF
>   
>   /*
> @@ -72,7 +64,6 @@
>   #define TTM_PL_FLAG_CACHED      (1 << 16)
>   #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>   #define TTM_PL_FLAG_WC          (1 << 18)
> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>   #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>   #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>   
> @@ -82,14 +73,4 @@
>   
>   #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>   
> -/*
> - * Access flags to be used for CPU- and GPU- mappings.
> - * The idea is that the TTM synchronization mechanism will
> - * allow concurrent READ access and exclusive write access.
> - * Currently GPU- and CPU accesses are exclusive.
> - */
> -
> -#define TTM_ACCESS_READ         (1 << 0)
> -#define TTM_ACCESS_WRITE        (1 << 1)
> -
>   #endif

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h
  2016-09-08 13:41 ` [PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h Christian König
@ 2016-09-09  6:42   ` zhoucm1
  0 siblings, 0 replies; 14+ messages in thread
From: zhoucm1 @ 2016-09-09  6:42 UTC (permalink / raw)
  To: Christian König, dri-devel



On 2016年09月08日 21:41, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Makes more sense to keep that together.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   include/drm/ttm/ttm_bo_api.h    | 32 +-------------------------------
>   include/drm/ttm/ttm_placement.h | 35 +++++++++++++++++++++++++++++++++++
>   2 files changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 97aaf5c..d73c7c2 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -45,37 +45,7 @@ struct ttm_bo_device;
>   
>   struct drm_mm_node;
>   
> -/**
> - * struct ttm_place
> - *
> - * @fpfn:	first valid page frame number to put the object
> - * @lpfn:	last valid page frame number to put the object
> - * @flags:	memory domain and caching flags for the object
> - *
> - * Structure indicating a possible place to put an object.
> - */
> -struct ttm_place {
> -	unsigned	fpfn;
> -	unsigned	lpfn;
> -	uint32_t	flags;
> -};
> -
> -/**
> - * struct ttm_placement
> - *
> - * @num_placement:	number of preferred placements
> - * @placement:		preferred placements
> - * @num_busy_placement:	number of preferred placements when need to evict buffer
> - * @busy_placement:	preferred placements when need to evict buffer
> - *
> - * Structure indicating the placement you request for an object.
> - */
> -struct ttm_placement {
> -	unsigned		num_placement;
> -	const struct ttm_place	*placement;
> -	unsigned		num_busy_placement;
> -	const struct ttm_place	*busy_placement;
> -};
> +struct ttm_placement;
>   
>   /**
>    * struct ttm_bus_placement
> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> index 20219d9..ff5195c 100644
> --- a/include/drm/ttm/ttm_placement.h
> +++ b/include/drm/ttm/ttm_placement.h
> @@ -30,6 +30,9 @@
>   
>   #ifndef _TTM_PLACEMENT_H_
>   #define _TTM_PLACEMENT_H_
> +
> +#include <linux/types.h>
> +
>   /*
>    * Memory regions for data placement.
>    */
> @@ -73,4 +76,36 @@
>   
>   #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>   
> +/**
> + * struct ttm_place
> + *
> + * @fpfn:	first valid page frame number to put the object
> + * @lpfn:	last valid page frame number to put the object
> + * @flags:	memory domain and caching flags for the object
> + *
> + * Structure indicating a possible place to put an object.
> + */
> +struct ttm_place {
> +	unsigned	fpfn;
> +	unsigned	lpfn;
> +	uint32_t	flags;
> +};
> +
> +/**
> + * struct ttm_placement
> + *
> + * @num_placement:	number of preferred placements
> + * @placement:		preferred placements
> + * @num_busy_placement:	number of preferred placements when need to evict buffer
> + * @busy_placement:	preferred placements when need to evict buffer
> + *
> + * Structure indicating the placement you request for an object.
> + */
> +struct ttm_placement {
> +	unsigned		num_placement;
> +	const struct ttm_place	*placement;
> +	unsigned		num_busy_placement;
> +	const struct ttm_place	*busy_placement;
> +};
> +
>   #endif

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]     ` <57D2598C.1070903-5C7GfCeVMHo@public.gmane.org>
@ 2016-09-09  7:01       ` Flora Cui
  2016-09-09  9:06         ` Christian König
  0 siblings, 1 reply; 14+ messages in thread
From: Flora Cui @ 2016-09-09  7:01 UTC (permalink / raw)
  To: zhoucm1
  Cc: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

yes. please don't do this, I need them.

On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
> 
> 
> On 2016年09月08日 21:41, Christian König wrote:
> >From: Christian König <christian.koenig@amd.com>
> >
> >Either never used or not used in quite a while.
> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2. And
> you cannot make sure there isn't no one using them in other closed projects,
> right?
> If you removed now, that obviously will break her implementation and brings
> her many troubles.
> 
> 
> Regards,
> David Zhou
> >
> >Signed-off-by: Christian König <christian.koenig@amd.com>
> >---
> >  drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
> >  include/drm/ttm/ttm_placement.h | 19 -------------------
> >  2 files changed, 1 insertion(+), 20 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >index bc37f02..4d2e8f2 100644
> >--- a/drivers/gpu/drm/ttm/ttm_bo.c
> >+++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >@@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
> >  {
> >  	int i;
> >-	for (i = 0; i <= TTM_PL_PRIV5; i++)
> >+	for (i = 0; i <= TTM_PL_PRIV2; i++)
> >  		if (place->flags & (1 << i)) {
> >  			*mem_type = i;
> >  			return 0;
> >diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> >index 8ed44f9..20219d9 100644
> >--- a/include/drm/ttm/ttm_placement.h
> >+++ b/include/drm/ttm/ttm_placement.h
> >@@ -40,10 +40,6 @@
> >  #define TTM_PL_PRIV0            3
> >  #define TTM_PL_PRIV1            4
> >  #define TTM_PL_PRIV2            5
> >-#define TTM_PL_PRIV3            6
> >-#define TTM_PL_PRIV4            7
> >-#define TTM_PL_PRIV5            8
> >-#define TTM_PL_SWAPPED          15
> >  #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
> >  #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
> >@@ -51,10 +47,6 @@
> >  #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
> >  #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
> >  #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
> >-#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
> >-#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
> >-#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
> >-#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
> >  #define TTM_PL_MASK_MEM         0x0000FFFF
> >  /*
> >@@ -72,7 +64,6 @@
> >  #define TTM_PL_FLAG_CACHED      (1 << 16)
> >  #define TTM_PL_FLAG_UNCACHED    (1 << 17)
> >  #define TTM_PL_FLAG_WC          (1 << 18)
> >-#define TTM_PL_FLAG_SHARED      (1 << 20)
> >  #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
> >  #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
> >@@ -82,14 +73,4 @@
> >  #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
> >-/*
> >- * Access flags to be used for CPU- and GPU- mappings.
> >- * The idea is that the TTM synchronization mechanism will
> >- * allow concurrent READ access and exclusive write access.
> >- * Currently GPU- and CPU accesses are exclusive.
> >- */
> >-
> >-#define TTM_ACCESS_READ         (1 << 0)
> >-#define TTM_ACCESS_WRITE        (1 << 1)
> >-
> >  #endif
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
  2016-09-09  7:01       ` Flora Cui
@ 2016-09-09  9:06         ` Christian König
       [not found]           ` <79675437-85ae-2fbf-93e1-b49321e6a41e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 14+ messages in thread
From: Christian König @ 2016-09-09  9:06 UTC (permalink / raw)
  To: Flora Cui, zhoucm1; +Cc: amd-gfx, dri-devel

In this case please just add them back in your tree. That should be a 
two liner.

For upstream it certainly doesn't make sense to keep them.

Regards,
Christian.

Am 09.09.2016 um 09:01 schrieb Flora Cui:
> yes. please don't do this, I need them.
>
> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>
>> On 2016年09月08日 21:41, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> Either never used or not used in quite a while.
>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2. And
>> you cannot make sure there isn't no one using them in other closed projects,
>> right?
>> If you removed now, that obviously will break her implementation and brings
>> her many troubles.
>>
>>
>> Regards,
>> David Zhou
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>>>   include/drm/ttm/ttm_placement.h | 19 -------------------
>>>   2 files changed, 1 insertion(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index bc37f02..4d2e8f2 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>>>   {
>>>   	int i;
>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>   		if (place->flags & (1 << i)) {
>>>   			*mem_type = i;
>>>   			return 0;
>>> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
>>> index 8ed44f9..20219d9 100644
>>> --- a/include/drm/ttm/ttm_placement.h
>>> +++ b/include/drm/ttm/ttm_placement.h
>>> @@ -40,10 +40,6 @@
>>>   #define TTM_PL_PRIV0            3
>>>   #define TTM_PL_PRIV1            4
>>>   #define TTM_PL_PRIV2            5
>>> -#define TTM_PL_PRIV3            6
>>> -#define TTM_PL_PRIV4            7
>>> -#define TTM_PL_PRIV5            8
>>> -#define TTM_PL_SWAPPED          15
>>>   #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>>>   #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
>>> @@ -51,10 +47,6 @@
>>>   #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>>>   #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>>>   #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>>>   #define TTM_PL_MASK_MEM         0x0000FFFF
>>>   /*
>>> @@ -72,7 +64,6 @@
>>>   #define TTM_PL_FLAG_CACHED      (1 << 16)
>>>   #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>>>   #define TTM_PL_FLAG_WC          (1 << 18)
>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>>>   #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>>>   #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>>> @@ -82,14 +73,4 @@
>>>   #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>>> -/*
>>> - * Access flags to be used for CPU- and GPU- mappings.
>>> - * The idea is that the TTM synchronization mechanism will
>>> - * allow concurrent READ access and exclusive write access.
>>> - * Currently GPU- and CPU accesses are exclusive.
>>> - */
>>> -
>>> -#define TTM_ACCESS_READ         (1 << 0)
>>> -#define TTM_ACCESS_WRITE        (1 << 1)
>>> -
>>>   #endif
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]           ` <79675437-85ae-2fbf-93e1-b49321e6a41e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2016-09-09 10:21             ` Zhang, Hawking
  2016-09-09 10:35             ` Zhang, Hawking
  1 sibling, 0 replies; 14+ messages in thread
From: Zhang, Hawking @ 2016-09-09 10:21 UTC (permalink / raw)
  To: Christian König, Cui, Flora, Zhou, David(ChunMing)
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Hi Chris,

Removing the flag will make ttm_mem_type_from_place skip counting the corresponding placement and thus have impact on mem region create and bo movement. There is no guarantee that amdgpu would never introduce new memory domain in future. In such case, I'd like to vote for keeping these flags instead of adding them back when amdgpu need to add new memory domain.

As you mentioned that it just a two liner. And there is actually no functionality break with these flags. Then how about keep these flags?

Regards,
Hawking

-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig
Sent: Friday, September 09, 2016 17:07
To: Cui, Flora <Flora.Cui@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags

In this case please just add them back in your tree. That should be a two liner.

For upstream it certainly doesn't make sense to keep them.

Regards,
Christian.

Am 09.09.2016 um 09:01 schrieb Flora Cui:
> yes. please don't do this, I need them.
>
> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>
>> On 2016年09月08日 21:41, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> Either never used or not used in quite a while.
>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2. 
>> And you cannot make sure there isn't no one using them in other 
>> closed projects, right?
>> If you removed now, that obviously will break her implementation and 
>> brings her many troubles.
>>
>>
>> Regards,
>> David Zhou
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>>>   include/drm/ttm/ttm_placement.h | 19 -------------------
>>>   2 files changed, 1 insertion(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>>>   {
>>>   	int i;
>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>   		if (place->flags & (1 << i)) {
>>>   			*mem_type = i;
>>>   			return 0;
>>> diff --git a/include/drm/ttm/ttm_placement.h 
>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
>>> --- a/include/drm/ttm/ttm_placement.h
>>> +++ b/include/drm/ttm/ttm_placement.h
>>> @@ -40,10 +40,6 @@
>>>   #define TTM_PL_PRIV0            3
>>>   #define TTM_PL_PRIV1            4
>>>   #define TTM_PL_PRIV2            5
>>> -#define TTM_PL_PRIV3            6
>>> -#define TTM_PL_PRIV4            7
>>> -#define TTM_PL_PRIV5            8
>>> -#define TTM_PL_SWAPPED          15
>>>   #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>>>   #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
>>> @@ -51,10 +47,6 @@
>>>   #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>>>   #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>>>   #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>>>   #define TTM_PL_MASK_MEM         0x0000FFFF
>>>   /*
>>> @@ -72,7 +64,6 @@
>>>   #define TTM_PL_FLAG_CACHED      (1 << 16)
>>>   #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>>>   #define TTM_PL_FLAG_WC          (1 << 18)
>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>>>   #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>>>   #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>>> @@ -82,14 +73,4 @@
>>>   #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>>> -/*
>>> - * Access flags to be used for CPU- and GPU- mappings.
>>> - * The idea is that the TTM synchronization mechanism will
>>> - * allow concurrent READ access and exclusive write access.
>>> - * Currently GPU- and CPU accesses are exclusive.
>>> - */
>>> -
>>> -#define TTM_ACCESS_READ         (1 << 0)
>>> -#define TTM_ACCESS_WRITE        (1 << 1)
>>> -
>>>   #endif
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]           ` <79675437-85ae-2fbf-93e1-b49321e6a41e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2016-09-09 10:21             ` Zhang, Hawking
@ 2016-09-09 10:35             ` Zhang, Hawking
       [not found]               ` <BN6PR12MB120407ACA888A0E57CA16D7AFCFA0-/b2+HYfkarSdTuMsQheahAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  1 sibling, 1 reply; 14+ messages in thread
From: Zhang, Hawking @ 2016-09-09 10:35 UTC (permalink / raw)
  To: Koenig, Christian, Cui, Flora, Zhou, David(ChunMing)
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Zhang, Hawking

Hi Chris,

Removing the flag will make ttm_mem_type_from_place skip counting the corresponding placement and thus have impact on mem region create and bo movement. There is no guarantee that amdgpu would never introduce new memory domain in future. In such case, I'd like to vote for keeping these flags instead of adding them back when amdgpu need to add new memory domain.

As you mentioned that it just a two liner. And there is actually no functionality break with these flags. Then how about keep these flags?

Regards,
Hawking

-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig
Sent: Friday, September 09, 2016 17:07
To: Cui, Flora <Flora.Cui@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags

In this case please just add them back in your tree. That should be a two liner.

For upstream it certainly doesn't make sense to keep them.

Regards,
Christian.

Am 09.09.2016 um 09:01 schrieb Flora Cui:
> yes. please don't do this, I need them.
>
> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>
>> On 2016年09月08日 21:41, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> Either never used or not used in quite a while.
>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2. 
>> And you cannot make sure there isn't no one using them in other 
>> closed projects, right?
>> If you removed now, that obviously will break her implementation and 
>> brings her many troubles.
>>
>>
>> Regards,
>> David Zhou
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>>>   include/drm/ttm/ttm_placement.h | 19 -------------------
>>>   2 files changed, 1 insertion(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>>>   {
>>>   	int i;
>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>   		if (place->flags & (1 << i)) {
>>>   			*mem_type = i;
>>>   			return 0;
>>> diff --git a/include/drm/ttm/ttm_placement.h 
>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
>>> --- a/include/drm/ttm/ttm_placement.h
>>> +++ b/include/drm/ttm/ttm_placement.h
>>> @@ -40,10 +40,6 @@
>>>   #define TTM_PL_PRIV0            3
>>>   #define TTM_PL_PRIV1            4
>>>   #define TTM_PL_PRIV2            5
>>> -#define TTM_PL_PRIV3            6
>>> -#define TTM_PL_PRIV4            7
>>> -#define TTM_PL_PRIV5            8
>>> -#define TTM_PL_SWAPPED          15
>>>   #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>>>   #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
>>> @@ -51,10 +47,6 @@
>>>   #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>>>   #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>>>   #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>>>   #define TTM_PL_MASK_MEM         0x0000FFFF
>>>   /*
>>> @@ -72,7 +64,6 @@
>>>   #define TTM_PL_FLAG_CACHED      (1 << 16)
>>>   #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>>>   #define TTM_PL_FLAG_WC          (1 << 18)
>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>>>   #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>>>   #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>>> @@ -82,14 +73,4 @@
>>>   #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>>> -/*
>>> - * Access flags to be used for CPU- and GPU- mappings.
>>> - * The idea is that the TTM synchronization mechanism will
>>> - * allow concurrent READ access and exclusive write access.
>>> - * Currently GPU- and CPU accesses are exclusive.
>>> - */
>>> -
>>> -#define TTM_ACCESS_READ         (1 << 0)
>>> -#define TTM_ACCESS_WRITE        (1 << 1)
>>> -
>>>   #endif
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]               ` <BN6PR12MB120407ACA888A0E57CA16D7AFCFA0-/b2+HYfkarSdTuMsQheahAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-09-09 11:24                 ` Christian König
  2016-09-09 13:41                   ` Deucher, Alexander
  2016-09-09 13:54                   ` Emil Velikov
  0 siblings, 2 replies; 14+ messages in thread
From: Christian König @ 2016-09-09 11:24 UTC (permalink / raw)
  To: Zhang, Hawking, Koenig, Christian, Cui, Flora, Zhou,
	David(ChunMing),
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Hi Hawking,

> Removing the flag will make ttm_mem_type_from_place skip counting the corresponding placement and thus have impact on mem region create and bo movement.
And that is exactly the reason why I want to remove the unused flags.

> There is no guarantee that amdgpu would never introduce new memory domain in future.
Irrelevant, if any driver wants to use additional domains it should add 
them when they are used.

BTW: Why would we want to add another TTM domain? I really don't see any 
need for that.

> Then how about keep these flags?
Actually we used to have automated scanners which complain about unused 
code. I'm wondering why they don't detected that earlier.

Anyway any code which isn't used in a while should be removed.

Regards,
Christian.

Am 09.09.2016 um 12:35 schrieb Zhang, Hawking:
> Hi Chris,
>
> Removing the flag will make ttm_mem_type_from_place skip counting the corresponding placement and thus have impact on mem region create and bo movement. There is no guarantee that amdgpu would never introduce new memory domain in future. In such case, I'd like to vote for keeping these flags instead of adding them back when amdgpu need to add new memory domain.
>
> As you mentioned that it just a two liner. And there is actually no functionality break with these flags. Then how about keep these flags?
>
> Regards,
> Hawking
>
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Christian K?nig
> Sent: Friday, September 09, 2016 17:07
> To: Cui, Flora <Flora.Cui@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
>
> In this case please just add them back in your tree. That should be a two liner.
>
> For upstream it certainly doesn't make sense to keep them.
>
> Regards,
> Christian.
>
> Am 09.09.2016 um 09:01 schrieb Flora Cui:
>> yes. please don't do this, I need them.
>>
>> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>> On 2016年09月08日 21:41, Christian König wrote:
>>>> From: Christian König <christian.koenig@amd.com>
>>>>
>>>> Either never used or not used in quite a while.
>>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2.
>>> And you cannot make sure there isn't no one using them in other
>>> closed projects, right?
>>> If you removed now, that obviously will break her implementation and
>>> brings her many troubles.
>>>
>>>
>>> Regards,
>>> David Zhou
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>>>>    include/drm/ttm/ttm_placement.h | 19 -------------------
>>>>    2 files changed, 1 insertion(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>>>>    {
>>>>    	int i;
>>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
>>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>>    		if (place->flags & (1 << i)) {
>>>>    			*mem_type = i;
>>>>    			return 0;
>>>> diff --git a/include/drm/ttm/ttm_placement.h
>>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
>>>> --- a/include/drm/ttm/ttm_placement.h
>>>> +++ b/include/drm/ttm/ttm_placement.h
>>>> @@ -40,10 +40,6 @@
>>>>    #define TTM_PL_PRIV0            3
>>>>    #define TTM_PL_PRIV1            4
>>>>    #define TTM_PL_PRIV2            5
>>>> -#define TTM_PL_PRIV3            6
>>>> -#define TTM_PL_PRIV4            7
>>>> -#define TTM_PL_PRIV5            8
>>>> -#define TTM_PL_SWAPPED          15
>>>>    #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>>>>    #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
>>>> @@ -51,10 +47,6 @@
>>>>    #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>>>>    #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>>>>    #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
>>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
>>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
>>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
>>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>>>>    #define TTM_PL_MASK_MEM         0x0000FFFF
>>>>    /*
>>>> @@ -72,7 +64,6 @@
>>>>    #define TTM_PL_FLAG_CACHED      (1 << 16)
>>>>    #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>>>>    #define TTM_PL_FLAG_WC          (1 << 18)
>>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>>>>    #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>>>>    #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>>>> @@ -82,14 +73,4 @@
>>>>    #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)
>>>> -/*
>>>> - * Access flags to be used for CPU- and GPU- mappings.
>>>> - * The idea is that the TTM synchronization mechanism will
>>>> - * allow concurrent READ access and exclusive write access.
>>>> - * Currently GPU- and CPU accesses are exclusive.
>>>> - */
>>>> -
>>>> -#define TTM_ACCESS_READ         (1 << 0)
>>>> -#define TTM_ACCESS_WRITE        (1 << 1)
>>>> -
>>>>    #endif
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] drm/ttm: remove unused placement flags
  2016-09-09 11:24                 ` Christian König
@ 2016-09-09 13:41                   ` Deucher, Alexander
       [not found]                     ` <MWHPR12MB1694DB49A6A9336A654384C3F7FA0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  2016-09-09 13:54                   ` Emil Velikov
  1 sibling, 1 reply; 14+ messages in thread
From: Deucher, Alexander @ 2016-09-09 13:41 UTC (permalink / raw)
  To: 'Christian König',
	Zhang, Hawking, Koenig, Christian, Cui, Flora, Zhou,
	David(ChunMing),
	amd-gfx
  Cc: dri-devel

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Friday, September 09, 2016 7:24 AM
> To: Zhang, Hawking; Koenig, Christian; Cui, Flora; Zhou, David(ChunMing);
> amd-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
> 
> Hi Hawking,
> 
> > Removing the flag will make ttm_mem_type_from_place skip counting the
> corresponding placement and thus have impact on mem region create and
> bo movement.
> And that is exactly the reason why I want to remove the unused flags.
> 
> > There is no guarantee that amdgpu would never introduce new memory
> domain in future.
> Irrelevant, if any driver wants to use additional domains it should add
> them when they are used.
> 
> BTW: Why would we want to add another TTM domain? I really don't see any
> need for that.

We need it for the hybrid driver in the near feature and the open driver may use it in the future depending on the use cases.  Removing it just makes our lives more difficult for supporting dkms and distro integration for very minimal gain.

Alex

> 
> > Then how about keep these flags?
> Actually we used to have automated scanners which complain about unused
> code. I'm wondering why they don't detected that earlier.
> 
> Anyway any code which isn't used in a while should be removed.
> 
> Regards,
> Christian.
> 
> Am 09.09.2016 um 12:35 schrieb Zhang, Hawking:
> > Hi Chris,
> >
> > Removing the flag will make ttm_mem_type_from_place skip counting the
> corresponding placement and thus have impact on mem region create and
> bo movement. There is no guarantee that amdgpu would never introduce
> new memory domain in future. In such case, I'd like to vote for keeping
> these flags instead of adding them back when amdgpu need to add new
> memory domain.
> >
> > As you mentioned that it just a two liner. And there is actually no
> functionality break with these flags. Then how about keep these flags?
> >
> > Regards,
> > Hawking
> >
> > -----Original Message-----
> > From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Christian K?nig
> > Sent: Friday, September 09, 2016 17:07
> > To: Cui, Flora <Flora.Cui@amd.com>; Zhou, David(ChunMing)
> <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
> >
> > In this case please just add them back in your tree. That should be a two
> liner.
> >
> > For upstream it certainly doesn't make sense to keep them.
> >
> > Regards,
> > Christian.
> >
> > Am 09.09.2016 um 09:01 schrieb Flora Cui:
> >> yes. please don't do this, I need them.
> >>
> >> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
> >>> On 2016年09月08日 21:41, Christian König wrote:
> >>>> From: Christian König <christian.koenig@amd.com>
> >>>>
> >>>> Either never used or not used in quite a while.
> >>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2.
> >>> And you cannot make sure there isn't no one using them in other
> >>> closed projects, right?
> >>> If you removed now, that obviously will break her implementation and
> >>> brings her many troubles.
> >>>
> >>>
> >>> Regards,
> >>> David Zhou
> >>>> Signed-off-by: Christian König <christian.koenig@amd.com>
> >>>> ---
> >>>>    drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
> >>>>    include/drm/ttm/ttm_placement.h | 19 -------------------
> >>>>    2 files changed, 1 insertion(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> >>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
> >>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> >>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const
> struct ttm_place *place,
> >>>>    {
> >>>>    	int i;
> >>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
> >>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
> >>>>    		if (place->flags & (1 << i)) {
> >>>>    			*mem_type = i;
> >>>>    			return 0;
> >>>> diff --git a/include/drm/ttm/ttm_placement.h
> >>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
> >>>> --- a/include/drm/ttm/ttm_placement.h
> >>>> +++ b/include/drm/ttm/ttm_placement.h
> >>>> @@ -40,10 +40,6 @@
> >>>>    #define TTM_PL_PRIV0            3
> >>>>    #define TTM_PL_PRIV1            4
> >>>>    #define TTM_PL_PRIV2            5
> >>>> -#define TTM_PL_PRIV3            6
> >>>> -#define TTM_PL_PRIV4            7
> >>>> -#define TTM_PL_PRIV5            8
> >>>> -#define TTM_PL_SWAPPED          15
> >>>>    #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
> >>>>    #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
> >>>> @@ -51,10 +47,6 @@
> >>>>    #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
> >>>>    #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
> >>>>    #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
> >>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
> >>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
> >>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
> >>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
> >>>>    #define TTM_PL_MASK_MEM         0x0000FFFF
> >>>>    /*
> >>>> @@ -72,7 +64,6 @@
> >>>>    #define TTM_PL_FLAG_CACHED      (1 << 16)
> >>>>    #define TTM_PL_FLAG_UNCACHED    (1 << 17)
> >>>>    #define TTM_PL_FLAG_WC          (1 << 18)
> >>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
> >>>>    #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
> >>>>    #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
> >>>> @@ -82,14 +73,4 @@
> >>>>    #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM |
> TTM_PL_MASK_CACHING)
> >>>> -/*
> >>>> - * Access flags to be used for CPU- and GPU- mappings.
> >>>> - * The idea is that the TTM synchronization mechanism will
> >>>> - * allow concurrent READ access and exclusive write access.
> >>>> - * Currently GPU- and CPU accesses are exclusive.
> >>>> - */
> >>>> -
> >>>> -#define TTM_ACCESS_READ         (1 << 0)
> >>>> -#define TTM_ACCESS_WRITE        (1 << 1)
> >>>> -
> >>>>    #endif
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
  2016-09-09 11:24                 ` Christian König
  2016-09-09 13:41                   ` Deucher, Alexander
@ 2016-09-09 13:54                   ` Emil Velikov
       [not found]                     ` <CACvgo53AAEsxAoQvAe=ffMnQiZiE=MPWne2Abg=rAt4bRkK8NQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 14+ messages in thread
From: Emil Velikov @ 2016-09-09 13:54 UTC (permalink / raw)
  To: Christian König
  Cc: dri-devel, amd-gfx, Cui, Flora, Koenig, Christian, Zhang, Hawking

On 9 September 2016 at 12:24, Christian König <deathsimple@vodafone.de> wrote:
> Hi Hawking,
>
>> Removing the flag will make ttm_mem_type_from_place skip counting the
>> corresponding placement and thus have impact on mem region create and bo
>> movement.
>
> And that is exactly the reason why I want to remove the unused flags.
>
>> There is no guarantee that amdgpu would never introduce new memory domain
>> in future.
>
> Irrelevant, if any driver wants to use additional domains it should add them
> when they are used.
>
> BTW: Why would we want to add another TTM domain? I really don't see any
> need for that.
>
>> Then how about keep these flags?
>
> Actually we used to have automated scanners which complain about unused
> code. I'm wondering why they don't detected that earlier.
>
> Anyway any code which isn't used in a while should be removed.
>
Fwiw I second Christian here. If they are unused in open-source
drivers there's no reason to keep them.
If/as that changes the (newly introduced) user can add back the relevant code.

If closed-source driver(s) use them, then they can keep it as part of
their blob. Upstream kernel does not cater for closed-source drivers,
period.
I realise that's not the answer some are hoping for, so if you want to
question it take it up with Linus and co.

Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]                     ` <MWHPR12MB1694DB49A6A9336A654384C3F7FA0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-09-09 14:17                       ` Christian König
  0 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2016-09-09 14:17 UTC (permalink / raw)
  To: Deucher, Alexander, 'Christian König',
	Zhang, Hawking, Cui, Flora, Zhou, David(ChunMing),
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 09.09.2016 um 15:41 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Christian König
>> Sent: Friday, September 09, 2016 7:24 AM
>> To: Zhang, Hawking; Koenig, Christian; Cui, Flora; Zhou, David(ChunMing);
>> amd-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org
>> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
>>
>> Hi Hawking,
>>
>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>> corresponding placement and thus have impact on mem region create and
>> bo movement.
>> And that is exactly the reason why I want to remove the unused flags.
>>
>>> There is no guarantee that amdgpu would never introduce new memory
>> domain in future.
>> Irrelevant, if any driver wants to use additional domains it should add
>> them when they are used.
>>
>> BTW: Why would we want to add another TTM domain? I really don't see any
>> need for that.
> We need it for the hybrid driver in the near feature and the open driver may use it in the future depending on the use cases.  Removing it just makes our lives more difficult for supporting dkms and distro integration for very minimal gain.

Yeah, agree. The problem is I already did so without knowing that.

I will send out a V2 only removing the stuff we explicitly don't need.

Hopefully nobody will notice and question further,
Christian.

>
> Alex
>
>>> Then how about keep these flags?
>> Actually we used to have automated scanners which complain about unused
>> code. I'm wondering why they don't detected that earlier.
>>
>> Anyway any code which isn't used in a while should be removed.
>>
>> Regards,
>> Christian.
>>
>> Am 09.09.2016 um 12:35 schrieb Zhang, Hawking:
>>> Hi Chris,
>>>
>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>> corresponding placement and thus have impact on mem region create and
>> bo movement. There is no guarantee that amdgpu would never introduce
>> new memory domain in future. In such case, I'd like to vote for keeping
>> these flags instead of adding them back when amdgpu need to add new
>> memory domain.
>>> As you mentioned that it just a two liner. And there is actually no
>> functionality break with these flags. Then how about keep these flags?
>>> Regards,
>>> Hawking
>>>
>>> -----Original Message-----
>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Christian K?nig
>>> Sent: Friday, September 09, 2016 17:07
>>> To: Cui, Flora <Flora.Cui@amd.com>; Zhou, David(ChunMing)
>> <David1.Zhou@amd.com>
>>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
>>> Subject: Re: [PATCH 1/2] drm/ttm: remove unused placement flags
>>>
>>> In this case please just add them back in your tree. That should be a two
>> liner.
>>> For upstream it certainly doesn't make sense to keep them.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 09.09.2016 um 09:01 schrieb Flora Cui:
>>>> yes. please don't do this, I need them.
>>>>
>>>> On Fri, Sep 09, 2016 at 02:41:16PM +0800, zhoucm1 wrote:
>>>>> On 2016年09月08日 21:41, Christian König wrote:
>>>>>> From: Christian König <christian.koenig@amd.com>
>>>>>>
>>>>>> Either never used or not used in quite a while.
>>>>> No, I remember Flora's Direct GMA is using them like GDS use PRIV0-2.
>>>>> And you cannot make sure there isn't no one using them in other
>>>>> closed projects, right?
>>>>> If you removed now, that obviously will break her implementation and
>>>>> brings her many troubles.
>>>>>
>>>>>
>>>>> Regards,
>>>>> David Zhou
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/ttm/ttm_bo.c    |  2 +-
>>>>>>     include/drm/ttm/ttm_placement.h | 19 -------------------
>>>>>>     2 files changed, 1 insertion(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> b/drivers/gpu/drm/ttm/ttm_bo.c index bc37f02..4d2e8f2 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>>>> @@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const
>> struct ttm_place *place,
>>>>>>     {
>>>>>>     	int i;
>>>>>> -	for (i = 0; i <= TTM_PL_PRIV5; i++)
>>>>>> +	for (i = 0; i <= TTM_PL_PRIV2; i++)
>>>>>>     		if (place->flags & (1 << i)) {
>>>>>>     			*mem_type = i;
>>>>>>     			return 0;
>>>>>> diff --git a/include/drm/ttm/ttm_placement.h
>>>>>> b/include/drm/ttm/ttm_placement.h index 8ed44f9..20219d9 100644
>>>>>> --- a/include/drm/ttm/ttm_placement.h
>>>>>> +++ b/include/drm/ttm/ttm_placement.h
>>>>>> @@ -40,10 +40,6 @@
>>>>>>     #define TTM_PL_PRIV0            3
>>>>>>     #define TTM_PL_PRIV1            4
>>>>>>     #define TTM_PL_PRIV2            5
>>>>>> -#define TTM_PL_PRIV3            6
>>>>>> -#define TTM_PL_PRIV4            7
>>>>>> -#define TTM_PL_PRIV5            8
>>>>>> -#define TTM_PL_SWAPPED          15
>>>>>>     #define TTM_PL_FLAG_SYSTEM      (1 << TTM_PL_SYSTEM)
>>>>>>     #define TTM_PL_FLAG_TT          (1 << TTM_PL_TT)
>>>>>> @@ -51,10 +47,6 @@
>>>>>>     #define TTM_PL_FLAG_PRIV0       (1 << TTM_PL_PRIV0)
>>>>>>     #define TTM_PL_FLAG_PRIV1       (1 << TTM_PL_PRIV1)
>>>>>>     #define TTM_PL_FLAG_PRIV2       (1 << TTM_PL_PRIV2)
>>>>>> -#define TTM_PL_FLAG_PRIV3       (1 << TTM_PL_PRIV3)
>>>>>> -#define TTM_PL_FLAG_PRIV4       (1 << TTM_PL_PRIV4)
>>>>>> -#define TTM_PL_FLAG_PRIV5       (1 << TTM_PL_PRIV5)
>>>>>> -#define TTM_PL_FLAG_SWAPPED     (1 << TTM_PL_SWAPPED)
>>>>>>     #define TTM_PL_MASK_MEM         0x0000FFFF
>>>>>>     /*
>>>>>> @@ -72,7 +64,6 @@
>>>>>>     #define TTM_PL_FLAG_CACHED      (1 << 16)
>>>>>>     #define TTM_PL_FLAG_UNCACHED    (1 << 17)
>>>>>>     #define TTM_PL_FLAG_WC          (1 << 18)
>>>>>> -#define TTM_PL_FLAG_SHARED      (1 << 20)
>>>>>>     #define TTM_PL_FLAG_NO_EVICT    (1 << 21)
>>>>>>     #define TTM_PL_FLAG_TOPDOWN     (1 << 22)
>>>>>> @@ -82,14 +73,4 @@
>>>>>>     #define TTM_PL_MASK_MEMTYPE     (TTM_PL_MASK_MEM |
>> TTM_PL_MASK_CACHING)
>>>>>> -/*
>>>>>> - * Access flags to be used for CPU- and GPU- mappings.
>>>>>> - * The idea is that the TTM synchronization mechanism will
>>>>>> - * allow concurrent READ access and exclusive write access.
>>>>>> - * Currently GPU- and CPU accesses are exclusive.
>>>>>> - */
>>>>>> -
>>>>>> -#define TTM_ACCESS_READ         (1 << 0)
>>>>>> -#define TTM_ACCESS_WRITE        (1 << 1)
>>>>>> -
>>>>>>     #endif
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
       [not found]                     ` <CACvgo53AAEsxAoQvAe=ffMnQiZiE=MPWne2Abg=rAt4bRkK8NQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-09-09 14:30                       ` Christian König
  2016-09-09 15:11                         ` Emil Velikov
  0 siblings, 1 reply; 14+ messages in thread
From: Christian König @ 2016-09-09 14:30 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Zhou, David(ChunMing),
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Cui, Flora, Koenig,
	Christian, Zhang, Hawking

Am 09.09.2016 um 15:54 schrieb Emil Velikov:
> On 9 September 2016 at 12:24, Christian König <deathsimple@vodafone.de> wrote:
>> Hi Hawking,
>>
>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>>> corresponding placement and thus have impact on mem region create and bo
>>> movement.
>> And that is exactly the reason why I want to remove the unused flags.
>>
>>> There is no guarantee that amdgpu would never introduce new memory domain
>>> in future.
>> Irrelevant, if any driver wants to use additional domains it should add them
>> when they are used.
>>
>> BTW: Why would we want to add another TTM domain? I really don't see any
>> need for that.
>>
>>> Then how about keep these flags?
>> Actually we used to have automated scanners which complain about unused
>> code. I'm wondering why they don't detected that earlier.
>>
>> Anyway any code which isn't used in a while should be removed.
>>
> Fwiw I second Christian here. If they are unused in open-source
> drivers there's no reason to keep them.
> If/as that changes the (newly introduced) user can add back the relevant code.

Crap to late :( I was about to send a V2 of the patch to keep the PRIV 
flags.

> If closed-source driver(s) use them, then they can keep it as part of
> their blob. Upstream kernel does not cater for closed-source drivers,
> period.
> I realise that's not the answer some are hoping for, so if you want to
> question it take it up with Linus and co.

It's not an issue between closed vs. open, but rather additional work of 
rebasing the open code when we start to use additional domains.

But on the other hand I still haven't seen a good reason for using 
those. As far as I know we have covered all resource in the current and 
next hardware generation with the existing flags.

Regards,
Christian.

> Regards,
> Emil


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] drm/ttm: remove unused placement flags
  2016-09-09 14:30                       ` Christian König
@ 2016-09-09 15:11                         ` Emil Velikov
  0 siblings, 0 replies; 14+ messages in thread
From: Emil Velikov @ 2016-09-09 15:11 UTC (permalink / raw)
  To: Christian König
  Cc: dri-devel, amd-gfx, Cui, Flora, Koenig, Christian, Zhang, Hawking

On 9 September 2016 at 15:30, Christian König <deathsimple@vodafone.de> wrote:
> Am 09.09.2016 um 15:54 schrieb Emil Velikov:
>>
>> On 9 September 2016 at 12:24, Christian König <deathsimple@vodafone.de>
>> wrote:
>>>
>>> Hi Hawking,
>>>
>>>> Removing the flag will make ttm_mem_type_from_place skip counting the
>>>> corresponding placement and thus have impact on mem region create and bo
>>>> movement.
>>>
>>> And that is exactly the reason why I want to remove the unused flags.
>>>
>>>> There is no guarantee that amdgpu would never introduce new memory
>>>> domain
>>>> in future.
>>>
>>> Irrelevant, if any driver wants to use additional domains it should add
>>> them
>>> when they are used.
>>>
>>> BTW: Why would we want to add another TTM domain? I really don't see any
>>> need for that.
>>>
>>>> Then how about keep these flags?
>>>
>>> Actually we used to have automated scanners which complain about unused
>>> code. I'm wondering why they don't detected that earlier.
>>>
>>> Anyway any code which isn't used in a while should be removed.
>>>
>> Fwiw I second Christian here. If they are unused in open-source
>> drivers there's no reason to keep them.
>> If/as that changes the (newly introduced) user can add back the relevant
>> code.
>
>
> Crap to late :( I was about to send a V2 of the patch to keep the PRIV
> flags.
>
Oops, sorry :-)

>> If closed-source driver(s) use them, then they can keep it as part of
>> their blob. Upstream kernel does not cater for closed-source drivers,
>> period.
>> I realise that's not the answer some are hoping for, so if you want to
>> question it take it up with Linus and co.
>
>
> It's not an issue between closed vs. open, but rather additional work of
> rebasing the open code when we start to use additional domains.
>
This should serve as a greater initiative to develop and upstream
things in a gradual manner, no ?
We all get carried away sometimes creating a massive branch which just
cannot go in at once.

> But on the other hand I still haven't seen a good reason for using those. As
> far as I know we have covered all resource in the current and next hardware
> generation with the existing flags.
>
This in itself is a pretty good point as well, considering you know
the hardware fairly well and you've worked in the kernel for quite a
while now.

Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-09-09 15:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-08 13:41 [PATCH 1/2] drm/ttm: remove unused placement flags Christian König
2016-09-08 13:41 ` [PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h Christian König
2016-09-09  6:42   ` zhoucm1
     [not found] ` <1473342102-324-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-09-09  6:41   ` [PATCH 1/2] drm/ttm: remove unused placement flags zhoucm1
     [not found]     ` <57D2598C.1070903-5C7GfCeVMHo@public.gmane.org>
2016-09-09  7:01       ` Flora Cui
2016-09-09  9:06         ` Christian König
     [not found]           ` <79675437-85ae-2fbf-93e1-b49321e6a41e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-09-09 10:21             ` Zhang, Hawking
2016-09-09 10:35             ` Zhang, Hawking
     [not found]               ` <BN6PR12MB120407ACA888A0E57CA16D7AFCFA0-/b2+HYfkarSdTuMsQheahAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-09-09 11:24                 ` Christian König
2016-09-09 13:41                   ` Deucher, Alexander
     [not found]                     ` <MWHPR12MB1694DB49A6A9336A654384C3F7FA0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-09-09 14:17                       ` Christian König
2016-09-09 13:54                   ` Emil Velikov
     [not found]                     ` <CACvgo53AAEsxAoQvAe=ffMnQiZiE=MPWne2Abg=rAt4bRkK8NQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-09-09 14:30                       ` Christian König
2016-09-09 15:11                         ` Emil Velikov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.