diff mbox series

[v4,02/11] drm/i915: Read graphics/media/display arch version from hw

Message ID 20220902060342.151824-3-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series Initial Meteorlake Support | expand

Commit Message

Sripada, Radhakrishna Sept. 2, 2022, 6:03 a.m. UTC 
From: Matt Roper <matthew.d.roper@intel.com>

Going forward, the hardware teams no longer consider new platforms to
have a "generation" in the way we've defined it for past platforms.
Instead, each IP block (graphics, media, display) will have their own
architecture major.minor versions and stepping ID's which should be read
directly from a register in the MMIO space.  New hardware programming
styles, features, and workarounds should be conditional solely on the
architecture version, and should no longer be derived from the PCI
device ID, revision ID, or platform-specific feature flags.

Bspec: 63361, 64111

v2:
  - Move the IP version readout to intel_device_info.c
  - Convert the macro into a function

v3:
  - Move subplatform init to runtime early init
  - Cache runtime ver, release info to compare with hardware values.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  2 +
 drivers/gpu/drm/i915/i915_driver.c       |  3 +-
 drivers/gpu/drm/i915/i915_drv.h          |  2 +
 drivers/gpu/drm/i915/i915_pci.c          |  1 +
 drivers/gpu/drm/i915/i915_reg.h          |  7 +++
 drivers/gpu/drm/i915/intel_device_info.c | 74 +++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_device_info.h | 12 +++-
 7 files changed, 98 insertions(+), 3 deletions(-)

Comments

Lucas De Marchi Sept. 7, 2022, 8:49 p.m. UTC | #1 
On Thu, Sep 01, 2022 at 11:03:33PM -0700, Radhakrishna Sripada wrote:
>From: Matt Roper <matthew.d.roper@intel.com>
>
>Going forward, the hardware teams no longer consider new platforms to
>have a "generation" in the way we've defined it for past platforms.
>Instead, each IP block (graphics, media, display) will have their own
>architecture major.minor versions and stepping ID's which should be read
>directly from a register in the MMIO space.  New hardware programming
>styles, features, and workarounds should be conditional solely on the
>architecture version, and should no longer be derived from the PCI
>device ID, revision ID, or platform-specific feature flags.
>
>Bspec: 63361, 64111
>
>v2:
>  - Move the IP version readout to intel_device_info.c
>  - Convert the macro into a function
>
>v3:
>  - Move subplatform init to runtime early init
>  - Cache runtime ver, release info to compare with hardware values.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  2 +
> drivers/gpu/drm/i915/i915_driver.c       |  3 +-
> drivers/gpu/drm/i915/i915_drv.h          |  2 +
> drivers/gpu/drm/i915/i915_pci.c          |  1 +
> drivers/gpu/drm/i915/i915_reg.h          |  7 +++
> drivers/gpu/drm/i915/intel_device_info.c | 74 +++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_device_info.h | 12 +++-
> 7 files changed, 98 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>index d414785003cc..579da62158c4 100644
>--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>@@ -39,6 +39,8 @@
> #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
> #define FORCEWAKE_ACK_MEDIA_GEN9		_MMIO(0xd88)
>
>+#define GMD_ID_GRAPHICS				_MMIO(0xd8c)
>+
> #define MCFG_MCR_SELECTOR			_MMIO(0xfd0)
> #define SF_MCR_SELECTOR				_MMIO(0xfd8)
> #define GEN8_MCR_SELECTOR			_MMIO(0xfdc)
>diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>index 56a2bcddb2af..a1ab49521d19 100644
>--- a/drivers/gpu/drm/i915/i915_driver.c
>+++ b/drivers/gpu/drm/i915/i915_driver.c
>@@ -323,7 +323,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> 	if (i915_inject_probe_failure(dev_priv))
> 		return -ENODEV;
>
>-	intel_device_info_subplatform_init(dev_priv);
>+	intel_device_info_runtime_init_early(dev_priv);
>+
> 	intel_step_init(dev_priv);
>
> 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index f85a470397a5..405b59b8c05c 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -936,6 +936,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>
> #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
>
>+#define HAS_GMD_ID(i915)	INTEL_INFO(i915)->has_gmd_id
>+
> #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
>
> #define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>index f6aaf938c53c..4672894f4bc1 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -1129,6 +1129,7 @@ static const struct intel_device_info mtl_info = {
> 	PLATFORM(INTEL_METEORLAKE),
> 	.display.has_modular_fia = 1,
> 	.has_flat_ccs = 0,
>+	.has_gmd_id = 1,
> 	.has_snoop = 1,
> 	.__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
> 	.__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 5e6239864c35..e02e461a4b5d 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -5798,6 +5798,11 @@
> #define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> #define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
>
>+#define GMD_ID_DISPLAY				_MMIO(0x510a0)
>+#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
>+#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
>+#define   GMD_ID_STEP				REG_GENMASK(5, 0)
>+
> /*GEN11 chicken */
> #define _PIPEA_CHICKEN				0x70038
> #define _PIPEB_CHICKEN				0x71038
>@@ -8298,4 +8303,6 @@ enum skl_power_gate {
> #define  MTL_LATENCY_LEVEL_EVEN_MASK	REG_GENMASK(12, 0)
> #define  MTL_LATENCY_LEVEL_ODD_MASK	REG_GENMASK(28, 16)
>
>+#define MTL_MEDIA_GSI_BASE		0x380000
>+
> #endif /* _I915_REG_H_ */
>diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>index 56f19683dd55..a5bafc9be1fa 100644
>--- a/drivers/gpu/drm/i915/intel_device_info.c
>+++ b/drivers/gpu/drm/i915/intel_device_info.c
>@@ -29,6 +29,7 @@
>
> #include "display/intel_cdclk.h"
> #include "display/intel_de.h"
>+#include "gt/intel_gt_regs.h"
> #include "intel_device_info.h"
> #include "i915_drv.h"
> #include "i915_utils.h"
>@@ -231,7 +232,7 @@ static bool find_devid(u16 id, const u16 *p, unsigned int num)
> 	return false;
> }
>
>-void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>+static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> {
> 	const struct intel_device_info *info = INTEL_INFO(i915);
> 	const struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
>@@ -288,6 +289,77 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
> }
>
>+static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct ip_version *ip)
>+{
>+	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>+	void __iomem *addr;
>+	u32 val;
>+	u8 ver = ip->ver;
>+	u8 rel = ip->rel;
>+
>+	addr = pci_iomap_range(pdev, 0, offset, sizeof(u32));
>+	if (drm_WARN_ON(&i915->drm, !addr))
>+		return;
>+
>+	val = ioread32(addr);
>+	pci_iounmap(pdev, addr);
>+
>+	ip->ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val);
>+	ip->rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
>+	ip->step = REG_FIELD_GET(GMD_ID_STEP, val);
>+
>+	/* Sanity check against expected versions from device info */
>+	if (ip->ver != ver || ip->rel > rel)

this doesn't seem correct.. if we have

	ip->ver == 12, 	ip->rel == 1
	ver == 12, rel == 0

we will print

	Hardware reports GMD IP version 12.1 but minimum expected is 12.0"

should it be `|| ip->rel < rel`? Once we land the static number in
device info, we can only expect new .rel versions greater than that.

Lucas De Marchi
Matt Roper Sept. 7, 2022, 10:13 p.m. UTC | #2 
On Wed, Sep 07, 2022 at 01:49:25PM -0700, Lucas De Marchi wrote:
> On Thu, Sep 01, 2022 at 11:03:33PM -0700, Radhakrishna Sripada wrote:
> > From: Matt Roper <matthew.d.roper@intel.com>
> > 
> > Going forward, the hardware teams no longer consider new platforms to
> > have a "generation" in the way we've defined it for past platforms.
> > Instead, each IP block (graphics, media, display) will have their own
> > architecture major.minor versions and stepping ID's which should be read
> > directly from a register in the MMIO space.  New hardware programming
> > styles, features, and workarounds should be conditional solely on the
> > architecture version, and should no longer be derived from the PCI
> > device ID, revision ID, or platform-specific feature flags.
> > 
> > Bspec: 63361, 64111
> > 
> > v2:
> >  - Move the IP version readout to intel_device_info.c
> >  - Convert the macro into a function
> > 
> > v3:
> >  - Move subplatform init to runtime early init
> >  - Cache runtime ver, release info to compare with hardware values.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  2 +
> > drivers/gpu/drm/i915/i915_driver.c       |  3 +-
> > drivers/gpu/drm/i915/i915_drv.h          |  2 +
> > drivers/gpu/drm/i915/i915_pci.c          |  1 +
> > drivers/gpu/drm/i915/i915_reg.h          |  7 +++
> > drivers/gpu/drm/i915/intel_device_info.c | 74 +++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_device_info.h | 12 +++-
> > 7 files changed, 98 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index d414785003cc..579da62158c4 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -39,6 +39,8 @@
> > #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
> > #define FORCEWAKE_ACK_MEDIA_GEN9		_MMIO(0xd88)
> > 
> > +#define GMD_ID_GRAPHICS				_MMIO(0xd8c)
> > +
> > #define MCFG_MCR_SELECTOR			_MMIO(0xfd0)
> > #define SF_MCR_SELECTOR				_MMIO(0xfd8)
> > #define GEN8_MCR_SELECTOR			_MMIO(0xfdc)
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index 56a2bcddb2af..a1ab49521d19 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -323,7 +323,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
> > 	if (i915_inject_probe_failure(dev_priv))
> > 		return -ENODEV;
> > 
> > -	intel_device_info_subplatform_init(dev_priv);
> > +	intel_device_info_runtime_init_early(dev_priv);
> > +
> > 	intel_step_init(dev_priv);
> > 
> > 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index f85a470397a5..405b59b8c05c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -936,6 +936,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > 
> > #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
> > 
> > +#define HAS_GMD_ID(i915)	INTEL_INFO(i915)->has_gmd_id
> > +
> > #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
> > 
> > #define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index f6aaf938c53c..4672894f4bc1 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -1129,6 +1129,7 @@ static const struct intel_device_info mtl_info = {
> > 	PLATFORM(INTEL_METEORLAKE),
> > 	.display.has_modular_fia = 1,
> > 	.has_flat_ccs = 0,
> > +	.has_gmd_id = 1,
> > 	.has_snoop = 1,
> > 	.__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
> > 	.__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5e6239864c35..e02e461a4b5d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5798,6 +5798,11 @@
> > #define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> > #define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
> > 
> > +#define GMD_ID_DISPLAY				_MMIO(0x510a0)
> > +#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
> > +#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
> > +#define   GMD_ID_STEP				REG_GENMASK(5, 0)
> > +
> > /*GEN11 chicken */
> > #define _PIPEA_CHICKEN				0x70038
> > #define _PIPEB_CHICKEN				0x71038
> > @@ -8298,4 +8303,6 @@ enum skl_power_gate {
> > #define  MTL_LATENCY_LEVEL_EVEN_MASK	REG_GENMASK(12, 0)
> > #define  MTL_LATENCY_LEVEL_ODD_MASK	REG_GENMASK(28, 16)
> > 
> > +#define MTL_MEDIA_GSI_BASE		0x380000
> > +
> > #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 56f19683dd55..a5bafc9be1fa 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -29,6 +29,7 @@
> > 
> > #include "display/intel_cdclk.h"
> > #include "display/intel_de.h"
> > +#include "gt/intel_gt_regs.h"
> > #include "intel_device_info.h"
> > #include "i915_drv.h"
> > #include "i915_utils.h"
> > @@ -231,7 +232,7 @@ static bool find_devid(u16 id, const u16 *p, unsigned int num)
> > 	return false;
> > }
> > 
> > -void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> > +static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> > {
> > 	const struct intel_device_info *info = INTEL_INFO(i915);
> > 	const struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
> > @@ -288,6 +289,77 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> > 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
> > }
> > 
> > +static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct ip_version *ip)
> > +{
> > +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > +	void __iomem *addr;
> > +	u32 val;
> > +	u8 ver = ip->ver;
> > +	u8 rel = ip->rel;
> > +
> > +	addr = pci_iomap_range(pdev, 0, offset, sizeof(u32));
> > +	if (drm_WARN_ON(&i915->drm, !addr))
> > +		return;
> > +
> > +	val = ioread32(addr);
> > +	pci_iounmap(pdev, addr);
> > +
> > +	ip->ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val);
> > +	ip->rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
> > +	ip->step = REG_FIELD_GET(GMD_ID_STEP, val);
> > +
> > +	/* Sanity check against expected versions from device info */
> > +	if (ip->ver != ver || ip->rel > rel)
> 
> this doesn't seem correct.. if we have
> 
> 	ip->ver == 12, 	ip->rel == 1
> 	ver == 12, rel == 0
> 
> we will print
> 
> 	Hardware reports GMD IP version 12.1 but minimum expected is 12.0"
> 
> should it be `|| ip->rel < rel`? Once we land the static number in
> device info, we can only expect new .rel versions greater than that.

I'm not sure how much value there is in keeping this sanity check at all
given that we plan to remove the device info versions completely for
platforms with GMD_ID.  But if you want to test this in an easy-to-read
manner in the short term,

        if (IP_VER(ip->ver, ip->rel) < IP_VER(ver, rel))

would probably be best.


Matt

> 
> Lucas De Marchi
Lucas De Marchi Sept. 7, 2022, 10:21 p.m. UTC | #3 
On Wed, Sep 07, 2022 at 03:13:31PM -0700, Matt Roper wrote:
>On Wed, Sep 07, 2022 at 01:49:25PM -0700, Lucas De Marchi wrote:
>> On Thu, Sep 01, 2022 at 11:03:33PM -0700, Radhakrishna Sripada wrote:
>> > From: Matt Roper <matthew.d.roper@intel.com>
>> >
>> > Going forward, the hardware teams no longer consider new platforms to
>> > have a "generation" in the way we've defined it for past platforms.
>> > Instead, each IP block (graphics, media, display) will have their own
>> > architecture major.minor versions and stepping ID's which should be read
>> > directly from a register in the MMIO space.  New hardware programming
>> > styles, features, and workarounds should be conditional solely on the
>> > architecture version, and should no longer be derived from the PCI
>> > device ID, revision ID, or platform-specific feature flags.
>> >
>> > Bspec: 63361, 64111
>> >
>> > v2:
>> >  - Move the IP version readout to intel_device_info.c
>> >  - Convert the macro into a function
>> >
>> > v3:
>> >  - Move subplatform init to runtime early init
>> >  - Cache runtime ver, release info to compare with hardware values.
>> >
>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  2 +
>> > drivers/gpu/drm/i915/i915_driver.c       |  3 +-
>> > drivers/gpu/drm/i915/i915_drv.h          |  2 +
>> > drivers/gpu/drm/i915/i915_pci.c          |  1 +
>> > drivers/gpu/drm/i915/i915_reg.h          |  7 +++
>> > drivers/gpu/drm/i915/intel_device_info.c | 74 +++++++++++++++++++++++-
>> > drivers/gpu/drm/i915/intel_device_info.h | 12 +++-
>> > 7 files changed, 98 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > index d414785003cc..579da62158c4 100644
>> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>> > @@ -39,6 +39,8 @@
>> > #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
>> > #define FORCEWAKE_ACK_MEDIA_GEN9		_MMIO(0xd88)
>> >
>> > +#define GMD_ID_GRAPHICS				_MMIO(0xd8c)
>> > +
>> > #define MCFG_MCR_SELECTOR			_MMIO(0xfd0)
>> > #define SF_MCR_SELECTOR				_MMIO(0xfd8)
>> > #define GEN8_MCR_SELECTOR			_MMIO(0xfdc)
>> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> > index 56a2bcddb2af..a1ab49521d19 100644
>> > --- a/drivers/gpu/drm/i915/i915_driver.c
>> > +++ b/drivers/gpu/drm/i915/i915_driver.c
>> > @@ -323,7 +323,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>> > 	if (i915_inject_probe_failure(dev_priv))
>> > 		return -ENODEV;
>> >
>> > -	intel_device_info_subplatform_init(dev_priv);
>> > +	intel_device_info_runtime_init_early(dev_priv);
>> > +
>> > 	intel_step_init(dev_priv);
>> >
>> > 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index f85a470397a5..405b59b8c05c 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -936,6 +936,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>> >
>> > #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
>> >
>> > +#define HAS_GMD_ID(i915)	INTEL_INFO(i915)->has_gmd_id
>> > +
>> > #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
>> >
>> > #define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
>> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> > index f6aaf938c53c..4672894f4bc1 100644
>> > --- a/drivers/gpu/drm/i915/i915_pci.c
>> > +++ b/drivers/gpu/drm/i915/i915_pci.c
>> > @@ -1129,6 +1129,7 @@ static const struct intel_device_info mtl_info = {
>> > 	PLATFORM(INTEL_METEORLAKE),
>> > 	.display.has_modular_fia = 1,
>> > 	.has_flat_ccs = 0,
>> > +	.has_gmd_id = 1,
>> > 	.has_snoop = 1,
>> > 	.__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
>> > 	.__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 5e6239864c35..e02e461a4b5d 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -5798,6 +5798,11 @@
>> > #define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
>> > #define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
>> >
>> > +#define GMD_ID_DISPLAY				_MMIO(0x510a0)
>> > +#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
>> > +#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
>> > +#define   GMD_ID_STEP				REG_GENMASK(5, 0)
>> > +
>> > /*GEN11 chicken */
>> > #define _PIPEA_CHICKEN				0x70038
>> > #define _PIPEB_CHICKEN				0x71038
>> > @@ -8298,4 +8303,6 @@ enum skl_power_gate {
>> > #define  MTL_LATENCY_LEVEL_EVEN_MASK	REG_GENMASK(12, 0)
>> > #define  MTL_LATENCY_LEVEL_ODD_MASK	REG_GENMASK(28, 16)
>> >
>> > +#define MTL_MEDIA_GSI_BASE		0x380000
>> > +
>> > #endif /* _I915_REG_H_ */
>> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> > index 56f19683dd55..a5bafc9be1fa 100644
>> > --- a/drivers/gpu/drm/i915/intel_device_info.c
>> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> > @@ -29,6 +29,7 @@
>> >
>> > #include "display/intel_cdclk.h"
>> > #include "display/intel_de.h"
>> > +#include "gt/intel_gt_regs.h"
>> > #include "intel_device_info.h"
>> > #include "i915_drv.h"
>> > #include "i915_utils.h"
>> > @@ -231,7 +232,7 @@ static bool find_devid(u16 id, const u16 *p, unsigned int num)
>> > 	return false;
>> > }
>> >
>> > -void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>> > +static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>> > {
>> > 	const struct intel_device_info *info = INTEL_INFO(i915);
>> > 	const struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
>> > @@ -288,6 +289,77 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>> > 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
>> > }
>> >
>> > +static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct ip_version *ip)
>> > +{
>> > +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>> > +	void __iomem *addr;
>> > +	u32 val;
>> > +	u8 ver = ip->ver;
>> > +	u8 rel = ip->rel;
>> > +
>> > +	addr = pci_iomap_range(pdev, 0, offset, sizeof(u32));
>> > +	if (drm_WARN_ON(&i915->drm, !addr))
>> > +		return;
>> > +
>> > +	val = ioread32(addr);
>> > +	pci_iounmap(pdev, addr);
>> > +
>> > +	ip->ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val);
>> > +	ip->rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
>> > +	ip->step = REG_FIELD_GET(GMD_ID_STEP, val);
>> > +
>> > +	/* Sanity check against expected versions from device info */
>> > +	if (ip->ver != ver || ip->rel > rel)
>>
>> this doesn't seem correct.. if we have
>>
>> 	ip->ver == 12, 	ip->rel == 1
>> 	ver == 12, rel == 0
>>
>> we will print
>>
>> 	Hardware reports GMD IP version 12.1 but minimum expected is 12.0"
>>
>> should it be `|| ip->rel < rel`? Once we land the static number in
>> device info, we can only expect new .rel versions greater than that.
>
>I'm not sure how much value there is in keeping this sanity check at all
>given that we plan to remove the device info versions completely for
>platforms with GMD_ID.  But if you want to test this in an easy-to-read
>manner in the short term,
>
>        if (IP_VER(ip->ver, ip->rel) < IP_VER(ver, rel))
>
>would probably be best.

I'm ok with removing the sanity check as long as we log the value read.
We still do when printing all the info, so lgtm.

Lucas De Marchi

>
>
>Matt
>
>>
>> Lucas De Marchi
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
Sripada, Radhakrishna Sept. 7, 2022, 10:38 p.m. UTC | #4 
Hi Lucas/Matt,

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Wednesday, September 7, 2022 3:21 PM
> To: Roper, Matthew D <matthew.d.roper@intel.com>
> Cc: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v4 02/11] drm/i915: Read graphics/media/display
> arch version from hw
> 
> On Wed, Sep 07, 2022 at 03:13:31PM -0700, Matt Roper wrote:
> >On Wed, Sep 07, 2022 at 01:49:25PM -0700, Lucas De Marchi wrote:
> >> On Thu, Sep 01, 2022 at 11:03:33PM -0700, Radhakrishna Sripada wrote:
> >> > From: Matt Roper <matthew.d.roper@intel.com>
> >> >
> >> > Going forward, the hardware teams no longer consider new platforms to
> >> > have a "generation" in the way we've defined it for past platforms.
> >> > Instead, each IP block (graphics, media, display) will have their own
> >> > architecture major.minor versions and stepping ID's which should be read
> >> > directly from a register in the MMIO space.  New hardware programming
> >> > styles, features, and workarounds should be conditional solely on the
> >> > architecture version, and should no longer be derived from the PCI
> >> > device ID, revision ID, or platform-specific feature flags.
> >> >
> >> > Bspec: 63361, 64111
> >> >
> >> > v2:
> >> >  - Move the IP version readout to intel_device_info.c
> >> >  - Convert the macro into a function
> >> >
> >> > v3:
> >> >  - Move subplatform init to runtime early init
> >> >  - Cache runtime ver, release info to compare with hardware values.
> >> >
> >> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  2 +
> >> > drivers/gpu/drm/i915/i915_driver.c       |  3 +-
> >> > drivers/gpu/drm/i915/i915_drv.h          |  2 +
> >> > drivers/gpu/drm/i915/i915_pci.c          |  1 +
> >> > drivers/gpu/drm/i915/i915_reg.h          |  7 +++
> >> > drivers/gpu/drm/i915/intel_device_info.c | 74
> +++++++++++++++++++++++-
> >> > drivers/gpu/drm/i915/intel_device_info.h | 12 +++-
> >> > 7 files changed, 98 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> >> > index d414785003cc..579da62158c4 100644
> >> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> >> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> >> > @@ -39,6 +39,8 @@
> >> > #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
> >> > #define FORCEWAKE_ACK_MEDIA_GEN9		_MMIO(0xd88)
> >> >
> >> > +#define GMD_ID_GRAPHICS
> 	_MMIO(0xd8c)
> >> > +
> >> > #define MCFG_MCR_SELECTOR			_MMIO(0xfd0)
> >> > #define SF_MCR_SELECTOR				_MMIO(0xfd8)
> >> > #define GEN8_MCR_SELECTOR			_MMIO(0xfdc)
> >> > diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> >> > index 56a2bcddb2af..a1ab49521d19 100644
> >> > --- a/drivers/gpu/drm/i915/i915_driver.c
> >> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> >> > @@ -323,7 +323,8 @@ static int i915_driver_early_probe(struct
> drm_i915_private *dev_priv)
> >> > 	if (i915_inject_probe_failure(dev_priv))
> >> > 		return -ENODEV;
> >> >
> >> > -	intel_device_info_subplatform_init(dev_priv);
> >> > +	intel_device_info_runtime_init_early(dev_priv);
> >> > +
> >> > 	intel_step_init(dev_priv);
> >> >
> >> > 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
> >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> >> > index f85a470397a5..405b59b8c05c 100644
> >> > --- a/drivers/gpu/drm/i915/i915_drv.h
> >> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> > @@ -936,6 +936,8 @@ IS_SUBPLATFORM(const struct drm_i915_private
> *i915,
> >> >
> >> > #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
> >> >
> >> > +#define HAS_GMD_ID(i915)	INTEL_INFO(i915)->has_gmd_id
> >> > +
> >> > #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
> >> >
> >> > #define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
> >> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> b/drivers/gpu/drm/i915/i915_pci.c
> >> > index f6aaf938c53c..4672894f4bc1 100644
> >> > --- a/drivers/gpu/drm/i915/i915_pci.c
> >> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> > @@ -1129,6 +1129,7 @@ static const struct intel_device_info mtl_info = {
> >> > 	PLATFORM(INTEL_METEORLAKE),
> >> > 	.display.has_modular_fia = 1,
> >> > 	.has_flat_ccs = 0,
> >> > +	.has_gmd_id = 1,
> >> > 	.has_snoop = 1,
> >> > 	.__runtime.memory_regions = REGION_SMEM |
> REGION_STOLEN_LMEM,
> >> > 	.__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 5e6239864c35..e02e461a4b5d 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -5798,6 +5798,11 @@
> >> > #define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> >> > #define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
> >> >
> >> > +#define GMD_ID_DISPLAY				_MMIO(0x510a0)
> >> > +#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
> >> > +#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
> >> > +#define   GMD_ID_STEP				REG_GENMASK(5, 0)
> >> > +
> >> > /*GEN11 chicken */
> >> > #define _PIPEA_CHICKEN				0x70038
> >> > #define _PIPEB_CHICKEN				0x71038
> >> > @@ -8298,4 +8303,6 @@ enum skl_power_gate {
> >> > #define  MTL_LATENCY_LEVEL_EVEN_MASK	REG_GENMASK(12, 0)
> >> > #define  MTL_LATENCY_LEVEL_ODD_MASK	REG_GENMASK(28, 16)
> >> >
> >> > +#define MTL_MEDIA_GSI_BASE		0x380000
> >> > +
> >> > #endif /* _I915_REG_H_ */
> >> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> >> > index 56f19683dd55..a5bafc9be1fa 100644
> >> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> >> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> >> > @@ -29,6 +29,7 @@
> >> >
> >> > #include "display/intel_cdclk.h"
> >> > #include "display/intel_de.h"
> >> > +#include "gt/intel_gt_regs.h"
> >> > #include "intel_device_info.h"
> >> > #include "i915_drv.h"
> >> > #include "i915_utils.h"
> >> > @@ -231,7 +232,7 @@ static bool find_devid(u16 id, const u16 *p,
> unsigned int num)
> >> > 	return false;
> >> > }
> >> >
> >> > -void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> >> > +static void intel_device_info_subplatform_init(struct drm_i915_private
> *i915)
> >> > {
> >> > 	const struct intel_device_info *info = INTEL_INFO(i915);
> >> > 	const struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
> >> > @@ -288,6 +289,77 @@ void intel_device_info_subplatform_init(struct
> drm_i915_private *i915)
> >> > 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
> >> > }
> >> >
> >> > +static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct
> ip_version *ip)
> >> > +{
> >> > +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> >> > +	void __iomem *addr;
> >> > +	u32 val;
> >> > +	u8 ver = ip->ver;
> >> > +	u8 rel = ip->rel;
> >> > +
> >> > +	addr = pci_iomap_range(pdev, 0, offset, sizeof(u32));
> >> > +	if (drm_WARN_ON(&i915->drm, !addr))
> >> > +		return;
> >> > +
> >> > +	val = ioread32(addr);
> >> > +	pci_iounmap(pdev, addr);
> >> > +
> >> > +	ip->ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val);
> >> > +	ip->rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
> >> > +	ip->step = REG_FIELD_GET(GMD_ID_STEP, val);
> >> > +
> >> > +	/* Sanity check against expected versions from device info */
> >> > +	if (ip->ver != ver || ip->rel > rel)
> >>
> >> this doesn't seem correct.. if we have
> >>
> >> 	ip->ver == 12, 	ip->rel == 1
> >> 	ver == 12, rel == 0
> >>
> >> we will print
> >>
> >> 	Hardware reports GMD IP version 12.1 but minimum expected is 12.0"
> >>
> >> should it be `|| ip->rel < rel`? Once we land the static number in
> >> device info, we can only expect new .rel versions greater than that.
> >
> >I'm not sure how much value there is in keeping this sanity check at all
> >given that we plan to remove the device info versions completely for
> >platforms with GMD_ID.  But if you want to test this in an easy-to-read
> >manner in the short term,
> >
> >        if (IP_VER(ip->ver, ip->rel) < IP_VER(ver, rel))
> >
> >would probably be best.
> 
> I'm ok with removing the sanity check as long as we log the value read.
> We still do when printing all the info, so lgtm.	
I agree with Matt's suggestion. Will spin a new rev with IP_VER based check.
Although we print the info later. It is worth doing a sanity check as we read the info.

-RK
> 
> Lucas De Marchi
> 
> >
> >
> >Matt
> >
> >>
> >> Lucas De Marchi
> >
> >--
> >Matt Roper
> >Graphics Software Engineer
> >VTT-OSGC Platform Enablement
> >Intel Corporation
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index d414785003cc..579da62158c4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -39,6 +39,8 @@ 
 #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
 #define FORCEWAKE_ACK_MEDIA_GEN9		_MMIO(0xd88)
 
+#define GMD_ID_GRAPHICS				_MMIO(0xd8c)
+
 #define MCFG_MCR_SELECTOR			_MMIO(0xfd0)
 #define SF_MCR_SELECTOR				_MMIO(0xfd8)
 #define GEN8_MCR_SELECTOR			_MMIO(0xfdc)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 56a2bcddb2af..a1ab49521d19 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -323,7 +323,8 @@  static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	if (i915_inject_probe_failure(dev_priv))
 		return -ENODEV;
 
-	intel_device_info_subplatform_init(dev_priv);
+	intel_device_info_runtime_init_early(dev_priv);
+
 	intel_step_init(dev_priv);
 
 	intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f85a470397a5..405b59b8c05c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -936,6 +936,8 @@  IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
 
+#define HAS_GMD_ID(i915)	INTEL_INFO(i915)->has_gmd_id
+
 #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
 
 #define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f6aaf938c53c..4672894f4bc1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1129,6 +1129,7 @@  static const struct intel_device_info mtl_info = {
 	PLATFORM(INTEL_METEORLAKE),
 	.display.has_modular_fia = 1,
 	.has_flat_ccs = 0,
+	.has_gmd_id = 1,
 	.has_snoop = 1,
 	.__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
 	.__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5e6239864c35..e02e461a4b5d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5798,6 +5798,11 @@ 
 #define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
 #define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
 
+#define GMD_ID_DISPLAY				_MMIO(0x510a0)
+#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
+#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
+#define   GMD_ID_STEP				REG_GENMASK(5, 0)
+
 /*GEN11 chicken */
 #define _PIPEA_CHICKEN				0x70038
 #define _PIPEB_CHICKEN				0x71038
@@ -8298,4 +8303,6 @@  enum skl_power_gate {
 #define  MTL_LATENCY_LEVEL_EVEN_MASK	REG_GENMASK(12, 0)
 #define  MTL_LATENCY_LEVEL_ODD_MASK	REG_GENMASK(28, 16)
 
+#define MTL_MEDIA_GSI_BASE		0x380000
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 56f19683dd55..a5bafc9be1fa 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -29,6 +29,7 @@ 
 
 #include "display/intel_cdclk.h"
 #include "display/intel_de.h"
+#include "gt/intel_gt_regs.h"
 #include "intel_device_info.h"
 #include "i915_drv.h"
 #include "i915_utils.h"
@@ -231,7 +232,7 @@  static bool find_devid(u16 id, const u16 *p, unsigned int num)
 	return false;
 }
 
-void intel_device_info_subplatform_init(struct drm_i915_private *i915)
+static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 {
 	const struct intel_device_info *info = INTEL_INFO(i915);
 	const struct intel_runtime_info *rinfo = RUNTIME_INFO(i915);
@@ -288,6 +289,77 @@  void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
 }
 
+static void ip_ver_read(struct drm_i915_private *i915, u32 offset, struct ip_version *ip)
+{
+	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+	void __iomem *addr;
+	u32 val;
+	u8 ver = ip->ver;
+	u8 rel = ip->rel;
+
+	addr = pci_iomap_range(pdev, 0, offset, sizeof(u32));
+	if (drm_WARN_ON(&i915->drm, !addr))
+		return;
+
+	val = ioread32(addr);
+	pci_iounmap(pdev, addr);
+
+	ip->ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val);
+	ip->rel = REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
+	ip->step = REG_FIELD_GET(GMD_ID_STEP, val);
+
+	/* Sanity check against expected versions from device info */
+	if (ip->ver != ver || ip->rel > rel)
+		drm_dbg(&i915->drm,
+			"Hardware reports GMD IP version %u.%u but minimum expected is %u.%u\n",
+			ip->ver, ip->rel, ver, rel);
+}
+
+/**
+ * Setup the graphics version for the current device.  This must be done before
+ * any code that performs checks on GRAPHICS_VER or DISPLAY_VER, so this
+ * function should be called very early in the driver initialization sequence.
+ *
+ * Regular MMIO access is not yet setup at the point this function is called so
+ * we peek at the appropriate MMIO offset directly.  The GMD_ID register is
+ * part of an 'always on' power well by design, so we don't need to worry about
+ * forcewake while reading it.
+ */
+static void intel_ipver_early_init(struct drm_i915_private *i915)
+{
+	struct intel_runtime_info *runtime = RUNTIME_INFO(i915);
+
+	if (!HAS_GMD_ID(i915))
+		return;
+
+	ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_GRAPHICS),
+		    &runtime->graphics.version);
+	ip_ver_read(i915, i915_mmio_reg_offset(GMD_ID_DISPLAY),
+		    &runtime->display.version);
+	ip_ver_read(i915, MTL_MEDIA_GSI_BASE + i915_mmio_reg_offset(GMD_ID_GRAPHICS),
+		    &runtime->media.version);
+}
+
+/**
+ * intel_device_info_runtime_init_early - initialize early runtime info
+ * @i915: the i915 device
+ *
+ * Determine early intel_device_info fields at runtime.
+ *
+ * Use it when:
+ *   - Early init of certain runtime info fields are to be initialized
+ *
+ * This function needs to be called:
+ *   - before the MMIO has been setup as we are reading registers,
+ *   - before the PCH has been detected,
+ *   - before the first usage of the fields it can tweak.
+ */
+void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
+{
+	intel_ipver_early_init(i915);
+	intel_device_info_subplatform_init(i915);
+}
+
 /**
  * intel_device_info_runtime_init - initialize runtime info
  * @dev_priv: the i915 device
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index d36cf25f1d5f..4a938cb897d0 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -152,6 +152,7 @@  enum intel_ppgtt_type {
 	func(has_4tile); \
 	func(has_flat_ccs); \
 	func(has_global_mocs); \
+	func(has_gmd_id); \
 	func(has_gt_uc); \
 	func(has_heci_pxp); \
 	func(has_heci_gscfi); \
@@ -197,9 +198,18 @@  enum intel_ppgtt_type {
 struct ip_version {
 	u8 ver;
 	u8 rel;
+	u8 step;
 };
 
 struct intel_runtime_info {
+	/*
+	 * On modern platforms, the architecture major.minor version numbers
+	 * and stepping are read directly from the hardware rather than derived
+	 * from the PCI device and revision ID's.
+	 *
+	 * Note that the hardware gives us a single "graphics" number that
+	 * should represent render, compute, and copy behavior.
+	 */
 	struct {
 		struct ip_version version;
 	} graphics;
@@ -305,7 +315,7 @@  struct intel_driver_caps {
 
 const char *intel_platform_name(enum intel_platform platform);
 
-void intel_device_info_subplatform_init(struct drm_i915_private *dev_priv);
+void intel_device_info_runtime_init_early(struct drm_i915_private *dev_priv);
 void intel_device_info_runtime_init(struct drm_i915_private *dev_priv);
 
 void intel_device_info_print(const struct intel_device_info *info,