Skip to content
Snippets Groups Projects
  1. Aug 03, 2023
  2. Aug 02, 2023
  3. Aug 01, 2023
    • Douglas Anderson's avatar
      drm/panel: Add a way for other devices to follow panel state · de087416
      Douglas Anderson authored
      These days, it's fairly common to see panels that have touchscreens
      attached to them. The panel and the touchscreen can somewhat be
      thought of as totally separate devices and, historically, this is how
      Linux has treated them. However, treating them as separate isn't
      necessarily the best way to model the two devices, it was just that
      there was no better way. Specifically, there is little practical
      reason to have the touchscreen powered on when the panel is turned
      off, but if we model the devices separately we have no way to keep the
      two devices' power states in sync with each other.
      
      The issue described above makes it sound as if the problem here is
      just about efficiency. We're wasting power keeping the touchscreen
      powered up when the screen is off. While that's true, the problem can
      go deeper. Specifically, hardware designers see that there's no reason
      to have the touchscreen on while the screen is off and then build
      hardware assuming that software would never turn the touchscreen on
      while the screen is off.
      
      In the very simplest case of hardware designs like this, the
      touchscreen and the panel share some power rails. In most cases, this
      turns out not to be terrible and is, again, just a little less
      efficient. Specifically if we tell Linux that the touchscreen and the
      panel are using the same rails then Linux will keep the rails on when
      _either_ device is turned on. That ends to work OK-ish, but now if you
      turn the panel off not only will the touchscreen remain powered, but
      the power rails for the panel itself won't be switched off, burning
      extra power.
      
      The above two inefficiencies are _extra_ minor when you consider the
      fact that laptops rarely spend much time with the screen off. The main
      use case would be when an external screen (and presumably a power
      supply) is attached.
      
      Unfortunately, it gets worse from here. On sc7180-trogdor-homestar,
      for instance, the display's TCON (timing controller) sometimes crashes
      if you don't power cycle it whenever you stop and restart the video
      stream (like during a modeset). The touchscreen keeping the power
      rails on causes real problems. One proposal in the homestar timeframe
      was to move the touchscreen to an always-on rail, dedicating the main
      power rail to the panel. That caused _different_ problems as talked
      about in commit 557e05fa ("HID: i2c-hid: goodix: Stop tying the
      reset line to the regulator"). The end result of all of this was to
      add an extra regulator to the board, increasing cost.
      
      Recently, Cong Yang posted a patch [1] where things are even worse.
      The panel and touch controller on that system seem even more
      intimately tied together and really can't be thought of separately.
      
      To address this issue, let's start allowing devices to register
      themselves as "panel followers". These devices will get called after a
      panel has been powered on and before a panel is powered off. This
      makes the panel the primary device in charge of the power state, which
      matches how userspace uses it.
      
      The panel follower API should be fairly straightforward to use. The
      current code assumes that panel followers are using device tree and
      have a "panel" property pointing to the panel to follow. More
      flexibility and non-DT implementations could be added as needed.
      
      Right now, panel followers can follow the prepare/unprepare functions.
      There could be arguments made that, instead, they should follow
      enable/disable. I've chosen prepare/unprepare for now since those
      functions are guaranteed to power up/power down the panel and it seems
      better to start the process earlier.
      
      A bit of explaining about why this is a roll-your-own API instead of
      using something more standard:
      1. In standard APIs in Linux, parent devices are automatically powered
         on when a child needs power. Applying that here, it would mean that
         we'd force the panel on any time someone was listening to the
         touchscreen. That, unfortunately, would have broken homestar's need
         (if we hadn't changed the hardware, as per above) where the panel
         absolutely needs to be able to power cycle itself. While one could
         argue that homestar is broken hardware and we shouldn't have the
         API do backflips for it, _officially_ the eDP timing guidelines
         agree with homestar's needs and the panel power sequencing diagrams
         show power going off. It's nice to be able to support this.
      2. We could, conceibably, try to add a new flag to device_link causing
         the parent to be in charge of power. Then we could at least use
         normal pm_runtime APIs. This sounds great, except that we run into
         problems with initial probe. As talked about in the later patch
         ("HID: i2c-hid: Support being a panel follower") the initial power
         on of a panel follower might need to do things (like add
         sub-devices) that aren't allowed in a runtime_resume function.
      
      The above complexities explain why this API isn't using common
      functions. That being said, this patch is very small and
      self-contained, so if someone was later able to adapt it to using more
      common APIs while solving the above issues then that could happen in
      the future.
      
      [1] https://lore.kernel.org/r/20230519032316.3464732-1-yangcong5@huaqin.corp-partner.google.com
      
      
      
      Reviewed-by: default avatarMaxime Ripard <mripard@kernel.org>
      Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.3.Icd5f96342d2242051c754364f4bee13ef2b986d4@changeid
      de087416
    • Douglas Anderson's avatar
      drm/panel: Check for already prepared/enabled in drm_panel · d2aacaf0
      Douglas Anderson authored
      In a whole pile of panel drivers, we have code to make the
      prepare/unprepare/enable/disable callbacks behave as no-ops if they've
      already been called. It's silly to have this code duplicated
      everywhere. Add it to the core instead so that we can eventually
      delete it from all the drivers. Note: to get some idea of the
      duplicated code, try:
        git grep 'if.*>prepared' -- drivers/gpu/drm/panel
        git grep 'if.*>enabled' -- drivers/gpu/drm/panel
      
      NOTE: arguably, the right thing to do here is actually to skip this
      patch and simply remove all the extra checks from the individual
      drivers. Perhaps the checks were needed at some point in time in the
      past but maybe they no longer are? Certainly as we continue
      transitioning over to "panel_bridge" then we expect there to be much
      less variety in how these calls are made. When we're called as part of
      the bridge chain, things should be pretty simple. In fact, there was
      some discussion in the past about these checks [1], including a
      discussion about whether the checks were needed and whether the calls
      ought to be refcounted. At the time, I decided not to mess with it
      because it felt too risky.
      
      Looking closer at it now, I'm fairly certain that nothing in the
      existing codebase is expecting these calls to be refcounted. The only
      real question is whether someone is already doing something to ensure
      prepare()/unprepare() match and enabled()/disable() match. I would say
      that, even if there is something else ensuring that things match,
      there's enough complexity that adding an extra bool and an extra
      double-check here is a good idea. Let's add a drm_warn() to let people
      know that it's considered a minor error to take advantage of
      drm_panel's double-checking but we'll still make things work fine.
      
      We'll also add an entry to the official DRM todo list to remove the
      now pointless check from the panels after this patch lands and,
      eventually, fixup anyone who is triggering the new warning.
      
      [1] https://lore.kernel.org/r/20210416153909.v4.27.I502f2a92ddd36c3d28d014dd75e170c2d405a0a5@changeid
      
      
      
      Acked-by: default avatarNeil Armstrong <neil.armstrong@linaro.org>
      Reviewed-by: default avatarMaxime Ripard <mripard@kernel.org>
      Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230727101636.v4.2.I59b417d4c29151cc2eff053369ec4822b606f375@changeid
      d2aacaf0
  4. Jul 31, 2023
  5. Jul 29, 2023
  6. Jul 24, 2023
  7. Jul 20, 2023
  8. Jul 17, 2023
  9. Jul 12, 2023
  10. Jul 11, 2023
    • Petr Pavlu's avatar
      vmlinux.lds.h: Remove a reference to no longer used sections .text..refcount · 5fc52248
      Petr Pavlu authored
      
      Sections .text..refcount were previously used to hold an error path code
      for fast refcount overflow protection on x86, see commit 7a46ec0e
      ("locking/refcounts, x86/asm: Implement fast refcount overflow
      protection") and commit 564c9cc8 ("locking/refcounts, x86/asm: Use
      unique .text section for refcount exceptions").
      
      The code was replaced and removed in commit fb041bb7
      ("locking/refcount: Consolidate implementations of refcount_t") and no
      sections .text..refcount are present since then.
      
      Remove then a relic referencing these sections from TEXT_TEXT to avoid
      confusing people, like me. This is a non-functional change.
      
      Signed-off-by: default avatarPetr Pavlu <petr.pavlu@suse.com>
      Link: https://lore.kernel.org/r/20230711125054.9000-1-petr.pavlu@suse.com
      
      
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      5fc52248
    • Masami Hiramatsu (Google)'s avatar
      fprobe: Ensure running fprobe_exit_handler() finished before calling rethook_free() · 195b9cb5
      Masami Hiramatsu (Google) authored
      Ensure running fprobe_exit_handler() has finished before
      calling rethook_free() in the unregister_fprobe() so that caller can free
      the fprobe right after unregister_fprobe().
      
      unregister_fprobe() ensured that all running fprobe_entry/exit_handler()
      have finished by calling unregister_ftrace_function() which synchronizes
      RCU. But commit 5f810187 ("fprobe: Release rethook after the ftrace_ops
      is unregistered") changed to call rethook_free() after
      unregister_ftrace_function(). So call rethook_stop() to make rethook
      disabled before unregister_ftrace_function() and ensure it again.
      
      Here is the possible code flow that can call the exit handler after
      unregister_fprobe().
      
      ------
       CPU1                              CPU2
       call unregister_fprobe(fp)
       ...
                                         __fprobe_handler()
                                         rethook_hook() on probed function
       unregister_ftrace_function()
                                         return from probed function
                                         rethook hooks
                                         find rh->handler == fprobe_exit_handler
                                         call fprobe_exit_handler()
       rethook_free():
         set rh->handler = NULL;
       return from unreigster_fprobe;
                                         call fp->exit_handler() <- (*)
      ------
      
      (*) At this point, the exit handler is called after returning from
      unregister_fprobe().
      
      This fixes it as following;
      ------
       CPU1                              CPU2
       call unregister_fprobe()
       ...
       rethook_stop():
         set rh->handler = NULL;
                                         __fprobe_handler()
                                         rethook_hook() on probed function
       unregister_ftrace_function()
                                         return from probed function
                                         rethook hooks
                                         find rh->handler == NULL
                                         return from rethook
       rethook_free()
       return from unreigster_fprobe;
      ------
      
      Link: https://lore.kernel.org/all/168873859949.156157.13039240432299335849.stgit@devnote2/
      
      
      
      Fixes: 5f810187 ("fprobe: Release rethook after the ftrace_ops is unregistered")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
      Reviewed-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
      195b9cb5
  11. Jul 10, 2023
  12. Jul 08, 2023
Loading