target/linux/generic/backport-5.15/830-v6.7-2-leds-turris-omnia-Make-set_brightness-more-efficient.patch

   1 From 4d5ed2621c2437d40b8fe92bd0fd34b0b1870753 Mon Sep 17 00:00:00 2001
   2 From: Marek Behún <kabel@kernel.org>
   3 Date: Mon, 18 Sep 2023 18:11:02 +0200
   4 Subject: leds: turris-omnia: Make set_brightness() more efficient
   5 MIME-Version: 1.0
   6 Content-Type: text/plain; charset=UTF-8
   7 Content-Transfer-Encoding: 8bit
   8
   9 Implement caching of the LED color and state values that are sent to MCU
  10 in order to make the set_brightness() operation more efficient by
  11 avoiding I2C transactions which are not needed.
  12
  13 On Turris Omnia's MCU, which acts as the RGB LED controller, each LED
  14 has a RGB color, and a ON/OFF state, which are configurable via I2C
  15 commands CMD_LED_COLOR and CMD_LED_STATE.
  16
  17 The CMD_LED_COLOR command sends 5 bytes and the CMD_LED_STATE command 2
  18 bytes over the I2C bus, which operates at 100 kHz. With I2C overhead
  19 this allows ~1670 color changing commands and ~3200 state changing
  20 commands per second (or around 1000 color + state changes per second).
  21 This may seem more than enough, but the issue is that the I2C bus is
  22 shared with another peripheral, the MCU. The MCU exposes an interrupt
  23 interface, and it can trigger hundreds of interrupts per second. Each
  24 time, we need to read the interrupt state register over this I2C bus.
  25 Whenever we are sending a LED color/state changing command, the
  26 interrupt reading is waiting.
  27
  28 Currently, every time LED brightness or LED multi intensity is changed,
  29 we send a CMD_LED_STATE command, and if the computed color (brightness
  30 adjusted multi_intensity) is non-zero, we also send a CMD_LED_COLOR
  31 command.
  32
  33 Consider for example the situation when we have a netdev trigger enabled
  34 for a LED. The netdev trigger does not change the LED color, only the
  35 brightness (either to 0 or to currently configured brightness), and so
  36 there is no need to send the CMD_LED_COLOR command. But each change of
  37 brightness to 0 sends one CMD_LED_STATE command, and each change of
  38 brightness to max_brightness sends one CMD_LED_STATE command and one
  39 CMD_LED_COLOR command:
  40     set_brightness(0)   ->  CMD_LED_STATE
  41     set_brightness(255) ->  CMD_LED_STATE + CMD_LED_COLOR
  42                                             (unnecessary)
  43
  44 We can avoid the unnecessary I2C transactions if we cache the values of
  45 state and color that are sent to the controller. If the color does not
  46 change from the one previously sent, there is no need to do the
  47 CMD_LED_COLOR I2C transaction, and if the state does not change, there
  48 is no need to do the CMD_LED_STATE transaction.
  49
  50 Because we need to make sure that our cached values are consistent with
  51 the controller state, add explicit setting of the LED color to white at
  52 probe time (this is the default setting when MCU resets, but does not
  53 necessarily need to be the case, for example if U-Boot played with the
  54 LED colors).
  55
  56 Signed-off-by: Marek Behún <kabel@kernel.org>
  57 Link: https://lore.kernel.org/r/20230918161104.20860-3-kabel@kernel.org
  58 Signed-off-by: Lee Jones <lee@kernel.org>
  59 ---
  60  drivers/leds/leds-turris-omnia.c | 96 ++++++++++++++++++++++++++++++++--------
  61  1 file changed, 78 insertions(+), 18 deletions(-)
  62
  63 --- a/drivers/leds/leds-turris-omnia.c
  64 +++ b/drivers/leds/leds-turris-omnia.c
  65 @@ -30,6 +30,8 @@
  66  struct omnia_led {
  67         struct led_classdev_mc mc_cdev;
  68         struct mc_subled subled_info[OMNIA_LED_NUM_CHANNELS];
  69 +       u8 cached_channels[OMNIA_LED_NUM_CHANNELS];
  70 +       bool on;
  71         int reg;
  72  };
  73
  74 @@ -72,36 +74,82 @@ static int omnia_cmd_read_u8(const struc
  75                 return -EIO;
  76  }
  77
  78 +static int omnia_led_send_color_cmd(const struct i2c_client *client,
  79 +                                   struct omnia_led *led)
  80 +{
  81 +       char cmd[5];
  82 +       int ret;
  83 +
  84 +       cmd[0] = CMD_LED_COLOR;
  85 +       cmd[1] = led->reg;
  86 +       cmd[2] = led->subled_info[0].brightness;
  87 +       cmd[3] = led->subled_info[1].brightness;
  88 +       cmd[4] = led->subled_info[2].brightness;
  89 +
  90 +       /* Send the color change command */
  91 +       ret = i2c_master_send(client, cmd, 5);
  92 +       if (ret < 0)
  93 +               return ret;
  94 +
  95 +       /* Cache the RGB channel brightnesses */
  96 +       for (int i = 0; i < OMNIA_LED_NUM_CHANNELS; ++i)
  97 +               led->cached_channels[i] = led->subled_info[i].brightness;
  98 +
  99 +       return 0;
 100 +}
 101 +
 102 +/* Determine if the computed RGB channels are different from the cached ones */
 103 +static bool omnia_led_channels_changed(struct omnia_led *led)
 104 +{
 105 +       for (int i = 0; i < OMNIA_LED_NUM_CHANNELS; ++i)
 106 +               if (led->subled_info[i].brightness != led->cached_channels[i])
 107 +                       return true;
 108 +
 109 +       return false;
 110 +}
 111 +
 112  static int omnia_led_brightness_set_blocking(struct led_classdev *cdev,
 113                                              enum led_brightness brightness)
 114  {
 115         struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
 116         struct omnia_leds *leds = dev_get_drvdata(cdev->dev->parent);
 117         struct omnia_led *led = to_omnia_led(mc_cdev);
 118 -       u8 buf[5], state;
 119 -       int ret;
 120 +       int err = 0;
 121
 122         mutex_lock(&leds->lock);
 123
 124 -       led_mc_calc_color_components(&led->mc_cdev, brightness);
 125 +       /*
 126 +        * Only recalculate RGB brightnesses from intensities if brightness is
 127 +        * non-zero. Otherwise we won't be using them and we can save ourselves
 128 +        * some software divisions (Omnia's CPU does not implement the division
 129 +        * instruction).
 130 +        */
 131 +       if (brightness) {
 132 +               led_mc_calc_color_components(mc_cdev, brightness);
 133 +
 134 +               /*
 135 +                * Send color command only if brightness is non-zero and the RGB
 136 +                * channel brightnesses changed.
 137 +                */
 138 +               if (omnia_led_channels_changed(led))
 139 +                       err = omnia_led_send_color_cmd(leds->client, led);
 140 +       }
 141
 142 -       buf[0] = CMD_LED_COLOR;
 143 -       buf[1] = led->reg;
 144 -       buf[2] = mc_cdev->subled_info[0].brightness;
 145 -       buf[3] = mc_cdev->subled_info[1].brightness;
 146 -       buf[4] = mc_cdev->subled_info[2].brightness;
 147 -
 148 -       state = CMD_LED_STATE_LED(led->reg);
 149 -       if (buf[2] || buf[3] || buf[4])
 150 -               state |= CMD_LED_STATE_ON;
 151 -
 152 -       ret = omnia_cmd_write_u8(leds->client, CMD_LED_STATE, state);
 153 -       if (ret >= 0 && (state & CMD_LED_STATE_ON))
 154 -               ret = i2c_master_send(leds->client, buf, 5);
 155 +       /* Send on/off state change only if (bool)brightness changed */
 156 +       if (!err && !brightness != !led->on) {
 157 +               u8 state = CMD_LED_STATE_LED(led->reg);
 158 +
 159 +               if (brightness)
 160 +                       state |= CMD_LED_STATE_ON;
 161 +
 162 +               err = omnia_cmd_write_u8(leds->client, CMD_LED_STATE, state);
 163 +               if (!err)
 164 +                       led->on = !!brightness;
 165 +       }
 166
 167         mutex_unlock(&leds->lock);
 168
 169 -       return ret;
 170 +       return err;
 171  }
 172
 173  static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
 174 @@ -129,11 +177,15 @@ static int omnia_led_register(struct i2c
 175         }
 176
 177         led->subled_info[0].color_index = LED_COLOR_ID_RED;
 178 -       led->subled_info[0].channel = 0;
 179         led->subled_info[1].color_index = LED_COLOR_ID_GREEN;
 180 -       led->subled_info[1].channel = 1;
 181         led->subled_info[2].color_index = LED_COLOR_ID_BLUE;
 182 -       led->subled_info[2].channel = 2;
 183 +
 184 +       /* Initial color is white */
 185 +       for (int i = 0; i < OMNIA_LED_NUM_CHANNELS; ++i) {
 186 +               led->subled_info[i].intensity = 255;
 187 +               led->subled_info[i].brightness = 255;
 188 +               led->subled_info[i].channel = i;
 189 +       }
 190
 191         led->mc_cdev.subled_info = led->subled_info;
 192         led->mc_cdev.num_colors = OMNIA_LED_NUM_CHANNELS;
 193 @@ -162,6 +214,14 @@ static int omnia_led_register(struct i2c
 194                 return ret;
 195         }
 196
 197 +       /* Set initial color and cache it */
 198 +       ret = omnia_led_send_color_cmd(client, led);
 199 +       if (ret < 0) {
 200 +               dev_err(dev, "Cannot set LED %pOF initial color: %i\n", np,
 201 +                       ret);
 202 +               return ret;
 203 +       }
 204 +
 205         ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc_cdev,
 206                                                         &init_data);
 207         if (ret < 0) {