* [PATCH v2 1/5] drm/i915/bxt: VBT changes for hpd as wakeup feature
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
@ 2016-04-07 14:52 ` Animesh Manna
2016-04-07 14:52 ` [PATCH v2 2/5] drm/i915/bxt: Added _DSM call to set HPD_CTL Animesh Manna
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Animesh Manna @ 2016-04-07 14:52 UTC (permalink / raw)
To: intel-gfx
To support hpd during sleep a new feature flag is
added in vbt and also in dev_priv for enabling/disabling
inside driver. By default this feature will be
disabled and based on oem request this feature can
be enabled by changing vbt feature flag.
v1: Initial version as RFC.
v2: Based on review comments from Jani,
- Used bool instead of enum for hpd feature flag.
- Updating feature flag at the first place based on vbt
entry.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_bios.c | 6 ++++++
drivers/gpu/drm/i915/intel_vbt_defs.h | 3 ++-
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd18772..c604f03 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1485,6 +1485,9 @@ struct intel_vbt_data {
const u8 *sequence[MIPI_SEQ_MAX];
} dsi;
+ /* HPD as wakesoure for DC9 BXT */
+ bool hpd_wakeup_enabled;
+
int crt_ddc_pin;
int child_dev_num;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 9c406b0..d5cd879 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -529,6 +529,12 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
dev_priv->vbt.edp.support = 1;
+ if (driver->hpd_wakeup_source) {
+ dev_priv->vbt.hpd_wakeup_enabled = true;
+ DRM_DEBUG_KMS("HPD as wakeup feature is enabled\n");
+ } else
+ DRM_DEBUG_KMS("HPD wakeup source feature is disabled in VBT\n");
+
DRM_DEBUG_KMS("DRRS State Enabled:%d\n", driver->drrs_enabled);
/*
* If DRRS is not supported, drrs_type has to be set to 0.
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 749dcea..8e2b765 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -547,7 +547,8 @@ struct bdb_driver_features {
u16 tbt_enabled:1;
u16 psr_enabled:1;
u16 ips_enabled:1;
- u16 reserved3:4;
+ u16 reserved3:3;
+ u16 hpd_wakeup_source:1;
u16 pc_feature_valid:1;
} __packed;
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 2/5] drm/i915/bxt: Added _DSM call to set HPD_CTL.
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
2016-04-07 14:52 ` [PATCH v2 1/5] drm/i915/bxt: VBT changes for hpd as wakeup feature Animesh Manna
@ 2016-04-07 14:52 ` Animesh Manna
2016-04-07 14:52 ` [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt Animesh Manna
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Animesh Manna @ 2016-04-07 14:52 UTC (permalink / raw)
To: intel-gfx
_DSM is added to program HPD_CTL(0x1094) register
of PMC from i915 driver which will be called
based on driver feature flag. PMC hpd control register
programming will enable PMC to get hpd interrupt
during dc9.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/intel_acpi.c | 45 +++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..8f3d672 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -10,6 +10,8 @@
#define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
#define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
+#define INTEL_DSM_SET_HPD_WAKEUP 17
+#define HPD_WAKEUP_EN_VAL 0xFCF0
static struct intel_dsm_priv {
acpi_handle dhandle;
@@ -110,23 +112,52 @@ static void intel_dsm_platform_mux_info(void)
ACPI_FREE(pkg);
}
+static void intel_dsm_set_hpd_wakeup(void)
+{
+ union acpi_object *obj;
+ union acpi_object argv4 = {
+ .integer.type = ACPI_TYPE_INTEGER,
+ .integer.value = HPD_WAKEUP_EN_VAL,
+ };
+
+ obj = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, intel_dsm_guid,
+ INTEL_DSM_REVISION_ID, INTEL_DSM_SET_HPD_WAKEUP,
+ &argv4, ACPI_TYPE_INTEGER);
+
+ if (!obj)
+ DRM_DEBUG_DRIVER("failed to evaluate _DSM\n");
+
+ ACPI_FREE(obj);
+}
+
static bool intel_dsm_pci_probe(struct pci_dev *pdev)
{
acpi_handle dhandle;
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct drm_i915_private *dev_priv = dev->dev_private;
dhandle = ACPI_HANDLE(&pdev->dev);
- if (!dhandle)
- return false;
- if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
- 1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
- DRM_DEBUG_KMS("no _DSM method for intel device\n");
+ if (!dhandle)
return false;
- }
intel_dsm_priv.dhandle = dhandle;
- intel_dsm_platform_mux_info();
+ if (acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
+ 1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
+ intel_dsm_platform_mux_info();
+ else
+ DRM_DEBUG_KMS("no _DSM method for mux-info\n");
+
+ /* Need to ensure vbt parsing is completed. */
+ if (dev_priv->vbt.hpd_wakeup_enabled &&
+ acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
+ 1 << INTEL_DSM_SET_HPD_WAKEUP))
+ intel_dsm_set_hpd_wakeup();
+ else {
+ dev_priv->vbt.hpd_wakeup_enabled = false;
+ DRM_DEBUG_KMS("no _DSM method for hpd-enabling\n");
+ }
return true;
}
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt.
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
2016-04-07 14:52 ` [PATCH v2 1/5] drm/i915/bxt: VBT changes for hpd as wakeup feature Animesh Manna
2016-04-07 14:52 ` [PATCH v2 2/5] drm/i915/bxt: Added _DSM call to set HPD_CTL Animesh Manna
@ 2016-04-07 14:52 ` Animesh Manna
2016-04-08 9:00 ` Jani Nikula
2016-04-07 14:52 ` [PATCH v2 4/5] drm/i915/bxt: Block D3 during suspend Animesh Manna
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Animesh Manna @ 2016-04-07 14:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Bharath K Veera, Ananth Krishna R
Guid is changed for bxt platform, so corrected the guid for bxt.
Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/intel_acpi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index 8f3d672..8ba30b3 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -17,7 +17,7 @@ static struct intel_dsm_priv {
acpi_handle dhandle;
} intel_dsm_priv;
-static const u8 intel_dsm_guid[] = {
+static u8 intel_dsm_guid[] = {
0xd3, 0x73, 0xd8, 0x7e,
0xd0, 0xc2,
0x4f, 0x4e,
@@ -25,6 +25,9 @@ static const u8 intel_dsm_guid[] = {
0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
};
+static u8 intel_bxt_dsm_guid[] =
+ "3E5B41C6-EB1D-4260-9D15-C71FBADAE414";
+
static char *intel_dsm_port_name(u8 id)
{
switch (id) {
@@ -143,6 +146,9 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
intel_dsm_priv.dhandle = dhandle;
+ if (IS_BROXTON(dev))
+ acpi_str_to_uuid(intel_bxt_dsm_guid, intel_dsm_guid);
+
if (acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
intel_dsm_platform_mux_info();
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt.
2016-04-07 14:52 ` [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt Animesh Manna
@ 2016-04-08 9:00 ` Jani Nikula
2016-04-08 12:55 ` David Weinehall
0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2016-04-08 9:00 UTC (permalink / raw)
To: Animesh Manna, intel-gfx; +Cc: Bharath K Veera, Ananth Krishna R
On Thu, 07 Apr 2016, Animesh Manna <animesh.manna@intel.com> wrote:
> Guid is changed for bxt platform, so corrected the guid for bxt.
>
> Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
> Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/intel_acpi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> index 8f3d672..8ba30b3 100644
> --- a/drivers/gpu/drm/i915/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/intel_acpi.c
> @@ -17,7 +17,7 @@ static struct intel_dsm_priv {
> acpi_handle dhandle;
> } intel_dsm_priv;
>
> -static const u8 intel_dsm_guid[] = {
> +static u8 intel_dsm_guid[] = {
> 0xd3, 0x73, 0xd8, 0x7e,
> 0xd0, 0xc2,
> 0x4f, 0x4e,
> @@ -25,6 +25,9 @@ static const u8 intel_dsm_guid[] = {
> 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
> };
>
> +static u8 intel_bxt_dsm_guid[] =
> + "3E5B41C6-EB1D-4260-9D15-C71FBADAE414";
> +
So I want both of these to be either in string form or in binary uuid
form, not a mixture. Having them in binary makes the code simpler, so
I'd go for that.
BR,
Jani.
> static char *intel_dsm_port_name(u8 id)
> {
> switch (id) {
> @@ -143,6 +146,9 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
>
> intel_dsm_priv.dhandle = dhandle;
>
> + if (IS_BROXTON(dev))
> + acpi_str_to_uuid(intel_bxt_dsm_guid, intel_dsm_guid);
> +
> if (acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
> 1 << INTEL_DSM_FN_PLATFORM_MUX_INFO))
> intel_dsm_platform_mux_info();
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt.
2016-04-08 9:00 ` Jani Nikula
@ 2016-04-08 12:55 ` David Weinehall
2016-04-13 12:45 ` Animesh Manna
0 siblings, 1 reply; 14+ messages in thread
From: David Weinehall @ 2016-04-08 12:55 UTC (permalink / raw)
To: Jani Nikula; +Cc: Bharath K Veera, Ananth Krishna R, intel-gfx
On Fri, Apr 08, 2016 at 12:00:22PM +0300, Jani Nikula wrote:
> On Thu, 07 Apr 2016, Animesh Manna <animesh.manna@intel.com> wrote:
> > Guid is changed for bxt platform, so corrected the guid for bxt.
> >
> > Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
> > Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_acpi.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
> > index 8f3d672..8ba30b3 100644
> > --- a/drivers/gpu/drm/i915/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/intel_acpi.c
> > @@ -17,7 +17,7 @@ static struct intel_dsm_priv {
> > acpi_handle dhandle;
> > } intel_dsm_priv;
> >
> > -static const u8 intel_dsm_guid[] = {
> > +static u8 intel_dsm_guid[] = {
> > 0xd3, 0x73, 0xd8, 0x7e,
> > 0xd0, 0xc2,
> > 0x4f, 0x4e,
> > @@ -25,6 +25,9 @@ static const u8 intel_dsm_guid[] = {
> > 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
> > };
> >
> > +static u8 intel_bxt_dsm_guid[] =
> > + "3E5B41C6-EB1D-4260-9D15-C71FBADAE414";
> > +
>
> So I want both of these to be either in string form or in binary uuid
> form, not a mixture. Having them in binary makes the code simpler, so
> I'd go for that.
I was just gonna suggest the same. Also, isn't this a fix we want
merged independently from the HPD-related changes? If so this
should submitted separately (or at least be modified so it can be
the first patch in the series).
Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt.
2016-04-08 12:55 ` David Weinehall
@ 2016-04-13 12:45 ` Animesh Manna
0 siblings, 0 replies; 14+ messages in thread
From: Animesh Manna @ 2016-04-13 12:45 UTC (permalink / raw)
To: Jani Nikula, intel-gfx, Bharath K Veera, Ananth Krishna R
On 4/8/2016 6:25 PM, David Weinehall wrote:
> On Fri, Apr 08, 2016 at 12:00:22PM +0300, Jani Nikula wrote:
>> On Thu, 07 Apr 2016, Animesh Manna <animesh.manna@intel.com> wrote:
>>> Guid is changed for bxt platform, so corrected the guid for bxt.
>>>
>>> Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
>>> Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
>>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_acpi.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
>>> index 8f3d672..8ba30b3 100644
>>> --- a/drivers/gpu/drm/i915/intel_acpi.c
>>> +++ b/drivers/gpu/drm/i915/intel_acpi.c
>>> @@ -17,7 +17,7 @@ static struct intel_dsm_priv {
>>> acpi_handle dhandle;
>>> } intel_dsm_priv;
>>>
>>> -static const u8 intel_dsm_guid[] = {
>>> +static u8 intel_dsm_guid[] = {
>>> 0xd3, 0x73, 0xd8, 0x7e,
>>> 0xd0, 0xc2,
>>> 0x4f, 0x4e,
>>> @@ -25,6 +25,9 @@ static const u8 intel_dsm_guid[] = {
>>> 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
>>> };
>>>
>>> +static u8 intel_bxt_dsm_guid[] =
>>> + "3E5B41C6-EB1D-4260-9D15-C71FBADAE414";
>>> +
>> So I want both of these to be either in string form or in binary uuid
>> form, not a mixture. Having them in binary makes the code simpler, so
>> I'd go for that.
> I was just gonna suggest the same. Also, isn't this a fix we want
> merged independently from the HPD-related changes? If so this
> should submitted separately (or at least be modified so it can be
> the first patch in the series).
Thanks Jani and David for review, will update as per your suggestion.
Will post as first patch in the next patch series.
>
>
> Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/5] drm/i915/bxt: Block D3 during suspend.
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
` (2 preceding siblings ...)
2016-04-07 14:52 ` [PATCH v2 3/5] drm/i915/bxt: Corrected the guid for bxt Animesh Manna
@ 2016-04-07 14:52 ` Animesh Manna
2016-04-08 11:46 ` David Weinehall
2016-04-07 14:52 ` [PATCH v2 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt Animesh Manna
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Animesh Manna @ 2016-04-07 14:52 UTC (permalink / raw)
To: intel-gfx
For BXT, display engine can not generate interrupt when in D3.
On the othen hand S0ix can be achieved without display in D3. So,
Display should not put into D3 for HPD to work and will not
have any power impact.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 020a31c..6b8c906 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1547,6 +1547,9 @@ static int intel_runtime_suspend(struct device *device)
assert_forcewakes_inactive(dev_priv);
+ if (dev_priv->vbt.hpd_wakeup_enabled)
+ pci_save_state(pdev);
+
DRM_DEBUG_KMS("Device suspended\n");
return 0;
}
@@ -1563,6 +1566,9 @@ static int intel_runtime_resume(struct device *device)
DRM_DEBUG_KMS("Resuming device\n");
+ if (dev_priv->vbt.hpd_wakeup_enabled)
+ pci_restore_state(pdev);
+
WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count));
disable_rpm_wakeref_asserts(dev_priv);
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 4/5] drm/i915/bxt: Block D3 during suspend.
2016-04-07 14:52 ` [PATCH v2 4/5] drm/i915/bxt: Block D3 during suspend Animesh Manna
@ 2016-04-08 11:46 ` David Weinehall
0 siblings, 0 replies; 14+ messages in thread
From: David Weinehall @ 2016-04-08 11:46 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
On Thu, Apr 07, 2016 at 08:22:11PM +0530, Animesh Manna wrote:
> For BXT, display engine can not generate interrupt when in D3.
> On the othen hand S0ix can be achieved without display in D3. So,
other
> Display should not put into D3 for HPD to work and will not
> have any power impact.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 020a31c..6b8c906 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1547,6 +1547,9 @@ static int intel_runtime_suspend(struct device *device)
>
> assert_forcewakes_inactive(dev_priv);
>
> + if (dev_priv->vbt.hpd_wakeup_enabled)
> + pci_save_state(pdev);
> +
> DRM_DEBUG_KMS("Device suspended\n");
> return 0;
> }
> @@ -1563,6 +1566,9 @@ static int intel_runtime_resume(struct device *device)
>
> DRM_DEBUG_KMS("Resuming device\n");
>
> + if (dev_priv->vbt.hpd_wakeup_enabled)
> + pci_restore_state(pdev);
If the state hasn't been saved pci_restore_state() will return
without doing anything, so the conditional shouldn't be necessary.
> +
> WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count));
> disable_rpm_wakeref_asserts(dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
` (3 preceding siblings ...)
2016-04-07 14:52 ` [PATCH v2 4/5] drm/i915/bxt: Block D3 during suspend Animesh Manna
@ 2016-04-07 14:52 ` Animesh Manna
2016-04-07 16:59 ` ✗ Fi.CI.BAT: failure for HPD support during suspend. (rev2) Patchwork
2016-04-07 21:05 ` [PATCH v2 0/6] HPD support during suspend Imre Deak
6 siblings, 0 replies; 14+ messages in thread
From: Animesh Manna @ 2016-04-07 14:52 UTC (permalink / raw)
To: intel-gfx
While suspending the device hpd related interrupts are enabled
to get the interrupt when device is in suspend state.
Though display is in DC9 but system can be in S0 or S0i3 state.
Hot plug during S0 state will generate de_port_interrupt but if
system is in S0i3 state then display driver will get hotplug
interrupt as pcu_hpd_interrupt which will come via pmc. So
added the interrupt handling for pcu hpd interrupt.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 56 ++++++++++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/i915_reg.h | 12 +++++++++
2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c85eb8d..35d7423 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -110,9 +110,9 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
/* BXT hpd list */
static const u32 hpd_bxt[HPD_NUM_PINS] = {
- [HPD_PORT_A] = BXT_DE_PORT_HP_DDIA,
- [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
- [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
+ [HPD_PORT_A] = (BXT_DE_PORT_HP_DDIA | BXT_PCU_DC9_HP_DDIA),
+ [HPD_PORT_B] = (BXT_DE_PORT_HP_DDIB | BXT_PCU_DC9_HP_DDIB),
+ [HPD_PORT_C] = (BXT_DE_PORT_HP_DDIC | BXT_PCU_DC9_HP_DDIC)
};
/* IIR can theoretically queue up two events. Be paranoid. */
@@ -2337,6 +2337,22 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
DRM_ERROR("The master control interrupt lied (DE PORT)!\n");
}
+ if (master_ctl & GEN8_PCU_IRQ) {
+ iir = I915_READ(GEN8_PCU_IIR);
+ if (iir) {
+ u32 tmp_mask;
+ I915_WRITE(GEN8_PCU_IIR, iir);
+ ret = IRQ_HANDLED;
+ if (IS_BROXTON(dev)) {
+ tmp_mask = iir & BXT_PCU_DC9_HOTPLUG_MASK;
+ if (tmp_mask)
+ bxt_hpd_irq_handler(dev, tmp_mask, hpd_bxt);
+ } else
+ DRM_ERROR("Unexpected PCU interrupt\n");
+ } else
+ DRM_ERROR("The master control interrupt lied (PCU)!\n");
+ }
+
for_each_pipe(dev_priv, pipe) {
u32 flip_done, fault_errors;
@@ -4681,6 +4697,19 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
dev_priv->pm.irqs_enabled = false;
}
+static void bxt_enable_pcu_interrupt(struct drm_i915_private *dev_priv)
+{
+ u32 de_pcu_hpd_enable_mask, de_pcu_imr, de_pcu_ier;
+
+ de_pcu_hpd_enable_mask = GEN9_DE_PCU_PORTA_HOTPLUG |
+ GEN9_DE_PCU_PORTB_HOTPLUG |
+ GEN9_DE_PCU_PORTC_HOTPLUG;
+
+ de_pcu_imr = (I915_READ(GEN8_PCU_IMR) & 0x0);
+ de_pcu_ier = (I915_READ(GEN8_PCU_IER) | de_pcu_hpd_enable_mask);
+ GEN5_IRQ_INIT(GEN8_PCU_, de_pcu_imr, de_pcu_ier);
+}
+
/**
* intel_runtime_pm_disable_interrupts - runtime interrupt disabling
* @dev_priv: i915 device instance
@@ -4690,8 +4719,29 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
*/
void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned long flags = 0;
+
dev_priv->dev->driver->irq_uninstall(dev_priv->dev);
dev_priv->pm.irqs_enabled = false;
+
+ if (IS_BROXTON(dev) && dev_priv->vbt.hpd_wakeup_enabled) {
+
+ /* Enable HPD related interrupts during DC9 for HPD wakeup */
+
+ I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+ POSTING_READ(GEN8_MASTER_IRQ);
+
+ spin_lock_irqsave(&dev_priv->irq_lock, flags);
+ if (dev_priv->display.hpd_irq_setup)
+ dev_priv->display.hpd_irq_setup(dev_priv->dev);
+ spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+ bxt_enable_pcu_interrupt(dev_priv);
+
+ dev_priv->pm.irqs_enabled = true;
+ }
+
synchronize_irq(dev_priv->dev->irq);
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 12f5103..3af9147 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5954,6 +5954,18 @@ enum skl_disp_power_wells {
#define GEN8_PCU_IIR _MMIO(0x444e8)
#define GEN8_PCU_IER _MMIO(0x444ec)
+/* BXT PCU DC9 hotplug control */
+#define BXT_PCU_DC9_HP_DDIA (1<<31)
+#define BXT_PCU_DC9_HP_DDIB (1<<30)
+#define BXT_PCU_DC9_HP_DDIC (1<<29)
+#define BXT_PCU_DC9_HOTPLUG_MASK (BXT_PCU_DC9_HP_DDIA | \
+ BXT_PCU_DC9_HP_DDIB | \
+ BXT_PCU_DC9_HP_DDIC)
+
+#define GEN9_DE_PCU_PORTA_HOTPLUG (1 << 31)
+#define GEN9_DE_PCU_PORTB_HOTPLUG (1 << 30)
+#define GEN9_DE_PCU_PORTC_HOTPLUG (1 << 29)
+
#define ILK_DISPLAY_CHICKEN2 _MMIO(0x42004)
/* Required on all Ironlake and Sandybridge according to the B-Spec. */
#define ILK_ELPIN_409_SELECT (1 << 25)
--
2.0.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* ✗ Fi.CI.BAT: failure for HPD support during suspend. (rev2)
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
` (4 preceding siblings ...)
2016-04-07 14:52 ` [PATCH v2 5/5] drm/i915: Enable HPD interrupts with master ctl interrupt Animesh Manna
@ 2016-04-07 16:59 ` Patchwork
2016-04-07 21:05 ` [PATCH v2 0/6] HPD support during suspend Imre Deak
6 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-04-07 16:59 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
== Series Details ==
Series: HPD support during suspend. (rev2)
URL : https://patchwork.freedesktop.org/series/5322/
State : failure
== Summary ==
Series 5322v2 HPD support during suspend.
http://patchwork.freedesktop.org/api/1.0/series/5322/revisions/2/mbox/
Test drv_getparams_basic:
Subgroup basic-subslice-total:
dmesg-warn -> PASS (bsw-nuc-2)
Test drv_module_reload_basic:
incomplete -> PASS (bsw-nuc-2)
Test gem_basic:
Subgroup bad-close:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup create-close:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup create-fd-close:
dmesg-warn -> PASS (bsw-nuc-2)
Test gem_ctx_param_basic:
Subgroup basic:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup invalid-ctx-set:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup invalid-param-get:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup invalid-param-set:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup root-set:
dmesg-warn -> PASS (bsw-nuc-2)
Test gem_ctx_switch:
Subgroup basic-default:
skip -> PASS (bsw-nuc-2)
Test gem_exec_basic:
Subgroup basic-blt:
skip -> PASS (bsw-nuc-2)
Subgroup gtt-bsd:
skip -> PASS (bsw-nuc-2)
Subgroup readonly-bsd:
skip -> PASS (bsw-nuc-2)
Subgroup readonly-render:
skip -> PASS (bsw-nuc-2)
Subgroup readonly-vebox:
skip -> PASS (bsw-nuc-2)
Test gem_exec_nop:
Subgroup basic:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_store:
Subgroup basic-all:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-default:
skip -> PASS (bsw-nuc-2)
Subgroup basic-vebox:
skip -> PASS (bsw-nuc-2)
Test gem_exec_whisper:
Subgroup basic:
skip -> PASS (bsw-nuc-2)
Test gem_mmap:
Subgroup basic-small-bo:
dmesg-warn -> PASS (bsw-nuc-2)
Test gem_mmap_gtt:
Subgroup basic-write:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup basic-write-gtt:
dmesg-warn -> PASS (bsw-nuc-2)
Test gem_ringfill:
Subgroup basic-default-hang:
skip -> PASS (bsw-nuc-2)
Subgroup basic-default-interruptible:
pass -> SKIP (bsw-nuc-2)
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-FAIL (bsw-nuc-2)
Test gem_sync:
Subgroup basic-blt:
skip -> PASS (bsw-nuc-2)
Subgroup basic-vebox:
skip -> PASS (bsw-nuc-2)
Test gem_tiled_pread_basic:
dmesg-warn -> PASS (bsw-nuc-2)
Test kms_addfb_basic:
Subgroup addfb25-modifier-no-flag:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup addfb25-y-tiled:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup bad-pitch-256:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-pitch-65536:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup basic-x-tiled:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup framebuffer-vs-set-tiling:
dmesg-warn -> PASS (bsw-nuc-2)
Subgroup too-wide:
dmesg-warn -> PASS (bsw-nuc-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-FAIL (bsw-nuc-2)
Test kms_force_connector_basic:
Subgroup force-edid:
skip -> PASS (ivb-t430s)
Subgroup prune-stale-modes:
pass -> SKIP (ivb-t430s)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (hsw-gt2)
Test prime_self_import:
Subgroup basic-with_fd_dup:
dmesg-warn -> PASS (bsw-nuc-2)
bdw-nuci7 total:196 pass:184 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:196 pass:175 dwarn:0 dfail:0 fail:0 skip:21
bsw-nuc-2 total:184 pass:129 dwarn:9 dfail:6 fail:0 skip:39
byt-nuc total:196 pass:161 dwarn:0 dfail:0 fail:0 skip:35
hsw-brixbox total:196 pass:174 dwarn:0 dfail:0 fail:0 skip:22
hsw-gt2 total:168 pass:151 dwarn:0 dfail:0 fail:0 skip:16
ilk-hp8440p total:196 pass:132 dwarn:0 dfail:0 fail:0 skip:64
ivb-t430s total:196 pass:170 dwarn:0 dfail:0 fail:0 skip:26
skl-i7k-2 total:196 pass:173 dwarn:0 dfail:0 fail:0 skip:23
skl-nuci5 total:196 pass:185 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:196 pass:162 dwarn:0 dfail:0 fail:0 skip:34
Results at /archive/results/CI_IGT_test/Patchwork_1833/
851708c7e97537ed618fadbe5d342eaf8fa5146d drm-intel-nightly: 2016y-04m-07d-13h-56m-00s UTC integration manifest
f601fae1fe9f6bbb556fbb12ab962393fccc3e7d drm/i915: Enable HPD interrupts with master ctl interrupt
b1a96def036d0aba1ddbe3ebfb8d61b571a03789 drm/i915/bxt: Block D3 during suspend.
621b157b1b7621ea63bedb90d6d9f8417f4a7813 drm/i915/bxt: Corrected the guid for bxt.
0dba4f32f9c3a954855f912a2f542e7fc96dbc7e drm/i915/bxt: Added _DSM call to set HPD_CTL.
6734386e1d506fbe92bf8fa9460d811db30b9602 drm/i915/bxt: VBT changes for hpd as wakeup feature
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] HPD support during suspend.
2016-04-07 14:52 [PATCH v2 0/6] HPD support during suspend Animesh Manna
` (5 preceding siblings ...)
2016-04-07 16:59 ` ✗ Fi.CI.BAT: failure for HPD support during suspend. (rev2) Patchwork
@ 2016-04-07 21:05 ` Imre Deak
2016-04-13 13:47 ` Animesh Manna
6 siblings, 1 reply; 14+ messages in thread
From: Imre Deak @ 2016-04-07 21:05 UTC (permalink / raw)
To: Animesh Manna, intel-gfx
On Thu, 2016-04-07 at 20:22 +0530, Animesh Manna wrote:
> Along with below patches sharing some background details/design.
> - On BXT, Display cannot generate an interrupt when in D3.
> - Without display in D3, S0ix can be achieved, Power impact
> will be zero if d3 is blocked. PMCSR for Graphics/Display
> is irrelevant, as the power management for them is taken
> care by the PUNIT using RC6/DC9 hints and *not* through
> PMCSR write trigger.
>
> So solution is based on below principles:
> - Display should not be put into D3 for HPD to work.
> - With D0+DC9 we can achieve S0ix and at the same time
> helps to get the interrupt.
> - Using pci_save_state() during suspend to take control
> from OSPM and blocking D3, while resuming, giving back
> to OSPM by pci_restore_state().
> - _DSM method is used to program pmc hpd control register
> to enable hpd during suspend.
>
> Please have a look and send your comments/suggestions.
Good to see this getting fixed. Some notes:
Although I also suspect that D3 doesn't provide any power saving over
RC6/DC9, D3 is still a standard PCI device state shown as supported by
the GFX PCI device capability descriptor. So before going ahead with
this approach we'd need a better explanation on why preventing D3 is
ok, instead of just stating it:
- What's the exact effect of D3? We know that it at least masks the
device interrupts, but is it guaranteed (by some specification for
example) that there aren't and (and in the future won't be) any other
effects, especially power related?
- How does the GFX device compare to other devices in the system in
this regard, are there devices with a similar limitation? If so, is the
solution on those the same (preventing D3)?
- Why isn't the standard PCI wake-up mechanism supported (that is PME)?
Just some design-time overlook, or some practical problem with its
implementation? Can we expect that PME will be supported in the future?
- How does BXT compare to the other platforms regarding this problem?
We know that all of them with runtime power management support have the
same problem. On some of them (at least HSW, BDW) we could use the same
workaround (preventing D3) as a solution, but what's the exact list of
these platforms? Does preventing D3 on any of these have a negative
impact?
I noticed that the BXT HPD pins can be configured to work in GPIO mode
(GPIO 199, 200). Could we instead of the above approach use these as a
wake-up source while putting the device into D3?
About the actual patch: now we'll do the whole interrupt processing
while the device is still runtime suspended. This may work, but is
unusual which I'd rather avoid. Instead I'd just handle the PCU IRQ
itself and do the rest of processing after resuming the device (from a
scheduled work).
--Imre
> Animesh Manna (6):
> drm/i915/bxt: VBT changes for hpd as wakeup feature
> drm/i915/bxt: Added _DSM call to set HPD_CTL.
> drm/i915/bxt: Corrected the guid for bxt.
> drm/i915/bxt: Block D3 during suspend.
> drm/i915: Enable HPD interrupts with master ctl interrupt
> drm/i915/bxt: Enable HPD during suspend.
>
> drivers/gpu/drm/i915/i915_drv.c | 6 ++++
> drivers/gpu/drm/i915/i915_drv.h | 8 +++++
> drivers/gpu/drm/i915/i915_irq.c | 56
> +++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++
> drivers/gpu/drm/i915/intel_acpi.c | 53
> ++++++++++++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_bios.c | 7 +++++
> drivers/gpu/drm/i915/intel_vbt_defs.h | 3 +-
> 7 files changed, 134 insertions(+), 12 deletions(-)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] HPD support during suspend.
2016-04-07 21:05 ` [PATCH v2 0/6] HPD support during suspend Imre Deak
@ 2016-04-13 13:47 ` Animesh Manna
2016-04-13 14:00 ` Animesh Manna
0 siblings, 1 reply; 14+ messages in thread
From: Animesh Manna @ 2016-04-13 13:47 UTC (permalink / raw)
To: imre.deak, intel-gfx
On 4/8/2016 2:35 AM, Imre Deak wrote:
> On Thu, 2016-04-07 at 20:22 +0530, Animesh Manna wrote:
>> Along with below patches sharing some background details/design.
>> - On BXT, Display cannot generate an interrupt when in D3.
>> - Without display in D3, S0ix can be achieved, Power impact
>> will be zero if d3 is blocked. PMCSR for Graphics/Display
>> is irrelevant, as the power management for them is taken
>> care by the PUNIT using RC6/DC9 hints and *not* through
>> PMCSR write trigger.
>>
>> So solution is based on below principles:
>> - Display should not be put into D3 for HPD to work.
>> - With D0+DC9 we can achieve S0ix and at the same time
>> helps to get the interrupt.
>> - Using pci_save_state() during suspend to take control
>> from OSPM and blocking D3, while resuming, giving back
>> to OSPM by pci_restore_state().
>> - _DSM method is used to program pmc hpd control register
>> to enable hpd during suspend.
>>
>> Please have a look and send your comments/suggestions.
> Good to see this getting fixed. Some notes:
>
> Although I also suspect that D3 doesn't provide any power saving over
> RC6/DC9, D3 is still a standard PCI device state shown as supported by
> the GFX PCI device capability descriptor. So before going ahead with
> this approach we'd need a better explanation on why preventing D3 is
> ok, instead of just stating it:
> - What's the exact effect of D3? We know that it at least masks the
> device interrupts, but is it guaranteed (by some specification for
> example) that there aren't and (and in the future won't be) any other
> effects, especially power related?
Thanks Imre for review, First of all Display/Gfx is a single
function pci device. As per PMC HAS display should not put into
D3 for HPD to work. Once Punit gets a PM response with DC9 Ready,
and conditions outside display indicate the system is ready for
power down, Punit will save device 2 PCI config and device 2 MMIO
registers that are marked for save in the RDL/XML, then remove
power from display completely (Vnn). So I feel if Vnn is down
there is no chance of power leak.
> - How does the GFX device compare to other devices in the system in
> this regard, are there devices with a similar limitation? If so, is the
> solution on those the same (preventing D3)?
I think these will be based on h/w architecture. As you mentioned
below #PME is one way which I feel followed by wifi/ethernet devices.
But this can not be general solution for other devices which might
block S0ix, but without display/gfx in D3 as we can achieve S0ix
so have taken this approach.
> - Why isn't the standard PCI wake-up mechanism supported (that is PME)?
> Just some design-time overlook, or some practical problem with its
> implementation? Can we expect that PME will be supported in the future?
Not have much idea why it is not supported and is there any plan to support
in future .. :(
> - How does BXT compare to the other platforms regarding this problem?
> We know that all of them with runtime power management support have the
> same problem. On some of them (at least HSW, BDW) we could use the same
> workaround (preventing D3) as a solution, but what's the exact list of
> these platforms? Does preventing D3 on any of these have a negative
> impact?
Only BXT has a special HPD control register in PMC which helps routing
the hpd interrupt during sleep. So other platform we can not support
this feature.
>
> I noticed that the BXT HPD pins can be configured to work in GPIO mode
> (GPIO 199, 200). Could we instead of the above approach use these as a
> wake-up source while putting the device into D3?
Need to explore with platform architect, but we already had a discussion and this
approach is recommended by platform architecture team.
>
> About the actual patch: now we'll do the whole interrupt processing
> while the device is still runtime suspended. This may work, but is
> unusual which I'd rather avoid. Instead I'd just handle the PCU IRQ
> itself and do the rest of processing after resuming the device (from a
> scheduled work).
Ok .. I can try out. This is doable.
Regards,
Animesh
>
> --Imre
>
>> Animesh Manna (6):
>> drm/i915/bxt: VBT changes for hpd as wakeup feature
>> drm/i915/bxt: Added _DSM call to set HPD_CTL.
>> drm/i915/bxt: Corrected the guid for bxt.
>> drm/i915/bxt: Block D3 during suspend.
>> drm/i915: Enable HPD interrupts with master ctl interrupt
>> drm/i915/bxt: Enable HPD during suspend.
>>
>> drivers/gpu/drm/i915/i915_drv.c | 6 ++++
>> drivers/gpu/drm/i915/i915_drv.h | 8 +++++
>> drivers/gpu/drm/i915/i915_irq.c | 56
>> +++++++++++++++++++++++++++++++++--
>> drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++
>> drivers/gpu/drm/i915/intel_acpi.c | 53
>> ++++++++++++++++++++++++++++-----
>> drivers/gpu/drm/i915/intel_bios.c | 7 +++++
>> drivers/gpu/drm/i915/intel_vbt_defs.h | 3 +-
>> 7 files changed, 134 insertions(+), 12 deletions(-)
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/6] HPD support during suspend.
2016-04-13 13:47 ` Animesh Manna
@ 2016-04-13 14:00 ` Animesh Manna
0 siblings, 0 replies; 14+ messages in thread
From: Animesh Manna @ 2016-04-13 14:00 UTC (permalink / raw)
To: imre.deak, intel-gfx
On 4/13/2016 7:17 PM, Animesh Manna wrote:
>
>
> On 4/8/2016 2:35 AM, Imre Deak wrote:
>> On Thu, 2016-04-07 at 20:22 +0530, Animesh Manna wrote:
>>> Along with below patches sharing some background details/design.
>>> - On BXT, Display cannot generate an interrupt when in D3.
>>> - Without display in D3, S0ix can be achieved, Power impact
>>> will be zero if d3 is blocked. PMCSR for Graphics/Display
>>> is irrelevant, as the power management for them is taken
>>> care by the PUNIT using RC6/DC9 hints and *not* through
>>> PMCSR write trigger.
>>>
>>> So solution is based on below principles:
>>> - Display should not be put into D3 for HPD to work.
>>> - With D0+DC9 we can achieve S0ix and at the same time
>>> helps to get the interrupt.
>>> - Using pci_save_state() during suspend to take control
>>> from OSPM and blocking D3, while resuming, giving back
>>> to OSPM by pci_restore_state().
>>> - _DSM method is used to program pmc hpd control register
>>> to enable hpd during suspend.
>>>
>>> Please have a look and send your comments/suggestions.
>> Good to see this getting fixed. Some notes:
>>
>> Although I also suspect that D3 doesn't provide any power saving over
>> RC6/DC9, D3 is still a standard PCI device state shown as supported by
>> the GFX PCI device capability descriptor. So before going ahead with
>> this approach we'd need a better explanation on why preventing D3 is
>> ok, instead of just stating it:
>> - What's the exact effect of D3? We know that it at least masks the
>> device interrupts, but is it guaranteed (by some specification for
>> example) that there aren't and (and in the future won't be) any other
>> effects, especially power related?
>
> Thanks Imre for review, First of all Display/Gfx is a single
> function pci device. As per PMC HAS display should not put into
> D3 for HPD to work. Once Punit gets a PM response with DC9 Ready,
> and conditions outside display indicate the system is ready for
> power down, Punit will save device 2 PCI config and device 2 MMIO
> registers that are marked for save in the RDL/XML, then remove
> power from display completely (Vnn). So I feel if Vnn is down
> there is no chance of power leak.
>
>> - How does the GFX device compare to other devices in the system in
>> this regard, are there devices with a similar limitation? If so, is the
>> solution on those the same (preventing D3)?
>
> I think these will be based on h/w architecture. As you mentioned
> below #PME is one way which I feel followed by wifi/ethernet devices.
> But this can not be general solution for other devices which might
> block S0ix, but without display/gfx in D3 as we can achieve S0ix
> so have taken this approach.
In my previous response using "this" I wanted to mean as "Blocking D3".
>
>
>> - Why isn't the standard PCI wake-up mechanism supported (that is PME)?
>> Just some design-time overlook, or some practical problem with its
>> implementation? Can we expect that PME will be supported in the future?
>
> Not have much idea why it is not supported and is there any plan to
> support
> in future .. :(
>
>> - How does BXT compare to the other platforms regarding this problem?
>> We know that all of them with runtime power management support have the
>> same problem. On some of them (at least HSW, BDW) we could use the same
>> workaround (preventing D3) as a solution, but what's the exact list of
>> these platforms? Does preventing D3 on any of these have a negative
>> impact?
>
> Only BXT has a special HPD control register in PMC which helps routing
> the hpd interrupt during sleep. So other platform we can not support
> this feature.
>
>>
>> I noticed that the BXT HPD pins can be configured to work in GPIO mode
>> (GPIO 199, 200). Could we instead of the above approach use these as a
>> wake-up source while putting the device into D3?
>
> Need to explore with platform architect, but we already had a
> discussion and this
> approach is recommended by platform architecture team.
>
>>
>> About the actual patch: now we'll do the whole interrupt processing
>> while the device is still runtime suspended. This may work, but is
>> unusual which I'd rather avoid. Instead I'd just handle the PCU IRQ
>> itself and do the rest of processing after resuming the device (from a
>> scheduled work).
>
> Ok .. I can try out. This is doable.
>
> Regards,
> Animesh
>
>>
>> --Imre
>>
>>> Animesh Manna (6):
>>> drm/i915/bxt: VBT changes for hpd as wakeup feature
>>> drm/i915/bxt: Added _DSM call to set HPD_CTL.
>>> drm/i915/bxt: Corrected the guid for bxt.
>>> drm/i915/bxt: Block D3 during suspend.
>>> drm/i915: Enable HPD interrupts with master ctl interrupt
>>> drm/i915/bxt: Enable HPD during suspend.
>>>
>>> drivers/gpu/drm/i915/i915_drv.c | 6 ++++
>>> drivers/gpu/drm/i915/i915_drv.h | 8 +++++
>>> drivers/gpu/drm/i915/i915_irq.c | 56
>>> +++++++++++++++++++++++++++++++++--
>>> drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++
>>> drivers/gpu/drm/i915/intel_acpi.c | 53
>>> ++++++++++++++++++++++++++++-----
>>> drivers/gpu/drm/i915/intel_bios.c | 7 +++++
>>> drivers/gpu/drm/i915/intel_vbt_defs.h | 3 +-
>>> 7 files changed, 134 insertions(+), 12 deletions(-)
>>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread