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.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham 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 58DB9C83000 for ; Tue, 28 Apr 2020 18:18:54 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 35B5F208FE for ; Tue, 28 Apr 2020 18:18:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 35B5F208FE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 924FA8984C; Tue, 28 Apr 2020 18:18:53 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 625406E0AF for ; Tue, 28 Apr 2020 18:18:51 +0000 (UTC) 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 53B42804B9; Tue, 28 Apr 2020 20:18:46 +0200 (CEST) Date: Tue, 28 Apr 2020 20:18:45 +0200 From: Sam Ravnborg To: Gareth Williams Subject: Re: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Message-ID: <20200428181845.GD27234@ravnborg.org> References: <1587975709-2092-1-git-send-email-gareth.williams.jx@renesas.com> <1587975709-2092-2-git-send-email-gareth.williams.jx@renesas.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1587975709-2092-2-git-send-email-gareth.williams.jx@renesas.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=yC-0_ovQAAAA:8 a=8b9GpE9nAAAA:8 a=e5mUnYsNAAAA:8 a=hNxGGNvzTs52B8eoPUEA:9 a=_NC1VaQZqfszIdzZ:21 a=E2HU7c-VcNwjPWcH:21 a=ARyByvqPOCuah2br:21 a=CjuIK1q_8ugA:10 a=QsnFDINu91a9xkgZirup:22 a=T3LWEMljR5ZiDmsYVIUa:22 a=Vxmtnl_E_bksehYqCbjh:22 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Phil Edworthy , Maxime Ripard , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, David Airlie , Sean Paul Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Gareth. On Mon, Apr 27, 2020 at 09:21:47AM +0100, Gareth Williams wrote: > Add DRM support for the Digital Blocks DB9000 LCD Controller > with the Renesas RZ/N1 specific changes. > > Signed-off-by: Gareth Williams > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/digital-blocks/Kconfig | 13 + > drivers/gpu/drm/digital-blocks/Makefile | 3 + > drivers/gpu/drm/digital-blocks/db9000-du.c | 953 +++++++++++++++++++++++++++++ > drivers/gpu/drm/digital-blocks/db9000-du.h | 192 ++++++ > 6 files changed, 1164 insertions(+) > create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig > create mode 100644 drivers/gpu/drm/digital-blocks/Makefile > create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c > create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h The general impression is a well written driver. It looks like it was wrtten some tiem ago and thus fail to take full benefit from the improvements impemented the last year or so. The driver has a size so it is a candidate to belong in the tiny/ directory. If you do not see any other DRM drivers for digital-blocks or that this driver should be extended, then please consider a move to tiny/ If you do so embed the header file in the .c file so it is a single file driver. Other general comments: The driver looks like a one plane - one crtc - one encoder driver. Please use drm_simple - or explain why you cannot use drm_simple. For the encoder use drm_simple_encoder_init A small intro to the driver would be good. For example that is includes a pwm etc. I provided a mix of diverse comments in the following. Looks forward for the next revision! Sam > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 3c88420..159832d 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -280,6 +280,8 @@ source "drivers/gpu/drm/mgag200/Kconfig" > > source "drivers/gpu/drm/cirrus/Kconfig" > > +source "drivers/gpu/drm/digital-blocks/Kconfig" > + > source "drivers/gpu/drm/armada/Kconfig" > > source "drivers/gpu/drm/atmel-hlcdc/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 9f0d2ee..f525807 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_MGAG200) += mgag200/ > obj-$(CONFIG_DRM_V3D) += v3d/ > obj-$(CONFIG_DRM_VC4) += vc4/ > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/ > +obj-$(CONFIG_DRM_DB9000) += digital-blocks/ > obj-$(CONFIG_DRM_SIS) += sis/ > obj-$(CONFIG_DRM_SAVAGE)+= savage/ > obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/ > diff --git a/drivers/gpu/drm/digital-blocks/Kconfig b/drivers/gpu/drm/digital-blocks/Kconfig > new file mode 100644 > index 0000000..436a7c0 > --- /dev/null > +++ b/drivers/gpu/drm/digital-blocks/Kconfig > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0 > +config DRM_DB9000 > + bool "DRM Support for DB9000 LCD Controller" > + depends on DRM && (ARCH_MULTIPLATFORM || COMPILE_TEST) > + select DRM_KMS_HELPER > + select DRM_GEM_CMA_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_PANEL_BRIDGE > + select VIDEOMODE_HELPERS > + select FB_PROVIDE_GET_FB_UNMAPPED_AREA if FB > + > + help > + Enable DRM support for the DB9000 LCD controller. > diff --git a/drivers/gpu/drm/digital-blocks/Makefile b/drivers/gpu/drm/digital-blocks/Makefile > new file mode 100644 > index 0000000..9f78492 > --- /dev/null > +++ b/drivers/gpu/drm/digital-blocks/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_DRM_DB9000) += db9000-du.o > diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.c b/drivers/gpu/drm/digital-blocks/db9000-du.c > new file mode 100644 > index 0000000..d84d446 > --- /dev/null > +++ b/drivers/gpu/drm/digital-blocks/db9000-du.c > @@ -0,0 +1,953 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (C) 2019 Renesas Electronics Europe Ltd. 2020? > + * > + * Author: Gareth Williams > + * > + * Based on ltdc.c > + * Copyright (C) STMicroelectronics SA 2017 > + * > + * Authors: Philippe Cornu > + * Yannick Fertre > + * Fabien Dessenne > + * Mickael Reulier > + */ > +#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 > + > +#include