drm/i915: Parsing VBT if size of VBT exceeds 6KB

Submitted by Deepak M on Dec. 14, 2015, 12:16 p.m.

Details

Message ID 1450095401-24259-1-git-send-email-m.deepak@intel.com
State New
Headers show
Series "Patches to support the version 3 of MIPI sequence in VBT." ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Deepak M Dec. 14, 2015, 12:16 p.m.
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@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
---

 drivers/gpu/drm/i915/intel_opregion.c | 47 +++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 13 deletions(-)

Patch hide | download patch | download mbox

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);
+		dev_priv->opregion.vbt_size = vbt->vbt_size;
+	}
+
+	opregion->vbt = vbt_base;
 	return 0;
 
 err_out:

Comments

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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Deepak M <m.deepak@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.
-Chris
On Mon, 14 Dec 2015, Chris Wilson <chris@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.kahola@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Deepak M <m.deepak@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.nikula@intel.com