Linus Torvalds On Linux 6.8 DRM: "Testing Is Seriously Lacking"
Linus Torvalds has been known over the years to air his frustrations occasionally about graphics driver code quality. He's expressed frustrations over DRM driver code quality over the years and occasionally having to reject DRM code. With Linux 6.8 he's expressed frustrations on the Linux kernel mailing list over testing that is "seriously lacking" when it comes to bits of the new Intel Xe drive code.
Torvalds wrote a few minutes ago on the kernel mailing list:
Your testing is seriously lacking.
This doesn't even build. The reason seems to be that commit b49e894c3fd8 ("drm/i915: Replace custom intel runtime_pm tracker with ref_tracker library") changed the 'intel_wakeref_t' type from a 'depot_stack_handle_t' to 'unsigned long', and as a result did this:
- drm_dbg(&i915->drm, "async_put_wakeref %u\n",
+ drm_dbg(&i915->drm, "async_put_wakeref %lu\n",
power_domains->async_put_wakeref);
meanwhile, the Xe driver has this:
drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h:
typedef bool intel_wakeref_t;
which has never been valid, but now the build fails with
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 ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘int’ [-Werror=format=]
because the drm header files have this disgusting thing where a *header* file includes a *C* file:
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:
nasty.
I made it build by fixing that broken Xe compat header file, but this is definitely *NOT* how things should have worked. How did this ever get to me without any kind of build testing?
And why the %^!@$% does a header file include a C file? That's wrong regardless of this bug.
Linus
The message is rather tame compared to some of Torvalds' prior correspondence from years ago, but does raise valid questions over how these Xe driver issues went uncaught. His comments can be found on the LKML while waiting to see if any developers step up and respond.
In any event Linus has gone ahead and merged the new code for Linux 6.8. Hopefully this is just a one-off issue for the Intel Xe driver and not part of larger code quality issues.
46 Comments
