Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1587880280.git.xji@analogixsemi.com> <4d14400b6c19f17c28267f6ebdbce9673333c05c.1587880280.git.xji@analogixsemi.com>
In-Reply-To: <4d14400b6c19f17c28267f6ebdbce9673333c05c.1587880280.git.xji@analogixsemi.com>
From:   Nicolas Boichat <drinkcat@chromium.org>
Date:   Wed, 29 Apr 2020 10:10:05 +0800
Message-ID: <CANMq1KC2EUPMDqTcaU-HLqfFivR4=7wxrmiSk-VzF0bjjLgNHA@mail.gmail.com>
Subject: Re: [PATCH v8 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to
 DP bridge driver
To:     Xin Ji <xji@analogixsemi.com>
Cc:     devel@driverdev.osuosl.org,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Andrzej Hajda <a.hajda@samsung.com>,
        Nicolas Boichat <drinkcat@google.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@siol.net>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        lkml <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Pi-Hsun Shih <pihsun@chromium.org>,
        Sheng Pan <span@analogixsemi.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 27, 2020 at 2:18 PM Xin Ji <xji@analogixsemi.com> wrote:
>
> The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed
> for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K.
>
> The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI
> to DP feature. This driver only enabled MIPI DSI/DPI to DP feature.
>
> Signed-off-by: Xin Ji <xji@analogixsemi.com>
> ---
>  drivers/gpu/drm/bridge/Makefile           |    2 +-
>  drivers/gpu/drm/bridge/analogix/Kconfig   |    6 +
>  drivers/gpu/drm/bridge/analogix/Makefile  |    1 +
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 2158 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/bridge/analogix/anx7625.h |  410 ++++++
>  5 files changed, 2576 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c
>  create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h
>
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 4934fcf..bcd388a 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>  obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>  obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>  obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +obj-y += analogix/
>  obj-y += synopsys/
> diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig
> index e930ff9..b2f127e 100644
> --- a/drivers/gpu/drm/bridge/analogix/Kconfig
> +++ b/drivers/gpu/drm/bridge/analogix/Kconfig
> @@ -2,3 +2,9 @@
>  config DRM_ANALOGIX_DP
>         tristate
>         depends on DRM
> +
> +config ANALOGIX_ANX7625
> +       tristate "Analogix MIPI to DP interface support"
> +       help
> +               ANX7625 is an ultra-low power 4K mobile HD transmitter designed
> +               for portable devices. It converts MIPI/DPI to DisplayPort1.3 4K.
> diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile
> index fdbf3fd..8a52867 100644
> --- a/drivers/gpu/drm/bridge/analogix/Makefile
> +++ b/drivers/gpu/drm/bridge/analogix/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o
>  analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> new file mode 100644
> index 0000000..fff7a49
> [snip]
> +static int anx7625_attach_dsi(struct anx7625_data *ctx)
> +{
> +       struct mipi_dsi_host *host;
> +       struct mipi_dsi_device *dsi;
> +       struct device_node *mipi_host_node;
> +       struct device *dev = &ctx->client->dev;
> +       const struct mipi_dsi_device_info info = {
> +               .type = "anx7625",
> +               .channel = 0,
> +               .node = NULL,
> +       };
> +
> +       DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n");
> +
> +       if (ctx->pdata.dsi_supported)
> +               mipi_host_node = ctx->pdata.node.mipi_dsi_host_node;
> +       else
> +               mipi_host_node = ctx->pdata.node.mipi_dpi_host_node;
> +
> +       if (!mipi_host_node) {
> +               DRM_ERROR("dsi host is not configured.\n");
> +               return -EINVAL;
> +       }
> +
> +       host = of_find_mipi_dsi_host_by_node(mipi_host_node);

I tried this driver when connected to a dpi interface, and this fails,
as of_find_mipi_dsi_host_by_node is not able to find the dpi interface
from the SoC.

I'm not too familiar with how dpi bridges are supposed to work in the
kernel, but should we even call "anx7625_attach_dsi" for DPI
interface?

> +       if (!host) {
> +               DRM_ERROR("failed to find dsi host.\n");
> +               return -EINVAL;
> +       }
> +
> +       dsi = mipi_dsi_device_register_full(host, &info);
> +       if (IS_ERR(dsi)) {
> +               DRM_ERROR("failed to create dsi device.\n");
> +               return -EINVAL;
> +       }
> +
> +       dsi->lanes = 4;
> +       dsi->format = MIPI_DSI_FMT_RGB888;
> +       dsi->mode_flags = MIPI_DSI_MODE_VIDEO   |
> +               MIPI_DSI_MODE_VIDEO_SYNC_PULSE  |
> +               MIPI_DSI_MODE_EOT_PACKET        |
> +               MIPI_DSI_MODE_VIDEO_HSE;
> +
> +       if (mipi_dsi_attach(dsi) < 0) {
> +               DRM_ERROR("failed to attach dsi to host.\n");
> +               mipi_dsi_device_unregister(dsi);
> +               return -EINVAL;
> +       }
> +
> +       ctx->dsi = dsi;
> +
> +       DRM_DEV_DEBUG_DRIVER(dev, "attach dsi succeeded.\n");
> +
> +       return 0;
> +}
> +
> [snip]
> +static int anx7625_bridge_attach(struct drm_bridge *bridge)
> +{
> +       struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> +       int err;
> +       struct device *dev = &ctx->client->dev;
> +
> +       DRM_DEV_DEBUG_DRIVER(dev, "drm attach\n");
> +       if (!bridge->encoder) {
> +               DRM_ERROR("Parent encoder object not found");
> +               return -ENODEV;
> +       }
> +
> +       err = drm_connector_init(bridge->dev, &ctx->connector,
> +                                &anx7625_connector_funcs,
> +                                ctx->pdata.internal_panel ?
> +                                       DRM_MODE_CONNECTOR_eDP :
> +                                       DRM_MODE_CONNECTOR_DisplayPort);
> +       if (err) {
> +               DRM_ERROR("Failed to initialize connector: %d\n", err);
> +               return err;
> +       }
> +
> +       drm_connector_helper_add(&ctx->connector,
> +                                &anx7625_connector_helper_funcs);
> +
> +       err = drm_connector_register(&ctx->connector);
> +       if (err) {
> +               DRM_ERROR("Failed to register connector: %d\n", err);
> +               return err;
> +       }
> +
> +       ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +
> +       err = drm_connector_attach_encoder(&ctx->connector, bridge->encoder);
> +       if (err) {
> +               DRM_ERROR("Failed to link up connector to encoder: %d\n", err);
> +               drm_connector_unregister(&ctx->connector);
> +               return err;
> +       }
> +
> +       err = anx7625_attach_dsi(ctx);
> +       if (err) {
> +               DRM_ERROR("Failed to attach to dsi : %d\n", err);
> +               drm_connector_unregister(&ctx->connector);
> +               return err;
> +       }
> +
> +       ctx->bridge_attached = 1;
> +
> +       return 0;
> +}
> +