[PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

The following cases are possible for pr_debug():

1. CONFIG_DYNAMIC_DEBUG disabled
   a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
      it never generates any output.
   b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
      i.e. it generates output which doesn't appear in dmesg by default,
      can be enabled dynamically.

2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
   dynamic_pr_debug()
   a) DEBUG not defined: dynamic_pr_debug() generates no output by
      default, can be enabled dynamically.
   b) DEBUG defined: dynamic_pr_debug() generates output by default,
      can be disabled dynamically.

The intention for drm_debug_printer() is to generate output which
doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.

Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/drm_print.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..ee56e4a1b343 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -23,11 +23,13 @@
  * Rob Clark <robdclark@gmail.com>
  */
 
-#define DEBUG /* for pr_debug() */
-
 #include <stdarg.h>
 #include <linux/seq_file.h>
 #include <drm/drmP.h>
+
+#ifndef CONFIG_DYNAMIC_DEBUG
+#define DEBUG /* for pr_debug() */
+#endif
 #include <drm/drm_print.h>
 
 void __drm_puts_coredump(struct drm_printer *p, const char *str)
-- 
2.20.0.rc2

[PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

The following cases are possible for pr_debug():

1. CONFIG_DYNAMIC_DEBUG disabled
   a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
      it never generates any output.
   b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
      i.e. it generates output which doesn't appear in dmesg by default,
      can be enabled dynamically.

2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
   dynamic_pr_debug()
   a) DEBUG not defined: dynamic_pr_debug() generates no output by
      default, can be enabled dynamically.
   b) DEBUG defined: dynamic_pr_debug() generates output by default,
      can be disabled dynamically.

The intention for drm_debug_printer() is to generate output which
doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.

Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/drm_print.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..ee56e4a1b343 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -23,11 +23,13 @@
  * Rob Clark <robdclark@gmail.com>
  */
 
-#define DEBUG /* for pr_debug() */
-
 #include <stdarg.h>
 #include <linux/seq_file.h>
 #include <drm/drmP.h>
+
+#ifndef CONFIG_DYNAMIC_DEBUG
+#define DEBUG /* for pr_debug() */
+#endif
 #include <drm/drm_print.h>
 
 void __drm_puts_coredump(struct drm_printer *p, const char *str)
-- 
2.20.0.rc2

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

[PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

All the output is related, so it should all be printed the same way.
Some of it was using pr_debug, but some of it appeared in dmesg by
default. The caller should handle failure, so there's no need to spam
dmesg with potentially quite a lot of output by default.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 39 ++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d87935bf8e30..5e9b9dd91629 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -77,38 +77,39 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
 	return 0;
 }
 
-static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
+static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p,
+			       int mem_type)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
-	struct drm_printer p = drm_debug_printer(TTM_PFX);
 
-	pr_err("    has_type: %d\n", man->has_type);
-	pr_err("    use_type: %d\n", man->use_type);
-	pr_err("    flags: 0x%08X\n", man->flags);
-	pr_err("    gpu_offset: 0x%08llX\n", man->gpu_offset);
-	pr_err("    size: %llu\n", man->size);
-	pr_err("    available_caching: 0x%08X\n", man->available_caching);
-	pr_err("    default_caching: 0x%08X\n", man->default_caching);
+	drm_printf(p, "    has_type: %d\n", man->has_type);
+	drm_printf(p, "    use_type: %d\n", man->use_type);
+	drm_printf(p, "    flags: 0x%08X\n", man->flags);
+	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
+	drm_printf(p, "    size: %llu\n", man->size);
+	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
+	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
 	if (mem_type != TTM_PL_SYSTEM)
-		(*man->func->debug)(man, &p);
+		(*man->func->debug)(man, p);
 }
 
 static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 					struct ttm_placement *placement)
 {
+	struct drm_printer p = drm_debug_printer(TTM_PFX);
 	int i, ret, mem_type;
 
-	pr_err("No space for %p (%lu pages, %luK, %luM)\n",
-	       bo, bo->mem.num_pages, bo->mem.size >> 10,
-	       bo->mem.size >> 20);
+	drm_printf(&p, "No space for %p (%lu pages, %luK, %luM)\n",
+		   bo, bo->mem.num_pages, bo->mem.size >> 10,
+		   bo->mem.size >> 20);
 	for (i = 0; i < placement->num_placement; i++) {
 		ret = ttm_mem_type_from_place(&placement->placement[i],
 						&mem_type);
 		if (ret)
 			return;
-		pr_err("  placement[%d]=0x%08X (%d)\n",
-		       i, placement->placement[i].flags, mem_type);
-		ttm_mem_type_debug(bo->bdev, mem_type);
+		drm_printf(&p, "  placement[%d]=0x%08X (%d)\n",
+			   i, placement->placement[i].flags, mem_type);
+		ttm_mem_type_debug(bo->bdev, &p, mem_type);
 	}
 }
 
@@ -728,8 +729,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
 	if (ret) {
 		if (ret != -ERESTARTSYS) {
-			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
-			       bo);
+			pr_debug("Failed to find memory space for buffer 0x%p eviction\n",
+				 bo);
 			ttm_bo_mem_space_debug(bo, &placement);
 		}
 		goto out;
@@ -738,7 +739,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
 	if (unlikely(ret)) {
 		if (ret != -ERESTARTSYS)
-			pr_err("Buffer eviction failed\n");
+			pr_debug("Buffer eviction failed\n");
 		ttm_bo_mem_put(bo, &evict_mem);
 		goto out;
 	}
-- 
2.20.0.rc2

[PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

All the output is related, so it should all be printed the same way.
Some of it was using pr_debug, but some of it appeared in dmesg by
default. The caller should handle failure, so there's no need to spam
dmesg with potentially quite a lot of output by default.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 39 ++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d87935bf8e30..5e9b9dd91629 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -77,38 +77,39 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
 	return 0;
 }
 
-static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
+static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p,
+			       int mem_type)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
-	struct drm_printer p = drm_debug_printer(TTM_PFX);
 
-	pr_err("    has_type: %d\n", man->has_type);
-	pr_err("    use_type: %d\n", man->use_type);
-	pr_err("    flags: 0x%08X\n", man->flags);
-	pr_err("    gpu_offset: 0x%08llX\n", man->gpu_offset);
-	pr_err("    size: %llu\n", man->size);
-	pr_err("    available_caching: 0x%08X\n", man->available_caching);
-	pr_err("    default_caching: 0x%08X\n", man->default_caching);
+	drm_printf(p, "    has_type: %d\n", man->has_type);
+	drm_printf(p, "    use_type: %d\n", man->use_type);
+	drm_printf(p, "    flags: 0x%08X\n", man->flags);
+	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
+	drm_printf(p, "    size: %llu\n", man->size);
+	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
+	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
 	if (mem_type != TTM_PL_SYSTEM)
-		(*man->func->debug)(man, &p);
+		(*man->func->debug)(man, p);
 }
 
 static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 					struct ttm_placement *placement)
 {
+	struct drm_printer p = drm_debug_printer(TTM_PFX);
 	int i, ret, mem_type;
 
-	pr_err("No space for %p (%lu pages, %luK, %luM)\n",
-	       bo, bo->mem.num_pages, bo->mem.size >> 10,
-	       bo->mem.size >> 20);
+	drm_printf(&p, "No space for %p (%lu pages, %luK, %luM)\n",
+		   bo, bo->mem.num_pages, bo->mem.size >> 10,
+		   bo->mem.size >> 20);
 	for (i = 0; i < placement->num_placement; i++) {
 		ret = ttm_mem_type_from_place(&placement->placement[i],
 						&mem_type);
 		if (ret)
 			return;
-		pr_err("  placement[%d]=0x%08X (%d)\n",
-		       i, placement->placement[i].flags, mem_type);
-		ttm_mem_type_debug(bo->bdev, mem_type);
+		drm_printf(&p, "  placement[%d]=0x%08X (%d)\n",
+			   i, placement->placement[i].flags, mem_type);
+		ttm_mem_type_debug(bo->bdev, &p, mem_type);
 	}
 }
 
@@ -728,8 +729,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
 	if (ret) {
 		if (ret != -ERESTARTSYS) {
-			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
-			       bo);
+			pr_debug("Failed to find memory space for buffer 0x%p eviction\n",
+				 bo);
 			ttm_bo_mem_space_debug(bo, &placement);
 		}
 		goto out;
@@ -738,7 +739,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
 	if (unlikely(ret)) {
 		if (ret != -ERESTARTSYS)
-			pr_err("Buffer eviction failed\n");
+			pr_debug("Buffer eviction failed\n");
 		ttm_bo_mem_put(bo, &evict_mem);
 		goto out;
 	}
-- 
2.20.0.rc2

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

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Zhang, Jerry(Junwei)]

On 12/6/18 12:56 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> The following cases are possible for pr_debug():
>
> 1. CONFIG_DYNAMIC_DEBUG disabled
>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>        it never generates any output.
>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>        i.e. it generates output which doesn't appear in dmesg by default,
>        can be enabled dynamically.
>
> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>     dynamic_pr_debug()
>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>        default, can be enabled dynamically.
>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>        can be disabled dynamically.
>
> The intention for drm_debug_printer() is to generate output which
> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>
> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/drm_print.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..ee56e4a1b343 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -23,11 +23,13 @@
>    * Rob Clark <robdclark@gmail.com>
>    */
>   
> -#define DEBUG /* for pr_debug() */
> -
>   #include <stdarg.h>
>   #include <linux/seq_file.h>
>   #include <drm/drmP.h>
> +
> +#ifndef CONFIG_DYNAMIC_DEBUG
> +#define DEBUG /* for pr_debug() */
> +#endif
>   #include <drm/drm_print.h>
>   
>   void __drm_puts_coredump(struct drm_printer *p, const char *str)

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Zhang, Jerry(Junwei)]

On 12/6/18 12:56 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> The following cases are possible for pr_debug():
>
> 1. CONFIG_DYNAMIC_DEBUG disabled
>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>        it never generates any output.
>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>        i.e. it generates output which doesn't appear in dmesg by default,
>        can be enabled dynamically.
>
> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>     dynamic_pr_debug()
>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>        default, can be enabled dynamically.
>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>        can be disabled dynamically.
>
> The intention for drm_debug_printer() is to generate output which
> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>
> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   drivers/gpu/drm/drm_print.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..ee56e4a1b343 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -23,11 +23,13 @@
>    * Rob Clark <robdclark@gmail.com>
>    */
>   
> -#define DEBUG /* for pr_debug() */
> -
>   #include <stdarg.h>
>   #include <linux/seq_file.h>
>   #include <drm/drmP.h>
> +
> +#ifndef CONFIG_DYNAMIC_DEBUG
> +#define DEBUG /* for pr_debug() */
> +#endif
>   #include <drm/drm_print.h>
>   
>   void __drm_puts_coredump(struct drm_printer *p, const char *str)

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

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Zhang, Jerry(Junwei)]

On 12/6/18 12:56 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> All the output is related, so it should all be printed the same way.
> Some of it was using pr_debug, but some of it appeared in dmesg by
> default. The caller should handle failure, so there's no need to spam
> dmesg with potentially quite a lot of output by default.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Sounds reasonable, but personally prefer to show error when some
vital incident happens, e.g. no memory on eviction.

Regards,
Jerry

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 39 ++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index d87935bf8e30..5e9b9dd91629 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -77,38 +77,39 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>   	return 0;
>   }
>   
> -static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
> +static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p,
> +			       int mem_type)
>   {
>   	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
> -	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   
> -	pr_err("    has_type: %d\n", man->has_type);
> -	pr_err("    use_type: %d\n", man->use_type);
> -	pr_err("    flags: 0x%08X\n", man->flags);
> -	pr_err("    gpu_offset: 0x%08llX\n", man->gpu_offset);
> -	pr_err("    size: %llu\n", man->size);
> -	pr_err("    available_caching: 0x%08X\n", man->available_caching);
> -	pr_err("    default_caching: 0x%08X\n", man->default_caching);
> +	drm_printf(p, "    has_type: %d\n", man->has_type);
> +	drm_printf(p, "    use_type: %d\n", man->use_type);
> +	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> +	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
> +	drm_printf(p, "    size: %llu\n", man->size);
> +	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
> +	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
>   	if (mem_type != TTM_PL_SYSTEM)
> -		(*man->func->debug)(man, &p);
> +		(*man->func->debug)(man, p);
>   }
>   
>   static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>   					struct ttm_placement *placement)
>   {
> +	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   	int i, ret, mem_type;
>   
> -	pr_err("No space for %p (%lu pages, %luK, %luM)\n",
> -	       bo, bo->mem.num_pages, bo->mem.size >> 10,
> -	       bo->mem.size >> 20);
> +	drm_printf(&p, "No space for %p (%lu pages, %luK, %luM)\n",
> +		   bo, bo->mem.num_pages, bo->mem.size >> 10,
> +		   bo->mem.size >> 20);
>   	for (i = 0; i < placement->num_placement; i++) {
>   		ret = ttm_mem_type_from_place(&placement->placement[i],
>   						&mem_type);
>   		if (ret)
>   			return;
> -		pr_err("  placement[%d]=0x%08X (%d)\n",
> -		       i, placement->placement[i].flags, mem_type);
> -		ttm_mem_type_debug(bo->bdev, mem_type);
> +		drm_printf(&p, "  placement[%d]=0x%08X (%d)\n",
> +			   i, placement->placement[i].flags, mem_type);
> +		ttm_mem_type_debug(bo->bdev, &p, mem_type);
>   	}
>   }
>   
> @@ -728,8 +729,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
>   	if (ret) {
>   		if (ret != -ERESTARTSYS) {
> -			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
> -			       bo);
> +			pr_debug("Failed to find memory space for buffer 0x%p eviction\n",
> +				 bo);
>   			ttm_bo_mem_space_debug(bo, &placement);
>   		}
>   		goto out;
> @@ -738,7 +739,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
>   	if (unlikely(ret)) {
>   		if (ret != -ERESTARTSYS)
> -			pr_err("Buffer eviction failed\n");
> +			pr_debug("Buffer eviction failed\n");
>   		ttm_bo_mem_put(bo, &evict_mem);
>   		goto out;
>   	}

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Zhang, Jerry(Junwei)]

On 12/6/18 12:56 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> All the output is related, so it should all be printed the same way.
> Some of it was using pr_debug, but some of it appeared in dmesg by
> default. The caller should handle failure, so there's no need to spam
> dmesg with potentially quite a lot of output by default.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Sounds reasonable, but personally prefer to show error when some
vital incident happens, e.g. no memory on eviction.

Regards,
Jerry

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 39 ++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index d87935bf8e30..5e9b9dd91629 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -77,38 +77,39 @@ static inline int ttm_mem_type_from_place(const struct ttm_place *place,
>   	return 0;
>   }
>   
> -static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
> +static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p,
> +			       int mem_type)
>   {
>   	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
> -	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   
> -	pr_err("    has_type: %d\n", man->has_type);
> -	pr_err("    use_type: %d\n", man->use_type);
> -	pr_err("    flags: 0x%08X\n", man->flags);
> -	pr_err("    gpu_offset: 0x%08llX\n", man->gpu_offset);
> -	pr_err("    size: %llu\n", man->size);
> -	pr_err("    available_caching: 0x%08X\n", man->available_caching);
> -	pr_err("    default_caching: 0x%08X\n", man->default_caching);
> +	drm_printf(p, "    has_type: %d\n", man->has_type);
> +	drm_printf(p, "    use_type: %d\n", man->use_type);
> +	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> +	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
> +	drm_printf(p, "    size: %llu\n", man->size);
> +	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
> +	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
>   	if (mem_type != TTM_PL_SYSTEM)
> -		(*man->func->debug)(man, &p);
> +		(*man->func->debug)(man, p);
>   }
>   
>   static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>   					struct ttm_placement *placement)
>   {
> +	struct drm_printer p = drm_debug_printer(TTM_PFX);
>   	int i, ret, mem_type;
>   
> -	pr_err("No space for %p (%lu pages, %luK, %luM)\n",
> -	       bo, bo->mem.num_pages, bo->mem.size >> 10,
> -	       bo->mem.size >> 20);
> +	drm_printf(&p, "No space for %p (%lu pages, %luK, %luM)\n",
> +		   bo, bo->mem.num_pages, bo->mem.size >> 10,
> +		   bo->mem.size >> 20);
>   	for (i = 0; i < placement->num_placement; i++) {
>   		ret = ttm_mem_type_from_place(&placement->placement[i],
>   						&mem_type);
>   		if (ret)
>   			return;
> -		pr_err("  placement[%d]=0x%08X (%d)\n",
> -		       i, placement->placement[i].flags, mem_type);
> -		ttm_mem_type_debug(bo->bdev, mem_type);
> +		drm_printf(&p, "  placement[%d]=0x%08X (%d)\n",
> +			   i, placement->placement[i].flags, mem_type);
> +		ttm_mem_type_debug(bo->bdev, &p, mem_type);
>   	}
>   }
>   
> @@ -728,8 +729,8 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_mem_space(bo, &placement, &evict_mem, ctx);
>   	if (ret) {
>   		if (ret != -ERESTARTSYS) {
> -			pr_err("Failed to find memory space for buffer 0x%p eviction\n",
> -			       bo);
> +			pr_debug("Failed to find memory space for buffer 0x%p eviction\n",
> +				 bo);
>   			ttm_bo_mem_space_debug(bo, &placement);
>   		}
>   		goto out;
> @@ -738,7 +739,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx);
>   	if (unlikely(ret)) {
>   		if (ret != -ERESTARTSYS)
> -			pr_err("Buffer eviction failed\n");
> +			pr_debug("Buffer eviction failed\n");
>   		ttm_bo_mem_put(bo, &evict_mem);
>   		goto out;
>   	}

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

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Joe Perches]

On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote:
> On 12/6/18 12:56 AM, Michel Dänzer wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> > 
> > The following cases are possible for pr_debug():
> > 
> > 1. CONFIG_DYNAMIC_DEBUG disabled
> >     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> >        it never generates any output.
> >     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> >        i.e. it generates output which doesn't appear in dmesg by default,
> >        can be enabled dynamically.
> > 
> > 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> >     dynamic_pr_debug()
> >     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> >        default, can be enabled dynamically.
> >     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> >        can be disabled dynamically.
> > 
> > The intention for drm_debug_printer() is to generate output which
> > doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> > cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> > instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> > 
> > Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")

I very much doubt this is a fix.

Did you read the commit log for this commit?

It says "make sure it will always produce output"

And why didn't you cc Chris Wilson, the author of that patch?

> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
> 
> > ---
> >   drivers/gpu/drm/drm_print.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index 0e7fc3e7dfb4..ee56e4a1b343 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -23,11 +23,13 @@
> >    * Rob Clark <robdclark@gmail.com>
> >    */
> >   
> > -#define DEBUG /* for pr_debug() */
> > -
> >   #include <stdarg.h>
> >   #include <linux/seq_file.h>
> >   #include <drm/drmP.h>
> > +
> > +#ifndef CONFIG_DYNAMIC_DEBUG
> > +#define DEBUG /* for pr_debug() */
> > +#endif
> >   #include <drm/drm_print.h>
> >   
> >   void __drm_puts_coredump(struct drm_printer *p, const char *str)

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> All the output is related, so it should all be printed the same way.
>> Some of it was using pr_debug, but some of it appeared in dmesg by
>> default. The caller should handle failure, so there's no need to spam
>> dmesg with potentially quite a lot of output by default.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> Sounds reasonable, but personally prefer to show error when some
> vital incident happens, e.g. no memory on eviction.

The amdgpu driver still prints these in that case:

 [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
 [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!

That's plenty as far as I'm concerned. :)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> All the output is related, so it should all be printed the same way.
>> Some of it was using pr_debug, but some of it appeared in dmesg by
>> default. The caller should handle failure, so there's no need to spam
>> dmesg with potentially quite a lot of output by default.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> Sounds reasonable, but personally prefer to show error when some
> vital incident happens, e.g. no memory on eviction.

The amdgpu driver still prints these in that case:

 [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
 [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!

That's plenty as far as I'm concerned. :)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Chris Wilson]

Quoting Zhang, Jerry(Junwei) (2018-12-06 02:40:42)
> On 12/6/18 12:56 AM, Michel Dänzer wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> >
> > The following cases are possible for pr_debug():
> >
> > 1. CONFIG_DYNAMIC_DEBUG disabled
> >     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> >        it never generates any output.
> >     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> >        i.e. it generates output which doesn't appear in dmesg by default,
> >        can be enabled dynamically.
> >
> > 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> >     dynamic_pr_debug()
> >     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> >        default, can be enabled dynamically.
> >     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> >        can be disabled dynamically.
> >
> > The intention for drm_debug_printer() is to generate output which
> > doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> > cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> > instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> >
> > Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

At the cost of 1a? Nah.
-Chris

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Chris Wilson]

Quoting Zhang, Jerry(Junwei) (2018-12-06 02:40:42)
> On 12/6/18 12:56 AM, Michel Dänzer wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> >
> > The following cases are possible for pr_debug():
> >
> > 1. CONFIG_DYNAMIC_DEBUG disabled
> >     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> >        it never generates any output.
> >     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> >        i.e. it generates output which doesn't appear in dmesg by default,
> >        can be enabled dynamically.
> >
> > 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> >     dynamic_pr_debug()
> >     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> >        default, can be enabled dynamically.
> >     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> >        can be disabled dynamically.
> >
> > The intention for drm_debug_printer() is to generate output which
> > doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> > cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> > instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> >
> > Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

At the cost of 1a? Nah.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 10:12 a.m., Chris Wilson wrote:
> Quoting Zhang, Jerry(Junwei) (2018-12-06 02:40:42)
>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> The following cases are possible for pr_debug():
>>>
>>> 1. CONFIG_DYNAMIC_DEBUG disabled
>>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>>>        it never generates any output.
>>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>>>        i.e. it generates output which doesn't appear in dmesg by default,
>>>        can be enabled dynamically.
>>>
>>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>>>     dynamic_pr_debug()
>>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>>>        default, can be enabled dynamically.
>>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>>>        can be disabled dynamically.
>>>
>>> The intention for drm_debug_printer() is to generate output which
>>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
>>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
>>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>>>
>>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
> 
> At the cost of 1a? Nah.

We still #define DEBUG if CONFIG_DYNAMIC_DEBUG isn't defined, so it's
still 1b), not 1a).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 3:51 a.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote:
>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> The following cases are possible for pr_debug():
>>>
>>> 1. CONFIG_DYNAMIC_DEBUG disabled
>>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>>>        it never generates any output.
>>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>>>        i.e. it generates output which doesn't appear in dmesg by default,
>>>        can be enabled dynamically.
>>>
>>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>>>     dynamic_pr_debug()
>>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>>>        default, can be enabled dynamically.
>>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>>>        can be disabled dynamically.
>>>
>>> The intention for drm_debug_printer() is to generate output which
>>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
>>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
>>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>>>
>>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> 
> I very much doubt this is a fix.
> 
> Did you read the commit log for this commit?
> 
> It says "make sure it will always produce output"

I thought the commit log covered this, suggestions for improvement welcome.

Chris' change addressed case 1a), but also took us from 2a) to 2b). But
we want 2a).

I suspect Chris missed that pr_debug()'s output is visible by default if
CONFIG_DYNAMIC_DEBUG and DEBUG are both defined.


> And why didn't you cc Chris Wilson, the author of that patch?

I used the get_maintainer.pl script. Thanks for adding Chris.


P.S. FYI, your e-mail had a very aggressive tone to me, not sure what for.


>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>
>>> ---
>>>   drivers/gpu/drm/drm_print.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
>>> index 0e7fc3e7dfb4..ee56e4a1b343 100644
>>> --- a/drivers/gpu/drm/drm_print.c
>>> +++ b/drivers/gpu/drm/drm_print.c
>>> @@ -23,11 +23,13 @@
>>>    * Rob Clark <robdclark@gmail.com>
>>>    */
>>>   
>>> -#define DEBUG /* for pr_debug() */
>>> -
>>>   #include <stdarg.h>
>>>   #include <linux/seq_file.h>
>>>   #include <drm/drmP.h>
>>> +
>>> +#ifndef CONFIG_DYNAMIC_DEBUG
>>> +#define DEBUG /* for pr_debug() */
>>> +#endif
>>>   #include <drm/drm_print.h>
>>>   
>>>   void __drm_puts_coredump(struct drm_printer *p, const char *str)
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Chris Wilson]

Quoting Michel Dänzer (2018-12-06 09:21:40)
> On 2018-12-06 10:12 a.m., Chris Wilson wrote:
> > Quoting Zhang, Jerry(Junwei) (2018-12-06 02:40:42)
> >> On 12/6/18 12:56 AM, Michel Dänzer wrote:
> >>> From: Michel Dänzer <michel.daenzer@amd.com>
> >>>
> >>> The following cases are possible for pr_debug():
> >>>
> >>> 1. CONFIG_DYNAMIC_DEBUG disabled
> >>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> >>>        it never generates any output.
> >>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> >>>        i.e. it generates output which doesn't appear in dmesg by default,
> >>>        can be enabled dynamically.
> >>>
> >>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> >>>     dynamic_pr_debug()
> >>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> >>>        default, can be enabled dynamically.
> >>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> >>>        can be disabled dynamically.
> >>>
> >>> The intention for drm_debug_printer() is to generate output which
> >>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> >>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> >>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> >>>
> >>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> >>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> >> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
> > 
> > At the cost of 1a? Nah.
> 
> We still #define DEBUG if CONFIG_DYNAMIC_DEBUG isn't defined, so it's
> still 1b), not 1a).

I completely fluffed my reading of ifndef.

#if !IS_ENABLED(CONFIG_DYNAMIC_DEBUG)
-Chris

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Chris Wilson]

Quoting Michel Dänzer (2018-12-06 09:21:40)
> On 2018-12-06 10:12 a.m., Chris Wilson wrote:
> > Quoting Zhang, Jerry(Junwei) (2018-12-06 02:40:42)
> >> On 12/6/18 12:56 AM, Michel Dänzer wrote:
> >>> From: Michel Dänzer <michel.daenzer@amd.com>
> >>>
> >>> The following cases are possible for pr_debug():
> >>>
> >>> 1. CONFIG_DYNAMIC_DEBUG disabled
> >>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> >>>        it never generates any output.
> >>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> >>>        i.e. it generates output which doesn't appear in dmesg by default,
> >>>        can be enabled dynamically.
> >>>
> >>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> >>>     dynamic_pr_debug()
> >>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> >>>        default, can be enabled dynamically.
> >>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> >>>        can be disabled dynamically.
> >>>
> >>> The intention for drm_debug_printer() is to generate output which
> >>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> >>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> >>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> >>>
> >>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> >>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> >> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
> > 
> > At the cost of 1a? Nah.
> 
> We still #define DEBUG if CONFIG_DYNAMIC_DEBUG isn't defined, so it's
> still 1b), not 1a).

I completely fluffed my reading of ifndef.

#if !IS_ENABLED(CONFIG_DYNAMIC_DEBUG)
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Koenig, Christian]

Am 06.12.18 um 10:09 schrieb Michel Dänzer:
> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> All the output is related, so it should all be printed the same way.
>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>> default. The caller should handle failure, so there's no need to spam
>>> dmesg with potentially quite a lot of output by default.
>>>
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Sounds reasonable, but personally prefer to show error when some
>> vital incident happens, e.g. no memory on eviction.
> The amdgpu driver still prints these in that case:
>
>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
>
> That's plenty as far as I'm concerned. :)

Yeah, but in this case I would rather make the amdgpu messages debug 
level and leave the TTM meassages on error level.

Christian.

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Koenig, Christian]

Am 06.12.18 um 10:09 schrieb Michel Dänzer:
> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> All the output is related, so it should all be printed the same way.
>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>> default. The caller should handle failure, so there's no need to spam
>>> dmesg with potentially quite a lot of output by default.
>>>
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Sounds reasonable, but personally prefer to show error when some
>> vital incident happens, e.g. no memory on eviction.
> The amdgpu driver still prints these in that case:
>
>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
>
> That's plenty as far as I'm concerned. :)

Yeah, but in this case I would rather make the amdgpu messages debug 
level and leave the TTM meassages on error level.

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

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 10:33 a.m., Koenig, Christian wrote:
> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> All the output is related, so it should all be printed the same way.
>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>> default. The caller should handle failure, so there's no need to spam
>>>> dmesg with potentially quite a lot of output by default.
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Sounds reasonable, but personally prefer to show error when some
>>> vital incident happens, e.g. no memory on eviction.
>> The amdgpu driver still prints these in that case:
>>
>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
>>
>> That's plenty as far as I'm concerned. :)
> 
> Yeah, but in this case I would rather make the amdgpu messages debug 
> level and leave the TTM meassages on error level.

That makes no sense to me.

The amdgpu messages have some value for normal users / bug reports,
indicating that something isn't going quite as planned.

The TTM messages are orders of magnitude longer, and are basically noise
for a normal user.

Seems like a no-brainer to me which of these should be visible by default.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 10:33 a.m., Koenig, Christian wrote:
> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> All the output is related, so it should all be printed the same way.
>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>> default. The caller should handle failure, so there's no need to spam
>>>> dmesg with potentially quite a lot of output by default.
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Sounds reasonable, but personally prefer to show error when some
>>> vital incident happens, e.g. no memory on eviction.
>> The amdgpu driver still prints these in that case:
>>
>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
>>
>> That's plenty as far as I'm concerned. :)
> 
> Yeah, but in this case I would rather make the amdgpu messages debug 
> level and leave the TTM meassages on error level.

That makes no sense to me.

The amdgpu messages have some value for normal users / bug reports,
indicating that something isn't going quite as planned.

The TTM messages are orders of magnitude longer, and are basically noise
for a normal user.

Seems like a no-brainer to me which of these should be visible by default.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Zhang, Jerry(Junwei)]

On 12/6/18 5:33 PM, Koenig, Christian wrote:
> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> All the output is related, so it should all be printed the same way.
>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>> default. The caller should handle failure, so there's no need to spam
>>>> dmesg with potentially quite a lot of output by default.
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Sounds reasonable, but personally prefer to show error when some
>>> vital incident happens, e.g. no memory on eviction.
>> The amdgpu driver still prints these in that case:
>>
>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!

That's from cs submit, perhaps it may come from other places by 
ttm_bo_evict_mm().
Is that right? Christian.

Regards,
Jerry
>>
>> That's plenty as far as I'm concerned. :)
> Yeah, but in this case I would rather make the amdgpu messages debug
> level and leave the TTM meassages on error level.
>
> Christian.

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Zhang, Jerry(Junwei)]

On 12/6/18 5:33 PM, Koenig, Christian wrote:
> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> All the output is related, so it should all be printed the same way.
>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>> default. The caller should handle failure, so there's no need to spam
>>>> dmesg with potentially quite a lot of output by default.
>>>>
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Sounds reasonable, but personally prefer to show error when some
>>> vital incident happens, e.g. no memory on eviction.
>> The amdgpu driver still prints these in that case:
>>
>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!

That's from cs submit, perhaps it may come from other places by 
ttm_bo_evict_mm().
Is that right? Christian.

Regards,
Jerry
>>
>> That's plenty as far as I'm concerned. :)
> Yeah, but in this case I would rather make the amdgpu messages debug
> level and leave the TTM meassages on error level.
>
> Christian.

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

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Christian König]

Am 06.12.18 um 10:39 schrieb Zhang, Jerry(Junwei):
> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>
>>>>> All the output is related, so it should all be printed the same way.
>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>> default. The caller should handle failure, so there's no need to spam
>>>>> dmesg with potentially quite a lot of output by default.
>>>>>
>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>> Sounds reasonable, but personally prefer to show error when some
>>>> vital incident happens, e.g. no memory on eviction.
>>> The amdgpu driver still prints these in that case:
>>>
>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* 
>>> amdgpu_cs_list_validate(validated) failed.
>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for 
>>> command submission!
>
> That's from cs submit, perhaps it may come from other places by 
> ttm_bo_evict_mm().
> Is that right? Christian.

Yeah, exactly my thinking as well. When we silence the TTM messages we 
might miss those cases.

Additional to that other drivers using TTM might not have those messages 
either.

If TTM is to noisy we should use ratelimit and/or reduce the number and 
size of the warning messages.

A simple "Warning, I ran out of memory during eviction!" should do.

Regards,
Christian.

>
> Regards,
> Jerry
>>>
>>> That's plenty as far as I'm concerned. :)
>> Yeah, but in this case I would rather make the amdgpu messages debug
>> level and leave the TTM meassages on error level.
>>
>> Christian.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Christian König]

Am 06.12.18 um 10:39 schrieb Zhang, Jerry(Junwei):
> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>
>>>>> All the output is related, so it should all be printed the same way.
>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>> default. The caller should handle failure, so there's no need to spam
>>>>> dmesg with potentially quite a lot of output by default.
>>>>>
>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>> Sounds reasonable, but personally prefer to show error when some
>>>> vital incident happens, e.g. no memory on eviction.
>>> The amdgpu driver still prints these in that case:
>>>
>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* 
>>> amdgpu_cs_list_validate(validated) failed.
>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for 
>>> command submission!
>
> That's from cs submit, perhaps it may come from other places by 
> ttm_bo_evict_mm().
> Is that right? Christian.

Yeah, exactly my thinking as well. When we silence the TTM messages we 
might miss those cases.

Additional to that other drivers using TTM might not have those messages 
either.

If TTM is to noisy we should use ratelimit and/or reduce the number and 
size of the warning messages.

A simple "Warning, I ran out of memory during eviction!" should do.

Regards,
Christian.

>
> Regards,
> Jerry
>>>
>>> That's plenty as far as I'm concerned. :)
>> Yeah, but in this case I would rather make the amdgpu messages debug
>> level and leave the TTM meassages on error level.
>>
>> Christian.
>
> _______________________________________________
> 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 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 10:38 a.m., Michel Dänzer wrote:
> On 2018-12-06 10:33 a.m., Koenig, Christian wrote:
>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>
>>>>> All the output is related, so it should all be printed the same way.
>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>> default. The caller should handle failure, so there's no need to spam
>>>>> dmesg with potentially quite a lot of output by default.
>>>>>
>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>> Sounds reasonable, but personally prefer to show error when some
>>>> vital incident happens, e.g. no memory on eviction.
>>> The amdgpu driver still prints these in that case:
>>>
>>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>>   [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
>>>
>>> That's plenty as far as I'm concerned. :)
>>
>> Yeah, but in this case I would rather make the amdgpu messages debug 
>> level and leave the TTM meassages on error level.
> 
> That makes no sense to me.
> 
> The amdgpu messages have some value for normal users / bug reports,
> indicating that something isn't going quite as planned.
> 
> The TTM messages are orders of magnitude longer, and are basically noise
> for a normal user.
> 
> Seems like a no-brainer to me which of these should be visible by default.

Moreover, not every case producing the driver output also produces the
TTM output, so it could make it difficult to realize that there's a
memory pressure situation.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 10:49 a.m., Christian König wrote:
> Am 06.12.18 um 10:39 schrieb Zhang, Jerry(Junwei):
>> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>
>>>>>> All the output is related, so it should all be printed the same way.
>>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>>> default. The caller should handle failure, so there's no need to spam
>>>>>> dmesg with potentially quite a lot of output by default.
>>>>>>
>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>>> Sounds reasonable, but personally prefer to show error when some
>>>>> vital incident happens, e.g. no memory on eviction.
>>>> The amdgpu driver still prints these in that case:
>>>>
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR*
>>>> amdgpu_cs_list_validate(validated) failed.
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for
>>>> command submission!
>>
>> That's from cs submit, perhaps it may come from other places by
>> ttm_bo_evict_mm().
>> Is that right? Christian.
> 
> Yeah, exactly my thinking as well. When we silence the TTM messages we
> might miss those cases.
> 
> Additional to that other drivers using TTM might not have those messages
> either.
> 
> If TTM is to noisy we should use ratelimit and/or reduce the number and
> size of the warning messages.
> 
> A simple "Warning, I ran out of memory during eviction!" should do.

Just dropping the last hunk of this patch should do then. I can do that.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 10:49 a.m., Christian König wrote:
> Am 06.12.18 um 10:39 schrieb Zhang, Jerry(Junwei):
>> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>
>>>>>> All the output is related, so it should all be printed the same way.
>>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>>> default. The caller should handle failure, so there's no need to spam
>>>>>> dmesg with potentially quite a lot of output by default.
>>>>>>
>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>>> Sounds reasonable, but personally prefer to show error when some
>>>>> vital incident happens, e.g. no memory on eviction.
>>>> The amdgpu driver still prints these in that case:
>>>>
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR*
>>>> amdgpu_cs_list_validate(validated) failed.
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for
>>>> command submission!
>>
>> That's from cs submit, perhaps it may come from other places by
>> ttm_bo_evict_mm().
>> Is that right? Christian.
> 
> Yeah, exactly my thinking as well. When we silence the TTM messages we
> might miss those cases.
> 
> Additional to that other drivers using TTM might not have those messages
> either.
> 
> If TTM is to noisy we should use ratelimit and/or reduce the number and
> size of the warning messages.
> 
> A simple "Warning, I ran out of memory during eviction!" should do.

Just dropping the last hunk of this patch should do then. I can do that.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Joe Perches]

On Thu, 2018-12-06 at 10:23 +0100, Michel Dänzer wrote:
> On 2018-12-06 3:51 a.m., Joe Perches wrote:
> > On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote:
> > > On 12/6/18 12:56 AM, Michel Dänzer wrote:
> > > > From: Michel Dänzer <michel.daenzer@amd.com>
> > > > 
> > > > The following cases are possible for pr_debug():
> > > > 
> > > > 1. CONFIG_DYNAMIC_DEBUG disabled
> > > >     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
> > > >        it never generates any output.
> > > >     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
> > > >        i.e. it generates output which doesn't appear in dmesg by default,
> > > >        can be enabled dynamically.
> > > > 
> > > > 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
> > > >     dynamic_pr_debug()
> > > >     a) DEBUG not defined: dynamic_pr_debug() generates no output by
> > > >        default, can be enabled dynamically.
> > > >     b) DEBUG defined: dynamic_pr_debug() generates output by default,
> > > >        can be disabled dynamically.
> > > > 
> > > > The intention for drm_debug_printer() is to generate output which
> > > > doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
> > > > cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
> > > > instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
> > > > 
> > > > Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
> > 
> > I very much doubt this is a fix.
> > 
> > Did you read the commit log for this commit?
> > 
> > It says "make sure it will always produce output"
> 
> I thought the commit log covered this, suggestions for improvement welcome.
> 
> Chris' change addressed case 1a), but also took us from 2a) to 2b). But
> we want 2a)

> I suspect Chris missed that pr_debug()'s output is visible by default if
> CONFIG_DYNAMIC_DEBUG and DEBUG are both defined.

I believe you and Chris have different goals and that your
point of 2b is
misguided.

I think the language used in the Chris' commit log is
plain ans obvious.  'always produce output'.

> > And why didn't you cc Chris Wilson, the author of that patch?k
> 
> I used the get_maintainer.pl script. Thanks for adding Chris.
> P.S. FYI, your e-mail had a very aggressive tone to me, not sure what for.

No worries.

I think the language I used is also plain and obvious
and you are overreading.

cheers, Joe

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 12:41 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 10:23 +0100, Michel Dänzer wrote:
>> On 2018-12-06 3:51 a.m., Joe Perches wrote:
>>> On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote:
>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>
>>>>> The following cases are possible for pr_debug():
>>>>>
>>>>> 1. CONFIG_DYNAMIC_DEBUG disabled
>>>>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>>>>>        it never generates any output.
>>>>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>>>>>        i.e. it generates output which doesn't appear in dmesg by default,
>>>>>        can be enabled dynamically.
>>>>>
>>>>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>>>>>     dynamic_pr_debug()
>>>>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>>>>>        default, can be enabled dynamically.
>>>>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>>>>>        can be disabled dynamically.
>>>>>
>>>>> The intention for drm_debug_printer() is to generate output which
>>>>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
>>>>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
>>>>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>>>>>
>>>>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
>>>
>>> I very much doubt this is a fix.
>>>
>>> Did you read the commit log for this commit?
>>>
>>> It says "make sure it will always produce output"
>>
>> I thought the commit log covered this, suggestions for improvement welcome.
>>
>> Chris' change addressed case 1a), but also took us from 2a) to 2b). But
>> we want 2a)
> 
>> I suspect Chris missed that pr_debug()'s output is visible by default if
>> CONFIG_DYNAMIC_DEBUG and DEBUG are both defined.
> 
> I believe you and Chris have different goals and that your
> point of 2b is
> misguided.
> 
> I think the language used in the Chris' commit log is
> plain ans obvious.  'always produce output'.

That was probably meant as "make sure there always can be output", which
isn't the case with 1a (no_printk suppresses the output at compile time,
the strings might not even exist in the binaries).

In contrast to the 2b case, the pr_debug output isn't visible by default
with 1b, so the latter doesn't fit "always produce output" either. But
Chris' initial follow-up in this thread seems to confirm my assumption
that his change was about 1a/b.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 12:41 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 10:23 +0100, Michel Dänzer wrote:
>> On 2018-12-06 3:51 a.m., Joe Perches wrote:
>>> On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote:
>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>
>>>>> The following cases are possible for pr_debug():
>>>>>
>>>>> 1. CONFIG_DYNAMIC_DEBUG disabled
>>>>>     a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e.
>>>>>        it never generates any output.
>>>>>     b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...),
>>>>>        i.e. it generates output which doesn't appear in dmesg by default,
>>>>>        can be enabled dynamically.
>>>>>
>>>>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to
>>>>>     dynamic_pr_debug()
>>>>>     a) DEBUG not defined: dynamic_pr_debug() generates no output by
>>>>>        default, can be enabled dynamically.
>>>>>     b) DEBUG defined: dynamic_pr_debug() generates output by default,
>>>>>        can be disabled dynamically.
>>>>>
>>>>> The intention for drm_debug_printer() is to generate output which
>>>>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e.
>>>>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b)
>>>>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled.
>>>>>
>>>>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer")
>>>
>>> I very much doubt this is a fix.
>>>
>>> Did you read the commit log for this commit?
>>>
>>> It says "make sure it will always produce output"
>>
>> I thought the commit log covered this, suggestions for improvement welcome.
>>
>> Chris' change addressed case 1a), but also took us from 2a) to 2b). But
>> we want 2a)
> 
>> I suspect Chris missed that pr_debug()'s output is visible by default if
>> CONFIG_DYNAMIC_DEBUG and DEBUG are both defined.
> 
> I believe you and Chris have different goals and that your
> point of 2b is
> misguided.
> 
> I think the language used in the Chris' commit log is
> plain ans obvious.  'always produce output'.

That was probably meant as "make sure there always can be output", which
isn't the case with 1a (no_printk suppresses the output at compile time,
the strings might not even exist in the binaries).

In contrast to the 2b case, the pr_debug output isn't visible by default
with 1b, so the latter doesn't fit "always produce output" either. But
Chris' initial follow-up in this thread seems to confirm my assumption
that his change was about 1a/b.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Joe Perches]

On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
> In contrast to the 2b case, the pr_debug output isn't visible by default
> with 1b, so the latter doesn't fit "always produce output" either.

I think you are mistaken here.

Adding #define DEBUG as Chris did enables pr_debug output
and is your 1b.

Perhaps your default console logging level is set to a
non-default value.

CONSOLE_LOGLEVEL_DEFAULT

lib/Kconfig.debug:config CONSOLE_LOGLEVEL_DEFAULT
lib/Kconfig.debug-      int "Default console loglevel (1-15)"
lib/Kconfig.debug-      range 1 15
lib/Kconfig.debug-      default "7"

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 1:23 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
>> In contrast to the 2b case, the pr_debug output isn't visible by default
>> with 1b, so the latter doesn't fit "always produce output" either.
> 
> I think you are mistaken here.

Still puzzled as to what you're hoping to achieve with that kind of
language. None of the confusion about this patch has been on my part. :)


> Adding #define DEBUG as Chris did enables pr_debug output
> and is your 1b.
> 
> Perhaps your default console logging level is set to a
> non-default value.

I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
by this patch is that messages from drm_debug_printer are visible by
default (case 2b), whereas they shouldn't be (case 2a, like 1b).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 1:23 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
>> In contrast to the 2b case, the pr_debug output isn't visible by default
>> with 1b, so the latter doesn't fit "always produce output" either.
> 
> I think you are mistaken here.

Still puzzled as to what you're hoping to achieve with that kind of
language. None of the confusion about this patch has been on my part. :)


> Adding #define DEBUG as Chris did enables pr_debug output
> and is your 1b.
> 
> Perhaps your default console logging level is set to a
> non-default value.

I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
by this patch is that messages from drm_debug_printer are visible by
default (case 2b), whereas they shouldn't be (case 2a, like 1b).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Daniel Thompson]

On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote:
> On 2018-12-06 1:23 p.m., Joe Perches wrote:
> > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
> >> In contrast to the 2b case, the pr_debug output isn't visible by default
> >> with 1b, so the latter doesn't fit "always produce output" either.
> > 
> > I think you are mistaken here.
> 
> Still puzzled as to what you're hoping to achieve with that kind of
> language. None of the confusion about this patch has been on my part. :)
> 
> 
> > Adding #define DEBUG as Chris did enables pr_debug output
> > and is your 1b.
> > 
> > Perhaps your default console logging level is set to a
> > non-default value.
> 
> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
> by this patch is that messages from drm_debug_printer are visible by
> default (case 2b), whereas they shouldn't be (case 2a, like 1b).

When enabled (either dynamically or statically) pr_debug() will emit
output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG
is defined or not.

Thus unless you change additional settings (either dynamically or
statically) then debug messages should not be shown on the console
because the default settings filter KERN_DEBUG messages. However they
are available via dmesg and system loggers (syslogd, journal, etc).

The patch proposed will change the behaviour of the debug messages
w.r.t. system loggers based on whether the user has enabled
CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise.


Daniel.

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Daniel Thompson]

On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote:
> On 2018-12-06 1:23 p.m., Joe Perches wrote:
> > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
> >> In contrast to the 2b case, the pr_debug output isn't visible by default
> >> with 1b, so the latter doesn't fit "always produce output" either.
> > 
> > I think you are mistaken here.
> 
> Still puzzled as to what you're hoping to achieve with that kind of
> language. None of the confusion about this patch has been on my part. :)
> 
> 
> > Adding #define DEBUG as Chris did enables pr_debug output
> > and is your 1b.
> > 
> > Perhaps your default console logging level is set to a
> > non-default value.
> 
> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
> by this patch is that messages from drm_debug_printer are visible by
> default (case 2b), whereas they shouldn't be (case 2a, like 1b).

When enabled (either dynamically or statically) pr_debug() will emit
output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG
is defined or not.

Thus unless you change additional settings (either dynamically or
statically) then debug messages should not be shown on the console
because the default settings filter KERN_DEBUG messages. However they
are available via dmesg and system loggers (syslogd, journal, etc).

The patch proposed will change the behaviour of the debug messages
w.r.t. system loggers based on whether the user has enabled
CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise.


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

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 5:10 p.m., Daniel Thompson wrote:
> On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote:
>> On 2018-12-06 1:23 p.m., Joe Perches wrote:
>>> On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
>>>> In contrast to the 2b case, the pr_debug output isn't visible by default
>>>> with 1b, so the latter doesn't fit "always produce output" either.
>>>
>>> I think you are mistaken here.
>>
>> Still puzzled as to what you're hoping to achieve with that kind of
>> language. None of the confusion about this patch has been on my part. :)
>>
>>
>>> Adding #define DEBUG as Chris did enables pr_debug output
>>> and is your 1b.
>>>
>>> Perhaps your default console logging level is set to a
>>> non-default value.
>>
>> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
>> by this patch is that messages from drm_debug_printer are visible by
>> default (case 2b), whereas they shouldn't be (case 2a, like 1b).
> 
> When enabled (either dynamically or statically) pr_debug() will emit
> output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG
> is defined or not.
> 
> Thus unless you change additional settings (either dynamically or
> statically) then debug messages should not be shown on the console
> because the default settings filter KERN_DEBUG messages. However they
> are available via dmesg and system loggers (syslogd, journal, etc).
> 
> The patch proposed will change the behaviour of the debug messages
> w.r.t. system loggers based on whether the user has enabled
> CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise.

Ah, that makes sense now, thanks.

I'm withdrawing this patch.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Michel Dänzer]

On 2018-12-06 5:10 p.m., Daniel Thompson wrote:
> On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote:
>> On 2018-12-06 1:23 p.m., Joe Perches wrote:
>>> On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
>>>> In contrast to the 2b case, the pr_debug output isn't visible by default
>>>> with 1b, so the latter doesn't fit "always produce output" either.
>>>
>>> I think you are mistaken here.
>>
>> Still puzzled as to what you're hoping to achieve with that kind of
>> language. None of the confusion about this patch has been on my part. :)
>>
>>
>>> Adding #define DEBUG as Chris did enables pr_debug output
>>> and is your 1b.
>>>
>>> Perhaps your default console logging level is set to a
>>> non-default value.
>>
>> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
>> by this patch is that messages from drm_debug_printer are visible by
>> default (case 2b), whereas they shouldn't be (case 2a, like 1b).
> 
> When enabled (either dynamically or statically) pr_debug() will emit
> output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG
> is defined or not.
> 
> Thus unless you change additional settings (either dynamically or
> statically) then debug messages should not be shown on the console
> because the default settings filter KERN_DEBUG messages. However they
> are available via dmesg and system loggers (syslogd, journal, etc).
> 
> The patch proposed will change the behaviour of the debug messages
> w.r.t. system loggers based on whether the user has enabled
> CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise.

Ah, that makes sense now, thanks.

I'm withdrawing this patch.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled

[Joe Perches]

On Thu, 2018-12-06 at 15:41 +0100, Michel Dänzer wrote:
> On 2018-12-06 1:23 p.m., Joe Perches wrote:
> > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote:
> > > In contrast to the 2b case, the pr_debug output isn't visible by default
> > > with 1b, so the latter doesn't fit "always produce output" either.
> > 
> > I think you are mistaken here.
> 
> Still puzzled as to what you're hoping to achieve with that kind of
> language. None of the confusion about this patch has been on my part. :)

Doubtful.

> > Adding #define DEBUG as Chris did enables pr_debug output
> > and is your 1b.
> > 
> > Perhaps your default console logging level is set to a
> > non-default value.
> 
> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed
> by this patch is that messages from drm_debug_printer are visible by
> default (case 2b), whereas they shouldn't be (case 2a, like 1b).

The default for config_dynamic_debug when DEBUG is defined is
to output the message equivalent to when DEBUG is defined and
config_dynamic_debug is not.

That's a good thing and has been the general default since 2011.

You want to override that, I suppose that's OK but it's
definitely not a _fix_ as you have written.

It's just a change in behavior.

You added a "Fixes:" line.  I believe that wrong.

A long time ago, at my suggestion:

commit b558c96ffa53f4b3dd52b774e4fb7a52982ab52b
Author: Jim Cromie <jim.cromie@gmail.com>
Date:   Mon Dec 19 17:11:18 2011 -0500

    dynamic_debug: make dynamic-debug supersede DEBUG ccflag
    
    If CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that
    pr_debug()s are controllable, instead of always-on.  When DEBUG is
    also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by
    default.

cheers, Joe

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Joe Perches]

On Thu, 2018-12-06 at 17:39 +0800, Zhang, Jerry(Junwei) wrote:
> On 12/6/18 5:33 PM, Koenig, Christian wrote:
> > Am 06.12.18 um 10:09 schrieb Michel Dänzer:
> > > On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
> > > > On 12/6/18 12:56 AM, Michel Dänzer wrote:
> > > > > From: Michel Dänzer <michel.daenzer@amd.com>
> > > > > 
> > > > > All the output is related, so it should all be printed the same way.
> > > > > Some of it was using pr_debug, but some of it appeared in dmesg by
> > > > > default. The caller should handle failure, so there's no need to spam
> > > > > dmesg with potentially quite a lot of output by default.
> > > > > 
> > > > > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> > > > Sounds reasonable, but personally prefer to show error when some
> > > > vital incident happens, e.g. no memory on eviction.
> > > The amdgpu driver still prints these in that case:
> > > 
> > >    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
> > >    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!

Aren't dump_stack()s already done on all these allocation failures?
I don't notice any use of __GFP_NOWARN on generic allocations in drm.

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 5:46 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 17:39 +0800, Zhang, Jerry(Junwei) wrote:
>> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>
>>>>>> All the output is related, so it should all be printed the same way.
>>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>>> default. The caller should handle failure, so there's no need to spam
>>>>>> dmesg with potentially quite a lot of output by default.
>>>>>>
>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>>> Sounds reasonable, but personally prefer to show error when some
>>>>> vital incident happens, e.g. no memory on eviction.
>>>> The amdgpu driver still prints these in that case:
>>>>
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
> 
> Aren't dump_stack()s already done on all these allocation failures?
> I don't notice any use of __GFP_NOWARN on generic allocations in drm.

Most of the time, these messages are due to being unable to allocate a
TTM BO or move it where it needs to go, not due to the kernel failing to
allocate memory in general.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH 2/2] drm/ttm: Use pr_debug for all output from ttm_bo_evict

[Michel Dänzer]

On 2018-12-06 5:46 p.m., Joe Perches wrote:
> On Thu, 2018-12-06 at 17:39 +0800, Zhang, Jerry(Junwei) wrote:
>> On 12/6/18 5:33 PM, Koenig, Christian wrote:
>>> Am 06.12.18 um 10:09 schrieb Michel Dänzer:
>>>> On 2018-12-06 3:43 a.m., Zhang, Jerry(Junwei) wrote:
>>>>> On 12/6/18 12:56 AM, Michel Dänzer wrote:
>>>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>>>
>>>>>> All the output is related, so it should all be printed the same way.
>>>>>> Some of it was using pr_debug, but some of it appeared in dmesg by
>>>>>> default. The caller should handle failure, so there's no need to spam
>>>>>> dmesg with potentially quite a lot of output by default.
>>>>>>
>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>>> Sounds reasonable, but personally prefer to show error when some
>>>>> vital incident happens, e.g. no memory on eviction.
>>>> The amdgpu driver still prints these in that case:
>>>>
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed.
>>>>    [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission!
> 
> Aren't dump_stack()s already done on all these allocation failures?
> I don't notice any use of __GFP_NOWARN on generic allocations in drm.

Most of the time, these messages are due to being unable to allocate a
TTM BO or move it where it needs to go, not due to the kernel failing to
allocate memory in general.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel