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=-6.6 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS 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 9F577C433E1 for ; Fri, 15 May 2020 06:53:36 +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 3D4E1206F1 for ; Fri, 15 May 2020 06:53:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=renesasgroup.onmicrosoft.com header.i=@renesasgroup.onmicrosoft.com header.b="IPvwRhlV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3D4E1206F1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=renesas.com 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 E35F46EBD7; Fri, 15 May 2020 06:53:00 +0000 (UTC) Received: from JPN01-TY1-obe.outbound.protection.outlook.com (mail-eopbgr1400100.outbound.protection.outlook.com [40.107.140.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2285C6E162 for ; Wed, 13 May 2020 10:47:39 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nJ/NR769RUfPEr7JKpUSA2bg5UWFZ7VKuCFJPqDCzYxGXtZIMs+KZgEOPg9VwOLrGXQqc9jH7+qdSnocVnr1CpzmzzzmKVrJSll+rtq+KORVQSGvusMwgxtqj9LckQ5PrqLBtSkPcOCDAJBnm+7uM3MtAcH6fOuAJks7/uKmkrdPNswmbccQqTrvupV7Hkc0Qw0rHqxqDQWfzb64rfzaozQoRq4PrOQOdoPyugZ30nMIQBSjxQt1x7BRMyOZinRRJmE6lgK/T7WN6WAmZTHnxwJ4n/+r71C1lTBin0dGGghm9IxvFWevMLqYEJcZZ1xTXOLmBZ7f1J3azz+ytwL4aw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jecCoVCmHsJmr8yYD5kGGIVyaDXx4yMOEzcI8M+1e2M=; b=Gyj0P6WV27ywRnFRnltlsOCUiyT3FlqyImB1pNXPqD48hmSbY4Ndbr5MW5mY3DFlcI9B0SYgy1gZJnGDB5vy10kpcve12AWSSfXiTi1D3ChWGWPEvEH/gwU6olXoEtI7vDW68x3TVXnWuDgcBv9BQ9nJGgRtXTnbe6zYDn2r0rI5AvYygmQ1XpNEMNhTZ9wqa9Cqm+f1Ry1/5I4zfZV7OwN8VbqTBKTdsOqa9aAY73on1Q02m+UzJmpeTTykSxXV5HcqS1vetaxMVf45bb+Rod7olHCEkuj0P2o8MVQhB4P5N52FDmztljhiTRHDqjli5RD1Gwm4ukncC1c9GtuD3g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=renesas.com; dmarc=pass action=none header.from=renesas.com; dkim=pass header.d=renesas.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=renesasgroup.onmicrosoft.com; s=selector2-renesasgroup-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jecCoVCmHsJmr8yYD5kGGIVyaDXx4yMOEzcI8M+1e2M=; b=IPvwRhlVguj/sidRhOrUBl3fC18MC/wZ8ofXjEeGk34//xNB27MplZkLzJD0tN5lwe6XtYWp+3oplAoD7Qxidt5qBBkNkGVYvlfQwxJKvkcJc//Rbdc08dA3xyE6mlzvdWMixSf8bG6rluUbwqdz8ARED0QiY4Ncq+QYWlDSP7o= Received: from OSAPR01MB2914.jpnprd01.prod.outlook.com (52.134.247.13) by OSAPR01MB3700.jpnprd01.prod.outlook.com (20.178.101.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2979.29; Wed, 13 May 2020 10:47:36 +0000 Received: from OSAPR01MB2914.jpnprd01.prod.outlook.com ([fe80::c4a6:b6af:b928:e31f]) by OSAPR01MB2914.jpnprd01.prod.outlook.com ([fe80::c4a6:b6af:b928:e31f%3]) with mapi id 15.20.2979.033; Wed, 13 May 2020 10:47:36 +0000 From: Gareth Williams To: Daniel Vetter , Sam Ravnborg Subject: RE: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Thread-Topic: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Thread-Index: AQHWHGz7z+82HiGUfk6EsDwG4gHLEqiO2g6AgAABbgCAASxjAA== Date: Wed, 13 May 2020 10:47:36 +0000 Message-ID: References: <1587975709-2092-1-git-send-email-gareth.williams.jx@renesas.com> <1587975709-2092-2-git-send-email-gareth.williams.jx@renesas.com> <20200428181845.GD27234@ravnborg.org> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: ffwll.ch; dkim=none (message not signed) header.d=none;ffwll.ch; dmarc=none action=none header.from=renesas.com; x-originating-ip: [193.141.219.24] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 36f3a12c-69f5-45c7-16b9-08d7f72b0c94 x-ms-traffictypediagnostic: OSAPR01MB3700:|OSAPR01MB3700: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6790; x-forefront-prvs: 0402872DA1 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Z6XAdrxeTjoP5P9k6h+fiR11D1VBR0ZV0ZWtRuqnbfVNHy7RWM7Ky0ZAkBo94z7yfA92itZROnYQM95Czw3TEtjAnsShtF2cPtmFhA54780Ce7buvn9DWCxXC2ue+YjhAcFRQwmVHcK2dXzWlQWjNa45mZvqnDWhsgvKbUXWeZ6figTlDXARndYWYAxmWKegUjxdeunSQpfoU9C85O8kgc2rIoinuNh3OiIAwI/o+I1XkFbhTO5lASKP/8djsnJ6B0JMwhP76Iwyx0CA/2fe/RemtSIzsv54jhW7Ym3mIUNs1XL6bs6SL6lUdqh5FhxW4oHmNxHsB/EKDQNxueBO2AGJDkSt0rkjRUkrWgCT7r7KOymCOxQjU4IXH7K/6UvEBPnq5qIyt7FENoCZtdTFfos/YyO0ou3HLDWp32Gva82d0oUAwlcySD3bz9m1o53yuPNBQbZ2fU0PdEv6TAPxKv3X5v6iNLY0zPSUwL1NXMDqPO4fpHbkOpcQjPYJg2+kT7WHLYhIx69UQhAunFraYKlGEPvXRDSB2ZbWyPzy473GH8L+WbBU3iSKxzEsU5t+cgIec+tI3sE71VXb1CEiZdsAskGvn9wjLEz0E6j6QTg= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:OSAPR01MB2914.jpnprd01.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(136003)(366004)(39860400002)(396003)(346002)(376002)(33430700001)(33440700001)(110136005)(54906003)(8676002)(9686003)(316002)(66946007)(83080400001)(66446008)(64756008)(6506007)(53546011)(7696005)(33656002)(86362001)(66556008)(8936002)(66476007)(76116006)(26005)(52536014)(4326008)(55016002)(478600001)(2906002)(30864003)(966005)(186003)(71200400001)(5660300002)(579004)(559001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: yTncEUJLlnGAf8iaKUgMep1Bmo7/JB7v7GimODoQIFxLyrcvh3OO8y89YZ0qTcKvHI9Rpf5X69JLtiwbYsm7v7wsv174D2vRVzpS89mGU7SndtGUXmJMGO46nTydliMcSUGoy7kNgo4pP1wnIgSGANETPxYYfehVUddK9AjRmzcH9zJr/zmmeVqZCoNXBC5k4/OtjoBipkqc1Lr2Q/ybE7RFVY8KtmHyQcalxole5c9S0Nxfad74nR4StXi/a+yVrXX9m5cV0wNXjX4vgN6+xRCtQTlx1qT60ruTCHmJCkouERSd9dHv2Ku1Wk3YeEHPxds9shuEk9lwhpa1ax8wWBCedRmDEqWSZ2yCM9xX/9bXoteCZlcuwOZKOX5iiuNCrmjGaky9XpdmMDiemxoB3dafLUelnuAoBjnimX8l4Us1Qt5RPzpOUYoo3/VzYhnvKGeNKozeDuL3RCaNiWuaqsjTCXIRi2Z0feEM43zj43OxNrVvKIwJUQq8k0p+ytHp MIME-Version: 1.0 X-OriginatorOrg: renesas.com X-MS-Exchange-CrossTenant-Network-Message-Id: 36f3a12c-69f5-45c7-16b9-08d7f72b0c94 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 May 2020 10:47:36.1441 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: e3IaDJPRLpKf3AsUypWI9IyFRLJ6wVQBdSqhQ8gLyDLv44TWTb1bxh9ytzHE9yNkGcAwtk4VfMjMWtI2ez06/AY9HR7/UGlPRY/eucHckKQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: OSAPR01MB3700 X-Mailman-Approved-At: Fri, 15 May 2020 06:52:55 +0000 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 Mailing List , dri-devel , 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 Sam/Daniel, This is all very useful feedback, thank you. On Tue, Apr 28, 2020 at 19:24 PM Daniel Vetter wrote: > > On Tue, Apr 28, 2020 at 8:18 PM Sam Ravnborg wrote: > > > > 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. That's okay with me, I'll merge the two files and move it to tinydrm. > > > > 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. I'll take a look at this in more detail. I've tested with drm_simple and it is feasible. Thanks for the direction. > > > > 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'll add an introduction to the commit message and cover letter. > > One more since I just landed my series: Please use devm_drm_dev_alloc. > There's a ton of examples now in-tree. > -Daniel Thanks Daniel, I might have missed that otherwise. > > > > > 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? I'll correct this in the next version, thanks. > > > > > + * > > > + * 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