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

   1 From 91dc4d4202c571edfcbc9357642609b07a9cbee8 Mon Sep 17 00:00:00 2001
   2 From: Maxime Ripard <maxime@cerno.tech>
   3 Date: Wed, 23 Nov 2022 16:25:52 +0100
   4 Subject: [PATCH] drm/vc4: kms: Sort the CRTCs by output before
   5  assigning them
   6
   7 On the vc4 devices (and later), the blending is done by a single device
   8 called the HVS. The HVS has three FIFO that can operate in parallel, and
   9 route their output to 6 CRTCs and 7 encoders on the BCM2711.
  10
  11 Each of these CRTCs and encoders have some constraints on which FIFO
  12 they can feed from, so we need some code to take all those constraints
  13 into account and assign FIFOs to CRTCs.
  14
  15 The problem can be simplified by assigning those FIFOs to CRTCs by
  16 ascending output index number. We had a comment mentioning it already,
  17 but we were never actually enforcing it.
  18
  19 It was working still in most situations because the probe order is
  20 roughly equivalent, except for the (optional, and fairly rarely used on
  21 the Pi4) VEC which was last in the probe order sequence, but one of the
  22 earliest device to assign.
  23
  24 This resulted in configurations that were rejected by our code but were
  25 still valid with a different assignment.
  26
  27 We can fix this by making sure we assign CRTCs to FIFOs by ordering
  28 them by ascending HVS output index.
  29
  30 Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically")
  31 Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
  32 Signed-off-by: Maxime Ripard <maxime@cerno.tech>
  33 Link: https://patchwork.freedesktop.org/patch/msgid/20221123-rpi-kunit-tests-v1-10-051a0bb60a16@cerno.tech
  34 ---
  35  drivers/gpu/drm/vc4/vc4_kms.c | 109 ++++++++++++++++++++++++----------
  36  1 file changed, 76 insertions(+), 33 deletions(-)
  37
  38 --- a/drivers/gpu/drm/vc4/vc4_kms.c
  39 +++ b/drivers/gpu/drm/vc4/vc4_kms.c
  40 @@ -12,6 +12,7 @@
  41   */
  42
  43  #include <linux/clk.h>
  44 +#include <linux/sort.h>
  45
  46  #include <drm/drm_atomic.h>
  47  #include <drm/drm_atomic_helper.h>
  48 @@ -791,6 +792,20 @@ static int vc4_hvs_channels_obj_init(str
  49         return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL);
  50  }
  51
  52 +static int cmp_vc4_crtc_hvs_output(const void *a, const void *b)
  53 +{
  54 +       const struct vc4_crtc *crtc_a =
  55 +               to_vc4_crtc(*(const struct drm_crtc **)a);
  56 +       const struct vc4_crtc_data *data_a =
  57 +               vc4_crtc_to_vc4_crtc_data(crtc_a);
  58 +       const struct vc4_crtc *crtc_b =
  59 +               to_vc4_crtc(*(const struct drm_crtc **)b);
  60 +       const struct vc4_crtc_data *data_b =
  61 +               vc4_crtc_to_vc4_crtc_data(crtc_b);
  62 +
  63 +       return data_a->hvs_output - data_b->hvs_output;
  64 +}
  65 +
  66  /*
  67   * The BCM2711 HVS has up to 7 outputs connected to the pixelvalves and
  68   * the TXP (and therefore all the CRTCs found on that platform).
  69 @@ -826,10 +841,11 @@ static int vc4_pv_muxing_atomic_check(st
  70  {
  71         struct vc4_dev *vc4 = to_vc4_dev(state->dev);
  72         struct vc4_hvs_state *hvs_new_state;
  73 -       struct drm_crtc_state *old_crtc_state, *new_crtc_state;
  74 +       struct drm_crtc **sorted_crtcs;
  75         struct drm_crtc *crtc;
  76         unsigned int unassigned_channels = 0;
  77         unsigned int i;
  78 +       int ret;
  79
  80         hvs_new_state = vc4_hvs_get_global_state(state);
  81         if (IS_ERR(hvs_new_state))
  82 @@ -839,18 +855,62 @@ static int vc4_pv_muxing_atomic_check(st
  83                 if (!hvs_new_state->fifo_state[i].in_use)
  84                         unassigned_channels |= BIT(i);
  85
  86 -       for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
  87 -               struct vc4_crtc_state *old_vc4_crtc_state =
  88 -                       to_vc4_crtc_state(old_crtc_state);
  89 -               struct vc4_crtc_state *new_vc4_crtc_state =
  90 -                       to_vc4_crtc_state(new_crtc_state);
  91 -               struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
  92 +       /*
  93 +        * The problem we have to solve here is that we have up to 7
  94 +        * encoders, connected to up to 6 CRTCs.
  95 +        *
  96 +        * Those CRTCs, depending on the instance, can be routed to 1, 2
  97 +        * or 3 HVS FIFOs, and we need to set the muxing between FIFOs and
  98 +        * outputs in the HVS accordingly.
  99 +        *
 100 +        * It would be pretty hard to come up with an algorithm that
 101 +        * would generically solve this. However, the current routing
 102 +        * trees we support allow us to simplify a bit the problem.
 103 +        *
 104 +        * Indeed, with the current supported layouts, if we try to
 105 +        * assign in the ascending crtc index order the FIFOs, we can't
 106 +        * fall into the situation where an earlier CRTC that had
 107 +        * multiple routes is assigned one that was the only option for
 108 +        * a later CRTC.
 109 +        *
 110 +        * If the layout changes and doesn't give us that in the future,
 111 +        * we will need to have something smarter, but it works so far.
 112 +        */
 113 +       sorted_crtcs = kmalloc_array(dev->num_crtcs, sizeof(*sorted_crtcs), GFP_KERNEL);
 114 +       if (!sorted_crtcs)
 115 +               return -ENOMEM;
 116 +
 117 +       i = 0;
 118 +       drm_for_each_crtc(crtc, dev)
 119 +               sorted_crtcs[i++] = crtc;
 120 +
 121 +       sort(sorted_crtcs, i, sizeof(*sorted_crtcs), cmp_vc4_crtc_hvs_output, NULL);
 122 +
 123 +       for (i = 0; i < dev->num_crtcs; i++) {
 124 +               struct vc4_crtc_state *old_vc4_crtc_state, *new_vc4_crtc_state;
 125 +               struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 126 +               struct vc4_crtc *vc4_crtc;
 127                 unsigned int matching_channels;
 128                 unsigned int channel;
 129
 130                 if (vc4->firmware_kms)
 131                         continue;
 132
 133 +               crtc = sorted_crtcs[i];
 134 +               if (!crtc)
 135 +                       continue;
 136 +               vc4_crtc = to_vc4_crtc(crtc);
 137 +
 138 +               old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc);
 139 +               if (!old_crtc_state)
 140 +                       continue;
 141 +               old_vc4_crtc_state = to_vc4_crtc_state(old_crtc_state);
 142 +
 143 +               new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 144 +               if (!new_crtc_state)
 145 +                       continue;
 146 +               new_vc4_crtc_state = to_vc4_crtc_state(new_crtc_state);
 147 +
 148                 drm_dbg(dev, "%s: Trying to find a channel.\n", crtc->name);
 149
 150                 /* Nothing to do here, let's skip it */
 151 @@ -879,33 +939,11 @@ static int vc4_pv_muxing_atomic_check(st
 152                         continue;
 153                 }
 154
 155 -               /*
 156 -                * The problem we have to solve here is that we have
 157 -                * up to 7 encoders, connected to up to 6 CRTCs.
 158 -                *
 159 -                * Those CRTCs, depending on the instance, can be
 160 -                * routed to 1, 2 or 3 HVS FIFOs, and we need to set
 161 -                * the change the muxing between FIFOs and outputs in
 162 -                * the HVS accordingly.
 163 -                *
 164 -                * It would be pretty hard to come up with an
 165 -                * algorithm that would generically solve
 166 -                * this. However, the current routing trees we support
 167 -                * allow us to simplify a bit the problem.
 168 -                *
 169 -                * Indeed, with the current supported layouts, if we
 170 -                * try to assign in the ascending crtc index order the
 171 -                * FIFOs, we can't fall into the situation where an
 172 -                * earlier CRTC that had multiple routes is assigned
 173 -                * one that was the only option for a later CRTC.
 174 -                *
 175 -                * If the layout changes and doesn't give us that in
 176 -                * the future, we will need to have something smarter,
 177 -                * but it works so far.
 178 -                */
 179                 matching_channels = unassigned_channels & vc4_crtc->data->hvs_available_channels;
 180 -               if (!matching_channels)
 181 -                       return -EINVAL;
 182 +               if (!matching_channels) {
 183 +                       ret = -EINVAL;
 184 +                       goto err_free_crtc_array;
 185 +               }
 186
 187                 channel = ffs(matching_channels) - 1;
 188
 189 @@ -915,7 +953,12 @@ static int vc4_pv_muxing_atomic_check(st
 190                 hvs_new_state->fifo_state[channel].in_use = true;
 191         }
 192
 193 +       kfree(sorted_crtcs);
 194         return 0;
 195 +
 196 +err_free_crtc_array:
 197 +       kfree(sorted_crtcs);
 198 +       return ret;
 199  }
 200
 201  static int