From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Enric Balletbo Serra <eballetbo@gmail.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Adrian Pop <pop.adrian61@gmail.com>,
Jonas Karlman <jonas@kwiboo.se>,
Martyn Welch <martyn.welch@collabora.com>,
Sjoerd Simons <sjoerd.simons@collabora.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Emil Velikov <emil.velikov@collabora.com>,
Andrzej Hajda <a.hajda@samsung.com>,
Collabora Kernel ML <kernel@collabora.com>,
linux-stm32@st-md-mailman.stormreply.com,
Arnaud Ferraris <arnaud.ferraris@collabora.com>,
linux-imx@nxp.com
Subject: Re: [PATCH v6 4/8] drm: imx: Add i.MX 6 MIPI DSI host platform driver
Date: Fri, 17 Apr 2020 12:56:17 +0300 [thread overview]
Message-ID: <87ftd2jv0e.fsf@collabora.com> (raw)
In-Reply-To: <20200415173018.GK4758@pendragon.ideasonboard.com>
Hi Enric & Laurent,
On Wed, 15 Apr 2020, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Enric,
>
> On Wed, Apr 15, 2020 at 07:26:02PM +0200, Enric Balletbo Serra
> wrote:
>> Missatge de Adrian Ratiu <adrian.ratiu@collabora.com> del dia
>> dt., 14 d’abr. 2020 a les 17:19:
>> >
>> > This adds support for the Synopsis DesignWare MIPI DSI v1.01
>> > host controller which is embedded in i.MX 6 SoCs.
>> >
>> > Based on following patches, but updated/extended to work with
>> > existing support found in the kernel:
>> >
>> > - drm: imx: Support Synopsys DesignWare MIPI DSI host
>> > controller
>> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> >
>> > Cc: Fabio Estevam <festevam@gmail.com> Reviewed-by: Emil
>> > Velikov <emil.velikov@collabora.com> Tested-by: Adrian Pop
>> > <pop.adrian61@gmail.com> Tested-by: Arnaud Ferraris
>> > <arnaud.ferraris@collabora.com> Signed-off-by: Sjoerd Simons
>> > <sjoerd.simons@collabora.com> Signed-off-by: Martyn Welch
>> > <martyn.welch@collabora.com> Signed-off-by: Adrian Ratiu
>> > <adrian.ratiu@collabora.com> --- Changes since v5:
>> > - Reword to remove unrelated device tree patch mention
>> > (Fabio) - Move pllref_clk enable/disable to bind/unbind
>> > (Ezequiel) - Fix freescale.com -> nxp.com email addresses
>> > (Fabio) - Also added myself as module author (Fabio) - Use
>> > DRM_DEV_* macros for consistency, print more error msg
>> >
>> > Changes since v4:
>> > - Split off driver-specific configuration of phy timings
>> > due to new upstream API. - Move regmap infrastructure
>> > logic to separate commit (Ezequiel) - Move dsi v1.01 layout
>> > addition to a separate commit (Ezequiel) - Minor warnings
>> > and driver name fixes
>> >
>> > Changes since v3:
>> > - Renamed platform driver to reflect it's i.MX6
>> > only. (Fabio)
>> >
>> > Changes since v2:
>> > - Fixed commit tags. (Emil)
>> >
>> > Changes since v1:
>> > - Moved register definitions & regmap initialization into
>> > bridge module. Platform drivers get the regmap via
>> > plat_data after calling the bridge probe. (Emil)
>> > ---
>> > drivers/gpu/drm/imx/Kconfig | 7 +
>> > drivers/gpu/drm/imx/Makefile | 1 +
>> > drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 409
>> > +++++++++++++++++++++++++ 3 files changed, 417 insertions(+)
>> > create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> >
>> > diff --git a/drivers/gpu/drm/imx/Kconfig
>> > b/drivers/gpu/drm/imx/Kconfig index
>> > 207bf7409dfb..b49e70e7f0fd 100644 ---
>> > a/drivers/gpu/drm/imx/Kconfig +++
>> > b/drivers/gpu/drm/imx/Kconfig @@ -39,3 +39,10 @@ config
>> > DRM_IMX_HDMI
>> > depends on DRM_IMX help
>> > Choose this if you want to use HDMI on i.MX6.
>> > + +config DRM_IMX6_MIPI_DSI + tristate "Freescale i.MX6
>> > DRM MIPI DSI" + select DRM_DW_MIPI_DSI + depends
>> > on DRM_IMX
>> Should it depend on CONFIG_OF too? I suspect you'll get build
>> errors if OF is not selected
>> > + help + Choose this if you want to use MIPI
>> > DSI on i.MX6. diff --git a/drivers/gpu/drm/imx/Makefile
>> > b/drivers/gpu/drm/imx/Makefile index
>> > 21cdcc2faabc..9a7843c59347 100644 ---
>> > a/drivers/gpu/drm/imx/Makefile +++
>> > b/drivers/gpu/drm/imx/Makefile @@ -9,3 +9,4 @@
>> > obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
>> > obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>> >
>> > obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
>> > +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o diff
>> > --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c new file mode 100644
>> > index 000000000000..6914db8ce8cb --- /dev/null +++
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c @@ -0,0 +1,409 @@
>> > +// SPDX-License-Identifier: GPL-2.0+ +/* + * i.MX6 drm
>> > driver - MIPI DSI Host Controller + * + * Copyright (C)
>> > 2011-2015 Freescale Semiconductor, Inc. + * Copyright (C)
>> > 2019-2020 Collabora, Ltd. + */ + +#include <linux/clk.h>
>> > +#include <linux/component.h> +#include <linux/mfd/syscon.h>
>> > +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> +#include
>> > <linux/module.h> +#include <linux/of_device.h> +#include
>> > <linux/regmap.h> +#include <linux/videodev2.h> +#include
>> > <drm/bridge/dw_mipi_dsi.h> +#include <drm/drm_crtc_helper.h>
>> > +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h>
>> > +#include <drm/drm_print.h> + +#include "imx-drm.h" +
>> > +#define DSI_PWR_UP 0x04 +#define RESET
>> > 0 +#define POWERUP BIT(0) +
>> > +#define DSI_PHY_IF_CTRL 0x5c +#define
>> > PHY_IF_CTRL_RESET 0x0 + +#define
>> > DSI_PHY_TST_CTRL0 0x64 +#define PHY_TESTCLK
>> > BIT(1) +#define PHY_UNTESTCLK 0 +#define
>> > PHY_TESTCLR BIT(0) +#define PHY_UNTESTCLR
>> > 0 + +#define DSI_PHY_TST_CTRL1 0x68 +#define
>> > PHY_TESTEN BIT(16) +#define PHY_UNTESTEN
>> > 0 +#define PHY_TESTDOUT(n) (((n) &
>> > 0xff) << 8) +#define PHY_TESTDIN(n) (((n) &
>> > 0xff) << 0) + +struct imx_mipi_dsi { + struct
>> > drm_encoder encoder; + struct device *dev; +
>> > struct regmap *mux_sel; + struct dw_mipi_dsi *mipi_dsi;
>> > + struct clk *pllref_clk; + + void __iomem *base;
>> > + unsigned int lane_mbps; +}; + +struct
>> > dphy_pll_testdin_map { + unsigned int max_mbps; +
>> > u8 testdin; +}; + +/* The table is based on 27MHz DPHY pll
>> > reference clock. */ +static const struct dphy_pll_testdin_map
>> > dptdin_map[] = { + {160, 0x04}, {180, 0x24}, {200,
>> > 0x44}, {210, 0x06}, + {240, 0x26}, {250, 0x46}, {270,
>> > 0x08}, {300, 0x28}, + {330, 0x48}, {360, 0x2a}, {400,
>> > 0x4a}, {450, 0x0c}, + {500, 0x2c}, {550, 0x0e}, {600,
>> > 0x2e}, {650, 0x10}, + {700, 0x30}, {750, 0x12}, {800,
>> > 0x32}, {850, 0x14}, + {900, 0x34}, {950, 0x54}, {1000,
>> > 0x74} +}; + +static inline struct imx_mipi_dsi
>> > *enc_to_dsi(struct drm_encoder *enc) +{ + return
>> > container_of(enc, struct imx_mipi_dsi, encoder); +} + +static
>> > void imx_mipi_dsi_set_ipu_di_mux(struct imx_mipi_dsi *dsi,
>> > int ipu_di) +{ + regmap_update_bits(dsi->mux_sel,
>> > IOMUXC_GPR3, +
>> > IMX6Q_GPR3_MIPI_MUX_CTL_MASK, +
>> > ipu_di << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT); +} + +static const
>> > struct drm_encoder_funcs imx_mipi_dsi_encoder_funcs = { +
>> > .destroy = imx_drm_encoder_destroy, +}; + +static bool
>> > imx_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
>> > + const struct
>> > drm_display_mode *mode, +
>> > struct drm_display_mode *adj_mode) +{ + return true; +}
>> > + +static int imx_mipi_dsi_encoder_atomic_check(struct
>> > drm_encoder *encoder, +
>> > struct drm_crtc_state *crtc_state, +
>> > struct drm_connector_state *conn) +{ + struct
>> > imx_crtc_state *imx_crtc_state =
>> > to_imx_crtc_state(crtc_state); + + /* The following
>> > values are taken from dw_hdmi_imx_atomic_check */ +
>> > imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24; +
>> > imx_crtc_state->di_hsync_pin = 2; +
>> > imx_crtc_state->di_vsync_pin = 3; + + return 0; +} +
>> > +static void imx_mipi_dsi_encoder_commit(struct drm_encoder
>> > *encoder) +{ + struct imx_mipi_dsi *dsi =
>> > enc_to_dsi(encoder); + int mux =
>> > drm_of_encoder_active_port_id(dsi->dev->of_node, encoder); +
>> > + imx_mipi_dsi_set_ipu_di_mux(dsi, mux); +} + +static
>> > void imx_mipi_dsi_encoder_disable(struct drm_encoder
>> > *encoder) +{ +} + +static const struct
>> > drm_encoder_helper_funcs imx_mipi_dsi_encoder_helpers = {
>> AFAIK (or at least this is the feedback I received)
>> drm_encoder is kind of deprecated, and nowadays all is migrated
>> to drm_bridge (encoders, bridges and panels). Of course, a
>> drm_encoder is needed, as the DRM core expects one, but should
>> just be a dummy object that you can probably create with just
>> calling drm_simple_encoder_init(). DRM maintainers, please
>> shout if I am wrong.
>
> That certainly matches my position :-) I would also add that the
> DRM encoder should be created in imx-drm-core.c or in
> ipuv3-crtc.c, not here.
>
Yes, I will do this change in v7 especially also because a new
commit 62fbddda2f72 ("drm/imx: Use simple encoder") landed in
-next recently which broke the current imx6 implementation by
removing the imx_drm_encoder_destroy() from imx-drm-core.c.
>> > + .mode_fixup = imx_mipi_dsi_encoder_mode_fixup,
>> > + .commit = imx_mipi_dsi_encoder_commit,
>> > + .disable = imx_mipi_dsi_encoder_disable,
>> > + .atomic_check = imx_mipi_dsi_encoder_atomic_check,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_register(struct drm_device *drm,
>> > + struct imx_mipi_dsi *dsi)
>> > +{
>> > + int ret;
>> > +
>> > + ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + drm_encoder_helper_add(&dsi->encoder,
>> > + &imx_mipi_dsi_encoder_helpers);
>> > + drm_encoder_init(drm, &dsi->encoder, &imx_mipi_dsi_encoder_funcs,
>> > + DRM_MODE_ENCODER_DSI, NULL);
>> > + return 0;
>> > +}
>> > +
>> > +static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
>> > + const struct drm_display_mode *mode)
>> > +{
>> > + /*
>> > + * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->hdisplay > 0x7ff)
>> > + return MODE_BAD_HVALUE;
>> > +
>> > + /*
>> > + * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->vdisplay > 0x7ff)
>> > + return MODE_BAD_VVALUE;
>> > +
>> > + return MODE_OK;
>> > +}
>> > +
>> > +
>> > +static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
>> > +{
>> > + unsigned int i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> > + if (dptdin_map[i].max_mbps == max_mbps)
>> > + return dptdin_map[i].testdin;
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
>> > +{
>> > + writel(val, dsi->base + reg);
>> > +}
>> > +
>> > +static int imx_mipi_dsi_phy_init(void *priv_data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int testdin;
>> > +
>> > + testdin = max_mbps_to_testdin(dsi->lane_mbps);
>> > + if (testdin < 0) {
>> > + DRM_DEV_ERROR(dsi->dev,
>> > + "failed to get testdin for %dmbps lane clock\n",
>> > + dsi->lane_mbps);
>> > + return testdin;
>> > + }
>> > +
>> > + dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
>> > + dsi_write(dsi, DSI_PWR_UP, POWERUP);
>> > +
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(0x44));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(testdin));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
>> > + const struct drm_display_mode *mode,
>> > + unsigned long mode_flags, u32 lanes,
>> > + u32 format, unsigned int *lane_mbps)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int bpp;
>> > + unsigned int i, target_mbps, mpclk;
>> > + unsigned long pllref;
>> > +
>> > + bpp = mipi_dsi_pixel_format_to_bpp(format);
>> > + if (bpp < 0) {
>> > + DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
>> > + format, bpp);
>> > + return bpp;
>> > + }
>> > +
>> > + pllref = clk_get_rate(dsi->pllref_clk);
>> > + if (pllref != 27000000)
>> > + DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
>> > +
>> > + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>> > + if (mpclk) {
>> > + /* take 1/0.7 blanking overhead into consideration */
>> > + target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
>> > + } else {
>> > + DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
>> > + target_mbps = 1000;
>> > + }
>> > +
>> > + DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
>> > + target_mbps);
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
>> > + if (target_mbps < dptdin_map[i].max_mbps) {
>> > + *lane_mbps = dptdin_map[i].max_mbps;
>> > + dsi->lane_mbps = *lane_mbps;
>> > + DRM_DEV_DEBUG(dsi->dev,
>> > + "real pllref_clk frequency is %uMbps\n",
>> > + *lane_mbps);
>> > + return 0;
>> > + }
>> > + }
>> > +
>> > + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
>> > + target_mbps);
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static int
>> > +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
>> > + struct dw_mipi_dsi_dphy_timing *timing)
>> > +{
>> > + timing->clk_hs2lp = 0x40;
>> > + timing->clk_lp2hs = 0x40;
>> > + timing->data_hs2lp = 0x40;
>> > + timing->data_lp2hs = 0x40;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
>> > + .init = imx_mipi_dsi_phy_init,
>> > + .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
>> > + .get_timing = dw_mipi_dsi_phy_get_timing,
>> > +};
>> > +
>> > +static struct dw_mipi_dsi_plat_data imx6q_mipi_dsi_drv_data = {
>> > + .max_data_lanes = 2,
>> > + .mode_valid = imx_mipi_dsi_mode_valid,
>> > + .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
>> > +};
>> > +
>> > +static const struct of_device_id imx_dsi_dt_ids[] = {
>> > + {
>> > + .compatible = "fsl,imx6q-mipi-dsi",
>> > + .data = &imx6q_mipi_dsi_drv_data,
>> > + },
>> > + { /* sentinel */ }
>> > +};
>> > +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
>> > +
>> > +static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > + struct drm_device *drm = data;
>> > + int ret;
>> > +
>> > + ret = clk_prepare_enable(dsi->pllref_clk);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = imx_mipi_dsi_register(drm, dsi);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to register: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > +
>> > + dw_mipi_dsi_unbind(dsi->mipi_dsi);
>> > +
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > +}
>> > +
>> > +static const struct component_ops imx_mipi_dsi_ops = {
>> > + .bind = imx_mipi_dsi_bind,
>> > + .unbind = imx_mipi_dsi_unbind,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_probe(struct platform_device *pdev)
>> > +{
>> > + struct device *dev = &pdev->dev;
>> > + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
>> > + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
>> > + struct imx_mipi_dsi *dsi;
>> > + struct resource *res;
>> > + int ret;
>> > +
>> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>> > + if (!dsi)
>> > + return -ENOMEM;
>> > +
>> > + dsi->dev = dev;
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + dsi->base = devm_ioremap_resource(dev, res);
>> > + if (IS_ERR(dsi->base)) {
>> > + ret = PTR_ERR(dsi->base);
>> > + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->pllref_clk = devm_clk_get(dev, "ref");
>> > + if (IS_ERR(dsi->pllref_clk)) {
>> > + ret = PTR_ERR(dsi->pllref_clk);
>> > + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
>> > + if (IS_ERR(dsi->mux_sel)) {
>> > + ret = PTR_ERR(dsi->mux_sel);
>> > + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dev_set_drvdata(dev, dsi);
>> > +
>> > + imx6q_mipi_dsi_drv_data.base = dsi->base;
>> > + imx6q_mipi_dsi_drv_data.priv_data = dsi;
>> > +
>> > + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
>> > + if (IS_ERR(dsi->mipi_dsi)) {
>> > + ret = PTR_ERR(dsi->mipi_dsi);
>> > + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
>> > + goto err_clkdisable;
>> > + }
>> > +
>> > + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
>> > +
>> > +err_clkdisable:
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > + return ret;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_remove(struct platform_device *pdev)
>> > +{
>> > + component_del(&pdev->dev, &imx_mipi_dsi_ops);
>> > + return 0;
>> > +}
>> > +
>> > +static struct platform_driver imx_mipi_dsi_driver = {
>> > + .probe = imx_mipi_dsi_probe,
>> > + .remove = imx_mipi_dsi_remove,
>> > + .driver = {
>> > + .of_match_table = imx_dsi_dt_ids,
>> > + .name = "dw-mipi-dsi-imx6",
>> > + },
>> > +};
>> > +module_platform_driver(imx_mipi_dsi_driver);
>> > +
>> > +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
>> > +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>> > +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
>> > +MODULE_LICENSE("GPL");
>
> --
> Regards,
>
> Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Adrian Ratiu <adrian.ratiu-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
To: Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
Enric Balletbo Serra
<eballetbo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Linux ARM
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>,
Adrian Pop <pop.adrian61-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Jonas Karlman <jonas-uIzNG4q0ceqzQB+pC5nmwQ@public.gmane.org>,
Martyn Welch
<martyn.welch-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
Sjoerd Simons
<sjoerd.simons-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
linux-kernel
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
dri-devel
<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
Emil Velikov
<emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
Collabora Kernel ML
<kernel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
linux-stm32-XDFAJ8BFU24N7RejjzZ/Li2xQDfSxrLKVpNB7YpNyf8@public.gmane.org,
Arnaud Ferraris
<arnaud.ferraris-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
linux-imx-3arQi8VN3Tc@public.gmane.org
Subject: Re: [PATCH v6 4/8] drm: imx: Add i.MX 6 MIPI DSI host platform driver
Date: Fri, 17 Apr 2020 12:56:17 +0300 [thread overview]
Message-ID: <87ftd2jv0e.fsf@collabora.com> (raw)
In-Reply-To: <20200415173018.GK4758-N3hz7ZxfLydczECFQUw77jytWr6r+dGw0E9HWUfgJXw@public.gmane.org>
Hi Enric & Laurent,
On Wed, 15 Apr 2020, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Hi Enric,
>
> On Wed, Apr 15, 2020 at 07:26:02PM +0200, Enric Balletbo Serra
> wrote:
>> Missatge de Adrian Ratiu <adrian.ratiu-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> del dia
>> dt., 14 d’abr. 2020 a les 17:19:
>> >
>> > This adds support for the Synopsis DesignWare MIPI DSI v1.01
>> > host controller which is embedded in i.MX 6 SoCs.
>> >
>> > Based on following patches, but updated/extended to work with
>> > existing support found in the kernel:
>> >
>> > - drm: imx: Support Synopsys DesignWare MIPI DSI host
>> > controller
>> > Signed-off-by: Liu Ying <Ying.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> >
>> > Cc: Fabio Estevam <festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Reviewed-by: Emil
>> > Velikov <emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> Tested-by: Adrian Pop
>> > <pop.adrian61-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Tested-by: Arnaud Ferraris
>> > <arnaud.ferraris-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> Signed-off-by: Sjoerd Simons
>> > <sjoerd.simons-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> Signed-off-by: Martyn Welch
>> > <martyn.welch-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> Signed-off-by: Adrian Ratiu
>> > <adrian.ratiu-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> --- Changes since v5:
>> > - Reword to remove unrelated device tree patch mention
>> > (Fabio) - Move pllref_clk enable/disable to bind/unbind
>> > (Ezequiel) - Fix freescale.com -> nxp.com email addresses
>> > (Fabio) - Also added myself as module author (Fabio) - Use
>> > DRM_DEV_* macros for consistency, print more error msg
>> >
>> > Changes since v4:
>> > - Split off driver-specific configuration of phy timings
>> > due to new upstream API. - Move regmap infrastructure
>> > logic to separate commit (Ezequiel) - Move dsi v1.01 layout
>> > addition to a separate commit (Ezequiel) - Minor warnings
>> > and driver name fixes
>> >
>> > Changes since v3:
>> > - Renamed platform driver to reflect it's i.MX6
>> > only. (Fabio)
>> >
>> > Changes since v2:
>> > - Fixed commit tags. (Emil)
>> >
>> > Changes since v1:
>> > - Moved register definitions & regmap initialization into
>> > bridge module. Platform drivers get the regmap via
>> > plat_data after calling the bridge probe. (Emil)
>> > ---
>> > drivers/gpu/drm/imx/Kconfig | 7 +
>> > drivers/gpu/drm/imx/Makefile | 1 +
>> > drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 409
>> > +++++++++++++++++++++++++ 3 files changed, 417 insertions(+)
>> > create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> >
>> > diff --git a/drivers/gpu/drm/imx/Kconfig
>> > b/drivers/gpu/drm/imx/Kconfig index
>> > 207bf7409dfb..b49e70e7f0fd 100644 ---
>> > a/drivers/gpu/drm/imx/Kconfig +++
>> > b/drivers/gpu/drm/imx/Kconfig @@ -39,3 +39,10 @@ config
>> > DRM_IMX_HDMI
>> > depends on DRM_IMX help
>> > Choose this if you want to use HDMI on i.MX6.
>> > + +config DRM_IMX6_MIPI_DSI + tristate "Freescale i.MX6
>> > DRM MIPI DSI" + select DRM_DW_MIPI_DSI + depends
>> > on DRM_IMX
>> Should it depend on CONFIG_OF too? I suspect you'll get build
>> errors if OF is not selected
>> > + help + Choose this if you want to use MIPI
>> > DSI on i.MX6. diff --git a/drivers/gpu/drm/imx/Makefile
>> > b/drivers/gpu/drm/imx/Makefile index
>> > 21cdcc2faabc..9a7843c59347 100644 ---
>> > a/drivers/gpu/drm/imx/Makefile +++
>> > b/drivers/gpu/drm/imx/Makefile @@ -9,3 +9,4 @@
>> > obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
>> > obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>> >
>> > obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
>> > +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o diff
>> > --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c new file mode 100644
>> > index 000000000000..6914db8ce8cb --- /dev/null +++
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c @@ -0,0 +1,409 @@
>> > +// SPDX-License-Identifier: GPL-2.0+ +/* + * i.MX6 drm
>> > driver - MIPI DSI Host Controller + * + * Copyright (C)
>> > 2011-2015 Freescale Semiconductor, Inc. + * Copyright (C)
>> > 2019-2020 Collabora, Ltd. + */ + +#include <linux/clk.h>
>> > +#include <linux/component.h> +#include <linux/mfd/syscon.h>
>> > +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> +#include
>> > <linux/module.h> +#include <linux/of_device.h> +#include
>> > <linux/regmap.h> +#include <linux/videodev2.h> +#include
>> > <drm/bridge/dw_mipi_dsi.h> +#include <drm/drm_crtc_helper.h>
>> > +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h>
>> > +#include <drm/drm_print.h> + +#include "imx-drm.h" +
>> > +#define DSI_PWR_UP 0x04 +#define RESET
>> > 0 +#define POWERUP BIT(0) +
>> > +#define DSI_PHY_IF_CTRL 0x5c +#define
>> > PHY_IF_CTRL_RESET 0x0 + +#define
>> > DSI_PHY_TST_CTRL0 0x64 +#define PHY_TESTCLK
>> > BIT(1) +#define PHY_UNTESTCLK 0 +#define
>> > PHY_TESTCLR BIT(0) +#define PHY_UNTESTCLR
>> > 0 + +#define DSI_PHY_TST_CTRL1 0x68 +#define
>> > PHY_TESTEN BIT(16) +#define PHY_UNTESTEN
>> > 0 +#define PHY_TESTDOUT(n) (((n) &
>> > 0xff) << 8) +#define PHY_TESTDIN(n) (((n) &
>> > 0xff) << 0) + +struct imx_mipi_dsi { + struct
>> > drm_encoder encoder; + struct device *dev; +
>> > struct regmap *mux_sel; + struct dw_mipi_dsi *mipi_dsi;
>> > + struct clk *pllref_clk; + + void __iomem *base;
>> > + unsigned int lane_mbps; +}; + +struct
>> > dphy_pll_testdin_map { + unsigned int max_mbps; +
>> > u8 testdin; +}; + +/* The table is based on 27MHz DPHY pll
>> > reference clock. */ +static const struct dphy_pll_testdin_map
>> > dptdin_map[] = { + {160, 0x04}, {180, 0x24}, {200,
>> > 0x44}, {210, 0x06}, + {240, 0x26}, {250, 0x46}, {270,
>> > 0x08}, {300, 0x28}, + {330, 0x48}, {360, 0x2a}, {400,
>> > 0x4a}, {450, 0x0c}, + {500, 0x2c}, {550, 0x0e}, {600,
>> > 0x2e}, {650, 0x10}, + {700, 0x30}, {750, 0x12}, {800,
>> > 0x32}, {850, 0x14}, + {900, 0x34}, {950, 0x54}, {1000,
>> > 0x74} +}; + +static inline struct imx_mipi_dsi
>> > *enc_to_dsi(struct drm_encoder *enc) +{ + return
>> > container_of(enc, struct imx_mipi_dsi, encoder); +} + +static
>> > void imx_mipi_dsi_set_ipu_di_mux(struct imx_mipi_dsi *dsi,
>> > int ipu_di) +{ + regmap_update_bits(dsi->mux_sel,
>> > IOMUXC_GPR3, +
>> > IMX6Q_GPR3_MIPI_MUX_CTL_MASK, +
>> > ipu_di << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT); +} + +static const
>> > struct drm_encoder_funcs imx_mipi_dsi_encoder_funcs = { +
>> > .destroy = imx_drm_encoder_destroy, +}; + +static bool
>> > imx_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
>> > + const struct
>> > drm_display_mode *mode, +
>> > struct drm_display_mode *adj_mode) +{ + return true; +}
>> > + +static int imx_mipi_dsi_encoder_atomic_check(struct
>> > drm_encoder *encoder, +
>> > struct drm_crtc_state *crtc_state, +
>> > struct drm_connector_state *conn) +{ + struct
>> > imx_crtc_state *imx_crtc_state =
>> > to_imx_crtc_state(crtc_state); + + /* The following
>> > values are taken from dw_hdmi_imx_atomic_check */ +
>> > imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24; +
>> > imx_crtc_state->di_hsync_pin = 2; +
>> > imx_crtc_state->di_vsync_pin = 3; + + return 0; +} +
>> > +static void imx_mipi_dsi_encoder_commit(struct drm_encoder
>> > *encoder) +{ + struct imx_mipi_dsi *dsi =
>> > enc_to_dsi(encoder); + int mux =
>> > drm_of_encoder_active_port_id(dsi->dev->of_node, encoder); +
>> > + imx_mipi_dsi_set_ipu_di_mux(dsi, mux); +} + +static
>> > void imx_mipi_dsi_encoder_disable(struct drm_encoder
>> > *encoder) +{ +} + +static const struct
>> > drm_encoder_helper_funcs imx_mipi_dsi_encoder_helpers = {
>> AFAIK (or at least this is the feedback I received)
>> drm_encoder is kind of deprecated, and nowadays all is migrated
>> to drm_bridge (encoders, bridges and panels). Of course, a
>> drm_encoder is needed, as the DRM core expects one, but should
>> just be a dummy object that you can probably create with just
>> calling drm_simple_encoder_init(). DRM maintainers, please
>> shout if I am wrong.
>
> That certainly matches my position :-) I would also add that the
> DRM encoder should be created in imx-drm-core.c or in
> ipuv3-crtc.c, not here.
>
Yes, I will do this change in v7 especially also because a new
commit 62fbddda2f72 ("drm/imx: Use simple encoder") landed in
-next recently which broke the current imx6 implementation by
removing the imx_drm_encoder_destroy() from imx-drm-core.c.
>> > + .mode_fixup = imx_mipi_dsi_encoder_mode_fixup,
>> > + .commit = imx_mipi_dsi_encoder_commit,
>> > + .disable = imx_mipi_dsi_encoder_disable,
>> > + .atomic_check = imx_mipi_dsi_encoder_atomic_check,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_register(struct drm_device *drm,
>> > + struct imx_mipi_dsi *dsi)
>> > +{
>> > + int ret;
>> > +
>> > + ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + drm_encoder_helper_add(&dsi->encoder,
>> > + &imx_mipi_dsi_encoder_helpers);
>> > + drm_encoder_init(drm, &dsi->encoder, &imx_mipi_dsi_encoder_funcs,
>> > + DRM_MODE_ENCODER_DSI, NULL);
>> > + return 0;
>> > +}
>> > +
>> > +static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
>> > + const struct drm_display_mode *mode)
>> > +{
>> > + /*
>> > + * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->hdisplay > 0x7ff)
>> > + return MODE_BAD_HVALUE;
>> > +
>> > + /*
>> > + * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->vdisplay > 0x7ff)
>> > + return MODE_BAD_VVALUE;
>> > +
>> > + return MODE_OK;
>> > +}
>> > +
>> > +
>> > +static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
>> > +{
>> > + unsigned int i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> > + if (dptdin_map[i].max_mbps == max_mbps)
>> > + return dptdin_map[i].testdin;
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
>> > +{
>> > + writel(val, dsi->base + reg);
>> > +}
>> > +
>> > +static int imx_mipi_dsi_phy_init(void *priv_data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int testdin;
>> > +
>> > + testdin = max_mbps_to_testdin(dsi->lane_mbps);
>> > + if (testdin < 0) {
>> > + DRM_DEV_ERROR(dsi->dev,
>> > + "failed to get testdin for %dmbps lane clock\n",
>> > + dsi->lane_mbps);
>> > + return testdin;
>> > + }
>> > +
>> > + dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
>> > + dsi_write(dsi, DSI_PWR_UP, POWERUP);
>> > +
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(0x44));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(testdin));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
>> > + const struct drm_display_mode *mode,
>> > + unsigned long mode_flags, u32 lanes,
>> > + u32 format, unsigned int *lane_mbps)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int bpp;
>> > + unsigned int i, target_mbps, mpclk;
>> > + unsigned long pllref;
>> > +
>> > + bpp = mipi_dsi_pixel_format_to_bpp(format);
>> > + if (bpp < 0) {
>> > + DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
>> > + format, bpp);
>> > + return bpp;
>> > + }
>> > +
>> > + pllref = clk_get_rate(dsi->pllref_clk);
>> > + if (pllref != 27000000)
>> > + DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
>> > +
>> > + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>> > + if (mpclk) {
>> > + /* take 1/0.7 blanking overhead into consideration */
>> > + target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
>> > + } else {
>> > + DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
>> > + target_mbps = 1000;
>> > + }
>> > +
>> > + DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
>> > + target_mbps);
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
>> > + if (target_mbps < dptdin_map[i].max_mbps) {
>> > + *lane_mbps = dptdin_map[i].max_mbps;
>> > + dsi->lane_mbps = *lane_mbps;
>> > + DRM_DEV_DEBUG(dsi->dev,
>> > + "real pllref_clk frequency is %uMbps\n",
>> > + *lane_mbps);
>> > + return 0;
>> > + }
>> > + }
>> > +
>> > + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
>> > + target_mbps);
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static int
>> > +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
>> > + struct dw_mipi_dsi_dphy_timing *timing)
>> > +{
>> > + timing->clk_hs2lp = 0x40;
>> > + timing->clk_lp2hs = 0x40;
>> > + timing->data_hs2lp = 0x40;
>> > + timing->data_lp2hs = 0x40;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
>> > + .init = imx_mipi_dsi_phy_init,
>> > + .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
>> > + .get_timing = dw_mipi_dsi_phy_get_timing,
>> > +};
>> > +
>> > +static struct dw_mipi_dsi_plat_data imx6q_mipi_dsi_drv_data = {
>> > + .max_data_lanes = 2,
>> > + .mode_valid = imx_mipi_dsi_mode_valid,
>> > + .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
>> > +};
>> > +
>> > +static const struct of_device_id imx_dsi_dt_ids[] = {
>> > + {
>> > + .compatible = "fsl,imx6q-mipi-dsi",
>> > + .data = &imx6q_mipi_dsi_drv_data,
>> > + },
>> > + { /* sentinel */ }
>> > +};
>> > +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
>> > +
>> > +static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > + struct drm_device *drm = data;
>> > + int ret;
>> > +
>> > + ret = clk_prepare_enable(dsi->pllref_clk);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = imx_mipi_dsi_register(drm, dsi);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to register: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > +
>> > + dw_mipi_dsi_unbind(dsi->mipi_dsi);
>> > +
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > +}
>> > +
>> > +static const struct component_ops imx_mipi_dsi_ops = {
>> > + .bind = imx_mipi_dsi_bind,
>> > + .unbind = imx_mipi_dsi_unbind,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_probe(struct platform_device *pdev)
>> > +{
>> > + struct device *dev = &pdev->dev;
>> > + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
>> > + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
>> > + struct imx_mipi_dsi *dsi;
>> > + struct resource *res;
>> > + int ret;
>> > +
>> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>> > + if (!dsi)
>> > + return -ENOMEM;
>> > +
>> > + dsi->dev = dev;
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + dsi->base = devm_ioremap_resource(dev, res);
>> > + if (IS_ERR(dsi->base)) {
>> > + ret = PTR_ERR(dsi->base);
>> > + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->pllref_clk = devm_clk_get(dev, "ref");
>> > + if (IS_ERR(dsi->pllref_clk)) {
>> > + ret = PTR_ERR(dsi->pllref_clk);
>> > + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
>> > + if (IS_ERR(dsi->mux_sel)) {
>> > + ret = PTR_ERR(dsi->mux_sel);
>> > + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dev_set_drvdata(dev, dsi);
>> > +
>> > + imx6q_mipi_dsi_drv_data.base = dsi->base;
>> > + imx6q_mipi_dsi_drv_data.priv_data = dsi;
>> > +
>> > + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
>> > + if (IS_ERR(dsi->mipi_dsi)) {
>> > + ret = PTR_ERR(dsi->mipi_dsi);
>> > + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
>> > + goto err_clkdisable;
>> > + }
>> > +
>> > + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
>> > +
>> > +err_clkdisable:
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > + return ret;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_remove(struct platform_device *pdev)
>> > +{
>> > + component_del(&pdev->dev, &imx_mipi_dsi_ops);
>> > + return 0;
>> > +}
>> > +
>> > +static struct platform_driver imx_mipi_dsi_driver = {
>> > + .probe = imx_mipi_dsi_probe,
>> > + .remove = imx_mipi_dsi_remove,
>> > + .driver = {
>> > + .of_match_table = imx_dsi_dt_ids,
>> > + .name = "dw-mipi-dsi-imx6",
>> > + },
>> > +};
>> > +module_platform_driver(imx_mipi_dsi_driver);
>> > +
>> > +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
>> > +MODULE_AUTHOR("Liu Ying <victor.liu-3arQi8VN3Tc@public.gmane.org>");
>> > +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>");
>> > +MODULE_LICENSE("GPL");
>
> --
> Regards,
>
> Laurent Pinchart
WARNING: multiple messages have this Message-ID (diff)
From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Enric Balletbo Serra <eballetbo@gmail.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Adrian Pop <pop.adrian61@gmail.com>,
Jonas Karlman <jonas@kwiboo.se>,
Martyn Welch <martyn.welch@collabora.com>,
Sjoerd Simons <sjoerd.simons@collabora.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Andrzej Hajda <a.hajda@samsung.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
linux-imx@nxp.com,
Arnaud Ferraris <arnaud.ferraris@collabora.com>,
Collabora Kernel ML <kernel@collabora.com>,
linux-stm32@st-md-mailman.stormreply.com,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v6 4/8] drm: imx: Add i.MX 6 MIPI DSI host platform driver
Date: Fri, 17 Apr 2020 12:56:17 +0300 [thread overview]
Message-ID: <87ftd2jv0e.fsf@collabora.com> (raw)
In-Reply-To: <20200415173018.GK4758@pendragon.ideasonboard.com>
Hi Enric & Laurent,
On Wed, 15 Apr 2020, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Enric,
>
> On Wed, Apr 15, 2020 at 07:26:02PM +0200, Enric Balletbo Serra
> wrote:
>> Missatge de Adrian Ratiu <adrian.ratiu@collabora.com> del dia
>> dt., 14 d’abr. 2020 a les 17:19:
>> >
>> > This adds support for the Synopsis DesignWare MIPI DSI v1.01
>> > host controller which is embedded in i.MX 6 SoCs.
>> >
>> > Based on following patches, but updated/extended to work with
>> > existing support found in the kernel:
>> >
>> > - drm: imx: Support Synopsys DesignWare MIPI DSI host
>> > controller
>> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> >
>> > Cc: Fabio Estevam <festevam@gmail.com> Reviewed-by: Emil
>> > Velikov <emil.velikov@collabora.com> Tested-by: Adrian Pop
>> > <pop.adrian61@gmail.com> Tested-by: Arnaud Ferraris
>> > <arnaud.ferraris@collabora.com> Signed-off-by: Sjoerd Simons
>> > <sjoerd.simons@collabora.com> Signed-off-by: Martyn Welch
>> > <martyn.welch@collabora.com> Signed-off-by: Adrian Ratiu
>> > <adrian.ratiu@collabora.com> --- Changes since v5:
>> > - Reword to remove unrelated device tree patch mention
>> > (Fabio) - Move pllref_clk enable/disable to bind/unbind
>> > (Ezequiel) - Fix freescale.com -> nxp.com email addresses
>> > (Fabio) - Also added myself as module author (Fabio) - Use
>> > DRM_DEV_* macros for consistency, print more error msg
>> >
>> > Changes since v4:
>> > - Split off driver-specific configuration of phy timings
>> > due to new upstream API. - Move regmap infrastructure
>> > logic to separate commit (Ezequiel) - Move dsi v1.01 layout
>> > addition to a separate commit (Ezequiel) - Minor warnings
>> > and driver name fixes
>> >
>> > Changes since v3:
>> > - Renamed platform driver to reflect it's i.MX6
>> > only. (Fabio)
>> >
>> > Changes since v2:
>> > - Fixed commit tags. (Emil)
>> >
>> > Changes since v1:
>> > - Moved register definitions & regmap initialization into
>> > bridge module. Platform drivers get the regmap via
>> > plat_data after calling the bridge probe. (Emil)
>> > ---
>> > drivers/gpu/drm/imx/Kconfig | 7 +
>> > drivers/gpu/drm/imx/Makefile | 1 +
>> > drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 409
>> > +++++++++++++++++++++++++ 3 files changed, 417 insertions(+)
>> > create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> >
>> > diff --git a/drivers/gpu/drm/imx/Kconfig
>> > b/drivers/gpu/drm/imx/Kconfig index
>> > 207bf7409dfb..b49e70e7f0fd 100644 ---
>> > a/drivers/gpu/drm/imx/Kconfig +++
>> > b/drivers/gpu/drm/imx/Kconfig @@ -39,3 +39,10 @@ config
>> > DRM_IMX_HDMI
>> > depends on DRM_IMX help
>> > Choose this if you want to use HDMI on i.MX6.
>> > + +config DRM_IMX6_MIPI_DSI + tristate "Freescale i.MX6
>> > DRM MIPI DSI" + select DRM_DW_MIPI_DSI + depends
>> > on DRM_IMX
>> Should it depend on CONFIG_OF too? I suspect you'll get build
>> errors if OF is not selected
>> > + help + Choose this if you want to use MIPI
>> > DSI on i.MX6. diff --git a/drivers/gpu/drm/imx/Makefile
>> > b/drivers/gpu/drm/imx/Makefile index
>> > 21cdcc2faabc..9a7843c59347 100644 ---
>> > a/drivers/gpu/drm/imx/Makefile +++
>> > b/drivers/gpu/drm/imx/Makefile @@ -9,3 +9,4 @@
>> > obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
>> > obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>> >
>> > obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
>> > +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o diff
>> > --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c new file mode 100644
>> > index 000000000000..6914db8ce8cb --- /dev/null +++
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c @@ -0,0 +1,409 @@
>> > +// SPDX-License-Identifier: GPL-2.0+ +/* + * i.MX6 drm
>> > driver - MIPI DSI Host Controller + * + * Copyright (C)
>> > 2011-2015 Freescale Semiconductor, Inc. + * Copyright (C)
>> > 2019-2020 Collabora, Ltd. + */ + +#include <linux/clk.h>
>> > +#include <linux/component.h> +#include <linux/mfd/syscon.h>
>> > +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> +#include
>> > <linux/module.h> +#include <linux/of_device.h> +#include
>> > <linux/regmap.h> +#include <linux/videodev2.h> +#include
>> > <drm/bridge/dw_mipi_dsi.h> +#include <drm/drm_crtc_helper.h>
>> > +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h>
>> > +#include <drm/drm_print.h> + +#include "imx-drm.h" +
>> > +#define DSI_PWR_UP 0x04 +#define RESET
>> > 0 +#define POWERUP BIT(0) +
>> > +#define DSI_PHY_IF_CTRL 0x5c +#define
>> > PHY_IF_CTRL_RESET 0x0 + +#define
>> > DSI_PHY_TST_CTRL0 0x64 +#define PHY_TESTCLK
>> > BIT(1) +#define PHY_UNTESTCLK 0 +#define
>> > PHY_TESTCLR BIT(0) +#define PHY_UNTESTCLR
>> > 0 + +#define DSI_PHY_TST_CTRL1 0x68 +#define
>> > PHY_TESTEN BIT(16) +#define PHY_UNTESTEN
>> > 0 +#define PHY_TESTDOUT(n) (((n) &
>> > 0xff) << 8) +#define PHY_TESTDIN(n) (((n) &
>> > 0xff) << 0) + +struct imx_mipi_dsi { + struct
>> > drm_encoder encoder; + struct device *dev; +
>> > struct regmap *mux_sel; + struct dw_mipi_dsi *mipi_dsi;
>> > + struct clk *pllref_clk; + + void __iomem *base;
>> > + unsigned int lane_mbps; +}; + +struct
>> > dphy_pll_testdin_map { + unsigned int max_mbps; +
>> > u8 testdin; +}; + +/* The table is based on 27MHz DPHY pll
>> > reference clock. */ +static const struct dphy_pll_testdin_map
>> > dptdin_map[] = { + {160, 0x04}, {180, 0x24}, {200,
>> > 0x44}, {210, 0x06}, + {240, 0x26}, {250, 0x46}, {270,
>> > 0x08}, {300, 0x28}, + {330, 0x48}, {360, 0x2a}, {400,
>> > 0x4a}, {450, 0x0c}, + {500, 0x2c}, {550, 0x0e}, {600,
>> > 0x2e}, {650, 0x10}, + {700, 0x30}, {750, 0x12}, {800,
>> > 0x32}, {850, 0x14}, + {900, 0x34}, {950, 0x54}, {1000,
>> > 0x74} +}; + +static inline struct imx_mipi_dsi
>> > *enc_to_dsi(struct drm_encoder *enc) +{ + return
>> > container_of(enc, struct imx_mipi_dsi, encoder); +} + +static
>> > void imx_mipi_dsi_set_ipu_di_mux(struct imx_mipi_dsi *dsi,
>> > int ipu_di) +{ + regmap_update_bits(dsi->mux_sel,
>> > IOMUXC_GPR3, +
>> > IMX6Q_GPR3_MIPI_MUX_CTL_MASK, +
>> > ipu_di << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT); +} + +static const
>> > struct drm_encoder_funcs imx_mipi_dsi_encoder_funcs = { +
>> > .destroy = imx_drm_encoder_destroy, +}; + +static bool
>> > imx_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
>> > + const struct
>> > drm_display_mode *mode, +
>> > struct drm_display_mode *adj_mode) +{ + return true; +}
>> > + +static int imx_mipi_dsi_encoder_atomic_check(struct
>> > drm_encoder *encoder, +
>> > struct drm_crtc_state *crtc_state, +
>> > struct drm_connector_state *conn) +{ + struct
>> > imx_crtc_state *imx_crtc_state =
>> > to_imx_crtc_state(crtc_state); + + /* The following
>> > values are taken from dw_hdmi_imx_atomic_check */ +
>> > imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24; +
>> > imx_crtc_state->di_hsync_pin = 2; +
>> > imx_crtc_state->di_vsync_pin = 3; + + return 0; +} +
>> > +static void imx_mipi_dsi_encoder_commit(struct drm_encoder
>> > *encoder) +{ + struct imx_mipi_dsi *dsi =
>> > enc_to_dsi(encoder); + int mux =
>> > drm_of_encoder_active_port_id(dsi->dev->of_node, encoder); +
>> > + imx_mipi_dsi_set_ipu_di_mux(dsi, mux); +} + +static
>> > void imx_mipi_dsi_encoder_disable(struct drm_encoder
>> > *encoder) +{ +} + +static const struct
>> > drm_encoder_helper_funcs imx_mipi_dsi_encoder_helpers = {
>> AFAIK (or at least this is the feedback I received)
>> drm_encoder is kind of deprecated, and nowadays all is migrated
>> to drm_bridge (encoders, bridges and panels). Of course, a
>> drm_encoder is needed, as the DRM core expects one, but should
>> just be a dummy object that you can probably create with just
>> calling drm_simple_encoder_init(). DRM maintainers, please
>> shout if I am wrong.
>
> That certainly matches my position :-) I would also add that the
> DRM encoder should be created in imx-drm-core.c or in
> ipuv3-crtc.c, not here.
>
Yes, I will do this change in v7 especially also because a new
commit 62fbddda2f72 ("drm/imx: Use simple encoder") landed in
-next recently which broke the current imx6 implementation by
removing the imx_drm_encoder_destroy() from imx-drm-core.c.
>> > + .mode_fixup = imx_mipi_dsi_encoder_mode_fixup,
>> > + .commit = imx_mipi_dsi_encoder_commit,
>> > + .disable = imx_mipi_dsi_encoder_disable,
>> > + .atomic_check = imx_mipi_dsi_encoder_atomic_check,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_register(struct drm_device *drm,
>> > + struct imx_mipi_dsi *dsi)
>> > +{
>> > + int ret;
>> > +
>> > + ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + drm_encoder_helper_add(&dsi->encoder,
>> > + &imx_mipi_dsi_encoder_helpers);
>> > + drm_encoder_init(drm, &dsi->encoder, &imx_mipi_dsi_encoder_funcs,
>> > + DRM_MODE_ENCODER_DSI, NULL);
>> > + return 0;
>> > +}
>> > +
>> > +static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
>> > + const struct drm_display_mode *mode)
>> > +{
>> > + /*
>> > + * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->hdisplay > 0x7ff)
>> > + return MODE_BAD_HVALUE;
>> > +
>> > + /*
>> > + * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->vdisplay > 0x7ff)
>> > + return MODE_BAD_VVALUE;
>> > +
>> > + return MODE_OK;
>> > +}
>> > +
>> > +
>> > +static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
>> > +{
>> > + unsigned int i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> > + if (dptdin_map[i].max_mbps == max_mbps)
>> > + return dptdin_map[i].testdin;
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
>> > +{
>> > + writel(val, dsi->base + reg);
>> > +}
>> > +
>> > +static int imx_mipi_dsi_phy_init(void *priv_data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int testdin;
>> > +
>> > + testdin = max_mbps_to_testdin(dsi->lane_mbps);
>> > + if (testdin < 0) {
>> > + DRM_DEV_ERROR(dsi->dev,
>> > + "failed to get testdin for %dmbps lane clock\n",
>> > + dsi->lane_mbps);
>> > + return testdin;
>> > + }
>> > +
>> > + dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
>> > + dsi_write(dsi, DSI_PWR_UP, POWERUP);
>> > +
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(0x44));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(testdin));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
>> > + const struct drm_display_mode *mode,
>> > + unsigned long mode_flags, u32 lanes,
>> > + u32 format, unsigned int *lane_mbps)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int bpp;
>> > + unsigned int i, target_mbps, mpclk;
>> > + unsigned long pllref;
>> > +
>> > + bpp = mipi_dsi_pixel_format_to_bpp(format);
>> > + if (bpp < 0) {
>> > + DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
>> > + format, bpp);
>> > + return bpp;
>> > + }
>> > +
>> > + pllref = clk_get_rate(dsi->pllref_clk);
>> > + if (pllref != 27000000)
>> > + DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
>> > +
>> > + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>> > + if (mpclk) {
>> > + /* take 1/0.7 blanking overhead into consideration */
>> > + target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
>> > + } else {
>> > + DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
>> > + target_mbps = 1000;
>> > + }
>> > +
>> > + DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
>> > + target_mbps);
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
>> > + if (target_mbps < dptdin_map[i].max_mbps) {
>> > + *lane_mbps = dptdin_map[i].max_mbps;
>> > + dsi->lane_mbps = *lane_mbps;
>> > + DRM_DEV_DEBUG(dsi->dev,
>> > + "real pllref_clk frequency is %uMbps\n",
>> > + *lane_mbps);
>> > + return 0;
>> > + }
>> > + }
>> > +
>> > + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
>> > + target_mbps);
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static int
>> > +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
>> > + struct dw_mipi_dsi_dphy_timing *timing)
>> > +{
>> > + timing->clk_hs2lp = 0x40;
>> > + timing->clk_lp2hs = 0x40;
>> > + timing->data_hs2lp = 0x40;
>> > + timing->data_lp2hs = 0x40;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
>> > + .init = imx_mipi_dsi_phy_init,
>> > + .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
>> > + .get_timing = dw_mipi_dsi_phy_get_timing,
>> > +};
>> > +
>> > +static struct dw_mipi_dsi_plat_data imx6q_mipi_dsi_drv_data = {
>> > + .max_data_lanes = 2,
>> > + .mode_valid = imx_mipi_dsi_mode_valid,
>> > + .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
>> > +};
>> > +
>> > +static const struct of_device_id imx_dsi_dt_ids[] = {
>> > + {
>> > + .compatible = "fsl,imx6q-mipi-dsi",
>> > + .data = &imx6q_mipi_dsi_drv_data,
>> > + },
>> > + { /* sentinel */ }
>> > +};
>> > +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
>> > +
>> > +static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > + struct drm_device *drm = data;
>> > + int ret;
>> > +
>> > + ret = clk_prepare_enable(dsi->pllref_clk);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = imx_mipi_dsi_register(drm, dsi);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to register: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > +
>> > + dw_mipi_dsi_unbind(dsi->mipi_dsi);
>> > +
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > +}
>> > +
>> > +static const struct component_ops imx_mipi_dsi_ops = {
>> > + .bind = imx_mipi_dsi_bind,
>> > + .unbind = imx_mipi_dsi_unbind,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_probe(struct platform_device *pdev)
>> > +{
>> > + struct device *dev = &pdev->dev;
>> > + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
>> > + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
>> > + struct imx_mipi_dsi *dsi;
>> > + struct resource *res;
>> > + int ret;
>> > +
>> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>> > + if (!dsi)
>> > + return -ENOMEM;
>> > +
>> > + dsi->dev = dev;
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + dsi->base = devm_ioremap_resource(dev, res);
>> > + if (IS_ERR(dsi->base)) {
>> > + ret = PTR_ERR(dsi->base);
>> > + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->pllref_clk = devm_clk_get(dev, "ref");
>> > + if (IS_ERR(dsi->pllref_clk)) {
>> > + ret = PTR_ERR(dsi->pllref_clk);
>> > + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
>> > + if (IS_ERR(dsi->mux_sel)) {
>> > + ret = PTR_ERR(dsi->mux_sel);
>> > + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dev_set_drvdata(dev, dsi);
>> > +
>> > + imx6q_mipi_dsi_drv_data.base = dsi->base;
>> > + imx6q_mipi_dsi_drv_data.priv_data = dsi;
>> > +
>> > + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
>> > + if (IS_ERR(dsi->mipi_dsi)) {
>> > + ret = PTR_ERR(dsi->mipi_dsi);
>> > + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
>> > + goto err_clkdisable;
>> > + }
>> > +
>> > + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
>> > +
>> > +err_clkdisable:
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > + return ret;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_remove(struct platform_device *pdev)
>> > +{
>> > + component_del(&pdev->dev, &imx_mipi_dsi_ops);
>> > + return 0;
>> > +}
>> > +
>> > +static struct platform_driver imx_mipi_dsi_driver = {
>> > + .probe = imx_mipi_dsi_probe,
>> > + .remove = imx_mipi_dsi_remove,
>> > + .driver = {
>> > + .of_match_table = imx_dsi_dt_ids,
>> > + .name = "dw-mipi-dsi-imx6",
>> > + },
>> > +};
>> > +module_platform_driver(imx_mipi_dsi_driver);
>> > +
>> > +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
>> > +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>> > +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
>> > +MODULE_LICENSE("GPL");
>
> --
> Regards,
>
> Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Adrian Ratiu <adrian.ratiu@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Enric Balletbo Serra <eballetbo@gmail.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Adrian Pop <pop.adrian61@gmail.com>,
Jonas Karlman <jonas@kwiboo.se>,
Martyn Welch <martyn.welch@collabora.com>,
Sjoerd Simons <sjoerd.simons@collabora.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Andrzej Hajda <a.hajda@samsung.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
linux-imx@nxp.com,
Arnaud Ferraris <arnaud.ferraris@collabora.com>,
Collabora Kernel ML <kernel@collabora.com>,
linux-stm32@st-md-mailman.stormreply.com,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Emil Velikov <emil.velikov@collabora.com>
Subject: Re: [PATCH v6 4/8] drm: imx: Add i.MX 6 MIPI DSI host platform driver
Date: Fri, 17 Apr 2020 12:56:17 +0300 [thread overview]
Message-ID: <87ftd2jv0e.fsf@collabora.com> (raw)
In-Reply-To: <20200415173018.GK4758@pendragon.ideasonboard.com>
Hi Enric & Laurent,
On Wed, 15 Apr 2020, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Enric,
>
> On Wed, Apr 15, 2020 at 07:26:02PM +0200, Enric Balletbo Serra
> wrote:
>> Missatge de Adrian Ratiu <adrian.ratiu@collabora.com> del dia
>> dt., 14 d’abr. 2020 a les 17:19:
>> >
>> > This adds support for the Synopsis DesignWare MIPI DSI v1.01
>> > host controller which is embedded in i.MX 6 SoCs.
>> >
>> > Based on following patches, but updated/extended to work with
>> > existing support found in the kernel:
>> >
>> > - drm: imx: Support Synopsys DesignWare MIPI DSI host
>> > controller
>> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> >
>> > Cc: Fabio Estevam <festevam@gmail.com> Reviewed-by: Emil
>> > Velikov <emil.velikov@collabora.com> Tested-by: Adrian Pop
>> > <pop.adrian61@gmail.com> Tested-by: Arnaud Ferraris
>> > <arnaud.ferraris@collabora.com> Signed-off-by: Sjoerd Simons
>> > <sjoerd.simons@collabora.com> Signed-off-by: Martyn Welch
>> > <martyn.welch@collabora.com> Signed-off-by: Adrian Ratiu
>> > <adrian.ratiu@collabora.com> --- Changes since v5:
>> > - Reword to remove unrelated device tree patch mention
>> > (Fabio) - Move pllref_clk enable/disable to bind/unbind
>> > (Ezequiel) - Fix freescale.com -> nxp.com email addresses
>> > (Fabio) - Also added myself as module author (Fabio) - Use
>> > DRM_DEV_* macros for consistency, print more error msg
>> >
>> > Changes since v4:
>> > - Split off driver-specific configuration of phy timings
>> > due to new upstream API. - Move regmap infrastructure
>> > logic to separate commit (Ezequiel) - Move dsi v1.01 layout
>> > addition to a separate commit (Ezequiel) - Minor warnings
>> > and driver name fixes
>> >
>> > Changes since v3:
>> > - Renamed platform driver to reflect it's i.MX6
>> > only. (Fabio)
>> >
>> > Changes since v2:
>> > - Fixed commit tags. (Emil)
>> >
>> > Changes since v1:
>> > - Moved register definitions & regmap initialization into
>> > bridge module. Platform drivers get the regmap via
>> > plat_data after calling the bridge probe. (Emil)
>> > ---
>> > drivers/gpu/drm/imx/Kconfig | 7 +
>> > drivers/gpu/drm/imx/Makefile | 1 +
>> > drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 409
>> > +++++++++++++++++++++++++ 3 files changed, 417 insertions(+)
>> > create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> >
>> > diff --git a/drivers/gpu/drm/imx/Kconfig
>> > b/drivers/gpu/drm/imx/Kconfig index
>> > 207bf7409dfb..b49e70e7f0fd 100644 ---
>> > a/drivers/gpu/drm/imx/Kconfig +++
>> > b/drivers/gpu/drm/imx/Kconfig @@ -39,3 +39,10 @@ config
>> > DRM_IMX_HDMI
>> > depends on DRM_IMX help
>> > Choose this if you want to use HDMI on i.MX6.
>> > + +config DRM_IMX6_MIPI_DSI + tristate "Freescale i.MX6
>> > DRM MIPI DSI" + select DRM_DW_MIPI_DSI + depends
>> > on DRM_IMX
>> Should it depend on CONFIG_OF too? I suspect you'll get build
>> errors if OF is not selected
>> > + help + Choose this if you want to use MIPI
>> > DSI on i.MX6. diff --git a/drivers/gpu/drm/imx/Makefile
>> > b/drivers/gpu/drm/imx/Makefile index
>> > 21cdcc2faabc..9a7843c59347 100644 ---
>> > a/drivers/gpu/drm/imx/Makefile +++
>> > b/drivers/gpu/drm/imx/Makefile @@ -9,3 +9,4 @@
>> > obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
>> > obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>> >
>> > obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
>> > +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o diff
>> > --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c new file mode 100644
>> > index 000000000000..6914db8ce8cb --- /dev/null +++
>> > b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c @@ -0,0 +1,409 @@
>> > +// SPDX-License-Identifier: GPL-2.0+ +/* + * i.MX6 drm
>> > driver - MIPI DSI Host Controller + * + * Copyright (C)
>> > 2011-2015 Freescale Semiconductor, Inc. + * Copyright (C)
>> > 2019-2020 Collabora, Ltd. + */ + +#include <linux/clk.h>
>> > +#include <linux/component.h> +#include <linux/mfd/syscon.h>
>> > +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> +#include
>> > <linux/module.h> +#include <linux/of_device.h> +#include
>> > <linux/regmap.h> +#include <linux/videodev2.h> +#include
>> > <drm/bridge/dw_mipi_dsi.h> +#include <drm/drm_crtc_helper.h>
>> > +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_of.h>
>> > +#include <drm/drm_print.h> + +#include "imx-drm.h" +
>> > +#define DSI_PWR_UP 0x04 +#define RESET
>> > 0 +#define POWERUP BIT(0) +
>> > +#define DSI_PHY_IF_CTRL 0x5c +#define
>> > PHY_IF_CTRL_RESET 0x0 + +#define
>> > DSI_PHY_TST_CTRL0 0x64 +#define PHY_TESTCLK
>> > BIT(1) +#define PHY_UNTESTCLK 0 +#define
>> > PHY_TESTCLR BIT(0) +#define PHY_UNTESTCLR
>> > 0 + +#define DSI_PHY_TST_CTRL1 0x68 +#define
>> > PHY_TESTEN BIT(16) +#define PHY_UNTESTEN
>> > 0 +#define PHY_TESTDOUT(n) (((n) &
>> > 0xff) << 8) +#define PHY_TESTDIN(n) (((n) &
>> > 0xff) << 0) + +struct imx_mipi_dsi { + struct
>> > drm_encoder encoder; + struct device *dev; +
>> > struct regmap *mux_sel; + struct dw_mipi_dsi *mipi_dsi;
>> > + struct clk *pllref_clk; + + void __iomem *base;
>> > + unsigned int lane_mbps; +}; + +struct
>> > dphy_pll_testdin_map { + unsigned int max_mbps; +
>> > u8 testdin; +}; + +/* The table is based on 27MHz DPHY pll
>> > reference clock. */ +static const struct dphy_pll_testdin_map
>> > dptdin_map[] = { + {160, 0x04}, {180, 0x24}, {200,
>> > 0x44}, {210, 0x06}, + {240, 0x26}, {250, 0x46}, {270,
>> > 0x08}, {300, 0x28}, + {330, 0x48}, {360, 0x2a}, {400,
>> > 0x4a}, {450, 0x0c}, + {500, 0x2c}, {550, 0x0e}, {600,
>> > 0x2e}, {650, 0x10}, + {700, 0x30}, {750, 0x12}, {800,
>> > 0x32}, {850, 0x14}, + {900, 0x34}, {950, 0x54}, {1000,
>> > 0x74} +}; + +static inline struct imx_mipi_dsi
>> > *enc_to_dsi(struct drm_encoder *enc) +{ + return
>> > container_of(enc, struct imx_mipi_dsi, encoder); +} + +static
>> > void imx_mipi_dsi_set_ipu_di_mux(struct imx_mipi_dsi *dsi,
>> > int ipu_di) +{ + regmap_update_bits(dsi->mux_sel,
>> > IOMUXC_GPR3, +
>> > IMX6Q_GPR3_MIPI_MUX_CTL_MASK, +
>> > ipu_di << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT); +} + +static const
>> > struct drm_encoder_funcs imx_mipi_dsi_encoder_funcs = { +
>> > .destroy = imx_drm_encoder_destroy, +}; + +static bool
>> > imx_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
>> > + const struct
>> > drm_display_mode *mode, +
>> > struct drm_display_mode *adj_mode) +{ + return true; +}
>> > + +static int imx_mipi_dsi_encoder_atomic_check(struct
>> > drm_encoder *encoder, +
>> > struct drm_crtc_state *crtc_state, +
>> > struct drm_connector_state *conn) +{ + struct
>> > imx_crtc_state *imx_crtc_state =
>> > to_imx_crtc_state(crtc_state); + + /* The following
>> > values are taken from dw_hdmi_imx_atomic_check */ +
>> > imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24; +
>> > imx_crtc_state->di_hsync_pin = 2; +
>> > imx_crtc_state->di_vsync_pin = 3; + + return 0; +} +
>> > +static void imx_mipi_dsi_encoder_commit(struct drm_encoder
>> > *encoder) +{ + struct imx_mipi_dsi *dsi =
>> > enc_to_dsi(encoder); + int mux =
>> > drm_of_encoder_active_port_id(dsi->dev->of_node, encoder); +
>> > + imx_mipi_dsi_set_ipu_di_mux(dsi, mux); +} + +static
>> > void imx_mipi_dsi_encoder_disable(struct drm_encoder
>> > *encoder) +{ +} + +static const struct
>> > drm_encoder_helper_funcs imx_mipi_dsi_encoder_helpers = {
>> AFAIK (or at least this is the feedback I received)
>> drm_encoder is kind of deprecated, and nowadays all is migrated
>> to drm_bridge (encoders, bridges and panels). Of course, a
>> drm_encoder is needed, as the DRM core expects one, but should
>> just be a dummy object that you can probably create with just
>> calling drm_simple_encoder_init(). DRM maintainers, please
>> shout if I am wrong.
>
> That certainly matches my position :-) I would also add that the
> DRM encoder should be created in imx-drm-core.c or in
> ipuv3-crtc.c, not here.
>
Yes, I will do this change in v7 especially also because a new
commit 62fbddda2f72 ("drm/imx: Use simple encoder") landed in
-next recently which broke the current imx6 implementation by
removing the imx_drm_encoder_destroy() from imx-drm-core.c.
>> > + .mode_fixup = imx_mipi_dsi_encoder_mode_fixup,
>> > + .commit = imx_mipi_dsi_encoder_commit,
>> > + .disable = imx_mipi_dsi_encoder_disable,
>> > + .atomic_check = imx_mipi_dsi_encoder_atomic_check,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_register(struct drm_device *drm,
>> > + struct imx_mipi_dsi *dsi)
>> > +{
>> > + int ret;
>> > +
>> > + ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + drm_encoder_helper_add(&dsi->encoder,
>> > + &imx_mipi_dsi_encoder_helpers);
>> > + drm_encoder_init(drm, &dsi->encoder, &imx_mipi_dsi_encoder_funcs,
>> > + DRM_MODE_ENCODER_DSI, NULL);
>> > + return 0;
>> > +}
>> > +
>> > +static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
>> > + const struct drm_display_mode *mode)
>> > +{
>> > + /*
>> > + * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->hdisplay > 0x7ff)
>> > + return MODE_BAD_HVALUE;
>> > +
>> > + /*
>> > + * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
>> > + * register is 11-bit.
>> > + */
>> > + if (mode->vdisplay > 0x7ff)
>> > + return MODE_BAD_VVALUE;
>> > +
>> > + return MODE_OK;
>> > +}
>> > +
>> > +
>> > +static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
>> > +{
>> > + unsigned int i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
>> > + if (dptdin_map[i].max_mbps == max_mbps)
>> > + return dptdin_map[i].testdin;
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
>> > +{
>> > + writel(val, dsi->base + reg);
>> > +}
>> > +
>> > +static int imx_mipi_dsi_phy_init(void *priv_data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int testdin;
>> > +
>> > + testdin = max_mbps_to_testdin(dsi->lane_mbps);
>> > + if (testdin < 0) {
>> > + DRM_DEV_ERROR(dsi->dev,
>> > + "failed to get testdin for %dmbps lane clock\n",
>> > + dsi->lane_mbps);
>> > + return testdin;
>> > + }
>> > +
>> > + dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
>> > + dsi_write(dsi, DSI_PWR_UP, POWERUP);
>> > +
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(0x44));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
>> > + PHY_TESTDIN(testdin));
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
>> > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
>> > + const struct drm_display_mode *mode,
>> > + unsigned long mode_flags, u32 lanes,
>> > + u32 format, unsigned int *lane_mbps)
>> > +{
>> > + struct imx_mipi_dsi *dsi = priv_data;
>> > + int bpp;
>> > + unsigned int i, target_mbps, mpclk;
>> > + unsigned long pllref;
>> > +
>> > + bpp = mipi_dsi_pixel_format_to_bpp(format);
>> > + if (bpp < 0) {
>> > + DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
>> > + format, bpp);
>> > + return bpp;
>> > + }
>> > +
>> > + pllref = clk_get_rate(dsi->pllref_clk);
>> > + if (pllref != 27000000)
>> > + DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
>> > +
>> > + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>> > + if (mpclk) {
>> > + /* take 1/0.7 blanking overhead into consideration */
>> > + target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
>> > + } else {
>> > + DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
>> > + target_mbps = 1000;
>> > + }
>> > +
>> > + DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
>> > + target_mbps);
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
>> > + if (target_mbps < dptdin_map[i].max_mbps) {
>> > + *lane_mbps = dptdin_map[i].max_mbps;
>> > + dsi->lane_mbps = *lane_mbps;
>> > + DRM_DEV_DEBUG(dsi->dev,
>> > + "real pllref_clk frequency is %uMbps\n",
>> > + *lane_mbps);
>> > + return 0;
>> > + }
>> > + }
>> > +
>> > + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
>> > + target_mbps);
>> > +
>> > + return -EINVAL;
>> > +}
>> > +
>> > +static int
>> > +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
>> > + struct dw_mipi_dsi_dphy_timing *timing)
>> > +{
>> > + timing->clk_hs2lp = 0x40;
>> > + timing->clk_lp2hs = 0x40;
>> > + timing->data_hs2lp = 0x40;
>> > + timing->data_lp2hs = 0x40;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
>> > + .init = imx_mipi_dsi_phy_init,
>> > + .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
>> > + .get_timing = dw_mipi_dsi_phy_get_timing,
>> > +};
>> > +
>> > +static struct dw_mipi_dsi_plat_data imx6q_mipi_dsi_drv_data = {
>> > + .max_data_lanes = 2,
>> > + .mode_valid = imx_mipi_dsi_mode_valid,
>> > + .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
>> > +};
>> > +
>> > +static const struct of_device_id imx_dsi_dt_ids[] = {
>> > + {
>> > + .compatible = "fsl,imx6q-mipi-dsi",
>> > + .data = &imx6q_mipi_dsi_drv_data,
>> > + },
>> > + { /* sentinel */ }
>> > +};
>> > +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
>> > +
>> > +static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > + struct drm_device *drm = data;
>> > + int ret;
>> > +
>> > + ret = clk_prepare_enable(dsi->pllref_clk);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = imx_mipi_dsi_register(drm, dsi);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to register: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder);
>> > + if (ret) {
>> > + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
>> > + void *data)
>> > +{
>> > + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> > +
>> > + dw_mipi_dsi_unbind(dsi->mipi_dsi);
>> > +
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > +}
>> > +
>> > +static const struct component_ops imx_mipi_dsi_ops = {
>> > + .bind = imx_mipi_dsi_bind,
>> > + .unbind = imx_mipi_dsi_unbind,
>> > +};
>> > +
>> > +static int imx_mipi_dsi_probe(struct platform_device *pdev)
>> > +{
>> > + struct device *dev = &pdev->dev;
>> > + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
>> > + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
>> > + struct imx_mipi_dsi *dsi;
>> > + struct resource *res;
>> > + int ret;
>> > +
>> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>> > + if (!dsi)
>> > + return -ENOMEM;
>> > +
>> > + dsi->dev = dev;
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + dsi->base = devm_ioremap_resource(dev, res);
>> > + if (IS_ERR(dsi->base)) {
>> > + ret = PTR_ERR(dsi->base);
>> > + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->pllref_clk = devm_clk_get(dev, "ref");
>> > + if (IS_ERR(dsi->pllref_clk)) {
>> > + ret = PTR_ERR(dsi->pllref_clk);
>> > + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
>> > + if (IS_ERR(dsi->mux_sel)) {
>> > + ret = PTR_ERR(dsi->mux_sel);
>> > + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
>> > + return ret;
>> > + }
>> > +
>> > + dev_set_drvdata(dev, dsi);
>> > +
>> > + imx6q_mipi_dsi_drv_data.base = dsi->base;
>> > + imx6q_mipi_dsi_drv_data.priv_data = dsi;
>> > +
>> > + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
>> > + if (IS_ERR(dsi->mipi_dsi)) {
>> > + ret = PTR_ERR(dsi->mipi_dsi);
>> > + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
>> > + goto err_clkdisable;
>> > + }
>> > +
>> > + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
>> > +
>> > +err_clkdisable:
>> > + clk_disable_unprepare(dsi->pllref_clk);
>> > + return ret;
>> > +}
>> > +
>> > +static int imx_mipi_dsi_remove(struct platform_device *pdev)
>> > +{
>> > + component_del(&pdev->dev, &imx_mipi_dsi_ops);
>> > + return 0;
>> > +}
>> > +
>> > +static struct platform_driver imx_mipi_dsi_driver = {
>> > + .probe = imx_mipi_dsi_probe,
>> > + .remove = imx_mipi_dsi_remove,
>> > + .driver = {
>> > + .of_match_table = imx_dsi_dt_ids,
>> > + .name = "dw-mipi-dsi-imx6",
>> > + },
>> > +};
>> > +module_platform_driver(imx_mipi_dsi_driver);
>> > +
>> > +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
>> > +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>> > +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
>> > +MODULE_LICENSE("GPL");
>
> --
> Regards,
>
> Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-04-17 9:55 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-14 15:19 [PATCH v6 0/8] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 1/8] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-15 15:53 ` Enric Balletbo Serra
2020-04-15 15:53 ` Enric Balletbo Serra
2020-04-15 15:53 ` Enric Balletbo Serra
2020-04-15 15:53 ` Enric Balletbo Serra
2020-04-16 18:25 ` Adrian Ratiu
2020-04-16 18:25 ` Adrian Ratiu
2020-04-16 18:25 ` Adrian Ratiu
2020-04-16 18:25 ` Adrian Ratiu
2020-04-16 18:41 ` Adrian Ratiu
2020-04-16 18:41 ` Adrian Ratiu
2020-04-16 18:41 ` Adrian Ratiu
2020-04-16 18:41 ` Adrian Ratiu
2020-04-16 20:15 ` Enric Balletbo Serra
2020-04-16 20:15 ` Enric Balletbo Serra
2020-04-16 20:15 ` Enric Balletbo Serra
2020-04-16 20:15 ` Enric Balletbo Serra
2020-04-17 8:07 ` Adrian Ratiu
2020-04-17 8:07 ` Adrian Ratiu
2020-04-17 8:07 ` Adrian Ratiu
2020-04-17 8:07 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 2/8] drm: bridge: dw_mipi_dsi: abstract register access using reg_fields Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 3/8] drm: bridge: synopsis: add dsi v1.01 support Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 4/8] drm: imx: Add i.MX 6 MIPI DSI host platform driver Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-15 17:26 ` Enric Balletbo Serra
2020-04-15 17:26 ` Enric Balletbo Serra
2020-04-15 17:26 ` Enric Balletbo Serra
2020-04-15 17:26 ` Enric Balletbo Serra
2020-04-15 17:30 ` Laurent Pinchart
2020-04-15 17:30 ` Laurent Pinchart
2020-04-15 17:30 ` Laurent Pinchart
2020-04-15 17:30 ` Laurent Pinchart
2020-04-17 9:56 ` Adrian Ratiu [this message]
2020-04-17 9:56 ` Adrian Ratiu
2020-04-17 9:56 ` Adrian Ratiu
2020-04-17 9:56 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 5/8] dt-bindings: display: add i.MX6 MIPI DSI host controller doc Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 20:42 ` Laurent Pinchart
2020-04-14 20:42 ` Laurent Pinchart
2020-04-14 20:42 ` Laurent Pinchart
2020-04-14 20:42 ` Laurent Pinchart
2020-04-15 11:28 ` Adrian Ratiu
2020-04-15 11:28 ` Adrian Ratiu
2020-04-15 11:28 ` Adrian Ratiu
2020-04-15 11:28 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 6/8] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 7/8] drm: bridge: dw-mipi-dsi: split low power cfg register into fields Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` [PATCH v6 8/8] drm: bridge: dw-mipi-dsi: fix bad register field offsets Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
2020-04-14 15:19 ` Adrian Ratiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ftd2jv0e.fsf@collabora.com \
--to=adrian.ratiu@collabora.com \
--cc=a.hajda@samsung.com \
--cc=arnaud.ferraris@collabora.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=eballetbo@gmail.com \
--cc=emil.velikov@collabora.com \
--cc=jernej.skrabec@siol.net \
--cc=jonas@kwiboo.se \
--cc=kernel@collabora.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=martyn.welch@collabora.com \
--cc=pop.adrian61@gmail.com \
--cc=sjoerd.simons@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.