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=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 15E6AC4CECC for ; Mon, 27 Apr 2020 20:01:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D45F3206A5 for ; Mon, 27 Apr 2020 20:01:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726777AbgD0UBL (ORCPT ); Mon, 27 Apr 2020 16:01:11 -0400 Received: from asavdk4.altibox.net ([109.247.116.15]:53280 "EHLO asavdk4.altibox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726205AbgD0UBL (ORCPT ); Mon, 27 Apr 2020 16:01:11 -0400 Received: from ravnborg.org (unknown [158.248.194.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id 0B484804B9; Mon, 27 Apr 2020 22:00:45 +0200 (CEST) Date: Mon, 27 Apr 2020 22:00:44 +0200 From: Sam Ravnborg To: Xin Ji Cc: devel@driverdev.osuosl.org, Laurent Pinchart , Andrzej Hajda , Nicolas Boichat , Jernej Skrabec , Nicolas Boichat , Pi-Hsun Shih , Jonas Karlman , David Airlie , Neil Armstrong , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Sheng Pan , Dan Carpenter Subject: Re: [PATCH v8 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver Message-ID: <20200427200044.GC15880@ravnborg.org> References: <4d14400b6c19f17c28267f6ebdbce9673333c05c.1587880280.git.xji@analogixsemi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4d14400b6c19f17c28267f6ebdbce9673333c05c.1587880280.git.xji@analogixsemi.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=MOBOZvRl c=1 sm=1 tr=0 a=UWs3HLbX/2nnQ3s7vZ42gw==:117 a=UWs3HLbX/2nnQ3s7vZ42gw==:17 a=kj9zAlcOel0A:10 a=VwQbUJbxAAAA:8 a=7gkXJVJtAAAA:8 a=bbNUuHX0AAAA:8 a=e5mUnYsNAAAA:8 a=vJ7ZIUqJpn7mxAx9tZkA:9 a=JbwdoYpX1fRYziDF:21 a=ln0l6cApyDuOQl7l:21 a=JJ9dnML-QyfdiVvf:21 a=CjuIK1q_8ugA:10 a=AjGcO6oz07-iQ99wixmX:22 a=E9Po1WZjFZOl8hwRPBS3:22 a=3b-t3vAtY4IUXy2q2Ylb:22 a=Vxmtnl_E_bksehYqCbjh:22 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Xin Ji On Mon, Apr 27, 2020 at 02:18:44PM +0800, Xin Ji wrote: > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K. > > The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI > to DP feature. This driver only enabled MIPI DSI/DPI to DP feature. You are sending this patch in an interesting time for bridge drivers. We are migrating to an approach where the individual brdge drivers exposes operations and where the connector creation is now optional. Laurent Pinchart is the architect behind it - and the required interfaces is well documented. You may find inspiration in a patchset I sent today: https://lore.kernel.org/dri-devel/20200427081850.17512-1-sam@ravnborg.org/T/#t This is not reviewed - so keep an eye out for feedback. It would be great to base this on top of drm-misc-next, as this is where we will apply it eventually. As it is now it will not build due to internal API changes. The driver looks well structured with nice coding. I am missing an explanation why the current analogix infrastructure cannot be used. I have no clue but I just see other drivers in same dir that benefits from the infrastructure. So the questions seems relevant to be addressed. See a few more comments in the following, which you need to decide what to follow and what to ignore. I will make it obvious when something is a must to change, if I find anything such. Sorry for providing such massive feedback on v8. Please keep up the spirit and submit a v9 soon! Sam > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/Makefile | 2 +- > drivers/gpu/drm/bridge/analogix/Kconfig | 6 + > drivers/gpu/drm/bridge/analogix/Makefile | 1 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 2158 +++++++++++++++++++++++++++++ > drivers/gpu/drm/bridge/analogix/anx7625.h | 410 ++++++ > 5 files changed, 2576 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 4934fcf..bcd388a 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o > obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o > obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o > obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > +obj-y += analogix/ > obj-y += synopsys/ With this change we will always visit analogix/ which will trigger build of too much i think. Use: obj-$(CONFIG_ANALOGIX_ANX7625) += analogix/ like the other drivers do, if for nothing else then for consistency. > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig > index e930ff9..b2f127e 100644 > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > @@ -2,3 +2,9 @@ > config DRM_ANALOGIX_DP > tristate > depends on DRM > + > +config ANALOGIX_ANX7625 > + tristate "Analogix MIPI to DP interface support" > + help > + ANX7625 is an ultra-low power 4K mobile HD transmitter designed > + for portable devices. It converts MIPI/DPI to DisplayPort1.3 4K. > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile b/drivers/gpu/drm/bridge/analogix/Makefile > index fdbf3fd..8a52867 100644 > --- a/drivers/gpu/drm/bridge/analogix/Makefile > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c > new file mode 100644 > index 0000000..fff7a49 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -0,0 +1,2158 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright(c) 2016, Analogix Semiconductor. All rights reserved. 2020? > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include