2024-01-02 00:12:43

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build warning after merge of the drm tree

Hi all,

After merging the drm tree, today's linux-next build (x86_64 allmodconfig)
produced this warning:

In file included from include/drm/drm_mm.h:51,
from drivers/gpu/drm/xe/xe_bo_types.h:11,
from drivers/gpu/drm/xe/xe_bo.h:11,
from drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h:11,
from drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:15,
from drivers/gpu/drm/i915/display/intel_display_power.c:8:
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'print_async_put_domains_state':
drivers/gpu/drm/i915/display/intel_display_power.c:408:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'int' [-Wformat=]
408 | drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~
409 | power_domains->async_put_wakeref);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| int
include/drm/drm_print.h:410:39: note: in definition of macro 'drm_dev_dbg'
410 | __drm_dev_dbg(NULL, dev, cat, fmt, ##__VA_ARGS__)
| ^~~
include/drm/drm_print.h:510:33: note: in expansion of macro 'drm_dbg_driver'
510 | #define drm_dbg(drm, fmt, ...) drm_dbg_driver(drm, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:408:9: note: in expansion of macro 'drm_dbg'
408 | drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
| ^~~~~~~
drivers/gpu/drm/i915/display/intel_display_power.c:408:50: note: format string is defined here
408 | drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
| ~~^
| |
| long unsigned int
| %u

Introduced by commit

b49e894c3fd8 ("drm/i915: Replace custom intel runtime_pm tracker with ref_tracker library")

This would be an error except that I am building with CONFIG_WERROR=n
--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2024-01-03 01:19:32

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the drm tree

Hi all,

On Tue, 2 Jan 2024 11:12:22 +1100 Stephen Rothwell <[email protected]> wrote:
>
> After merging the drm tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
>
> In file included from include/drm/drm_mm.h:51,
> from drivers/gpu/drm/xe/xe_bo_types.h:11,
> from drivers/gpu/drm/xe/xe_bo.h:11,
> from drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h:11,
> from drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:15,
> from drivers/gpu/drm/i915/display/intel_display_power.c:8:
> drivers/gpu/drm/i915/display/intel_display_power.c: In function 'print_async_put_domains_state':
> drivers/gpu/drm/i915/display/intel_display_power.c:408:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'int' [-Wformat=]
> 408 | drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> 409 | power_domains->async_put_wakeref);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | |
> | int
>
> Introduced by commit
>
> b49e894c3fd8 ("drm/i915: Replace custom intel runtime_pm tracker with ref_tracker library")
>
> This would be an error except that I am building with CONFIG_WERROR=n

OK, so I have turned off CONFIG_WERROR=n in the run up to the merge
window opening and so this is now a build failure. I have tried
applying the following patch for today:

From: Stephen Rothwell <[email protected]>
Date: Wed, 3 Jan 2024 11:40:26 +1100
Subject: [PATCH] fix up for "drm/i915: Replace custom intel runtime_pm tracker
with ref_tracker library"

Signed-off-by: Stephen Rothwell <[email protected]>
---
drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 5f091502719b..f23080a4368d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -405,7 +405,7 @@ print_async_put_domains_state(struct i915_power_domains *power_domains)
struct drm_i915_private,
display.power.domains);

- drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
+ drm_dbg(&i915->drm, "async_put_wakeref %u\n",
power_domains->async_put_wakeref);

print_power_domains(power_domains, "async_put_domains[0]",
--
2.43.0

but that produces this failure:

In file included from include/drm/ttm/ttm_resource.h:34,
from include/drm/ttm/ttm_device.h:30,
from drivers/gpu/drm/i915/i915_drv.h:37,
from drivers/gpu/drm/i915/display/intel_display_power.c:8:
drivers/gpu/drm/i915/display/intel_display_power.c: In function 'print_async_put_domains_state':
drivers/gpu/drm/i915/display/intel_display_power.c:408:29: error: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'intel_wakeref_t' {aka 'long unsigned int'} [-Werror=format=]
408 | drm_dbg(&i915->drm, "async_put_wakeref %u\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~
409 | power_domains->async_put_wakeref);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| intel_wakeref_t {aka long unsigned int}

I don't understand how the above patch can change the compilation like
that. I must be missing something obvious. Maybe my compiler is
strangely broken? I have applied the following instead (which at least
builds):

From: Stephen Rothwell <[email protected]>
Date: Wed, 3 Jan 2024 11:40:26 +1100
Subject: [PATCH] fix up for "drm/i915: Replace custom intel runtime_pm tracker
with ref_tracker library"

Signed-off-by: Stephen Rothwell <[email protected]>
---
drivers/gpu/drm/i915/display/intel_display_power.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 5f091502719b..6253ce061d20 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -406,7 +406,7 @@ print_async_put_domains_state(struct i915_power_domains *power_domains)
display.power.domains);

drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
- power_domains->async_put_wakeref);
+ (unsigned long int)power_domains->async_put_wakeref);

print_power_domains(power_domains, "async_put_domains[0]",
&power_domains->async_put_domains[0]);
--
2.43.0

--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2024-01-03 01:28:00

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the drm tree

Hi all,

On Wed, 3 Jan 2024 12:19:11 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Tue, 2 Jan 2024 11:12:22 +1100 Stephen Rothwell <[email protected]> wrote:
> >
> > After merging the drm tree, today's linux-next build (x86_64 allmodconfig)
> > produced this warning:
> >
> > In file included from include/drm/drm_mm.h:51,
> > from drivers/gpu/drm/xe/xe_bo_types.h:11,
> > from drivers/gpu/drm/xe/xe_bo.h:11,
> > from drivers/gpu/drm/xe/compat-i915-headers/gem/i915_gem_object.h:11,
> > from drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h:15,
> > from drivers/gpu/drm/i915/display/intel_display_power.c:8:
> > drivers/gpu/drm/i915/display/intel_display_power.c: In function 'print_async_put_domains_state':
> > drivers/gpu/drm/i915/display/intel_display_power.c:408:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'int' [-Wformat=]
> > 408 | drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > 409 | power_domains->async_put_wakeref);
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > | |
> > | int
> >
>
> In file included from include/drm/ttm/ttm_resource.h:34,
> from include/drm/ttm/ttm_device.h:30,
> from drivers/gpu/drm/i915/i915_drv.h:37,
> from drivers/gpu/drm/i915/display/intel_display_power.c:8:
> drivers/gpu/drm/i915/display/intel_display_power.c: In function 'print_async_put_domains_state':
> drivers/gpu/drm/i915/display/intel_display_power.c:408:29: error: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'intel_wakeref_t' {aka 'long unsigned int'} [-Werror=format=]
> 408 | drm_dbg(&i915->drm, "async_put_wakeref %u\n",
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> 409 | power_domains->async_put_wakeref);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | |
> | intel_wakeref_t {aka long unsigned int}
>
> I don't understand how the above patch can change the compilation like
> that. I must be missing something obvious. Maybe my compiler is
> strangely broken?

OK, the only thing I can find is that there are 2 intel_wakeref.h files
that have different definitions for intel_wakeref_t:

./drivers/gpu/drm/i915/intel_wakeref.h:typedef unsigned long intel_wakeref_t;
./drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h:typedef bool intel_wakeref_t;

and the two compilations above seem to use different include paths, but
how the single character change causes that is beyond me.
--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2024-01-04 16:52:20

by Jani Nikula

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the drm tree

On Wed, 03 Jan 2024, Stephen Rothwell <[email protected]> wrote:
> OK, the only thing I can find is that there are 2 intel_wakeref.h files
> that have different definitions for intel_wakeref_t:
>
> ./drivers/gpu/drm/i915/intel_wakeref.h:typedef unsigned long intel_wakeref_t;
> ./drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h:typedef bool intel_wakeref_t;
>
> and the two compilations above seem to use different include paths, but
> how the single character change causes that is beyond me.

There are a few things going on here, but the gist of it is that
intel_wakeref_t is supposed to be an opaque cookie, and printing its
value does not make sense, especially not when you can't be certain
which printf format should be used for it.

Fix at [1], thanks for the report.

BR,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]


--
Jani Nikula, Intel