[Intel-gfx] ✗ Fi.CI.IGT: warning for drm/i915: Stop using long platform names on clock gating functions.
== Series Details == Series: drm/i915: Stop using long platform names on clock gating functions. URL : https://patchwork.freedesktop.org/series/29453/ State : warning == Summary == Test kms_chv_cursor_fail: Subgroup pipe-C-128x128-bottom-edge: pass -> SKIP (shard-hsw) Test kms_frontbuffer_tracking: Subgroup fbc-1p-offscren-pri-shrfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Test kms_flip: Subgroup plain-flip-ts-check: fail -> PASS (shard-hsw) Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 shard-hswtotal:2230 pass:1228 dwarn:0 dfail:0 fail:18 skip:984 time:9621s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5515/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] tests/kms_frontbuffer_tracking: increase FBC wait timeout to 5s
I can no longer reproduce the flip/flopping "FBC disabled" on the kms_frontbuffer_tracking tests. Instead I hit: WARNING: CPU: 2 PID: 25732 at drivers/gpu/drm/i915/intel_fbc.c:1173 WARNING: CPU: 2 PID: 25732 at drivers/gpu/drm/i915/intel_fbc.c:1141 /Marta > -Original Message- > From: Lofstedt, Marta > Sent: Friday, August 25, 2017 4:34 PM > To: Chris Wilson ; intel-gfx@lists.freedesktop.org > Cc: Zanoni, Paulo R > Subject: RE: [Intel-gfx] [PATCH i-g-t v2] tests/kms_frontbuffer_tracking: > increase FBC wait timeout to 5s > > +paulo > > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: Friday, August 25, 2017 4:12 PM > > To: Lofstedt, Marta ; intel- > > g...@lists.freedesktop.org > > Subject: RE: [Intel-gfx] [PATCH i-g-t v2] tests/kms_frontbuffer_tracking: > > increase FBC wait timeout to 5s > > > > Quoting Lofstedt, Marta (2017-08-25 13:50:16) > > > > > > > > > > -Original Message- > > > > From: Lofstedt, Marta > > > > Sent: Friday, August 25, 2017 2:54 PM > > > > To: 'Chris Wilson' ; > > > > intel-gfx@lists.freedesktop.org > > > > Subject: RE: [Intel-gfx] [PATCH i-g-t v2] > tests/kms_frontbuffer_tracking: > > > > increase FBC wait timeout to 5s > > > > > > > > > > > > > > > > > -Original Message- > > > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > > > Sent: Friday, August 25, 2017 1:47 PM > > > > > To: Lofstedt, Marta ; intel- > > > > > g...@lists.freedesktop.org > > > > > Subject: Re: [Intel-gfx] [PATCH i-g-t v2] > > tests/kms_frontbuffer_tracking: > > > > > increase FBC wait timeout to 5s > > > > > > > > > > Quoting Marta Lofstedt (2017-08-25 11:40:29) > > > > > > From: "Lofstedt, Marta" > > > > > > > > > > > > The subtests: igt@kms_frontbuffer_tracking@fbc-*draw* > > > > > > has non-consistent results, pending between fail and pass. > > > > > > The fails are always due to "FBC disabled". > > > > > > With this increase in timeout the flip-flop behavior is no > > > > > > longer reproducible. > > > > > > > > > > > > This is a partial revert of: > > > > > > 64590c7b768dc8d8dd962f812d5ff5a39e7e8b54, > > > > > > where the timeout was decreased from 5s to 2s. > > > > > > After investigating the timeout needed, the conclusion is that > > > > > > the longer timeout is only needed when the test swaps between > > > > > > some specific draw domains, typically blt vs. mmap_cpu. > > > > > > The objective of the FBC part of the tests is not to benchmark > > > > > > draw domain changes, it is to check that FBC was (re-)enabled. > > > > > > > > > > > > V2: Added documentation > > > > > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101623 > > > > > > Signed-off-by: Marta Lofstedt > > > > > > Acked-by: Paulo Zanoni > > > > > > --- > > > > > > tests/kms_frontbuffer_tracking.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/tests/kms_frontbuffer_tracking.c > > > > > > b/tests/kms_frontbuffer_tracking.c > > > > > > index e03524f1..2538450c 100644 > > > > > > --- a/tests/kms_frontbuffer_tracking.c > > > > > > +++ b/tests/kms_frontbuffer_tracking.c > > > > > > @@ -924,7 +924,7 @@ static bool fbc_stride_not_supported(void) > > > > > > > > > > > > static bool fbc_wait_until_enabled(void) { > > > > > > > > > > Try igt_drop_caches_set(device, DROP_RETIRE); instead of > > > > > relaxing the timeout. > > > > > -Chris > > > > > > > > OK, I will test that and do a V3 if it works! > > > > /Marta > > > > > > I did some initial testing with igt_drop_caches_set inside > > fbc_wait_until_enabled and it looks good, I will add this to my > > weekend tests to get more results. This also appear to improve the > > runtime of the tests quite a bit. So, maybe the igt_drop_caches_set > > should be placed somewhere else so it will give runtime improvements > > not only for the FBC related sub- tests. > > > > Sure, all the waits can do with the retire first, give it a common > > function and a comment for the rationale (which should pretty much the > > same as given in the changelog). Anytime we use the GPU to invalidate > > the frontbuffer tracking, we have to wait for a retire to do the flush. > > Retirement is lazy, and is normally driven by GPU activity but we have > > a background kworker to make sure we notice when the system becomes > > idle independent of userspace - except it's low frequency. > > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4] pm_rps: Changes in waitboost scenario
On Mon, 2017-08-28 at 10:50 +0200, Katarzyna Dec wrote: > CI is observing sporadical failures in pm_rps subtests. > There are a couple of reasons. One of them is the fact that > on gen6, gen7 and gen7.5, max frequency (as in the HW limit) > is not set to RP0, but the value obtaind from PCODE (which > may be different from RP0). Thus the test is operating under > wrong assumptions (SOFTMAX == RP0 == BOOST which is simply > not the case). Let's compare current frequency with BOOST > frequency rather than SOFTMAX to get the test behaviour under control. > In boost_freq function I set MAX freq to midium freqency, which ensures > that we for sure reach BOOST frequency. This could help with failures > with boost frequency failing to drop down. > GPU reset needs to be modified so we are not dependent on kernel's low > priority retire worker. Reset method was replaced by igt_force_gpu_reset() > and in reset testcase we make sure that we can recover from hang. > > v2: Commit message, simplified waiting for boost to finish, drop > noisy whitespace cleanup. > > v3: Removed reading from i915_rps_boost_info debugfs because it not > the same on every kernel. Removed function waiting for boost. > Instead of that I made sure we will reach in boost by setting MAX freq to > fmid. > > v4: Moved proposal with making test drm master to other patch > > v5: Used igt_force_gpu_reset() to reset GPU. Modified "reset" testcase. > > Cc: Chris Wilson > Cc: Jeff Mcgee > Cc: Petri Latvala > Cc: Jani Saarinen > Cc: Radoslaw Szwichtenberg > Signed-off-by: Katarzyna Dec > --- > tests/pm_rps.c | 63 +++ > --- > 1 file changed, 38 insertions(+), 25 deletions(-) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index f0455e78..a6c6f1eb 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -50,6 +50,7 @@ enum { > RP0, > RP1, > RPn, > + BOOST, > NUMFREQ > }; > > @@ -60,7 +61,7 @@ struct junk { > const char *mode; > FILE *filp; > } stuff[] = { > - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, > { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, > NULL, NULL } > + { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, > { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", > "rb+", NULL }, { NULL, NULL, NULL } > }; > > static int readval(FILE *filp) > @@ -167,7 +168,7 @@ static void dump(const int *freqs) > } > > enum load { > - LOW, > + LOW = 0, > HIGH > }; > > @@ -185,9 +186,10 @@ static struct load_helper { > > static void load_helper_signal_handler(int sig) > { > - if (sig == SIGUSR2) > - lh.load = lh.load == LOW ? HIGH : LOW; > - else > + if (sig == SIGUSR2) { > + lh.load = !lh.load; > + igt_debug("Switching background load to %s\n", lh.load ? > "high" : "low"); > + } else > lh.exit = true; > } > > @@ -238,6 +240,7 @@ static void load_helper_run(enum load load) > return; > } > > + lh.exit = false; > lh.load = load; > > igt_fork_helper(&lh.igt_proc) { > @@ -263,6 +266,8 @@ static void load_helper_run(enum load load) > if (intel_gen(lh.devid) >= 6) > execbuf.flags = I915_EXEC_BLT; > > + igt_debug("Applying %s load...\n", lh.load ? "high" : "low"); > + > while (!lh.exit) { > memset(&object, 0, sizeof(object)); > object.handle = fences[val%3]; > @@ -296,6 +301,8 @@ static void load_helper_run(enum load load) > gem_close(drm_fd, fences[0]); > gem_close(drm_fd, fences[1]); > gem_close(drm_fd, fences[2]); > + > + igt_drop_caches_set(drm_fd, DROP_RETIRE); > } > } > > @@ -553,38 +560,43 @@ static void stabilize_check(int *out) > igt_debug("Waited %d msec to stabilize cur\n", wait); > } > > -static void reset_gpu(void) > -{ > - int fd = drm_open_driver(DRIVER_INTEL); > - igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); > - close(fd); > -} > - > static void boost_freq(int fd, int *boost_freqs) > { > int64_t timeout = 1; > - int ring = -1; > igt_spin_t *load; > + unsigned int engine; > + int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > > - load = igt_spin_batch_new(fd, ring, 0); > + fmid = get_hw_rounded_freq(fmid); > + //set max freq to less then boost freq Looks good to me. Just one very minor comment - we do use two different comment styles, should we instead use one? Do you think we should add any simple test descriptions above the tests or these tests are easy to understand? > + writeval(stuff[MAX].filp, fmid); > > + /* put boost on the same engine as low load */ > + engine = I915_EXEC_RENDER; Otherwise Reviewed-by: Radoslaw Szwichtenberg https://lists
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Unify skylake plane update
On Mon, 28 Aug 2017, Juha-Pekka Heikkila wrote: > Don't handle skylake primary plane separately as it is similar > plane as the others. This misses another reference to skylake_update_primary_plane, as reported by CI. BR, Jani. > > Signed-off-by: Juha-Pekka Heikkila > --- > drivers/gpu/drm/i915/intel_display.c | 79 > +--- > drivers/gpu/drm/i915/intel_drv.h | 3 ++ > drivers/gpu/drm/i915/intel_sprite.c | 2 +- > 3 files changed, 5 insertions(+), 79 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index f922e2f..96eac33 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3534,83 +3534,6 @@ u32 skl_plane_ctl(const struct intel_crtc_state > *crtc_state, > return plane_ctl; > } > > -static void skylake_update_primary_plane(struct intel_plane *plane, > - const struct intel_crtc_state > *crtc_state, > - const struct intel_plane_state > *plane_state) > -{ > - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > - const struct drm_framebuffer *fb = plane_state->base.fb; > - enum plane_id plane_id = plane->id; > - enum pipe pipe = plane->pipe; > - u32 plane_ctl = plane_state->ctl; > - unsigned int rotation = plane_state->base.rotation; > - u32 stride = skl_plane_stride(fb, 0, rotation); > - u32 aux_stride = skl_plane_stride(fb, 1, rotation); > - u32 surf_addr = plane_state->main.offset; > - int scaler_id = plane_state->scaler_id; > - int src_x = plane_state->main.x; > - int src_y = plane_state->main.y; > - int src_w = drm_rect_width(&plane_state->base.src) >> 16; > - int src_h = drm_rect_height(&plane_state->base.src) >> 16; > - int dst_x = plane_state->base.dst.x1; > - int dst_y = plane_state->base.dst.y1; > - int dst_w = drm_rect_width(&plane_state->base.dst); > - int dst_h = drm_rect_height(&plane_state->base.dst); > - unsigned long irqflags; > - > - /* Sizes are 0 based */ > - src_w--; > - src_h--; > - dst_w--; > - dst_h--; > - > - crtc->dspaddr_offset = surf_addr; > - > - crtc->adjusted_x = src_x; > - crtc->adjusted_y = src_y; > - > - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > - > - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) { > - I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), > - PLANE_COLOR_PIPE_GAMMA_ENABLE | > - PLANE_COLOR_PIPE_CSC_ENABLE | > - PLANE_COLOR_PLANE_GAMMA_DISABLE); > - } > - > - I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); > - I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x); > - I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); > - I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); > - I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), > - (plane_state->aux.offset - surf_addr) | aux_stride); > - I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), > - (plane_state->aux.y << 16) | plane_state->aux.x); > - > - if (scaler_id >= 0) { > - uint32_t ps_ctrl = 0; > - > - WARN_ON(!dst_w || !dst_h); > - ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane_id) | > - crtc_state->scaler_state.scalers[scaler_id].mode; > - I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); > - I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); > - I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x << 16) | > dst_y); > - I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | > dst_h); > - I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); > - } else { > - I915_WRITE_FW(PLANE_POS(pipe, plane_id), (dst_y << 16) | dst_x); > - } > - > - I915_WRITE_FW(PLANE_SURF(pipe, plane_id), > - intel_plane_ggtt_offset(plane_state) + surf_addr); > - > - POSTING_READ_FW(PLANE_SURF(pipe, plane_id)); > - > - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > -} > - > static void skylake_disable_primary_plane(struct intel_plane *primary, > struct intel_crtc *crtc) > { > @@ -13275,7 +13198,7 @@ intel_primary_plane_create(struct drm_i915_private > *dev_priv, enum pipe pipe) > else > modifiers = skl_format_modifiers_noccs; > > - primary->update_plane = skylake_update_primary_plane; > + primary->update_plane = skl_update_plane; > primary->disable_plane = skylake_disable_primary_plane; > } else if (INTEL_GEN(dev_priv) >= 4) { > intel_primary_formats = i965_primary_formats; > diff --git a/drive
[Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
The private PAT management is to support PPAT entry manipulation. Two APIs are introduced for dynamically managing PPAT entries: intel_ppat_get and intel_ppat_put. intel_ppat_get will search for an existing PPAT entry which perfectly matches the required PPAT value. If not, it will try to allocate or return a partially matched PPAT entry if there is any available PPAT indexes or not. intel_ppat_put will put back the PPAT entry which comes from intel_ppat_get. If it's dynamically allocated, the reference count will be decreased. If the reference count turns into zero, the PPAT index is freed again. Besides, another two callbacks are introduced to support the private PAT management framework. One is ppat->update(), which writes the PPAT configurations in ppat->entries into HW. Another one is ppat->match, which will return a score to show how two PPAT values match with each other. v5: - Add check and warnnings for those platforms which don't have PPAT. v3: - Introduce dirty bitmap for PPAT registers. (Chris) - Change the name of the pointer "dev_priv" to "i915". (Chris) - intel_ppat_{get, put} returns/takes a const intel_ppat_entry *. (Chris) v2: - API re-design. (Chris) Cc: Ben Widawsky Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Joonas Lahtinen Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem_gtt.c | 289 ++-- drivers/gpu/drm/i915/i915_gem_gtt.h | 34 + 3 files changed, 276 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7587ef5..5ffde10 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2312,6 +2312,8 @@ struct drm_i915_private { DECLARE_HASHTABLE(mm_structs, 7); struct mutex mm_lock; + struct intel_ppat ppat; + /* Kernel Modesetting */ struct intel_crtc *plane_to_crtc_mapping[I915_MAX_PIPES]; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b74fa9d..b99b6ca 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2816,41 +2816,215 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size) return 0; } -static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv) +static struct intel_ppat_entry *alloc_ppat_entry(struct intel_ppat *ppat, +unsigned int index, +u8 value) { + struct intel_ppat_entry *entry = &ppat->entries[index]; + + entry->ppat = ppat; + entry->value = value; + kref_init(&entry->ref_count); + set_bit(index, ppat->used); + set_bit(index, ppat->dirty); + + return entry; +} + +static void free_ppat_entry(struct intel_ppat_entry *entry) +{ + struct intel_ppat *ppat = entry->ppat; + int index = entry - ppat->entries; + + entry->value = ppat->dummy_value; + clear_bit(index, ppat->used); + set_bit(index, ppat->dirty); +} + +/** + * intel_ppat_get - get a usable PPAT entry + * @i915: i915 device instance + * @value: the PPAT value required by the caller + * + * The function tries to search if there is an existing PPAT entry which + * matches with the required value. If perfectly matched, the existing PPAT + * entry will be used. If only partially matched, it will try to check if + * there is any available PPAT index. If yes, it will allocate a new PPAT + * index for the required entry and update the HW. If not, the partially + * matched entry will be used. + */ +const struct intel_ppat_entry *intel_ppat_get(struct drm_i915_private *i915, + u8 value) +{ + struct intel_ppat *ppat = &i915->ppat; + struct intel_ppat_entry *entry; + int i, used; + unsigned int score, best_score; + + if (WARN_ON(!ppat->max_entries)) + return ERR_PTR(-ENODEV); + + score = best_score = 0; + used = 0; + + /* First, find a suitable value from available entries */ + for_each_set_bit(i, ppat->used, ppat->max_entries) { + score = ppat->match(ppat->entries[i].value, value); + /* Perfect match */ + if (score == INTEL_PPAT_PERFECT_MATCH) { + entry = &ppat->entries[i]; + kref_get(&entry->ref_count); + return entry; + } + + if (score > best_score) { + entry = &ppat->entries[i]; + best_score = score; + } + used++; + } + + /* No matched entry and we can't allocate a new entry. */ + if (!best_score && used == ppat->max_entries) { + DRM_ERROR("Fail to get PPAT entry\n"); + return ERR_PTR(-ENOSPC); + } + + /* +* Found a matched entry which is
[Intel-gfx] [RFCv5 1/2] drm/i915: Factor out setup_private_pat()
Factor out setup_private_pat() for introducing the following patches. Cc: Ben Widawsky Cc: Rodrigo Vivi Cc: Chris Wilson Cc: Joonas Lahtinen Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/i915_gem_gtt.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 708b95c..b74fa9d 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2915,6 +2915,16 @@ static void gen6_gmch_remove(struct i915_address_space *vm) cleanup_scratch_page(vm); } +static void setup_private_pat(struct drm_i915_private *dev_priv) +{ + if (INTEL_GEN(dev_priv) >= 10) + cnl_setup_private_ppat(dev_priv); + else if (IS_CHERRYVIEW(dev_priv) || IS_GEN9_LP(dev_priv)) + chv_setup_private_ppat(dev_priv); + else + bdw_setup_private_ppat(dev_priv); +} + static int gen8_gmch_probe(struct i915_ggtt *ggtt) { struct drm_i915_private *dev_priv = ggtt->base.i915; @@ -2947,14 +2957,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) } ggtt->base.total = (size / sizeof(gen8_pte_t)) << PAGE_SHIFT; - - if (INTEL_GEN(dev_priv) >= 10) - cnl_setup_private_ppat(dev_priv); - else if (IS_CHERRYVIEW(dev_priv) || IS_GEN9_LP(dev_priv)) - chv_setup_private_ppat(dev_priv); - else - bdw_setup_private_ppat(dev_priv); - ggtt->base.cleanup = gen6_gmch_remove; ggtt->base.bind_vma = ggtt_bind_vma; ggtt->base.unbind_vma = ggtt_unbind_vma; @@ -2975,6 +2977,8 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) ggtt->invalidate = gen6_ggtt_invalidate; + setup_private_pat(dev_priv); + return ggtt_probe_common(ggtt, size); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 2/3] docs: Add user and developer documentation about Chamelium support
On Mon, 2017-08-28 at 11:12 +0300, Paul Kocialkowski wrote: > This introduces plain-text documentation about the Chamelium aimed at > users who wish to deploy the platform, as well as developers who wish > to work on improving IGT support for it. > > Given the contents of this documentation, it felt more relevant to make > it part of the tree instead of the API reference. > > Signed-off-by: Paul Kocialkowski Read it twice and is pretty clear to me. Also no spelling mistakes. Reviewed-by: Radoslaw Szwichtenberg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [RFCv5,1/2] drm/i915: Factor out setup_private_pat()
== Series Details == Series: series starting with [RFCv5,1/2] drm/i915: Factor out setup_private_pat() URL : https://patchwork.freedesktop.org/series/29456/ State : success == Summary == Series 29456v1 series starting with [RFCv5,1/2] drm/i915: Factor out setup_private_pat() https://patchwork.freedesktop.org/api/1.0/series/29456/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:452s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:442s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:359s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:552s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:523s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:525s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:511s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:432s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:613s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:449s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:425s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:500s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:480s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:600s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:599s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:527s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:471s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:486s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:497s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:443s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:557s fi-snb-2600 total:279 pass:248 dwarn:0 dfail:0 fail:2 skip:29 time:405s fi-skl-x1585l failed to connect after reboot 79cec1e1ffe2fcb29c714d06db0441a49b9569d6 drm-tip: 2017y-08m-29d-06h-07m-10s UTC integration manifest 94ad27228d45 drm/i915: Introduce private PAT management 98045f6c7b22 drm/i915: Factor out setup_private_pat() == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5516/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4] pm_rps: Changes in waitboost scenario
On Tue, Aug 29, 2017 at 07:43:20AM +, Szwichtenberg, Radoslaw wrote: > On Mon, 2017-08-28 at 10:50 +0200, Katarzyna Dec wrote: > > CI is observing sporadical failures in pm_rps subtests. > > There are a couple of reasons. One of them is the fact that > > on gen6, gen7 and gen7.5, max frequency (as in the HW limit) > > is not set to RP0, but the value obtaind from PCODE (which > > may be different from RP0). Thus the test is operating under > > wrong assumptions (SOFTMAX == RP0 == BOOST which is simply > > not the case). Let's compare current frequency with BOOST > > frequency rather than SOFTMAX to get the test behaviour under control. > > In boost_freq function I set MAX freq to midium freqency, which ensures > > that we for sure reach BOOST frequency. This could help with failures > > with boost frequency failing to drop down. > > GPU reset needs to be modified so we are not dependent on kernel's low > > priority retire worker. Reset method was replaced by igt_force_gpu_reset() > > and in reset testcase we make sure that we can recover from hang. > > > > v2: Commit message, simplified waiting for boost to finish, drop > > noisy whitespace cleanup. > > > > v3: Removed reading from i915_rps_boost_info debugfs because it not > > the same on every kernel. Removed function waiting for boost. > > Instead of that I made sure we will reach in boost by setting MAX freq to > > fmid. > > > > v4: Moved proposal with making test drm master to other patch > > > > v5: Used igt_force_gpu_reset() to reset GPU. Modified "reset" testcase. > > > > Cc: Chris Wilson > > Cc: Jeff Mcgee > > Cc: Petri Latvala > > Cc: Jani Saarinen > > Cc: Radoslaw Szwichtenberg > > Signed-off-by: Katarzyna Dec > > --- > > tests/pm_rps.c | 63 +++ > > --- > > 1 file changed, 38 insertions(+), 25 deletions(-) > > > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > > index f0455e78..a6c6f1eb 100644 > > --- a/tests/pm_rps.c > > +++ b/tests/pm_rps.c > > @@ -50,6 +50,7 @@ enum { > > RP0, > > RP1, > > RPn, > > + BOOST, > > NUMFREQ > > }; > > > > @@ -60,7 +61,7 @@ struct junk { > > const char *mode; > > FILE *filp; > > } stuff[] = { > > - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, > > { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, > > NULL, NULL } > > + { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, > > { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", > > "rb+", NULL }, { NULL, NULL, NULL } > > }; > > > > static int readval(FILE *filp) > > @@ -167,7 +168,7 @@ static void dump(const int *freqs) > > } > > > > enum load { > > - LOW, > > + LOW = 0, > > HIGH > > }; > > > > @@ -185,9 +186,10 @@ static struct load_helper { > > > > static void load_helper_signal_handler(int sig) > > { > > - if (sig == SIGUSR2) > > - lh.load = lh.load == LOW ? HIGH : LOW; > > - else > > + if (sig == SIGUSR2) { > > + lh.load = !lh.load; > > + igt_debug("Switching background load to %s\n", lh.load ? > > "high" : "low"); > > + } else > > lh.exit = true; > > } > > > > @@ -238,6 +240,7 @@ static void load_helper_run(enum load load) > > return; > > } > > > > + lh.exit = false; > > lh.load = load; > > > > igt_fork_helper(&lh.igt_proc) { > > @@ -263,6 +266,8 @@ static void load_helper_run(enum load load) > > if (intel_gen(lh.devid) >= 6) > > execbuf.flags = I915_EXEC_BLT; > > > > + igt_debug("Applying %s load...\n", lh.load ? "high" : "low"); > > + > > while (!lh.exit) { > > memset(&object, 0, sizeof(object)); > > object.handle = fences[val%3]; > > @@ -296,6 +301,8 @@ static void load_helper_run(enum load load) > > gem_close(drm_fd, fences[0]); > > gem_close(drm_fd, fences[1]); > > gem_close(drm_fd, fences[2]); > > + > > + igt_drop_caches_set(drm_fd, DROP_RETIRE); > > } > > } > > > > @@ -553,38 +560,43 @@ static void stabilize_check(int *out) > > igt_debug("Waited %d msec to stabilize cur\n", wait); > > } > > > > -static void reset_gpu(void) > > -{ > > - int fd = drm_open_driver(DRIVER_INTEL); > > - igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); > > - close(fd); > > -} > > - > > static void boost_freq(int fd, int *boost_freqs) > > { > > int64_t timeout = 1; > > - int ring = -1; > > igt_spin_t *load; > > + unsigned int engine; > > + int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > > > > - load = igt_spin_batch_new(fd, ring, 0); > > + fmid = get_hw_rounded_freq(fmid); > > + //set max freq to less then boost freq > Looks good to me. > Just one very minor comment - we do use two different comment styles, should > we > instead use one? Do you think we should add any simple test descr
Re: [Intel-gfx] [PATCH i-g-t] tests/gem_exec_reuse: Adds cleanup at the end of test.
Quoting Antonio Argenziano (2017-08-29 00:18:34) > This patch introduces a fixture at the end of the test to destroy > objects that have been created and stop the hang detector. We shouldn't have to. Resources are cleaned up by the kernel, and hang detector is using the fork helper which is meant to make sure all is well upon exit. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/1] igt/dapc: Test Driver Assisted Performance Capture (DAPC)
Hi Sagar, Thanks for writing this test. It looks promising but there are a few issues that needs to be addressed for this to run in CI. Please have a look at the comments below. Thanks! On 28/08/17 10:53, Sagar Arun Kamble wrote: This test verifies different i915 perf sampling options for fields like PID, CTX ID, Timestamp, OA Report, TAG, MMIO. Cc: Lionel Landwerlin Signed-off-by: Sourab Gupta Signed-off-by: Sagar Arun Kamble --- tests/Makefile.sources |1 + tests/dapc.c | 1017 2 files changed, 1018 insertions(+) create mode 100644 tests/dapc.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index bb013c7..61feb0d 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -26,6 +26,7 @@ TESTS_progs = \ core_getversion \ core_prop_blob \ core_setmaster_vs_auth \ + dapc \ debugfs_test \ drm_import_export \ drm_mm \ diff --git a/tests/dapc.c b/tests/dapc.c new file mode 100644 index 000..f49b1cd --- /dev/null +++ b/tests/dapc.c @@ -0,0 +1,1017 @@ +/* + * Copyright © 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * dapc: Driver Assisted Performance Capture + * This tests the i915 perf functionality to sample various metrics by + * associating with the CS stream or just standalone periodic OA samples. + * Verifies fields like PID, CTX ID, Timestamp, OA Report, MMIO, Tags are + * generated properly for each sample. + * + * Authors: + * Sourab Gupta + * Sagar Arun Kamble + * + */ +#define _GNU_SOURCE +#include "xf86drm.h" +#include "i915_drm.h" +#include "igt_core.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + To be able to run this test in the continuous integration system, we need it to be autonomous. The following macro requires user interaction. Unfortunately that won't work. Please look at the other tests to create subtests and make sure we can run this in the CI. Thanks! +#define COLLECT_DATA { \ + printf("(%s) Collecting data. ", __func__); \ + printf("Press enter to continue...\n"); \ + getc(stdin); \ +} + It would be good to test stream configurations with different sizes. For example only Pid, or Tag & Pid or SourceInfo & ctx ID & Tag, etc... And verify that we get reports with appropriate sizes. +#define OA_SAMPLE_SIZE_MAX (8 +/* drm_i915_perf_record_header */ \ +8 +/* source info */ \ +8 +/* ctx ID */ \ +8 +/* Pid */ \ +8 +/* Tag */ \ +256) /* raw OA counter snapshot */ + +#define TS_SAMPLE_SIZE_MAX (8 +/* drm_i915_perf_record_header */ \ +8 +/* ctx ID */ \ +8 +/* Pid */ \ +8 +/* Tag */ \ +8) /* Timestamp */ \ + +#define TS_MMIO_SAMPLE_SIZE_MAX(8 + /* drm_i915_perf_record_header */ \ +8 + /* ctx ID */ \ +8 + /* Pid */ \ +8 + /* Tag */ \ +8 + /* Timestamp */ \ +4*I915_PERF_MMIO_NUM_MAX) /* MMIO reg */ + +#define OA_TS_MMIO_SAMPLE_SIZE_MAX (8 + /* drm_i915_perf_record_header */ \ + 8 + /* source info */ \ + 8 + /* ctx ID */ \ + 8 + /* Pid */ \ + 8 + /* Tag */ \ +
Re: [Intel-gfx] [PATCH v2 00/10] Improve robustness of the i915 perf tests
On 28/08/17 15:23, Arkadiusz Hiler wrote: On Mon, Aug 28, 2017 at 02:33:13PM +0100, Lionel Landwerlin wrote: On 28/08/17 08:21, Arkadiusz Hiler wrote: On Wed, Aug 23, 2017 at 10:43:08AM +0100, Lionel Landwerlin wrote: Hi all, Here is an updated patch series containing mostly cleanups. Cheers, Hi, Our CI hasn't pick this series up for a spin due to missing a "i-g-t" tag. Direct quote from CONTRIBUTING file: - Please use --subject-prefix="PATCH i-g-t" so that i-g-t patches are easily identified in the massive amount mails on intel-gfx. To ensure this is always done, autogen.sh will run: git config format.subjectprefix "PATCH i-g-t" on its first invocation. - If only there was a wrapper around git (much like chromium has git cl), it's too easy to get this stuff wrong when you want to send a v2 and you have to use --subject-prefix :/ Luckily there is, quote from man git-format-patch: -v , --reroll-count= Mark the series as the -th iteration of the topic. The output filenames have v prepended to them, and the subject prefix ("PATCH" by default, but configurable via the --subject-prefix option) has ` v` appended to it. E.g. --reroll-count=4 may produce v4-0001-add-makefile.patch file that has "Subject: [PATCH v4 1/20] Add makefile" in it. I wasn't aware of that until recently I found as an usage example posted on some internet forum and was really surprised it an option :-) Thanks! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v5] pm_rps: Changes in waitboost scenario
CI is observing sporadical failures in pm_rps subtests. There are a couple of reasons. One of them is the fact that on gen6, gen7 and gen7.5, max frequency (as in the HW limit) is not set to RP0, but the value obtaind from PCODE (which may be different from RP0). Thus the test is operating under wrong assumptions (SOFTMAX == RP0 == BOOST which is simply not the case). Let's compare current frequency with BOOST frequency rather than SOFTMAX to get the test behaviour under control. In boost_freq function I set MAX freq to midium freqency, which ensures that we for sure reach BOOST frequency. This could help with failures with boost frequency failing to drop down. GPU reset needs to be modified so we are not dependent on kernel's low priority retire worker. Reset method was replaced by igt_force_gpu_reset() and in reset testcase we make sure that we can recover from hang. v2: Commit message, simplified waiting for boost to finish, drop noisy whitespace cleanup. v3: Removed reading from i915_rps_boost_info debugfs because it not the same on every kernel. Removed function waiting for boost. Instead of that I made sure we will reach in boost by setting MAX freq to fmid. v4: Moved proposal with making test drm master to other patch v5: Used igt_force_gpu_reset() to reset GPU. Modified "reset" testcase. v6: Changes in commenting style Cc: Chris Wilson Cc: Jeff Mcgee Cc: Petri Latvala Cc: Jani Saarinen Cc: Radoslaw Szwichtenberg Reviewed-by: Radoslaw Szwichtenberg Signed-off-by: Katarzyna Dec --- tests/pm_rps.c | 63 +++--- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/tests/pm_rps.c b/tests/pm_rps.c index f0455e78..adc0c299 100644 --- a/tests/pm_rps.c +++ b/tests/pm_rps.c @@ -50,6 +50,7 @@ enum { RP0, RP1, RPn, + BOOST, NUMFREQ }; @@ -60,7 +61,7 @@ struct junk { const char *mode; FILE *filp; } stuff[] = { - { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { NULL, NULL, NULL } + { "cur", "r", NULL }, { "min", "rb+", NULL }, { "max", "rb+", NULL }, { "RP0", "r", NULL }, { "RP1", "r", NULL }, { "RPn", "r", NULL }, { "boost", "rb+", NULL }, { NULL, NULL, NULL } }; static int readval(FILE *filp) @@ -167,7 +168,7 @@ static void dump(const int *freqs) } enum load { - LOW, + LOW = 0, HIGH }; @@ -185,9 +186,10 @@ static struct load_helper { static void load_helper_signal_handler(int sig) { - if (sig == SIGUSR2) - lh.load = lh.load == LOW ? HIGH : LOW; - else + if (sig == SIGUSR2) { + lh.load = !lh.load; + igt_debug("Switching background load to %s\n", lh.load ? "high" : "low"); + } else lh.exit = true; } @@ -238,6 +240,7 @@ static void load_helper_run(enum load load) return; } + lh.exit = false; lh.load = load; igt_fork_helper(&lh.igt_proc) { @@ -263,6 +266,8 @@ static void load_helper_run(enum load load) if (intel_gen(lh.devid) >= 6) execbuf.flags = I915_EXEC_BLT; + igt_debug("Applying %s load...\n", lh.load ? "high" : "low"); + while (!lh.exit) { memset(&object, 0, sizeof(object)); object.handle = fences[val%3]; @@ -296,6 +301,8 @@ static void load_helper_run(enum load load) gem_close(drm_fd, fences[0]); gem_close(drm_fd, fences[1]); gem_close(drm_fd, fences[2]); + + igt_drop_caches_set(drm_fd, DROP_RETIRE); } } @@ -553,38 +560,43 @@ static void stabilize_check(int *out) igt_debug("Waited %d msec to stabilize cur\n", wait); } -static void reset_gpu(void) -{ - int fd = drm_open_driver(DRIVER_INTEL); - igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); - close(fd); -} - static void boost_freq(int fd, int *boost_freqs) { int64_t timeout = 1; - int ring = -1; igt_spin_t *load; + unsigned int engine; + int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; - load = igt_spin_batch_new(fd, ring, 0); + fmid = get_hw_rounded_freq(fmid); + /* set max freq to less then boost freq */ + writeval(stuff[MAX].filp, fmid); + /* put boost on the same engine as low load */ + engine = I915_EXEC_RENDER; + if (intel_gen(lh.devid) >= 6) + engine = I915_EXEC_BLT; + load = igt_spin_batch_new(fd, engine, 0); /* Waiting will grant us a boost to maximum */ gem_wait(fd, load->handle, &timeout); read_freqs(boost_freqs); dump(boost_freqs); + /* Avoid downlocking till boost request is pending */ + igt_spin_batch_end(load); + gem_sync(fd, load->handle); igt_spi
Re: [Intel-gfx] [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere
Quoting Jeff McGee (2017-08-28 21:18:44) > On Mon, Aug 28, 2017 at 08:44:48PM +0100, Chris Wilson wrote: > > Quoting jeff.mc...@intel.com (2017-08-28 20:25:30) > > > From: Jeff McGee > > > > > > If someone else is resetting the engine we should clear our own bit as > > > part of skipping that engine. Otherwise we will later believe that it > > > has not been reset successfully and then trigger full gpu reset. If the > > > other guy's reset actually fails, he will trigger the full gpu reset. > > > > The reason we did continue on to the global reset was to serialise > > i915_handle_error() with the other thread. Not a huge issue, but a > > reasonable property to keep -- and we definitely want a to explain why > > only one reset at a time is important. > > > > bool intel_engine_lock_reset() { > > if (!test_and_set_bit(I915_RESET_ENGINE + engine->id, > > &engine->i915->gpu_error.flags)) > > return true; > > > > intel_engine_wait_for_reset(engine); > The current code doesn't wait for the other thread to finish the reset, but > this would add that wait. Pardon? If we can't reset the engine, we go to the full reset which is serialised, both with individual engine resets and other globals. > Did you intend that as an additional change to > the current code? I don't think it is necessary. Each thread wants to > reset some subset of engines, so it seems the thread can safely exit as soon > as it knows each of those engines has been reset or is being reset as part > of another thread that got the lock first. If any of the threads fail to > reset an engine they "own", then full gpu reset is assured. It's unexpected for this function to return before the reset. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for pm_rps: Changes in waitboost scenario (rev7)
== Series Details == Series: pm_rps: Changes in waitboost scenario (rev7) URL : https://patchwork.freedesktop.org/series/28966/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3013 79cec1e1ffe2 drm-tip: 2017y-08m-29d-06h-07m-10s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:462s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:442s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:556s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:255s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:527s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:526s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:530s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:442s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:619s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:442s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:426s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:509s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:476s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:479s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:598s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:598s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:522s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:471s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:483s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:493s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:443s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:508s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:550s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:408s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_115/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2 2/3] drm/i915/pmu: serve global events and support perf stat
On Wed, Aug 23, 2017 at 11:38:43PM +, Rogozhkin, Dmitry V wrote: > On Wed, 2017-08-23 at 08:26 -0700, Dmitry Rogozhkin wrote: > > +static cpumask_t i915_pmu_cpumask = CPU_MASK_CPU0; > > Peter, this hardcoding of cpumask to use CPU0 works, but should I > implement something smarter or this will be sufficient? I see that > cstate.c you have pointed me to tries to track CPUs going online/offline > and migrates PMU context to another CPU if selected one went offline. > Should I follow this way? Yes.. x86 used to not allow hotplug of CPU0, but they 'fixed' that :/ And the perf core needs _a_ valid CPU to run things from, which leaves you having to track online/offline things. Now, I suppose its all fairly similar for a lot of uncore PMUs, so maybe you can pull some of this into a library and avoid the endless duplication between all (most?) uncore driveres. > If I should track CPUs going online/offline, then I have questions: > 1. How I should register tracking callbacks? I see that cstate.c > registers CPUHP_AP_PERF_X86_CSTATE_STARTING and > CPUHP_AP_PERF_X86_CSTATE_ONLINE, uncore.c registers > CPUHP_AP_PERF_X86_UNCORE_ONLINE. What I should use? I incline to UNCORE. Egads, what a mess :/ Clearly I didn't pay too much attention there. So ideally we'd not hate a state per PMU, and __cpuhp_setup_state_cpuslocked() has a .multi_instance argument that allows reuse of a state. So yes, please use the PERF_X86_UNCORE ones if possible. > 2. If I will register for, say UNCORE, then how double registrations > will be handled if both uncore.c and i915.c will register callbacks? Any > conflict here? Should work with .multi_instance I _think_, I've not had the pleasure of using the new and improved CPU hotplug infrastructure much. > 3. What I should pass as 2nd argument? Will "perf/x86/intel/i915:online" > be ok? Yeah, whatever I think.. something unique. Someone or something will eventually yell if its no good I suppose ;-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 04/10] drm/i915: Expose a PMU interface for perf queries
On Mon, Aug 28, 2017 at 10:43:17PM +, Rogozhkin, Dmitry V wrote: > Hi Peter, > > I have updated my fixes to Tvrtko's PMU, they are here: > https://patchwork.freedesktop.org/series/28842/, and I started to check > whether we will be able to cover all the use cases for this PMU which we > had in mind. Here I have some concerns and further questions. > > So, as soon as I registered PMU with the perf_invalid_context, i.e. as > an uncore PMU, I got the effect that metrics from our PMU are available > under root only. This happens since we fall to the following case > described in 'man perf_event_open': "A pid == -1 and cpu >= 0 setting is > per-CPU and measures all processes on the specified CPU. Per-CPU events > need the CAP_SYS_ADMIN capability or > a /proc/sys/kernel/perf_event_paranoid value of less than 1." > > This a trouble point for us... So, could you, please, clarify: > 1. How PMU API is positioned? It is for debug purposes only or it can be > used in the end-user release applications to monitor system activity and > make some decisions based on that? Perf is meant to also be usable for end-users, _however_ by default it will only allow users to profile their own userspace (tasks). Allowing unpriv users access to kernel data is a clear data leak (you instantly void KASLR for instance). And since uncore PMUs are not uniquely tied to individual user context, unpriv users have no access. > 2. How applications can access uncore PMU metrics from non-privileged > applications? Only by lowering sysctl.kernel.perf_event_paranoid. Since uuncore is shared among many users, its events can be used to construct side-channel attacks. So from a security pov this is not something that can otherwise be done. Imagine user A using the GPU to do crypto and user B reading the GPU events to infer state or similar things. Without privilege separation we cannot allow unpriv access to system wide resources. > 3. Is that a strong requirement to restrict uncore PMU metrics reporting > to privileged applications or this can be relaxed? Pretty strict, people tend to get fairly upset every time we leak stuff. In fact Debian and Android carry a perf_event_paranoid patch that default disables _everything_ :-( ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
On Tue, 2017-08-29 at 16:00 +0800, Zhi Wang wrote: > The private PAT management is to support PPAT entry manipulation. Two > APIs are introduced for dynamically managing PPAT entries: intel_ppat_get > and intel_ppat_put. > > intel_ppat_get will search for an existing PPAT entry which perfectly > matches the required PPAT value. If not, it will try to allocate or > return a partially matched PPAT entry if there is any available PPAT > indexes or not. > > intel_ppat_put will put back the PPAT entry which comes from > intel_ppat_get. If it's dynamically allocated, the reference count will > be decreased. If the reference count turns into zero, the PPAT index is > freed again. > > Besides, another two callbacks are introduced to support the private PAT > management framework. One is ppat->update(), which writes the PPAT > configurations in ppat->entries into HW. Another one is ppat->match, which > will return a score to show how two PPAT values match with each other. > > v5: > > - Add check and warnnings for those platforms which don't have PPAT. > > v3: > > - Introduce dirty bitmap for PPAT registers. (Chris) > - Change the name of the pointer "dev_priv" to "i915". (Chris) > - intel_ppat_{get, put} returns/takes a const intel_ppat_entry *. (Chris) > > v2: > > - API re-design. (Chris) > > Cc: Ben Widawsky > Cc: Rodrigo Vivi > Cc: Chris Wilson > Cc: Joonas Lahtinen > Signed-off-by: Zhi Wang > -static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv) > +static struct intel_ppat_entry *alloc_ppat_entry(struct intel_ppat *ppat, > + unsigned int index, > + u8 value) > { > + struct intel_ppat_entry *entry = &ppat->entries[index]; > + > + entry->ppat = ppat; > + entry->value = value; > + kref_init(&entry->ref_count); > + set_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > + > + return entry; > +} > + > +static void free_ppat_entry(struct intel_ppat_entry *entry) > +{ > + struct intel_ppat *ppat = entry->ppat; > + int index = entry - ppat->entries; > + > + entry->value = ppat->dummy_value; > + clear_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > +} Above functions should be __ prefixed as they do no checking if they override existing data. The suitable names might be __ppat_get and __ppat_put. > + > +/** > + * intel_ppat_get - get a usable PPAT entry > + * @i915: i915 device instance > + * @value: the PPAT value required by the caller > + * > + * The function tries to search if there is an existing PPAT entry which > + * matches with the required value. If perfectly matched, the existing PPAT > + * entry will be used. If only partially matched, it will try to check if > + * there is any available PPAT index. If yes, it will allocate a new PPAT > + * index for the required entry and update the HW. If not, the partially > + * matched entry will be used. > + */ > +const struct intel_ppat_entry *intel_ppat_get(struct drm_i915_private *i915, Maybe split the function type and name to avoid exceeding 80 chars on next line. > + u8 value) > +{ > + struct intel_ppat *ppat = &i915->ppat; > + struct intel_ppat_entry *entry; > + int i, used; > + unsigned int score, best_score; > + > + if (WARN_ON(!ppat->max_entries)) > + return ERR_PTR(-ENODEV); No need for extra check like this, this will just lead to ENOSPC. > + > + score = best_score = 0; > + used = 0; This variable behaves more like scanned. > + > + /* First, find a suitable value from available entries */ The next two lines repeat this information, no need to document "what". > + for_each_set_bit(i, ppat->used, ppat->max_entries) { score could be declared in this scope. > + score = ppat->match(ppat->entries[i].value, value); > + /* Perfect match */ This comment is literally repeating what code says. > + if (score == INTEL_PPAT_PERFECT_MATCH) { > + entry = &ppat->entries[i]; > + kref_get(&entry->ref_count); > + return entry; > + } > + > + if (score > best_score) { > + entry = &ppat->entries[i]; Above could be simplified: if (score == INTEL_PPAT_PERFECT_MATCH) { kref_get(&entry->ref); return entry; } > + best_score = score; > + } > + used++; > + } > + > + /* No matched entry and we can't allocate a new entry. */ DRM_ERROR replicates this comment's information. > + if (!best_score && used == ppat->max_entries) { > + DRM_ERROR("Fail to get PPAT entry\n"); DRM_DEBUG_DRIVER at most. > + return ERR_PTR(-ENOSPC); > + } > + > + /* > +
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [RFCv5,1/2] drm/i915: Factor out setup_private_pat()
== Series Details == Series: series starting with [RFCv5,1/2] drm/i915: Factor out setup_private_pat() URL : https://patchwork.freedesktop.org/series/29456/ State : failure == Summary == Test kms_flip: Subgroup plain-flip-fb-recreate: pass -> FAIL (shard-hsw) shard-hswtotal:2230 pass:1231 dwarn:0 dfail:0 fail:18 skip:981 time:9536s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5516/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: dspaddr_offset doesn't need to be more than local variable
Move u32 dspaddr_offset from struct intel_crtc member into local variable in i9xx_update_primary_plane() Signed-off-by: Juha-Pekka Heikkila --- drivers/gpu/drm/i915/intel_display.c | 11 ++- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7cd392f..f922e2f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3287,13 +3287,14 @@ static void i9xx_update_primary_plane(struct intel_plane *primary, int x = plane_state->main.x; int y = plane_state->main.y; unsigned long irqflags; + u32 dspaddr_offset; linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0); if (INTEL_GEN(dev_priv) >= 4) - crtc->dspaddr_offset = plane_state->main.offset; + dspaddr_offset = plane_state->main.offset; else - crtc->dspaddr_offset = linear_offset; + dspaddr_offset = linear_offset; crtc->adjusted_x = x; crtc->adjusted_y = y; @@ -3322,18 +3323,18 @@ static void i9xx_update_primary_plane(struct intel_plane *primary, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { I915_WRITE_FW(DSPSURF(plane), intel_plane_ggtt_offset(plane_state) + - crtc->dspaddr_offset); + dspaddr_offset); I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x); } else if (INTEL_GEN(dev_priv) >= 4) { I915_WRITE_FW(DSPSURF(plane), intel_plane_ggtt_offset(plane_state) + - crtc->dspaddr_offset); + dspaddr_offset); I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x); I915_WRITE_FW(DSPLINOFF(plane), linear_offset); } else { I915_WRITE_FW(DSPADDR(plane), intel_plane_ggtt_offset(plane_state) + - crtc->dspaddr_offset); + dspaddr_offset); } POSTING_READ_FW(reg); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 17649f1..0d0abed1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -805,7 +805,6 @@ struct intel_crtc { /* Display surface base address adjustement for pageflips. Note that on * gen4+ this only adjusts up to a tile, offsets within a tile are * handled in the hw itself (with the TILEOFF register). */ - u32 dspaddr_offset; int adjusted_x; int adjusted_y; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Unify skylake plane disable
Don't handle skylake primary plane separately as it is similar plane as the others. Signed-off-by: Juha-Pekka Heikkila --- drivers/gpu/drm/i915/intel_display.c | 21 ++--- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_sprite.c | 2 +- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9082a2c..93a37e6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3534,23 +3534,6 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, return plane_ctl; } -static void skylake_disable_primary_plane(struct intel_plane *primary, - struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = to_i915(primary->base.dev); - enum plane_id plane_id = primary->id; - enum pipe pipe = primary->pipe; - unsigned long irqflags; - - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - - I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0); - I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0); - POSTING_READ_FW(PLANE_SURF(pipe, plane_id)); - - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); -} - static int __intel_display_resume(struct drm_device *dev, struct drm_atomic_state *state, @@ -13189,7 +13172,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) modifiers = skl_format_modifiers_ccs; primary->update_plane = skl_update_plane; - primary->disable_plane = skylake_disable_primary_plane; + primary->disable_plane = skl_disable_plane; } else if (INTEL_GEN(dev_priv) >= 9) { intel_primary_formats = skl_primary_formats; num_formats = ARRAY_SIZE(skl_primary_formats); @@ -13199,7 +13182,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) modifiers = skl_format_modifiers_noccs; primary->update_plane = skl_update_plane; - primary->disable_plane = skylake_disable_primary_plane; + primary->disable_plane = skl_disable_plane; } else if (INTEL_GEN(dev_priv) >= 4) { intel_primary_formats = i965_primary_formats; num_formats = ARRAY_SIZE(i965_primary_formats); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1efd612..18752b7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1890,6 +1890,7 @@ void intel_pipe_update_end(struct intel_crtc *crtc); void skl_update_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); +void skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc); /* intel_tv.c */ void intel_tv_init(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index ef16519..bc6bae6 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -306,7 +306,7 @@ skl_update_plane(struct intel_plane *plane, spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } -static void +void skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Unify skylake plane update
Don't handle skylake primary plane separately as it is similar plane as the others. Signed-off-by: Juha-Pekka Heikkila --- drivers/gpu/drm/i915/intel_display.c | 81 +--- drivers/gpu/drm/i915/intel_drv.h | 3 ++ drivers/gpu/drm/i915/intel_sprite.c | 2 +- 3 files changed, 6 insertions(+), 80 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f922e2f..9082a2c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3534,83 +3534,6 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state, return plane_ctl; } -static void skylake_update_primary_plane(struct intel_plane *plane, -const struct intel_crtc_state *crtc_state, -const struct intel_plane_state *plane_state) -{ - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - const struct drm_framebuffer *fb = plane_state->base.fb; - enum plane_id plane_id = plane->id; - enum pipe pipe = plane->pipe; - u32 plane_ctl = plane_state->ctl; - unsigned int rotation = plane_state->base.rotation; - u32 stride = skl_plane_stride(fb, 0, rotation); - u32 aux_stride = skl_plane_stride(fb, 1, rotation); - u32 surf_addr = plane_state->main.offset; - int scaler_id = plane_state->scaler_id; - int src_x = plane_state->main.x; - int src_y = plane_state->main.y; - int src_w = drm_rect_width(&plane_state->base.src) >> 16; - int src_h = drm_rect_height(&plane_state->base.src) >> 16; - int dst_x = plane_state->base.dst.x1; - int dst_y = plane_state->base.dst.y1; - int dst_w = drm_rect_width(&plane_state->base.dst); - int dst_h = drm_rect_height(&plane_state->base.dst); - unsigned long irqflags; - - /* Sizes are 0 based */ - src_w--; - src_h--; - dst_w--; - dst_h--; - - crtc->dspaddr_offset = surf_addr; - - crtc->adjusted_x = src_x; - crtc->adjusted_y = src_y; - - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) { - I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), - PLANE_COLOR_PIPE_GAMMA_ENABLE | - PLANE_COLOR_PIPE_CSC_ENABLE | - PLANE_COLOR_PLANE_GAMMA_DISABLE); - } - - I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); - I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x); - I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); - I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); - I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), - (plane_state->aux.offset - surf_addr) | aux_stride); - I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), - (plane_state->aux.y << 16) | plane_state->aux.x); - - if (scaler_id >= 0) { - uint32_t ps_ctrl = 0; - - WARN_ON(!dst_w || !dst_h); - ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane_id) | - crtc_state->scaler_state.scalers[scaler_id].mode; - I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl); - I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); - I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x << 16) | dst_y); - I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h); - I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); - } else { - I915_WRITE_FW(PLANE_POS(pipe, plane_id), (dst_y << 16) | dst_x); - } - - I915_WRITE_FW(PLANE_SURF(pipe, plane_id), - intel_plane_ggtt_offset(plane_state) + surf_addr); - - POSTING_READ_FW(PLANE_SURF(pipe, plane_id)); - - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); -} - static void skylake_disable_primary_plane(struct intel_plane *primary, struct intel_crtc *crtc) { @@ -13265,7 +13188,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) num_formats = ARRAY_SIZE(skl_primary_formats); modifiers = skl_format_modifiers_ccs; - primary->update_plane = skylake_update_primary_plane; + primary->update_plane = skl_update_plane; primary->disable_plane = skylake_disable_primary_plane; } else if (INTEL_GEN(dev_priv) >= 9) { intel_primary_formats = skl_primary_formats; @@ -13275,7 +13198,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) else modifiers = skl_format_modifiers_noccs; - primary->updat
[Intel-gfx] [PATCH 0/3] drm/i915: Skylake plane update/disable unifications [v2]
Fixed missed references which were brough on rebase. /Juha-Pekka Juha-Pekka Heikkila (3): drm/i915: dspaddr_offset doesn't need to be more than local variable drm/i915: Unify skylake plane update drm/i915: Unify skylake plane disable drivers/gpu/drm/i915/intel_display.c | 113 --- drivers/gpu/drm/i915/intel_drv.h | 5 +- drivers/gpu/drm/i915/intel_sprite.c | 4 +- 3 files changed, 16 insertions(+), 106 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove excess indent in intel_finish_reset() caught by sparse
Quoting Daniel Vetter (2017-08-28 23:12:57) > On Mon, Aug 28, 2017 at 11:46:04AM +0100, Chris Wilson wrote: > > CHECK drivers/gpu/drm/i915/intel_display.c > > drivers/gpu/drm/i915/intel_display.c:3753 intel_finish_reset() warn: > > inconsistent indenting > > > > Signed-off-by: Chris Wilson > > Ooops, I think that was me :-( > > Reviewed-by: Daniel Vetter And the trivial cleanup pushed. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT
Quoting Patchwork (2017-08-28 21:15:28) > == Series Details == > > Series: drm/i915: Always wake the device to flush the GTT > URL : https://patchwork.freedesktop.org/series/29436/ > State : success > > == Summary == > > Test kms_flip: > Subgroup plain-flip-ts-check: > fail -> PASS (shard-hsw) > Test kms_setmode: > Subgroup basic: > pass -> FAIL (shard-hsw) fdo#99912 > > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > shard-hswtotal:2230 pass:1230 dwarn:0 dfail:0 fail:18 skip:982 > time:9646s > > == Logs == > > For more details see: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards.html Hard to tell if the flips are fixed since this only shows the changes, but I need to see the full list. Anyone know how to get the actual results? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Recreate vmapping even when the object is pinned
Quoting Joonas Lahtinen (2017-08-28 14:44:36) > On Mon, 2017-08-28 at 11:46 +0100, Chris Wilson wrote: > > Sometimes we know we are the only user of the bo, but since we take a > > protective pin_pages early on, an attempt to change the vmap on the > > object is denied because it is busy. i915_gem_object_pin_map() cannot > > tell from our single pin_count if the operation is safe. Instead we must > > pass that information down from the caller in the manner of > > I915_MAP_OVERRIDE. > > > > This issue has existed from the introduction of the mapping, but was > > never noticed as the only place where this conflict might happen is for > > cached kernel buffers (such as allocated by i915_gem_batch_pool_get()). > > Until recently there was only a single user (the cmdparser) so no > > conflicts ever occurred. However, we now use it to allocate batches for > > different operations (using MAP_WC on !llc for writes) in addition to the > > existing shadow batch (using MAP_WB for reads). > > > > We could either keep both mappings cached, or use a different write > > mechanism if we detect a MAP_WB already exists (i.e. clflush > > afterwards), but as we haven't seen this issue in the wild (it requires > > hitting the GPU reloc path in addition to the cmdparser) for simplicity > > just allow the mappings to be recreated. > > > > v2: Include the i915_MAP_OVERRIDE bit in the enum so the compiler knows > > about all the valid values. > > > > Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing") > > Testcase: igt/gem_lut_handle # byt, completely by accident > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Reviewed-by: Joonas Lahtinen And pushed. Only afterwards when looking for the link did I notice I forgot to mention the bugzilla. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Quoting Zhi Wang (2017-08-29 09:00:51) > +static void cnl_setup_private_ppat(struct intel_ppat *ppat) > +{ > + ppat->max_entries = 8; > + ppat->update = cnl_private_pat_update; > + ppat->match = bdw_private_pat_match; > + ppat->dummy_value = GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3); > + > /* XXX: spec is unclear if this is still needed for CNL+ */ > - if (!USES_PPGTT(dev_priv)) { > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_UC); > + if (!USES_PPGTT(ppat->i915)) { > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > return; > } > > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_WB | GEN8_PPAT_LLC); > - I915_WRITE(GEN10_PAT_INDEX(1), GEN8_PPAT_WC | GEN8_PPAT_LLCELLC); > - I915_WRITE(GEN10_PAT_INDEX(2), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > - I915_WRITE(GEN10_PAT_INDEX(3), GEN8_PPAT_UC); > - I915_WRITE(GEN10_PAT_INDEX(4), GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)); > - I915_WRITE(GEN10_PAT_INDEX(5), GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(1)); > - I915_WRITE(GEN10_PAT_INDEX(6), GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(2)); > - I915_WRITE(GEN10_PAT_INDEX(7), GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3)); > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)); > } > > /* The GGTT and PPGTT need a private PPAT setup in order to handle > cacheability > * bits. When using advanced contexts each context stores its own PAT, but > * writing this data shouldn't be harmful even in those cases. */ > -static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv) > +static void bdw_setup_private_ppat(struct intel_ppat *ppat) > { > - u64 pat; > - > - pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for normal > objects, no eLLC */ > - GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for > something pointing to ptes? */ > - GEN8_PPAT(2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC) | /* for scanout > with eLLC */ > - GEN8_PPAT(3, GEN8_PPAT_UC) | /* Uncached > objects, mostly for scanout */ > - GEN8_PPAT(4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(0)) | > - GEN8_PPAT(5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(1)) | > - GEN8_PPAT(6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(2)) | > - GEN8_PPAT(7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(3)); > + ppat->max_entries = 8; > + ppat->update = bdw_private_pat_update; > + ppat->match = bdw_private_pat_match; > + ppat->dummy_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(3); > > - if (!USES_PPGTT(dev_priv)) > + if (!USES_PPGTT(dev_priv)) { > /* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry, > * so RTL will always use the value corresponding to > * pat_sel = 000". > @@ -2864,17 +3038,22 @@ static void bdw_setup_private_ppat(struct > drm_i915_private *dev_priv) > * So we can still hold onto all our assumptions wrt cpu > * clflushing on LLC machines. > */ > - pat = GEN8_PPAT(0, GEN8_PPAT_UC); > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > + return; > + } > > - /* XXX: spec defines this as 2 distinct registers. It's unclear if a > 64b > -* write would work. */ > - I915_WRITE(GEN8_PRIVATE_PAT_LO, pat); > - I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32); > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* for > normal objects, no eLLC */ > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for > scanout with eLLC */ > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* > Uncached objects, mostly for scanout */ > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(0)); > } > > -static void chv_setup_private_ppat(struct drm_i915_private *dev_priv) > +static void chv_setup_private_ppat(struct intel_ppat *ppat) > { > - u64 pat; > + ppat->max_entries = 8; > + ppat->update = bdw_private_pat_update; > + ppat->match = chv_private_pat_match; > + ppat->dummy_value = CHV_PPAT_SNOOP; > > /* > * Map WB on BDW to snooped on CHV. > @@ -2894,17 +3073,11 @@ static void chv_setup_private_ppat(struct > drm_i915_private *dev_priv) > * Which means we must set the snoop bit in PAT entry 0 > * in order to keep the global status page working. > */ > - pat = GEN8_PPAT(0, CHV_PPAT_SNOOP) | > - GEN8_PPAT(1, 0) | > - GEN8_PPAT(2, 0) | > - GEN8_PPAT(3, 0) | > - GEN8_PPAT
Re: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT
HI, > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of > Chris Wilson > Sent: Tuesday, August 29, 2017 12:59 PM > To: intel-gfx@lists.freedesktop.org; Patchwork > > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Always wake the > device to flush the GTT > > Quoting Patchwork (2017-08-28 21:15:28) > > == Series Details == > > > > Series: drm/i915: Always wake the device to flush the GTT > > URL : https://patchwork.freedesktop.org/series/29436/ > > State : success > > > > == Summary == > > > > Test kms_flip: > > Subgroup plain-flip-ts-check: > > fail -> PASS (shard-hsw) > > Test kms_setmode: > > Subgroup basic: > > pass -> FAIL (shard-hsw) fdo#99912 > > > > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > > > shard-hswtotal:2230 pass:1230 dwarn:0 dfail:0 fail:18 skip:982 > time:9646s > > > > == Logs == > > > > For more details see: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards.html You mean https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards-all.html > > Hard to tell if the flips are fixed since this only shows the changes, but I > need > to see the full list. Anyone know how to get the actual results? > -Chris Br, Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/kms_atomic: subtest atomic_invalid_params requires CRTC
Check for valid crtc is missing in igt@kms_atomic@atomic_invalid_params. This leads to segfault on machines where the subtest should be skipped. Signed-off-by: Marta Lofstedt --- tests/kms_atomic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c index d77a526f..042a7c26 100644 --- a/tests/kms_atomic.c +++ b/tests/kms_atomic.c @@ -1596,6 +1596,7 @@ igt_main struct kms_atomic_connector_state *conn = find_connector(scratch, crtc); + igt_require(crtc); igt_require(plane); igt_require(conn); atomic_invalid_params(crtc, plane, conn); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: warning for pm_rps: Changes in waitboost scenario (rev7)
== Series Details == Series: pm_rps: Changes in waitboost scenario (rev7) URL : https://patchwork.freedesktop.org/series/28966/ State : warning == Summary == Test pm_rps: Subgroup reset: fail -> PASS (shard-hsw) fdo#102250 +1 Test perf: Subgroup blocking: pass -> FAIL (shard-hsw) fdo#102252 +1 Test kms_busy: Subgroup extended-modeset-hang-newfb-with-reset-render-B: pass -> DMESG-WARN (shard-hsw) fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250 fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 shard-hswtotal:2230 pass:1231 dwarn:1 dfail:0 fail:17 skip:981 time:9636s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_115/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Always wake the device to flush the GTT
Since we hold the device wakeref when writing through the GTT (otherwise the writes would fail), we presumed that before the device sleeps those writes would naturally be flushed and that we wouldn't need our mmio read trick. However, that presumption seems false and a sleepy bxt seems to require us to always manually flush the GTT writes prior to direct access. Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 43834dee4e8d..890fe2802973 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -695,12 +695,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains) switch (obj->base.write_domain) { case I915_GEM_DOMAIN_GTT: if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) { - if (intel_runtime_pm_get_if_in_use(dev_priv)) { - spin_lock_irq(&dev_priv->uncore.lock); - POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base)); - spin_unlock_irq(&dev_priv->uncore.lock); - intel_runtime_pm_put(dev_priv); - } + intel_runtime_pm_get(dev_priv); + spin_lock_irq(&dev_priv->uncore.lock); + POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base)); + spin_unlock_irq(&dev_priv->uncore.lock); + intel_runtime_pm_put(dev_priv); } intel_fb_obj_flush(obj, -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't use GPU relocations prior to cmdparser stalls
Quoting Joonas Lahtinen (2017-08-28 10:26:40) > On Sat, 2017-08-26 at 14:56 +0100, Chris Wilson wrote: > > If we are using the cmdparser, we will have to copy the batch and so > > stall for the relocations. Rather than prolong that stall by adding more > > relocation requests, just use CPU relocations and do the stall upfront. > > > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Reviewed-by: Joonas Lahtinen And applied. Thanks for the review, -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Skylake plane update/disable unifications [v2]
== Series Details == Series: drm/i915: Skylake plane update/disable unifications [v2] URL : https://patchwork.freedesktop.org/series/29462/ State : warning == Summary == Series 29462v1 drm/i915: Skylake plane update/disable unifications [v2] https://patchwork.freedesktop.org/api/1.0/series/29462/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100215 Subgroup basic-flip-before-cursor-atomic: pass -> DMESG-WARN (fi-glk-2a) Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:468s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:439s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:560s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:252s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:516s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:517s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:515s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:443s fi-glk-2atotal:279 pass:259 dwarn:1 dfail:0 fail:0 skip:19 time:619s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:443s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:424s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:508s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:475s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:593s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:598s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:525s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:476s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:480s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:489s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:446s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:507s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:542s fi-snb-2600 total:279 pass:248 dwarn:0 dfail:0 fail:2 skip:29 time:404s 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest a62bb67be4e0 drm/i915: Unify skylake plane disable dd43b5dbfd7d drm/i915: Unify skylake plane update 369291625771 drm/i915: dspaddr_offset doesn't need to be more than local variable == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5517/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Quoting Chris Wilson (2017-08-29 11:05:21) > Quoting Zhi Wang (2017-08-29 09:00:51) > > +static void cnl_setup_private_ppat(struct intel_ppat *ppat) > > +{ > > + ppat->max_entries = 8; > > + ppat->update = cnl_private_pat_update; > > + ppat->match = bdw_private_pat_match; > > + ppat->dummy_value = GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3); > > + > > /* XXX: spec is unclear if this is still needed for CNL+ */ > > - if (!USES_PPGTT(dev_priv)) { > > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_UC); > > + if (!USES_PPGTT(ppat->i915)) { > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > return; > > } > > > > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_WB | GEN8_PPAT_LLC); > > - I915_WRITE(GEN10_PAT_INDEX(1), GEN8_PPAT_WC | GEN8_PPAT_LLCELLC); > > - I915_WRITE(GEN10_PAT_INDEX(2), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > > - I915_WRITE(GEN10_PAT_INDEX(3), GEN8_PPAT_UC); > > - I915_WRITE(GEN10_PAT_INDEX(4), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(0)); > > - I915_WRITE(GEN10_PAT_INDEX(5), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(1)); > > - I915_WRITE(GEN10_PAT_INDEX(6), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(2)); > > - I915_WRITE(GEN10_PAT_INDEX(7), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(3)); > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)); > > } > > > > /* The GGTT and PPGTT need a private PPAT setup in order to handle > > cacheability > > * bits. When using advanced contexts each context stores its own PAT, but > > * writing this data shouldn't be harmful even in those cases. */ > > -static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv) > > +static void bdw_setup_private_ppat(struct intel_ppat *ppat) > > { > > - u64 pat; > > - > > - pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for > > normal objects, no eLLC */ > > - GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for > > something pointing to ptes? */ > > - GEN8_PPAT(2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC) | /* for > > scanout with eLLC */ > > - GEN8_PPAT(3, GEN8_PPAT_UC) | /* Uncached > > objects, mostly for scanout */ > > - GEN8_PPAT(4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(0)) | > > - GEN8_PPAT(5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(1)) | > > - GEN8_PPAT(6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(2)) | > > - GEN8_PPAT(7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(3)); > > + ppat->max_entries = 8; > > + ppat->update = bdw_private_pat_update; > > + ppat->match = bdw_private_pat_match; > > + ppat->dummy_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(3); > > > > - if (!USES_PPGTT(dev_priv)) > > + if (!USES_PPGTT(dev_priv)) { > > /* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry, > > * so RTL will always use the value corresponding to > > * pat_sel = 000". > > @@ -2864,17 +3038,22 @@ static void bdw_setup_private_ppat(struct > > drm_i915_private *dev_priv) > > * So we can still hold onto all our assumptions wrt cpu > > * clflushing on LLC machines. > > */ > > - pat = GEN8_PPAT(0, GEN8_PPAT_UC); > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > + return; > > + } > > > > - /* XXX: spec defines this as 2 distinct registers. It's unclear if > > a 64b > > -* write would work. */ > > - I915_WRITE(GEN8_PRIVATE_PAT_LO, pat); > > - I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32); > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* for > > normal objects, no eLLC */ > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* for > > scanout with eLLC */ > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* > > Uncached objects, mostly for scanout */ > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(0)); > > } > > > > -static void chv_setup_private_ppat(struct drm_i915_private *dev_priv) > > +static void chv_setup_private_ppat(struct intel_ppat *ppat) > > { > > - u64 pat; > > + ppat->max_entries = 8; > > + ppat->update = bdw_private_pat_update; > > + ppat->match = chv_private_pat_match; > > + ppat->dummy_value = CHV_PPAT_SNOOP; > > > > /* > > * Map WB on BDW to snooped on CHV. > > @@ -2894,17 +3073,11 @@ static void chv_setup_private_ppat(struct > > drm_i915_private *dev_priv) > > * Which means we must set the sn
Re: [Intel-gfx] [PATCH i-g-t] tests/kms_atomic: subtest atomic_invalid_params requires CRTC
In my view, this is a valid check. Reviewed-by: Mika Kahola On Tue, 2017-08-29 at 13:32 +0300, Marta Lofstedt wrote: > Check for valid crtc is missing in igt@kms_atomic@atomic_invalid_para > ms. > This leads to segfault on machines where the subtest should be > skipped. > > Signed-off-by: Marta Lofstedt > --- > tests/kms_atomic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c > index d77a526f..042a7c26 100644 > --- a/tests/kms_atomic.c > +++ b/tests/kms_atomic.c > @@ -1596,6 +1596,7 @@ igt_main > struct kms_atomic_connector_state *conn = > find_connector(scratch, crtc); > > + igt_require(crtc); > igt_require(plane); > igt_require(conn); > atomic_invalid_params(crtc, plane, conn); -- Mika Kahola - Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Always wake the device to flush the GTT (rev2)
== Series Details == Series: drm/i915: Always wake the device to flush the GTT (rev2) URL : https://patchwork.freedesktop.org/series/29436/ State : warning == Summary == Series 29436v2 drm/i915: Always wake the device to flush the GTT https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/2/mbox/ Test kms_frontbuffer_tracking: Subgroup basic: pass -> DMESG-WARN (fi-bdw-5557u) fi-bdw-5557u total:279 pass:267 dwarn:1 dfail:0 fail:0 skip:11 time:461s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:445s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:360s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:547s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:518s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:521s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:511s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:439s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:614s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:444s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:425s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:508s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:476s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:595s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:598s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:529s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:471s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:481s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:487s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:449s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:486s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:548s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:402s 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest 2a4825b281be drm/i915: Always wake the device to flush the GTT == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5518/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for tests/kms_atomic: subtest atomic_invalid_params requires CRTC
== Series Details == Series: tests/kms_atomic: subtest atomic_invalid_params requires CRTC URL : https://patchwork.freedesktop.org/series/29464/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3014 627598734e5e drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100215 +1 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:459s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:444s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:361s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:557s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:254s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:523s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:525s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:516s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:443s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:612s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:448s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:423s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:498s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:480s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:600s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:600s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:525s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:477s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:481s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:488s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:446s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:489s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:552s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:404s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_116/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Hi Chris: There is mapping between i915 cache level -> PPAT index. Currently it's: static gen8_pte_t gen8_pte_encode(dma_addr_t addr, enum i915_cache_level level) { ... switch (level) { case I915_CACHE_NONE: pte |= PPAT_UNCACHED_INDEX; break; case I915_CACHE_WT: pte |= PPAT_DISPLAY_ELLC_INDEX; break; default: pte |= PPAT_CACHED_INDEX; break; } ... According to bspec, the PPAT index filled in the page table is calculated as: PPAT index = 4 * PAT + 2 * PCD + PWT In the i915_gem_gtt.c #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD) // PPAT INDEX = 1 + 2 * 1 = 3 #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */ // PPAT INDEX = 0 #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */ // PPAT INDEX = 4 * 1 = 4 #define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */ // PPAT INDEX = 2 * 1 = 2 Actually the PPAT index used by i915 are: 0 2 3 4, which has already been reserved in the RFC patches. The reason I want to reserve that PPAT entries for host is to keep the mapping between I915_CACHE_* and PPAT_*_INDEX unchanged. Thanks, Zhi. -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, August 29, 2017 1:40 PM To: Wang, Zhi A ; intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org Cc: joonas.lahti...@linux.intel.com; zhen...@linux.intel.com; Wang, Zhi A ; Widawsky, Benjamin ; Vivi, Rodrigo Subject: Re: [RFCv5 2/2] drm/i915: Introduce private PAT management Quoting Chris Wilson (2017-08-29 11:05:21) > Quoting Zhi Wang (2017-08-29 09:00:51) > > +static void cnl_setup_private_ppat(struct intel_ppat *ppat) { > > + ppat->max_entries = 8; > > + ppat->update = cnl_private_pat_update; > > + ppat->match = bdw_private_pat_match; > > + ppat->dummy_value = GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(3); > > + > > /* XXX: spec is unclear if this is still needed for CNL+ */ > > - if (!USES_PPGTT(dev_priv)) { > > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_UC); > > + if (!USES_PPGTT(ppat->i915)) { > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > return; > > } > > > > - I915_WRITE(GEN10_PAT_INDEX(0), GEN8_PPAT_WB | GEN8_PPAT_LLC); > > - I915_WRITE(GEN10_PAT_INDEX(1), GEN8_PPAT_WC | GEN8_PPAT_LLCELLC); > > - I915_WRITE(GEN10_PAT_INDEX(2), GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > > - I915_WRITE(GEN10_PAT_INDEX(3), GEN8_PPAT_UC); > > - I915_WRITE(GEN10_PAT_INDEX(4), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(0)); > > - I915_WRITE(GEN10_PAT_INDEX(5), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(1)); > > - I915_WRITE(GEN10_PAT_INDEX(6), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(2)); > > - I915_WRITE(GEN10_PAT_INDEX(7), GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(3)); > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_LLCELLC | > > + GEN8_PPAT_AGE(0)); > > } > > > > /* The GGTT and PPGTT need a private PPAT setup in order to handle > > cacheability > > * bits. When using advanced contexts each context stores its own PAT, but > > * writing this data shouldn't be harmful even in those cases. */ > > -static void bdw_setup_private_ppat(struct drm_i915_private > > *dev_priv) > > +static void bdw_setup_private_ppat(struct intel_ppat *ppat) > > { > > - u64 pat; > > - > > - pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for > > normal objects, no eLLC */ > > - GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for > > something pointing to ptes? */ > > - GEN8_PPAT(2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC) | /* for > > scanout with eLLC */ > > - GEN8_PPAT(3, GEN8_PPAT_UC) | /* Uncached > > objects, mostly for scanout */ > > - GEN8_PPAT(4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(0)) | > > - GEN8_PPAT(5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(1)) | > > - GEN8_PPAT(6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(2)) | > > - GEN8_PPAT(7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > GEN8_PPAT_AGE(3)); > > + ppat->max_entries = 8; > > + ppat->update = bdw_private_pat_update; > > + ppat->match = bdw_private_pat_match; > > + ppat->dummy_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > + GEN8_PPAT_AGE(3); > > > > - if (!USES_PPGTT(dev_priv)) > > + if (!USES_PPGTT(dev_priv)) { > > /* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry, > > * so RTL will always use the value corresponding
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Thanks Joonas! :) -Original Message- From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com] Sent: Tuesday, August 29, 2017 12:37 PM To: Wang, Zhi A ; intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org Cc: ch...@chris-wilson.co.uk; zhen...@linux.intel.com; Widawsky, Benjamin ; Vivi, Rodrigo Subject: Re: [RFCv5 2/2] drm/i915: Introduce private PAT management On Tue, 2017-08-29 at 16:00 +0800, Zhi Wang wrote: > The private PAT management is to support PPAT entry manipulation. Two > APIs are introduced for dynamically managing PPAT entries: > intel_ppat_get and intel_ppat_put. > > intel_ppat_get will search for an existing PPAT entry which perfectly > matches the required PPAT value. If not, it will try to allocate or > return a partially matched PPAT entry if there is any available PPAT > indexes or not. > > intel_ppat_put will put back the PPAT entry which comes from > intel_ppat_get. If it's dynamically allocated, the reference count > will be decreased. If the reference count turns into zero, the PPAT > index is freed again. > > Besides, another two callbacks are introduced to support the private > PAT management framework. One is ppat->update(), which writes the PPAT > configurations in ppat->entries into HW. Another one is ppat->match, > which will return a score to show how two PPAT values match with each other. > > v5: > > - Add check and warnnings for those platforms which don't have PPAT. > > v3: > > - Introduce dirty bitmap for PPAT registers. (Chris) > - Change the name of the pointer "dev_priv" to "i915". (Chris) > - intel_ppat_{get, put} returns/takes a const intel_ppat_entry *. > (Chris) > > v2: > > - API re-design. (Chris) > > Cc: Ben Widawsky > Cc: Rodrigo Vivi > Cc: Chris Wilson > Cc: Joonas Lahtinen > Signed-off-by: Zhi Wang > -static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv) > +static struct intel_ppat_entry *alloc_ppat_entry(struct intel_ppat *ppat, > + unsigned int index, > + u8 value) > { > + struct intel_ppat_entry *entry = &ppat->entries[index]; > + > + entry->ppat = ppat; > + entry->value = value; > + kref_init(&entry->ref_count); > + set_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > + > + return entry; > +} > + > +static void free_ppat_entry(struct intel_ppat_entry *entry) { > + struct intel_ppat *ppat = entry->ppat; > + int index = entry - ppat->entries; > + > + entry->value = ppat->dummy_value; > + clear_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > +} Above functions should be __ prefixed as they do no checking if they override existing data. The suitable names might be __ppat_get and __ppat_put. > + > +/** > + * intel_ppat_get - get a usable PPAT entry > + * @i915: i915 device instance > + * @value: the PPAT value required by the caller > + * > + * The function tries to search if there is an existing PPAT entry > +which > + * matches with the required value. If perfectly matched, the > +existing PPAT > + * entry will be used. If only partially matched, it will try to > +check if > + * there is any available PPAT index. If yes, it will allocate a new > +PPAT > + * index for the required entry and update the HW. If not, the > +partially > + * matched entry will be used. > + */ > +const struct intel_ppat_entry *intel_ppat_get(struct drm_i915_private > +*i915, Maybe split the function type and name to avoid exceeding 80 chars on next line. > + u8 value) > +{ > + struct intel_ppat *ppat = &i915->ppat; > + struct intel_ppat_entry *entry; > + int i, used; > + unsigned int score, best_score; > + > + if (WARN_ON(!ppat->max_entries)) > + return ERR_PTR(-ENODEV); No need for extra check like this, this will just lead to ENOSPC. > + > + score = best_score = 0; > + used = 0; This variable behaves more like scanned. > + > + /* First, find a suitable value from available entries */ The next two lines repeat this information, no need to document "what". > + for_each_set_bit(i, ppat->used, ppat->max_entries) { score could be declared in this scope. > + score = ppat->match(ppat->entries[i].value, value); > + /* Perfect match */ This comment is literally repeating what code says. > + if (score == INTEL_PPAT_PERFECT_MATCH) { > + entry = &ppat->entries[i]; > + kref_get(&entry->ref_count); > + return entry; > + } > + > + if (score > best_score) { > + entry = &ppat->entries[i]; Above could be simplified: if (score == INTEL_PPAT_PERFECT_MATCH) { kref_get(&entry->ref); return entry;
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Quoting Wang, Zhi A (2017-08-29 12:13:26) > Hi Chris: > There is mapping between i915 cache level -> PPAT index. Currently it's: > > static gen8_pte_t gen8_pte_encode(dma_addr_t addr, > enum i915_cache_level level) > { > ... > switch (level) { > case I915_CACHE_NONE: > pte |= PPAT_UNCACHED_INDEX; > break; > case I915_CACHE_WT: > pte |= PPAT_DISPLAY_ELLC_INDEX; > break; > default: > pte |= PPAT_CACHED_INDEX; > break; > } > ... > > According to bspec, the PPAT index filled in the page table is calculated as: > > PPAT index = 4 * PAT + 2 * PCD + PWT > > In the i915_gem_gtt.c > > #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD) // PPAT > INDEX = 1 + 2 * 1 = 3 > #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */ // > PPAT INDEX = 0 > #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */ // > PPAT INDEX = 4 * 1 = 4 > #define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */ // > PPAT INDEX = 2 * 1 = 2 > > Actually the PPAT index used by i915 are: 0 2 3 4, which has already been > reserved in the RFC patches. So we can use these values in alloc_ppat, right? Still very concerned about the hole -- it kind of implies there is an entry we should be using but have forgotten! > > > @@ -2864,17 +3038,22 @@ static void bdw_setup_private_ppat(struct > > > drm_i915_private *dev_priv) > > > * So we can still hold onto all our assumptions wrt cpu > > > * clflushing on LLC machines. > > > */ > > > - pat = GEN8_PPAT(0, GEN8_PPAT_UC); > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > > + return; > > > + } > > > > > > - /* XXX: spec defines this as 2 distinct registers. It's unclear > > > if a 64b > > > -* write would work. */ > > > - I915_WRITE(GEN8_PRIVATE_PAT_LO, pat); > > > - I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32); > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* > > > for normal objects, no eLLC */ > > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* > > > for scanout with eLLC */ > > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* > > > Uncached objects, mostly for scanout */ > > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | /* See gen8_pte_encode() for the mapping from cache-level to ppat */ alloc_ppage_entry(ppat, PPAT_CACHED_PDE_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLC); alloc_ppage_entry(ppat, PPAT_DISPLAY_ELLC_INDEX, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); alloc_ppage_entry(ppat, PPAT_UNCACHED_INDEX, GEN8_PPAT_UC); alloc_ppage_entry(ppat, PPAT_CACHED_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)); etc. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Thanks for the reply! For the hole, per my understanding, the author wanted the mapping to be consistent: i915 cache level <-> PPAT index <-> cache attribute in IA page table in case the GPU and IA may share page tables in future, since actually PPAT index is represented as PAT/PCD/PWT bits in GPU page table entries. We can see the PPAT index is defined with those PAT/PCD/PWT bits from IA page tables. Maybe that's the reason of the hole. :) Thanks, Zhi. -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, August 29, 2017 2:24 PM To: Wang, Zhi A ; intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org Cc: joonas.lahti...@linux.intel.com; zhen...@linux.intel.com; Widawsky, Benjamin ; Vivi, Rodrigo Subject: RE: [RFCv5 2/2] drm/i915: Introduce private PAT management Quoting Wang, Zhi A (2017-08-29 12:13:26) > Hi Chris: > There is mapping between i915 cache level -> PPAT index. Currently it's: > > static gen8_pte_t gen8_pte_encode(dma_addr_t addr, > enum i915_cache_level level) { ... > switch (level) { > case I915_CACHE_NONE: > pte |= PPAT_UNCACHED_INDEX; > break; > case I915_CACHE_WT: > pte |= PPAT_DISPLAY_ELLC_INDEX; > break; > default: > pte |= PPAT_CACHED_INDEX; > break; > } > ... > > According to bspec, the PPAT index filled in the page table is calculated as: > > PPAT index = 4 * PAT + 2 * PCD + PWT > > In the i915_gem_gtt.c > > #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD) // PPAT > INDEX = 1 + 2 * 1 = 3 > #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */ // > PPAT INDEX = 0 > #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */ // > PPAT INDEX = 4 * 1 = 4 > #define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */ // > PPAT INDEX = 2 * 1 = 2 > > Actually the PPAT index used by i915 are: 0 2 3 4, which has already been > reserved in the RFC patches. So we can use these values in alloc_ppat, right? Still very concerned about the hole -- it kind of implies there is an entry we should be using but have forgotten! > > > @@ -2864,17 +3038,22 @@ static void bdw_setup_private_ppat(struct > > > drm_i915_private *dev_priv) > > > * So we can still hold onto all our assumptions wrt cpu > > > * clflushing on LLC machines. > > > */ > > > - pat = GEN8_PPAT(0, GEN8_PPAT_UC); > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > > + return; > > > + } > > > > > > - /* XXX: spec defines this as 2 distinct registers. It's unclear > > > if a 64b > > > -* write would work. */ > > > - I915_WRITE(GEN8_PRIVATE_PAT_LO, pat); > > > - I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32); > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* > > > for normal objects, no eLLC */ > > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* > > > for scanout with eLLC */ > > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* > > > Uncached objects, mostly for scanout */ > > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC > > > + | /* See gen8_pte_encode() for the mapping from cache-level to ppat */ alloc_ppage_entry(ppat, PPAT_CACHED_PDE_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLC); alloc_ppage_entry(ppat, PPAT_DISPLAY_ELLC_INDEX, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); alloc_ppage_entry(ppat, PPAT_UNCACHED_INDEX, GEN8_PPAT_UC); alloc_ppage_entry(ppat, PPAT_CACHED_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | GEN8_PPAT_AGE(0)); etc. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
On Tue, 2017-08-29 at 12:23 +0100, Chris Wilson wrote: > Quoting Wang, Zhi A (2017-08-29 12:13:26) > > Hi Chris: > > There is mapping between i915 cache level -> PPAT index. Currently it's: > > > > static gen8_pte_t gen8_pte_encode(dma_addr_t addr, > > enum i915_cache_level level) > > { > > ... > > switch (level) { > > case I915_CACHE_NONE: > > pte |= PPAT_UNCACHED_INDEX; > > break; > > case I915_CACHE_WT: > > pte |= PPAT_DISPLAY_ELLC_INDEX; > > break; > > default: > > pte |= PPAT_CACHED_INDEX; > > break; > > } > > ... > > > > According to bspec, the PPAT index filled in the page table is calculated > > as: > > > > PPAT index = 4 * PAT + 2 * PCD + PWT > > > > In the i915_gem_gtt.c > > > > #define PPAT_UNCACHED_INDEX (_PAGE_PWT | _PAGE_PCD) // PPAT > > INDEX = 1 + 2 * 1 = 3 > > #define PPAT_CACHED_PDE_INDEX 0 /* WB LLC */ // > > PPAT INDEX = 0 > > #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */ // > > PPAT INDEX = 4 * 1 = 4 > > #define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */ > > // PPAT INDEX = 2 * 1 = 2 > > > > Actually the PPAT index used by i915 are: 0 2 3 4, which has already been > > reserved in the RFC patches. > > So we can use these values in alloc_ppat, right? Still very concerned > about the hole -- it kind of implies there is an entry we should be > using but have forgotten! > > > > > @@ -2864,17 +3038,22 @@ static void bdw_setup_private_ppat(struct > > > > drm_i915_private *dev_priv) > > > > * So we can still hold onto all our assumptions wrt cpu > > > > * clflushing on LLC machines. > > > > */ > > > > - pat = GEN8_PPAT(0, GEN8_PPAT_UC); > > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC); > > > > + return; > > > > + } > > > > > > > > - /* XXX: spec defines this as 2 distinct registers. It's unclear > > > > if a 64b > > > > -* write would work. */ > > > > - I915_WRITE(GEN8_PRIVATE_PAT_LO, pat); > > > > - I915_WRITE(GEN8_PRIVATE_PAT_HI, pat >> 32); > > > > + alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC); /* > > > > for normal objects, no eLLC */ > > > > + alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC); /* > > > > for scanout with eLLC */ > > > > + alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC); /* > > > > Uncached objects, mostly for scanout */ > > > > + alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > > /* See gen8_pte_encode() for the mapping from cache-level to ppat */ > alloc_ppage_entry(ppat, PPAT_CACHED_PDE_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLC); > alloc_ppage_entry(ppat, PPAT_DISPLAY_ELLC_INDEX, GEN8_PPAT_WT | > GEN8_PPAT_LLCELLC); > alloc_ppage_entry(ppat, PPAT_UNCACHED_INDEX, GEN8_PPAT_UC); > alloc_ppage_entry(ppat, PPAT_CACHED_INDEX, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | > GEN8_PPAT_AGE(0)); > +1 on that idea as follow-up patch which should do these entry changes. I'd first make sure this patch leaves all actual register writes in equal state as they were before the patch, just the code is restructured. Bisecting will be easier if the actual added holes will be brought in by separate patch. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/chamelium: Let the Chamelium itself wait for a stable video input
On Mon, 2017-08-28 at 17:55 +0300, Paul Kocialkowski wrote: > Before capturing video, the Chamelium will always wait for the video > input to be stable (and perform the FSM if it was not). This means > that > there is no need to explicitly do it beforehand. > > When the receiver needs to be reset, the call will result in a > timeout, > after which the follow-up call to capture the video will perform the > FSM > that resets it. Skipping the explicit wait for video input stable > allows > the Chamelium to perform the FSM directly, which saves valuable time. > > Removing the associated call does not negatively impact the execution > of > the CRC and frame comparison tests either. I got an email from the mailer daemon indicating that this patch could not be delivered to Lyude. Lyude: did you receive the patch or should I resend it? > Signed-off-by: Paul Kocialkowski > --- > tests/chamelium.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/tests/chamelium.c b/tests/chamelium.c > index 484bb537..e3d81357 100644 > --- a/tests/chamelium.c > +++ b/tests/chamelium.c > @@ -474,9 +474,6 @@ enable_output(data_t *data, > if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA) > usleep(25); > > - chamelium_port_wait_video_input_stable(data->chamelium, port, > -HOTPLUG_TIMEOUT); > - > drmModeFreeConnector(connector); > } > -- Paul Kocialkowski Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Quoting Zhi Wang (2017-08-29 09:00:51) > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index b74fa9d..b99b6ca 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2816,41 +2816,215 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, > u64 size) > return 0; > } > > -static void cnl_setup_private_ppat(struct drm_i915_private *dev_priv) > +static struct intel_ppat_entry *alloc_ppat_entry(struct intel_ppat *ppat, > +unsigned int index, > +u8 value) > { > + struct intel_ppat_entry *entry = &ppat->entries[index]; Considering the magic numbers flying around (using values defined elsewhere) add GEM_BUG_ON(index >= ppat->max_entries); GEM_BUG_ON(test_bit(index, ppat->used)); > + > + entry->ppat = ppat; > + entry->value = value; > + kref_init(&entry->ref_count); > + set_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > + > + return entry; > +} > + > +static void free_ppat_entry(struct intel_ppat_entry *entry) > +{ > + struct intel_ppat *ppat = entry->ppat; > + int index = entry - ppat->entries; unsigned int index; GEM_BUG_ON(index >= ppat->max_entries); GEM_BUG_ON(!test_bit(index, ppat->used)); > + > + entry->value = ppat->dummy_value; > + clear_bit(index, ppat->used); > + set_bit(index, ppat->dirty); > +} ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Quoting Zhi Wang (2017-08-29 09:00:51) > The private PAT management is to support PPAT entry manipulation. Two > APIs are introduced for dynamically managing PPAT entries: intel_ppat_get > and intel_ppat_put. > > intel_ppat_get will search for an existing PPAT entry which perfectly > matches the required PPAT value. If not, it will try to allocate or > return a partially matched PPAT entry if there is any available PPAT > indexes or not. > > intel_ppat_put will put back the PPAT entry which comes from > intel_ppat_get. If it's dynamically allocated, the reference count will > be decreased. If the reference count turns into zero, the PPAT index is > freed again. > > Besides, another two callbacks are introduced to support the private PAT > management framework. One is ppat->update(), which writes the PPAT > configurations in ppat->entries into HW. Another one is ppat->match, which > will return a score to show how two PPAT values match with each other. Oh and since you are exporting an interface, I bet you would appreciate it if we had some unittests in selftests/ ;) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for tests/kms_atomic: subtest atomic_invalid_params requires CRTC
== Series Details == Series: tests/kms_atomic: subtest atomic_invalid_params requires CRTC URL : https://patchwork.freedesktop.org/series/29464/ State : success == Summary == Test kms_plane: Subgroup plane-panning-bottom-right-suspend-pipe-C-planes: skip -> PASS (shard-hsw) Subgroup plane-position-hole-dpms-pipe-C-planes: skip -> PASS (shard-hsw) Test perf: Subgroup polling: fail -> PASS (shard-hsw) fdo#102252 Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 Test kms_properties: Subgroup plane-properties-legacy: skip -> PASS (shard-hsw) Test kms_atomic_transition: Subgroup plane-all-transition-fencing: skip -> PASS (shard-hsw) fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 shard-hswtotal:2230 pass:1230 dwarn:0 dfail:0 fail:18 skip:982 time:9693s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_116/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Always sanity check engine state upon idling
Quoting Chris Wilson (2017-08-26 12:09:33) > When we do a locked idle we know that afterwards all requests have been > completed and the engines have been cleared of tasks. For whatever > reason, this doesn't always happen and we may go into a suspend with > ELSP still full, and this causes an issue upon resume as we get very, > very confused. > > If the engines refuse to idle, mark the device as wedged. In the process > we get rid of the maybe unused open-coded version of wait_for_engines > reported by Nick Desaulniers and Matthias Kaehlcke. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101891 References: https://bugs.freedesktop.org/show_bug.cgi?id=102456 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Joonas Lahtinen > Cc: Matthias Kaehlcke ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv5 2/2] drm/i915: Introduce private PAT management
Sure. I'm digging selftest now. -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Tuesday, August 29, 2017 3:18 PM To: Wang, Zhi A ; intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org Cc: joonas.lahti...@linux.intel.com; zhen...@linux.intel.com; Wang, Zhi A ; Widawsky, Benjamin ; Vivi, Rodrigo Subject: Re: [RFCv5 2/2] drm/i915: Introduce private PAT management Quoting Zhi Wang (2017-08-29 09:00:51) > The private PAT management is to support PPAT entry manipulation. Two > APIs are introduced for dynamically managing PPAT entries: > intel_ppat_get and intel_ppat_put. > > intel_ppat_get will search for an existing PPAT entry which perfectly > matches the required PPAT value. If not, it will try to allocate or > return a partially matched PPAT entry if there is any available PPAT > indexes or not. > > intel_ppat_put will put back the PPAT entry which comes from > intel_ppat_get. If it's dynamically allocated, the reference count > will be decreased. If the reference count turns into zero, the PPAT > index is freed again. > > Besides, another two callbacks are introduced to support the private > PAT management framework. One is ppat->update(), which writes the PPAT > configurations in ppat->entries into HW. Another one is ppat->match, > which will return a score to show how two PPAT values match with each other. Oh and since you are exporting an interface, I bet you would appreciate it if we had some unittests in selftests/ ;) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Open DRM driver without master for hpd storm exit
When running the full chamelium test binary, it occurs that the hpd storm exit handler (that restores its initial value) will fail when trying to acquire DRM master. This happens even though the previously-held DRM file descriptor was closed already. Since there is no need to get DRM master for debugfs access purposes, open the DRM node without requesting master to fix the issue. Signed-off-by: Paul Kocialkowski --- lib/igt_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index ee1f0f54..090b56e0 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -579,7 +579,7 @@ void igt_require_pipe_crc(int fd) static void igt_hpd_storm_exit_handler(int sig) { - int fd = drm_open_driver_master(DRIVER_INTEL); + int fd = drm_open_driver(DRIVER_INTEL); /* Here we assume that only one i915 device will be ever present */ igt_hpd_storm_reset(fd); -- 2.14.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Always sanity check engine state upon idling
On Sat, 2017-08-26 at 12:09 +0100, Chris Wilson wrote: > When we do a locked idle we know that afterwards all requests have been > completed and the engines have been cleared of tasks. For whatever > reason, this doesn't always happen and we may go into a suspend with > ELSP still full, and this causes an issue upon resume as we get very, > very confused. > > If the engines refuse to idle, mark the device as wedged. In the process > we get rid of the maybe unused open-coded version of wait_for_engines > reported by Nick Desaulniers and Matthias Kaehlcke. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101891 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Joonas Lahtinen > Cc: Matthias Kaehlcke I assume GEM_WARN_ON -> DRM_ERROR was intentional. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for lib/igt_debugfs: Open DRM driver without master for hpd storm exit
== Series Details == Series: lib/igt_debugfs: Open DRM driver without master for hpd storm exit URL : https://patchwork.freedesktop.org/series/29470/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3014 627598734e5e drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (fi-byt-j1900) fdo#101705 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:453s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:439s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:366s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:564s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:254s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:525s fi-byt-j1900 total:279 pass:255 dwarn:0 dfail:0 fail:0 skip:24 time:525s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:520s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:446s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:615s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:445s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:426s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:501s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:470s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:595s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:602s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:528s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:465s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:486s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:491s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:448s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:505s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:551s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:406s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_117/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Always sanity check engine state upon idling
Quoting Joonas Lahtinen (2017-08-29 14:07:40) > On Sat, 2017-08-26 at 12:09 +0100, Chris Wilson wrote: > > When we do a locked idle we know that afterwards all requests have been > > completed and the engines have been cleared of tasks. For whatever > > reason, this doesn't always happen and we may go into a suspend with > > ELSP still full, and this causes an issue upon resume as we get very, > > very confused. > > > > If the engines refuse to idle, mark the device as wedged. In the process > > we get rid of the maybe unused open-coded version of wait_for_engines > > reported by Nick Desaulniers and Matthias Kaehlcke. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101891 > > Signed-off-by: Chris Wilson > > Cc: Mika Kuoppala > > Cc: Joonas Lahtinen > > Cc: Matthias Kaehlcke > > I assume GEM_WARN_ON -> DRM_ERROR was intentional. Yes. The first time the unused function was reported, the thread drifted off in the direction of "that we probably want to always do the test" rather than only in CI. Now that we have seen glk actually fail in this way, we need to code defensively here (as it is no longer a theoretical programming error). As it still is a hw issue we want the warning (especially as this will cause the suspend to fail, we want a reason why) and as a general rule all wedging should indicate an error (because it is a last resort around driver bugs). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Always sanity check engine state upon idling
Chris Wilson writes: > When we do a locked idle we know that afterwards all requests have been > completed and the engines have been cleared of tasks. For whatever > reason, this doesn't always happen and we may go into a suspend with > ELSP still full, and this causes an issue upon resume as we get very, > very confused. > > If the engines refuse to idle, mark the device as wedged. In the process > we get rid of the maybe unused open-coded version of wait_for_engines > reported by Nick Desaulniers and Matthias Kaehlcke. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101891 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Joonas Lahtinen > Cc: Matthias Kaehlcke I noticed that when actually do switch to kernel context, it's async. And then we always do wait for idle. So as all our usage is sync, why don't we just wait the req in i915_gem_switch_to_kernel_context(i915) to pinpoint the request uncompletion. And in addition have this as a further harderning. But for the unconditional wedge and warn, Reviewed-by: Mika Kuoppala -Mika > --- > drivers/gpu/drm/i915/i915_gem.c | 20 > 1 file changed, 4 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index ac02785fdaff..c1520c0d2084 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3371,24 +3371,12 @@ static int wait_for_timeline(struct i915_gem_timeline > *tl, unsigned int flags) > return 0; > } > > -static int wait_for_engine(struct intel_engine_cs *engine, int timeout_ms) > -{ > - return wait_for(intel_engine_is_idle(engine), timeout_ms); > -} > - > static int wait_for_engines(struct drm_i915_private *i915) > { > - struct intel_engine_cs *engine; > - enum intel_engine_id id; > - > - for_each_engine(engine, i915, id) { > - if (GEM_WARN_ON(wait_for_engine(engine, 50))) { > - i915_gem_set_wedged(i915); > - return -EIO; > - } > - > - GEM_BUG_ON(intel_engine_get_seqno(engine) != > -intel_engine_last_submit(engine)); > + if (wait_for(intel_engines_are_idle(i915), 50)) { > + DRM_ERROR("Failed to idle engines, declaring wedged!\n"); > + i915_gem_set_wedged(i915); > + return -EIO; > } > > return 0; > -- > 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/12] tests/perf: rework oa-exponent test
On 23/08/17 23:17, Matthew Auld wrote: On 08/23, Lionel Landwerlin wrote: New issues that were discovered while making the tests work on Gen8+ : - we need to measure timings between periodic reports and discard all other kind of reports - it seems periodicity of the reports can be affected outside of RC6 (frequency change), we can detect this by looking at the amount of clock cycles per timestamp deltas Signed-off-by: Lionel Landwerlin --- tests/perf.c | 734 --- 1 file changed, 600 insertions(+), 134 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index ca69440d..e54a14ed 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -306,6 +307,25 @@ static uint32_t (*read_report_ticks)(uint32_t *report, static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t *oa_report1, enum drm_i915_oa_format format); +static bool +timestamp_delta_within(uint32_t delta, + uint32_t expected_delta, + uint32_t margin) +{ + return delta >= (expected_delta - margin) && + delta <= (expected_delta + margin); +} + +static bool +double_value_within(double value, + double expected, + double percent_margin) +{ + return value >= (expected - expected * percent_margin / 100.0) && + value <= (expected + expected * percent_margin / 100.0); + +} + static void __perf_close(int fd) { @@ -472,6 +492,20 @@ gen8_read_report_ticks(uint32_t *report, enum drm_i915_oa_format format) return report[3]; } +static void +gen8_read_report_clock_ratios(uint32_t *report, + uint32_t *slice_freq_mhz, + uint32_t *unslice_freq_mhz) +{ + uint32_t unslice_freq = report[0] & 0x1ff; + uint32_t slice_freq_low = (report[0] >> 25) & 0x7f; + uint32_t slice_freq_high = (report[0] >> 9) & 0x3; + uint32_t slice_freq = slice_freq_low | (slice_freq_high << 7); + + *slice_freq_mhz = (slice_freq * 1) / 1000; + *unslice_freq_mhz = (unslice_freq * 1) / 1000; +} + static const char * gen8_read_report_reason(const uint32_t *report) { @@ -494,29 +528,6 @@ gen8_read_report_reason(const uint32_t *report) return "unknown"; } -static bool -oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report) -{ - if (IS_HASWELL(devid)) { - /* For Haswell we don't have a documented report reason field -* (though empirically report[0] bit 10 does seem to correlate -* with a timer trigger reason) so we instead infer which -* reports are timer triggered by checking if the least -* significant bits are zero and the exponent bit is set. -*/ - uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1; - - if ((report[1] & oa_exponent_mask) != (1 << oa_exponent)) - return true; - } else { - if ((report[0] >> OAREPORT_REASON_SHIFT) & - OAREPORT_REASON_TIMER) - return true; - } - - return false; -} - static uint64_t timebase_scale(uint32_t u32_delta) { @@ -563,6 +574,29 @@ oa_exponent_to_ns(int exponent) return 10ULL * (2ULL << exponent) / timestamp_frequency; } +static bool +oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report) +{ + if (IS_HASWELL(devid)) { + /* For Haswell we don't have a documented report reason field +* (though empirically report[0] bit 10 does seem to correlate +* with a timer trigger reason) so we instead infer which +* reports are timer triggered by checking if the least +* significant bits are zero and the exponent bit is set. +*/ + uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1; + + if ((report[1] & oa_exponent_mask) == (1 << oa_exponent)) + return true; + } else { + if ((report[0] >> OAREPORT_REASON_SHIFT) & + OAREPORT_REASON_TIMER) + return true; + } + + return false; +} + static bool oa_report_ctx_is_valid(uint32_t *report) { @@ -578,6 +612,128 @@ oa_report_ctx_is_valid(uint32_t *report) igt_assert(!"reached"); } +static uint32_t +oa_report_get_ctx_id(uint32_t *report) +{ + if (!oa_report_ctx_is_valid(report)) + return 0x; + return report[2]; +} + +static double +oa_reports_tick_per_period(uint32_t *report0, uint32_t *report1) +{ + if (intel_gen(devid) < 8) + return 0.0; + + /* Measure the number GPU tick delta t
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Clear wedged status upon resume
Chris Wilson writes: > When we wait up from suspend, the device has been powered down and > should come back afresh. We should be able to safely remove the wedged > status from the previous session and start afresh. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Joonas Lahtinen Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/i915_gem.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c1520c0d2084..9dc24b915aa7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4518,6 +4518,12 @@ static void assert_kernel_context_is_current(struct > drm_i915_private *dev_priv) > > void i915_gem_sanitize(struct drm_i915_private *i915) > { > + if (i915_terminally_wedged(&i915->gpu_error)) { > + mutex_lock(&i915->drm.struct_mutex); > + i915_gem_unset_wedged(i915); > + mutex_unlock(&i915->drm.struct_mutex); > + } > + > /* >* If we inherit context state from the BIOS or earlier occupants >* of the GPU, the GPU may be in an inconsistent state when we > -- > 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 03/12] tests/perf: update max buffer size for reading reports
Signed-off-by: Lionel Landwerlin --- tests/perf.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index 5058315c..bc5ea133 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -1280,9 +1280,7 @@ read_2_oa_reports(int format_id, /* Note: we allocate a large buffer so that each read() iteration * should scrape *all* pending records. * -* The largest buffer the OA unit supports is 16MB and the smallest -* OA report format is 64bytes allowing up to 262144 reports to -* be buffered. +* The largest buffer the OA unit supports is 16MB. * * Being sure we are fetching all buffered reports allows us to * potentially throw away / skip all reports whenever we see @@ -1295,7 +1293,8 @@ read_2_oa_reports(int format_id, * to indicate that the OA unit may be over taxed if lots of reports * are being lost. */ - int buf_size = 262144 * (64 + sizeof(struct drm_i915_perf_record_header)); + int max_reports = (16 * 1024 * 1024) / format_size; + int buf_size = sample_size * max_reports * 1.5; uint8_t *buf = malloc(buf_size); int n = 0; @@ -1307,6 +1306,7 @@ read_2_oa_reports(int format_id, ; igt_assert(len > 0); + igt_debug("read %d bytes\n", (int)len); for (size_t offset = 0; offset < len; offset += header->size) { const uint32_t *report; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 04/12] tests/perf: rc6: try to guess when rc6 is disabled
Signed-off-by: Lionel Landwerlin --- tests/perf.c | 13 + 1 file changed, 13 insertions(+) diff --git a/tests/perf.c b/tests/perf.c index bc5ea133..1b441601 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -3445,6 +3445,17 @@ gen8_test_single_ctx_render_target_writes_a_counter(void) } while (WEXITSTATUS(child_ret) == EAGAIN); } +static bool +rc6_enabled(void) +{ + char *rc6_status = read_debugfs_record(drm_fd, "i915_drpc_info", + "RC6 Enabled"); + bool enabled = strcmp(rc6_status, "yes") == 0; + + free(rc6_status); + return enabled; +} + static void test_rc6_disable(void) { @@ -3464,6 +3475,8 @@ test_rc6_disable(void) }; uint64_t n_events_start, n_events_end; + igt_skip_on(!rc6_enabled()); + stream_fd = __perf_open(drm_fd, ¶m); n_events_start = read_debugfs_u64_record(drm_fd, "i915_drpc_info", -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 00/12] Improve robustness of the i915 perf tests
Hi, Some nits by Matthew in patch 6. Cheers, Lionel Landwerlin (11): tests/perf: make stream_fd a global variable tests/perf: update max buffer size for reading reports tests/perf: rc6: try to guess when rc6 is disabled tests/perf: remove frequency related changes tests/perf: rework oa-exponent test tests/perf: make enable-disable more reliable tests/perf: make buffer-fill more reliable tests/perf: add Kabylake support tests/perf: add Geminilake support tests/perf: estimate number of blocking/polling based on time spent tests/perf: prevent power management to kick in when necessary Robert Bragg (1): tests/perf: add per context filtering test for gen8+ tests/perf.c | 2037 +++--- 1 file changed, 1672 insertions(+), 365 deletions(-) -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 02/12] tests/perf: add per context filtering test for gen8+
From: Robert Bragg Signed-off-by: Robert Bragg Signed-off-by: Lionel Landwerlin --- tests/perf.c | 777 --- 1 file changed, 745 insertions(+), 32 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index ca5bfdc5..5058315c 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -48,7 +48,9 @@ IGT_TEST_DESCRIPTION("Test the i915 perf metrics streaming interface"); #define OAREPORT_REASON_MASK 0x3f #define OAREPORT_REASON_SHIFT 19 #define OAREPORT_REASON_TIMER (1<<0) +#define OAREPORT_REASON_INTERNAL (3<<1) #define OAREPORT_REASON_CTX_SWITCH (1<<3) +#define OAREPORT_REASON_GO (1<<4) #define OAREPORT_REASON_CLK_RATIO (1<<5) #define GFX_OP_PIPE_CONTROL ((3 << 29) | (3 << 27) | (2 << 24)) @@ -574,6 +576,22 @@ oa_exponent_to_ns(int exponent) return 10ULL * (2ULL << exponent) / timestamp_frequency; } +static bool +oa_report_ctx_is_valid(uint32_t *report) +{ + if (IS_HASWELL(devid)) { + return false; /* TODO */ + } else if (IS_GEN8(devid)) { + return report[0] & (1ul << 25); + } else if (IS_GEN9(devid)) { + return report[0] & (1ul << 16); + } + + /* Need to update this function for newer Gen. */ + igt_assert(!"reached"); +} + + static void hsw_sanity_check_render_basic_reports(uint32_t *oa_report0, uint32_t *oa_report1, enum drm_i915_oa_format fmt) @@ -678,6 +696,100 @@ gen8_40bit_a_delta(uint64_t value0, uint64_t value1) return value1 - value0; } +static void +accumulate_uint32(size_t offset, + uint32_t *report0, + uint32_t *report1, + uint64_t *delta) +{ + uint32_t value0 = *(uint32_t *)(((uint8_t *)report0) + offset); + uint32_t value1 = *(uint32_t *)(((uint8_t *)report1) + offset); + + *delta += (uint32_t)(value1 - value0); +} + +static void +accumulate_uint40(int a_index, + uint32_t *report0, + uint32_t *report1, + enum drm_i915_oa_format format, + uint64_t *delta) +{ + uint64_t value0 = gen8_read_40bit_a_counter(report0, format, a_index), +value1 = gen8_read_40bit_a_counter(report1, format, a_index); + + *delta += gen8_40bit_a_delta(value0, value1); +} + +static void +accumulate_reports(struct accumulator *accumulator, + uint32_t *start, + uint32_t *end) +{ + enum drm_i915_oa_format format = accumulator->format; + uint64_t *deltas = accumulator->deltas; + int idx = 0; + + if (intel_gen(devid) >= 8) { + /* timestamp */ + accumulate_uint32(4, start, end, deltas + idx++); + + /* clock cycles */ + accumulate_uint32(12, start, end, deltas + idx++); + } else { + /* timestamp */ + accumulate_uint32(4, start, end, deltas + idx++); + } + + for (int i = 0; i < oa_formats[format].n_a40; i++) + accumulate_uint40(i, start, end, format, deltas + idx++); + + for (int i = 0; i < oa_formats[format].n_a; i++) { + accumulate_uint32(oa_formats[format].a_off + 4 * i, + start, end, deltas + idx++); + } + + for (int i = 0; i < oa_formats[format].n_b; i++) { + accumulate_uint32(oa_formats[format].b_off + 4 * i, + start, end, deltas + idx++); + } + + for (int i = 0; i < oa_formats[format].n_c; i++) { + accumulate_uint32(oa_formats[format].c_off + 4 * i, + start, end, deltas + idx++); + } +} + +static void +accumulator_print(struct accumulator *accumulator, const char *title) +{ + enum drm_i915_oa_format format = accumulator->format; + uint64_t *deltas = accumulator->deltas; + int idx = 0; + + igt_debug("%s:\n", title); + if (intel_gen(devid) >= 8) { + igt_debug("\ttime delta = %lu\n", deltas[idx++]); + igt_debug("\tclock cycle delta = %lu\n", deltas[idx++]); + + for (int i = 0; i < oa_formats[format].n_a40; i++) + igt_debug("\tA%u = %lu\n", i, deltas[idx++]); + } else { + igt_debug("\ttime delta = %lu\n", deltas[idx++]); + } + + for (int i = 0; i < oa_formats[format].n_a; i++) { + int a_id = oa_formats[format].first_a + i; + igt_debug("\tA%u = %lu\n", a_id, deltas[idx++]); + } + + for (int i = 0; i < oa_formats[format].n_a; i++) + igt_debug("\tB%u = %lu\n", i, deltas[idx++]); + + for (int i = 0; i < oa_formats[format].n_c; i++) + igt_debug("\tC%u = %lu\n", i, deltas[idx++]); +} + /* The TestOa metric set is designed so */ static void gen8_sanity_check_test_
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Always sanity check engine state upon idling
Quoting Mika Kuoppala (2017-08-29 14:36:57) > Chris Wilson writes: > > > When we do a locked idle we know that afterwards all requests have been > > completed and the engines have been cleared of tasks. For whatever > > reason, this doesn't always happen and we may go into a suspend with > > ELSP still full, and this causes an issue upon resume as we get very, > > very confused. > > > > If the engines refuse to idle, mark the device as wedged. In the process > > we get rid of the maybe unused open-coded version of wait_for_engines > > reported by Nick Desaulniers and Matthias Kaehlcke. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101891 > > Signed-off-by: Chris Wilson > > Cc: Mika Kuoppala > > Cc: Joonas Lahtinen > > Cc: Matthias Kaehlcke > > I noticed that when actually do switch to kernel context, it's > async. And then we always do wait for idle. > > So as all our usage is sync, why don't we just wait the req in > i915_gem_switch_to_kernel_context(i915) to pinpoint the request > uncompletion. And in addition have this as a further harderning. They are separate for historical reasons, i.e. they have been used independently. Note that the switch to kernel context may be between 0 and one request per engine to wait upon, and yet we still want to wait. However, we can move the wait-for-idle into switch-to-kernel-context as that is common across all callers at present. * spots an open coded switch to kernel context. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 01/12] tests/perf: make stream_fd a global variable
When debugging unstable tests on new platforms we currently we don't cleanup everything well in between different tests. Since only a single OA stream fd can be opened at a time, having the stream_fd as a global variable helps us cleanup the state between tests. Signed-off-by: Lionel Landwerlin --- tests/perf.c | 121 --- 1 file changed, 65 insertions(+), 56 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index dd2263ee..ca5bfdc5 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -285,6 +285,7 @@ static bool hsw_undefined_a_counters[45] = { static bool gen8_undefined_a_counters[45]; static int drm_fd = -1; +static int stream_fd = -1; static uint32_t devid; static int card = -1; static int n_eus; @@ -306,10 +307,22 @@ static uint32_t (*read_report_ticks)(uint32_t *report, static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t *oa_report1, enum drm_i915_oa_format format); +static void +__perf_close(int fd) +{ + close(fd); + stream_fd = -1; +} + static int __perf_open(int fd, struct drm_i915_perf_open_param *param) { - int ret = igt_ioctl(fd, DRM_IOCTL_I915_PERF_OPEN, param); + int ret; + + if (stream_fd >= 0) + __perf_close(stream_fd); + + ret = igt_ioctl(fd, DRM_IOCTL_I915_PERF_OPEN, param); igt_assert(ret >= 0); errno = 0; @@ -960,14 +973,12 @@ test_system_wide_paranoid(void) .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; - int stream_fd; - write_u64_file("/proc/sys/dev/i915/perf_stream_paranoid", 0); igt_drop_root(); stream_fd = __perf_open(drm_fd, ¶m); - close(stream_fd); + __perf_close(stream_fd); } igt_waitchildren(); @@ -1015,7 +1026,6 @@ test_invalid_oa_metric_set_id(void) .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; - int stream_fd; do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m, EINVAL); @@ -1025,7 +1035,7 @@ test_invalid_oa_metric_set_id(void) /* Check that we aren't just seeing false positives... */ properties[ARRAY_SIZE(properties) - 1] = test_metric_set_id; stream_fd = __perf_open(drm_fd, ¶m); - close(stream_fd); + __perf_close(stream_fd); /* There's no valid default OA metric set ID... */ param.num_properties--; @@ -1050,7 +1060,6 @@ test_invalid_oa_format_id(void) .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; - int stream_fd; do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m, EINVAL); @@ -1060,7 +1069,7 @@ test_invalid_oa_format_id(void) /* Check that we aren't just seeing false positives... */ properties[ARRAY_SIZE(properties) - 1] = test_oa_format; stream_fd = __perf_open(drm_fd, ¶m); - close(stream_fd); + __perf_close(stream_fd); /* There's no valid default OA format... */ param.num_properties--; @@ -1088,8 +1097,7 @@ test_missing_sample_flags(void) } static void -read_2_oa_reports(int stream_fd, - int format_id, +read_2_oa_reports(int format_id, int exponent, uint32_t *oa_report0, uint32_t *oa_report1, @@ -1223,12 +1231,13 @@ open_and_read_2_oa_reports(int format_id, .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; - int stream_fd = __perf_open(drm_fd, ¶m); - read_2_oa_reports(stream_fd, format_id, exponent, + stream_fd = __perf_open(drm_fd, ¶m); + + read_2_oa_reports(format_id, exponent, oa_report0, oa_report1, timer_only); - close(stream_fd); + __perf_close(stream_fd); } static void @@ -1528,9 +1537,10 @@ test_invalid_oa_exponent(void) .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; - int stream_fd = __perf_open(drm_fd, ¶m); - close(stream_fd); + stream_fd = __perf_open(drm_fd, ¶m); + + __perf_close(stream_fd); for (int i = 32; i < 65; i++) { properties[7] = i; @@ -1580,12 +1590,10 @@ test_low_oa_exponent_permissions(void) properties[7] = ok_exponent; igt_fork(child, 1) { - int stream_fd; - igt_drop_root(); stream_fd = __perf_open(drm_fd, ¶m); - close(stream_fd); + __perf_close(stream_fd); } igt_waitchildren(); @@ -1634,7 +1642,6 @@ test_per_context_mode_unprivileged(void) igt_fork(child,
[Intel-gfx] [PATCH i-g-t v4 07/12] tests/perf: make enable-disable more reliable
Estimation of the amount of reports can only refer to periodic ones, as context switch reports completely depend on what happens on the system. Also generate some load to prevent clock frequency changes to impact our measurement. Signed-off-by: Lionel Landwerlin --- tests/perf.c | 96 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index efaa4a06..0177f017 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -2778,10 +2778,18 @@ test_enable_disable(void) int n_full_oa_reports = oa_buf_size / report_size; uint64_t fill_duration = n_full_oa_reports * oa_period; + load_helper_init(); + load_helper_run(HIGH); + stream_fd = __perf_open(drm_fd, ¶m); for (int i = 0; i < 5; i++) { int len; + uint32_t n_periodic_reports; + struct drm_i915_perf_record_header *header; + uint32_t first_timestamp = 0, last_timestamp = 0; + uint32_t last_periodic_report[64]; + double tick_per_period; /* Giving enough time for an overflow might help catch whether * the OA unit has been enabled even if the driver might at @@ -2801,18 +2809,91 @@ test_enable_disable(void) nanosleep(&(struct timespec){ .tv_sec = 0, .tv_nsec = fill_duration / 2 }, - NULL); + NULL); - while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR) - ; + n_periodic_reports = 0; - igt_assert_neq(len, -1); + /* Because of the race condition between notification of new +* reports and reports landing in memory, we need to rely on +* timestamps to figure whether we've read enough of them. +*/ + while (((last_timestamp - first_timestamp) * oa_exponent_to_ns(oa_exponent)) < + (fill_duration / 2)) { - igt_assert(len > report_size * n_full_oa_reports * 0.45); - igt_assert(len < report_size * n_full_oa_reports * 0.55); + while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR) + ; + + igt_assert_neq(len, -1); + + for (int offset = 0; offset < len; offset += header->size) { + uint32_t *report; + double previous_tick_per_period; + + header = (void *) (buf + offset); + report = (void *) (header + 1); + + switch (header->type) { + case DRM_I915_PERF_RECORD_OA_REPORT_LOST: + break; + case DRM_I915_PERF_RECORD_SAMPLE: + if (first_timestamp == 0) + first_timestamp = report[1]; + last_timestamp = report[1]; + + previous_tick_per_period = tick_per_period; + + if (n_periodic_reports > 0 && + oa_report_is_periodic(oa_exponent, report)) { + tick_per_period = + oa_reports_tick_per_period(last_periodic_report, + report); + + if (!double_value_within(tick_per_period, + previous_tick_per_period, 5)) + igt_debug("clock change!\n"); + + igt_debug(" > report ts=%u" + " ts_delta_last_periodic=%8u is_timer=%i ctx_id=%8x gpu_ticks=%u nb_periodic=%u\n", + report[1], + report[1] - last_periodic_report[1], + oa_report_is_periodic(oa_exponent, report), + oa_report_get_ctx_id(report), + report[3] - last_periodic_report[3], + n_periodic_reports); + + memcpy(last_periodic_report, report, + sizeof(last_periodic_report)); +
[Intel-gfx] [PATCH i-g-t v4 11/12] tests/perf: estimate number of blocking/polling based on time spent
Blocking & polling tests define an amount of time to spend in the test and then estimate the number of syscalls that should successfully return. The problem is that while running the test we might spend slightly more time than initiallly planned. This change estimates the number of syscalls based on time spent after the fact. Signed-off-by: Lionel Landwerlin --- tests/perf.c | 42 +- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index 24df7c2a..6c062d20 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -2372,7 +2372,8 @@ test_blocking(void) DRM_I915_PERF_PROP_OA_EXPONENT, oa_exponent, }; struct drm_i915_perf_open_param param = { - .flags = I915_PERF_FLAG_FD_CLOEXEC, + .flags = I915_PERF_FLAG_FD_CLOEXEC | + I915_PERF_FLAG_DISABLED, .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; @@ -2397,16 +2398,17 @@ test_blocking(void) */ int min_iterations = (test_duration_ns / (oa_period + 600ull)); - int64_t start; + int64_t start, end; int n = 0; stream_fd = __perf_open(drm_fd, ¶m); times(&start_times); - igt_debug("tick length = %dns, test duration = %"PRIu64"ns, min iter. = %d, max iter. = %d\n", + igt_debug("tick length = %dns, test duration = %"PRIu64"ns, min iter. = %d," + " estimated max iter. = %d, oa_period = %"PRIu64"ns\n", (int)tick_ns, test_duration_ns, - min_iterations, max_iterations); + min_iterations, max_iterations, oa_period); /* In the loop we perform blocking polls while the HW is sampling at * ~25Hz, with the expectation that we spend most of our time blocked @@ -2425,8 +2427,13 @@ test_blocking(void) * floor(real_stime)). * * We Loop for 1000 x tick_ns so one tick corresponds to 0.1% +* +* Also enable the stream just before poll/read to minimize +* the error delta. */ - for (start = get_time(); (get_time() - start) < test_duration_ns; /* nop */) { + start = get_time(); + do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0); + for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) { struct drm_i915_perf_record_header *header; bool timer_report_read = false; bool non_timer_report_read = false; @@ -2468,6 +2475,12 @@ test_blocking(void) n++; } + /* Updated the maximum of iterations based on the time spent +* in the loop. +*/ + max_iterations = (end - start) / oa_period + 1; + igt_debug("adjusted max iter. = %d\n", max_iterations); + times(&end_times); /* Using nanosecond units is fairly silly here, given the tick in- @@ -2524,6 +2537,7 @@ test_polling(void) }; struct drm_i915_perf_open_param param = { .flags = I915_PERF_FLAG_FD_CLOEXEC | + I915_PERF_FLAG_DISABLED | I915_PERF_FLAG_FD_NONBLOCK, .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), @@ -2548,7 +2562,7 @@ test_polling(void) * to check for data and giving some time to read(). */ int min_iterations = (test_duration_ns / (oa_period + 600ull)); - int64_t start; + int64_t start, end; int n = 0; stream_fd = __perf_open(drm_fd, ¶m); @@ -2576,8 +2590,13 @@ test_polling(void) * floor(real_stime)). * * We Loop for 1000 x tick_ns so one tick corresponds to 0.1% +* +* Also enable the stream just before poll/read to minimize +* the error delta. */ - for (start = get_time(); (get_time() - start) < test_duration_ns; /* nop */) { + start = get_time(); + do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0); + for (/* nop */; ((end = get_time()) - start) < test_duration_ns; /* nop */) { struct pollfd pollfd = { .fd = stream_fd, .events = POLLIN }; struct drm_i915_perf_record_header *header; bool timer_report_read = false; @@ -2625,8 +2644,7 @@ test_polling(void) if (header->type == DRM_I915_PERF_RECORD_SAMPLE) { uint32_t *report = (void *)(header + 1); - if (oa_report_is_periodic(oa_exponent, - report)) + if (oa_report_is_periodic(oa_exponent, report)) timer_report_read = true; else
[Intel-gfx] [PATCH i-g-t v4 10/12] tests/perf: add Geminilake support
Signed-off-by: Lionel Landwerlin Reviewed-by: Matthew Auld --- tests/perf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/perf.c b/tests/perf.c index 32f34ec4..24df7c2a 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -1124,6 +1124,9 @@ init_sys_info(void) return false; } timestamp_frequency = 1200; + } else if (IS_GEMINILAKE(devid)) { + test_set_uuid = "dd3fd789-e783-4204-8cd0-b671bbccb0cf"; + timestamp_frequency = 1920; } else { igt_debug("unsupported GT\n"); return false; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 09/12] tests/perf: add Kabylake support
Signed-off-by: Lionel Landwerlin Reviewed-by: Matthew Auld --- tests/perf.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/perf.c b/tests/perf.c index 32d53ea1..32f34ec4 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -,8 +,23 @@ init_sys_info(void) } else if (IS_BROXTON(devid)) { test_set_uuid = "5ee72f5c-092f-421e-8b70-225f7c3e9612"; timestamp_frequency = 1920; - } else + } else if (IS_KABYLAKE(devid)) { + switch (intel_gt(devid)) { + case 1: + test_set_uuid = "baa3c7e4-52b6-4b85-801e-465a94b746dd"; + break; + case 2: + test_set_uuid = "f1792f32-6db2-4b50-b4b2-557128f1688d"; + break; + default: + igt_debug("unsupported Kabylake GT size\n"); + return false; + } + timestamp_frequency = 1200; + } else { + igt_debug("unsupported GT\n"); return false; + } gp.param = I915_PARAM_EU_TOTAL; gp.value = &n_eus; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4 05/12] tests/perf: remove frequency related changes
Experience shows that most of the issues we face with periodicity of the reports produced by the OA unit are related to power management, not frequency. Signed-off-by: Lionel Landwerlin --- tests/perf.c | 141 --- 1 file changed, 9 insertions(+), 132 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index 1b441601..ca69440d 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -293,12 +293,9 @@ static int card = -1; static int n_eus; static uint64_t test_metric_set_id = UINT64_MAX; -static uint64_t gt_min_freq_mhz_saved = 0; -static uint64_t gt_max_freq_mhz_saved = 0; -static uint64_t gt_min_freq_mhz = 0; -static uint64_t gt_max_freq_mhz = 0; static uint64_t timestamp_frequency = 1250; +static uint64_t gt_max_freq_mhz = 0; static enum drm_i915_oa_format test_oa_format; static bool *undefined_a_counters; static uint64_t oa_exp_1_millisec; @@ -402,16 +399,6 @@ sysfs_read(const char *file) return read_u64_file(buf); } -static void -sysfs_write(const char *file, uint64_t val) -{ - char buf[512]; - - snprintf(buf, sizeof(buf), "/sys/class/drm/card%d/%s", card, file); - - write_u64_file(buf, val); -} - static char * read_debugfs_record(int device, const char *file, const char *key) { @@ -990,54 +977,6 @@ init_sys_info(void) return try_read_u64_file(buf, &test_metric_set_id); } -static void -gt_frequency_range_save(void) -{ - gt_min_freq_mhz_saved = sysfs_read("gt_min_freq_mhz"); - gt_max_freq_mhz_saved = sysfs_read("gt_max_freq_mhz"); - - gt_min_freq_mhz = gt_min_freq_mhz_saved; - gt_max_freq_mhz = gt_max_freq_mhz_saved; -} - -static void -gt_frequency_pin(int gt_freq_mhz) -{ - igt_debug("requesting pinned GT freq = %dmhz\n", gt_freq_mhz); - - if (gt_freq_mhz > gt_max_freq_mhz) { - sysfs_write("gt_max_freq_mhz", gt_freq_mhz); - sysfs_write("gt_min_freq_mhz", gt_freq_mhz); - } else { - sysfs_write("gt_min_freq_mhz", gt_freq_mhz); - sysfs_write("gt_max_freq_mhz", gt_freq_mhz); - } - gt_min_freq_mhz = gt_freq_mhz; - gt_max_freq_mhz = gt_freq_mhz; -} - -static void -gt_frequency_range_restore(void) -{ - igt_debug("restoring GT frequency range: min = %dmhz, max =%dmhz, current: min=%dmhz, max=%dmhz\n", - (int)gt_min_freq_mhz_saved, - (int)gt_max_freq_mhz_saved, - (int)gt_min_freq_mhz, - (int)gt_max_freq_mhz); - - /* Assume current min/max are the same */ - if (gt_min_freq_mhz_saved > gt_max_freq_mhz) { - sysfs_write("gt_max_freq_mhz", gt_max_freq_mhz_saved); - sysfs_write("gt_min_freq_mhz", gt_min_freq_mhz_saved); - } else { - sysfs_write("gt_min_freq_mhz", gt_min_freq_mhz_saved); - sysfs_write("gt_max_freq_mhz", gt_max_freq_mhz_saved); - } - - gt_min_freq_mhz = gt_min_freq_mhz_saved; - gt_max_freq_mhz = gt_max_freq_mhz_saved; -} - static int i915_read_reports_until_timestamp(enum drm_i915_oa_format oa_format, uint8_t *buf, @@ -1614,33 +1553,9 @@ test_oa_formats(void) } static void -test_oa_exponents(int gt_freq_mhz) +test_oa_exponents(void) { - uint32_t freq_margin; - - /* This test tries to use the sysfs interface for pinning the GT -* frequency so we have another point of reference for comparing with -* the clock frequency as derived from OA reports. -* -* This test has been finicky to stabilise while the -* gt_min/max_freq_mhz files in sysfs don't seem to be a reliable -* mechanism for fixing the gpu frequency. -* -* Since these unit tests are focused on the OA unit not the ability to -* pin the frequency via sysfs we make the test account for pinning not -* being reliable and read back the current frequency for each -* iteration of this test to take this into account. -*/ - gt_frequency_pin(gt_freq_mhz); - - igt_debug("Testing OA timer exponents with requested GT frequency = %dmhz\n", - gt_freq_mhz); - - /* allow a +- 10% error margin when checking that the frequency -* calculated from the OA reports matches the frequency according to -* sysfs. -*/ - freq_margin = gt_freq_mhz * 0.1; + igt_debug("Testing OA timer exponents\n"); /* It's asking a lot to sample with a 160 nanosecond period and the * test can fail due to buffer overflows if it wasn't possible to @@ -1655,7 +1570,6 @@ test_oa_exponents(int gt_freq_mhz) uint32_t clock_delta; uint32_t freq; int n_tested = 0; - int n_freq_matches = 0; /* The exponent is effectively selecting a bit in the timestamp * to trigger reports on and so in practic
[Intel-gfx] [PATCH i-g-t v4 08/12] tests/perf: make buffer-fill more reliable
Filling rate of the buffer must discard context switch reports as they do not depend upon the periodicity, instead they're a factor on the amount of different applications concurrently running on the system. Signed-off-by: Lionel Landwerlin Tested-by: Matthew Auld Reviewed-by: Matthew Auld --- tests/perf.c | 120 ++- 1 file changed, 103 insertions(+), 17 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index 0177f017..32d53ea1 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -2686,22 +2686,30 @@ test_buffer_fill(void) .num_properties = sizeof(properties) / 16, .properties_ptr = to_user_pointer(properties), }; + struct drm_i915_perf_record_header *header; int buf_size = 65536 * (256 + sizeof(struct drm_i915_perf_record_header)); uint8_t *buf = malloc(buf_size); + int len; size_t oa_buf_size = 16 * 1024 * 1024; size_t report_size = oa_formats[test_oa_format].size; int n_full_oa_reports = oa_buf_size / report_size; uint64_t fill_duration = n_full_oa_reports * oa_period; + load_helper_init(); + load_helper_run(HIGH); + igt_assert(fill_duration < 10); stream_fd = __perf_open(drm_fd, ¶m); for (int i = 0; i < 5; i++) { - struct drm_i915_perf_record_header *header; bool overflow_seen; - int offset = 0; - int len; + uint32_t n_periodic_reports; + uint32_t first_timestamp = 0, last_timestamp = 0; + uint32_t last_periodic_report[64]; + double tick_per_period; + + do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0); nanosleep(&(struct timespec){ .tv_sec = 0, .tv_nsec = fill_duration * 1.25 }, @@ -2713,7 +2721,7 @@ test_buffer_fill(void) igt_assert_neq(len, -1); overflow_seen = false; - for (offset = 0; offset < len; offset += header->size) { + for (int offset = 0; offset < len; offset += header->size) { header = (void *)(buf + offset); if (header->type == DRM_I915_PERF_RECORD_OA_BUFFER_LOST) @@ -2722,32 +2730,110 @@ test_buffer_fill(void) igt_assert_eq(overflow_seen, true); + do_ioctl(stream_fd, I915_PERF_IOCTL_DISABLE, 0); + + igt_debug("fill_duration = %luns, oa_exponent = %u\n", + fill_duration, oa_exponent); + + do_ioctl(stream_fd, I915_PERF_IOCTL_ENABLE, 0); + nanosleep(&(struct timespec){ .tv_sec = 0, - .tv_nsec = fill_duration / 2 }, - NULL); + .tv_nsec = fill_duration / 2 }, + NULL); - while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR) - ; + n_periodic_reports = 0; - igt_assert_neq(len, -1); + /* Because of the race condition between notification of new +* reports and reports landing in memory, we need to rely on +* timestamps to figure whether we've read enough of them. +*/ + while (((last_timestamp - first_timestamp) * oa_period) < (fill_duration / 2)) { - igt_assert(len > report_size * n_full_oa_reports * 0.45); - igt_assert(len < report_size * n_full_oa_reports * 0.55); + igt_debug("dts=%u elapsed=%lu duration=%lu\n", + last_timestamp - first_timestamp, + (last_timestamp - first_timestamp) * oa_period, + fill_duration / 2); - overflow_seen = false; - for (offset = 0; offset < len; offset += header->size) { - header = (void *)(buf + offset); + while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR) + ; - if (header->type == DRM_I915_PERF_RECORD_OA_BUFFER_LOST) - overflow_seen = true; + igt_assert_neq(len, -1); + + for (int offset = 0; offset < len; offset += header->size) { + uint32_t *report; + double previous_tick_per_period; + + header = (void *) (buf + offset); + report = (void *) (header + 1); + + switch (header->type) { + case DRM_I915_PERF_RECORD_OA_REPORT_LOST: + igt_debug("report loss, trying again\n"); +
[Intel-gfx] [PATCH i-g-t v4 12/12] tests/perf: prevent power management to kick in when necessary
Some of our tests measure that the OA unit produces reports at expected time intervals (as configured through the PERF_OPEN ioctl). It turns out the power management plays a role in the decision of the OA unit to write reports to memory. Under normal circumstances we don't really mind if the unit misses one report here or there, but for our tests it makes pretty difficult to verify whether we've made a mistake in the configuration. To work around this, let's prevent power management to kick in by holding /dev/cpu_dma_latency opened for the following tests : - blocking - polling - buffer-fill - oa-exponents Many thanks to Chris Wilson for suggesting this! Signed-off-by: Lionel Landwerlin --- tests/perf.c | 64 ++-- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index 6c062d20..070dee97 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -288,6 +288,7 @@ static bool hsw_undefined_a_counters[45] = { static bool gen8_undefined_a_counters[45]; static int drm_fd = -1; +static int pm_fd = -1; static int stream_fd = -1; static uint32_t devid; static int card = -1; @@ -331,21 +332,38 @@ __perf_close(int fd) { close(fd); stream_fd = -1; + + if (pm_fd >= 0) { + close(pm_fd); + pm_fd = -1; + } } static int -__perf_open(int fd, struct drm_i915_perf_open_param *param) +__perf_open(int fd, struct drm_i915_perf_open_param *param, bool prevent_pm) { int ret; + int32_t pm_value = 0; if (stream_fd >= 0) __perf_close(stream_fd); + if (pm_fd >= 0) { + close(pm_fd); + pm_fd = -1; + } ret = igt_ioctl(fd, DRM_IOCTL_I915_PERF_OPEN, param); igt_assert(ret >= 0); errno = 0; + if (prevent_pm) { + pm_fd = open("/dev/cpu_dma_latency", O_RDWR); + igt_assert(pm_fd >= 0); + + igt_assert_eq(write(pm_fd, &pm_value, sizeof(pm_value)), sizeof(pm_value)); + } + return ret; } @@ -1257,7 +1275,7 @@ test_system_wide_paranoid(void) igt_drop_root(); - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); } @@ -1314,7 +1332,7 @@ test_invalid_oa_metric_set_id(void) /* Check that we aren't just seeing false positives... */ properties[ARRAY_SIZE(properties) - 1] = test_metric_set_id; - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); /* There's no valid default OA metric set ID... */ @@ -1348,7 +1366,7 @@ test_invalid_oa_format_id(void) /* Check that we aren't just seeing false positives... */ properties[ARRAY_SIZE(properties) - 1] = test_oa_format; - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); /* There's no valid default OA format... */ @@ -1512,7 +1530,7 @@ open_and_read_2_oa_reports(int format_id, .properties_ptr = to_user_pointer(properties), }; - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); read_2_oa_reports(format_id, exponent, oa_report0, oa_report1, timer_only); @@ -1916,7 +1934,7 @@ test_oa_exponents(void) oa_exponent_to_ns(exponent) / 1000.0, oa_exponent_to_ns(exponent) / (1000.0 * 1000.0)); - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, true /* prevent_pm */); /* Right after opening the OA stream, read a * first timestamp as way to filter previously @@ -2192,7 +2210,7 @@ test_invalid_oa_exponent(void) .properties_ptr = to_user_pointer(properties), }; - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); @@ -2246,7 +2264,7 @@ test_low_oa_exponent_permissions(void) igt_fork(child, 1) { igt_drop_root(); - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); } @@ -2312,7 +2330,7 @@ test_per_context_mode_unprivileged(void) properties[1] = ctx_id; - stream_fd = __perf_open(drm_fd, ¶m); + stream_fd = __perf_open(drm_fd, ¶m, false); __perf_close(stream_fd); drm_intel_gem_context_destroy(context); @@ -2401,7 +2419,7 @@ test_blocking(void) int64_t start, end; int n = 0; - stream_fd = __perf_open(drm_fd, ¶m); + st
[Intel-gfx] [PATCH i-g-t v4 06/12] tests/perf: rework oa-exponent test
New issues that were discovered while making the tests work on Gen8+ : - we need to measure timings between periodic reports and discard all other kind of reports - it seems periodicity of the reports can be affected outside of RC6 (frequency change), we can detect this by looking at the amount of clock cycles per timestamp deltas v2: Drop some unused variables (Matthew) Signed-off-by: Lionel Landwerlin --- tests/perf.c | 733 --- 1 file changed, 599 insertions(+), 134 deletions(-) diff --git a/tests/perf.c b/tests/perf.c index ca69440d..efaa4a06 100644 --- a/tests/perf.c +++ b/tests/perf.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -306,6 +307,25 @@ static uint32_t (*read_report_ticks)(uint32_t *report, static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t *oa_report1, enum drm_i915_oa_format format); +static bool +timestamp_delta_within(uint32_t delta, + uint32_t expected_delta, + uint32_t margin) +{ + return delta >= (expected_delta - margin) && + delta <= (expected_delta + margin); +} + +static bool +double_value_within(double value, + double expected, + double percent_margin) +{ + return value >= (expected - expected * percent_margin / 100.0) && + value <= (expected + expected * percent_margin / 100.0); + +} + static void __perf_close(int fd) { @@ -472,6 +492,20 @@ gen8_read_report_ticks(uint32_t *report, enum drm_i915_oa_format format) return report[3]; } +static void +gen8_read_report_clock_ratios(uint32_t *report, + uint32_t *slice_freq_mhz, + uint32_t *unslice_freq_mhz) +{ + uint32_t unslice_freq = report[0] & 0x1ff; + uint32_t slice_freq_low = (report[0] >> 25) & 0x7f; + uint32_t slice_freq_high = (report[0] >> 9) & 0x3; + uint32_t slice_freq = slice_freq_low | (slice_freq_high << 7); + + *slice_freq_mhz = (slice_freq * 1) / 1000; + *unslice_freq_mhz = (unslice_freq * 1) / 1000; +} + static const char * gen8_read_report_reason(const uint32_t *report) { @@ -494,29 +528,6 @@ gen8_read_report_reason(const uint32_t *report) return "unknown"; } -static bool -oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report) -{ - if (IS_HASWELL(devid)) { - /* For Haswell we don't have a documented report reason field -* (though empirically report[0] bit 10 does seem to correlate -* with a timer trigger reason) so we instead infer which -* reports are timer triggered by checking if the least -* significant bits are zero and the exponent bit is set. -*/ - uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1; - - if ((report[1] & oa_exponent_mask) != (1 << oa_exponent)) - return true; - } else { - if ((report[0] >> OAREPORT_REASON_SHIFT) & - OAREPORT_REASON_TIMER) - return true; - } - - return false; -} - static uint64_t timebase_scale(uint32_t u32_delta) { @@ -563,6 +574,29 @@ oa_exponent_to_ns(int exponent) return 10ULL * (2ULL << exponent) / timestamp_frequency; } +static bool +oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report) +{ + if (IS_HASWELL(devid)) { + /* For Haswell we don't have a documented report reason field +* (though empirically report[0] bit 10 does seem to correlate +* with a timer trigger reason) so we instead infer which +* reports are timer triggered by checking if the least +* significant bits are zero and the exponent bit is set. +*/ + uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1; + + if ((report[1] & oa_exponent_mask) == (1 << oa_exponent)) + return true; + } else { + if ((report[0] >> OAREPORT_REASON_SHIFT) & + OAREPORT_REASON_TIMER) + return true; + } + + return false; +} + static bool oa_report_ctx_is_valid(uint32_t *report) { @@ -578,6 +612,128 @@ oa_report_ctx_is_valid(uint32_t *report) igt_assert(!"reached"); } +static uint32_t +oa_report_get_ctx_id(uint32_t *report) +{ + if (!oa_report_ctx_is_valid(report)) + return 0x; + return report[2]; +} + +static double +oa_reports_tick_per_period(uint32_t *report0, uint32_t *report1) +{ + if (intel_gen(devid) < 8) + return 0.0; + + /* Measure the number GPU tick delta to timestamp delta. */ + return (double) (report1[3] - report0[3]) / +
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Discard the request queue if we fail to sleep before suspend
Chris Wilson writes: > If we fail to clear the outstanding request queue before suspending, > mark those requests as lost. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102037 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_gem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 9dc24b915aa7..37fbc64d9ffe 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4585,7 +4585,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) >* reset the GPU back to its idle, low power state. >*/ > WARN_ON(dev_priv->gt.awake); > - WARN_ON(!intel_engines_are_idle(dev_priv)); > + if (WARN_ON(!intel_engines_are_idle(dev_priv))) > + i915_gem_set_wedged(dev_priv); /* no hope, so reset everthing */ s/ever/every Reviewed-by: Mika Kuoppala > > /* >* Neither the BIOS, ourselves or any other kernel > -- > 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state.
Most code only cares about the current commit or previous commit. Fortunately we already have a place to track those. Move it to drm_crtc_state where it belongs. :) The per-crtc commit_list is kept for places where we have to look deeper than the current or previous commit for checking whether to stall on unpin. This is used in drm_atomic_helper_setup_commit and intel_has_pending_fb_unpin. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_atomic.c| 7 --- drivers/gpu/drm/drm_atomic_helper.c | 92 ++--- include/drm/drm_atomic.h| 1 - include/drm/drm_crtc.h | 9 4 files changed, 32 insertions(+), 77 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2fd383d7253a..2cce48f203e0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, state->crtcs[i].state); - if (state->crtcs[i].commit) { - kfree(state->crtcs[i].commit->event); - state->crtcs[i].commit->event = NULL; - drm_crtc_commit_put(state->crtcs[i].commit); - } - - state->crtcs[i].commit = NULL; state->crtcs[i].ptr = NULL; state->crtcs[i].state = NULL; } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 4e53aae9a1fb..9c2888cb4081 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, struct drm_atomic_state *old_state) { - struct drm_crtc_state *unused; + struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; int i; - for_each_new_crtc_in_state(old_state, crtc, unused, i) { - struct drm_crtc_commit *commit = old_state->crtcs[i].commit; + for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { + struct drm_crtc_commit *commit = new_crtc_state->commit; int ret; if (!commit) @@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; - struct drm_crtc_commit *commit; struct drm_plane *__plane, *plane = NULL; struct drm_plane_state *__plane_state, *plane_state = NULL; const struct drm_plane_helper_funcs *funcs; - int i, j, n_planes = 0; + int i, n_planes = 0; for_each_new_crtc_in_state(state, crtc, crtc_state, i) { if (drm_atomic_crtc_needs_modeset(crtc_state)) @@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, return -EINVAL; /* -* Don't do an async update if there is an outstanding commit modifying +* XXX: Don't do an async update if there is an outstanding commit modifying * the plane. This prevents our async update's changes from getting * overridden by a previous synchronous update's state. */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - if (plane->crtc != crtc) - continue; - - spin_lock(&crtc->commit_lock); - commit = list_first_entry_or_null(&crtc->commit_list, - struct drm_crtc_commit, - commit_entry); - if (!commit) { - spin_unlock(&crtc->commit_lock); - continue; - } - spin_unlock(&crtc->commit_lock); - - if (!crtc->state->state) - continue; - - for_each_plane_in_state(crtc->state->state, __plane, - __plane_state, j) { - if (__plane == plane) - return -EINVAL; - } - } return funcs->atomic_async_check(plane, plane_state); } @@ -1731,7 +1707,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, kref_init(&commit->ref); commit->crtc = crtc; - state->crtcs[i].commit = commit; + new_crtc_state->commit = commit; ret = stall_checks(crtc, nonblock); if (ret) @@ -1769,22 +1745,6 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_helper_setup_commit); - -static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc) -{ - str
[Intel-gfx] [PATCH 2/2] drm/atomic: Fix freeing connector/plane state too early by tracking commits
Currently we neatly track the crtc state, but forget to look at plane/connector state. When doing a nonblocking modeset, immediately followed by a setprop before the modeset completes, the setprop will see the modesets new state as the old state and free it. This has to be solved by waiting for hw_done on the connector, even if it's not assigned to a crtc. When a connector is unbound we take the last crtc commit, and when it stays unbound we create a new crtc commit for the connector that gets signaled on hw_done. We wait for it the same way as we do for crtc's, which will make sure we never run into a use-after-free situation. Signed-off-by: Maarten Lankhorst Testcase: kms_atomic_transition.plane-use-after-nonblocking-unbind* Cc: Laurent Pinchart --- drivers/gpu/drm/drm_atomic_helper.c | 171 ++-- include/drm/drm_connector.h | 7 ++ include/drm/drm_plane.h | 7 ++ 3 files changed, 179 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9c2888cb4081..a4fd500d6200 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1644,6 +1644,39 @@ static void release_crtc_commit(struct completion *completion) drm_crtc_commit_put(commit); } +static void init_commit(struct drm_crtc_commit *commit, struct drm_crtc *crtc) +{ + init_completion(&commit->flip_done); + init_completion(&commit->hw_done); + init_completion(&commit->cleanup_done); + INIT_LIST_HEAD(&commit->commit_entry); + kref_init(&commit->ref); + commit->crtc = crtc; +} + +static struct drm_crtc_commit * +init_or_ref_crtc_commit(struct drm_atomic_state *state, struct drm_crtc *crtc) +{ + struct drm_crtc_commit *commit; + + if (crtc) { + struct drm_crtc_state *new_crtc_state; + + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + + commit = new_crtc_state->commit; + drm_crtc_commit_get(commit); + } else { + commit = kzalloc(sizeof(*commit), GFP_KERNEL); + if (!commit) + return NULL; + + init_commit(commit, NULL); + } + + return commit; +} + /** * drm_atomic_helper_setup_commit - setup possibly nonblocking commit * @state: new modeset state to be committed @@ -1692,6 +1725,10 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_connector *conn; + struct drm_connector_state *old_conn_state, *new_conn_state; + struct drm_plane *plane; + struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_crtc_commit *commit; int i, ret; @@ -1700,12 +1737,7 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, if (!commit) return -ENOMEM; - init_completion(&commit->flip_done); - init_completion(&commit->hw_done); - init_completion(&commit->cleanup_done); - INIT_LIST_HEAD(&commit->commit_entry); - kref_init(&commit->ref); - commit->crtc = crtc; + init_commit(commit, crtc); new_crtc_state->commit = commit; @@ -1741,6 +1773,36 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state, drm_crtc_commit_get(commit); } + for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) { + if (new_conn_state->crtc) + continue; + + if (nonblock && old_conn_state->commit && + !try_wait_for_completion(&old_conn_state->commit->flip_done)) + return -EBUSY; + + commit = init_or_ref_crtc_commit(state, old_conn_state->crtc); + if (!commit) + return -ENOMEM; + + new_conn_state->commit = commit; + } + + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { + if (new_plane_state->crtc) + continue; + + if (nonblock && old_plane_state->commit && + !try_wait_for_completion(&old_plane_state->commit->flip_done)) + return -EBUSY; + + commit = init_or_ref_crtc_commit(state, old_plane_state->crtc); + if (!commit) + return -ENOMEM; + + new_plane_state->commit = commit; + } + return 0; } EXPORT_SYMBOL(drm_atomic_helper_setup_commit); @@ -1761,6 +1823,10 @@ void drm_atomic_helper_wait_for_dependencies(struct drm_atomic_state *old_state) { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; + struct drm_plane *p
[Intel-gfx] [PATCH 2/2] drm/i915/perf: Remove open-coding of i915_gem_switch_to_kernel_context()
The kernel context does not need to be updated for the oa config, since it is *never* used for anything but idling the device; it should never be required to emit OA samples. As such we can forgo tweaking the context image, and just do a plain switch to enforce the GPU barrier so that we can then update all other context images. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Joonas Lahtinen Cc: Lionel Landwerlin Cc: Matthew Auld \o/ --- drivers/gpu/drm/i915/i915_perf.c | 113 +-- 1 file changed, 1 insertion(+), 112 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 94185d610673..b44199726897 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1623,112 +1623,6 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, } } -/* - * Same as gen8_update_reg_state_unlocked only through the batchbuffer. This - * is only used by the kernel context. - */ -static int gen8_emit_oa_config(struct drm_i915_gem_request *req, - const struct i915_oa_config *oa_config) -{ - struct drm_i915_private *dev_priv = req->i915; - /* The MMIO offsets for Flex EU registers aren't contiguous */ - u32 flex_mmio[] = { - i915_mmio_reg_offset(EU_PERF_CNTL0), - i915_mmio_reg_offset(EU_PERF_CNTL1), - i915_mmio_reg_offset(EU_PERF_CNTL2), - i915_mmio_reg_offset(EU_PERF_CNTL3), - i915_mmio_reg_offset(EU_PERF_CNTL4), - i915_mmio_reg_offset(EU_PERF_CNTL5), - i915_mmio_reg_offset(EU_PERF_CNTL6), - }; - u32 *cs; - int i; - - cs = intel_ring_begin(req, ARRAY_SIZE(flex_mmio) * 2 + 4); - if (IS_ERR(cs)) - return PTR_ERR(cs); - - *cs++ = MI_LOAD_REGISTER_IMM(ARRAY_SIZE(flex_mmio) + 1); - - *cs++ = i915_mmio_reg_offset(GEN8_OACTXCONTROL); - *cs++ = (dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) | - (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) | - GEN8_OA_COUNTER_RESUME; - - for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) { - u32 mmio = flex_mmio[i]; - - /* -* This arbitrary default will select the 'EU FPU0 Pipeline -* Active' event. In the future it's anticipated that there -* will be an explicit 'No Event' we can select, but not -* yet... -*/ - u32 value = 0; - - if (oa_config) { - u32 j; - - for (j = 0; j < oa_config->flex_regs_len; j++) { - if (i915_mmio_reg_offset(oa_config->flex_regs[j].addr) == mmio) { - value = oa_config->flex_regs[j].value; - break; - } - } - } - - *cs++ = mmio; - *cs++ = value; - } - - *cs++ = MI_NOOP; - intel_ring_advance(req, cs); - - return 0; -} - -static int gen8_switch_to_updated_kernel_context(struct drm_i915_private *dev_priv, -const struct i915_oa_config *oa_config) -{ - struct intel_engine_cs *engine = dev_priv->engine[RCS]; - struct i915_gem_timeline *timeline; - struct drm_i915_gem_request *req; - int ret; - - lockdep_assert_held(&dev_priv->drm.struct_mutex); - - i915_gem_retire_requests(dev_priv); - - req = i915_gem_request_alloc(engine, dev_priv->kernel_context); - if (IS_ERR(req)) - return PTR_ERR(req); - - ret = gen8_emit_oa_config(req, oa_config); - if (ret) { - i915_add_request(req); - return ret; - } - - /* Queue this switch after all other activity */ - list_for_each_entry(timeline, &dev_priv->gt.timelines, link) { - struct drm_i915_gem_request *prev; - struct intel_timeline *tl; - - tl = &timeline->engine[engine->id]; - prev = i915_gem_active_raw(&tl->last_request, - &dev_priv->drm.struct_mutex); - if (prev) - i915_sw_fence_await_sw_fence_gfp(&req->submit, -&prev->submit, -GFP_KERNEL); - } - - ret = i915_switch_context(req); - i915_add_request(req); - - return ret; -} - /* * Manages updating the per-context aspects of the OA stream * configuration across all contexts. @@ -1771,11 +1665,6 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, mutex_lock(&dev_priv->drm.struct_mutex); } - /* Switch aw
[Intel-gfx] [PATCH 1/2] drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context()
All callers do want a synchronous switch to the kernel context, that is by the time the call returns, the GPU has evicted all user contexts and now has the kernel context pinned. As all callers want this behaviour, refactor the common wait-for-idle into the switch. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_gem.c | 6 -- drivers/gpu/drm/i915/i915_gem_context.c | 4 +++- drivers/gpu/drm/i915/i915_gem_evict.c | 14 +- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 890fe2802973..18ba74be286c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4564,12 +4564,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) if (ret) goto err_unlock; - ret = i915_gem_wait_for_idle(dev_priv, -I915_WAIT_INTERRUPTIBLE | -I915_WAIT_LOCKED); - if (ret) - goto err_unlock; - assert_kernel_context_is_current(dev_priv); i915_gem_contexts_lost(dev_priv); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 58a2a44f88bd..f70b05e682ac 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -924,7 +924,9 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv) return ret; } - return 0; + return i915_gem_wait_for_idle(dev_priv, +I915_WAIT_INTERRUPTIBLE | +I915_WAIT_LOCKED); } static bool client_is_banned(struct drm_i915_file_private *file_priv) diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 4df039ef2ce3..5cf73ad4801a 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -52,25 +52,13 @@ static bool ggtt_is_idle(struct drm_i915_private *dev_priv) static int ggtt_flush(struct drm_i915_private *i915) { - int err; - /* Not everything in the GGTT is tracked via vma (otherwise we * could evict as required with minimal stalling) so we are forced * to idle the GPU and explicitly retire outstanding requests in * the hopes that we can then remove contexts and the like only * bound by their active reference. */ - err = i915_gem_switch_to_kernel_context(i915); - if (err) - return err; - - err = i915_gem_wait_for_idle(i915, -I915_WAIT_INTERRUPTIBLE | -I915_WAIT_LOCKED); - if (err) - return err; - - return 0; + return i915_gem_switch_to_kernel_context(i915); } static bool -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for lib/igt_debugfs: Open DRM driver without master for hpd storm exit
== Series Details == Series: lib/igt_debugfs: Open DRM driver without master for hpd storm exit URL : https://patchwork.freedesktop.org/series/29470/ State : success == Summary == Test kms_atomic_transition: Subgroup plane-all-transition-fencing: skip -> PASS (shard-hsw) Test perf: Subgroup blocking: fail -> PASS (shard-hsw) fdo#102252 +1 Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 Test vgem_basic: Subgroup unload: skip -> PASS (shard-hsw) fdo#102453 Test kms_plane: Subgroup plane-panning-bottom-right-suspend-pipe-C-planes: skip -> PASS (shard-hsw) Subgroup plane-position-hole-dpms-pipe-C-planes: skip -> PASS (shard-hsw) Test kms_properties: Subgroup plane-properties-legacy: skip -> PASS (shard-hsw) fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 fdo#102453 https://bugs.freedesktop.org/show_bug.cgi?id=102453 shard-hswtotal:2230 pass:1232 dwarn:0 dfail:0 fail:17 skip:981 time:9575s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_117/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/23] mm/shmem: introduce shmem_file_setup_with_mnt
On Fri, 2017-08-25 at 13:49 -0700, Andrew Morton wrote: > On Thu, 24 Aug 2017 13:04:09 +0100 Matthew Auld > wrote: > > > On 23 August 2017 at 23:34, Andrew Morton wrote: > > > On Wed, 23 Aug 2017 12:31:28 +0300 Joonas Lahtinen > > > wrote: > > > > > > > This patch has been floating around for a while now Acked and without > > > > further comments. It is blocking us from merging huge page support to > > > > drm/i915. > > > > > > > > Would you mind merging it, or prodding the right people to get it in? > > > > > > > > Regards, Joonas > > > > > > > > On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > > > > > We are planning to use our own tmpfs mnt in i915 in place of the > > > > > shm_mnt, such that we can control the mount options, in particular > > > > > huge=, which we require to support huge-gtt-pages. So rather than roll > > > > > our own version of __shmem_file_setup, it would be preferred if we > > > > > could > > > > > just give shmem our mnt, and let it do the rest. > > > > > > hm, it's a bit odd. I'm having trouble locating the code which handles > > > huge=within_size (and any other options?). > > > > See here https://patchwork.freedesktop.org/patch/172771/, currently we > > only care about huge=within_size. > > > > > What other approaches were considered? > > > > We also tried https://patchwork.freedesktop.org/patch/156528/, where > > it was suggested that we mount our own tmpfs instance. > > > > Following from that we now have our own tmps mnt mounted with > > huge=within_size. With this patch we avoid having to roll our own > > __shmem_file_setup like in > > https://patchwork.freedesktop.org/patch/163024/. > > > > > Was it not feasible to add i915-specific mount options to > > > mm/shmem.c (for example?). > > > > Hmm, I think within_size should suffice for our needs. > > hm, ok, well, unless someone can think of something cleaner, please add > my ack and include it in the appropriate drm tree. Thanks, I will do that. It'll first get incorporated into drm-tip ( https://cgit.freedesktop.org/drm-tip) once the kselftests are finalized (now that we know we're not facing third rewrite for core MM dependency). And eventually into drm-next through a pull request to Dave Airlie. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Improve robustness of the i915 perf tests (rev2)
== Series Details == Series: Improve robustness of the i915 perf tests (rev2) URL : https://patchwork.freedesktop.org/series/28373/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3014 627598734e5e drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (fi-byt-j1900) fdo#101705 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:456s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:440s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:361s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:557s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:521s fi-byt-j1900 total:279 pass:255 dwarn:0 dfail:0 fail:0 skip:24 time:526s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:518s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:444s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:618s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:443s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:426s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:506s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:477s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:601s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:523s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:472s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:482s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:490s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:439s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:506s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:544s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:405s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_118/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state.
== Series Details == Series: series starting with [1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state. URL : https://patchwork.freedesktop.org/series/29476/ State : success == Summary == Series 29476v1 series starting with [1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state. https://patchwork.freedesktop.org/api/1.0/series/29476/revisions/1/mbox/ fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:455s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:444s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:573s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:527s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:529s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:526s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:436s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:618s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:448s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:424s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:426s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:516s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:601s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:473s fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:482s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:493s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:440s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:484s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:548s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:410s fi-pnv-d510 failed to connect after reboot 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest 73a398a3ba87 drm/atomic: Fix freeing connector/plane state too early by tracking commits 23c240dcae13 drm/atomic: Move drm_crtc_commit to drm_crtc_state. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5519/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t PATCH 00/10] tools/intel_vbt_decode: switch to using kernel intel_vbt_defs.h
On Tue, 29 Aug 2017, Daniel Vetter wrote: > On Mon, Aug 28, 2017 at 03:19:52PM +0300, Jani Nikula wrote: >> There's little point in duplicating the efforts of describing the same >> data in two places. This series lets us use the verbatim copy of the >> intel_vbt_defs.h from kernel. Going forward, we should add the changes >> in kernel first, then copy the header over to igt. >> >> If we need local tweaks, we can still have them in intel_bios.h, and >> indeed we'll still have some after this series. > > I assume the end result matches what we now have in the kernel. Might be > good practice to reference the sha1 of the kernel commit (needs to be a > stable sha1, not drm-tip ofc) when resyncing in the future. On the series: > > Acked-by: Daniel Vetter Pushed, with the kernel commit reference added. There'd be plenty to tweak and fix here, but the main goal of reducing duplication was reached, so that'll do for now. BR, Jani. >> >> BR, >> Jani. >> >> Jani Nikula (10): >> tools/intel_lid: use local register definition >> tools/intel_vbt_decode: remove unused definitions from intel_bios.h >> tools/intel_vbt_decode: clean up struct lvds_dvo_timing >> tools/intel_vbt_decode: start migrating to kernel intel_vbt_defs.h >> tools/intel_vbt_decode: migrate timing dumping to kernel struct >> tools/intel_vbt_decode: migrate child device dumping to kernel struct >> tools/intel_vbt_decode: migrate psr dumping to kernel struct >> tools/intel_vbt_decode: migrate edp dumping to kernel struct >> tools/intel_vbt_decode: migrate child device type bits decoding to >> kernel defs >> tools/intel_vbt_defs: migrate backlight dumping to kernel struct >> >> tools/intel_bios.h | 760 +-- >> tools/intel_lid.c| 5 +- >> tools/intel_vbt_decode.c | 180 +- >> tools/intel_vbt_defs.h | 897 >> +++ >> 4 files changed, 996 insertions(+), 846 deletions(-) >> create mode 100644 tools/intel_vbt_defs.h >> >> -- >> 2.11.0 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/23] drm/i915: introduce simple gemfs
On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so > moves us away from the shmemfs shm_mnt, and gives us the much needed > flexibility to do things like set our own mount options, namely huge= > which should allow us to enable the use of transparent-huge-pages for > our shmem backed objects. > > v2: various improvements suggested by Joonas > > v3: move gemfs instance to i915.mm and simplify now that we have > file_setup_with_mnt > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Chris Wilson > Cc: Dave Hansen > Cc: Kirill A. Shutemov > Cc: Hugh Dickins > Cc: linux...@kvack.org > @@ -4288,6 +4289,25 @@ static const struct drm_i915_gem_object_ops > i915_gem_object_ops = { > .pwrite = i915_gem_object_pwrite_gtt, > }; > > +static int i915_gem_object_create_shmem(struct drm_device *dev, > + struct drm_gem_object *obj, > + size_t size) > +{ > + struct drm_i915_private *i915 = to_i915(dev); > + struct file *filp; > + > + drm_gem_private_object_init(dev, obj, size); > + > + filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, > + VM_NORESERVE); Can you double-check that /proc/meminfo is unaffected by this change? If we stop appearing under "Shemem:" we maybe need to maybe highlight this somewhere (at least in commit message). > +int i915_gemfs_init(struct drm_i915_private *i915) > +{ > + struct file_system_type *type; > + struct vfsmount *gemfs; > + > + type = get_fs_type("tmpfs"); > + if (!type) > + return -ENODEV; > + > + gemfs = kern_mount(type); > + if (IS_ERR(gemfs)) > + return PTR_ERR(gemfs); By occasionally checking that "i915->mm.gemfs" might be NULL, could we continue without our own gemfs mount and just lose the additional features? Or is it not worth the hassle? Anyway, this is: Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/23] drm/i915: push set_pages down to the callers
On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > Each backend is now responsible for calling __i915_gem_object_set_pages > upon successfully gathering its backing storage. This eliminates the > inconsistency between the async and sync paths, which stands out even > more when we start throwing around an sg_mask in a later patch. > > Suggested-by: Chris Wilson > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Chris Wilson > @@ -2485,12 +2490,10 @@ static int i915_gem_object_get_pages(struct > drm_i915_gem_object *obj) > return -EFAULT; > } > > - pages = obj->ops->get_pages(obj); > - if (unlikely(IS_ERR(pages))) > - return PTR_ERR(pages); > + ret = obj->ops->get_pages(obj); > + GEM_BUG_ON(ret == 0 && IS_ERR_OR_NULL(obj->mm.pages)); !ret should be equally readable. Especially if you call the variable "err". Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/23] drm/i915/gemfs: enable THP
On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > Enable transparent-huge-pages through gemfs by mounting with > huge=within_size. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Chris Wilson > +++ b/drivers/gpu/drm/i915/i915_gemfs.c > @@ -24,6 +24,7 @@ > > #include > #include > +#include > > #include "i915_drv.h" > #include "i915_gemfs.h" > @@ -41,6 +42,20 @@ int i915_gemfs_init(struct drm_i915_private *i915) > if (IS_ERR(gemfs)) > return PTR_ERR(gemfs); > > + if (has_transparent_hugepage()) { > + struct super_block *sb = gemfs->mnt_sb; > + char options[] = "huge=within_size"; char const options[] ? > + int flags = 0; > + > + /* We don't consider failure to remount fatal, since this should > + * only ever attempt to modify the mount options of the sb, and > + * so should always leave us with a working mount upon failure. > + * Hence decoupling this from the actual kern_mount is probably > + * advisable. > + */ > + WARN_ON(sb->s_op->remount_fs(sb, &flags, options)); Not to have too many fallback paths, would it make sense for any error in setting up gemfs to result in NULL gemfs and fallback to tmpfs? With the string constified, this is: Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context()
== Series Details == Series: series starting with [1/2] drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context() URL : https://patchwork.freedesktop.org/series/29478/ State : success == Summary == Series 29478v1 series starting with [1/2] drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context() https://patchwork.freedesktop.org/api/1.0/series/29478/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: fail -> PASS (fi-snb-2600) fdo#100215 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:455s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:439s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:358s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:551s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:524s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:518s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:430s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:610s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:448s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:424s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:421s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:497s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:475s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:598s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:599s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:522s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:481s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:489s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:441s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:478s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:547s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:414s 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest a4f2c16aa596 drm/i915/perf: Remove open-coding of i915_gem_switch_to_kernel_context() 0046c657b34c drm/i915: Pull wait-for-idle into i915_gem_switch_to_kernel_context() == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5520/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Always wake the device to flush the GTT
On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote: > Since we hold the device wakeref when writing through the GTT (otherwise > the writes would fail), we presumed that before the device sleeps those > writes would naturally be flushed and that we wouldn't need our mmio > read trick. However, that presumption seems false and a sleepy bxt seems > to require us to always manually flush the GTT writes prior to direct > access. > > Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing > GTT writes") > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen Got any Bugzilla, Testcase, Tested-by? Does what the message describes. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Always wake the device to flush the GTT
Quoting Joonas Lahtinen (2017-08-29 15:54:06) > On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote: > > Since we hold the device wakeref when writing through the GTT (otherwise > > the writes would fail), we presumed that before the device sleeps those > > writes would naturally be flushed and that we wouldn't need our mmio > > read trick. However, that presumption seems false and a sleepy bxt seems > > to require us to always manually flush the GTT writes prior to direct > > access. > > > > Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before > > flushing GTT writes") > > Signed-off-by: Chris Wilson > > Cc: Joonas Lahtinen > > Got any Bugzilla, Testcase, Tested-by? Original bugzilla hasn't been reopened, so I its looks like they were happy enough with the original patches that fixed the problem on my bxt. The testcase seems to be very system dependent, my suspicion is that it has to do with the wacky runtime pm exhibited by CI bxt. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere
On Tue, Aug 29, 2017 at 10:07:18AM +0100, Chris Wilson wrote: > Quoting Jeff McGee (2017-08-28 21:18:44) > > On Mon, Aug 28, 2017 at 08:44:48PM +0100, Chris Wilson wrote: > > > Quoting jeff.mc...@intel.com (2017-08-28 20:25:30) > > > > From: Jeff McGee > > > > > > > > If someone else is resetting the engine we should clear our own bit as > > > > part of skipping that engine. Otherwise we will later believe that it > > > > has not been reset successfully and then trigger full gpu reset. If the > > > > other guy's reset actually fails, he will trigger the full gpu reset. > > > > > > The reason we did continue on to the global reset was to serialise > > > i915_handle_error() with the other thread. Not a huge issue, but a > > > reasonable property to keep -- and we definitely want a to explain why > > > only one reset at a time is important. > > > > > > bool intel_engine_lock_reset() { > > > if (!test_and_set_bit(I915_RESET_ENGINE + engine->id, > > > &engine->i915->gpu_error.flags)) > > > return true; > > > > > > intel_engine_wait_for_reset(engine); > > The current code doesn't wait for the other thread to finish the reset, but > > this would add that wait. > > Pardon? If we can't reset the engine, we go to the full reset which is > serialised, both with individual engine resets and other globals. > > > Did you intend that as an additional change to > > the current code? I don't think it is necessary. Each thread wants to > > reset some subset of engines, so it seems the thread can safely exit as soon > > as it knows each of those engines has been reset or is being reset as part > > of another thread that got the lock first. If any of the threads fail to > > reset an engine they "own", then full gpu reset is assured. > > It's unexpected for this function to return before the reset. > -Chris I'm a bit confused, so let's go back to the original code that I was trying to fix: /* * Try engine reset when available. We fall back to full reset if * single reset fails. */ if (intel_has_reset_engine(dev_priv)) { for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); if (test_and_set_bit(I915_RESET_ENGINE + engine->id, &dev_priv->gpu_error.flags)) continue; if (i915_reset_engine(engine, 0) == 0) engine_mask &= ~intel_engine_flag(engine); clear_bit(I915_RESET_ENGINE + engine->id, &dev_priv->gpu_error.flags); wake_up_bit(&dev_priv->gpu_error.flags, I915_RESET_ENGINE + engine->id); } } if (!engine_mask) goto out; /* Full reset needs the mutex, stop any other user trying to do so. */ Let's say that 2 threads are here intending to reset render. #1 gets the lock and starts the render engine-only reset. #2 fails to get the lock which implies that someone else is in the process of resetting the render engine (with single engine reset or full gpu reset). #2 continues on without waiting but doesn't clear the render bit in engine_mask. So #2 will proceed to initiate a full gpu reset when it may not be necessary. That's the problem I was trying to address with my initial patch. Do you agree that #2 must clear this bit to avoid always triggering full gpu reset? If the engine-only reset done by #1 fails, #1 will do the fallback to full gpu reset, so there is no risk that we would miss the full gpu reset if it is really needed. Then there is the question of whether #2 should wait around for the render engine reset by #1 to complete. It doesn't in current code and I don't see why it needs to. But that can be a separate discussion from the above. -Jeff ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] docs/chamelium: Explain that the Chamelium should only target one DUT
This adds an explanation about why the Chamelium should only be connected to one target device at once to the in-tree documentation. Signed-off-by: Paul Kocialkowski --- docs/chamelium.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/docs/chamelium.txt b/docs/chamelium.txt index 77594284..ae7ac34a 100644 --- a/docs/chamelium.txt +++ b/docs/chamelium.txt @@ -54,6 +54,12 @@ CI system with a shared testlist) to remove the Chamelium configuration from the hosts that shouldn't connect to the Chamelium so that they can be skipped, which is faster than a network timeout. +It should also be noted that each Chamelium platform should only be used for +testing a single target device at a time. This is because the reset call issued +by the IGT tests is common to all connectors and thus one machine running a test +on a given connector may reset the chamelium while another machine is running +a test on another connector. + An example fully-featured configuration follows: [Common] FrameDumpPath=/root/ -- 2.14.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for Improve robustness of the i915 perf tests (rev2)
== Series Details == Series: Improve robustness of the i915 perf tests (rev2) URL : https://patchwork.freedesktop.org/series/28373/ State : success == Summary == Test perf: Subgroup oa-exponents: fail -> PASS (shard-hsw) fdo#102254 Subgroup polling: fail -> PASS (shard-hsw) fdo#102252 Test kms_atomic_transition: Subgroup plane-all-transition-fencing: skip -> PASS (shard-hsw) Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 Test kms_plane: Subgroup plane-panning-bottom-right-suspend-pipe-C-planes: skip -> PASS (shard-hsw) Subgroup plane-position-hole-dpms-pipe-C-planes: skip -> PASS (shard-hsw) Test kms_properties: Subgroup plane-properties-legacy: skip -> PASS (shard-hsw) fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254 fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 shard-hswtotal:2231 pass:1231 dwarn:0 dfail:0 fail:17 skip:983 time:9558s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_118/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere
Quoting Jeff McGee (2017-08-29 16:04:17) > On Tue, Aug 29, 2017 at 10:07:18AM +0100, Chris Wilson wrote: > > Quoting Jeff McGee (2017-08-28 21:18:44) > > > On Mon, Aug 28, 2017 at 08:44:48PM +0100, Chris Wilson wrote: > > > > Quoting jeff.mc...@intel.com (2017-08-28 20:25:30) > > > > > From: Jeff McGee > > > > > > > > > > If someone else is resetting the engine we should clear our own bit as > > > > > part of skipping that engine. Otherwise we will later believe that it > > > > > has not been reset successfully and then trigger full gpu reset. If > > > > > the > > > > > other guy's reset actually fails, he will trigger the full gpu reset. > > > > > > > > The reason we did continue on to the global reset was to serialise > > > > i915_handle_error() with the other thread. Not a huge issue, but a > > > > reasonable property to keep -- and we definitely want a to explain why > > > > only one reset at a time is important. > > > > > > > > bool intel_engine_lock_reset() { > > > > if (!test_and_set_bit(I915_RESET_ENGINE + engine->id, > > > > &engine->i915->gpu_error.flags)) > > > > return true; > > > > > > > > intel_engine_wait_for_reset(engine); > > > The current code doesn't wait for the other thread to finish the reset, > > > but > > > this would add that wait. > > > > Pardon? If we can't reset the engine, we go to the full reset which is > > serialised, both with individual engine resets and other globals. > > > > > Did you intend that as an additional change to > > > the current code? I don't think it is necessary. Each thread wants to > > > reset some subset of engines, so it seems the thread can safely exit as > > > soon > > > as it knows each of those engines has been reset or is being reset as part > > > of another thread that got the lock first. If any of the threads fail to > > > reset an engine they "own", then full gpu reset is assured. > > > > It's unexpected for this function to return before the reset. > > -Chris > > I'm a bit confused, so let's go back to the original code that I was trying > to fix: > > > /* > * Try engine reset when available. We fall back to full reset if > * single reset fails. > */ > if (intel_has_reset_engine(dev_priv)) { > for_each_engine_masked(engine, dev_priv, engine_mask, tmp) { > BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); > if (test_and_set_bit(I915_RESET_ENGINE + engine->id, > &dev_priv->gpu_error.flags)) > continue; > > if (i915_reset_engine(engine, 0) == 0) > engine_mask &= ~intel_engine_flag(engine); > > clear_bit(I915_RESET_ENGINE + engine->id, > &dev_priv->gpu_error.flags); > wake_up_bit(&dev_priv->gpu_error.flags, > I915_RESET_ENGINE + engine->id); > } > } > > if (!engine_mask) > goto out; > > /* Full reset needs the mutex, stop any other user trying to do so. */ > > Let's say that 2 threads are here intending to reset render. #1 gets the lock > and starts the render engine-only reset. #2 fails to get the lock which > implies > that someone else is in the process of resetting the render engine (with > single > engine reset or full gpu reset). #2 continues on without waiting but doesn't > clear the render bit in engine_mask. So #2 will proceed to initiate a full > gpu reset when it may not be necessary. That's the problem I was trying > to address with my initial patch. Do you agree that #2 must clear this bit > to avoid always triggering full gpu reset? If the engine-only reset done by > #1 fails, #1 will do the fallback to full gpu reset, so there is no risk that > we would miss the full gpu reset if it is really needed. > > Then there is the question of whether #2 should wait around for the > render engine reset by #1 to complete. It doesn't in current code and I don't > see why it needs to. But that can be a separate discussion from the above. It very much does in the current code. If we can not do the per-engine reset, it falls back to the global reset. The global reset is serialised with itself and the per-engine resets. Ergo it waits, and that was intentional. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere
Quoting Jeff McGee (2017-08-28 20:46:00) > On Mon, Aug 28, 2017 at 12:41:58PM -0700, Michel Thierry wrote: > > On 28/08/17 12:25, jeff.mc...@intel.com wrote: > > >From: Jeff McGee > > > > > >If someone else is resetting the engine we should clear our own bit as > > >part of skipping that engine. Otherwise we will later believe that it > > >has not been reset successfully and then trigger full gpu reset. If the > > >other guy's reset actually fails, he will trigger the full gpu reset. > > > > > > > Did you hit this by manually setting wedged to 'x' ring repeatedly? > > > I haven't actually reproduced it. Have just been looking at the code a > lot to try to develop reset for preemption enforcement. The implementation > will call i915_handle_error from another work item that can run concurrent > with hangcheck. Note to hit it in practice is a nasty bug. The assumption is that between a pair of resets there was sufficient time for the engine to recover, and so if we reset too quickly we conclude that the reset/recovery mechanism is broken. And if you do start playing with fast resets, you very quickly find that kthread_park is a livelock waiting to happen. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for docs/chamelium: Explain that the Chamelium should only target one DUT
== Series Details == Series: docs/chamelium: Explain that the Chamelium should only target one DUT URL : https://patchwork.freedesktop.org/series/29482/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3014 627598734e5e drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:460s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:447s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:367s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:567s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:252s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:523s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:525s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:525s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:438s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:617s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:446s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:431s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:425s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:507s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:475s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:478s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:598s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:601s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:517s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:471s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:494s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:443s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:502s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:547s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:411s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_119/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] HAX drm/i915: Disable runtime-pm for shard-apl
--- drivers/gpu/drm/i915/i915_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index a1e6b696bcfa..9a8a2ec9fbcd 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -397,6 +397,7 @@ static const struct intel_device_info intel_skylake_gt3_info = { static const struct intel_device_info intel_broxton_info = { GEN9_LP_FEATURES, .platform = INTEL_BROXTON, + .has_runtime_pm = 0, .ddb_size = 512, }; -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/audio: Add suspend and hibernate tests for HDMI signal integrity
This introduces tests for HDMI signal integrity after suspend and hibernate. They simply test that signal integrity is ensured before and after suspend or hibernate. Signed-off-by: Paul Kocialkowski --- tests/audio.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/tests/audio.c b/tests/audio.c index 7fb91c97..560876a3 100644 --- a/tests/audio.c +++ b/tests/audio.c @@ -167,8 +167,27 @@ static void test_integrity(const char *device_name) free(data.alsa); } +static void test_suspend_resume_integrity(const char *device_name, + enum igt_suspend_state state, + enum igt_suspend_test test) +{ + test_integrity(device_name); + + igt_system_suspend_autoresume(state, test); + + test_integrity(device_name); +} + igt_main { igt_subtest("hdmi-integrity") test_integrity("HDMI"); + + igt_subtest("hdmi-integrity-after-suspend") + test_suspend_resume_integrity("HDMI", SUSPEND_STATE_MEM, + SUSPEND_TEST_NONE); + + igt_subtest("hdmi-integrity-after-hibernate") + test_suspend_resume_integrity("HDMI", SUSPEND_STATE_DISK, + SUSPEND_TEST_DEVICES); } -- 2.14.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Flush indirect GTT writes upon runtime-suspend
Our assumption is that indirect writes via the GTT are naturally flushed when we enter runtime suspend. However, from the look of bxt in our CI, this is not true and so we must apply our trick of doing a mmio to serialise the writes. Signed-off-by: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 43834dee4e8d..6b9352248925 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2023,6 +2023,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj) void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) { struct drm_i915_gem_object *obj, *on; + unsigned long flags; int i; /* @@ -2063,6 +2064,17 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) GEM_BUG_ON(!list_empty(®->vma->obj->userfault_link)); reg->dirty = true; } + + spin_lock_irqsave(&dev_priv->uncore.lock, flags); + POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base)); + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); + + list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) { + if (obj->base.read_domains & I915_GEM_DOMAIN_GTT) { + obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT; + obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT; + } + } } static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for HAX drm/i915: Disable runtime-pm for shard-apl
== Series Details == Series: HAX drm/i915: Disable runtime-pm for shard-apl URL : https://patchwork.freedesktop.org/series/29484/ State : warning == Summary == Series 29484v1 HAX drm/i915: Disable runtime-pm for shard-apl https://patchwork.freedesktop.org/api/1.0/series/29484/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100215 +1 Test kms_flip: Subgroup basic-flip-vs-modeset: skip -> PASS (fi-skl-x1585l) fdo#101781 Test pm_rpm: Subgroup basic-pci-d3-state: pass -> SKIP (fi-bxt-j4205) Subgroup basic-rte: pass -> SKIP (fi-bxt-j4205) fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:463s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:458s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:360s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:555s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:258 dwarn:0 dfail:0 fail:0 skip:21 time:520s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:514s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:442s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:613s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:446s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:422s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:503s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:483s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:597s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:596s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:520s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:469s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:493s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:446s fi-skl-x1585ltotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:504s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:556s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:403s fi-skl-6700k failed to connect after reboot 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest 531c23a24d7b HAX drm/i915: Disable runtime-pm for shard-apl == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5521/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state.
== Series Details == Series: series starting with [1/2] drm/atomic: Move drm_crtc_commit to drm_crtc_state. URL : https://patchwork.freedesktop.org/series/29476/ State : failure == Summary == Test perf: Subgroup blocking: fail -> PASS (shard-hsw) fdo#102252 Test kms_cursor_legacy: Subgroup nonblocking-modeset-vs-cursor-atomic: pass -> INCOMPLETE (shard-hsw) Subgroup long-nonblocking-modeset-vs-cursor-atomic: pass -> INCOMPLETE (shard-hsw) Test kms_atomic_transition: Subgroup plane-use-after-nonblocking-unbind: fail -> PASS (shard-hsw) fdo#101847 +1 Subgroup plane-all-transition-fencing: skip -> PASS (shard-hsw) Test kms_flip: Subgroup dpms-vs-vblank-race-interruptible: pass -> FAIL (shard-hsw) Test kms_busy: Subgroup extended-modeset-hang-oldfb-with-reset-render-B: pass -> INCOMPLETE (shard-hsw) Test kms_plane: Subgroup plane-panning-bottom-right-suspend-pipe-C-planes: skip -> PASS (shard-hsw) Subgroup plane-position-hole-dpms-pipe-C-planes: skip -> PASS (shard-hsw) Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 Test kms_properties: Subgroup plane-properties-legacy: skip -> PASS (shard-hsw) fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#101847 https://bugs.freedesktop.org/show_bug.cgi?id=101847 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 shard-hswtotal:2105 pass:1158 dwarn:0 dfail:0 fail:16 skip:928 time:8690s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5519/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Always wake the device to flush the GTT (rev3)
== Series Details == Series: drm/i915: Always wake the device to flush the GTT (rev3) URL : https://patchwork.freedesktop.org/series/29436/ State : success == Summary == Series 29436v3 drm/i915: Always wake the device to flush the GTT https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/3/mbox/ Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100215 +1 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:458s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:445s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:359s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:553s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:253s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:522s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:527s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:518s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:435s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:620s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:442s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:421s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:508s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:471s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:591s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:599s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:526s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:471s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:483s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:446s fi-skl-x1585ltotal:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:481s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:547s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:403s fi-skl-6700k failed to connect after reboot 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest aee73c805b40 drm/i915: Flush indirect GTT writes upon runtime-suspend == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5522/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Skylake plane update/disable unifications [v2]
== Series Details == Series: drm/i915: Skylake plane update/disable unifications [v2] URL : https://patchwork.freedesktop.org/series/29462/ State : warning == Summary == Series 29462v1 drm/i915: Skylake plane update/disable unifications [v2] https://patchwork.freedesktop.org/api/1.0/series/29462/revisions/1/mbox/ Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: fail -> PASS (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> SKIP (fi-skl-x1585l) fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:458s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:443s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:359s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:548s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:254s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:518s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:521s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:516s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:436s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:618s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:443s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:423s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:504s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:477s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:596s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:595s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:525s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:465s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:480s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:445s fi-skl-x1585ltotal:279 pass:267 dwarn:0 dfail:0 fail:0 skip:12 time:475s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:545s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:408s fi-skl-6700k failed to connect after reboot 627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest e2421d33d3d1 drm/i915: Unify skylake plane disable 50c7cd3e4795 drm/i915: Unify skylake plane update 6a622aad5fc6 drm/i915: dspaddr_offset doesn't need to be more than local variable == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5523/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for tests/audio: Add suspend and hibernate tests for HDMI signal integrity
== Series Details == Series: tests/audio: Add suspend and hibernate tests for HDMI signal integrity URL : https://patchwork.freedesktop.org/series/29485/ State : success == Summary == IGT patchset tested on top of latest successful build bf45d253648250fc402eee02237366c8882b2053 igt: Add gem_close with latest DRM-Tip kernel build CI_DRM_3014 627598734e5e drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100215 +1 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:456s fi-bdw-gvtdvmtotal:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:439s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:365s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:554s fi-bwr-2160 total:279 pass:184 dwarn:0 dfail:0 fail:0 skip:95 time:251s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:517s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:517s fi-elk-e7500 total:279 pass:230 dwarn:0 dfail:0 fail:0 skip:49 time:444s fi-glk-2atotal:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:611s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:445s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:426s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:430s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:506s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:482s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:600s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:599s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:521s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:476s fi-skl-6770hqtotal:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:491s fi-skl-gvtdvmtotal:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:445s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:553s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:403s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_120/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] drm/i915: add perf support for Coffeelake
Hi all, This series adds support for perf on Coffeelake GT2. This requires some changes in order to identify GT2s chipsets. It seems the scheme that was used before in device IDs isn't there anymore. Cheers, Lionel Landwerlin (3): drm/i915: add GT number to intel_device_info drm/i915: rework IS_*_GT* macros drm/i915/perf: add support for Coffeelake GT2 drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_drv.h | 17 +++--- drivers/gpu/drm/i915/i915_oa_cflgt2.c | 109 + drivers/gpu/drm/i915/i915_oa_cflgt2.h | 34 +++ drivers/gpu/drm/i915/i915_pci.c | 111 +++--- drivers/gpu/drm/i915/i915_perf.c | 5 ++ include/drm/i915_pciids.h | 110 ++--- 7 files changed, 310 insertions(+), 79 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt2.c create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt2.h -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx