Received: by 2002:a25:c593:0:0:0:0:0 with SMTP id v141csp2402384ybe; Sat, 14 Sep 2019 14:18:06 -0700 (PDT) X-Google-Smtp-Source: APXvYqyIPPt52TkXr7MzPNOxXS9DCV3bwGuRdDg8GpMpwGkgBvKgYJ4QlEZaYmHQqE4wAwbCJVSC X-Received: by 2002:a17:906:b211:: with SMTP id p17mr44484621ejz.11.1568495886558; Sat, 14 Sep 2019 14:18:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1568495886; cv=none; d=google.com; s=arc-20160816; b=D7+xixcfPJANE1M1IXusUD1EGSeJ7D2L8cxO74rvPYEd0FIyOX61MT6BZKp531h3p1 l3DzZYGi5J2y5mU6NmLMnl/5add0AvtfmG7OwELHyZ3yyk1IwxTVblP6ySxISQWfRwxj 7fd9Y4aORGV9zfM7rmh2vBy/NbosX/5dSPLEQwvFXy9vVdFrQf/ZYuvJMjcTcSgFZ+ic hxgt9dptWR8rwxsJ/EYQyS8KqZ1w0e5ERmbMPvmee5jdKeVD/A3vIl63ufP8MIle2fB/ NCpi61k/Cr8lP/VkK78utX/rAg4epPcE0welRG9shgsTD6SmpyqtTMulZF/Ktw23S3iU 85kQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date; bh=q4k59A/R4G6fKjlh2PlkKA8rOpOiBOR6D/C8X91z52Y=; b=CwaQOuPXfEKK4WTeAsl4KR3rECgdL656u6iAy1JNNlfCCq4oAjnMs3+hNP8JRlazWy SzKqM7ea5gGRgFAjE2+bWHIlzOR9VbZtc85pHPgtyuAXncKSpBvleic2tURwH7hoNeue 1LmLPr9BLyRtMxOnF/RFbNEA2utVi16bNevUW4rfIcwYGMJ06Fy1zwExqFlS19fUt+GH frEqVqcQe10kYyZl+9VL5vXsKFp5SsOqvzxJ6TBm0Kf51h7/T+CsBV1Fi9VdPFfEd2CF AM3quWuRRkDwDjAHAJ8TuY96kVJhuE8HORveWz3SD+OCnNXyGCFewvvP3PCfaod3o0U3 wLRA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z6si9527867edp.79.2019.09.14.14.17.42; Sat, 14 Sep 2019 14:18:06 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727267AbfINQML (ORCPT + 99 others); Sat, 14 Sep 2019 12:12:11 -0400 Received: from honk.sigxcpu.org ([24.134.29.49]:60856 "EHLO honk.sigxcpu.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725805AbfINQMK (ORCPT ); Sat, 14 Sep 2019 12:12:10 -0400 Received: from localhost (localhost [127.0.0.1]) by honk.sigxcpu.org (Postfix) with ESMTP id D6537FB03; Sat, 14 Sep 2019 18:12:03 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at honk.sigxcpu.org Received: from honk.sigxcpu.org ([127.0.0.1]) by localhost (honk.sigxcpu.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qZ_0aIOv2yVx; Sat, 14 Sep 2019 18:11:58 +0200 (CEST) Received: by bogon.sigxcpu.org (Postfix, from userid 1000) id 42386488DD; Sat, 14 Sep 2019 09:11:55 -0700 (PDT) Date: Sat, 14 Sep 2019 09:11:55 -0700 From: Guido =?iso-8859-1?Q?G=FCnther?= To: Andrzej Hajda Cc: David Airlie , Daniel Vetter , Rob Herring , Mark Rutland , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Neil Armstrong , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Lee Jones , dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Robert Chiras , Sam Ravnborg , Arnd Bergmann Subject: Re: [PATCH v5 2/2] drm/bridge: Add NWL MIPI DSI host controller support Message-ID: <20190914161155.GA2933@bogon.m.sigxcpu.org> References: <3ce1891ea41249bf4a9985e2cee8640fb36de42e.1567995854.git.agx@sigxcpu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andrzej, thanks for having a look! On Fri, Sep 13, 2019 at 11:31:43AM +0200, Andrzej Hajda wrote: > On 09.09.2019 04:25, Guido G?nther wrote: > > This adds initial support for the NWL MIPI DSI Host controller found on > > i.MX8 SoCs. > > > > It adds support for the i.MX8MQ but the same IP can be found on > > e.g. the i.MX8QXP. > > > > It has been tested on the Librem 5 devkit using mxsfb. > > > > Signed-off-by: Guido G?nther > > Co-developed-by: Robert Chiras > > Signed-off-by: Robert Chiras > > Tested-by: Robert Chiras > > --- > > drivers/gpu/drm/bridge/Kconfig | 2 + > > drivers/gpu/drm/bridge/Makefile | 1 + > > drivers/gpu/drm/bridge/nwl-dsi/Kconfig | 16 + > > drivers/gpu/drm/bridge/nwl-dsi/Makefile | 4 + > > drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c | 499 ++++++++++++++++ > > drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h | 65 +++ > > drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c | 696 +++++++++++++++++++++++ > > drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h | 112 ++++ > > > Why do you need separate files nwl-drv.[ch] and nwl-dsi.[ch] ? I guess > you can merge all into one file, maybe with separate file for NWL > register definitions. Idea is to have driver setup, soc specific hooks and revision specific quirks in one file and the dsi specific parts in another. If that doesn't fly I can merge into one if that's a requirement. > > 8 files changed, 1395 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Kconfig > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Makefile > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c > > create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h > > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > index 1cc9f502c1f2..7980b5c2156f 100644 > > --- a/drivers/gpu/drm/bridge/Kconfig > > +++ b/drivers/gpu/drm/bridge/Kconfig > > @@ -154,6 +154,8 @@ source "drivers/gpu/drm/bridge/analogix/Kconfig" > > > > source "drivers/gpu/drm/bridge/adv7511/Kconfig" > > > > +source "drivers/gpu/drm/bridge/nwl-dsi/Kconfig" > > + > > source "drivers/gpu/drm/bridge/synopsys/Kconfig" > > > > endmenu > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > > index 4934fcf5a6f8..d9f6c0f77592 100644 > > --- a/drivers/gpu/drm/bridge/Makefile > > +++ b/drivers/gpu/drm/bridge/Makefile > > @@ -16,4 +16,5 @@ 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-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi/ > > obj-y += synopsys/ > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Kconfig b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig > > new file mode 100644 > > index 000000000000..7fa678e3b5e2 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig > > @@ -0,0 +1,16 @@ > > +config DRM_NWL_MIPI_DSI > > + tristate "Northwest Logic MIPI DSI Host controller" > > + depends on DRM > > + depends on COMMON_CLK > > + depends on OF && HAS_IOMEM > > + select DRM_KMS_HELPER > > + select DRM_MIPI_DSI > > + select DRM_PANEL_BRIDGE > > + select GENERIC_PHY_MIPI_DPHY > > + select MFD_SYSCON > > + select MULTIPLEXER > > + select REGMAP_MMIO > > + help > > + This enables the Northwest Logic MIPI DSI Host controller as > > + for example found on NXP's i.MX8 Processors. > > + > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Makefile b/drivers/gpu/drm/bridge/nwl-dsi/Makefile > > new file mode 100644 > > index 000000000000..804baf2f1916 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/nwl-dsi/Makefile > > @@ -0,0 +1,4 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +nwl-mipi-dsi-y := nwl-drv.o nwl-dsi.o > > +obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-mipi-dsi.o > > +header-test-y += nwl-drv.h nwl-dsi.h > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c > > new file mode 100644 > > index 000000000000..9ff43d2de127 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c > > @@ -0,0 +1,499 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * i.MX8 NWL MIPI DSI host driver > > + * > > + * Copyright (C) 2017 NXP > > + * Copyright (C) 2019 Purism SPC > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > > Alphabetic order Fixed for v6. > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include "nwl-drv.h" > > +#include "nwl-dsi.h" > > + > > +#define DRV_NAME "nwl-dsi" > > + > > +/* Possible platform specific clocks */ > > +#define NWL_DSI_CLK_CORE "core" > > + > > +static const struct regmap_config nwl_dsi_regmap_config = { > > + .reg_bits = 16, > > + .val_bits = 32, > > + .reg_stride = 4, > > + .max_register = NWL_DSI_IRQ_MASK2, > > + .name = DRV_NAME, > > +}; > > > What is the point in using regmap here, why not simple writel/readl. For me cat /sys/kernel/debug/regmap/30a00000.mipi_dsi-imx-nwl-dsi/registers justifies it's use to help debugging problems when e.g. having it connected to panels I don't own, so I think it's worth keeping if possible. > > + > > +struct nwl_dsi_platform_data { > > + int (*poweron)(struct nwl_dsi *dsi); > > + int (*poweroff)(struct nwl_dsi *dsi); > > + int (*select_input)(struct nwl_dsi *dsi); > > + int (*deselect_input)(struct nwl_dsi *dsi); > > + struct nwl_dsi_plat_clk_config clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS]; > > +}; > > > Another construct which do not have justification, at least for now. > Please simplify the driver, remove callbacks/intermediate > structs/quirks > > - for now they are useless. > > Unless there is a serious reason - in such case please describe it in > comments. They're needed for i.mx 8QM SoC support (the current driver only supports the i.mx 8MQ). It will be relatively easy to add with these so I expect these to show up quickly. I'll add a comment. The quirks on the other hand only apply to some i.mx8MQ mask revisions so they need to be conditionalized. (or maybe I misunderstood you). > > + > > +static inline struct nwl_dsi *bridge_to_dsi(struct drm_bridge *bridge) > > +{ > > + return container_of(bridge, struct nwl_dsi, bridge); > > +} > > + > > +static int nwl_dsi_set_platform_clocks(struct nwl_dsi *dsi, bool enable) > > +{ > > + struct device *dev = dsi->dev; > > + const char *id; > > + struct clk *clk; > > + size_t i; > > + unsigned long rate; > > + int ret, result = 0; > > + > > + DRM_DEV_DEBUG_DRIVER(dev, "%s platform clocks\n", > > + enable ? "enabling" : "disabling"); > > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) { > > + if (!dsi->clk_config[i].present) > > + continue; > > + id = dsi->clk_config[i].id; > > + clk = dsi->clk_config[i].clk; > > + > > + if (enable) { > > + ret = clk_prepare_enable(clk); > > + if (ret < 0) { > > + DRM_DEV_ERROR(dev, > > + "Failed to enable %s clk: %d\n", > > + id, ret); > > + result = result ?: ret; > > + } > > + rate = clk_get_rate(clk); > > + DRM_DEV_DEBUG_DRIVER(dev, "Enabled %s clk @%lu Hz\n", > > + id, rate); > > + } else { > > + clk_disable_unprepare(clk); > > + DRM_DEV_DEBUG_DRIVER(dev, "Disabled %s clk\n", id); > > + } > > + } > > + > > + return result; > > +} > > + > > +static int nwl_dsi_plat_enable(struct nwl_dsi *dsi) > > +{ > > + struct device *dev = dsi->dev; > > + int ret; > > + > > + if (dsi->pdata->select_input) > > + dsi->pdata->select_input(dsi); > > + > > + ret = nwl_dsi_set_platform_clocks(dsi, true); > > + if (ret < 0) > > + return ret; > > + > > + ret = dsi->pdata->poweron(dsi); > > + if (ret < 0) > > + DRM_DEV_ERROR(dev, "Failed to power on DSI: %d\n", ret); > > + return ret; > > +} > > + > > +static void nwl_dsi_plat_disable(struct nwl_dsi *dsi) > > +{ > > + dsi->pdata->poweroff(dsi); > > + nwl_dsi_set_platform_clocks(dsi, false); > > + if (dsi->pdata->deselect_input) > > + dsi->pdata->deselect_input(dsi); > > +} > > + > > +static void nwl_dsi_bridge_disable(struct drm_bridge *bridge) > > +{ > > + struct nwl_dsi *dsi = bridge_to_dsi(bridge); > > + > > + nwl_dsi_disable(dsi); > > + nwl_dsi_plat_disable(dsi); > > + pm_runtime_put(dsi->dev); > > +} > > + > > +static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi, > > + const struct drm_display_mode *mode, > > + union phy_configure_opts *phy_opts) > > +{ > > + unsigned long rate; > > + int ret; > > + > > + if (dsi->lanes < 1 || dsi->lanes > 4) > > + return -EINVAL; > > + > > + /* > > + * So far the DPHY spec minimal timings work for both mixel > > + * dphy and nwl dsi host > > + */ > > + ret = phy_mipi_dphy_get_default_config( > > + mode->crtc_clock * 1000, > > + mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes, > > + &phy_opts->mipi_dphy); > > + if (ret < 0) > > + return ret; > > + > > + rate = clk_get_rate(dsi->tx_esc_clk); > > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "LP clk is @%lu Hz\n", rate); > > + phy_opts->mipi_dphy.lp_clk_rate = rate; > > + > > + return 0; > > +} > > + > > +static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{ > > + /* At least LCDIF + NWL needs active high sync */ > > + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC); > > + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC); > > + > > + return true; > > +} > > + > > +static enum drm_mode_status > > +nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode) > > +{ > > + struct nwl_dsi *dsi = bridge_to_dsi(bridge); > > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > > + > > + if (mode->clock * bpp > 15000000 * dsi->lanes) > > + return MODE_CLOCK_HIGH; > > + > > + if (mode->clock * bpp < 80000 * dsi->lanes) > > + return MODE_CLOCK_LOW; > > + > > + return MODE_OK; > > +} > > + > > +static void > > +nwl_dsi_bridge_mode_set(struct drm_bridge *bridge, > > + const struct drm_display_mode *mode, > > + const struct drm_display_mode *adjusted_mode) > > +{ > > + struct nwl_dsi *dsi = bridge_to_dsi(bridge); > > + struct device *dev = dsi->dev; > > + union phy_configure_opts new_cfg; > > + unsigned long phy_ref_rate; > > + int ret; > > + > > + ret = nwl_dsi_get_dphy_params(dsi, adjusted_mode, &new_cfg); > > + if (ret < 0) > > + return; > > + > > + /* > > + * If hs clock is unchanged, we're all good - all parameters are > > + * derived from it atm. > > + */ > > + if (new_cfg.mipi_dphy.hs_clk_rate == dsi->phy_cfg.mipi_dphy.hs_clk_rate) > > + return; > > + > > + phy_ref_rate = clk_get_rate(dsi->phy_ref_clk); > > + DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n", phy_ref_rate); > > + /* Save the new desired phy config */ > > + memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg)); > > + > > + memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode)); > > + drm_mode_debug_printmodeline(adjusted_mode); > > +} > > + > > +static void nwl_dsi_bridge_pre_enable(struct drm_bridge *bridge) > > +{ > > + struct nwl_dsi *dsi = bridge_to_dsi(bridge); > > + > > + pm_runtime_get_sync(dsi->dev); > > + nwl_dsi_plat_enable(dsi); > > + nwl_dsi_enable(dsi); > > +} > > + > > +static int nwl_dsi_bridge_attach(struct drm_bridge *bridge) > > +{ > > + struct nwl_dsi *dsi = bridge->driver_private; > > + > > + return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge); > > +} > > + > > +static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = { > > + .pre_enable = nwl_dsi_bridge_pre_enable, > > + .disable = nwl_dsi_bridge_disable, > > + .mode_fixup = nwl_dsi_bridge_mode_fixup, > > + .mode_set = nwl_dsi_bridge_mode_set, > > + .mode_valid = nwl_dsi_bridge_mode_valid, > > + .attach = nwl_dsi_bridge_attach, > > +}; > > + > > +static int nwl_dsi_parse_dt(struct nwl_dsi *dsi) > > +{ > > + struct platform_device *pdev = to_platform_device(dsi->dev); > > + struct clk *clk; > > + const char *clk_id; > > + void __iomem *base; > > + int i, ret; > > + > > + dsi->phy = devm_phy_get(dsi->dev, "dphy"); > > + if (IS_ERR(dsi->phy)) { > > + ret = PTR_ERR(dsi->phy); > > + if (ret != -EPROBE_DEFER) > > + DRM_DEV_ERROR(dsi->dev, "Could not get PHY: %d\n", ret); > > + return ret; > > + } > > + > > + /* Platform dependent clocks */ > > + memcpy(dsi->clk_config, dsi->pdata->clk_config, > > + sizeof(dsi->pdata->clk_config)); > > + > > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) { > > + if (!dsi->clk_config[i].present) > > + continue; > > + > > + clk_id = dsi->clk_config[i].id; > > + clk = devm_clk_get(dsi->dev, clk_id); > > + if (IS_ERR(clk)) { > > + ret = PTR_ERR(clk); > > + DRM_DEV_ERROR(dsi->dev, "Failed to get %s clock: %d\n", > > + clk_id, ret); > > + return ret; > > + } > > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "Setup clk %s (rate: %lu)\n", > > + clk_id, clk_get_rate(clk)); > > + dsi->clk_config[i].clk = clk; > > + } > > + > > + /* DSI clocks */ > > + clk = devm_clk_get(dsi->dev, "phy_ref"); > > + if (IS_ERR(clk)) { > > + ret = PTR_ERR(clk); > > + DRM_DEV_ERROR(dsi->dev, "Failed to get phy_ref clock: %d\n", > > + ret); > > + return ret; > > + } > > + dsi->phy_ref_clk = clk; > > + > > + clk = devm_clk_get(dsi->dev, "rx_esc"); > > + if (IS_ERR(clk)) { > > + ret = PTR_ERR(clk); > > + DRM_DEV_ERROR(dsi->dev, "Failed to get rx_esc clock: %d\n", > > + ret); > > + return ret; > > + } > > + dsi->rx_esc_clk = clk; > > + > > + clk = devm_clk_get(dsi->dev, "tx_esc"); > > + if (IS_ERR(clk)) { > > + ret = PTR_ERR(clk); > > + DRM_DEV_ERROR(dsi->dev, "Failed to get tx_esc clock: %d\n", > > + ret); > > + return ret; > > + } > > + dsi->tx_esc_clk = clk; > > + > > + dsi->mux = devm_mux_control_get(dsi->dev, NULL); > > + if (IS_ERR(dsi->mux)) { > > + ret = PTR_ERR(dsi->mux); > > + if (ret != -EPROBE_DEFER) > > + DRM_DEV_ERROR(dsi->dev, "Failed to get mux: %d\n", ret); > > + return ret; > > + } > > + > > + base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(base)) > > + return PTR_ERR(base); > > + > > + dsi->regmap = > > + devm_regmap_init_mmio(dsi->dev, base, &nwl_dsi_regmap_config); > > + if (IS_ERR(dsi->regmap)) { > > + ret = PTR_ERR(dsi->regmap); > > + DRM_DEV_ERROR(dsi->dev, "Failed to create NWL DSI regmap: %d\n", > > + ret); > > + return ret; > > + } > > + > > + dsi->irq = platform_get_irq(pdev, 0); > > + if (dsi->irq < 0) { > > + DRM_DEV_ERROR(dsi->dev, "Failed to get device IRQ: %d\n", > > + dsi->irq); > > + return dsi->irq; > > + } > > + > > + dsi->rstc = devm_reset_control_array_get(dsi->dev, false, true); > > + if (IS_ERR(dsi->rstc)) { > > + DRM_DEV_ERROR(dsi->dev, "Failed to get resets: %ld\n", > > + PTR_ERR(dsi->rstc)); > > + return PTR_ERR(dsi->rstc); > > + } > > + > > + return 0; > > +} > > + > > +static int imx8mq_dsi_select_input(struct nwl_dsi *dsi) > > +{ > > + struct device_node *remote; > > + u32 use_dcss = 1; > > + int ret; > > + > > + remote = of_graph_get_remote_node(dsi->dev->of_node, 0, 0); > > + if (strcmp(remote->name, "lcdif") == 0) > > + use_dcss = 0; > > > Relying on node name seems to me wrong. I am not sure if whole logic for > input select should be here. > > My 1st impression is that selecting should be done rather in DCSS or > LCDIF driver, why do you want to put it here? Doing it in here keeps it at a single location where on the other hand it would need to be done in mxsfb (which handles other SoCs as well) and upcoming dcss. Also we can have in the dsi enable path which e.g. mxsfb doesn't even know about at this point. Cheers, -- Guido > > Regards > > Andrzej > > > > + > > + DRM_DEV_INFO(dsi->dev, "Using %s as input source\n", > > + (use_dcss) ? "DCSS" : "LCDIF"); > > + > > + ret = mux_control_try_select(dsi->mux, use_dcss); > > + if (ret < 0) > > + DRM_DEV_ERROR(dsi->dev, "Failed to select input: %d\n", ret); > > + > > + of_node_put(remote); > > + return ret; > > +} > > + > > + > > +static int imx8mq_dsi_deselect_input(struct nwl_dsi *dsi) > > +{ > > + int ret; > > + > > + ret = mux_control_deselect(dsi->mux); > > + if (ret < 0) > > + DRM_DEV_ERROR(dsi->dev, "Failed to deselect input: %d\n", ret); > > + > > + return ret; > > +} > > + > > + > > +static int imx8mq_dsi_poweron(struct nwl_dsi *dsi) > > +{ > > + int ret = 0; > > + > > + /* otherwise the display stays blank */ > > + usleep_range(200, 300); > > + > > + if (dsi->rstc) > > + ret = reset_control_deassert(dsi->rstc); > > + > > + return ret; > > +} > > + > > +static int imx8mq_dsi_poweroff(struct nwl_dsi *dsi) > > +{ > > + int ret = 0; > > + > > + if (dsi->quirks & SRC_RESET_QUIRK) > > + return 0; > > + > > + if (dsi->rstc) > > + ret = reset_control_assert(dsi->rstc); > > + return ret; > > +} > > + > > +static const struct drm_bridge_timings nwl_dsi_timings = { > > + .input_bus_flags = DRM_BUS_FLAG_DE_LOW, > > +}; > > + > > +static const struct nwl_dsi_platform_data imx8mq_dev = { > > + .poweron = &imx8mq_dsi_poweron, > > + .poweroff = &imx8mq_dsi_poweroff, > > + .select_input = &imx8mq_dsi_select_input, > > + .deselect_input = &imx8mq_dsi_deselect_input, > > + .clk_config = { > > + { .id = NWL_DSI_CLK_CORE, .present = true }, > > + }, > > +}; > > + > > +static const struct of_device_id nwl_dsi_dt_ids[] = { > > + { .compatible = "fsl,imx8mq-nwl-dsi", .data = &imx8mq_dev, }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(of, nwl_dsi_dt_ids); > > + > > +static const struct soc_device_attribute nwl_dsi_quirks_match[] = { > > + { .soc_id = "i.MX8MQ", .revision = "2.0", > > + .data = (void *)(E11418_HS_MODE_QUIRK | SRC_RESET_QUIRK) }, > > + { /* sentinel. */ }, > > +}; > > + > > +static int nwl_dsi_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + const struct of_device_id *of_id = of_match_device(nwl_dsi_dt_ids, dev); > > + const struct nwl_dsi_platform_data *pdata = of_id->data; > > + const struct soc_device_attribute *attr; > > + struct nwl_dsi *dsi; > > + int ret; > > + > > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL); > > + if (!dsi) > > + return -ENOMEM; > > + > > + dsi->dev = dev; > > + dsi->pdata = pdata; > > + > > + ret = nwl_dsi_parse_dt(dsi); > > + if (ret) > > + return ret; > > + > > + ret = devm_request_irq(dev, dsi->irq, nwl_dsi_irq_handler, 0, > > + dev_name(dev), dsi); > > + if (ret < 0) { > > + DRM_DEV_ERROR(dev, "Failed to request IRQ %d: %d\n", dsi->irq, > > + ret); > > + return ret; > > + } > > + > > + dsi->dsi_host.ops = &nwl_dsi_host_ops; > > + dsi->dsi_host.dev = dev; > > + ret = mipi_dsi_host_register(&dsi->dsi_host); > > + if (ret) { > > + DRM_DEV_ERROR(dev, "Failed to register MIPI host: %d\n", ret); > > + return ret; > > + } > > + > > + attr = soc_device_match(nwl_dsi_quirks_match); > > + if (attr) > > + dsi->quirks = (uintptr_t)attr->data; > > + > > + dsi->bridge.driver_private = dsi; > > + dsi->bridge.funcs = &nwl_dsi_bridge_funcs; > > + dsi->bridge.of_node = dev->of_node; > > + dsi->bridge.timings = &nwl_dsi_timings; > > + > > + dev_set_drvdata(dev, dsi); > > + pm_runtime_enable(dev); > > + return 0; > > +} > > + > > +static int nwl_dsi_remove(struct platform_device *pdev) > > +{ > > + struct nwl_dsi *dsi = platform_get_drvdata(pdev); > > + > > + mipi_dsi_host_unregister(&dsi->dsi_host); > > + pm_runtime_disable(&pdev->dev); > > + return 0; > > +} > > + > > +static struct platform_driver nwl_dsi_driver = { > > + .probe = nwl_dsi_probe, > > + .remove = nwl_dsi_remove, > > + .driver = { > > + .of_match_table = nwl_dsi_dt_ids, > > + .name = DRV_NAME, > > + }, > > +}; > > + > > +module_platform_driver(nwl_dsi_driver); > > + > > +MODULE_AUTHOR("NXP Semiconductor"); > > +MODULE_AUTHOR("Purism SPC"); > > +MODULE_DESCRIPTION("Northwest Logic MIPI-DSI driver"); > > +MODULE_LICENSE("GPL"); /* GPLv2 or later */ > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h > > new file mode 100644 > > index 000000000000..1e72a9221401 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h > > @@ -0,0 +1,65 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * NWL MIPI DSI host driver > > + * > > + * Copyright (C) 2017 NXP > > + * Copyright (C) 2019 Purism SPC > > + */ > > + > > +#ifndef __NWL_DRV_H__ > > +#define __NWL_DRV_H__ > > + > > +#include > > +#include > > + > > +#include > > +#include > > + > > +struct nwl_dsi_platform_data; > > + > > +/* i.MX8 NWL quirks */ > > +/* i.MX8MQ errata E11418 */ > > +#define E11418_HS_MODE_QUIRK BIT(0) > > +/* Skip DSI bits in SRC on disable to avoid blank display on enable */ > > +#define SRC_RESET_QUIRK BIT(1) > > + > > +#define NWL_DSI_MAX_PLATFORM_CLOCKS 1 > > +struct nwl_dsi_plat_clk_config { > > + const char *id; > > + struct clk *clk; > > + bool present; > > +}; > > + > > +struct nwl_dsi { > > + struct drm_bridge bridge; > > + struct mipi_dsi_host dsi_host; > > + struct drm_bridge *panel_bridge; > > + struct device *dev; > > + struct phy *phy; > > + union phy_configure_opts phy_cfg; > > + unsigned int quirks; > > + > > + struct regmap *regmap; > > + int irq; > > + struct reset_control *rstc; > > + struct mux_control *mux; > > + > > + /* DSI clocks */ > > + struct clk *phy_ref_clk; > > + struct clk *rx_esc_clk; > > + struct clk *tx_esc_clk; > > + /* Platform dependent clocks */ > > + struct nwl_dsi_plat_clk_config clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS]; > > + > > + /* dsi lanes */ > > + u32 lanes; > > + enum mipi_dsi_pixel_format format; > > + struct drm_display_mode mode; > > + unsigned long dsi_mode_flags; > > + > > + struct nwl_dsi_transfer *xfer; > > + > > + const struct nwl_dsi_platform_data *pdata; > > +}; > > + > > +#endif /* __NWL_DRV_H__ */ > > diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c > > new file mode 100644 > > index 000000000000..e6038cb4e849 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c > > @@ -0,0 +1,696 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * NWL MIPI DSI host driver > > + * > > + * Copyright (C) 2017 NXP > > + * Copyright (C) 2019 Purism SPC > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include