Received: by 2002:a25:1985:0:0:0:0:0 with SMTP id 127csp3273049ybz; Mon, 27 Apr 2020 13:06:52 -0700 (PDT) X-Google-Smtp-Source: APiQypICTPQro5j8T4Hlugz3IwevHDz8msifW5TN0rzEBNrIi3xMs0Q7RmBfxPOm8+xCq0ioFUBx X-Received: by 2002:a05:6402:1d09:: with SMTP id dg9mr20651705edb.118.1588018011903; Mon, 27 Apr 2020 13:06:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1588018011; cv=none; d=google.com; s=arc-20160816; b=bUu4qP1Yr5cnlqhinof6e4aUGPd9TI7XnuIlbVqtKFHlxMKAzfZ2G7DrHVjkcZeheQ 4q0ijU8erYjkBAsS0aRHzBOwO1vT21rVz9ZGsxiFlJMkDTpksrHmuZs4pzeQYIhHxiXO PnguMAMxWqB8gyHB5qZ8K8UysefXpeql0jG4C2Njjox9GxUgSisEiMLnGS8NVA6xzB6N IGcMrU6STb7GWCBUxZoYaaXVb8OEfj5abn/VR17cN/wvDJIcmVZ9a6y4kvcEAleICD8v SI1Ubcc8iPVonugYp1r/gi9pKylIU6SrTTB/lfMoCuvBzVHa6DLmFcRl7elUwvpj+O/G iqTA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=nFAcAPW4WU5t1YlQmETa32eQD/g2KcZd64/r2WgZku8=; b=lZOHwcDv1ObkVlqk1AUIN5Km+RlFsjsrWVYu0DmQf/t+A09W2gaSPG/1zc4qT/TcpA e4BPvo3nMI7rgwLoFuLCp1pIoBVjfOkInaTnp/rSuAN0hB9m/jE2Ns6ZNOxoXNO1srmP /TcgycYAtQZr0uaY0ipeyfhgvu6CmSuL/0ymi7x1oa6EDgg3QvokhOw7Mn+LSkBND1rC c8jlswxLhNsIp7pi5bzvZcUcRqE6UovBrTsjZfUN5hoU9nifMHPg2LEbuwHmjWWnBz6s P/Zn0F/paXs4ZbpwTTno0b93F7Vkj5Mku5TU4gX18S4Co+Z8OKXaLJUUiEt9NtKRyba5 0kZw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id u19si406609ejt.101.2020.04.27.13.06.27; Mon, 27 Apr 2020 13:06:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726773AbgD0UBL (ORCPT + 99 others); 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