| Message ID | 20180831154704.3800568-1-bob.j.paauwe@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Rename full ppgtt configuration to be more generic | expand |
Quoting Bob Paauwe (2018-08-31 16:47:04) > For ppgtt, what we're really interested in is the number of page > walk levels for each platform. Rename the device info fields to > reflect this: > > .has_full_48b_ppgtt -> .has_full_4lvl_ppgtt > .has_full_ppgtt -> .has_full_3lvl_ppgtt > > Also add a new field, full_ppgtt_bits, that defines the actual > address range. This gives us more flexibility and will work for > cases where we have platforms with different address ranges but > share the same page walk levels. > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > CC: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 34 +++++++++++++----------- > drivers/gpu/drm/i915/i915_params.c | 3 ++- > drivers/gpu/drm/i915/i915_pci.c | 17 +++++++----- > drivers/gpu/drm/i915/intel_device_info.h | 7 +++-- > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > 11 files changed, 45 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e5b9d3c77139..b9f7903e60d1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2569,8 +2569,8 @@ intel_info(const struct drm_i915_private *dev_priv) > #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv) > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > -#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > +#define USES_FULL_3LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > +#define USES_FULL_4LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > GEM_BUG_ON((sizes) == 0); \ > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index f15a039772db..a0dc3170b358 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -361,7 +361,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > if (IS_ERR(ctx)) > return ctx; > > - if (USES_FULL_PPGTT(dev_priv)) { > + if (USES_FULL_3LVL_PPGTT(dev_priv)) { That is not an improvement. It really is a question of whether or not full-ppgtt is enabled. > struct i915_hw_ppgtt *ppgtt; > > ppgtt = i915_ppgtt_create(dev_priv, file_priv); > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index a926d7d47183..166f1ea1786f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -2201,7 +2201,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); > > eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; > - if (USES_FULL_PPGTT(eb.i915)) > + if (USES_FULL_3LVL_PPGTT(eb.i915)) Again the same complaint. I think you need to rethink the semantics carefully. -Chris
On Fri, 31 Aug 2018 16:51:29 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Bob Paauwe (2018-08-31 16:47:04) > > For ppgtt, what we're really interested in is the number of page > > walk levels for each platform. Rename the device info fields to > > reflect this: > > > > .has_full_48b_ppgtt -> .has_full_4lvl_ppgtt > > .has_full_ppgtt -> .has_full_3lvl_ppgtt > > > > Also add a new field, full_ppgtt_bits, that defines the actual > > address range. This gives us more flexibility and will work for > > cases where we have platforms with different address ranges but > > share the same page walk levels. > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > > CC: Michel Thierry <michel.thierry@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 34 +++++++++++++----------- > > drivers/gpu/drm/i915/i915_params.c | 3 ++- > > drivers/gpu/drm/i915/i915_pci.c | 17 +++++++----- > > drivers/gpu/drm/i915/intel_device_info.h | 7 +++-- > > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > > drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- > > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- > > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > > 11 files changed, 45 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index e5b9d3c77139..b9f7903e60d1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2569,8 +2569,8 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv) > > > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > > -#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > +#define USES_FULL_3LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > +#define USES_FULL_4LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > > GEM_BUG_ON((sizes) == 0); \ > > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > > index f15a039772db..a0dc3170b358 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -361,7 +361,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > > if (IS_ERR(ctx)) > > return ctx; > > > > - if (USES_FULL_PPGTT(dev_priv)) { > > + if (USES_FULL_3LVL_PPGTT(dev_priv)) { > > That is not an improvement. It really is a question of whether or not > full-ppgtt is enabled. > > > struct i915_hw_ppgtt *ppgtt; > > > > ppgtt = i915_ppgtt_create(dev_priv, file_priv); > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index a926d7d47183..166f1ea1786f 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -2201,7 +2201,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); > > > > eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; > > - if (USES_FULL_PPGTT(eb.i915)) > > + if (USES_FULL_3LVL_PPGTT(eb.i915)) > > Again the same complaint. > > I think you need to rethink the semantics carefully. > -Chris Would USES_FULL_PPGTT() and USES_EXTENDED_PPGTT() make more sense then? I think the biggest issue is with the FULL_48BIT_PPGTT name going forward. Bob
On Fri, Aug 31, 2018 at 04:51:29PM +0100, Chris Wilson wrote: > Quoting Bob Paauwe (2018-08-31 16:47:04) > > For ppgtt, what we're really interested in is the number of page > > walk levels for each platform. Rename the device info fields to > > reflect this: > > > > .has_full_48b_ppgtt -> .has_full_4lvl_ppgtt > > .has_full_ppgtt -> .has_full_3lvl_ppgtt > > > > Also add a new field, full_ppgtt_bits, that defines the actual > > address range. This gives us more flexibility and will work for > > cases where we have platforms with different address ranges but > > share the same page walk levels. > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > > CC: Michel Thierry <michel.thierry@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 34 +++++++++++++----------- > > drivers/gpu/drm/i915/i915_params.c | 3 ++- > > drivers/gpu/drm/i915/i915_pci.c | 17 +++++++----- > > drivers/gpu/drm/i915/intel_device_info.h | 7 +++-- > > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > > drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- > > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- > > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > > 11 files changed, 45 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index e5b9d3c77139..b9f7903e60d1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2569,8 +2569,8 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv) > > > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > > -#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > +#define USES_FULL_3LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > +#define USES_FULL_4LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > > GEM_BUG_ON((sizes) == 0); \ > > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > > index f15a039772db..a0dc3170b358 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -361,7 +361,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > > if (IS_ERR(ctx)) > > return ctx; > > > > - if (USES_FULL_PPGTT(dev_priv)) { > > + if (USES_FULL_3LVL_PPGTT(dev_priv)) { > > That is not an improvement. It really is a question of whether or not > full-ppgtt is enabled. I think we do need this change, but only with USES_FULL_PPGTT macro and the rest should be checked with the full_ppgtt number of bits if needed. > > > struct i915_hw_ppgtt *ppgtt; > > > > ppgtt = i915_ppgtt_create(dev_priv, file_priv); > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index a926d7d47183..166f1ea1786f 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -2201,7 +2201,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); > > > > eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; > > - if (USES_FULL_PPGTT(eb.i915)) > > + if (USES_FULL_3LVL_PPGTT(eb.i915)) > > Again the same complaint. > > I think you need to rethink the semantics carefully. > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 31 Aug 2018 13:21:40 -0700 Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Fri, Aug 31, 2018 at 04:51:29PM +0100, Chris Wilson wrote: > > Quoting Bob Paauwe (2018-08-31 16:47:04) > > > For ppgtt, what we're really interested in is the number of page > > > walk levels for each platform. Rename the device info fields to > > > reflect this: > > > > > > .has_full_48b_ppgtt -> .has_full_4lvl_ppgtt > > > .has_full_ppgtt -> .has_full_3lvl_ppgtt > > > > > > Also add a new field, full_ppgtt_bits, that defines the actual > > > address range. This gives us more flexibility and will work for > > > cases where we have platforms with different address ranges but > > > share the same page walk levels. > > > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > > > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > CC: Michel Thierry <michel.thierry@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > > > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 34 +++++++++++++----------- > > > drivers/gpu/drm/i915/i915_params.c | 3 ++- > > > drivers/gpu/drm/i915/i915_pci.c | 17 +++++++----- > > > drivers/gpu/drm/i915/intel_device_info.h | 7 +++-- > > > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > > > drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- > > > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- > > > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > > > 11 files changed, 45 insertions(+), 32 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index e5b9d3c77139..b9f7903e60d1 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -2569,8 +2569,8 @@ intel_info(const struct drm_i915_private *dev_priv) > > > #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv) > > > > > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > > > -#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > > +#define USES_FULL_3LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > > +#define USES_FULL_4LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > > > GEM_BUG_ON((sizes) == 0); \ > > > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > > > index f15a039772db..a0dc3170b358 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > > @@ -361,7 +361,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, > > > if (IS_ERR(ctx)) > > > return ctx; > > > > > > - if (USES_FULL_PPGTT(dev_priv)) { > > > + if (USES_FULL_3LVL_PPGTT(dev_priv)) { > > > > That is not an improvement. It really is a question of whether or not > > full-ppgtt is enabled. > > I think we do need this change, but only with USES_FULL_PPGTT macro > and the rest should be checked with the full_ppgtt number of bits if > needed. > USES_FULL_PPGTT() is currently used to mean at least 3 level, but could be 4 level so I get Chris's point that USES_FULL_3LVL_PPGTT doesn't really work, at least not as a simple substitution. There are only a couple of places where we care that it's actually 4 level so changing those to actually check the number of bits seems to make sense to me. Either #define USES_FULL_4LVL_PPGTT(i915) (i915.number_of_bits > 32) or just use the condition without the macro where needed. > > > > > struct i915_hw_ppgtt *ppgtt; > > > > > > ppgtt = i915_ppgtt_create(dev_priv, file_priv); > > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > index a926d7d47183..166f1ea1786f 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > > @@ -2201,7 +2201,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > > > eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); > > > > > > eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; > > > - if (USES_FULL_PPGTT(eb.i915)) > > > + if (USES_FULL_3LVL_PPGTT(eb.i915)) > > > > Again the same complaint. > > > > I think you need to rethink the semantics carefully. > > -Chris > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 03/10/2018 11:29, Patchwork wrote: > == Series Details == > > Series: drm/i915: Rename full ppgtt configuration to be more generic (rev5) > URL : https://patchwork.freedesktop.org/series/49021/ > State : failure > > == Summary == > > = CI Bug Log - changes from CI_DRM_4915_full -> Patchwork_10327_full = > > == Summary - FAILURE == > > Serious unknown changes coming with Patchwork_10327_full absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_10327_full, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > > > == Possible new issues == > > Here are the unknown changes that may have been introduced in Patchwork_10327_full: > > === IGT changes === > > ==== Possible regressions ==== > > igt@kms_color@pipe-a-ctm-max: > shard-apl: PASS -> FAIL Known issue: https://bugs.freedesktop.org/show_bug.cgi?id=108147 Martin > > {igt@kms_plane_alpha_blend@pipe-b-coverage-7efc}: > shard-skl: NOTRUN -> FAIL > > > == Known issues == > > Here are the changes found in Patchwork_10327_full that come from known issues: > > === IGT changes === > > ==== Issues hit ==== > > igt@drv_suspend@shrink: > shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106886) > > igt@gem_exec_await@wide-contexts: > shard-skl: PASS -> FAIL (fdo#106680) > > igt@gem_exec_big: > shard-hsw: PASS -> TIMEOUT (fdo#107937) > > igt@gem_exec_schedule@pi-ringfull-bsd: > shard-skl: NOTRUN -> FAIL (fdo#103158) > > igt@gem_workarounds@suspend-resume: > shard-skl: NOTRUN -> INCOMPLETE (fdo#104108) > > igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c: > shard-skl: NOTRUN -> DMESG-WARN (fdo#107956) > shard-kbl: PASS -> DMESG-WARN (fdo#107956) > > igt@kms_cursor_crc@cursor-64x21-sliding: > shard-apl: PASS -> FAIL (fdo#103232) > > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff: > shard-apl: PASS -> FAIL (fdo#103167) +2 > > igt@kms_plane_multiple@atomic-pipe-b-tiling-yf: > shard-apl: PASS -> FAIL (fdo#103166) > > igt@kms_setmode@basic: > shard-snb: NOTRUN -> FAIL (fdo#99912) > > igt@kms_vblank@pipe-b-ts-continuation-dpms-rpm: > shard-apl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +9 > > igt@pm_rpm@basic-rte: > shard-skl: PASS -> INCOMPLETE (fdo#107807) > > igt@pm_rpm@gem-execbuf: > shard-skl: PASS -> INCOMPLETE (fdo#107807, fdo#107803) > > igt@pm_rpm@modeset-non-lpsp-stress-no-wait: > shard-skl: SKIP -> INCOMPLETE (fdo#107807) > > > ==== Possible fixes ==== > > igt@kms_ccs@pipe-b-missing-ccs-buffer: > shard-kbl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS +14 > > igt@kms_chv_cursor_fail@pipe-a-256x256-top-edge: > shard-skl: FAIL (fdo#104671) -> PASS > > igt@kms_color@pipe-b-ctm-max: > shard-apl: FAIL -> PASS +1 > > igt@kms_cursor_crc@cursor-256x85-onscreen: > shard-apl: FAIL (fdo#103232) -> PASS +3 > > igt@kms_cursor_crc@cursor-64x64-suspend: > shard-apl: FAIL (fdo#103191, fdo#103232) -> PASS > > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite: > shard-apl: FAIL (fdo#103167) -> PASS +1 > > igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite: > shard-skl: FAIL (fdo#103167) -> PASS +2 > > igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-msflip-blt: > shard-skl: FAIL (fdo#105682) -> PASS > > igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-render: > shard-skl: FAIL (fdo#103167) -> SKIP > > igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence: > shard-skl: FAIL (fdo#103191) -> PASS > > igt@kms_plane@plane-panning-top-left-pipe-a-planes: > shard-skl: FAIL (fdo#103166) -> PASS > > {igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}: > shard-skl: FAIL -> PASS > > igt@kms_plane_multiple@atomic-pipe-a-tiling-x: > shard-apl: FAIL (fdo#103166) -> PASS +1 > > igt@kms_setmode@basic: > shard-apl: FAIL (fdo#99912) -> PASS > shard-kbl: FAIL (fdo#99912) -> PASS > > > {name}: This element is suppressed. This means it is ignored when computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158 > fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166 > fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 > fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 > fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232 > fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558 > fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 > fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 > fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671 > fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 > fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682 > fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680 > fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 > fdo#107803 https://bugs.freedesktop.org/show_bug.cgi?id=107803 > fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807 > fdo#107937 https://bugs.freedesktop.org/show_bug.cgi?id=107937 > fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956 > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > > == Participating hosts (6 -> 5) == > > Missing (1): shard-glk > > > == Build changes == > > * Linux: CI_DRM_4915 -> Patchwork_10327 > > CI_DRM_4915: 26e7a7d954a9c28b97af8ca7813f430fd9117232 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_4660: d0975646c50568e66e65b44b81d28232d059b94e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_10327: 2aa31655991f71141ddda3d1b7ffd252e3a72f78 @ git://anongit.freedesktop.org/gfx-ci/linux > piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10327/shards.html > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e5b9d3c77139..b9f7903e60d1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2569,8 +2569,8 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_EXECLISTS(dev_priv) HAS_LOGICAL_RING_CONTEXTS(dev_priv) #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) -#define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) +#define USES_FULL_3LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) +#define USES_FULL_4LVL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ GEM_BUG_ON((sizes) == 0); \ ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index f15a039772db..a0dc3170b358 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -361,7 +361,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv, if (IS_ERR(ctx)) return ctx; - if (USES_FULL_PPGTT(dev_priv)) { + if (USES_FULL_3LVL_PPGTT(dev_priv)) { struct i915_hw_ppgtt *ppgtt; ppgtt = i915_ppgtt_create(dev_priv, file_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a926d7d47183..166f1ea1786f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2201,7 +2201,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; - if (USES_FULL_PPGTT(eb.i915)) + if (USES_FULL_3LVL_PPGTT(eb.i915)) eb.invalid_flags |= EXEC_OBJECT_NEEDS_GTT; reloc_cache_init(&eb.reloc_cache, eb.i915); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4137af4bd8f5..15f957a6ae38 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -136,19 +136,19 @@ static inline void i915_ggtt_invalidate(struct drm_i915_private *i915) int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, int enable_ppgtt) { - bool has_full_ppgtt; - bool has_full_48bit_ppgtt; + bool has_full_3lvl_ppgtt; + bool has_full_4lvl_ppgtt; if (!dev_priv->info.has_aliasing_ppgtt) return 0; - has_full_ppgtt = dev_priv->info.has_full_ppgtt; - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; + has_full_3lvl_ppgtt = dev_priv->info.has_full_3lvl_ppgtt; + has_full_4lvl_ppgtt = dev_priv->info.has_full_4lvl_ppgtt; if (intel_vgpu_active(dev_priv)) { /* GVT-g has no support for 32bit ppgtt */ - has_full_ppgtt = false; - has_full_48bit_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); + has_full_3lvl_ppgtt = false; + has_full_4lvl_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); } /* @@ -161,10 +161,10 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, if (enable_ppgtt == 1) return 1; - if (enable_ppgtt == 2 && has_full_ppgtt) + if (enable_ppgtt == 2 && has_full_3lvl_ppgtt) return 2; - if (enable_ppgtt == 3 && has_full_48bit_ppgtt) + if (enable_ppgtt == 3 && has_full_4lvl_ppgtt) return 3; /* Disable ppgtt on SNB if VT-d is on. */ @@ -173,10 +173,10 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, return 0; } - if (has_full_48bit_ppgtt) + if (has_full_4lvl_ppgtt) return 3; - if (has_full_ppgtt) + if (has_full_3lvl_ppgtt) return 2; return 1; @@ -1647,9 +1647,10 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915) ppgtt->vm.i915 = i915; ppgtt->vm.dma = &i915->drm.pdev->dev; - ppgtt->vm.total = USES_FULL_48BIT_PPGTT(i915) ? - 1ULL << 48 : - 1ULL << 32; + if (USES_FULL_3LVL_PPGTT(i915) && !USES_FULL_4LVL_PPGTT(i915)) + ppgtt->vm.total = 1ULL << 32; + else + ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits; /* * From bdw, there is support for read-only pages in the PPGTT. @@ -1788,7 +1789,7 @@ static void gen8_ppgtt_enable(struct drm_i915_private *dev_priv) enum intel_engine_id id; for_each_engine(engine, dev_priv, id) { - u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ? + u32 four_level = USES_FULL_4LVL_PPGTT(dev_priv) ? GEN8_GFX_PPGTT_48B : 0; I915_WRITE(RING_MODE_GEN7(engine), _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); @@ -2958,7 +2959,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) /* And finally clear the reserved guard page */ ggtt->vm.clear_range(&ggtt->vm, ggtt->vm.total - PAGE_SIZE, PAGE_SIZE); - if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) { + if (USES_PPGTT(dev_priv) && !USES_FULL_3LVL_PPGTT(dev_priv)) { ret = i915_gem_init_aliasing_ppgtt(dev_priv); if (ret) goto err; @@ -3408,7 +3409,8 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) ggtt->vm.cleanup = gen6_gmch_remove; ggtt->vm.insert_page = gen8_ggtt_insert_page; ggtt->vm.clear_range = nop_clear_range; - if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv)) + if (!USES_FULL_3LVL_PPGTT(dev_priv) || + intel_scanout_needs_vtd_wa(dev_priv)) ggtt->vm.clear_range = gen8_ggtt_clear_range; ggtt->vm.insert_entries = gen8_ggtt_insert_entries; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 295e981e4a39..71ac381807a6 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -84,7 +84,8 @@ i915_param_named_unsafe(enable_hangcheck, bool, 0644, i915_param_named_unsafe(enable_ppgtt, int, 0400, "Override PPGTT usage. " - "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)"); + "(-1=auto [default], 0=disabled, 1=aliasing, 2=full with 32 bits, " + "3=full with extended address space)"); i915_param_named_unsafe(enable_psr, int, 0600, "Enable PSR " diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index d6f7b9fe1d26..6e0d4476e553 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -298,7 +298,8 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = { .has_rc6 = 1, \ .has_rc6p = 1, \ .has_aliasing_ppgtt = 1, \ - .has_full_ppgtt = 1, \ + .has_full_3lvl_ppgtt = 1, \ + .full_ppgtt_bits = 32, \ GEN_DEFAULT_PIPEOFFSETS, \ GEN_DEFAULT_PAGE_SIZES, \ IVB_CURSOR_OFFSETS @@ -352,7 +353,8 @@ static const struct intel_device_info intel_valleyview_info = { .has_gmch_display = 1, .has_hotplug = 1, .has_aliasing_ppgtt = 1, - .has_full_ppgtt = 1, + .has_full_3lvl_ppgtt = 1, + .full_ppgtt_bits = 32, .has_snoop = true, .has_coherent_ggtt = false, .ring_mask = RENDER_RING | BSD_RING | BLT_RING, @@ -399,7 +401,8 @@ static const struct intel_device_info intel_haswell_gt3_info = { .page_sizes = I915_GTT_PAGE_SIZE_4K | \ I915_GTT_PAGE_SIZE_2M, \ .has_logical_ring_contexts = 1, \ - .has_full_48bit_ppgtt = 1, \ + .has_full_4lvl_ppgtt = 1, \ + .full_ppgtt_bits = 48, \ .has_64bit_reloc = 1, \ .has_reset_engine = 1 @@ -444,7 +447,8 @@ static const struct intel_device_info intel_cherryview_info = { .has_logical_ring_contexts = 1, .has_gmch_display = 1, .has_aliasing_ppgtt = 1, - .has_full_ppgtt = 1, + .has_full_3lvl_ppgtt = 1, + .full_ppgtt_bits = 32, .has_reset_engine = 1, .has_snoop = true, .has_coherent_ggtt = false, @@ -519,8 +523,9 @@ static const struct intel_device_info intel_skylake_gt4_info = { .has_logical_ring_preemption = 1, \ .has_guc = 1, \ .has_aliasing_ppgtt = 1, \ - .has_full_ppgtt = 1, \ - .has_full_48bit_ppgtt = 1, \ + .has_full_3lvl_ppgtt = 1, \ + .has_full_4lvl_ppgtt = 1, \ + .full_ppgtt_bits = 48, \ .has_reset_engine = 1, \ .has_snoop = true, \ .has_coherent_ggtt = false, \ diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 6eecd64734d5..df3263b97c7d 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -87,8 +87,8 @@ enum intel_platform { func(has_reset_engine); \ func(has_fbc); \ func(has_fpga_dbg); \ - func(has_full_ppgtt); \ - func(has_full_48bit_ppgtt); \ + func(has_full_3lvl_ppgtt); \ + func(has_full_4lvl_ppgtt); \ func(has_gmch_display); \ func(has_guc); \ func(has_guc_ct); \ @@ -182,6 +182,9 @@ struct intel_device_info { u16 degamma_lut_size; u16 gamma_lut_size; } color; + + /* PPGTT bit size */ + int full_ppgtt_bits; }; struct intel_driver_caps { diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index e272127783fe..9f74244ef3e1 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -1434,7 +1434,7 @@ static int igt_ppgtt_pin_update(void *arg) * huge-gtt-pages. */ - if (!USES_FULL_48BIT_PPGTT(dev_priv)) { + if (!USES_FULL_4LVL_PPGTT(dev_priv)) { pr_info("48b PPGTT not supported, skipping\n"); return 0; } diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c index 128ad1cf0647..05df36863694 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c @@ -351,7 +351,7 @@ static int igt_evict_contexts(void *arg) * where the GTT space of the request is separate from the GGTT * allocation required to build the request. */ - if (!USES_FULL_PPGTT(i915)) + if (!USES_FULL_3LVL_PPGTT(i915)) return 0; mutex_lock(&i915->drm.struct_mutex); diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index 8e2e269db97e..49420e98f374 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -1001,7 +1001,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv, IGT_TIMEOUT(end_time); int err; - if (!USES_FULL_PPGTT(dev_priv)) + if (!USES_FULL_3LVL_PPGTT(dev_priv)) return 0; file = mock_file(dev_priv); diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 43ed8b28aeaa..33d7225edbbb 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) I915_GTT_PAGE_SIZE_64K | I915_GTT_PAGE_SIZE_2M; + mkwrite_device_info(i915)->full_ppgtt_bits = 48; + mock_uncore_init(i915); i915_gem_init__mm(i915);
For ppgtt, what we're really interested in is the number of page walk levels for each platform. Rename the device info fields to reflect this: .has_full_48b_ppgtt -> .has_full_4lvl_ppgtt .has_full_ppgtt -> .has_full_3lvl_ppgtt Also add a new field, full_ppgtt_bits, that defines the actual address range. This gives us more flexibility and will work for cases where we have platforms with different address ranges but share the same page walk levels. Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> CC: Rodrigo Vivi <rodrigo.vivi@intel.com> CC: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 4 +-- drivers/gpu/drm/i915/i915_gem_context.c | 2 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 34 +++++++++++++----------- drivers/gpu/drm/i915/i915_params.c | 3 ++- drivers/gpu/drm/i915/i915_pci.c | 17 +++++++----- drivers/gpu/drm/i915/intel_device_info.h | 7 +++-- drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ 11 files changed, 45 insertions(+), 32 deletions(-)