| Message ID | 20200618004240.16263-8-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce DG1 | expand |
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
On Mon, 22 Jun 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > 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? I'll defer the decision on lmem to folks who actually know this stuff, and focus on display. And from that perspective, I'd like to unblock merging the display patches. BR, Jani. > -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
On Mon, 22 Jun 2020 at 19:55, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 22 Jun 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > > 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? > > I'll defer the decision on lmem to folks who actually know this stuff, > and focus on display. And from that perspective, I'd like to unblock > merging the display patches. > How does display work without LMEM, I'm assuming you have to scan out from VRAM on DG1, even if we only have internal non-uapi is this enough to bring up fbcon? Dave.
On Tue, Jun 23, 2020 at 6:54 AM Dave Airlie <airlied@gmail.com> wrote: > > On Mon, 22 Jun 2020 at 19:55, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > > On Mon, 22 Jun 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > > > 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? > > > > I'll defer the decision on lmem to folks who actually know this stuff, > > and focus on display. And from that perspective, I'd like to unblock > > merging the display patches. > > > > How does display work without LMEM, I'm assuming you have to scan out > from VRAM on DG1, even if we only have internal non-uapi is this > enough to bring up fbcon? It doesn't. And I think merging completely non-functional code to upstream would be rather pointless, no one (outside from intel) can test it, so the usual Dave Airlie "how does this benefit upstream" would get a "not really, just dead weight". But we have some basic lmem code in upstream already, and afaik it's not much code to wire this up into dumb_create and intel_fbdev.c. Which would give us a working kms-only driver (more or less, still early enabling), which can be tested with what's in upstream only (so actually somewhat useful, and unblocks all the display upstreaming, heck we could even do kms-only CI with igt to make sure it keeps working). And as long as we remove the ioctls and DRIVER_RENDER flag (wedging the gpu is a bit overkill) I don't think there's any risk for uapi: As long as the i915 getparam ioctl doesn't work for reading the chipset id, umds skip (might need to double-check that, worst case we have to rename the driver to something like "intel-display" to make sure no one matches). But yeah I think this series here is not quite there yet. And then once Joonas we can try and figure out a new plan for upstreaming lmem refactoring and uapi. When we discussed this all late last summer just upstreaming kms-only was the first planned step, altough back then with the render uapi hidden behind a compile flag. Joonas thinks that's a bit too risk now that we're a lot later. -Daniel
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, \ + .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 };