Some minor fixes and changes to help porting i915 to arm64, or even anything !x86.
v3: No changes, just submit to the right mailing list.
Lucas De Marchi (3): drm: Stop spamming log with drm_cache message drm/i915: Fix header test for !CONFIG_X86 drm/i915: Do not spam log with missing arch support
drivers/gpu/drm/drm_cache.c | 9 +++------ drivers/gpu/drm/i915/i915_mm.h | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-)
Only x86 and in some cases PPC have support added in drm_cache.c for the clflush class of functions. However warning once is sufficient to taint the log instead of spamming it with "Architecture has no drm_cache.c support" every few millisecond. Switch to WARN_ONCE() so we still get the log message, but only once, together with the warning. E.g:
------------[ cut here ]------------ Architecture has no drm_cache.c support WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] ...
v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err()
Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com ---
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/drm_cache.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index f19d9acbe959..2c3fa5677f7e 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -112,8 +112,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages) kunmap_atomic(page_virtual); } #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_pages); @@ -143,8 +142,7 @@ drm_clflush_sg(struct sg_table *st) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_sg); @@ -177,8 +175,7 @@ drm_clflush_virt_range(void *addr, unsigned long length) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_virt_range);
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote:
Only x86 and in some cases PPC have support added in drm_cache.c for the clflush class of functions. However warning once is sufficient to taint the log instead of spamming it with "Architecture has no drm_cache.c support" every few millisecond. Switch to WARN_ONCE() so we still get the log message, but only once, together with the warning. E.g:
------------[ cut here ]------------ Architecture has no drm_cache.c support WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] ...
v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err()
Reviewed-by: José Roberto de Souza jose.souza@intel.com
But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? Anyways, it is better than before.
Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/drm_cache.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index f19d9acbe959..2c3fa5677f7e 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -112,8 +112,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages) kunmap_atomic(page_virtual); } #else
- pr_err("Architecture has no drm_cache.c support\n");
- WARN_ON_ONCE(1);
- WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
#endif } EXPORT_SYMBOL(drm_clflush_pages); @@ -143,8 +142,7 @@ drm_clflush_sg(struct sg_table *st) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else
- pr_err("Architecture has no drm_cache.c support\n");
- WARN_ON_ONCE(1);
- WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
#endif } EXPORT_SYMBOL(drm_clflush_sg); @@ -177,8 +175,7 @@ drm_clflush_virt_range(void *addr, unsigned long length) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else
- pr_err("Architecture has no drm_cache.c support\n");
- WARN_ON_ONCE(1);
- WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
#endif } EXPORT_SYMBOL(drm_clflush_virt_range);
On Tue, Feb 01, 2022 at 09:12:05AM -0800, Jose Souza wrote:
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote:
Only x86 and in some cases PPC have support added in drm_cache.c for the clflush class of functions. However warning once is sufficient to taint the log instead of spamming it with "Architecture has no drm_cache.c support" every few millisecond. Switch to WARN_ONCE() so we still get the log message, but only once, together with the warning. E.g:
------------[ cut here ]------------ Architecture has no drm_cache.c support WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] ...
v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err()
Reviewed-by: José Roberto de Souza jose.souza@intel.com
But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? Anyways, it is better than before.
I thought about that, but it didn't seem justifiable because:
1) drm_WARN_ONCE will basically add dev_driver_string() to the log. However the warning message here is basically helping the bootstrap of additional archs. They shouldn't be seen on anything properly supported.
2) This seems all to be a layer below drm anyway and could even be used in places outside easy access to a drm pointer.
So, it seems the benefit of using the subsystem-specific drm_WARN_ONCE doesn't justify the hassle of changing the callers, possibly adding additional back pointers to have access to the drm device pointer.
thanks Lucas De Marchi
On Tue, Feb 01, 2022 at 09:41:33AM -0800, Lucas De Marchi wrote:
On Tue, Feb 01, 2022 at 09:12:05AM -0800, Jose Souza wrote:
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote:
Only x86 and in some cases PPC have support added in drm_cache.c for the clflush class of functions. However warning once is sufficient to taint the log instead of spamming it with "Architecture has no drm_cache.c support" every few millisecond. Switch to WARN_ONCE() so we still get the log message, but only once, together with the warning. E.g:
------------[ cut here ]------------ Architecture has no drm_cache.c support WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] ...
v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err()
Reviewed-by: José Roberto de Souza jose.souza@intel.com
But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? Anyways, it is better than before.
I thought about that, but it didn't seem justifiable because:
- drm_WARN_ONCE will basically add dev_driver_string() to the log.
However the warning message here is basically helping the bootstrap of additional archs. They shouldn't be seen on anything properly supported.
- This seems all to be a layer below drm anyway and could even be used
in places outside easy access to a drm pointer.
So, it seems the benefit of using the subsystem-specific drm_WARN_ONCE doesn't justify the hassle of changing the callers, possibly adding additional back pointers to have access to the drm device pointer.
Initially I had same feeling as Jose, but good points raised here.
Acked-by: Rodrigo Vivi rodrigo.vivi@intel.com
thanks Lucas De Marchi
Architectures others than x86 have a stub implementation calling WARN_ON_ONCE(). The appropriate headers need to be included, otherwise the header-test target will fail with:
HDRTEST drivers/gpu/drm/i915/i915_mm.h In file included from <command-line>: ./drivers/gpu/drm/i915/i915_mm.h: In function ‘remap_io_mapping’: ./drivers/gpu/drm/i915/i915_mm.h:26:2: error: implicit declaration of function ‘WARN_ON_ONCE’ [-Werror=implicit-function-declaration] 26 | WARN_ON_ONCE(1); | ^~~~~~~~~~~~
v2: Do not include <linux/printk.h> since call to pr_err() has been removed
Fixes: 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms") Cc: Siva Mullati siva.mullati@intel.com Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com ---
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/i915/i915_mm.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h index 76f1d53bdf34..3ad22bbe80eb 100644 --- a/drivers/gpu/drm/i915/i915_mm.h +++ b/drivers/gpu/drm/i915/i915_mm.h @@ -6,6 +6,7 @@ #ifndef __I915_MM_H__ #define __I915_MM_H__
+#include <linux/bug.h> #include <linux/types.h>
struct vm_area_struct;
Reviewed-by: Siva Mullati siva.mullati@intel.com
On 31/01/22 10:29 pm, Lucas De Marchi wrote:
Architectures others than x86 have a stub implementation calling WARN_ON_ONCE(). The appropriate headers need to be included, otherwise the header-test target will fail with:
HDRTEST drivers/gpu/drm/i915/i915_mm.h In file included from <command-line>: ./drivers/gpu/drm/i915/i915_mm.h: In function ‘remap_io_mapping’: ./drivers/gpu/drm/i915/i915_mm.h:26:2: error: implicit declaration of function ‘WARN_ON_ONCE’ [-Werror=implicit-function-declaration] 26 | WARN_ON_ONCE(1); | ^~~~~~~~~~~~
v2: Do not include <linux/printk.h> since call to pr_err() has been removed
Fixes: 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms") Cc: Siva Mullati siva.mullati@intel.com Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/i915/i915_mm.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h index 76f1d53bdf34..3ad22bbe80eb 100644 --- a/drivers/gpu/drm/i915/i915_mm.h +++ b/drivers/gpu/drm/i915/i915_mm.h @@ -6,6 +6,7 @@ #ifndef __I915_MM_H__ #define __I915_MM_H__
+#include <linux/bug.h> #include <linux/types.h>
struct vm_area_struct;
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote:
Architectures others than x86 have a stub implementation calling WARN_ON_ONCE(). The appropriate headers need to be included, otherwise the header-test target will fail with:
HDRTEST drivers/gpu/drm/i915/i915_mm.h In file included from <command-line>: ./drivers/gpu/drm/i915/i915_mm.h: In function ‘remap_io_mapping’: ./drivers/gpu/drm/i915/i915_mm.h:26:2: error: implicit declaration of function ‘WARN_ON_ONCE’ [-Werror=implicit-function-declaration] 26 | WARN_ON_ONCE(1); | ^~~~~~~~~~~~
v2: Do not include <linux/printk.h> since call to pr_err() has been removed
Reviewed-by: José Roberto de Souza jose.souza@intel.com
Fixes: 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms") Cc: Siva Mullati siva.mullati@intel.com Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/i915/i915_mm.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h index 76f1d53bdf34..3ad22bbe80eb 100644 --- a/drivers/gpu/drm/i915/i915_mm.h +++ b/drivers/gpu/drm/i915/i915_mm.h @@ -6,6 +6,7 @@ #ifndef __I915_MM_H__ #define __I915_MM_H__
+#include <linux/bug.h> #include <linux/types.h>
struct vm_area_struct;
Following what was done in drm_cache.c, when the stub for remap_io_mapping() was added in commit 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms"), it included a log message with pr_err(). However just the warning is already enough and switching to WARN_ONCE() allows us to keep the log message while avoiding log spam.
Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com ---
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/i915/i915_mm.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h index 3ad22bbe80eb..04c8974d822b 100644 --- a/drivers/gpu/drm/i915/i915_mm.h +++ b/drivers/gpu/drm/i915/i915_mm.h @@ -23,8 +23,7 @@ int remap_io_mapping(struct vm_area_struct *vma, unsigned long addr, unsigned long pfn, unsigned long size, struct io_mapping *iomap) { - pr_err("Architecture has no %s() and shouldn't be calling this function\n", __func__); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); return 0; } #endif
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote:
Following what was done in drm_cache.c, when the stub for remap_io_mapping() was added in commit 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms"), it included a log message with pr_err(). However just the warning is already enough and switching to WARN_ONCE() allows us to keep the log message while avoiding log spam.
Reviewed-by: José Roberto de Souza jose.souza@intel.com
But same suggestion as the first patch in this series about drm_WARN_ONCE().
Signed-off-by: Lucas De Marchi lucas.demarchi@intel.com
v3: No changes from previous version, just submitting to the right mailing list
drivers/gpu/drm/i915/i915_mm.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h index 3ad22bbe80eb..04c8974d822b 100644 --- a/drivers/gpu/drm/i915/i915_mm.h +++ b/drivers/gpu/drm/i915/i915_mm.h @@ -23,8 +23,7 @@ int remap_io_mapping(struct vm_area_struct *vma, unsigned long addr, unsigned long pfn, unsigned long size, struct io_mapping *iomap) {
- pr_err("Architecture has no %s() and shouldn't be calling this function\n", __func__);
- WARN_ON_ONCE(1);
- WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); return 0;
} #endif
dri-devel@lists.freedesktop.org
-
Lucas De Marchi -
Rodrigo Vivi -
Siva Mullati -
Souza, Jose