Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions
On Tue, 2020-05-26 at 10:51 -0700, Lucas De Marchi wrote:
> On Tue, May 26, 2020 at 10:34:47AM -0700, Jose Souza wrote:
> > On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
> > > From: Abdiel Janulgue
> > >
> > > Bspec: 33617, 33617
> > >
> > > Cc: José Roberto de Souza
> > > Cc: Daniele Ceraolo Spurio
> > > Cc: Stuart Summers
> > > Cc: Vanshidhar Konda
> > > Cc: Lucas De Marchi
> > > Cc: Aravind Iddamsetty
> > > Cc: Matt Roper
> > > Signed-off-by: Abdiel Janulgue
> > > Signed-off-by: Lucas De Marchi
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 7 +++
> > > drivers/gpu/drm/i915/i915_pci.c | 12
> > > drivers/gpu/drm/i915/intel_device_info.c | 1 +
> > > drivers/gpu/drm/i915/intel_device_info.h | 1 +
> > > 4 files changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index a9846205a5e2..382703a6c17d 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1421,6 +1421,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > > #define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> > > #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> > > #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> > > +#define IS_DG1(dev_priv)IS_PLATFORM(dev_priv, INTEL_DG1)
> > > #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> > > (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
> > > #define IS_BDW_ULT(dev_priv) \
> > > @@ -1541,6 +1542,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > > #define IS_RKL_REVID(p, since, until) \
> > > (IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
> > >
> > > +#define DG1_REVID_A0 0x0
> > > +#define DG1_REVID_B0 0x1
> > > +
> > > +#define IS_DG1_REVID(p, since, until) \
> > > + (IS_DG1(p) && IS_REVID(p, since, until))
> > > +
> > > #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> > > #define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
> > > #define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) &&
> > > !IS_LP(dev_priv))
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > b/drivers/gpu/drm/i915/i915_pci.c
> > > index be52d1b76b2e..e5a851a2dfe7 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -874,9 +874,21 @@ static const struct intel_device_info rkl_info = {
> > >
> > > #define GEN12_DGFX_FEATURES \
> > > GEN12_FEATURES, \
> > > + .memory_regions = REGION_SMEM | REGION_LMEM, \
> >
> > REGION_LMEM should be left here for all GEN12 platforms?
>
> no, this define is GEN12_DGFX_FEATURES. For dgfx, yes... we should leave
> lmem region there.
Ops.
Also ignore my has_master_unit_irq comment.
Reviewed-by: José Roberto de Souza
>
> Lucas De Marchi
>
> > I guess it will be detected in runtime that TGL and RKL don't have it but
> > would be better to not have it in all GEN12 but will let memory experts to
> > shine in, other than LGTM.
> >
> >
> > > .is_dgfx = 1, \
> > > + .has_master_unit_irq = 1, \
> > > .has_snoop_pcie = 1
> > >
> > > +static const struct intel_device_info intel_dg1_info = {
> > > + GEN12_DGFX_FEATURES,
> > > + PLATFORM(INTEL_DG1),
> > > + .pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D),
> > > + .require_force_probe = 1,
> > > + .engine_mask =
> > > + BIT(RCS0) | BIT(BCS0) | BIT(VECS0) |
> > > + BIT(VCS0) | BIT(VCS2),
> > > +};
> > > +
> > > #undef GEN
> > > #undef PLATFORM
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > > b/drivers/gpu/drm/i915/intel_device_info.c
> > > index c245c10c9bee..207244b9a852 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> > > PLATFORM_NAME(ELKHARTLAKE),
> > > PLATFORM_NAME(TIGERLAKE),
> > > PLATFORM_NAME(ROCKETLAKE),
> > > + PLATFORM_NAME(DG1),
> > > };
> > > #undef PLATFORM_NAME
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > > b/drivers/gpu/drm/i915/intel_device_info.h
> > > index 4bcaa0d6a9e6..64260faac006 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > @@ -81,6 +81,7 @@ enum intel_platform {
> > > /* gen12 */
> > > INTEL_TIGERLAKE,
> > > INTEL_ROCKETLAKE,
> > > + INTEL_DG1,
> > > INTEL_MAX_PLATFORMS
> > > };
> > >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions
On Tue, May 26, 2020 at 10:34:47AM -0700, Jose Souza wrote:
On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
From: Abdiel Janulgue
Bspec: 33617, 33617
Cc: José Roberto de Souza
Cc: Daniele Ceraolo Spurio
Cc: Stuart Summers
Cc: Vanshidhar Konda
Cc: Lucas De Marchi
Cc: Aravind Iddamsetty
Cc: Matt Roper
Signed-off-by: Abdiel Janulgue
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++
drivers/gpu/drm/i915/i915_pci.c | 12
drivers/gpu/drm/i915/intel_device_info.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
4 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a9846205a5e2..382703a6c17d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1421,6 +1421,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
#define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
#define IS_ROCKETLAKE(dev_priv)IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
+#define IS_DG1(dev_priv)IS_PLATFORM(dev_priv, INTEL_DG1)
#define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
(INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
#define IS_BDW_ULT(dev_priv) \
@@ -1541,6 +1542,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define IS_RKL_REVID(p, since, until) \
(IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
+#define DG1_REVID_A0 0x0
+#define DG1_REVID_B0 0x1
+
+#define IS_DG1_REVID(p, since, until) \
+ (IS_DG1(p) && IS_REVID(p, since, until))
+
#define IS_LP(dev_priv)(INTEL_INFO(dev_priv)->is_lp)
#define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
#define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index be52d1b76b2e..e5a851a2dfe7 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -874,9 +874,21 @@ static const struct intel_device_info rkl_info = {
#define GEN12_DGFX_FEATURES \
GEN12_FEATURES, \
+ .memory_regions = REGION_SMEM | REGION_LMEM, \
REGION_LMEM should be left here for all GEN12 platforms?
no, this define is GEN12_DGFX_FEATURES. For dgfx, yes... we should leave
lmem region there.
Lucas De Marchi
I guess it will be detected in runtime that TGL and RKL don't have it but would
be better to not have it in all GEN12 but will let memory experts to
shine in, other than LGTM.
.is_dgfx = 1, \
+ .has_master_unit_irq = 1, \
.has_snoop_pcie = 1
+static const struct intel_device_info intel_dg1_info = {
+ GEN12_DGFX_FEATURES,
+ PLATFORM(INTEL_DG1),
+ .pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D),
+ .require_force_probe = 1,
+ .engine_mask =
+ BIT(RCS0) | BIT(BCS0) | BIT(VECS0) |
+ BIT(VCS0) | BIT(VCS2),
+};
+
#undef GEN
#undef PLATFORM
diff --git a/drivers/gpu/drm/i915/intel_device_info.c
b/drivers/gpu/drm/i915/intel_device_info.c
index c245c10c9bee..207244b9a852 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -62,6 +62,7 @@ static const char * const platform_names[] = {
PLATFORM_NAME(ELKHARTLAKE),
PLATFORM_NAME(TIGERLAKE),
PLATFORM_NAME(ROCKETLAKE),
+ PLATFORM_NAME(DG1),
};
#undef PLATFORM_NAME
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index 4bcaa0d6a9e6..64260faac006 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -81,6 +81,7 @@ enum intel_platform {
/* gen12 */
INTEL_TIGERLAKE,
INTEL_ROCKETLAKE,
+ INTEL_DG1,
INTEL_MAX_PLATFORMS
};
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions
On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
> From: Abdiel Janulgue
>
> Bspec: 33617, 33617
>
> Cc: José Roberto de Souza
> Cc: Daniele Ceraolo Spurio
> Cc: Stuart Summers
> Cc: Vanshidhar Konda
> Cc: Lucas De Marchi
> Cc: Aravind Iddamsetty
> Cc: Matt Roper
> Signed-off-by: Abdiel Janulgue
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++
> drivers/gpu/drm/i915/i915_pci.c | 12
> drivers/gpu/drm/i915/intel_device_info.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> 4 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a9846205a5e2..382703a6c17d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1421,6 +1421,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> +#define IS_DG1(dev_priv)IS_PLATFORM(dev_priv, INTEL_DG1)
> #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
> #define IS_BDW_ULT(dev_priv) \
> @@ -1541,6 +1542,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_RKL_REVID(p, since, until) \
> (IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
>
> +#define DG1_REVID_A0 0x0
> +#define DG1_REVID_B0 0x1
> +
> +#define IS_DG1_REVID(p, since, until) \
> + (IS_DG1(p) && IS_REVID(p, since, until))
> +
> #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> #define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
> #define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index be52d1b76b2e..e5a851a2dfe7 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -874,9 +874,21 @@ static const struct intel_device_info rkl_info = {
>
> #define GEN12_DGFX_FEATURES \
> GEN12_FEATURES, \
> + .memory_regions = REGION_SMEM | REGION_LMEM, \
> .is_dgfx = 1, \
> + .has_master_unit_irq = 1, \
Also has_master_unit_irq is wrong, it should be added only to intel_dg1_info,
TGL don't have DG1_MSTR_UNIT_INTR according to the "drm/i915/dg1: add
support for the master unit interrupt" and BSpec: 50862
> .has_snoop_pcie = 1
>
> +static const struct intel_device_info intel_dg1_info = {
> + GEN12_DGFX_FEATURES,
> + PLATFORM(INTEL_DG1),
> + .pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D),
> + .require_force_probe = 1,
> + .engine_mask =
> + BIT(RCS0) | BIT(BCS0) | BIT(VECS0) |
> + BIT(VCS0) | BIT(VCS2),
> +};
> +
> #undef GEN
> #undef PLATFORM
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index c245c10c9bee..207244b9a852 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> PLATFORM_NAME(ELKHARTLAKE),
> PLATFORM_NAME(TIGERLAKE),
> PLATFORM_NAME(ROCKETLAKE),
> + PLATFORM_NAME(DG1),
> };
> #undef PLATFORM_NAME
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index 4bcaa0d6a9e6..64260faac006 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -81,6 +81,7 @@ enum intel_platform {
> /* gen12 */
> INTEL_TIGERLAKE,
> INTEL_ROCKETLAKE,
> + INTEL_DG1,
> INTEL_MAX_PLATFORMS
> };
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/37] drm/i915/dg1: add initial DG-1 definitions
On Wed, 2020-05-20 at 17:37 -0700, Lucas De Marchi wrote:
> From: Abdiel Janulgue
>
> Bspec: 33617, 33617
>
> Cc: José Roberto de Souza
> Cc: Daniele Ceraolo Spurio
> Cc: Stuart Summers
> Cc: Vanshidhar Konda
> Cc: Lucas De Marchi
> Cc: Aravind Iddamsetty
> Cc: Matt Roper
> Signed-off-by: Abdiel Janulgue
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++
> drivers/gpu/drm/i915/i915_pci.c | 12
> drivers/gpu/drm/i915/intel_device_info.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> 4 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a9846205a5e2..382703a6c17d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1421,6 +1421,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> +#define IS_DG1(dev_priv)IS_PLATFORM(dev_priv, INTEL_DG1)
> #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
> #define IS_BDW_ULT(dev_priv) \
> @@ -1541,6 +1542,12 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_RKL_REVID(p, since, until) \
> (IS_ROCKETLAKE(p) && IS_REVID(p, since, until))
>
> +#define DG1_REVID_A0 0x0
> +#define DG1_REVID_B0 0x1
> +
> +#define IS_DG1_REVID(p, since, until) \
> + (IS_DG1(p) && IS_REVID(p, since, until))
> +
> #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> #define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) && IS_LP(dev_priv))
> #define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) && !IS_LP(dev_priv))
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index be52d1b76b2e..e5a851a2dfe7 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -874,9 +874,21 @@ static const struct intel_device_info rkl_info = {
>
> #define GEN12_DGFX_FEATURES \
> GEN12_FEATURES, \
> + .memory_regions = REGION_SMEM | REGION_LMEM, \
REGION_LMEM should be left here for all GEN12 platforms?
I guess it will be detected in runtime that TGL and RKL don't have it but would
be better to not have it in all GEN12 but will let memory experts to
shine in, other than LGTM.
> .is_dgfx = 1, \
> + .has_master_unit_irq = 1, \
> .has_snoop_pcie = 1
>
> +static const struct intel_device_info intel_dg1_info = {
> + GEN12_DGFX_FEATURES,
> + PLATFORM(INTEL_DG1),
> + .pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) | BIT(PIPE_D),
> + .require_force_probe = 1,
> + .engine_mask =
> + BIT(RCS0) | BIT(BCS0) | BIT(VECS0) |
> + BIT(VCS0) | BIT(VCS2),
> +};
> +
> #undef GEN
> #undef PLATFORM
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index c245c10c9bee..207244b9a852 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> PLATFORM_NAME(ELKHARTLAKE),
> PLATFORM_NAME(TIGERLAKE),
> PLATFORM_NAME(ROCKETLAKE),
> + PLATFORM_NAME(DG1),
> };
> #undef PLATFORM_NAME
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index 4bcaa0d6a9e6..64260faac006 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -81,6 +81,7 @@ enum intel_platform {
> /* gen12 */
> INTEL_TIGERLAKE,
> INTEL_ROCKETLAKE,
> + INTEL_DG1,
> INTEL_MAX_PLATFORMS
> };
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
