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