On Mon, 14 Dec 2015, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, Dec 14, 2015 at 05:46:41PM +0530, Deepak M 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. >> >> v3: -Splitted the patch into small ones >> -Handeled memory unmap in intel_opregion_fini >> -removed the new file created for opregion macro`s >> v4: Moving the vbt assignment after the opregion fields are assigned >> >> Cc: Mika Kahola <mika.kahola@xxxxxxxxx> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> >> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx> >> --- >> >> drivers/gpu/drm/i915/intel_opregion.c | 47 +++++++++++++++++++++++++---------- >> 1 file changed, 34 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c >> index 7908a1d..5116690 100644 >> --- a/drivers/gpu/drm/i915/intel_opregion.c >> +++ b/drivers/gpu/drm/i915/intel_opregion.c >> @@ -856,6 +856,8 @@ void intel_opregion_fini(struct drm_device *dev) >> } >> >> /* just clear all opregion memory pointers now */ >> + if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) >> + memunmap(opregion->vbt); >> memunmap(opregion->header); >> opregion->header = NULL; >> opregion->acpi = NULL; >> @@ -933,7 +935,8 @@ int intel_opregion_setup(struct drm_device *dev) >> char buf[sizeof(OPREGION_SIGNATURE)]; >> const struct vbt_header *vbt = NULL; >> int err = 0; >> - void *base; >> + void *base, *vbt_base; >> + size_t size; >> >> BUILD_BUG_ON(sizeof(struct opregion_header) != 0x100); >> BUILD_BUG_ON(sizeof(struct opregion_acpi) != 0x100); >> @@ -963,19 +966,7 @@ int intel_opregion_setup(struct drm_device *dev) >> goto err_out; >> } >> >> - vbt = validate_vbt(base + OPREGION_VBT_OFFSET, >> - MAILBOX_4_SIZE, "OpRegion"); >> - >> - if (vbt == NULL) { >> - err = -EINVAL; >> - goto err_out; >> - } >> - >> - vbt = (const struct vbt_header *)(base + OPREGION_VBT_OFFSET); >> - dev_priv->opregion.vbt_size = vbt->vbt_size; >> - >> opregion->header = base; >> - opregion->vbt = base + OPREGION_VBT_OFFSET; >> >> opregion->lid_state = base + ACPI_CLID; >> opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET; >> @@ -998,6 +989,36 @@ int intel_opregion_setup(struct drm_device *dev) >> opregion->asle->ardy = ASLE_ARDY_NOT_READY; >> } >> >> + /* >> + * 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 = memremap(opregion->asle->rvda, >> + size, MEMREMAP_WB); >> + } else { >> + size = MAILBOX_4_SIZE; >> + vbt_base = base + OPREGION_VBT_OFFSET; >> + } >> + >> + vbt = validate_vbt(vbt_base, size, "OpRegion"); >> + >> + if (vbt == NULL) { >> + err = -EINVAL; >> + goto err_out; >> + } >> + >> + /* Assigning the vbt_size based on the VBT location */ >> + if (opregion->header->opregion_ver >= 2 && opregion->asle->rvda) >> + dev_priv->opregion.vbt_size = opregion->asle->rvds; >> + else { >> + vbt = (const struct vbt_header *)(base + OPREGION_VBT_OFFSET); > i.e. vbt = vbt_base; > > which is already done by vbt = validate_vbt; > >> + dev_priv->opregion.vbt_size = vbt->vbt_size; >> + } > > So this reduces down to: > > /* Explanation why the new method cannot store the size in vbt->vbt_size */ > if (vbt != opregion->asle->rvda) > size = vbt->vbt_size; > dev_priv->opregion.vbt_size = size; > opregrion->vbt = vbt; > > And the vbt_base variable is redundant. > > Cut out the tautology and reduce the apparent complex > interdependence between paths. I rewrote patches 2-6 in this series into a new VBT/opregion refactoring series [1] that should clean this up. BR, Jani. [1] http://mid.gmane.org/cover.1450089383.git.jani.nikula@xxxxxxxxx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx