From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm/panel: Add driver for Seiko 43WVF1G panel Date: Wed, 14 Jun 2017 19:49:52 +0200 Message-ID: <20170614174952.GC14971@ulmo> References: <1493137115-10757-1-git-send-email-marco.franchi@nxp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2102315895==" Return-path: Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id 337D46E5B9 for ; Wed, 14 Jun 2017 17:50:01 +0000 (UTC) Received: by mail-wr0-x242.google.com with SMTP id u101so1854014wrc.1 for ; Wed, 14 Jun 2017 10:50:01 -0700 (PDT) In-Reply-To: <1493137115-10757-1-git-send-email-marco.franchi@nxp.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Marco Franchi Cc: robh+dt@kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============2102315895== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wxDdMuZNg1r63Hyj" Content-Disposition: inline --wxDdMuZNg1r63Hyj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 25, 2017 at 01:18:35PM -0300, Marco Franchi wrote: > Add driver for Seiko Instruments Inc. 4.3" WVGA (800 x RGB x 480) > TFT with Touch-Panel. >=20 > Datasheet available at: > http://www.glyn.de/data/glyn/media/doc/43wvf1g-0.pdf >=20 > Seiko 43WVF1G panel has two power supplies: AVDD and DVDD and they > require a specific power on/down sequence. > For this reason the simple panel driver cannot be used to drive this > panel, so create a new one heavily based on simple panel. >=20 > Based on initial patch submission from Breno Lima. >=20 > Signed-off-by: Marco Franchi > --- > .../bindings/display/panel/seiko,43wvf1g.txt | 23 ++ > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 372 +++++++++++++++= ++++++ > 4 files changed, 405 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/seiko= ,43wvf1g.txt > create mode 100644 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c >=20 > diff --git a/Documentation/devicetree/bindings/display/panel/seiko,43wvf1= g.txt b/Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt > new file mode 100644 > index 0000000..cd1eeda > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt > @@ -0,0 +1,23 @@ > +Seiko Instruments Inc. 4.3" WVGA (800 x RGB x 480) TFT with Touch-Panel > + > +Required properties: > +- compatible: should be "sii,43wvf1g". > +- "DVDD-supply": 3v3 digital regulator. > +- "AVDD-supply": 5v analog regulator. I don't think we should be using all-uppercase for supply names. So the above should be "dvdd-supply" and "avdd-supply". No need to resend only for that change, I can fix it up when applying. Rob, can I have your Acked-by with the above change? Just two minor comments below. > diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/dr= m/panel/panel-seiko-43wvf1g.c [...] > +struct seiko_panel_desc { > + const struct drm_display_mode *modes; > + unsigned int num_modes; > + const struct display_timing *timings; > + unsigned int num_timings; > + > + unsigned int bpc; > + > + /** > + * @width: width (in millimeters) of the panel's active display area > + * @height: height (in millimeters) of the panel's active display area > + */ > + struct { > + unsigned int width; > + unsigned int height; > + } size; > + > + u32 bus_format; > + u32 bus_flags; > +}; It's somewhat unfortunate how this has to duplicate a lot of the panel-simple driver just because it uses two regulators. However, with some of the work going on to make panel-simple code more reusable this might improve in the future. For now I think this doesn't hurt. > +struct seiko_panel { > + struct drm_panel base; > + bool prepared; > + bool enabled; > + const struct seiko_panel_desc *desc; > + struct backlight_device *backlight; > + struct regulator *DVDD; > + struct regulator *AVDD; > +}; Same as for the binding: we don't use all-uppercase names for variables, so I'll fix those up to be lowercase when applying. Thierry --wxDdMuZNg1r63Hyj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAllBdz0ACgkQ3SOs138+ s6HvKw/+ODqiAhy+NeMC2YjXVkCYtkJ6i8kG26P6nGHKwW566xhZsZfqqNqQB3jk n6pLY3Xy95qwUh3JvLiSX1UFUlml87j7vkFIAzePgQsi9l9YbpHdfOd1SLT/QtCg eAHs8tKI9j8aoetDPW2s+yzkAhlHH2G7jiVllKTNQ9C0ao5glQuW4gqAPwGMQ3zv Cc/3xCH9yTGEdoyZdpfECoIJ4BJGM4bVCc+BTEyYmR/irYJhj4OFj4U0WSVkmTt6 Ey5+FeK23FbnDehFEFIaZzmUN3iJ8xrw5TgJIpUT9XuurHtQ9bL4UNu0SK3HoCpD AnyFme/5J5k/OqHfxSonQX0Z7w9mVRqRBmJTs2Gn/PjU5G1TH8gWM49V85c04BQc 0h//vPh1LcPyVZVJURaNgofmR/bqV6CKj5pprMMLzKGG2h1uh8OAUw1FbLt27hTA V1tldt/ONZSWYUtK2tEw+8rY1XaYKx2c8Sc+7jare7Sd8Vrf8z4PNKdW3DicCR88 JLQkXwh4PMIYurf86EG8yFnolFR6atO3XXcjfdTwbo9r31uz7JsUuxOM3pBvPipB i+khT3At1I69zjpoXX+s+I2/dl/Wz2uQxAcY2kRabZ71R0LRR1WAHLwvipfO9NJE jl/hO73ptCfu6Ngw21bOJD4wXGZ09bOu3KJCaof77D5w2c4Kze4= =X/C4 -----END PGP SIGNATURE----- --wxDdMuZNg1r63Hyj-- --===============2102315895== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============2102315895==--