2013/8/30 Jani Nikula <jani.nikula@xxxxxxxxx>:
> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> + } 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
