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

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

[Kai-Heng Feng]

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.

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

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

[Jani Nikula]

On Tue, 16 Aug 2022, 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.

Is this the policy decision that we want to unconditionally make,
though?

BR,
Jani.

>
> 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);
> +	}
>  }
>  
>  /*

-- 
Jani Nikula, Intel Open Source Graphics Center

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

[Kai-Heng Feng]

On Tue, Aug 16, 2022 at 4:06 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Tue, 16 Aug 2022, 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.
>
> 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

>
> BR,
> Jani.
>
> >
> > 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);
> > +     }
> >  }
> >
> >  /*
>
> --
> Jani Nikula, Intel Open Source Graphics Center

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

[Lyude Paul]

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@linux.intel.com> wrote:
> > 
> > On Tue, 16 Aug 2022, 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.

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.

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@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);
> > > +     }
> > >  }
> > > 
> > >  /*
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> 

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

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

[Lyude Paul]

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@linux.intel.com> wrote:
> > > 
> > > On Tue, 16 Aug 2022, 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.
> 
> 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@redhat.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@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);
> > > > +     }
> > > >  }
> > > > 
> > > >  /*
> > > 
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
> > 
> 

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

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

[Kai-Heng Feng]

On Wed, Aug 17, 2022 at 2:36 AM Lyude Paul <lyude@redhat.com> wrote:
>
> 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@linux.intel.com> wrote:
> > > >
> > > > On Tue, 16 Aug 2022, 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.
> >
> > 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@redhat.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.

The change should be agnostic to any discrete GPU, so I don't think
it's easy to make i915 be aware of the presence of AMD or NVIDIA.
As I replied in previous mail, the external displays may not even work
on Intel GPU, so I really hope we can get this merged.

Will the 'GSP rework' finish anytime soon?

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@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);
> > > > > +     }
> > > > >  }
> > > > >
> > > > >  /*
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > >
> >
>
> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
>

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

[Kai-Heng Feng]

On Wed, Aug 17, 2022 at 2:24 AM Lyude Paul <lyude@redhat.com> 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@linux.intel.com> wrote:
> > >
> > > On Tue, 16 Aug 2022, 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.
>
> 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.

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@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);
> > > > +     }
> > > >  }
> > > >
> > > >  /*
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
> >
>
> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
>

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@redhat.com> 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@linux.intel.com> wrote:
> > > > 
> > > > On Tue, 16 Aug 2022, 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.
> > 
> > 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@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);
> > > > > +     }
> > > > >  }
> > > > > 
> > > > >  /*
> > > > 
> > > > --
> > > > 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

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

[Mark Pearson]

On 2022-08-17 13:56, Lyude Paul wrote:
> 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@redhat.com> 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@linux.intel.com> wrote:
>>>>>
>>>>> On Tue, 16 Aug 2022, 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.
>>>
>>> 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?

I'll have to do some checking for this years models and get back to you
I know on last years that we had a HW mux available on the P1. There
wasn't one on the P17 (and I'm not sure about the P15).

My understanding is the Intel display just wasn't available at all
for external display on the P17 (and I do agree with your comments on
the wiseness of this!). I believe it was hardwired with no way to switch
it - we lost a sale from this, and if there had been an alternative we
would have pursued it.
I'll confirm with the HW/FW team.

We also have lots of users on Legion gaming systems - but unfortunately I
don't know what happens on those platforms as they're not in our Linux
program.

Mark

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

[Karol Herbst]

On Wed, Aug 24, 2022 at 7:50 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> On Tue, Aug 16, 2022 at 4:06 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Tue, 16 Aug 2022, 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.
> >
> > 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.
>

if there wouldn't be any drawbacks, yes, but sadly there are and I
don't see that hurting _all_ users affected with this by making their
system consume/generate around 10-15W more power/heat just that maybe
one user can use 4 instead of 3 displays at 4K is really worth it...

> Kai-Heng
>
> >
> > BR,
> > Jani.
> >
> > >
> > > 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);
> > > +     }
> > >  }
> > >
> > >  /*
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
>

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

[Lukas Wunner]

On Tue, Aug 16, 2022 at 11:06:18AM +0300, Jani Nikula wrote:
> On Tue, 16 Aug 2022, 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.
> 
> Is this the policy decision that we want to unconditionally make,
> though?

In general, we handle switching of outputs between GPUs in vga_switcheroo.c
upon a request from user space via sysfs (well, debugfs currently).
It's up to users to decide which policy suits their needs best.

That said, we never grew support to allow different switching policies for
the built-in panel and external outputs.  Laptops supporting this are
rare.  Older MacBook Pros introduced between 2008 and 2010 are among them:
They have separate muxes for the panel and external DP port.  Our policy
is documented in a code comment in drivers/platform/x86/apple-gmux.c:

 * The external DP port is only fully switchable on the first two unibody
 * MacBook Pro generations, MBP5 2008/09 and MBP6 2010. This is done by an
 * `NXP CBTL06141`_ which is controlled by gmux.
 [...]
 * Our switching policy for the external port is that on those generations
 * which are able to switch it fully, the port is switched together with the
 * panel when IGD / DIS commands are issued to vga_switcheroo. It is thus
 * possible to drive e.g. a beamer on battery power with the integrated GPU.
 * The user may manually switch to the discrete GPU if more performance is
 * needed.
 *
 * On all newer generations, the external port can only be driven by the
 * discrete GPU. If a display is plugged in while the panel is switched to
 * the integrated GPU, *both* GPUs will be in use for maximum performance.
 * To decrease power consumption, the user may manually switch to the
 * discrete GPU, thereby suspending the integrated GPU.

In other words, on these older MacBook Pros, we switch the panel and
external DP port in unison, thus always allowing one of the two GPUs
to runtime suspend and save power.

Thanks,

Lukas

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

[Karol Herbst]

On Thu, Aug 18, 2022 at 2:09 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Tue, Aug 16, 2022 at 11:06:18AM +0300, Jani Nikula wrote:
> > On Tue, 16 Aug 2022, 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.
> >
> > Is this the policy decision that we want to unconditionally make,
> > though?
>
> In general, we handle switching of outputs between GPUs in vga_switcheroo.c
> upon a request from user space via sysfs (well, debugfs currently).
> It's up to users to decide which policy suits their needs best.
>
> That said, we never grew support to allow different switching policies for
> the built-in panel and external outputs.  Laptops supporting this are
> rare.  Older MacBook Pros introduced between 2008 and 2010 are among them:
> They have separate muxes for the panel and external DP port.  Our policy
> is documented in a code comment in drivers/platform/x86/apple-gmux.c:
>
>  * The external DP port is only fully switchable on the first two unibody
>  * MacBook Pro generations, MBP5 2008/09 and MBP6 2010. This is done by an
>  * `NXP CBTL06141`_ which is controlled by gmux.
>  [...]
>  * Our switching policy for the external port is that on those generations
>  * which are able to switch it fully, the port is switched together with the
>  * panel when IGD / DIS commands are issued to vga_switcheroo. It is thus
>  * possible to drive e.g. a beamer on battery power with the integrated GPU.
>  * The user may manually switch to the discrete GPU if more performance is
>  * needed.
>  *
>  * On all newer generations, the external port can only be driven by the
>  * discrete GPU. If a display is plugged in while the panel is switched to
>  * the integrated GPU, *both* GPUs will be in use for maximum performance.
>  * To decrease power consumption, the user may manually switch to the
>  * discrete GPU, thereby suspending the integrated GPU.
>
> In other words, on these older MacBook Pros, we switch the panel and
> external DP port in unison, thus always allowing one of the two GPUs
> to runtime suspend and save power.
>
> Thanks,
>
> Lukas
>

sure, but this is changing now. I do have a laptop with a muxable
internal display. But this is considered to be a dynamic on demand
switching thing not a boot time switch.

Anyway, I am still not convinced that doing that unconditionally is
what we want, especially as userspace has to support dynamic switching
regardless.

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

[Kai-Heng Feng]

On Sat, Aug 20, 2022 at 1:01 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Thu, Aug 18, 2022 at 2:09 PM Lukas Wunner <lukas@wunner.de> wrote:
> >
> > On Tue, Aug 16, 2022 at 11:06:18AM +0300, Jani Nikula wrote:
> > > On Tue, 16 Aug 2022, 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.
> > >
> > > Is this the policy decision that we want to unconditionally make,
> > > though?
> >
> > In general, we handle switching of outputs between GPUs in vga_switcheroo.c
> > upon a request from user space via sysfs (well, debugfs currently).
> > It's up to users to decide which policy suits their needs best.
> >
> > That said, we never grew support to allow different switching policies for
> > the built-in panel and external outputs.  Laptops supporting this are
> > rare.  Older MacBook Pros introduced between 2008 and 2010 are among them:
> > They have separate muxes for the panel and external DP port.  Our policy
> > is documented in a code comment in drivers/platform/x86/apple-gmux.c:
> >
> >  * The external DP port is only fully switchable on the first two unibody
> >  * MacBook Pro generations, MBP5 2008/09 and MBP6 2010. This is done by an
> >  * `NXP CBTL06141`_ which is controlled by gmux.
> >  [...]
> >  * Our switching policy for the external port is that on those generations
> >  * which are able to switch it fully, the port is switched together with the
> >  * panel when IGD / DIS commands are issued to vga_switcheroo. It is thus
> >  * possible to drive e.g. a beamer on battery power with the integrated GPU.
> >  * The user may manually switch to the discrete GPU if more performance is
> >  * needed.
> >  *
> >  * On all newer generations, the external port can only be driven by the
> >  * discrete GPU. If a display is plugged in while the panel is switched to
> >  * the integrated GPU, *both* GPUs will be in use for maximum performance.
> >  * To decrease power consumption, the user may manually switch to the
> >  * discrete GPU, thereby suspending the integrated GPU.
> >
> > In other words, on these older MacBook Pros, we switch the panel and
> > external DP port in unison, thus always allowing one of the two GPUs
> > to runtime suspend and save power.
> >
> > Thanks,
> >
> > Lukas
> >
>
> sure, but this is changing now. I do have a laptop with a muxable
> internal display. But this is considered to be a dynamic on demand
> switching thing not a boot time switch.
>
> Anyway, I am still not convinced that doing that unconditionally is
> what we want, especially as userspace has to support dynamic switching
> regardless.
>

According to the doc, there's no MUX in TGL-H DP-IN. The dGPU outputs
are routed through TGL-H TCSS directly.
That's probably the reason why it can't be dynamic.

Kai-Heng

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

[Karol Herbst]

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
>

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

[Kai-Heng Feng]

On Wed, Aug 17, 2022 at 2:50 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> 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?

I there's a very good reason to support more external monitors,
especially when eDP is already 4K so iGPU don't have enough buffer for
more displays.

>
> 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.

The system is designed in this way.

And many (if not all) gaming laptops and mobile workstations use
discrete GPU for external monitors.
We just recently found out we have to use a switch to make it work.

>
> 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.

This is a boot-time switch, so it's not possible to switch it dynamically.

>
> 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?

This is a use case that hardware vendor never bother to test.
And this is not a special case - the system is designed to use dGPU
for external monitors.

Kai-Heng

>
> > 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
> >
>

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

[Karol Herbst]

On Wed, Aug 17, 2022 at 3:18 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> On Wed, Aug 17, 2022 at 2:50 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > 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?
>
> I there's a very good reason to support more external monitors,
> especially when eDP is already 4K so iGPU don't have enough buffer for
> more displays.
>

well.. they do have it. What's the limit? 3 or 4 4K displays with gen
11th+? I find conflicting information, but 3 4K displays are no
problem. It might be if you get to higher refresh rates or something.

I know that 2 work quite reliably and I know I can put even more on
the Intel GPU.

> >
> > 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.
>
> The system is designed in this way.
>

?!? This makes no sense. If the discrete GPU is turned on, it means
the system has to cool away more heat, because it consumes more power.
It then causes louder fans. No idea how a "system design" can just go
around simple physics...

Even the CPU consumes more power, because on some systems it prevents
deeper package sleeping modes due to the active PCIe bridge
controller.

But if you have certain systems where you want to enable this behavior
despite the drawbacks, maybe maintain a list of systems where to apply
this method?

> And many (if not all) gaming laptops and mobile workstations use
> discrete GPU for external monitors.
> We just recently found out we have to use a switch to make it work.
>

yeah some do, and if people buy those, they already deal with loud
fans and just accept this fact.

Others might want silent fans... and why do you have to switch? Out of
the box Intel GPUs support 3 4K displays. I want to see the general
use case for 4 4K displays.

So what systems are actually affected and do users have the option to
disable it, if they prefer a more silent system?

> >
> > 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.
>
> This is a boot-time switch, so it's not possible to switch it dynamically.
>

This makes it even worse.

> >
> > 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?
>
> This is a use case that hardware vendor never bother to test.
> And this is not a special case - the system is designed to use dGPU
> for external monitors.
>
> Kai-Heng
>

so instead of hard wiring, they added a software switch to do the same thing?

And then don't bother to test both possibilities?

Anyway.. it doesn't make any sense and this opens up more questions
than I initially had.

I honestly still don't see the point here and still doubt that
pleasing a handful of users is worth accepting the drawbacks.

I also have no say if it comes to the i915 driver, but from a user
perspective none of this makes much sense tbh.

> >
> > > 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
> > >
> >
>

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

[Kai-Heng Feng]

On Wed, Aug 17, 2022 at 9:49 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Wed, Aug 17, 2022 at 3:18 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > On Wed, Aug 17, 2022 at 2:50 AM Karol Herbst <kherbst@redhat.com> wrote:
> > >
> > > 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?
> >
> > I there's a very good reason to support more external monitors,
> > especially when eDP is already 4K so iGPU don't have enough buffer for
> > more displays.
> >
>
> well.. they do have it. What's the limit? 3 or 4 4K displays with gen
> 11th+? I find conflicting information, but 3 4K displays are no
> problem. It might be if you get to higher refresh rates or something.
>
> I know that 2 work quite reliably and I know I can put even more on
> the Intel GPU.

More monitors can be supported via a thunderbolt dock.

>
> > >
> > > 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.
> >
> > The system is designed in this way.
> >
>
> ?!? This makes no sense. If the discrete GPU is turned on, it means
> the system has to cool away more heat, because it consumes more power.
> It then causes louder fans. No idea how a "system design" can just go
> around simple physics...

The spec from HP [1] says:
Multi Display Support
Without HP Thunderbolt™ Dock G2
UMA Graphics: Unit supports up to 4 independent displays. Any
combination of displays outputs may be used except one of
Thunderbolt™ 4 and HDMI.
Hybrid Graphics: Unit supports up 5 simultaneous displays (4 from dGPU
+ 1 from iGPU). Any combination of displays outputs may
be used except when using one USBC and HDMI are exclusive

With HP Thunderbolt™ Dock G2
UMA Graphics: Unit supports up to 4 simultaneous displays. Any
combination of displays outputs may be used except one of
Thunderbolt™ 4 and HDMI.
Hybrid Graphics (NVIDIA): Unit supports up to 5 simultaneous displays
(4 from dGPU + 1 from iGPU). Any combination of displays
outputs may be used except when using one USBC and HDMI are exclusive
Hybrid Graphics (AMD): Unit supports up to 5 simultaneous displays (5
from dGPU + 1 from iGPU). Any combination of displays
outputs may be used except when using one USBC and HDMI are exclusive

So it's "designed" to use dGPU on the hybrid configs.

Let's hope the copper tubes have can dissipate the heat fast enough.

>
> Even the CPU consumes more power, because on some systems it prevents
> deeper package sleeping modes due to the active PCIe bridge
> controller.
>
> But if you have certain systems where you want to enable this behavior
> despite the drawbacks, maybe maintain a list of systems where to apply
> this method?

The behavior will be enabled only when _DSM function 20 is present.
So it's already a selected few.

>
> > And many (if not all) gaming laptops and mobile workstations use
> > discrete GPU for external monitors.
> > We just recently found out we have to use a switch to make it work.
> >
>
> yeah some do, and if people buy those, they already deal with loud
> fans and just accept this fact.
>
> Others might want silent fans... and why do you have to switch? Out of
> the box Intel GPUs support 3 4K displays. I want to see the general
> use case for 4 4K displays.

It's not for us to decide.
When a user buys a laptop according to the spec, the expectation is to
have those supported.

>
> So what systems are actually affected and do users have the option to
> disable it, if they prefer a more silent system?

Maybe adding a new i915 option to override the default behavior? But
it depends on i915 maintainers' opinion.

>
> > >
> > > 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.
> >
> > This is a boot-time switch, so it's not possible to switch it dynamically.
> >
>
> This makes it even worse.
>
> > >
> > > 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?
> >
> > This is a use case that hardware vendor never bother to test.
> > And this is not a special case - the system is designed to use dGPU
> > for external monitors.
> >
> > Kai-Heng
> >
>
> so instead of hard wiring, they added a software switch to do the same thing?

I think it's a TGL-H thing, to provide a way so discrete GPU can use
the generic TCSS. I don't know the reason though.

>
> And then don't bother to test both possibilities?

That's ODM and hardware vendors :)

>
> Anyway.. it doesn't make any sense and this opens up more questions
> than I initially had.
>
> I honestly still don't see the point here and still doubt that
> pleasing a handful of users is worth accepting the drawbacks.

Users may purchase the system based on the number of external monitors
it can support.
Besides, the drawbacks (if any) will only happen on systems with this
design (i.e. with _DSM function 20 supported).

>
> I also have no say if it comes to the i915 driver, but from a user
> perspective none of this makes much sense tbh.

It makes much sense for users that need more monitors.

[1] https://h20195.www2.hp.com/v2/GetDocument.aspx?docname=c07606964

Kai-Heng


>
> > >
> > > > 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
> > > >
> > >
> >
>

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

[Ville Syrjälä]

On Wed, Aug 17, 2022 at 11:04:21AM +0800, Kai-Heng Feng wrote:
> On Wed, Aug 17, 2022 at 9:49 AM Karol Herbst <kherbst@redhat.com> wrote:
> >
> > On Wed, Aug 17, 2022 at 3:18 AM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> > >
> > > On Wed, Aug 17, 2022 at 2:50 AM Karol Herbst <kherbst@redhat.com> wrote:
> > > >
> > > > 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?
> > >
> > > I there's a very good reason to support more external monitors,
> > > especially when eDP is already 4K so iGPU don't have enough buffer for
> > > more displays.
> > >
> >
> > well.. they do have it. What's the limit? 3 or 4 4K displays with gen
> > 11th+? I find conflicting information, but 3 4K displays are no
> > problem. It might be if you get to higher refresh rates or something.
> >
> > I know that 2 work quite reliably and I know I can put even more on
> > the Intel GPU.
> 
> More monitors can be supported via a thunderbolt dock.
> 
> >
> > > >
> > > > 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.
> > >
> > > The system is designed in this way.
> > >
> >
> > ?!? This makes no sense. If the discrete GPU is turned on, it means
> > the system has to cool away more heat, because it consumes more power.
> > It then causes louder fans. No idea how a "system design" can just go
> > around simple physics...
> 
> The spec from HP [1] says:
> Multi Display Support
> Without HP Thunderbolt™ Dock G2
> UMA Graphics: Unit supports up to 4 independent displays. Any
> combination of displays outputs may be used except one of
> Thunderbolt™ 4 and HDMI.
> Hybrid Graphics: Unit supports up 5 simultaneous displays (4 from dGPU
> + 1 from iGPU). Any combination of displays outputs may
> be used except when using one USBC and HDMI are exclusive
> 
> With HP Thunderbolt™ Dock G2
> UMA Graphics: Unit supports up to 4 simultaneous displays. Any
> combination of displays outputs may be used except one of
> Thunderbolt™ 4 and HDMI.
> Hybrid Graphics (NVIDIA): Unit supports up to 5 simultaneous displays
> (4 from dGPU + 1 from iGPU). Any combination of displays
> outputs may be used except when using one USBC and HDMI are exclusive
> Hybrid Graphics (AMD): Unit supports up to 5 simultaneous displays (5
> from dGPU + 1 from iGPU). Any combination of displays
> outputs may be used except when using one USBC and HDMI are exclusive
> 
> So it's "designed" to use dGPU on the hybrid configs.
> 
> Let's hope the copper tubes have can dissipate the heat fast enough.
> 
> >
> > Even the CPU consumes more power, because on some systems it prevents
> > deeper package sleeping modes due to the active PCIe bridge
> > controller.
> >
> > But if you have certain systems where you want to enable this behavior
> > despite the drawbacks, maybe maintain a list of systems where to apply
> > this method?
> 
> The behavior will be enabled only when _DSM function 20 is present.
> So it's already a selected few.

I had a quick trawl through some Windows stuff for this and
it does seem to do a few extra checks:
- platform must be TGL-H (nothing else has the DPin stuff I guess)
- OpRegion header must indicate dGPU presence

Otherwise it does call this DSM uncoditionally on boot/S4 resume
so seems like that is the only really validated configuration.
Although it does seem to explicitly turn off displays prior to
the DSM so that does perhaps indicate that those ports might have
also been enabled via the iGPU by the BIOS. Not sure if disabling
the ports would work correctly after the DSM or not. If not then
the DSM call would need to happen after state readout/sanitization
so that we can shut things down gracefully.

Additionally after the DSM call it scans the FIA TC live state
bits to check for DPin usage. Looks like its trying to make sure
the driver stops poking at the relevant power wells once in DPin
mode. i915 doesn't check that stuff atm so we might end up
mangling something while the dGPU is driving the port.

-- 
Ville Syrjälä
Intel

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

[Kai-Heng Feng]

On Wed, Aug 17, 2022 at 7:59 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:

[snipped]

> I had a quick trawl through some Windows stuff for this and
> it does seem to do a few extra checks:
> - platform must be TGL-H (nothing else has the DPin stuff I guess)
> - OpRegion header must indicate dGPU presence

Is the dGPU presence denoted by the return bitmask of
INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED?

IIUC the mask 20 won't be set when dGPU is not present.

>
> Otherwise it does call this DSM uncoditionally on boot/S4 resume
> so seems like that is the only really validated configuration.
> Although it does seem to explicitly turn off displays prior to
> the DSM so that does perhaps indicate that those ports might have
> also been enabled via the iGPU by the BIOS. Not sure if disabling
> the ports would work correctly after the DSM or not. If not then
> the DSM call would need to happen after state readout/sanitization
> so that we can shut things down gracefully.
>
> Additionally after the DSM call it scans the FIA TC live state
> bits to check for DPin usage. Looks like its trying to make sure
> the driver stops poking at the relevant power wells once in DPin
> mode. i915 doesn't check that stuff atm so we might end up
> mangling something while the dGPU is driving the port.

Thanks for investigating this. I am not really familiar with other
stuffs you mentioned, but I am happy to test any follow-up patch.

Kai-Heng

>
> --
> Ville Syrjälä
> Intel

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

[Ville Syrjälä]

On Wed, Aug 17, 2022 at 08:15:58PM +0800, Kai-Heng Feng wrote:
> On Wed, Aug 17, 2022 at 7:59 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> 
> [snipped]
> 
> > I had a quick trawl through some Windows stuff for this and
> > it does seem to do a few extra checks:
> > - platform must be TGL-H (nothing else has the DPin stuff I guess)
> > - OpRegion header must indicate dGPU presence
> 
> Is the dGPU presence denoted by the return bitmask of
> INTEL_DSM_FN_GET_BIOS_DATA_FUNCS_SUPPORTED

No, there are apparently some extra bits in the OpRegion
header which we're not currently even decoding.

> 
> IIUC the mask 20 won't be set when dGPU is not present.

Not sure whether that bit would change depending on the dGPU
presence or not. Windows doesn't seem trust it alone, so either
it won't change or someone was just extra paranoid.

> 
> >
> > Otherwise it does call this DSM uncoditionally on boot/S4 resume
> > so seems like that is the only really validated configuration.
> > Although it does seem to explicitly turn off displays prior to
> > the DSM so that does perhaps indicate that those ports might have
> > also been enabled via the iGPU by the BIOS. Not sure if disabling
> > the ports would work correctly after the DSM or not. If not then
> > the DSM call would need to happen after state readout/sanitization
> > so that we can shut things down gracefully.
> >
> > Additionally after the DSM call it scans the FIA TC live state
> > bits to check for DPin usage. Looks like its trying to make sure
> > the driver stops poking at the relevant power wells once in DPin
> > mode. i915 doesn't check that stuff atm so we might end up
> > mangling something while the dGPU is driving the port.
> 
> Thanks for investigating this. I am not really familiar with other
> stuffs you mentioned, but I am happy to test any follow-up patch.
> 
> Kai-Heng
> 
> >
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel