On Thu, 30 Oct 2025, Luca Coelho <[email protected]> wrote: > On Wed, 2025-10-22 at 18:17 +0300, Jani Nikula wrote: >> Add intel_display_utils.c for display utilities that need more than a >> header. >> >> Start off with intel_display_run_as_guest(). The implementation is >> intentional duplication of the i915_utils.h i915_run_as_guest(), with >> the idea that it's small enough to not matter. >> >> Signed-off-by: Jani Nikula <[email protected]> >> --- > > > >> drivers/gpu/drm/i915/Makefile | 1 + >> .../gpu/drm/i915/display/intel_display_utils.c | 18 ++++++++++++++++++ >> .../gpu/drm/i915/display/intel_display_utils.h | 6 ++++++ >> drivers/gpu/drm/i915/display/intel_pch.c | 4 ++-- >> drivers/gpu/drm/xe/Makefile | 1 + >> 5 files changed, 28 insertions(+), 2 deletions(-) >> create mode 100644 drivers/gpu/drm/i915/display/intel_display_utils.c >> >> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile >> index 47bac9b2c611..046f9282fb65 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -255,6 +255,7 @@ i915-y += \ >> display/intel_display_rpm.o \ >> display/intel_display_rps.o \ >> display/intel_display_snapshot.o \ >> + display/intel_display_utils.o \ >> display/intel_display_wa.o \ >> display/intel_dmc.o \ >> display/intel_dmc_wl.o \ >> diff --git a/drivers/gpu/drm/i915/display/intel_display_utils.c >> b/drivers/gpu/drm/i915/display/intel_display_utils.c >> new file mode 100644 >> index 000000000000..13d3999dd580 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/display/intel_display_utils.c >> @@ -0,0 +1,18 @@ >> +// SPDX-License-Identifier: MIT >> +/* Copyright © 2025 Intel Corporation */ >> + >> +#ifdef CONFIG_X86 >> +#include <asm/hypervisor.h> >> +#endif >> + >> +#include "intel_display_utils.h" >> + >> +bool intel_display_run_as_guest(struct intel_display *display) >> +{ >> +#if IS_ENABLED(CONFIG_X86) >> + return !hypervisor_is_type(X86_HYPER_NATIVE); >> +#else >> + /* Not supported yet */ >> + return false; >> +#endif >> +} > > Why can't this be an inline in the header file?
I'll turn it around. I think there needs to be a rationale for inlining, not the other way around. A regular function should be the default. I think the primary reason for inlining would be performance, but I'll accept small "superfluous" static inlines that don't require pulling in other headers. I don't think either is true here. Additionally the static inline exposes all of that ifdef mess and the implementation details in the header too. A change in asm/hypervisor.h leads to a rebuild of everything that includes intel_display_utils.h, making the header dependencies worse. (Maybe a change in asm/hypervisor.h leads to a rebuild of everything anyway, but you get the general point.) BR, Jani. -- Jani Nikula, Intel
