Re: [Intel-gfx] [PATCH v2 07/32] drm/i915/dg1: add initial DG-1 definitions

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 17, 2020 at 05:42:15PM -0700, Lucas De Marchi wrote:
> From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>
> Bspec: 33617, 33617
>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> Cc: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  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 2f8057a0b2280..f79c09257eb6b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1428,6 +1428,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) \
> @@ -1556,6 +1557,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 e5fdf17cd9cdd..58cceeaa0ffa5 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -896,8 +896,20 @@ static const struct intel_device_info rkl_info = {
>
>  #define GEN12_DGFX_FEATURES \
>       GEN12_FEATURES, \
> +     .memory_regions = REGION_SMEM | REGION_LMEM, \

This has lmem, and we need a new uapi for that. Last year we discussed a
plan to have that behind a very scary compile-time only option, to make
absolutely sure no one will start relying on this broken state before we
aligned everything. And we know the current status is wrong since this
patch series doesn't include any of the lmem specific uapi that's being
worked on.

But now almost a year passed, so that original plan needs to be
renegotiated. Personally I'm not sure the scary compile option makes
sense any longer, we're way later, users will soon have real hw, and once
they have that they will find ways to enable it and we're potentially
screwed.  Discussed this also with Joonas last week or so in private, and
he shares similar concerns.

So I think best option here (since keeping patches out of tree is rarely
best option) would be to merge just the display enabling for DG1, but also
making sure that we have all the rendering support completely disabled.
This would mean:
- wedge gpu on startup, just to make sure
- drm_driver->num_ioctls = 0 (plus ioctls = NULL)
- no setting DRIVER_RENDER and DRIVER_SYNCOBJ, we'd be a pure
display-only driver

Adding Dave and drm-intel maintainers to quickly hash this out. Thoughts?
-Daniel


> +     .has_master_unit_irq = 1, \
>       .is_dgfx = 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 544ac61fbc363..2e40a6649d142 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -63,6 +63,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 770d07003ce60..bdd21cde917cc 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -82,6 +82,7 @@ enum intel_platform {
>       /* gen12 */
>       INTEL_TIGERLAKE,
>       INTEL_ROCKETLAKE,
> +     INTEL_DG1,
>       INTEL_MAX_PLATFORMS
>  };
>
> --
> 2.26.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx