Re: [PATCH] percpu: improve percpu_alloc_percpu_fail event trace
Hello, On Mon, Jan 22, 2024 at 08:55:39PM -0500, Steven Rostedt wrote: > On Tue, 23 Jan 2024 09:44:43 +0800 > George Guo wrote: > > > There are two reasons of percpu_alloc failed without warnings: > > > > 1. do_warn is false > > 2. do_warn is true and warn_limit is reached the limit. > > Yes I know the reasons. > > > > > Showing do_warn and warn_limit makes things simple, maybe dont need > > kprobe again. > > It's up to the maintainers of that code to decide if it's worth it or not, > but honestly, my opinion it is not. > I agree, I don't think this is a worthwhile change. If we do change this, I'd like it to be more actionable in some way and as a result something we can fix or tune accordingly. George is this a common problem you're seeing? > The trace event in question is to trace that percpu_alloc failed and why. > It's not there to determine why it did not produce a printk message. > > -- Steve Thanks, Dennis
Re: [PATCH v4 0/4] percpu: partial chunk depopulation
On Tue, Apr 20, 2021 at 04:37:02PM +0530, Pratik Sampat wrote: > > On 20/04/21 4:27 am, Dennis Zhou wrote: > > On Mon, Apr 19, 2021 at 10:50:43PM +, Dennis Zhou wrote: > > > Hello, > > > > > > This series is a continuation of Roman's series in [1]. It aims to solve > > > chunks holding onto free pages by adding a reclaim process to the percpu > > > balance work item. > > > > > > The main difference is that the nr_empty_pop_pages is now managed at > > > time of isolation instead of intermixed. This helps with deciding which > > > chunks to free instead of having to interleave returning chunks to > > > active duty. > > > > > > The allocation priority is as follows: > > >1) appropriate chunk slot increasing until fit > > >2) sidelined chunks > > >3) full free chunks > > > > > > The last slot for to_depopulate is never used for allocations. > > > > > > A big thanks to Roman for initiating the work and being available for > > > iterating on these ideas. > > > > > > This patchset contains the following 4 patches: > > >0001-percpu-factor-out-pcpu_check_block_hint.patch > > >0002-percpu-use-pcpu_free_slot-instead-of-pcpu_nr_slots-1.patch > > >0003-percpu-implement-partial-chunk-depopulation.patch > > >0004-percpu-use-reclaim-threshold-instead-of-running-for-.patch > > > > > > 0001 and 0002 are clean ups. 0003 implement partial chunk depopulation > > > initially from Roman. 0004 adds a reclaim threshold so we do not need to > > > schedule for every page freed. > > > > > > This series is on top of percpu$for-5.14 67c2669d69fb. > > > > > > diffstats below: > > > > > > Dennis Zhou (2): > > >percpu: use pcpu_free_slot instead of pcpu_nr_slots - 1 > > >percpu: use reclaim threshold instead of running for every page > > > > > > Roman Gushchin (2): > > >percpu: factor out pcpu_check_block_hint() > > >percpu: implement partial chunk depopulation > > > > > > mm/percpu-internal.h | 5 + > > > mm/percpu-km.c | 5 + > > > mm/percpu-stats.c| 20 ++-- > > > mm/percpu-vm.c | 30 ++ > > > mm/percpu.c | 252 ++- > > > 5 files changed, 278 insertions(+), 34 deletions(-) > > > > > > Thanks, > > > Dennis > > Hello Pratik, > > > > Do you mind testing this series again on POWER9? The base is available > > here: > > https://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git/log/?h=for-5.14 > > > > Thanks, > > Dennis > > Hello Dennis, I have tested this patchset on POWER9. > > I have tried variations of the percpu_test in the top level and nested cgroups > creation as the test with 1000:10 didn't show any benefits. This is most likely because the 1 in every 11 still pins every page while 1 in 50 does not. Can you try the patch below on top? I think it may show slightly better perf as well. If it doesn't I'll just drop it. > > The following example shows more consistent benefits with the de-allocation > strategy. > Outer: 1000 > Inner: 50 > # ./percpu_test.sh > Percpu: 6912 kB > Percpu: 532736 kB > Percpu: 278784 kB > > I believe it could be a result of bulk freeing within > "free_unref_page_commit", > where pages are only free'd if pcp->count >= pcp->high. As POWER has a larger > page size it would end up creating lesser number of pages but with the > effects of fragmentation. This is unrelated to per cpu pages in slab/slub. Percpu is a separate memory allocator. > > Having said that, the patchset and its behavior does look good to me. Thanks, can I throw the following on the appropriate patches? In the future it's good to be explicit about this because some prefer to credit different emails. Tested-by: Pratik Sampat Thanks, Dennis The following may do a little better on power9: --- >From a1464c4d5900cca68fd95b935178d72bb74837d5 Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Tue, 20 Apr 2021 14:25:20 + Subject: [PATCH] percpu: convert free page float to bytes The percpu memory allocator keeps around a minimum number of free pages to ensure we can satisfy atomic allocations. However, we've always kept this number in terms of pages. On certain architectures like arm and powerpc, the default page size could be 64k instead of 4k. So, start with a target number of free bytes and then co
Re: [PATCH v4 0/4] percpu: partial chunk depopulation
On Mon, Apr 19, 2021 at 10:50:43PM +, Dennis Zhou wrote: > Hello, > > This series is a continuation of Roman's series in [1]. It aims to solve > chunks holding onto free pages by adding a reclaim process to the percpu > balance work item. > > The main difference is that the nr_empty_pop_pages is now managed at > time of isolation instead of intermixed. This helps with deciding which > chunks to free instead of having to interleave returning chunks to > active duty. > > The allocation priority is as follows: > 1) appropriate chunk slot increasing until fit > 2) sidelined chunks > 3) full free chunks > > The last slot for to_depopulate is never used for allocations. > > A big thanks to Roman for initiating the work and being available for > iterating on these ideas. > > This patchset contains the following 4 patches: > 0001-percpu-factor-out-pcpu_check_block_hint.patch > 0002-percpu-use-pcpu_free_slot-instead-of-pcpu_nr_slots-1.patch > 0003-percpu-implement-partial-chunk-depopulation.patch > 0004-percpu-use-reclaim-threshold-instead-of-running-for-.patch > > 0001 and 0002 are clean ups. 0003 implement partial chunk depopulation > initially from Roman. 0004 adds a reclaim threshold so we do not need to > schedule for every page freed. > > This series is on top of percpu$for-5.14 67c2669d69fb. > > diffstats below: > > Dennis Zhou (2): > percpu: use pcpu_free_slot instead of pcpu_nr_slots - 1 > percpu: use reclaim threshold instead of running for every page > > Roman Gushchin (2): > percpu: factor out pcpu_check_block_hint() > percpu: implement partial chunk depopulation > > mm/percpu-internal.h | 5 + > mm/percpu-km.c | 5 + > mm/percpu-stats.c| 20 ++-- > mm/percpu-vm.c | 30 ++ > mm/percpu.c | 252 ++- > 5 files changed, 278 insertions(+), 34 deletions(-) > > Thanks, > Dennis Hello Pratik, Do you mind testing this series again on POWER9? The base is available here: https://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git/log/?h=for-5.14 Thanks, Dennis
Re: [PATCH v4 0/4] percpu: partial chunk depopulation
On Mon, Apr 19, 2021 at 10:50:43PM +, Dennis Zhou wrote: > Hello, > > This series is a continuation of Roman's series in [1]. It aims to solve > chunks holding onto free pages by adding a reclaim process to the percpu > balance work item. > And I forgot to link [1]... [1] https://lore.kernel.org/lkml/20210408035736.883861-1-g...@fb.com/ > The main difference is that the nr_empty_pop_pages is now managed at > time of isolation instead of intermixed. This helps with deciding which > chunks to free instead of having to interleave returning chunks to > active duty. > > The allocation priority is as follows: > 1) appropriate chunk slot increasing until fit > 2) sidelined chunks > 3) full free chunks > > The last slot for to_depopulate is never used for allocations. > > A big thanks to Roman for initiating the work and being available for > iterating on these ideas. > > This patchset contains the following 4 patches: > 0001-percpu-factor-out-pcpu_check_block_hint.patch > 0002-percpu-use-pcpu_free_slot-instead-of-pcpu_nr_slots-1.patch > 0003-percpu-implement-partial-chunk-depopulation.patch > 0004-percpu-use-reclaim-threshold-instead-of-running-for-.patch > > 0001 and 0002 are clean ups. 0003 implement partial chunk depopulation > initially from Roman. 0004 adds a reclaim threshold so we do not need to > schedule for every page freed. > > This series is on top of percpu$for-5.14 67c2669d69fb. > > diffstats below: > > Dennis Zhou (2): > percpu: use pcpu_free_slot instead of pcpu_nr_slots - 1 > percpu: use reclaim threshold instead of running for every page > > Roman Gushchin (2): > percpu: factor out pcpu_check_block_hint() > percpu: implement partial chunk depopulation > > mm/percpu-internal.h | 5 + > mm/percpu-km.c | 5 + > mm/percpu-stats.c| 20 ++-- > mm/percpu-vm.c | 30 ++ > mm/percpu.c | 252 ++- > 5 files changed, 278 insertions(+), 34 deletions(-) > > Thanks, > Dennis
[PATCH 3/4] percpu: implement partial chunk depopulation
From: Roman Gushchin
This patch implements partial depopulation of percpu chunks.
As of now, a chunk can be depopulated only as a part of the final
destruction, if there are no more outstanding allocations. However
to minimize a memory waste it might be useful to depopulate a
partially filed chunk, if a small number of outstanding allocations
prevents the chunk from being fully reclaimed.
This patch implements the following depopulation process: it scans
over the chunk pages, looks for a range of empty and populated pages
and performs the depopulation. To avoid races with new allocations,
the chunk is previously isolated. After the depopulation the chunk is
sidelined to a special list or freed. New allocations prefer using
active chunks to sidelined chunks. If a sidelined chunk is used, it is
reintegrated to the active lists.
The depopulation is scheduled on the free path if the chunk is all of
the following:
1) has more than 1/4 of total pages free and populated
2) the system has enough free percpu pages aside of this chunk
3) isn't the reserved chunk
4) isn't the first chunk
If it's already depopulated but got free populated pages, it's a good
target too. The chunk is moved to a special slot,
pcpu_to_depopulate_slot, chunk->isolated is set, and the balance work
item is scheduled. On isolation, these pages are removed from the
pcpu_nr_empty_pop_pages. It is constantly replaced to the
to_depopulate_slot when it meets these qualifications.
pcpu_reclaim_populated() iterates over the to_depopulate_slot until it
becomes empty. The depopulation is performed in the reverse direction to
keep populated pages close to the beginning. Depopulated chunks are
sidelined to preferentially avoid them for new allocations. When no
active chunk can suffice a new allocation, sidelined chunks are first
checked before creating a new chunk.
Signed-off-by: Roman Gushchin
Co-developed-by: Dennis Zhou
Signed-off-by: Dennis Zhou
---
mm/percpu-internal.h | 4 +
mm/percpu-km.c | 5 ++
mm/percpu-stats.c| 12 +--
mm/percpu-vm.c | 30
mm/percpu.c | 180 +++
5 files changed, 211 insertions(+), 20 deletions(-)
diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
index 095d7eaa0db4..10604dce806f 100644
--- a/mm/percpu-internal.h
+++ b/mm/percpu-internal.h
@@ -67,6 +67,8 @@ struct pcpu_chunk {
void*data; /* chunk data */
boolimmutable; /* no [de]population allowed */
+ boolisolated; /* isolated from active chunk
+ slots */
int start_offset; /* the overlap with the previous
region to have a page aligned
base_addr */
@@ -87,6 +89,8 @@ extern spinlock_t pcpu_lock;
extern struct list_head *pcpu_chunk_lists;
extern int pcpu_nr_slots;
+extern int pcpu_sidelined_slot;
+extern int pcpu_to_depopulate_slot;
extern int pcpu_nr_empty_pop_pages[];
extern struct pcpu_chunk *pcpu_first_chunk;
diff --git a/mm/percpu-km.c b/mm/percpu-km.c
index 35c9941077ee..c84a9f781a6c 100644
--- a/mm/percpu-km.c
+++ b/mm/percpu-km.c
@@ -118,3 +118,8 @@ static int __init pcpu_verify_alloc_info(const struct
pcpu_alloc_info *ai)
return 0;
}
+
+static bool pcpu_should_reclaim_chunk(struct pcpu_chunk *chunk)
+{
+ return false;
+}
diff --git a/mm/percpu-stats.c b/mm/percpu-stats.c
index f6026dbcdf6b..2125981acfb9 100644
--- a/mm/percpu-stats.c
+++ b/mm/percpu-stats.c
@@ -219,13 +219,15 @@ static int percpu_stats_show(struct seq_file *m, void *v)
for (slot = 0; slot < pcpu_nr_slots; slot++) {
list_for_each_entry(chunk, &pcpu_chunk_list(type)[slot],
list) {
- if (chunk == pcpu_first_chunk) {
+ if (chunk == pcpu_first_chunk)
seq_puts(m, "Chunk: <- First Chunk\n");
- chunk_map_stats(m, chunk, buffer);
- } else {
+ else if (slot == pcpu_to_depopulate_slot)
+ seq_puts(m, "Chunk (to_depopulate)\n");
+ else if (slot == pcpu_sidelined_slot)
+ seq_puts(m, "Chunk (sidelined):\n");
+ else
seq_puts(m, "Chunk:\n");
- chunk_map_stats(m, chunk, buffer);
- }
+ chunk_map_stats(m, chunk, buffer);
}
}
}
diff --git a/mm/perc
[PATCH 4/4] percpu: use reclaim threshold instead of running for every page
The last patch implements reclaim by adding 2 additional lists where a
chunk's lifecycle is:
active_slot -> to_depopulate_slot -> sidelined_slot
This worked great because we're able to nicely converge paths into
isolation. However, it's a bit aggressive to run for every free page.
Let's accumulate a few free pages before we do this. To do this, the new
lifecycle is:
active_slot -> sidelined_slot -> to_depopulate_slot -> sidelined_slot
The transition from sidelined_slot -> to_depopulate_slot occurs on a
threshold instead of before where it directly went to the
to_depopulate_slot. pcpu_nr_isolated_empty_pop_pages[] is introduced to
aid with this.
Suggested-by: Roman Gushchin
Signed-off-by: Dennis Zhou
---
mm/percpu-internal.h | 1 +
mm/percpu-stats.c| 8 ++--
mm/percpu.c | 44 +---
3 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
index 10604dce806f..b3e43b016276 100644
--- a/mm/percpu-internal.h
+++ b/mm/percpu-internal.h
@@ -92,6 +92,7 @@ extern int pcpu_nr_slots;
extern int pcpu_sidelined_slot;
extern int pcpu_to_depopulate_slot;
extern int pcpu_nr_empty_pop_pages[];
+extern int pcpu_nr_isolated_empty_pop_pages[];
extern struct pcpu_chunk *pcpu_first_chunk;
extern struct pcpu_chunk *pcpu_reserved_chunk;
diff --git a/mm/percpu-stats.c b/mm/percpu-stats.c
index 2125981acfb9..facc804eb86c 100644
--- a/mm/percpu-stats.c
+++ b/mm/percpu-stats.c
@@ -145,7 +145,7 @@ static int percpu_stats_show(struct seq_file *m, void *v)
int slot, max_nr_alloc;
int *buffer;
enum pcpu_chunk_type type;
- int nr_empty_pop_pages;
+ int nr_empty_pop_pages, nr_isolated_empty_pop_pages;
alloc_buffer:
spin_lock_irq(&pcpu_lock);
@@ -167,8 +167,11 @@ static int percpu_stats_show(struct seq_file *m, void *v)
}
nr_empty_pop_pages = 0;
- for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++)
+ nr_isolated_empty_pop_pages = 0;
+ for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++) {
nr_empty_pop_pages += pcpu_nr_empty_pop_pages[type];
+ nr_isolated_empty_pop_pages +=
pcpu_nr_isolated_empty_pop_pages[type];
+ }
#define PL(X) \
seq_printf(m, " %-20s: %12lld\n", #X, (long long int)pcpu_stats_ai.X)
@@ -202,6 +205,7 @@ static int percpu_stats_show(struct seq_file *m, void *v)
PU(min_alloc_size);
PU(max_alloc_size);
P("empty_pop_pages", nr_empty_pop_pages);
+ P("iso_empty_pop_pages", nr_isolated_empty_pop_pages);
seq_putc(m, '\n');
#undef PU
diff --git a/mm/percpu.c b/mm/percpu.c
index 79eebc80860d..ba13e683d022 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -110,6 +110,9 @@
#define PCPU_EMPTY_POP_PAGES_LOW 2
#define PCPU_EMPTY_POP_PAGES_HIGH 4
+/* only schedule reclaim if there are at least N empty pop pages sidelined */
+#define PCPU_EMPTY_POP_RECLAIM_THRESHOLD 4
+
#ifdef CONFIG_SMP
/* default addr <-> pcpu_ptr mapping, override in asm/percpu.h if necessary */
#ifndef __addr_to_pcpu_ptr
@@ -183,6 +186,7 @@ static LIST_HEAD(pcpu_map_extend_chunks);
* The reserved chunk doesn't contribute to the count.
*/
int pcpu_nr_empty_pop_pages[PCPU_NR_CHUNK_TYPES];
+int pcpu_nr_isolated_empty_pop_pages[PCPU_NR_CHUNK_TYPES];
/*
* The number of populated pages in use by the allocator, protected by
@@ -582,8 +586,10 @@ static void pcpu_isolate_chunk(struct pcpu_chunk *chunk)
if (!chunk->isolated) {
chunk->isolated = true;
pcpu_nr_empty_pop_pages[type] -= chunk->nr_empty_pop_pages;
+ pcpu_nr_isolated_empty_pop_pages[type] +=
+ chunk->nr_empty_pop_pages;
+ list_move(&chunk->list, &pcpu_slot[pcpu_sidelined_slot]);
}
- list_move(&chunk->list, &pcpu_slot[pcpu_to_depopulate_slot]);
}
static void pcpu_reintegrate_chunk(struct pcpu_chunk *chunk)
@@ -595,6 +601,8 @@ static void pcpu_reintegrate_chunk(struct pcpu_chunk *chunk)
if (chunk->isolated) {
chunk->isolated = false;
pcpu_nr_empty_pop_pages[type] += chunk->nr_empty_pop_pages;
+ pcpu_nr_isolated_empty_pop_pages[type] -=
+ chunk->nr_empty_pop_pages;
pcpu_chunk_relocate(chunk, -1);
}
}
@@ -610,9 +618,15 @@ static void pcpu_reintegrate_chunk(struct pcpu_chunk
*chunk)
*/
static inline void pcpu_update_empty_pages(struct pcpu_chunk *chunk, int nr)
{
+ enum pcpu_chunk_type type = pcpu_chunk_type(chunk);
+
chunk->nr_empty_pop_pages += nr;
- if (chunk != pcpu_reserved_chunk && !chunk->isolated)
- pcpu_nr_empty_pop_pages[pcpu_chunk_typ
[PATCH 2/4] percpu: use pcpu_free_slot instead of pcpu_nr_slots - 1
This prepares for adding a to_depopulate list and sidelined list after
the free slot in the set of lists in pcpu_slot.
Signed-off-by: Dennis Zhou
---
mm/percpu.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 5edc7bd88133..d46f4adc 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -135,6 +135,7 @@ static int pcpu_unit_size __ro_after_init;
static int pcpu_nr_units __ro_after_init;
static int pcpu_atom_size __ro_after_init;
int pcpu_nr_slots __ro_after_init;
+int pcpu_free_slot __ro_after_init;
static size_t pcpu_chunk_struct_size __ro_after_init;
/* cpus with the lowest and highest unit addresses */
@@ -237,7 +238,7 @@ static int __pcpu_size_to_slot(int size)
static int pcpu_size_to_slot(int size)
{
if (size == pcpu_unit_size)
- return pcpu_nr_slots - 1;
+ return pcpu_free_slot;
return __pcpu_size_to_slot(size);
}
@@ -1806,7 +1807,7 @@ static void __percpu *pcpu_alloc(size_t size, size_t
align, bool reserved,
goto fail;
}
- if (list_empty(&pcpu_slot[pcpu_nr_slots - 1])) {
+ if (list_empty(&pcpu_slot[pcpu_free_slot])) {
chunk = pcpu_create_chunk(type, pcpu_gfp);
if (!chunk) {
err = "failed to allocate new chunk";
@@ -1958,7 +1959,7 @@ static void pcpu_balance_free(enum pcpu_chunk_type type)
{
LIST_HEAD(to_free);
struct list_head *pcpu_slot = pcpu_chunk_list(type);
- struct list_head *free_head = &pcpu_slot[pcpu_nr_slots - 1];
+ struct list_head *free_head = &pcpu_slot[pcpu_free_slot];
struct pcpu_chunk *chunk, *next;
/*
@@ -2033,7 +2034,7 @@ static void pcpu_balance_populated(enum pcpu_chunk_type
type)
0, PCPU_EMPTY_POP_PAGES_HIGH);
}
- for (slot = pcpu_size_to_slot(PAGE_SIZE); slot < pcpu_nr_slots; slot++)
{
+ for (slot = pcpu_size_to_slot(PAGE_SIZE); slot <= pcpu_free_slot;
slot++) {
unsigned int nr_unpop = 0, rs, re;
if (!nr_to_pop)
@@ -2140,7 +2141,7 @@ void free_percpu(void __percpu *ptr)
if (chunk->free_bytes == pcpu_unit_size) {
struct pcpu_chunk *pos;
- list_for_each_entry(pos, &pcpu_slot[pcpu_nr_slots - 1], list)
+ list_for_each_entry(pos, &pcpu_slot[pcpu_free_slot], list)
if (pos != chunk) {
need_balance = true;
break;
@@ -2562,7 +2563,8 @@ void __init pcpu_setup_first_chunk(const struct
pcpu_alloc_info *ai,
* Allocate chunk slots. The additional last slot is for
* empty chunks.
*/
- pcpu_nr_slots = __pcpu_size_to_slot(pcpu_unit_size) + 2;
+ pcpu_free_slot = __pcpu_size_to_slot(pcpu_unit_size) + 1;
+ pcpu_nr_slots = pcpu_free_slot + 1;
pcpu_chunk_lists = memblock_alloc(pcpu_nr_slots *
sizeof(pcpu_chunk_lists[0]) *
PCPU_NR_CHUNK_TYPES,
--
2.31.1.368.gbe11c130af-goog
[PATCH 1/4] percpu: factor out pcpu_check_block_hint()
From: Roman Gushchin
Factor out the pcpu_check_block_hint() helper, which will be useful
in the future. The new function checks if the allocation can likely
fit within the contig hint.
Signed-off-by: Roman Gushchin
Signed-off-by: Dennis Zhou
---
mm/percpu.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 61339b3d9337..5edc7bd88133 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -306,6 +306,25 @@ static unsigned long pcpu_block_off_to_off(int index, int
off)
return index * PCPU_BITMAP_BLOCK_BITS + off;
}
+/**
+ * pcpu_check_block_hint - check against the contig hint
+ * @block: block of interest
+ * @bits: size of allocation
+ * @align: alignment of area (max PAGE_SIZE)
+ *
+ * Check to see if the allocation can fit in the block's contig hint.
+ * Note, a chunk uses the same hints as a block so this can also check against
+ * the chunk's contig hint.
+ */
+static bool pcpu_check_block_hint(struct pcpu_block_md *block, int bits,
+ size_t align)
+{
+ int bit_off = ALIGN(block->contig_hint_start, align) -
+ block->contig_hint_start;
+
+ return bit_off + bits <= block->contig_hint;
+}
+
/*
* pcpu_next_hint - determine which hint to use
* @block: block of interest
@@ -1066,14 +1085,11 @@ static int pcpu_find_block_fit(struct pcpu_chunk
*chunk, int alloc_bits,
int bit_off, bits, next_off;
/*
-* Check to see if the allocation can fit in the chunk's contig hint.
-* This is an optimization to prevent scanning by assuming if it
-* cannot fit in the global hint, there is memory pressure and creating
-* a new chunk would happen soon.
+* This is an optimization to prevent scanning by assuming if the
+* allocation cannot fit in the global hint, there is memory pressure
+* and creating a new chunk would happen soon.
*/
- bit_off = ALIGN(chunk_md->contig_hint_start, align) -
- chunk_md->contig_hint_start;
- if (bit_off + alloc_bits > chunk_md->contig_hint)
+ if (!pcpu_check_block_hint(chunk_md, alloc_bits, align))
return -1;
bit_off = pcpu_next_hint(chunk_md, alloc_bits);
--
2.31.1.368.gbe11c130af-goog
[PATCH v4 0/4] percpu: partial chunk depopulation
Hello, This series is a continuation of Roman's series in [1]. It aims to solve chunks holding onto free pages by adding a reclaim process to the percpu balance work item. The main difference is that the nr_empty_pop_pages is now managed at time of isolation instead of intermixed. This helps with deciding which chunks to free instead of having to interleave returning chunks to active duty. The allocation priority is as follows: 1) appropriate chunk slot increasing until fit 2) sidelined chunks 3) full free chunks The last slot for to_depopulate is never used for allocations. A big thanks to Roman for initiating the work and being available for iterating on these ideas. This patchset contains the following 4 patches: 0001-percpu-factor-out-pcpu_check_block_hint.patch 0002-percpu-use-pcpu_free_slot-instead-of-pcpu_nr_slots-1.patch 0003-percpu-implement-partial-chunk-depopulation.patch 0004-percpu-use-reclaim-threshold-instead-of-running-for-.patch 0001 and 0002 are clean ups. 0003 implement partial chunk depopulation initially from Roman. 0004 adds a reclaim threshold so we do not need to schedule for every page freed. This series is on top of percpu$for-5.14 67c2669d69fb. diffstats below: Dennis Zhou (2): percpu: use pcpu_free_slot instead of pcpu_nr_slots - 1 percpu: use reclaim threshold instead of running for every page Roman Gushchin (2): percpu: factor out pcpu_check_block_hint() percpu: implement partial chunk depopulation mm/percpu-internal.h | 5 + mm/percpu-km.c | 5 + mm/percpu-stats.c| 20 ++-- mm/percpu-vm.c | 30 ++ mm/percpu.c | 252 ++- 5 files changed, 278 insertions(+), 34 deletions(-) Thanks, Dennis
Re: [PATCH v3 0/6] percpu: partial chunk depopulation
Hello, On Sat, Apr 17, 2021 at 01:14:03AM +0530, Pratik Sampat wrote: > > > On 17/04/21 12:39 am, Roman Gushchin wrote: > > On Sat, Apr 17, 2021 at 12:11:37AM +0530, Pratik Sampat wrote: > > > > > > On 17/04/21 12:04 am, Roman Gushchin wrote: > > > > On Fri, Apr 16, 2021 at 11:57:03PM +0530, Pratik Sampat wrote: > > > > > On 16/04/21 10:43 pm, Roman Gushchin wrote: > > > > > > On Fri, Apr 16, 2021 at 08:58:33PM +0530, Pratik Sampat wrote: > > > > > > > Hello Dennis, > > > > > > > > > > > > > > I apologize for the clutter of logs before, I'm pasting the logs > > > > > > > of before and > > > > > > > after the percpu test in the case of the patchset being applied > > > > > > > on 5.12-rc6 and > > > > > > > the vanilla kernel 5.12-rc6. > > > > > > > > > > > > > > On 16/04/21 7:48 pm, Dennis Zhou wrote: > > > > > > > > Hello, > > > > > > > > > > > > > > > > On Fri, Apr 16, 2021 at 06:26:15PM +0530, Pratik Sampat wrote: > > > > > > > > > Hello Roman, > > > > > > > > > > > > > > > > > > I've tried the v3 patch series on a POWER9 and an x86 KVM > > > > > > > > > setup. > > > > > > > > > > > > > > > > > > My results of the percpu_test are as follows: > > > > > > > > > Intel KVM 4CPU:4G > > > > > > > > > Vanilla 5.12-rc6 > > > > > > > > > # ./percpu_test.sh > > > > > > > > > Percpu: 1952 kB > > > > > > > > > Percpu: 219648 kB > > > > > > > > > Percpu: 219648 kB > > > > > > > > > > > > > > > > > > 5.12-rc6 + with patchset applied > > > > > > > > > # ./percpu_test.sh > > > > > > > > > Percpu: 2080 kB > > > > > > > > > Percpu: 219712 kB > > > > > > > > > Percpu: 72672 kB > > > > > > > > > > > > > > > > > > I'm able to see improvement comparable to that of what you're > > > > > > > > > see too. > > > > > > > > > > > > > > > > > > However, on POWERPC I'm unable to reproduce these > > > > > > > > > improvements with the patchset in the same configuration > > > > > > > > > > > > > > > > > > POWER9 KVM 4CPU:4G > > > > > > > > > Vanilla 5.12-rc6 > > > > > > > > > # ./percpu_test.sh > > > > > > > > > Percpu: 5888 kB > > > > > > > > > Percpu: 118272 kB > > > > > > > > > Percpu: 118272 kB > > > > > > > > > > > > > > > > > > 5.12-rc6 + with patchset applied > > > > > > > > > # ./percpu_test.sh > > > > > > > > > Percpu: 6144 kB > > > > > > > > > Percpu: 119040 kB > > > > > > > > > Percpu: 119040 kB > > > > > > > > > > > > > > > > > > I'm wondering if there's any architectural specific code that > > > > > > > > > needs plumbing > > > > > > > > > here? > > > > > > > > > > > > > > > > > There shouldn't be. Can you send me the percpu_stats debug > > > > > > > > output before > > > > > > > > and after? > > > > > > > I'll paste the whole debug stats before and after here. > > > > > > > 5.12-rc6 + patchset > > > > > > > -BEFORE- > > > > > > > Percpu Memory Statistics > > > > > > > Allocation Info: > > > > > > Hm, this looks highly suspicious. Here is your stats in a more > > > > > > compact form: > > > > > > > > > > > > Vanilla > > > > > > > > > > > > nr_alloc: 903
Re: [PATCH v3 5/6] percpu: factor out pcpu_check_chunk_hint()
Hello,
On Wed, Apr 07, 2021 at 08:57:35PM -0700, Roman Gushchin wrote:
> Factor out the pcpu_check_chunk_hint() helper, which will be useful
> in the future. The new function checks if the allocation can likely
> fit the given chunk.
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu.c | 30 +-
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index e20119668c42..357fd6994278 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -306,6 +306,26 @@ static unsigned long pcpu_block_off_to_off(int index,
> int off)
> return index * PCPU_BITMAP_BLOCK_BITS + off;
> }
>
> +/**
> + * pcpu_check_chunk_hint - check that allocation can fit a chunk
> + * @chunk_md: chunk's block
nit for consistency: @block: block of interest
> + * @bits: size of request in allocation units
> + * @align: alignment of area (max PAGE_SIZE)
> + *
> + * Check to see if the allocation can fit in the chunk's contig hint.
> + * This is an optimization to prevent scanning by assuming if it
> + * cannot fit in the global hint, there is memory pressure and creating
> + * a new chunk would happen soon.
> + */
It occurred to me, That I converged block_md and chunk_md to be the same
object as 1 is just a degenerative case of the other. Can we rename this
to be pcpu_check_block_hint() and have it take in pcpu_block_md?
> +static bool pcpu_check_chunk_hint(struct pcpu_block_md *chunk_md, int bits,
> + size_t align)
> +{
> + int bit_off = ALIGN(chunk_md->contig_hint_start, align) -
> + chunk_md->contig_hint_start;
> +
> + return bit_off + bits <= chunk_md->contig_hint;
> +}
> +
> /*
> * pcpu_next_hint - determine which hint to use
> * @block: block of interest
> @@ -1065,15 +1085,7 @@ static int pcpu_find_block_fit(struct pcpu_chunk
> *chunk, int alloc_bits,
> struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> int bit_off, bits, next_off;
>
> - /*
> - * Check to see if the allocation can fit in the chunk's contig hint.
> - * This is an optimization to prevent scanning by assuming if it
> - * cannot fit in the global hint, there is memory pressure and creating
> - * a new chunk would happen soon.
> - */
> - bit_off = ALIGN(chunk_md->contig_hint_start, align) -
> - chunk_md->contig_hint_start;
> - if (bit_off + alloc_bits > chunk_md->contig_hint)
> + if (!pcpu_check_chunk_hint(chunk_md, alloc_bits, align))
> return -1;
>
> bit_off = pcpu_next_hint(chunk_md, alloc_bits);
> --
> 2.30.2
>
Thanks,
Dennis
Re: [PATCH v3 4/6] percpu: generalize pcpu_balance_populated()
*/
> + if (pcpu_atomic_alloc_failed) {
> + nr_to_pop = PCPU_EMPTY_POP_PAGES_HIGH;
> + /* best effort anyway, don't worry about synchronization */
> + pcpu_atomic_alloc_failed = false;
> + pcpu_grow_populated(type, nr_to_pop);
> + } else if (pcpu_nr_empty_pop_pages[type] < PCPU_EMPTY_POP_PAGES_HIGH) {
> + nr_to_pop = PCPU_EMPTY_POP_PAGES_HIGH -
> pcpu_nr_empty_pop_pages[type];
> + pcpu_grow_populated(type, nr_to_pop);
> + }
> +}
> +
> /**
> * pcpu_balance_workfn - manage the amount of free chunks and populated pages
> * @work: unused
> --
> 2.30.2
>
I've applied this for-5.14.
Thanks,
Dennis
Re: [PATCH v3 3/6] percpu: make pcpu_nr_empty_pop_pages per chunk type
Hello,
On Wed, Apr 07, 2021 at 08:57:33PM -0700, Roman Gushchin wrote:
> nr_empty_pop_pages is used to guarantee that there are some free
> populated pages to satisfy atomic allocations. Accounted and
> non-accounted allocations are using separate sets of chunks,
> so both need to have a surplus of empty pages.
>
> This commit makes pcpu_nr_empty_pop_pages and the corresponding logic
> per chunk type.
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu-internal.h | 2 +-
> mm/percpu-stats.c| 9 +++--
> mm/percpu.c | 14 +++---
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
> index 18b768ac7dca..095d7eaa0db4 100644
> --- a/mm/percpu-internal.h
> +++ b/mm/percpu-internal.h
> @@ -87,7 +87,7 @@ extern spinlock_t pcpu_lock;
>
> extern struct list_head *pcpu_chunk_lists;
> extern int pcpu_nr_slots;
> -extern int pcpu_nr_empty_pop_pages;
> +extern int pcpu_nr_empty_pop_pages[];
>
> extern struct pcpu_chunk *pcpu_first_chunk;
> extern struct pcpu_chunk *pcpu_reserved_chunk;
> diff --git a/mm/percpu-stats.c b/mm/percpu-stats.c
> index c8400a2adbc2..f6026dbcdf6b 100644
> --- a/mm/percpu-stats.c
> +++ b/mm/percpu-stats.c
> @@ -145,6 +145,7 @@ static int percpu_stats_show(struct seq_file *m, void *v)
> int slot, max_nr_alloc;
> int *buffer;
> enum pcpu_chunk_type type;
> + int nr_empty_pop_pages;
>
> alloc_buffer:
> spin_lock_irq(&pcpu_lock);
> @@ -165,7 +166,11 @@ static int percpu_stats_show(struct seq_file *m, void *v)
> goto alloc_buffer;
> }
>
> -#define PL(X) \
> + nr_empty_pop_pages = 0;
> + for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++)
> + nr_empty_pop_pages += pcpu_nr_empty_pop_pages[type];
> +
> +#define PL(X)
> \
> seq_printf(m, " %-20s: %12lld\n", #X, (long long int)pcpu_stats_ai.X)
>
> seq_printf(m,
> @@ -196,7 +201,7 @@ static int percpu_stats_show(struct seq_file *m, void *v)
> PU(nr_max_chunks);
> PU(min_alloc_size);
> PU(max_alloc_size);
> - P("empty_pop_pages", pcpu_nr_empty_pop_pages);
> + P("empty_pop_pages", nr_empty_pop_pages);
> seq_putc(m, '\n');
>
> #undef PU
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 7e31e1b8725f..61339b3d9337 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -176,10 +176,10 @@ struct list_head *pcpu_chunk_lists __ro_after_init; /*
> chunk list slots */
> static LIST_HEAD(pcpu_map_extend_chunks);
>
> /*
> - * The number of empty populated pages, protected by pcpu_lock. The
> - * reserved chunk doesn't contribute to the count.
> + * The number of empty populated pages by chunk type, protected by pcpu_lock.
> + * The reserved chunk doesn't contribute to the count.
> */
> -int pcpu_nr_empty_pop_pages;
> +int pcpu_nr_empty_pop_pages[PCPU_NR_CHUNK_TYPES];
>
> /*
> * The number of populated pages in use by the allocator, protected by
> @@ -559,7 +559,7 @@ static inline void pcpu_update_empty_pages(struct
> pcpu_chunk *chunk, int nr)
> {
> chunk->nr_empty_pop_pages += nr;
> if (chunk != pcpu_reserved_chunk)
> - pcpu_nr_empty_pop_pages += nr;
> + pcpu_nr_empty_pop_pages[pcpu_chunk_type(chunk)] += nr;
> }
>
> /*
> @@ -1835,7 +1835,7 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> mutex_unlock(&pcpu_alloc_mutex);
> }
>
> - if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_LOW)
> + if (pcpu_nr_empty_pop_pages[type] < PCPU_EMPTY_POP_PAGES_LOW)
> pcpu_schedule_balance_work();
>
> /* clear the areas and return address relative to base address */
> @@ -2013,7 +2013,7 @@ static void pcpu_balance_populated(enum pcpu_chunk_type
> type)
> pcpu_atomic_alloc_failed = false;
> } else {
> nr_to_pop = clamp(PCPU_EMPTY_POP_PAGES_HIGH -
> - pcpu_nr_empty_pop_pages,
> + pcpu_nr_empty_pop_pages[type],
> 0, PCPU_EMPTY_POP_PAGES_HIGH);
> }
>
> @@ -2595,7 +2595,7 @@ void __init pcpu_setup_first_chunk(const struct
> pcpu_alloc_info *ai,
>
> /* link the first chunk in */
> pcpu_first_chunk = chunk;
> - pcpu_nr_empty_pop_pages = pcpu_first_chunk->nr_empty_pop_pages;
> + pcpu_nr_empty_pop_pages[PCPU_CHUNK_ROOT] =
> pcpu_first_chunk->nr_empty_pop_pages;
> pcpu_chunk_relocate(pcpu_first_chunk, -1);
>
> /* include all regions of the first chunk */
> --
> 2.30.2
>
This turns out to have been a more pressing issue. Thanks for fixing
this. I ran this to Linus for v5.12-rc7 [1].
https://lore.kernel.org/lkml/yhhs618esvkhy...@google.com/
Thanks,
Dennis
Re: [PATCH v3 2/6] percpu: split __pcpu_balance_workfn()
Hello,
On Wed, Apr 07, 2021 at 08:57:32PM -0700, Roman Gushchin wrote:
> __pcpu_balance_workfn() became fairly big and hard to follow, but in
> fact it consists of two fully independent parts, responsible for
> the destruction of excessive free chunks and population of necessarily
> amount of free pages.
>
> In order to simplify the code and prepare for adding of a new
> functionality, split it in two functions:
>
> 1) pcpu_balance_free,
> 2) pcpu_balance_populated.
>
> Move the taking/releasing of the pcpu_alloc_mutex to an upper level
> to keep the current synchronization in place.
>
> Signed-off-by: Roman Gushchin
> Reviewed-by: Dennis Zhou
> ---
> mm/percpu.c | 46 +-
> 1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 2f27123bb489..7e31e1b8725f 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1933,31 +1933,22 @@ void __percpu *__alloc_reserved_percpu(size_t size,
> size_t align)
> }
>
> /**
> - * __pcpu_balance_workfn - manage the amount of free chunks and populated
> pages
> + * pcpu_balance_free - manage the amount of free chunks
> * @type: chunk type
> *
> - * Reclaim all fully free chunks except for the first one. This is also
> - * responsible for maintaining the pool of empty populated pages. However,
> - * it is possible that this is called when physical memory is scarce causing
> - * OOM killer to be triggered. We should avoid doing so until an actual
> - * allocation causes the failure as it is possible that requests can be
> - * serviced from already backed regions.
> + * Reclaim all fully free chunks except for the first one.
> */
> -static void __pcpu_balance_workfn(enum pcpu_chunk_type type)
> +static void pcpu_balance_free(enum pcpu_chunk_type type)
> {
> - /* gfp flags passed to underlying allocators */
> - const gfp_t gfp = GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN;
> LIST_HEAD(to_free);
> struct list_head *pcpu_slot = pcpu_chunk_list(type);
> struct list_head *free_head = &pcpu_slot[pcpu_nr_slots - 1];
> struct pcpu_chunk *chunk, *next;
> - int slot, nr_to_pop, ret;
>
> /*
>* There's no reason to keep around multiple unused chunks and VM
>* areas can be scarce. Destroy all free chunks except for one.
>*/
> - mutex_lock(&pcpu_alloc_mutex);
> spin_lock_irq(&pcpu_lock);
>
> list_for_each_entry_safe(chunk, next, free_head, list) {
> @@ -1985,6 +1976,25 @@ static void __pcpu_balance_workfn(enum pcpu_chunk_type
> type)
> pcpu_destroy_chunk(chunk);
> cond_resched();
> }
> +}
> +
> +/**
> + * pcpu_balance_populated - manage the amount of populated pages
> + * @type: chunk type
> + *
> + * Maintain a certain amount of populated pages to satisfy atomic
> allocations.
> + * It is possible that this is called when physical memory is scarce causing
> + * OOM killer to be triggered. We should avoid doing so until an actual
> + * allocation causes the failure as it is possible that requests can be
> + * serviced from already backed regions.
> + */
> +static void pcpu_balance_populated(enum pcpu_chunk_type type)
> +{
> + /* gfp flags passed to underlying allocators */
> + const gfp_t gfp = GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN;
> + struct list_head *pcpu_slot = pcpu_chunk_list(type);
> + struct pcpu_chunk *chunk;
> + int slot, nr_to_pop, ret;
>
> /*
>* Ensure there are certain number of free populated pages for
> @@ -2054,22 +2064,24 @@ static void __pcpu_balance_workfn(enum
> pcpu_chunk_type type)
> goto retry_pop;
> }
> }
> -
> - mutex_unlock(&pcpu_alloc_mutex);
> }
>
> /**
> * pcpu_balance_workfn - manage the amount of free chunks and populated pages
> * @work: unused
> *
> - * Call __pcpu_balance_workfn() for each chunk type.
> + * Call pcpu_balance_free() and pcpu_balance_populated() for each chunk type.
> */
> static void pcpu_balance_workfn(struct work_struct *work)
> {
> enum pcpu_chunk_type type;
>
> - for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++)
> - __pcpu_balance_workfn(type);
> + for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++) {
> + mutex_lock(&pcpu_alloc_mutex);
> + pcpu_balance_free(type);
> + pcpu_balance_populated(type);
> + mutex_unlock(&pcpu_alloc_mutex);
> + }
> }
>
> /**
> --
> 2.30.2
>
>
I've applied this to for-5.14.
Thanks,
Dennis
Re: [PATCH v3 1/6] percpu: fix a comment about the chunks ordering
Hello,
On Wed, Apr 07, 2021 at 08:57:31PM -0700, Roman Gushchin wrote:
> Since the commit 3e54097beb22 ("percpu: manage chunks based on
> contig_bits instead of free_bytes") chunks are sorted based on the
> size of the biggest continuous free area instead of the total number
> of free bytes. Update the corresponding comment to reflect this.
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 6596a0a4286e..2f27123bb489 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -99,7 +99,10 @@
>
> #include "percpu-internal.h"
>
> -/* the slots are sorted by free bytes left, 1-31 bytes share the same slot */
> +/*
> + * The slots are sorted by the size of the biggest continuous free area.
> + * 1-31 bytes share the same slot.
> + */
> #define PCPU_SLOT_BASE_SHIFT 5
> /* chunks in slots below this are subject to being sidelined on failed alloc
> */
> #define PCPU_SLOT_FAIL_THRESHOLD 3
> --
> 2.30.2
>
I've applied this to for-5.14.
Thanks,
Dennis
Re: [PATCH v3 0/6] percpu: partial chunk depopulation
> > v1: > >- depopulation heuristics changed and optimized > >- chunks are put into a separate list, depopulation scan this list > >- chunk->isolated is introduced, chunk->depopulate is dropped > >- rearranged patches a bit > >- fixed a panic discovered by krobot > >- made pcpu_nr_empty_pop_pages per chunk type > >- minor fixes > > > > rfc: > >https://lwn.net/Articles/850508/ > > > > > > Roman Gushchin (6): > >percpu: fix a comment about the chunks ordering > >percpu: split __pcpu_balance_workfn() > >percpu: make pcpu_nr_empty_pop_pages per chunk type > >percpu: generalize pcpu_balance_populated() > >percpu: factor out pcpu_check_chunk_hint() > >percpu: implement partial chunk depopulation > > > > mm/percpu-internal.h | 4 +- > > mm/percpu-stats.c| 9 +- > > mm/percpu.c | 306 +++ > > 3 files changed, 261 insertions(+), 58 deletions(-) > > > Roman, sorry for the delay. I'm looking to apply this today to for-5.14. Thanks, Dennis
Re: [PATCH 1/5] mm/swapfile: add percpu_ref support for swap
On Thu, Apr 15, 2021 at 01:24:31PM +0800, Huang, Ying wrote:
> Dennis Zhou writes:
>
> > On Wed, Apr 14, 2021 at 01:44:58PM +0800, Huang, Ying wrote:
> >> Dennis Zhou writes:
> >>
> >> > On Wed, Apr 14, 2021 at 11:59:03AM +0800, Huang, Ying wrote:
> >> >> Dennis Zhou writes:
> >> >>
> >> >> > Hello,
> >> >> >
> >> >> > On Wed, Apr 14, 2021 at 10:06:48AM +0800, Huang, Ying wrote:
> >> >> >> Miaohe Lin writes:
> >> >> >>
> >> >> >> > On 2021/4/14 9:17, Huang, Ying wrote:
> >> >> >> >> Miaohe Lin writes:
> >> >> >> >>
> >> >> >> >>> On 2021/4/12 15:24, Huang, Ying wrote:
> >> >> >> >>>> "Huang, Ying" writes:
> >> >> >> >>>>
> >> >> >> >>>>> Miaohe Lin writes:
> >> >> >> >>>>>
> >> >> >> >>>>>> We will use percpu-refcount to serialize against concurrent
> >> >> >> >>>>>> swapoff. This
> >> >> >> >>>>>> patch adds the percpu_ref support for later fixup.
> >> >> >> >>>>>>
> >> >> >> >>>>>> Signed-off-by: Miaohe Lin
> >> >> >> >>>>>> ---
> >> >> >> >>>>>> include/linux/swap.h | 2 ++
> >> >> >> >>>>>> mm/swapfile.c| 25 ++---
> >> >> >> >>>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
> >> >> >> >>>>>>
> >> >> >> >>>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> >> >> >>>>>> index 144727041e78..849ba5265c11 100644
> >> >> >> >>>>>> --- a/include/linux/swap.h
> >> >> >> >>>>>> +++ b/include/linux/swap.h
> >> >> >> >>>>>> @@ -240,6 +240,7 @@ struct swap_cluster_list {
> >> >> >> >>>>>> * The in-memory structure used to track swap areas.
> >> >> >> >>>>>> */
> >> >> >> >>>>>> struct swap_info_struct {
> >> >> >> >>>>>> + struct percpu_ref users;/* serialization
> >> >> >> >>>>>> against concurrent swapoff */
> >> >> >> >>>>>> unsigned long flags; /* SWP_USED etc: see
> >> >> >> >>>>>> above */
> >> >> >> >>>>>> signed shortprio; /* swap priority of
> >> >> >> >>>>>> this type */
> >> >> >> >>>>>> struct plist_node list; /* entry in
> >> >> >> >>>>>> swap_active_head */
> >> >> >> >>>>>> @@ -260,6 +261,7 @@ struct swap_info_struct {
> >> >> >> >>>>>> struct block_device *bdev; /* swap device or bdev
> >> >> >> >>>>>> of swap file */
> >> >> >> >>>>>> struct file *swap_file; /* seldom referenced */
> >> >> >> >>>>>> unsigned int old_block_size;/* seldom referenced */
> >> >> >> >>>>>> + struct completion comp; /* seldom referenced */
> >> >> >> >>>>>> #ifdef CONFIG_FRONTSWAP
> >> >> >> >>>>>> unsigned long *frontswap_map; /* frontswap in-use,
> >> >> >> >>>>>> one bit per page */
> >> >> >> >>>>>> atomic_t frontswap_pages; /* frontswap pages
> >> >> >> >>>>>> in-use counter */
> >> >> >> >>>>>> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> >> >> >>>>>> index 149e77454e3c..724173cd7d0c 100644
> >> >> >> >>>>>> --- a/mm/swapfile.c
> >> >> >> >>>>>> +++ b/mm/swapfile.c
> >> >> >> >>>>>>
Re: [PATCH 1/5] mm/swapfile: add percpu_ref support for swap
On Thu, Apr 15, 2021 at 11:16:42AM +0800, Miaohe Lin wrote:
> On 2021/4/14 22:53, Dennis Zhou wrote:
> > On Wed, Apr 14, 2021 at 01:44:58PM +0800, Huang, Ying wrote:
> >> Dennis Zhou writes:
> >>
> >>> On Wed, Apr 14, 2021 at 11:59:03AM +0800, Huang, Ying wrote:
> >>>> Dennis Zhou writes:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>> On Wed, Apr 14, 2021 at 10:06:48AM +0800, Huang, Ying wrote:
> >>>>>> Miaohe Lin writes:
> >>>>>>
> >>>>>>> On 2021/4/14 9:17, Huang, Ying wrote:
> >>>>>>>> Miaohe Lin writes:
> >>>>>>>>
> >>>>>>>>> On 2021/4/12 15:24, Huang, Ying wrote:
> >>>>>>>>>> "Huang, Ying" writes:
> >>>>>>>>>>
> >>>>>>>>>>> Miaohe Lin writes:
> >>>>>>>>>>>
> >>>>>>>>>>>> We will use percpu-refcount to serialize against concurrent
> >>>>>>>>>>>> swapoff. This
> >>>>>>>>>>>> patch adds the percpu_ref support for later fixup.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Miaohe Lin
> >>>>>>>>>>>> ---
> >>>>>>>>>>>> include/linux/swap.h | 2 ++
> >>>>>>>>>>>> mm/swapfile.c| 25 ++---
> >>>>>>>>>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >>>>>>>>>>>> index 144727041e78..849ba5265c11 100644
> >>>>>>>>>>>> --- a/include/linux/swap.h
> >>>>>>>>>>>> +++ b/include/linux/swap.h
> >>>>>>>>>>>> @@ -240,6 +240,7 @@ struct swap_cluster_list {
> >>>>>>>>>>>> * The in-memory structure used to track swap areas.
> >>>>>>>>>>>> */
> >>>>>>>>>>>> struct swap_info_struct {
> >>>>>>>>>>>> +struct percpu_ref users;/* serialization
> >>>>>>>>>>>> against concurrent swapoff */
> >>>>>>>>>>>> unsigned long flags; /* SWP_USED etc: see
> >>>>>>>>>>>> above */
> >>>>>>>>>>>> signed shortprio; /* swap priority of
> >>>>>>>>>>>> this type */
> >>>>>>>>>>>> struct plist_node list; /* entry in
> >>>>>>>>>>>> swap_active_head */
> >>>>>>>>>>>> @@ -260,6 +261,7 @@ struct swap_info_struct {
> >>>>>>>>>>>> struct block_device *bdev; /* swap device or bdev
> >>>>>>>>>>>> of swap file */
> >>>>>>>>>>>> struct file *swap_file; /* seldom referenced */
> >>>>>>>>>>>> unsigned int old_block_size;/* seldom referenced */
> >>>>>>>>>>>> +struct completion comp; /* seldom referenced */
> >>>>>>>>>>>> #ifdef CONFIG_FRONTSWAP
> >>>>>>>>>>>> unsigned long *frontswap_map; /* frontswap in-use,
> >>>>>>>>>>>> one bit per page */
> >>>>>>>>>>>> atomic_t frontswap_pages; /* frontswap pages
> >>>>>>>>>>>> in-use counter */
> >>>>>>>>>>>> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >>>>>>>>>>>> index 149e77454e3c..724173cd7d0c 100644
> >>>>>>>>>>>> --- a/mm/swapfile.c
> >>>>>>>>>>>> +++ b/mm/swapfile.c
> >>>>>>>>>>>> @@ -39,6 +39,7 @@
> >>>>>>>>>>>> #include
> >>>>>>>>
Re: [PATCH 1/5] mm/swapfile: add percpu_ref support for swap
On Wed, Apr 14, 2021 at 01:44:58PM +0800, Huang, Ying wrote:
> Dennis Zhou writes:
>
> > On Wed, Apr 14, 2021 at 11:59:03AM +0800, Huang, Ying wrote:
> >> Dennis Zhou writes:
> >>
> >> > Hello,
> >> >
> >> > On Wed, Apr 14, 2021 at 10:06:48AM +0800, Huang, Ying wrote:
> >> >> Miaohe Lin writes:
> >> >>
> >> >> > On 2021/4/14 9:17, Huang, Ying wrote:
> >> >> >> Miaohe Lin writes:
> >> >> >>
> >> >> >>> On 2021/4/12 15:24, Huang, Ying wrote:
> >> >> >>>> "Huang, Ying" writes:
> >> >> >>>>
> >> >> >>>>> Miaohe Lin writes:
> >> >> >>>>>
> >> >> >>>>>> We will use percpu-refcount to serialize against concurrent
> >> >> >>>>>> swapoff. This
> >> >> >>>>>> patch adds the percpu_ref support for later fixup.
> >> >> >>>>>>
> >> >> >>>>>> Signed-off-by: Miaohe Lin
> >> >> >>>>>> ---
> >> >> >>>>>> include/linux/swap.h | 2 ++
> >> >> >>>>>> mm/swapfile.c| 25 ++---
> >> >> >>>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
> >> >> >>>>>>
> >> >> >>>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> >> >>>>>> index 144727041e78..849ba5265c11 100644
> >> >> >>>>>> --- a/include/linux/swap.h
> >> >> >>>>>> +++ b/include/linux/swap.h
> >> >> >>>>>> @@ -240,6 +240,7 @@ struct swap_cluster_list {
> >> >> >>>>>> * The in-memory structure used to track swap areas.
> >> >> >>>>>> */
> >> >> >>>>>> struct swap_info_struct {
> >> >> >>>>>> +struct percpu_ref users;/* serialization
> >> >> >>>>>> against concurrent swapoff */
> >> >> >>>>>> unsigned long flags; /* SWP_USED etc: see
> >> >> >>>>>> above */
> >> >> >>>>>> signed shortprio; /* swap priority of
> >> >> >>>>>> this type */
> >> >> >>>>>> struct plist_node list; /* entry in
> >> >> >>>>>> swap_active_head */
> >> >> >>>>>> @@ -260,6 +261,7 @@ struct swap_info_struct {
> >> >> >>>>>> struct block_device *bdev; /* swap device or bdev
> >> >> >>>>>> of swap file */
> >> >> >>>>>> struct file *swap_file; /* seldom referenced */
> >> >> >>>>>> unsigned int old_block_size;/* seldom referenced */
> >> >> >>>>>> +struct completion comp; /* seldom referenced */
> >> >> >>>>>> #ifdef CONFIG_FRONTSWAP
> >> >> >>>>>> unsigned long *frontswap_map; /* frontswap in-use,
> >> >> >>>>>> one bit per page */
> >> >> >>>>>> atomic_t frontswap_pages; /* frontswap pages
> >> >> >>>>>> in-use counter */
> >> >> >>>>>> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> >> >>>>>> index 149e77454e3c..724173cd7d0c 100644
> >> >> >>>>>> --- a/mm/swapfile.c
> >> >> >>>>>> +++ b/mm/swapfile.c
> >> >> >>>>>> @@ -39,6 +39,7 @@
> >> >> >>>>>> #include
> >> >> >>>>>> #include
> >> >> >>>>>> #include
> >> >> >>>>>> +#include
> >> >> >>>>>>
> >> >> >>>>>> #include
> >> >> >>>>>> #include
> >> >> >>>>>> @@ -511,6 +512,15 @@ static void swap_discard_work(struct
> >> >> >>>>>> work_struct *work)
Re: [PATCH 1/5] mm/swapfile: add percpu_ref support for swap
On Wed, Apr 14, 2021 at 11:59:03AM +0800, Huang, Ying wrote:
> Dennis Zhou writes:
>
> > Hello,
> >
> > On Wed, Apr 14, 2021 at 10:06:48AM +0800, Huang, Ying wrote:
> >> Miaohe Lin writes:
> >>
> >> > On 2021/4/14 9:17, Huang, Ying wrote:
> >> >> Miaohe Lin writes:
> >> >>
> >> >>> On 2021/4/12 15:24, Huang, Ying wrote:
> >> >>>> "Huang, Ying" writes:
> >> >>>>
> >> >>>>> Miaohe Lin writes:
> >> >>>>>
> >> >>>>>> We will use percpu-refcount to serialize against concurrent
> >> >>>>>> swapoff. This
> >> >>>>>> patch adds the percpu_ref support for later fixup.
> >> >>>>>>
> >> >>>>>> Signed-off-by: Miaohe Lin
> >> >>>>>> ---
> >> >>>>>> include/linux/swap.h | 2 ++
> >> >>>>>> mm/swapfile.c| 25 ++---
> >> >>>>>> 2 files changed, 24 insertions(+), 3 deletions(-)
> >> >>>>>>
> >> >>>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> >>>>>> index 144727041e78..849ba5265c11 100644
> >> >>>>>> --- a/include/linux/swap.h
> >> >>>>>> +++ b/include/linux/swap.h
> >> >>>>>> @@ -240,6 +240,7 @@ struct swap_cluster_list {
> >> >>>>>> * The in-memory structure used to track swap areas.
> >> >>>>>> */
> >> >>>>>> struct swap_info_struct {
> >> >>>>>> + struct percpu_ref users;/* serialization against
> >> >>>>>> concurrent swapoff */
> >> >>>>>> unsigned long flags; /* SWP_USED etc: see above */
> >> >>>>>> signed shortprio; /* swap priority of this type */
> >> >>>>>> struct plist_node list; /* entry in swap_active_head */
> >> >>>>>> @@ -260,6 +261,7 @@ struct swap_info_struct {
> >> >>>>>> struct block_device *bdev; /* swap device or bdev of swap
> >> >>>>>> file */
> >> >>>>>> struct file *swap_file; /* seldom referenced */
> >> >>>>>> unsigned int old_block_size;/* seldom referenced */
> >> >>>>>> + struct completion comp; /* seldom referenced */
> >> >>>>>> #ifdef CONFIG_FRONTSWAP
> >> >>>>>> unsigned long *frontswap_map; /* frontswap in-use, one bit
> >> >>>>>> per page */
> >> >>>>>> atomic_t frontswap_pages; /* frontswap pages in-use
> >> >>>>>> counter */
> >> >>>>>> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> >>>>>> index 149e77454e3c..724173cd7d0c 100644
> >> >>>>>> --- a/mm/swapfile.c
> >> >>>>>> +++ b/mm/swapfile.c
> >> >>>>>> @@ -39,6 +39,7 @@
> >> >>>>>> #include
> >> >>>>>> #include
> >> >>>>>> #include
> >> >>>>>> +#include
> >> >>>>>>
> >> >>>>>> #include
> >> >>>>>> #include
> >> >>>>>> @@ -511,6 +512,15 @@ static void swap_discard_work(struct
> >> >>>>>> work_struct *work)
> >> >>>>>> spin_unlock(&si->lock);
> >> >>>>>> }
> >> >>>>>>
> >> >>>>>> +static void swap_users_ref_free(struct percpu_ref *ref)
> >> >>>>>> +{
> >> >>>>>> + struct swap_info_struct *si;
> >> >>>>>> +
> >> >>>>>> + si = container_of(ref, struct swap_info_struct, users);
> >> >>>>>> + complete(&si->comp);
> >> >>>>>> + percpu_ref_exit(&si->users);
> >> >>>>>
> >> >>>>> Because percpu_ref_exit() is used, we cannot use percpu_ref_tryget()
> >> >>>>> in
> >> >>>>>
Re: [PATCH 1/5] mm/swapfile: add percpu_ref support for swap
I read
> >>> the
> >>> implementation code of the percpu_ref and found percpu_ref_tryget_live()
> >>> could
> >>> be called after exit now. But you're right we need to follow the API
> >>> definition
> >>> to avoid potential issues in the long term.
> >>>
> >>>>
> >>>> And we need to call percpu_ref_init() before insert the swap_info_struct
> >>>> into the swap_info[].
> >>>
> >>> If we remove the call to percpu_ref_exit(), we should not use
> >>> percpu_ref_init()
> >>> here because *percpu_ref->data is assumed to be NULL* in
> >>> percpu_ref_init() while
> >>> this is not the case as we do not call percpu_ref_exit(). Maybe
> >>> percpu_ref_reinit()
> >>> or percpu_ref_resurrect() will do the work.
> >>>
> >>> One more thing, how could I distinguish the killed percpu_ref from newly
> >>> allocated one?
> >>> It seems percpu_ref_is_dying is only safe to call when @ref is between
> >>> init and exit.
> >>> Maybe I could do this in alloc_swap_info()?
> >>
> >> Yes. In alloc_swap_info(), you can distinguish newly allocated and
> >> reused swap_info_struct.
> >>
> >>>>
> >>>>>> +}
> >>>>>> +
> >>>>>> static void alloc_cluster(struct swap_info_struct *si, unsigned long
> >>>>>> idx)
> >>>>>> {
> >>>>>>struct swap_cluster_info *ci = si->cluster_info;
> >>>>>> @@ -2500,7 +2510,7 @@ static void enable_swap_info(struct
> >>>>>> swap_info_struct *p, int prio,
> >>>>>> * Guarantee swap_map, cluster_info, etc. fields are valid
> >>>>>> * between get/put_swap_device() if SWP_VALID bit is set
> >>>>>> */
> >>>>>> - synchronize_rcu();
> >>>>>> + percpu_ref_reinit(&p->users);
> >>>>>
> >>>>> Although the effect is same, I think it's better to use
> >>>>> percpu_ref_resurrect() here to improve code readability.
> >>>>
> >>>> Check the original commit description for commit eb085574a752 "mm, swap:
> >>>> fix race between swapoff and some swap operations" and discussion email
> >>>> thread as follows again,
> >>>>
> >>>> https://lore.kernel.org/linux-mm/20171219053650.gb7...@linux.vnet.ibm.com/
> >>>>
> >>>> I found that the synchronize_rcu() here is to avoid to call smp_rmb() or
> >>>> smp_load_acquire() in get_swap_device(). Now we will use
> >>>> percpu_ref_tryget_live() in get_swap_device(), so we will need to add
> >>>> the necessary memory barrier, or make sure percpu_ref_tryget_live() has
> >>>> ACQUIRE semantics. Per my understanding, we need to change
> >>>> percpu_ref_tryget_live() for that.
> >>>>
> >>>
> >>> Do you mean the below scene is possible?
> >>>
> >>> cpu1
> >>> swapon()
> >>> ...
> >>> percpu_ref_init
> >>> ...
> >>> setup_swap_info
> >>> /* smp_store_release() is inside percpu_ref_reinit */
> >>> percpu_ref_reinit
> >>
> >> spin_unlock() has RELEASE semantics already.
> >>
> >>> ...
> >>>
> >>> cpu2
> >>> get_swap_device()
> >>> /* ignored smp_rmb() */
> >>> percpu_ref_tryget_live
> >>
> >> Some kind of ACQUIRE is required here to guarantee the refcount is
> >> checked before fetching the other fields of swap_info_struct. I have
> >> sent out a RFC patch to mailing list to discuss this.
I'm just catching up and following along a little bit. I apologize I
haven't read the swap code, but my understanding is you are trying to
narrow a race condition with swapoff. That makes sense to me. I'm not
sure I follow the need to race with reinitializing the ref though? Is it
not possible to wait out the dying swap info and then create a new one
rather than push acquire semantics?
> >
> > Many thanks.
> > But We may still need to add a smp_rmb() in get_swap_device() in case
> > we can't add ACQUIRE for refcount.
>
> Yes.
>
> Best Regards,
> Huang, Ying
>
Thanks,
Dennis
[GIT PULL] percpu changes for v5.12-rc7
Hi Linus, This pull request contains a fix for sporadically failing atomic percpu allocations. I only caught it recently while I was reviewing a new series [1] and simultaneously saw reports by btrfs in xfstests [2] and [3]. In v5.9, memcg accounting was extended to percpu done by adding a second type of chunk. I missed an interaction with the free page float count used to ensure we can support atomic allocations. If 1 type of chunk has no free pages, but the other has enough to satisfy the free page float requirement, we will not repopulate the free pages for the former type of chunk. This led to sporadically failing atomic allocations. [1] https://lore.kernel.org/linux-mm/20210324190626.564297-1-g...@fb.com/ [2] https://lore.kernel.org/linux-mm/20210401185158.3275.40950...@e16-tech.com/ [3] https://lore.kernel.org/linux-mm/cal3q7h5rnbjci708gh7jnczaoe0blnact9c+obga-dx9jhb...@mail.gmail.com/ Thanks, Dennis The following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git for-5.12-fixes for you to fetch changes up to 0760fa3d8f7fceeea508b98899f1c826e10ffe78: percpu: make pcpu_nr_empty_pop_pages per chunk type (2021-04-09 13:58:38 +) Roman Gushchin (1): percpu: make pcpu_nr_empty_pop_pages per chunk type mm/percpu-internal.h | 2 +- mm/percpu-stats.c| 9 +++-- mm/percpu.c | 14 +++--- 3 files changed, 15 insertions(+), 10 deletions(-)
Re: linux-next: Signed-off-by missing for commit in the rdma tree
On 4/8/2021 6:00 PM, Stephen Rothwell wrote:
Hi all,
Commit
042a00f93aad ("IB/{ipoib,hfi1}: Add a timeout handler for rdma_netdev")
is missing a Signed-off-by from its author.
Doh! That's my fault. I must have fat fingered the delete button instead
of editing the line when I was converting our email addresses to the new
name.
Jason do you want a v2 of the patch?
-Denny
Re: [PATCH v2 5/5] percpu: implement partial chunk depopulation
Hello,
On Wed, Apr 07, 2021 at 11:26:18AM -0700, Roman Gushchin wrote:
> This patch implements partial depopulation of percpu chunks.
>
> As now, a chunk can be depopulated only as a part of the final
> destruction, if there are no more outstanding allocations. However
> to minimize a memory waste it might be useful to depopulate a
> partially filed chunk, if a small number of outstanding allocations
> prevents the chunk from being fully reclaimed.
>
> This patch implements the following depopulation process: it scans
> over the chunk pages, looks for a range of empty and populated pages
> and performs the depopulation. To avoid races with new allocations,
> the chunk is previously isolated. After the depopulation the chunk is
> sidelined to a special list or freed. New allocations can't be served
> using a sidelined chunk. The chunk can be moved back to a corresponding
> slot if there are not enough chunks with empty populated pages.
>
> The depopulation is scheduled on the free path. Is the chunk:
> 1) has more than 1/4 of total pages free and populated
> 2) the system has enough free percpu pages aside of this chunk
> 3) isn't the reserved chunk
> 4) isn't the first chunk
> 5) isn't entirely free
> it's a good target for depopulation. If it's already depopulated
> but got free populated pages, it's a good target too.
> The chunk is moved to a special pcpu_depopulate_list, chunk->isolate
> flag is set and the async balancing is scheduled.
>
> The async balancing moves pcpu_depopulate_list to a local list
> (because pcpu_depopulate_list can be changed when pcpu_lock is
> releases), and then tries to depopulate each chunk. The depopulation
> is performed in the reverse direction to keep populated pages close to
> the beginning, if the global number of empty pages is reached.
> Depopulated chunks are sidelined to prevent further allocations.
> Skipped and fully empty chunks are returned to the corresponding slot.
>
> On the allocation path, if there are no suitable chunks found,
> the list of sidelined chunks in scanned prior to creating a new chunk.
> If there is a good sidelined chunk, it's placed back to the slot
> and the scanning is restarted.
>
> Many thanks to Dennis Zhou for his great ideas and a very constructive
> discussion which led to many improvements in this patchset!
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu-internal.h | 2 +
> mm/percpu.c | 164 ++-
> 2 files changed, 164 insertions(+), 2 deletions(-)
>
> diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
> index 095d7eaa0db4..8e432663c41e 100644
> --- a/mm/percpu-internal.h
> +++ b/mm/percpu-internal.h
> @@ -67,6 +67,8 @@ struct pcpu_chunk {
>
> void*data; /* chunk data */
> boolimmutable; /* no [de]population allowed */
> + boolisolated; /* isolated from chunk slot
> lists */
> + booldepopulated;/* sidelined after depopulation
> */
> int start_offset; /* the overlap with the previous
> region to have a page aligned
> base_addr */
> diff --git a/mm/percpu.c b/mm/percpu.c
> index e20119668c42..0a5a5e84e0a4 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -181,6 +181,19 @@ static LIST_HEAD(pcpu_map_extend_chunks);
> */
> int pcpu_nr_empty_pop_pages[PCPU_NR_CHUNK_TYPES];
>
> +/*
> + * List of chunks with a lot of free pages. Used to depopulate them
> + * asynchronously.
> + */
> +static struct list_head pcpu_depopulate_list[PCPU_NR_CHUNK_TYPES];
> +
> +/*
> + * List of previously depopulated chunks. They are not usually used for new
> + * allocations, but can be returned back to service if a need arises.
> + */
> +static struct list_head pcpu_sideline_list[PCPU_NR_CHUNK_TYPES];
> +
> +
> /*
> * The number of populated pages in use by the allocator, protected by
> * pcpu_lock. This number is kept per a unit per chunk (i.e. when a page
> gets
> @@ -542,6 +555,12 @@ static void pcpu_chunk_relocate(struct pcpu_chunk
> *chunk, int oslot)
> {
> int nslot = pcpu_chunk_slot(chunk);
>
> + /*
> + * Keep isolated and depopulated chunks on a sideline.
> + */
> + if (chunk->isolated || chunk->depopulated)
> + return;
> +
> if (oslot != nslot)
> __pcpu_chunk_move(chunk, nslot, oslot < nslot);
> }
> @@ -1778,6 +1797,25 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align
Re: [PATCH v1 5/5] percpu: implement partial chunk depopulation
On Thu, Apr 01, 2021 at 02:43:01PM -0700, Roman Gushchin wrote:
> This patch implements partial depopulation of percpu chunks.
>
> As now, a chunk can be depopulated only as a part of the final
> destruction, if there are no more outstanding allocations. However
> to minimize a memory waste it might be useful to depopulate a
> partially filed chunk, if a small number of outstanding allocations
> prevents the chunk from being fully reclaimed.
>
> This patch implements the following depopulation process: it scans
> over the chunk pages, looks for a range of empty and populated pages
> and performs the depopulation. To avoid races with new allocations,
> the chunk is previously isolated. After the depopulation the chunk is
> returned to the original slot (but is appended to the tail of the list
> to minimize the chances of population).
>
> Because the pcpu_lock is dropped while calling pcpu_depopulate_chunk(),
> the chunk can be concurrently moved to a different slot. To prevent
> this, bool chunk->isolated flag is introduced. If set, the chunk can't
> be moved to a different slot.
>
> The depopulation is scheduled on the free path. Is the chunk:
> 1) has more than 1/8 of total pages free and populated
> 2) the system has enough free percpu pages aside of this chunk
> 3) isn't the reserved chunk
> 4) isn't the first chunk
> 5) isn't entirely free
> it's a good target for depopulation.
>
> If so, the chunk is moved to a special pcpu_depopulate_list,
> chunk->isolate flag is set and the async balancing is scheduled.
>
> The async balancing moves pcpu_depopulate_list to a local list
> (because pcpu_depopulate_list can be changed when pcpu_lock is
> releases), and then tries to depopulate each chunk. Successfully
> or not, at the end all chunks are returned to appropriate slots
> and their isolated flags are cleared.
>
> Many thanks to Dennis Zhou for his great ideas and a very constructive
> discussion which led to many improvements in this patchset!
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu-internal.h | 1 +
> mm/percpu.c | 101 ++-
> 2 files changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
> index 095d7eaa0db4..ff318752915d 100644
> --- a/mm/percpu-internal.h
> +++ b/mm/percpu-internal.h
> @@ -67,6 +67,7 @@ struct pcpu_chunk {
>
> void*data; /* chunk data */
> boolimmutable; /* no [de]population allowed */
> + boolisolated; /* isolated from chunk slot
> lists */
> int start_offset; /* the overlap with the previous
> region to have a page aligned
> base_addr */
> diff --git a/mm/percpu.c b/mm/percpu.c
> index e20119668c42..dae0b870e10a 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -181,6 +181,12 @@ static LIST_HEAD(pcpu_map_extend_chunks);
> */
> int pcpu_nr_empty_pop_pages[PCPU_NR_CHUNK_TYPES];
>
> +/*
> + * List of chunks with a lot of free pages. Used to depopulate them
> + * asynchronously.
> + */
> +static LIST_HEAD(pcpu_depopulate_list);
> +
Now that pcpu_nr_empty_pop_pages is per chunk_type I think the
depopulate_list should be per chunk_type.
> /*
> * The number of populated pages in use by the allocator, protected by
> * pcpu_lock. This number is kept per a unit per chunk (i.e. when a page
> gets
> @@ -542,7 +548,7 @@ static void pcpu_chunk_relocate(struct pcpu_chunk *chunk,
> int oslot)
> {
> int nslot = pcpu_chunk_slot(chunk);
>
> - if (oslot != nslot)
> + if (!chunk->isolated && oslot != nslot)
> __pcpu_chunk_move(chunk, nslot, oslot < nslot);
> }
>
> @@ -2048,6 +2054,82 @@ static void pcpu_grow_populated(enum pcpu_chunk_type
> type, int nr_to_pop)
> }
> }
>
> +/**
> + * pcpu_shrink_populated - scan chunks and release unused pages to the system
> + * @type: chunk type
> + *
> + * Scan over all chunks, find those marked with the depopulate flag and
> + * try to release unused pages to the system. On every attempt clear the
> + * chunk's depopulate flag to avoid wasting CPU by scanning the same
> + * chunk again and again.
> + */
There no longer is a depopulate flag.
> +static void pcpu_shrink_populated(enum pcpu_chunk_type type)
> +{
> + struct pcpu_block_md *block;
> + struct pcpu_chunk *chunk, *tmp;
> + LIST_HEAD(to_depopulate);
> + int i, start;
> +
> + spin_lock_irq(&pcpu_lock
Re: [PATCH rfc 3/4] percpu: on demand chunk depopulation
On Mon, Mar 29, 2021 at 01:10:10PM -0700, Roman Gushchin wrote:
> On Mon, Mar 29, 2021 at 07:21:24PM +0000, Dennis Zhou wrote:
> > On Wed, Mar 24, 2021 at 12:06:25PM -0700, Roman Gushchin wrote:
> > > To return unused memory to the system schedule an async
> > > depopulation of percpu chunks.
> > >
> > > To balance between scanning too much and creating an overhead because
> > > of the pcpu_lock contention and scanning not enough, let's track an
> > > amount of chunks to scan and mark chunks which are potentially a good
> > > target for the depopulation with a new boolean flag. The async
> > > depopulation work will clear the flag after trying to depopulate a
> > > chunk (successfully or not).
> > >
> > > This commit suggest the following logic: if a chunk
> > > 1) has more than 1/4 of total pages free and populated
> > > 2) isn't a reserved chunk
> > > 3) isn't entirely free
> > > 4) isn't alone in the corresponding slot
> >
> > I'm not sure I like the check for alone that much. The reason being what
> > about some odd case where each slot has a single chunk, but every slot
> > is populated. It doesn't really make sense to keep them all around.
>
> Yeah, I agree, I'm not sure either. Maybe we can just look at the total
> number of populated empty pages and make sure it's not too low and not
> too high. Btw, we should probably double PCPU_EMPTY_POP_PAGES_LOW/HIGH
> if memcg accounting is on.
>
Hmmm. pcpu_nr_populated and pcpu_nr_empty_pop_pages should probably be
per chunk type now that you mention it.
> >
> > I think there is some decision making we can do here to handle packing
> > post depopulation allocations into a handful of chunks. Depopulated
> > chunks could be sidelined with say a flag ->depopulated to prevent the
> > first attempt of allocations from using them. And then we could bring
> > back a chunk 1 by 1 somehow to attempt to suffice the allocation.
> > I'm not too sure if this is a good idea, just a thought.
>
> I thought about it in this way: depopulated chunks are not different to
> new chunks, which are not yet fully populated. And they are naturally
> de-prioritized by being located in higher slots (and at the tail of the list).
> So I'm not sure we should handle them any special.
>
I'm thinking of the following. Imagine 3 chunks, A and B in slot X, and
C in slot X+1. If B gets depopulated followed by A getting exhausted,
which chunk B or C should be used? If C is fully populated, we might
want to use that one.
I see that the priority is chunks at the very end, but I don't want to
take something that doesn't reasonable generalize to any slot PAGE_SIZE
and up. Or it should explicitly try to tackle only say the last N slots
(but preferably the former).
> >
> > > it's a good target for depopulation.
> > >
> > > If there are 2 or more of such chunks, an async depopulation
> > > is scheduled.
> > >
> > > Because chunk population and depopulation are opposite processes
> > > which make a little sense together, split out the shrinking part of
> > > pcpu_balance_populated() into pcpu_grow_populated() and make
> > > pcpu_balance_populated() calling into pcpu_grow_populated() or
> > > pcpu_shrink_populated() conditionally.
> > >
> > > Signed-off-by: Roman Gushchin
> > > ---
> > > mm/percpu-internal.h | 1 +
> > > mm/percpu.c | 111 ---
> > > 2 files changed, 85 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h
> > > index 18b768ac7dca..1c5b92af02eb 100644
> > > --- a/mm/percpu-internal.h
> > > +++ b/mm/percpu-internal.h
> > > @@ -67,6 +67,7 @@ struct pcpu_chunk {
> > >
> > > void*data; /* chunk data */
> > > boolimmutable; /* no [de]population allowed */
> > > + booldepopulate; /* depopulation hint */
> > > int start_offset; /* the overlap with the previous
> > > region to have a page aligned
> > > base_addr */
> > > diff --git a/mm/percpu.c b/mm/percpu.c
> > > index 015d076893f5..148137f0fc0b 100644
> > > --- a/mm/percpu.c
> > > +++ b/mm/percpu.c
> > > @@ -178,6 +178,12 @@ static LIST_HEAD(pcpu_map_extend_chunks);
> > > */
Re: [PATCH rfc 1/4] percpu: implement partial chunk depopulation
On Mon, Mar 29, 2021 at 11:29:22AM -0700, Roman Gushchin wrote:
> On Mon, Mar 29, 2021 at 05:20:55PM +0000, Dennis Zhou wrote:
> > On Wed, Mar 24, 2021 at 12:06:23PM -0700, Roman Gushchin wrote:
> > > This patch implements partial depopulation of percpu chunks.
> > >
> > > As now, a chunk can be depopulated only as a part of the final
> > > destruction, when there are no more outstanding allocations. However
> > > to minimize a memory waste, it might be useful to depopulate a
> > > partially filed chunk, if a small number of outstanding allocations
> > > prevents the chunk from being reclaimed.
> > >
> > > This patch implements the following depopulation process: it scans
> > > over the chunk pages, looks for a range of empty and populated pages
> > > and performs the depopulation. To avoid races with new allocations,
> > > the chunk is previously isolated. After the depopulation the chunk is
> > > returned to the original slot (but is appended to the tail of the list
> > > to minimize the chances of population).
> > >
> > > Because the pcpu_lock is dropped while calling pcpu_depopulate_chunk(),
> > > the chunk can be concurrently moved to a different slot. So we need
> > > to isolate it again on each step. pcpu_alloc_mutex is held, so the
> > > chunk can't be populated/depopulated asynchronously.
> > >
> > > Signed-off-by: Roman Gushchin
> > > ---
> > > mm/percpu.c | 90 +
> > > 1 file changed, 90 insertions(+)
> > >
> > > diff --git a/mm/percpu.c b/mm/percpu.c
> > > index 6596a0a4286e..78c55c73fa28 100644
> > > --- a/mm/percpu.c
> > > +++ b/mm/percpu.c
> > > @@ -2055,6 +2055,96 @@ static void __pcpu_balance_workfn(enum
> > > pcpu_chunk_type type)
> > > mutex_unlock(&pcpu_alloc_mutex);
> > > }
> > >
> > > +/**
> > > + * pcpu_shrink_populated - scan chunks and release unused pages to the
> > > system
> > > + * @type: chunk type
> > > + *
> > > + * Scan over all chunks, find those marked with the depopulate flag and
> > > + * try to release unused pages to the system. On every attempt clear the
> > > + * chunk's depopulate flag to avoid wasting CPU by scanning the same
> > > + * chunk again and again.
> > > + */
> > > +static void pcpu_shrink_populated(enum pcpu_chunk_type type)
> > > +{
> > > + struct list_head *pcpu_slot = pcpu_chunk_list(type);
> > > + struct pcpu_chunk *chunk;
> > > + int slot, i, off, start;
> > > +
> > > + spin_lock_irq(&pcpu_lock);
> > > + for (slot = pcpu_nr_slots - 1; slot >= 0; slot--) {
> > > +restart:
> > > + list_for_each_entry(chunk, &pcpu_slot[slot], list) {
> > > + bool isolated = false;
> > > +
> > > + if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_HIGH)
> > > + break;
> > > +
> >
> > Deallocation makes me a little worried for the atomic case as now we
> > could in theory pathologically scan deallocated chunks before finding a
> > populated one.
> >
> > I wonder if we should do something like once a chunk gets depopulated,
> > it gets deprioritized and then only once we exhaust looking through
> > allocated chunks we then find a depopulated chunk and add it back into
> > the rotation. Possibly just add another set of slots? I guess it adds a
> > few dimensions to pcpu_slots after the memcg change.
>
> Please, take a look at patch 3 in the series ("percpu: on demand chunk
> depopulation").
> Chunks considered to be a good target for the depopulation are in advance
> marked with a special flag, so we'll actually try to depopulate only
> few chunks at once. While the total number of chunks is fairly low,
> I think it should work.
>
> Another option is to link all such chunks into a list and scan over it,
> instead of iterating over all slots.
>
> Adding new dimensions to pcpu_slots is an option too, but I hope we can avoid
> this, as it would complicate the code.
>
Yeah, depopulation has been on the todo list for a while. It adds the
dimension/opportunity of bin packing by sidelining chunks and I'm
wondering if that is the right thing to do.
Do you have a rough idea of the distribution of # of chunks you're
seeing?
> >
> > > + for (i = 0, start = -1; i < chunk->nr_pages; i++) {
> > > +
Re: [PATCH rfc 3/4] percpu: on demand chunk depopulation
t pcpu_chunk *pos;
> -
> + /*
> + * If there are more than one fully free chunks,
> + * wake up grim reaper.
> + */
> list_for_each_entry(pos, &pcpu_slot[pcpu_nr_slots - 1], list)
> if (pos != chunk) {
> need_balance = true;
> break;
> }
> +
> + } else if (chunk->nr_empty_pop_pages > chunk->nr_pages / 4) {
We should have this ignore the first and reserved chunks. While it
shouldn't be possible in theory, it would be nice to just make it
explicit here.
> + /*
> + * If there is more than one chunk in the slot and
> + * at least 1/4 of its pages are empty, mark the chunk
> + * as a target for the depopulation. If there is more
> + * than one chunk like this, schedule an async balancing.
> + */
> + int nslot = pcpu_chunk_slot(chunk);
> +
> + list_for_each_entry(pos, &pcpu_slot[nslot], list)
> + if (pos != chunk && !chunk->depopulate &&
> + !chunk->immutable) {
> + chunk->depopulate = true;
> + pcpu_nr_chunks_to_depopulate++;
> + break;
> + }
> +
> + if (pcpu_nr_chunks_to_depopulate > 1)
> + need_balance = true;
> }
>
> trace_percpu_free_percpu(chunk->base_addr, off, ptr);
> --
> 2.30.2
>
Some questions I have:
1. How do we prevent unnecessary scanning for atomic allocations?
2. Even in the normal case, should we try to pack future allocations
into a smaller # of chunks in after depopulation?
3. What is the right frequency to do depopulation scanning? I think of
the pcpu work item as a way to defer the 2 the freeing of chunks and in
a way more immediately replenish free pages. Depopulation isn't
necessarily as high a priority.
Thanks,
Dennis
Re: [PATCH rfc 2/4] percpu: split __pcpu_balance_workfn()
On Wed, Mar 24, 2021 at 12:06:24PM -0700, Roman Gushchin wrote:
> __pcpu_balance_workfn() became fairly big and hard to follow, but in
> fact it consists of two fully independent parts, responsible for
> the destruction of excessive free chunks and population of necessarily
> amount of free pages.
>
> In order to simplify the code and prepare for adding of a new
> functionality, split it in two functions:
>
> 1) pcpu_balance_free,
> 2) pcpu_balance_populated.
>
> Move the taking/releasing of the pcpu_alloc_mutex to an upper level
> to keep the current synchronization in place.
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu.c | 46 +-
> 1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 78c55c73fa28..015d076893f5 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1930,31 +1930,22 @@ void __percpu *__alloc_reserved_percpu(size_t size,
> size_t align)
> }
>
> /**
> - * __pcpu_balance_workfn - manage the amount of free chunks and populated
> pages
> + * pcpu_balance_free - manage the amount of free chunks
> * @type: chunk type
> *
> - * Reclaim all fully free chunks except for the first one. This is also
> - * responsible for maintaining the pool of empty populated pages. However,
> - * it is possible that this is called when physical memory is scarce causing
> - * OOM killer to be triggered. We should avoid doing so until an actual
> - * allocation causes the failure as it is possible that requests can be
> - * serviced from already backed regions.
> + * Reclaim all fully free chunks except for the first one.
> */
> -static void __pcpu_balance_workfn(enum pcpu_chunk_type type)
> +static void pcpu_balance_free(enum pcpu_chunk_type type)
> {
> - /* gfp flags passed to underlying allocators */
> - const gfp_t gfp = GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN;
> LIST_HEAD(to_free);
> struct list_head *pcpu_slot = pcpu_chunk_list(type);
> struct list_head *free_head = &pcpu_slot[pcpu_nr_slots - 1];
> struct pcpu_chunk *chunk, *next;
> - int slot, nr_to_pop, ret;
>
> /*
>* There's no reason to keep around multiple unused chunks and VM
>* areas can be scarce. Destroy all free chunks except for one.
>*/
> - mutex_lock(&pcpu_alloc_mutex);
> spin_lock_irq(&pcpu_lock);
>
> list_for_each_entry_safe(chunk, next, free_head, list) {
> @@ -1982,6 +1973,25 @@ static void __pcpu_balance_workfn(enum pcpu_chunk_type
> type)
> pcpu_destroy_chunk(chunk);
> cond_resched();
> }
> +}
> +
> +/**
> + * pcpu_balance_populated - manage the amount of populated pages
> + * @type: chunk type
> + *
> + * Maintain a certain amount of populated pages to satisfy atomic
> allocations.
> + * It is possible that this is called when physical memory is scarce causing
> + * OOM killer to be triggered. We should avoid doing so until an actual
> + * allocation causes the failure as it is possible that requests can be
> + * serviced from already backed regions.
> + */
> +static void pcpu_balance_populated(enum pcpu_chunk_type type)
> +{
> + /* gfp flags passed to underlying allocators */
> + const gfp_t gfp = GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN;
> + struct list_head *pcpu_slot = pcpu_chunk_list(type);
> + struct pcpu_chunk *chunk;
> + int slot, nr_to_pop, ret;
>
> /*
>* Ensure there are certain number of free populated pages for
> @@ -2051,8 +2061,6 @@ static void __pcpu_balance_workfn(enum pcpu_chunk_type
> type)
> goto retry_pop;
> }
> }
> -
> - mutex_unlock(&pcpu_alloc_mutex);
> }
>
> /**
> @@ -2149,14 +2157,18 @@ static void pcpu_shrink_populated(enum
> pcpu_chunk_type type)
> * pcpu_balance_workfn - manage the amount of free chunks and populated pages
> * @work: unused
> *
> - * Call __pcpu_balance_workfn() for each chunk type.
> + * Call pcpu_balance_free() and pcpu_balance_populated() for each chunk type.
> */
> static void pcpu_balance_workfn(struct work_struct *work)
> {
> enum pcpu_chunk_type type;
>
> - for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++)
> - __pcpu_balance_workfn(type);
> + for (type = 0; type < PCPU_NR_CHUNK_TYPES; type++) {
> + mutex_lock(&pcpu_alloc_mutex);
> + pcpu_balance_free(type);
> + pcpu_balance_populated(type);
> + mutex_unlock(&pcpu_alloc_mutex);
> + }
> }
>
> /**
> --
> 2.30.2
>
Reviewed-by: Dennis Zhou
This makes sense. If you want me to pick this and the last patch up
first I can. Otherwise, do you mind moving this to the front of the
stack because it is a clean up?
Thanks,
Dennis
Re: [PATCH rfc 1/4] percpu: implement partial chunk depopulation
On Wed, Mar 24, 2021 at 12:06:23PM -0700, Roman Gushchin wrote:
> This patch implements partial depopulation of percpu chunks.
>
> As now, a chunk can be depopulated only as a part of the final
> destruction, when there are no more outstanding allocations. However
> to minimize a memory waste, it might be useful to depopulate a
> partially filed chunk, if a small number of outstanding allocations
> prevents the chunk from being reclaimed.
>
> This patch implements the following depopulation process: it scans
> over the chunk pages, looks for a range of empty and populated pages
> and performs the depopulation. To avoid races with new allocations,
> the chunk is previously isolated. After the depopulation the chunk is
> returned to the original slot (but is appended to the tail of the list
> to minimize the chances of population).
>
> Because the pcpu_lock is dropped while calling pcpu_depopulate_chunk(),
> the chunk can be concurrently moved to a different slot. So we need
> to isolate it again on each step. pcpu_alloc_mutex is held, so the
> chunk can't be populated/depopulated asynchronously.
>
> Signed-off-by: Roman Gushchin
> ---
> mm/percpu.c | 90 +
> 1 file changed, 90 insertions(+)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 6596a0a4286e..78c55c73fa28 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -2055,6 +2055,96 @@ static void __pcpu_balance_workfn(enum pcpu_chunk_type
> type)
> mutex_unlock(&pcpu_alloc_mutex);
> }
>
> +/**
> + * pcpu_shrink_populated - scan chunks and release unused pages to the system
> + * @type: chunk type
> + *
> + * Scan over all chunks, find those marked with the depopulate flag and
> + * try to release unused pages to the system. On every attempt clear the
> + * chunk's depopulate flag to avoid wasting CPU by scanning the same
> + * chunk again and again.
> + */
> +static void pcpu_shrink_populated(enum pcpu_chunk_type type)
> +{
> + struct list_head *pcpu_slot = pcpu_chunk_list(type);
> + struct pcpu_chunk *chunk;
> + int slot, i, off, start;
> +
> + spin_lock_irq(&pcpu_lock);
> + for (slot = pcpu_nr_slots - 1; slot >= 0; slot--) {
> +restart:
> + list_for_each_entry(chunk, &pcpu_slot[slot], list) {
> + bool isolated = false;
> +
> + if (pcpu_nr_empty_pop_pages < PCPU_EMPTY_POP_PAGES_HIGH)
> + break;
> +
Deallocation makes me a little worried for the atomic case as now we
could in theory pathologically scan deallocated chunks before finding a
populated one.
I wonder if we should do something like once a chunk gets depopulated,
it gets deprioritized and then only once we exhaust looking through
allocated chunks we then find a depopulated chunk and add it back into
the rotation. Possibly just add another set of slots? I guess it adds a
few dimensions to pcpu_slots after the memcg change.
> + for (i = 0, start = -1; i < chunk->nr_pages; i++) {
> + if (!chunk->nr_empty_pop_pages)
> + break;
> +
> + /*
> + * If the page is empty and populated, start or
> + * extend the [start, i) range.
> + */
> + if (test_bit(i, chunk->populated)) {
> + off = find_first_bit(
> + pcpu_index_alloc_map(chunk, i),
> + PCPU_BITMAP_BLOCK_BITS);
> + if (off >= PCPU_BITMAP_BLOCK_BITS) {
> + if (start == -1)
> + start = i;
> + continue;
> + }
Here instead of looking at the alloc_map, you can look at the
pcpu_block_md and look for a fully free contig_hint.
> + }
> +
> + /*
> + * Otherwise check if there is an active range,
> + * and if yes, depopulate it.
> + */
> + if (start == -1)
> + continue;
> +
> + /*
> + * Isolate the chunk, so new allocations
> + * wouldn't be served using this chunk.
> + * Async releases can still happen.
> + */
> + if (!list_empty(&chunk->list)) {
> + list_del_init(&chunk->list);
> + isolated = true;
Maybe when freeing a chunk, we should consider just isolating it period
and preventing pcpu_free_a
[GIT PULL] percpu changes for v5.12-rc1
Hi Linus, Percpu had a cleanup come in that makes use of the cpu bitmask helpers instead of the current iterative approach. This clean up has an adverse interaction when clang's inlining sensitivity is changed such that not all sites are inlined resulting in modpost being upset with section mismatch due to percpu setup being marked __init. It is fixed by introducing __flatten to compiler_attributes.h. This has been supported since clang 3.5 and gcc 4.4 [1]. [1] https://lore.kernel.org/lkml/CAKwvOdnxnooqtyeSem63V_P5980jc0Z2PDG=0im8ixeytsa...@mail.gmail.com/ Thanks, Dennis The following changes since commit 92bf22614b21a2706f4993b278017e437f7785b3: Linux 5.11-rc7 (2021-02-07 13:57:38 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git for-5.12 for you to fetch changes up to 258e0815e2b1706e87c0d874211097aa8a7aa52f: percpu: fix clang modpost section mismatch (2021-02-14 18:15:15 +) Dennis Zhou (1): percpu: fix clang modpost section mismatch Wonhyuk Yang (1): percpu: reduce the number of cpu distance comparisons include/linux/compiler_attributes.h | 6 ++ mm/percpu.c | 36 +--- 2 files changed, 27 insertions(+), 15 deletions(-)
[PATCH] percpu: fix clang modpost section mismatch
pcpu_build_alloc_info() is an __init function that makes a call to
cpumask_clear_cpu(). With CONFIG_GCOV_PROFILE_ALL enabled, the inline
heuristics are modified and such cpumask_clear_cpu() which is marked
inline doesn't get inlined. Because it works on mask in __initdata,
modpost throws a section mismatch error.
Arnd sent a patch with the flatten attribute as an alternative [2]. I've
added it to compiler_attributes.h.
modpost complaint:
WARNING: modpost: vmlinux.o(.text+0x735425): Section mismatch in reference
from the function cpumask_clear_cpu() to the variable
.init.data:pcpu_build_alloc_info.mask
The function cpumask_clear_cpu() references
the variable __initdata pcpu_build_alloc_info.mask.
This is often because cpumask_clear_cpu lacks a __initdata
annotation or the annotation of pcpu_build_alloc_info.mask is wrong.
clang output:
mm/percpu.c:2724:5: remark: cpumask_clear_cpu not inlined into
pcpu_build_alloc_info because too costly to inline (cost=725, threshold=325)
[-Rpass-missed=inline]
[1] https://lore.kernel.org/linux-mm/202012220454.9f6bkz9q-...@intel.com/
[2]
https://lore.kernel.org/lkml/cak8p3a2zwfnexksm8k_suhhwkor17jfo3xaplxjzfpqx0eu...@mail.gmail.com/
Reported-by: kernel test robot
Cc: Arnd Bergmann
Cc: Nick Desaulniers
Signed-off-by: Dennis Zhou
---
include/linux/compiler_attributes.h | 6 ++
mm/percpu.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/linux/compiler_attributes.h
b/include/linux/compiler_attributes.h
index ea5e04e75845..c043b8d2b17b 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -210,6 +210,12 @@
# define fallthroughdo {} while (0) /* fallthrough */
#endif
+/*
+ * gcc:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#flatten
+ */
+# define __flatten __attribute__((flatten))
+
/*
* Note the missing underscores.
*
diff --git a/mm/percpu.c b/mm/percpu.c
index 80f8f885a990..6596a0a4286e 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -2663,7 +2663,7 @@ early_param("percpu_alloc", percpu_alloc_setup);
* On success, pointer to the new allocation_info is returned. On
* failure, ERR_PTR value is returned.
*/
-static struct pcpu_alloc_info * __init pcpu_build_alloc_info(
+static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
size_t reserved_size, size_t dyn_size,
size_t atom_size,
pcpu_fc_cpu_distance_fn_t cpu_distance_fn)
--
2.30.0.478.g8a0d178c01-goog
Re: [PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info()
Hi Nick, On Mon, Jan 25, 2021 at 10:27:11AM -0800, Nick Desaulniers wrote: > On Mon, Jan 25, 2021 at 3:07 AM Arnd Bergmann wrote: > > > > On Tue, Jan 5, 2021 at 1:55 AM Dennis Zhou wrote: > > > > > > On Mon, Jan 04, 2021 at 04:46:51PM -0700, Nathan Chancellor wrote: > > > > On Thu, Dec 31, 2020 at 09:28:52PM +, Dennis Zhou wrote: > > > > > > > > > > > Hi Nathan, > > > > > > > > > > > Hi Dennis, > > > > > > > > I did a bisect of the problematic config against defconfig and it points > > > > out that CONFIG_GCOV_PROFILE_ALL is in the bad config but not the good > > > > config, which makes some sense as that will mess with clang's inlining > > > > heuristics. It does not appear to be the single config that makes a > > > > difference but it gives some clarity. > > > > > > > > > > Ah, thanks. To me it's kind of a corner case that I don't have a lot of > > > insight into. __init code is pretty limited and this warning is really > > > at the compilers whim. However, in this case only clang throws this > > > warning. > > > > > > > I do not personally have any strong opinions around the patch but is it > > > > really that much wasted memory to just annotate mask with __refdata? > > > > > > It's really not much memory, 1 bit per max # of cpus. The reported > > > config is on the extreme side compiling with 8k NR_CPUS, so 1kb. I'm > > > just not in love with the idea of adding a patch to improve readability > > > and it cost idle memory to resolve a compile time warning. > > > > > > If no one else chimes in in the next few days, I'll probably just apply > > > it and go from there. If another issue comes up I'll drop this and tag > > > it as __refdata. > > > > I've come across this one again in linux-next today, and found that > > I had an old patch for it already, that I had never submitted: > > > > From 7d6f40414490092b86f1a64d8c42426ee350da1a Mon Sep 17 00:00:00 2001 > > From: Arnd Bergmann > > Date: Mon, 7 Dec 2020 23:24:20 +0100 > > Subject: [PATCH] mm: percpu: fix section mismatch warning > > > > Building with arm64 clang sometimes (fairly rarely) shows a > > warning about the pcpu_build_alloc_info() function: > > > > WARNING: modpost: vmlinux.o(.text+0x21697c): Section mismatch in > > reference from the function cpumask_clear_cpu() to the variable > > .init.data:pcpu_build_alloc_info.mask > > The function cpumask_clear_cpu() references > > the variable __initdata pcpu_build_alloc_info.mask. > > This is often because cpumask_clear_cpu lacks a __initdata > > annotation or the annotation of pcpu_build_alloc_info.mask is wrong. > > > > What appears to be going on here is that the compiler decides to not > > inline the cpumask_clear_cpu() function that is marked 'inline' but not > > 'always_inline', and it then produces a specialized version of it that > > references the static mask unconditionally as an optimization. > > > > Marking cpumask_clear_cpu() as __always_inline would fix it, as would > > removing the __initdata annotation on the variable. I went for marking > > the function as __attribute__((flatten)) instead because all functions > > I had to look this one up; it's new to me! > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes > https://awesomekling.github.io/Smarter-C++-inlining-with-attribute-flatten/ > > Seems pretty cool/flexible to control inlining on the caller side! > > At the least though, we should avoid open coding the function attributes. See > include/linux/compiler_attributes.h > Arnd do you mind spinning a new version to add __flatten to compiler_attributes.h? > Testing quickly in godbolt, __flatten__ has been supported since at > least clang 3.5 and gcc 4.4, FWIW (so it doesn't need a > __has_attribute guard). > Thanks for testing this! Thanks, Dennis
Re: [PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info()
On Mon, Jan 25, 2021 at 12:07:24PM +0100, Arnd Bergmann wrote:
> On Tue, Jan 5, 2021 at 1:55 AM Dennis Zhou wrote:
> >
> > On Mon, Jan 04, 2021 at 04:46:51PM -0700, Nathan Chancellor wrote:
> > > On Thu, Dec 31, 2020 at 09:28:52PM +, Dennis Zhou wrote:
> > > >
> >
> > Hi Nathan,
> >
> > >
> > > Hi Dennis,
> > >
> > > I did a bisect of the problematic config against defconfig and it points
> > > out that CONFIG_GCOV_PROFILE_ALL is in the bad config but not the good
> > > config, which makes some sense as that will mess with clang's inlining
> > > heuristics. It does not appear to be the single config that makes a
> > > difference but it gives some clarity.
> > >
> >
> > Ah, thanks. To me it's kind of a corner case that I don't have a lot of
> > insight into. __init code is pretty limited and this warning is really
> > at the compilers whim. However, in this case only clang throws this
> > warning.
> >
> > > I do not personally have any strong opinions around the patch but is it
> > > really that much wasted memory to just annotate mask with __refdata?
> >
> > It's really not much memory, 1 bit per max # of cpus. The reported
> > config is on the extreme side compiling with 8k NR_CPUS, so 1kb. I'm
> > just not in love with the idea of adding a patch to improve readability
> > and it cost idle memory to resolve a compile time warning.
> >
> > If no one else chimes in in the next few days, I'll probably just apply
> > it and go from there. If another issue comes up I'll drop this and tag
> > it as __refdata.
>
> I've come across this one again in linux-next today, and found that
> I had an old patch for it already, that I had never submitted:
>
> From 7d6f40414490092b86f1a64d8c42426ee350da1a Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann
> Date: Mon, 7 Dec 2020 23:24:20 +0100
> Subject: [PATCH] mm: percpu: fix section mismatch warning
>
> Building with arm64 clang sometimes (fairly rarely) shows a
> warning about the pcpu_build_alloc_info() function:
>
> WARNING: modpost: vmlinux.o(.text+0x21697c): Section mismatch in
> reference from the function cpumask_clear_cpu() to the variable
> .init.data:pcpu_build_alloc_info.mask
> The function cpumask_clear_cpu() references
> the variable __initdata pcpu_build_alloc_info.mask.
> This is often because cpumask_clear_cpu lacks a __initdata
> annotation or the annotation of pcpu_build_alloc_info.mask is wrong.
>
> What appears to be going on here is that the compiler decides to not
> inline the cpumask_clear_cpu() function that is marked 'inline' but not
> 'always_inline', and it then produces a specialized version of it that
> references the static mask unconditionally as an optimization.
>
> Marking cpumask_clear_cpu() as __always_inline would fix it, as would
> removing the __initdata annotation on the variable. I went for marking
> the function as __attribute__((flatten)) instead because all functions
> called from it are really meant to be inlined here, and it prevents
> the same problem happening here again. This is unlikely to be a problem
> elsewhere because there are very few function-local static __initdata
> variables in the kernel.
>
> Fixes: 6c207504ae79 ("percpu: reduce the number of cpu distance comparisons")
> Signed-off-by: Arnd Bergmann
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 5ede8dd407d5..527181c46b08 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -2662,10 +2662,9 @@ early_param("percpu_alloc", percpu_alloc_setup);
> * On success, pointer to the new allocation_info is returned. On
> * failure, ERR_PTR value is returned.
> */
> -static struct pcpu_alloc_info * __init pcpu_build_alloc_info(
> - size_t reserved_size, size_t dyn_size,
> - size_t atom_size,
> - pcpu_fc_cpu_distance_fn_t cpu_distance_fn)
> +static struct pcpu_alloc_info * __init __attribute__((flatten))
> +pcpu_build_alloc_info(size_t reserved_size, size_t dyn_size, size_t
> atom_size,
> + pcpu_fc_cpu_distance_fn_t cpu_distance_fn)
> {
> static int group_map[NR_CPUS] __initdata;
> static int group_cnt[NR_CPUS] __initdata;
>
>
> Not sure if this would be any better than your patch.
>
>Arnd
Hi Arnd,
I like this solution a lot more than my previous solution because this
is a lot less fragile.
Thanks,
Dennis
Re: [PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info()
On Mon, Jan 04, 2021 at 04:46:51PM -0700, Nathan Chancellor wrote:
> On Thu, Dec 31, 2020 at 09:28:52PM +0000, Dennis Zhou wrote:
> > This is an unusual situation so I thought it best to explain it in a
> > separate patch.
> >
> > "percpu: reduce the number of cpu distance comparisons" introduces a
> > dependency on cpumask helper functions in __init code. This code
> > references a struct cpumask annotated __initdata. When the function is
> > inlined (gcc), everything is fine, but clang decides not to inline these
> > function calls. This causes modpost to warn about an __initdata access
> > by a function not annotated with __init [1].
> >
> > Ways I thought about fixing it:
> > 1. figure out why clang thinks this inlining is too costly.
> > 2. create a wrapper function annotated __init (this).
> > 3. annotate cpumask with __refdata.
> >
> > Ultimately it comes down to if it's worth saving the cpumask memory and
> > allowing it to be freed. IIUC, __refdata won't be freed, so option 3 is
> > just a little wasteful. 1 is out of my depth, leaving 2. I don't feel
> > great about this behavior being dependent on inlining semantics, but
> > cpumask helpers are small and probably should be inlined.
> >
> > modpost complaint:
> > WARNING: modpost: vmlinux.o(.text+0x735425): Section mismatch in
> > reference from the function cpumask_clear_cpu() to the variable
> > .init.data:pcpu_build_alloc_info.mask
> > The function cpumask_clear_cpu() references
> > the variable __initdata pcpu_build_alloc_info.mask.
> > This is often because cpumask_clear_cpu lacks a __initdata
> > annotation or the annotation of pcpu_build_alloc_info.mask is wrong.
> >
> > clang output:
> > mm/percpu.c:2724:5: remark: cpumask_clear_cpu not inlined into
> > pcpu_build_alloc_info because too costly to inline (cost=725,
> > threshold=325) [-Rpass-missed=inline]
> >
> > [1] https://lore.kernel.org/linux-mm/202012220454.9f6bkz9q-...@intel.com/
> >
> > Reported-by: kernel test robot
> > Signed-off-by: Dennis Zhou
> > ---
> > This is on top of percpu#for-5.12.
> >
> > mm/percpu.c | 16 ++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index 80f8f885a990..357977c4cb00 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -2642,6 +2642,18 @@ early_param("percpu_alloc", percpu_alloc_setup);
> >
> > /* pcpu_build_alloc_info() is used by both embed and page first chunk */
> > #if defined(BUILD_EMBED_FIRST_CHUNK) || defined(BUILD_PAGE_FIRST_CHUNK)
> > +
> > +/*
> > + * This wrapper is to avoid a warning where cpumask_clear_cpu() is not
> > inlined
> > + * when compiling with clang causing modpost to warn about accessing
> > __initdata
> > + * from a non __init function. By doing this, we allow the struct cpumask
> > to be
> > + * freed instead of it taking space by annotating with __refdata.
> > + */
> > +static void __init pcpu_cpumask_clear_cpu(int cpu, struct cpumask *mask)
> > +{
> > + cpumask_clear_cpu(cpu, mask);
> > +}
> > +
> > /**
> > * pcpu_build_alloc_info - build alloc_info considering distances between
> > CPUs
> > * @reserved_size: the size of reserved percpu area in bytes
> > @@ -2713,7 +2725,7 @@ static struct pcpu_alloc_info * __init
> > pcpu_build_alloc_info(
> > cpu = cpumask_first(&mask);
> > group_map[cpu] = group;
> > group_cnt[group]++;
> > - cpumask_clear_cpu(cpu, &mask);
> > + pcpu_cpumask_clear_cpu(cpu, &mask);
> >
> > for_each_cpu(tcpu, &mask) {
> > if (!cpu_distance_fn ||
> > @@ -2721,7 +2733,7 @@ static struct pcpu_alloc_info * __init
> > pcpu_build_alloc_info(
> > cpu_distance_fn(tcpu, cpu) == LOCAL_DISTANCE)) {
> > group_map[tcpu] = group;
> > group_cnt[group]++;
> > - cpumask_clear_cpu(tcpu, &mask);
> > + pcpu_cpumask_clear_cpu(tcpu, &mask);
> > }
> > }
> > }
> > --
> > 2.29.2.729.g45daf8777d-goog
> >
Hi Nathan,
>
> Hi Dennis,
>
> I did a bisect of the problematic config against defconfig and it points
> out that CONFIG_GCOV_PROFILE_ALL is in the bad config but not the good
> config, which makes some sen
[PATCH] percpu: fix clang modpost warning in pcpu_build_alloc_info()
This is an unusual situation so I thought it best to explain it in a
separate patch.
"percpu: reduce the number of cpu distance comparisons" introduces a
dependency on cpumask helper functions in __init code. This code
references a struct cpumask annotated __initdata. When the function is
inlined (gcc), everything is fine, but clang decides not to inline these
function calls. This causes modpost to warn about an __initdata access
by a function not annotated with __init [1].
Ways I thought about fixing it:
1. figure out why clang thinks this inlining is too costly.
2. create a wrapper function annotated __init (this).
3. annotate cpumask with __refdata.
Ultimately it comes down to if it's worth saving the cpumask memory and
allowing it to be freed. IIUC, __refdata won't be freed, so option 3 is
just a little wasteful. 1 is out of my depth, leaving 2. I don't feel
great about this behavior being dependent on inlining semantics, but
cpumask helpers are small and probably should be inlined.
modpost complaint:
WARNING: modpost: vmlinux.o(.text+0x735425): Section mismatch in reference
from the function cpumask_clear_cpu() to the variable
.init.data:pcpu_build_alloc_info.mask
The function cpumask_clear_cpu() references
the variable __initdata pcpu_build_alloc_info.mask.
This is often because cpumask_clear_cpu lacks a __initdata
annotation or the annotation of pcpu_build_alloc_info.mask is wrong.
clang output:
mm/percpu.c:2724:5: remark: cpumask_clear_cpu not inlined into
pcpu_build_alloc_info because too costly to inline (cost=725, threshold=325)
[-Rpass-missed=inline]
[1] https://lore.kernel.org/linux-mm/202012220454.9f6bkz9q-...@intel.com/
Reported-by: kernel test robot
Signed-off-by: Dennis Zhou
---
This is on top of percpu#for-5.12.
mm/percpu.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 80f8f885a990..357977c4cb00 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -2642,6 +2642,18 @@ early_param("percpu_alloc", percpu_alloc_setup);
/* pcpu_build_alloc_info() is used by both embed and page first chunk */
#if defined(BUILD_EMBED_FIRST_CHUNK) || defined(BUILD_PAGE_FIRST_CHUNK)
+
+/*
+ * This wrapper is to avoid a warning where cpumask_clear_cpu() is not inlined
+ * when compiling with clang causing modpost to warn about accessing __initdata
+ * from a non __init function. By doing this, we allow the struct cpumask to
be
+ * freed instead of it taking space by annotating with __refdata.
+ */
+static void __init pcpu_cpumask_clear_cpu(int cpu, struct cpumask *mask)
+{
+ cpumask_clear_cpu(cpu, mask);
+}
+
/**
* pcpu_build_alloc_info - build alloc_info considering distances between CPUs
* @reserved_size: the size of reserved percpu area in bytes
@@ -2713,7 +2725,7 @@ static struct pcpu_alloc_info * __init
pcpu_build_alloc_info(
cpu = cpumask_first(&mask);
group_map[cpu] = group;
group_cnt[group]++;
- cpumask_clear_cpu(cpu, &mask);
+ pcpu_cpumask_clear_cpu(cpu, &mask);
for_each_cpu(tcpu, &mask) {
if (!cpu_distance_fn ||
@@ -2721,7 +2733,7 @@ static struct pcpu_alloc_info * __init
pcpu_build_alloc_info(
cpu_distance_fn(tcpu, cpu) == LOCAL_DISTANCE)) {
group_map[tcpu] = group;
group_cnt[group]++;
- cpumask_clear_cpu(tcpu, &mask);
+ pcpu_cpumask_clear_cpu(tcpu, &mask);
}
}
}
--
2.29.2.729.g45daf8777d-goog
[PATCH] staging/vc04_services/bcm2835-audio: Parenthesize alsa2chip macro
Add parenthesis around the alsa2chip macro to remove a checkpatch error.
Signed-off-by: Dennis Skovborg Jørgensen
---
drivers/staging/vc04_services/bcm2835-audio/bcm2835.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
index 1b36475872d6..51066ac8eea5 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
@@ -22,7 +22,7 @@ enum {
/* macros for alsa2chip and chip2alsa, instead of functions */
// convert alsa to chip volume (defined as macro rather than function call)
-#define alsa2chip(vol) (uint)(-(((vol) << 8) / 100))
+#define alsa2chip(vol) ((uint)(-(((vol) << 8) / 100)))
// convert chip to alsa volume
#define chip2alsa(vol) -(((vol) * 100) >> 8)
--
2.29.2
[GIT PULL] percpu fix for v5.10-rc4
Hi Linus, A fix for a Wshadow warning in asm-generic percpu macros came in and then I tacked on the removal of flexible array initializers in the percpu allocator which was discussed in the 5.9 pull request. Thanks, Dennis The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git for-5.10-fixes for you to fetch changes up to 61cf93d3e14a29288e4d5522aecb6e58268eec62: percpu: convert flexible array initializers to use struct_size() (2020-10-30 23:02:28 +) Arnd Bergmann (1): asm-generic: percpu: avoid Wshadow warning Dennis Zhou (1): percpu: convert flexible array initializers to use struct_size() include/asm-generic/percpu.h | 18 +- mm/percpu.c | 8 2 files changed, 13 insertions(+), 13 deletions(-)
[PATCH] percpu: convert flexible array initializers to use struct_size()
Use the safer macro as sparked by the long discussion in [1].
[1] https://lore.kernel.org/lkml/20200917204514.ga2880...@google.com/
Signed-off-by: Dennis Zhou
---
I'll apply it to for-5.10-fixes.
mm/percpu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 66a93f096394..ad7a37ee74ef 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1315,8 +1315,8 @@ static struct pcpu_chunk * __init
pcpu_alloc_first_chunk(unsigned long tmp_addr,
region_size = ALIGN(start_offset + map_size, lcm_align);
/* allocate chunk */
- alloc_size = sizeof(struct pcpu_chunk) +
- BITS_TO_LONGS(region_size >> PAGE_SHIFT) * sizeof(unsigned
long);
+ alloc_size = struct_size(chunk, populated,
+BITS_TO_LONGS(region_size >> PAGE_SHIFT));
chunk = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
if (!chunk)
panic("%s: Failed to allocate %zu bytes\n", __func__,
@@ -2521,8 +2521,8 @@ void __init pcpu_setup_first_chunk(const struct
pcpu_alloc_info *ai,
pcpu_unit_pages = ai->unit_size >> PAGE_SHIFT;
pcpu_unit_size = pcpu_unit_pages << PAGE_SHIFT;
pcpu_atom_size = ai->atom_size;
- pcpu_chunk_struct_size = sizeof(struct pcpu_chunk) +
- BITS_TO_LONGS(pcpu_unit_pages) * sizeof(unsigned long);
+ pcpu_chunk_struct_size = struct_size(chunk, populated,
+BITS_TO_LONGS(pcpu_unit_pages));
pcpu_stats_save_ai(ai);
--
2.29.1.341.ge80a0c044ae-goog
Re: [PATCH] asm-generic: percpu: avoid Wshadow warning
Hello,
On Mon, Oct 26, 2020 at 04:53:48PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Nesting macros that use the same local variable names causes
> warnings when building with "make W=2":
>
> include/asm-generic/percpu.h:117:14: warning: declaration of '__ret' shadows
> a previous local [-Wshadow]
> include/asm-generic/percpu.h:126:14: warning: declaration of '__ret' shadows
> a previous local [-Wshadow]
>
> These are fairly harmless, but since the warning comes from
> a global header, the warning happens every time the headers
> are included, which is fairly annoying.
>
> Rename the variables to avoid shadowing and shut up the warning.
>
> Signed-off-by: Arnd Bergmann
> ---
> include/asm-generic/percpu.h | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
> index 35e4a53b83e6..6432a7fade91 100644
> --- a/include/asm-generic/percpu.h
> +++ b/include/asm-generic/percpu.h
> @@ -114,21 +114,21 @@ do {
> \
>
> #define __this_cpu_generic_read_nopreempt(pcp)
> \
> ({ \
> - typeof(pcp) __ret; \
> + typeof(pcp) ___ret; \
> preempt_disable_notrace(); \
> - __ret = READ_ONCE(*raw_cpu_ptr(&(pcp)));\
> + ___ret = READ_ONCE(*raw_cpu_ptr(&(pcp))); \
> preempt_enable_notrace(); \
> - __ret; \
> + ___ret; \
> })
>
> #define __this_cpu_generic_read_noirq(pcp) \
> ({ \
> - typeof(pcp) __ret; \
> - unsigned long __flags; \
> - raw_local_irq_save(__flags);\
> - __ret = raw_cpu_generic_read(pcp); \
> - raw_local_irq_restore(__flags); \
> - __ret; \
> + typeof(pcp) ___ret; \
> + unsigned long ___flags; \
> + raw_local_irq_save(___flags); \
> + ___ret = raw_cpu_generic_read(pcp); \
> + raw_local_irq_restore(___flags);\
> + ___ret; \
> })
>
> #define this_cpu_generic_read(pcp) \
> --
> 2.27.0
>
I've applied this to percpu#for-5.10-fixes.
Thanks,
Dennis
Greetings
I am Mr.Dennis, i work as an accountant in a bank, i am contacting you in regards to a business transfer of a huge sum of money from a deceased account. I need your urgent assistance in transferring the sum of $11.6million dollars to your private bank account,the fund belongs to one of our foreign customer who died along with his entire family since July 22, 2003.The money has been here in our Bank lying dormant for years now without anybody coming for the claim of it. I now seek your permission to have you stand in as the deceased relative to enable the funds be released in your favor as the beneficiary and the next of kin to our deceased customer,the Banking laws here does not allow such money to stay more than 19years,because the money will be recalled to the Bank treasury account as unclaimed fund.I am ready to share with you 40% for you and 60% will be kept for me, by indicating your interest i will send you the full details on how the business will be executed.
Re: [PATCH] IB/hfi1: Avoid allocing memory on memoryless numa node
On 10/16/2020 10:11 AM, Jason Gunthorpe wrote: On Mon, Oct 12, 2020 at 08:36:57AM -0400, Dennis Dalessandro wrote: On 10/10/2020 4:57 AM, Xianting Tian wrote: In architecture like powerpc, we can have cpus without any local memory attached to it. In such cases the node does not have real memory. Use local_memory_node(), which is guaranteed to have memory. local_memory_node is a noop in other architectures that does not support memoryless nodes. Signed-off-by: Xianting Tian drivers/infiniband/hw/hfi1/file_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index 8ca51e43c..79fa22cc7 100644 +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -965,7 +965,7 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd, */ fd->rec_cpu_num = hfi1_get_proc_affinity(dd->node); if (fd->rec_cpu_num != -1) - numa = cpu_to_node(fd->rec_cpu_num); + numa = local_memory_node(cpu_to_node(fd->rec_cpu_num)); else numa = numa_node_id(); ret = hfi1_create_ctxtdata(dd->pport, numa, &uctxt); The hfi1 driver depends on X86_64. I'm not sure what this patch buys, can you expand a bit? Yikes, that is strongly discouraged. Hmm. This was never raised as an issue before. Regardless I can't recall why we did this in the first place. I'll do some digging, try to jog my memory. -Denny
Re: [PATCH] IB/hfi1: Avoid allocing memory on memoryless numa node
On 10/10/2020 4:57 AM, Xianting Tian wrote: In architecture like powerpc, we can have cpus without any local memory attached to it. In such cases the node does not have real memory. Use local_memory_node(), which is guaranteed to have memory. local_memory_node is a noop in other architectures that does not support memoryless nodes. Signed-off-by: Xianting Tian --- drivers/infiniband/hw/hfi1/file_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index 8ca51e43c..79fa22cc7 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -965,7 +965,7 @@ static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd, */ fd->rec_cpu_num = hfi1_get_proc_affinity(dd->node); if (fd->rec_cpu_num != -1) - numa = cpu_to_node(fd->rec_cpu_num); + numa = local_memory_node(cpu_to_node(fd->rec_cpu_num)); else numa = numa_node_id(); ret = hfi1_create_ctxtdata(dd->pport, numa, &uctxt); The hfi1 driver depends on X86_64. I'm not sure what this patch buys, can you expand a bit? -Denny
Re: [PATCH] IB/rdmavt: Fix sizeof mismatch
On 2020-10-08 05:52, Colin King wrote:
From: Colin Ian King
An incorrect sizeof is being used, struct rvt_ibport ** is not correct,
it should be struct rvt_ibport *. Note that since ** is the same size
as
* this is not causing any issues. Improve this fix by using
sizeof(*rdi->ports) as this allows us to not even reference the type
of the pointer. Also remove line breaks as the entire statement can
fit on one line.
Addresses-Coverity: ("Sizeof not portable (SIZEOF_MISMATCH)")
Fixes: ff6acd69518e ("IB/rdmavt: Add device structure allocation")
Signed-off-by: Colin Ian King
---
drivers/infiniband/sw/rdmavt/vt.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/infiniband/sw/rdmavt/vt.c
b/drivers/infiniband/sw/rdmavt/vt.c
index f904bb34477a..2d534c450f3c 100644
--- a/drivers/infiniband/sw/rdmavt/vt.c
+++ b/drivers/infiniband/sw/rdmavt/vt.c
@@ -95,9 +95,7 @@ struct rvt_dev_info *rvt_alloc_device(size_t size,
int nports)
if (!rdi)
return rdi;
-rdi->ports = kcalloc(nports,
- sizeof(struct rvt_ibport **),
- GFP_KERNEL);
+rdi->ports = kcalloc(nports, sizeof(*rdi->ports), GFP_KERNEL);
if (!rdi->ports)
ib_dealloc_device(&rdi->ibdev);
Acked-by: Dennis Dalessandro
Re: [PATCH v3 8/9] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Hi Matthias,
On Mon, 2020-09-21 at 18:19 +0200, Matthias Brugger wrote:
>
> On 07/07/2020 17:45, Dennis YC Hsieh wrote:
> > Add clear parameter to let client decide if
> > event should be clear to 0 after GCE receive it.
> >
> > Change since v2:
> > - Keep behavior in drm crtc driver and
> >separate bug fix code into another patch.
>
> This, should go...
>
> >
> > Signed-off-by: Dennis YC Hsieh
> > ---
>
> ...here :)
>
> I fixed to commit message and pushed the patch to v5.9-next/soc
got it, thanks a lot
Regards,
Dennis
>
> Thanks!
>
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
> > drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
> > include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
> > include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
> > 4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index ec6c9ffbf35e..c84e7a14d4a8 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
> > *mtk_crtc)
> > mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> > cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index d55dc3296105..505651b0d715 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
> >
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
> > {
> > struct cmdq_instruction inst = { {0} };
> > + u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
> >
> > if (event >= CMDQ_MAX_EVENT)
> > return -EINVAL;
> >
> > inst.op = CMDQ_CODE_WFE;
> > - inst.value = CMDQ_WFE_OPTION;
> > + inst.value = CMDQ_WFE_OPTION | clear_option;
> > inst.event = event;
> >
> > return cmdq_pkt_append_command(pkt, inst);
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index efbd8a9eb2d1..d5a983d65f05 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -28,8 +28,7 @@
> >* bit 16-27: update value
> >* bit 31: 1 - update, 0 - no update
> >*/
> > -#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
> > CMDQ_WFE_WAIT | \
> > - CMDQ_WFE_WAIT_VALUE)
> > +#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
> > CMDQ_WFE_WAIT_VALUE)
> >
> > /** cmdq event maximum */
> > #define CMDQ_MAX_EVENT0x3ff
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 34354e952f60..960704d75994 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > /**
> >* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> >* @pkt: the CMDQ packet
> > - * @event: the desired event type to "wait and CLEAR"
> > + * @event: the desired event type to wait
> > + * @clear: clear event or not after event arrive
> >*
> >* Return: 0 for success; else the error code is returned
> >*/
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> >
> > /**
> >* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> >
[GIT PULL] percpu fix for v5.9-rc6
Hi Linus,
This is a fix for the first chunk size calculation where the variable
length array incorrectly used # of longs instead of bytes of longs. This
came in as a code fix and not a bug report, so I don't think it was
widely problematic. I believe it worked out due to it being memblock
memory and alignment requirements working in our favor.
Thanks,
Dennis
The following changes since commit f75aef392f869018f78cfedf3c320a6b3fcfda6b:
Linux 5.9-rc3 (2020-08-30 16:01:54 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git for-5.9-fixes
for you to fetch changes up to b3b33d3c43bbe0177d70653f4e889c78cc37f097:
percpu: fix first chunk size calculation for populated bitmap (2020-09-17
17:34:39 +)
Sunghyun Jin (1):
percpu: fix first chunk size calculation for populated bitmap
mm/percpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index f4709629e6de..1ed1a349eab8 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1316,7 +1316,7 @@ static struct pcpu_chunk * __init
pcpu_alloc_first_chunk(unsigned long tmp_addr,
/* allocate chunk */
alloc_size = sizeof(struct pcpu_chunk) +
- BITS_TO_LONGS(region_size >> PAGE_SHIFT);
+ BITS_TO_LONGS(region_size >> PAGE_SHIFT) * sizeof(unsigned
long);
chunk = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
if (!chunk)
panic("%s: Failed to allocate %zu bytes\n", __func__,
RE: [PATCH] drm/amd/display: remove unintended executable mode
[AMD Official Use Only - Internal Distribution Only]
Hi, Lukas,
Thanks for your fix. This issue was caused by that I modified these files
in windows system with Samba. I will take care in the future.
Best Regards
Dennis Li
-Original Message-
From: Lukas Bulwahn
Sent: Wednesday, August 19, 2020 4:18 PM
To: Deucher, Alexander ; Koenig, Christian
; Li, Dennis ; Zuo, Jerry
Cc: amd-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
linux-kernel@vger.kernel.org; Chen, Guchun ; Wu, Hersen
; Lukas Bulwahn
Subject: [PATCH] drm/amd/display: remove unintended executable mode
Besides the intended change, commit 4cc1178e166a ("drm/amdgpu: replace DRM
prefix with PCI device info for gfx/mmhub") also set the source files
mmhub_v1_0.c and gfx_v9_4.c to be executable, i.e., changed fromold mode
644 to new mode 755.
Commit 241b2ec9317e ("drm/amd/display: Add dcn30 Headers (v2)") added the four
header files {dpcs,dcn}_3_0_0_{offset,sh_mask}.h as executable, i.e., mode 755.
Set to the usual modes for source and headers files and clean up those
mistakes. No functional change.
Signed-off-by: Lukas Bulwahn
---
applies cleanly on current master and next-20200819
Alex, Christian, please pick this minor non-urgent cleanup patch.
Dennis, Jerry, please ack.
Dennis, Jerry, you might want to check your development environment introducing
those executable modes on files.
drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c | 0
drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 0
drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_offset.h | 0
drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_sh_mask.h | 0
drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_offset.h | 0
drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_sh_mask.h | 0
6 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644
drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
mode change 100755 => 100644 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
mode change 100755 => 100644
drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_offset.h
mode change 100755 => 100644
drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_sh_mask.h
mode change 100755 => 100644
drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_offset.h
mode change 100755 => 100644
drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_sh_mask.h
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
old mode 100755
new mode 100644
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
old mode 100755
new mode 100644
diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_offset.h
b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_offset.h
old mode 100755
new mode 100644
diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_sh_mask.h
b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_3_0_0_sh_mask.h
old mode 100755
new mode 100644
diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_offset.h
b/drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_offset.h
old mode 100755
new mode 100644
diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_sh_mask.h
b/drivers/gpu/drm/amd/include/asic_reg/dcn/dpcs_3_0_0_sh_mask.h
old mode 100755
new mode 100644
--
2.17.1
Good morning
-- Hello. Greetings dd you received my previous email?
Re: [PATCH v3 00/11] i386 Clang support
On Thu, Jul 23, 2020 at 01:08:42AM +0200, Thomas Gleixner wrote: > Dennis Zhou writes: > > On Mon, Jul 20, 2020 at 01:49:14PM -0700, Nick Desaulniers wrote: > >> Resend of Brian's v2 with Acks from Peter and Linus collected, as well > >> as the final patch (mine) added. The commit of the final patch discusses > >> some of the architectural differences between GCC and Clang, and the > >> kernels tickling of this difference for i386, which necessitated these > >> patches. > >> > >> Brian Gerst (10): > >> x86/percpu: Introduce size abstraction macros > >> x86/percpu: Clean up percpu_to_op() > >> x86/percpu: Clean up percpu_from_op() > >> x86/percpu: Clean up percpu_add_op() > >> x86/percpu: Remove "e" constraint from XADD > >> x86/percpu: Clean up percpu_add_return_op() > >> x86/percpu: Clean up percpu_xchg_op() > >> x86/percpu: Clean up percpu_cmpxchg_op() > >> x86/percpu: Clean up percpu_stable_op() > >> x86/percpu: Remove unused PER_CPU() macro > >> > >> Nick Desaulniers (1): > >> x86: support i386 with Clang > >> > >> arch/x86/include/asm/percpu.h | 510 +++-- > >> arch/x86/include/asm/uaccess.h | 4 +- > >> 2 files changed, 175 insertions(+), 339 deletions(-) > >> > >> -- > >> 2.28.0.rc0.105.gf9edc3c819-goog > >> > > > > This looks great to me! I applied it to for-5.9. > > You applied it? I'm not aware that you're maintaining x86 nowadays. > > Thanks, > > tglx I'm sorry I overstepped. I've dropped them. Please take them with my ack. Thanks, Dennis
Re: [PATCH v3 00/11] i386 Clang support
On Mon, Jul 20, 2020 at 01:49:14PM -0700, Nick Desaulniers wrote: > Resend of Brian's v2 with Acks from Peter and Linus collected, as well > as the final patch (mine) added. The commit of the final patch discusses > some of the architectural differences between GCC and Clang, and the > kernels tickling of this difference for i386, which necessitated these > patches. > > Brian Gerst (10): > x86/percpu: Introduce size abstraction macros > x86/percpu: Clean up percpu_to_op() > x86/percpu: Clean up percpu_from_op() > x86/percpu: Clean up percpu_add_op() > x86/percpu: Remove "e" constraint from XADD > x86/percpu: Clean up percpu_add_return_op() > x86/percpu: Clean up percpu_xchg_op() > x86/percpu: Clean up percpu_cmpxchg_op() > x86/percpu: Clean up percpu_stable_op() > x86/percpu: Remove unused PER_CPU() macro > > Nick Desaulniers (1): > x86: support i386 with Clang > > arch/x86/include/asm/percpu.h | 510 +++-- > arch/x86/include/asm/uaccess.h | 4 +- > 2 files changed, 175 insertions(+), 339 deletions(-) > > -- > 2.28.0.rc0.105.gf9edc3c819-goog > This looks great to me! I applied it to for-5.9. Thanks, Dennis
Greetings
I am Mr.Dennis ,i work as an accountant in a bank.I am contacting you independently of my investigation in my bank,i need your urgent assistance in transferring the sum of $11.6million dollars to your private bank account,the fund belongs to one of our foreign customer who died a longtime with his supposed next of kin since July 22, 2003.The money has been here in our Bank lying dormant for years now without anybody coming for the claim of it. I want the bank to release the money to you as the relative and the next of kin to our deceased customer,the Banking laws here does not allow such money to stay more than 19years,because the money will be recalled to the Bank treasury account as unclaimed fund.I am ready to share with you 40% for you and 60% will be kept for me, by indicating your interest i will send you the full details on how the business will be executed.
[PATCH v3 7/9] soc: mediatek: cmdq: add jump function
Add jump function so that client can jump to any address which
contains instruction.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 13 +
include/linux/soc/mediatek/mtk-cmdq.h | 11 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index b6e25f216605..d55dc3296105 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -13,6 +13,7 @@
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
#define CMDQ_REG_TYPE 1
+#define CMDQ_JUMP_RELATIVE 1
struct cmdq_instruction {
union {
@@ -407,6 +408,18 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_JUMP;
+ inst.offset = CMDQ_JUMP_RELATIVE;
+ inst.value = addr >>
+ cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan);
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_jump);
+
int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index d9390d76ee14..34354e952f60 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -253,6 +253,17 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
/**
+ * cmdq_pkt_jump() - Append jump command to the CMDQ packet, ask GCE
+ * to execute an instruction that change current thread PC to
+ * a physical address which should contains more instruction.
+ * @pkt:the CMDQ packet
+ * @addr: physical address of target instruction buffer
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr);
+
+/**
* cmdq_pkt_finalize() - Append EOC and jump command to pkt.
* @pkt: the CMDQ packet
*
--
1.7.9.5
[PATCH v3 5/9] soc: mediatek: cmdq: add write_s value function
add write_s function in cmdq helper functions which
writes a constant value to address with large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 14 ++
include/linux/soc/mediatek/mtk-cmdq.h | 13 +
2 files changed, 27 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ed9f5e63c195..4e86b65815fc 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -280,6 +280,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index cd7ec714344e..ae73e10da274 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,19 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx, u32 mask);
/**
+ * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which
+ * write value to a physical address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v3 9/9] drm/mediatek: reduce clear event
No need to clear event again since event always clear before wait.
This fix depend on patch:
"soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api"
Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow")
Signed-off-by: Dennis YC Hsieh
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index c84e7a14d4a8..ba6cf956b239 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
- cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
+ cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
--
1.7.9.5
[PATCH v3 2/9] soc: mediatek: cmdq: add write_s function
add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
3 files changed, 39 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9faf78fbed3a..880349b3f16c 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -18,6 +18,10 @@ struct cmdq_instruction {
union {
u32 value;
u32 mask;
+ struct {
+ u16 arg_c;
+ u16 src_reg;
+ };
};
union {
u16 offset;
@@ -223,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 05eea1aef5aa..1f76cfedb16d 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 2249ecaf77e4..9b0c57a0063d 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -12,6 +12,8 @@
#include
#define CMDQ_NO_TIMEOUT0xu
+#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
struct cmdq_pkt;
@@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
/**
+ * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v3 1/9] soc: mediatek: cmdq: add address shift in jump
Add address shift when compose jump instruction to compatible with 35bit format. Change since v1: - Rename cmdq_mbox_shift() to cmdq_get_shift_pa(). Signed-off-by: Dennis YC Hsieh --- drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index dc644cfb6419..9faf78fbed3a 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -329,7 +329,8 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) /* JUMP to end */ inst.op = CMDQ_CODE_JUMP; - inst.value = CMDQ_JUMP_PASS; + inst.value = CMDQ_JUMP_PASS >> + cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan); err = cmdq_pkt_append_command(pkt, inst); return err; -- 1.7.9.5
[PATCH v3 4/9] soc: mediatek: cmdq: add read_s function
Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 12
3 files changed, 28 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 550e9e7e3ff2..ed9f5e63c195 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -227,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_READ_S;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.reg_dst = reg_idx;
+ inst.src_reg = addr_low;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 90d1d8e64412..efbd8a9eb2d1 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 53230341bf94..cd7ec714344e 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -104,6 +104,18 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev,
int index,
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/*
+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @reg_idx: the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx);
+
/**
* cmdq_pkt_write_s() - append write_s command to the CMDQ packet
* @pkt: the CMDQ packet
--
1.7.9.5
[PATCH v3 3/9] soc: mediatek: cmdq: add write_s_mask function
add write_s_mask function in cmdq helper functions which
writes value contains in internal register to address
with mask and large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 18 ++
3 files changed, 42 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 880349b3f16c..550e9e7e3ff2 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -242,6 +242,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s);
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.mask = 0;
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 1f76cfedb16d..90d1d8e64412 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -61,6 +61,7 @@ enum cmdq_code {
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
CMDQ_CODE_WRITE_S = 0x90,
+ CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 9b0c57a0063d..53230341bf94 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -122,6 +122,24 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx);
/**
+ * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ
packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v3 0/9] support cmdq helper function on mt6779 platform
This patch support more gce helper function on mt6779 platform. depends on patch: support gce on mt6779 platform and depends on following applied patches soc: mediatek: cmdq: add set event function soc: mediatek: cmdq: export finalize function soc: mediatek: cmdq: add assign function Change since v2: - Keep behavior in drm crtc driver and separate bug fix code into another patch. Change since v1: - Rename cmdq_mbox_shift() to cmdq_get_shift_pa(). Dennis YC Hsieh (9): soc: mediatek: cmdq: add address shift in jump soc: mediatek: cmdq: add write_s function soc: mediatek: cmdq: add write_s_mask function soc: mediatek: cmdq: add read_s function soc: mediatek: cmdq: add write_s value function soc: mediatek: cmdq: add write_s_mask value function soc: mediatek: cmdq: add jump function soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api drm/mediatek: reduce clear event drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- drivers/soc/mediatek/mtk-cmdq-helper.c | 113 ++- include/linux/mailbox/mtk-cmdq-mailbox.h | 6 +- include/linux/soc/mediatek/mtk-cmdq.h| 93 ++- 4 files changed, 206 insertions(+), 8 deletions(-) -- 2.18.0
[PATCH v3 6/9] soc: mediatek: cmdq: add write_s_mask value function
add write_s_mask_value function in cmdq helper functions which
writes a constant value to address with mask and large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 21 +
include/linux/soc/mediatek/mtk-cmdq.h | 15 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 4e86b65815fc..b6e25f216605 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -294,6 +294,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index ae73e10da274..d9390d76ee14 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -165,6 +165,21 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
u16 addr_low, u32 value);
/**
+ * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ
+ *packet which write value to a physical
+ *address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ * @mask: the specified target mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v3 8/9] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Add clear parameter to let client decide if
event should be clear to 0 after GCE receive it.
Change since v2:
- Keep behavior in drm crtc driver and
separate bug fix code into another patch.
Signed-off-by: Dennis YC Hsieh
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index ec6c9ffbf35e..c84e7a14d4a8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
- cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
+ cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index d55dc3296105..505651b0d715 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
{
struct cmdq_instruction inst = { {0} };
+ u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
inst.op = CMDQ_CODE_WFE;
- inst.value = CMDQ_WFE_OPTION;
+ inst.value = CMDQ_WFE_OPTION | clear_option;
inst.event = event;
return cmdq_pkt_append_command(pkt, inst);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index efbd8a9eb2d1..d5a983d65f05 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -28,8 +28,7 @@
* bit 16-27: update value
* bit 31: 1 - update, 0 - no update
*/
-#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
CMDQ_WFE_WAIT | \
- CMDQ_WFE_WAIT_VALUE)
+#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
CMDQ_WFE_WAIT_VALUE)
/** cmdq event maximum */
#define CMDQ_MAX_EVENT 0x3ff
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 34354e952f60..960704d75994 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
- * @event: the desired event type to "wait and CLEAR"
+ * @event: the desired event type to wait
+ * @clear: clear event or not after event arrive
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
--
1.7.9.5
Re: [mm] 4e2c82a409: ltp.overcommit_memory01.fail
ndler(struct ctl_table *table, int write, void
> *buffer,
> size_t *lenp, loff_t *ppos)
> {
> int ret;
>
> ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> - if (ret == 0 && write)
> + if (ret == 0 && write) {
> + if (sysctl_overcommit_memory == OVERCOMMIT_NEVER)
> + schedule_on_each_cpu(sync_overcommit_as);
> +
> mm_compute_batch();
> + }
>
> return ret;
> }
> --
> 2.7.4
>
>
> On Sun, Jul 05, 2020 at 10:36:14PM -0400, Qian Cai wrote:
> > > In my last email, I was not saying OVERCOMMIT_NEVER is not a normal case,
> > > but I don't think user will too frequently runtime change the overcommit
> > > policy. And the fix patch of syncing 'vm_committed_as' is only called when
> > > user calls 'sysctl -w vm.overcommit_memory=2'.
> > >
> > > > The question is now if any of those regression fixes would now regress
> > > > performance of OVERCOMMIT_NEVER workloads or just in-par with the data
> > > > before the patchset?
> > >
> > > For the original patchset, it keeps vm_committed_as unchanged for
> > > OVERCOMMIT_NEVER policy and enlarge it for the other 2 loose policies
> > > OVERCOMMIT_ALWAYS and OVERCOMMIT_GUESS, and I don't expect the
> > > "OVERCOMMIT_NEVER
> > > workloads" performance will be impacted. If you have suggetions for this
> > > kind of benchmarks, I can test them to better verify the patchset, thanks!
> >
> > Then, please capture those information into a proper commit log when you
> > submit the regression fix on top of the patchset, and CC PER-CPU MEMORY
> > ALLOCATOR maintainers, so they might be able to review it properly.
>
>
>
Thanks,
Dennis
Re: [PATCH v2 8/8] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Hi CK,
Thanks for your comment.
On Tue, 2020-07-07 at 07:46 +0800, Chun-Kuang Hu wrote:
> Hi, Dennis:
>
> Dennis YC Hsieh 於 2020年7月6日 週一 下午3:20寫道：
> >
> > Add clear parameter to let client decide if
> > event should be clear to 0 after GCE receive it.
> >
> > Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow")
>
> I think this patch include two things, one is bug fix, another is
> changing interface.
> below is the bug fix part.
>
> -#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
> CMDQ_WFE_WAIT | \
> - CMDQ_WFE_WAIT_VALUE)
> +#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
> CMDQ_WFE_WAIT_VALUE)
>
> the other is changing interface part. So this patch should be broken
> into two patches.
ok I'll break into two patches
Regards,
Dennis
>
> Regards,
> Chun-Kuang.
>
> > Signed-off-by: Dennis YC Hsieh
> > Reviewed-by: CK Hu
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
> > drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
> > include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
> > include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
> > 4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index ec6c9ffbf35e..ba6cf956b239 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
> > *mtk_crtc)
> > mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> > cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client,
> > PAGE_SIZE);
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index d55dc3296105..505651b0d715 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
> >
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
> > {
> > struct cmdq_instruction inst = { {0} };
> > + u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
> >
> > if (event >= CMDQ_MAX_EVENT)
> > return -EINVAL;
> >
> > inst.op = CMDQ_CODE_WFE;
> > - inst.value = CMDQ_WFE_OPTION;
> > + inst.value = CMDQ_WFE_OPTION | clear_option;
> > inst.event = event;
> >
> > return cmdq_pkt_append_command(pkt, inst);
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index efbd8a9eb2d1..d5a983d65f05 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -28,8 +28,7 @@
> > * bit 16-27: update value
> > * bit 31: 1 - update, 0 - no update
> > */
> > -#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
> > CMDQ_WFE_WAIT | \
> > - CMDQ_WFE_WAIT_VALUE)
> > +#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
> > CMDQ_WFE_WAIT_VALUE)
> >
> > /** cmdq event maximum */
> > #define CMDQ_MAX_EVENT 0x3ff
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 34354e952f60..960704d75994 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> > - * @event: the desired event type to "wait and CLEAR"
> > + * @event: the desired event
Re: [PATCH v2 1/8] soc: mediatek: cmdq: add address shift in jump
Hi Matthias, thanks for your comment On Mon, 2020-07-06 at 16:03 +0200, Matthias Brugger wrote: > > On 05/07/2020 08:48, Dennis YC Hsieh wrote: > > Add address shift when compose jump instruction > > to compatible with 35bit format. > > > > Signed-off-by: Dennis YC Hsieh > > You are missing Bibby's Reviewed-by. Please honour the effort reviewers do by > adding the appropriate tags. > > Please double check the series and resend with all tags added. > > Also, it would be good if you could provide a change log. That makes it easier > for the maintainer to see which statements you addressed. this patch changed since cmdq_mbox_shift() rename to cmdq_get_shift_pa() by Bibby's comment [1], so I removed reviewed tags from this patch. I'll provide change log to this patch and resend later, thanks. [1] http://lists.infradead.org/pipermail/linux-mediatek/2020-June/013387.html Regards, Dennis > > Thanks, > Matthias > > > --- > > drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > > b/drivers/soc/mediatek/mtk-cmdq-helper.c > > index dc644cfb6419..9faf78fbed3a 100644 > > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > > @@ -329,7 +329,8 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > > > > /* JUMP to end */ > > inst.op = CMDQ_CODE_JUMP; > > - inst.value = CMDQ_JUMP_PASS; > > + inst.value = CMDQ_JUMP_PASS >> > > + cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan); > > err = cmdq_pkt_append_command(pkt, inst); > > > > return err; > >
[PATCH v2 4/8] soc: mediatek: cmdq: add read_s function
Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 12
3 files changed, 28 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 550e9e7e3ff2..ed9f5e63c195 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -227,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_READ_S;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.reg_dst = reg_idx;
+ inst.src_reg = addr_low;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 90d1d8e64412..efbd8a9eb2d1 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 53230341bf94..cd7ec714344e 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -104,6 +104,18 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev,
int index,
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/*
+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @reg_idx: the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx);
+
/**
* cmdq_pkt_write_s() - append write_s command to the CMDQ packet
* @pkt: the CMDQ packet
--
1.7.9.5
Subject: [PATCH v1 0/8] support cmdq helper function on mt6779 platform
This patch support more gce helper function on mt6779 platform. depends on patch: support gce on mt6779 platform and depends on following applied patches soc: mediatek: cmdq: add set event function soc: mediatek: cmdq: export finalize function soc: mediatek: cmdq: add assign function Change since v1: - Rename cmdq_mbox_shift() to cmdq_get_shift_pa(). Dennis YC Hsieh (8): soc: mediatek: cmdq: add address shift in jump soc: mediatek: cmdq: add write_s function soc: mediatek: cmdq: add write_s_mask function soc: mediatek: cmdq: add read_s function soc: mediatek: cmdq: add write_s value function soc: mediatek: cmdq: add write_s_mask value function soc: mediatek: cmdq: add jump function soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- drivers/soc/mediatek/mtk-cmdq-helper.c | 113 ++- include/linux/mailbox/mtk-cmdq-mailbox.h | 6 +- include/linux/soc/mediatek/mtk-cmdq.h| 93 ++- 4 files changed, 206 insertions(+), 8 deletions(-) -- 2.18.0
[PATCH v2 3/8] soc: mediatek: cmdq: add write_s_mask function
add write_s_mask function in cmdq helper functions which
writes value contains in internal register to address
with mask and large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 18 ++
3 files changed, 42 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 880349b3f16c..550e9e7e3ff2 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -242,6 +242,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s);
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.mask = 0;
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 1f76cfedb16d..90d1d8e64412 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -61,6 +61,7 @@ enum cmdq_code {
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
CMDQ_CODE_WRITE_S = 0x90,
+ CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 9b0c57a0063d..53230341bf94 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -122,6 +122,24 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx);
/**
+ * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ
packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v2 5/8] soc: mediatek: cmdq: add write_s value function
add write_s function in cmdq helper functions which
writes a constant value to address with large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 14 ++
include/linux/soc/mediatek/mtk-cmdq.h | 13 +
2 files changed, 27 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ed9f5e63c195..4e86b65815fc 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -280,6 +280,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index cd7ec714344e..ae73e10da274 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,19 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx, u32 mask);
/**
+ * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which
+ * write value to a physical address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v2 1/8] soc: mediatek: cmdq: add address shift in jump
Add address shift when compose jump instruction to compatible with 35bit format. Signed-off-by: Dennis YC Hsieh --- drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index dc644cfb6419..9faf78fbed3a 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -329,7 +329,8 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) /* JUMP to end */ inst.op = CMDQ_CODE_JUMP; - inst.value = CMDQ_JUMP_PASS; + inst.value = CMDQ_JUMP_PASS >> + cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan); err = cmdq_pkt_append_command(pkt, inst); return err; -- 1.7.9.5
[PATCH v2 7/8] soc: mediatek: cmdq: add jump function
Add jump function so that client can jump to any address which
contains instruction.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 13 +
include/linux/soc/mediatek/mtk-cmdq.h | 11 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index b6e25f216605..d55dc3296105 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -13,6 +13,7 @@
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
#define CMDQ_REG_TYPE 1
+#define CMDQ_JUMP_RELATIVE 1
struct cmdq_instruction {
union {
@@ -407,6 +408,18 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_JUMP;
+ inst.offset = CMDQ_JUMP_RELATIVE;
+ inst.value = addr >>
+ cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan);
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_jump);
+
int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index d9390d76ee14..34354e952f60 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -253,6 +253,17 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
/**
+ * cmdq_pkt_jump() - Append jump command to the CMDQ packet, ask GCE
+ * to execute an instruction that change current thread PC to
+ * a physical address which should contains more instruction.
+ * @pkt:the CMDQ packet
+ * @addr: physical address of target instruction buffer
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr);
+
+/**
* cmdq_pkt_finalize() - Append EOC and jump command to pkt.
* @pkt: the CMDQ packet
*
--
1.7.9.5
[PATCH v2 2/8] soc: mediatek: cmdq: add write_s function
add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
3 files changed, 39 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9faf78fbed3a..880349b3f16c 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -18,6 +18,10 @@ struct cmdq_instruction {
union {
u32 value;
u32 mask;
+ struct {
+ u16 arg_c;
+ u16 src_reg;
+ };
};
union {
u16 offset;
@@ -223,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 05eea1aef5aa..1f76cfedb16d 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 2249ecaf77e4..9b0c57a0063d 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -12,6 +12,8 @@
#include
#define CMDQ_NO_TIMEOUT0xu
+#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
struct cmdq_pkt;
@@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
/**
+ * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v2 8/8] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Add clear parameter to let client decide if
event should be clear to 0 after GCE receive it.
Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow")
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index ec6c9ffbf35e..ba6cf956b239 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
- cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
+ cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index d55dc3296105..505651b0d715 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
{
struct cmdq_instruction inst = { {0} };
+ u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
inst.op = CMDQ_CODE_WFE;
- inst.value = CMDQ_WFE_OPTION;
+ inst.value = CMDQ_WFE_OPTION | clear_option;
inst.event = event;
return cmdq_pkt_append_command(pkt, inst);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index efbd8a9eb2d1..d5a983d65f05 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -28,8 +28,7 @@
* bit 16-27: update value
* bit 31: 1 - update, 0 - no update
*/
-#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
CMDQ_WFE_WAIT | \
- CMDQ_WFE_WAIT_VALUE)
+#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
CMDQ_WFE_WAIT_VALUE)
/** cmdq event maximum */
#define CMDQ_MAX_EVENT 0x3ff
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 34354e952f60..960704d75994 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
- * @event: the desired event type to "wait and CLEAR"
+ * @event: the desired event type to wait
+ * @clear: clear event or not after event arrive
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
--
1.7.9.5
[PATCH v2 6/8] soc: mediatek: cmdq: add write_s_mask value function
add write_s_mask_value function in cmdq helper functions which
writes a constant value to address with mask and large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 21 +
include/linux/soc/mediatek/mtk-cmdq.h | 15 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 4e86b65815fc..b6e25f216605 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -294,6 +294,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index ae73e10da274..d9390d76ee14 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -165,6 +165,21 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
u16 addr_low, u32 value);
/**
+ * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ
+ *packet which write value to a physical
+ *address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ * @mask: the specified target mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v8 2/4] mailbox: cmdq: variablize address shift in platform
Some gce hardware shift pc and end address in register to support
large dram addressing.
Implement gce address shift when write or read pc and end register.
And add shift bit in platform definition.
Signed-off-by: Dennis YC Hsieh
---
drivers/mailbox/mtk-cmdq-mailbox.c | 57 +++---
include/linux/mailbox/mtk-cmdq-mailbox.h |2 ++
2 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index b24822ad8409..49d9264145aa 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -75,8 +75,22 @@ struct cmdq {
struct cmdq_thread *thread;
struct clk *clock;
boolsuspended;
+ u8 shift_pa;
};
+struct gce_plat {
+ u32 thread_nr;
+ u8 shift;
+};
+
+u8 cmdq_get_shift_pa(struct mbox_chan *chan)
+{
+ struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
+
+ return cmdq->shift_pa;
+}
+EXPORT_SYMBOL(cmdq_get_shift_pa);
+
static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread *thread)
{
u32 status;
@@ -183,13 +197,15 @@ static void cmdq_task_handle_error(struct cmdq_task *task)
{
struct cmdq_thread *thread = task->thread;
struct cmdq_task *next_task;
+ struct cmdq *cmdq = task->cmdq;
- dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
- WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
+ dev_err(cmdq->mbox.dev, "task 0x%p error\n", task);
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
next_task = list_first_entry_or_null(&thread->task_busy_list,
struct cmdq_task, list_entry);
if (next_task)
- writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
+ writel(next_task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
cmdq_thread_resume(thread);
}
@@ -219,7 +235,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
else
return;
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
@@ -335,27 +351,31 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan,
void *data)
WARN_ON(clk_enable(cmdq->clock) < 0);
WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
- writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
- writel(task->pa_base + pkt->cmd_buf_size,
+ writel(task->pa_base >> cmdq->shift_pa,
+ thread->base + CMDQ_THR_CURR_ADDR);
+ writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
thread->base + CMDQ_THR_END_ADDR);
+
writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABLE_TASK);
} else {
WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
- curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
- end_pa = readl(thread->base + CMDQ_THR_END_ADDR);
+ curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) <<
+ cmdq->shift_pa;
+ end_pa = readl(thread->base + CMDQ_THR_END_ADDR) <<
+ cmdq->shift_pa;
/* check boundary */
if (curr_pa == end_pa - CMDQ_INST_SIZE ||
curr_pa == end_pa) {
/* set to this task directly */
- writel(task->pa_base,
+ writel(task->pa_base >> cmdq->shift_pa,
thread->base + CMDQ_THR_CURR_ADDR);
} else {
cmdq_task_insert_into_thread(task);
smp_mb(); /* modify jump before enable thread */
}
- writel(task->pa_base + pkt->cmd_buf_size,
+ writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
thread->base + CMDQ_THR_END_ADDR);
cmdq_thread_resume(thread);
}
@@ -453,6 +473,7 @@ static int cmdq_probe(struct platform_device *pdev)
struct resource *res;
struct cmdq *cmdq;
int err, i;
+ struct gce_plat *plat_data;
cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
if (!cmdq)
@@ -471,7 +492,14 @@
[PATCH v8 0/4] support gce on mt6779 platform
This patch support gce on mt6779 platform. Change since v7: - Rename cmdq_mbox_shift() to cmdq_get_shift_pa(). Change since v6: - Separate all helper function to another patchset. Change since v5: - spearate address shift code in client helper and mailbox controller - separate write_s/write_s_mask and write_s_value/write_s_mask_value so that client can decide use mask or not - fix typo in header [... snip ...] Dennis YC Hsieh (4): dt-binding: gce: add gce header file for mt6779 mailbox: cmdq: variablize address shift in platform mailbox: cmdq: support mt6779 gce platform definition mailbox: mediatek: cmdq: clear task in channel before shutdown .../devicetree/bindings/mailbox/mtk-gce.txt | 8 +- drivers/mailbox/mtk-cmdq-mailbox.c| 97 +++- include/dt-bindings/gce/mt6779-gce.h | 222 ++ include/linux/mailbox/mtk-cmdq-mailbox.h | 2 + 4 files changed, 313 insertions(+), 16 deletions(-) create mode 100644 include/dt-bindings/gce/mt6779-gce.h -- 2.18.0
[PATCH v8 3/4] mailbox: cmdq: support mt6779 gce platform definition
Add gce v4 hardware support with different thread number and shift.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Reviewed-by: Matthias Brugger
Reviewed-by: Bibby Hsieh
---
drivers/mailbox/mtk-cmdq-mailbox.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 49d9264145aa..08bd4f1eb469 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -564,10 +564,12 @@ static int cmdq_probe(struct platform_device *pdev)
static const struct gce_plat gce_plat_v2 = {.thread_nr = 16};
static const struct gce_plat gce_plat_v3 = {.thread_nr = 24};
+static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3};
static const struct of_device_id cmdq_of_ids[] = {
{.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
{.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
+ {.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4},
{}
};
--
1.7.9.5
[PATCH v8 1/4] dt-binding: gce: add gce header file for mt6779
Add documentation for the mt6779 gce. Add gce header file defined the gce hardware event, subsys number and constant for mt6779. Signed-off-by: Dennis YC Hsieh Reviewed-by: Rob Herring Reviewed-by: CK Hu Reviewed-by: Bibby Hsieh --- .../devicetree/bindings/mailbox/mtk-gce.txt|8 +- include/dt-bindings/gce/mt6779-gce.h | 222 2 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/gce/mt6779-gce.h diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt index 0b5b2a6bcc48..cf48cd806e00 100644 --- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt @@ -9,7 +9,8 @@ CMDQ driver uses mailbox framework for communication. Please refer to mailbox.txt for generic information about mailbox device-tree bindings. Required properties: -- compatible: can be "mediatek,mt8173-gce" or "mediatek,mt8183-gce" +- compatible: can be "mediatek,mt8173-gce", "mediatek,mt8183-gce" or + "mediatek,mt6779-gce". - reg: Address range of the GCE unit - interrupts: The interrupt signal from the GCE block - clock: Clocks according to the common clock binding @@ -34,8 +35,9 @@ Optional properties for a client device: start_offset: the start offset of register address that GCE can access. size: the total size of register address that GCE can access. -Some vaules of properties are defined in 'dt-bindings/gce/mt8173-gce.h' -or 'dt-binding/gce/mt8183-gce.h'. Such as sub-system ids, thread priority, event ids. +Some vaules of properties are defined in 'dt-bindings/gce/mt8173-gce.h', +'dt-binding/gce/mt8183-gce.h' or 'dt-bindings/gce/mt6779-gce.h'. Such as +sub-system ids, thread priority, event ids. Example: diff --git a/include/dt-bindings/gce/mt6779-gce.h b/include/dt-bindings/gce/mt6779-gce.h new file mode 100644 index ..06101316ace4 --- /dev/null +++ b/include/dt-bindings/gce/mt6779-gce.h @@ -0,0 +1,222 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2019 MediaTek Inc. + * Author: Dennis-YC Hsieh + */ + +#ifndef _DT_BINDINGS_GCE_MT6779_H +#define _DT_BINDINGS_GCE_MT6779_H + +#define CMDQ_NO_TIMEOUT0x + +/* GCE HW thread priority */ +#define CMDQ_THR_PRIO_LOWEST 0 +#define CMDQ_THR_PRIO_11 +#define CMDQ_THR_PRIO_22 +#define CMDQ_THR_PRIO_33 +#define CMDQ_THR_PRIO_44 +#define CMDQ_THR_PRIO_55 +#define CMDQ_THR_PRIO_66 +#define CMDQ_THR_PRIO_HIGHEST 7 + +/* GCE subsys table */ +#define SUBSYS_13000 +#define SUBSYS_14001 +#define SUBSYS_14012 +#define SUBSYS_14023 +#define SUBSYS_15024 +#define SUBSYS_18805 +#define SUBSYS_18816 +#define SUBSYS_18827 +#define SUBSYS_18838 +#define SUBSYS_18849 +#define SUBSYS_100010 +#define SUBSYS_100111 +#define SUBSYS_100212 +#define SUBSYS_100313 +#define SUBSYS_100414 +#define SUBSYS_100515 +#define SUBSYS_102016 +#define SUBSYS_102817 +#define SUBSYS_170018 +#define SUBSYS_170119 +#define SUBSYS_170220 +#define SUBSYS_170321 +#define SUBSYS_180022 +#define SUBSYS_180123 +#define SUBSYS_180224 +#define SUBSYS_180425 +#define SUBSYS_180526 +#define SUBSYS_180827 +#define SUBSYS_180a28 +#define SUBSYS_180b29 +#define CMDQ_SUBSYS_OFF32 + +/* GCE hardware events */ +#define CMDQ_EVENT_DISP_RDMA0_SOF 0 +#define CMDQ_EVENT_DISP_RDMA1_SOF 1 +#define CMDQ_EVENT_MDP_RDMA0_SOF 2 +#define CMDQ_EVENT_MDP_RDMA1_SOF 3 +#define CMDQ_EVENT_MDP_RSZ0_SOF4 +#define CMDQ_EVENT_MDP_RSZ1_SOF5 +#define CMDQ_EVENT_MDP_TDSHP_SOF 6 +#define CMDQ_EVENT_MDP_WROT0_SOF 7 +#define CMDQ_EVENT_MDP_WROT1_SOF 8 +#define CMDQ_EVENT_DISP_OVL0_SOF 9 +#define CMDQ_EVENT_DISP_2L_OVL0_SOF10 +#define CMDQ_EVENT_DISP_2L_OVL1_SOF11 +#define CMDQ_EVENT_DISP_WDMA0_SOF 12 +#define CMDQ_EVENT_DISP_COLOR0_SOF 13 +#define CMDQ_EVENT_DISP_CCORR0_SOF 14 +#define CMDQ_EVENT_DISP_AAL0_SOF 15 +#define CMDQ_EVENT_DISP_GAMMA0_SOF
[PATCH v8 4/4] mailbox: mediatek: cmdq: clear task in channel before shutdown
Do success callback in channel when shutdown. For those task not finish,
callback with error code thus client has chance to cleanup or reset.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Reviewed-by: Bibby Hsieh
---
drivers/mailbox/mtk-cmdq-mailbox.c | 38
1 file changed, 38 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 08bd4f1eb469..484d4438cd83 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -349,6 +349,12 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan,
void *data)
if (list_empty(&thread->task_busy_list)) {
WARN_ON(clk_enable(cmdq->clock) < 0);
+ /*
+* The thread reset will clear thread related register to 0,
+* including pc, end, priority, irq, suspend and enable. Thus
+* set CMDQ_THR_ENABLED to CMDQ_THR_ENABLE_TASK will enable
+* thread and make it running.
+*/
WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
writel(task->pa_base >> cmdq->shift_pa,
@@ -391,6 +397,38 @@ static int cmdq_mbox_startup(struct mbox_chan *chan)
static void cmdq_mbox_shutdown(struct mbox_chan *chan)
{
+ struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
+ struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
+ struct cmdq_task *task, *tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&thread->chan->lock, flags);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
+
+ /* make sure executed tasks have success callback */
+ cmdq_thread_irq_handler(cmdq, thread);
+ if (list_empty(&thread->task_busy_list))
+ goto done;
+
+ list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
+list_entry) {
+ cmdq_task_exec_done(task, CMDQ_CB_ERROR);
+ kfree(task);
+ }
+
+ cmdq_thread_disable(cmdq, thread);
+ clk_disable(cmdq->clock);
+done:
+ /*
+* The thread->task_busy_list empty means thread already disable. The
+* cmdq_mbox_send_data() always reset thread which clear disable and
+* suspend statue when first pkt send to channel, so there is no need
+* to do any operation here, only unlock and leave.
+*/
+ spin_unlock_irqrestore(&thread->chan->lock, flags);
}
static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
--
1.7.9.5
Re: [PATCH v2 3/8] IB/hfi1: Convert PCIBIOS_* errors to generic -E* errors
On 6/15/2020 3:32 AM, refactormys...@gmail.com wrote: From: Bolarinwa Olayemi Saheed restore_pci_variables() and save_pci_variables() return PCIBIOS_ error codes from PCIe capability accessors. PCIBIOS_ error codes have positive values. Passing on these values is inconsistent with functions which return only a negative value on failure. Before passing on the return value of PCIe capability accessors, call pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to negative generic error values. Fix redundant initialisation. Suggested-by: Bjorn Helgaas Signed-off-by: Bolarinwa Olayemi Saheed Looks like we may have had a problem when calling pci_read_config_dword() from the init dd path and doing a check for < 0 to bail. So this looks like goodness to me. Reviewed-by: Dennis Dalessandro
Re: [PATCH v2 2/8] IB/hfi1: Convert PCIBIOS_* errors to generic -E* errors
On 6/15/2020 3:32 AM, refactormys...@gmail.com wrote:
From: Bolarinwa Olayemi Saheed
pcie_speeds() returns PCIBIOS_ error codes from PCIe capability accessors.
PCIBIOS_ error codes have positive values. Passing on these values is
inconsistent with functions which return only a negative value on failure.
Before passing on the return value of PCIe capability accessors, call
pcibios_err_to_errno() to convert any positive PCIBIOS_ error codes to
negative generic error values.
Suggested-by: Bjorn Helgaas
Signed-off-by: Bolarinwa Olayemi Saheed
---
drivers/infiniband/hw/hfi1/pcie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/hfi1/pcie.c
b/drivers/infiniband/hw/hfi1/pcie.c
index 1a6268d61977..eb53781d0c6a 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -306,7 +306,7 @@ int pcie_speeds(struct hfi1_devdata *dd)
ret = pcie_capability_read_dword(dd->pcidev, PCI_EXP_LNKCAP, &linkcap);
if (ret) {
dd_dev_err(dd, "Unable to read from PCI config\n");
- return ret;
+ return pcibios_err_to_errno(ret);
}
if ((linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_8_0GB) {
Reviewed-by: Dennis Dalessandro
Re: [PATCH] IB/hfi1: Add explicit cast OPA_MTU_8192 to 'enum ib_mtu'
On 6/22/2020 8:52 PM, Nathan Chancellor wrote:
Clang warns:
drivers/infiniband/hw/hfi1/qp.c:198:9: warning: implicit conversion from
enumeration type 'enum opa_mtu' to different enumeration type 'enum
ib_mtu' [-Wenum-conversion]
mtu = OPA_MTU_8192;
~ ^~~~
1 warning generated.
enum opa_mtu extends enum ib_mtu. There are typically two ways to deal
with this:
* Remove the expected types and just use 'int' for all parameters and
types.
* Explicitly cast the enums between each other.
This driver chooses to do the later so do the same thing here.
Fixes: 6d72344cf6c4 ("IB/ipoib: Increase ipoib Datagram mode MTU's upper limit")
Link: https://github.com/ClangBuiltLinux/linux/issues/1062
Link:
https://lore.kernel.org/linux-rdma/20200527040350.GA3118979@ubuntu-s3-xlarge-x86/
Signed-off-by: Nathan Chancellor
---
Acked-by: Dennis Dalessandro
Re: [PATCH v1 03/11] soc: mediatek: cmdq: add write_s function
Hi Matthias,
On Mon, 2020-06-22 at 19:08 +0200, Matthias Brugger wrote:
>
> On 22/06/2020 18:12, Dennis-YC Hsieh wrote:
> > Hi Matthias,
> >
> > On Mon, 2020-06-22 at 17:54 +0200, Matthias Brugger wrote:
> >>
> >> On 22/06/2020 17:36, Dennis-YC Hsieh wrote:
> >>> Hi Matthias,
> >>>
> >>> thanks for your comment.
> >>>
> >>> On Mon, 2020-06-22 at 13:07 +0200, Matthias Brugger wrote:
> >>>>
> >>>> On 21/06/2020 16:18, Dennis YC Hsieh wrote:
> >>>>> add write_s function in cmdq helper functions which
> >>>>> writes value contains in internal register to address
> >>>>> with large dma access support.
> >>>>>
> >>>>> Signed-off-by: Dennis YC Hsieh
> >>>>> ---
> >>>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
> >>>>> include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
> >>>>> include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
> >>>>> 3 files changed, 39 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>>>> b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>>>> index bf32e3b2ca6c..817a5a97dbe5 100644
> >>>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>>>> @@ -18,6 +18,10 @@ struct cmdq_instruction {
> >>>>> union {
> >>>>> u32 value;
> >>>>> u32 mask;
> >>>>> + struct {
> >>>>> + u16 arg_c;
> >>>>> + u16 src_reg;
> >>>>> + };
> >>>>> };
> >>>>> union {
> >>>>> u16 offset;
> >>>>> @@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8
> >>>>> subsys,
> >>>>> }
> >>>>> EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >>>>>
> >>>>> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> >>>>> +u16 addr_low, u16 src_reg_idx)
> >>>>> +{
> >>>>
> >>>> Do I understand correctly that we use CMDQ_ADDR_HIGH(addr) and
> >>>> CMDQ_ADDR_LOW(addr) to calculate in the client high_addr_reg_idx and
> >>>> addr_low
> >>>> respectively?
> >>>>
> >>>> In that case I think a better interface would be to pass the address and
> >>>> do the
> >>>> high/low calculation in the cmdq_pkt_write_s
> >>>
> >>> Not exactly. The high_addr_reg_idx parameter is index of internal
> >>> register (which store address bit[47:16]), not result of
> >>> CMDQ_ADDR_HIGH(addr).
> >>>
> >>> The CMDQ_ADDR_HIGH macro use in patch 02/11 cmdq_pkt_assign() api. This
> >>> api helps assign address bit[47:16] into one of internal register by
> >>> index. And same index could be use in cmdq_pkt_write_s(). The gce
> >>> combine bit[47:16] in internal register and bit[15:0] in addr_low
> >>> parameter to final address. So it is better to keep interface in this
> >>> way.
> >>>
> >>
> >> Got it, but then why don't we call cmdq_pkt_assign() in
> >> cmdq_pkt_write_s()? This
> >> way we would get a clean API for what we want to do.
> >> Do we expect other users of cmdq_pkt_assign()? Otherwise we could keep it
> >> private the this file and don't export it.
> >
> > Considering this case: write 2 register 0xaabb00c0 0xaabb00d0.
> >
> > If we call assign inside write_s api it will be:
> > assign aabb to internal reg 0
> > write reg 0 + 0x00c0
> > assign aabb to internal reg 0
> > write reg 0 + 0x00d0
> >
> >
> > But if we let client decide timing to call assign, it will be like:
> > assign aabb to internal reg 0
> > write reg 0 + 0x00c0
> > write reg 0 + 0x00d0
> >
>
> Ok, thanks for clarification. Is this something you exepect to see in the gce
> consumer driver?
>
yes it is, less command means better performance and save memory, so it
is a good practice for consumer.
> >
> > The first way uses 4 command and second one uses only 3 comm
Re: [PATCH v1 03/11] soc: mediatek: cmdq: add write_s function
Hi Matthias,
On Mon, 2020-06-22 at 17:54 +0200, Matthias Brugger wrote:
>
> On 22/06/2020 17:36, Dennis-YC Hsieh wrote:
> > Hi Matthias,
> >
> > thanks for your comment.
> >
> > On Mon, 2020-06-22 at 13:07 +0200, Matthias Brugger wrote:
> >>
> >> On 21/06/2020 16:18, Dennis YC Hsieh wrote:
> >>> add write_s function in cmdq helper functions which
> >>> writes value contains in internal register to address
> >>> with large dma access support.
> >>>
> >>> Signed-off-by: Dennis YC Hsieh
> >>> ---
> >>> drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
> >>> include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
> >>> include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
> >>> 3 files changed, 39 insertions(+)
> >>>
> >>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> index bf32e3b2ca6c..817a5a97dbe5 100644
> >>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> @@ -18,6 +18,10 @@ struct cmdq_instruction {
> >>> union {
> >>> u32 value;
> >>> u32 mask;
> >>> + struct {
> >>> + u16 arg_c;
> >>> + u16 src_reg;
> >>> + };
> >>> };
> >>> union {
> >>> u16 offset;
> >>> @@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8
> >>> subsys,
> >>> }
> >>> EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >>>
> >>> +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> >>> + u16 addr_low, u16 src_reg_idx)
> >>> +{
> >>
> >> Do I understand correctly that we use CMDQ_ADDR_HIGH(addr) and
> >> CMDQ_ADDR_LOW(addr) to calculate in the client high_addr_reg_idx and
> >> addr_low
> >> respectively?
> >>
> >> In that case I think a better interface would be to pass the address and
> >> do the
> >> high/low calculation in the cmdq_pkt_write_s
> >
> > Not exactly. The high_addr_reg_idx parameter is index of internal
> > register (which store address bit[47:16]), not result of
> > CMDQ_ADDR_HIGH(addr).
> >
> > The CMDQ_ADDR_HIGH macro use in patch 02/11 cmdq_pkt_assign() api. This
> > api helps assign address bit[47:16] into one of internal register by
> > index. And same index could be use in cmdq_pkt_write_s(). The gce
> > combine bit[47:16] in internal register and bit[15:0] in addr_low
> > parameter to final address. So it is better to keep interface in this
> > way.
> >
>
> Got it, but then why don't we call cmdq_pkt_assign() in cmdq_pkt_write_s()?
> This
> way we would get a clean API for what we want to do.
> Do we expect other users of cmdq_pkt_assign()? Otherwise we could keep it
> private the this file and don't export it.
Considering this case: write 2 register 0xaabb00c0 0xaabb00d0.
If we call assign inside write_s api it will be:
assign aabb to internal reg 0
write reg 0 + 0x00c0
assign aabb to internal reg 0
write reg 0 + 0x00d0
But if we let client decide timing to call assign, it will be like:
assign aabb to internal reg 0
write reg 0 + 0x00c0
write reg 0 + 0x00d0
The first way uses 4 command and second one uses only 3 command.
Thus it is better to let client call assign explicitly.
>
> By the way, why do you postfix the _s, I understand that it reflects the large
> DMA access but I wonder why you choose '_s'.
>
The name of this command is "write_s" which is hardware spec.
I'm just following it since it is a common language between gce sw/hw
designers.
Regards,
Dennis
> Regards,
> Matthias
>
> >
> > Regards,
> > Dennis
> >
> >>
> >> Regards,
> >> Matthias
> >>
> >>> + struct cmdq_instruction inst = {};
> >>> +
> >>> + inst.op = CMDQ_CODE_WRITE_S;
> >>> + inst.src_t = CMDQ_REG_TYPE;
> >>> + inst.sop = high_addr_reg_idx;
> >>> + inst.offset = addr_low;
> >>> + inst.src_reg = src_reg_idx;
> >>> +
> >>> + return cmdq_pkt_append_command(pkt, inst);
> >>> +}
> >>> +EXPORT_SYMBOL(cmdq_pkt_write_s);
> >>> +
> >>> int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> >>> {
> >>> struct cmdq_instruction
Re: [PATCH v1 10/11] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Hi Matthias,
thanks for your comment.
On Mon, 2020-06-22 at 13:19 +0200, Matthias Brugger wrote:
>
> On 21/06/2020 16:18, Dennis YC Hsieh wrote:
> > Add clear parameter to let client decide if
> > event should be clear to 0 after GCE receive it.
> >
> > Signed-off-by: Dennis YC Hsieh
> > Reviewed-by: CK Hu
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
> > drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
> > include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
> > include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
> > 4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 7daaabc26eb1..a065b3a412cf 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -488,7 +488,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
> > *mtk_crtc)
> > if (mtk_crtc->cmdq_client) {
> > cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > - cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > + cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
>
> This does not set CMDQ_WFE_UPDATE while the old code did. Is this a bug fix
> or a
> bug in the code?
> If it's a fix, please provide a fixes tag.
no need to to update again since event always clear before wait.
I'll provide a fix tag, thanks.
Regards,
Dennis
>
> Thanks,
> Matthias
>
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 009f86ae72c6..13f78c9b5901 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -315,15 +315,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
> >
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
> > {
> > struct cmdq_instruction inst = { {0} };
> > + u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
> >
> > if (event >= CMDQ_MAX_EVENT)
> > return -EINVAL;
> >
> > inst.op = CMDQ_CODE_WFE;
> > - inst.value = CMDQ_WFE_OPTION;
> > + inst.value = CMDQ_WFE_OPTION | clear_option;
> > inst.event = event;
> >
> > return cmdq_pkt_append_command(pkt, inst);
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 3f6bc0dfd5da..42d2a30e6a70 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -27,8 +27,7 @@
> > * bit 16-27: update value
> > * bit 31: 1 - update, 0 - no update
> > */
> > -#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
> > CMDQ_WFE_WAIT | \
> > - CMDQ_WFE_WAIT_VALUE)
> > +#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
> > CMDQ_WFE_WAIT_VALUE)
> >
> > /** cmdq event maximum */
> > #define CMDQ_MAX_EVENT 0x3ff
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 18364d81e8f7..4b5f5d154bad 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt,
> > u8 high_addr_reg_idx,
> > /**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> > - * @event: the desired event type to "wait and CLEAR"
> > + * @event: the desired event type to wait
> > + * @clear: clear event or not after event arrive
> > *
> > * Return: 0 for success; else the error code is returned
> > */
> > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
> > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> >
> > /**
> > * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
> >
Re: [PATCH v1 03/11] soc: mediatek: cmdq: add write_s function
Hi Matthias,
thanks for your comment.
On Mon, 2020-06-22 at 13:07 +0200, Matthias Brugger wrote:
>
> On 21/06/2020 16:18, Dennis YC Hsieh wrote:
> > add write_s function in cmdq helper functions which
> > writes value contains in internal register to address
> > with large dma access support.
> >
> > Signed-off-by: Dennis YC Hsieh
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
> > include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
> > include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
> > 3 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index bf32e3b2ca6c..817a5a97dbe5 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -18,6 +18,10 @@ struct cmdq_instruction {
> > union {
> > u32 value;
> > u32 mask;
> > + struct {
> > + u16 arg_c;
> > + u16 src_reg;
> > + };
> > };
> > union {
> > u16 offset;
> > @@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8
> > subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_write_mask);
> >
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > +u16 addr_low, u16 src_reg_idx)
> > +{
>
> Do I understand correctly that we use CMDQ_ADDR_HIGH(addr) and
> CMDQ_ADDR_LOW(addr) to calculate in the client high_addr_reg_idx and addr_low
> respectively?
>
> In that case I think a better interface would be to pass the address and do
> the
> high/low calculation in the cmdq_pkt_write_s
Not exactly. The high_addr_reg_idx parameter is index of internal
register (which store address bit[47:16]), not result of
CMDQ_ADDR_HIGH(addr).
The CMDQ_ADDR_HIGH macro use in patch 02/11 cmdq_pkt_assign() api. This
api helps assign address bit[47:16] into one of internal register by
index. And same index could be use in cmdq_pkt_write_s(). The gce
combine bit[47:16] in internal register and bit[15:0] in addr_low
parameter to final address. So it is better to keep interface in this
way.
Regards,
Dennis
>
> Regards,
> Matthias
>
> > + struct cmdq_instruction inst = {};
> > +
> > + inst.op = CMDQ_CODE_WRITE_S;
> > + inst.src_t = CMDQ_REG_TYPE;
> > + inst.sop = high_addr_reg_idx;
> > + inst.offset = addr_low;
> > + inst.src_reg = src_reg_idx;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_write_s);
> > +
> > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
> > {
> > struct cmdq_instruction inst = { {0} };
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 121c3bb6d3de..ee67dd3b86f5 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -59,6 +59,7 @@ enum cmdq_code {
> > CMDQ_CODE_JUMP = 0x10,
> > CMDQ_CODE_WFE = 0x20,
> > CMDQ_CODE_EOC = 0x40,
> > + CMDQ_CODE_WRITE_S = 0x90,
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 83340211e1d3..e1c5a7549b4f 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -12,6 +12,8 @@
> > #include
> >
> > #define CMDQ_NO_TIMEOUT0xu
> > +#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
> > +#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
> >
> > struct cmdq_pkt;
> >
> > @@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8
> > subsys,
> > u16 offset, u32 value, u32 mask);
> >
> > /**
> > + * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @high_addr_reg_idx: internal register ID which contains high
> > address of pa
> > + * @addr_low: low address of pa
> > + * @src_reg_idx: the CMDQ internal register ID which cache source value
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + *
> > + * Support write value to physical address without subsys. Use
> > CMDQ_ADDR_HIGH()
> > + * to get high address and call cmdq_pkt_assign() to assign value into
> > internal
> > + * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter
> > when
> > + * call to this function.
> > + */
> > +int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
> > +u16 addr_low, u16 src_reg_idx);
> > +
> > +/**
> > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
> > * @pkt: the CMDQ packet
> > * @event: the desired event type to "wait and CLEAR"
> >
Re: [PATCH v1 0/11] support cmdq helper function on mt6779 platform
Hi Bibby, On Mon, 2020-06-22 at 10:40 +0800, Bibby Hsieh wrote: > Hi, Dennis, > > Please add "depends on patch: support gce on mt6779 platform" in cover > letter. Thanks ok will do, thanks Regards, Dennis > > Bibby > > On Sun, 2020-06-21 at 22:18 +0800, Dennis YC Hsieh wrote: > > This patch support cmdq helper function on mt6779 platform, > > based on "support gce on mt6779 platform" patchset. > > > > > > Dennis YC Hsieh (11): > > soc: mediatek: cmdq: add address shift in jump > > soc: mediatek: cmdq: add assign function > > soc: mediatek: cmdq: add write_s function > > soc: mediatek: cmdq: add write_s_mask function > > soc: mediatek: cmdq: add read_s function > > soc: mediatek: cmdq: add write_s value function > > soc: mediatek: cmdq: add write_s_mask value function > > soc: mediatek: cmdq: export finalize function > > soc: mediatek: cmdq: add jump function > > soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api > > soc: mediatek: cmdq: add set event function > > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- > > drivers/soc/mediatek/mtk-cmdq-helper.c | 159 +-- > > include/linux/mailbox/mtk-cmdq-mailbox.h | 8 +- > > include/linux/soc/mediatek/mtk-cmdq.h| 124 +- > > 4 files changed, 280 insertions(+), 14 deletions(-) > > > >
Re: [PATCH v7 2/4] mailbox: cmdq: variablize address shift in platform
Hi Bibby,
Thanks for your comment.
On Mon, 2020-06-22 at 10:33 +0800, Bibby Hsieh wrote:
> On Sun, 2020-06-21 at 21:22 +0800, Dennis YC Hsieh wrote:
> > Some gce hardware shift pc and end address in register to support
> > large dram addressing.
> > Implement gce address shift when write or read pc and end register.
> > And add shift bit in platform definition.
> >
> > Signed-off-by: Dennis YC Hsieh
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 61
> > ++
> > include/linux/mailbox/mtk-cmdq-mailbox.h |2 +
> > 2 files changed, 48 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> > b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 9a6ce9f5a7db..4dbee9258127 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -76,8 +76,22 @@ struct cmdq {
> > struct cmdq_thread *thread;
> > struct clk *clock;
> > boolsuspended;
> > + u8 shift_pa;
> > };
> >
> > +struct gce_plat {
> > + u32 thread_nr;
> > + u8 shift;
> > +};
> > +
> > +u8 cmdq_mbox_shift(struct mbox_chan *chan)
>
> How about rename this function as cmdq_get_shift_pa()?
ok, I'll rename it, thanks
Regards,
Dennis
>
>
> Bibby
>
> > +{
> > + struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
> > +
> > + return cmdq->shift_pa;
> > +}
> > +EXPORT_SYMBOL(cmdq_mbox_shift);
> > +
> > static int cmdq_thread_suspend(struct cmdq *cmdq, struct cmdq_thread
> > *thread)
> > {
> > u32 status;
> > @@ -183,7 +197,7 @@ static void cmdq_task_remove_wfe(struct cmdq_task *task)
> > for (i = 0; i < CMDQ_NUM_CMD(task->pkt); i++)
> > if (cmdq_command_is_wfe(base[i]))
> > base[i] = (u64)CMDQ_JUMP_BY_OFFSET << 32 |
> > - CMDQ_JUMP_PASS;
> > + CMDQ_JUMP_PASS >> task->cmdq->shift_pa;
> > dma_sync_single_for_device(dev, task->pa_base, task->pkt->cmd_buf_size,
> >DMA_TO_DEVICE);
> > }
> > @@ -221,13 +235,15 @@ static void cmdq_task_handle_error(struct cmdq_task
> > *task)
> > {
> > struct cmdq_thread *thread = task->thread;
> > struct cmdq_task *next_task;
> > + struct cmdq *cmdq = task->cmdq;
> >
> > - dev_err(task->cmdq->mbox.dev, "task 0x%p error\n", task);
> > - WARN_ON(cmdq_thread_suspend(task->cmdq, thread) < 0);
> > + dev_err(cmdq->mbox.dev, "task 0x%p error\n", task);
> > + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > next_task = list_first_entry_or_null(&thread->task_busy_list,
> > struct cmdq_task, list_entry);
> > if (next_task)
> > - writel(next_task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > + writel(next_task->pa_base >> cmdq->shift_pa,
> > + thread->base + CMDQ_THR_CURR_ADDR);
> > cmdq_thread_resume(thread);
> > }
> >
> > @@ -257,7 +273,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> > else
> > return;
> >
> > - curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR);
> > + curr_pa = readl(thread->base + CMDQ_THR_CURR_ADDR) << cmdq->shift_pa;
> >
> > list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > list_entry) {
> > @@ -373,16 +389,20 @@ static int cmdq_mbox_send_data(struct mbox_chan
> > *chan, void *data)
> > WARN_ON(clk_enable(cmdq->clock) < 0);
> > WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> >
> > - writel(task->pa_base, thread->base + CMDQ_THR_CURR_ADDR);
> > - writel(task->pa_base + pkt->cmd_buf_size,
> > + writel(task->pa_base >> cmdq->shift_pa,
> > + thread->base + CMDQ_THR_CURR_ADDR);
> > + writel((task->pa_base + pkt->cmd_buf_size) >> cmdq->shift_pa,
> >thread->base + CMDQ_THR_END_ADDR);
> > +
> > writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> > writel(CMDQ_THR_IRQ_EN, thread->base + CMDQ_THR_IRQ_ENABLE);
> > writel(CMDQ_THR_ENABLED, thread->base + CMDQ_THR_ENABL
[PATCH v1 02/11] soc: mediatek: cmdq: add assign function
Add assign function in cmdq helper which assign constant value into
internal register by index.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 24 +++-
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 14 ++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 98f23ba3ba47..bf32e3b2ca6c 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -12,6 +12,7 @@
#define CMDQ_WRITE_ENABLE_MASK BIT(0)
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
+#define CMDQ_REG_TYPE 1
struct cmdq_instruction {
union {
@@ -21,8 +22,17 @@ struct cmdq_instruction {
union {
u16 offset;
u16 event;
+ u16 reg_dst;
+ };
+ union {
+ u8 subsys;
+ struct {
+ u8 sop:5;
+ u8 arg_c_t:1;
+ u8 src_t:1;
+ u8 dst_t:1;
+ };
};
- u8 subsys;
u8 op;
};
@@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_poll_mask);
+int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_LOGIC;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.reg_dst = reg_idx;
+ inst.value = value;
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_assign);
+
static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index dfe5b2eb85cc..121c3bb6d3de 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_LOGIC = 0xa0,
};
enum cmdq_cb_status {
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index a74c1d5acdf3..83340211e1d3 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
*/
int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+
+/**
+ * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
+ *to execute an instruction that set a constant value into
+ *internal register and use as value, mask or address in
+ *read/write instruction.
+ * @pkt: the CMDQ packet
+ * @reg_idx: the CMDQ internal register ID
+ * @value: the specified value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
+
/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
* packet and call back at the end of done packet
--
1.7.9.5
[PATCH v1 04/11] soc: mediatek: cmdq: add write_s_mask function
add write_s_mask function in cmdq helper functions which
writes value contains in internal register to address
with mask and large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 18 ++
3 files changed, 42 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 817a5a97dbe5..13b888779093 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -241,6 +241,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s);
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.mask = 0;
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index ee67dd3b86f5..8ef87e1bd03b 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
CMDQ_CODE_WRITE_S = 0x90,
+ CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index e1c5a7549b4f..ca9c75fd8125 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -122,6 +122,24 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx);
/**
+ * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ
packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ * @mask: the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v1 03/11] soc: mediatek: cmdq: add write_s function
add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 19 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 19 +++
3 files changed, 39 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index bf32e3b2ca6c..817a5a97dbe5 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -18,6 +18,10 @@ struct cmdq_instruction {
union {
u32 value;
u32 mask;
+ struct {
+ u16 arg_c;
+ u16 src_reg;
+ };
};
union {
u16 offset;
@@ -222,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.src_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.src_reg = src_reg_idx;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 121c3bb6d3de..ee67dd3b86f5 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_LOGIC = 0xa0,
};
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 83340211e1d3..e1c5a7549b4f 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -12,6 +12,8 @@
#include
#define CMDQ_NO_TIMEOUT0xu
+#define CMDQ_ADDR_HIGH(addr) ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
struct cmdq_pkt;
@@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
/**
+ * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @src_reg_idx: the CMDQ internal register ID which cache source value
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v1 01/11] soc: mediatek: cmdq: add address shift in jump
Add address shift when compose jump instruction to compatible with 35bit format. Signed-off-by: Dennis YC Hsieh --- drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index c67081759728..98f23ba3ba47 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -291,7 +291,8 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) /* JUMP to end */ inst.op = CMDQ_CODE_JUMP; - inst.value = CMDQ_JUMP_PASS; + inst.value = CMDQ_JUMP_PASS >> + cmdq_mbox_shift(((struct cmdq_client *)pkt->cl)->chan); err = cmdq_pkt_append_command(pkt, inst); return err; -- 1.7.9.5
[PATCH v1 11/11] soc: mediatek: cmdq: add set event function
Add set event function in cmdq helper functions to set specific event.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h|9 +
3 files changed, 25 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 13f78c9b5901..e6133a42d229 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -346,6 +346,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
}
EXPORT_SYMBOL(cmdq_pkt_clear_event);
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
+{
+ struct cmdq_instruction inst = {};
+
+ if (event >= CMDQ_MAX_EVENT)
+ return -EINVAL;
+
+ inst.op = CMDQ_CODE_WFE;
+ inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
+ inst.event = event;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_set_event);
+
int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 42d2a30e6a70..ba2d811183a9 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -17,6 +17,7 @@
#define CMDQ_JUMP_PASS CMDQ_INST_SIZE
#define CMDQ_WFE_UPDATEBIT(31)
+#define CMDQ_WFE_UPDATE_VALUE BIT(16)
#define CMDQ_WFE_WAIT BIT(15)
#define CMDQ_WFE_WAIT_VALUE0x1
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 4b5f5d154bad..960704d75994 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -199,6 +199,15 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
/**
+ * cmdq_pkt_set_event() - append set event command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @event: the desired event to be set
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
+
+/**
* cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
* execute an instruction that wait for a specified
* hardware register to check for the value w/o mask.
--
1.7.9.5
[PATCH v1 10/11] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
Add clear parameter to let client decide if
event should be clear to 0 after GCE receive it.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
drivers/soc/mediatek/mtk-cmdq-helper.c |5 +++--
include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 7daaabc26eb1..a065b3a412cf 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -488,7 +488,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
if (mtk_crtc->cmdq_client) {
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
- cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
+ cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 009f86ae72c6..13f78c9b5901 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -315,15 +315,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
{
struct cmdq_instruction inst = { {0} };
+ u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
if (event >= CMDQ_MAX_EVENT)
return -EINVAL;
inst.op = CMDQ_CODE_WFE;
- inst.value = CMDQ_WFE_OPTION;
+ inst.value = CMDQ_WFE_OPTION | clear_option;
inst.event = event;
return cmdq_pkt_append_command(pkt, inst);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 3f6bc0dfd5da..42d2a30e6a70 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -27,8 +27,7 @@
* bit 16-27: update value
* bit 31: 1 - update, 0 - no update
*/
-#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE |
CMDQ_WFE_WAIT | \
- CMDQ_WFE_WAIT_VALUE)
+#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT |
CMDQ_WFE_WAIT_VALUE)
/** cmdq event maximum */
#define CMDQ_MAX_EVENT 0x3ff
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 18364d81e8f7..4b5f5d154bad 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
- * @event: the desired event type to "wait and CLEAR"
+ * @event: the desired event type to wait
+ * @clear: clear event or not after event arrive
*
* Return: 0 for success; else the error code is returned
*/
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
/**
* cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
--
1.7.9.5
[PATCH v1 05/11] soc: mediatek: cmdq: add read_s function
Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++
include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
include/linux/soc/mediatek/mtk-cmdq.h| 12
3 files changed, 28 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 13b888779093..58075589509b 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -226,6 +226,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_write_mask);
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_READ_S;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = high_addr_reg_idx;
+ inst.reg_dst = reg_idx;
+ inst.src_reg = addr_low;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 8ef87e1bd03b..3f6bc0dfd5da 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -59,6 +59,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+ CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index ca9c75fd8125..40fe1eb52190 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -104,6 +104,18 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev,
int index,
int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/*
+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @reg_idx: the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+ u16 reg_idx);
+
/**
* cmdq_pkt_write_s() - append write_s command to the CMDQ packet
* @pkt: the CMDQ packet
--
1.7.9.5
[PATCH v1 07/11] soc: mediatek: cmdq: add write_s_mask value function
add write_s_mask_value function in cmdq helper functions which
writes a constant value to address with mask and large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 21 +
include/linux/soc/mediatek/mtk-cmdq.h | 15 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 2ad78df46636..e372ae065240 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -293,6 +293,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask)
+{
+ struct cmdq_instruction inst = {};
+ int err;
+
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_WRITE_S_MASK;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 7f1c115a66b8..6e8caacedc80 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -165,6 +165,21 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8
high_addr_reg_idx,
u16 addr_low, u32 value);
/**
+ * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ
+ *packet which write value to a physical
+ *address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ * @mask: the specified target mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value, u32 mask);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v1 09/11] soc: mediatek: cmdq: add jump function
Add jump function so that client can jump to any address which
contains instruction.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 13 +
include/linux/soc/mediatek/mtk-cmdq.h | 11 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 248945108a36..009f86ae72c6 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -13,6 +13,7 @@
#define CMDQ_POLL_ENABLE_MASK BIT(0)
#define CMDQ_EOC_IRQ_ENBIT(0)
#define CMDQ_REG_TYPE 1
+#define CMDQ_JUMP_RELATIVE 1
struct cmdq_instruction {
union {
@@ -391,6 +392,18 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_JUMP;
+ inst.offset = CMDQ_JUMP_RELATIVE;
+ inst.value = addr >>
+ cmdq_mbox_shift(((struct cmdq_client *)pkt->cl)->chan);
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_jump);
+
int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index eac1405e4872..18364d81e8f7 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -244,6 +244,17 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
/**
+ * cmdq_pkt_jump() - Append jump command to the CMDQ packet, ask GCE
+ * to execute an instruction that change current thread PC to
+ * a physical address which should contains more instruction.
+ * @pkt:the CMDQ packet
+ * @addr: physical address of target instruction buffer
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr);
+
+/**
* cmdq_pkt_finalize() - Append EOC and jump command to pkt.
* @pkt: the CMDQ packet
*
--
1.7.9.5
[PATCH v1 06/11] soc: mediatek: cmdq: add write_s value function
add write_s function in cmdq helper functions which
writes a constant value to address with large dma
access support.
Signed-off-by: Dennis YC Hsieh
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 14 ++
include/linux/soc/mediatek/mtk-cmdq.h | 13 +
2 files changed, 27 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 58075589509b..2ad78df46636 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -279,6 +279,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
}
EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value)
+{
+ struct cmdq_instruction inst = {};
+
+ inst.op = CMDQ_CODE_WRITE_S;
+ inst.sop = high_addr_reg_idx;
+ inst.offset = addr_low;
+ inst.value = value;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+
int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
{
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 40fe1eb52190..7f1c115a66b8 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,19 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16
high_addr_reg_idx,
u16 addr_low, u16 src_reg_idx, u32 mask);
/**
+ * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which
+ * write value to a physical address
+ * @pkt: the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low: low address of pa
+ * @value: the specified target value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+ u16 addr_low, u32 value);
+
+/**
* cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
* @pkt: the CMDQ packet
* @event: the desired event type to "wait and CLEAR"
--
1.7.9.5
[PATCH v1 08/11] soc: mediatek: cmdq: export finalize function
Export finalize function to client which helps append eoc and jump
command to pkt. Let client decide call finalize or not.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Acked-by: Chun-Kuang Hu
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c |1 +
drivers/soc/mediatek/mtk-cmdq-helper.c |7 ++-
include/linux/soc/mediatek/mtk-cmdq.h |8
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 0dfcd1787e65..7daaabc26eb1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc
*mtk_crtc)
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
mtk_crtc_ddp_config(crtc, cmdq_handle);
+ cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
}
#endif
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index e372ae065240..248945108a36 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -391,7 +391,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32
value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
-static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
int err;
@@ -411,6 +411,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
return err;
}
+EXPORT_SYMBOL(cmdq_pkt_finalize);
static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
{
@@ -445,10 +446,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt,
cmdq_async_flush_cb cb,
unsigned long flags = 0;
struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
- err = cmdq_pkt_finalize(pkt);
- if (err < 0)
- return err;
-
pkt->cb.cb = cb;
pkt->cb.data = data;
pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
b/include/linux/soc/mediatek/mtk-cmdq.h
index 6e8caacedc80..eac1405e4872 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -244,6 +244,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
/**
+ * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
+ * @pkt: the CMDQ packet
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
+
+/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
* packet and call back at the end of done packet
* @pkt: the CMDQ packet
--
1.7.9.5
[PATCH v1 0/11] support cmdq helper function on mt6779 platform
This patch support cmdq helper function on mt6779 platform, based on "support gce on mt6779 platform" patchset. Dennis YC Hsieh (11): soc: mediatek: cmdq: add address shift in jump soc: mediatek: cmdq: add assign function soc: mediatek: cmdq: add write_s function soc: mediatek: cmdq: add write_s_mask function soc: mediatek: cmdq: add read_s function soc: mediatek: cmdq: add write_s value function soc: mediatek: cmdq: add write_s_mask value function soc: mediatek: cmdq: export finalize function soc: mediatek: cmdq: add jump function soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api soc: mediatek: cmdq: add set event function drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +- drivers/soc/mediatek/mtk-cmdq-helper.c | 159 +-- include/linux/mailbox/mtk-cmdq-mailbox.h | 8 +- include/linux/soc/mediatek/mtk-cmdq.h| 124 +- 4 files changed, 280 insertions(+), 14 deletions(-) -- 2.18.0
[PATCH v7 3/4] mailbox: cmdq: support mt6779 gce platform definition
Add gce v4 hardware support with different thread number and shift.
Signed-off-by: Dennis YC Hsieh
Reviewed-by: CK Hu
Reviewed-by: Matthias Brugger
---
drivers/mailbox/mtk-cmdq-mailbox.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index 4dbee9258127..9994ac9426d6 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -572,10 +572,12 @@ static int cmdq_probe(struct platform_device *pdev)
static const struct gce_plat gce_plat_v2 = {.thread_nr = 16};
static const struct gce_plat gce_plat_v3 = {.thread_nr = 24};
+static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3};
static const struct of_device_id cmdq_of_ids[] = {
{.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2},
{.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3},
+ {.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4},
{}
};
--
1.7.9.5
