Skip to content

L
linux
Switch branch/tag
  1. 14 Feb, 2020 5 commits
    • Samuel Holland's avatar
      drm/sun4i: dsi: Remove incorrect use of runtime PM · 215be713
      Samuel Holland authored 
      The driver currently uses runtime PM to perform some of the module
initialization and cleanup. This has three problems:

1) There is no Kconfig dependency on CONFIG_PM, so if runtime PM is
   disabled, the driver will not work at all, since the module will
   never be initialized.

2) The driver does not ensure that the device is suspended when
   sun6i_dsi_probe() fails or when sun6i_dsi_remove() is called. It
   simply disables runtime PM. From the docs of pm_runtime_disable():

      The device can be either active or suspended after its runtime PM
      has been disabled.

   And indeed, the device will likely still be active if sun6i_dsi_probe
   fails. For example, if the panel driver is not yet loaded, we have
   the following sequence:

   sun6i_dsi_probe()
      pm_runtime_enable()
      mipi_dsi_host_register()
         of_mipi_dsi_device_add(child)
            ...device_add()...
               __device_attach()
                 pm_runtime_get_sync(dev->parent) -> Causes resume
                 bus_for_each_drv()
                    __device_attach_driver() -> No match for panel
                 pm_runtime_put(dev->parent) -> Async idle request
      component_add()
         __component_add()
            try_to_bring_up_masters()
               try_to_bring_up_master()
                  sun4i_drv_bind()
                     component_bind_all()
                        component_bind()
                           sun6i_dsi_bind() -> Fails with -EPROBE_DEFER
      mipi_dsi_host_unregister()
      pm_runtime_disable()
         __pm_runtime_disable()
            __pm_runtime_barrier() -> Idle request is still pending
               cancel_work_sync()  -> DSI host is *not* suspended!

   Since the device is not suspended, the clock and regulator are never
   disabled. The imbalance causes a WARN at devres free time.

3) The driver relies on being suspended when sun6i_dsi_encoder_enable()
   is called. The resume callback has a comment that says:

      Some part of it can only be done once we get a number of
      lanes, see sun6i_dsi_inst_init

   And then part of the resume callback only runs if dsi->device is not
   NULL (that is, if sun6i_dsi_attach() has been called). However, as
   the above call graph shows, the resume callback is guaranteed to be
   called before sun6i_dsi_attach(); it is called before child devices
   get their drivers attached.

   Therefore, part of the controller initialization will only run if the
   device is suspended between the calls to mipi_dsi_host_register() and
   component_add() (which ends up calling sun6i_dsi_encoder_enable()).
   Again, as shown by the above call graph, this is not the case. It
   appears that the controller happens to work because it is still
   initialized by the bootloader.

   Because the connector is hardcoded to always be connected, the
   device's runtime PM reference is not dropped until system suspend,
   when sun4i_drv_drm_sys_suspend() ends up calling
   sun6i_dsi_encoder_disable(). However, that is done as a system sleep
   PM hook, and at that point the system PM core has already taken
   another runtime PM reference, so sun6i_dsi_runtime_suspend() is
   not called. Likewise, by the time the PM core releases its reference,
   sun4i_drv_drm_sys_resume() has already re-enabled the encoder.

   So after system suspend and resume, we have *still never called*
   sun6i_dsi_inst_init(), and now that the rest of the display pipeline
   has been reset, the DSI host is unable to communicate with the panel,
   causing VBLANK timeouts.

Fix all of these issues by inlining the runtime PM hooks into the
encoder enable/disable functions, which are guaranteed to run after a
panel is attached. This allows sun6i_dsi_inst_init() to be called
unconditionally. Furthermore, this causes the hardware to be turned off
during system suspend and reinitialized on resume, which was not
happening before.

Fixes: 133add5b ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support")
Signed-off-by: default avatarSamuel Holland <samuel@sholland.org>
Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211072858.30784-4-samuel@sholland.org
      215be713
    • Samuel Holland's avatar
      drm/sun4i: dsi: Allow binding the host without a panel · 1a2703bd
      Samuel Holland authored 
      Currently, the DSI host blocks binding the display pipeline until the
panel is available. This unnecessarily prevents other display outpus
from working, and adds logspam to dmesg when the panel driver is built
as a module (the component master is unsuccessfully brought up several
times during boot).

Flip the dependency, instead requiring the host to be bound before the
panel is attached. The panel driver provides no functionality outside of
the display pipeline anyway.

Since the panel is now probed after the DRM connector, we need a hotplug
event to turn on the connector after the panel is attached.

This has the added benefit of fixing panel module removal/insertion.
Previously, the panel would be turned off when its module was removed.
But because the connector state was hardcoded, nothing knew to turn the
panel back on when it was re-attached. Now, with hotplug events
available, the connector state will follow the panel module state, and
the panel will be re-enabled properly.

Fixes: 133add5b ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support")
Signed-off-by: default avatarSamuel Holland <samuel@sholland.org>
Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211072858.30784-3-samuel@sholland.org
      1a2703bd
    • Samuel Holland's avatar
      drm/sun4i: dsi: Use NULL to signify "no panel" · 0e4e3fb4
      Samuel Holland authored 
      The continued use of an ERR_PTR to signify "no panel" outside of
sun6i_dsi_attach is confusing because it is a double negative. Because
the connector always reports itself as connected, there is also the
possibility of sending an ERR_PTR to drm_panel_get_modes(), which would
crash.

Solve both of these by only storing the panel pointer if it is valid.

Fixes: 133add5b ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support")
Signed-off-by: default avatarSamuel Holland <samuel@sholland.org>
Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211072858.30784-2-samuel@sholland.org
      0e4e3fb4
    • Samuel Holland's avatar
      drm/sun4i: dsi: Remove unused drv from driver context · dc84f091
      Samuel Holland authored 
      This member is never used, so remove it.

Fixes: 133add5b ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller support")
Signed-off-by: default avatarSamuel Holland <samuel@sholland.org>
Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211072858.30784-1-samuel@sholland.org
      dc84f091
    • Gerd Hoffmann's avatar
      drm/virtio: fix error check · 2c77ae22
      Gerd Hoffmann authored 
      The >= compare op must happen in cpu byte order, doing it in
little endian fails on big endian machines like s390.
Reported-by: default avatarSebastian Mitterle <smitterl@redhat.com>
Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
Reviewed-by: default avatarCornelia Huck <cohuck@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20200214080100.1273-1-kraxel@redhat.com
      2c77ae22
  3. 13 Feb, 2020 35 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7