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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Feb 08, 2019 at 05:09:51PM +0200, Jani Nikula wrote:
> On Fri, 08 Feb 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> > The u32 version field encodes major version in the high word. We've been
> > checking for version >= 0.2.
> >
> > Add opregion version logging while at it.
> >
> > 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 | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 30ae96c5c97c..7e4152d97c45 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -46,6 +46,9 @@
> >  #define OPREGION_ASLE_EXT_OFFSET	0x1C00
> >  
> >  #define OPREGION_SIGNATURE "IntelGraphicsMem"
> > +
> > +#define OPREGION_VERSION(major, minor) (((major) << 16) | (minor))
> > +
> >  #define MBOX_ACPI      (1<<0)
> >  #define MBOX_SWSCI     (1<<1)
> >  #define MBOX_ASLE      (1<<2)
> > @@ -924,6 +927,10 @@ 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\n",
> > +			 opregion->header->opregion_ver >> 16,
> > +			 opregion->header->opregion_ver & 0xffff);
> > +
> 
> This is ridiculous and maddening. On our CI APL this prints [1]:
> 
> <7>[    7.029368] [drm:intel_opregion_setup [i915]] ACPI OpRegion version 512.0
> 
> Yes, version 512.0.
> 
> Digging into it, I found one random version of the opregion spec that
> has:
> 
> 	Bits [31:16] - Major Version Number
> 	Bits [23:0] - Minor Version Number
> 
> The minor is supposed to be [15:0]. But this APL (maybe others, need to
> investigate) has the major version encoded to bits 31:24.

Most copies of the spec I have seem to have the 24 + 16 bits mess.

They also claim that HSW+ should generally have version 3.0. So
not sure this 2.0 vs. 2.1 business even makes sense.

We should probably grab the opregion from all the machines we have
around and see what they actully look like.

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