On Mon, 14 Dec 2015, Chris Wilson <ch...@chris-wilson.co.uk> 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.kah...@intel.com>
>> Cc: Jani Nikula <jani.nik...@intel.com>
>> Signed-off-by: Deepak M <m.dee...@intel.com>
>> ---
>> 
>>  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.nik...@intel.com



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to