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 X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C1808C43461 for ; Tue, 8 Sep 2020 08:47:01 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E39DB215A4 for ; Tue, 8 Sep 2020 08:47:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="gPMEtMz+"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="fBtJVR0h" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E39DB215A4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mFG0vHnk0ZecP5GYvXHxCoEAoks0oEdThIQyGqOIrrU=; b=gPMEtMz+lj6NJDkTMXo3OljI5 7Xat9l6RaQH2dmI5zdEoXBmfXVibg67m9XBbEuqwNaVTlxB0i84tVbwS6ZSbAMj/tA0CmBf3RwPAq uCQe5CMdVPy3Qj/8L3vefKy/lEHnfiGEae7mDv/SNzLVJDbcbppmCuGNwct1Qihk2RnihMxfRvVjT GMbIf2dhAHYM6K8vsPa45C4glbaj8J5Znd7PDKIEQpR0MV7ABPDioQnS401Al+aw+u/Qe5YJD5/OQ r3bBYlHBzibnUxMl2wcCjpS5RB6Vw/MFcKMI0hjkxAfIhyihaI22108sbBne1Y3TbeJOup7yqJjzz 9BdOLQtLw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFZH7-0000gg-NM; Tue, 08 Sep 2020 08:46:49 +0000 Received: from mail-wr1-x441.google.com ([2a00:1450:4864:20::441]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFZH5-0000ew-98 for linux-amlogic@lists.infradead.org; Tue, 08 Sep 2020 08:46:49 +0000 Received: by mail-wr1-x441.google.com with SMTP id e16so18203594wrm.2 for ; Tue, 08 Sep 2020 01:46:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=WsjbvmmZKMC8MceZdsV0yPxKISX87E/DDuYAfGmxSEY=; b=fBtJVR0hdGCRQN6pQejVwyqvj4WARlY7n3kR5Jq6DLPNpw/t/j7MeChvHRdzRZDPBs P3CS2hNHu4yzXcjzDQYEKra3JyH7GWLo8t1YJgPVWDWBVZM8fKghLGZYxNa5weIiCmC2 dHQD21mCUbCQIUpsW1v3omqZ+25OJkI/yQbMw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=WsjbvmmZKMC8MceZdsV0yPxKISX87E/DDuYAfGmxSEY=; b=DcFJO160Q/rasYXDI1/JVIodiTf8yhSJKoS+4buvaD09Psy34MMT0A4sugiVjKdE57 7tQLFQxZsoWlYX4NZfKk5kKfGZYpGsSeqL1oMr2OEi2MXEPptFBNs69uE706HAoRziU8 A1CpvdKxxJa2wLojOyKUE8BhRf8n9im+5dePvD1EpJrPp6ggU7Sbrx7iTC5MikVDFBSa WWctPi0Vfw71x2mia3cHlU8YOymyAjR6IzxzyzfL3qj2D8WAwWe236P2HOcNz+z0so0i QtD7pfm7rAwiCo67LDTx24YzWvroMXfGlTzJnACixkSAhAm2+x8FHab8ODvmyNNYKEiF jwsA== X-Gm-Message-State: AOAM532Vr9H1fTWpc/t59g9cYTqlfH5qMtiCy+uMQYLXcyQC+iDzwJuU HLGboxrJWDWXi40o5r9yT4Mf8Q== X-Google-Smtp-Source: ABdhPJwlRwBcD/dkDnmeNt/pmsZ/RICLEMEd9+YvOB934Q64ggiWpJyKdIPX011RteQqsUvs1SSr2Q== X-Received: by 2002:adf:81e6:: with SMTP id 93mr25732190wra.412.1599554804650; Tue, 08 Sep 2020 01:46:44 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id f19sm31169981wmh.44.2020.09.08.01.46.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Sep 2020 01:46:43 -0700 (PDT) Date: Tue, 8 Sep 2020 10:46:42 +0200 From: Daniel Vetter To: Neil Armstrong Subject: Re: [PATCH 6/6] drm/meson: add support for MIPI-DSI transceiver Message-ID: <20200908084642.GG2352366@phenom.ffwll.local> Mail-Followup-To: Neil Armstrong , dri-devel@lists.freedesktop.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org References: <20200907081825.1654-1-narmstrong@baylibre.com> <20200907081825.1654-7-narmstrong@baylibre.com> <20200907084351.GV2352366@phenom.ffwll.local> <20200907084423.GW2352366@phenom.ffwll.local> <20200907180315.GY2352366@phenom.ffwll.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 5.7.0-1-amd64 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200908_044647_351592_C6A1FE90 X-CRM114-Status: GOOD ( 44.01 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org On Tue, Sep 08, 2020 at 10:06:03AM +0200, Neil Armstrong wrote: > Hi, > > On 07/09/2020 20:03, Daniel Vetter wrote: > > On Mon, Sep 07, 2020 at 11:03:29AM +0200, Neil Armstrong wrote: > >> On 07/09/2020 10:44, Daniel Vetter wrote: > >>> On Mon, Sep 07, 2020 at 10:43:51AM +0200, Daniel Vetter wrote: > >>>> On Mon, Sep 07, 2020 at 10:18:25AM +0200, Neil Armstrong wrote: > >>>>> The Amlogic AXg SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom > >>>>> glue managing the IP resets, clock and data input similar to the DW-HDMI Glue on other > >>>>> Amlogic SoCs. > >>>>> > >>>>> This adds support for the Glue managing the transceiver, mimicing the init flow provided > >>>>> by Amlogic to setup the ENCl encoder, the glue, the transceiver, the digital D-PHY and the > >>>>> Analog PHY in the proper way. > >>>>> > >>>>> The DW-MIPI-DSI transceiver + D-PHY are directly clocked by the VCLK2 clock, which pixel clock > >>>>> is derived and feeds the ENCL encoder and the VIU pixel reader. > >>>>> > >>>>> An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the > >>>>> DW-MIPI-DSI transceiver. > >>>>> > >>>>> Signed-off-by: Neil Armstrong > >>>> > >>>> More dw-hdmi drivers which aren't bridges but components, and the thing is > >>>> still midlayer-y as heck :-/ > >>> > >>> *dw-dsi, but really they both work the same way and should both be fixed > >>> ... > >> > >> They are bridges but since they have platform-dependent code due to theirs's generic IP > >> nature, they need to be intanciated by components to sync with the platform code. > > > > Yeah that's how it currently works, but there's not much reason why it > > needs to work like that. That platform glue code can also be put behind > > the drm_bridge abstraction, and we could use the normal dt based bridge > > lookup like for everything else. > > > > Afaiui the only reason dw-* bridge drivers work like that is because for > > historical reasons we only had component.c at first, and none of the more > > fancy dynamic bridge lookup. So would be really good to switch this all > > over to a proper&clean bridge abstraction, and not leak everything around. > > Does it mean we should avoit using components, right ? Yup, at least when there's an established specific mechanism to handle cross-driver interactions/EPROBE_DEFER. So if you want a drm_panel or drm_bridge or a clock or i2c or anything else standardized like that, no component.c please. That's just for the driver-specific glue when you have entirely IP-specific way of building up a driver from components, which will never be reused outside of that overall driver. Hence why dw-* using components is rather uncool. > > I've typed up what I think should be the way forward a few times already, > > but thus far not even the todo.rst entry materialized: > > > > https://lore.kernel.org/dri-devel/20200430135841.GE10381@phenom.ffwll.local/ > > > > If everyone is always about "not in my patch series" it'll never happen. > > Today, the dw-mipi-dsi and dw-mipi-hdmi has mandatory callbacks to implement > vendor specific features, like : > - hdmi/dsi phy handling when mixed into the glue/custom/synopsys but with platform specific values > - platform specific mode validation > - hpd setup => could use laurent's work here with no_connector, but how do we handle rxsense ??? > - dsi timings/lane mbps ? these are platform phy specific > > For amlogic, I can split the "component" code handling venc/vclk into a primary bridge and add a > secondary driver only handling the glue around dw-mipi-dsi/dw-mipi-hdmi, would that be a good start ? I think what we should do here: - type up todo.rst with the overall structure we want to aim for, i.e. where is the code, who sets up what, how are the bridges bound into the overall driver. - demidlayer dw-* enough so that the callbacks are gone, and it becomes more a toolbox/library for building a dw-* driver. Maybe we're there already. - All new drivers then need to use the toolbox and have everything behind a drm_bridge driver in drm/bridge/, with just default binding exposed to drivers, no EXPORT_SYMBOL at all. Also no exported symbols used, this should all be directly linked into the dw-*.ko imo and treated as internals. - We might need to split the header files to make that clean enough. - Once all existing dw-* users are converted, we ditch the EXPORT_SYMBOL stuff completely (since I expect we'll just have one overall dw-dsi.ko module with all bridge driver variants). Cheers, Daniel > > Neil > > > > > Cheers, Daniel > > > > > >> > >> Neil > >> > >>> > >>>> > >>>> Can we try to fix this? There's a ton of this going on, and the more we > >>>> add the old fashioned way the harder this gets to fix up for real. > >>>> -Daniel > >>>> > >>>>> --- > >>>>> drivers/gpu/drm/meson/Kconfig | 7 + > >>>>> drivers/gpu/drm/meson/Makefile | 1 + > >>>>> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 562 ++++++++++++++++++++++ > >>>>> 3 files changed, 570 insertions(+) > >>>>> create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.c > >>>>> > >>>>> diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig > >>>>> index 9f9281dd49f8..385f6f23839b 100644 > >>>>> --- a/drivers/gpu/drm/meson/Kconfig > >>>>> +++ b/drivers/gpu/drm/meson/Kconfig > >>>>> @@ -16,3 +16,10 @@ config DRM_MESON_DW_HDMI > >>>>> default y if DRM_MESON > >>>>> select DRM_DW_HDMI > >>>>> imply DRM_DW_HDMI_I2S_AUDIO > >>>>> + > >>>>> +config DRM_MESON_DW_MIPI_DSI > >>>>> + tristate "MIPI DSI Synopsys Controller support for Amlogic Meson Display" > >>>>> + depends on DRM_MESON > >>>>> + default y if DRM_MESON > >>>>> + select DRM_DW_MIPI_DSI > >>>>> + select GENERIC_PHY_MIPI_DPHY > >>>>> diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile > >>>>> index 28a519cdf66b..2cc870e91182 100644 > >>>>> --- a/drivers/gpu/drm/meson/Makefile > >>>>> +++ b/drivers/gpu/drm/meson/Makefile > >>>>> @@ -5,3 +5,4 @@ meson-drm-y += meson_rdma.o meson_osd_afbcd.o > >>>>> > >>>>> obj-$(CONFIG_DRM_MESON) += meson-drm.o > >>>>> obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o > >>>>> +obj-$(CONFIG_DRM_MESON_DW_MIPI_DSI) += meson_dw_mipi_dsi.o > >>>>> diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c > >>>>> new file mode 100644 > >>>>> index 000000000000..bbe1294fce7c > >>>>> --- /dev/null > >>>>> +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c > >>>>> @@ -0,0 +1,562 @@ > >>>>> +// SPDX-License-Identifier: GPL-2.0-or-later > >>>>> +/* > >>>>> + * Copyright (C) 2016 BayLibre, SAS > >>>>> + * Author: Neil Armstrong > >>>>> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved. > >>>>> + */ > >>>>> + > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> + > >>>>> +#include