From: Linus Walleij <linus.walleij@linaro.org> To: Alexey Minnekhanov <alexeymin@postmarketos.org> Cc: Thierry Reding <thierry.reding@gmail.com>, Sam Ravnborg <sam@ravnborg.org>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, open list <linux-kernel@vger.kernel.org>, "open list:DRM PANEL DRIVERS" <dri-devel@lists.freedesktop.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Hans de Goede <hdegoede@redhat.com>, Andy Shevchenko <andy.shevchenko@gmail.com>," <~postmarketos/upstreaming@lists.sr.ht>, phone-devel@vger.kernel.org Subject: Re: [PATCH 2/2] drm/panel: Add Samsung S6E3FA2 DSI panel driver Date: Mon, 26 Jul 2021 10:04:28 +0200 [thread overview] Message-ID: <CACRpkdbpxQdyTVKkKGCtjLGn5G9L2=w-dhyMpgJP42_tpLU6Pg@mail.gmail.com> (raw) In-Reply-To: <20210725140339.2465677-2-alexeymin@postmarketos.org> Hi Alexey, I had some gmail problems and replied to the very old driver by Iskren, sorry for the mess. I overall like this driver a lot. Some of Sam's comments could be addressed especially for backlight. I think the driver should indeed handle both the physical displays like you do here. On Sun, Jul 25, 2021 at 4:05 PM Alexey Minnekhanov <alexeymin@postmarketos.org> wrote: > Samsung S6E3FA2 panel is amoled 1080x1920 command mode DSI > panel used in Samsung Galaxy S5 phone. There are 2 known > variations of panel that were shipped in this phone, and > this driver handles both of them. > > Panel has built-in backlight (like all other AMOLED panels), > controlled over DSI by some vendor specific commands, some > of them include sending long byte sequences of what seems > to be called "smart dimming". > > Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org> (...) > +#define dsi_generic_write_seq(dsi, seq...) do { \ > + static const u8 d[] = { seq }; \ > + int ret; \ > + ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \ > + if (ret < 0) \ > + return ret; \ > + } while (0) > + > +#define dsi_dcs_write_seq(dsi, seq...) do { \ > + static const u8 d[] = { seq }; \ > + int ret; \ > + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \ > + if (ret < 0) \ > + return ret; \ > + } while (0) These look generic as pointed out in other mail. > +static int s6e3fa2_dsi_dcs_read1(struct mipi_dsi_device *dsi, const u8 cmd, > + u8 *data) > +{ > + int ret; > + > + ret = mipi_dsi_dcs_read(dsi, cmd, data, 1); > + if (ret < 0) { > + dev_err(&dsi->dev, "could not read DCS CMD %02x\n", cmd); > + return ret; > + } > + return 0; > +} I don't think this needs a wrapper, just call mipi_dsi_dcs_read() directly. > +/* Panel variants */ > +#define LCD_ID_S6E3FA2 0x602813 > +#define LCD_ID_EA8064G 0x622872 Interesting use of the "vendor" byte by Samsung here. It seems they are repurposing the non-standard MTP bytes as they seem fit. > +/* > + * Which AID sequence to use for each candela level. > + * This lookup table is same for both panels. > + */ > +static const u8 map_candela_to_aid[S6E3FA2_NUM_GAMMA_LEVELS] = { > + 0, 2, 3, 4, 6, 7, 8, 10, 11, 13, 14, 15, > + 16, 17, 18, 20, 21, 22, 23, 24, 25, 26, 27, 28, > + 29, 30, 31, 32, 33, 34, 35, 36, 36, 36, 36, 36, > + 36, 36, 36, 36, 36, 36, 36, 36, 36, 36, 37, 38, > + 39, 40, 41, 42, 43, 44, 44, 44, 44, 44, 44, 44 > +}; This and other things hints that we are dealing with the same display controller. > +/* Other panel drivers call these commands test_key_enable/disable */ > +static const u8 seq_s6e3fa2_test_key_en[6] = { > + 0xf0, 0x5a, 0x5a, > + 0xfc, 0x5a, 0x5a > +}; 0xf0 and 0xfc is obviously some "level 2 unlock" commands. Maybe #define them as pointed out in other comments. > +static const u8 seq_s6e3fa2_test_key_dis[6] = { > + 0xf0, 0xa5, 0xa5, > + 0xfc, 0xa5, 0xa5 > +}; > +static const u8 seq_ea8064g_test_key_en[6] = { > + 0xf0, 0x5a, 0x5a, > + 0xf1, 0x5a, 0x5a > +}; > +static const u8 seq_ea8064g_test_key_dis[6] = { > + 0xf1, 0xa5, 0xa5, > + 0xf0, 0xa5, 0xa5 > +}; The use of two different registers for locking is suspicious, that may point to different display controllers. :/ This is an icky panel, but it seems they are close enough to be handled by the same driver IMO. Yours, Linus Walleij
next prev parent reply other threads:[~2021-07-26 8:04 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-25 14:03 [PATCH 1/2] dt-bindings: panel: Add Samsung S6E3FA2 panel Alexey Minnekhanov 2021-07-25 14:03 ` [PATCH 2/2] drm/panel: Add Samsung S6E3FA2 DSI panel driver Alexey Minnekhanov [not found] ` <YP184rqayPLbWLx4@ravnborg.org> 2021-07-26 7:44 ` Linus Walleij 2021-07-26 8:04 ` Linus Walleij [this message] 2021-07-26 7:51 ` [PATCH 1/2] dt-bindings: panel: Add Samsung S6E3FA2 panel Linus Walleij 2021-07-29 22:30 ` Rob Herring 2021-07-29 22:31 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CACRpkdbpxQdyTVKkKGCtjLGn5G9L2=w-dhyMpgJP42_tpLU6Pg@mail.gmail.com' \ --to=linus.walleij@linaro.org \ --cc=airlied@linux.ie \ --cc=alexeymin@postmarketos.org \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=phone-devel@vger.kernel.org \ --cc=sam@ravnborg.org \ --cc=thierry.reding@gmail.com \ --cc=~postmarketos/upstreaming@lists.sr.ht \ --subject='Re: [PATCH 2/2] drm/panel: Add Samsung S6E3FA2 DSI panel driver' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).