Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 7 Mar 2022 14:12:50 +0800
From:   Chen-Yu Tsai <wenst@chromium.org>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        =?iso-8859-1?Q?N=EDcolas_F=2E_R=2E_A=2E?= Prado 
        <nfraprado@collabora.com>
Cc:     Xin Ji <xji@analogixsemi.com>, a.hajda@samsung.com,
        narmstrong@baylibre.com, dan.carpenter@oracle.com,
        robert.foss@linaro.org, jonas@kwiboo.se, jernej.skrabec@gmail.com,
        airlied@linux.ie, daniel@ffwll.ch, sam@ravnborg.org,
        pihsun@chromium.org, tzungbi@google.com, maxime@cerno.tech,
        drinkcat@google.com, hsinyi@chromium.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        bliang@analogixsemi.com, qwen@analogixsemi.com, robh+dt@kernel.org,
        devicetree@vger.kernel.org
Subject: Re: [PATCH v12 3/4] drm/bridge: anx7625: add MIPI DPI input feature
Message-ID: <YiWiYpEfTOOqgyAN@google.com>
References: <20211105031904.2641088-1-xji@analogixsemi.com>
 <20211105031904.2641088-3-xji@analogixsemi.com>
 <YiTruiCIkyxs3jTC@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YiTruiCIkyxs3jTC@pendragon.ideasonboard.com>
Precedence: bulk

On Sun, Mar 06, 2022 at 07:13:30PM +0200, Laurent Pinchart wrote:
> Hello Xin,
> 
> (Question for Rob below, and I'm afraid this is urgent as we need to
> merge a fix in v5.17).
> 
> On Fri, Nov 05, 2021 at 11:19:03AM +0800, Xin Ji wrote:
> > The basic anx7625 driver only support MIPI DSI rx signal input.
> > This patch add MIPI DPI rx input configuration support, after apply
> > this patch, the driver can support DSI rx or DPI rx by adding
> > 'bus-type' in DT.
> > 
> > Reviewed-by: Robert Foss <robert.foss@linaro.org>
> > Signed-off-by: Xin Ji <xji@analogixsemi.com>
> > ---
> >  drivers/gpu/drm/bridge/analogix/anx7625.c | 247 ++++++++++++++++------
> >  drivers/gpu/drm/bridge/analogix/anx7625.h |  18 +-
> >  2 files changed, 205 insertions(+), 60 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index f48e91134c20..f7c3386c8929 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c

[...]

> >  static int anx7625_parse_dt(struct device *dev,
> >  			    struct anx7625_platform_data *pdata)
> >  {
> > -	struct device_node *np = dev->of_node;
> > +	struct device_node *np = dev->of_node, *ep0;
> >  	struct drm_panel *panel;
> >  	int ret;
> > +	int bus_type, mipi_lanes;
> > +
> > +	anx7625_get_swing_setting(dev, pdata);
> >  
> > +	pdata->is_dpi = 1; /* default dpi mode */
> >  	pdata->mipi_host_node = of_graph_get_remote_node(np, 0, 0);
> >  	if (!pdata->mipi_host_node) {
> >  		DRM_DEV_ERROR(dev, "fail to get internal panel.\n");
> >  		return -ENODEV;
> >  	}
> >  
> > -	DRM_DEV_DEBUG_DRIVER(dev, "found dsi host node.\n");
> > +	bus_type = V4L2_FWNODE_BUS_TYPE_PARALLEL;
> > +	mipi_lanes = MAX_LANES_SUPPORT;
> > +	ep0 = of_graph_get_endpoint_by_regs(np, 0, 0);
> > +	if (ep0) {
> > +		if (of_property_read_u32(ep0, "bus-type", &bus_type))
> > +			bus_type = 0;
> > +
> > +		mipi_lanes = of_property_count_u32_elems(ep0, "data-lanes");
> > +	}
> > +
> > +	if (bus_type == V4L2_FWNODE_BUS_TYPE_PARALLEL) /* bus type is Parallel(DSI) */
> 
> This is not correct *at all*. V4L2_FWNODE_BUS_TYPE_PARALLEL has nothing
> to do with DSI. DSI stands for Digital *Serial* Interface. If anything,
> the V4L2_FWNODE_BUS_TYPE_PARALLEL type would map better to DPI, even if
> it's not an exact match.
> 
> This patch has landed in v5.17-rc1, along with the corresponding
> bindings. As DT bindings are an ABI, we should really fix this before
> v5.17 is released. There is no DSI bus types defined in DT, and adding
> one as a fix so late in the v5.17-rc cycle seems a bit of a stretch to
> me (unless Rob disagrees).
> 
> It would seem best to revert this series and the corresponding bindings,
> and retry in v5.18.

There is a DT patch using this property that is already queued up for 5.17
in the soc tree:

https://lore.kernel.org/all/20220214200507.2500693-1-nfraprado@collabora.com/

merged here:

http://git.kernel.org/soc/soc/c/32568ae37596b529628ac09b875f4874e614f63f

We will need to revert that one as well.

ChenYu