On Thu, Jun 16, 2022 at 05:31:00PM +0530, Anshuman Gupta wrote:
> DG2 NB SKU need to distinguish between MBD and AIC to probe
> the VRAM Self Refresh feature support. Adding those sub platform
> accordingly.
>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_device_info.c | 21 +++++++++++++++++++++
> drivers/gpu/drm/i915/intel_device_info.h | 11 +++++++----
> include/drm/i915_pciids.h | 23 ++++++++++++++++-------
> 4 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a5bc6a774c5a..f1f8699eedfd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1007,10 +1007,13 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO)
>
> #define IS_DG2_G10(dev_priv) \
> + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10_NB_MBD) || \
> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10)
> #define IS_DG2_G11(dev_priv) \
> + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11_NB_MBD) || \
> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G11)
> #define IS_DG2_G12(dev_priv) \
> + IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12_NB_MBD) || \
> IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G12)
> #define IS_ADLS_RPLS(dev_priv) \
> IS_SUBPLATFORM(dev_priv, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index f0bf23726ed8..93da555adc4e 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -187,6 +187,18 @@ static const u16 subplatform_rpl_ids[] = {
> INTEL_RPLP_IDS(0),
> };
>
> +static const u16 subplatform_g10_mb_mbd_ids[] = {
> + INTEL_DG2_G10_NB_MBD_IDS(0),
> +};
> +
> +static const u16 subplatform_g11_mb_mbd_ids[] = {
> + INTEL_DG2_G11_NB_MBD_IDS(0),
> +};
> +
> +static const u16 subplatform_g12_mb_mbd_ids[] = {
> + INTEL_DG2_G12_NB_MBD_IDS(0),
> +};
We only need a single MBD subplatform, not three new subplatforms.
Unless I'm forgetting something, a single device ID can be assigned two
two independent subplatforms at the same time. So the decision about
whether to set the G10, G11, or G12 bit is one decision. The decision
about whether to set the MBD bit is a completely separate decision that
doesn't care about the G10/G11/G12 stuff.
> +
> static const u16 subplatform_g10_ids[] = {
> INTEL_DG2_G10_IDS(0),
> INTEL_ATS_M150_IDS(0),
> @@ -246,6 +258,15 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> } else if (find_devid(devid, subplatform_rpl_ids,
> ARRAY_SIZE(subplatform_rpl_ids))) {
> mask = BIT(INTEL_SUBPLATFORM_RPL);
> + } else if (find_devid(devid, subplatform_g10_mb_mbd_ids,
> + ARRAY_SIZE(subplatform_g10_mb_mbd_ids))) {
> + mask = BIT(INTEL_SUBPLATFORM_G10_NB_MBD);
> + } else if (find_devid(devid, subplatform_g11_mb_mbd_ids,
> + ARRAY_SIZE(subplatform_g11_mb_mbd_ids))) {
> + mask = BIT(INTEL_SUBPLATFORM_G11_NB_MBD);
> + } else if (find_devid(devid, subplatform_g12_mb_mbd_ids,
> + ARRAY_SIZE(subplatform_g12_mb_mbd_ids))) {
> + mask = BIT(INTEL_SUBPLATFORM_G12_NB_MBD);
Assuming you consolidate MBD back down to just a single extra
subplatform, the lookup and bit setting should happen in a separate 'if'
statement (not an 'else' block).
if (find_devid(devid, subplatform_mbd_ids,
ARRAY_SIZE(subplatform_mbd_ids)))
mask |= BIT(INTEL_SUBPLATFORM_MBD);
Matt
> } else if (find_devid(devid, subplatform_g10_ids,
> ARRAY_SIZE(subplatform_g10_ids))) {
> mask = BIT(INTEL_SUBPLATFORM_G10);
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 08341174ee0a..c929e2d7e59c 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -97,7 +97,7 @@ enum intel_platform {
> * it is fine for the same bit to be used on multiple parent platforms.
> */
>
> -#define INTEL_SUBPLATFORM_BITS (3)
> +#define INTEL_SUBPLATFORM_BITS (6)
> #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1)
>
> /* HSW/BDW/SKL/KBL/CFL */
> @@ -111,9 +111,12 @@ enum intel_platform {
> #define INTEL_SUBPLATFORM_UY (0)
>
> /* DG2 */
> -#define INTEL_SUBPLATFORM_G10 0
> -#define INTEL_SUBPLATFORM_G11 1
> -#define INTEL_SUBPLATFORM_G12 2
> +#define INTEL_SUBPLATFORM_G10_NB_MBD 0
> +#define INTEL_SUBPLATFORM_G11_NB_MBD 1
> +#define INTEL_SUBPLATFORM_G12_NB_MBD 2
> +#define INTEL_SUBPLATFORM_G10 3
> +#define INTEL_SUBPLATFORM_G11 4
> +#define INTEL_SUBPLATFORM_G12 5
>
> /* ADL */
> #define INTEL_SUBPLATFORM_RPL 0
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 4585fed4e41e..198be417bb2d 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -693,32 +693,41 @@
> INTEL_VGA_DEVICE(0xA7A9, info)
>
> /* DG2 */
> -#define INTEL_DG2_G10_IDS(info) \
> +#define INTEL_DG2_G10_NB_MBD_IDS(info) \
> INTEL_VGA_DEVICE(0x5690, info), \
> INTEL_VGA_DEVICE(0x5691, info), \
> - INTEL_VGA_DEVICE(0x5692, info), \
> + INTEL_VGA_DEVICE(0x5692, info)
> +
> +#define INTEL_DG2_G11_NB_MBD_IDS(info) \
> + INTEL_VGA_DEVICE(0x5693, info), \
> + INTEL_VGA_DEVICE(0x5694, info), \
> + INTEL_VGA_DEVICE(0x5695, info)
> +
> +#define INTEL_DG2_G12_NB_MBD_IDS(info) \
> + INTEL_VGA_DEVICE(0x5696, info), \
> + INTEL_VGA_DEVICE(0x5697, info)
> +
> +#define INTEL_DG2_G10_IDS(info) \
> INTEL_VGA_DEVICE(0x56A0, info), \
> INTEL_VGA_DEVICE(0x56A1, info), \
> INTEL_VGA_DEVICE(0x56A2, info)
>
> #define INTEL_DG2_G11_IDS(info) \
> - INTEL_VGA_DEVICE(0x5693, info), \
> - INTEL_VGA_DEVICE(0x5694, info), \
> - INTEL_VGA_DEVICE(0x5695, info), \
> INTEL_VGA_DEVICE(0x56A5, info), \
> INTEL_VGA_DEVICE(0x56A6, info), \
> INTEL_VGA_DEVICE(0x56B0, info), \
> INTEL_VGA_DEVICE(0x56B1, info)
>
> #define INTEL_DG2_G12_IDS(info) \
> - INTEL_VGA_DEVICE(0x5696, info), \
> - INTEL_VGA_DEVICE(0x5697, info), \
> INTEL_VGA_DEVICE(0x56A3, info), \
> INTEL_VGA_DEVICE(0x56A4, info), \
> INTEL_VGA_DEVICE(0x56B2, info), \
> INTEL_VGA_DEVICE(0x56B3, info)
>
> #define INTEL_DG2_IDS(info) \
> + INTEL_DG2_G10_NB_MBD_IDS(info), \
> + INTEL_DG2_G11_NB_MBD_IDS(info), \
> + INTEL_DG2_G12_NB_MBD_IDS(info), \
> INTEL_DG2_G10_IDS(info), \
> INTEL_DG2_G11_IDS(info), \
> INTEL_DG2_G12_IDS(info)
> --
> 2.26.2
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
