Re: [Intel-gfx] [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Fri, 27 May 2022 at 01:55, Dmitry Osipenko wrote: > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > code duplication among DRM drivers by replacing theirs custom shrinker > implementations with the generic shrinker. > > In order to start using DRM SHMEM shrinker drivers should: > > 1. Implement new evict() shmem object callback. > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to >activate shrinking of shmem GEMs. > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > Signed-off-by: Daniel Almeida > Signed-off-by: Dmitry Osipenko So I guess I get a price for being blind since forever, because this thing existed since at least 2013. I just stumbled over llist_lru.[hc], a purpose built list helper for shrinkers. I think we should try to adopt that so that our gpu shrinkers look more like shrinkers for everything else. Apologies for this, since I fear this might cause a bit of churn. Hopefully it's all contained to the list manipulation code in shmem helpers, I don't think this should leak any further. -Daniel > --- > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > include/drm/drm_device.h | 4 + > include/drm/drm_gem_shmem_helper.h| 87 ++- > 5 files changed, 594 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 555fe212bd98..4cd0b5913492 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct > drm_device *dev, size_t > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem) > +{ > + return (shmem->madv >= 0) && shmem->evict && > + shmem->eviction_enabled && shmem->pages_use_count && > + !shmem->pages_pin_count && !shmem->base.dma_buf && > + !shmem->base.import_attach && shmem->sgt && !shmem->evicted; > +} > + > +static void > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > +{ > + struct drm_gem_object *obj = &shmem->base; > + struct drm_gem_shmem_shrinker *gem_shrinker = > obj->dev->shmem_shrinker; > + > + dma_resv_assert_held(shmem->base.resv); > + > + if (!gem_shrinker || obj->import_attach) > + return; > + > + mutex_lock(&gem_shrinker->lock); > + > + if (drm_gem_shmem_is_evictable(shmem) || > + drm_gem_shmem_is_purgeable(shmem)) > + list_move_tail(&shmem->madv_list, > &gem_shrinker->lru_evictable); > + else if (shmem->madv < 0) > + list_del_init(&shmem->madv_list); > + else if (shmem->evicted) > + list_move_tail(&shmem->madv_list, &gem_shrinker->lru_evicted); > + else if (!shmem->pages) > + list_del_init(&shmem->madv_list); > + else > + list_move_tail(&shmem->madv_list, &gem_shrinker->lru_pinned); > + > + mutex_unlock(&gem_shrinker->lock); > +} > + > /** > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > * @shmem: shmem GEM object to free > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > } else { > dma_resv_lock(shmem->base.resv, NULL); > > + /* take out shmem GEM object from the memory shrinker */ > + drm_gem_shmem_madvise(shmem, -1); > + > WARN_ON(shmem->vmap_use_count); > > if (shmem->sgt) { > @@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > sg_free_table(shmem->sgt); > kfree(shmem->sgt); > } > - if (shmem->pages) > + if (shmem->pages_use_count) > drm_gem_shmem_put_pages(shmem); > > WARN_ON(shmem->pages_use_count); > @@ -163,18 +202,82 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > *shmem) > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_free); > > -static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem) > +/** > + * drm_gem_shmem_set_evictable() - Make GEM evictable by memory shrinker > + * @shmem: shmem GEM object > + * > + * Tell memory shrinker that this GEM can be evicted. Initially eviction is > + * disabled for all GEMs. If GEM was purged, then -ENOMEM is returned. > + * > + * Returns: > + * 0 on success or a negative error code on failure. > + */ > +int drm_gem_shmem_set_evictable(struct drm_gem_shmem_object *shmem) > +{ > + dma_resv_lock(shmem->
Re: [Intel-gfx] [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Sun, Jun 5, 2022 at 9:47 AM Daniel Vetter wrote: > > On Fri, 27 May 2022 at 01:55, Dmitry Osipenko > wrote: > > > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > > code duplication among DRM drivers by replacing theirs custom shrinker > > implementations with the generic shrinker. > > > > In order to start using DRM SHMEM shrinker drivers should: > > > > 1. Implement new evict() shmem object callback. > > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to > >activate shrinking of shmem GEMs. > > > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > > > Signed-off-by: Daniel Almeida > > Signed-off-by: Dmitry Osipenko > > So I guess I get a price for being blind since forever, because this > thing existed since at least 2013. I just stumbled over > llist_lru.[hc], a purpose built list helper for shrinkers. I think we > should try to adopt that so that our gpu shrinkers look more like > shrinkers for everything else. followup from a bit of irc discussion w/ danvet about list_lru: * It seems to be missing a way to bail out of iteration before nr_to_scan is hit.. which is going to be inconvenient if you want to allow active bos on the LRU but bail scanning once you encounter the first one. * Not sure if the numa node awareness is super useful for GEM bos First issue is perhaps not too hard to fix. But maybe a better idea is a drm_gem_lru helper type thing which is more tailored to GEM buffers? BR, -R > Apologies for this, since I fear this might cause a bit of churn. > Hopefully it's all contained to the list manipulation code in shmem > helpers, I don't think this should leak any further. > -Daniel > > > --- > > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > > include/drm/drm_device.h | 4 + > > include/drm/drm_gem_shmem_helper.h| 87 ++- > > 5 files changed, 594 insertions(+), 49 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index 555fe212bd98..4cd0b5913492 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object > > *drm_gem_shmem_create(struct drm_device *dev, size_t > > } > > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem) > > +{ > > + return (shmem->madv >= 0) && shmem->evict && > > + shmem->eviction_enabled && shmem->pages_use_count && > > + !shmem->pages_pin_count && !shmem->base.dma_buf && > > + !shmem->base.import_attach && shmem->sgt && !shmem->evicted; > > +} > > + > > +static void > > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > > +{ > > + struct drm_gem_object *obj = &shmem->base; > > + struct drm_gem_shmem_shrinker *gem_shrinker = > > obj->dev->shmem_shrinker; > > + > > + dma_resv_assert_held(shmem->base.resv); > > + > > + if (!gem_shrinker || obj->import_attach) > > + return; > > + > > + mutex_lock(&gem_shrinker->lock); > > + > > + if (drm_gem_shmem_is_evictable(shmem) || > > + drm_gem_shmem_is_purgeable(shmem)) > > + list_move_tail(&shmem->madv_list, > > &gem_shrinker->lru_evictable); > > + else if (shmem->madv < 0) > > + list_del_init(&shmem->madv_list); > > + else if (shmem->evicted) > > + list_move_tail(&shmem->madv_list, > > &gem_shrinker->lru_evicted); > > + else if (!shmem->pages) > > + list_del_init(&shmem->madv_list); > > + else > > + list_move_tail(&shmem->madv_list, > > &gem_shrinker->lru_pinned); > > + > > + mutex_unlock(&gem_shrinker->lock); > > +} > > + > > /** > > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > > * @shmem: shmem GEM object to free > > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > *shmem) > > } else { > > dma_resv_lock(shmem->base.resv, NULL); > > > > + /* take out shmem GEM object from the memory shrinker */ > > + drm_gem_shmem_madvise(shmem, -1); > > + > > WARN_ON(shmem->vmap_use_count); > > > > if (shmem->sgt) { > > @@ -150,7 +189,7 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > *shmem) > > sg_free_table(shmem->sgt); > > kfree(shmem->sgt); > > } > > - if (shmem->pages) > > + if (shmem->pages_use_count) > >
Re: [Intel-gfx] [PATCH v6 17/22] drm/shmem-helper: Add generic memory shrinker
On Sun, 5 Jun 2022 at 20:32, Rob Clark wrote: > > On Sun, Jun 5, 2022 at 9:47 AM Daniel Vetter wrote: > > > > On Fri, 27 May 2022 at 01:55, Dmitry Osipenko > > wrote: > > > > > > Introduce a common DRM SHMEM shrinker framework that allows to reduce > > > code duplication among DRM drivers by replacing theirs custom shrinker > > > implementations with the generic shrinker. > > > > > > In order to start using DRM SHMEM shrinker drivers should: > > > > > > 1. Implement new evict() shmem object callback. > > > 2. Register shrinker using drm_gem_shmem_shrinker_register(drm_device). > > > 3. Use drm_gem_shmem_set_purgeable(shmem) and alike API functions to > > >activate shrinking of shmem GEMs. > > > > > > This patch is based on a ideas borrowed from Rob's Clark MSM shrinker, > > > Thomas' Zimmermann variant of SHMEM shrinker and Intel's i915 shrinker. > > > > > > Signed-off-by: Daniel Almeida > > > Signed-off-by: Dmitry Osipenko > > > > So I guess I get a price for being blind since forever, because this > > thing existed since at least 2013. I just stumbled over > > llist_lru.[hc], a purpose built list helper for shrinkers. I think we > > should try to adopt that so that our gpu shrinkers look more like > > shrinkers for everything else. > > followup from a bit of irc discussion w/ danvet about list_lru: > > * It seems to be missing a way to bail out of iteration before > nr_to_scan is hit.. which is going to be inconvenient if you > want to allow active bos on the LRU but bail scanning once > you encounter the first one. > > * Not sure if the numa node awareness is super useful for GEM > bos > > First issue is perhaps not too hard to fix. But maybe a better > idea is a drm_gem_lru helper type thing which is more tailored > to GEM buffers? Yeah I guess reusing list_lru isn't that good idea. So just open-coding it for now, and then drm_gem_bo_lru or so if we need to share it separately from shmem helpers with other drivers. Maybe will be needed for ttm or so. -Daniel > > BR, > -R > > > Apologies for this, since I fear this might cause a bit of churn. > > Hopefully it's all contained to the list manipulation code in shmem > > helpers, I don't think this should leak any further. > > -Daniel > > > > > --- > > > drivers/gpu/drm/drm_gem_shmem_helper.c| 540 -- > > > .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +- > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 + > > > include/drm/drm_device.h | 4 + > > > include/drm/drm_gem_shmem_helper.h| 87 ++- > > > 5 files changed, 594 insertions(+), 49 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > index 555fe212bd98..4cd0b5913492 100644 > > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > > @@ -126,6 +126,42 @@ struct drm_gem_shmem_object > > > *drm_gem_shmem_create(struct drm_device *dev, size_t > > > } > > > EXPORT_SYMBOL_GPL(drm_gem_shmem_create); > > > > > > +static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object > > > *shmem) > > > +{ > > > + return (shmem->madv >= 0) && shmem->evict && > > > + shmem->eviction_enabled && shmem->pages_use_count && > > > + !shmem->pages_pin_count && !shmem->base.dma_buf && > > > + !shmem->base.import_attach && shmem->sgt && > > > !shmem->evicted; > > > +} > > > + > > > +static void > > > +drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem) > > > +{ > > > + struct drm_gem_object *obj = &shmem->base; > > > + struct drm_gem_shmem_shrinker *gem_shrinker = > > > obj->dev->shmem_shrinker; > > > + > > > + dma_resv_assert_held(shmem->base.resv); > > > + > > > + if (!gem_shrinker || obj->import_attach) > > > + return; > > > + > > > + mutex_lock(&gem_shrinker->lock); > > > + > > > + if (drm_gem_shmem_is_evictable(shmem) || > > > + drm_gem_shmem_is_purgeable(shmem)) > > > + list_move_tail(&shmem->madv_list, > > > &gem_shrinker->lru_evictable); > > > + else if (shmem->madv < 0) > > > + list_del_init(&shmem->madv_list); > > > + else if (shmem->evicted) > > > + list_move_tail(&shmem->madv_list, > > > &gem_shrinker->lru_evicted); > > > + else if (!shmem->pages) > > > + list_del_init(&shmem->madv_list); > > > + else > > > + list_move_tail(&shmem->madv_list, > > > &gem_shrinker->lru_pinned); > > > + > > > + mutex_unlock(&gem_shrinker->lock); > > > +} > > > + > > > /** > > > * drm_gem_shmem_free - Free resources associated with a shmem GEM object > > > * @shmem: shmem GEM object to free > > > @@ -142,6 +178,9 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object > > > *shmem) > > > } else { > > > dma_resv_lock(shmem->base.resv, NULL); > > > > > > +
[Intel-gfx] [PATCH] drm/i915/dg2: Add Wa_14015795083
i915 must disable Render DOP clock gating globally. B.Spec: 52621 Cc: Matt Roper Cc: Badal Nilawar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 + 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index 58e9b464d564..55a291ab5536 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -630,6 +630,7 @@ #define GEN7_MISCCPCTL _MMIO(0x9424) #define GEN7_DOP_CLOCK_GATE_ENABLE (1 << 0) +#define GEN12_DOP_CLOCK_GATE_RENDER_ENABLE(1 << 1) #define GEN8_DOP_CLOCK_GATE_CFCLK_ENABLE (1 << 2) #define GEN8_DOP_CLOCK_GATE_GUC_ENABLE (1 << 4) #define GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE (1 << 6) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index a604bc7c0701..b957dec64eee 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1489,6 +1489,11 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal) * performance guide section. */ wa_write_or(wal, GEN12_SQCM, EN_32B_ACCESS); + + /* +* Wa_14015795083 +*/ + wa_write_clr(wal, GEN7_MISCCPCTL, GEN12_DOP_CLOCK_GATE_RENDER_ENABLE); } static void -- 2.26.2
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dg2: Add Wa_14015795083
== Series Details == Series: drm/i915/dg2: Add Wa_14015795083 URL : https://patchwork.freedesktop.org/series/104760/ State : success == Summary == CI Bug Log - changes from CI_DRM_11727 -> Patchwork_104760v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/index.html Participating hosts (48 -> 41) -- Additional (1): bat-jsl-1 Missing(8): fi-rkl-11600 bat-dg1-6 fi-tgl-u2 bat-dg1-5 bat-dg2-8 fi-hsw-4200u bat-adlm-1 fi-ctg-p8600 Known issues Here are the changes found in Patchwork_104760v1 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@execlists: - fi-kbl-soraka: [PASS][1] -> [INCOMPLETE][2] ([i915#794]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/fi-kbl-soraka/igt@i915_selftest@l...@execlists.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-kbl-soraka/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@requests: - fi-blb-e6850: [PASS][3] -> [DMESG-FAIL][4] ([i915#4528]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/fi-blb-e6850/igt@i915_selftest@l...@requests.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-blb-e6850/igt@i915_selftest@l...@requests.html * igt@i915_selftest@live@vma: - fi-bdw-5557u: [PASS][5] -> [INCOMPLETE][6] ([i915#5681]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/fi-bdw-5557u/igt@i915_selftest@l...@vma.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-bdw-5557u/igt@i915_selftest@l...@vma.html * igt@runner@aborted: - fi-kbl-soraka: NOTRUN -> [FAIL][7] ([i915#4312] / [i915#5257]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-kbl-soraka/igt@run...@aborted.html Possible fixes * igt@i915_pm_rpm@basic-rte: - fi-cfl-8109u: [DMESG-WARN][8] ([i915#1888] / [i915#62]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/fi-cfl-8109u/igt@i915_pm_...@basic-rte.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-cfl-8109u/igt@i915_pm_...@basic-rte.html * igt@i915_selftest@live@gt_timelines: - {bat-dg2-9}:[DMESG-WARN][10] ([i915#5763]) -> [PASS][11] +2 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/bat-dg2-9/igt@i915_selftest@live@gt_timelines.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/bat-dg2-9/igt@i915_selftest@live@gt_timelines.html * igt@kms_frontbuffer_tracking@basic: - fi-cfl-8109u: [DMESG-WARN][12] ([i915#62]) -> [PASS][13] +15 similar issues [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11727/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104760v1/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#3003]: https://gitlab.freedesktop.org/drm/intel/issues/3003 [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257 [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533 [i915#5681]: https://gitlab.freedesktop.org/drm/intel/issues/5681 [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763 [i915#5885]: https://gitlab.freedesktop.org/drm/intel/issues/5885 [i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903 [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62 [i915#794]: https://gitlab.freedesktop.org/drm/intel/issues/794 Build changes - * Linux: CI_DRM_11727 -> Patchwork_104760v1 CI-20190529: 20190529 CI_DRM_11727: 6034ccb11971698ace6b3af2f6ac02de120a2dc2 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6505: edb1a467fb622b23b927e28ff603fa43851fea97 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_104760v1: 6034ccb1197