Linus Torvalds On Linux 6.8 DRM: "Testing Is Seriously Lacking"

Written by Michael Larabel in Linux Kernel on 12 January 2024 at 02:47 PM EST. 44 Comments
LINUX KERNEL
While the Direct Rendering Manager (DRM) kernel graphics/display driver updates for Linux 6.8 excitingly include the new Intel "Xe" DRM and PowerVR Imagination drivers, AMD color management properties in experimental form, Raspberry Pi 5 graphics support, and more, Linus Torvalds isn't happy with some of the new Intel Xe driver code.

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.

Graphics cards and beer testing...


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.
Related News
About The Author
Michael Larabel

Michael Larabel is the principal author of Phoronix.com and founded the site in 2004 with a focus on enriching the Linux hardware experience. Michael has written more than 20,000 articles covering the state of Linux hardware support, Linux performance, graphics drivers, and other topics. Michael is also the lead developer of the Phoronix Test Suite, Phoromatic, and OpenBenchmarking.org automated benchmarking software. He can be followed via Twitter, LinkedIn, or contacted via MichaelLarabel.com.

Popular News This Week