Re: [PATCH] drm/i915: Switch TGL-H DP-IN to dGFX when it's supported

[Lyude Paul]

Adding Mark Pearson from Lenovo to this, Mark for reference the original patch
is here:

https://patchwork.freedesktop.org/patch/497807/?series=107312&rev=1

Comments from me down below

On Wed, 2022-08-17 at 09:02 +0800, Kai-Heng Feng wrote:
> On Wed, Aug 17, 2022 at 2:24 AM Lyude Paul <lyude@xxxxxxxxxx> wrote:
> > 
> > On Tue, 2022-08-16 at 19:29 +0800, Kai-Heng Feng wrote:
> > > On Tue, Aug 16, 2022 at 4:06 PM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> > > > 
> > > > On Tue, 16 Aug 2022, Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote:
> > > > > On mobile workstations like HP ZBook Fury G8, iGFX's DP-IN can switch to
> > > > > dGFX so external monitors are routed to dGFX, and more monitors can be
> > > > > supported as result.
> > > > > 
> > > > > To switch the DP-IN to dGFX, the driver needs to invoke _DSM function 20
> > > > > on intel_dsm_guid2. This method is described in Intel document 632107.
> > 
> > Is this documentation released anywhere? We've been wondering about these
> > interfaces for quite a long time, and it would be good to know if there's docs
> > for this we haven't really been seeing.
> > 
> > > > 
> > > > Is this the policy decision that we want to unconditionally make,
> > > > though?
> > > 
> > > I believes so, so more external monitors can be supported at the same time.
> > > 
> > > Kai-Heng
> > 
> > Is this for systems with dual Intel GPUs? I ask because if this affects
> > Intel/Nvidia hybrid systems then this is a huge no from me. Nouveau is able to
> > support these systems, but at a limited capacity. This would imply that we are
> > making external displays work for users of the nvidia proprietary driver, at
> > the expense making external display support for mainline kernel users
> > substantially worse for people who are using the mainline kernel. Which isn't
> > a choice we should be making, because nvidia's OOT driver is not a mainline
> > kernel driver.
> 
> Yes it's for Intel/NVIDIA hybrid systems.
> 
> The problem is that hardware vendor design the systems to use NVIDIA
> for external displays, so using external displays on Intel are never
> tested by the vendors.
> I don't think that's any good either.
> 

Sigh, the constant forcing of nvidia hardware into laptops from vendors is
seriously something I wish they would knock it off with considering they're
basically the most difficult hardware vendor to work with.

Anyway, if we -need- to route displays through the external GPU then we can.
But I'd like to at least get convinced first that this is an actual necessity
we should expect for multiple vendors, or the exception to the rule. Because
if these laptops are capable of driving displays through Intel, at the moment
not doing that is a huge downgrade in terms of functionality. -Especially- if
these machines were already working in the field as-is. Probably worth noting
I don't think I have yet to actually hear of any complaints about this being
the case, and I'd like to also make sure this isn't a change being done for
one or two vendors when most vendors aren't actually doing something like
this.

Note that for a lot of systems it won't -technically- be a big difference
since the current situation in the market right now is that a lot of laptops
will have all their external displays routed through the nvidia GPU and
nowhere else. It's not great compared to just being able to use the well
supported Intel GPU for everything though. And if we're controlling display
routing through ACPI, that implies things aren't directly hooked up and
someone went through the hassle of adding a display mux - which kind of seems
like a waste of engineering effort and money if it can't actually be used for
muxing between the two GPUs. Especially considering that up until very
recently muxes had more or less been dropped from the majority of laptop
vendors (I think Dell was an exception for this fwiw).

Mark, since you're from Lenovo can you help to confirm this as well?

Also re: external displays not even working: so then how exactly does the BIOS
handle this? Is the BIOS changing the routing to the nvidia GPU then switching
it back right before the OS load? I assume something must have been done to
make it so that external displays aren't just suddenly broken there.

And re: gsp work being done soon: it's going to be a while unfortunately,
there's a lot for us to catch up on so it's hard for me to give a precise
date.

> Kai-Heng
> 
> > 
> > If this is just for Intel/Intel systems though that's probably fine, and it
> > might also be fine for AMD systems.
> > 
> > > 
> > > > 
> > > > BR,
> > > > Jani.
> > > > 
> > > > > 
> > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_acpi.c | 18 +++++++++++++++++-
> > > > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c
> > > > > index e78430001f077..3bd5930e2769b 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > > > > @@ -20,6 +20,7 @@ static const guid_t intel_dsm_guid =
> > > > >                 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c);
> > > > > 
> > > > >  #define INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED 0 /* No args */
> > > > > +#define INTEL_DSM_FN_DP_IN_SWITCH_TO_DGFX 20 /* No args */
> > > > > 
> > > > >  static const guid_t intel_dsm_guid2 =
> > > > >       GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260,
> > > > > @@ -187,6 +188,7 @@ void intel_dsm_get_bios_data_funcs_supported(struct drm_i915_private *i915)
> > > > >       struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > > > >       acpi_handle dhandle;
> > > > >       union acpi_object *obj;
> > > > > +     int supported = 0;
> > > > > 
> > > > >       dhandle = ACPI_HANDLE(&pdev->dev);
> > > > >       if (!dhandle)
> > > > > @@ -194,8 +196,22 @@ void intel_dsm_get_bios_data_funcs_supported(struct drm_i915_private *i915)
> > > > > 
> > > > >       obj = acpi_evaluate_dsm(dhandle, &intel_dsm_guid2, INTEL_DSM_REVISION_ID,
> > > > >                               INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED, NULL);
> > > > > -     if (obj)
> > > > > +     if (obj) {
> > > > > +             if (obj->type == ACPI_TYPE_INTEGER)
> > > > > +                     supported = obj->integer.value;
> > > > > +
> > > > >               ACPI_FREE(obj);
> > > > > +     }
> > > > > +
> > > > > +     /* Tiger Lake H DP-IN Boot Time Switching from iGfx to dGfx */
> > > > > +     if (supported & BIT(20)) {
> > > > > +             obj = acpi_evaluate_dsm(dhandle, &intel_dsm_guid2,
> > > > > +                                     INTEL_DSM_REVISION_ID,
> > > > > +                                     INTEL_DSM_FN_DP_IN_SWITCH_TO_DGFX,
> > > > > +                                     NULL);
> > > > > +             if (obj)
> > > > > +                     ACPI_FREE(obj);
> > > > > +     }
> > > > >  }
> > > > > 
> > > > >  /*
> > > > 
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > > 
> > 
> > --
> > Cheers,
> >  Lyude Paul (she/her)
> >  Software Engineer at Red Hat
> > 
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat