Message-ID: <d1bf4ebe8378e32fa047ecd8912abf772695534c.camel@redhat.com>
Date:   Tue, 16 Aug 2022 14:36:03 -0400
From:   Lyude Paul <lyude@...hat.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>
Cc:     joonas.lahtinen@...ux.intel.com, rodrigo.vivi@...el.com,
        tvrtko.ursulin@...ux.intel.com, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Ville Syrjälä 
        <ville.syrjala@...ux.intel.com>, Zenghui Yu <yuzenghui@...wei.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/i915: Switch TGL-H DP-IN to dGFX when it's supported

On Tue, 2022-08-16 at 14:24 -0400, Lyude Paul 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@...ux.intel.com> wrote:
> > > 
> > > On Tue, 16 Aug 2022, Kai-Heng Feng <kai.heng.feng@...onical.com> 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.

Doing some quick research, unless the models mentioned in the commit message
are unreleased some of them are definitely Intel/Nvidia hybrids. So I'm going
to say:

NAK-by: Lyude Paul <lyude@...hat.com>

If you'd like to resubmit this for systems with amdgpus and Intel only, that's
perfectly fine with me if the Intel folks are ok with it. But please hold off
on this for Nvidia systems, at least until we've got GSP reworks functional in
nouveau. If nvidia's interested in making this work for their driver, they're
welcome to do the work there. For reference: the main limitations you would
hit as a result of this patch would be lagginess on the external displays as a
result of us not being able to reclock the GPU, which means PCIe bandwidth
will be pretty limited.

> 
> 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@...onical.com>
> > > > ---
> > > >  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

Powered by blists - more mailing lists