1. 03 Nov, 2022 3 commits
    • maxime@cerno.tech's avatar
      drm/vc4: hdmi: Fix HSM clock too low on Pi4 · 3bc6a37f
      maxime@cerno.tech authored
      Commit ae71ab58 ("drm/vc4: hdmi: Enforce the minimum rate at
      runtime_resume") reintroduced the call to clk_set_min_rate in an attempt
      to fix the boot without a monitor connected on the RaspberryPi3.
      
      However, that introduced a regression breaking the display output
      entirely (black screen but no vblank timeout) on the Pi4.
      
      This is due to the fact that we now have in a typical modeset at boot,
      in vc4_hdmi_encoder_pre_crtc_configure(), we have a first call to
      clk_set_min_rate() asking for the minimum rate of the HSM clock for our
      given resolution, and then a call to pm_runtime_resume_and_get(). We
      will thus execute vc4_hdmi_runtime_resume() which, since the commit
      mentioned above, will call clk_set_min_rate() a second time with the
      absolute minimum rate we want to enforce on the HSM clock.
      
      We're thus effectively erasing the minimum mandated by the mode we're
      trying to set. The fact that only the Pi4 is affected is due to the fact
      that it uses a different clock driver that tries to minimize the HSM
      clock at all time. It will thus lower the HSM clock rate to 120MHz on
      the second clk_set_min_rate() call.
      
      The Pi3 doesn't use the same driver and will not change the frequency on
      the second clk_set_min_rate() call since it's still within the new
      boundaries and it doesn't have the code to minimize the clock rate as
      needed. So even though the boundaries are still off, the clock rate is
      still the right one for our given mode, so everything works.
      
      There is a lot of moving parts, so I couldn't find any obvious
      solution:
      
        - Reverting the original is not an option, as that would break the Pi3
          again.
      
        - We can't move the clk_set_min_rate() call in _pre_crtc_configure()
          since because, on the Pi3, the HSM clock has the CLK_SET_RATE_GATE
          flag which prevents the clock rate from being changed after it's
          been enabled. Our calls to clk_set_min_rate() can change it, so they
          need to be done before clk_prepare_enable().
      
        - We can't remove the call to clk_prepare_enable() from the
          runtime_resume hook to put it into _pre_crtc_configure() either,
          since we need that clock to be enabled to access the registers, and
          we can't count on the fact that the display will be active in all
          situations (doing any CEC operation, or listing the modes while
          inactive are valid for example()).
      
        - We can't drop the call to clk_set_min_rate() in
          _pre_crtc_configure() since we would need to still enforce the
          minimum rate for a given resolution, and runtime_resume doesn't have
          access to the current mode, if there's any.
      
        - We can't copy the TMDS character rate into vc4_hdmi and reuse it
          since, because it's part of the KMS atomic state, it needs to be
          protected by a mutex. Unfortunately, some functions (CEC operations,
          mostly) can be reentrant (through the CEC framework) and still need
          a pm_runtime_get.
      
      However, we can work around this issue by leveraging the fact that the
      clk_set_min_rate() calls set boundaries for its given struct clk, and
      that each different clk_get() call will return a different instance of
      struct clk. The clock framework will then aggregate the boundaries for
      each struct clk instances linked to a given clock, plus its hardware
      boundaries, and will use that.
      
      We can thus get an extra HSM clock user for runtime_pm use only, and use
      our different clock instances depending on the context: runtime_pm will
      use its own to set the absolute minimum clock setup so that we never
      lock the CPU waiting for a register access, and the modeset part will
      set its requirement for the current resolution. And we let the CCF do
      the coordination.
      
      It's not an ideal solution, but it's fairly unintrusive and doesn't
      really change any part of the logic so it looks like a rather safe fix.
      
      Link: https://bugzilla.redhat.com/show_bug.cgi?id=2136234
      Fixes: ae71ab58 ("drm/vc4: hdmi: Enforce the minimum rate at runtime_resume")
      Reported-by: default avatarPeter Robinson <pbrobinson@gmail.com>
      Reviewed-by: default avatarJavier Martinez Canillas <javierm@redhat.com>
      Tested-by: default avatarPeter Robinson <pbrobinson@gmail.com>
      Link: https://lore.kernel.org/r/20221021131339.2203291-1-maxime@cerno.techSigned-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
      3bc6a37f
    • maxime@cerno.tech's avatar
      drm/vc4: hdmi: Fix outdated function name in comment · 76ffa2af
      maxime@cerno.tech authored
      A comment introduced by commit 6bed2ea3 ("drm/vc4: hdmi: Reset link
      on hotplug") mentions a drm_atomic_helper_connector_hdmi_reset_link()
      function that was part of the earlier versions but got moved internally
      and is now named vc4_hdmi_reset_link(). Let's fix the function name.
      
      Fixes: 6bed2ea3 ("drm/vc4: hdmi: Reset link on hotplug")
      Reviewed-by: default avatarJavier Martinez Canillas <javierm@redhat.com>
      Link: https://lore.kernel.org/r/20221024093634.118190-2-maxime@cerno.techSigned-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
      76ffa2af
    • maxime@cerno.tech's avatar
      drm/vc4: hdmi: Take our lock to reset the link · 682f99b8
      maxime@cerno.tech authored
      We access some fields protected by our internal mutex in
      vc4_hdmi_reset_link() (saved_adjusted_mode, output_bpc, output_format)
      and are calling functions that need to have that lock taken
      (vc4_hdmi_supports_scrambling()).
      
      However, the current code doesn't lock that mutex. Let's make sure it
      does.
      
      Fixes: 6bed2ea3 ("drm/vc4: hdmi: Reset link on hotplug")
      Reviewed-by: default avatarJavier Martinez Canillas <javierm@redhat.com>
      Link: https://lore.kernel.org/r/20221024093634.118190-1-maxime@cerno.techSigned-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
      682f99b8
  2. 01 Nov, 2022 2 commits
  3. 31 Oct, 2022 1 commit
    • Hector Martin's avatar
      drm/format-helper: Only advertise supported formats for conversion · 6fdaed8c
      Hector Martin authored
      drm_fb_build_fourcc_list() currently returns all emulated formats
      unconditionally as long as the native format is among them, even though
      not all combinations have conversion helpers. Although the list is
      arguably provided to userspace in precedence order, userspace can pick
      something out-of-order (and thus break when it shouldn't), or simply
      only support a format that is unsupported (and thus think it can work,
      which results in the appearance of a hang as FB blits fail later on,
      instead of the initialization error you'd expect in this case).
      
      Add checks to filter the list of emulated formats to only those
      supported for conversion to the native format. This presumes that there
      is a single native format (only the first is checked, if there are
      multiple). Refactoring this API to drop the native list or support it
      properly (by returning the appropriate emulated->native mapping table)
      is left for a future patch.
      
      The simpledrm driver is left as-is with a full table of emulated
      formats. This keeps all currently working conversions available and
      drops all the broken ones (i.e. this a strict bugfix patch, adding no
      new supported formats nor removing any actually working ones). In order
      to avoid proliferation of emulated formats, future drivers should
      advertise only XRGB8888 as the sole emulated format (since some
      userspace assumes its presence).
      
      This fixes a real user regression where the ?RGB2101010 support commit
      started advertising it unconditionally where not supported, and KWin
      decided to start to use it over the native format and broke, but also
      the fixes the spurious RGB565/RGB888 formats which have been wrongly
      unconditionally advertised since the dawn of simpledrm.
      
      Fixes: 6ea966fc ("drm/simpledrm: Add [AX]RGB2101010 formats")
      Fixes: 11e8f5fd ("drm: Add simpledrm driver")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarHector Martin <marcan@marcan.st>
      Acked-by: default avatarPekka Paalanen <pekka.paalanen@collabora.com>
      Reviewed-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Signed-off-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Link: https://patchwork.freedesktop.org/patch/msgid/20221027135711.24425-1-marcan@marcan.st
      6fdaed8c
  4. 29 Oct, 2022 7 commits
  5. 27 Oct, 2022 1 commit
  6. 25 Oct, 2022 1 commit
  7. 24 Oct, 2022 1 commit
  8. 23 Oct, 2022 9 commits
  9. 22 Oct, 2022 15 commits