152 lines
4.2 KiB
Diff
152 lines
4.2 KiB
Diff
|
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] =
|