public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications
@ 2013-08-23 10:17 Jani Nikula
  0 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2013-08-23 10:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

Hi all, here are some patches for letting the BIOS do some further power
savings on HSW, now based on intel-drm-nightly with Paulo's PC8+ work.

Patches 1-4 are the glue code to wrap the BIOS SWSCI calls into a few
sensible function calls.

Patches 5-6 are for reference only, and I presume they will have to be
tweaked based on testing. Where exactly to do the calls? What parameters
are to be used in the end? Possibly calls need to be added on the
suspend/resume paths too.

I don't have the setup to do the testing and measurements, but I hope
these will enable others.


BR,
Jani.



Jani Nikula (6):
  drm/i915: expose intel_ddi_get_encoder_port()
  drm/i915: add plumbing for SWSCI
  drm/i915: add opregion function to notify bios of encoder
    enable/disable
  drm/i915: add opregion function to notify bios of adapter power state
  DRAFT: drm/i915: do display power state notification on crtc
    enable/disable
  DRAFT: drm/i915: do adapter power state notification on PC8+
    enable/disable

 drivers/gpu/drm/i915/i915_drv.h       |   12 ++
 drivers/gpu/drm/i915/intel_ddi.c      |    2 +-
 drivers/gpu/drm/i915/intel_display.c  |   12 +-
 drivers/gpu/drm/i915/intel_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_opregion.c |  200 ++++++++++++++++++++++++++++++++-
 5 files changed, 223 insertions(+), 4 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications
@ 2013-08-30 16:40 Jani Nikula
  2013-08-30 16:40 ` [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port() Jani Nikula
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

Hi all, a new version of [1] addressing Paulo's review comments. I hope
I didn't rush the changes too much; the new info about requested
vs. supported callbacks is confusing to say the least...

I'm sure Daniel will appreciate any Tested-by's!


BR,
Jani.


[1] http://mid.gmane.org/cover.1377246881.git.jani.nikula@intel.com

Jani Nikula (6):
  drm/i915: expose intel_ddi_get_encoder_port()
  drm/i915: add plumbing for SWSCI
  drm/i915: add opregion function to notify bios of encoder
    enable/disable
  drm/i915: add opregion function to notify bios of adapter power state
  drm/i915: do display power state notification on crtc enable/disable
  DRAFT: drm/i915: do adapter power state notification on PC8+
    enable/disable

 drivers/gpu/drm/i915/i915_drv.h       |   17 ++
 drivers/gpu/drm/i915/intel_ddi.c      |    2 +-
 drivers/gpu/drm/i915/intel_display.c  |   10 +-
 drivers/gpu/drm/i915/intel_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_opregion.c |  276 ++++++++++++++++++++++++++++++++-
 5 files changed, 300 insertions(+), 6 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port()
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-09-02 14:08   ` Daniel Vetter
  2013-08-30 16:40 ` [PATCH 2/6] drm/i915: add plumbing for SWSCI Jani Nikula
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

In preparation for followup work.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |    2 +-
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 63aca49..060ea50 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -58,7 +58,7 @@ static const u32 hsw_ddi_translations_fdi[] = {
 	0x00FFFFFF, 0x00040006		/* HDMI parameters */
 };
 
-static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
+enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
 	int type = intel_encoder->type;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a38f5b2..5efb844 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -712,6 +712,7 @@ extern void intel_write_eld(struct drm_encoder *encoder,
 extern void intel_prepare_ddi(struct drm_device *dev);
 extern void hsw_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
+extern enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
 
 /* For use by IVB LP watermark workaround in intel_sprite.c */
 extern void intel_update_watermarks(struct drm_device *dev);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] drm/i915: add plumbing for SWSCI
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
  2013-08-30 16:40 ` [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port() Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-08-30 19:31   ` Paulo Zanoni
  2013-08-30 16:40 ` [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable Jani Nikula
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

SWSCI is a driver to bios call interface.

This checks for SWSCI availability and bios requested callbacks, and
filters out any calls that shouldn't happen. This way the callers don't
need to do the checks all over the place.

v2: silence some checkpatch nagging

v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)

v4: remove an extra #define (Jesse)

v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too

v6: per Paulo's review and more:
 - fix sub-function mask
 - add exit parameter
 - add define for set panel details call
 - return more errors from swsci
 - clean up the supported/requested callbacks bit masks mess
 - use DSLP for timeout
 - fix build for CONFIG_ACPI=n

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |    2 +
 drivers/gpu/drm/i915/intel_opregion.c |  198 ++++++++++++++++++++++++++++++++-
 2 files changed, 197 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0602c4b..67673e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -225,6 +225,8 @@ struct intel_opregion {
 	struct opregion_header __iomem *header;
 	struct opregion_acpi __iomem *acpi;
 	struct opregion_swsci __iomem *swsci;
+	u32 swsci_gbda_sub_functions;
+	u32 swsci_sbcb_sub_functions;
 	struct opregion_asle __iomem *asle;
 	void __iomem *vbt;
 	u32 __iomem *lid_state;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..c6a8d61 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -36,8 +36,11 @@
 #include "i915_drv.h"
 #include "intel_drv.h"
 
-#define PCI_ASLE 0xe4
-#define PCI_ASLS 0xfc
+#define PCI_ASLE		0xe4
+#define PCI_ASLS		0xfc
+#define PCI_SWSCI		0xe8
+#define PCI_SWSCI_SCISEL	(1 << 15)
+#define PCI_SWSCI_GSSCIE	(1 << 0)
 
 #define OPREGION_HEADER_OFFSET 0
 #define OPREGION_ACPI_OFFSET   0x100
@@ -151,6 +154,51 @@ struct opregion_asle {
 
 #define ASLE_CBLV_VALID         (1<<31)
 
+/* Software System Control Interrupt (SWSCI) */
+#define SWSCI_SCIC_INDICATOR		(1 << 0)
+#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
+#define SWSCI_SCIC_MAIN_FUNCTION_MASK	(0xf << 1)
+#define SWSCI_SCIC_SUB_FUNCTION_SHIFT	8
+#define SWSCI_SCIC_SUB_FUNCTION_MASK	(0xff << 8)
+#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT	8
+#define SWSCI_SCIC_EXIT_PARAMETER_MASK	(0xff << 8)
+#define SWSCI_SCIC_EXIT_STATUS_SHIFT	5
+#define SWSCI_SCIC_EXIT_STATUS_MASK	(7 << 5)
+#define SWSCI_SCIC_EXIT_STATUS_SUCCESS	1
+
+#define SWSCI_FUNCTION_CODE(main, sub) \
+	((main) << SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
+	 (sub) << SWSCI_SCIC_SUB_FUNCTION_SHIFT)
+
+/* SWSCI: Get BIOS Data (GBDA) */
+#define SWSCI_GBDA			4
+#define SWSCI_GBDA_SUPPORTED_CALLS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
+#define SWSCI_GBDA_REQUESTED_CALLBACKS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
+#define SWSCI_GBDA_BOOT_DISPLAY_PREF	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
+#define SWSCI_GBDA_PANEL_DETAILS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
+#define SWSCI_GBDA_TV_STANDARD		SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
+#define SWSCI_GBDA_INTERNAL_GRAPHICS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
+#define SWSCI_GBDA_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
+
+/* SWSCI: System BIOS Callbacks (SBCB) */
+#define SWSCI_SBCB			6
+#define SWSCI_SBCB_SUPPORTED_CALLBACKS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
+#define SWSCI_SBCB_INIT_COMPLETION	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
+#define SWSCI_SBCB_PRE_HIRES_SET_MODE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
+#define SWSCI_SBCB_POST_HIRES_SET_MODE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
+#define SWSCI_SBCB_DISPLAY_SWITCH	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
+#define SWSCI_SBCB_SET_TV_FORMAT	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
+#define SWSCI_SBCB_ADAPTER_POWER_STATE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
+#define SWSCI_SBCB_DISPLAY_POWER_STATE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
+#define SWSCI_SBCB_SET_BOOT_DISPLAY	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
+#define SWSCI_SBCB_SET_PANEL_DETAILS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
+#define SWSCI_SBCB_SET_INTERNAL_GFX	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
+#define SWSCI_SBCB_POST_HIRES_TO_DOS_FS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 16)
+#define SWSCI_SBCB_SUSPEND_RESUME	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
+#define SWSCI_SBCB_SET_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
+#define SWSCI_SBCB_POST_VBE_PM		SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
+#define SWSCI_SBCB_ENABLE_DISABLE_AUDIO	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
+
 #define ACPI_OTHER_OUTPUT (0<<8)
 #define ACPI_VGA_OUTPUT (1<<8)
 #define ACPI_TV_OUTPUT (2<<8)
@@ -158,6 +206,91 @@ struct opregion_asle {
 #define ACPI_LVDS_OUTPUT (4<<8)
 
 #ifdef CONFIG_ACPI
+static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct opregion_swsci __iomem *swsci = dev_priv->opregion.swsci;
+	u32 main_function, sub_function, scic;
+	u16 pci_swsci;
+	u32 dslp;
+
+	if (!swsci)
+		return -ENODEV;
+
+	main_function = (function & SWSCI_SCIC_MAIN_FUNCTION_MASK) >>
+		SWSCI_SCIC_MAIN_FUNCTION_SHIFT;
+	sub_function = (function & SWSCI_SCIC_SUB_FUNCTION_MASK) >>
+		SWSCI_SCIC_SUB_FUNCTION_SHIFT;
+
+	/* Check if we can call the function. See swsci_setup for details. */
+	if (main_function == SWSCI_SBCB) {
+		if ((dev_priv->opregion.swsci_sbcb_sub_functions &
+		     (1 << sub_function)) == 0)
+			return -EINVAL;
+	} else if (main_function == SWSCI_GBDA) {
+		if ((dev_priv->opregion.swsci_gbda_sub_functions &
+		     (1 << sub_function)) == 0)
+			return -EINVAL;
+	}
+
+	/* Driver sleep timeout in ms. */
+	dslp = ioread32(&swsci->dslp);
+	if (!dslp) {
+		dslp = 2;
+	} else if (dslp > 500) {
+		/* Hey bios, trust must be earned. */
+		WARN_ONCE(1, "excessive driver sleep timeout (DSPL) %u\n", dslp);
+		dslp = 500;
+	}
+
+	/* The spec tells us to do this, but we are the only user... */
+	scic = ioread32(&swsci->scic);
+	if (scic & SWSCI_SCIC_INDICATOR) {
+		DRM_DEBUG_DRIVER("SWSCI request already in progress\n");
+		return -EBUSY;
+	}
+
+	scic = function | SWSCI_SCIC_INDICATOR;
+
+	iowrite32(parm, &swsci->parm);
+	iowrite32(scic, &swsci->scic);
+
+	/* Ensure SCI event is selected and event trigger is cleared. */
+	pci_read_config_word(dev->pdev, PCI_SWSCI, &pci_swsci);
+	if (!(pci_swsci & PCI_SWSCI_SCISEL) || (pci_swsci & PCI_SWSCI_GSSCIE)) {
+		pci_swsci |= PCI_SWSCI_SCISEL;
+		pci_swsci &= ~PCI_SWSCI_GSSCIE;
+		pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
+	}
+
+	/* Use event trigger to tell bios to check the mail. */
+	pci_swsci |= PCI_SWSCI_GSSCIE;
+	pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
+
+	/* Poll for the result. */
+#define C (((scic = ioread32(&swsci->scic)) & SWSCI_SCIC_INDICATOR) == 0)
+	if (wait_for(C, dslp)) {
+		DRM_DEBUG_DRIVER("SWSCI request timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	scic = (scic & SWSCI_SCIC_EXIT_STATUS_MASK) >>
+		SWSCI_SCIC_EXIT_STATUS_SHIFT;
+
+	/* Note: scic == 0 is an error! */
+	if (scic != SWSCI_SCIC_EXIT_STATUS_SUCCESS) {
+		DRM_DEBUG_DRIVER("SWSCI request error %u\n", scic);
+		return -EIO;
+	}
+
+	if (parm_out)
+		*parm_out = ioread32(&swsci->parm);
+
+	return 0;
+
+#undef C
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -447,7 +580,65 @@ void intel_opregion_fini(struct drm_device *dev)
 	opregion->asle = NULL;
 	opregion->vbt = NULL;
 }
-#endif
+
+static void swsci_setup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_opregion *opregion = &dev_priv->opregion;
+	bool requested_callbacks = false;
+	u32 tmp;
+
+	/* Sub-function code 0 is okay, let's allow them. */
+	opregion->swsci_gbda_sub_functions = 1;
+	opregion->swsci_sbcb_sub_functions = 1;
+
+	/* We use GBDA to ask for supported GBDA calls. */
+	if (swsci(dev, SWSCI_GBDA_SUPPORTED_CALLS, 0, &tmp) == 0) {
+		/* make the bits match the sub-function codes */
+		tmp <<= 1;
+		opregion->swsci_gbda_sub_functions |= tmp;
+	}
+
+	/*
+	 * We also use GBDA to ask for _requested_ SBCB callbacks. The driver
+	 * must not call interfaces that are not specifically requested by the
+	 * bios.
+	 */
+	if (swsci(dev, SWSCI_GBDA_REQUESTED_CALLBACKS, 0, &tmp) == 0) {
+		/* here, the bits already match sub-function codes */
+		opregion->swsci_sbcb_sub_functions |= tmp;
+		requested_callbacks = true;
+	}
+
+	/*
+	 * But we use SBCB to ask for _supported_ SBCB calls. This does not mean
+	 * the callback is _requested_. But we still can't call interfaces that
+	 * are not requested.
+	 */
+	if (swsci(dev, SWSCI_SBCB_SUPPORTED_CALLBACKS, 0, &tmp) == 0) {
+		/* make the bits match the sub-function codes */
+		u32 low = tmp & 0x7ff;
+		u32 high = tmp & ~0xfff;
+		tmp = (high << 4) | (low << 1) | 1;
+
+		/* best guess what to do with supported wrt requested */
+		if (requested_callbacks) {
+			if (opregion->swsci_sbcb_sub_functions ^ tmp)
+				DRM_DEBUG_DRIVER("SWSCI requested %08x vs. supported %08x SBCB callbacks mismatch\n", opregion->swsci_sbcb_sub_functions, tmp);
+			/* XXX: for now, trust the requested callbacks */
+			/* opregion->swsci_sbcb_sub_functions &= tmp; */
+		} else {
+			opregion->swsci_sbcb_sub_functions |= tmp;
+		}
+	}
+
+	DRM_DEBUG_DRIVER("SWSCI GBDA callbacks %08x, SBCB callbacks %08x\n",
+			 opregion->swsci_gbda_sub_functions,
+			 opregion->swsci_sbcb_sub_functions);
+}
+#else /* CONFIG_ACPI */
+static inline void swsci_setup(struct drm_device *dev) {}
+#endif  /* CONFIG_ACPI */
 
 int intel_opregion_setup(struct drm_device *dev)
 {
@@ -490,6 +681,7 @@ int intel_opregion_setup(struct drm_device *dev)
 	if (mboxes & MBOX_SWSCI) {
 		DRM_DEBUG_DRIVER("SWSCI supported\n");
 		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
+		swsci_setup(dev);
 	}
 	if (mboxes & MBOX_ASLE) {
 		DRM_DEBUG_DRIVER("ASLE supported\n");
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
  2013-08-30 16:40 ` [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port() Jani Nikula
  2013-08-30 16:40 ` [PATCH 2/6] drm/i915: add plumbing for SWSCI Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-08-30 16:40 ` [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state Jani Nikula
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

The bios interface seems messy, and it's hard to tell what the bios
really wants. At first, only add support for DDI based machines (hsw+),
and see how it turns out.

The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.

v2:
 - squash notification function and callers patches together (Daniel)
 - move callers to haswell_crtc_{enable,disable} (Daniel)
 - rename notification function (Chris)

v3:
 - separate notification function and callers again, as it's not clear
   whether the display power state notification is the right thing to do
   after all

v4: per Paulo's review:
 - drop LVDS
 - WARN on unsupported encoder types

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |    8 ++++++
 drivers/gpu/drm/i915/intel_opregion.c |   51 +++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 67673e2..5339297 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2182,15 +2182,23 @@ static inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
 extern void intel_i2c_reset(struct drm_device *dev);
 
 /* intel_opregion.c */
+struct intel_encoder;
 extern int intel_opregion_setup(struct drm_device *dev);
 #ifdef CONFIG_ACPI
 extern void intel_opregion_init(struct drm_device *dev);
 extern void intel_opregion_fini(struct drm_device *dev);
 extern void intel_opregion_asle_intr(struct drm_device *dev);
+extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+					 bool enable);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
 static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
+static inline int
+intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
+{
+	return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index c6a8d61..791991b 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -291,6 +291,57 @@ static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
 #undef C
 }
 
+#define DISPLAY_TYPE_CRT			0
+#define DISPLAY_TYPE_TV				1
+#define DISPLAY_TYPE_EXTERNAL_FLAT_PANEL	2
+#define DISPLAY_TYPE_INTERNAL_FLAT_PANEL	3
+
+int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+				  bool enable)
+{
+	struct drm_device *dev = intel_encoder->base.dev;
+	u32 parm = 0;
+	u32 type = 0;
+	u32 port;
+
+	/* don't care about old stuff for now */
+	if (!HAS_DDI(dev))
+		return 0;
+
+	port = intel_ddi_get_encoder_port(intel_encoder);
+	if (port == PORT_E) {
+		port = 0;
+	} else {
+		parm |= 1 << port;
+		port++;
+	}
+
+	if (!enable)
+		parm |= 4 << 8;
+
+	switch (intel_encoder->type) {
+	case INTEL_OUTPUT_ANALOG:
+		type = DISPLAY_TYPE_CRT;
+		break;
+	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_HDMI:
+		type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
+		break;
+	case INTEL_OUTPUT_EDP:
+		type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
+		break;
+	default:
+		WARN_ONCE(1, "unsupported intel_encoder type %d\n",
+			  intel_encoder->type);
+		return -EINVAL;
+	}
+
+	parm |= type << (16 + port * 3);
+
+	return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
                   ` (2 preceding siblings ...)
  2013-08-30 16:40 ` [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-08-30 19:47   ` Paulo Zanoni
  2013-08-30 16:40 ` [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable Jani Nikula
  2013-08-30 16:40 ` [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable Jani Nikula
  5 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

Notifying the bios lets it enter power saving states.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |    7 +++++++
 drivers/gpu/drm/i915/intel_opregion.c |   27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5339297..7daae2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2190,6 +2190,8 @@ extern void intel_opregion_fini(struct drm_device *dev);
 extern void intel_opregion_asle_intr(struct drm_device *dev);
 extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 					 bool enable);
+extern int intel_opregion_notify_adapter(struct drm_device *dev,
+					 pci_power_t state);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
@@ -2199,6 +2201,11 @@ intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
 {
 	return 0;
 }
+static inline int
+intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
+{
+	return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 791991b..05243df 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -342,6 +342,33 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
 }
 
+static const struct {
+	pci_power_t pci_power_state;
+	u32 parm;
+} power_state_map[] = {
+	{ PCI_D0,	0x00 },
+	{ PCI_D1,	0x01 },
+	{ PCI_D2,	0x02 },
+	{ PCI_D3hot,	0x04 },
+	{ PCI_D3cold,	0x04 },
+};
+
+int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
+{
+	int i;
+
+	if (!HAS_DDI(dev))
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(power_state_map); i++) {
+		if (state == power_state_map[i].pci_power_state)
+			return swsci(dev, SWSCI_SBCB_ADAPTER_POWER_STATE,
+				     power_state_map[i].parm, NULL);
+	}
+
+	return -EINVAL;
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
                   ` (3 preceding siblings ...)
  2013-08-30 16:40 ` [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-08-30 19:51   ` Paulo Zanoni
  2013-08-30 16:40 ` [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable Jani Nikula
  5 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

---

Paulo, still okay with the r-b?
---
 drivers/gpu/drm/i915/intel_display.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9222e570..83d853f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3425,8 +3425,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		encoder->enable(encoder);
+		intel_opregion_notify_encoder(encoder, true);
+	}
 
 	/*
 	 * There seems to be a race in PCH platform hw (at least on some
@@ -3540,8 +3542,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
+		intel_opregion_notify_encoder(encoder, false);
 		encoder->disable(encoder);
+	}
 
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
  2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
                   ` (4 preceding siblings ...)
  2013-08-30 16:40 ` [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable Jani Nikula
@ 2013-08-30 16:40 ` Jani Nikula
  2013-08-30 19:55   ` Paulo Zanoni
  5 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-08-30 16:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, kristen.c.accardi

v2:
 - go to PCI_D3cold
 - shuffle the call site a bit

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 83d853f..e33fa6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6126,6 +6126,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
 
 	lpt_disable_clkout_dp(dev);
 	hsw_pc8_disable_interrupts(dev);
+	intel_opregion_notify_adapter(dev, PCI_D3cold);
 	hsw_disable_lcpll(dev_priv, true, true);
 }
 
@@ -6163,6 +6164,7 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_KMS("Disabling package C8+\n");
 
 	hsw_restore_lcpll(dev_priv);
+	intel_opregion_notify_adapter(dev, PCI_D0);
 	hsw_pc8_restore_interrupts(dev);
 	lpt_init_pch_refclk(dev);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/6] drm/i915: add plumbing for SWSCI
  2013-08-30 16:40 ` [PATCH 2/6] drm/i915: add plumbing for SWSCI Jani Nikula
@ 2013-08-30 19:31   ` Paulo Zanoni
  2013-09-02  7:38     ` [PATCH v(N+1)] " Jani Nikula
  0 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2013-08-30 19:31 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development, Kristen Carlson Accardi

2013/8/30 Jani Nikula <jani.nikula@intel.com>:
> SWSCI is a driver to bios call interface.
>
> This checks for SWSCI availability and bios requested callbacks, and
> filters out any calls that shouldn't happen. This way the callers don't
> need to do the checks all over the place.
>
> v2: silence some checkpatch nagging
>
> v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)
>
> v4: remove an extra #define (Jesse)
>
> v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too
>
> v6: per Paulo's review and more:
>  - fix sub-function mask
>  - add exit parameter
>  - add define for set panel details call
>  - return more errors from swsci
>  - clean up the supported/requested callbacks bit masks mess
>  - use DSLP for timeout
>  - fix build for CONFIG_ACPI=n
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |    2 +
>  drivers/gpu/drm/i915/intel_opregion.c |  198 ++++++++++++++++++++++++++++++++-
>  2 files changed, 197 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0602c4b..67673e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -225,6 +225,8 @@ struct intel_opregion {
>         struct opregion_header __iomem *header;
>         struct opregion_acpi __iomem *acpi;
>         struct opregion_swsci __iomem *swsci;
> +       u32 swsci_gbda_sub_functions;
> +       u32 swsci_sbcb_sub_functions;
>         struct opregion_asle __iomem *asle;
>         void __iomem *vbt;
>         u32 __iomem *lid_state;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index cfb8fb6..c6a8d61 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -36,8 +36,11 @@
>  #include "i915_drv.h"
>  #include "intel_drv.h"
>
> -#define PCI_ASLE 0xe4
> -#define PCI_ASLS 0xfc
> +#define PCI_ASLE               0xe4
> +#define PCI_ASLS               0xfc
> +#define PCI_SWSCI              0xe8
> +#define PCI_SWSCI_SCISEL       (1 << 15)
> +#define PCI_SWSCI_GSSCIE       (1 << 0)
>
>  #define OPREGION_HEADER_OFFSET 0
>  #define OPREGION_ACPI_OFFSET   0x100
> @@ -151,6 +154,51 @@ struct opregion_asle {
>
>  #define ASLE_CBLV_VALID         (1<<31)
>
> +/* Software System Control Interrupt (SWSCI) */
> +#define SWSCI_SCIC_INDICATOR           (1 << 0)
> +#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
> +#define SWSCI_SCIC_MAIN_FUNCTION_MASK  (0xf << 1)
> +#define SWSCI_SCIC_SUB_FUNCTION_SHIFT  8
> +#define SWSCI_SCIC_SUB_FUNCTION_MASK   (0xff << 8)
> +#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT        8
> +#define SWSCI_SCIC_EXIT_PARAMETER_MASK (0xff << 8)
> +#define SWSCI_SCIC_EXIT_STATUS_SHIFT   5
> +#define SWSCI_SCIC_EXIT_STATUS_MASK    (7 << 5)
> +#define SWSCI_SCIC_EXIT_STATUS_SUCCESS 1
> +
> +#define SWSCI_FUNCTION_CODE(main, sub) \
> +       ((main) << SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
> +        (sub) << SWSCI_SCIC_SUB_FUNCTION_SHIFT)
> +
> +/* SWSCI: Get BIOS Data (GBDA) */
> +#define SWSCI_GBDA                     4
> +#define SWSCI_GBDA_SUPPORTED_CALLS     SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
> +#define SWSCI_GBDA_REQUESTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
> +#define SWSCI_GBDA_BOOT_DISPLAY_PREF   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
> +#define SWSCI_GBDA_PANEL_DETAILS       SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
> +#define SWSCI_GBDA_TV_STANDARD         SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
> +#define SWSCI_GBDA_INTERNAL_GRAPHICS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
> +#define SWSCI_GBDA_SPREAD_SPECTRUM     SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
> +
> +/* SWSCI: System BIOS Callbacks (SBCB) */
> +#define SWSCI_SBCB                     6
> +#define SWSCI_SBCB_SUPPORTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
> +#define SWSCI_SBCB_INIT_COMPLETION     SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
> +#define SWSCI_SBCB_PRE_HIRES_SET_MODE  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
> +#define SWSCI_SBCB_POST_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
> +#define SWSCI_SBCB_DISPLAY_SWITCH      SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
> +#define SWSCI_SBCB_SET_TV_FORMAT       SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
> +#define SWSCI_SBCB_ADAPTER_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
> +#define SWSCI_SBCB_DISPLAY_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
> +#define SWSCI_SBCB_SET_BOOT_DISPLAY    SWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
> +#define SWSCI_SBCB_SET_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
> +#define SWSCI_SBCB_SET_INTERNAL_GFX    SWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
> +#define SWSCI_SBCB_POST_HIRES_TO_DOS_FS        SWSCI_FUNCTION_CODE(SWSCI_SBCB, 16)
> +#define SWSCI_SBCB_SUSPEND_RESUME      SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
> +#define SWSCI_SBCB_SET_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
> +#define SWSCI_SBCB_POST_VBE_PM         SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
> +#define SWSCI_SBCB_ENABLE_DISABLE_AUDIO        SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
> +
>  #define ACPI_OTHER_OUTPUT (0<<8)
>  #define ACPI_VGA_OUTPUT (1<<8)
>  #define ACPI_TV_OUTPUT (2<<8)
> @@ -158,6 +206,91 @@ struct opregion_asle {
>  #define ACPI_LVDS_OUTPUT (4<<8)
>
>  #ifdef CONFIG_ACPI
> +static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
> +{
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct opregion_swsci __iomem *swsci = dev_priv->opregion.swsci;
> +       u32 main_function, sub_function, scic;
> +       u16 pci_swsci;
> +       u32 dslp;
> +
> +       if (!swsci)
> +               return -ENODEV;
> +
> +       main_function = (function & SWSCI_SCIC_MAIN_FUNCTION_MASK) >>
> +               SWSCI_SCIC_MAIN_FUNCTION_SHIFT;
> +       sub_function = (function & SWSCI_SCIC_SUB_FUNCTION_MASK) >>
> +               SWSCI_SCIC_SUB_FUNCTION_SHIFT;
> +
> +       /* Check if we can call the function. See swsci_setup for details. */
> +       if (main_function == SWSCI_SBCB) {
> +               if ((dev_priv->opregion.swsci_sbcb_sub_functions &
> +                    (1 << sub_function)) == 0)
> +                       return -EINVAL;
> +       } else if (main_function == SWSCI_GBDA) {
> +               if ((dev_priv->opregion.swsci_gbda_sub_functions &
> +                    (1 << sub_function)) == 0)
> +                       return -EINVAL;
> +       }
> +
> +       /* Driver sleep timeout in ms. */
> +       dslp = ioread32(&swsci->dslp);
> +       if (!dslp) {
> +               dslp = 2;
> +       } else if (dslp > 500) {
> +               /* Hey bios, trust must be earned. */
> +               WARN_ONCE(1, "excessive driver sleep timeout (DSPL) %u\n", dslp);
> +               dslp = 500;
> +       }
> +
> +       /* The spec tells us to do this, but we are the only user... */
> +       scic = ioread32(&swsci->scic);
> +       if (scic & SWSCI_SCIC_INDICATOR) {
> +               DRM_DEBUG_DRIVER("SWSCI request already in progress\n");
> +               return -EBUSY;
> +       }
> +
> +       scic = function | SWSCI_SCIC_INDICATOR;
> +
> +       iowrite32(parm, &swsci->parm);
> +       iowrite32(scic, &swsci->scic);
> +
> +       /* Ensure SCI event is selected and event trigger is cleared. */
> +       pci_read_config_word(dev->pdev, PCI_SWSCI, &pci_swsci);
> +       if (!(pci_swsci & PCI_SWSCI_SCISEL) || (pci_swsci & PCI_SWSCI_GSSCIE)) {
> +               pci_swsci |= PCI_SWSCI_SCISEL;
> +               pci_swsci &= ~PCI_SWSCI_GSSCIE;
> +               pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
> +       }
> +
> +       /* Use event trigger to tell bios to check the mail. */
> +       pci_swsci |= PCI_SWSCI_GSSCIE;
> +       pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
> +
> +       /* Poll for the result. */
> +#define C (((scic = ioread32(&swsci->scic)) & SWSCI_SCIC_INDICATOR) == 0)
> +       if (wait_for(C, dslp)) {
> +               DRM_DEBUG_DRIVER("SWSCI request timed out\n");
> +               return -ETIMEDOUT;
> +       }
> +
> +       scic = (scic & SWSCI_SCIC_EXIT_STATUS_MASK) >>
> +               SWSCI_SCIC_EXIT_STATUS_SHIFT;
> +
> +       /* Note: scic == 0 is an error! */
> +       if (scic != SWSCI_SCIC_EXIT_STATUS_SUCCESS) {
> +               DRM_DEBUG_DRIVER("SWSCI request error %u\n", scic);
> +               return -EIO;
> +       }
> +
> +       if (parm_out)
> +               *parm_out = ioread32(&swsci->parm);
> +
> +       return 0;
> +
> +#undef C
> +}
> +
>  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -447,7 +580,65 @@ void intel_opregion_fini(struct drm_device *dev)
>         opregion->asle = NULL;
>         opregion->vbt = NULL;
>  }
> -#endif
> +
> +static void swsci_setup(struct drm_device *dev)
> +{
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_opregion *opregion = &dev_priv->opregion;
> +       bool requested_callbacks = false;
> +       u32 tmp;
> +
> +       /* Sub-function code 0 is okay, let's allow them. */
> +       opregion->swsci_gbda_sub_functions = 1;
> +       opregion->swsci_sbcb_sub_functions = 1;
> +
> +       /* We use GBDA to ask for supported GBDA calls. */
> +       if (swsci(dev, SWSCI_GBDA_SUPPORTED_CALLS, 0, &tmp) == 0) {
> +               /* make the bits match the sub-function codes */
> +               tmp <<= 1;
> +               opregion->swsci_gbda_sub_functions |= tmp;
> +       }
> +
> +       /*
> +        * We also use GBDA to ask for _requested_ SBCB callbacks. The driver
> +        * must not call interfaces that are not specifically requested by the
> +        * bios.
> +        */
> +       if (swsci(dev, SWSCI_GBDA_REQUESTED_CALLBACKS, 0, &tmp) == 0) {
> +               /* here, the bits already match sub-function codes */
> +               opregion->swsci_sbcb_sub_functions |= tmp;
> +               requested_callbacks = true;
> +       }
> +
> +       /*
> +        * But we use SBCB to ask for _supported_ SBCB calls. This does not mean
> +        * the callback is _requested_. But we still can't call interfaces that
> +        * are not requested.
> +        */

This is the point where you stop and wonder "if it knows we're not
supposed to call it, why does it advertise the function?". </rant>


> +       if (swsci(dev, SWSCI_SBCB_SUPPORTED_CALLBACKS, 0, &tmp) == 0) {
> +               /* make the bits match the sub-function codes */
> +               u32 low = tmp & 0x7ff;
> +               u32 high = tmp & ~0xfff;

u32 middle = tmp & 0x800 ?

Luckily the forgotten bit is reserved :)
And we don't know if it's high or low.


> +               tmp = (high << 4) | (low << 1) | 1;
> +
> +               /* best guess what to do with supported wrt requested */
> +               if (requested_callbacks) {
> +                       if (opregion->swsci_sbcb_sub_functions ^ tmp)
> +                               DRM_DEBUG_DRIVER("SWSCI requested %08x vs. supported %08x SBCB callbacks mismatch\n", opregion->swsci_sbcb_sub_functions, tmp);

My machine:
[   12.307122] [drm:swsci_setup], SWSCI requested 00300483 vs.
supported 00f80fbb SBCB callbacks mismatch

When I see "mismatch" message in our driver I usually get worried
(because of all that modeset-checking code messages).  But if
requested is just a subset of supported, there's no problem. We should
probably print an error message in case we detect a requested function
is not supported.


> +                       /* XXX: for now, trust the requested callbacks */
> +                       /* opregion->swsci_sbcb_sub_functions &= tmp; */

The line above makes sense, I'm not against it. I'm just against
paradoxical BIOSes...


With or without any changes: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>



> +               } else {
> +                       opregion->swsci_sbcb_sub_functions |= tmp;
> +               }
> +       }
> +
> +       DRM_DEBUG_DRIVER("SWSCI GBDA callbacks %08x, SBCB callbacks %08x\n",
> +                        opregion->swsci_gbda_sub_functions,
> +                        opregion->swsci_sbcb_sub_functions);
> +}
> +#else /* CONFIG_ACPI */
> +static inline void swsci_setup(struct drm_device *dev) {}
> +#endif  /* CONFIG_ACPI */
>
>  int intel_opregion_setup(struct drm_device *dev)
>  {
> @@ -490,6 +681,7 @@ int intel_opregion_setup(struct drm_device *dev)
>         if (mboxes & MBOX_SWSCI) {
>                 DRM_DEBUG_DRIVER("SWSCI supported\n");
>                 opregion->swsci = base + OPREGION_SWSCI_OFFSET;
> +               swsci_setup(dev);
>         }
>         if (mboxes & MBOX_ASLE) {
>                 DRM_DEBUG_DRIVER("ASLE supported\n");
> --
> 1.7.10.4
>



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state
  2013-08-30 16:40 ` [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state Jani Nikula
@ 2013-08-30 19:47   ` Paulo Zanoni
  0 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2013-08-30 19:47 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development, Kristen Carlson Accardi

2013/8/30 Jani Nikula <jani.nikula@intel.com>:
> Notifying the bios lets it enter power saving states.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h       |    7 +++++++
>  drivers/gpu/drm/i915/intel_opregion.c |   27 +++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5339297..7daae2a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2190,6 +2190,8 @@ extern void intel_opregion_fini(struct drm_device *dev);
>  extern void intel_opregion_asle_intr(struct drm_device *dev);
>  extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>                                          bool enable);
> +extern int intel_opregion_notify_adapter(struct drm_device *dev,
> +                                        pci_power_t state);
>  #else
>  static inline void intel_opregion_init(struct drm_device *dev) { return; }
>  static inline void intel_opregion_fini(struct drm_device *dev) { return; }
> @@ -2199,6 +2201,11 @@ intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
>  {
>         return 0;
>  }
> +static inline int
> +intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
> +{
> +       return 0;
> +}
>  #endif
>
>  /* intel_acpi.c */
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 791991b..05243df 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -342,6 +342,33 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>         return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
>  }
>
> +static const struct {
> +       pci_power_t pci_power_state;
> +       u32 parm;
> +} power_state_map[] = {
> +       { PCI_D0,       0x00 },
> +       { PCI_D1,       0x01 },
> +       { PCI_D2,       0x02 },
> +       { PCI_D3hot,    0x04 },
> +       { PCI_D3cold,   0x04 },
> +};
> +
> +int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
> +{
> +       int i;
> +
> +       if (!HAS_DDI(dev))
> +               return 0;
> +
> +       for (i = 0; i < ARRAY_SIZE(power_state_map); i++) {
> +               if (state == power_state_map[i].pci_power_state)
> +                       return swsci(dev, SWSCI_SBCB_ADAPTER_POWER_STATE,
> +                                    power_state_map[i].parm, NULL);
> +       }
> +
> +       return -EINVAL;
> +}
> +
>  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> --
> 1.7.10.4
>



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable
  2013-08-30 16:40 ` [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable Jani Nikula
@ 2013-08-30 19:51   ` Paulo Zanoni
  2013-09-02 14:10     ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2013-08-30 19:51 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development, Kristen Carlson Accardi

2013/8/30 Jani Nikula <jani.nikula@intel.com>:
> The spec says to notify prior to power down and after power up. It is
> unclear whether it makes a difference.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> ---
>
> Paulo, still okay with the r-b?

Yes :)

> ---
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9222e570..83d853f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3425,8 +3425,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         intel_update_fbc(dev);
>         mutex_unlock(&dev->struct_mutex);
>
> -       for_each_encoder_on_crtc(dev, crtc, encoder)
> +       for_each_encoder_on_crtc(dev, crtc, encoder) {
>                 encoder->enable(encoder);
> +               intel_opregion_notify_encoder(encoder, true);
> +       }
>
>         /*
>          * There seems to be a race in PCH platform hw (at least on some
> @@ -3540,8 +3542,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>         if (!intel_crtc->active)
>                 return;
>
> -       for_each_encoder_on_crtc(dev, crtc, encoder)
> +       for_each_encoder_on_crtc(dev, crtc, encoder) {
> +               intel_opregion_notify_encoder(encoder, false);
>                 encoder->disable(encoder);
> +       }
>
>         intel_crtc_wait_for_pending_flips(crtc);
>         drm_vblank_off(dev, pipe);
> --
> 1.7.10.4
>



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
  2013-08-30 16:40 ` [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable Jani Nikula
@ 2013-08-30 19:55   ` Paulo Zanoni
  0 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2013-08-30 19:55 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development, Kristen Carlson Accardi

2013/8/30 Jani Nikula <jani.nikula@intel.com>:
> v2:
>  - go to PCI_D3cold
>  - shuffle the call site a bit

Ok, so I know I'm the one who requested to shuffle the call site, but
it's because I thought that when we disable LCPLL we actually put the
device in D3. After some experimentation last week, we discovered we
need to write a PCI config register to actually enable D3, so your
call to intel_opregion_notify_adapter should probably be glued to that
write (which we don't have yet). The problem is that if we really put
the device in D3, all the registers go away, so we need to properly
recover from that. I think it makes sense to postpone this patch until
we have the actual code to put our device in D3 and properly restore
from it. So my suggestion is to merge patches 1-5 so we can work on
the D3 feature on top of the work you already completed.

Thanks,
Paulo

>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 83d853f..e33fa6d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6126,6 +6126,7 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>
>         lpt_disable_clkout_dp(dev);
>         hsw_pc8_disable_interrupts(dev);
> +       intel_opregion_notify_adapter(dev, PCI_D3cold);
>         hsw_disable_lcpll(dev_priv, true, true);
>  }
>
> @@ -6163,6 +6164,7 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
>         DRM_DEBUG_KMS("Disabling package C8+\n");
>
>         hsw_restore_lcpll(dev_priv);
> +       intel_opregion_notify_adapter(dev, PCI_D0);
>         hsw_pc8_restore_interrupts(dev);
>         lpt_init_pch_refclk(dev);
>
> --
> 1.7.10.4
>



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v(N+1)] drm/i915: add plumbing for SWSCI
  2013-08-30 19:31   ` Paulo Zanoni
@ 2013-09-02  7:38     ` Jani Nikula
  2013-09-02 14:02       ` Paulo Zanoni
  0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2013-09-02  7:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

SWSCI is a driver to bios call interface.

This checks for SWSCI availability and bios requested callbacks, and
filters out any calls that shouldn't happen. This way the callers don't
need to do the checks all over the place.

v2: silence some checkpatch nagging

v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)

v4: remove an extra #define (Jesse)

v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too

v6: per Paulo's review and more:
 - fix sub-function mask
 - add exit parameter
 - add define for set panel details call
 - return more errors from swsci
 - clean up the supported/requested callbacks bit masks mess
 - use DSLP for timeout
 - fix build for CONFIG_ACPI=n

v7: tiny adjustment of requested vs. supported SBCB callbacks handling (Paulo)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |    2 +
 drivers/gpu/drm/i915/intel_opregion.c |  199 ++++++++++++++++++++++++++++++++-
 2 files changed, 198 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0602c4b..67673e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -225,6 +225,8 @@ struct intel_opregion {
 	struct opregion_header __iomem *header;
 	struct opregion_acpi __iomem *acpi;
 	struct opregion_swsci __iomem *swsci;
+	u32 swsci_gbda_sub_functions;
+	u32 swsci_sbcb_sub_functions;
 	struct opregion_asle __iomem *asle;
 	void __iomem *vbt;
 	u32 __iomem *lid_state;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..c45fec5 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -36,8 +36,11 @@
 #include "i915_drv.h"
 #include "intel_drv.h"
 
-#define PCI_ASLE 0xe4
-#define PCI_ASLS 0xfc
+#define PCI_ASLE		0xe4
+#define PCI_ASLS		0xfc
+#define PCI_SWSCI		0xe8
+#define PCI_SWSCI_SCISEL	(1 << 15)
+#define PCI_SWSCI_GSSCIE	(1 << 0)
 
 #define OPREGION_HEADER_OFFSET 0
 #define OPREGION_ACPI_OFFSET   0x100
@@ -151,6 +154,51 @@ struct opregion_asle {
 
 #define ASLE_CBLV_VALID         (1<<31)
 
+/* Software System Control Interrupt (SWSCI) */
+#define SWSCI_SCIC_INDICATOR		(1 << 0)
+#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
+#define SWSCI_SCIC_MAIN_FUNCTION_MASK	(0xf << 1)
+#define SWSCI_SCIC_SUB_FUNCTION_SHIFT	8
+#define SWSCI_SCIC_SUB_FUNCTION_MASK	(0xff << 8)
+#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT	8
+#define SWSCI_SCIC_EXIT_PARAMETER_MASK	(0xff << 8)
+#define SWSCI_SCIC_EXIT_STATUS_SHIFT	5
+#define SWSCI_SCIC_EXIT_STATUS_MASK	(7 << 5)
+#define SWSCI_SCIC_EXIT_STATUS_SUCCESS	1
+
+#define SWSCI_FUNCTION_CODE(main, sub) \
+	((main) << SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
+	 (sub) << SWSCI_SCIC_SUB_FUNCTION_SHIFT)
+
+/* SWSCI: Get BIOS Data (GBDA) */
+#define SWSCI_GBDA			4
+#define SWSCI_GBDA_SUPPORTED_CALLS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
+#define SWSCI_GBDA_REQUESTED_CALLBACKS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
+#define SWSCI_GBDA_BOOT_DISPLAY_PREF	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
+#define SWSCI_GBDA_PANEL_DETAILS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
+#define SWSCI_GBDA_TV_STANDARD		SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
+#define SWSCI_GBDA_INTERNAL_GRAPHICS	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
+#define SWSCI_GBDA_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
+
+/* SWSCI: System BIOS Callbacks (SBCB) */
+#define SWSCI_SBCB			6
+#define SWSCI_SBCB_SUPPORTED_CALLBACKS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
+#define SWSCI_SBCB_INIT_COMPLETION	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
+#define SWSCI_SBCB_PRE_HIRES_SET_MODE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
+#define SWSCI_SBCB_POST_HIRES_SET_MODE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
+#define SWSCI_SBCB_DISPLAY_SWITCH	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
+#define SWSCI_SBCB_SET_TV_FORMAT	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
+#define SWSCI_SBCB_ADAPTER_POWER_STATE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
+#define SWSCI_SBCB_DISPLAY_POWER_STATE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
+#define SWSCI_SBCB_SET_BOOT_DISPLAY	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
+#define SWSCI_SBCB_SET_PANEL_DETAILS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
+#define SWSCI_SBCB_SET_INTERNAL_GFX	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
+#define SWSCI_SBCB_POST_HIRES_TO_DOS_FS	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 16)
+#define SWSCI_SBCB_SUSPEND_RESUME	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
+#define SWSCI_SBCB_SET_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
+#define SWSCI_SBCB_POST_VBE_PM		SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
+#define SWSCI_SBCB_ENABLE_DISABLE_AUDIO	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
+
 #define ACPI_OTHER_OUTPUT (0<<8)
 #define ACPI_VGA_OUTPUT (1<<8)
 #define ACPI_TV_OUTPUT (2<<8)
@@ -158,6 +206,91 @@ struct opregion_asle {
 #define ACPI_LVDS_OUTPUT (4<<8)
 
 #ifdef CONFIG_ACPI
+static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct opregion_swsci __iomem *swsci = dev_priv->opregion.swsci;
+	u32 main_function, sub_function, scic;
+	u16 pci_swsci;
+	u32 dslp;
+
+	if (!swsci)
+		return -ENODEV;
+
+	main_function = (function & SWSCI_SCIC_MAIN_FUNCTION_MASK) >>
+		SWSCI_SCIC_MAIN_FUNCTION_SHIFT;
+	sub_function = (function & SWSCI_SCIC_SUB_FUNCTION_MASK) >>
+		SWSCI_SCIC_SUB_FUNCTION_SHIFT;
+
+	/* Check if we can call the function. See swsci_setup for details. */
+	if (main_function == SWSCI_SBCB) {
+		if ((dev_priv->opregion.swsci_sbcb_sub_functions &
+		     (1 << sub_function)) == 0)
+			return -EINVAL;
+	} else if (main_function == SWSCI_GBDA) {
+		if ((dev_priv->opregion.swsci_gbda_sub_functions &
+		     (1 << sub_function)) == 0)
+			return -EINVAL;
+	}
+
+	/* Driver sleep timeout in ms. */
+	dslp = ioread32(&swsci->dslp);
+	if (!dslp) {
+		dslp = 2;
+	} else if (dslp > 500) {
+		/* Hey bios, trust must be earned. */
+		WARN_ONCE(1, "excessive driver sleep timeout (DSPL) %u\n", dslp);
+		dslp = 500;
+	}
+
+	/* The spec tells us to do this, but we are the only user... */
+	scic = ioread32(&swsci->scic);
+	if (scic & SWSCI_SCIC_INDICATOR) {
+		DRM_DEBUG_DRIVER("SWSCI request already in progress\n");
+		return -EBUSY;
+	}
+
+	scic = function | SWSCI_SCIC_INDICATOR;
+
+	iowrite32(parm, &swsci->parm);
+	iowrite32(scic, &swsci->scic);
+
+	/* Ensure SCI event is selected and event trigger is cleared. */
+	pci_read_config_word(dev->pdev, PCI_SWSCI, &pci_swsci);
+	if (!(pci_swsci & PCI_SWSCI_SCISEL) || (pci_swsci & PCI_SWSCI_GSSCIE)) {
+		pci_swsci |= PCI_SWSCI_SCISEL;
+		pci_swsci &= ~PCI_SWSCI_GSSCIE;
+		pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
+	}
+
+	/* Use event trigger to tell bios to check the mail. */
+	pci_swsci |= PCI_SWSCI_GSSCIE;
+	pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
+
+	/* Poll for the result. */
+#define C (((scic = ioread32(&swsci->scic)) & SWSCI_SCIC_INDICATOR) == 0)
+	if (wait_for(C, dslp)) {
+		DRM_DEBUG_DRIVER("SWSCI request timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	scic = (scic & SWSCI_SCIC_EXIT_STATUS_MASK) >>
+		SWSCI_SCIC_EXIT_STATUS_SHIFT;
+
+	/* Note: scic == 0 is an error! */
+	if (scic != SWSCI_SCIC_EXIT_STATUS_SUCCESS) {
+		DRM_DEBUG_DRIVER("SWSCI request error %u\n", scic);
+		return -EIO;
+	}
+
+	if (parm_out)
+		*parm_out = ioread32(&swsci->parm);
+
+	return 0;
+
+#undef C
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -447,7 +580,66 @@ void intel_opregion_fini(struct drm_device *dev)
 	opregion->asle = NULL;
 	opregion->vbt = NULL;
 }
-#endif
+
+static void swsci_setup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_opregion *opregion = &dev_priv->opregion;
+	bool requested_callbacks = false;
+	u32 tmp;
+
+	/* Sub-function code 0 is okay, let's allow them. */
+	opregion->swsci_gbda_sub_functions = 1;
+	opregion->swsci_sbcb_sub_functions = 1;
+
+	/* We use GBDA to ask for supported GBDA calls. */
+	if (swsci(dev, SWSCI_GBDA_SUPPORTED_CALLS, 0, &tmp) == 0) {
+		/* make the bits match the sub-function codes */
+		tmp <<= 1;
+		opregion->swsci_gbda_sub_functions |= tmp;
+	}
+
+	/*
+	 * We also use GBDA to ask for _requested_ SBCB callbacks. The driver
+	 * must not call interfaces that are not specifically requested by the
+	 * bios.
+	 */
+	if (swsci(dev, SWSCI_GBDA_REQUESTED_CALLBACKS, 0, &tmp) == 0) {
+		/* here, the bits already match sub-function codes */
+		opregion->swsci_sbcb_sub_functions |= tmp;
+		requested_callbacks = true;
+	}
+
+	/*
+	 * But we use SBCB to ask for _supported_ SBCB calls. This does not mean
+	 * the callback is _requested_. But we still can't call interfaces that
+	 * are not requested.
+	 */
+	if (swsci(dev, SWSCI_SBCB_SUPPORTED_CALLBACKS, 0, &tmp) == 0) {
+		/* make the bits match the sub-function codes */
+		u32 low = tmp & 0x7ff;
+		u32 high = tmp & ~0xfff; /* bit 11 is reserved */
+		tmp = (high << 4) | (low << 1) | 1;
+
+		/* best guess what to do with supported wrt requested */
+		if (requested_callbacks) {
+			u32 req = opregion->swsci_sbcb_sub_functions;
+			if ((req & tmp) != req)
+				DRM_DEBUG_DRIVER("SWSCI BIOS requested (%08x) SBCB callbacks that are not supported (%08x)\n", req, tmp);
+			/* XXX: for now, trust the requested callbacks */
+			/* opregion->swsci_sbcb_sub_functions &= tmp; */
+		} else {
+			opregion->swsci_sbcb_sub_functions |= tmp;
+		}
+	}
+
+	DRM_DEBUG_DRIVER("SWSCI GBDA callbacks %08x, SBCB callbacks %08x\n",
+			 opregion->swsci_gbda_sub_functions,
+			 opregion->swsci_sbcb_sub_functions);
+}
+#else /* CONFIG_ACPI */
+static inline void swsci_setup(struct drm_device *dev) {}
+#endif  /* CONFIG_ACPI */
 
 int intel_opregion_setup(struct drm_device *dev)
 {
@@ -490,6 +682,7 @@ int intel_opregion_setup(struct drm_device *dev)
 	if (mboxes & MBOX_SWSCI) {
 		DRM_DEBUG_DRIVER("SWSCI supported\n");
 		opregion->swsci = base + OPREGION_SWSCI_OFFSET;
+		swsci_setup(dev);
 	}
 	if (mboxes & MBOX_ASLE) {
 		DRM_DEBUG_DRIVER("ASLE supported\n");
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v(N+1)] drm/i915: add plumbing for SWSCI
  2013-09-02  7:38     ` [PATCH v(N+1)] " Jani Nikula
@ 2013-09-02 14:02       ` Paulo Zanoni
  0 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2013-09-02 14:02 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel Graphics Development

2013/9/2 Jani Nikula <jani.nikula@intel.com>:
> SWSCI is a driver to bios call interface.
>
> This checks for SWSCI availability and bios requested callbacks, and
> filters out any calls that shouldn't happen. This way the callers don't
> need to do the checks all over the place.
>
> v2: silence some checkpatch nagging
>
> v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)
>
> v4: remove an extra #define (Jesse)
>
> v5: spec says s/w is responsible for clearing PCI_SWSCI bit 0 too
>
> v6: per Paulo's review and more:
>  - fix sub-function mask
>  - add exit parameter
>  - add define for set panel details call
>  - return more errors from swsci
>  - clean up the supported/requested callbacks bit masks mess
>  - use DSLP for timeout
>  - fix build for CONFIG_ACPI=n
>
> v7: tiny adjustment of requested vs. supported SBCB callbacks handling (Paulo)
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>


> ---
>  drivers/gpu/drm/i915/i915_drv.h       |    2 +
>  drivers/gpu/drm/i915/intel_opregion.c |  199 ++++++++++++++++++++++++++++++++-
>  2 files changed, 198 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0602c4b..67673e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -225,6 +225,8 @@ struct intel_opregion {
>         struct opregion_header __iomem *header;
>         struct opregion_acpi __iomem *acpi;
>         struct opregion_swsci __iomem *swsci;
> +       u32 swsci_gbda_sub_functions;
> +       u32 swsci_sbcb_sub_functions;
>         struct opregion_asle __iomem *asle;
>         void __iomem *vbt;
>         u32 __iomem *lid_state;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index cfb8fb6..c45fec5 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -36,8 +36,11 @@
>  #include "i915_drv.h"
>  #include "intel_drv.h"
>
> -#define PCI_ASLE 0xe4
> -#define PCI_ASLS 0xfc
> +#define PCI_ASLE               0xe4
> +#define PCI_ASLS               0xfc
> +#define PCI_SWSCI              0xe8
> +#define PCI_SWSCI_SCISEL       (1 << 15)
> +#define PCI_SWSCI_GSSCIE       (1 << 0)
>
>  #define OPREGION_HEADER_OFFSET 0
>  #define OPREGION_ACPI_OFFSET   0x100
> @@ -151,6 +154,51 @@ struct opregion_asle {
>
>  #define ASLE_CBLV_VALID         (1<<31)
>
> +/* Software System Control Interrupt (SWSCI) */
> +#define SWSCI_SCIC_INDICATOR           (1 << 0)
> +#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
> +#define SWSCI_SCIC_MAIN_FUNCTION_MASK  (0xf << 1)
> +#define SWSCI_SCIC_SUB_FUNCTION_SHIFT  8
> +#define SWSCI_SCIC_SUB_FUNCTION_MASK   (0xff << 8)
> +#define SWSCI_SCIC_EXIT_PARAMETER_SHIFT        8
> +#define SWSCI_SCIC_EXIT_PARAMETER_MASK (0xff << 8)
> +#define SWSCI_SCIC_EXIT_STATUS_SHIFT   5
> +#define SWSCI_SCIC_EXIT_STATUS_MASK    (7 << 5)
> +#define SWSCI_SCIC_EXIT_STATUS_SUCCESS 1
> +
> +#define SWSCI_FUNCTION_CODE(main, sub) \
> +       ((main) << SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
> +        (sub) << SWSCI_SCIC_SUB_FUNCTION_SHIFT)
> +
> +/* SWSCI: Get BIOS Data (GBDA) */
> +#define SWSCI_GBDA                     4
> +#define SWSCI_GBDA_SUPPORTED_CALLS     SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
> +#define SWSCI_GBDA_REQUESTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
> +#define SWSCI_GBDA_BOOT_DISPLAY_PREF   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
> +#define SWSCI_GBDA_PANEL_DETAILS       SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
> +#define SWSCI_GBDA_TV_STANDARD         SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
> +#define SWSCI_GBDA_INTERNAL_GRAPHICS   SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
> +#define SWSCI_GBDA_SPREAD_SPECTRUM     SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
> +
> +/* SWSCI: System BIOS Callbacks (SBCB) */
> +#define SWSCI_SBCB                     6
> +#define SWSCI_SBCB_SUPPORTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
> +#define SWSCI_SBCB_INIT_COMPLETION     SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
> +#define SWSCI_SBCB_PRE_HIRES_SET_MODE  SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
> +#define SWSCI_SBCB_POST_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
> +#define SWSCI_SBCB_DISPLAY_SWITCH      SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
> +#define SWSCI_SBCB_SET_TV_FORMAT       SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
> +#define SWSCI_SBCB_ADAPTER_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
> +#define SWSCI_SBCB_DISPLAY_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
> +#define SWSCI_SBCB_SET_BOOT_DISPLAY    SWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
> +#define SWSCI_SBCB_SET_PANEL_DETAILS   SWSCI_FUNCTION_CODE(SWSCI_SBCB, 10)
> +#define SWSCI_SBCB_SET_INTERNAL_GFX    SWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
> +#define SWSCI_SBCB_POST_HIRES_TO_DOS_FS        SWSCI_FUNCTION_CODE(SWSCI_SBCB, 16)
> +#define SWSCI_SBCB_SUSPEND_RESUME      SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
> +#define SWSCI_SBCB_SET_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
> +#define SWSCI_SBCB_POST_VBE_PM         SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
> +#define SWSCI_SBCB_ENABLE_DISABLE_AUDIO        SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
> +
>  #define ACPI_OTHER_OUTPUT (0<<8)
>  #define ACPI_VGA_OUTPUT (1<<8)
>  #define ACPI_TV_OUTPUT (2<<8)
> @@ -158,6 +206,91 @@ struct opregion_asle {
>  #define ACPI_LVDS_OUTPUT (4<<8)
>
>  #ifdef CONFIG_ACPI
> +static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
> +{
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct opregion_swsci __iomem *swsci = dev_priv->opregion.swsci;
> +       u32 main_function, sub_function, scic;
> +       u16 pci_swsci;
> +       u32 dslp;
> +
> +       if (!swsci)
> +               return -ENODEV;
> +
> +       main_function = (function & SWSCI_SCIC_MAIN_FUNCTION_MASK) >>
> +               SWSCI_SCIC_MAIN_FUNCTION_SHIFT;
> +       sub_function = (function & SWSCI_SCIC_SUB_FUNCTION_MASK) >>
> +               SWSCI_SCIC_SUB_FUNCTION_SHIFT;
> +
> +       /* Check if we can call the function. See swsci_setup for details. */
> +       if (main_function == SWSCI_SBCB) {
> +               if ((dev_priv->opregion.swsci_sbcb_sub_functions &
> +                    (1 << sub_function)) == 0)
> +                       return -EINVAL;
> +       } else if (main_function == SWSCI_GBDA) {
> +               if ((dev_priv->opregion.swsci_gbda_sub_functions &
> +                    (1 << sub_function)) == 0)
> +                       return -EINVAL;
> +       }
> +
> +       /* Driver sleep timeout in ms. */
> +       dslp = ioread32(&swsci->dslp);
> +       if (!dslp) {
> +               dslp = 2;
> +       } else if (dslp > 500) {
> +               /* Hey bios, trust must be earned. */
> +               WARN_ONCE(1, "excessive driver sleep timeout (DSPL) %u\n", dslp);
> +               dslp = 500;
> +       }
> +
> +       /* The spec tells us to do this, but we are the only user... */
> +       scic = ioread32(&swsci->scic);
> +       if (scic & SWSCI_SCIC_INDICATOR) {
> +               DRM_DEBUG_DRIVER("SWSCI request already in progress\n");
> +               return -EBUSY;
> +       }
> +
> +       scic = function | SWSCI_SCIC_INDICATOR;
> +
> +       iowrite32(parm, &swsci->parm);
> +       iowrite32(scic, &swsci->scic);
> +
> +       /* Ensure SCI event is selected and event trigger is cleared. */
> +       pci_read_config_word(dev->pdev, PCI_SWSCI, &pci_swsci);
> +       if (!(pci_swsci & PCI_SWSCI_SCISEL) || (pci_swsci & PCI_SWSCI_GSSCIE)) {
> +               pci_swsci |= PCI_SWSCI_SCISEL;
> +               pci_swsci &= ~PCI_SWSCI_GSSCIE;
> +               pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
> +       }
> +
> +       /* Use event trigger to tell bios to check the mail. */
> +       pci_swsci |= PCI_SWSCI_GSSCIE;
> +       pci_write_config_word(dev->pdev, PCI_SWSCI, pci_swsci);
> +
> +       /* Poll for the result. */
> +#define C (((scic = ioread32(&swsci->scic)) & SWSCI_SCIC_INDICATOR) == 0)
> +       if (wait_for(C, dslp)) {
> +               DRM_DEBUG_DRIVER("SWSCI request timed out\n");
> +               return -ETIMEDOUT;
> +       }
> +
> +       scic = (scic & SWSCI_SCIC_EXIT_STATUS_MASK) >>
> +               SWSCI_SCIC_EXIT_STATUS_SHIFT;
> +
> +       /* Note: scic == 0 is an error! */
> +       if (scic != SWSCI_SCIC_EXIT_STATUS_SUCCESS) {
> +               DRM_DEBUG_DRIVER("SWSCI request error %u\n", scic);
> +               return -EIO;
> +       }
> +
> +       if (parm_out)
> +               *parm_out = ioread32(&swsci->parm);
> +
> +       return 0;
> +
> +#undef C
> +}
> +
>  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -447,7 +580,66 @@ void intel_opregion_fini(struct drm_device *dev)
>         opregion->asle = NULL;
>         opregion->vbt = NULL;
>  }
> -#endif
> +
> +static void swsci_setup(struct drm_device *dev)
> +{
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_opregion *opregion = &dev_priv->opregion;
> +       bool requested_callbacks = false;
> +       u32 tmp;
> +
> +       /* Sub-function code 0 is okay, let's allow them. */
> +       opregion->swsci_gbda_sub_functions = 1;
> +       opregion->swsci_sbcb_sub_functions = 1;
> +
> +       /* We use GBDA to ask for supported GBDA calls. */
> +       if (swsci(dev, SWSCI_GBDA_SUPPORTED_CALLS, 0, &tmp) == 0) {
> +               /* make the bits match the sub-function codes */
> +               tmp <<= 1;
> +               opregion->swsci_gbda_sub_functions |= tmp;
> +       }
> +
> +       /*
> +        * We also use GBDA to ask for _requested_ SBCB callbacks. The driver
> +        * must not call interfaces that are not specifically requested by the
> +        * bios.
> +        */
> +       if (swsci(dev, SWSCI_GBDA_REQUESTED_CALLBACKS, 0, &tmp) == 0) {
> +               /* here, the bits already match sub-function codes */
> +               opregion->swsci_sbcb_sub_functions |= tmp;
> +               requested_callbacks = true;
> +       }
> +
> +       /*
> +        * But we use SBCB to ask for _supported_ SBCB calls. This does not mean
> +        * the callback is _requested_. But we still can't call interfaces that
> +        * are not requested.
> +        */
> +       if (swsci(dev, SWSCI_SBCB_SUPPORTED_CALLBACKS, 0, &tmp) == 0) {
> +               /* make the bits match the sub-function codes */
> +               u32 low = tmp & 0x7ff;
> +               u32 high = tmp & ~0xfff; /* bit 11 is reserved */
> +               tmp = (high << 4) | (low << 1) | 1;
> +
> +               /* best guess what to do with supported wrt requested */
> +               if (requested_callbacks) {
> +                       u32 req = opregion->swsci_sbcb_sub_functions;
> +                       if ((req & tmp) != req)
> +                               DRM_DEBUG_DRIVER("SWSCI BIOS requested (%08x) SBCB callbacks that are not supported (%08x)\n", req, tmp);
> +                       /* XXX: for now, trust the requested callbacks */
> +                       /* opregion->swsci_sbcb_sub_functions &= tmp; */
> +               } else {
> +                       opregion->swsci_sbcb_sub_functions |= tmp;
> +               }
> +       }
> +
> +       DRM_DEBUG_DRIVER("SWSCI GBDA callbacks %08x, SBCB callbacks %08x\n",
> +                        opregion->swsci_gbda_sub_functions,
> +                        opregion->swsci_sbcb_sub_functions);
> +}
> +#else /* CONFIG_ACPI */
> +static inline void swsci_setup(struct drm_device *dev) {}
> +#endif  /* CONFIG_ACPI */
>
>  int intel_opregion_setup(struct drm_device *dev)
>  {
> @@ -490,6 +682,7 @@ int intel_opregion_setup(struct drm_device *dev)
>         if (mboxes & MBOX_SWSCI) {
>                 DRM_DEBUG_DRIVER("SWSCI supported\n");
>                 opregion->swsci = base + OPREGION_SWSCI_OFFSET;
> +               swsci_setup(dev);
>         }
>         if (mboxes & MBOX_ASLE) {
>                 DRM_DEBUG_DRIVER("ASLE supported\n");
> --
> 1.7.9.5
>



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port()
  2013-08-30 16:40 ` [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port() Jani Nikula
@ 2013-09-02 14:08   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2013-09-02 14:08 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, kristen.c.accardi

On Fri, Aug 30, 2013 at 07:40:28PM +0300, Jani Nikula wrote:
> In preparation for followup work.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Maintainer-bikeshed: I don't really see the value of "export foo" patches
since there's not much to judge them by. I prefer them squashed into the
first patch that starts to use it, maybe with a short note in the commit
message saying that something had to be exported.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |    2 +-
>  drivers/gpu/drm/i915/intel_drv.h |    1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 63aca49..060ea50 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -58,7 +58,7 @@ static const u32 hsw_ddi_translations_fdi[] = {
>  	0x00FFFFFF, 0x00040006		/* HDMI parameters */
>  };
>  
> -static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
> +enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
>  {
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	int type = intel_encoder->type;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a38f5b2..5efb844 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -712,6 +712,7 @@ extern void intel_write_eld(struct drm_encoder *encoder,
>  extern void intel_prepare_ddi(struct drm_device *dev);
>  extern void hsw_fdi_link_train(struct drm_crtc *crtc);
>  extern void intel_ddi_init(struct drm_device *dev, enum port port);
> +extern enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
>  
>  /* For use by IVB LP watermark workaround in intel_sprite.c */
>  extern void intel_update_watermarks(struct drm_device *dev);
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable
  2013-08-30 19:51   ` Paulo Zanoni
@ 2013-09-02 14:10     ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2013-09-02 14:10 UTC (permalink / raw)
  To: Paulo Zanoni
  Cc: Jani Nikula, Intel Graphics Development, Kristen Carlson Accardi

On Fri, Aug 30, 2013 at 04:51:09PM -0300, Paulo Zanoni wrote:
> 2013/8/30 Jani Nikula <jani.nikula@intel.com>:
> > The spec says to notify prior to power down and after power up. It is
> > unclear whether it makes a difference.
> >
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > ---
> >
> > Paulo, still okay with the r-b?
> 
> Yes :)

Ok I've merged the first 6 patches from this series to dinq.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-09-02 14:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-30 16:40 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula
2013-08-30 16:40 ` [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port() Jani Nikula
2013-09-02 14:08   ` Daniel Vetter
2013-08-30 16:40 ` [PATCH 2/6] drm/i915: add plumbing for SWSCI Jani Nikula
2013-08-30 19:31   ` Paulo Zanoni
2013-09-02  7:38     ` [PATCH v(N+1)] " Jani Nikula
2013-09-02 14:02       ` Paulo Zanoni
2013-08-30 16:40 ` [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable Jani Nikula
2013-08-30 16:40 ` [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state Jani Nikula
2013-08-30 19:47   ` Paulo Zanoni
2013-08-30 16:40 ` [PATCH 5/6] drm/i915: do display power state notification on crtc enable/disable Jani Nikula
2013-08-30 19:51   ` Paulo Zanoni
2013-09-02 14:10     ` Daniel Vetter
2013-08-30 16:40 ` [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable Jani Nikula
2013-08-30 19:55   ` Paulo Zanoni
  -- strict thread matches above, loose matches on Subject: below --
2013-08-23 10:17 [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox