mbox series

[00/14] improve Analogix DP AUX channel handling

Message ID 20240503151129.3901815-1-l.stach@pengutronix.de (mailing list archive)
Headers show
Series improve Analogix DP AUX channel handling | expand

Message

Lucas Stach May 3, 2024, 3:11 p.m. UTC
Currently the AUX channel support in the Analogix DP driver is severely
limited as the AUX block of the bridge is only initialized when the video
link is to be enabled. This is okay for the purposes of link training,
but does not allow to detect displays by probing for EDID. This series
reworks the driver to allow AUX transactions before the video link is
active.

As this requires to rework some of the controller initialization and
also handling of both internal and external clocks, the series includes
quite a few changes to add better runtime PM handling.

Lucas Stach (14):
  drm/bridge: analogix_dp: remove unused platform power_on_end callback
  drm/rockchip: analogix_dp: add runtime PM handling
  drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
  drm/bridge: analogix_dp: handle clock via runtime PM
  drm/bridge: analogix_dp: remove unused analogix_dp_remove
  drm/bridge: analogix_dp: remove clk handling from
    analogix_dp_set_bridge
  drm/bridge: analogix_dp: move platform and PHY power handling into
    runtime PM
  drm/bridge: analogix_dp: move basic controller init into runtime PM
  drm/bridge: analogix_dp: remove PLL lock check from
    analogix_dp_config_video
  drm/bridge: analogix_dp: move macro reset after link bandwidth setting
  drm/bridge: analogix_dp: don't wait for PLL lock too early
  drm/bridge: analogix_dp: simplify and correct PLL lock checks
  drm/bridge: analogix_dp: only read AUX status when an error occured
  drm/bridge: analogix_dp: handle AUX transfer timeouts

 .../drm/bridge/analogix/analogix_dp_core.c    | 196 ++++++++----------
 .../drm/bridge/analogix/analogix_dp_core.h    |   7 +-
 .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  38 ++--
 .../gpu/drm/bridge/analogix/analogix_dp_reg.h |   9 +
 drivers/gpu/drm/exynos/exynos_dp.c            |   5 +-
 .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  26 +--
 include/drm/bridge/analogix_dp.h              |   4 +-
 7 files changed, 120 insertions(+), 165 deletions(-)

Comments

Robert Foss May 7, 2024, 1:10 p.m. UTC | #1
On Fri, May 3, 2024 at 5:12 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Currently the AUX channel support in the Analogix DP driver is severely
> limited as the AUX block of the bridge is only initialized when the video
> link is to be enabled. This is okay for the purposes of link training,
> but does not allow to detect displays by probing for EDID. This series
> reworks the driver to allow AUX transactions before the video link is
> active.
>
> As this requires to rework some of the controller initialization and
> also handling of both internal and external clocks, the series includes
> quite a few changes to add better runtime PM handling.
>
> Lucas Stach (14):
>   drm/bridge: analogix_dp: remove unused platform power_on_end callback
>   drm/rockchip: analogix_dp: add runtime PM handling
>   drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
>   drm/bridge: analogix_dp: handle clock via runtime PM
>   drm/bridge: analogix_dp: remove unused analogix_dp_remove
>   drm/bridge: analogix_dp: remove clk handling from
>     analogix_dp_set_bridge
>   drm/bridge: analogix_dp: move platform and PHY power handling into
>     runtime PM
>   drm/bridge: analogix_dp: move basic controller init into runtime PM
>   drm/bridge: analogix_dp: remove PLL lock check from
>     analogix_dp_config_video
>   drm/bridge: analogix_dp: move macro reset after link bandwidth setting
>   drm/bridge: analogix_dp: don't wait for PLL lock too early
>   drm/bridge: analogix_dp: simplify and correct PLL lock checks
>   drm/bridge: analogix_dp: only read AUX status when an error occured
>   drm/bridge: analogix_dp: handle AUX transfer timeouts
>
>  .../drm/bridge/analogix/analogix_dp_core.c    | 196 ++++++++----------
>  .../drm/bridge/analogix/analogix_dp_core.h    |   7 +-
>  .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  38 ++--
>  .../gpu/drm/bridge/analogix/analogix_dp_reg.h |   9 +
>  drivers/gpu/drm/exynos/exynos_dp.c            |   5 +-
>  .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  26 +--
>  include/drm/bridge/analogix_dp.h              |   4 +-
>  7 files changed, 120 insertions(+), 165 deletions(-)
>
> --
> 2.39.2
>

There are some checkpatch --strict warnings, and the patch 10/14 does
not apply. Other than that the series looks very good.

Maybe rebase on drm-misc/drm-misc-next, fix the applicable checkpatch
--strict warnings and send a new version of this series. Then the last
patches can be reviewed.
Lucas Stach May 15, 2024, 10:31 a.m. UTC | #2
Hi Robert,

Am Dienstag, dem 07.05.2024 um 15:10 +0200 schrieb Robert Foss:
> On Fri, May 3, 2024 at 5:12 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> > 
> > Currently the AUX channel support in the Analogix DP driver is severely
> > limited as the AUX block of the bridge is only initialized when the video
> > link is to be enabled. This is okay for the purposes of link training,
> > but does not allow to detect displays by probing for EDID. This series
> > reworks the driver to allow AUX transactions before the video link is
> > active.
> > 
> > As this requires to rework some of the controller initialization and
> > also handling of both internal and external clocks, the series includes
> > quite a few changes to add better runtime PM handling.
> > 
> > Lucas Stach (14):
> >   drm/bridge: analogix_dp: remove unused platform power_on_end callback
> >   drm/rockchip: analogix_dp: add runtime PM handling
> >   drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
> >   drm/bridge: analogix_dp: handle clock via runtime PM
> >   drm/bridge: analogix_dp: remove unused analogix_dp_remove
> >   drm/bridge: analogix_dp: remove clk handling from
> >     analogix_dp_set_bridge
> >   drm/bridge: analogix_dp: move platform and PHY power handling into
> >     runtime PM
> >   drm/bridge: analogix_dp: move basic controller init into runtime PM
> >   drm/bridge: analogix_dp: remove PLL lock check from
> >     analogix_dp_config_video
> >   drm/bridge: analogix_dp: move macro reset after link bandwidth setting
> >   drm/bridge: analogix_dp: don't wait for PLL lock too early
> >   drm/bridge: analogix_dp: simplify and correct PLL lock checks
> >   drm/bridge: analogix_dp: only read AUX status when an error occured
> >   drm/bridge: analogix_dp: handle AUX transfer timeouts
> > 
> >  .../drm/bridge/analogix/analogix_dp_core.c    | 196 ++++++++----------
> >  .../drm/bridge/analogix/analogix_dp_core.h    |   7 +-
> >  .../gpu/drm/bridge/analogix/analogix_dp_reg.c |  38 ++--
> >  .../gpu/drm/bridge/analogix/analogix_dp_reg.h |   9 +
> >  drivers/gpu/drm/exynos/exynos_dp.c            |   5 +-
> >  .../gpu/drm/rockchip/analogix_dp-rockchip.c   |  26 +--
> >  include/drm/bridge/analogix_dp.h              |   4 +-
> >  7 files changed, 120 insertions(+), 165 deletions(-)
> > 
> > --
> > 2.39.2
> > 
> 
> There are some checkpatch --strict warnings, and the patch 10/14 does
> not apply. Other than that the series looks very good.
> 
> Maybe rebase on drm-misc/drm-misc-next, fix the applicable checkpatch
> --strict warnings and send a new version of this series. Then the last
> patches can be reviewed.

Thanks for the review so far. Patch 10/14 probably doesn't apply, as I
based the series on some patches I sent earlier [1]. Maybe you are also
willing to take a look at those, so I could squash them into a single
series for the resend?

Regards,
Lucas

[1] https://lore.kernel.org/dri-devel/20240318203925.2837689-1-l.stach@pengutronix.de/
Heiko Stuebner June 10, 2024, 12:47 p.m. UTC | #3
Am Freitag, 3. Mai 2024, 17:11:15 CEST schrieb Lucas Stach:
> Currently the AUX channel support in the Analogix DP driver is severely
> limited as the AUX block of the bridge is only initialized when the video
> link is to be enabled. This is okay for the purposes of link training,
> but does not allow to detect displays by probing for EDID. This series
> reworks the driver to allow AUX transactions before the video link is
> active.
> 
> As this requires to rework some of the controller initialization and
> also handling of both internal and external clocks, the series includes
> quite a few changes to add better runtime PM handling.
> 
> Lucas Stach (14):
>   drm/bridge: analogix_dp: remove unused platform power_on_end callback
>   drm/rockchip: analogix_dp: add runtime PM handling
>   drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
>   drm/bridge: analogix_dp: handle clock via runtime PM
>   drm/bridge: analogix_dp: remove unused analogix_dp_remove
>   drm/bridge: analogix_dp: remove clk handling from
>     analogix_dp_set_bridge
>   drm/bridge: analogix_dp: move platform and PHY power handling into
>     runtime PM
>   drm/bridge: analogix_dp: move basic controller init into runtime PM
>   drm/bridge: analogix_dp: remove PLL lock check from
>     analogix_dp_config_video
>   drm/bridge: analogix_dp: move macro reset after link bandwidth setting
>   drm/bridge: analogix_dp: don't wait for PLL lock too early
>   drm/bridge: analogix_dp: simplify and correct PLL lock checks
>   drm/bridge: analogix_dp: only read AUX status when an error occured
>   drm/bridge: analogix_dp: handle AUX transfer timeouts

my knowledge of Analgix-dp internals is limited, but at least both
rk3288-veyron and rk3399 gru still had working displays with this series
applied and both device classes using the analogix-dp for their main
display.

So on rk3288-veyron and rk3399-gru
Tested-by: Heiko Stuebner <heiko@sntech.de>