[MIPI,SEQ,PARSING,v2,03/11] drm/i915: Parsing VBT if size of VBT exceeds 6KB

Commit Message

Deepak M Sept. 9, 2015, 11:24 p.m. UTC

Currently the iomap for VBT works only if the size of the
VBT is less than 6KB, but if the size of the VBT exceeds
6KB than the physical address and the size of the VBT to
be iomapped is specified in the mailbox3 and is iomapped
accordingly.

v2: - Moving the validate_vbt to opregion file (Jani)
    - Fix the i915_opregion() in debugfs (Jani)

Signed-off-by: Deepak M <[e-mail]>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++-
 drivers/gpu/drm/i915/i915_drv.h       |    4 +
 drivers/gpu/drm/i915/intel_bios.c     |   49 +-----
 drivers/gpu/drm/i915/intel_opregion.c |  279 +++++++++------------------------
 drivers/gpu/drm/i915/intel_opregion.h |  230 +++++++++++++++++++++++++++
 5 files changed, 329 insertions(+), 257 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_opregion.h

Comments

Jani Nikula Sept. 16, 2015, 1:24 p.m. UTC | #1

On Thu, 10 Sep 2015, Deepak M <[e-mail]> wrote:
> Currently the iomap for VBT works only if the size of the
> VBT is less than 6KB, but if the size of the VBT exceeds
> 6KB than the physical address and the size of the VBT to
> be iomapped is specified in the mailbox3 and is iomapped
> accordingly.
>
> v2: - Moving the validate_vbt to opregion file (Jani)
>     - Fix the i915_opregion() in debugfs (Jani)
>
> Signed-off-by: Deepak M <[e-mail]>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++-
>  drivers/gpu/drm/i915/i915_drv.h       |    4 +
>  drivers/gpu/drm/i915/intel_bios.c     |   49 +-----
>  drivers/gpu/drm/i915/intel_opregion.c |  279 +++++++++------------------------
>  drivers/gpu/drm/i915/intel_opregion.h |  230 +++++++++++++++++++++++++++

Why are you adding this file? I think it's better to have all of this in
the .c file so that nobody will have any ideas about accessing the data
outside of intel_opregion.c.

BR,
Jani.

>  5 files changed, 329 insertions(+), 257 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_opregion.h
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 41629fa..5534aa2 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -26,6 +26,8 @@
>   *
>   */
>  
> +#include <linux/acpi.h>
> +#include <acpi/video.h>
>  #include <linux/seq_file.h>
>  #include <linux/circ_buf.h>
>  #include <linux/ctype.h>
> @@ -39,6 +41,7 @@
>  #include "intel_ringbuffer.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "intel_opregion.h"
>  
>  enum {
>  	ACTIVE_LIST,
> @@ -1832,7 +1835,7 @@ static int i915_opregion(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_opregion *opregion = &dev_priv->opregion;
> -	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
> +	void *data = kmalloc(OPREGION_VBT_OFFSET, GFP_KERNEL);
>  	int ret;
>  
>  	if (data == NULL)
> @@ -1843,12 +1846,25 @@ static int i915_opregion(struct seq_file *m, void *unused)
>  		goto out;
>  
>  	if (opregion->header) {
> -		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
> -		seq_write(m, data, OPREGION_SIZE);
> +		memcpy_fromio(data, opregion->header, OPREGION_VBT_OFFSET);
> +		seq_write(m, data, OPREGION_VBT_OFFSET);
> +		kfree(data);
> +		if (opregion->asle->rvda) {
> +			data = kmalloc(opregion->asle->rvds, GFP_KERNEL);
> +			memcpy_fromio(data,
> +				(const void __iomem *) opregion->asle->rvda,
> +				opregion->asle->rvds);
> +			seq_write(m, data, opregion->asle->rvds);
> +		} else {
> +			data = kmalloc(OPREGION_SIZE - OPREGION_VBT_OFFSET,
> +					GFP_KERNEL);
> +			memcpy_fromio(data, opregion->vbt,
> +					OPREGION_SIZE - OPREGION_VBT_OFFSET);
> +			seq_write(m, data, OPREGION_SIZE - OPREGION_VBT_OFFSET);
> +		}
>  	}
>  
>  	mutex_unlock(&dev->struct_mutex);
> -
>  out:
>  	kfree(data);
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1287007..507d57a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1780,6 +1780,7 @@ struct drm_i915_private {
>  	struct i915_hotplug hotplug;
>  	struct i915_fbc fbc;
>  	struct i915_drrs drrs;
> +	const struct bdb_header *bdb_start;
>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> @@ -3306,6 +3307,9 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>  }
>  #endif
>  
> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
> +			size_t size, const char *source);
> +
>  /* intel_acpi.c */
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 0bf0942..1932a86 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1227,49 +1227,6 @@ static const struct dmi_system_id intel_no_opregion_vbt[] = {
>  	{ }
>  };
>  
> -static const struct bdb_header *validate_vbt(const void __iomem *_base,
> -					     size_t size,
> -					     const void __iomem *_vbt,
> -					     const char *source)
> -{
> -	/*
> -	 * This is the one place where we explicitly discard the address space
> -	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
> -	 * From now on everything is based on 'base', and treated as regular
> -	 * memory.
> -	 */
> -	const void *base = (const void *) _base;
> -	size_t offset = _vbt - _base;
> -	const struct vbt_header *vbt = base + offset;
> -	const struct bdb_header *bdb;
> -
> -	if (offset + sizeof(struct vbt_header) > size) {
> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> -		return NULL;
> -	}
> -
> -	if (memcmp(vbt->signature, "$VBT", 4)) {
> -		DRM_DEBUG_DRIVER("VBT invalid signature\n");
> -		return NULL;
> -	}
> -
> -	offset += vbt->bdb_offset;
> -	if (offset + sizeof(struct bdb_header) > size) {
> -		DRM_DEBUG_DRIVER("BDB header incomplete\n");
> -		return NULL;
> -	}
> -
> -	bdb = base + offset;
> -	if (offset + bdb->bdb_size > size) {
> -		DRM_DEBUG_DRIVER("BDB incomplete\n");
> -		return NULL;
> -	}
> -
> -	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
> -		      source, vbt->signature);
> -	return bdb;
> -}
> -
>  static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>  {
>  	const struct bdb_header *bdb = NULL;
> @@ -1278,7 +1235,7 @@ static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>  	/* Scour memory looking for the VBT signature. */
>  	for (i = 0; i + 4 < size; i++) {
>  		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
> -			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
> +			bdb = validate_vbt(bios + i, size - i, "PCI ROM");
>  			break;
>  		}
>  	}
> @@ -1308,10 +1265,8 @@ intel_parse_bios(struct drm_device *dev)
>  
>  	init_vbt_defaults(dev_priv);
>  
> -	/* XXX Should this validation be moved to intel_opregion.c? */
>  	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv->opregion.vbt)
> -		bdb = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE,
> -				   dev_priv->opregion.vbt, "OpRegion");
> +		bdb = dev_priv->bdb_start;
>  
>  	if (bdb == NULL) {
>  		size_t size;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index f46231f..3a43db9 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -32,210 +32,7 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include "intel_drv.h"
> -
> -#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
> -#define   ACPI_CLID 0x01ac /* current lid state indicator */
> -#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> -#define OPREGION_SWSCI_OFFSET  0x200
> -#define OPREGION_ASLE_OFFSET   0x300
> -#define OPREGION_VBT_OFFSET    0x400
> -
> -#define OPREGION_SIGNATURE "IntelGraphicsMem"
> -#define MBOX_ACPI      (1<<0)
> -#define MBOX_SWSCI     (1<<1)
> -#define MBOX_ASLE      (1<<2)
> -#define MBOX_ASLE_EXT  (1<<4)
> -
> -struct opregion_header {
> -	u8 signature[16];
> -	u32 size;
> -	u32 opregion_ver;
> -	u8 bios_ver[32];
> -	u8 vbios_ver[16];
> -	u8 driver_ver[16];
> -	u32 mboxes;
> -	u32 driver_model;
> -	u32 pcon;
> -	u8 dver[32];
> -	u8 rsvd[124];
> -} __packed;
> -
> -/* OpRegion mailbox #1: public ACPI methods */
> -struct opregion_acpi {
> -	u32 drdy;       /* driver readiness */
> -	u32 csts;       /* notification status */
> -	u32 cevt;       /* current event */
> -	u8 rsvd1[20];
> -	u32 didl[8];    /* supported display devices ID list */
> -	u32 cpdl[8];    /* currently presented display list */
> -	u32 cadl[8];    /* currently active display list */
> -	u32 nadl[8];    /* next active devices list */
> -	u32 aslp;       /* ASL sleep time-out */
> -	u32 tidx;       /* toggle table index */
> -	u32 chpd;       /* current hotplug enable indicator */
> -	u32 clid;       /* current lid state*/
> -	u32 cdck;       /* current docking state */
> -	u32 sxsw;       /* Sx state resume */
> -	u32 evts;       /* ASL supported events */
> -	u32 cnot;       /* current OS notification */
> -	u32 nrdy;       /* driver status */
> -	u32 did2[7];	/* extended supported display devices ID list */
> -	u32 cpd2[7];	/* extended attached display devices list */
> -	u8 rsvd2[4];
> -} __packed;
> -
> -/* OpRegion mailbox #2: SWSCI */
> -struct opregion_swsci {
> -	u32 scic;       /* SWSCI command|status|data */
> -	u32 parm;       /* command parameters */
> -	u32 dslp;       /* driver sleep time-out */
> -	u8 rsvd[244];
> -} __packed;
> -
> -/* OpRegion mailbox #3: ASLE */
> -struct opregion_asle {
> -	u32 ardy;       /* driver readiness */
> -	u32 aslc;       /* ASLE interrupt command */
> -	u32 tche;       /* technology enabled indicator */
> -	u32 alsi;       /* current ALS illuminance reading */
> -	u32 bclp;       /* backlight brightness to set */
> -	u32 pfit;       /* panel fitting state */
> -	u32 cblv;       /* current brightness level */
> -	u16 bclm[20];   /* backlight level duty cycle mapping table */
> -	u32 cpfm;       /* current panel fitting mode */
> -	u32 epfm;       /* enabled panel fitting modes */
> -	u8 plut[74];    /* panel LUT and identifier */
> -	u32 pfmb;       /* PWM freq and min brightness */
> -	u32 cddv;       /* color correction default values */
> -	u32 pcft;       /* power conservation features */
> -	u32 srot;       /* supported rotation angles */
> -	u32 iuer;       /* IUER events */
> -	u64 fdss;
> -	u32 fdsp;
> -	u32 stat;
> -	u64 rvda;	/* Physical address of raw vbt data */
> -	u32 rvds;	/* Size of raw vbt data */
> -	u8 rsvd[58];
> -} __packed;
> -
> -/* Driver readiness indicator */
> -#define ASLE_ARDY_READY		(1 << 0)
> -#define ASLE_ARDY_NOT_READY	(0 << 0)
> -
> -/* ASLE Interrupt Command (ASLC) bits */
> -#define ASLC_SET_ALS_ILLUM		(1 << 0)
> -#define ASLC_SET_BACKLIGHT		(1 << 1)
> -#define ASLC_SET_PFIT			(1 << 2)
> -#define ASLC_SET_PWM_FREQ		(1 << 3)
> -#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
> -#define ASLC_BUTTON_ARRAY		(1 << 5)
> -#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
> -#define ASLC_DOCKING_INDICATOR		(1 << 7)
> -#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
> -#define ASLC_REQ_MSK			0x1ff
> -/* response bits */
> -#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
> -#define ASLC_BACKLIGHT_FAILED		(1 << 12)
> -#define ASLC_PFIT_FAILED		(1 << 14)
> -#define ASLC_PWM_FREQ_FAILED		(1 << 16)
> -#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
> -#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
> -#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
> -#define ASLC_DOCKING_FAILED		(1 << 24)
> -#define ASLC_ISCT_STATE_FAILED		(1 << 26)
> -
> -/* Technology enabled indicator */
> -#define ASLE_TCHE_ALS_EN	(1 << 0)
> -#define ASLE_TCHE_BLC_EN	(1 << 1)
> -#define ASLE_TCHE_PFIT_EN	(1 << 2)
> -#define ASLE_TCHE_PFMB_EN	(1 << 3)
> -
> -/* ASLE backlight brightness to set */
> -#define ASLE_BCLP_VALID                (1<<31)
> -#define ASLE_BCLP_MSK          (~(1<<31))
> -
> -/* ASLE panel fitting request */
> -#define ASLE_PFIT_VALID         (1<<31)
> -#define ASLE_PFIT_CENTER (1<<0)
> -#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> -#define ASLE_PFIT_STRETCH_GFX (1<<2)
> -
> -/* PWM frequency and minimum brightness */
> -#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> -#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> -#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> -#define ASLE_PFMB_PWM_VALID (1<<31)
> -
> -#define ASLE_CBLV_VALID         (1<<31)
> -
> -/* IUER */
> -#define ASLE_IUER_DOCKING		(1 << 7)
> -#define ASLE_IUER_CONVERTIBLE		(1 << 6)
> -#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
> -#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
> -#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
> -#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
> -#define ASLE_IUER_POWER_BTN		(1 << 0)
> -
> -/* 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)
> -#define ACPI_DIGITAL_OUTPUT (3<<8)
> -#define ACPI_LVDS_OUTPUT (4<<8)
> -
> -#define MAX_DSLP	1500
> +#include "intel_opregion.h"
>  
>  #ifdef CONFIG_ACPI
>  static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
> @@ -892,13 +689,55 @@ static void swsci_setup(struct drm_device *dev)
>  static inline void swsci_setup(struct drm_device *dev) {}
>  #endif  /* CONFIG_ACPI */
>  
> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
> +					size_t size,
> +					const char *source)
> +{
> +	/*
> +	 * This is the one place where we explicitly discard the address space
> +	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
> +	 */
> +	const struct vbt_header *vbt = (const struct vbt_header *)_vbt;
> +	const struct bdb_header *bdb;
> +	size_t offset;
> +
> +	if (sizeof(struct vbt_header) > size) {
> +		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> +		return NULL;
> +	}
> +
> +	if (memcmp(vbt->signature, "$VBT", 4)) {
> +		DRM_DEBUG_DRIVER("VBT invalid signature\n");
> +		return NULL;
> +	}
> +
> +	offset = vbt->bdb_offset;
> +	if (offset + sizeof(struct bdb_header) > size) {
> +		DRM_DEBUG_DRIVER("BDB header incomplete\n");
> +		return NULL;
> +	}
> +
> +	bdb = (const void *)_vbt + offset;
> +	if (offset + bdb->bdb_size > size) {
> +		DRM_DEBUG_DRIVER("BDB incomplete\n");
> +		return NULL;
> +	}
> +
> +	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
> +			source, vbt->signature);
> +	return bdb;
> +}
> +
>  int intel_opregion_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_opregion *opregion = &dev_priv->opregion;
>  	void __iomem *base;
> +	void __iomem *vbt_base;
>  	u32 asls, mboxes;
>  	char buf[sizeof(OPREGION_SIGNATURE)];
> +	const struct bdb_header *bdb = NULL;
> +	size_t size;
>  	int err = 0;
>  
>  	BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100);
> @@ -917,7 +756,7 @@ int intel_opregion_setup(struct drm_device *dev)
>  	INIT_WORK(&opregion->asle_work, asle_work);
>  #endif
>  
> -	base = acpi_os_ioremap(asls, OPREGION_SIZE);
> +	base = acpi_os_ioremap(asls, OPREGION_VBT_OFFSET);
>  	if (!base)
>  		return -ENOMEM;
>  
> @@ -929,7 +768,33 @@ int intel_opregion_setup(struct drm_device *dev)
>  		goto err_out;
>  	}
>  	opregion->header = base;
> -	opregion->vbt = base + OPREGION_VBT_OFFSET;
> +	/* Assigning the alse to the mailbox 3 of the opregion */
> +	opregion->asle = base + OPREGION_ASLE_OFFSET;
> +
> +	/*
> +	 * Non-zero value in rvda field is an indication to driver that a
> +	 * valid Raw VBT is stored in that address and driver should not refer
> +	 * to mailbox4 for getting VBT.
> +	 */
> +	if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) {
> +			size = opregion->asle->rvds;
> +			vbt_base = acpi_os_ioremap(opregion->asle->rvda,
> +					size);
> +	} else {
> +			size = OPREGION_SIZE - OPREGION_VBT_OFFSET;
> +			vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET,
> +					size);
> +	}
> +
> +	bdb = validate_vbt(vbt_base, size, "OpRegion");
> +
> +	if (bdb == NULL) {
> +		err = -EINVAL;
> +		goto err_vbt;
> +	}
> +
> +	dev_priv->bdb_start = bdb;
> +	opregion->vbt = vbt_base;
>  
>  	opregion->lid_state = base + ACPI_CLID;
>  
> @@ -953,6 +818,8 @@ int intel_opregion_setup(struct drm_device *dev)
>  
>  	return 0;
>  
> +err_vbt:
> +	iounmap(vbt_base);
>  err_out:
>  	iounmap(base);
>  	return err;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.h b/drivers/gpu/drm/i915/intel_opregion.h
> new file mode 100644
> index 0000000..bcb45ec
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_opregion.h
> @@ -0,0 +1,230 @@
> +/*
> + * Copyright 2008 Intel Corporation <[e-mail]>
> + * Copyright 2008 Red Hat <[e-mail]>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#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
> +#define   ACPI_CLID 0x01ac /* current lid state indicator */
> +#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> +#define OPREGION_SWSCI_OFFSET  0x200
> +#define OPREGION_ASLE_OFFSET   0x300
> +#define OPREGION_VBT_OFFSET    0x400
> +
> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> +#define MBOX_ACPI      (1<<0)
> +#define MBOX_SWSCI     (1<<1)
> +#define MBOX_ASLE      (1<<2)
> +#define MBOX_ASLE_EXT  (1<<4)
> +
> +struct opregion_header {
> +	u8 signature[16];
> +	u32 size;
> +	u32 opregion_ver;
> +	u8 bios_ver[32];
> +	u8 vbios_ver[16];
> +	u8 driver_ver[16];
> +	u32 mboxes;
> +	u32 driver_model;
> +	u32 pcon;
> +	u8 dver[32];
> +	u8 rsvd[124];
> +} __packed;
> +
> +/* OpRegion mailbox #1: public ACPI methods */
> +struct opregion_acpi {
> +	u32 drdy;       /* driver readiness */
> +	u32 csts;       /* notification status */
> +	u32 cevt;       /* current event */
> +	u8 rsvd1[20];
> +	u32 didl[8];    /* supported display devices ID list */
> +	u32 cpdl[8];    /* currently presented display list */
> +	u32 cadl[8];    /* currently active display list */
> +	u32 nadl[8];    /* next active devices list */
> +	u32 aslp;       /* ASL sleep time-out */
> +	u32 tidx;       /* toggle table index */
> +	u32 chpd;       /* current hotplug enable indicator */
> +	u32 clid;       /* current lid state*/
> +	u32 cdck;       /* current docking state */
> +	u32 sxsw;       /* Sx state resume */
> +	u32 evts;       /* ASL supported events */
> +	u32 cnot;       /* current OS notification */
> +	u32 nrdy;       /* driver status */
> +	u32 did2[7];	/* extended supported display devices ID list */
> +	u32 cpd2[7];	/* extended attached display devices list */
> +	u8 rsvd2[4];
> +} __packed;
> +
> +/* OpRegion mailbox #2: SWSCI */
> +struct opregion_swsci {
> +	u32 scic;       /* SWSCI command|status|data */
> +	u32 parm;       /* command parameters */
> +	u32 dslp;       /* driver sleep time-out */
> +	u8 rsvd[244];
> +} __packed;
> +
> +/* OpRegion mailbox #3: ASLE */
> +struct opregion_asle {
> +	u32 ardy;       /* driver readiness */
> +	u32 aslc;       /* ASLE interrupt command */
> +	u32 tche;       /* technology enabled indicator */
> +	u32 alsi;       /* current ALS illuminance reading */
> +	u32 bclp;       /* backlight brightness to set */
> +	u32 pfit;       /* panel fitting state */
> +	u32 cblv;       /* current brightness level */
> +	u16 bclm[20];   /* backlight level duty cycle mapping table */
> +	u32 cpfm;       /* current panel fitting mode */
> +	u32 epfm;       /* enabled panel fitting modes */
> +	u8 plut[74];    /* panel LUT and identifier */
> +	u32 pfmb;       /* PWM freq and min brightness */
> +	u32 cddv;       /* color correction default values */
> +	u32 pcft;       /* power conservation features */
> +	u32 srot;       /* supported rotation angles */
> +	u32 iuer;       /* IUER events */
> +	u64 fdss;
> +	u32 fdsp;
> +	u32 stat;
> +	u64 rvda;	/* Physical address of raw vbt data */
> +	u32 rvds;	/* Size of raw vbt data */
> +	u8 rsvd[58];
> +} __packed;
> +
> +/* Driver readiness indicator */
> +#define ASLE_ARDY_READY		(1 << 0)
> +#define ASLE_ARDY_NOT_READY	(0 << 0)
> +
> +/* ASLE Interrupt Command (ASLC) bits */
> +#define ASLC_SET_ALS_ILLUM		(1 << 0)
> +#define ASLC_SET_BACKLIGHT		(1 << 1)
> +#define ASLC_SET_PFIT			(1 << 2)
> +#define ASLC_SET_PWM_FREQ		(1 << 3)
> +#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
> +#define ASLC_BUTTON_ARRAY		(1 << 5)
> +#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
> +#define ASLC_DOCKING_INDICATOR		(1 << 7)
> +#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
> +#define ASLC_REQ_MSK			0x1ff
> +/* response bits */
> +#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
> +#define ASLC_BACKLIGHT_FAILED		(1 << 12)
> +#define ASLC_PFIT_FAILED		(1 << 14)
> +#define ASLC_PWM_FREQ_FAILED		(1 << 16)
> +#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
> +#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
> +#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
> +#define ASLC_DOCKING_FAILED		(1 << 24)
> +#define ASLC_ISCT_STATE_FAILED		(1 << 26)
> +
> +/* Technology enabled indicator */
> +#define ASLE_TCHE_ALS_EN	(1 << 0)
> +#define ASLE_TCHE_BLC_EN	(1 << 1)
> +#define ASLE_TCHE_PFIT_EN	(1 << 2)
> +#define ASLE_TCHE_PFMB_EN	(1 << 3)
> +
> +/* ASLE backlight brightness to set */
> +#define ASLE_BCLP_VALID                (1<<31)
> +#define ASLE_BCLP_MSK          (~(1<<31))
> +
> +/* ASLE panel fitting request */
> +#define ASLE_PFIT_VALID         (1<<31)
> +#define ASLE_PFIT_CENTER (1<<0)
> +#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> +#define ASLE_PFIT_STRETCH_GFX (1<<2)
> +
> +/* PWM frequency and minimum brightness */
> +#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> +#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> +#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> +#define ASLE_PFMB_PWM_VALID (1<<31)
> +
> +#define ASLE_CBLV_VALID         (1<<31)
> +
> +/* IUER */
> +#define ASLE_IUER_DOCKING		(1 << 7)
> +#define ASLE_IUER_CONVERTIBLE		(1 << 6)
> +#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
> +#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
> +#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
> +#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
> +#define ASLE_IUER_POWER_BTN		(1 << 0)
> +
> +/* 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)
> +#define ACPI_DIGITAL_OUTPUT (3<<8)
> +#define ACPI_LVDS_OUTPUT (4<<8)
> +
> +#define MAX_DSLP	1500
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula Sept. 17, 2015, 12:10 p.m. UTC | #2

On Thu, 10 Sep 2015, Deepak M <[e-mail]> wrote:
> Currently the iomap for VBT works only if the size of the
> VBT is less than 6KB, but if the size of the VBT exceeds
> 6KB than the physical address and the size of the VBT to
> be iomapped is specified in the mailbox3 and is iomapped
> accordingly.
>
> v2: - Moving the validate_vbt to opregion file (Jani)
>     - Fix the i915_opregion() in debugfs (Jani)
>
> Signed-off-by: Deepak M <[e-mail]>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++-
>  drivers/gpu/drm/i915/i915_drv.h       |    4 +
>  drivers/gpu/drm/i915/intel_bios.c     |   49 +-----
>  drivers/gpu/drm/i915/intel_opregion.c |  279 +++++++++------------------------
>  drivers/gpu/drm/i915/intel_opregion.h |  230 +++++++++++++++++++++++++++
>  5 files changed, 329 insertions(+), 257 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_opregion.h
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 41629fa..5534aa2 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -26,6 +26,8 @@
>   *
>   */
>  
> +#include <linux/acpi.h>
> +#include <acpi/video.h>
>  #include <linux/seq_file.h>
>  #include <linux/circ_buf.h>
>  #include <linux/ctype.h>
> @@ -39,6 +41,7 @@
>  #include "intel_ringbuffer.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "intel_opregion.h"
>  
>  enum {
>  	ACTIVE_LIST,
> @@ -1832,7 +1835,7 @@ static int i915_opregion(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_opregion *opregion = &dev_priv->opregion;
> -	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
> +	void *data = kmalloc(OPREGION_VBT_OFFSET, GFP_KERNEL);
>  	int ret;
>  
>  	if (data == NULL)
> @@ -1843,12 +1846,25 @@ static int i915_opregion(struct seq_file *m, void *unused)
>  		goto out;
>  
>  	if (opregion->header) {
> -		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
> -		seq_write(m, data, OPREGION_SIZE);
> +		memcpy_fromio(data, opregion->header, OPREGION_VBT_OFFSET);
> +		seq_write(m, data, OPREGION_VBT_OFFSET);
> +		kfree(data);
> +		if (opregion->asle->rvda) {
> +			data = kmalloc(opregion->asle->rvds, GFP_KERNEL);
> +			memcpy_fromio(data,
> +				(const void __iomem *) opregion->asle->rvda,
> +				opregion->asle->rvds);
> +			seq_write(m, data, opregion->asle->rvds);
> +		} else {
> +			data = kmalloc(OPREGION_SIZE - OPREGION_VBT_OFFSET,
> +					GFP_KERNEL);
> +			memcpy_fromio(data, opregion->vbt,
> +					OPREGION_SIZE - OPREGION_VBT_OFFSET);
> +			seq_write(m, data, OPREGION_SIZE - OPREGION_VBT_OFFSET);
> +		}

If rvda != 0, this debugfs file no longer represents the opregion
contents. Mailboxes #4 and #5 are dropped from the output. BTW, what is
mailbox #4 expected to contain when rvda != 0? (I still don't have
access to the latest opregion spec version, so can't check what it
actually says.)

I am beginning to think we should leave "i915_opregion" debugfs file
intact, and add a new "i915_vbt" file that contains either mailbox #4 or
the data in rvda. This might be a cleaner approach.

See my comments below, and you'll see how this would be feasible.

>  	}
>  
>  	mutex_unlock(&dev->struct_mutex);
> -
>  out:
>  	kfree(data);
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1287007..507d57a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1780,6 +1780,7 @@ struct drm_i915_private {
>  	struct i915_hotplug hotplug;
>  	struct i915_fbc fbc;
>  	struct i915_drrs drrs;
> +	const struct bdb_header *bdb_start;

What is wrong with using dev_priv->opregion.vbt for this?

>  	struct intel_opregion opregion;
>  	struct intel_vbt_data vbt;
>  
> @@ -3306,6 +3307,9 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>  }
>  #endif
>  
> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
> +			size_t size, const char *source);
> +
>  /* intel_acpi.c */
>  #ifdef CONFIG_ACPI
>  extern void intel_register_dsm_handler(void);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 0bf0942..1932a86 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1227,49 +1227,6 @@ static const struct dmi_system_id intel_no_opregion_vbt[] = {
>  	{ }
>  };
>  
> -static const struct bdb_header *validate_vbt(const void __iomem *_base,
> -					     size_t size,
> -					     const void __iomem *_vbt,
> -					     const char *source)
> -{
> -	/*
> -	 * This is the one place where we explicitly discard the address space
> -	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
> -	 * From now on everything is based on 'base', and treated as regular
> -	 * memory.
> -	 */
> -	const void *base = (const void *) _base;
> -	size_t offset = _vbt - _base;
> -	const struct vbt_header *vbt = base + offset;
> -	const struct bdb_header *bdb;
> -
> -	if (offset + sizeof(struct vbt_header) > size) {
> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> -		return NULL;
> -	}
> -
> -	if (memcmp(vbt->signature, "$VBT", 4)) {
> -		DRM_DEBUG_DRIVER("VBT invalid signature\n");
> -		return NULL;
> -	}
> -
> -	offset += vbt->bdb_offset;
> -	if (offset + sizeof(struct bdb_header) > size) {
> -		DRM_DEBUG_DRIVER("BDB header incomplete\n");
> -		return NULL;
> -	}
> -
> -	bdb = base + offset;
> -	if (offset + bdb->bdb_size > size) {
> -		DRM_DEBUG_DRIVER("BDB incomplete\n");
> -		return NULL;
> -	}
> -
> -	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
> -		      source, vbt->signature);
> -	return bdb;
> -}
> -

Moving of this function should be a separate prep patch.

>  static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>  {
>  	const struct bdb_header *bdb = NULL;
> @@ -1278,7 +1235,7 @@ static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>  	/* Scour memory looking for the VBT signature. */
>  	for (i = 0; i + 4 < size; i++) {
>  		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
> -			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
> +			bdb = validate_vbt(bios + i, size - i, "PCI ROM");
>  			break;
>  		}
>  	}
> @@ -1308,10 +1265,8 @@ intel_parse_bios(struct drm_device *dev)
>  
>  	init_vbt_defaults(dev_priv);
>  
> -	/* XXX Should this validation be moved to intel_opregion.c? */
>  	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv->opregion.vbt)
> -		bdb = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE,
> -				   dev_priv->opregion.vbt, "OpRegion");
> +		bdb = dev_priv->bdb_start;

This should be dev_priv->opregion.vbt.

>  
>  	if (bdb == NULL) {
>  		size_t size;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index f46231f..3a43db9 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -32,210 +32,7 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include "intel_drv.h"
> -
> -#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
> -#define   ACPI_CLID 0x01ac /* current lid state indicator */
> -#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> -#define OPREGION_SWSCI_OFFSET  0x200
> -#define OPREGION_ASLE_OFFSET   0x300
> -#define OPREGION_VBT_OFFSET    0x400
> -
> -#define OPREGION_SIGNATURE "IntelGraphicsMem"
> -#define MBOX_ACPI      (1<<0)
> -#define MBOX_SWSCI     (1<<1)
> -#define MBOX_ASLE      (1<<2)
> -#define MBOX_ASLE_EXT  (1<<4)
> -
> -struct opregion_header {
> -	u8 signature[16];
> -	u32 size;
> -	u32 opregion_ver;
> -	u8 bios_ver[32];
> -	u8 vbios_ver[16];
> -	u8 driver_ver[16];
> -	u32 mboxes;
> -	u32 driver_model;
> -	u32 pcon;
> -	u8 dver[32];
> -	u8 rsvd[124];
> -} __packed;
> -
> -/* OpRegion mailbox #1: public ACPI methods */
> -struct opregion_acpi {
> -	u32 drdy;       /* driver readiness */
> -	u32 csts;       /* notification status */
> -	u32 cevt;       /* current event */
> -	u8 rsvd1[20];
> -	u32 didl[8];    /* supported display devices ID list */
> -	u32 cpdl[8];    /* currently presented display list */
> -	u32 cadl[8];    /* currently active display list */
> -	u32 nadl[8];    /* next active devices list */
> -	u32 aslp;       /* ASL sleep time-out */
> -	u32 tidx;       /* toggle table index */
> -	u32 chpd;       /* current hotplug enable indicator */
> -	u32 clid;       /* current lid state*/
> -	u32 cdck;       /* current docking state */
> -	u32 sxsw;       /* Sx state resume */
> -	u32 evts;       /* ASL supported events */
> -	u32 cnot;       /* current OS notification */
> -	u32 nrdy;       /* driver status */
> -	u32 did2[7];	/* extended supported display devices ID list */
> -	u32 cpd2[7];	/* extended attached display devices list */
> -	u8 rsvd2[4];
> -} __packed;
> -
> -/* OpRegion mailbox #2: SWSCI */
> -struct opregion_swsci {
> -	u32 scic;       /* SWSCI command|status|data */
> -	u32 parm;       /* command parameters */
> -	u32 dslp;       /* driver sleep time-out */
> -	u8 rsvd[244];
> -} __packed;
> -
> -/* OpRegion mailbox #3: ASLE */
> -struct opregion_asle {
> -	u32 ardy;       /* driver readiness */
> -	u32 aslc;       /* ASLE interrupt command */
> -	u32 tche;       /* technology enabled indicator */
> -	u32 alsi;       /* current ALS illuminance reading */
> -	u32 bclp;       /* backlight brightness to set */
> -	u32 pfit;       /* panel fitting state */
> -	u32 cblv;       /* current brightness level */
> -	u16 bclm[20];   /* backlight level duty cycle mapping table */
> -	u32 cpfm;       /* current panel fitting mode */
> -	u32 epfm;       /* enabled panel fitting modes */
> -	u8 plut[74];    /* panel LUT and identifier */
> -	u32 pfmb;       /* PWM freq and min brightness */
> -	u32 cddv;       /* color correction default values */
> -	u32 pcft;       /* power conservation features */
> -	u32 srot;       /* supported rotation angles */
> -	u32 iuer;       /* IUER events */
> -	u64 fdss;
> -	u32 fdsp;
> -	u32 stat;
> -	u64 rvda;	/* Physical address of raw vbt data */
> -	u32 rvds;	/* Size of raw vbt data */
> -	u8 rsvd[58];
> -} __packed;
> -
> -/* Driver readiness indicator */
> -#define ASLE_ARDY_READY		(1 << 0)
> -#define ASLE_ARDY_NOT_READY	(0 << 0)
> -
> -/* ASLE Interrupt Command (ASLC) bits */
> -#define ASLC_SET_ALS_ILLUM		(1 << 0)
> -#define ASLC_SET_BACKLIGHT		(1 << 1)
> -#define ASLC_SET_PFIT			(1 << 2)
> -#define ASLC_SET_PWM_FREQ		(1 << 3)
> -#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
> -#define ASLC_BUTTON_ARRAY		(1 << 5)
> -#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
> -#define ASLC_DOCKING_INDICATOR		(1 << 7)
> -#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
> -#define ASLC_REQ_MSK			0x1ff
> -/* response bits */
> -#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
> -#define ASLC_BACKLIGHT_FAILED		(1 << 12)
> -#define ASLC_PFIT_FAILED		(1 << 14)
> -#define ASLC_PWM_FREQ_FAILED		(1 << 16)
> -#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
> -#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
> -#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
> -#define ASLC_DOCKING_FAILED		(1 << 24)
> -#define ASLC_ISCT_STATE_FAILED		(1 << 26)
> -
> -/* Technology enabled indicator */
> -#define ASLE_TCHE_ALS_EN	(1 << 0)
> -#define ASLE_TCHE_BLC_EN	(1 << 1)
> -#define ASLE_TCHE_PFIT_EN	(1 << 2)
> -#define ASLE_TCHE_PFMB_EN	(1 << 3)
> -
> -/* ASLE backlight brightness to set */
> -#define ASLE_BCLP_VALID                (1<<31)
> -#define ASLE_BCLP_MSK          (~(1<<31))
> -
> -/* ASLE panel fitting request */
> -#define ASLE_PFIT_VALID         (1<<31)
> -#define ASLE_PFIT_CENTER (1<<0)
> -#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> -#define ASLE_PFIT_STRETCH_GFX (1<<2)
> -
> -/* PWM frequency and minimum brightness */
> -#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> -#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> -#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> -#define ASLE_PFMB_PWM_VALID (1<<31)
> -
> -#define ASLE_CBLV_VALID         (1<<31)
> -
> -/* IUER */
> -#define ASLE_IUER_DOCKING		(1 << 7)
> -#define ASLE_IUER_CONVERTIBLE		(1 << 6)
> -#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
> -#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
> -#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
> -#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
> -#define ASLE_IUER_POWER_BTN		(1 << 0)
> -
> -/* 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)
> -#define ACPI_DIGITAL_OUTPUT (3<<8)
> -#define ACPI_LVDS_OUTPUT (4<<8)
> -
> -#define MAX_DSLP	1500
> +#include "intel_opregion.h"

As said, I don't see the need to move these defines to a separate
header. This is definitely stuff that we want to keep hidden in one
place, and nobody outside of intel_opregion.c should use these.

>  
>  #ifdef CONFIG_ACPI
>  static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
> @@ -892,13 +689,55 @@ static void swsci_setup(struct drm_device *dev)
>  static inline void swsci_setup(struct drm_device *dev) {}
>  #endif  /* CONFIG_ACPI */
>  
> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
> +					size_t size,
> +					const char *source)
> +{
> +	/*
> +	 * This is the one place where we explicitly discard the address space
> +	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
> +	 */
> +	const struct vbt_header *vbt = (const struct vbt_header *)_vbt;
> +	const struct bdb_header *bdb;
> +	size_t offset;
> +
> +	if (sizeof(struct vbt_header) > size) {
> +		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> +		return NULL;
> +	}
> +
> +	if (memcmp(vbt->signature, "$VBT", 4)) {
> +		DRM_DEBUG_DRIVER("VBT invalid signature\n");
> +		return NULL;
> +	}
> +
> +	offset = vbt->bdb_offset;
> +	if (offset + sizeof(struct bdb_header) > size) {
> +		DRM_DEBUG_DRIVER("BDB header incomplete\n");
> +		return NULL;
> +	}
> +
> +	bdb = (const void *)_vbt + offset;
> +	if (offset + bdb->bdb_size > size) {
> +		DRM_DEBUG_DRIVER("BDB incomplete\n");
> +		return NULL;
> +	}
> +
> +	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
> +			source, vbt->signature);
> +	return bdb;
> +}
> +
>  int intel_opregion_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_opregion *opregion = &dev_priv->opregion;
>  	void __iomem *base;
> +	void __iomem *vbt_base;
>  	u32 asls, mboxes;
>  	char buf[sizeof(OPREGION_SIGNATURE)];
> +	const struct bdb_header *bdb = NULL;
> +	size_t size;
>  	int err = 0;
>  
>  	BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100);
> @@ -917,7 +756,7 @@ int intel_opregion_setup(struct drm_device *dev)
>  	INIT_WORK(&opregion->asle_work, asle_work);
>  #endif
>  
> -	base = acpi_os_ioremap(asls, OPREGION_SIZE);
> +	base = acpi_os_ioremap(asls, OPREGION_VBT_OFFSET);

Now you leave out mailbox #5. I don't think there's any reason *not* to
ioremap all of opregion here.

>  	if (!base)
>  		return -ENOMEM;
>  
> @@ -929,7 +768,33 @@ int intel_opregion_setup(struct drm_device *dev)
>  		goto err_out;
>  	}
>  	opregion->header = base;
> -	opregion->vbt = base + OPREGION_VBT_OFFSET;
> +	/* Assigning the alse to the mailbox 3 of the opregion */
> +	opregion->asle = base + OPREGION_ASLE_OFFSET;

That's assigned towards the end of the function *if* the mbox is
supported.

> +
> +	/*
> +	 * Non-zero value in rvda field is an indication to driver that a
> +	 * valid Raw VBT is stored in that address and driver should not refer
> +	 * to mailbox4 for getting VBT.
> +	 */
> +	if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) {
> +			size = opregion->asle->rvds;
> +			vbt_base = acpi_os_ioremap(opregion->asle->rvda,
> +					size);
> +	} else {
> +			size = OPREGION_SIZE - OPREGION_VBT_OFFSET;
> +			vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET,
> +					size);
> +	}
> +
> +	bdb = validate_vbt(vbt_base, size, "OpRegion");
> +
> +	if (bdb == NULL) {
> +		err = -EINVAL;
> +		goto err_vbt;
> +	}
> +
> +	dev_priv->bdb_start = bdb;

Again, I don't see why you should store this here. Nor I see the need to
actually validate vbt here. You can just as well do it in intel_bios
like before, as long as you assign opregion->vbt here appropriately.

You'll also need to iounmap vbt_base in intel_opregion_fini. That may
need some additional checks if you unconditionally ioremap all of
opregion and conditionally ioremap rvda, and point opregion->vbt to it.

> +	opregion->vbt = vbt_base;
>  
>  	opregion->lid_state = base + ACPI_CLID;
>  
> @@ -953,6 +818,8 @@ int intel_opregion_setup(struct drm_device *dev)
>  
>  	return 0;
>  
> +err_vbt:
> +	iounmap(vbt_base);
>  err_out:
>  	iounmap(base);
>  	return err;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.h b/drivers/gpu/drm/i915/intel_opregion.h
> new file mode 100644
> index 0000000..bcb45ec
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_opregion.h
> @@ -0,0 +1,230 @@
> +/*
> + * Copyright 2008 Intel Corporation <[e-mail]>
> + * Copyright 2008 Red Hat <[e-mail]>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#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
> +#define   ACPI_CLID 0x01ac /* current lid state indicator */
> +#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
> +#define OPREGION_SWSCI_OFFSET  0x200
> +#define OPREGION_ASLE_OFFSET   0x300
> +#define OPREGION_VBT_OFFSET    0x400
> +
> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> +#define MBOX_ACPI      (1<<0)
> +#define MBOX_SWSCI     (1<<1)
> +#define MBOX_ASLE      (1<<2)
> +#define MBOX_ASLE_EXT  (1<<4)
> +
> +struct opregion_header {
> +	u8 signature[16];
> +	u32 size;
> +	u32 opregion_ver;
> +	u8 bios_ver[32];
> +	u8 vbios_ver[16];
> +	u8 driver_ver[16];
> +	u32 mboxes;
> +	u32 driver_model;
> +	u32 pcon;
> +	u8 dver[32];
> +	u8 rsvd[124];
> +} __packed;
> +
> +/* OpRegion mailbox #1: public ACPI methods */
> +struct opregion_acpi {
> +	u32 drdy;       /* driver readiness */
> +	u32 csts;       /* notification status */
> +	u32 cevt;       /* current event */
> +	u8 rsvd1[20];
> +	u32 didl[8];    /* supported display devices ID list */
> +	u32 cpdl[8];    /* currently presented display list */
> +	u32 cadl[8];    /* currently active display list */
> +	u32 nadl[8];    /* next active devices list */
> +	u32 aslp;       /* ASL sleep time-out */
> +	u32 tidx;       /* toggle table index */
> +	u32 chpd;       /* current hotplug enable indicator */
> +	u32 clid;       /* current lid state*/
> +	u32 cdck;       /* current docking state */
> +	u32 sxsw;       /* Sx state resume */
> +	u32 evts;       /* ASL supported events */
> +	u32 cnot;       /* current OS notification */
> +	u32 nrdy;       /* driver status */
> +	u32 did2[7];	/* extended supported display devices ID list */
> +	u32 cpd2[7];	/* extended attached display devices list */
> +	u8 rsvd2[4];
> +} __packed;
> +
> +/* OpRegion mailbox #2: SWSCI */
> +struct opregion_swsci {
> +	u32 scic;       /* SWSCI command|status|data */
> +	u32 parm;       /* command parameters */
> +	u32 dslp;       /* driver sleep time-out */
> +	u8 rsvd[244];
> +} __packed;
> +
> +/* OpRegion mailbox #3: ASLE */
> +struct opregion_asle {
> +	u32 ardy;       /* driver readiness */
> +	u32 aslc;       /* ASLE interrupt command */
> +	u32 tche;       /* technology enabled indicator */
> +	u32 alsi;       /* current ALS illuminance reading */
> +	u32 bclp;       /* backlight brightness to set */
> +	u32 pfit;       /* panel fitting state */
> +	u32 cblv;       /* current brightness level */
> +	u16 bclm[20];   /* backlight level duty cycle mapping table */
> +	u32 cpfm;       /* current panel fitting mode */
> +	u32 epfm;       /* enabled panel fitting modes */
> +	u8 plut[74];    /* panel LUT and identifier */
> +	u32 pfmb;       /* PWM freq and min brightness */
> +	u32 cddv;       /* color correction default values */
> +	u32 pcft;       /* power conservation features */
> +	u32 srot;       /* supported rotation angles */
> +	u32 iuer;       /* IUER events */
> +	u64 fdss;
> +	u32 fdsp;
> +	u32 stat;
> +	u64 rvda;	/* Physical address of raw vbt data */
> +	u32 rvds;	/* Size of raw vbt data */
> +	u8 rsvd[58];
> +} __packed;
> +
> +/* Driver readiness indicator */
> +#define ASLE_ARDY_READY		(1 << 0)
> +#define ASLE_ARDY_NOT_READY	(0 << 0)
> +
> +/* ASLE Interrupt Command (ASLC) bits */
> +#define ASLC_SET_ALS_ILLUM		(1 << 0)
> +#define ASLC_SET_BACKLIGHT		(1 << 1)
> +#define ASLC_SET_PFIT			(1 << 2)
> +#define ASLC_SET_PWM_FREQ		(1 << 3)
> +#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
> +#define ASLC_BUTTON_ARRAY		(1 << 5)
> +#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
> +#define ASLC_DOCKING_INDICATOR		(1 << 7)
> +#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
> +#define ASLC_REQ_MSK			0x1ff
> +/* response bits */
> +#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
> +#define ASLC_BACKLIGHT_FAILED		(1 << 12)
> +#define ASLC_PFIT_FAILED		(1 << 14)
> +#define ASLC_PWM_FREQ_FAILED		(1 << 16)
> +#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
> +#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
> +#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
> +#define ASLC_DOCKING_FAILED		(1 << 24)
> +#define ASLC_ISCT_STATE_FAILED		(1 << 26)
> +
> +/* Technology enabled indicator */
> +#define ASLE_TCHE_ALS_EN	(1 << 0)
> +#define ASLE_TCHE_BLC_EN	(1 << 1)
> +#define ASLE_TCHE_PFIT_EN	(1 << 2)
> +#define ASLE_TCHE_PFMB_EN	(1 << 3)
> +
> +/* ASLE backlight brightness to set */
> +#define ASLE_BCLP_VALID                (1<<31)
> +#define ASLE_BCLP_MSK          (~(1<<31))
> +
> +/* ASLE panel fitting request */
> +#define ASLE_PFIT_VALID         (1<<31)
> +#define ASLE_PFIT_CENTER (1<<0)
> +#define ASLE_PFIT_STRETCH_TEXT (1<<1)
> +#define ASLE_PFIT_STRETCH_GFX (1<<2)
> +
> +/* PWM frequency and minimum brightness */
> +#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
> +#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
> +#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
> +#define ASLE_PFMB_PWM_VALID (1<<31)
> +
> +#define ASLE_CBLV_VALID         (1<<31)
> +
> +/* IUER */
> +#define ASLE_IUER_DOCKING		(1 << 7)
> +#define ASLE_IUER_CONVERTIBLE		(1 << 6)
> +#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
> +#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
> +#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
> +#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
> +#define ASLE_IUER_POWER_BTN		(1 << 0)
> +
> +/* 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)
> +#define ACPI_DIGITAL_OUTPUT (3<<8)
> +#define ACPI_LVDS_OUTPUT (4<<8)
> +
> +#define MAX_DSLP	1500

The bottom line here is that I think you could get all of this done with
*much* smaller changes. If we get a regression report pointing at this
patch, we'll have a lot of debugging to do to figure out what failed.

BR,
Jani.



> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Deepak M Sept. 22, 2015, 6:37 a.m. UTC | #3

>-----Original Message-----
>From: Jani Nikula [mailto:[e-mail]]
>Sent: Thursday, September 17, 2015 5:41 PM
>To: Deepak, M; [e-mail]
>Cc: Deepak, M
>Subject: Re: [Intel-gfx] [MIPI SEQ PARSING v2 PATCH 03/11] drm/i915: Parsing
>VBT if size of VBT exceeds 6KB
>
>On Thu, 10 Sep 2015, Deepak M <[e-mail]> wrote:
>> Currently the iomap for VBT works only if the size of the VBT is less
>> than 6KB, but if the size of the VBT exceeds 6KB than the physical
>> address and the size of the VBT to be iomapped is specified in the
>> mailbox3 and is iomapped accordingly.
>>
>> v2: - Moving the validate_vbt to opregion file (Jani)
>>     - Fix the i915_opregion() in debugfs (Jani)
>>
>> Signed-off-by: Deepak M <[e-mail]>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++-
>>  drivers/gpu/drm/i915/i915_drv.h       |    4 +
>>  drivers/gpu/drm/i915/intel_bios.c     |   49 +-----
>>  drivers/gpu/drm/i915/intel_opregion.c |  279
>> +++++++++------------------------
>> drivers/gpu/drm/i915/intel_opregion.h |  230
>> +++++++++++++++++++++++++++
>>  5 files changed, 329 insertions(+), 257 deletions(-)  create mode
>> 100644 drivers/gpu/drm/i915/intel_opregion.h
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 41629fa..5534aa2 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -26,6 +26,8 @@
>>   *
>>   */
>>
>> +#include <linux/acpi.h>
>> +#include <acpi/video.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/circ_buf.h>
>>  #include <linux/ctype.h>
>> @@ -39,6 +41,7 @@
>>  #include "intel_ringbuffer.h"
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>> +#include "intel_opregion.h"
>>
>>  enum {
>>  	ACTIVE_LIST,
>> @@ -1832,7 +1835,7 @@ static int i915_opregion(struct seq_file *m, void
>*unused)
>>  	struct drm_device *dev = node->minor->dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_opregion *opregion = &dev_priv->opregion;
>> -	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
>> +	void *data = kmalloc(OPREGION_VBT_OFFSET, GFP_KERNEL);
>>  	int ret;
>>
>>  	if (data == NULL)
>> @@ -1843,12 +1846,25 @@ static int i915_opregion(struct seq_file *m, void
>*unused)
>>  		goto out;
>>
>>  	if (opregion->header) {
>> -		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
>> -		seq_write(m, data, OPREGION_SIZE);
>> +		memcpy_fromio(data, opregion->header,
>OPREGION_VBT_OFFSET);
>> +		seq_write(m, data, OPREGION_VBT_OFFSET);
>> +		kfree(data);
>> +		if (opregion->asle->rvda) {
>> +			data = kmalloc(opregion->asle->rvds, GFP_KERNEL);
>> +			memcpy_fromio(data,
>> +				(const void __iomem *) opregion->asle->rvda,
>> +				opregion->asle->rvds);
>> +			seq_write(m, data, opregion->asle->rvds);
>> +		} else {
>> +			data = kmalloc(OPREGION_SIZE -
>OPREGION_VBT_OFFSET,
>> +					GFP_KERNEL);
>> +			memcpy_fromio(data, opregion->vbt,
>> +					OPREGION_SIZE -
>OPREGION_VBT_OFFSET);
>> +			seq_write(m, data, OPREGION_SIZE -
>OPREGION_VBT_OFFSET);
>> +		}
>
>If rvda != 0, this debugfs file no longer represents the opregion contents.
>Mailboxes #4 and #5 are dropped from the output. BTW, what is mailbox #4
>expected to contain when rvda != 0? (I still don't have access to the latest
>opregion spec version, so can't check what it actually says.)
>
>I am beginning to think we should leave "i915_opregion" debugfs file intact,
>and add a new "i915_vbt" file that contains either mailbox #4 or the data in
>rvda. This might be a cleaner approach.
>
>See my comments below, and you'll see how this would be feasible.
>
[Deepak M] I was thinking of splitting this function into 5 for dumping each mailbox. Which 
I felt will be cleaner.
>>  	}
>>
>>  	mutex_unlock(&dev->struct_mutex);
>> -
>>  out:
>>  	kfree(data);
>>  	return 0;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h index 1287007..507d57a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1780,6 +1780,7 @@ struct drm_i915_private {
>>  	struct i915_hotplug hotplug;
>>  	struct i915_fbc fbc;
>>  	struct i915_drrs drrs;
>> +	const struct bdb_header *bdb_start;
>
>What is wrong with using dev_priv->opregion.vbt for this?
>
>>  	struct intel_opregion opregion;
>>  	struct intel_vbt_data vbt;
>>
>> @@ -3306,6 +3307,9 @@ intel_opregion_notify_adapter(struct drm_device
>> *dev, pci_power_t state)  }  #endif
>>
>> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
>> +			size_t size, const char *source);
>> +
>>  /* intel_acpi.c */
>>  #ifdef CONFIG_ACPI
>>  extern void intel_register_dsm_handler(void); diff --git
>> a/drivers/gpu/drm/i915/intel_bios.c
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 0bf0942..1932a86 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1227,49 +1227,6 @@ static const struct dmi_system_id
>intel_no_opregion_vbt[] = {
>>  	{ }
>>  };
>>
>> -static const struct bdb_header *validate_vbt(const void __iomem *_base,
>> -					     size_t size,
>> -					     const void __iomem *_vbt,
>> -					     const char *source)
>> -{
>> -	/*
>> -	 * This is the one place where we explicitly discard the address space
>> -	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse
>complaint.)
>> -	 * From now on everything is based on 'base', and treated as regular
>> -	 * memory.
>> -	 */
>> -	const void *base = (const void *) _base;
>> -	size_t offset = _vbt - _base;
>> -	const struct vbt_header *vbt = base + offset;
>> -	const struct bdb_header *bdb;
>> -
>> -	if (offset + sizeof(struct vbt_header) > size) {
>> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>> -		return NULL;
>> -	}
>> -
>> -	if (memcmp(vbt->signature, "$VBT", 4)) {
>> -		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>> -		return NULL;
>> -	}
>> -
>> -	offset += vbt->bdb_offset;
>> -	if (offset + sizeof(struct bdb_header) > size) {
>> -		DRM_DEBUG_DRIVER("BDB header incomplete\n");
>> -		return NULL;
>> -	}
>> -
>> -	bdb = base + offset;
>> -	if (offset + bdb->bdb_size > size) {
>> -		DRM_DEBUG_DRIVER("BDB incomplete\n");
>> -		return NULL;
>> -	}
>> -
>> -	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
>> -		      source, vbt->signature);
>> -	return bdb;
>> -}
>> -
>
>Moving of this function should be a separate prep patch.
>
>>  static const struct bdb_header *find_vbt(void __iomem *bios, size_t
>> size)  {
>>  	const struct bdb_header *bdb = NULL; @@ -1278,7 +1235,7 @@ static
>> const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>>  	/* Scour memory looking for the VBT signature. */
>>  	for (i = 0; i + 4 < size; i++) {
>>  		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
>> -			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
>> +			bdb = validate_vbt(bios + i, size - i, "PCI ROM");
>>  			break;
>>  		}
>>  	}
>> @@ -1308,10 +1265,8 @@ intel_parse_bios(struct drm_device *dev)
>>
>>  	init_vbt_defaults(dev_priv);
>>
>> -	/* XXX Should this validation be moved to intel_opregion.c? */
>>  	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv-
>>opregion.vbt)
>> -		bdb = validate_vbt(dev_priv->opregion.header,
>OPREGION_SIZE,
>> -				   dev_priv->opregion.vbt, "OpRegion");
>> +		bdb = dev_priv->bdb_start;
>
>This should be dev_priv->opregion.vbt.
[Deepak M] validate_vbt function always returns the bdb header and our parsing starts from the bdb_header and not form the dev_priv->opregion.vbt where the VBT header starts. So I have added bdb_start in the dev_priv structure.
>
>>
>>  	if (bdb == NULL) {
>>  		size_t size;
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
>> b/drivers/gpu/drm/i915/intel_opregion.c
>> index f46231f..3a43db9 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -32,210 +32,7 @@
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>>  #include "intel_drv.h"
>> -
>> -#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
>> -#define   ACPI_CLID 0x01ac /* current lid state indicator */
>> -#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
>> -#define OPREGION_SWSCI_OFFSET  0x200
>> -#define OPREGION_ASLE_OFFSET   0x300
>> -#define OPREGION_VBT_OFFSET    0x400
>> -
>> -#define OPREGION_SIGNATURE "IntelGraphicsMem"
>> -#define MBOX_ACPI      (1<<0)
>> -#define MBOX_SWSCI     (1<<1)
>> -#define MBOX_ASLE      (1<<2)
>> -#define MBOX_ASLE_EXT  (1<<4)
>> -
>> -struct opregion_header {
>> -	u8 signature[16];
>> -	u32 size;
>> -	u32 opregion_ver;
>> -	u8 bios_ver[32];
>> -	u8 vbios_ver[16];
>> -	u8 driver_ver[16];
>> -	u32 mboxes;
>> -	u32 driver_model;
>> -	u32 pcon;
>> -	u8 dver[32];
>> -	u8 rsvd[124];
>> -} __packed;
>> -
>> -/* OpRegion mailbox #1: public ACPI methods */ -struct opregion_acpi
>> {
>> -	u32 drdy;       /* driver readiness */
>> -	u32 csts;       /* notification status */
>> -	u32 cevt;       /* current event */
>> -	u8 rsvd1[20];
>> -	u32 didl[8];    /* supported display devices ID list */
>> -	u32 cpdl[8];    /* currently presented display list */
>> -	u32 cadl[8];    /* currently active display list */
>> -	u32 nadl[8];    /* next active devices list */
>> -	u32 aslp;       /* ASL sleep time-out */
>> -	u32 tidx;       /* toggle table index */
>> -	u32 chpd;       /* current hotplug enable indicator */
>> -	u32 clid;       /* current lid state*/
>> -	u32 cdck;       /* current docking state */
>> -	u32 sxsw;       /* Sx state resume */
>> -	u32 evts;       /* ASL supported events */
>> -	u32 cnot;       /* current OS notification */
>> -	u32 nrdy;       /* driver status */
>> -	u32 did2[7];	/* extended supported display devices ID list */
>> -	u32 cpd2[7];	/* extended attached display devices list */
>> -	u8 rsvd2[4];
>> -} __packed;
>> -
>> -/* OpRegion mailbox #2: SWSCI */
>> -struct opregion_swsci {
>> -	u32 scic;       /* SWSCI command|status|data */
>> -	u32 parm;       /* command parameters */
>> -	u32 dslp;       /* driver sleep time-out */
>> -	u8 rsvd[244];
>> -} __packed;
>> -
>> -/* OpRegion mailbox #3: ASLE */
>> -struct opregion_asle {
>> -	u32 ardy;       /* driver readiness */
>> -	u32 aslc;       /* ASLE interrupt command */
>> -	u32 tche;       /* technology enabled indicator */
>> -	u32 alsi;       /* current ALS illuminance reading */
>> -	u32 bclp;       /* backlight brightness to set */
>> -	u32 pfit;       /* panel fitting state */
>> -	u32 cblv;       /* current brightness level */
>> -	u16 bclm[20];   /* backlight level duty cycle mapping table */
>> -	u32 cpfm;       /* current panel fitting mode */
>> -	u32 epfm;       /* enabled panel fitting modes */
>> -	u8 plut[74];    /* panel LUT and identifier */
>> -	u32 pfmb;       /* PWM freq and min brightness */
>> -	u32 cddv;       /* color correction default values */
>> -	u32 pcft;       /* power conservation features */
>> -	u32 srot;       /* supported rotation angles */
>> -	u32 iuer;       /* IUER events */
>> -	u64 fdss;
>> -	u32 fdsp;
>> -	u32 stat;
>> -	u64 rvda;	/* Physical address of raw vbt data */
>> -	u32 rvds;	/* Size of raw vbt data */
>> -	u8 rsvd[58];
>> -} __packed;
>> -
>> -/* Driver readiness indicator */
>> -#define ASLE_ARDY_READY		(1 << 0)
>> -#define ASLE_ARDY_NOT_READY	(0 << 0)
>> -
>> -/* ASLE Interrupt Command (ASLC) bits */
>> -#define ASLC_SET_ALS_ILLUM		(1 << 0)
>> -#define ASLC_SET_BACKLIGHT		(1 << 1)
>> -#define ASLC_SET_PFIT			(1 << 2)
>> -#define ASLC_SET_PWM_FREQ		(1 << 3)
>> -#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
>> -#define ASLC_BUTTON_ARRAY		(1 << 5)
>> -#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
>> -#define ASLC_DOCKING_INDICATOR		(1 << 7)
>> -#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
>> -#define ASLC_REQ_MSK			0x1ff
>> -/* response bits */
>> -#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
>> -#define ASLC_BACKLIGHT_FAILED		(1 << 12)
>> -#define ASLC_PFIT_FAILED		(1 << 14)
>> -#define ASLC_PWM_FREQ_FAILED		(1 << 16)
>> -#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
>> -#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
>> -#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
>> -#define ASLC_DOCKING_FAILED		(1 << 24)
>> -#define ASLC_ISCT_STATE_FAILED		(1 << 26)
>> -
>> -/* Technology enabled indicator */
>> -#define ASLE_TCHE_ALS_EN	(1 << 0)
>> -#define ASLE_TCHE_BLC_EN	(1 << 1)
>> -#define ASLE_TCHE_PFIT_EN	(1 << 2)
>> -#define ASLE_TCHE_PFMB_EN	(1 << 3)
>> -
>> -/* ASLE backlight brightness to set */
>> -#define ASLE_BCLP_VALID                (1<<31)
>> -#define ASLE_BCLP_MSK          (~(1<<31))
>> -
>> -/* ASLE panel fitting request */
>> -#define ASLE_PFIT_VALID         (1<<31)
>> -#define ASLE_PFIT_CENTER (1<<0)
>> -#define ASLE_PFIT_STRETCH_TEXT (1<<1) -#define
>ASLE_PFIT_STRETCH_GFX
>> (1<<2)
>> -
>> -/* PWM frequency and minimum brightness */ -#define
>> ASLE_PFMB_BRIGHTNESS_MASK (0xff) -#define
>ASLE_PFMB_BRIGHTNESS_VALID
>> (1<<8) -#define ASLE_PFMB_PWM_MASK (0x7ffffe00) -#define
>> ASLE_PFMB_PWM_VALID (1<<31)
>> -
>> -#define ASLE_CBLV_VALID         (1<<31)
>> -
>> -/* IUER */
>> -#define ASLE_IUER_DOCKING		(1 << 7)
>> -#define ASLE_IUER_CONVERTIBLE		(1 << 6)
>> -#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
>> -#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
>> -#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
>> -#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>> -#define ASLE_IUER_POWER_BTN		(1 << 0)
>> -
>> -/* 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)
>> -#define ACPI_DIGITAL_OUTPUT (3<<8)
>> -#define ACPI_LVDS_OUTPUT (4<<8)
>> -
>> -#define MAX_DSLP	1500
>> +#include "intel_opregion.h"
>
>As said, I don't see the need to move these defines to a separate header. This
>is definitely stuff that we want to keep hidden in one place, and nobody
>outside of intel_opregion.c should use these.
>
[Deepak M] Need some of these definitions in the debugfs.c file, because of this
had created a new file for macros.

>>
>>  #ifdef CONFIG_ACPI
>>  static int swsci(struct drm_device *dev, u32 function, u32 parm, u32
>> *parm_out) @@ -892,13 +689,55 @@ static void swsci_setup(struct
>> drm_device *dev)  static inline void swsci_setup(struct drm_device
>> *dev) {}  #endif  /* CONFIG_ACPI */
>>
>> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
>> +					size_t size,
>> +					const char *source)
>> +{
>> +	/*
>> +	 * This is the one place where we explicitly discard the address space
>> +	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse
>complaint.)
>> +	 */
>> +	const struct vbt_header *vbt = (const struct vbt_header *)_vbt;
>> +	const struct bdb_header *bdb;
>> +	size_t offset;
>> +
>> +	if (sizeof(struct vbt_header) > size) {
>> +		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>> +		return NULL;
>> +	}
>> +
>> +	if (memcmp(vbt->signature, "$VBT", 4)) {
>> +		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>> +		return NULL;
>> +	}
>> +
>> +	offset = vbt->bdb_offset;
>> +	if (offset + sizeof(struct bdb_header) > size) {
>> +		DRM_DEBUG_DRIVER("BDB header incomplete\n");
>> +		return NULL;
>> +	}
>> +
>> +	bdb = (const void *)_vbt + offset;
>> +	if (offset + bdb->bdb_size > size) {
>> +		DRM_DEBUG_DRIVER("BDB incomplete\n");
>> +		return NULL;
>> +	}
>> +
>> +	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
>> +			source, vbt->signature);
>> +	return bdb;
>> +}
>> +
>>  int intel_opregion_setup(struct drm_device *dev)  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	struct intel_opregion *opregion = &dev_priv->opregion;
>>  	void __iomem *base;
>> +	void __iomem *vbt_base;
>>  	u32 asls, mboxes;
>>  	char buf[sizeof(OPREGION_SIGNATURE)];
>> +	const struct bdb_header *bdb = NULL;
>> +	size_t size;
>>  	int err = 0;
>>
>>  	BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100); @@ -
>917,7
>> +756,7 @@ int intel_opregion_setup(struct drm_device *dev)
>>  	INIT_WORK(&opregion->asle_work, asle_work);  #endif
>>
>> -	base = acpi_os_ioremap(asls, OPREGION_SIZE);
>> +	base = acpi_os_ioremap(asls, OPREGION_VBT_OFFSET);
>
>Now you leave out mailbox #5. I don't think there's any reason *not* to
>ioremap all of opregion here.
>
>>  	if (!base)
>>  		return -ENOMEM;
>>
>> @@ -929,7 +768,33 @@ int intel_opregion_setup(struct drm_device *dev)
>>  		goto err_out;
>>  	}
>>  	opregion->header = base;
>> -	opregion->vbt = base + OPREGION_VBT_OFFSET;
>> +	/* Assigning the alse to the mailbox 3 of the opregion */
>> +	opregion->asle = base + OPREGION_ASLE_OFFSET;
>
>That's assigned towards the end of the function *if* the mbox is supported.
>
>> +
>> +	/*
>> +	 * Non-zero value in rvda field is an indication to driver that a
>> +	 * valid Raw VBT is stored in that address and driver should not refer
>> +	 * to mailbox4 for getting VBT.
>> +	 */
>> +	if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) {
>> +			size = opregion->asle->rvds;
>> +			vbt_base = acpi_os_ioremap(opregion->asle->rvda,
>> +					size);
>> +	} else {
>> +			size = OPREGION_SIZE - OPREGION_VBT_OFFSET;
>> +			vbt_base = acpi_os_ioremap(asls +
>OPREGION_VBT_OFFSET,
>> +					size);
>> +	}
>> +
>> +	bdb = validate_vbt(vbt_base, size, "OpRegion");
>> +
>> +	if (bdb == NULL) {
>> +		err = -EINVAL;
>> +		goto err_vbt;
>> +	}
>> +
>> +	dev_priv->bdb_start = bdb;
>
>Again, I don't see why you should store this here. Nor I see the need to
>actually validate vbt here. You can just as well do it in intel_bios like before, as
>long as you assign opregion->vbt here appropriately.
>
>You'll also need to iounmap vbt_base in intel_opregion_fini. That may need
>some additional checks if you unconditionally ioremap all of opregion and
>conditionally ioremap rvda, and point opregion->vbt to it.
>
[Deepak M] Okay will handle this.
>> +	opregion->vbt = vbt_base;
>>
>>  	opregion->lid_state = base + ACPI_CLID;
>>
>> @@ -953,6 +818,8 @@ int intel_opregion_setup(struct drm_device *dev)
>>
>>  	return 0;
>>
>> +err_vbt:
>> +	iounmap(vbt_base);
>>  err_out:
>>  	iounmap(base);
>>  	return err;
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.h
>> b/drivers/gpu/drm/i915/intel_opregion.h
>> new file mode 100644
>> index 0000000..bcb45ec
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_opregion.h
>> @@ -0,0 +1,230 @@
>> +/*
>> + * Copyright 2008 Intel Corporation <[e-mail]>
>> + * Copyright 2008 Red Hat <[e-mail]>
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining
>> + * a copy of this software and associated documentation files (the
>> + * "Software"), to deal in the Software without restriction,
>> +including
>> + * without limitation the rights to use, copy, modify, merge,
>> +publish,
>> + * distribute, sub license, and/or sell copies of the Software, and
>> +to
>> + * permit persons to whom the Software is furnished to do so, subject
>> +to
>> + * the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> +the
>> + * next paragraph) shall be included in all copies or substantial
>> + * portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
>KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
>WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS
>BE
>> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
>OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>THE
>> + * SOFTWARE.
>> + *
>> + */
>> +
>> +#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
>> +#define   ACPI_CLID 0x01ac /* current lid state indicator */
>> +#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
>> +#define OPREGION_SWSCI_OFFSET  0x200
>> +#define OPREGION_ASLE_OFFSET   0x300
>> +#define OPREGION_VBT_OFFSET    0x400
>> +
>> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
>> +#define MBOX_ACPI      (1<<0)
>> +#define MBOX_SWSCI     (1<<1)
>> +#define MBOX_ASLE      (1<<2)
>> +#define MBOX_ASLE_EXT  (1<<4)
>> +
>> +struct opregion_header {
>> +	u8 signature[16];
>> +	u32 size;
>> +	u32 opregion_ver;
>> +	u8 bios_ver[32];
>> +	u8 vbios_ver[16];
>> +	u8 driver_ver[16];
>> +	u32 mboxes;
>> +	u32 driver_model;
>> +	u32 pcon;
>> +	u8 dver[32];
>> +	u8 rsvd[124];
>> +} __packed;
>> +
>> +/* OpRegion mailbox #1: public ACPI methods */ struct opregion_acpi {
>> +	u32 drdy;       /* driver readiness */
>> +	u32 csts;       /* notification status */
>> +	u32 cevt;       /* current event */
>> +	u8 rsvd1[20];
>> +	u32 didl[8];    /* supported display devices ID list */
>> +	u32 cpdl[8];    /* currently presented display list */
>> +	u32 cadl[8];    /* currently active display list */
>> +	u32 nadl[8];    /* next active devices list */
>> +	u32 aslp;       /* ASL sleep time-out */
>> +	u32 tidx;       /* toggle table index */
>> +	u32 chpd;       /* current hotplug enable indicator */
>> +	u32 clid;       /* current lid state*/
>> +	u32 cdck;       /* current docking state */
>> +	u32 sxsw;       /* Sx state resume */
>> +	u32 evts;       /* ASL supported events */
>> +	u32 cnot;       /* current OS notification */
>> +	u32 nrdy;       /* driver status */
>> +	u32 did2[7];	/* extended supported display devices ID list */
>> +	u32 cpd2[7];	/* extended attached display devices list */
>> +	u8 rsvd2[4];
>> +} __packed;
>> +
>> +/* OpRegion mailbox #2: SWSCI */
>> +struct opregion_swsci {
>> +	u32 scic;       /* SWSCI command|status|data */
>> +	u32 parm;       /* command parameters */
>> +	u32 dslp;       /* driver sleep time-out */
>> +	u8 rsvd[244];
>> +} __packed;
>> +
>> +/* OpRegion mailbox #3: ASLE */
>> +struct opregion_asle {
>> +	u32 ardy;       /* driver readiness */
>> +	u32 aslc;       /* ASLE interrupt command */
>> +	u32 tche;       /* technology enabled indicator */
>> +	u32 alsi;       /* current ALS illuminance reading */
>> +	u32 bclp;       /* backlight brightness to set */
>> +	u32 pfit;       /* panel fitting state */
>> +	u32 cblv;       /* current brightness level */
>> +	u16 bclm[20];   /* backlight level duty cycle mapping table */
>> +	u32 cpfm;       /* current panel fitting mode */
>> +	u32 epfm;       /* enabled panel fitting modes */
>> +	u8 plut[74];    /* panel LUT and identifier */
>> +	u32 pfmb;       /* PWM freq and min brightness */
>> +	u32 cddv;       /* color correction default values */
>> +	u32 pcft;       /* power conservation features */
>> +	u32 srot;       /* supported rotation angles */
>> +	u32 iuer;       /* IUER events */
>> +	u64 fdss;
>> +	u32 fdsp;
>> +	u32 stat;
>> +	u64 rvda;	/* Physical address of raw vbt data */
>> +	u32 rvds;	/* Size of raw vbt data */
>> +	u8 rsvd[58];
>> +} __packed;
>> +
>> +/* Driver readiness indicator */
>> +#define ASLE_ARDY_READY		(1 << 0)
>> +#define ASLE_ARDY_NOT_READY	(0 << 0)
>> +
>> +/* ASLE Interrupt Command (ASLC) bits */
>> +#define ASLC_SET_ALS_ILLUM		(1 << 0)
>> +#define ASLC_SET_BACKLIGHT		(1 << 1)
>> +#define ASLC_SET_PFIT			(1 << 2)
>> +#define ASLC_SET_PWM_FREQ		(1 << 3)
>> +#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
>> +#define ASLC_BUTTON_ARRAY		(1 << 5)
>> +#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
>> +#define ASLC_DOCKING_INDICATOR		(1 << 7)
>> +#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
>> +#define ASLC_REQ_MSK			0x1ff
>> +/* response bits */
>> +#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
>> +#define ASLC_BACKLIGHT_FAILED		(1 << 12)
>> +#define ASLC_PFIT_FAILED		(1 << 14)
>> +#define ASLC_PWM_FREQ_FAILED		(1 << 16)
>> +#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
>> +#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
>> +#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
>> +#define ASLC_DOCKING_FAILED		(1 << 24)
>> +#define ASLC_ISCT_STATE_FAILED		(1 << 26)
>> +
>> +/* Technology enabled indicator */
>> +#define ASLE_TCHE_ALS_EN	(1 << 0)
>> +#define ASLE_TCHE_BLC_EN	(1 << 1)
>> +#define ASLE_TCHE_PFIT_EN	(1 << 2)
>> +#define ASLE_TCHE_PFMB_EN	(1 << 3)
>> +
>> +/* ASLE backlight brightness to set */
>> +#define ASLE_BCLP_VALID                (1<<31)
>> +#define ASLE_BCLP_MSK          (~(1<<31))
>> +
>> +/* ASLE panel fitting request */
>> +#define ASLE_PFIT_VALID         (1<<31)
>> +#define ASLE_PFIT_CENTER (1<<0)
>> +#define ASLE_PFIT_STRETCH_TEXT (1<<1) #define
>ASLE_PFIT_STRETCH_GFX
>> +(1<<2)
>> +
>> +/* PWM frequency and minimum brightness */ #define
>> +ASLE_PFMB_BRIGHTNESS_MASK (0xff) #define
>ASLE_PFMB_BRIGHTNESS_VALID
>> +(1<<8) #define ASLE_PFMB_PWM_MASK (0x7ffffe00) #define
>> +ASLE_PFMB_PWM_VALID (1<<31)
>> +
>> +#define ASLE_CBLV_VALID         (1<<31)
>> +
>> +/* IUER */
>> +#define ASLE_IUER_DOCKING		(1 << 7)
>> +#define ASLE_IUER_CONVERTIBLE		(1 << 6)
>> +#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
>> +#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
>> +#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
>> +#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>> +#define ASLE_IUER_POWER_BTN		(1 << 0)
>> +
>> +/* 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)
>> +#define ACPI_DIGITAL_OUTPUT (3<<8)
>> +#define ACPI_LVDS_OUTPUT (4<<8)
>> +
>> +#define MAX_DSLP	1500
>
>The bottom line here is that I think you could get all of this done with
>*much* smaller changes. If we get a regression report pointing at this patch,
>we'll have a lot of debugging to do to figure out what failed.
>
[Deepak M] Okay will try to break this patch into smaller one`s.
>BR,
>Jani.
>
>
>
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> [e-mail]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Jani Nikula, Intel Open Source Technology Center

Jani Nikula Sept. 22, 2015, 7:24 a.m. UTC | #4

On Tue, 22 Sep 2015, "Deepak, M" <[e-mail]> wrote:
>>-----Original Message-----
>>From: Jani Nikula [mailto:[e-mail]]
>>Sent: Thursday, September 17, 2015 5:41 PM
>>To: Deepak, M; [e-mail]
>>Cc: Deepak, M
>>Subject: Re: [Intel-gfx] [MIPI SEQ PARSING v2 PATCH 03/11] drm/i915: Parsing
>>VBT if size of VBT exceeds 6KB
>>
>>On Thu, 10 Sep 2015, Deepak M <[e-mail]> wrote:
>>> Currently the iomap for VBT works only if the size of the VBT is less
>>> than 6KB, but if the size of the VBT exceeds 6KB than the physical
>>> address and the size of the VBT to be iomapped is specified in the
>>> mailbox3 and is iomapped accordingly.
>>>
>>> v2: - Moving the validate_vbt to opregion file (Jani)
>>>     - Fix the i915_opregion() in debugfs (Jani)
>>>
>>> Signed-off-by: Deepak M <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/i915/i915_debugfs.c   |   24 ++-
>>>  drivers/gpu/drm/i915/i915_drv.h       |    4 +
>>>  drivers/gpu/drm/i915/intel_bios.c     |   49 +-----
>>>  drivers/gpu/drm/i915/intel_opregion.c |  279
>>> +++++++++------------------------
>>> drivers/gpu/drm/i915/intel_opregion.h |  230
>>> +++++++++++++++++++++++++++
>>>  5 files changed, 329 insertions(+), 257 deletions(-)  create mode
>>> 100644 drivers/gpu/drm/i915/intel_opregion.h
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 41629fa..5534aa2 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -26,6 +26,8 @@
>>>   *
>>>   */
>>>
>>> +#include <linux/acpi.h>
>>> +#include <acpi/video.h>
>>>  #include <linux/seq_file.h>
>>>  #include <linux/circ_buf.h>
>>>  #include <linux/ctype.h>
>>> @@ -39,6 +41,7 @@
>>>  #include "intel_ringbuffer.h"
>>>  #include <drm/i915_drm.h>
>>>  #include "i915_drv.h"
>>> +#include "intel_opregion.h"
>>>
>>>  enum {
>>>  	ACTIVE_LIST,
>>> @@ -1832,7 +1835,7 @@ static int i915_opregion(struct seq_file *m, void
>>*unused)
>>>  	struct drm_device *dev = node->minor->dev;
>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>  	struct intel_opregion *opregion = &dev_priv->opregion;
>>> -	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
>>> +	void *data = kmalloc(OPREGION_VBT_OFFSET, GFP_KERNEL);
>>>  	int ret;
>>>
>>>  	if (data == NULL)
>>> @@ -1843,12 +1846,25 @@ static int i915_opregion(struct seq_file *m, void
>>*unused)
>>>  		goto out;
>>>
>>>  	if (opregion->header) {
>>> -		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
>>> -		seq_write(m, data, OPREGION_SIZE);
>>> +		memcpy_fromio(data, opregion->header,
>>OPREGION_VBT_OFFSET);
>>> +		seq_write(m, data, OPREGION_VBT_OFFSET);
>>> +		kfree(data);
>>> +		if (opregion->asle->rvda) {
>>> +			data = kmalloc(opregion->asle->rvds, GFP_KERNEL);
>>> +			memcpy_fromio(data,
>>> +				(const void __iomem *) opregion->asle->rvda,
>>> +				opregion->asle->rvds);
>>> +			seq_write(m, data, opregion->asle->rvds);
>>> +		} else {
>>> +			data = kmalloc(OPREGION_SIZE -
>>OPREGION_VBT_OFFSET,
>>> +					GFP_KERNEL);
>>> +			memcpy_fromio(data, opregion->vbt,
>>> +					OPREGION_SIZE -
>>OPREGION_VBT_OFFSET);
>>> +			seq_write(m, data, OPREGION_SIZE -
>>OPREGION_VBT_OFFSET);
>>> +		}
>>
>>If rvda != 0, this debugfs file no longer represents the opregion contents.
>>Mailboxes #4 and #5 are dropped from the output. BTW, what is mailbox #4
>>expected to contain when rvda != 0? (I still don't have access to the latest
>>opregion spec version, so can't check what it actually says.)
>>
>>I am beginning to think we should leave "i915_opregion" debugfs file intact,
>>and add a new "i915_vbt" file that contains either mailbox #4 or the data in
>>rvda. This might be a cleaner approach.
>>
>>See my comments below, and you'll see how this would be feasible.
>>
> [Deepak M] I was thinking of splitting this function into 5 for dumping each mailbox. Which 
> I felt will be cleaner.

Please have i915_opregion and i915_vbt as I explained.

>>>  	}
>>>
>>>  	mutex_unlock(&dev->struct_mutex);
>>> -
>>>  out:
>>>  	kfree(data);
>>>  	return 0;
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h index 1287007..507d57a 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1780,6 +1780,7 @@ struct drm_i915_private {
>>>  	struct i915_hotplug hotplug;
>>>  	struct i915_fbc fbc;
>>>  	struct i915_drrs drrs;
>>> +	const struct bdb_header *bdb_start;
>>
>>What is wrong with using dev_priv->opregion.vbt for this?
>>
>>>  	struct intel_opregion opregion;
>>>  	struct intel_vbt_data vbt;
>>>
>>> @@ -3306,6 +3307,9 @@ intel_opregion_notify_adapter(struct drm_device
>>> *dev, pci_power_t state)  }  #endif
>>>
>>> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
>>> +			size_t size, const char *source);
>>> +
>>>  /* intel_acpi.c */
>>>  #ifdef CONFIG_ACPI
>>>  extern void intel_register_dsm_handler(void); diff --git
>>> a/drivers/gpu/drm/i915/intel_bios.c
>>> b/drivers/gpu/drm/i915/intel_bios.c
>>> index 0bf0942..1932a86 100644
>>> --- a/drivers/gpu/drm/i915/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> @@ -1227,49 +1227,6 @@ static const struct dmi_system_id
>>intel_no_opregion_vbt[] = {
>>>  	{ }
>>>  };
>>>
>>> -static const struct bdb_header *validate_vbt(const void __iomem *_base,
>>> -					     size_t size,
>>> -					     const void __iomem *_vbt,
>>> -					     const char *source)
>>> -{
>>> -	/*
>>> -	 * This is the one place where we explicitly discard the address space
>>> -	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse
>>complaint.)
>>> -	 * From now on everything is based on 'base', and treated as regular
>>> -	 * memory.
>>> -	 */
>>> -	const void *base = (const void *) _base;
>>> -	size_t offset = _vbt - _base;
>>> -	const struct vbt_header *vbt = base + offset;
>>> -	const struct bdb_header *bdb;
>>> -
>>> -	if (offset + sizeof(struct vbt_header) > size) {
>>> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>>> -		return NULL;
>>> -	}
>>> -
>>> -	if (memcmp(vbt->signature, "$VBT", 4)) {
>>> -		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>>> -		return NULL;
>>> -	}
>>> -
>>> -	offset += vbt->bdb_offset;
>>> -	if (offset + sizeof(struct bdb_header) > size) {
>>> -		DRM_DEBUG_DRIVER("BDB header incomplete\n");
>>> -		return NULL;
>>> -	}
>>> -
>>> -	bdb = base + offset;
>>> -	if (offset + bdb->bdb_size > size) {
>>> -		DRM_DEBUG_DRIVER("BDB incomplete\n");
>>> -		return NULL;
>>> -	}
>>> -
>>> -	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
>>> -		      source, vbt->signature);
>>> -	return bdb;
>>> -}
>>> -
>>
>>Moving of this function should be a separate prep patch.
>>
>>>  static const struct bdb_header *find_vbt(void __iomem *bios, size_t
>>> size)  {
>>>  	const struct bdb_header *bdb = NULL; @@ -1278,7 +1235,7 @@ static
>>> const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>>>  	/* Scour memory looking for the VBT signature. */
>>>  	for (i = 0; i + 4 < size; i++) {
>>>  		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
>>> -			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
>>> +			bdb = validate_vbt(bios + i, size - i, "PCI ROM");
>>>  			break;
>>>  		}
>>>  	}
>>> @@ -1308,10 +1265,8 @@ intel_parse_bios(struct drm_device *dev)
>>>
>>>  	init_vbt_defaults(dev_priv);
>>>
>>> -	/* XXX Should this validation be moved to intel_opregion.c? */
>>>  	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv-
>>>opregion.vbt)
>>> -		bdb = validate_vbt(dev_priv->opregion.header,
>>OPREGION_SIZE,
>>> -				   dev_priv->opregion.vbt, "OpRegion");
>>> +		bdb = dev_priv->bdb_start;
>>
>>This should be dev_priv->opregion.vbt.
> [Deepak M] validate_vbt function always returns the bdb header and our parsing starts from the bdb_header and not form the dev_priv->opregion.vbt where the VBT header starts. So I have added bdb_start in the dev_priv structure.
>>

Please don't add new fields when you can reuse existing ones.

>>>
>>>  	if (bdb == NULL) {
>>>  		size_t size;
>>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
>>> b/drivers/gpu/drm/i915/intel_opregion.c
>>> index f46231f..3a43db9 100644
>>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>>> @@ -32,210 +32,7 @@
>>>  #include <drm/i915_drm.h>
>>>  #include "i915_drv.h"
>>>  #include "intel_drv.h"
>>> -
>>> -#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
>>> -#define   ACPI_CLID 0x01ac /* current lid state indicator */
>>> -#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
>>> -#define OPREGION_SWSCI_OFFSET  0x200
>>> -#define OPREGION_ASLE_OFFSET   0x300
>>> -#define OPREGION_VBT_OFFSET    0x400
>>> -
>>> -#define OPREGION_SIGNATURE "IntelGraphicsMem"
>>> -#define MBOX_ACPI      (1<<0)
>>> -#define MBOX_SWSCI     (1<<1)
>>> -#define MBOX_ASLE      (1<<2)
>>> -#define MBOX_ASLE_EXT  (1<<4)
>>> -
>>> -struct opregion_header {
>>> -	u8 signature[16];
>>> -	u32 size;
>>> -	u32 opregion_ver;
>>> -	u8 bios_ver[32];
>>> -	u8 vbios_ver[16];
>>> -	u8 driver_ver[16];
>>> -	u32 mboxes;
>>> -	u32 driver_model;
>>> -	u32 pcon;
>>> -	u8 dver[32];
>>> -	u8 rsvd[124];
>>> -} __packed;
>>> -
>>> -/* OpRegion mailbox #1: public ACPI methods */ -struct opregion_acpi
>>> {
>>> -	u32 drdy;       /* driver readiness */
>>> -	u32 csts;       /* notification status */
>>> -	u32 cevt;       /* current event */
>>> -	u8 rsvd1[20];
>>> -	u32 didl[8];    /* supported display devices ID list */
>>> -	u32 cpdl[8];    /* currently presented display list */
>>> -	u32 cadl[8];    /* currently active display list */
>>> -	u32 nadl[8];    /* next active devices list */
>>> -	u32 aslp;       /* ASL sleep time-out */
>>> -	u32 tidx;       /* toggle table index */
>>> -	u32 chpd;       /* current hotplug enable indicator */
>>> -	u32 clid;       /* current lid state*/
>>> -	u32 cdck;       /* current docking state */
>>> -	u32 sxsw;       /* Sx state resume */
>>> -	u32 evts;       /* ASL supported events */
>>> -	u32 cnot;       /* current OS notification */
>>> -	u32 nrdy;       /* driver status */
>>> -	u32 did2[7];	/* extended supported display devices ID list */
>>> -	u32 cpd2[7];	/* extended attached display devices list */
>>> -	u8 rsvd2[4];
>>> -} __packed;
>>> -
>>> -/* OpRegion mailbox #2: SWSCI */
>>> -struct opregion_swsci {
>>> -	u32 scic;       /* SWSCI command|status|data */
>>> -	u32 parm;       /* command parameters */
>>> -	u32 dslp;       /* driver sleep time-out */
>>> -	u8 rsvd[244];
>>> -} __packed;
>>> -
>>> -/* OpRegion mailbox #3: ASLE */
>>> -struct opregion_asle {
>>> -	u32 ardy;       /* driver readiness */
>>> -	u32 aslc;       /* ASLE interrupt command */
>>> -	u32 tche;       /* technology enabled indicator */
>>> -	u32 alsi;       /* current ALS illuminance reading */
>>> -	u32 bclp;       /* backlight brightness to set */
>>> -	u32 pfit;       /* panel fitting state */
>>> -	u32 cblv;       /* current brightness level */
>>> -	u16 bclm[20];   /* backlight level duty cycle mapping table */
>>> -	u32 cpfm;       /* current panel fitting mode */
>>> -	u32 epfm;       /* enabled panel fitting modes */
>>> -	u8 plut[74];    /* panel LUT and identifier */
>>> -	u32 pfmb;       /* PWM freq and min brightness */
>>> -	u32 cddv;       /* color correction default values */
>>> -	u32 pcft;       /* power conservation features */
>>> -	u32 srot;       /* supported rotation angles */
>>> -	u32 iuer;       /* IUER events */
>>> -	u64 fdss;
>>> -	u32 fdsp;
>>> -	u32 stat;
>>> -	u64 rvda;	/* Physical address of raw vbt data */
>>> -	u32 rvds;	/* Size of raw vbt data */
>>> -	u8 rsvd[58];
>>> -} __packed;
>>> -
>>> -/* Driver readiness indicator */
>>> -#define ASLE_ARDY_READY		(1 << 0)
>>> -#define ASLE_ARDY_NOT_READY	(0 << 0)
>>> -
>>> -/* ASLE Interrupt Command (ASLC) bits */
>>> -#define ASLC_SET_ALS_ILLUM		(1 << 0)
>>> -#define ASLC_SET_BACKLIGHT		(1 << 1)
>>> -#define ASLC_SET_PFIT			(1 << 2)
>>> -#define ASLC_SET_PWM_FREQ		(1 << 3)
>>> -#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
>>> -#define ASLC_BUTTON_ARRAY		(1 << 5)
>>> -#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
>>> -#define ASLC_DOCKING_INDICATOR		(1 << 7)
>>> -#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
>>> -#define ASLC_REQ_MSK			0x1ff
>>> -/* response bits */
>>> -#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
>>> -#define ASLC_BACKLIGHT_FAILED		(1 << 12)
>>> -#define ASLC_PFIT_FAILED		(1 << 14)
>>> -#define ASLC_PWM_FREQ_FAILED		(1 << 16)
>>> -#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
>>> -#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
>>> -#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
>>> -#define ASLC_DOCKING_FAILED		(1 << 24)
>>> -#define ASLC_ISCT_STATE_FAILED		(1 << 26)
>>> -
>>> -/* Technology enabled indicator */
>>> -#define ASLE_TCHE_ALS_EN	(1 << 0)
>>> -#define ASLE_TCHE_BLC_EN	(1 << 1)
>>> -#define ASLE_TCHE_PFIT_EN	(1 << 2)
>>> -#define ASLE_TCHE_PFMB_EN	(1 << 3)
>>> -
>>> -/* ASLE backlight brightness to set */
>>> -#define ASLE_BCLP_VALID                (1<<31)
>>> -#define ASLE_BCLP_MSK          (~(1<<31))
>>> -
>>> -/* ASLE panel fitting request */
>>> -#define ASLE_PFIT_VALID         (1<<31)
>>> -#define ASLE_PFIT_CENTER (1<<0)
>>> -#define ASLE_PFIT_STRETCH_TEXT (1<<1) -#define
>>ASLE_PFIT_STRETCH_GFX
>>> (1<<2)
>>> -
>>> -/* PWM frequency and minimum brightness */ -#define
>>> ASLE_PFMB_BRIGHTNESS_MASK (0xff) -#define
>>ASLE_PFMB_BRIGHTNESS_VALID
>>> (1<<8) -#define ASLE_PFMB_PWM_MASK (0x7ffffe00) -#define
>>> ASLE_PFMB_PWM_VALID (1<<31)
>>> -
>>> -#define ASLE_CBLV_VALID         (1<<31)
>>> -
>>> -/* IUER */
>>> -#define ASLE_IUER_DOCKING		(1 << 7)
>>> -#define ASLE_IUER_CONVERTIBLE		(1 << 6)
>>> -#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
>>> -#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
>>> -#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
>>> -#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>>> -#define ASLE_IUER_POWER_BTN		(1 << 0)
>>> -
>>> -/* 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)
>>> -#define ACPI_DIGITAL_OUTPUT (3<<8)
>>> -#define ACPI_LVDS_OUTPUT (4<<8)
>>> -
>>> -#define MAX_DSLP	1500
>>> +#include "intel_opregion.h"
>>
>>As said, I don't see the need to move these defines to a separate header. This
>>is definitely stuff that we want to keep hidden in one place, and nobody
>>outside of intel_opregion.c should use these.
>>
> [Deepak M] Need some of these definitions in the debugfs.c file, because of this
> had created a new file for macros.

Please rework your debugfs handling to not need these. If needed, add an
interface to intel_opregion.c to do what you want.

>
>>>
>>>  #ifdef CONFIG_ACPI
>>>  static int swsci(struct drm_device *dev, u32 function, u32 parm, u32
>>> *parm_out) @@ -892,13 +689,55 @@ static void swsci_setup(struct
>>> drm_device *dev)  static inline void swsci_setup(struct drm_device
>>> *dev) {}  #endif  /* CONFIG_ACPI */
>>>
>>> +const struct bdb_header *validate_vbt(const void __iomem *_vbt,
>>> +					size_t size,
>>> +					const char *source)
>>> +{
>>> +	/*
>>> +	 * This is the one place where we explicitly discard the address space
>>> +	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse
>>complaint.)
>>> +	 */
>>> +	const struct vbt_header *vbt = (const struct vbt_header *)_vbt;
>>> +	const struct bdb_header *bdb;
>>> +	size_t offset;
>>> +
>>> +	if (sizeof(struct vbt_header) > size) {
>>> +		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>>> +		return NULL;
>>> +	}
>>> +
>>> +	if (memcmp(vbt->signature, "$VBT", 4)) {
>>> +		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>>> +		return NULL;
>>> +	}
>>> +
>>> +	offset = vbt->bdb_offset;
>>> +	if (offset + sizeof(struct bdb_header) > size) {
>>> +		DRM_DEBUG_DRIVER("BDB header incomplete\n");
>>> +		return NULL;
>>> +	}
>>> +
>>> +	bdb = (const void *)_vbt + offset;
>>> +	if (offset + bdb->bdb_size > size) {
>>> +		DRM_DEBUG_DRIVER("BDB incomplete\n");
>>> +		return NULL;
>>> +	}
>>> +
>>> +	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
>>> +			source, vbt->signature);
>>> +	return bdb;
>>> +}
>>> +
>>>  int intel_opregion_setup(struct drm_device *dev)  {
>>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>>  	struct intel_opregion *opregion = &dev_priv->opregion;
>>>  	void __iomem *base;
>>> +	void __iomem *vbt_base;
>>>  	u32 asls, mboxes;
>>>  	char buf[sizeof(OPREGION_SIGNATURE)];
>>> +	const struct bdb_header *bdb = NULL;
>>> +	size_t size;
>>>  	int err = 0;
>>>
>>>  	BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100); @@ -
>>917,7
>>> +756,7 @@ int intel_opregion_setup(struct drm_device *dev)
>>>  	INIT_WORK(&opregion->asle_work, asle_work);  #endif
>>>
>>> -	base = acpi_os_ioremap(asls, OPREGION_SIZE);
>>> +	base = acpi_os_ioremap(asls, OPREGION_VBT_OFFSET);
>>
>>Now you leave out mailbox #5. I don't think there's any reason *not* to
>>ioremap all of opregion here.
>>
>>>  	if (!base)
>>>  		return -ENOMEM;
>>>
>>> @@ -929,7 +768,33 @@ int intel_opregion_setup(struct drm_device *dev)
>>>  		goto err_out;
>>>  	}
>>>  	opregion->header = base;
>>> -	opregion->vbt = base + OPREGION_VBT_OFFSET;
>>> +	/* Assigning the alse to the mailbox 3 of the opregion */
>>> +	opregion->asle = base + OPREGION_ASLE_OFFSET;
>>
>>That's assigned towards the end of the function *if* the mbox is supported.
>>
>>> +
>>> +	/*
>>> +	 * Non-zero value in rvda field is an indication to driver that a
>>> +	 * valid Raw VBT is stored in that address and driver should not refer
>>> +	 * to mailbox4 for getting VBT.
>>> +	 */
>>> +	if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) {
>>> +			size = opregion->asle->rvds;
>>> +			vbt_base = acpi_os_ioremap(opregion->asle->rvda,
>>> +					size);
>>> +	} else {
>>> +			size = OPREGION_SIZE - OPREGION_VBT_OFFSET;
>>> +			vbt_base = acpi_os_ioremap(asls +
>>OPREGION_VBT_OFFSET,
>>> +					size);
>>> +	}
>>> +
>>> +	bdb = validate_vbt(vbt_base, size, "OpRegion");
>>> +
>>> +	if (bdb == NULL) {
>>> +		err = -EINVAL;
>>> +		goto err_vbt;
>>> +	}
>>> +
>>> +	dev_priv->bdb_start = bdb;
>>
>>Again, I don't see why you should store this here. Nor I see the need to
>>actually validate vbt here. You can just as well do it in intel_bios like before, as
>>long as you assign opregion->vbt here appropriately.
>>
>>You'll also need to iounmap vbt_base in intel_opregion_fini. That may need
>>some additional checks if you unconditionally ioremap all of opregion and
>>conditionally ioremap rvda, and point opregion->vbt to it.
>>
> [Deepak M] Okay will handle this.
>>> +	opregion->vbt = vbt_base;
>>>
>>>  	opregion->lid_state = base + ACPI_CLID;
>>>
>>> @@ -953,6 +818,8 @@ int intel_opregion_setup(struct drm_device *dev)
>>>
>>>  	return 0;
>>>
>>> +err_vbt:
>>> +	iounmap(vbt_base);
>>>  err_out:
>>>  	iounmap(base);
>>>  	return err;
>>> diff --git a/drivers/gpu/drm/i915/intel_opregion.h
>>> b/drivers/gpu/drm/i915/intel_opregion.h
>>> new file mode 100644
>>> index 0000000..bcb45ec
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_opregion.h
>>> @@ -0,0 +1,230 @@
>>> +/*
>>> + * Copyright 2008 Intel Corporation <[e-mail]>
>>> + * Copyright 2008 Red Hat <[e-mail]>
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person
>>> +obtaining
>>> + * a copy of this software and associated documentation files (the
>>> + * "Software"), to deal in the Software without restriction,
>>> +including
>>> + * without limitation the rights to use, copy, modify, merge,
>>> +publish,
>>> + * distribute, sub license, and/or sell copies of the Software, and
>>> +to
>>> + * permit persons to whom the Software is furnished to do so, subject
>>> +to
>>> + * the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice (including
>>> +the
>>> + * next paragraph) shall be included in all copies or substantial
>>> + * portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
>>KIND,
>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
>>WARRANTIES OF
>>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>> + * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS
>>BE
>>> + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
>>OR IN
>>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>>THE
>>> + * SOFTWARE.
>>> + *
>>> + */
>>> +
>>> +#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
>>> +#define   ACPI_CLID 0x01ac /* current lid state indicator */
>>> +#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
>>> +#define OPREGION_SWSCI_OFFSET  0x200
>>> +#define OPREGION_ASLE_OFFSET   0x300
>>> +#define OPREGION_VBT_OFFSET    0x400
>>> +
>>> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
>>> +#define MBOX_ACPI      (1<<0)
>>> +#define MBOX_SWSCI     (1<<1)
>>> +#define MBOX_ASLE      (1<<2)
>>> +#define MBOX_ASLE_EXT  (1<<4)
>>> +
>>> +struct opregion_header {
>>> +	u8 signature[16];
>>> +	u32 size;
>>> +	u32 opregion_ver;
>>> +	u8 bios_ver[32];
>>> +	u8 vbios_ver[16];
>>> +	u8 driver_ver[16];
>>> +	u32 mboxes;
>>> +	u32 driver_model;
>>> +	u32 pcon;
>>> +	u8 dver[32];
>>> +	u8 rsvd[124];
>>> +} __packed;
>>> +
>>> +/* OpRegion mailbox #1: public ACPI methods */ struct opregion_acpi {
>>> +	u32 drdy;       /* driver readiness */
>>> +	u32 csts;       /* notification status */
>>> +	u32 cevt;       /* current event */
>>> +	u8 rsvd1[20];
>>> +	u32 didl[8];    /* supported display devices ID list */
>>> +	u32 cpdl[8];    /* currently presented display list */
>>> +	u32 cadl[8];    /* currently active display list */
>>> +	u32 nadl[8];    /* next active devices list */
>>> +	u32 aslp;       /* ASL sleep time-out */
>>> +	u32 tidx;       /* toggle table index */
>>> +	u32 chpd;       /* current hotplug enable indicator */
>>> +	u32 clid;       /* current lid state*/
>>> +	u32 cdck;       /* current docking state */
>>> +	u32 sxsw;       /* Sx state resume */
>>> +	u32 evts;       /* ASL supported events */
>>> +	u32 cnot;       /* current OS notification */
>>> +	u32 nrdy;       /* driver status */
>>> +	u32 did2[7];	/* extended supported display devices ID list */
>>> +	u32 cpd2[7];	/* extended attached display devices list */
>>> +	u8 rsvd2[4];
>>> +} __packed;
>>> +
>>> +/* OpRegion mailbox #2: SWSCI */
>>> +struct opregion_swsci {
>>> +	u32 scic;       /* SWSCI command|status|data */
>>> +	u32 parm;       /* command parameters */
>>> +	u32 dslp;       /* driver sleep time-out */
>>> +	u8 rsvd[244];
>>> +} __packed;
>>> +
>>> +/* OpRegion mailbox #3: ASLE */
>>> +struct opregion_asle {
>>> +	u32 ardy;       /* driver readiness */
>>> +	u32 aslc;       /* ASLE interrupt command */
>>> +	u32 tche;       /* technology enabled indicator */
>>> +	u32 alsi;       /* current ALS illuminance reading */
>>> +	u32 bclp;       /* backlight brightness to set */
>>> +	u32 pfit;       /* panel fitting state */
>>> +	u32 cblv;       /* current brightness level */
>>> +	u16 bclm[20];   /* backlight level duty cycle mapping table */
>>> +	u32 cpfm;       /* current panel fitting mode */
>>> +	u32 epfm;       /* enabled panel fitting modes */
>>> +	u8 plut[74];    /* panel LUT and identifier */
>>> +	u32 pfmb;       /* PWM freq and min brightness */
>>> +	u32 cddv;       /* color correction default values */
>>> +	u32 pcft;       /* power conservation features */
>>> +	u32 srot;       /* supported rotation angles */
>>> +	u32 iuer;       /* IUER events */
>>> +	u64 fdss;
>>> +	u32 fdsp;
>>> +	u32 stat;
>>> +	u64 rvda;	/* Physical address of raw vbt data */
>>> +	u32 rvds;	/* Size of raw vbt data */
>>> +	u8 rsvd[58];
>>> +} __packed;
>>> +
>>> +/* Driver readiness indicator */
>>> +#define ASLE_ARDY_READY		(1 << 0)
>>> +#define ASLE_ARDY_NOT_READY	(0 << 0)
>>> +
>>> +/* ASLE Interrupt Command (ASLC) bits */
>>> +#define ASLC_SET_ALS_ILLUM		(1 << 0)
>>> +#define ASLC_SET_BACKLIGHT		(1 << 1)
>>> +#define ASLC_SET_PFIT			(1 << 2)
>>> +#define ASLC_SET_PWM_FREQ		(1 << 3)
>>> +#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
>>> +#define ASLC_BUTTON_ARRAY		(1 << 5)
>>> +#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
>>> +#define ASLC_DOCKING_INDICATOR		(1 << 7)
>>> +#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
>>> +#define ASLC_REQ_MSK			0x1ff
>>> +/* response bits */
>>> +#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
>>> +#define ASLC_BACKLIGHT_FAILED		(1 << 12)
>>> +#define ASLC_PFIT_FAILED		(1 << 14)
>>> +#define ASLC_PWM_FREQ_FAILED		(1 << 16)
>>> +#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
>>> +#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
>>> +#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
>>> +#define ASLC_DOCKING_FAILED		(1 << 24)
>>> +#define ASLC_ISCT_STATE_FAILED		(1 << 26)
>>> +
>>> +/* Technology enabled indicator */
>>> +#define ASLE_TCHE_ALS_EN	(1 << 0)
>>> +#define ASLE_TCHE_BLC_EN	(1 << 1)
>>> +#define ASLE_TCHE_PFIT_EN	(1 << 2)
>>> +#define ASLE_TCHE_PFMB_EN	(1 << 3)
>>> +
>>> +/* ASLE backlight brightness to set */
>>> +#define ASLE_BCLP_VALID                (1<<31)
>>> +#define ASLE_BCLP_MSK          (~(1<<31))
>>> +
>>> +/* ASLE panel fitting request */
>>> +#define ASLE_PFIT_VALID         (1<<31)
>>> +#define ASLE_PFIT_CENTER (1<<0)
>>> +#define ASLE_PFIT_STRETCH_TEXT (1<<1) #define
>>ASLE_PFIT_STRETCH_GFX
>>> +(1<<2)
>>> +
>>> +/* PWM frequency and minimum brightness */ #define
>>> +ASLE_PFMB_BRIGHTNESS_MASK (0xff) #define
>>ASLE_PFMB_BRIGHTNESS_VALID
>>> +(1<<8) #define ASLE_PFMB_PWM_MASK (0x7ffffe00) #define
>>> +ASLE_PFMB_PWM_VALID (1<<31)
>>> +
>>> +#define ASLE_CBLV_VALID         (1<<31)
>>> +
>>> +/* IUER */
>>> +#define ASLE_IUER_DOCKING		(1 << 7)
>>> +#define ASLE_IUER_CONVERTIBLE		(1 << 6)
>>> +#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
>>> +#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
>>> +#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
>>> +#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>>> +#define ASLE_IUER_POWER_BTN		(1 << 0)
>>> +
>>> +/* 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)
>>> +#define ACPI_DIGITAL_OUTPUT (3<<8)
>>> +#define ACPI_LVDS_OUTPUT (4<<8)
>>> +
>>> +#define MAX_DSLP	1500
>>
>>The bottom line here is that I think you could get all of this done with
>>*much* smaller changes. If we get a regression report pointing at this patch,
>>we'll have a lot of debugging to do to figure out what failed.
>>
> [Deepak M] Okay will try to break this patch into smaller one`s.
>>BR,
>>Jani.
>>
>>
>>
>>> --
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> [e-mail]
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>--
>>Jani Nikula, Intel Open Source Technology Center

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 41629fa..5534aa2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -26,6 +26,8 @@ 
  *
  */
 
+#include <linux/acpi.h>
+#include <acpi/video.h>
 #include <linux/seq_file.h>
 #include <linux/circ_buf.h>
 #include <linux/ctype.h>
@@ -39,6 +41,7 @@ 
 #include "intel_ringbuffer.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_opregion.h"
 
 enum {
 	ACTIVE_LIST,
@@ -1832,7 +1835,7 @@  static int i915_opregion(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
-	void *data = kmalloc(OPREGION_SIZE, GFP_KERNEL);
+	void *data = kmalloc(OPREGION_VBT_OFFSET, GFP_KERNEL);
 	int ret;
 
 	if (data == NULL)
@@ -1843,12 +1846,25 @@  static int i915_opregion(struct seq_file *m, void *unused)
 		goto out;
 
 	if (opregion->header) {
-		memcpy_fromio(data, opregion->header, OPREGION_SIZE);
-		seq_write(m, data, OPREGION_SIZE);
+		memcpy_fromio(data, opregion->header, OPREGION_VBT_OFFSET);
+		seq_write(m, data, OPREGION_VBT_OFFSET);
+		kfree(data);
+		if (opregion->asle->rvda) {
+			data = kmalloc(opregion->asle->rvds, GFP_KERNEL);
+			memcpy_fromio(data,
+				(const void __iomem *) opregion->asle->rvda,
+				opregion->asle->rvds);
+			seq_write(m, data, opregion->asle->rvds);
+		} else {
+			data = kmalloc(OPREGION_SIZE - OPREGION_VBT_OFFSET,
+					GFP_KERNEL);
+			memcpy_fromio(data, opregion->vbt,
+					OPREGION_SIZE - OPREGION_VBT_OFFSET);
+			seq_write(m, data, OPREGION_SIZE - OPREGION_VBT_OFFSET);
+		}
 	}
 
 	mutex_unlock(&dev->struct_mutex);
-
 out:
 	kfree(data);
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1287007..507d57a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1780,6 +1780,7 @@  struct drm_i915_private {
 	struct i915_hotplug hotplug;
 	struct i915_fbc fbc;
 	struct i915_drrs drrs;
+	const struct bdb_header *bdb_start;
 	struct intel_opregion opregion;
 	struct intel_vbt_data vbt;
 
@@ -3306,6 +3307,9 @@  intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 }
 #endif
 
+const struct bdb_header *validate_vbt(const void __iomem *_vbt,
+			size_t size, const char *source);
+
 /* intel_acpi.c */
 #ifdef CONFIG_ACPI
 extern void intel_register_dsm_handler(void);
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 0bf0942..1932a86 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1227,49 +1227,6 @@  static const struct dmi_system_id intel_no_opregion_vbt[] = {
 	{ }
 };
 
-static const struct bdb_header *validate_vbt(const void __iomem *_base,
-					     size_t size,
-					     const void __iomem *_vbt,
-					     const char *source)
-{
-	/*
-	 * This is the one place where we explicitly discard the address space
-	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
-	 * From now on everything is based on 'base', and treated as regular
-	 * memory.
-	 */
-	const void *base = (const void *) _base;
-	size_t offset = _vbt - _base;
-	const struct vbt_header *vbt = base + offset;
-	const struct bdb_header *bdb;
-
-	if (offset + sizeof(struct vbt_header) > size) {
-		DRM_DEBUG_DRIVER("VBT header incomplete\n");
-		return NULL;
-	}
-
-	if (memcmp(vbt->signature, "$VBT", 4)) {
-		DRM_DEBUG_DRIVER("VBT invalid signature\n");
-		return NULL;
-	}
-
-	offset += vbt->bdb_offset;
-	if (offset + sizeof(struct bdb_header) > size) {
-		DRM_DEBUG_DRIVER("BDB header incomplete\n");
-		return NULL;
-	}
-
-	bdb = base + offset;
-	if (offset + bdb->bdb_size > size) {
-		DRM_DEBUG_DRIVER("BDB incomplete\n");
-		return NULL;
-	}
-
-	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
-		      source, vbt->signature);
-	return bdb;
-}
-
 static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
 {
 	const struct bdb_header *bdb = NULL;
@@ -1278,7 +1235,7 @@  static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
 	/* Scour memory looking for the VBT signature. */
 	for (i = 0; i + 4 < size; i++) {
 		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
-			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
+			bdb = validate_vbt(bios + i, size - i, "PCI ROM");
 			break;
 		}
 	}
@@ -1308,10 +1265,8 @@  intel_parse_bios(struct drm_device *dev)
 
 	init_vbt_defaults(dev_priv);
 
-	/* XXX Should this validation be moved to intel_opregion.c? */
 	if (!dmi_check_system(intel_no_opregion_vbt) && dev_priv->opregion.vbt)
-		bdb = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE,
-				   dev_priv->opregion.vbt, "OpRegion");
+		bdb = dev_priv->bdb_start;
 
 	if (bdb == NULL) {
 		size_t size;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index f46231f..3a43db9 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -32,210 +32,7 @@ 
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
-
-#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
-#define   ACPI_CLID 0x01ac /* current lid state indicator */
-#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
-#define OPREGION_SWSCI_OFFSET  0x200
-#define OPREGION_ASLE_OFFSET   0x300
-#define OPREGION_VBT_OFFSET    0x400
-
-#define OPREGION_SIGNATURE "IntelGraphicsMem"
-#define MBOX_ACPI      (1<<0)
-#define MBOX_SWSCI     (1<<1)
-#define MBOX_ASLE      (1<<2)
-#define MBOX_ASLE_EXT  (1<<4)
-
-struct opregion_header {
-	u8 signature[16];
-	u32 size;
-	u32 opregion_ver;
-	u8 bios_ver[32];
-	u8 vbios_ver[16];
-	u8 driver_ver[16];
-	u32 mboxes;
-	u32 driver_model;
-	u32 pcon;
-	u8 dver[32];
-	u8 rsvd[124];
-} __packed;
-
-/* OpRegion mailbox #1: public ACPI methods */
-struct opregion_acpi {
-	u32 drdy;       /* driver readiness */
-	u32 csts;       /* notification status */
-	u32 cevt;       /* current event */
-	u8 rsvd1[20];
-	u32 didl[8];    /* supported display devices ID list */
-	u32 cpdl[8];    /* currently presented display list */
-	u32 cadl[8];    /* currently active display list */
-	u32 nadl[8];    /* next active devices list */
-	u32 aslp;       /* ASL sleep time-out */
-	u32 tidx;       /* toggle table index */
-	u32 chpd;       /* current hotplug enable indicator */
-	u32 clid;       /* current lid state*/
-	u32 cdck;       /* current docking state */
-	u32 sxsw;       /* Sx state resume */
-	u32 evts;       /* ASL supported events */
-	u32 cnot;       /* current OS notification */
-	u32 nrdy;       /* driver status */
-	u32 did2[7];	/* extended supported display devices ID list */
-	u32 cpd2[7];	/* extended attached display devices list */
-	u8 rsvd2[4];
-} __packed;
-
-/* OpRegion mailbox #2: SWSCI */
-struct opregion_swsci {
-	u32 scic;       /* SWSCI command|status|data */
-	u32 parm;       /* command parameters */
-	u32 dslp;       /* driver sleep time-out */
-	u8 rsvd[244];
-} __packed;
-
-/* OpRegion mailbox #3: ASLE */
-struct opregion_asle {
-	u32 ardy;       /* driver readiness */
-	u32 aslc;       /* ASLE interrupt command */
-	u32 tche;       /* technology enabled indicator */
-	u32 alsi;       /* current ALS illuminance reading */
-	u32 bclp;       /* backlight brightness to set */
-	u32 pfit;       /* panel fitting state */
-	u32 cblv;       /* current brightness level */
-	u16 bclm[20];   /* backlight level duty cycle mapping table */
-	u32 cpfm;       /* current panel fitting mode */
-	u32 epfm;       /* enabled panel fitting modes */
-	u8 plut[74];    /* panel LUT and identifier */
-	u32 pfmb;       /* PWM freq and min brightness */
-	u32 cddv;       /* color correction default values */
-	u32 pcft;       /* power conservation features */
-	u32 srot;       /* supported rotation angles */
-	u32 iuer;       /* IUER events */
-	u64 fdss;
-	u32 fdsp;
-	u32 stat;
-	u64 rvda;	/* Physical address of raw vbt data */
-	u32 rvds;	/* Size of raw vbt data */
-	u8 rsvd[58];
-} __packed;
-
-/* Driver readiness indicator */
-#define ASLE_ARDY_READY		(1 << 0)
-#define ASLE_ARDY_NOT_READY	(0 << 0)
-
-/* ASLE Interrupt Command (ASLC) bits */
-#define ASLC_SET_ALS_ILLUM		(1 << 0)
-#define ASLC_SET_BACKLIGHT		(1 << 1)
-#define ASLC_SET_PFIT			(1 << 2)
-#define ASLC_SET_PWM_FREQ		(1 << 3)
-#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
-#define ASLC_BUTTON_ARRAY		(1 << 5)
-#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
-#define ASLC_DOCKING_INDICATOR		(1 << 7)
-#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
-#define ASLC_REQ_MSK			0x1ff
-/* response bits */
-#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
-#define ASLC_BACKLIGHT_FAILED		(1 << 12)
-#define ASLC_PFIT_FAILED		(1 << 14)
-#define ASLC_PWM_FREQ_FAILED		(1 << 16)
-#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
-#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
-#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
-#define ASLC_DOCKING_FAILED		(1 << 24)
-#define ASLC_ISCT_STATE_FAILED		(1 << 26)
-
-/* Technology enabled indicator */
-#define ASLE_TCHE_ALS_EN	(1 << 0)
-#define ASLE_TCHE_BLC_EN	(1 << 1)
-#define ASLE_TCHE_PFIT_EN	(1 << 2)
-#define ASLE_TCHE_PFMB_EN	(1 << 3)
-
-/* ASLE backlight brightness to set */
-#define ASLE_BCLP_VALID                (1<<31)
-#define ASLE_BCLP_MSK          (~(1<<31))
-
-/* ASLE panel fitting request */
-#define ASLE_PFIT_VALID         (1<<31)
-#define ASLE_PFIT_CENTER (1<<0)
-#define ASLE_PFIT_STRETCH_TEXT (1<<1)
-#define ASLE_PFIT_STRETCH_GFX (1<<2)
-
-/* PWM frequency and minimum brightness */
-#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
-#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
-#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
-#define ASLE_PFMB_PWM_VALID (1<<31)
-
-#define ASLE_CBLV_VALID         (1<<31)
-
-/* IUER */
-#define ASLE_IUER_DOCKING		(1 << 7)
-#define ASLE_IUER_CONVERTIBLE		(1 << 6)
-#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
-#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
-#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
-#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
-#define ASLE_IUER_POWER_BTN		(1 << 0)
-
-/* 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)
-#define ACPI_DIGITAL_OUTPUT (3<<8)
-#define ACPI_LVDS_OUTPUT (4<<8)
-
-#define MAX_DSLP	1500
+#include "intel_opregion.h"
 
 #ifdef CONFIG_ACPI
 static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
@@ -892,13 +689,55 @@  static void swsci_setup(struct drm_device *dev)
 static inline void swsci_setup(struct drm_device *dev) {}
 #endif  /* CONFIG_ACPI */
 
+const struct bdb_header *validate_vbt(const void __iomem *_vbt,
+					size_t size,
+					const char *source)
+{
+	/*
+	 * This is the one place where we explicitly discard the address space
+	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
+	 */
+	const struct vbt_header *vbt = (const struct vbt_header *)_vbt;
+	const struct bdb_header *bdb;
+	size_t offset;
+
+	if (sizeof(struct vbt_header) > size) {
+		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		return NULL;
+	}
+
+	if (memcmp(vbt->signature, "$VBT", 4)) {
+		DRM_DEBUG_DRIVER("VBT invalid signature\n");
+		return NULL;
+	}
+
+	offset = vbt->bdb_offset;
+	if (offset + sizeof(struct bdb_header) > size) {
+		DRM_DEBUG_DRIVER("BDB header incomplete\n");
+		return NULL;
+	}
+
+	bdb = (const void *)_vbt + offset;
+	if (offset + bdb->bdb_size > size) {
+		DRM_DEBUG_DRIVER("BDB incomplete\n");
+		return NULL;
+	}
+
+	DRM_DEBUG_KMS("Using VBT from %s: %20s\n",
+			source, vbt->signature);
+	return bdb;
+}
+
 int intel_opregion_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_opregion *opregion = &dev_priv->opregion;
 	void __iomem *base;
+	void __iomem *vbt_base;
 	u32 asls, mboxes;
 	char buf[sizeof(OPREGION_SIGNATURE)];
+	const struct bdb_header *bdb = NULL;
+	size_t size;
 	int err = 0;
 
 	BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100);
@@ -917,7 +756,7 @@  int intel_opregion_setup(struct drm_device *dev)
 	INIT_WORK(&opregion->asle_work, asle_work);
 #endif
 
-	base = acpi_os_ioremap(asls, OPREGION_SIZE);
+	base = acpi_os_ioremap(asls, OPREGION_VBT_OFFSET);
 	if (!base)
 		return -ENOMEM;
 
@@ -929,7 +768,33 @@  int intel_opregion_setup(struct drm_device *dev)
 		goto err_out;
 	}
 	opregion->header = base;
-	opregion->vbt = base + OPREGION_VBT_OFFSET;
+	/* Assigning the alse to the mailbox 3 of the opregion */
+	opregion->asle = base + OPREGION_ASLE_OFFSET;
+
+	/*
+	 * Non-zero value in rvda field is an indication to driver that a
+	 * valid Raw VBT is stored in that address and driver should not refer
+	 * to mailbox4 for getting VBT.
+	 */
+	if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) {
+			size = opregion->asle->rvds;
+			vbt_base = acpi_os_ioremap(opregion->asle->rvda,
+					size);
+	} else {
+			size = OPREGION_SIZE - OPREGION_VBT_OFFSET;
+			vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET,
+					size);
+	}
+
+	bdb = validate_vbt(vbt_base, size, "OpRegion");
+
+	if (bdb == NULL) {
+		err = -EINVAL;
+		goto err_vbt;
+	}
+
+	dev_priv->bdb_start = bdb;
+	opregion->vbt = vbt_base;
 
 	opregion->lid_state = base + ACPI_CLID;
 
@@ -953,6 +818,8 @@  int intel_opregion_setup(struct drm_device *dev)
 
 	return 0;
 
+err_vbt:
+	iounmap(vbt_base);
 err_out:
 	iounmap(base);
 	return err;
diff --git a/drivers/gpu/drm/i915/intel_opregion.h b/drivers/gpu/drm/i915/intel_opregion.h
new file mode 100644
index 0000000..bcb45ec
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_opregion.h
@@ -0,0 +1,230 @@ 
+/*
+ * Copyright 2008 Intel Corporation <[e-mail]>
+ * Copyright 2008 Red Hat <[e-mail]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NON-INFRINGEMENT.  IN NO EVENT SHALL INTEL AND/OR ITS SUPPLIERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+#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
+#define   ACPI_CLID 0x01ac /* current lid state indicator */
+#define   ACPI_CDCK 0x01b0 /* current docking state indicator */
+#define OPREGION_SWSCI_OFFSET  0x200
+#define OPREGION_ASLE_OFFSET   0x300
+#define OPREGION_VBT_OFFSET    0x400
+
+#define OPREGION_SIGNATURE "IntelGraphicsMem"
+#define MBOX_ACPI      (1<<0)
+#define MBOX_SWSCI     (1<<1)
+#define MBOX_ASLE      (1<<2)
+#define MBOX_ASLE_EXT  (1<<4)
+
+struct opregion_header {
+	u8 signature[16];
+	u32 size;
+	u32 opregion_ver;
+	u8 bios_ver[32];
+	u8 vbios_ver[16];
+	u8 driver_ver[16];
+	u32 mboxes;
+	u32 driver_model;
+	u32 pcon;
+	u8 dver[32];
+	u8 rsvd[124];
+} __packed;
+
+/* OpRegion mailbox #1: public ACPI methods */
+struct opregion_acpi {
+	u32 drdy;       /* driver readiness */
+	u32 csts;       /* notification status */
+	u32 cevt;       /* current event */
+	u8 rsvd1[20];
+	u32 didl[8];    /* supported display devices ID list */
+	u32 cpdl[8];    /* currently presented display list */
+	u32 cadl[8];    /* currently active display list */
+	u32 nadl[8];    /* next active devices list */
+	u32 aslp;       /* ASL sleep time-out */
+	u32 tidx;       /* toggle table index */
+	u32 chpd;       /* current hotplug enable indicator */
+	u32 clid;       /* current lid state*/
+	u32 cdck;       /* current docking state */
+	u32 sxsw;       /* Sx state resume */
+	u32 evts;       /* ASL supported events */
+	u32 cnot;       /* current OS notification */
+	u32 nrdy;       /* driver status */
+	u32 did2[7];	/* extended supported display devices ID list */
+	u32 cpd2[7];	/* extended attached display devices list */
+	u8 rsvd2[4];
+} __packed;
+
+/* OpRegion mailbox #2: SWSCI */
+struct opregion_swsci {
+	u32 scic;       /* SWSCI command|status|data */
+	u32 parm;       /* command parameters */
+	u32 dslp;       /* driver sleep time-out */
+	u8 rsvd[244];
+} __packed;
+
+/* OpRegion mailbox #3: ASLE */
+struct opregion_asle {
+	u32 ardy;       /* driver readiness */
+	u32 aslc;       /* ASLE interrupt command */
+	u32 tche;       /* technology enabled indicator */
+	u32 alsi;       /* current ALS illuminance reading */
+	u32 bclp;       /* backlight brightness to set */
+	u32 pfit;       /* panel fitting state */
+	u32 cblv;       /* current brightness level */
+	u16 bclm[20];   /* backlight level duty cycle mapping table */
+	u32 cpfm;       /* current panel fitting mode */
+	u32 epfm;       /* enabled panel fitting modes */
+	u8 plut[74];    /* panel LUT and identifier */
+	u32 pfmb;       /* PWM freq and min brightness */
+	u32 cddv;       /* color correction default values */
+	u32 pcft;       /* power conservation features */
+	u32 srot;       /* supported rotation angles */
+	u32 iuer;       /* IUER events */
+	u64 fdss;
+	u32 fdsp;
+	u32 stat;
+	u64 rvda;	/* Physical address of raw vbt data */
+	u32 rvds;	/* Size of raw vbt data */
+	u8 rsvd[58];
+} __packed;
+
+/* Driver readiness indicator */
+#define ASLE_ARDY_READY		(1 << 0)
+#define ASLE_ARDY_NOT_READY	(0 << 0)
+
+/* ASLE Interrupt Command (ASLC) bits */
+#define ASLC_SET_ALS_ILLUM		(1 << 0)
+#define ASLC_SET_BACKLIGHT		(1 << 1)
+#define ASLC_SET_PFIT			(1 << 2)
+#define ASLC_SET_PWM_FREQ		(1 << 3)
+#define ASLC_SUPPORTED_ROTATION_ANGLES	(1 << 4)
+#define ASLC_BUTTON_ARRAY		(1 << 5)
+#define ASLC_CONVERTIBLE_INDICATOR	(1 << 6)
+#define ASLC_DOCKING_INDICATOR		(1 << 7)
+#define ASLC_ISCT_STATE_CHANGE		(1 << 8)
+#define ASLC_REQ_MSK			0x1ff
+/* response bits */
+#define ASLC_ALS_ILLUM_FAILED		(1 << 10)
+#define ASLC_BACKLIGHT_FAILED		(1 << 12)
+#define ASLC_PFIT_FAILED		(1 << 14)
+#define ASLC_PWM_FREQ_FAILED		(1 << 16)
+#define ASLC_ROTATION_ANGLES_FAILED	(1 << 18)
+#define ASLC_BUTTON_ARRAY_FAILED	(1 << 20)
+#define ASLC_CONVERTIBLE_FAILED		(1 << 22)
+#define ASLC_DOCKING_FAILED		(1 << 24)
+#define ASLC_ISCT_STATE_FAILED		(1 << 26)
+
+/* Technology enabled indicator */
+#define ASLE_TCHE_ALS_EN	(1 << 0)
+#define ASLE_TCHE_BLC_EN	(1 << 1)
+#define ASLE_TCHE_PFIT_EN	(1 << 2)
+#define ASLE_TCHE_PFMB_EN	(1 << 3)
+
+/* ASLE backlight brightness to set */
+#define ASLE_BCLP_VALID                (1<<31)
+#define ASLE_BCLP_MSK          (~(1<<31))
+
+/* ASLE panel fitting request */
+#define ASLE_PFIT_VALID         (1<<31)
+#define ASLE_PFIT_CENTER (1<<0)
+#define ASLE_PFIT_STRETCH_TEXT (1<<1)
+#define ASLE_PFIT_STRETCH_GFX (1<<2)
+
+/* PWM frequency and minimum brightness */
+#define ASLE_PFMB_BRIGHTNESS_MASK (0xff)
+#define ASLE_PFMB_BRIGHTNESS_VALID (1<<8)
+#define ASLE_PFMB_PWM_MASK (0x7ffffe00)
+#define ASLE_PFMB_PWM_VALID (1<<31)
+
+#define ASLE_CBLV_VALID         (1<<31)
+
+/* IUER */
+#define ASLE_IUER_DOCKING		(1 << 7)
+#define ASLE_IUER_CONVERTIBLE		(1 << 6)
+#define ASLE_IUER_ROTATION_LOCK_BTN	(1 << 4)
+#define ASLE_IUER_VOLUME_DOWN_BTN	(1 << 3)
+#define ASLE_IUER_VOLUME_UP_BTN		(1 << 2)
+#define ASLE_IUER_WINDOWS_BTN		(1 << 1)
+#define ASLE_IUER_POWER_BTN		(1 << 0)
+
+/* 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)
+#define ACPI_DIGITAL_OUTPUT (3<<8)
+#define ACPI_LVDS_OUTPUT (4<<8)
+
+#define MAX_DSLP	1500