On Fri, 14 May 2021, Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote: > On Wed, May 12, 2021 at 2:19 AM Ville Syrjälä > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >> >> On Mon, Apr 26, 2021 at 11:24:10PM +0800, Kai-Heng Feng wrote: >> > On HP Fury G7 Workstations, graphics output is re-routed from Intel GFX >> > to discrete GFX after S3. This is not desirable, because userspace will >> > treat connected display as a new one, losing display settings. >> > >> > The expected behavior is to let discrete GFX drives all external >> > displays. >> > >> > The platform in question uses ACPI method \_SB.PCI0.HGME to enable MUX. >> > The method is inside the another _DSM, so add the _DSM and call it >> > accordingly. >> > >> > I also tested some MUX-less and iGPU only laptops with that _DSM, no >> > regression was found. >> > >> > v3: >> > - Remove BXT from names. >> > - Change the parameter type. >> > - Fold the function into intel_modeset_init_hw(). >> > >> > v2: >> > - Forward declare struct pci_dev. >> > >> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3113 >> > References: https://lore.kernel.org/intel-gfx/1460040732-31417-4-git-send-email-animesh.manna@xxxxxxxxx/ >> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/display/intel_acpi.c | 18 ++++++++++++++++++ >> > drivers/gpu/drm/i915/display/intel_acpi.h | 3 +++ >> > drivers/gpu/drm/i915/display/intel_display.c | 2 ++ >> > 3 files changed, 23 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c >> > index 833d0c1be4f1..d008d3976261 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c >> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c >> > @@ -13,12 +13,17 @@ >> > #include "intel_display_types.h" >> > >> > #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */ >> > +#define INTEL_DSM_FN_PLATFORM_MUX_ENABLE 0 /* No args */ >> >> This block of defines is for the other DSM. We don't want to >> mix these up. We also want to name it according to the spec, >> so something like GET_BIOS_DATA_FUNCS_SUPPORTED. Similarly >> for the intel_dsm_enable_mux() wrapper function. + it needs >> a comment to document that some BIOSes abuse it to do MUX >> initialization and whatnot. > > Will do. > > >> >> We should perhaps rename all the old DSM stuff to >> something a bit less generic as well... > > I can rename them as well. But what's the naming scheme you prefer? > >> >> > #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */ >> > >> > static const guid_t intel_dsm_guid = >> > GUID_INIT(0x7ed873d3, 0xc2d0, 0x4e4f, >> > 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c); >> > >> > +static const guid_t intel_dsm_guid2 = >> > + GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260, >> > + 0x9d, 0x15, 0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14); >> > + >> > static char *intel_dsm_port_name(u8 id) >> > { >> > switch (id) { >> > @@ -176,6 +181,19 @@ void intel_unregister_dsm_handler(void) >> > { >> > } >> > >> > +void intel_dsm_enable_mux(struct drm_i915_private *i915) >> > +{ >> > + struct pci_dev *pdev = i915->drm.pdev; >> > + acpi_handle dhandle; >> > + >> > + dhandle = ACPI_HANDLE(&pdev->dev); >> > + if (!dhandle) >> > + return; >> > + >> > + acpi_evaluate_dsm(dhandle, &intel_dsm_guid2, INTEL_DSM_REVISION_ID, >> > + INTEL_DSM_FN_PLATFORM_MUX_ENABLE, NULL); >> > +} >> > + >> > /* >> > * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All Devices >> > * Attached to the Display Adapter). >> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h b/drivers/gpu/drm/i915/display/intel_acpi.h >> > index e8b068661d22..def013cf6308 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_acpi.h >> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.h >> > @@ -11,11 +11,14 @@ struct drm_i915_private; >> > #ifdef CONFIG_ACPI >> > void intel_register_dsm_handler(void); >> > void intel_unregister_dsm_handler(void); >> > +void intel_dsm_enable_mux(struct drm_i915_private *i915); >> > void intel_acpi_device_id_update(struct drm_i915_private *i915); >> > #else >> > static inline void intel_register_dsm_handler(void) { return; } >> > static inline void intel_unregister_dsm_handler(void) { return; } >> > static inline >> > +void intel_dsm_enable_mux(struct drm_i915_private *i915) { return; } >> > +static inline >> > void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; } >> > #endif /* CONFIG_ACPI */ >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> > index a10e26380ef3..d79dae370b20 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_display.c >> > +++ b/drivers/gpu/drm/i915/display/intel_display.c >> > @@ -11472,6 +11472,8 @@ void intel_modeset_init_hw(struct drm_i915_private *i915) >> > { >> > struct intel_cdclk_state *cdclk_state; >> > >> > + intel_dsm_enable_mux(i915); >> > + >> >> This should probably be somewhere around where we do all the other >> semi ACPI related init (OpRegion/etc.). > > Hmm, but Jani prefers to put it inside intel_modeset_*() helpers. But > I don't see any opregion related functions are being called by > intel_modeset_*() helpers. Any suggestion? I think I mainly wanted it in intel_display.c instead of at the driver top level. BR, Jani. > > Kai-Heng > >> >> > if (!HAS_DISPLAY(i915)) >> > return; >> > >> > -- >> > 2.30.2 >> >> -- >> Ville Syrjälä >> Intel -- Jani Nikula, Intel Open Source Graphics Center
