| Message ID | 1438077670-12953-3-git-send-email-m.deepak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 28 Jul 2015, Deepak M <m.deepak@intel.com> 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. > > Signed-off-by: Deepak M <m.deepak@intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 13 +++++++---- > drivers/gpu/drm/i915/intel_opregion.c | 39 ++++++++++++++++++++++++++++++--- > 2 files changed, 45 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 2583587..1b9164e 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1219,6 +1219,7 @@ intel_parse_bios(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct pci_dev *pdev = dev->pdev; > const struct bdb_header *bdb = NULL; > + const struct vbt_header *vbt = NULL; > u8 __iomem *bios = NULL; > > if (HAS_PCH_NOP(dev)) > @@ -1226,10 +1227,14 @@ 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"); > + if (!dmi_check_system(intel_no_opregion_vbt) && > + dev_priv->opregion.vbt) { > + vbt = (struct vbt_header *)dev_priv->opregion.vbt; > + bdb = (struct bdb_header *)(dev_priv->opregion.vbt + > + vbt->bdb_offset); > + DRM_DEBUG_KMS("Using VBT from Opregion: %20s\n", > + vbt->signature); > + } Please read the comment in the beginning of validate_vbt. Please keep things that way. I put in some effort to clean this up and get rid of a bunch of extra casts, so please don't add new ones back. You should probably move some of this stuff to intel_opregion.c and have a function to return the struct bdb_header *pointer from there. Please also look into in i915_opregion in i915_debugfs.c, and fix that. > > 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 71e87ab..1372e39 100644 > --- a/drivers/gpu/drm/i915/intel_opregion.c > +++ b/drivers/gpu/drm/i915/intel_opregion.c > @@ -50,6 +50,7 @@ > #define OPREGION_VBT_OFFSET 0x400 > > #define OPREGION_SIGNATURE "IntelGraphicsMem" > +#define VBT_SIGNATURE "$VBT" Adding this does you no good when you're duplicating this from intel_bios.c and not removing any from there... really the check should be in one place only. > #define MBOX_ACPI (1<<0) > #define MBOX_SWSCI (1<<1) > #define MBOX_ASLE (1<<2) > @@ -113,7 +114,12 @@ struct opregion_asle { > u32 pcft; /* power conservation features */ > u32 srot; /* supported rotation angles */ > u32 iuer; /* IUER events */ > - u8 rsvd[86]; > + u64 fdss; /* DSS Buffer address allocated for IFFS feature */ > + u32 fdsp; /* Size of DSS Buffer */ > + u32 stat; /* State Indicator */ > + u64 rvda; /* Physical address of raw vbt data */ > + u32 rvds; /* Size of raw vbt data */ > + u8 rsvd[58]; These should be a prep patch that could be merged quickly with a check against the spec. > } __packed; > > /* Driver readiness indicator */ > @@ -858,8 +864,10 @@ 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)]; > + char vbt_sig_buf[sizeof(VBT_SIGNATURE)]; > int err = 0; > > pci_read_config_dword(dev->pdev, PCI_ASLS, &asls); > @@ -873,7 +881,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; > > @@ -884,8 +892,31 @@ int intel_opregion_setup(struct drm_device *dev) > err = -EINVAL; > goto err_out; > } > + > opregion->header = base; > - opregion->vbt = base + OPREGION_VBT_OFFSET; > + opregion->asle = base + OPREGION_ASLE_OFFSET; Why is this addition needed or even correct? > + > + if (opregion->header->opregion_ver >= 2) { > + if (opregion->asle->rvda) > + vbt_base = acpi_os_ioremap(opregion->asle->rvda, > + opregion->asle->rvds); > + else > + vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET, > + OPREGION_SIZE - OPREGION_VBT_OFFSET); > + } else > + vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET, > + OPREGION_SIZE - OPREGION_VBT_OFFSET); The two else branches are identical. > + > + > + memcpy_fromio(vbt_sig_buf, vbt_base, sizeof(vbt_sig_buf)); This is silly, just do what validate_vbt does. (I know, there's silliness for the opregion signature there already.) > + > + if (memcmp(vbt_sig_buf, VBT_SIGNATURE, 4)) { > + DRM_ERROR("VBT signature mismatch\n"); > + err = -EINVAL; > + goto err_vbt; > + } > + > + opregion->vbt = vbt_base; > > opregion->lid_state = base + ACPI_CLID; > > @@ -909,6 +940,8 @@ int intel_opregion_setup(struct drm_device *dev) > > return 0; > > +err_vbt: > + iounmap(vbt_base); > err_out: > iounmap(base); > return err; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 2583587..1b9164e 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1219,6 +1219,7 @@ intel_parse_bios(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct pci_dev *pdev = dev->pdev; const struct bdb_header *bdb = NULL; + const struct vbt_header *vbt = NULL; u8 __iomem *bios = NULL; if (HAS_PCH_NOP(dev)) @@ -1226,10 +1227,14 @@ 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"); + if (!dmi_check_system(intel_no_opregion_vbt) && + dev_priv->opregion.vbt) { + vbt = (struct vbt_header *)dev_priv->opregion.vbt; + bdb = (struct bdb_header *)(dev_priv->opregion.vbt + + vbt->bdb_offset); + DRM_DEBUG_KMS("Using VBT from Opregion: %20s\n", + vbt->signature); + } 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 71e87ab..1372e39 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -50,6 +50,7 @@ #define OPREGION_VBT_OFFSET 0x400 #define OPREGION_SIGNATURE "IntelGraphicsMem" +#define VBT_SIGNATURE "$VBT" #define MBOX_ACPI (1<<0) #define MBOX_SWSCI (1<<1) #define MBOX_ASLE (1<<2) @@ -113,7 +114,12 @@ struct opregion_asle { u32 pcft; /* power conservation features */ u32 srot; /* supported rotation angles */ u32 iuer; /* IUER events */ - u8 rsvd[86]; + u64 fdss; /* DSS Buffer address allocated for IFFS feature */ + u32 fdsp; /* Size of DSS Buffer */ + u32 stat; /* State Indicator */ + u64 rvda; /* Physical address of raw vbt data */ + u32 rvds; /* Size of raw vbt data */ + u8 rsvd[58]; } __packed; /* Driver readiness indicator */ @@ -858,8 +864,10 @@ 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)]; + char vbt_sig_buf[sizeof(VBT_SIGNATURE)]; int err = 0; pci_read_config_dword(dev->pdev, PCI_ASLS, &asls); @@ -873,7 +881,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; @@ -884,8 +892,31 @@ int intel_opregion_setup(struct drm_device *dev) err = -EINVAL; goto err_out; } + opregion->header = base; - opregion->vbt = base + OPREGION_VBT_OFFSET; + opregion->asle = base + OPREGION_ASLE_OFFSET; + + if (opregion->header->opregion_ver >= 2) { + if (opregion->asle->rvda) + vbt_base = acpi_os_ioremap(opregion->asle->rvda, + opregion->asle->rvds); + else + vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET, + OPREGION_SIZE - OPREGION_VBT_OFFSET); + } else + vbt_base = acpi_os_ioremap(asls + OPREGION_VBT_OFFSET, + OPREGION_SIZE - OPREGION_VBT_OFFSET); + + + memcpy_fromio(vbt_sig_buf, vbt_base, sizeof(vbt_sig_buf)); + + if (memcmp(vbt_sig_buf, VBT_SIGNATURE, 4)) { + DRM_ERROR("VBT signature mismatch\n"); + err = -EINVAL; + goto err_vbt; + } + + opregion->vbt = vbt_base; opregion->lid_state = base + ACPI_CLID; @@ -909,6 +940,8 @@ int intel_opregion_setup(struct drm_device *dev) return 0; +err_vbt: + iounmap(vbt_base); err_out: iounmap(base); return err;
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. Signed-off-by: Deepak M <m.deepak@intel.com> --- drivers/gpu/drm/i915/intel_bios.c | 13 +++++++---- drivers/gpu/drm/i915/intel_opregion.c | 39 ++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 7 deletions(-)