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_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS 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 CFC33C38A2A for ; Sun, 10 May 2020 04:04:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ADC072495E for ; Sun, 10 May 2020 04:04:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QqWwYDGr" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725994AbgEJEE1 (ORCPT ); Sun, 10 May 2020 00:04:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725308AbgEJEE0 (ORCPT ); Sun, 10 May 2020 00:04:26 -0400 Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 41B22C061A0C; Sat, 9 May 2020 21:04:26 -0700 (PDT) Received: by mail-il1-x144.google.com with SMTP id n11so5267328ilj.4; Sat, 09 May 2020 21:04:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=NFGUrRHVRBZqxgUnXFdXEUjce2fH5vtH195ampR5PEA=; b=QqWwYDGrBO6PLB4Gqmp22QfAboa1XVZsMal0iw3IUXOdq5LAc6iUXhLtq8Ywx1+i/f 8f/4sQ9xHoO9orc9X/IhdtXYi9TEcrDFxt5QBUFQHqRcxMlnWFip3shgz42o1AxMKH9v wpMpPDO+qNgEmrTkHC58lVEeRyglGwXcTAhg7J/Vs4uMNDWWr5qUBRBBdr5XTacNoxlb xgWqxRUHBqFO858Q+oEzCeQtoT7XlwHhnwNWQ5AnH5C5+7HNFkMupCjb9UPgFijyre+V Sx7cX9QLY5/Jq0Fp0uN0De/AbpEhkXR7/BZtWLfxd9l7gzbH1O8/D4lQNQiYuv408Z+A BKdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=NFGUrRHVRBZqxgUnXFdXEUjce2fH5vtH195ampR5PEA=; b=OzMEY/vH5zt/AN5JVE/hqwWCSH/GUIAbPqozYr0VWt90nGR8csGbIoZXggTx6CA6Ni WQ2sKLMAaRGXALJHCgx5W1rHL7VhWoc5HI8oVcYciW3sLDxzQIhdWU8RxhYtQXlblXbv A1GD7A1gvrlkM17jka58xcDUe8+FyT0dgz6kqKIoC1WgwIFgcktOhERKSsBPHk/Bkm9A zBJg5R8aEyAqBP01M931WKcrvRdFVX9ESvoyR1MYnmn+zlkUGvaMJQd4U69nBQOS3KPW aZIcGkFDmwh2s7eTsVKaAUv3/vxOducvvY94bx/m9HPj23fIo/eXWgTVKCSg25AyYSnk yqmg== X-Gm-Message-State: AGi0PuZZCJaQMHsAaLpu7RgIsWiJXDBnu5gMiThdv6/o+1/VPjRUOVM9 IcI0v0EqJ0yjeqA4vNLJZ1JyMnP7gPAurOxhyl4= X-Google-Smtp-Source: APiQypJHvmTcEhe1NDXmQGxKQLWv3DQIDB1K9KIG5hF9BPFEiZxg67rOfaCIvDz1dqlCZ1IGAwrTAradFsOdiVsxOj8= X-Received: by 2002:a05:6e02:589:: with SMTP id c9mr3767891ils.271.1589083465133; Sat, 09 May 2020 21:04:25 -0700 (PDT) MIME-Version: 1.0 References: <1588911194-12433-1-git-send-email-dillon.minfei@gmail.com> <1588911194-12433-6-git-send-email-dillon.minfei@gmail.com> <20200508090247.GA11575@ravnborg.org> <20200509200654.GC30802@ravnborg.org> In-Reply-To: <20200509200654.GC30802@ravnborg.org> From: dillon min Date: Sun, 10 May 2020 12:04:13 +0800 Message-ID: Subject: Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels To: Sam Ravnborg Cc: mcoquelin.stm32@gmail.com, Alexandre Torgue , devicetree@vger.kernel.org, airlied@linux.ie, Michael Turquette , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-clk@vger.kernel.org, sboyd@kernel.org, robh+dt@kernel.org, thierry.reding@gmail.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sam Ravnborg =E4=BA=8E2020=E5=B9=B45=E6=9C=8810=E6=97=A5= =E5=91=A8=E6=97=A5 =E4=B8=8A=E5=8D=884:06=E5=86=99=E9=81=93=EF=BC=9A > > Hi Dillon. > > On Fri, May 08, 2020 at 06:13:43PM +0800, dillon min wrote: > > Hi Sam, > > > > Thanks for your comments, i will rework this panel driver after l3gd2= 0 > > patch submission. > > > > Sam Ravnborg =E4=BA=8E2020=E5=B9=B45=E6=9C=888=E6=97= =A5=E5=91=A8=E4=BA=94 =E4=B8=8B=E5=8D=885:02=E5=86=99=E9=81=93=EF=BC=9A > > > > > Hi Dillon. > > > > > > Patch submissions starts to look fine. > > > > > > On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.minfei@gmail.com wro= te: > > > > From: dillon min > > > > > > > > This is a driver for 320x240 TFT panels, accepting a rgb input > > > > streams that get adapted and scaled to the panel. > > > This driver is, I suppose, prepared to be a driver for ILI9341 based > > > panles, and as such not for a fixed resolution. > > > I expect (hope) we in the future will see more panels added. > > > > > > As i checked ili9341 datasheets, this panel just support 240x320 > > resolution only. if i'm wrong , please correct me. thanks > > https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf > > > > This panel can support 9 different kinds of interface , "3/4-line Seria= l > > Interface" have been supported by tiny/ili9341.c. i was verified it > > but the performance on stm32f4 is very low. > It had somehow escaped my mind we already have a driver for ili9341 > based panels. > And we do not want onyl one driver for this IC. > So please extent the current driver to match your panel as well as > the original panel. > And then also extend the current binding for ili9341, which > you may have to convert to DT Schema in the process. > > This is more work now but when it is done we end up with a much better > solution than if we introduce an additional driver for ili9341. > > Sorry for not catching this in the inital review. > > Sam > > Hi Sam, Yes, it's should be more reasonable to support different interface of a panel in one driver. i will add mipi dpi & dbi support to this panel in one driver just like panel-simple.c(dpi & dsi) one question, what to do next: 1 should i just add code to tiny/ili9341.c to support mipi dpi. which means remove panel/panel-ilitek-ili9341.c 2 merge tiny/ili9341.c mipi dbi support to panel/panel-ilitek-ili9341.c , which means remove tiny/ili9341.c after this work done. to extend dts binding for different interface. Ie. original: 164 static const struct of_device_id ili9341_of_match[] =3D { 165 { .compatible =3D "adafruit,yx240qv29" }, 166 { } 167 }; 168 MODULE_DEVICE_TABLE(of, ili9341_of_match); 169 170 static const struct spi_device_id ili9341_id[] =3D { 171 { "yx240qv29", 0 }, 172 { } 173 }; 174 MODULE_DEVICE_TABLE(spi, ili9341_id); i want to extend of_device_id and spi_device_id for driver to identify dpi or dbi in probe. "adafruit,yx240qv29" is for users who use original tiny drm driver mipi dbi compatible for panel's parallel mcu and parallel rgb interface. static const struct of_device_id ili9341_of_match[] =3D { { .compatible =3D "adafruit,yx240qv29" }, { .compatible =3D "ili9341,parallel-mcu" }, { .compatible =3D "ili9341,parallel-rgb" }, { } }; static const struct spi_device_id ili9341_id[] =3D { { "yx240qv29", 0 }, { "parallel-mcu", 0 }, { "parallel-rgb", 0 }, { } }; static int ili9341_probe(struct spi_device *spi) { ... const struct spi_device_id *id =3D spi_get_device_id(spi); if (!strcmp(id->name, "yx240qv29")) { mipi dbi register } else if (!strcmp(id->name, "parallel-rgb") || !strcmp(id->name, "parallel-mcu") { mipi dpi register } else { failed handle } .... } if this is a good idea to extend dts binding ? thanks, best regards. Dillon > > > > currently, i just have stm32f429-disco in hands, with 18-bit parallel r= gb > > bus connected on this board. reference to panel-ilitek-ili9322 and > > panel-ilitek-ili9881 driver, i have some plan to rewrite this driver. > > > > 1 add your below comments in. > > 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel > > interface. > > 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel > > paramter. like tiny/ili9341.c > > 4 support rgb-16, rgb-18 interface. > > 5 use optional regulator or power gpio to control panel's power, as pan= el > > power is always on for my board, so i can't test this part. could i add= the > > code which can't be tested? > > 6 support rotation in panel config (currently , i rotate the screen by > > kernel cmdline paramter) > > > > if you have any other common panel configuration should be add , please > > inform me. > > > > thanks. > > > > > > > > Some things to fix, see comments in the follwoing. > > > > > > Sam > > > > > > > > > > > Signed-off-by: dillon min > > > > --- > > > > drivers/gpu/drm/panel/Kconfig | 8 + > > > > drivers/gpu/drm/panel/Makefile | 1 + > > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 > > > +++++++++++++++++++++++++++ > > > > 3 files changed, 570 insertions(+) > > > > create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > > > > > diff --git a/drivers/gpu/drm/panel/Kconfig > > > b/drivers/gpu/drm/panel/Kconfig > > > > index a1723c1..e42692c 100644 > > > > --- a/drivers/gpu/drm/panel/Kconfig > > > > +++ b/drivers/gpu/drm/panel/Kconfig > > > > @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 > > > > Say Y here if you want to enable support for Ilitek IL9322 > > > > QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. > > > > > > > > +config DRM_PANEL_ILITEK_IL9341 > > > ILI9341 - so the config name matches the name of the driver IC. > > > > > > > + tristate "Ilitek ILI9341 240x320 QVGA panels" > > > > + depends on OF && SPI > > > > + select REGMAP > > > > + help > > > > + Say Y here if you want to enable support for Ilitek IL9341 > > > > + QVGA (240x320) RGB panels. > > > See comment to the changelog, the driver is more generic - I assume. > > > So the wording here can be improved to express this. > > > > > > Add support RGB 16bits and RGB 18bits bus only ? > > > > > > + > > > > config DRM_PANEL_ILITEK_ILI9881C > > > > tristate "Ilitek ILI9881C-based panels" > > > > depends on OF > > > > diff --git a/drivers/gpu/drm/panel/Makefile > > > b/drivers/gpu/drm/panel/Makefile > > > > index 96a883c..d123543 100644 > > > > --- a/drivers/gpu/drm/panel/Makefile > > > > +++ b/drivers/gpu/drm/panel/Makefile > > > > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) +=3D > > > panel-elida-kd35t133.o > > > > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) +=3D > > > panel-feixin-k101-im2ba02.o > > > > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) +=3D > > > panel-feiyang-fy07024di26a30d.o > > > > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) +=3D panel-ilitek-ili9322.o > > > > +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) +=3D panel-ilitek-ili9341.o > > > > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) +=3D panel-ilitek-ili9881c= .o > > > > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) +=3D panel-innolux-p079zca= .o > > > > obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) +=3D panel-jdi-lt070me050= 00.o > > > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > new file mode 100644 > > > > index 0000000..ec22d80 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > @@ -0,0 +1,561 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * Ilitek ILI9341 TFT LCD drm_panel driver. > > > > + * > > > > + * This panel can be configured to support: > > > > + * - 16-bit parallel RGB interface > > > The interface to ILI9341 is SPI, and the interface between the ILI934= 1 > > > and the panel is more of an itnernal thing. Or did I get this worng? > > > > > > SPI is for register configuration. RGB parallel for data transfer > > > > > + * > > > > + * Copyright (C) 2020 Dillon Min > > > > + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c > > > > + */ > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#include