Re: [PATCH RESEND 2/4] drm: bridge: anx7688: Add anx7688 bridge driver support.

[Laurent Pinchart]

Hi Nicolas,

On Tue, Dec 17, 2019 at 08:40:51AM +0800, Nicolas Boichat wrote:
> (Brilliant, I managed to accidentally send the email below, and send
> it as HTML, sorry about that... ASCII art in gmail is hard ,-(

No worries. I have been told it's indeed painful.

> Take 2:)
> 
> Hi Laurent,
> 
> > On Tue, Dec 17, 2019 at 12:39 AM Laurent Pinchart wrote:
> > > On Mon, Dec 16, 2019 at 06:19:24PM +0800, Nicolas Boichat wrote:
> > > > On Mon, Dec 16, 2019 at 4:46 PM Hsin-Yi Wang wrote:
> > > > > On Sat, Dec 14, 2019 at 6:38 AM Laurent Pinchart wrote:
> > > > > > On Wed, Dec 11, 2019 at 02:19:09PM +0800, Hsin-Yi Wang wrote:
> > > > > > > From: Nicolas Boichat <drinkcat@xxxxxxxxxxxx>
> > > > > > >
> > > > > > > ANX7688 is a HDMI to DP converter (as well as USB-C port controller),
> > > > > > > that has an internal microcontroller.
> > > > > > >
> > > > > > > The only reason a Linux kernel driver is necessary is to reject
> > > > > > > resolutions that require more bandwidth than what is available on
> > > > > > > the DP side. DP bandwidth and lane count are reported by the bridge
> > > > > > > via 2 registers on I2C.
> > > > > >
> > > > > > How about power, doesn't this chip have power supplies that potentially
> > > > > > need to be controlled ?
> > > > > >
> > > > > Ideally we should add power supplies as well, but the power is
> > > > > supplied by ec in mt8173 oak board. And we only have this board can
> > > > > test this driver. If we add power supplies in driver we can't test it.
> > > >
> > > > To clarify a bit more, this is because this chip is actually a
> > > > TCPC+mux+HDMI=>DP converter
> > > > (https://www.analogix.com/en/products/convertersbridges/anx7688). In
> > > > Chromebook architecture, TCPC+mux is controlled by the EC (including
> > > > power and other control pins), and the only reason we need a driver
> > > > for the HDMI=>DP converter is to get the number of lanes on the DP
> > > > side and filter out resolutions. Also, the converter is on a different
> > > > I2C address and it could almost be considered as a separate device.
> > > >
> > > > (of course we could write a kernel driver for the TCPC+mux but we'll
> > > > leave that to others if there's ever a board that is built with the
> > > > TCPC part connected to the AP)
> > >
> > > Is the mux the one that is handled through a gpio-mux driver in this
> > > series, or a different mux ?
> >
> 
> It's a different mux: it's the usual USB-C mux that takes in USB 3.0
> and DP (internally converted from HDMI), and decides which 2 lanes to
> use for each (4 lanes in total, but DP can only take 2 with this
> converter), and flip if necessary. This is all controlled by the EC
> (like on most other Chromebooks), so this is transparent to the kernel
> on this hardware.
> 
> > > It would really, really help if you could
> > > show a block diagram of the related hardware (including the EC), as this
> > > is quite confusing. With every e-mail exchanged there's a bit more
> > > information that change my understanding of the issue, I can't really
> > > provide guidance without a full overview.
> 
> https://lkml.org/lkml/2019/12/9/548 that you drew is accurate for the
> display part of the problem.
> 
> You can just add a USB3 connection to the above (there's also I2C
> interface to the EC of course to control the TCPC/mux aspect of it,
> but that's on different I2C addresses). Something like this:
> 
>                                       +-----------+
>  +---------+         +------+    /--> | HDMI      |
>  | MT8173  |  HDMI   |   -->| --/     | Connector |
>  |  HDMI   | ------> |--/   |         +-----------+
>  | Encoder |         |    ->| --\     +-----------+      +-----------+
>  +---------+         +------+    \--> | ANX7688   | ---> | USB-C     |
>                                       | Bridge    |      | Connector |
>                               USB3--> | + mux     |      |           |
>                                       +-----------+      +-----------+
>                                          ^     ^
>                                    (I2C) |     | (I2C)
>    MT8173 (DP lane count/bw readback) -- +     + -- EC (TCPC+mux control)
> 
> Power is also fully controlled by the EC.

Could I ask you to also explain how the HDMI mux is controlled, and
where the HPD-related signals for the HDMI connector and USB-C connector
are routed to ?

> (the product brief has a good diagram of the internals of the ANX7688:
> https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief.pdf)
> 
> The ANX7688 bridge could _almost_ work driverless (and it does
> already), the _only_ thing that the driver is doing is filtering out
> impossible resolution based on DP (over USB-C) number of lanes and
> bandwidth. This is required to support, for example, old monitors that
> may only do RBR over DP (so we can't drive the full resolution over 2
> DP lanes, we'd need 4 lanes, and we need to filter out the higher
> resolution modes).
> 
> > > > > > > Signed-off-by: Nicolas Boichat <drinkcat@xxxxxxxxxxxx>
> > > > > > > Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/bridge/Kconfig            |   9 +
> > > > > > >  drivers/gpu/drm/bridge/Makefile           |   1 +
> > > > > > >  drivers/gpu/drm/bridge/analogix-anx7688.c | 202 ++++++++++++++++++++++
> > > > > > >  3 files changed, 212 insertions(+)
> > > > > > >  create mode 100644 drivers/gpu/drm/bridge/analogix-anx7688.c
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > > > > > > index 34362976cd6f..1f3fc6bec842 100644
> > > > > > > --- a/drivers/gpu/drm/bridge/Kconfig
> > > > > > > +++ b/drivers/gpu/drm/bridge/Kconfig
> > > > > > > @@ -16,6 +16,15 @@ config DRM_PANEL_BRIDGE
> > > > > > >  menu "Display Interface Bridges"
> > > > > > >       depends on DRM && DRM_BRIDGE
> > > > > > >
> > > > > > > +config DRM_ANALOGIX_ANX7688
> > > > > > > +     tristate "Analogix ANX7688 bridge"
> > > > > > > +     select DRM_KMS_HELPER
> > > > > > > +     select REGMAP_I2C
> > > > > > > +     ---help---
> > > > > > > +       ANX7688 is a transmitter to support DisplayPort over USB-C for
> > > > > > > +       smartphone and tablets.
> > > > > > > +       This driver only supports the HDMI to DP component of the chip.
> > > > > > > +
> > > > > > >  config DRM_ANALOGIX_ANX78XX
> > > > > > >       tristate "Analogix ANX78XX bridge"
> > > > > > >       select DRM_KMS_HELPER
> > > > > > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> > > > > > > index 4934fcf5a6f8..7a1e0ec032e6 100644
> > > > > > > --- a/drivers/gpu/drm/bridge/Makefile
> > > > > > > +++ b/drivers/gpu/drm/bridge/Makefile
> > > > > > > @@ -1,4 +1,5 @@
> > > > > > >  # SPDX-License-Identifier: GPL-2.0
> > > > > > > +obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o
> > > > > > >  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > > > > > >  obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> > > > > > >  obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> > > > > > > diff --git a/drivers/gpu/drm/bridge/analogix-anx7688.c b/drivers/gpu/drm/bridge/analogix-anx7688.c
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..baaed48d6201
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/gpu/drm/bridge/analogix-anx7688.c
> > > > > > > @@ -0,0 +1,202 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > > +/*
> > > > > > > + * ANX7688 HDMI->DP bridge driver
> > > > > > > + *
> > > > > > > + * Copyright 2016 Google LLC
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <linux/i2c.h>
> > > > > > > +#include <linux/module.h>
> > > > > > > +#include <linux/regmap.h>
> > > > > > > +#include <drm/drm_bridge.h>
> > > > > > > +
> > > > > > > +/* Register addresses */
> > > > > > > +#define VENDOR_ID_REG 0x00
> > > > > > > +#define DEVICE_ID_REG 0x02
> > > > > > > +
> > > > > > > +#define FW_VERSION_REG 0x80
> > > > > > > +
> > > > > > > +#define DP_BANDWIDTH_REG 0x85
> > > > > > > +#define DP_LANE_COUNT_REG 0x86
> > > > > >
> > > > > > Are these registers defined by the ANX7688 hardware, or by the firmware
> > > > > > running on the chip (and, I assume, developed by Google) ?
> > > > > >
> > > > > By firmware developed by ANX provided to Google.
> > > >
> > > > We asked for these registers to be added to ANX FW, and this is the FW
> > > > that is used by all elm/hana Chromebooks (I have no idea about other
> > > > ANX customers...). We have facilities to update the ANX FW from
> > > > coreboot/depthcharge on Chromebooks, but that does not really matter:
> > > > the factory FW of all MP Chromebooks does provide these registers.
> > >
> > > So the driver is specific to Chromebooks, it doesn't support all
> > > ANX7688. Sweet :-(
> 
> FWIW, this is a 3+ year old part, so it appears that nobody else cares anyway?

That's good news :-)

> Also, this driver is only required to implement the mode filtering,
> which, possibly, is only supported by the Google version of the FW (I
> have no idea what other customers ANX has for this part, if they care
> about this problem, and if so, how they solve it).

-- 
Regards,

Laurent Pinchart