Re: [Intel-gfx] [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl
Quoting Saarinen, Jani (2019-05-31 07:20:10) > Hi, > > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: torstai 30. toukokuuta 2019 18.23 > > To: Ceraolo Spurio, Daniele ; intel- > > g...@lists.freedesktop.org > > Cc: Ceraolo Spurio, Daniele ; Saarinen, > > Jani > > > > Subject: Re: [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl > > > > Quoting Daniele Ceraolo Spurio (2019-05-30 16:16:21) > > > Accidentally added during the merge of drm-next. > > > > It's a dim issue. A dinq patch cherry-picked into dif that git isn't > > eliminating the > > duplication when dim build tips. > > https://drm.pages.freedesktop.org/maintainer-tools/drm-tip.html#resolving- > > conflicts-when-rebuilding-drm-tip > > See "Fixing Silent Conflicts" > > > > Note already fixed up. > When this can be seen also in CI? When somebody restarts CI. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl
Hi, > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: torstai 30. toukokuuta 2019 18.23 > To: Ceraolo Spurio, Daniele ; intel- > g...@lists.freedesktop.org > Cc: Ceraolo Spurio, Daniele ; Saarinen, Jani > > Subject: Re: [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl > > Quoting Daniele Ceraolo Spurio (2019-05-30 16:16:21) > > Accidentally added during the merge of drm-next. > > It's a dim issue. A dinq patch cherry-picked into dif that git isn't > eliminating the > duplication when dim build tips. > https://drm.pages.freedesktop.org/maintainer-tools/drm-tip.html#resolving- > conflicts-when-rebuilding-drm-tip > See "Fixing Silent Conflicts" > > Note already fixed up. When this can be seen also in CI? > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/ehl: Update MOCS table for EHL
EHL defines two new MOCS table entries but is otherwise compatible with the ICL MOCS table. These table entries (16 and 17) should still be considered unused for ICL and as such their behavior remains undefined for that platform. Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_mocs.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c index 79df66022d3a..1f9db50b1869 100644 --- a/drivers/gpu/drm/i915/gt/intel_mocs.c +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c @@ -200,6 +200,14 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { MOCS_ENTRY(15, \ LE_3_WB | LE_TC_1_LLC | LE_LRUM(2) | LE_AOM(1), \ L3_3_WB), \ + /* Bypass LLC - Uncached (EHL+) */ \ + MOCS_ENTRY(16, \ + LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \ + L3_1_UC), \ + /* Bypass LLC - L3 (Read-Only) (EHL+) */ \ + MOCS_ENTRY(17, \ + LE_1_UC | LE_TC_1_LLC | LE_SCF(1), \ + L3_3_WB), \ /* Self-Snoop - L3 + LLC */ \ MOCS_ENTRY(18, \ LE_3_WB | LE_TC_1_LLC | LE_LRUM(3) | LE_SSE(3), \ -- 2.14.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
On Thu, May 30, 2019 at 12:33:40PM -0700, Almahallawy, Khaled wrote: > On Wed, 2019-05-22 at 12:25 -0700, Manasi Navare wrote: > > On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote: > > > On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote: > > > > According to DP 1.4 standard, if the source supports four pre- > > > > emphasis levels, then the source shall set the bit MAX_PRE- > > > > EMPHASIS_REACHED = 1 only when trasmitter programmed PRE- > > > > EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis > > > > level 3 is the maximum pre-emphasis level that the source > > > > supports. > > > > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the > > > > Max Pre-Emphasis level for certain Swing Level. This > > > > interpretation fails Link Layer compliance test 400.3.1.15 step > > > > 17 according to the following Fail condition: > > > > TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active > > > > lanes) and the Source DUT supports pre-emphasis level 3 (9.5db). > > > > > > Hmm. I guess that's correct. The spec doesn't say anything about > > > per-vswing pre-emphasis when talking about the 'max reached' bit. > > > > > > > > > > > Cc: Clint Taylor > > > > Cc: Manasi Navare > > > > Signed-off-by: Khaled Almahallawy > > > > --- > > > > drivers/gpu/drm/i915/intel_ddi.c | 20 > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +- > > > > 2 files changed, 1 insertion(+), 21 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > > index 0af47f343faa..6540c979c098 100644 > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct > > > > intel_encoder *encoder) > > > > DP_TRAIN_VOLTAGE_SWING_MASK; > > > > } > > > > > > > > -/* > > > > - * We assume that the full set of pre-emphasis values can be > > > > - * used on all DDI platforms. Should that change we need to > > > > - * rethink this code. > > > > - */ > > > > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, > > > > u8 voltage_swing) > > > > -{ > > > > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) { > > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0: > > > > - return DP_TRAIN_PRE_EMPH_LEVEL_3; > > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1: > > > > - return DP_TRAIN_PRE_EMPH_LEVEL_2; > > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2: > > > > - return DP_TRAIN_PRE_EMPH_LEVEL_1; > > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3: > > > > - default: > > > > - return DP_TRAIN_PRE_EMPH_LEVEL_0; > > > > - } > > > > -} > > > > - > > > > static void cnl_ddi_vswing_program(struct intel_encoder > > > > *encoder, > > > >int level, enum intel_output_type > > > > type) > > > > { > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > index 77ba4da6b981..f94759e45862 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp > > > > *intel_dp, u8 voltage_swing) > > > > enum port port = encoder->port; > > > > > > > > if (HAS_DDI(dev_priv)) { > > > > - return intel_ddi_dp_pre_emphasis_max(encoder, > > > > voltage_swing); > > > > + return DP_TRAIN_PRE_EMPH_LEVEL_3; > > > > > > We're going to have to change this for all platforms. > > Yes, I'm going to change for all platforms in intel_dp_pre_emphasis_max > function. I will also add the missing condition: > else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) > similar to intel_dp_voltage_max function > > > > > > > Also we need to update the code to pick the correct swing/pre- > > > emphasis > > > when we can't do what is being requested. > > > > This check will need to be added in adjust_train() function > > sure, I will implement this logic in intel_get_adjust_train > > > > > > > > The spec says: > > > "When the combination of the requested pre-emphasis level and > > > voltage > > > swing exceeds the capability of a DPTX, the DPTX shall set the > > > pre-emphasis > > > level according to the request and use the highest voltage swing > > > it can > > > output with the given pre-emphasis level." > > > and > > > "When a DPTX reads a request beyond the limits of this Standard, > > > the > > > DPTX shall set the pre-emphasis level according to the request and > > > set > > > the highest voltage swing level it can output with the given pre- > > > emphasis > > > level. If a DPTX is requested for 9.5dB of pre-emphasis level (may > > > be > > > supported for a DPTX) and cannot support that level, it shall set > > > the > > > pre-emphasis level to the next highest level, 6dB." > > > > So my interpretation o
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Track the purgeable objects on a separate eviction list
== Series Details == Series: series starting with [1/2] drm/i915: Track the purgeable objects on a separate eviction list URL : https://patchwork.freedesktop.org/series/61405/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915: Track the purgeable objects on a separate eviction list Okay! Commit: drm/i915: Report all objects with allocated pages to the shrinker -./include/linux/mm.h:652:13: error: not a function ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list
Currently the purgeable objects, I915_MADV_DONTNEED, are mixed in the normal bound/unbound lists. Every shrinker pass starts with an attempt to purge from this set of unneeded objects, which entails us doing a walk over both lists looking for any candidates. If there are none, and since we are shrinking we can reasonably assume that the lists are full!, this becomes a very slow futile walk. If we separate out the purgeable objects into own list, this search then becomes its own phase that is preferentially handled during shrinking. Instead the cost becomes that we then need to filter the purgeable list if we want to distinguish between bound and unbound objects. Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Matthew Auld Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 14 - drivers/gpu/drm/i915/gem/i915_gem_object.c | 11 +- drivers/gpu/drm/i915/gem/i915_gem_pm.c | 1 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 4 +--- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 22 +--- drivers/gpu/drm/i915/i915_drv.h | 16 -- drivers/gpu/drm/i915/i915_gem.c | 20 -- drivers/gpu/drm/i915/i915_vma.c | 3 ++- 8 files changed, 61 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index cce96e6c6e52..52b73e90c9f4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -462,7 +462,6 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj) { struct drm_i915_private *i915 = to_i915(obj->base.dev); - struct list_head *list; struct i915_vma *vma; GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj)); @@ -476,10 +475,15 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj) } mutex_unlock(&i915->ggtt.vm.mutex); - spin_lock(&i915->mm.obj_lock); - list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list; - list_move_tail(&obj->mm.link, list); - spin_unlock(&i915->mm.obj_lock); + if (obj->mm.madv == I915_MADV_WILLNEED) { + struct list_head *list; + + spin_lock(&i915->mm.obj_lock); + list = obj->bind_count ? + &i915->mm.bound_list : &i915->mm.unbound_list; + list_move_tail(&obj->mm.link, list); + spin_unlock(&i915->mm.obj_lock); + } } void diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index f064876f1214..a42763e4dd5f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -334,9 +334,18 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->mm.quirked) __i915_gem_object_unpin_pages(obj); - if (discard_backing_storage(obj)) + if (discard_backing_storage(obj)) { + struct drm_i915_private *i915 = to_i915(obj->base.dev); + obj->mm.madv = I915_MADV_DONTNEED; + if (i915_gem_object_has_pages(obj)) { + spin_lock(&i915->mm.obj_lock); + list_move_tail(&obj->mm.link, &i915->mm.purge_list); + spin_unlock(&i915->mm.obj_lock); + } + } + /* * Before we free the object, make sure any pure RCU-only * read-side critical sections are complete, e.g. diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index 11890e96ed65..89bb6d822f6e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -164,6 +164,7 @@ void i915_gem_suspend_late(struct drm_i915_private *i915) struct list_head *phases[] = { &i915->mm.unbound_list, &i915->mm.bound_list, + &i915->mm.purge_list, NULL }, **phase; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 665f22ebf8e8..19d9ecdb2894 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -80,9 +80,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj) sg_page_sizes = 0; for (i = 0; i < page_count; i++) { const unsigned int shrink[] = { - (I915_SHRINK_BOUND | -I915_SHRINK_UNBOUND | -I915_SHRINK_PURGEABLE), + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND, 0, }, *s = shrink; gfp_t gfp = noreclaim; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/dr
[Intel-gfx] [PATCH 2/2] drm/i915: Report all objects with allocated pages to the shrinker
Currently, we try to report to the shrinker the precise number of objects (pages) that are available to be reaped at this moment. This requires searching all objects with allocated pages to see if they fulfill the search criteria, and this count is performed quite frequently. (The shrinker tries to free ~128 pages on each invocation, before which we count all the objects; counting takes longer than unbinding the objects!) If we take the pragmatic view that with sufficient desire, all objects are eventually reapable (they become inactive, or no longer used as framebuffer etc), we can simply return the count of pinned pages maintained during get_pages/put_pages rather than walk the lists every time. The downside is that we may (slightly) over-report the number of objects/pages we could shrink and so penalize ourselves by shrinking more than required. This is mitigated by keeping the order in which we shrink objects such that we avoid penalizing active and frequently used objects, and if memory is so tight that we need to free them we would need to anyway. v2: Only expose shrinkable objects to the shrinker; a small reduction in not considering stolen and foreign objects. v3: Restore the tracking from a "backup" copy from before the gem/ split Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 33 +++ drivers/gpu/drm/i915/gem/i915_gem_pages.c| 20 +-- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 28 ++ drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +- drivers/gpu/drm/i915/i915_debugfs.c | 58 ++-- drivers/gpu/drm/i915/i915_drv.h | 7 +-- drivers/gpu/drm/i915/i915_gem.c | 23 drivers/gpu/drm/i915/i915_vma.c | 16 -- 9 files changed, 63 insertions(+), 128 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index 52b73e90c9f4..e5deae62681f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -475,7 +475,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj) } mutex_unlock(&i915->ggtt.vm.mutex); - if (obj->mm.madv == I915_MADV_WILLNEED) { + if (i915_gem_object_is_shrinkable(obj) && + obj->mm.madv == I915_MADV_WILLNEED) { struct list_head *list; spin_lock(&i915->mm.obj_lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index a42763e4dd5f..49c959c11b2a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -44,25 +44,6 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj) return kmem_cache_free(global.slab_objects, obj); } -/* some bookkeeping */ -static void i915_gem_info_add_obj(struct drm_i915_private *i915, - u64 size) -{ - spin_lock(&i915->mm.object_stat_lock); - i915->mm.object_count++; - i915->mm.object_memory += size; - spin_unlock(&i915->mm.object_stat_lock); -} - -static void i915_gem_info_remove_obj(struct drm_i915_private *i915, -u64 size) -{ - spin_lock(&i915->mm.object_stat_lock); - i915->mm.object_count--; - i915->mm.object_memory -= size; - spin_unlock(&i915->mm.object_stat_lock); -} - static void frontbuffer_retire(struct i915_active_request *active, struct i915_request *request) @@ -98,8 +79,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, obj->mm.madv = I915_MADV_WILLNEED; INIT_RADIX_TREE(&obj->mm.get_page.radix, GFP_KERNEL | __GFP_NOWARN); mutex_init(&obj->mm.get_page.lock); - - i915_gem_info_add_obj(to_i915(obj->base.dev), obj->base.size); } /** @@ -163,11 +142,14 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file) static bool discard_backing_storage(struct drm_i915_gem_object *obj) { - /* If we are the last user of the backing storage (be it shmemfs + /* +* If we are the last user of the backing storage (be it shmemfs * pages or stolen etc), we know that the pages are going to be * immediately released. In this case, we can then skip copying * back the contents from the GPU. */ + if (!i915_gem_object_is_shrinkable(obj)) + return false; if (obj->mm.madv != I915_MADV_WILLNEED) return false; @@ -208,13 +190,15 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, GEM_BUG_ON(!list_empty(&obj->vma.list)); GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree)); - /* This serializes freeing with the shrinker. Since the free + /* +
Re: [Intel-gfx] [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
On Wed, 2019-05-22 at 12:25 -0700, Manasi Navare wrote: > On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote: > > On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote: > > > According to DP 1.4 standard, if the source supports four pre- > > > emphasis levels, then the source shall set the bit MAX_PRE- > > > EMPHASIS_REACHED = 1 only when trasmitter programmed PRE- > > > EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis > > > level 3 is the maximum pre-emphasis level that the source > > > supports. > > > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the > > > Max Pre-Emphasis level for certain Swing Level. This > > > interpretation fails Link Layer compliance test 400.3.1.15 step > > > 17 according to the following Fail condition: > > > TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active > > > lanes) and the Source DUT supports pre-emphasis level 3 (9.5db). > > > > Hmm. I guess that's correct. The spec doesn't say anything about > > per-vswing pre-emphasis when talking about the 'max reached' bit. > > > > > > > > Cc: Clint Taylor > > > Cc: Manasi Navare > > > Signed-off-by: Khaled Almahallawy > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 20 > > > drivers/gpu/drm/i915/intel_dp.c | 2 +- > > > 2 files changed, 1 insertion(+), 21 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > index 0af47f343faa..6540c979c098 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct > > > intel_encoder *encoder) > > > DP_TRAIN_VOLTAGE_SWING_MASK; > > > } > > > > > > -/* > > > - * We assume that the full set of pre-emphasis values can be > > > - * used on all DDI platforms. Should that change we need to > > > - * rethink this code. > > > - */ > > > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, > > > u8 voltage_swing) > > > -{ > > > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) { > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0: > > > - return DP_TRAIN_PRE_EMPH_LEVEL_3; > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1: > > > - return DP_TRAIN_PRE_EMPH_LEVEL_2; > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2: > > > - return DP_TRAIN_PRE_EMPH_LEVEL_1; > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3: > > > - default: > > > - return DP_TRAIN_PRE_EMPH_LEVEL_0; > > > - } > > > -} > > > - > > > static void cnl_ddi_vswing_program(struct intel_encoder > > > *encoder, > > > int level, enum intel_output_type > > > type) > > > { > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index 77ba4da6b981..f94759e45862 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp > > > *intel_dp, u8 voltage_swing) > > > enum port port = encoder->port; > > > > > > if (HAS_DDI(dev_priv)) { > > > - return intel_ddi_dp_pre_emphasis_max(encoder, > > > voltage_swing); > > > + return DP_TRAIN_PRE_EMPH_LEVEL_3; > > > > We're going to have to change this for all platforms. Yes, I'm going to change for all platforms in intel_dp_pre_emphasis_max function. I will also add the missing condition: else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) similar to intel_dp_voltage_max function > > > > Also we need to update the code to pick the correct swing/pre- > > emphasis > > when we can't do what is being requested. > > This check will need to be added in adjust_train() function sure, I will implement this logic in intel_get_adjust_train > > > > > The spec says: > > "When the combination of the requested pre-emphasis level and > > voltage > > swing exceeds the capability of a DPTX, the DPTX shall set the > > pre-emphasis > > level according to the request and use the highest voltage swing > > it can > > output with the given pre-emphasis level." > > and > > "When a DPTX reads a request beyond the limits of this Standard, > > the > > DPTX shall set the pre-emphasis level according to the request and > > set > > the highest voltage swing level it can output with the given pre- > > emphasis > > level. If a DPTX is requested for 9.5dB of pre-emphasis level (may > > be > > supported for a DPTX) and cannot support that level, it shall set > > the > > pre-emphasis level to the next highest level, 6dB." > > So my interpretation of this is : > > In adjust_train() function: > > vswing_max = intel_dp_voltage_max() which is set per platform > pre_emphasis_max = set to level 3 pre_emphasis_max = intel_dp_pre_emphasis_max because it is set per platfrom as well, similar to vswing_max > > v = get_requested_voltage_swing() - Limit this to vmax > p = get_requested_pre_emphasis() - Limit this to pmax > > Now rewrite t
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Report all objects with allocated pages to the shrinker
Quoting Matthew Auld (2019-05-30 17:10:16) > On Tue, 28 May 2019 at 20:50, Chris Wilson wrote: > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index fb2e89133e78..770c54b87de6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -926,10 +926,9 @@ struct i915_gem_mm { > > /** Bit 6 swizzling required for Y tiling */ > > u32 bit_6_swizzle_y; > > > > - /* accounting, useful for userland debugging */ > > - spinlock_t object_stat_lock; > > - u64 object_memory; > > - u32 object_count; > > + /* shrinker accounting, also useful for userland debugging */ > > + u64 shrink_memory; > > + u32 shrink_count; > > Colour me confused. I can't see where we set these? Or is my brain fried? They used to be set on add/removing obj->mm.pages... That looks to have vanished. I wonder if I have an older version to save me the hassle of adding two functions :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: add a new perf configuration execbuf parameter
On 28/05/2019 11:52, Chris Wilson wrote: Quoting Lionel Landwerlin (2019-05-21 15:08:54) @@ -2048,6 +2081,42 @@ static int eb_submit(struct i915_execbuffer *eb) return err; } if (eb->oa_config) { err = i915_active_request_set(&eb->i915->perf.oa.oa_config_active, eb->request); if (err) return err; } with the addition of struct i915_active_request oa_config_active; to i915->perf.oa, and i915_active_init; That will ensure that the oa_config can't be changed before execution (and the ordering restriction is essentially a no-op if only one context has a specified oa_config). + if (eb->oa_config && + eb->oa_config != eb->i915->perf.oa.exclusive_stream->oa_config) { Fwiw, I would move these to eb_oa_config(). if (eb->oa_config) { err = eb_oa_config(eb); if (err) return err; } How does eb_oa_config mix with the global gen8_configure_all_contexts()? Excellent point, I should document this. HW configurations have roughly 3 parts : - NOA configuration, network configuration to source specific data from anywhere (global, non power saved/restored) - Boolean counters, filters signals brought by NOA (global, can't remember whether saved/restored) - Flex counters, filters on events happening within the EUs (per context, saved/restored) First two will affect all running contexts (because global), last one will only be applied to the context that triggered the execbuf. That should be fine because of the other requirement we need (don't preempt the context running a performance query). -Lionel -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Report all objects with allocated pages to the shrinker
On Tue, 28 May 2019 at 20:50, Chris Wilson wrote: > > Currently, we try to report to the shrinker the precise number of > objects (pages) that are available to be reaped at this moment. This > requires searching all objects with allocated pages to see if they > fulfill the search criteria, and this count is performed quite > frequently. (The shrinker tries to free ~128 pages on each invocation, > before which we count all the objects; counting takes longer than > unbinding the objects!) If we take the pragmatic view that with > sufficient desire, all objects are eventually reapable (they become > inactive, or no longer used as framebuffer etc), we can simply return > the count of pinned pages maintained during get_pages/put_pages rather > than walk the lists every time. > > The downside is that we may (slightly) over-report the number of > objects/pages we could shrink and so penalize ourselves by shrinking > more than required. This is mitigated by keeping the order in which we > shrink objects such that we avoid penalizing active and frequently used > objects, and if memory is so tight that we need to free them we would > need to anyway. > > v2: Only expose shrinkable objects to the shrinker; a small reduction in > not considering stolen and foreign objects. > > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > Cc: Matthew Auld > --- > drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 22 > drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 28 ++ > drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 3 +- > drivers/gpu/drm/i915/i915_debugfs.c | 58 ++-- > drivers/gpu/drm/i915/i915_drv.h | 7 +-- > drivers/gpu/drm/i915/i915_gem.c | 23 > drivers/gpu/drm/i915/i915_vma.c | 16 -- > 8 files changed, 41 insertions(+), 119 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c > b/drivers/gpu/drm/i915/gem/i915_gem_domain.c > index 52b73e90c9f4..e5deae62681f 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c > @@ -475,7 +475,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct > drm_i915_gem_object *obj) > } > mutex_unlock(&i915->ggtt.vm.mutex); > > - if (obj->mm.madv == I915_MADV_WILLNEED) { > + if (i915_gem_object_is_shrinkable(obj) && > + obj->mm.madv == I915_MADV_WILLNEED) { > struct list_head *list; > > spin_lock(&i915->mm.obj_lock); > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c > b/drivers/gpu/drm/i915/gem/i915_gem_object.c > index f064876f1214..1fccb1de5851 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > @@ -44,25 +44,6 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj) > return kmem_cache_free(global.slab_objects, obj); > } > > -/* some bookkeeping */ > -static void i915_gem_info_add_obj(struct drm_i915_private *i915, > - u64 size) > -{ > - spin_lock(&i915->mm.object_stat_lock); > - i915->mm.object_count++; > - i915->mm.object_memory += size; > - spin_unlock(&i915->mm.object_stat_lock); > -} > - > -static void i915_gem_info_remove_obj(struct drm_i915_private *i915, > -u64 size) > -{ > - spin_lock(&i915->mm.object_stat_lock); > - i915->mm.object_count--; > - i915->mm.object_memory -= size; > - spin_unlock(&i915->mm.object_stat_lock); > -} > - > static void > frontbuffer_retire(struct i915_active_request *active, >struct i915_request *request) > @@ -98,8 +79,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, > obj->mm.madv = I915_MADV_WILLNEED; > INIT_RADIX_TREE(&obj->mm.get_page.radix, GFP_KERNEL | __GFP_NOWARN); > mutex_init(&obj->mm.get_page.lock); > - > - i915_gem_info_add_obj(to_i915(obj->base.dev), obj->base.size); > } > > /** > @@ -240,7 +219,6 @@ static void __i915_gem_free_objects(struct > drm_i915_private *i915, > > reservation_object_fini(&obj->__builtin_resv); > drm_gem_object_release(&obj->base); > - i915_gem_info_remove_obj(i915, obj->base.size); > > bitmap_free(obj->bit_17); > i915_gem_object_free(obj); > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > index 6a93e326abf3..d71e630c6fb8 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > @@ -309,30 +309,14 @@ i915_gem_shrinker_count(struct shrinker *shrinker, > struct shrink_control *sc) > { > struct drm_i915_private *i915 = > container_of(shrinker, struct drm_i915_private, mm.shrinker); > - struct drm_i915_gem_object *obj; > - unsigne
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: remove duplicated WaDisableBankHangMode:icl
== Series Details == Series: drm/i915: remove duplicated WaDisableBankHangMode:icl URL : https://patchwork.freedesktop.org/series/61396/ State : failure == Summary == Applying: drm/i915: remove duplicated WaDisableBankHangMode:icl Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/gt/intel_workarounds.c Falling back to patching base and 3-way merge... No changes -- Patch already applied. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Track the purgeable objects on a separate eviction list
On Tue, 28 May 2019 at 20:50, Chris Wilson wrote: > > Currently the purgeable objects, I915_MADV_DONTNEED, as mixed in the > normal bound/unbound lists. Every shrinker pass starts with an attempt > to purge from this set of unneeded objects, which entails us doing a > walk over both lists looking for any candidates. If there are none, and > since we are shrinking we can reasonably assume that the lists are > full!, this becomes a very slow futile walk. > > If we separate out the purgeable objects into own list, this search then > becomes its own phase that is preferentially handled during shrinking. > Instead the cost becomes that we then need to filter the purgeable list > if we want to distinguish between bound and unbound objects. > > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > Cc: Matthew Auld Makes sense, Reviewed-by: Matthew Auld ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl
Quoting Daniele Ceraolo Spurio (2019-05-30 16:16:21) > Accidentally added during the merge of drm-next. It's a dim issue. A dinq patch cherry-picked into dif that git isn't eliminating the duplication when dim build tips. https://drm.pages.freedesktop.org/maintainer-tools/drm-tip.html#resolving-conflicts-when-rebuilding-drm-tip See "Fixing Silent Conflicts" Note already fixed up. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: remove duplicated WaDisableBankHangMode:icl
Accidentally added during the merge of drm-next. Fixes: 7126b65091c4 ("Merge remote-tracking branch 'drm/drm-next' into drm-tip") Reported-by: Jani Saarinen Signed-off-by: Daniele Ceraolo Spurio Cc: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index dcfa6ca09f7b..133d069244f4 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -530,12 +530,6 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, { struct drm_i915_private *i915 = engine->i915; - /* WaDisableBankHangMode:icl */ - wa_write(wal, -GEN8_L3CNTLREG, -intel_uncore_read(engine->uncore, GEN8_L3CNTLREG) | -GEN8_ERRDETBCTRL); - /* WaDisableBankHangMode:icl */ wa_write(wal, GEN8_L3CNTLREG, -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
On 5/30/19 1:29 AM, Saarinen, Jani wrote: Hi, -Original Message- From: Summers, Stuart Sent: keskiviikko 29. toukokuuta 2019 19.02 To: Saarinen, Jani ; Ceraolo Spurio, Daniele ; Navare, Manasi D Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote: On 5/28/19 11:48 PM, Saarinen, Jani wrote: Hi, -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Summers, Stuart Sent: tiistai 28. toukokuuta 2019 21.33 To: Navare, Manasi D Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote: Pushed to dinq, thanks for the patches and the reviews! Thanks for the push Manasi and the reviews Daniele and others! This broke all the ICL systems because CI data was not looked that they did not actually even boot at all. All ICL's in BAT and whole ICL shards. Can we change the CI reply for the case where there are extra missing machines compared to the reference run from SUCCESS to WARNING or something like that, so people have a clearer indication that something might have gone wrong? I agree here. I'm sure with time and experience these types of things will get easier to parse, but this was very unobvious to me when posting. I have no problem reworking, but would really appreciate a solution to this from the CI side to ensure we don't hit this type of thing in the future. Sure, CI team already discussed on this. But going forward. Can you fix this still today that ICL's systems are green not orange on ci-grid. So would be good to get to the state that was on CI_DRM_6158. Reference eg. : https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly after module reload we fail. <3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten w/a for reg 7034 (mask: , value: 8280) Br, Jani This doesn't seem to have anything to do with this series though. Looks like we duplicated the WA when merging 'drm/drm-next' into drm-tip: static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { struct drm_i915_private *i915 = engine->i915; /* WaDisableBankHangMode:icl */ wa_write(wal, GEN8_L3CNTLREG, intel_uncore_read(engine->uncore, GEN8_L3CNTLREG) | GEN8_ERRDETBCTRL); /* WaDisableBankHangMode:icl */ wa_write(wal, GEN8_L3CNTLREG, intel_uncore_read(engine->uncore, GEN8_L3CNTLREG) | GEN8_ERRDETBCTRL); AFAICS the duplication is added by: commit 7126b65091c417e757b638065ae4fdc2a5dc2f5c Merge: 3aea8d02f801 14ee642c2ab0 Author: Chris Wilson Date: Thu May 30 12:06:55 2019 +0100 Merge remote-tracking branch 'drm/drm-next' into drm-tip Daniele Thanks, Stuart Daniele -Stuart Regards Manasi On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote: This patch series contains a few code clean-up patches, followed by a patch which changes the storage of the subslice mask to better match the userspace access through the I915_QUERY_TOPOLOGY_INFO ioctl. The index into the subslice_mask array is then calculated: slice * subslice stride + subslice index / 8 v2: fix i915_pm_sseu test failure v3: no changes to patches in the series, just resending to pick up in CI correctly v4: rebase v5: fix header test v6: address review comments from Jari address minor checkpatch warning in existing code use eu_stride for EU div-by-8 v7: another rebase v8: address review comments from Tvrtko and Daniele v9: address review comments from Daniele v10: add reviewed-by on last patch with minor suggested change, rebase, and repost for CI Stuart Summers (5): drm/i915: Use local variable for SSEU info in GETPARAM ioctl drm/i915: Add macro for SSEU stride calculation drm/i915: Move calculation of subslices per slice to new function drm/i915: Refactor sseu helper functions drm/i915: Expand subslice mask drivers/gpu/drm/i915/gt/intel_engine_cs.c| 24 ++- drivers/gpu/drm/i915/gt/intel_engine_types.h | 30 ++-- drivers/gpu/drm/i915/gt/intel_hangcheck.c| 3 +- drivers/gpu/drm/i915/gt/intel_sseu.c | 62 +++ drivers/gpu/drm/i915/gt/intel_sseu.h | 35 +++- drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 46 ++--- drivers/gpu/drm/i915/i915_drv.c | 15 +- drivers/gpu/drm/i915/i915_gpu_error.c| 5 +- drivers/gpu/drm/i915/i915_query.c| 15 +- drivers/gpu/drm/i915/intel_device_info.c | 176 +++-- -- drivers/gpu/drm/i915/intel_device_info.h | 47 - 12 files changed, 280 insertions(+), 180 delet
Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
On Thu, 2019-05-30 at 09:29 +0100, Saarinen, Jani wrote: > Hi, > > > > -Original Message- > > From: Summers, Stuart > > Sent: keskiviikko 29. toukokuuta 2019 19.02 > > To: Saarinen, Jani ; Ceraolo Spurio, > > Daniele > > ; Navare, Manasi D > > > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask > > > > On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote: > > > > > > On 5/28/19 11:48 PM, Saarinen, Jani wrote: > > > > Hi, > > > > > > > > > -Original Message- > > > > > From: Intel-gfx [mailto: > > > > > intel-gfx-boun...@lists.freedesktop.org] > > > > > On Behalf Of > > > > > Summers, Stuart > > > > > Sent: tiistai 28. toukokuuta 2019 21.33 > > > > > To: Navare, Manasi D > > > > > Cc: intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice > > > > > mask > > > > > > > > > > On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote: > > > > > > Pushed to dinq, thanks for the patches and the reviews! > > > > > > > > > > Thanks for the push Manasi and the reviews Daniele and > > > > > others!sk: , value: 8280) > > > > > > > > This broke all the ICL systems because CI data was not looked > > > > that > > > > they did not actually even boot at all. > > > > All ICL's in BAT and whole ICL shards. > > > > > > > > > > Can we change the CI reply for the case where there are extra > > > missing > > > machines compared to the reference run from SUCCESS to WARNING or > > > something like that, so people have a clearer indication that > > > something > > > might have gone wrong? > > > > I agree here. I'm sure with time and experience these types of > > things > > will get easier to parse, but this was very unobvious to me when > > posting. I have no problem reworking, but would really appreciate a > > solution to this from the CI side to ensure we don't hit this type > > of > > thing in the future. > > Sure, CI team already discussed on this. > But going forward. Can you fix this still today that ICL's systems > are green not orange on ci-grid. > So would be good to get to the state that was on CI_DRM_6158. > Reference eg. : > https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly > after module reload we fail. > <3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten > w/a for reg 7034 (mask: , value: 8280) Hi Jani, I will likely not have time to get to this today or maybe even tomorrow unfortunately. I'll try to look at this as part of my rework of the SSEU revert from yesterday. Thanks, Stuart > > Br, > Jani > > > > Thanks, > > Stuart > > > > > > > > Daniele > > > > > > > > > > > > > > > > > -Stuart > > > > > > > > > > > > > > > > > Regards > > > > > > Manasi > > > > > > > > > > > > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers > > > > > > wrote: > > > > > > > This patch series contains a few code clean-up patches, > > > > > > > followed by > > > > > > > a patch which changes the storage of the subslice mask to > > > > > > > better > > > > > > > match the userspace access through the > > > > > > > I915_QUERY_TOPOLOGY_INFO > > > > > > > ioctl. The index into the subslice_mask array is then > > > > > > > calculated: > > > > > > >slice * subslice stride + subslice index / 8 > > > > > > > > > > > > > > v2: fix i915_pm_sseu test failure > > > > > > > v3: no changes to patches in the series, just resending > > > > > > > to > > > > > > > pick up > > > > > > > in CI correctly > > > > > > > v4: rebase > > > > > > > v5: fix header test > > > > > > > v6: address review comments from Jari > > > > > > > address minor checkpatch warning in existing code > > > > > > > use eu_stride for EU div-by-8 > > > > > > > v7: another rebase > > > > > > > v8: address review comments from Tvrtko and Daniele > > > > > > > v9: address review comments from Daniele > > > > > > > v10: add reviewed-by on last patch with minor suggested > > > > > > > change, > > > > > > > rebase, and repost for CI > > > > > > > > > > > > > > Stuart Summers (5): > > > > > > >drm/i915: Use local variable for SSEU info in GETPARAM > > > > > > > ioctl > > > > > > >drm/i915: Add macro for SSEU stride calculation > > > > > > >drm/i915: Move calculation of subslices per slice to > > > > > > > new > > > > > > > function > > > > > > >drm/i915: Refactor sseu helper functions > > > > > > >drm/i915: Expand subslice mask > > > > > > > > > > > > > > drivers/gpu/drm/i915/gt/intel_engine_cs.c| 24 ++- > > > > > > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 30 ++-- > > > > > > > drivers/gpu/drm/i915/gt/intel_hangcheck.c| 3 +- > > > > > > > drivers/gpu/drm/i915/gt/intel_sseu.c | 62 > > > > > > > +++ > > > > > > > drivers/gpu/drm/i915/gt/intel_sseu.h | 35 +++- > > > > > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- > > > > > > > drivers/gpu/drm/i915/i915_debug
[Intel-gfx] [PATCH v2] drm/i915: Split off pci_driver.remove() tail to drm_driver.release()
In order to support driver hot unbind, some cleanup operations, now performed on PCI driver remove, must be called later, after all device file descriptors are closed. Split out those operations from the tail of pci_driver.remove() callback and put them into drm_driver.release() which is called as soon as all references to the driver are put. As a result, those cleanups will be now run on last drm_dev_put(), either still called from pci_driver.remove() if all device file descriptors are already closed, or on last drm_release() file operation. Signed-off-by: Janusz Krzysztofik Reviewed-by: Chris Wilson --- Changelog: v1 -> v2: - defer intel_engines_cleanup() as well. (Chris) drivers/gpu/drm/i915/i915_drv.c | 17 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 10 +- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 83d2eb9e74cb..8be69f84eb6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -738,6 +738,7 @@ static int i915_load_modeset_init(struct drm_device *dev) cleanup_gem: i915_gem_suspend(dev_priv); + i915_gem_fini_hw(dev_priv); i915_gem_fini(dev_priv); cleanup_modeset: intel_modeset_cleanup(dev); @@ -1685,7 +1686,6 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) pci_disable_msi(pdev); pm_qos_remove_request(&dev_priv->pm_qos); - i915_ggtt_cleanup_hw(dev_priv); } /** @@ -1909,6 +1909,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) out_cleanup_hw: i915_driver_cleanup_hw(dev_priv); + i915_ggtt_cleanup_hw(dev_priv); out_cleanup_mmio: i915_driver_cleanup_mmio(dev_priv); out_runtime_pm_put: @@ -1960,21 +1961,29 @@ void i915_driver_unload(struct drm_device *dev) cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); i915_reset_error_state(dev_priv); - i915_gem_fini(dev_priv); + i915_gem_fini_hw(dev_priv); intel_power_domains_fini_hw(dev_priv); i915_driver_cleanup_hw(dev_priv); - i915_driver_cleanup_mmio(dev_priv); enable_rpm_wakeref_asserts(dev_priv); - intel_runtime_pm_cleanup(dev_priv); } static void i915_driver_release(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + disable_rpm_wakeref_asserts(dev_priv); + + i915_gem_fini(dev_priv); + + i915_ggtt_cleanup_hw(dev_priv); + i915_driver_cleanup_mmio(dev_priv); + + enable_rpm_wakeref_asserts(dev_priv); + intel_runtime_pm_cleanup(dev_priv); + i915_driver_cleanup_early(dev_priv); i915_driver_destroy(dev_priv); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a2664ea1395b..d08e7bd83544 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3047,6 +3047,7 @@ void i915_gem_init_mmio(struct drm_i915_private *i915); int __must_check i915_gem_init(struct drm_i915_private *dev_priv); int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv); void i915_gem_init_swizzling(struct drm_i915_private *dev_priv); +void i915_gem_fini_hw(struct drm_i915_private *dev_priv); void i915_gem_fini(struct drm_i915_private *dev_priv); int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, unsigned int flags, long timeout); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7cafd5612f71..20d3f7532cef 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4667,7 +4667,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) return ret; } -void i915_gem_fini(struct drm_i915_private *dev_priv) +void i915_gem_fini_hw(struct drm_i915_private *dev_priv) { GEM_BUG_ON(dev_priv->gt.awake); @@ -4680,6 +4680,14 @@ void i915_gem_fini(struct drm_i915_private *dev_priv) mutex_lock(&dev_priv->drm.struct_mutex); intel_uc_fini_hw(dev_priv); intel_uc_fini(dev_priv); + mutex_unlock(&dev_priv->drm.struct_mutex); + + i915_gem_drain_freed_objects(dev_priv); +} + +void i915_gem_fini(struct drm_i915_private *dev_priv) +{ + mutex_lock(&dev_priv->drm.struct_mutex); intel_engines_cleanup(dev_priv); i915_gem_contexts_fini(dev_priv); i915_gem_fini_scratch(dev_priv); -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Always enable mmio debugging for CI
The default behaviour is to periodically check for a mmio access error, and once detected enable mmio access checking. However this is useless if the error only occurs once. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/Kconfig.debug | 12 drivers/gpu/drm/i915/i915_params.h | 8 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 04b686d2c2d0..53c5f8cd9a1e 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -32,6 +32,7 @@ config DRM_I915_DEBUG select DRM_I915_SW_FENCE_DEBUG_OBJECTS select DRM_I915_SELFTEST select DRM_I915_DEBUG_RUNTIME_PM + select DRM_I915_DEBUG_MMIO default n help Choose this option to turn on extra driver debugging that may affect @@ -41,6 +42,17 @@ config DRM_I915_DEBUG If in doubt, say "N". +config DRM_I915_DEBUG_MMIO +bool "Always insert extra checks around mmio access" +default n +help + Always enables the extra sanity checks (extra register reads) + around every mmio (register) access that will slow the system down. + + Recommended for driver developers only. + + If in doubt, say "N". + config DRM_I915_DEBUG_GEM bool "Insert extra checks into the GEM internals" default n diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 3f14e9881a0d..89018bb2d059 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -33,6 +33,12 @@ struct drm_printer; #define ENABLE_GUC_SUBMISSION BIT(0) #define ENABLE_GUC_LOAD_HUCBIT(1) +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) +#define MMIO_DEBUG_DFL -1 +#else +#define MMIO_DEBUG_DFL 0 +#endif + /* * Invoke param, a function-like macro, for each i915 param, with arguments: * @@ -59,7 +65,7 @@ struct drm_printer; param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ param(char *, dmc_firmware_path, NULL) \ - param(int, mmio_debug, 0) \ + param(int, mmio_debug, MMIO_DEBUG_DFL) \ param(int, edp_vswing, 0) \ param(int, reset, 2) \ param(unsigned int, inject_load_failure, 0) \ -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_workarounds: Mix and match SRM with mmio reads
On Wed, 29 May 2019 at 22:14, Chris Wilson wrote: > > On apl, mmio reads fail, reading 0. > On cml, SRM reads fail, reading 0. > > Combine both approaches, starting with SRM and fixing in the blanks with > mmio reads. > > Signed-off-by: Chris Wilson > Cc: Matthew Auld > Cc: Mika Kuoppala Reviewed-by: Matthew Auld ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Split off pci_driver.remove() tail to drm_driver.release()
== Series Details == Series: drm/i915: Split off pci_driver.remove() tail to drm_driver.release() URL : https://patchwork.freedesktop.org/series/61376/ State : failure == Summary == CI Bug Log - changes from CI_DRM_6168 -> Patchwork_13137 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_13137 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_13137, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_13137: ### IGT changes ### Possible regressions * igt@i915_selftest@live_execlists: - fi-icl-u3: NOTRUN -> [DMESG-WARN][1] +1 similar issue [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-icl-u3/igt@i915_selftest@live_execlists.html Warnings * igt@i915_selftest@live_hangcheck: - fi-icl-u3: [INCOMPLETE][2] ([fdo#107713] / [fdo#108569]) -> [DMESG-WARN][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-icl-u3/igt@i915_selftest@live_hangcheck.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-icl-u3/igt@i915_selftest@live_hangcheck.html Known issues Here are the changes found in Patchwork_13137 that come from known issues: ### IGT changes ### Issues hit * igt@gem_exec_suspend@basic-s4-devices: - fi-blb-e6850: [PASS][4] -> [INCOMPLETE][5] ([fdo#107718]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-blb-e6850/igt@gem_exec_susp...@basic-s4-devices.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-blb-e6850/igt@gem_exec_susp...@basic-s4-devices.html * igt@kms_chamelium@dp-crc-fast: - fi-cml-u2: [PASS][6] -> [FAIL][7] ([fdo#110627]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-cml-u2/igt@kms_chamel...@dp-crc-fast.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-cml-u2/igt@kms_chamel...@dp-crc-fast.html * igt@kms_frontbuffer_tracking@basic: - fi-hsw-peppy: [PASS][8] -> [DMESG-WARN][9] ([fdo#102614]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-hsw-peppy/igt@kms_frontbuffer_track...@basic.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-hsw-peppy/igt@kms_frontbuffer_track...@basic.html Possible fixes * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence: - fi-icl-u3: [DMESG-WARN][10] ([fdo#107724]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-icl-u3/igt@kms_pipe_crc_ba...@nonblocking-crc-pipe-a-frame-sequence.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-icl-u3/igt@kms_pipe_crc_ba...@nonblocking-crc-pipe-a-frame-sequence.html * igt@prime_vgem@basic-fence-flip: - fi-ilk-650: [DMESG-WARN][12] ([fdo#106387]) -> [PASS][13] +1 similar issue [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6168/fi-ilk-650/igt@prime_v...@basic-fence-flip.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/fi-ilk-650/igt@prime_v...@basic-fence-flip.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614 [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387 [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724 [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569 [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100 [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627 Participating hosts (50 -> 45) -- Missing(5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes - * Linux: CI_DRM_6168 -> Patchwork_13137 CI_DRM_6168: ef1f9911a52a7d00cb52cc9447f411340b653bcc @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5024: f414756be2ac57e194919973da7b86644ba61241 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_13137: a8438c2c0735c8654275bcfa4d08943c4ef15595 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == a8438c2c0735 drm/i915: Split off pci_driver.remove() tail to drm_driver.release() == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13137/ ___ Intel-gfx mailing list Intel-gf
Re: [Intel-gfx] [RFC PATCH 1/1] drm/i915: Split off pci_driver.remove() tail to drm_driver.release()
Quoting Janusz Krzysztofik (2019-05-30 10:24:26) > In order to support driver hot unbind, some cleanup operations, now > performed on PCI driver remove, must be called later, after all device > file descriptors are closed. > > Split out those operations from the tail of pci_driver.remove() > callback and put them into drm_driver.release() which is called as soon > as all references to the driver are put. As a result, those cleanups > will be now run on last drm_dev_put(), either still called from > pci_driver.remove() if all device file descriptors are already closed, > or on last drm_release() file operation. > > Signed-off-by: Janusz Krzysztofik > --- > drivers/gpu/drm/i915/i915_drv.c | 17 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem.c | 10 +- > 3 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 83d2eb9e74cb..8be69f84eb6d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -738,6 +738,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > cleanup_gem: > i915_gem_suspend(dev_priv); > + i915_gem_fini_hw(dev_priv); > i915_gem_fini(dev_priv); > cleanup_modeset: > intel_modeset_cleanup(dev); > @@ -1685,7 +1686,6 @@ static void i915_driver_cleanup_hw(struct > drm_i915_private *dev_priv) > pci_disable_msi(pdev); > > pm_qos_remove_request(&dev_priv->pm_qos); > - i915_ggtt_cleanup_hw(dev_priv); > } > > /** > @@ -1909,6 +1909,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct > pci_device_id *ent) Would it make sense to rename load/unload from the legacy drm stubs over to match the pci entry points? > out_cleanup_hw: > i915_driver_cleanup_hw(dev_priv); > + i915_ggtt_cleanup_hw(dev_priv); > out_cleanup_mmio: > i915_driver_cleanup_mmio(dev_priv); > out_runtime_pm_put: > @@ -1960,21 +1961,29 @@ void i915_driver_unload(struct drm_device *dev) > cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); > i915_reset_error_state(dev_priv); > > - i915_gem_fini(dev_priv); > + i915_gem_fini_hw(dev_priv); > > intel_power_domains_fini_hw(dev_priv); > > i915_driver_cleanup_hw(dev_priv); > - i915_driver_cleanup_mmio(dev_priv); > > enable_rpm_wakeref_asserts(dev_priv); > - intel_runtime_pm_cleanup(dev_priv); > } > > static void i915_driver_release(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > > + disable_rpm_wakeref_asserts(dev_priv); > + > + i915_gem_fini(dev_priv); > + > + i915_ggtt_cleanup_hw(dev_priv); > + i915_driver_cleanup_mmio(dev_priv); > + > + enable_rpm_wakeref_asserts(dev_priv); > + intel_runtime_pm_cleanup(dev_priv); We should really propagate the release nomenclature down and replace our mixed fini/cleanup. Consistency is helpful when trying to work out which phase the code is in. > i915_driver_cleanup_early(dev_priv); > i915_driver_destroy(dev_priv); > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a2664ea1395b..d08e7bd83544 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3047,6 +3047,7 @@ void i915_gem_init_mmio(struct drm_i915_private *i915); > int __must_check i915_gem_init(struct drm_i915_private *dev_priv); > int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv); > void i915_gem_init_swizzling(struct drm_i915_private *dev_priv); > +void i915_gem_fini_hw(struct drm_i915_private *dev_priv); > void i915_gem_fini(struct drm_i915_private *dev_priv); > int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, >unsigned int flags, long timeout); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 7cafd5612f71..c6a8e665a6ba 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4667,7 +4667,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) > return ret; > } > > -void i915_gem_fini(struct drm_i915_private *dev_priv) > +void i915_gem_fini_hw(struct drm_i915_private *dev_priv) > { > GEM_BUG_ON(dev_priv->gt.awake); > > @@ -4681,6 +4681,14 @@ void i915_gem_fini(struct drm_i915_private *dev_priv) > intel_uc_fini_hw(dev_priv); > intel_uc_fini(dev_priv); > intel_engines_cleanup(dev_priv); intel_engines_cleanup -> i915_gem_fini -- that is in principle just freeing structs. One side effect it does have is to make all engines unavailable (but it doesn't update the engine_mask so the inconsistency might catch us out if it is not one of the last cleanup actions). intel_uc_fini() is a bit of a mixed bag. It looks like it flushes runtime state, so preferrably that flush should
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Drop non-NULL llist_for_each_entry_safe check for
== Series Details == Series: drm/i915: Drop non-NULL llist_for_each_entry_safe check for URL : https://patchwork.freedesktop.org/series/61374/ State : success == Summary == CI Bug Log - changes from CI_DRM_6167 -> Patchwork_13136 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/ Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_13136: ### IGT changes ### Suppressed The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_module_load@reload: - {fi-icl-dsi}: NOTRUN -> [DMESG-WARN][1] +22 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-dsi/igt@i915_module_l...@reload.html Known issues Here are the changes found in Patchwork_13136 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live_contexts: - fi-bdw-gvtdvm: [PASS][2] -> [DMESG-FAIL][3] ([fdo#110235]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size: - fi-icl-u3: [PASS][4] -> [DMESG-WARN][5] ([fdo#107724]) +2 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-u3/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-u3/igt@kms_cursor_leg...@basic-flip-after-cursor-varying-size.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-blb-e6850: [PASS][6] -> [INCOMPLETE][7] ([fdo#107718]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-blb-e6850/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-blb-e6850/igt@kms_pipe_crc_ba...@suspend-read-crc-pipe-a.html Possible fixes * igt@gem_basic@bad-close: - fi-icl-u3: [DMESG-WARN][8] ([fdo#107724]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-u3/igt@gem_ba...@bad-close.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-u3/igt@gem_ba...@bad-close.html * igt@gem_ctx_create@basic-files: - {fi-icl-dsi}: [INCOMPLETE][10] ([fdo#107713] / [fdo#109100]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-dsi/igt@gem_ctx_cre...@basic-files.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-dsi/igt@gem_ctx_cre...@basic-files.html * igt@i915_selftest@live_contexts: - fi-hsw-peppy: [DMESG-FAIL][12] ([fdo#110235]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-hsw-peppy/igt@i915_selftest@live_contexts.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-hsw-peppy/igt@i915_selftest@live_contexts.html * igt@kms_frontbuffer_tracking@basic: - fi-icl-u3: [FAIL][14] ([fdo#103167]) -> [PASS][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-u3/igt@kms_frontbuffer_track...@basic.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-u3/igt@kms_frontbuffer_track...@basic.html * igt@prime_vgem@basic-fence-flip: - fi-ilk-650: [DMESG-WARN][16] ([fdo#106387]) -> [PASS][17] +2 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-ilk-650/igt@prime_v...@basic-fence-flip.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-ilk-650/igt@prime_v...@basic-fence-flip.html Warnings * igt@i915_selftest@live_hangcheck: - fi-icl-u2: [DMESG-WARN][18] -> [INCOMPLETE][19] ([fdo#107713] / [fdo#108569]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-u2/igt@i915_selftest@live_hangcheck.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-u2/igt@i915_selftest@live_hangcheck.html - fi-icl-u3: [DMESG-WARN][20] -> [INCOMPLETE][21] ([fdo#107713] / [fdo#108569]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-u3/igt@i915_selftest@live_hangcheck.html [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-u3/igt@i915_selftest@live_hangcheck.html - fi-icl-y: [DMESG-WARN][22] -> [INCOMPLETE][23] ([fdo#107713] / [fdo#108569]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6167/fi-icl-y/igt@i915_selftest@live_hangcheck.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13136/fi-icl-y/igt@i915_selftest@live_hangcheck.html {name}: This element is suppressed. This
[Intel-gfx] [RFC PATCH 0/1] drm/i915: Split off pci_driver.remove() tail to drm_driver.release()
Hi, I do realize more work needs to be done to get a clean hotunplug solution, however I need your comments to make sure that I'm going in the right direction. So far I have no good idea how to resolve pm_runtime_get_sync() failures on outstanding device file close after successfull driver unbind. Thanks, Janusz Janusz Krzysztofik (1): drm/i915: Split off pci_driver.remove() tail to drm_driver.release() drivers/gpu/drm/i915/i915_drv.c | 17 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 10 +- 3 files changed, 23 insertions(+), 5 deletions(-) -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 1/1] drm/i915: Split off pci_driver.remove() tail to drm_driver.release()
In order to support driver hot unbind, some cleanup operations, now performed on PCI driver remove, must be called later, after all device file descriptors are closed. Split out those operations from the tail of pci_driver.remove() callback and put them into drm_driver.release() which is called as soon as all references to the driver are put. As a result, those cleanups will be now run on last drm_dev_put(), either still called from pci_driver.remove() if all device file descriptors are already closed, or on last drm_release() file operation. Signed-off-by: Janusz Krzysztofik --- drivers/gpu/drm/i915/i915_drv.c | 17 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 10 +- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 83d2eb9e74cb..8be69f84eb6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -738,6 +738,7 @@ static int i915_load_modeset_init(struct drm_device *dev) cleanup_gem: i915_gem_suspend(dev_priv); + i915_gem_fini_hw(dev_priv); i915_gem_fini(dev_priv); cleanup_modeset: intel_modeset_cleanup(dev); @@ -1685,7 +1686,6 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) pci_disable_msi(pdev); pm_qos_remove_request(&dev_priv->pm_qos); - i915_ggtt_cleanup_hw(dev_priv); } /** @@ -1909,6 +1909,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent) out_cleanup_hw: i915_driver_cleanup_hw(dev_priv); + i915_ggtt_cleanup_hw(dev_priv); out_cleanup_mmio: i915_driver_cleanup_mmio(dev_priv); out_runtime_pm_put: @@ -1960,21 +1961,29 @@ void i915_driver_unload(struct drm_device *dev) cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); i915_reset_error_state(dev_priv); - i915_gem_fini(dev_priv); + i915_gem_fini_hw(dev_priv); intel_power_domains_fini_hw(dev_priv); i915_driver_cleanup_hw(dev_priv); - i915_driver_cleanup_mmio(dev_priv); enable_rpm_wakeref_asserts(dev_priv); - intel_runtime_pm_cleanup(dev_priv); } static void i915_driver_release(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + disable_rpm_wakeref_asserts(dev_priv); + + i915_gem_fini(dev_priv); + + i915_ggtt_cleanup_hw(dev_priv); + i915_driver_cleanup_mmio(dev_priv); + + enable_rpm_wakeref_asserts(dev_priv); + intel_runtime_pm_cleanup(dev_priv); + i915_driver_cleanup_early(dev_priv); i915_driver_destroy(dev_priv); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a2664ea1395b..d08e7bd83544 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3047,6 +3047,7 @@ void i915_gem_init_mmio(struct drm_i915_private *i915); int __must_check i915_gem_init(struct drm_i915_private *dev_priv); int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv); void i915_gem_init_swizzling(struct drm_i915_private *dev_priv); +void i915_gem_fini_hw(struct drm_i915_private *dev_priv); void i915_gem_fini(struct drm_i915_private *dev_priv); int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, unsigned int flags, long timeout); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7cafd5612f71..c6a8e665a6ba 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4667,7 +4667,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) return ret; } -void i915_gem_fini(struct drm_i915_private *dev_priv) +void i915_gem_fini_hw(struct drm_i915_private *dev_priv) { GEM_BUG_ON(dev_priv->gt.awake); @@ -4681,6 +4681,14 @@ void i915_gem_fini(struct drm_i915_private *dev_priv) intel_uc_fini_hw(dev_priv); intel_uc_fini(dev_priv); intel_engines_cleanup(dev_priv); + mutex_unlock(&dev_priv->drm.struct_mutex); + + i915_gem_drain_freed_objects(dev_priv); +} + +void i915_gem_fini(struct drm_i915_private *dev_priv) +{ + mutex_lock(&dev_priv->drm.struct_mutex); i915_gem_contexts_fini(dev_priv); i915_gem_fini_scratch(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); -- 2.21.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
Hi, > -Original Message- > From: Summers, Stuart > Sent: keskiviikko 29. toukokuuta 2019 19.02 > To: Saarinen, Jani ; Ceraolo Spurio, Daniele > ; Navare, Manasi D > > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask > > On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote: > > > > On 5/28/19 11:48 PM, Saarinen, Jani wrote: > > > Hi, > > > > > > > -Original Message- > > > > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] > > > > On Behalf Of > > > > Summers, Stuart > > > > Sent: tiistai 28. toukokuuta 2019 21.33 > > > > To: Navare, Manasi D > > > > Cc: intel-gfx@lists.freedesktop.org > > > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice > > > > mask > > > > > > > > On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote: > > > > > Pushed to dinq, thanks for the patches and the reviews! > > > > > > > > Thanks for the push Manasi and the reviews Daniele and others! > > > > > > This broke all the ICL systems because CI data was not looked that > > > they did not actually even boot at all. > > > All ICL's in BAT and whole ICL shards. > > > > > > > Can we change the CI reply for the case where there are extra > > missing > > machines compared to the reference run from SUCCESS to WARNING or > > something like that, so people have a clearer indication that > > something > > might have gone wrong? > > I agree here. I'm sure with time and experience these types of things > will get easier to parse, but this was very unobvious to me when > posting. I have no problem reworking, but would really appreciate a > solution to this from the CI side to ensure we don't hit this type of > thing in the future. Sure, CI team already discussed on this. But going forward. Can you fix this still today that ICL's systems are green not orange on ci-grid. So would be good to get to the state that was on CI_DRM_6158. Reference eg. : https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly after module reload we fail. <3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten w/a for reg 7034 (mask: , value: 8280) Br, Jani > > Thanks, > Stuart > > > > > Daniele > > > > > > > > > > > > > -Stuart > > > > > > > > > > > > > > Regards > > > > > Manasi > > > > > > > > > > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote: > > > > > > This patch series contains a few code clean-up patches, > > > > > > followed by > > > > > > a patch which changes the storage of the subslice mask to > > > > > > better > > > > > > match the userspace access through the > > > > > > I915_QUERY_TOPOLOGY_INFO > > > > > > ioctl. The index into the subslice_mask array is then > > > > > > calculated: > > > > > >slice * subslice stride + subslice index / 8 > > > > > > > > > > > > v2: fix i915_pm_sseu test failure > > > > > > v3: no changes to patches in the series, just resending to > > > > > > pick up > > > > > > in CI correctly > > > > > > v4: rebase > > > > > > v5: fix header test > > > > > > v6: address review comments from Jari > > > > > > address minor checkpatch warning in existing code > > > > > > use eu_stride for EU div-by-8 > > > > > > v7: another rebase > > > > > > v8: address review comments from Tvrtko and Daniele > > > > > > v9: address review comments from Daniele > > > > > > v10: add reviewed-by on last patch with minor suggested > > > > > > change, > > > > > > rebase, and repost for CI > > > > > > > > > > > > Stuart Summers (5): > > > > > >drm/i915: Use local variable for SSEU info in GETPARAM > > > > > > ioctl > > > > > >drm/i915: Add macro for SSEU stride calculation > > > > > >drm/i915: Move calculation of subslices per slice to new > > > > > > function > > > > > >drm/i915: Refactor sseu helper functions > > > > > >drm/i915: Expand subslice mask > > > > > > > > > > > > drivers/gpu/drm/i915/gt/intel_engine_cs.c| 24 ++- > > > > > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 30 ++-- > > > > > > drivers/gpu/drm/i915/gt/intel_hangcheck.c| 3 +- > > > > > > drivers/gpu/drm/i915/gt/intel_sseu.c | 62 +++ > > > > > > drivers/gpu/drm/i915/gt/intel_sseu.h | 35 +++- > > > > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- > > > > > > drivers/gpu/drm/i915/i915_debugfs.c | 46 ++--- > > > > > > drivers/gpu/drm/i915/i915_drv.c | 15 +- > > > > > > drivers/gpu/drm/i915/i915_gpu_error.c| 5 +- > > > > > > drivers/gpu/drm/i915/i915_query.c| 15 +- > > > > > > drivers/gpu/drm/i915/intel_device_info.c | 176 > > > > > > +++-- > > > > > > -- > > > > > > drivers/gpu/drm/i915/intel_device_info.h | 47 - > > > > > > 12 files changed, 280 insertions(+), 180 deletions(-) > > > > > > > > > > > > -- > > > > > > 2.21.0.5.gaeb582a983 > > > > > > > > > > > > ___ > > > > > > Intel-gfx m
[Intel-gfx] [PATCH] drm/i915: Drop non-NULL llist_for_each_entry_safe check for
Since the next entry is an offset from a pointer, it can not be NULL. For simplicity, drop the extra conditional before calling cond_resched() Reported-by: Dan Carpenter Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index f064876f1214..55e79fdb81aa 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -248,8 +248,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, GEM_BUG_ON(!atomic_read(&i915->mm.free_count)); atomic_dec(&i915->mm.free_count); - if (on) - cond_resched(); + cond_resched(); } intel_runtime_pm_put(i915, wakeref); } -- 2.20.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [bug report] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
Quoting Dan Carpenter (2019-05-30 09:13:01) > Hello Chris Wilson, > > The patch cc731f5a3b1f: "drm/i915: Trim struct_mutex hold duration > for i915_gem_free_objects" from Oct 13, 2017, leads to the following > static checker warning: > > drivers/gpu/drm/i915/gem/i915_gem_object.c:195 > __i915_gem_free_objects() > error: we previously assumed 'obj' could be null (see line 195) > > drivers/gpu/drm/i915/gem/i915_gem_object.c >188 static void __i915_gem_free_objects(struct drm_i915_private *i915, >189 struct llist_node *freed) >190 { >191 struct drm_i915_gem_object *obj, *on; >192 intel_wakeref_t wakeref; >193 >194 wakeref = intel_runtime_pm_get(i915); >195 llist_for_each_entry_safe(obj, on, freed, freed) { >^^ > &on->freed is a pointer to the next item in the list? Hmm. I was looking at llist_for_each_safe where the next pointer would be NULL. But for _entry, it will indeed be offset. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [bug report] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
Hello Chris Wilson, The patch cc731f5a3b1f: "drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects" from Oct 13, 2017, leads to the following static checker warning: drivers/gpu/drm/i915/gem/i915_gem_object.c:195 __i915_gem_free_objects() error: we previously assumed 'obj' could be null (see line 195) drivers/gpu/drm/i915/gem/i915_gem_object.c 188 static void __i915_gem_free_objects(struct drm_i915_private *i915, 189 struct llist_node *freed) 190 { 191 struct drm_i915_gem_object *obj, *on; 192 intel_wakeref_t wakeref; 193 194 wakeref = intel_runtime_pm_get(i915); 195 llist_for_each_entry_safe(obj, on, freed, freed) { ^^ &on->freed is a pointer to the next item in the list? 196 struct i915_vma *vma, *vn; 197 198 trace_i915_gem_object_destroy(obj); 199 200 mutex_lock(&i915->drm.struct_mutex); 201 202 GEM_BUG_ON(i915_gem_object_is_active(obj)); 203 list_for_each_entry_safe(vma, vn, &obj->vma.list, obj_link) { 204 GEM_BUG_ON(i915_vma_is_active(vma)); 205 vma->flags &= ~I915_VMA_PIN_MASK; 206 i915_vma_destroy(vma); 207 } 208 GEM_BUG_ON(!list_empty(&obj->vma.list)); 209 GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree)); 210 211 /* This serializes freeing with the shrinker. Since the free 212 * is delayed, first by RCU then by the workqueue, we want the 213 * shrinker to be able to free pages of unreferenced objects, 214 * or else we may oom whilst there are plenty of deferred 215 * freed objects. 216 */ 217 if (i915_gem_object_has_pages(obj)) { 218 spin_lock(&i915->mm.obj_lock); 219 list_del_init(&obj->mm.link); 220 spin_unlock(&i915->mm.obj_lock); 221 } 222 223 mutex_unlock(&i915->drm.struct_mutex); 224 225 GEM_BUG_ON(obj->bind_count); 226 GEM_BUG_ON(obj->userfault_count); 227 GEM_BUG_ON(atomic_read(&obj->frontbuffer_bits)); 228 GEM_BUG_ON(!list_empty(&obj->lut_list)); 229 230 if (obj->ops->release) 231 obj->ops->release(obj); 232 233 if (WARN_ON(i915_gem_object_has_pinned_pages(obj))) 234 atomic_set(&obj->mm.pages_pin_count, 0); 235 __i915_gem_object_put_pages(obj, I915_MM_NORMAL); 236 GEM_BUG_ON(i915_gem_object_has_pages(obj)); 237 238 if (obj->base.import_attach) 239 drm_prime_gem_destroy(&obj->base, NULL); 240 241 reservation_object_fini(&obj->__builtin_resv); 242 drm_gem_object_release(&obj->base); 243 i915_gem_info_remove_obj(i915, obj->base.size); 244 245 bitmap_free(obj->bit_17); 246 i915_gem_object_free(obj); 247 248 GEM_BUG_ON(!atomic_read(&i915->mm.free_count)); 249 atomic_dec(&i915->mm.free_count); 250 251 if (on) ^^ So hopefully "on" can't be NULL here or we are toasted. 252 cond_resched(); 253 } 254 intel_runtime_pm_put(i915, wakeref); 255 } regards, dan carpenter ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx