1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
|
From e3e645fde7d3141767c52a1ae0ffd2f94d0046a4 Mon Sep 17 00:00:00 2001
From: John Cox <jc@kynesim.co.uk>
Date: Thu, 24 Jun 2021 14:43:49 +0100
Subject: [PATCH] media: rpivid: Fix H265 aux ent reuse of the same
slot
It is legitimate, though unusual, for an aux ent associated with a slot
to be selected in phase 0 before a previous selection has been used and
released in phase 2. Fix such that if the slot is found to be in use
that the aux ent associated with it is reused rather than an new aux
ent being created. This fixes a problem where when the first aux ent
was released the second was lost track of.
This bug spotted in Nick's testing. It may explain some other occasional,
unreliable decode error reports where dmesg included "Missing DPB AUX
ent" logging.
Signed-off-by: John Cox <jc@kynesim.co.uk>
---
drivers/staging/media/rpivid/rpivid_h265.c | 75 ++++++++++++++--------
1 file changed, 49 insertions(+), 26 deletions(-)
--- a/drivers/staging/media/rpivid/rpivid_h265.c
+++ b/drivers/staging/media/rpivid/rpivid_h265.c
@@ -371,7 +371,8 @@ static void aux_q_free(struct rpivid_ctx
kfree(aq);
}
-static struct rpivid_q_aux *aux_q_alloc(struct rpivid_ctx *const ctx)
+static struct rpivid_q_aux *aux_q_alloc(struct rpivid_ctx *const ctx,
+ const unsigned int q_index)
{
struct rpivid_dev *const dev = ctx->dev;
struct rpivid_q_aux *const aq = kzalloc(sizeof(*aq), GFP_KERNEL);
@@ -379,11 +380,17 @@ static struct rpivid_q_aux *aux_q_alloc(
if (!aq)
return NULL;
- aq->refcount = 1;
if (gptr_alloc(dev, &aq->col, ctx->colmv_picsize,
DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_KERNEL_MAPPING))
goto fail;
+ /*
+ * Spinlock not required as called in P0 only and
+ * aux checks done by _new
+ */
+ aq->refcount = 1;
+ aq->q_index = q_index;
+ ctx->aux_ents[q_index] = aq;
return aq;
fail:
@@ -398,22 +405,38 @@ static struct rpivid_q_aux *aux_q_new(st
unsigned long lockflags;
spin_lock_irqsave(&ctx->aux_lock, lockflags);
- aq = ctx->aux_free;
- if (aq) {
+ /*
+ * If we already have this allocated to a slot then use that
+ * and assume that it will all work itself out in the pipeline
+ */
+ if ((aq = ctx->aux_ents[q_index]) != NULL) {
+ ++aq->refcount;
+ } else if ((aq = ctx->aux_free) != NULL) {
ctx->aux_free = aq->next;
aq->next = NULL;
aq->refcount = 1;
+ aq->q_index = q_index;
+ ctx->aux_ents[q_index] = aq;
}
spin_unlock_irqrestore(&ctx->aux_lock, lockflags);
- if (!aq) {
- aq = aux_q_alloc(ctx);
- if (!aq)
- return NULL;
- }
+ if (!aq)
+ aq = aux_q_alloc(ctx, q_index);
+
+ return aq;
+}
+
+static struct rpivid_q_aux *aux_q_ref_idx(struct rpivid_ctx *const ctx,
+ const int q_index)
+{
+ unsigned long lockflags;
+ struct rpivid_q_aux *aq;
+
+ spin_lock_irqsave(&ctx->aux_lock, lockflags);
+ if ((aq = ctx->aux_ents[q_index]) != NULL)
+ ++aq->refcount;
+ spin_unlock_irqrestore(&ctx->aux_lock, lockflags);
- aq->q_index = q_index;
- ctx->aux_ents[q_index] = aq;
return aq;
}
@@ -436,21 +459,21 @@ static void aux_q_release(struct rpivid_
struct rpivid_q_aux **const paq)
{
struct rpivid_q_aux *const aq = *paq;
- *paq = NULL;
-
- if (aq) {
- unsigned long lockflags;
+ unsigned long lockflags;
- spin_lock_irqsave(&ctx->aux_lock, lockflags);
+ if (!aq)
+ return;
- if (--aq->refcount == 0) {
- aq->next = ctx->aux_free;
- ctx->aux_free = aq;
- ctx->aux_ents[aq->q_index] = NULL;
- }
+ *paq = NULL;
- spin_unlock_irqrestore(&ctx->aux_lock, lockflags);
+ spin_lock_irqsave(&ctx->aux_lock, lockflags);
+ if (--aq->refcount == 0) {
+ aq->next = ctx->aux_free;
+ ctx->aux_free = aq;
+ ctx->aux_ents[aq->q_index] = NULL;
+ aq->q_index = ~0U;
}
+ spin_unlock_irqrestore(&ctx->aux_lock, lockflags);
}
static void aux_q_init(struct rpivid_ctx *const ctx)
@@ -1958,12 +1981,12 @@ static void rpivid_h265_setup(struct rpi
}
if (use_aux) {
- dpb_q_aux[i] = aux_q_ref(ctx,
- ctx->aux_ents[buffer_index]);
+ dpb_q_aux[i] = aux_q_ref_idx(ctx, buffer_index);
if (!dpb_q_aux[i])
v4l2_warn(&dev->v4l2_dev,
- "Missing DPB AUX ent %d index=%d\n",
- i, buffer_index);
+ "Missing DPB AUX ent %d, timestamp=%lld, index=%d\n",
+ i, (long long)sh->dpb[i].timestamp,
+ buffer_index);
}
de->ref_addrs[i] =
|
