git.openwrt.org Git - openwrt/staging/linusw.git/blob

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