On Thu, 2025-10-30 at 10:48 +0200, Jani Nikula wrote: > 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.)
Arguable but fair points, thanks. Reviewed-by: Luca Coelho <[email protected]> -- Cheers, Luca.
