Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220816025217.618181-1-kai.heng.feng@canonical.com>
In-Reply-To: <20220816025217.618181-1-kai.heng.feng@canonical.com>
From:   Karol Herbst <kherbst@redhat.com>
Date:   Tue, 16 Aug 2022 20:49:58 +0200
Message-ID: <CACO55tt=Op=0E94kK+1M8cDXNCk5Tkc=FMR8=OQFc5ohehjwaw@mail.gmail.com>
Subject: Re: [PATCH] drm/i915: Switch TGL-H DP-IN to dGFX when it's supported
To:     Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc:     jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
        rodrigo.vivi@intel.com, tvrtko.ursulin@linux.intel.com,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        David Airlie <airlied@linux.ie>,
        intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org,
        Hans de Goede <hdegoede@redhat.com>,
        Zenghui Yu <yuzenghui@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Aug 16, 2022 at 4:53 AM Kai-Heng Feng
<kai.heng.feng@canonical.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.
>

Can we please not do things like this just because?

It forces the discrete GPU to be on leading to higher thermal pressure
and power consumption of the system. Lower battery runtime or higher
fan noise is the result. Not everybody wants to use an AC simply just
because they attach an external display.

If the problem is "we run out of displays" then can we have something
more dynamic, where we are doing this only and only if we run out of
resources to drive as that many displays.

Most users will be fine with ports being driven by the iGPU. Why hurt
most users, because of some weird special case with mostly only
drawbacks?

> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.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);
> +       }
>  }
>
>  /*
> --
> 2.36.1
>