* [PATCH v12 0/6] Use drm_clflush* instead of clflush @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel This patch series re-work a few i915 functions to use drm_clflush_virt_range instead of calling clflush or clflushopt directly. This will prevent errors when building for non-x86 architectures. v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added more patches to convert additional clflush/clflushopt to use drm_clflush*. (Michael Cheng) v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran v4: Remove extra memory barriers v5: s/cache_clflush_range/drm_clflush_virt_range v6: Fix up "Drop invalidate_csb_entries" to use correct parameters. Also added in arm64 support for drm_clflush_virt_range. v7: Re-order patches, and use correct macro for dcache flush for arm64. v8: Remove ifdef for asm/cacheflush. v9: Rebased v10: Replaced asm/cacheflush with linux/cacheflush v11: Correctly get the sizeof certian addresses. Also rebased to the latest. v12: Drop include of cacheflush.h and use caches_clean_inval_pou instead of dcache_clean_inval_poc, since it is not exported for other modules to use. Michael Cheng (6): drm: Add arch arm64 for drm_clflush_virt_range drm/i915/gt: Re-work intel_write_status_page drm/i915/gt: Drop invalidate_csb_entries drm/i915/gt: Re-work reset_csb drm/i915/: Re-work clflush_write32 drm/i915/gt: replace cache_clflush_range drivers/gpu/drm/drm_cache.c | 5 +++++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++----- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 +++++------ drivers/gpu/drm/i915/gt/intel_engine.h | 13 ++++-------- .../drm/i915/gt/intel_execlists_submission.c | 20 +++++++------------ drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 8 files changed, 28 insertions(+), 36 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 0/6] Use drm_clflush* instead of clflush @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel This patch series re-work a few i915 functions to use drm_clflush_virt_range instead of calling clflush or clflushopt directly. This will prevent errors when building for non-x86 architectures. v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added more patches to convert additional clflush/clflushopt to use drm_clflush*. (Michael Cheng) v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran v4: Remove extra memory barriers v5: s/cache_clflush_range/drm_clflush_virt_range v6: Fix up "Drop invalidate_csb_entries" to use correct parameters. Also added in arm64 support for drm_clflush_virt_range. v7: Re-order patches, and use correct macro for dcache flush for arm64. v8: Remove ifdef for asm/cacheflush. v9: Rebased v10: Replaced asm/cacheflush with linux/cacheflush v11: Correctly get the sizeof certian addresses. Also rebased to the latest. v12: Drop include of cacheflush.h and use caches_clean_inval_pou instead of dcache_clean_inval_poc, since it is not exported for other modules to use. Michael Cheng (6): drm: Add arch arm64 for drm_clflush_virt_range drm/i915/gt: Re-work intel_write_status_page drm/i915/gt: Drop invalidate_csb_entries drm/i915/gt: Re-work reset_csb drm/i915/: Re-work clflush_write32 drm/i915/gt: replace cache_clflush_range drivers/gpu/drm/drm_cache.c | 5 +++++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++----- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 +++++------ drivers/gpu/drm/i915/gt/intel_engine.h | 13 ++++-------- .../drm/i915/gt/intel_execlists_submission.c | 20 +++++++------------ drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 8 files changed, 28 insertions(+), 36 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou performs a flush by first performing a clean, follow by an invalidation operation. v2 (Michael Cheng): Use correct macro for cleaning and invalidation the dcache. Thanks Tvrtko for the suggestion. v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a symbol that could be use by other modules, thus use caches_clean_inval_pou instead. Also this version removes include for cacheflush, since its already included base on architecture type. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/drm_cache.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index c3e6e615bf09..81c28714f930 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); + +#elif defined(CONFIG_ARM64) + void *end = addr + length; + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); + #else WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou performs a flush by first performing a clean, follow by an invalidation operation. v2 (Michael Cheng): Use correct macro for cleaning and invalidation the dcache. Thanks Tvrtko for the suggestion. v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a symbol that could be use by other modules, thus use caches_clean_inval_pou instead. Also this version removes include for cacheflush, since its already included base on architecture type. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/drm_cache.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index c3e6e615bf09..81c28714f930 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); + +#elif defined(CONFIG_ARM64) + void *end = addr + length; + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); + #else WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 16:28 ` Tvrtko Ursulin -1 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 16:28 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel On 25/02/2022 03:24, Michael Cheng wrote: > Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > performs a flush by first performing a clean, follow by an invalidation > operation. > > v2 (Michael Cheng): Use correct macro for cleaning and invalidation the > dcache. Thanks Tvrtko for the suggestion. > > v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > > v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > symbol that could be use by other modules, thus use > caches_clean_inval_pou instead. Also this version > removes include for cacheflush, since its already > included base on architecture type. What does it mean that it is included based on architecture type? Some of the other header already pulls it in? Regards, Tvrtko > Signed-off-by: Michael Cheng <michael.cheng@intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index c3e6e615bf09..81c28714f930 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) > > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > + > +#elif defined(CONFIG_ARM64) > + void *end = addr + length; > + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > + > #else > WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 16:28 ` Tvrtko Ursulin 0 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 16:28 UTC (permalink / raw) To: Michael Cheng, intel-gfx; +Cc: lucas.demarchi, dri-devel On 25/02/2022 03:24, Michael Cheng wrote: > Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > performs a flush by first performing a clean, follow by an invalidation > operation. > > v2 (Michael Cheng): Use correct macro for cleaning and invalidation the > dcache. Thanks Tvrtko for the suggestion. > > v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > > v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > symbol that could be use by other modules, thus use > caches_clean_inval_pou instead. Also this version > removes include for cacheflush, since its already > included base on architecture type. What does it mean that it is included based on architecture type? Some of the other header already pulls it in? Regards, Tvrtko > Signed-off-by: Michael Cheng <michael.cheng@intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index c3e6e615bf09..81c28714f930 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) > > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > + > +#elif defined(CONFIG_ARM64) > + void *end = addr + length; > + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > + > #else > WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 16:28 ` [Intel-gfx] " Tvrtko Ursulin @ 2022-02-25 16:52 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 16:52 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel [-- Attachment #1: Type: text/plain, Size: 2578 bytes --] Hi Tvrtko, It seems without cacheflush.h being included, when I build for arm64 or x86, it stills pulls in cacheflush.h: ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ So it seems without including it, cacheflush.h stills get pulled in, I think its because its a required kernel source to build the kernel per specific architecture, but please correct if I am wrong,as I am still trying to understand how things works! Michael Cheng On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 03:24, Michael Cheng wrote: >> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >> performs a flush by first performing a clean, follow by an invalidation >> operation. >> >> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >> dcache. Thanks Tvrtko for the suggestion. >> >> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >> >> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >> symbol that could be use by other modules, thus use >> caches_clean_inval_pou instead. Also this version >> removes include for cacheflush, since its already >> included base on architecture type. > > What does it mean that it is included based on architecture type? Some > of the other header already pulls it in? > > Regards, > > Tvrtko > >> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/drm_cache.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >> index c3e6e615bf09..81c28714f930 100644 >> --- a/drivers/gpu/drm/drm_cache.c >> +++ b/drivers/gpu/drm/drm_cache.c >> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >> length) >> if (wbinvd_on_all_cpus()) >> pr_err("Timed out waiting for cache flush\n"); >> + >> +#elif defined(CONFIG_ARM64) >> + void *end = addr + length; >> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >> + >> #else >> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >> #endif [-- Attachment #2: Type: text/html, Size: 7847 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 16:52 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 16:52 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx; +Cc: lucas.demarchi, dri-devel [-- Attachment #1: Type: text/plain, Size: 2578 bytes --] Hi Tvrtko, It seems without cacheflush.h being included, when I build for arm64 or x86, it stills pulls in cacheflush.h: ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ So it seems without including it, cacheflush.h stills get pulled in, I think its because its a required kernel source to build the kernel per specific architecture, but please correct if I am wrong,as I am still trying to understand how things works! Michael Cheng On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 03:24, Michael Cheng wrote: >> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >> performs a flush by first performing a clean, follow by an invalidation >> operation. >> >> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >> dcache. Thanks Tvrtko for the suggestion. >> >> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >> >> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >> symbol that could be use by other modules, thus use >> caches_clean_inval_pou instead. Also this version >> removes include for cacheflush, since its already >> included base on architecture type. > > What does it mean that it is included based on architecture type? Some > of the other header already pulls it in? > > Regards, > > Tvrtko > >> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/drm_cache.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >> index c3e6e615bf09..81c28714f930 100644 >> --- a/drivers/gpu/drm/drm_cache.c >> +++ b/drivers/gpu/drm/drm_cache.c >> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >> length) >> if (wbinvd_on_all_cpus()) >> pr_err("Timed out waiting for cache flush\n"); >> + >> +#elif defined(CONFIG_ARM64) >> + void *end = addr + length; >> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >> + >> #else >> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >> #endif [-- Attachment #2: Type: text/html, Size: 7847 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 16:52 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 17:33 ` Tvrtko Ursulin -1 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 17:33 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel On 25/02/2022 16:52, Michael Cheng wrote: > Hi Tvrtko, > > It seems without cacheflush.h being included, when I build for arm64 or > x86, it stills pulls in cacheflush.h: > > ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ > ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ > ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ > ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ > ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ > ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ > So it seems without including it, cacheflush.h stills get pulled in, > I think its because its a required kernel source to build the kernel > per specific architecture, but please correct if I am wrong,as I am still > trying to understand how things works! Probably: drm_cache.c: #include <linux/highmem.h> linux/highmem.h: #include <linux/cacheflush.h> But it is more correct to explicitly include what you use. So if drm_cache.c uses stuff declared in cacheflush.h, it should include it. Regards, Tvrtko > Michael Cheng > On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 03:24, Michael Cheng wrote: >>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>> performs a flush by first performing a clean, follow by an invalidation >>> operation. >>> >>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >>> dcache. Thanks Tvrtko for the suggestion. >>> >>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>> >>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>> symbol that could be use by other modules, thus use >>> caches_clean_inval_pou instead. Also this version >>> removes include for cacheflush, since its already >>> included base on architecture type. >> >> What does it mean that it is included based on architecture type? Some >> of the other header already pulls it in? >> >> Regards, >> >> Tvrtko >> >>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>> --- >>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>> index c3e6e615bf09..81c28714f930 100644 >>> --- a/drivers/gpu/drm/drm_cache.c >>> +++ b/drivers/gpu/drm/drm_cache.c >>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >>> length) >>> if (wbinvd_on_all_cpus()) >>> pr_err("Timed out waiting for cache flush\n"); >>> + >>> +#elif defined(CONFIG_ARM64) >>> + void *end = addr + length; >>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>> + >>> #else >>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 17:33 ` Tvrtko Ursulin 0 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 17:33 UTC (permalink / raw) To: Michael Cheng, intel-gfx; +Cc: lucas.demarchi, dri-devel On 25/02/2022 16:52, Michael Cheng wrote: > Hi Tvrtko, > > It seems without cacheflush.h being included, when I build for arm64 or > x86, it stills pulls in cacheflush.h: > > ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ > ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ > ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ > ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ > ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ > ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ > So it seems without including it, cacheflush.h stills get pulled in, > I think its because its a required kernel source to build the kernel > per specific architecture, but please correct if I am wrong,as I am still > trying to understand how things works! Probably: drm_cache.c: #include <linux/highmem.h> linux/highmem.h: #include <linux/cacheflush.h> But it is more correct to explicitly include what you use. So if drm_cache.c uses stuff declared in cacheflush.h, it should include it. Regards, Tvrtko > Michael Cheng > On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 03:24, Michael Cheng wrote: >>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>> performs a flush by first performing a clean, follow by an invalidation >>> operation. >>> >>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >>> dcache. Thanks Tvrtko for the suggestion. >>> >>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>> >>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>> symbol that could be use by other modules, thus use >>> caches_clean_inval_pou instead. Also this version >>> removes include for cacheflush, since its already >>> included base on architecture type. >> >> What does it mean that it is included based on architecture type? Some >> of the other header already pulls it in? >> >> Regards, >> >> Tvrtko >> >>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>> --- >>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>> index c3e6e615bf09..81c28714f930 100644 >>> --- a/drivers/gpu/drm/drm_cache.c >>> +++ b/drivers/gpu/drm/drm_cache.c >>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >>> length) >>> if (wbinvd_on_all_cpus()) >>> pr_err("Timed out waiting for cache flush\n"); >>> + >>> +#elif defined(CONFIG_ARM64) >>> + void *end = addr + length; >>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>> + >>> #else >>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 17:33 ` [Intel-gfx] " Tvrtko Ursulin @ 2022-02-25 17:40 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 17:40 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Ah, thanks for pointing that out, when I do include it though, it causes a few warning other systems such as h8300 and s390. Since it is already pulled is, would it be OK to leave it out for this case? Or we could use something like !IS_H8300 and !IS_S390 around the header file? On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 16:52, Michael Cheng wrote: >> Hi Tvrtko, >> >> It seems without cacheflush.h being included, when I build for arm64 >> or x86, it stills pulls in cacheflush.h: >> >> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >> So it seems without including it, cacheflush.h stills get pulled in, >> I think its because its a required kernel source to build the kernel >> per specific architecture, but please correct if I am wrong,as I am >> still >> trying to understand how things works! > > Probably: > > drm_cache.c: > > #include <linux/highmem.h> > > linux/highmem.h: > > #include <linux/cacheflush.h> > > But it is more correct to explicitly include what you use. So if > drm_cache.c uses stuff declared in cacheflush.h, it should include it. > > Regards, > > Tvrtko > >> Michael Cheng >> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>> >>> On 25/02/2022 03:24, Michael Cheng wrote: >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>> performs a flush by first performing a clean, follow by an >>>> invalidation >>>> operation. >>>> >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>> the >>>> dcache. Thanks Tvrtko for the suggestion. >>>> >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>> >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>> symbol that could be use by other modules, thus use >>>> caches_clean_inval_pou instead. Also this version >>>> removes include for cacheflush, since its already >>>> included base on architecture type. >>> >>> What does it mean that it is included based on architecture type? >>> Some of the other header already pulls it in? >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>> index c3e6e615bf09..81c28714f930 100644 >>>> --- a/drivers/gpu/drm/drm_cache.c >>>> +++ b/drivers/gpu/drm/drm_cache.c >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>> long length) >>>> if (wbinvd_on_all_cpus()) >>>> pr_err("Timed out waiting for cache flush\n"); >>>> + >>>> +#elif defined(CONFIG_ARM64) >>>> + void *end = addr + length; >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>> + >>>> #else >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 17:40 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 17:40 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx; +Cc: lucas.demarchi, dri-devel Ah, thanks for pointing that out, when I do include it though, it causes a few warning other systems such as h8300 and s390. Since it is already pulled is, would it be OK to leave it out for this case? Or we could use something like !IS_H8300 and !IS_S390 around the header file? On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 16:52, Michael Cheng wrote: >> Hi Tvrtko, >> >> It seems without cacheflush.h being included, when I build for arm64 >> or x86, it stills pulls in cacheflush.h: >> >> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >> So it seems without including it, cacheflush.h stills get pulled in, >> I think its because its a required kernel source to build the kernel >> per specific architecture, but please correct if I am wrong,as I am >> still >> trying to understand how things works! > > Probably: > > drm_cache.c: > > #include <linux/highmem.h> > > linux/highmem.h: > > #include <linux/cacheflush.h> > > But it is more correct to explicitly include what you use. So if > drm_cache.c uses stuff declared in cacheflush.h, it should include it. > > Regards, > > Tvrtko > >> Michael Cheng >> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>> >>> On 25/02/2022 03:24, Michael Cheng wrote: >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>> performs a flush by first performing a clean, follow by an >>>> invalidation >>>> operation. >>>> >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>> the >>>> dcache. Thanks Tvrtko for the suggestion. >>>> >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>> >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>> symbol that could be use by other modules, thus use >>>> caches_clean_inval_pou instead. Also this version >>>> removes include for cacheflush, since its already >>>> included base on architecture type. >>> >>> What does it mean that it is included based on architecture type? >>> Some of the other header already pulls it in? >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>> index c3e6e615bf09..81c28714f930 100644 >>>> --- a/drivers/gpu/drm/drm_cache.c >>>> +++ b/drivers/gpu/drm/drm_cache.c >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>> long length) >>>> if (wbinvd_on_all_cpus()) >>>> pr_err("Timed out waiting for cache flush\n"); >>>> + >>>> +#elif defined(CONFIG_ARM64) >>>> + void *end = addr + length; >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>> + >>>> #else >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 17:40 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 18:19 ` Tvrtko Ursulin -1 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 18:19 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel On 25/02/2022 17:40, Michael Cheng wrote: > Ah, thanks for pointing that out, when I do include it though, it causes > a few warning other systems such as h8300 and s390. Errors look like? I haven't heard that kernel code is not allowed to include something from linux/ on some arch yet. > Since it is already pulled is, would it be OK to leave it out for this > case? Or we could use something like !IS_H8300 and !IS_S390 > > around the header file? Unlikely, now you made me curious why it does not work. Regards, Tvrtko > > On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 16:52, Michael Cheng wrote: >>> Hi Tvrtko, >>> >>> It seems without cacheflush.h being included, when I build for arm64 >>> or x86, it stills pulls in cacheflush.h: >>> >>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>> So it seems without including it, cacheflush.h stills get pulled in, >>> I think its because its a required kernel source to build the kernel >>> per specific architecture, but please correct if I am wrong,as I am >>> still >>> trying to understand how things works! >> >> Probably: >> >> drm_cache.c: >> >> #include <linux/highmem.h> >> >> linux/highmem.h: >> >> #include <linux/cacheflush.h> >> >> But it is more correct to explicitly include what you use. So if >> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >> >> Regards, >> >> Tvrtko >> >>> Michael Cheng >>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>> >>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>> performs a flush by first performing a clean, follow by an >>>>> invalidation >>>>> operation. >>>>> >>>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>>> the >>>>> dcache. Thanks Tvrtko for the suggestion. >>>>> >>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>> >>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>>> symbol that could be use by other modules, thus use >>>>> caches_clean_inval_pou instead. Also this version >>>>> removes include for cacheflush, since its already >>>>> included base on architecture type. >>>> >>>> What does it mean that it is included based on architecture type? >>>> Some of the other header already pulls it in? >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>>> index c3e6e615bf09..81c28714f930 100644 >>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>> long length) >>>>> if (wbinvd_on_all_cpus()) >>>>> pr_err("Timed out waiting for cache flush\n"); >>>>> + >>>>> +#elif defined(CONFIG_ARM64) >>>>> + void *end = addr + length; >>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>>> + >>>>> #else >>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:19 ` Tvrtko Ursulin 0 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 18:19 UTC (permalink / raw) To: Michael Cheng, intel-gfx; +Cc: lucas.demarchi, dri-devel On 25/02/2022 17:40, Michael Cheng wrote: > Ah, thanks for pointing that out, when I do include it though, it causes > a few warning other systems such as h8300 and s390. Errors look like? I haven't heard that kernel code is not allowed to include something from linux/ on some arch yet. > Since it is already pulled is, would it be OK to leave it out for this > case? Or we could use something like !IS_H8300 and !IS_S390 > > around the header file? Unlikely, now you made me curious why it does not work. Regards, Tvrtko > > On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 16:52, Michael Cheng wrote: >>> Hi Tvrtko, >>> >>> It seems without cacheflush.h being included, when I build for arm64 >>> or x86, it stills pulls in cacheflush.h: >>> >>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>> So it seems without including it, cacheflush.h stills get pulled in, >>> I think its because its a required kernel source to build the kernel >>> per specific architecture, but please correct if I am wrong,as I am >>> still >>> trying to understand how things works! >> >> Probably: >> >> drm_cache.c: >> >> #include <linux/highmem.h> >> >> linux/highmem.h: >> >> #include <linux/cacheflush.h> >> >> But it is more correct to explicitly include what you use. So if >> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >> >> Regards, >> >> Tvrtko >> >>> Michael Cheng >>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>> >>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>> performs a flush by first performing a clean, follow by an >>>>> invalidation >>>>> operation. >>>>> >>>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>>> the >>>>> dcache. Thanks Tvrtko for the suggestion. >>>>> >>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>> >>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>>> symbol that could be use by other modules, thus use >>>>> caches_clean_inval_pou instead. Also this version >>>>> removes include for cacheflush, since its already >>>>> included base on architecture type. >>>> >>>> What does it mean that it is included based on architecture type? >>>> Some of the other header already pulls it in? >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>>> index c3e6e615bf09..81c28714f930 100644 >>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>> long length) >>>>> if (wbinvd_on_all_cpus()) >>>>> pr_err("Timed out waiting for cache flush\n"); >>>>> + >>>>> +#elif defined(CONFIG_ARM64) >>>>> + void *end = addr + length; >>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>>> + >>>>> #else >>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 18:19 ` [Intel-gfx] " Tvrtko Ursulin @ 2022-02-25 18:23 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 18:23 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel These seem to be pretty old arch and are day0 warnings, please refer to [1] to see the warnings. Also I am not sure why my patch series didn't append to the old one. [1] https://patchwork.freedesktop.org/patch/475829/?series=99450&rev=11 2022-02-25 10:19 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 17:40, Michael Cheng wrote: >> Ah, thanks for pointing that out, when I do include it though, it >> causes a few warning other systems such as h8300 and s390. > > Errors look like? I haven't heard that kernel code is not allowed to > include something from linux/ on some arch yet. > >> Since it is already pulled is, would it be OK to leave it out for >> this case? Or we could use something like !IS_H8300 and !IS_S390 >> >> around the header file? > > Unlikely, now you made me curious why it does not work. > > Regards, > > Tvrtko > >> >> On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >>> >>> On 25/02/2022 16:52, Michael Cheng wrote: >>>> Hi Tvrtko, >>>> >>>> It seems without cacheflush.h being included, when I build for >>>> arm64 or x86, it stills pulls in cacheflush.h: >>>> >>>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>>> So it seems without including it, cacheflush.h stills get pulled in, >>>> I think its because its a required kernel source to build the kernel >>>> per specific architecture, but please correct if I am wrong,as I am >>>> still >>>> trying to understand how things works! >>> >>> Probably: >>> >>> drm_cache.c: >>> >>> #include <linux/highmem.h> >>> >>> linux/highmem.h: >>> >>> #include <linux/cacheflush.h> >>> >>> But it is more correct to explicitly include what you use. So if >>> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Michael Cheng >>>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>>> >>>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>> performs a flush by first performing a clean, follow by an >>>>>> invalidation >>>>>> operation. >>>>>> >>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>> invalidation the >>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>> >>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>> >>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>> as a >>>>>> symbol that could be use by other modules, thus use >>>>>> caches_clean_inval_pou instead. Also this version >>>>>> removes include for cacheflush, since its already >>>>>> included base on architecture type. >>>>> >>>>> What does it mean that it is included based on architecture type? >>>>> Some of the other header already pulls it in? >>>>> >>>>> Regards, >>>>> >>>>> Tvrtko >>>>> >>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>> long length) >>>>>> if (wbinvd_on_all_cpus()) >>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>> + >>>>>> +#elif defined(CONFIG_ARM64) >>>>>> + void *end = addr + length; >>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>> long)end); >>>>>> + >>>>>> #else >>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:23 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 18:23 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx; +Cc: lucas.demarchi, dri-devel These seem to be pretty old arch and are day0 warnings, please refer to [1] to see the warnings. Also I am not sure why my patch series didn't append to the old one. [1] https://patchwork.freedesktop.org/patch/475829/?series=99450&rev=11 2022-02-25 10:19 a.m., Tvrtko Ursulin wrote: > > On 25/02/2022 17:40, Michael Cheng wrote: >> Ah, thanks for pointing that out, when I do include it though, it >> causes a few warning other systems such as h8300 and s390. > > Errors look like? I haven't heard that kernel code is not allowed to > include something from linux/ on some arch yet. > >> Since it is already pulled is, would it be OK to leave it out for >> this case? Or we could use something like !IS_H8300 and !IS_S390 >> >> around the header file? > > Unlikely, now you made me curious why it does not work. > > Regards, > > Tvrtko > >> >> On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >>> >>> On 25/02/2022 16:52, Michael Cheng wrote: >>>> Hi Tvrtko, >>>> >>>> It seems without cacheflush.h being included, when I build for >>>> arm64 or x86, it stills pulls in cacheflush.h: >>>> >>>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>>> So it seems without including it, cacheflush.h stills get pulled in, >>>> I think its because its a required kernel source to build the kernel >>>> per specific architecture, but please correct if I am wrong,as I am >>>> still >>>> trying to understand how things works! >>> >>> Probably: >>> >>> drm_cache.c: >>> >>> #include <linux/highmem.h> >>> >>> linux/highmem.h: >>> >>> #include <linux/cacheflush.h> >>> >>> But it is more correct to explicitly include what you use. So if >>> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Michael Cheng >>>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>>> >>>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>> performs a flush by first performing a clean, follow by an >>>>>> invalidation >>>>>> operation. >>>>>> >>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>> invalidation the >>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>> >>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>> >>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>> as a >>>>>> symbol that could be use by other modules, thus use >>>>>> caches_clean_inval_pou instead. Also this version >>>>>> removes include for cacheflush, since its already >>>>>> included base on architecture type. >>>>> >>>>> What does it mean that it is included based on architecture type? >>>>> Some of the other header already pulls it in? >>>>> >>>>> Regards, >>>>> >>>>> Tvrtko >>>>> >>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>> long length) >>>>>> if (wbinvd_on_all_cpus()) >>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>> + >>>>>> +#elif defined(CONFIG_ARM64) >>>>>> + void *end = addr + length; >>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>> long)end); >>>>>> + >>>>>> #else >>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 18:23 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 18:42 ` Tvrtko Ursulin -1 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 18:42 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, Matthew Wilcox (Oracle) On 25/02/2022 18:23, Michael Cheng wrote: > These seem to be pretty old arch and are day0 warnings, please refer to > [1] to see the warnings. Also I am not sure why my patch series didn't > append to the old one. > > [1] https://patchwork.freedesktop.org/patch/475829/?series=99450&rev=11 >> include/linux/cacheflush.h:12:46: warning: declaration of 'struct folio' will not be visible outside of this function [-Wvisibility] That? Looks like the #else path needs to forward declare struct folio or include the relevant header. +Matthew Wilcox Matthew, what do you think fix for this build warning on h8300 and s390 should be? Or perhaps a build environment issue with kernel test robot? Regards, Tvrtko > 2022-02-25 10:19 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 17:40, Michael Cheng wrote: >>> Ah, thanks for pointing that out, when I do include it though, it >>> causes a few warning other systems such as h8300 and s390. >> >> Errors look like? I haven't heard that kernel code is not allowed to >> include something from linux/ on some arch yet. >> >>> Since it is already pulled is, would it be OK to leave it out for >>> this case? Or we could use something like !IS_H8300 and !IS_S390 >>> >>> around the header file? >> >> Unlikely, now you made me curious why it does not work. >> >> Regards, >> >> Tvrtko >> >>> >>> On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >>>> >>>> On 25/02/2022 16:52, Michael Cheng wrote: >>>>> Hi Tvrtko, >>>>> >>>>> It seems without cacheflush.h being included, when I build for >>>>> arm64 or x86, it stills pulls in cacheflush.h: >>>>> >>>>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>>>> So it seems without including it, cacheflush.h stills get pulled in, >>>>> I think its because its a required kernel source to build the kernel >>>>> per specific architecture, but please correct if I am wrong,as I am >>>>> still >>>>> trying to understand how things works! >>>> >>>> Probably: >>>> >>>> drm_cache.c: >>>> >>>> #include <linux/highmem.h> >>>> >>>> linux/highmem.h: >>>> >>>> #include <linux/cacheflush.h> >>>> >>>> But it is more correct to explicitly include what you use. So if >>>> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> Michael Cheng >>>>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>>>> >>>>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>>> performs a flush by first performing a clean, follow by an >>>>>>> invalidation >>>>>>> operation. >>>>>>> >>>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>>> invalidation the >>>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>>> >>>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>>> >>>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>>> as a >>>>>>> symbol that could be use by other modules, thus use >>>>>>> caches_clean_inval_pou instead. Also this version >>>>>>> removes include for cacheflush, since its already >>>>>>> included base on architecture type. >>>>>> >>>>>> What does it mean that it is included based on architecture type? >>>>>> Some of the other header already pulls it in? >>>>>> >>>>>> Regards, >>>>>> >>>>>> Tvrtko >>>>>> >>>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>>> 1 file changed, 5 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>>> long length) >>>>>>> if (wbinvd_on_all_cpus()) >>>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>>> + >>>>>>> +#elif defined(CONFIG_ARM64) >>>>>>> + void *end = addr + length; >>>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>>> long)end); >>>>>>> + >>>>>>> #else >>>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:42 ` Tvrtko Ursulin 0 siblings, 0 replies; 55+ messages in thread From: Tvrtko Ursulin @ 2022-02-25 18:42 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: lucas.demarchi, dri-devel, Matthew Wilcox (Oracle) On 25/02/2022 18:23, Michael Cheng wrote: > These seem to be pretty old arch and are day0 warnings, please refer to > [1] to see the warnings. Also I am not sure why my patch series didn't > append to the old one. > > [1] https://patchwork.freedesktop.org/patch/475829/?series=99450&rev=11 >> include/linux/cacheflush.h:12:46: warning: declaration of 'struct folio' will not be visible outside of this function [-Wvisibility] That? Looks like the #else path needs to forward declare struct folio or include the relevant header. +Matthew Wilcox Matthew, what do you think fix for this build warning on h8300 and s390 should be? Or perhaps a build environment issue with kernel test robot? Regards, Tvrtko > 2022-02-25 10:19 a.m., Tvrtko Ursulin wrote: >> >> On 25/02/2022 17:40, Michael Cheng wrote: >>> Ah, thanks for pointing that out, when I do include it though, it >>> causes a few warning other systems such as h8300 and s390. >> >> Errors look like? I haven't heard that kernel code is not allowed to >> include something from linux/ on some arch yet. >> >>> Since it is already pulled is, would it be OK to leave it out for >>> this case? Or we could use something like !IS_H8300 and !IS_S390 >>> >>> around the header file? >> >> Unlikely, now you made me curious why it does not work. >> >> Regards, >> >> Tvrtko >> >>> >>> On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote: >>>> >>>> On 25/02/2022 16:52, Michael Cheng wrote: >>>>> Hi Tvrtko, >>>>> >>>>> It seems without cacheflush.h being included, when I build for >>>>> arm64 or x86, it stills pulls in cacheflush.h: >>>>> >>>>> ./.drm_cache.o.cmd:838: include/linux/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:830: include/linux/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \ >>>>> ./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \ >>>>> So it seems without including it, cacheflush.h stills get pulled in, >>>>> I think its because its a required kernel source to build the kernel >>>>> per specific architecture, but please correct if I am wrong,as I am >>>>> still >>>>> trying to understand how things works! >>>> >>>> Probably: >>>> >>>> drm_cache.c: >>>> >>>> #include <linux/highmem.h> >>>> >>>> linux/highmem.h: >>>> >>>> #include <linux/cacheflush.h> >>>> >>>> But it is more correct to explicitly include what you use. So if >>>> drm_cache.c uses stuff declared in cacheflush.h, it should include it. >>>> >>>> Regards, >>>> >>>> Tvrtko >>>> >>>>> Michael Cheng >>>>> On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote: >>>>>> >>>>>> On 25/02/2022 03:24, Michael Cheng wrote: >>>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>>> performs a flush by first performing a clean, follow by an >>>>>>> invalidation >>>>>>> operation. >>>>>>> >>>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>>> invalidation the >>>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>>> >>>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>>> >>>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>>> as a >>>>>>> symbol that could be use by other modules, thus use >>>>>>> caches_clean_inval_pou instead. Also this version >>>>>>> removes include for cacheflush, since its already >>>>>>> included base on architecture type. >>>>>> >>>>>> What does it mean that it is included based on architecture type? >>>>>> Some of the other header already pulls it in? >>>>>> >>>>>> Regards, >>>>>> >>>>>> Tvrtko >>>>>> >>>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>>> 1 file changed, 5 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>>> long length) >>>>>>> if (wbinvd_on_all_cpus()) >>>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>>> + >>>>>>> +#elif defined(CONFIG_ARM64) >>>>>>> + void *end = addr + length; >>>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>>> long)end); >>>>>>> + >>>>>>> #else >>>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 18:42 ` [Intel-gfx] " Tvrtko Ursulin @ 2022-02-25 18:58 ` Matthew Wilcox -1 siblings, 0 replies; 55+ messages in thread From: Matthew Wilcox @ 2022-02-25 18:58 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Michael Cheng, balasubramani.vivekanandan, wayne.boyer, intel-gfx, casey.g.bowman, lucas.demarchi, dri-devel On Fri, Feb 25, 2022 at 06:42:37PM +0000, Tvrtko Ursulin wrote: > Matthew, what do you think fix for this build warning on h8300 and s390 should be? Or perhaps a build environment issue with kernel test robot? I'd suggest this should do the job: +++ b/include/linux/cacheflush.h @@ -4,6 +4,8 @@ #include <asm/cacheflush.h> +struct folio; + #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_FOLIO void flush_dcache_folio(struct folio *folio); ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:58 ` Matthew Wilcox 0 siblings, 0 replies; 55+ messages in thread From: Matthew Wilcox @ 2022-02-25 18:58 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Michael Cheng, intel-gfx, lucas.demarchi, dri-devel On Fri, Feb 25, 2022 at 06:42:37PM +0000, Tvrtko Ursulin wrote: > Matthew, what do you think fix for this build warning on h8300 and s390 should be? Or perhaps a build environment issue with kernel test robot? I'd suggest this should do the job: +++ b/include/linux/cacheflush.h @@ -4,6 +4,8 @@ #include <asm/cacheflush.h> +struct folio; + #if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE #ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_FOLIO void flush_dcache_folio(struct folio *folio); ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng (?) @ 2022-02-25 18:24 ` Robin Murphy -1 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-02-25 18:24 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel [ +arm64 maintainers for their awareness, which would have been a good thing to do from the start ] On 2022-02-25 03:24, Michael Cheng wrote: > Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > performs a flush by first performing a clean, follow by an invalidation > operation. > > v2 (Michael Cheng): Use correct macro for cleaning and invalidation the > dcache. Thanks Tvrtko for the suggestion. > > v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > > v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > symbol that could be use by other modules, thus use > caches_clean_inval_pou instead. Also this version > removes include for cacheflush, since its already > included base on architecture type. > > Signed-off-by: Michael Cheng <michael.cheng@intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index c3e6e615bf09..81c28714f930 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) > > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > + > +#elif defined(CONFIG_ARM64) > + void *end = addr + length; > + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); Why does i915 need to ensure the CPU's instruction cache is coherent with its data cache? Is it a self-modifying driver? Robin. (Note that the above is somewhat of a loaded question, and I do actually have half an idea of what you're trying to do here and why it won't fly, but I'd like to at least assume you've read the documentation of the function you decided was OK to use) > + > #else > WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:24 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-02-25 18:24 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel [ +arm64 maintainers for their awareness, which would have been a good thing to do from the start ] On 2022-02-25 03:24, Michael Cheng wrote: > Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > performs a flush by first performing a clean, follow by an invalidation > operation. > > v2 (Michael Cheng): Use correct macro for cleaning and invalidation the > dcache. Thanks Tvrtko for the suggestion. > > v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > > v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > symbol that could be use by other modules, thus use > caches_clean_inval_pou instead. Also this version > removes include for cacheflush, since its already > included base on architecture type. > > Signed-off-by: Michael Cheng <michael.cheng@intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index c3e6e615bf09..81c28714f930 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) > > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > + > +#elif defined(CONFIG_ARM64) > + void *end = addr + length; > + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); Why does i915 need to ensure the CPU's instruction cache is coherent with its data cache? Is it a self-modifying driver? Robin. (Note that the above is somewhat of a loaded question, and I do actually have half an idea of what you're trying to do here and why it won't fly, but I'd like to at least assume you've read the documentation of the function you decided was OK to use) > + > #else > WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 18:24 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-02-25 18:24 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, linux-arm-kernel, Will Deacon, Catalin Marinas [ +arm64 maintainers for their awareness, which would have been a good thing to do from the start ] On 2022-02-25 03:24, Michael Cheng wrote: > Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > performs a flush by first performing a clean, follow by an invalidation > operation. > > v2 (Michael Cheng): Use correct macro for cleaning and invalidation the > dcache. Thanks Tvrtko for the suggestion. > > v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > > v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > symbol that could be use by other modules, thus use > caches_clean_inval_pou instead. Also this version > removes include for cacheflush, since its already > included base on architecture type. > > Signed-off-by: Michael Cheng <michael.cheng@intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/drm_cache.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index c3e6e615bf09..81c28714f930 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length) > > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > + > +#elif defined(CONFIG_ARM64) > + void *end = addr + length; > + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); Why does i915 need to ensure the CPU's instruction cache is coherent with its data cache? Is it a self-modifying driver? Robin. (Note that the above is somewhat of a loaded question, and I do actually have half an idea of what you're trying to do here and why it won't fly, but I'd like to at least assume you've read the documentation of the function you decided was OK to use) > + > #else > WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 18:24 ` Robin Murphy @ 2022-02-25 19:27 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 19:27 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 3073 bytes --] Hi Robin, [ +arm64 maintainers for their awareness, which would have been a good thing to do from the start ] * Thanks for adding the arm64 maintainer and sorry I didn't rope them in sooner. Why does i915 need to ensure the CPU's instruction cache is coherent with its data cache? Is it a self-modifying driver? * Also thanks for pointing this out. Initially I was using dcache_clean_inval_poc, which seem to be the equivalently to what x86 is doing for dcache flushing, but it was giving me build errors since its not on the global list of kernel symbols. And after revisiting the documentation for caches_clean_inval_pou, it won't fly for what we are trying to do. Moving forward, what would you (or someone in the ARM community) suggest we do? Could it be possible to export dcache_clean_inval_poc as a global symbol? On 2022-02-25 10:24 a.m., Robin Murphy wrote: > [ +arm64 maintainers for their awareness, which would have been a good > thing to do from the start ] > > On 2022-02-25 03:24, Michael Cheng wrote: >> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >> performs a flush by first performing a clean, follow by an invalidation >> operation. >> >> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >> dcache. Thanks Tvrtko for the suggestion. >> >> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >> >> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >> symbol that could be use by other modules, thus use >> caches_clean_inval_pou instead. Also this version >> removes include for cacheflush, since its already >> included base on architecture type. >> >> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/drm_cache.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >> index c3e6e615bf09..81c28714f930 100644 >> --- a/drivers/gpu/drm/drm_cache.c >> +++ b/drivers/gpu/drm/drm_cache.c >> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >> length) >> if (wbinvd_on_all_cpus()) >> pr_err("Timed out waiting for cache flush\n"); >> + >> +#elif defined(CONFIG_ARM64) >> + void *end = addr + length; >> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > > Why does i915 need to ensure the CPU's instruction cache is coherent > with its data cache? Is it a self-modifying driver? > > Robin. > > (Note that the above is somewhat of a loaded question, and I do > actually have half an idea of what you're trying to do here and why it > won't fly, but I'd like to at least assume you've read the > documentation of the function you decided was OK to use) > >> + >> #else >> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >> #endif [-- Attachment #2: Type: text/html, Size: 4868 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-02-25 19:27 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 19:27 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 3073 bytes --] Hi Robin, [ +arm64 maintainers for their awareness, which would have been a good thing to do from the start ] * Thanks for adding the arm64 maintainer and sorry I didn't rope them in sooner. Why does i915 need to ensure the CPU's instruction cache is coherent with its data cache? Is it a self-modifying driver? * Also thanks for pointing this out. Initially I was using dcache_clean_inval_poc, which seem to be the equivalently to what x86 is doing for dcache flushing, but it was giving me build errors since its not on the global list of kernel symbols. And after revisiting the documentation for caches_clean_inval_pou, it won't fly for what we are trying to do. Moving forward, what would you (or someone in the ARM community) suggest we do? Could it be possible to export dcache_clean_inval_poc as a global symbol? On 2022-02-25 10:24 a.m., Robin Murphy wrote: > [ +arm64 maintainers for their awareness, which would have been a good > thing to do from the start ] > > On 2022-02-25 03:24, Michael Cheng wrote: >> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >> performs a flush by first performing a clean, follow by an invalidation >> operation. >> >> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >> dcache. Thanks Tvrtko for the suggestion. >> >> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >> >> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >> symbol that could be use by other modules, thus use >> caches_clean_inval_pou instead. Also this version >> removes include for cacheflush, since its already >> included base on architecture type. >> >> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> --- >> drivers/gpu/drm/drm_cache.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >> index c3e6e615bf09..81c28714f930 100644 >> --- a/drivers/gpu/drm/drm_cache.c >> +++ b/drivers/gpu/drm/drm_cache.c >> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >> length) >> if (wbinvd_on_all_cpus()) >> pr_err("Timed out waiting for cache flush\n"); >> + >> +#elif defined(CONFIG_ARM64) >> + void *end = addr + length; >> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > > Why does i915 need to ensure the CPU's instruction cache is coherent > with its data cache? Is it a self-modifying driver? > > Robin. > > (Note that the above is somewhat of a loaded question, and I do > actually have half an idea of what you're trying to do here and why it > won't fly, but I'd like to at least assume you've read the > documentation of the function you decided was OK to use) > >> + >> #else >> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >> #endif [-- Attachment #2: Type: text/html, Size: 4868 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-02-25 19:27 ` [Intel-gfx] " Michael Cheng (?) @ 2022-03-02 12:49 ` Robin Murphy -1 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 12:49 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel On 2022-02-25 19:27, Michael Cheng wrote: > Hi Robin, > > [ +arm64 maintainers for their awareness, which would have been a good > thing to do from the start ] > > * Thanks for adding the arm64 maintainer and sorry I didn't rope them > in sooner. > > Why does i915 need to ensure the CPU's instruction cache is coherent > with its data cache? Is it a self-modifying driver? > > * Also thanks for pointing this out. Initially I was using > dcache_clean_inval_poc, which seem to be the equivalently to what > x86 is doing for dcache flushing, but it was giving me build errors > since its not on the global list of kernel symbols. And after > revisiting the documentation for caches_clean_inval_pou, it won't > fly for what we are trying to do. Moving forward, what would you (or > someone in the ARM community) suggest we do? Could it be possible to > export dcache_clean_inval_poc as a global symbol? Unlikely, unless something with a legitimate need for CPU-centric cache maintenance like kexec or CPU hotplug ever becomes modular. In the case of a device driver, it's not even the basic issues of assuming to find direct equivalents to x86 semantics in other CPU architectures, or effectively reinventing parts of the DMA API, it's even bigger than that. Once you move from being integrated in a single vendor's system architecture to being on a discrete card, you fundamentally *no longer have any control over cache coherency*. Whether the host CPU architecture happens to be AArch64, RISC-V, or whatever doesn't really matter, you're at the mercy of 3rd-party PCIe and interconnect IP vendors, and SoC integrators. You'll find yourself in systems where PCIe simply cannot snoop any caches, where you'd better have the correct DMA API calls in place to have any hope of even the most basic functionality working properly; you'll find yourself in systems where even if the PCIe root complex claims to support No Snoop, your uncached traffic will still end up snooping stale data that got prefetched back into caches you thought you'd invalidated; you'll find yourself in systems where your memory attributes may or may not get forcibly rewritten by an IOMMU depending on the kernel config and/or command line. It's not about simply finding a substitute for clflush, it's that the reasons you have for using clflush in the first place can no longer be assumed to be valid. Robin. > On 2022-02-25 10:24 a.m., Robin Murphy wrote: >> [ +arm64 maintainers for their awareness, which would have been a good >> thing to do from the start ] >> >> On 2022-02-25 03:24, Michael Cheng wrote: >>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>> performs a flush by first performing a clean, follow by an invalidation >>> operation. >>> >>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >>> dcache. Thanks Tvrtko for the suggestion. >>> >>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>> >>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>> symbol that could be use by other modules, thus use >>> caches_clean_inval_pou instead. Also this version >>> removes include for cacheflush, since its already >>> included base on architecture type. >>> >>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>> --- >>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>> index c3e6e615bf09..81c28714f930 100644 >>> --- a/drivers/gpu/drm/drm_cache.c >>> +++ b/drivers/gpu/drm/drm_cache.c >>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >>> length) >>> if (wbinvd_on_all_cpus()) >>> pr_err("Timed out waiting for cache flush\n"); >>> + >>> +#elif defined(CONFIG_ARM64) >>> + void *end = addr + length; >>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> Robin. >> >> (Note that the above is somewhat of a loaded question, and I do >> actually have half an idea of what you're trying to do here and why it >> won't fly, but I'd like to at least assume you've read the >> documentation of the function you decided was OK to use) >> >>> + >>> #else >>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 12:49 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 12:49 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel On 2022-02-25 19:27, Michael Cheng wrote: > Hi Robin, > > [ +arm64 maintainers for their awareness, which would have been a good > thing to do from the start ] > > * Thanks for adding the arm64 maintainer and sorry I didn't rope them > in sooner. > > Why does i915 need to ensure the CPU's instruction cache is coherent > with its data cache? Is it a self-modifying driver? > > * Also thanks for pointing this out. Initially I was using > dcache_clean_inval_poc, which seem to be the equivalently to what > x86 is doing for dcache flushing, but it was giving me build errors > since its not on the global list of kernel symbols. And after > revisiting the documentation for caches_clean_inval_pou, it won't > fly for what we are trying to do. Moving forward, what would you (or > someone in the ARM community) suggest we do? Could it be possible to > export dcache_clean_inval_poc as a global symbol? Unlikely, unless something with a legitimate need for CPU-centric cache maintenance like kexec or CPU hotplug ever becomes modular. In the case of a device driver, it's not even the basic issues of assuming to find direct equivalents to x86 semantics in other CPU architectures, or effectively reinventing parts of the DMA API, it's even bigger than that. Once you move from being integrated in a single vendor's system architecture to being on a discrete card, you fundamentally *no longer have any control over cache coherency*. Whether the host CPU architecture happens to be AArch64, RISC-V, or whatever doesn't really matter, you're at the mercy of 3rd-party PCIe and interconnect IP vendors, and SoC integrators. You'll find yourself in systems where PCIe simply cannot snoop any caches, where you'd better have the correct DMA API calls in place to have any hope of even the most basic functionality working properly; you'll find yourself in systems where even if the PCIe root complex claims to support No Snoop, your uncached traffic will still end up snooping stale data that got prefetched back into caches you thought you'd invalidated; you'll find yourself in systems where your memory attributes may or may not get forcibly rewritten by an IOMMU depending on the kernel config and/or command line. It's not about simply finding a substitute for clflush, it's that the reasons you have for using clflush in the first place can no longer be assumed to be valid. Robin. > On 2022-02-25 10:24 a.m., Robin Murphy wrote: >> [ +arm64 maintainers for their awareness, which would have been a good >> thing to do from the start ] >> >> On 2022-02-25 03:24, Michael Cheng wrote: >>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>> performs a flush by first performing a clean, follow by an invalidation >>> operation. >>> >>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >>> dcache. Thanks Tvrtko for the suggestion. >>> >>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>> >>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>> symbol that could be use by other modules, thus use >>> caches_clean_inval_pou instead. Also this version >>> removes include for cacheflush, since its already >>> included base on architecture type. >>> >>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>> --- >>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>> index c3e6e615bf09..81c28714f930 100644 >>> --- a/drivers/gpu/drm/drm_cache.c >>> +++ b/drivers/gpu/drm/drm_cache.c >>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >>> length) >>> if (wbinvd_on_all_cpus()) >>> pr_err("Timed out waiting for cache flush\n"); >>> + >>> +#elif defined(CONFIG_ARM64) >>> + void *end = addr + length; >>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> Robin. >> >> (Note that the above is somewhat of a loaded question, and I do >> actually have half an idea of what you're trying to do here and why it >> won't fly, but I'd like to at least assume you've read the >> documentation of the function you decided was OK to use) >> >>> + >>> #else >>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 12:49 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 12:49 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, linux-arm-kernel, Will Deacon, Catalin Marinas On 2022-02-25 19:27, Michael Cheng wrote: > Hi Robin, > > [ +arm64 maintainers for their awareness, which would have been a good > thing to do from the start ] > > * Thanks for adding the arm64 maintainer and sorry I didn't rope them > in sooner. > > Why does i915 need to ensure the CPU's instruction cache is coherent > with its data cache? Is it a self-modifying driver? > > * Also thanks for pointing this out. Initially I was using > dcache_clean_inval_poc, which seem to be the equivalently to what > x86 is doing for dcache flushing, but it was giving me build errors > since its not on the global list of kernel symbols. And after > revisiting the documentation for caches_clean_inval_pou, it won't > fly for what we are trying to do. Moving forward, what would you (or > someone in the ARM community) suggest we do? Could it be possible to > export dcache_clean_inval_poc as a global symbol? Unlikely, unless something with a legitimate need for CPU-centric cache maintenance like kexec or CPU hotplug ever becomes modular. In the case of a device driver, it's not even the basic issues of assuming to find direct equivalents to x86 semantics in other CPU architectures, or effectively reinventing parts of the DMA API, it's even bigger than that. Once you move from being integrated in a single vendor's system architecture to being on a discrete card, you fundamentally *no longer have any control over cache coherency*. Whether the host CPU architecture happens to be AArch64, RISC-V, or whatever doesn't really matter, you're at the mercy of 3rd-party PCIe and interconnect IP vendors, and SoC integrators. You'll find yourself in systems where PCIe simply cannot snoop any caches, where you'd better have the correct DMA API calls in place to have any hope of even the most basic functionality working properly; you'll find yourself in systems where even if the PCIe root complex claims to support No Snoop, your uncached traffic will still end up snooping stale data that got prefetched back into caches you thought you'd invalidated; you'll find yourself in systems where your memory attributes may or may not get forcibly rewritten by an IOMMU depending on the kernel config and/or command line. It's not about simply finding a substitute for clflush, it's that the reasons you have for using clflush in the first place can no longer be assumed to be valid. Robin. > On 2022-02-25 10:24 a.m., Robin Murphy wrote: >> [ +arm64 maintainers for their awareness, which would have been a good >> thing to do from the start ] >> >> On 2022-02-25 03:24, Michael Cheng wrote: >>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>> performs a flush by first performing a clean, follow by an invalidation >>> operation. >>> >>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation the >>> dcache. Thanks Tvrtko for the suggestion. >>> >>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>> >>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>> symbol that could be use by other modules, thus use >>> caches_clean_inval_pou instead. Also this version >>> removes include for cacheflush, since its already >>> included base on architecture type. >>> >>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>> --- >>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>> index c3e6e615bf09..81c28714f930 100644 >>> --- a/drivers/gpu/drm/drm_cache.c >>> +++ b/drivers/gpu/drm/drm_cache.c >>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long >>> length) >>> if (wbinvd_on_all_cpus()) >>> pr_err("Timed out waiting for cache flush\n"); >>> + >>> +#elif defined(CONFIG_ARM64) >>> + void *end = addr + length; >>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> Robin. >> >> (Note that the above is somewhat of a loaded question, and I do >> actually have half an idea of what you're trying to do here and why it >> won't fly, but I'd like to at least assume you've read the >> documentation of the function you decided was OK to use) >> >>> + >>> #else >>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>> #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-03-02 12:49 ` Robin Murphy (?) @ 2022-03-02 15:55 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-02 15:55 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel Thanks for the feedback Robin! Sorry my choices of word weren't that great, but what I meant is to understand how ARM flushes a range of dcache for device drivers, and not an equal to x86 clflush. I believe the concern is if the CPU writes an update, that update might only be sitting in the CPU cache and never make it to device memory where the device can see it; there are specific places that we are supposed to flush the CPU caches to make sure our updates are visible to the hardware. +Matt Roper Matt, Lucas, any feed back here? On 2022-03-02 4:49 a.m., Robin Murphy wrote: > On 2022-02-25 19:27, Michael Cheng wrote: >> Hi Robin, >> >> [ +arm64 maintainers for their awareness, which would have been a >> good thing to do from the start ] >> >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >> in sooner. >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> * Also thanks for pointing this out. Initially I was using >> dcache_clean_inval_poc, which seem to be the equivalently to what >> x86 is doing for dcache flushing, but it was giving me build errors >> since its not on the global list of kernel symbols. And after >> revisiting the documentation for caches_clean_inval_pou, it won't >> fly for what we are trying to do. Moving forward, what would you (or >> someone in the ARM community) suggest we do? Could it be possible to >> export dcache_clean_inval_poc as a global symbol? > > Unlikely, unless something with a legitimate need for CPU-centric > cache maintenance like kexec or CPU hotplug ever becomes modular. > > In the case of a device driver, it's not even the basic issues of > assuming to find direct equivalents to x86 semantics in other CPU > architectures, or effectively reinventing parts of the DMA API, it's > even bigger than that. Once you move from being integrated in a single > vendor's system architecture to being on a discrete card, you > fundamentally *no longer have any control over cache coherency*. > Whether the host CPU architecture happens to be AArch64, RISC-V, or > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > and interconnect IP vendors, and SoC integrators. You'll find yourself > in systems where PCIe simply cannot snoop any caches, where you'd > better have the correct DMA API calls in place to have any hope of > even the most basic functionality working properly; you'll find > yourself in systems where even if the PCIe root complex claims to > support No Snoop, your uncached traffic will still end up snooping > stale data that got prefetched back into caches you thought you'd > invalidated; you'll find yourself in systems where your memory > attributes may or may not get forcibly rewritten by an IOMMU depending > on the kernel config and/or command line. > > It's not about simply finding a substitute for clflush, it's that the > reasons you have for using clflush in the first place can no longer be > assumed to be valid. > > Robin. > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> On 2022-02-25 03:24, Michael Cheng wrote: >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>> performs a flush by first performing a clean, follow by an >>>> invalidation >>>> operation. >>>> >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>> the >>>> dcache. Thanks Tvrtko for the suggestion. >>>> >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>> >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>> symbol that could be use by other modules, thus use >>>> caches_clean_inval_pou instead. Also this version >>>> removes include for cacheflush, since its already >>>> included base on architecture type. >>>> >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>> index c3e6e615bf09..81c28714f930 100644 >>>> --- a/drivers/gpu/drm/drm_cache.c >>>> +++ b/drivers/gpu/drm/drm_cache.c >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>> long length) >>>> if (wbinvd_on_all_cpus()) >>>> pr_err("Timed out waiting for cache flush\n"); >>>> + >>>> +#elif defined(CONFIG_ARM64) >>>> + void *end = addr + length; >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> Robin. >>> >>> (Note that the above is somewhat of a loaded question, and I do >>> actually have half an idea of what you're trying to do here and why >>> it won't fly, but I'd like to at least assume you've read the >>> documentation of the function you decided was OK to use) >>> >>>> + >>>> #else >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 15:55 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-02 15:55 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, linux-arm-kernel, Will Deacon, Catalin Marinas, Matt Roper Thanks for the feedback Robin! Sorry my choices of word weren't that great, but what I meant is to understand how ARM flushes a range of dcache for device drivers, and not an equal to x86 clflush. I believe the concern is if the CPU writes an update, that update might only be sitting in the CPU cache and never make it to device memory where the device can see it; there are specific places that we are supposed to flush the CPU caches to make sure our updates are visible to the hardware. +Matt Roper Matt, Lucas, any feed back here? On 2022-03-02 4:49 a.m., Robin Murphy wrote: > On 2022-02-25 19:27, Michael Cheng wrote: >> Hi Robin, >> >> [ +arm64 maintainers for their awareness, which would have been a >> good thing to do from the start ] >> >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >> in sooner. >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> * Also thanks for pointing this out. Initially I was using >> dcache_clean_inval_poc, which seem to be the equivalently to what >> x86 is doing for dcache flushing, but it was giving me build errors >> since its not on the global list of kernel symbols. And after >> revisiting the documentation for caches_clean_inval_pou, it won't >> fly for what we are trying to do. Moving forward, what would you (or >> someone in the ARM community) suggest we do? Could it be possible to >> export dcache_clean_inval_poc as a global symbol? > > Unlikely, unless something with a legitimate need for CPU-centric > cache maintenance like kexec or CPU hotplug ever becomes modular. > > In the case of a device driver, it's not even the basic issues of > assuming to find direct equivalents to x86 semantics in other CPU > architectures, or effectively reinventing parts of the DMA API, it's > even bigger than that. Once you move from being integrated in a single > vendor's system architecture to being on a discrete card, you > fundamentally *no longer have any control over cache coherency*. > Whether the host CPU architecture happens to be AArch64, RISC-V, or > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > and interconnect IP vendors, and SoC integrators. You'll find yourself > in systems where PCIe simply cannot snoop any caches, where you'd > better have the correct DMA API calls in place to have any hope of > even the most basic functionality working properly; you'll find > yourself in systems where even if the PCIe root complex claims to > support No Snoop, your uncached traffic will still end up snooping > stale data that got prefetched back into caches you thought you'd > invalidated; you'll find yourself in systems where your memory > attributes may or may not get forcibly rewritten by an IOMMU depending > on the kernel config and/or command line. > > It's not about simply finding a substitute for clflush, it's that the > reasons you have for using clflush in the first place can no longer be > assumed to be valid. > > Robin. > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> On 2022-02-25 03:24, Michael Cheng wrote: >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>> performs a flush by first performing a clean, follow by an >>>> invalidation >>>> operation. >>>> >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>> the >>>> dcache. Thanks Tvrtko for the suggestion. >>>> >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>> >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>> symbol that could be use by other modules, thus use >>>> caches_clean_inval_pou instead. Also this version >>>> removes include for cacheflush, since its already >>>> included base on architecture type. >>>> >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>> index c3e6e615bf09..81c28714f930 100644 >>>> --- a/drivers/gpu/drm/drm_cache.c >>>> +++ b/drivers/gpu/drm/drm_cache.c >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>> long length) >>>> if (wbinvd_on_all_cpus()) >>>> pr_err("Timed out waiting for cache flush\n"); >>>> + >>>> +#elif defined(CONFIG_ARM64) >>>> + void *end = addr + length; >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> Robin. >>> >>> (Note that the above is somewhat of a loaded question, and I do >>> actually have half an idea of what you're trying to do here and why >>> it won't fly, but I'd like to at least assume you've read the >>> documentation of the function you decided was OK to use) >>> >>>> + >>>> #else >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>> #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 15:55 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-02 15:55 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel Thanks for the feedback Robin! Sorry my choices of word weren't that great, but what I meant is to understand how ARM flushes a range of dcache for device drivers, and not an equal to x86 clflush. I believe the concern is if the CPU writes an update, that update might only be sitting in the CPU cache and never make it to device memory where the device can see it; there are specific places that we are supposed to flush the CPU caches to make sure our updates are visible to the hardware. +Matt Roper Matt, Lucas, any feed back here? On 2022-03-02 4:49 a.m., Robin Murphy wrote: > On 2022-02-25 19:27, Michael Cheng wrote: >> Hi Robin, >> >> [ +arm64 maintainers for their awareness, which would have been a >> good thing to do from the start ] >> >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >> in sooner. >> >> Why does i915 need to ensure the CPU's instruction cache is coherent >> with its data cache? Is it a self-modifying driver? >> >> * Also thanks for pointing this out. Initially I was using >> dcache_clean_inval_poc, which seem to be the equivalently to what >> x86 is doing for dcache flushing, but it was giving me build errors >> since its not on the global list of kernel symbols. And after >> revisiting the documentation for caches_clean_inval_pou, it won't >> fly for what we are trying to do. Moving forward, what would you (or >> someone in the ARM community) suggest we do? Could it be possible to >> export dcache_clean_inval_poc as a global symbol? > > Unlikely, unless something with a legitimate need for CPU-centric > cache maintenance like kexec or CPU hotplug ever becomes modular. > > In the case of a device driver, it's not even the basic issues of > assuming to find direct equivalents to x86 semantics in other CPU > architectures, or effectively reinventing parts of the DMA API, it's > even bigger than that. Once you move from being integrated in a single > vendor's system architecture to being on a discrete card, you > fundamentally *no longer have any control over cache coherency*. > Whether the host CPU architecture happens to be AArch64, RISC-V, or > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > and interconnect IP vendors, and SoC integrators. You'll find yourself > in systems where PCIe simply cannot snoop any caches, where you'd > better have the correct DMA API calls in place to have any hope of > even the most basic functionality working properly; you'll find > yourself in systems where even if the PCIe root complex claims to > support No Snoop, your uncached traffic will still end up snooping > stale data that got prefetched back into caches you thought you'd > invalidated; you'll find yourself in systems where your memory > attributes may or may not get forcibly rewritten by an IOMMU depending > on the kernel config and/or command line. > > It's not about simply finding a substitute for clflush, it's that the > reasons you have for using clflush in the first place can no longer be > assumed to be valid. > > Robin. > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> On 2022-02-25 03:24, Michael Cheng wrote: >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>> performs a flush by first performing a clean, follow by an >>>> invalidation >>>> operation. >>>> >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>> the >>>> dcache. Thanks Tvrtko for the suggestion. >>>> >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>> >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>> symbol that could be use by other modules, thus use >>>> caches_clean_inval_pou instead. Also this version >>>> removes include for cacheflush, since its already >>>> included base on architecture type. >>>> >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>> index c3e6e615bf09..81c28714f930 100644 >>>> --- a/drivers/gpu/drm/drm_cache.c >>>> +++ b/drivers/gpu/drm/drm_cache.c >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>> long length) >>>> if (wbinvd_on_all_cpus()) >>>> pr_err("Timed out waiting for cache flush\n"); >>>> + >>>> +#elif defined(CONFIG_ARM64) >>>> + void *end = addr + length; >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> Robin. >>> >>> (Note that the above is somewhat of a loaded question, and I do >>> actually have half an idea of what you're trying to do here and why >>> it won't fly, but I'd like to at least assume you've read the >>> documentation of the function you decided was OK to use) >>> >>>> + >>>> #else >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-03-02 15:55 ` [Intel-gfx] " Michael Cheng (?) @ 2022-03-02 17:06 ` Alex Deucher -1 siblings, 0 replies; 55+ messages in thread From: Alex Deucher @ 2022-03-02 17:06 UTC (permalink / raw) To: Michael Cheng Cc: Tvrtko Ursulin, balasubramani.vivekanandan, Will Deacon, wayne.boyer, Catalin Marinas, Intel Graphics Development, casey.g.bowman, Lucas De Marchi, Maling list - DRI developers, Robin Murphy, linux-arm-kernel On Wed, Mar 2, 2022 at 10:55 AM Michael Cheng <michael.cheng@intel.com> wrote: > > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. > > +Matt Roper > > Matt, Lucas, any feed back here? MMIO (e.g., PCI BARs, etc.) should be mapped uncached. If it's not you'll have a lot of problems using a GPU on that architecture. One thing that you may want to check is if your device has its own caches or write queues on the BAR aperture. You may have to flush them after CPU access to the BAR to make sure CPU updates land in device memory. For system memory, PCI, per the spec, should be cache coherent with the CPU. If it's not, you'll have a lot of trouble using a GPU on that platform. Alex > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: > > On 2022-02-25 19:27, Michael Cheng wrote: > >> Hi Robin, > >> > >> [ +arm64 maintainers for their awareness, which would have been a > >> good thing to do from the start ] > >> > >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them > >> in sooner. > >> > >> Why does i915 need to ensure the CPU's instruction cache is coherent > >> with its data cache? Is it a self-modifying driver? > >> > >> * Also thanks for pointing this out. Initially I was using > >> dcache_clean_inval_poc, which seem to be the equivalently to what > >> x86 is doing for dcache flushing, but it was giving me build errors > >> since its not on the global list of kernel symbols. And after > >> revisiting the documentation for caches_clean_inval_pou, it won't > >> fly for what we are trying to do. Moving forward, what would you (or > >> someone in the ARM community) suggest we do? Could it be possible to > >> export dcache_clean_inval_poc as a global symbol? > > > > Unlikely, unless something with a legitimate need for CPU-centric > > cache maintenance like kexec or CPU hotplug ever becomes modular. > > > > In the case of a device driver, it's not even the basic issues of > > assuming to find direct equivalents to x86 semantics in other CPU > > architectures, or effectively reinventing parts of the DMA API, it's > > even bigger than that. Once you move from being integrated in a single > > vendor's system architecture to being on a discrete card, you > > fundamentally *no longer have any control over cache coherency*. > > Whether the host CPU architecture happens to be AArch64, RISC-V, or > > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > > and interconnect IP vendors, and SoC integrators. You'll find yourself > > in systems where PCIe simply cannot snoop any caches, where you'd > > better have the correct DMA API calls in place to have any hope of > > even the most basic functionality working properly; you'll find > > yourself in systems where even if the PCIe root complex claims to > > support No Snoop, your uncached traffic will still end up snooping > > stale data that got prefetched back into caches you thought you'd > > invalidated; you'll find yourself in systems where your memory > > attributes may or may not get forcibly rewritten by an IOMMU depending > > on the kernel config and/or command line. > > > > It's not about simply finding a substitute for clflush, it's that the > > reasons you have for using clflush in the first place can no longer be > > assumed to be valid. > > > > Robin. > > > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: > >>> [ +arm64 maintainers for their awareness, which would have been a > >>> good thing to do from the start ] > >>> > >>> On 2022-02-25 03:24, Michael Cheng wrote: > >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > >>>> performs a flush by first performing a clean, follow by an > >>>> invalidation > >>>> operation. > >>>> > >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation > >>>> the > >>>> dcache. Thanks Tvrtko for the suggestion. > >>>> > >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > >>>> > >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > >>>> symbol that could be use by other modules, thus use > >>>> caches_clean_inval_pou instead. Also this version > >>>> removes include for cacheflush, since its already > >>>> included base on architecture type. > >>>> > >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> > >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > >>>> --- > >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > >>>> index c3e6e615bf09..81c28714f930 100644 > >>>> --- a/drivers/gpu/drm/drm_cache.c > >>>> +++ b/drivers/gpu/drm/drm_cache.c > >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned > >>>> long length) > >>>> if (wbinvd_on_all_cpus()) > >>>> pr_err("Timed out waiting for cache flush\n"); > >>>> + > >>>> +#elif defined(CONFIG_ARM64) > >>>> + void *end = addr + length; > >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > >>> > >>> Why does i915 need to ensure the CPU's instruction cache is coherent > >>> with its data cache? Is it a self-modifying driver? > >>> > >>> Robin. > >>> > >>> (Note that the above is somewhat of a loaded question, and I do > >>> actually have half an idea of what you're trying to do here and why > >>> it won't fly, but I'd like to at least assume you've read the > >>> documentation of the function you decided was OK to use) > >>> > >>>> + > >>>> #else > >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 17:06 ` Alex Deucher 0 siblings, 0 replies; 55+ messages in thread From: Alex Deucher @ 2022-03-02 17:06 UTC (permalink / raw) To: Michael Cheng Cc: Robin Murphy, Intel Graphics Development, Tvrtko Ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, Lucas De Marchi, Maling list - DRI developers, Will Deacon, linux-arm-kernel On Wed, Mar 2, 2022 at 10:55 AM Michael Cheng <michael.cheng@intel.com> wrote: > > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. > > +Matt Roper > > Matt, Lucas, any feed back here? MMIO (e.g., PCI BARs, etc.) should be mapped uncached. If it's not you'll have a lot of problems using a GPU on that architecture. One thing that you may want to check is if your device has its own caches or write queues on the BAR aperture. You may have to flush them after CPU access to the BAR to make sure CPU updates land in device memory. For system memory, PCI, per the spec, should be cache coherent with the CPU. If it's not, you'll have a lot of trouble using a GPU on that platform. Alex > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: > > On 2022-02-25 19:27, Michael Cheng wrote: > >> Hi Robin, > >> > >> [ +arm64 maintainers for their awareness, which would have been a > >> good thing to do from the start ] > >> > >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them > >> in sooner. > >> > >> Why does i915 need to ensure the CPU's instruction cache is coherent > >> with its data cache? Is it a self-modifying driver? > >> > >> * Also thanks for pointing this out. Initially I was using > >> dcache_clean_inval_poc, which seem to be the equivalently to what > >> x86 is doing for dcache flushing, but it was giving me build errors > >> since its not on the global list of kernel symbols. And after > >> revisiting the documentation for caches_clean_inval_pou, it won't > >> fly for what we are trying to do. Moving forward, what would you (or > >> someone in the ARM community) suggest we do? Could it be possible to > >> export dcache_clean_inval_poc as a global symbol? > > > > Unlikely, unless something with a legitimate need for CPU-centric > > cache maintenance like kexec or CPU hotplug ever becomes modular. > > > > In the case of a device driver, it's not even the basic issues of > > assuming to find direct equivalents to x86 semantics in other CPU > > architectures, or effectively reinventing parts of the DMA API, it's > > even bigger than that. Once you move from being integrated in a single > > vendor's system architecture to being on a discrete card, you > > fundamentally *no longer have any control over cache coherency*. > > Whether the host CPU architecture happens to be AArch64, RISC-V, or > > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > > and interconnect IP vendors, and SoC integrators. You'll find yourself > > in systems where PCIe simply cannot snoop any caches, where you'd > > better have the correct DMA API calls in place to have any hope of > > even the most basic functionality working properly; you'll find > > yourself in systems where even if the PCIe root complex claims to > > support No Snoop, your uncached traffic will still end up snooping > > stale data that got prefetched back into caches you thought you'd > > invalidated; you'll find yourself in systems where your memory > > attributes may or may not get forcibly rewritten by an IOMMU depending > > on the kernel config and/or command line. > > > > It's not about simply finding a substitute for clflush, it's that the > > reasons you have for using clflush in the first place can no longer be > > assumed to be valid. > > > > Robin. > > > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: > >>> [ +arm64 maintainers for their awareness, which would have been a > >>> good thing to do from the start ] > >>> > >>> On 2022-02-25 03:24, Michael Cheng wrote: > >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > >>>> performs a flush by first performing a clean, follow by an > >>>> invalidation > >>>> operation. > >>>> > >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation > >>>> the > >>>> dcache. Thanks Tvrtko for the suggestion. > >>>> > >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > >>>> > >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > >>>> symbol that could be use by other modules, thus use > >>>> caches_clean_inval_pou instead. Also this version > >>>> removes include for cacheflush, since its already > >>>> included base on architecture type. > >>>> > >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> > >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > >>>> --- > >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > >>>> index c3e6e615bf09..81c28714f930 100644 > >>>> --- a/drivers/gpu/drm/drm_cache.c > >>>> +++ b/drivers/gpu/drm/drm_cache.c > >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned > >>>> long length) > >>>> if (wbinvd_on_all_cpus()) > >>>> pr_err("Timed out waiting for cache flush\n"); > >>>> + > >>>> +#elif defined(CONFIG_ARM64) > >>>> + void *end = addr + length; > >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > >>> > >>> Why does i915 need to ensure the CPU's instruction cache is coherent > >>> with its data cache? Is it a self-modifying driver? > >>> > >>> Robin. > >>> > >>> (Note that the above is somewhat of a loaded question, and I do > >>> actually have half an idea of what you're trying to do here and why > >>> it won't fly, but I'd like to at least assume you've read the > >>> documentation of the function you decided was OK to use) > >>> > >>>> + > >>>> #else > >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > >>>> #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 17:06 ` Alex Deucher 0 siblings, 0 replies; 55+ messages in thread From: Alex Deucher @ 2022-03-02 17:06 UTC (permalink / raw) To: Michael Cheng Cc: Will Deacon, Catalin Marinas, Intel Graphics Development, Lucas De Marchi, Maling list - DRI developers, Robin Murphy, linux-arm-kernel On Wed, Mar 2, 2022 at 10:55 AM Michael Cheng <michael.cheng@intel.com> wrote: > > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. > > +Matt Roper > > Matt, Lucas, any feed back here? MMIO (e.g., PCI BARs, etc.) should be mapped uncached. If it's not you'll have a lot of problems using a GPU on that architecture. One thing that you may want to check is if your device has its own caches or write queues on the BAR aperture. You may have to flush them after CPU access to the BAR to make sure CPU updates land in device memory. For system memory, PCI, per the spec, should be cache coherent with the CPU. If it's not, you'll have a lot of trouble using a GPU on that platform. Alex > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: > > On 2022-02-25 19:27, Michael Cheng wrote: > >> Hi Robin, > >> > >> [ +arm64 maintainers for their awareness, which would have been a > >> good thing to do from the start ] > >> > >> * Thanks for adding the arm64 maintainer and sorry I didn't rope them > >> in sooner. > >> > >> Why does i915 need to ensure the CPU's instruction cache is coherent > >> with its data cache? Is it a self-modifying driver? > >> > >> * Also thanks for pointing this out. Initially I was using > >> dcache_clean_inval_poc, which seem to be the equivalently to what > >> x86 is doing for dcache flushing, but it was giving me build errors > >> since its not on the global list of kernel symbols. And after > >> revisiting the documentation for caches_clean_inval_pou, it won't > >> fly for what we are trying to do. Moving forward, what would you (or > >> someone in the ARM community) suggest we do? Could it be possible to > >> export dcache_clean_inval_poc as a global symbol? > > > > Unlikely, unless something with a legitimate need for CPU-centric > > cache maintenance like kexec or CPU hotplug ever becomes modular. > > > > In the case of a device driver, it's not even the basic issues of > > assuming to find direct equivalents to x86 semantics in other CPU > > architectures, or effectively reinventing parts of the DMA API, it's > > even bigger than that. Once you move from being integrated in a single > > vendor's system architecture to being on a discrete card, you > > fundamentally *no longer have any control over cache coherency*. > > Whether the host CPU architecture happens to be AArch64, RISC-V, or > > whatever doesn't really matter, you're at the mercy of 3rd-party PCIe > > and interconnect IP vendors, and SoC integrators. You'll find yourself > > in systems where PCIe simply cannot snoop any caches, where you'd > > better have the correct DMA API calls in place to have any hope of > > even the most basic functionality working properly; you'll find > > yourself in systems where even if the PCIe root complex claims to > > support No Snoop, your uncached traffic will still end up snooping > > stale data that got prefetched back into caches you thought you'd > > invalidated; you'll find yourself in systems where your memory > > attributes may or may not get forcibly rewritten by an IOMMU depending > > on the kernel config and/or command line. > > > > It's not about simply finding a substitute for clflush, it's that the > > reasons you have for using clflush in the first place can no longer be > > assumed to be valid. > > > > Robin. > > > >> On 2022-02-25 10:24 a.m., Robin Murphy wrote: > >>> [ +arm64 maintainers for their awareness, which would have been a > >>> good thing to do from the start ] > >>> > >>> On 2022-02-25 03:24, Michael Cheng wrote: > >>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou > >>>> performs a flush by first performing a clean, follow by an > >>>> invalidation > >>>> operation. > >>>> > >>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation > >>>> the > >>>> dcache. Thanks Tvrtko for the suggestion. > >>>> > >>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h > >>>> > >>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a > >>>> symbol that could be use by other modules, thus use > >>>> caches_clean_inval_pou instead. Also this version > >>>> removes include for cacheflush, since its already > >>>> included base on architecture type. > >>>> > >>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> > >>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > >>>> --- > >>>> drivers/gpu/drm/drm_cache.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > >>>> index c3e6e615bf09..81c28714f930 100644 > >>>> --- a/drivers/gpu/drm/drm_cache.c > >>>> +++ b/drivers/gpu/drm/drm_cache.c > >>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned > >>>> long length) > >>>> if (wbinvd_on_all_cpus()) > >>>> pr_err("Timed out waiting for cache flush\n"); > >>>> + > >>>> +#elif defined(CONFIG_ARM64) > >>>> + void *end = addr + length; > >>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); > >>> > >>> Why does i915 need to ensure the CPU's instruction cache is coherent > >>> with its data cache? Is it a self-modifying driver? > >>> > >>> Robin. > >>> > >>> (Note that the above is somewhat of a loaded question, and I do > >>> actually have half an idea of what you're trying to do here and why > >>> it won't fly, but I'd like to at least assume you've read the > >>> documentation of the function you decided was OK to use) > >>> > >>>> + > >>>> #else > >>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > >>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-03-02 15:55 ` [Intel-gfx] " Michael Cheng (?) @ 2022-03-02 19:10 ` Robin Murphy -1 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 19:10 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel On 2022-03-02 15:55, Michael Cheng wrote: > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. Ah, OK, if it's more about ordering, and it's actually write buffers rather than caches that you care about flushing, then we might be a lot safer, phew! For a very simple overview, in a case where the device itself needs to observe memory writes in the correct order, e.g.: data_descriptor.valid = 1; clflush(&data_descriptor); command_descriptor.data = &data_descriptor writel(/* control register to read command to then read data */) then dma_wmb() between the first two writes should be the right tool to ensure that the command does not observe the command update while the data update is still sat somewhere in a CPU write buffer. If you want a slightly stronger notion that, at a given point, all prior writes have actually been issued and should now be visible (rather than just that they won't become visible in the wrong order whenever they do), then wmb() should suffice on arm64. Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can still be write-buffered, so barriers still matter even when actual cache maintenance ops don't (and as before if you're trying to perform cache maintenance outside the DMA API then you've already lost anyway). MMIO registers should be mapped as Device memory via ioremap(), which is not bufferable, hence the barrier implicit in writel() effectively pushes out any prior buffered writes ahead of a register write, which is why we don't need to worry about this most of the time. This is only a very rough overview, though, and I'm not familiar enough with x86 semantics, your hardware, or the exact use-case to be able to say whether barriers alone are anywhere near the right answer or not. Robin. > > +Matt Roper > > Matt, Lucas, any feed back here? > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: >> On 2022-02-25 19:27, Michael Cheng wrote: >>> Hi Robin, >>> >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >>> in sooner. >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> * Also thanks for pointing this out. Initially I was using >>> dcache_clean_inval_poc, which seem to be the equivalently to what >>> x86 is doing for dcache flushing, but it was giving me build errors >>> since its not on the global list of kernel symbols. And after >>> revisiting the documentation for caches_clean_inval_pou, it won't >>> fly for what we are trying to do. Moving forward, what would you (or >>> someone in the ARM community) suggest we do? Could it be possible to >>> export dcache_clean_inval_poc as a global symbol? >> >> Unlikely, unless something with a legitimate need for CPU-centric >> cache maintenance like kexec or CPU hotplug ever becomes modular. >> >> In the case of a device driver, it's not even the basic issues of >> assuming to find direct equivalents to x86 semantics in other CPU >> architectures, or effectively reinventing parts of the DMA API, it's >> even bigger than that. Once you move from being integrated in a single >> vendor's system architecture to being on a discrete card, you >> fundamentally *no longer have any control over cache coherency*. >> Whether the host CPU architecture happens to be AArch64, RISC-V, or >> whatever doesn't really matter, you're at the mercy of 3rd-party PCIe >> and interconnect IP vendors, and SoC integrators. You'll find yourself >> in systems where PCIe simply cannot snoop any caches, where you'd >> better have the correct DMA API calls in place to have any hope of >> even the most basic functionality working properly; you'll find >> yourself in systems where even if the PCIe root complex claims to >> support No Snoop, your uncached traffic will still end up snooping >> stale data that got prefetched back into caches you thought you'd >> invalidated; you'll find yourself in systems where your memory >> attributes may or may not get forcibly rewritten by an IOMMU depending >> on the kernel config and/or command line. >> >> It's not about simply finding a substitute for clflush, it's that the >> reasons you have for using clflush in the first place can no longer be >> assumed to be valid. >> >> Robin. >> >>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>> performs a flush by first performing a clean, follow by an >>>>> invalidation >>>>> operation. >>>>> >>>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>>> the >>>>> dcache. Thanks Tvrtko for the suggestion. >>>>> >>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>> >>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>>> symbol that could be use by other modules, thus use >>>>> caches_clean_inval_pou instead. Also this version >>>>> removes include for cacheflush, since its already >>>>> included base on architecture type. >>>>> >>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>>> index c3e6e615bf09..81c28714f930 100644 >>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>> long length) >>>>> if (wbinvd_on_all_cpus()) >>>>> pr_err("Timed out waiting for cache flush\n"); >>>>> + >>>>> +#elif defined(CONFIG_ARM64) >>>>> + void *end = addr + length; >>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is coherent >>>> with its data cache? Is it a self-modifying driver? >>>> >>>> Robin. >>>> >>>> (Note that the above is somewhat of a loaded question, and I do >>>> actually have half an idea of what you're trying to do here and why >>>> it won't fly, but I'd like to at least assume you've read the >>>> documentation of the function you decided was OK to use) >>>> >>>>> + >>>>> #else >>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 19:10 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 19:10 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel On 2022-03-02 15:55, Michael Cheng wrote: > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. Ah, OK, if it's more about ordering, and it's actually write buffers rather than caches that you care about flushing, then we might be a lot safer, phew! For a very simple overview, in a case where the device itself needs to observe memory writes in the correct order, e.g.: data_descriptor.valid = 1; clflush(&data_descriptor); command_descriptor.data = &data_descriptor writel(/* control register to read command to then read data */) then dma_wmb() between the first two writes should be the right tool to ensure that the command does not observe the command update while the data update is still sat somewhere in a CPU write buffer. If you want a slightly stronger notion that, at a given point, all prior writes have actually been issued and should now be visible (rather than just that they won't become visible in the wrong order whenever they do), then wmb() should suffice on arm64. Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can still be write-buffered, so barriers still matter even when actual cache maintenance ops don't (and as before if you're trying to perform cache maintenance outside the DMA API then you've already lost anyway). MMIO registers should be mapped as Device memory via ioremap(), which is not bufferable, hence the barrier implicit in writel() effectively pushes out any prior buffered writes ahead of a register write, which is why we don't need to worry about this most of the time. This is only a very rough overview, though, and I'm not familiar enough with x86 semantics, your hardware, or the exact use-case to be able to say whether barriers alone are anywhere near the right answer or not. Robin. > > +Matt Roper > > Matt, Lucas, any feed back here? > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: >> On 2022-02-25 19:27, Michael Cheng wrote: >>> Hi Robin, >>> >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >>> in sooner. >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> * Also thanks for pointing this out. Initially I was using >>> dcache_clean_inval_poc, which seem to be the equivalently to what >>> x86 is doing for dcache flushing, but it was giving me build errors >>> since its not on the global list of kernel symbols. And after >>> revisiting the documentation for caches_clean_inval_pou, it won't >>> fly for what we are trying to do. Moving forward, what would you (or >>> someone in the ARM community) suggest we do? Could it be possible to >>> export dcache_clean_inval_poc as a global symbol? >> >> Unlikely, unless something with a legitimate need for CPU-centric >> cache maintenance like kexec or CPU hotplug ever becomes modular. >> >> In the case of a device driver, it's not even the basic issues of >> assuming to find direct equivalents to x86 semantics in other CPU >> architectures, or effectively reinventing parts of the DMA API, it's >> even bigger than that. Once you move from being integrated in a single >> vendor's system architecture to being on a discrete card, you >> fundamentally *no longer have any control over cache coherency*. >> Whether the host CPU architecture happens to be AArch64, RISC-V, or >> whatever doesn't really matter, you're at the mercy of 3rd-party PCIe >> and interconnect IP vendors, and SoC integrators. You'll find yourself >> in systems where PCIe simply cannot snoop any caches, where you'd >> better have the correct DMA API calls in place to have any hope of >> even the most basic functionality working properly; you'll find >> yourself in systems where even if the PCIe root complex claims to >> support No Snoop, your uncached traffic will still end up snooping >> stale data that got prefetched back into caches you thought you'd >> invalidated; you'll find yourself in systems where your memory >> attributes may or may not get forcibly rewritten by an IOMMU depending >> on the kernel config and/or command line. >> >> It's not about simply finding a substitute for clflush, it's that the >> reasons you have for using clflush in the first place can no longer be >> assumed to be valid. >> >> Robin. >> >>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>> performs a flush by first performing a clean, follow by an >>>>> invalidation >>>>> operation. >>>>> >>>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>>> the >>>>> dcache. Thanks Tvrtko for the suggestion. >>>>> >>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>> >>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>>> symbol that could be use by other modules, thus use >>>>> caches_clean_inval_pou instead. Also this version >>>>> removes include for cacheflush, since its already >>>>> included base on architecture type. >>>>> >>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>>> index c3e6e615bf09..81c28714f930 100644 >>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>> long length) >>>>> if (wbinvd_on_all_cpus()) >>>>> pr_err("Timed out waiting for cache flush\n"); >>>>> + >>>>> +#elif defined(CONFIG_ARM64) >>>>> + void *end = addr + length; >>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is coherent >>>> with its data cache? Is it a self-modifying driver? >>>> >>>> Robin. >>>> >>>> (Note that the above is somewhat of a loaded question, and I do >>>> actually have half an idea of what you're trying to do here and why >>>> it won't fly, but I'd like to at least assume you've read the >>>> documentation of the function you decided was OK to use) >>>> >>>>> + >>>>> #else >>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-02 19:10 ` Robin Murphy 0 siblings, 0 replies; 55+ messages in thread From: Robin Murphy @ 2022-03-02 19:10 UTC (permalink / raw) To: Michael Cheng, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, linux-arm-kernel, Will Deacon, Catalin Marinas, Matt Roper On 2022-03-02 15:55, Michael Cheng wrote: > Thanks for the feedback Robin! > > Sorry my choices of word weren't that great, but what I meant is to > understand how ARM flushes a range of dcache for device drivers, and not > an equal to x86 clflush. > > I believe the concern is if the CPU writes an update, that update might > only be sitting in the CPU cache and never make it to device memory > where the device can see it; there are specific places that we are > supposed to flush the CPU caches to make sure our updates are visible to > the hardware. Ah, OK, if it's more about ordering, and it's actually write buffers rather than caches that you care about flushing, then we might be a lot safer, phew! For a very simple overview, in a case where the device itself needs to observe memory writes in the correct order, e.g.: data_descriptor.valid = 1; clflush(&data_descriptor); command_descriptor.data = &data_descriptor writel(/* control register to read command to then read data */) then dma_wmb() between the first two writes should be the right tool to ensure that the command does not observe the command update while the data update is still sat somewhere in a CPU write buffer. If you want a slightly stronger notion that, at a given point, all prior writes have actually been issued and should now be visible (rather than just that they won't become visible in the wrong order whenever they do), then wmb() should suffice on arm64. Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can still be write-buffered, so barriers still matter even when actual cache maintenance ops don't (and as before if you're trying to perform cache maintenance outside the DMA API then you've already lost anyway). MMIO registers should be mapped as Device memory via ioremap(), which is not bufferable, hence the barrier implicit in writel() effectively pushes out any prior buffered writes ahead of a register write, which is why we don't need to worry about this most of the time. This is only a very rough overview, though, and I'm not familiar enough with x86 semantics, your hardware, or the exact use-case to be able to say whether barriers alone are anywhere near the right answer or not. Robin. > > +Matt Roper > > Matt, Lucas, any feed back here? > > On 2022-03-02 4:49 a.m., Robin Murphy wrote: >> On 2022-02-25 19:27, Michael Cheng wrote: >>> Hi Robin, >>> >>> [ +arm64 maintainers for their awareness, which would have been a >>> good thing to do from the start ] >>> >>> * Thanks for adding the arm64 maintainer and sorry I didn't rope them >>> in sooner. >>> >>> Why does i915 need to ensure the CPU's instruction cache is coherent >>> with its data cache? Is it a self-modifying driver? >>> >>> * Also thanks for pointing this out. Initially I was using >>> dcache_clean_inval_poc, which seem to be the equivalently to what >>> x86 is doing for dcache flushing, but it was giving me build errors >>> since its not on the global list of kernel symbols. And after >>> revisiting the documentation for caches_clean_inval_pou, it won't >>> fly for what we are trying to do. Moving forward, what would you (or >>> someone in the ARM community) suggest we do? Could it be possible to >>> export dcache_clean_inval_poc as a global symbol? >> >> Unlikely, unless something with a legitimate need for CPU-centric >> cache maintenance like kexec or CPU hotplug ever becomes modular. >> >> In the case of a device driver, it's not even the basic issues of >> assuming to find direct equivalents to x86 semantics in other CPU >> architectures, or effectively reinventing parts of the DMA API, it's >> even bigger than that. Once you move from being integrated in a single >> vendor's system architecture to being on a discrete card, you >> fundamentally *no longer have any control over cache coherency*. >> Whether the host CPU architecture happens to be AArch64, RISC-V, or >> whatever doesn't really matter, you're at the mercy of 3rd-party PCIe >> and interconnect IP vendors, and SoC integrators. You'll find yourself >> in systems where PCIe simply cannot snoop any caches, where you'd >> better have the correct DMA API calls in place to have any hope of >> even the most basic functionality working properly; you'll find >> yourself in systems where even if the PCIe root complex claims to >> support No Snoop, your uncached traffic will still end up snooping >> stale data that got prefetched back into caches you thought you'd >> invalidated; you'll find yourself in systems where your memory >> attributes may or may not get forcibly rewritten by an IOMMU depending >> on the kernel config and/or command line. >> >> It's not about simply finding a substitute for clflush, it's that the >> reasons you have for using clflush in the first place can no longer be >> assumed to be valid. >> >> Robin. >> >>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>> performs a flush by first performing a clean, follow by an >>>>> invalidation >>>>> operation. >>>>> >>>>> v2 (Michael Cheng): Use correct macro for cleaning and invalidation >>>>> the >>>>> dcache. Thanks Tvrtko for the suggestion. >>>>> >>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>> >>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a >>>>> symbol that could be use by other modules, thus use >>>>> caches_clean_inval_pou instead. Also this version >>>>> removes include for cacheflush, since its already >>>>> included base on architecture type. >>>>> >>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c >>>>> index c3e6e615bf09..81c28714f930 100644 >>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>> long length) >>>>> if (wbinvd_on_all_cpus()) >>>>> pr_err("Timed out waiting for cache flush\n"); >>>>> + >>>>> +#elif defined(CONFIG_ARM64) >>>>> + void *end = addr + length; >>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is coherent >>>> with its data cache? Is it a self-modifying driver? >>>> >>>> Robin. >>>> >>>> (Note that the above is somewhat of a loaded question, and I do >>>> actually have half an idea of what you're trying to do here and why >>>> it won't fly, but I'd like to at least assume you've read the >>>> documentation of the function you decided was OK to use) >>>> >>>>> + >>>>> #else >>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>> #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range 2022-03-02 19:10 ` Robin Murphy (?) @ 2022-03-07 16:52 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-07 16:52 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, Catalin Marinas, casey.g.bowman, lucas.demarchi, dri-devel, james.ausmus, Will Deacon, linux-arm-kernel Ah Thanks for the great feedback! @Lucas or @Matt, could you please chime in? Michael Cheng On 2022-03-02 11:10 a.m., Robin Murphy wrote: > On 2022-03-02 15:55, Michael Cheng wrote: >> Thanks for the feedback Robin! >> >> Sorry my choices of word weren't that great, but what I meant is to >> understand how ARM flushes a range of dcache for device drivers, and >> not an equal to x86 clflush. >> >> I believe the concern is if the CPU writes an update, that update >> might only be sitting in the CPU cache and never make it to device >> memory where the device can see it; there are specific places that we >> are supposed to flush the CPU caches to make sure our updates are >> visible to the hardware. > > Ah, OK, if it's more about ordering, and it's actually write buffers > rather than caches that you care about flushing, then we might be a > lot safer, phew! > > For a very simple overview, in a case where the device itself needs to > observe memory writes in the correct order, e.g.: > > data_descriptor.valid = 1; > > clflush(&data_descriptor); > > command_descriptor.data = &data_descriptor > > writel(/* control register to read command to then read data */) > > then dma_wmb() between the first two writes should be the right tool > to ensure that the command does not observe the command update while > the data update is still sat somewhere in a CPU write buffer. > > If you want a slightly stronger notion that, at a given point, all > prior writes have actually been issued and should now be visible > (rather than just that they won't become visible in the wrong order > whenever they do), then wmb() should suffice on arm64. > > Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM > for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can > still be write-buffered, so barriers still matter even when actual > cache maintenance ops don't (and as before if you're trying to perform > cache maintenance outside the DMA API then you've already lost > anyway). MMIO registers should be mapped as Device memory via > ioremap(), which is not bufferable, hence the barrier implicit in > writel() effectively pushes out any prior buffered writes ahead of a > register write, which is why we don't need to worry about this most of > the time. > > This is only a very rough overview, though, and I'm not familiar > enough with x86 semantics, your hardware, or the exact use-case to be > able to say whether barriers alone are anywhere near the right answer > or not. > > Robin. > >> >> +Matt Roper >> >> Matt, Lucas, any feed back here? >> >> On 2022-03-02 4:49 a.m., Robin Murphy wrote: >>> On 2022-02-25 19:27, Michael Cheng wrote: >>>> Hi Robin, >>>> >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> * Thanks for adding the arm64 maintainer and sorry I didn't rope >>>> them >>>> in sooner. >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is >>>> coherent with its data cache? Is it a self-modifying driver? >>>> >>>> * Also thanks for pointing this out. Initially I was using >>>> dcache_clean_inval_poc, which seem to be the equivalently to what >>>> x86 is doing for dcache flushing, but it was giving me build >>>> errors >>>> since its not on the global list of kernel symbols. And after >>>> revisiting the documentation for caches_clean_inval_pou, it won't >>>> fly for what we are trying to do. Moving forward, what would >>>> you (or >>>> someone in the ARM community) suggest we do? Could it be >>>> possible to >>>> export dcache_clean_inval_poc as a global symbol? >>> >>> Unlikely, unless something with a legitimate need for CPU-centric >>> cache maintenance like kexec or CPU hotplug ever becomes modular. >>> >>> In the case of a device driver, it's not even the basic issues of >>> assuming to find direct equivalents to x86 semantics in other CPU >>> architectures, or effectively reinventing parts of the DMA API, it's >>> even bigger than that. Once you move from being integrated in a >>> single vendor's system architecture to being on a discrete card, you >>> fundamentally *no longer have any control over cache coherency*. >>> Whether the host CPU architecture happens to be AArch64, RISC-V, or >>> whatever doesn't really matter, you're at the mercy of 3rd-party >>> PCIe and interconnect IP vendors, and SoC integrators. You'll find >>> yourself in systems where PCIe simply cannot snoop any caches, where >>> you'd better have the correct DMA API calls in place to have any >>> hope of even the most basic functionality working properly; you'll >>> find yourself in systems where even if the PCIe root complex claims >>> to support No Snoop, your uncached traffic will still end up >>> snooping stale data that got prefetched back into caches you thought >>> you'd invalidated; you'll find yourself in systems where your memory >>> attributes may or may not get forcibly rewritten by an IOMMU >>> depending on the kernel config and/or command line. >>> >>> It's not about simply finding a substitute for clflush, it's that >>> the reasons you have for using clflush in the first place can no >>> longer be assumed to be valid. >>> >>> Robin. >>> >>>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>>> [ +arm64 maintainers for their awareness, which would have been a >>>>> good thing to do from the start ] >>>>> >>>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>> performs a flush by first performing a clean, follow by an >>>>>> invalidation >>>>>> operation. >>>>>> >>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>> invalidation the >>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>> >>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>> >>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>> as a >>>>>> symbol that could be use by other modules, thus use >>>>>> caches_clean_inval_pou instead. Also this version >>>>>> removes include for cacheflush, since its already >>>>>> included base on architecture type. >>>>>> >>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>> long length) >>>>>> if (wbinvd_on_all_cpus()) >>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>> + >>>>>> +#elif defined(CONFIG_ARM64) >>>>>> + void *end = addr + length; >>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>> long)end); >>>>> >>>>> Why does i915 need to ensure the CPU's instruction cache is >>>>> coherent with its data cache? Is it a self-modifying driver? >>>>> >>>>> Robin. >>>>> >>>>> (Note that the above is somewhat of a loaded question, and I do >>>>> actually have half an idea of what you're trying to do here and >>>>> why it won't fly, but I'd like to at least assume you've read the >>>>> documentation of the function you decided was OK to use) >>>>> >>>>>> + >>>>>> #else >>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-07 16:52 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-07 16:52 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: tvrtko.ursulin, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel, linux-arm-kernel, Will Deacon, Catalin Marinas, Matt Roper, james.ausmus Ah Thanks for the great feedback! @Lucas or @Matt, could you please chime in? Michael Cheng On 2022-03-02 11:10 a.m., Robin Murphy wrote: > On 2022-03-02 15:55, Michael Cheng wrote: >> Thanks for the feedback Robin! >> >> Sorry my choices of word weren't that great, but what I meant is to >> understand how ARM flushes a range of dcache for device drivers, and >> not an equal to x86 clflush. >> >> I believe the concern is if the CPU writes an update, that update >> might only be sitting in the CPU cache and never make it to device >> memory where the device can see it; there are specific places that we >> are supposed to flush the CPU caches to make sure our updates are >> visible to the hardware. > > Ah, OK, if it's more about ordering, and it's actually write buffers > rather than caches that you care about flushing, then we might be a > lot safer, phew! > > For a very simple overview, in a case where the device itself needs to > observe memory writes in the correct order, e.g.: > > data_descriptor.valid = 1; > > clflush(&data_descriptor); > > command_descriptor.data = &data_descriptor > > writel(/* control register to read command to then read data */) > > then dma_wmb() between the first two writes should be the right tool > to ensure that the command does not observe the command update while > the data update is still sat somewhere in a CPU write buffer. > > If you want a slightly stronger notion that, at a given point, all > prior writes have actually been issued and should now be visible > (rather than just that they won't become visible in the wrong order > whenever they do), then wmb() should suffice on arm64. > > Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM > for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can > still be write-buffered, so barriers still matter even when actual > cache maintenance ops don't (and as before if you're trying to perform > cache maintenance outside the DMA API then you've already lost > anyway). MMIO registers should be mapped as Device memory via > ioremap(), which is not bufferable, hence the barrier implicit in > writel() effectively pushes out any prior buffered writes ahead of a > register write, which is why we don't need to worry about this most of > the time. > > This is only a very rough overview, though, and I'm not familiar > enough with x86 semantics, your hardware, or the exact use-case to be > able to say whether barriers alone are anywhere near the right answer > or not. > > Robin. > >> >> +Matt Roper >> >> Matt, Lucas, any feed back here? >> >> On 2022-03-02 4:49 a.m., Robin Murphy wrote: >>> On 2022-02-25 19:27, Michael Cheng wrote: >>>> Hi Robin, >>>> >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> * Thanks for adding the arm64 maintainer and sorry I didn't rope >>>> them >>>> in sooner. >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is >>>> coherent with its data cache? Is it a self-modifying driver? >>>> >>>> * Also thanks for pointing this out. Initially I was using >>>> dcache_clean_inval_poc, which seem to be the equivalently to what >>>> x86 is doing for dcache flushing, but it was giving me build >>>> errors >>>> since its not on the global list of kernel symbols. And after >>>> revisiting the documentation for caches_clean_inval_pou, it won't >>>> fly for what we are trying to do. Moving forward, what would >>>> you (or >>>> someone in the ARM community) suggest we do? Could it be >>>> possible to >>>> export dcache_clean_inval_poc as a global symbol? >>> >>> Unlikely, unless something with a legitimate need for CPU-centric >>> cache maintenance like kexec or CPU hotplug ever becomes modular. >>> >>> In the case of a device driver, it's not even the basic issues of >>> assuming to find direct equivalents to x86 semantics in other CPU >>> architectures, or effectively reinventing parts of the DMA API, it's >>> even bigger than that. Once you move from being integrated in a >>> single vendor's system architecture to being on a discrete card, you >>> fundamentally *no longer have any control over cache coherency*. >>> Whether the host CPU architecture happens to be AArch64, RISC-V, or >>> whatever doesn't really matter, you're at the mercy of 3rd-party >>> PCIe and interconnect IP vendors, and SoC integrators. You'll find >>> yourself in systems where PCIe simply cannot snoop any caches, where >>> you'd better have the correct DMA API calls in place to have any >>> hope of even the most basic functionality working properly; you'll >>> find yourself in systems where even if the PCIe root complex claims >>> to support No Snoop, your uncached traffic will still end up >>> snooping stale data that got prefetched back into caches you thought >>> you'd invalidated; you'll find yourself in systems where your memory >>> attributes may or may not get forcibly rewritten by an IOMMU >>> depending on the kernel config and/or command line. >>> >>> It's not about simply finding a substitute for clflush, it's that >>> the reasons you have for using clflush in the first place can no >>> longer be assumed to be valid. >>> >>> Robin. >>> >>>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>>> [ +arm64 maintainers for their awareness, which would have been a >>>>> good thing to do from the start ] >>>>> >>>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>> performs a flush by first performing a clean, follow by an >>>>>> invalidation >>>>>> operation. >>>>>> >>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>> invalidation the >>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>> >>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>> >>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>> as a >>>>>> symbol that could be use by other modules, thus use >>>>>> caches_clean_inval_pou instead. Also this version >>>>>> removes include for cacheflush, since its already >>>>>> included base on architecture type. >>>>>> >>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>> long length) >>>>>> if (wbinvd_on_all_cpus()) >>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>> + >>>>>> +#elif defined(CONFIG_ARM64) >>>>>> + void *end = addr + length; >>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>> long)end); >>>>> >>>>> Why does i915 need to ensure the CPU's instruction cache is >>>>> coherent with its data cache? Is it a self-modifying driver? >>>>> >>>>> Robin. >>>>> >>>>> (Note that the above is somewhat of a loaded question, and I do >>>>> actually have half an idea of what you're trying to do here and >>>>> why it won't fly, but I'd like to at least assume you've read the >>>>> documentation of the function you decided was OK to use) >>>>> >>>>>> + >>>>>> #else >>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>> #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range @ 2022-03-07 16:52 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-03-07 16:52 UTC (permalink / raw) To: Robin Murphy, intel-gfx Cc: Catalin Marinas, lucas.demarchi, dri-devel, Will Deacon, linux-arm-kernel Ah Thanks for the great feedback! @Lucas or @Matt, could you please chime in? Michael Cheng On 2022-03-02 11:10 a.m., Robin Murphy wrote: > On 2022-03-02 15:55, Michael Cheng wrote: >> Thanks for the feedback Robin! >> >> Sorry my choices of word weren't that great, but what I meant is to >> understand how ARM flushes a range of dcache for device drivers, and >> not an equal to x86 clflush. >> >> I believe the concern is if the CPU writes an update, that update >> might only be sitting in the CPU cache and never make it to device >> memory where the device can see it; there are specific places that we >> are supposed to flush the CPU caches to make sure our updates are >> visible to the hardware. > > Ah, OK, if it's more about ordering, and it's actually write buffers > rather than caches that you care about flushing, then we might be a > lot safer, phew! > > For a very simple overview, in a case where the device itself needs to > observe memory writes in the correct order, e.g.: > > data_descriptor.valid = 1; > > clflush(&data_descriptor); > > command_descriptor.data = &data_descriptor > > writel(/* control register to read command to then read data */) > > then dma_wmb() between the first two writes should be the right tool > to ensure that the command does not observe the command update while > the data update is still sat somewhere in a CPU write buffer. > > If you want a slightly stronger notion that, at a given point, all > prior writes have actually been issued and should now be visible > (rather than just that they won't become visible in the wrong order > whenever they do), then wmb() should suffice on arm64. > > Note that wioth arm64 memory types, a Non-Cacheable mapping of DRAM > for a non-coherent DMA mapping, or of VRAM in a prefetchable BAR, can > still be write-buffered, so barriers still matter even when actual > cache maintenance ops don't (and as before if you're trying to perform > cache maintenance outside the DMA API then you've already lost > anyway). MMIO registers should be mapped as Device memory via > ioremap(), which is not bufferable, hence the barrier implicit in > writel() effectively pushes out any prior buffered writes ahead of a > register write, which is why we don't need to worry about this most of > the time. > > This is only a very rough overview, though, and I'm not familiar > enough with x86 semantics, your hardware, or the exact use-case to be > able to say whether barriers alone are anywhere near the right answer > or not. > > Robin. > >> >> +Matt Roper >> >> Matt, Lucas, any feed back here? >> >> On 2022-03-02 4:49 a.m., Robin Murphy wrote: >>> On 2022-02-25 19:27, Michael Cheng wrote: >>>> Hi Robin, >>>> >>>> [ +arm64 maintainers for their awareness, which would have been a >>>> good thing to do from the start ] >>>> >>>> * Thanks for adding the arm64 maintainer and sorry I didn't rope >>>> them >>>> in sooner. >>>> >>>> Why does i915 need to ensure the CPU's instruction cache is >>>> coherent with its data cache? Is it a self-modifying driver? >>>> >>>> * Also thanks for pointing this out. Initially I was using >>>> dcache_clean_inval_poc, which seem to be the equivalently to what >>>> x86 is doing for dcache flushing, but it was giving me build >>>> errors >>>> since its not on the global list of kernel symbols. And after >>>> revisiting the documentation for caches_clean_inval_pou, it won't >>>> fly for what we are trying to do. Moving forward, what would >>>> you (or >>>> someone in the ARM community) suggest we do? Could it be >>>> possible to >>>> export dcache_clean_inval_poc as a global symbol? >>> >>> Unlikely, unless something with a legitimate need for CPU-centric >>> cache maintenance like kexec or CPU hotplug ever becomes modular. >>> >>> In the case of a device driver, it's not even the basic issues of >>> assuming to find direct equivalents to x86 semantics in other CPU >>> architectures, or effectively reinventing parts of the DMA API, it's >>> even bigger than that. Once you move from being integrated in a >>> single vendor's system architecture to being on a discrete card, you >>> fundamentally *no longer have any control over cache coherency*. >>> Whether the host CPU architecture happens to be AArch64, RISC-V, or >>> whatever doesn't really matter, you're at the mercy of 3rd-party >>> PCIe and interconnect IP vendors, and SoC integrators. You'll find >>> yourself in systems where PCIe simply cannot snoop any caches, where >>> you'd better have the correct DMA API calls in place to have any >>> hope of even the most basic functionality working properly; you'll >>> find yourself in systems where even if the PCIe root complex claims >>> to support No Snoop, your uncached traffic will still end up >>> snooping stale data that got prefetched back into caches you thought >>> you'd invalidated; you'll find yourself in systems where your memory >>> attributes may or may not get forcibly rewritten by an IOMMU >>> depending on the kernel config and/or command line. >>> >>> It's not about simply finding a substitute for clflush, it's that >>> the reasons you have for using clflush in the first place can no >>> longer be assumed to be valid. >>> >>> Robin. >>> >>>> On 2022-02-25 10:24 a.m., Robin Murphy wrote: >>>>> [ +arm64 maintainers for their awareness, which would have been a >>>>> good thing to do from the start ] >>>>> >>>>> On 2022-02-25 03:24, Michael Cheng wrote: >>>>>> Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou >>>>>> performs a flush by first performing a clean, follow by an >>>>>> invalidation >>>>>> operation. >>>>>> >>>>>> v2 (Michael Cheng): Use correct macro for cleaning and >>>>>> invalidation the >>>>>> dcache. Thanks Tvrtko for the suggestion. >>>>>> >>>>>> v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h >>>>>> >>>>>> v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc >>>>>> as a >>>>>> symbol that could be use by other modules, thus use >>>>>> caches_clean_inval_pou instead. Also this version >>>>>> removes include for cacheflush, since its already >>>>>> included base on architecture type. >>>>>> >>>>>> Signed-off-by: Michael Cheng <michael.cheng@intel.com> >>>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/drm_cache.c | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_cache.c >>>>>> b/drivers/gpu/drm/drm_cache.c >>>>>> index c3e6e615bf09..81c28714f930 100644 >>>>>> --- a/drivers/gpu/drm/drm_cache.c >>>>>> +++ b/drivers/gpu/drm/drm_cache.c >>>>>> @@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned >>>>>> long length) >>>>>> if (wbinvd_on_all_cpus()) >>>>>> pr_err("Timed out waiting for cache flush\n"); >>>>>> + >>>>>> +#elif defined(CONFIG_ARM64) >>>>>> + void *end = addr + length; >>>>>> + caches_clean_inval_pou((unsigned long)addr, (unsigned >>>>>> long)end); >>>>> >>>>> Why does i915 need to ensure the CPU's instruction cache is >>>>> coherent with its data cache? Is it a self-modifying driver? >>>>> >>>>> Robin. >>>>> >>>>> (Note that the above is somewhat of a loaded question, and I do >>>>> actually have half an idea of what you're trying to do here and >>>>> why it won't fly, but I'd like to at least assume you've read the >>>>> documentation of the function you decided was OK to use) >>>>> >>>>>> + >>>>>> #else >>>>>> WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); >>>>>> #endif ^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v12 2/6] drm/i915/gt: Re-work intel_write_status_page 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Re-work intel_write_status_page to use drm_clflush_virt_range. This will prevent compiler errors when building for non-x86 architectures. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine.h | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index be4b1e65442f..818468e0a02e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -4,6 +4,7 @@ #include <asm/cacheflush.h> #include <drm/drm_util.h> +#include <drm/drm_cache.h> #include <linux/hashtable.h> #include <linux/irq_work.h> @@ -143,15 +144,9 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value) * of extra paranoia to try and ensure that the HWS takes the value * we give and that it doesn't end up trapped inside the CPU! */ - if (static_cpu_has(X86_FEATURE_CLFLUSH)) { - mb(); - clflush(&engine->status_page.addr[reg]); - engine->status_page.addr[reg] = value; - clflush(&engine->status_page.addr[reg]); - mb(); - } else { - WRITE_ONCE(engine->status_page.addr[reg], value); - } + drm_clflush_virt_range(&engine->status_page.addr[reg], sizeof(value)); + WRITE_ONCE(engine->status_page.addr[reg], value); + drm_clflush_virt_range(&engine->status_page.addr[reg], sizeof(value)); } /* -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 2/6] drm/i915/gt: Re-work intel_write_status_page @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Re-work intel_write_status_page to use drm_clflush_virt_range. This will prevent compiler errors when building for non-x86 architectures. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine.h | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index be4b1e65442f..818468e0a02e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -4,6 +4,7 @@ #include <asm/cacheflush.h> #include <drm/drm_util.h> +#include <drm/drm_cache.h> #include <linux/hashtable.h> #include <linux/irq_work.h> @@ -143,15 +144,9 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value) * of extra paranoia to try and ensure that the HWS takes the value * we give and that it doesn't end up trapped inside the CPU! */ - if (static_cpu_has(X86_FEATURE_CLFLUSH)) { - mb(); - clflush(&engine->status_page.addr[reg]); - engine->status_page.addr[reg] = value; - clflush(&engine->status_page.addr[reg]); - mb(); - } else { - WRITE_ONCE(engine->status_page.addr[reg], value); - } + drm_clflush_virt_range(&engine->status_page.addr[reg], sizeof(value)); + WRITE_ONCE(engine->status_page.addr[reg], value); + drm_clflush_virt_range(&engine->status_page.addr[reg], sizeof(value)); } /* -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v12 3/6] drm/i915/gt: Drop invalidate_csb_entries 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Drop invalidate_csb_entries and directly call drm_clflush_virt_range. This allows for one less function call, and prevent complier errors when building for non-x86 architectures. v2(Michael Cheng): Drop invalidate_csb_entries function and directly invoke drm_clflush_virt_range. Thanks to Tvrtko for the sugguestion. v3(Michael Cheng): Use correct parameters for drm_clflush_virt_range. Thanks to Tvrtko for pointing this out. v4(Michael Cheng): Simplify &execlists->csb_status[0] to execlists->csb_status. Thanks to Matt Roper for the suggestion. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- .../gpu/drm/i915/gt/intel_execlists_submission.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 961d795220a3..e5e73a1b2e4e 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -1646,12 +1646,6 @@ cancel_port_requests(struct intel_engine_execlists * const execlists, return inactive; } -static void invalidate_csb_entries(const u64 *first, const u64 *last) -{ - clflush((void *)first); - clflush((void *)last); -} - /* * Starting with Gen12, the status has a new format: * @@ -1999,7 +1993,7 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive) * the wash as hardware, working or not, will need to do the * invalidation before. */ - invalidate_csb_entries(&buf[0], &buf[num_entries - 1]); + drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0])); /* * We assume that any event reflects a change in context flow @@ -2783,8 +2777,9 @@ static void reset_csb_pointers(struct intel_engine_cs *engine) /* Check that the GPU does indeed update the CSB entries! */ memset(execlists->csb_status, -1, (reset_value + 1) * sizeof(u64)); - invalidate_csb_entries(&execlists->csb_status[0], - &execlists->csb_status[reset_value]); + drm_clflush_virt_range(execlists->csb_status, + execlists->csb_size * + sizeof(execlists->csb_status)); /* Once more for luck and our trusty paranoia */ ENGINE_WRITE(engine, RING_CONTEXT_STATUS_PTR, -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 3/6] drm/i915/gt: Drop invalidate_csb_entries @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Drop invalidate_csb_entries and directly call drm_clflush_virt_range. This allows for one less function call, and prevent complier errors when building for non-x86 architectures. v2(Michael Cheng): Drop invalidate_csb_entries function and directly invoke drm_clflush_virt_range. Thanks to Tvrtko for the sugguestion. v3(Michael Cheng): Use correct parameters for drm_clflush_virt_range. Thanks to Tvrtko for pointing this out. v4(Michael Cheng): Simplify &execlists->csb_status[0] to execlists->csb_status. Thanks to Matt Roper for the suggestion. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- .../gpu/drm/i915/gt/intel_execlists_submission.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 961d795220a3..e5e73a1b2e4e 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -1646,12 +1646,6 @@ cancel_port_requests(struct intel_engine_execlists * const execlists, return inactive; } -static void invalidate_csb_entries(const u64 *first, const u64 *last) -{ - clflush((void *)first); - clflush((void *)last); -} - /* * Starting with Gen12, the status has a new format: * @@ -1999,7 +1993,7 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive) * the wash as hardware, working or not, will need to do the * invalidation before. */ - invalidate_csb_entries(&buf[0], &buf[num_entries - 1]); + drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0])); /* * We assume that any event reflects a change in context flow @@ -2783,8 +2777,9 @@ static void reset_csb_pointers(struct intel_engine_cs *engine) /* Check that the GPU does indeed update the CSB entries! */ memset(execlists->csb_status, -1, (reset_value + 1) * sizeof(u64)); - invalidate_csb_entries(&execlists->csb_status[0], - &execlists->csb_status[reset_value]); + drm_clflush_virt_range(execlists->csb_status, + execlists->csb_size * + sizeof(execlists->csb_status)); /* Once more for luck and our trusty paranoia */ ENGINE_WRITE(engine, RING_CONTEXT_STATUS_PTR, -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v12 4/6] drm/i915/gt: Re-work reset_csb 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Use drm_clflush_virt_range instead of directly invoking clflush. This will prevent compiler errors when building for non-x86 architectures. v2(Michael Cheng): Remove extra clflush v3(Michael Cheng): Remove memory barrier since drm_clflush_virt_range takes care of it. v4(Michael Cheng): Get the size of value and not the size of the pointer when passing in execlists->csb_write. Thanks to Matt Roper for pointing this out. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index e5e73a1b2e4e..89aef3ce53f0 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2945,9 +2945,8 @@ reset_csb(struct intel_engine_cs *engine, struct i915_request **inactive) { struct intel_engine_execlists * const execlists = &engine->execlists; - mb(); /* paranoia: read the CSB pointers from after the reset */ - clflush(execlists->csb_write); - mb(); + drm_clflush_virt_range(execlists->csb_write, + sizeof(execlists->csb_write[0])); inactive = process_csb(engine, inactive); /* drain preemption events */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 4/6] drm/i915/gt: Re-work reset_csb @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Use drm_clflush_virt_range instead of directly invoking clflush. This will prevent compiler errors when building for non-x86 architectures. v2(Michael Cheng): Remove extra clflush v3(Michael Cheng): Remove memory barrier since drm_clflush_virt_range takes care of it. v4(Michael Cheng): Get the size of value and not the size of the pointer when passing in execlists->csb_write. Thanks to Matt Roper for pointing this out. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index e5e73a1b2e4e..89aef3ce53f0 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2945,9 +2945,8 @@ reset_csb(struct intel_engine_cs *engine, struct i915_request **inactive) { struct intel_engine_execlists * const execlists = &engine->execlists; - mb(); /* paranoia: read the CSB pointers from after the reset */ - clflush(execlists->csb_write); - mb(); + drm_clflush_virt_range(execlists->csb_write, + sizeof(execlists->csb_write[0])); inactive = process_csb(engine, inactive); /* drain preemption events */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v12 5/6] drm/i915/: Re-work clflush_write32 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Use drm_clflush_virt_range instead of clflushopt and remove the memory barrier, since drm_clflush_virt_range takes care of that. v2(Michael Cheng): Use sizeof(*addr) instead of sizeof(addr) to get the actual size of the page. Thanks to Matt Roper for pointing this out. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 89aa0557ade1..0ca6c3d810da 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1333,10 +1333,8 @@ static void *reloc_vaddr(struct i915_vma *vma, static void clflush_write32(u32 *addr, u32 value, unsigned int flushes) { if (unlikely(flushes & (CLFLUSH_BEFORE | CLFLUSH_AFTER))) { - if (flushes & CLFLUSH_BEFORE) { - clflushopt(addr); - mb(); - } + if (flushes & CLFLUSH_BEFORE) + drm_clflush_virt_range(addr, sizeof(*addr)); *addr = value; @@ -1348,7 +1346,7 @@ static void clflush_write32(u32 *addr, u32 value, unsigned int flushes) * to ensure ordering of clflush wrt to the system. */ if (flushes & CLFLUSH_AFTER) - clflushopt(addr); + drm_clflush_virt_range(addr, sizeof(*addr)); } else *addr = value; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 5/6] drm/i915/: Re-work clflush_write32 @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Use drm_clflush_virt_range instead of clflushopt and remove the memory barrier, since drm_clflush_virt_range takes care of that. v2(Michael Cheng): Use sizeof(*addr) instead of sizeof(addr) to get the actual size of the page. Thanks to Matt Roper for pointing this out. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 89aa0557ade1..0ca6c3d810da 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1333,10 +1333,8 @@ static void *reloc_vaddr(struct i915_vma *vma, static void clflush_write32(u32 *addr, u32 value, unsigned int flushes) { if (unlikely(flushes & (CLFLUSH_BEFORE | CLFLUSH_AFTER))) { - if (flushes & CLFLUSH_BEFORE) { - clflushopt(addr); - mb(); - } + if (flushes & CLFLUSH_BEFORE) + drm_clflush_virt_range(addr, sizeof(*addr)); *addr = value; @@ -1348,7 +1346,7 @@ static void clflush_write32(u32 *addr, u32 value, unsigned int flushes) * to ensure ordering of clflush wrt to the system. */ if (flushes & CLFLUSH_AFTER) - clflushopt(addr); + drm_clflush_virt_range(addr, sizeof(*addr)); } else *addr = value; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v12 6/6] drm/i915/gt: replace cache_clflush_range 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng @ 2022-02-25 3:24 ` Michael Cheng -1 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx Cc: tvrtko.ursulin, michael.cheng, balasubramani.vivekanandan, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel Replace all occurrence of cache_clflush_range with drm_clflush_virt_range. This will prevent compile errors on non-x86 platforms. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 ++++++------ drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index f574da00eff1..c7bd5d71b03e 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -454,11 +454,11 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, pd = pdp->entry[gen8_pd_index(idx, 2)]; } - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); vaddr = px_vaddr(i915_pt_entry(pd, gen8_pd_index(idx, 1))); } } while (1); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); return idx; } @@ -631,7 +631,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, } } while (rem >= page_size && index < I915_PDES); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); /* * Is it safe to mark the 2M block as 64K? -- Either we have @@ -647,7 +647,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, I915_GTT_PAGE_SIZE_2M)))) { vaddr = px_vaddr(pd); vaddr[maybe_64K] |= GEN8_PDE_IPS_64K; - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); page_size = I915_GTT_PAGE_SIZE_64K; /* @@ -668,7 +668,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, for (i = 1; i < index; i += 16) memset64(vaddr + i, encode, 15); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); } } @@ -722,7 +722,7 @@ static void gen8_ppgtt_insert_entry(struct i915_address_space *vm, vaddr = px_vaddr(pt); vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); - clflush_cache_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); + drm_clflush_virt_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); } static void __xehpsdv_ppgtt_insert_entry_lm(struct i915_address_space *vm, diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 89aef3ce53f0..d6f02dce45a0 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2823,7 +2823,7 @@ static void execlists_sanitize(struct intel_engine_cs *engine) sanitize_hwsp(engine); /* And scrub the dirty cachelines for the HWSP */ - clflush_cache_range(engine->status_page.addr, PAGE_SIZE); + drm_clflush_virt_range(engine->status_page.addr, PAGE_SIZE); intel_engine_reset_pinned_contexts(engine); } diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index c548c193cd35..fc314946d426 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -268,7 +268,7 @@ fill_page_dma(struct drm_i915_gem_object *p, const u64 val, unsigned int count) void *vaddr = __px_vaddr(p); memset64(vaddr, val, count); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); } static void poison_scratch_page(struct drm_i915_gem_object *scratch) diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c index d91e2beb7517..d8b94d638559 100644 --- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c @@ -91,7 +91,7 @@ write_dma_entry(struct drm_i915_gem_object * const pdma, u64 * const vaddr = __px_vaddr(pdma); vaddr[idx] = encoded_entry; - clflush_cache_range(&vaddr[idx], sizeof(u64)); + drm_clflush_virt_range(&vaddr[idx], sizeof(u64)); } void diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index b3a429a92c0d..89020706adc4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -3573,7 +3573,7 @@ static void guc_sanitize(struct intel_engine_cs *engine) sanitize_hwsp(engine); /* And scrub the dirty cachelines for the HWSP */ - clflush_cache_range(engine->status_page.addr, PAGE_SIZE); + drm_clflush_virt_range(engine->status_page.addr, PAGE_SIZE); intel_engine_reset_pinned_contexts(engine); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] [PATCH v12 6/6] drm/i915/gt: replace cache_clflush_range @ 2022-02-25 3:24 ` Michael Cheng 0 siblings, 0 replies; 55+ messages in thread From: Michael Cheng @ 2022-02-25 3:24 UTC (permalink / raw) To: intel-gfx; +Cc: michael.cheng, lucas.demarchi, dri-devel Replace all occurrence of cache_clflush_range with drm_clflush_virt_range. This will prevent compile errors on non-x86 platforms. Signed-off-by: Michael Cheng <michael.cheng@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 ++++++------ drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index f574da00eff1..c7bd5d71b03e 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -454,11 +454,11 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt, pd = pdp->entry[gen8_pd_index(idx, 2)]; } - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); vaddr = px_vaddr(i915_pt_entry(pd, gen8_pd_index(idx, 1))); } } while (1); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); return idx; } @@ -631,7 +631,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, } } while (rem >= page_size && index < I915_PDES); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); /* * Is it safe to mark the 2M block as 64K? -- Either we have @@ -647,7 +647,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, I915_GTT_PAGE_SIZE_2M)))) { vaddr = px_vaddr(pd); vaddr[maybe_64K] |= GEN8_PDE_IPS_64K; - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); page_size = I915_GTT_PAGE_SIZE_64K; /* @@ -668,7 +668,7 @@ static void gen8_ppgtt_insert_huge(struct i915_address_space *vm, for (i = 1; i < index; i += 16) memset64(vaddr + i, encode, 15); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); } } @@ -722,7 +722,7 @@ static void gen8_ppgtt_insert_entry(struct i915_address_space *vm, vaddr = px_vaddr(pt); vaddr[gen8_pd_index(idx, 0)] = gen8_pte_encode(addr, level, flags); - clflush_cache_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); + drm_clflush_virt_range(&vaddr[gen8_pd_index(idx, 0)], sizeof(*vaddr)); } static void __xehpsdv_ppgtt_insert_entry_lm(struct i915_address_space *vm, diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 89aef3ce53f0..d6f02dce45a0 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2823,7 +2823,7 @@ static void execlists_sanitize(struct intel_engine_cs *engine) sanitize_hwsp(engine); /* And scrub the dirty cachelines for the HWSP */ - clflush_cache_range(engine->status_page.addr, PAGE_SIZE); + drm_clflush_virt_range(engine->status_page.addr, PAGE_SIZE); intel_engine_reset_pinned_contexts(engine); } diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index c548c193cd35..fc314946d426 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -268,7 +268,7 @@ fill_page_dma(struct drm_i915_gem_object *p, const u64 val, unsigned int count) void *vaddr = __px_vaddr(p); memset64(vaddr, val, count); - clflush_cache_range(vaddr, PAGE_SIZE); + drm_clflush_virt_range(vaddr, PAGE_SIZE); } static void poison_scratch_page(struct drm_i915_gem_object *scratch) diff --git a/drivers/gpu/drm/i915/gt/intel_ppgtt.c b/drivers/gpu/drm/i915/gt/intel_ppgtt.c index d91e2beb7517..d8b94d638559 100644 --- a/drivers/gpu/drm/i915/gt/intel_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ppgtt.c @@ -91,7 +91,7 @@ write_dma_entry(struct drm_i915_gem_object * const pdma, u64 * const vaddr = __px_vaddr(pdma); vaddr[idx] = encoded_entry; - clflush_cache_range(&vaddr[idx], sizeof(u64)); + drm_clflush_virt_range(&vaddr[idx], sizeof(u64)); } void diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index b3a429a92c0d..89020706adc4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -3573,7 +3573,7 @@ static void guc_sanitize(struct intel_engine_cs *engine) sanitize_hwsp(engine); /* And scrub the dirty cachelines for the HWSP */ - clflush_cache_range(engine->status_page.addr, PAGE_SIZE); + drm_clflush_virt_range(engine->status_page.addr, PAGE_SIZE); intel_engine_reset_pinned_contexts(engine); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Use drm_clflush* instead of clflush 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng ` (6 preceding siblings ...) (?) @ 2022-02-25 7:28 ` Patchwork -1 siblings, 0 replies; 55+ messages in thread From: Patchwork @ 2022-02-25 7:28 UTC (permalink / raw) To: Michael Cheng; +Cc: intel-gfx == Series Details == Series: Use drm_clflush* instead of clflush URL : https://patchwork.freedesktop.org/series/100717/ State : warning == Summary == $ dim checkpatch origin/drm-tip 86596975e130 drm: Add arch arm64 for drm_clflush_virt_range -:35: WARNING:LINE_SPACING: Missing a blank line after declarations #35: FILE: drivers/gpu/drm/drm_cache.c:180: + void *end = addr + length; + caches_clean_inval_pou((unsigned long)addr, (unsigned long)end); total: 0 errors, 1 warnings, 0 checks, 11 lines checked 74d1a9bccc41 drm/i915/gt: Re-work intel_write_status_page 08cf76981869 drm/i915/gt: Drop invalidate_csb_entries 5568b6fef0f6 drm/i915/gt: Re-work reset_csb 78a5efb9813c drm/i915/: Re-work clflush_write32 43b085a28892 drm/i915/gt: replace cache_clflush_range ^ permalink raw reply [flat|nested] 55+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Use drm_clflush* instead of clflush 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng ` (7 preceding siblings ...) (?) @ 2022-02-25 7:29 ` Patchwork -1 siblings, 0 replies; 55+ messages in thread From: Patchwork @ 2022-02-25 7:29 UTC (permalink / raw) To: Michael Cheng; +Cc: intel-gfx == Series Details == Series: Use drm_clflush* instead of clflush URL : https://patchwork.freedesktop.org/series/100717/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. ^ permalink raw reply [flat|nested] 55+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Use drm_clflush* instead of clflush 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng ` (8 preceding siblings ...) (?) @ 2022-02-25 7:59 ` Patchwork -1 siblings, 0 replies; 55+ messages in thread From: Patchwork @ 2022-02-25 7:59 UTC (permalink / raw) To: Michael Cheng; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 6556 bytes --] == Series Details == Series: Use drm_clflush* instead of clflush URL : https://patchwork.freedesktop.org/series/100717/ State : success == Summary == CI Bug Log - changes from CI_DRM_11285 -> Patchwork_22408 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/index.html Participating hosts (39 -> 39) ------------------------------ Additional (2): fi-kbl-soraka fi-pnv-d510 Missing (2): bat-rpls-2 fi-bdw-samus Known issues ------------ Here are the changes found in Patchwork_22408 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_fence@basic-busy@bcs0: - fi-kbl-soraka: NOTRUN -> [SKIP][1] ([fdo#109271]) +8 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@gem_exec_fence@basic-busy@bcs0.html * igt@gem_huc_copy@huc-copy: - fi-skl-6600u: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html - fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html * igt@gem_lmem_swapping@basic: - fi-kbl-soraka: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html * igt@gem_lmem_swapping@verify-random: - fi-skl-6600u: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html * igt@i915_selftest@live@gt_pm: - fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][6] ([i915#1886] / [i915#2291]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html * igt@i915_selftest@live@hangcheck: - fi-snb-2600: [PASS][7] -> [INCOMPLETE][8] ([i915#3921]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/fi-snb-2600/igt@i915_selftest@live@hangcheck.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-snb-2600/igt@i915_selftest@live@hangcheck.html * igt@i915_selftest@live@requests: - fi-pnv-d510: NOTRUN -> [DMESG-FAIL][9] ([i915#2927] / [i915#4528]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-pnv-d510/igt@i915_selftest@live@requests.html * igt@kms_chamelium@common-hpd-after-suspend: - fi-kbl-soraka: NOTRUN -> [SKIP][10] ([fdo#109271] / [fdo#111827]) +8 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@kms_chamelium@common-hpd-after-suspend.html * igt@kms_chamelium@vga-edid-read: - fi-skl-6600u: NOTRUN -> [SKIP][11] ([fdo#109271] / [fdo#111827]) +8 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - fi-skl-6600u: NOTRUN -> [SKIP][12] ([fdo#109271]) +2 similar issues [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d: - fi-skl-6600u: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#533]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html - fi-kbl-soraka: NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#533]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-kbl-soraka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html * igt@kms_psr@primary_page_flip: - fi-skl-6600u: NOTRUN -> [INCOMPLETE][15] ([i915#4838]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-skl-6600u/igt@kms_psr@primary_page_flip.html * igt@prime_vgem@basic-userptr: - fi-pnv-d510: NOTRUN -> [SKIP][16] ([fdo#109271]) +39 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-pnv-d510/igt@prime_vgem@basic-userptr.html * igt@runner@aborted: - fi-pnv-d510: NOTRUN -> [FAIL][17] ([fdo#109271] / [i915#2403] / [i915#4312]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/fi-pnv-d510/igt@runner@aborted.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291 [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403 [i915#2927]: https://gitlab.freedesktop.org/drm/intel/issues/2927 [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921 [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#4838]: https://gitlab.freedesktop.org/drm/intel/issues/4838 [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533 Build changes ------------- * Linux: CI_DRM_11285 -> Patchwork_22408 CI-20190529: 20190529 CI_DRM_11285: 937927c7022766ecf991e0071ae055412fb3805f @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6357: 6546304ecf053b9c5ec278ee3c210d2c6d50a3a6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_22408: 43b085a28892e876f3525f69c5eff2601fde8326 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 43b085a28892 drm/i915/gt: replace cache_clflush_range 78a5efb9813c drm/i915/: Re-work clflush_write32 5568b6fef0f6 drm/i915/gt: Re-work reset_csb 08cf76981869 drm/i915/gt: Drop invalidate_csb_entries 74d1a9bccc41 drm/i915/gt: Re-work intel_write_status_page 86596975e130 drm: Add arch arm64 for drm_clflush_virt_range == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/index.html [-- Attachment #2: Type: text/html, Size: 8499 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for Use drm_clflush* instead of clflush 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng ` (9 preceding siblings ...) (?) @ 2022-02-26 1:56 ` Patchwork -1 siblings, 0 replies; 55+ messages in thread From: Patchwork @ 2022-02-26 1:56 UTC (permalink / raw) To: Michael Cheng; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 30259 bytes --] == Series Details == Series: Use drm_clflush* instead of clflush URL : https://patchwork.freedesktop.org/series/100717/ State : failure == Summary == CI Bug Log - changes from CI_DRM_11285_full -> Patchwork_22408_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_22408_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_22408_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (11 -> 11) ------------------------------ No changes in participating hosts Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_22408_full: ### IGT changes ### #### Possible regressions #### * igt@gem_workarounds@reset: - shard-snb: [PASS][1] -> [TIMEOUT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-snb5/igt@gem_workarounds@reset.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-snb4/igt@gem_workarounds@reset.html * igt@i915_suspend@forcewake: - shard-iclb: [PASS][3] -> [SKIP][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb4/igt@i915_suspend@forcewake.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb5/igt@i915_suspend@forcewake.html Known issues ------------ Here are the changes found in Patchwork_22408_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@feature_discovery@display-4x: - shard-iclb: NOTRUN -> [SKIP][5] ([i915#1839]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@feature_discovery@display-4x.html * igt@gem_ctx_persistence@engines-hang: - shard-snb: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#1099]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-snb6/igt@gem_ctx_persistence@engines-hang.html * igt@gem_eio@in-flight-contexts-immediate: - shard-tglb: [PASS][7] -> [TIMEOUT][8] ([i915#3063]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-tglb5/igt@gem_eio@in-flight-contexts-immediate.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-tglb5/igt@gem_eio@in-flight-contexts-immediate.html * igt@gem_exec_endless@dispatch@vecs0: - shard-tglb: [PASS][9] -> [INCOMPLETE][10] ([i915#3778]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-tglb2/igt@gem_exec_endless@dispatch@vecs0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-tglb6/igt@gem_exec_endless@dispatch@vecs0.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-skl: NOTRUN -> [SKIP][11] ([fdo#109271]) +108 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@gem_exec_fair@basic-flow@rcs0.html * igt@gem_exec_fair@basic-none-rrul@rcs0: - shard-kbl: NOTRUN -> [FAIL][12] ([i915#2842]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl1/igt@gem_exec_fair@basic-none-rrul@rcs0.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-glk: [PASS][13] -> [FAIL][14] ([i915#2842]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html * igt@gem_exec_fair@basic-pace@rcs0: - shard-kbl: [PASS][15] -> [FAIL][16] ([i915#2842]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-kbl1/igt@gem_exec_fair@basic-pace@rcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl4/igt@gem_exec_fair@basic-pace@rcs0.html * igt@gem_exec_fair@basic-pace@vcs1: - shard-iclb: [PASS][17] -> [FAIL][18] ([i915#2842]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb1/igt@gem_exec_fair@basic-pace@vcs1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb1/igt@gem_exec_fair@basic-pace@vcs1.html * igt@gem_exec_schedule@smoketest@rcs0: - shard-glk: [PASS][19] -> [DMESG-WARN][20] ([i915#118]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-glk5/igt@gem_exec_schedule@smoketest@rcs0.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-glk6/igt@gem_exec_schedule@smoketest@rcs0.html * igt@gem_lmem_swapping@heavy-multi: - shard-apl: NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#4613]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl2/igt@gem_lmem_swapping@heavy-multi.html - shard-kbl: NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#4613]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@gem_lmem_swapping@heavy-multi.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-skl: NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#4613]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@gem_lmem_swapping@heavy-verify-multi.html * igt@gem_ppgtt@flink-and-close-vma-leak: - shard-apl: [PASS][24] -> [FAIL][25] ([i915#644]) [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl4/igt@gem_ppgtt@flink-and-close-vma-leak.html [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl6/igt@gem_ppgtt@flink-and-close-vma-leak.html * igt@gem_pread@exhaustion: - shard-apl: NOTRUN -> [WARN][26] ([i915#2658]) [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl1/igt@gem_pread@exhaustion.html * igt@gem_pwrite@basic-exhaustion: - shard-skl: NOTRUN -> [WARN][27] ([i915#2658]) [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@gem_pwrite@basic-exhaustion.html * igt@gem_pxp@regular-baseline-src-copy-readible: - shard-iclb: NOTRUN -> [SKIP][28] ([i915#4270]) [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb6/igt@gem_pxp@regular-baseline-src-copy-readible.html * igt@gem_render_copy@y-tiled-mc-ccs-to-y-tiled-ccs: - shard-iclb: NOTRUN -> [SKIP][29] ([i915#768]) +1 similar issue [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@gem_render_copy@y-tiled-mc-ccs-to-y-tiled-ccs.html * igt@gem_userptr_blits@dmabuf-sync: - shard-iclb: NOTRUN -> [SKIP][30] ([i915#3323]) [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@gem_userptr_blits@dmabuf-sync.html * igt@gem_userptr_blits@dmabuf-unsync: - shard-iclb: NOTRUN -> [SKIP][31] ([i915#3297]) [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb6/igt@gem_userptr_blits@dmabuf-unsync.html * igt@gen7_exec_parse@basic-offset: - shard-iclb: NOTRUN -> [SKIP][32] ([fdo#109289]) [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@gen7_exec_parse@basic-offset.html * igt@gen9_exec_parse@bb-start-cmd: - shard-iclb: NOTRUN -> [SKIP][33] ([i915#2856]) [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@gen9_exec_parse@bb-start-cmd.html * igt@i915_pm_rpm@modeset-pc8-residency-stress: - shard-iclb: NOTRUN -> [SKIP][34] ([fdo#109293] / [fdo#109506]) [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@i915_pm_rpm@modeset-pc8-residency-stress.html * igt@i915_pm_sseu@full-enable: - shard-skl: [PASS][35] -> [FAIL][36] ([i915#3048]) [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl10/igt@i915_pm_sseu@full-enable.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl3/igt@i915_pm_sseu@full-enable.html * igt@kms_big_fb@x-tiled-8bpp-rotate-270: - shard-iclb: NOTRUN -> [SKIP][37] ([fdo#110725] / [fdo#111614]) [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_big_fb@x-tiled-8bpp-rotate-270.html * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip: - shard-apl: NOTRUN -> [SKIP][38] ([fdo#109271] / [i915#3777]) +2 similar issues [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl2/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip: - shard-kbl: NOTRUN -> [SKIP][39] ([fdo#109271] / [i915#3777]) +1 similar issue [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip.html * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-async-flip: - shard-skl: NOTRUN -> [FAIL][40] ([i915#3763]) [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl7/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-async-flip.html * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip: - shard-skl: NOTRUN -> [SKIP][41] ([fdo#109271] / [i915#3777]) [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl7/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip.html * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs: - shard-apl: NOTRUN -> [SKIP][42] ([fdo#109271]) +127 similar issues [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl4/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs.html * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs: - shard-iclb: NOTRUN -> [SKIP][43] ([fdo#109278] / [i915#3886]) +1 similar issue [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc: - shard-apl: NOTRUN -> [SKIP][44] ([fdo#109271] / [i915#3886]) +8 similar issues [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl4/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html * igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc: - shard-skl: NOTRUN -> [SKIP][45] ([fdo#109271] / [i915#3886]) +3 similar issues [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc: - shard-kbl: NOTRUN -> [SKIP][46] ([fdo#109271] / [i915#3886]) +3 similar issues [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl7/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html * igt@kms_cdclk@mode-transition: - shard-iclb: NOTRUN -> [SKIP][47] ([i915#3742]) [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_cdclk@mode-transition.html * igt@kms_chamelium@vga-hpd-for-each-pipe: - shard-apl: NOTRUN -> [SKIP][48] ([fdo#109271] / [fdo#111827]) +10 similar issues [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl1/igt@kms_chamelium@vga-hpd-for-each-pipe.html * igt@kms_color_chamelium@pipe-b-ctm-0-25: - shard-skl: NOTRUN -> [SKIP][49] ([fdo#109271] / [fdo#111827]) +5 similar issues [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl9/igt@kms_color_chamelium@pipe-b-ctm-0-25.html * igt@kms_color_chamelium@pipe-b-ctm-limited-range: - shard-kbl: NOTRUN -> [SKIP][50] ([fdo#109271] / [fdo#111827]) +4 similar issues [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@kms_color_chamelium@pipe-b-ctm-limited-range.html * igt@kms_color_chamelium@pipe-c-ctm-max: - shard-iclb: NOTRUN -> [SKIP][51] ([fdo#109284] / [fdo#111827]) +4 similar issues [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_color_chamelium@pipe-c-ctm-max.html * igt@kms_cursor_crc@pipe-b-cursor-512x170-rapid-movement: - shard-tglb: NOTRUN -> [SKIP][52] ([i915#3359]) [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-tglb2/igt@kms_cursor_crc@pipe-b-cursor-512x170-rapid-movement.html * igt@kms_cursor_crc@pipe-c-cursor-256x85-rapid-movement: - shard-snb: NOTRUN -> [SKIP][53] ([fdo#109271]) +36 similar issues [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-snb6/igt@kms_cursor_crc@pipe-c-cursor-256x85-rapid-movement.html * igt@kms_cursor_crc@pipe-c-cursor-512x170-random: - shard-iclb: NOTRUN -> [SKIP][54] ([fdo#109278] / [fdo#109279]) +1 similar issue [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_cursor_crc@pipe-c-cursor-512x170-random.html * igt@kms_cursor_crc@pipe-d-cursor-32x32-rapid-movement: - shard-iclb: NOTRUN -> [SKIP][55] ([fdo#109278]) +13 similar issues [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_cursor_crc@pipe-d-cursor-32x32-rapid-movement.html * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: - shard-iclb: NOTRUN -> [SKIP][56] ([fdo#109274] / [fdo#109278]) +1 similar issue [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size: - shard-skl: [PASS][57] -> [FAIL][58] ([i915#2346] / [i915#533]) [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html * igt@kms_flip@2x-flip-vs-fences: - shard-iclb: NOTRUN -> [SKIP][59] ([fdo#109274]) +1 similar issue [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_flip@2x-flip-vs-fences.html * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2: - shard-glk: [PASS][60] -> [FAIL][61] ([i915#79]) +1 similar issue [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-glk6/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-glk5/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html * igt@kms_flip@flip-vs-suspend-interruptible@b-edp1: - shard-iclb: [PASS][62] -> [DMESG-WARN][63] ([i915#4391]) [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb6/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb7/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html * igt@kms_flip@flip-vs-suspend-interruptible@c-edp1: - shard-iclb: [PASS][64] -> [DMESG-WARN][65] ([i915#2867]) [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb6/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb7/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1: - shard-skl: [PASS][66] -> [FAIL][67] ([i915#2122]) +2 similar issues [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl4/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-blt: - shard-iclb: NOTRUN -> [SKIP][68] ([fdo#109280]) +7 similar issues [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-blt.html * igt@kms_frontbuffer_tracking@fbc-suspend: - shard-kbl: NOTRUN -> [DMESG-WARN][69] ([i915#180]) [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html * igt@kms_hdr@static-toggle: - shard-iclb: NOTRUN -> [SKIP][70] ([i915#1187]) [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_hdr@static-toggle.html * igt@kms_pipe_crc_basic@disable-crc-after-crtc-pipe-d: - shard-skl: NOTRUN -> [SKIP][71] ([fdo#109271] / [i915#533]) [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl9/igt@kms_pipe_crc_basic@disable-crc-after-crtc-pipe-d.html * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes: - shard-kbl: [PASS][72] -> [DMESG-WARN][73] ([i915#180]) +3 similar issues [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc: - shard-skl: NOTRUN -> [FAIL][74] ([fdo#108145] / [i915#265]) +1 similar issue [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max: - shard-kbl: NOTRUN -> [FAIL][75] ([fdo#108145] / [i915#265]) +1 similar issue [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max.html * igt@kms_plane_alpha_blend@pipe-b-alpha-basic: - shard-apl: NOTRUN -> [FAIL][76] ([fdo#108145] / [i915#265]) +2 similar issues [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl2/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html * igt@kms_psr2_sf@cursor-plane-update-sf: - shard-skl: NOTRUN -> [SKIP][77] ([fdo#109271] / [i915#658]) +1 similar issue [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl8/igt@kms_psr2_sf@cursor-plane-update-sf.html * igt@kms_psr2_sf@overlay-plane-update-continuous-sf: - shard-kbl: NOTRUN -> [SKIP][78] ([fdo#109271] / [i915#658]) +1 similar issue [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@kms_psr2_sf@overlay-plane-update-continuous-sf.html * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area: - shard-apl: NOTRUN -> [SKIP][79] ([fdo#109271] / [i915#658]) +2 similar issues [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl1/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html * igt@kms_psr2_su@page_flip-xrgb8888: - shard-iclb: NOTRUN -> [SKIP][80] ([fdo#109642] / [fdo#111068] / [i915#658]) [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_psr2_su@page_flip-xrgb8888.html * igt@kms_psr@psr2_cursor_render: - shard-iclb: [PASS][81] -> [SKIP][82] ([fdo#109441]) +2 similar issues [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb2/igt@kms_psr@psr2_cursor_render.html [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb6/igt@kms_psr@psr2_cursor_render.html * igt@kms_psr@psr2_dpms: - shard-iclb: NOTRUN -> [SKIP][83] ([fdo#109441]) [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@kms_psr@psr2_dpms.html * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d: - shard-kbl: NOTRUN -> [SKIP][84] ([fdo#109271]) +58 similar issues [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl1/igt@kms_universal_plane@disable-primary-vs-flip-pipe-d.html * igt@kms_vblank@pipe-a-ts-continuation-suspend: - shard-apl: [PASS][85] -> [DMESG-WARN][86] ([i915#180] / [i915#295]) [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl8/igt@kms_vblank@pipe-a-ts-continuation-suspend.html * igt@kms_vblank@pipe-d-wait-idle: - shard-apl: NOTRUN -> [SKIP][87] ([fdo#109271] / [i915#533]) [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl1/igt@kms_vblank@pipe-d-wait-idle.html * igt@kms_writeback@writeback-pixel-formats: - shard-skl: NOTRUN -> [SKIP][88] ([fdo#109271] / [i915#2437]) [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl9/igt@kms_writeback@writeback-pixel-formats.html * igt@nouveau_crc@pipe-a-source-rg: - shard-iclb: NOTRUN -> [SKIP][89] ([i915#2530]) [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@nouveau_crc@pipe-a-source-rg.html * igt@prime_nv_api@i915_self_import: - shard-iclb: NOTRUN -> [SKIP][90] ([fdo#109291]) [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@prime_nv_api@i915_self_import.html * igt@prime_vgem@fence-read-hang: - shard-iclb: NOTRUN -> [SKIP][91] ([fdo#109295]) [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@prime_vgem@fence-read-hang.html * igt@sysfs_clients@create: - shard-apl: NOTRUN -> [SKIP][92] ([fdo#109271] / [i915#2994]) +3 similar issues [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl1/igt@sysfs_clients@create.html * igt@sysfs_clients@sema-50: - shard-kbl: NOTRUN -> [SKIP][93] ([fdo#109271] / [i915#2994]) +1 similar issue [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@sysfs_clients@sema-50.html * igt@sysfs_clients@split-50: - shard-iclb: NOTRUN -> [SKIP][94] ([i915#2994]) [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@sysfs_clients@split-50.html #### Possible fixes #### * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglb: [FAIL][95] ([i915#2842]) -> [PASS][96] [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-tglb6/igt@gem_exec_fair@basic-flow@rcs0.html [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html * igt@gem_exec_suspend@basic-s3@smem: - shard-snb: [SKIP][97] ([fdo#109271]) -> [PASS][98] [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-snb6/igt@gem_exec_suspend@basic-s3@smem.html [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-snb6/igt@gem_exec_suspend@basic-s3@smem.html * igt@gem_exec_whisper@basic-forked: - shard-glk: [DMESG-WARN][99] ([i915#118]) -> [PASS][100] +1 similar issue [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-glk7/igt@gem_exec_whisper@basic-forked.html [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-glk7/igt@gem_exec_whisper@basic-forked.html * igt@i915_pm_dc@dc6-psr: - shard-iclb: [FAIL][101] ([i915#454]) -> [PASS][102] [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb3/igt@i915_pm_dc@dc6-psr.html [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb7/igt@i915_pm_dc@dc6-psr.html * igt@i915_selftest@live@hangcheck: - shard-snb: [INCOMPLETE][103] ([i915#3921]) -> [PASS][104] [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-snb6/igt@i915_selftest@live@hangcheck.html [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-snb6/igt@i915_selftest@live@hangcheck.html * igt@kms_fbcon_fbt@fbc-suspend: - shard-kbl: [INCOMPLETE][105] ([i915#180] / [i915#636]) -> [PASS][106] [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-kbl1/igt@kms_fbcon_fbt@fbc-suspend.html [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl3/igt@kms_fbcon_fbt@fbc-suspend.html - shard-apl: [INCOMPLETE][107] ([i915#180]) -> [PASS][108] [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl8/igt@kms_fbcon_fbt@fbc-suspend.html [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl2/igt@kms_fbcon_fbt@fbc-suspend.html * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ac-hdmi-a1-hdmi-a2: - shard-glk: [FAIL][109] ([i915#2122]) -> [PASS][110] [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-glk3/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ac-hdmi-a1-hdmi-a2.html [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ac-hdmi-a1-hdmi-a2.html * igt@kms_flip@flip-vs-suspend@a-dp1: - shard-apl: [DMESG-WARN][111] ([i915#180]) -> [PASS][112] +1 similar issue [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl2/igt@kms_flip@flip-vs-suspend@a-dp1.html [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl4/igt@kms_flip@flip-vs-suspend@a-dp1.html * igt@kms_flip@plain-flip-fb-recreate@b-edp1: - shard-skl: [FAIL][113] ([i915#2122]) -> [PASS][114] [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl3/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl7/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html * igt@kms_hdr@bpc-switch-suspend: - shard-kbl: [DMESG-WARN][115] ([i915#180]) -> [PASS][116] +2 similar issues [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-kbl1/igt@kms_hdr@bpc-switch-suspend.html [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-kbl7/igt@kms_hdr@bpc-switch-suspend.html - shard-skl: [FAIL][117] ([i915#1188]) -> [PASS][118] [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl4/igt@kms_hdr@bpc-switch-suspend.html [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html * igt@kms_plane_lowres@pipe-a-tiling-x: - {shard-tglu}: [FAIL][119] ([i915#3957]) -> [PASS][120] [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-tglu-1/igt@kms_plane_lowres@pipe-a-tiling-x.html [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-tglu-3/igt@kms_plane_lowres@pipe-a-tiling-x.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-iclb: [SKIP][121] ([fdo#109441]) -> [PASS][122] [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb5/igt@kms_psr@psr2_cursor_plane_onoff.html [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html * igt@perf@polling-small-buf: - shard-skl: [FAIL][123] ([i915#1722]) -> [PASS][124] [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-skl9/igt@perf@polling-small-buf.html [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-skl6/igt@perf@polling-small-buf.html #### Warnings #### * igt@gem_exec_balancer@parallel: - shard-iclb: [SKIP][125] ([i915#4525]) -> [DMESG-WARN][126] ([i915#5076]) +2 similar issues [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb5/igt@gem_exec_balancer@parallel.html [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb2/igt@gem_exec_balancer@parallel.html * igt@gem_exec_balancer@parallel-keep-submit-fence: - shard-iclb: [DMESG-WARN][127] ([i915#5076]) -> [SKIP][128] ([i915#4525]) +1 similar issue [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb1/igt@gem_exec_balancer@parallel-keep-submit-fence.html [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb3/igt@gem_exec_balancer@parallel-keep-submit-fence.html * igt@gem_exec_balancer@parallel-ordering: - shard-iclb: [SKIP][129] ([i915#4525]) -> [DMESG-FAIL][130] ([i915#5076]) [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb8/igt@gem_exec_balancer@parallel-ordering.html [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb2/igt@gem_exec_balancer@parallel-ordering.html * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area: - shard-iclb: [SKIP][131] ([fdo#111068] / [i915#658]) -> [SKIP][132] ([i915#2920]) [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-iclb5/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-iclb2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html * igt@runner@aborted: - shard-apl: ([FAIL][133], [FAIL][134], [FAIL][135], [FAIL][136], [FAIL][137], [FAIL][138]) ([i915#180] / [i915#2426] / [i915#3002] / [i915#4312]) -> ([FAIL][139], [FAIL][140], [FAIL][141], [FAIL][142]) ([i915#1814] / [i915#2426] / [i915#3002] / [i915#4312]) [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl1/igt@runner@aborted.html [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl6/igt@runner@aborted.html [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl8/igt@runner@aborted.html [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl6/igt@runner@aborted.html [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl2/igt@runner@aborted.html [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11285/shard-apl8/igt@runner@aborted.html [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/shard-apl2/igt@runner@aborted.html [140]: h == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22408/index.html [-- Attachment #2: Type: text/html, Size: 33473 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v13 0/5] Use drm_clflush* instead of clflush @ 2022-03-21 22:38 Michael Cheng 2022-03-22 1:48 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 0 siblings, 1 reply; 55+ messages in thread From: Michael Cheng @ 2022-03-21 22:38 UTC (permalink / raw) To: intel-gfx Cc: michael.cheng, wayne.boyer, casey.g.bowman, lucas.demarchi, dri-devel This patch series re-work a few i915 functions to use drm_clflush_virt_range instead of calling clflush or clflushopt directly. This will prevent errors when building for non-x86 architectures. v2: s/PAGE_SIZE/sizeof(value) for Re-work intel_write_status_page and added more patches to convert additional clflush/clflushopt to use drm_clflush*. (Michael Cheng) v3: Drop invalidate_csb_entries and directly invoke drm_clflush_virt_ran v4: Remove extra memory barriers v5: s/cache_clflush_range/drm_clflush_virt_range v6: Fix up "Drop invalidate_csb_entries" to use correct parameters. Also added in arm64 support for drm_clflush_virt_range. v7: Re-order patches, and use correct macro for dcache flush for arm64. v8: Remove ifdef for asm/cacheflush. v9: Rebased v10: Replaced asm/cacheflush with linux/cacheflush v11: Correctly get the sizeof certian addresses. Also rebased to the latest. v12: Drop include of cacheflush.h and use caches_clean_inval_pou instead of dcache_clean_inval_poc, since it is not exported for other modules to use. v13: Drop arm64 implementation for drm_clflush_virt_range. This series will focus more on making i915 more architecture neutral by abstracting all clflush and clflush opt to the drm layer. Michael Cheng (5): drm/i915/gt: Re-work intel_write_status_page drm/i915/gt: Drop invalidate_csb_entries drm/i915/gt: Re-work reset_csb drm/i915/: Re-work clflush_write32 drm/i915/gt: replace cache_clflush_range .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++----- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 12 +++++------ drivers/gpu/drm/i915/gt/intel_engine.h | 13 ++++-------- .../drm/i915/gt/intel_execlists_submission.c | 20 +++++++------------ drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 2 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- 7 files changed, 23 insertions(+), 36 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 55+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Use drm_clflush* instead of clflush 2022-03-21 22:38 [PATCH v13 0/5] " Michael Cheng @ 2022-03-22 1:48 ` Patchwork 0 siblings, 0 replies; 55+ messages in thread From: Patchwork @ 2022-03-22 1:48 UTC (permalink / raw) To: Michael Cheng; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 6293 bytes --] == Series Details == Series: Use drm_clflush* instead of clflush URL : https://patchwork.freedesktop.org/series/101611/ State : success == Summary == CI Bug Log - changes from CI_DRM_11395 -> Patchwork_22635 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/index.html Participating hosts (47 -> 40) ------------------------------ Additional (1): bat-jsl-2 Missing (8): shard-tglu fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-kbl-8809g fi-pnv-d510 fi-bdw-samus fi-skl-6700k2 Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_22635: ### IGT changes ### #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_selftest@live@coherency: - {bat-rpls-2}: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/bat-rpls-2/igt@i915_selftest@live@coherency.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/bat-rpls-2/igt@i915_selftest@live@coherency.html Known issues ------------ Here are the changes found in Patchwork_22635 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@amdgpu/amd_cs_nop@fork-compute0: - fi-blb-e6850: NOTRUN -> [SKIP][3] ([fdo#109271]) +17 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/fi-blb-e6850/igt@amdgpu/amd_cs_nop@fork-compute0.html * igt@i915_selftest@live@hangcheck: - fi-hsw-4770: [PASS][4] -> [INCOMPLETE][5] ([i915#4785]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html - fi-snb-2600: [PASS][6] -> [INCOMPLETE][7] ([i915#3921]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/fi-snb-2600/igt@i915_selftest@live@hangcheck.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/fi-snb-2600/igt@i915_selftest@live@hangcheck.html * igt@runner@aborted: - fi-hsw-4770: NOTRUN -> [FAIL][8] ([fdo#109271] / [i915#1436] / [i915#4312]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/fi-hsw-4770/igt@runner@aborted.html #### Possible fixes #### * igt@i915_selftest@live@gem: - fi-blb-e6850: [DMESG-FAIL][9] ([i915#4528]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/fi-blb-e6850/igt@i915_selftest@live@gem.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/fi-blb-e6850/igt@i915_selftest@live@gem.html * igt@i915_selftest@live@vma: - {bat-rpls-2}: [DMESG-WARN][11] ([i915#4391]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/bat-rpls-2/igt@i915_selftest@live@vma.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/bat-rpls-2/igt@i915_selftest@live@vma.html * igt@kms_busy@basic@flip: - {bat-adlp-6}: [DMESG-WARN][13] ([i915#3576]) -> [PASS][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11395/bat-adlp-6/igt@kms_busy@basic@flip.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/bat-adlp-6/igt@kms_busy@basic@flip.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308 [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155 [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582 [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282 [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301 [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555 [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921 [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391 [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785 [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897 [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533 [i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341 Build changes ------------- * Linux: CI_DRM_11395 -> Patchwork_22635 CI-20190529: 20190529 CI_DRM_11395: a678f97326454b60ffbbde6abf52d23997d71a27 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6387: 04d012b18355b53798af5a55a8915afb1a421bba @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_22635: 914fa33bea6da34fac9ce77f9759e28b616653a1 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 914fa33bea6d drm/i915/gt: replace cache_clflush_range 2f0a938cdf83 drm/i915/: Re-work clflush_write32 b3612dcb509e drm/i915/gt: Re-work reset_csb c7049469aaf2 drm/i915/gt: Drop invalidate_csb_entries c55645dcea5e drm/i915/gt: Re-work intel_write_status_page == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22635/index.html [-- Attachment #2: Type: text/html, Size: 5831 bytes --] ^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2022-03-22 1:48 UTC | newest] Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-25 3:24 [PATCH v12 0/6] Use drm_clflush* instead of clflush Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 16:28 ` Tvrtko Ursulin 2022-02-25 16:28 ` [Intel-gfx] " Tvrtko Ursulin 2022-02-25 16:52 ` Michael Cheng 2022-02-25 16:52 ` [Intel-gfx] " Michael Cheng 2022-02-25 17:33 ` Tvrtko Ursulin 2022-02-25 17:33 ` [Intel-gfx] " Tvrtko Ursulin 2022-02-25 17:40 ` Michael Cheng 2022-02-25 17:40 ` [Intel-gfx] " Michael Cheng 2022-02-25 18:19 ` Tvrtko Ursulin 2022-02-25 18:19 ` [Intel-gfx] " Tvrtko Ursulin 2022-02-25 18:23 ` Michael Cheng 2022-02-25 18:23 ` [Intel-gfx] " Michael Cheng 2022-02-25 18:42 ` Tvrtko Ursulin 2022-02-25 18:42 ` [Intel-gfx] " Tvrtko Ursulin 2022-02-25 18:58 ` Matthew Wilcox 2022-02-25 18:58 ` [Intel-gfx] " Matthew Wilcox 2022-02-25 18:24 ` Robin Murphy 2022-02-25 18:24 ` [Intel-gfx] " Robin Murphy 2022-02-25 18:24 ` Robin Murphy 2022-02-25 19:27 ` Michael Cheng 2022-02-25 19:27 ` [Intel-gfx] " Michael Cheng 2022-03-02 12:49 ` Robin Murphy 2022-03-02 12:49 ` [Intel-gfx] " Robin Murphy 2022-03-02 12:49 ` Robin Murphy 2022-03-02 15:55 ` Michael Cheng 2022-03-02 15:55 ` Michael Cheng 2022-03-02 15:55 ` [Intel-gfx] " Michael Cheng 2022-03-02 17:06 ` Alex Deucher 2022-03-02 17:06 ` Alex Deucher 2022-03-02 17:06 ` [Intel-gfx] " Alex Deucher 2022-03-02 19:10 ` Robin Murphy 2022-03-02 19:10 ` [Intel-gfx] " Robin Murphy 2022-03-02 19:10 ` Robin Murphy 2022-03-07 16:52 ` Michael Cheng 2022-03-07 16:52 ` Michael Cheng 2022-03-07 16:52 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 2/6] drm/i915/gt: Re-work intel_write_status_page Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 3/6] drm/i915/gt: Drop invalidate_csb_entries Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 4/6] drm/i915/gt: Re-work reset_csb Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 5/6] drm/i915/: Re-work clflush_write32 Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 3:24 ` [PATCH v12 6/6] drm/i915/gt: replace cache_clflush_range Michael Cheng 2022-02-25 3:24 ` [Intel-gfx] " Michael Cheng 2022-02-25 7:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Use drm_clflush* instead of clflush Patchwork 2022-02-25 7:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2022-02-25 7:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2022-02-26 1:56 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2022-03-21 22:38 [PATCH v13 0/5] " Michael Cheng 2022-03-22 1:48 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.