[PATCH v3 1/3] drm/i915/opregion: fix version check

[PATCH v3 1/3] drm/i915/opregion: fix version check

[Jani Nikula]

The u32 version field encodes major, minor, revision and reserved. We've
basically been checking for any non-zero version.

Add opregion version logging while at it.

v2: Fix the fix of the version check

Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_opregion.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 30ae96c5c97c..f1b841580521 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -54,7 +54,12 @@
 struct opregion_header {
 	u8 signature[16];
 	u32 size;
-	u32 opregion_ver;
+	struct {
+		u8 rsvd;
+		u8 revision;
+		u8 minor;
+		u8 major;
+	}  __packed over;
 	u8 bios_ver[32];
 	u8 vbios_ver[16];
 	u8 driver_ver[16];
@@ -924,6 +929,11 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 	opregion->header = base;
 	opregion->lid_state = base + ACPI_CLID;
 
+	DRM_DEBUG_DRIVER("ACPI OpRegion version %u.%u.%u\n",
+			 opregion->header->over.major,
+			 opregion->header->over.minor,
+			 opregion->header->over.revision);
+
 	mboxes = opregion->header->mboxes;
 	if (mboxes & MBOX_ACPI) {
 		DRM_DEBUG_DRIVER("Public ACPI methods supported\n");
@@ -952,7 +962,7 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 	if (dmi_check_system(intel_no_opregion_vbt))
 		goto out;
 
-	if (opregion->header->opregion_ver >= 2 && opregion->asle &&
+	if (opregion->header->over.major >= 2 && opregion->asle &&
 	    opregion->asle->rvda && opregion->asle->rvds) {
 		opregion->rvda = memremap(opregion->asle->rvda,
 					  opregion->asle->rvds,
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

[Jani Nikula]

Starting from opregion version 2.1 (roughly corresponding to ICL+) the
RVDA field is relative from the beginning of opregion, not absolute
address.

Fix the error path while at it.

v2: Make relative vs. absolute conditional on the opregion version,
    bumped for the purpose. Turned out there are machines relying on
    absolute RVDA in the wild.

v3: Fix the version checks

Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_opregion.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index f1b841580521..5e00ee9270b5 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -123,7 +123,8 @@ struct opregion_asle {
 	u64 fdss;
 	u32 fdsp;
 	u32 stat;
-	u64 rvda;	/* Physical address of raw vbt data */
+	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
+			 * address of raw VBT data. */
 	u32 rvds;	/* Size of raw vbt data */
 	u8 rsvd[58];
 } __packed;
@@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 
 	if (opregion->header->over.major >= 2 && opregion->asle &&
 	    opregion->asle->rvda && opregion->asle->rvds) {
-		opregion->rvda = memremap(opregion->asle->rvda,
-					  opregion->asle->rvds,
+		resource_size_t rvda = opregion->asle->rvda;
+
+		/*
+		 * opregion 2.0: rvda is the physical VBT address.
+		 *
+		 * opregion 2.1+: rvda is unsigned, relative offset from
+		 * opregion base, and should never point within opregion.
+		 */
+		if (opregion->header->over.major > 2 ||
+		    opregion->header->over.minor >= 1) {
+			WARN_ON(rvda < OPREGION_SIZE);
+
+			rvda += asls;
+		}
+
+		opregion->rvda = memremap(rvda, opregion->asle->rvds,
 					  MEMREMAP_WB);
+
 		vbt = opregion->rvda;
 		vbt_size = opregion->asle->rvds;
 		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
@@ -976,6 +992,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 			goto out;
 		} else {
 			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
+			memunmap(opregion->rvda);
+			opregion->rvda = NULL;
 		}
 	}
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 3/3] HACK: drm/i915/opregion: ICL should have opregion 2.1+ and relative rvda

[Jani Nikula]

Temporary CI workaround before proper opregion version bump propagates
to the machines.

v2: Rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_opregion.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 5e00ee9270b5..5437effcb396 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -974,7 +974,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 		 * opregion base, and should never point within opregion.
 		 */
 		if (opregion->header->over.major > 2 ||
-		    opregion->header->over.minor >= 1) {
+		    opregion->header->over.minor >= 1 ||
+		    IS_ICELAKE(dev_priv)) {
 			WARN_ON(rvda < OPREGION_SIZE);
 
 			rvda += asls;
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915/opregion: fix version check

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/opregion: fix version check
URL   : https://patchwork.freedesktop.org/series/56429/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1fd6ccae8d76 drm/i915/opregion: fix version check
cb64272c9352 drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+
-:37: WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#37: FILE: drivers/gpu/drm/i915/intel_opregion.c:127:
+			 * address of raw VBT data. */

total: 0 errors, 1 warnings, 0 checks, 43 lines checked
a309bdc64118 HACK: drm/i915/opregion: ICL should have opregion 2.1+ and relative rvda

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

[Ville Syrjälä]

On Fri, Feb 08, 2019 at 08:42:53PM +0200, Jani Nikula wrote:
> Starting from opregion version 2.1 (roughly corresponding to ICL+) the
> RVDA field is relative from the beginning of opregion, not absolute
> address.
> 
> Fix the error path while at it.
> 
> v2: Make relative vs. absolute conditional on the opregion version,
>     bumped for the purpose. Turned out there are machines relying on
>     absolute RVDA in the wild.
> 
> v3: Fix the version checks
> 
> Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index f1b841580521..5e00ee9270b5 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -123,7 +123,8 @@ struct opregion_asle {
>  	u64 fdss;
>  	u32 fdsp;
>  	u32 stat;
> -	u64 rvda;	/* Physical address of raw vbt data */
> +	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
> +			 * address of raw VBT data. */
>  	u32 rvds;	/* Size of raw vbt data */
>  	u8 rsvd[58];
>  } __packed;
> @@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  
>  	if (opregion->header->over.major >= 2 && opregion->asle &&
>  	    opregion->asle->rvda && opregion->asle->rvds) {
> -		opregion->rvda = memremap(opregion->asle->rvda,
> -					  opregion->asle->rvds,
> +		resource_size_t rvda = opregion->asle->rvda;
> +
> +		/*
> +		 * opregion 2.0: rvda is the physical VBT address.
> +		 *
> +		 * opregion 2.1+: rvda is unsigned, relative offset from
> +		 * opregion base, and should never point within opregion.
> +		 */
> +		if (opregion->header->over.major > 2 ||
> +		    opregion->header->over.minor >= 1) {

What happens with version 1.1?

> +			WARN_ON(rvda < OPREGION_SIZE);
> +
> +			rvda += asls;
> +		}
> +
> +		opregion->rvda = memremap(rvda, opregion->asle->rvds,
>  					  MEMREMAP_WB);
> +
>  		vbt = opregion->rvda;
>  		vbt_size = opregion->asle->rvds;
>  		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> @@ -976,6 +992,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  			goto out;
>  		} else {
>  			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
> +			memunmap(opregion->rvda);
> +			opregion->rvda = NULL;
>  		}
>  	}
>  
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

[Jani Nikula]

On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Feb 08, 2019 at 08:42:53PM +0200, Jani Nikula wrote:
>> Starting from opregion version 2.1 (roughly corresponding to ICL+) the
>> RVDA field is relative from the beginning of opregion, not absolute
>> address.
>> 
>> Fix the error path while at it.
>> 
>> v2: Make relative vs. absolute conditional on the opregion version,
>>     bumped for the purpose. Turned out there are machines relying on
>>     absolute RVDA in the wild.
>> 
>> v3: Fix the version checks
>> 
>> Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_opregion.c | 24 +++++++++++++++++++++---
>>  1 file changed, 21 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> index f1b841580521..5e00ee9270b5 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -123,7 +123,8 @@ struct opregion_asle {
>>  	u64 fdss;
>>  	u32 fdsp;
>>  	u32 stat;
>> -	u64 rvda;	/* Physical address of raw vbt data */
>> +	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
>> +			 * address of raw VBT data. */
>>  	u32 rvds;	/* Size of raw vbt data */
>>  	u8 rsvd[58];
>>  } __packed;
>> @@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>>  
>>  	if (opregion->header->over.major >= 2 && opregion->asle &&

Push this line to short term memory.

>>  	    opregion->asle->rvda && opregion->asle->rvds) {
>> -		opregion->rvda = memremap(opregion->asle->rvda,
>> -					  opregion->asle->rvds,
>> +		resource_size_t rvda = opregion->asle->rvda;
>> +
>> +		/*
>> +		 * opregion 2.0: rvda is the physical VBT address.
>> +		 *
>> +		 * opregion 2.1+: rvda is unsigned, relative offset from
>> +		 * opregion base, and should never point within opregion.
>> +		 */
>> +		if (opregion->header->over.major > 2 ||
>> +		    opregion->header->over.minor >= 1) {
>
> What happens with version 1.1?

Pop. ;)

BR,
Jani.

>
>> +			WARN_ON(rvda < OPREGION_SIZE);
>> +
>> +			rvda += asls;
>> +		}
>> +
>> +		opregion->rvda = memremap(rvda, opregion->asle->rvds,
>>  					  MEMREMAP_WB);
>> +
>>  		vbt = opregion->rvda;
>>  		vbt_size = opregion->asle->rvds;
>>  		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>> @@ -976,6 +992,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>>  			goto out;
>>  		} else {
>>  			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
>> +			memunmap(opregion->rvda);
>> +			opregion->rvda = NULL;
>>  		}
>>  	}
>>  
>> -- 
>> 2.20.1

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

✓ Fi.CI.BAT: success for series starting with [v3,1/3] drm/i915/opregion: fix version check

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/opregion: fix version check
URL   : https://patchwork.freedesktop.org/series/56429/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5568 -> Patchwork_12181
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56429/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12181 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       PASS -> DMESG-FAIL [fdo#105079]

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6700hq:      PASS -> DMESG-WARN [fdo#105998]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@pm_rpm@module-reload:
    - fi-skl-6600u:       PASS -> DMESG-WARN [fdo#109513]
    - fi-kbl-7567u:       PASS -> DMESG-WARN [fdo#108529]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u2}:        FAIL [fdo#103167] -> PASS

  * igt@pm_rpm@module-reload:
    - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS
    - {fi-whl-u}:         DMESG-WARN [fdo#109513] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         DMESG-FAIL [fdo#103182] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109513]: https://bugs.freedesktop.org/show_bug.cgi?id=109513


Participating hosts (48 -> 44)
------------------------------

  Additional (2): fi-icl-u3 fi-pnv-d510 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-glk-j4005 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5568 -> Patchwork_12181

  CI_DRM_5568: d4794b009ccd1ef8816e15c833f07ab696911a8d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12181: a309bdc64118ee6306c289cc03e68d7dc27de907 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a309bdc64118 HACK: drm/i915/opregion: ICL should have opregion 2.1+ and relative rvda
cb64272c9352 drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+
1fd6ccae8d76 drm/i915/opregion: fix version check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12181/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

[Ville Syrjälä]

On Fri, Feb 08, 2019 at 09:00:59PM +0200, Jani Nikula wrote:
> On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Feb 08, 2019 at 08:42:53PM +0200, Jani Nikula wrote:
> >> Starting from opregion version 2.1 (roughly corresponding to ICL+) the
> >> RVDA field is relative from the beginning of opregion, not absolute
> >> address.
> >> 
> >> Fix the error path while at it.
> >> 
> >> v2: Make relative vs. absolute conditional on the opregion version,
> >>     bumped for the purpose. Turned out there are machines relying on
> >>     absolute RVDA in the wild.
> >> 
> >> v3: Fix the version checks
> >> 
> >> Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Imre Deak <imre.deak@intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_opregion.c | 24 +++++++++++++++++++++---
> >>  1 file changed, 21 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> >> index f1b841580521..5e00ee9270b5 100644
> >> --- a/drivers/gpu/drm/i915/intel_opregion.c
> >> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> >> @@ -123,7 +123,8 @@ struct opregion_asle {
> >>  	u64 fdss;
> >>  	u32 fdsp;
> >>  	u32 stat;
> >> -	u64 rvda;	/* Physical address of raw vbt data */
> >> +	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
> >> +			 * address of raw VBT data. */
> >>  	u32 rvds;	/* Size of raw vbt data */
> >>  	u8 rsvd[58];
> >>  } __packed;
> >> @@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
> >>  
> >>  	if (opregion->header->over.major >= 2 && opregion->asle &&
> 
> Push this line to short term memory.

Stack overflow.

Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> >>  	    opregion->asle->rvda && opregion->asle->rvds) {
> >> -		opregion->rvda = memremap(opregion->asle->rvda,
> >> -					  opregion->asle->rvds,
> >> +		resource_size_t rvda = opregion->asle->rvda;
> >> +
> >> +		/*
> >> +		 * opregion 2.0: rvda is the physical VBT address.
> >> +		 *
> >> +		 * opregion 2.1+: rvda is unsigned, relative offset from
> >> +		 * opregion base, and should never point within opregion.
> >> +		 */
> >> +		if (opregion->header->over.major > 2 ||
> >> +		    opregion->header->over.minor >= 1) {
> >
> > What happens with version 1.1?
> 
> Pop. ;)
> 
> BR,
> Jani.
> 
> >
> >> +			WARN_ON(rvda < OPREGION_SIZE);
> >> +
> >> +			rvda += asls;
> >> +		}
> >> +
> >> +		opregion->rvda = memremap(rvda, opregion->asle->rvds,
> >>  					  MEMREMAP_WB);
> >> +
> >>  		vbt = opregion->rvda;
> >>  		vbt_size = opregion->asle->rvds;
> >>  		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
> >> @@ -976,6 +992,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
> >>  			goto out;
> >>  		} else {
> >>  			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
> >> +			memunmap(opregion->rvda);
> >> +			opregion->rvda = NULL;
> >>  		}
> >>  	}
> >>  
> >> -- 
> >> 2.20.1
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v3,1/3] drm/i915/opregion: fix version check

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/opregion: fix version check
URL   : https://patchwork.freedesktop.org/series/56429/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5568_full -> Patchwork_12181_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12181_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@shrink:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#109244]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-glk:          PASS -> FAIL [fdo#109350]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_universal_plane@universal-plane-pipe-b-functional:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_universal_plane@universal-plane-pipe-c-functional:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1
    - shard-apl:          PASS -> FAIL [fdo#103166]

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-hsw:          INCOMPLETE [fdo#103540] / [fdo#109482] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +3

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
    - shard-snb:          DMESG-FAIL [fdo#107469] -> INCOMPLETE [fdo#105411]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109482]: https://bugs.freedesktop.org/show_bug.cgi?id=109482
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5568 -> Patchwork_12181

  CI_DRM_5568: d4794b009ccd1ef8816e15c833f07ab696911a8d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4815: 947301563259726b65ce47d3a3fe37931ed42efe @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12181: a309bdc64118ee6306c289cc03e68d7dc27de907 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12181/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

[Jani Nikula]

On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Feb 08, 2019 at 09:00:59PM +0200, Jani Nikula wrote:
>> On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Fri, Feb 08, 2019 at 08:42:53PM +0200, Jani Nikula wrote:
>> >> Starting from opregion version 2.1 (roughly corresponding to ICL+) the
>> >> RVDA field is relative from the beginning of opregion, not absolute
>> >> address.
>> >> 
>> >> Fix the error path while at it.
>> >> 
>> >> v2: Make relative vs. absolute conditional on the opregion version,
>> >>     bumped for the purpose. Turned out there are machines relying on
>> >>     absolute RVDA in the wild.
>> >> 
>> >> v3: Fix the version checks
>> >> 
>> >> Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Cc: Imre Deak <imre.deak@intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_opregion.c | 24 +++++++++++++++++++++---
>> >>  1 file changed, 21 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> >> index f1b841580521..5e00ee9270b5 100644
>> >> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> >> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> >> @@ -123,7 +123,8 @@ struct opregion_asle {
>> >>  	u64 fdss;
>> >>  	u32 fdsp;
>> >>  	u32 stat;
>> >> -	u64 rvda;	/* Physical address of raw vbt data */
>> >> +	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
>> >> +			 * address of raw VBT data. */
>> >>  	u32 rvds;	/* Size of raw vbt data */
>> >>  	u8 rsvd[58];
>> >>  } __packed;
>> >> @@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>> >>  
>> >>  	if (opregion->header->over.major >= 2 && opregion->asle &&
>> 
>> Push this line to short term memory.
>
> Stack overflow.
>
> Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, pushed patches 1&2 to dinq.

BR,
Jani.

>
>> 
>> >>  	    opregion->asle->rvda && opregion->asle->rvds) {
>> >> -		opregion->rvda = memremap(opregion->asle->rvda,
>> >> -					  opregion->asle->rvds,
>> >> +		resource_size_t rvda = opregion->asle->rvda;
>> >> +
>> >> +		/*
>> >> +		 * opregion 2.0: rvda is the physical VBT address.
>> >> +		 *
>> >> +		 * opregion 2.1+: rvda is unsigned, relative offset from
>> >> +		 * opregion base, and should never point within opregion.
>> >> +		 */
>> >> +		if (opregion->header->over.major > 2 ||
>> >> +		    opregion->header->over.minor >= 1) {
>> >
>> > What happens with version 1.1?
>> 
>> Pop. ;)
>> 
>> BR,
>> Jani.
>> 
>> >
>> >> +			WARN_ON(rvda < OPREGION_SIZE);
>> >> +
>> >> +			rvda += asls;
>> >> +		}
>> >> +
>> >> +		opregion->rvda = memremap(rvda, opregion->asle->rvds,
>> >>  					  MEMREMAP_WB);
>> >> +
>> >>  		vbt = opregion->rvda;
>> >>  		vbt_size = opregion->asle->rvds;
>> >>  		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>> >> @@ -976,6 +992,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>> >>  			goto out;
>> >>  		} else {
>> >>  			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
>> >> +			memunmap(opregion->rvda);
>> >> +			opregion->rvda = NULL;
>> >>  		}
>> >>  	}
>> >>  
>> >> -- 
>> >> 2.20.1
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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