From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8E9B7C433F5 for ; Tue, 4 Oct 2022 21:01:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229547AbiJDVBK (ORCPT ); Tue, 4 Oct 2022 17:01:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52858 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229459AbiJDVBH (ORCPT ); Tue, 4 Oct 2022 17:01:07 -0400 Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [IPv6:2a00:1450:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 27551647CF for ; Tue, 4 Oct 2022 14:01:04 -0700 (PDT) Received: by mail-ej1-x633.google.com with SMTP id a2so11363792ejx.10 for ; Tue, 04 Oct 2022 14:01:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amarulasolutions.com; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=DUSpKZy1pIehtfifTeG1uHTVtYiF7pH1Vfn1Aaxrnr4=; b=hNjgHaSxgFJe9O9AUlLxrS/sfC59d3qbVJo+Gg3PRA5K2HxzLjucN0B4ipFWjeQEdA fvOTutycEfre/zBrCEuE+E0eDSn/R/yWMa+LaeP8yS+3oDHGuX2WJEf4wGNlVpsnwn6o Oe/TMi/0OBGaRSVU6xslBbZ6CFA5VBcNzISfc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DUSpKZy1pIehtfifTeG1uHTVtYiF7pH1Vfn1Aaxrnr4=; b=DVVftk7+95e10q1wb4itmYQmEBWPp65KZKS7y9nz7oQ9OuP3+ZEtHeKmeEOPE7x5A1 YP6qgEmSvS8+H4SGvL9MUPkQEMoawGXmbRK1oV/l40LZYfUDZ8NbPeZVECwKKUOHsYkp iGQcqZF6GfXNkWMA0ajtkOjkJrleYs1bHMlckTChEYco1i2tKIRK5QPH8PJyf5XVXvek X/WYd1CUdCCQSMpixh9en58vWX9FvbXU9HJgBEl6x4QyMhvQ+9ayfjp+M9mAG6bC/F5w uYJxuGNgS0Eb2tNq33250rj2JJeBo3EcGntbT/raPWMxlDKarhg3Fk25ZrwJiVjDEGPr +NQA== X-Gm-Message-State: ACrzQf12Bz/58YaakyjI0fjBOmvnKvJz8feEdhoox3P9V7qs69RJzJIC BjNl32abTvi1diT62ZtF3gMXTSaAqC7YgvANV9RDxQ== X-Google-Smtp-Source: AMsMyM6Y5Mld63PxpEcIWvAH+WQ6xssr5LIa95KQRRdxzoEFSUCTCMhfjQdmoI9vMRi4GKMgbmCtaz7zKDm0fkocOj0= X-Received: by 2002:a17:907:9727:b0:78a:e09c:f2f9 with SMTP id jg39-20020a170907972700b0078ae09cf2f9mr10270865ejc.455.1664917262314; Tue, 04 Oct 2022 14:01:02 -0700 (PDT) MIME-Version: 1.0 References: <20221001080650.1007043-1-jagan@amarulasolutions.com> <20221001080650.1007043-2-jagan@amarulasolutions.com> <88eb9060-7240-3907-5d78-c05274d082df@samsung.com> In-Reply-To: <88eb9060-7240-3907-5d78-c05274d082df@samsung.com> From: Jagan Teki Date: Wed, 5 Oct 2022 02:30:50 +0530 Message-ID: Subject: Re: [PATCH v6 01/10] drm: bridge: Add Samsung DSIM bridge driver To: Marek Szyprowski Cc: Andrzej Hajda , Inki Dae , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Frieder Schrempf , Fancy Fang , Tim Harvey , Michael Nazzareno Trimarchi , Adam Ford , Neil Armstrong , Robert Foss , Laurent Pinchart , Tommaso Merciai , Marek Vasut , Matteo Lisi , dri-devel@lists.freedesktop.org, linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, NXP Linux Team , linux-amarula Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org On Mon, Oct 3, 2022 at 1:23 PM Marek Szyprowski wrote: > > Hi Jagan, > > On 01.10.2022 10:06, Jagan Teki wrote: > > Samsung MIPI DSIM controller is common DSI IP that can be used in various > > SoCs like Exynos, i.MX8M Mini/Nano. > > > > In order to access this DSI controller between various platform SoCs, > > the ideal way to incorporate this in the drm stack is via the drm bridge > > driver. > > > > This patch is trying to differentiate platform-specific and bridge driver > > code by maintaining exynos platform glue code in exynos_drm_dsi.c driver > > and common bridge driver code in samsung-dsim.c providing that the new > > platform-specific glue should be supported in the bridge driver, unlike > > exynos platform drm drivers. > > > > - Add samsung_dsim_plat_data for keeping platform-specific attributes like > > host_ops, irq_ops, and hw_type. > > > > - Initialize the plat_data hooks for exynos platform in exynos_drm_dsi.c. > > > > - samsung_dsim_probe is the common probe call across exynos_drm_dsi.c and > > samsung-dsim.c. > > > > - plat_data hooks like host_ops and irq_ops are invoked during the > > respective bridge call chains. > > > > v6: > > * handle previous bridge pointer for exynos dsi > > There are still issues, see my comments below. > > > v5: > > * [mszyprow] reworked driver initialization using the same approach as in > > drivers/gpu/drm/{exynos/exynos_dp.c, bridge/analogix/analogix_dp_core.c}, > > removed weak functions, moved exynos_dsi_driver back to exynos_drm_dsi.c > > and restored integration with exynos_drm custom initialization. > > * updated the commit message [Jagan] > > > > v4: > > * include Inki Dae in MAINTAINERS > > * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > > > > v3: > > * restore gpio related fixes > > * restore proper bridge chain > > * rework initialization issue > > * fix header includes in proper way > > > > v2: > > * fixed exynos dsi driver conversion (Marek Szyprowski) > > * updated commit message > > * updated MAINTAINERS file < snip> > > + /** > > + * FIXME: > > + * This has to remove once the bridge chain order is compatible > > + * with Exynos DSI drivers. > > + * > > + * DRM bridge chain ordering is not compatible with Exynos DSI > > + * drivers because DSI sink devices typically want the DSI host > > + * powered up and configured before they are powered up. > > + * > > + * Passing NULL to the previous bridge makes Exynos DSI drivers > > + * work which is exactly done before. > > + */ > > + if (!(dsi->plat_data->hw_type & SAMSUNG_DSIM_TYPE_IMX8MM)) > > The above should be 'if (!(dsi->plat_data->hw_type == SAMSUNG_DSIM_TYPE_IMX8MM))', hw_type is not a bitmask. Also SAMSUNG_DSIM_TYPE_IMX8MM is not yet defined in this patch, so this it breaks builds. I thought I fixed it in previous versions. I will fix this in the next version. > > > + previous = NULL; > > + > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, previous, > > + flags); > > +} > > + > > +static const struct drm_bridge_funcs samsung_dsim_bridge_funcs = { > > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > > + .atomic_reset = drm_atomic_helper_bridge_reset, > > + .atomic_pre_enable = samsung_dsim_atomic_pre_enable, > > + .atomic_enable = samsung_dsim_atomic_enable, > > + .atomic_disable = samsung_dsim_atomic_disable, > > + .atomic_post_disable = samsung_dsim_atomic_post_disable, > > + .mode_set = samsung_dsim_mode_set, > > + .attach = samsung_dsim_attach, > > +}; > > + > > +static int samsung_dsim_host_attach(struct mipi_dsi_host *host, > > + struct mipi_dsi_device *device) > > +{ > > + struct samsung_dsim *dsi = host_to_dsi(host); > > + const struct samsung_dsim_plat_data *pdata = dsi->plat_data; > > + struct device *dev = dsi->dev; > > + struct drm_panel *panel; > > + int ret; > > + > > + panel = of_drm_find_panel(device->dev.of_node); > > + if (!IS_ERR(panel)) { > > + dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel); > > + } else { > > + dsi->out_bridge = of_drm_find_bridge(device->dev.of_node); > > + if (!dsi->out_bridge) > > + dsi->out_bridge = ERR_PTR(-EINVAL); > > + } > > + > > + if (IS_ERR(dsi->out_bridge)) { > > + ret = PTR_ERR(dsi->out_bridge); > > + DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret); > > + return ret; > > + } > > + > > + DRM_DEV_INFO(dev, "Attached %s device\n", device->name); > > + > > + drm_bridge_add(&dsi->bridge); > > + > > + if (pdata->host_ops && pdata->host_ops->attach) { > > + ret = pdata->host_ops->attach(dsi, device); > > + if (ret < 0) > > + return ret; > > + } > > + > > + dsi->lanes = device->lanes; > > + dsi->format = device->format; > > + dsi->mode_flags = device->mode_flags; > > + > > + return 0; > > +} > > + > > +static int samsung_dsim_host_detach(struct mipi_dsi_host *host, > > + struct mipi_dsi_device *device) > > +{ > > + struct samsung_dsim *dsi = host_to_dsi(host); > > + const struct samsung_dsim_plat_data *pdata = dsi->plat_data; > > + int ret; > > + > > + if (dsi->out_bridge->funcs->detach) > > + dsi->out_bridge->funcs->detach(dsi->out_bridge); > > + > > + dsi->out_bridge = NULL; > > + > > + if (pdata->host_ops && pdata->host_ops->detach) { > > + ret = pdata->host_ops->detach(dsi, device); > > + if (ret < 0) > > + return ret; > > + } > > + > > + drm_bridge_remove(&dsi->bridge); > > + > > + return 0; > > +} > > + > > +static ssize_t samsung_dsim_host_transfer(struct mipi_dsi_host *host, > > + const struct mipi_dsi_msg *msg) > > +{ > > + struct samsung_dsim *dsi = host_to_dsi(host); > > + struct samsung_dsim_transfer xfer; > > + int ret; > > + > > + if (!(dsi->state & DSIM_STATE_ENABLED)) > > + return -EINVAL; > > + > > + if (!(dsi->state & DSIM_STATE_INITIALIZED)) { > > + ret = samsung_dsim_init(dsi); > > + if (ret) > > + return ret; > > + dsi->state |= DSIM_STATE_INITIALIZED; > > + } > > + > > + ret = mipi_dsi_create_packet(&xfer.packet, msg); > > + if (ret < 0) > > + return ret; > > + > > + xfer.rx_len = msg->rx_len; > > + xfer.rx_payload = msg->rx_buf; > > + xfer.flags = msg->flags; > > + > > + ret = samsung_dsim_transfer(dsi, &xfer); > > + return (ret < 0) ? ret : xfer.rx_done; > > +} > > + > > +static const struct mipi_dsi_host_ops samsung_dsim_ops = { > > + .attach = samsung_dsim_host_attach, > > + .detach = samsung_dsim_host_detach, > > + .transfer = samsung_dsim_host_transfer, > > +}; > > + > > +static int samsung_dsim_of_read_u32(const struct device_node *np, > > + const char *propname, u32 *out_value) > > +{ > > + int ret = of_property_read_u32(np, propname, out_value); > > + > > + if (ret < 0) > > + pr_err("%pOF: failed to get '%s' property\n", np, propname); > > + > > + return ret; > > +} > > + > > +static int samsung_dsim_parse_dt(struct samsung_dsim *dsi) > > +{ > > + struct device *dev = dsi->dev; > > + struct device_node *node = dev->of_node; > > + int ret; > > + > > + ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency", > > + &dsi->pll_clk_rate); > > + if (ret < 0) > > + return ret; > > + > > + ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency", > > + &dsi->burst_clk_rate); > > + if (ret < 0) > > + return ret; > > + > > + ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency", > > + &dsi->esc_clk_rate); > > + if (ret < 0) > > + return ret; > > + > > + return 0; > > +} > > + > > +static int samsung_dsim_register_host(struct samsung_dsim *dsi) > > +{ > > + return mipi_dsi_host_register(&dsi->dsi_host); > > +} > > + > > +static void samsung_dsim_unregister_host(struct samsung_dsim *dsi) > > +{ > > + mipi_dsi_host_unregister(&dsi->dsi_host); > > +} > > + > > +static const struct samsung_dsim_host_ops samsung_dsim_generic_host_ops = { > > + .register_host = samsung_dsim_register_host, > > + .unregister_host = samsung_dsim_unregister_host, > > +}; > > + > > +int samsung_dsim_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct samsung_dsim *dsi; > > + int ret, i; > > + > > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL); > > + if (!dsi) > > + return -ENOMEM; > > + > > + init_completion(&dsi->completed); > > + spin_lock_init(&dsi->transfer_lock); > > + INIT_LIST_HEAD(&dsi->transfer_list); > > + > > + dsi->dsi_host.ops = &samsung_dsim_ops; > > + dsi->dsi_host.dev = dev; > > + > > + dsi->dev = dev; > > + dsi->plat_data = of_device_get_match_data(dev); > > + dsi->driver_data = samsung_dsim_types[dsi->plat_data->hw_type]; > > + > > + dsi->supplies[0].supply = "vddcore"; > > + dsi->supplies[1].supply = "vddio"; > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dsi->supplies), > > + dsi->supplies); > > + if (ret) > > + return dev_err_probe(dev, ret, "failed to get regulators\n"); > > + > > + dsi->clks = devm_kcalloc(dev, dsi->driver_data->num_clks, > > + sizeof(*dsi->clks), GFP_KERNEL); > > + if (!dsi->clks) > > + return -ENOMEM; > > + > > + for (i = 0; i < dsi->driver_data->num_clks; i++) { > > + dsi->clks[i] = devm_clk_get(dev, clk_names[i]); > > + if (IS_ERR(dsi->clks[i])) { > > + if (strcmp(clk_names[i], "sclk_mipi") == 0) { > > + dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME); > > + if (!IS_ERR(dsi->clks[i])) > > + continue; > > + } > > + > > + dev_info(dev, "failed to get the clock: %s\n", clk_names[i]); > > + return PTR_ERR(dsi->clks[i]); > > + } > > + } > > + > > + dsi->reg_base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(dsi->reg_base)) > > + return PTR_ERR(dsi->reg_base); > > + > > + dsi->phy = devm_phy_get(dev, "dsim"); > > + if (IS_ERR(dsi->phy)) { > > + dev_info(dev, "failed to get dsim phy\n"); > > + return PTR_ERR(dsi->phy); > > + } > > + > > + dsi->irq = platform_get_irq(pdev, 0); > > + if (dsi->irq < 0) > > + return dsi->irq; > > + > > + ret = devm_request_threaded_irq(dev, dsi->irq, NULL, > > + samsung_dsim_irq, > > + IRQF_ONESHOT | IRQF_NO_AUTOEN, > > + dev_name(dev), dsi); > > + if (ret) { > > + dev_err(dev, "failed to request dsi irq\n"); > > + return ret; > > + } > > + > > + ret = samsung_dsim_parse_dt(dsi); > > + if (ret) > > + return ret; > > + > > + platform_set_drvdata(pdev, dsi); > > + > > + pm_runtime_enable(dev); > > + > > + dsi->bridge.funcs = &samsung_dsim_bridge_funcs; > > + dsi->bridge.of_node = dev->of_node; > > + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > + > > + if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->register_host) > > + ret = dsi->plat_data->host_ops->register_host(dsi); > > + > > + if (ret) > > + goto err_disable_runtime; > > + > > + return 0; > > + > > +err_disable_runtime: > > + pm_runtime_disable(dev); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(samsung_dsim_probe); > > + > > +int samsung_dsim_remove(struct platform_device *pdev) > > +{ > > + struct samsung_dsim *dsi = platform_get_drvdata(pdev); > > + > > + pm_runtime_disable(&pdev->dev); > > + > > + if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->unregister_host) > > + dsi->plat_data->host_ops->unregister_host(dsi); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(samsung_dsim_remove); > > + > > +static int __maybe_unused samsung_dsim_suspend(struct device *dev) > > +{ > > + struct samsung_dsim *dsi = dev_get_drvdata(dev); > > + const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > > + int ret, i; > > + > > + usleep_range(10000, 20000); > > + > > + if (dsi->state & DSIM_STATE_INITIALIZED) { > > + dsi->state &= ~DSIM_STATE_INITIALIZED; > > + > > + samsung_dsim_disable_clock(dsi); > > + > > + samsung_dsim_disable_irq(dsi); > > + } > > + > > + dsi->state &= ~DSIM_STATE_CMD_LPM; > > + > > + phy_power_off(dsi->phy); > > + > > + for (i = driver_data->num_clks - 1; i > -1; i--) > > + clk_disable_unprepare(dsi->clks[i]); > > + > > + ret = regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies); > > + if (ret < 0) > > + dev_err(dsi->dev, "cannot disable regulators %d\n", ret); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused samsung_dsim_resume(struct device *dev) > > +{ > > + struct samsung_dsim *dsi = dev_get_drvdata(dev); > > + const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > > + int ret, i; > > + > > + ret = regulator_bulk_enable(ARRAY_SIZE(dsi->supplies), dsi->supplies); > > + if (ret < 0) { > > + dev_err(dsi->dev, "cannot enable regulators %d\n", ret); > > + return ret; > > + } > > + > > + for (i = 0; i < driver_data->num_clks; i++) { > > + ret = clk_prepare_enable(dsi->clks[i]); > > + if (ret < 0) > > + goto err_clk; > > + } > > + > > + ret = phy_power_on(dsi->phy); > > + if (ret < 0) { > > + dev_err(dsi->dev, "cannot enable phy %d\n", ret); > > + goto err_clk; > > + } > > + > > + return 0; > > + > > +err_clk: > > + while (--i > -1) > > + clk_disable_unprepare(dsi->clks[i]); > > + regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies); > > + > > + return ret; > > +} > > + > > +const struct dev_pm_ops samsung_dsim_pm_ops = { > > + SET_RUNTIME_PM_OPS(samsung_dsim_suspend, samsung_dsim_resume, NULL) > > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > > + pm_runtime_force_resume) > > +}; > > +EXPORT_SYMBOL_GPL(samsung_dsim_pm_ops); > > + > > +static const struct of_device_id samsung_dsim_of_match[] = { > > + { /* sentinel. */ } > > +}; > > +MODULE_DEVICE_TABLE(of, samsung_dsim_of_match); > > + > > +static struct platform_driver samsung_dsim_driver = { > > + .probe = samsung_dsim_probe, > > + .remove = samsung_dsim_remove, > > + .driver = { > > + .name = "samsung-dsim", > > + .owner = THIS_MODULE, > > + .pm = &samsung_dsim_pm_ops, > > + .of_match_table = samsung_dsim_of_match, > > + }, > > +}; > > + > > +module_platform_driver(samsung_dsim_driver); > > + > > +MODULE_AUTHOR("Jagan Teki "); > > +MODULE_DESCRIPTION("Samsung MIPI DSIM controller bridge"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig > > index 3d2f025d4fd4..a65acfed15b9 100644 > > --- a/drivers/gpu/drm/exynos/Kconfig > > +++ b/drivers/gpu/drm/exynos/Kconfig > > @@ -59,6 +59,7 @@ config DRM_EXYNOS_DSI > > depends on DRM_EXYNOS_FIMD || DRM_EXYNOS5433_DECON || DRM_EXYNOS7_DECON > > select DRM_MIPI_DSI > > select DRM_PANEL > > + select DRM_SAMSUNG_DSIM > > default n > > help > > This enables support for Exynos MIPI-DSI device. > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index ec673223d6b7..eac52f438d04 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -1,1351 +1,83 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > /* > > - * Samsung SoC MIPI DSI Master driver. > > + * Samsung MIPI DSIM glue for Exynos SoCs. > > * > > * Copyright (c) 2014 Samsung Electronics Co., Ltd > > * > > * Contacts: Tomasz Figa > > -*/ > > + */ > > > > -#include > > -#include > > #include > > #include > > -#include > > #include > > -#include > > -#include > > -#include > > > > -#include > > - > > -#include