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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BFA68C4321E for ; Fri, 2 Dec 2022 14:56:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/dKWQ0HFRBIbjvOqNmOUUJFmiQVxd7S0qm3sz3aF/rg=; b=c18QO7nJyYPsNk Dfdgs36exEcMl+3zqFRNUUPaOMJmW0gXmT3bxy8ZdD9+4t+TdC0LyUgLUsLlnyhwjWXp5J0LCaUFC Vmb7xBWkoD1rNXdYa4dkm1FWztsUbIEPJhH36SHi7wZ3ykP/YhqmmI3I2lwO5PtJ6A6+VNIQRN6Dx 2fYaVddod9SOICodSBNefHPxshHhtR7F9uFE7mICjedumBE7zzkqacfA3zIjq4vD34+4PllVoIEJD oaOXNhBVOj0WPc3ZonimIw235dryShdmis77SPe0bqQqbzS1P7iQ6+JsOqNh/pEIzu8CsCEOQ7Uqq ClprcnRZqFh1uw7JiaFg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p17Ro-00Gvh5-Ig; Fri, 02 Dec 2022 14:55:28 +0000 Received: from mail-ua1-x92c.google.com ([2607:f8b0:4864:20::92c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p17Rk-00GvfC-P2 for linux-arm-kernel@lists.infradead.org; Fri, 02 Dec 2022 14:55:26 +0000 Received: by mail-ua1-x92c.google.com with SMTP id e24so1736795uam.10 for ; Fri, 02 Dec 2022 06:55:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.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=5DIEAfr9d518HvW1FSiaubDLh03ZD86HLPj22EhRASA=; b=MgMBJ7WIDH5uBn/FJmbn3JvpaNJXzjUsN/yoTbYt9iwGWdLJLFuD4gn2kZLqtbfqq2 YQgVY11UCEi/Lxex0dtDvmeEOO4C1GhpPIIqZlQ/Z9MXPK8EgYcDZR9CB9mdP2tnjRZS nO5uc/GoesNi1F5J2kPXj6IBid0bLH36qa/8KRg/JzXi9AL3eUOx6fjfH2yGS8pBnmyL TSdqFN6Spb7H/uZ9orilhwA8kK7mJOkD9l10giw/6eI9Lio+YX88eBE5Q5PMpK9Gu21N feaviY/Zts4+MPfxxIatBQYjOoIVWIBLtes2Ar692W8ZhmHXmK6wX2jDS0ZTDfH1TN1m hu/g== 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=5DIEAfr9d518HvW1FSiaubDLh03ZD86HLPj22EhRASA=; b=InhvYRsz7OKQiY5egOkB5aiHTIZIrr/3miUiIJyu9PwpPDP5hhNblM7GnncKWG+Not cjhTWv3WUopl8qiW+ICoVn17XM07F4ACCF3n8BcoAgb+qlw8jrIaydz8AuQKWhfbr2ay KvKclPjYLS1lLTAzHwMfn63UIAuUrRo0Sx+fCGDsmWGoz9ozb80e3zgt2ovvWVL8VpxA wWQAeCYnBKuNG0bjyw3MLF+dRB21wq1r6q2zJv7jfxd04erT7nLUePI1Q7lyhZjaBa/I 7DFvtkKh1jA0w/OZqJePW/4o0kvjWTjVp/7g8CcxjOQwHDkotFjjQlwKpMwivJsKiJjP GUgw== X-Gm-Message-State: ANoB5pnQ2beLpovoWBLftWIh4mYHRtgcITjzYYBHLGwqXW8hues2Urrd IbPBJbHxzRtbQW+lVPbKLNFLFdUwaUcE1kaDbqgZrA== X-Google-Smtp-Source: AA0mqf6K/loWDf6gEPPD+6VjHJZXYXH+Uqz6mfdnqU8g7iBWzsO3dLs+71EfuwIEbormn4UvaaFDfTRHeGU7vVhXFCA= X-Received: by 2002:ab0:1432:0:b0:418:d13c:f3a4 with SMTP id b47-20020ab01432000000b00418d13cf3a4mr34683049uae.105.1669992922723; Fri, 02 Dec 2022 06:55:22 -0800 (PST) MIME-Version: 1.0 References: <20221110183853.3678209-1-jagan@amarulasolutions.com> <20221110183853.3678209-7-jagan@amarulasolutions.com> <04fb17e2-1b55-fbd9-d846-da3e3da4edb8@denx.de> <58671662-9242-c7ef-53ef-60f9cdc3399a@denx.de> <9f08a902-049c-1c00-7fed-3d7ee18b3d2c@samsung.com> <2ef1aae1-8ff9-22bc-9817-69d1eae8b485@denx.de> In-Reply-To: <2ef1aae1-8ff9-22bc-9817-69d1eae8b485@denx.de> From: Dave Stevenson Date: Fri, 2 Dec 2022 14:55:06 +0000 Message-ID: Subject: Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization To: Marek Vasut Cc: Marek Szyprowski , Jagan Teki , 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 , Matteo Lisi , dri-devel@lists.freedesktop.org, linux-samsung-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, NXP Linux Team , linux-amarula X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221202_065524_922259_665465DE X-CRM114-Status: GOOD ( 50.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Marek On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote: > > On 12/2/22 11:52, Marek Szyprowski wrote: > > Hi, > > > > Sorry for delay, I was on a sick leave last 2 weeks. > > > > On 28.11.2022 15:43, Jagan Teki wrote: > >> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote: > >>> On 11/23/22 21:09, Jagan Teki wrote: > >>>> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote: > >>>>> On 11/17/22 14:04, Marek Szyprowski wrote: > >>>>>> On 17.11.2022 05:58, Marek Vasut wrote: > >>>>>>> On 11/10/22 19:38, Jagan Teki wrote: > >>>>>>>> DSI host initialization handling in previous exynos dsi driver has > >>>>>>>> some pitfalls. It initializes the host during host transfer() hook > >>>>>>>> that is indeed not the desired call flow for I2C and any other DSI > >>>>>>>> configured downstream bridges. > >>>>>>>> > >>>>>>>> Host transfer() is usually triggered for downstream DSI panels or > >>>>>>>> bridges and I2C-configured-DSI bridges miss these host initialization > >>>>>>>> as these downstream bridges use bridge operations hooks like pre_enable, > >>>>>>>> and enable in order to initialize or set up the host. > >>>>>>>> > >>>>>>>> This patch is trying to handle the host init handler to satisfy all > >>>>>>>> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state > >>>>>>>> flag to ensure that host init is also done on first cmd transfer, this > >>>>>>>> helps existing DSI panels work on exynos platform (form Marek > >>>>>>>> Szyprowski). > >>>>>>>> > >>>>>>>> v8, v7, v6, v5: > >>>>>>>> * none > >>>>>>>> > >>>>>>>> v4: > >>>>>>>> * update init handling to ensure host init done on first cmd transfer > >>>>>>>> > >>>>>>>> v3: > >>>>>>>> * none > >>>>>>>> > >>>>>>>> v2: > >>>>>>>> * check initialized state in samsung_dsim_init > >>>>>>>> > >>>>>>>> v1: > >>>>>>>> * keep DSI init in host transfer > >>>>>>>> > >>>>>>>> Signed-off-by: Marek Szyprowski > >>>>>>>> Signed-off-by: Jagan Teki > >>>>>>>> --- > >>>>>>>> drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++++++++++++++++-------- > >>>>>>>> include/drm/bridge/samsung-dsim.h | 5 +++-- > >>>>>>>> 2 files changed, 20 insertions(+), 10 deletions(-) > >>>>>>>> > >>>>>>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > >>>>>>>> b/drivers/gpu/drm/bridge/samsung-dsim.c > >>>>>>>> index bb1f45fd5a88..ec7e01ae02ea 100644 > >>>>>>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c > >>>>>>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > >>>>>>>> @@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct > >>>>>>>> samsung_dsim *dsi) > >>>>>>>> disable_irq(dsi->irq); > >>>>>>>> } > >>>>>>>> -static int samsung_dsim_init(struct samsung_dsim *dsi) > >>>>>>>> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int > >>>>>>>> flag) > >>>>>>>> { > >>>>>>>> const struct samsung_dsim_driver_data *driver_data = > >>>>>>>> dsi->driver_data; > >>>>>>>> + if (dsi->state & flag) > >>>>>>>> + return 0; > >>>>>>>> + > >>>>>>>> samsung_dsim_reset(dsi); > >>>>>>>> - samsung_dsim_enable_irq(dsi); > >>>>>>>> + > >>>>>>>> + if (!(dsi->state & DSIM_STATE_INITIALIZED)) > >>>>>>>> + samsung_dsim_enable_irq(dsi); > >>>>>>>> if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST) > >>>>>>>> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1); > >>>>>>>> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct > >>>>>>>> samsung_dsim *dsi) > >>>>>>>> samsung_dsim_set_phy_ctrl(dsi); > >>>>>>>> samsung_dsim_init_link(dsi); > >>>>>>>> + dsi->state |= flag; > >>>>>>>> + > >>>>>>>> return 0; > >>>>>>>> } > >>>>>>>> @@ -1269,6 +1276,10 @@ static void > >>>>>>>> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge, > >>>>>>>> } > >>>>>>>> dsi->state |= DSIM_STATE_ENABLED; > >>>>>>>> + > >>>>>>>> + ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED); > >>>>>>>> + if (ret) > >>>>>>>> + return; > >>>>>>>> } > >>>>>>>> static void samsung_dsim_atomic_enable(struct drm_bridge *bridge, > >>>>>>>> @@ -1458,12 +1469,9 @@ static ssize_t > >>>>>>>> samsung_dsim_host_transfer(struct mipi_dsi_host *host, > >>>>>>>> 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 = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED); > >>>>>>> This triggers full controller reset and reprogramming upon first > >>>>>>> command transfer, is such heavy handed reload really necessary ? > >>>>>> Yes it is, otherwise the proper DSI panels doesn't work with Exynos DRM > >>>>>> DSI. If this is a real issue for you, then maybe the driver could do the > >>>>>> initialization conditionally, in prepare() callback in case of IMX and > >>>>>> on the first transfer in case of Exynos? > >>>>> That's odd , it does actually break panel support for me, without this > >>>>> double reset the panel works again. But I have to wonder, why would such > >>>>> a full reset be necessary at all , even on the exynos ? > >>>> Is it breaking samsung_dsim_reset from host_transfer? maybe checking > >>>> whether a reset is required before calling it might fix the issue. I > >>>> agree with double initialization is odd but it seems it is required on > >>>> some panels in Exynos, I think tweaking them and adjusting the panel > >>>> code might resolve this discrepancy. > >>> Can someone provide further details on the exynos problem ? > >> If I'm correct this sequence is required in order to work the existing > >> panel/bridges on exynos. Adjusting these panel/bridge codes can > >> possibly fix the sequence further. > >> > >> Marek Szyprowski, please add if you have anything. > > > > > > Well, frankly speaking the double initialization is not a correct > > sequence, but this is the only one that actually works on Exynos based > > boards with DSI panels after moving the initialization to bridge's > > .prepare() callback. > > Somehow, I suspect this is related to the LP11 mode handling, which > differs for different panels, right ? I think the RPi people worked on > fixing that. > > +CC Dave Yes. I've just sent out a v3 of that patch set. Hopefully setting the pre_enable_prev_first flag on your peripheral's drm_bridge, or prepare_prev_first if a drm_panel, will result in a more sensible initialisation order for your panel. Note that host_transfer should ensure that the host is initialised, as it is valid to call it with the host in any state. If it has to initialise, then it should deinitialise once the transfer has completed. Dave > > I've already explained this and shared the results > > of my investigation in my replies to the previous versions of this > > patchset. The original Exynos DSI driver does the initialization on the > > first DSI command. This however doesn't work for Jagan with I2C > > controlled panels/bridges, so he moved the initialization to the > > .prepare() callback, what broke the Exynos case (in-short - all tested > > panels works fine only if the DSI host initialization is done AFTER > > turning the panel's power on). For more information, see this thread: > > https://lore.kernel.org/all/e96197f9-948a-997e-5453-9f9d179b5f5a@samsung.com/ > > > > Now, the more I think of it, the more I'm convinced that we simply > > should add a hack based on the HW type: do the initialization in > > .prepare() for non-Exynos case and before the first transfer for the > > Exynos case, as there is no way to detect the panel/next bridge type > > (I2C or DSI controlled). > > Let's see what Dave has to say about this, maybe there is some further help. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel