Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
On Mon, Apr 29, 2024 at 01:39:06PM +0200, Maxime Ripard wrote: > On Thu, Apr 25, 2024 at 06:04:50PM +0300, Andy Shevchenko wrote: > > On Thu, Apr 25, 2024 at 04:58:06PM +0200, Maxime Ripard wrote: > > > On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > > > > First of all, the driver was introduced when it was already > > > > two drivers available for Ilitek 9341 panels. > > > > > > > > Second, the most recent (fourth!) driver has incorporated this one > > > > and hence, when enabled, it covers the provided functionality. > > > > > > > > Taking into account the above, remove duplicative driver and make > > > > maintenance and support eaiser for everybody. > > > > > > > > Also see discussion [1] for details about Ilitek 9341 duplication > > > > code. > > > > > > > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > > > > Signed-off-by: Andy Shevchenko > > > > > > I think it should be the other way around and we should remove the > > > mipi-dbi handling from panel/panel-ilitek-ili9341.c > > > > Then please do it! I whining already for a few years about this. > > I have neither the hardware nor the interest to do so. Seems it looks > like you have plenty of the latter at least, I'm sure you'll find some > time to tackle this. Hmm... Since the use of Arduino part in panel IliTek 9341 is clarified in this thread, I won't use that, but I have no time to clean up the mess in DRM in the nearest future, sorry. And TBH it seems you, guys, know much better what you want. FYI: The drivers/gpu/drm/tiny/mi0283qt.c works for me (the plenty of the HW you referred to). TL;DR: consider this as a (bug/feature/cleanup) report. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
On Thu, Apr 25, 2024 at 06:04:50PM +0300, Andy Shevchenko wrote: > On Thu, Apr 25, 2024 at 04:58:06PM +0200, Maxime Ripard wrote: > > Hi, > > > > On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > > > First of all, the driver was introduced when it was already > > > two drivers available for Ilitek 9341 panels. > > > > > > Second, the most recent (fourth!) driver has incorporated this one > > > and hence, when enabled, it covers the provided functionality. > > > > > > Taking into account the above, remove duplicative driver and make > > > maintenance and support eaiser for everybody. > > > > > > Also see discussion [1] for details about Ilitek 9341 duplication > > > code. > > > > > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > > > Signed-off-by: Andy Shevchenko > > > > I think it should be the other way around and we should remove the > > mipi-dbi handling from panel/panel-ilitek-ili9341.c > > Then please do it! I whining already for a few years about this. I have neither the hardware nor the interest to do so. Seems it looks like you have plenty of the latter at least, I'm sure you'll find some time to tackle this. Maxime signature.asc Description: PGP signature
Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
On Thu, Apr 25, 2024 at 04:58:06PM +0200, Maxime Ripard wrote: > Hi, > > On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > > First of all, the driver was introduced when it was already > > two drivers available for Ilitek 9341 panels. > > > > Second, the most recent (fourth!) driver has incorporated this one > > and hence, when enabled, it covers the provided functionality. > > > > Taking into account the above, remove duplicative driver and make > > maintenance and support eaiser for everybody. > > > > Also see discussion [1] for details about Ilitek 9341 duplication > > code. > > > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > > Signed-off-by: Andy Shevchenko > > I think it should be the other way around and we should remove the > mipi-dbi handling from panel/panel-ilitek-ili9341.c Then please do it! I whining already for a few years about this. > It's basically two drivers glued together for no particular reason and > handling two very different use cases which just adds more complexity > than it needs to. > > And it's the only driver doing so afaik, so it's definitely not "least > surprise" compliant. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
Hi, On Thu, Apr 25, 2024 at 03:42:07PM +0300, Andy Shevchenko wrote: > First of all, the driver was introduced when it was already > two drivers available for Ilitek 9341 panels. > > Second, the most recent (fourth!) driver has incorporated this one > and hence, when enabled, it covers the provided functionality. > > Taking into account the above, remove duplicative driver and make > maintenance and support eaiser for everybody. > > Also see discussion [1] for details about Ilitek 9341 duplication > code. > > Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1] > Signed-off-by: Andy Shevchenko I think it should be the other way around and we should remove the mipi-dbi handling from panel/panel-ilitek-ili9341.c It's basically two drivers glued together for no particular reason and handling two very different use cases which just adds more complexity than it needs to. And it's the only driver doing so afaik, so it's definitely not "least surprise" compliant. Maxime signature.asc Description: PGP signature
[PATCH v1 1/1] drm/ili9341: Remove the duplicative driver
First of all, the driver was introduced when it was already
two drivers available for Ilitek 9341 panels.
Second, the most recent (fourth!) driver has incorporated this one
and hence, when enabled, it covers the provided functionality.
Taking into account the above, remove duplicative driver and make
maintenance and support eaiser for everybody.
Also see discussion [1] for details about Ilitek 9341 duplication
code.
Link: https://lore.kernel.org/r/zxm9pg-53v4s8...@smile.fi.intel.com [1]
Signed-off-by: Andy Shevchenko
---
drivers/gpu/drm/tiny/Kconfig | 13 --
drivers/gpu/drm/tiny/Makefile | 1 -
drivers/gpu/drm/tiny/ili9341.c | 253 -
3 files changed, 267 deletions(-)
delete mode 100644 drivers/gpu/drm/tiny/ili9341.c
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..2ab07bd0bb44 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -134,19 +134,6 @@ config TINYDRM_ILI9225
If M is selected the module will be called ili9225.
-config TINYDRM_ILI9341
- tristate "DRM support for ILI9341 display panels"
- depends on DRM && SPI
- select DRM_KMS_HELPER
- select DRM_GEM_DMA_HELPER
- select DRM_MIPI_DBI
- select BACKLIGHT_CLASS_DEVICE
- help
- DRM driver for the following Ilitek ILI9341 panels:
- * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4")
-
- If M is selected the module will be called ili9341.
-
config TINYDRM_ILI9486
tristate "DRM support for ILI9486 display panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..37cc9b27e79d 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -10,7 +10,6 @@ obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o
obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o
obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
-obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o
obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o
obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
deleted file mode 100644
index 47b61c3bf145..
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ /dev/null
@@ -1,253 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * DRM driver for Ilitek ILI9341 panels
- *
- * Copyright 2018 David Lechner
- *
- * Based on mi0283qt.c:
- * Copyright 2016 Noralf Trønnes
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#define ILI9341_FRMCTR10xb1
-#define ILI9341_DISCTRL0xb6
-#define ILI9341_ETMOD 0xb7
-
-#define ILI9341_PWCTRL10xc0
-#define ILI9341_PWCTRL20xc1
-#define ILI9341_VMCTRL10xc5
-#define ILI9341_VMCTRL20xc7
-#define ILI9341_PWCTRLA0xcb
-#define ILI9341_PWCTRLB0xcf
-
-#define ILI9341_PGAMCTRL 0xe0
-#define ILI9341_NGAMCTRL 0xe1
-#define ILI9341_DTCTRLA0xe8
-#define ILI9341_DTCTRLB0xea
-#define ILI9341_PWRSEQ 0xed
-
-#define ILI9341_EN3GAM 0xf2
-#define ILI9341_PUMPCTRL 0xf7
-
-#define ILI9341_MADCTL_BGR BIT(3)
-#define ILI9341_MADCTL_MV BIT(5)
-#define ILI9341_MADCTL_MX BIT(6)
-#define ILI9341_MADCTL_MY BIT(7)
-
-static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
-struct drm_crtc_state *crtc_state,
-struct drm_plane_state *plane_state)
-{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct mipi_dbi *dbi = >dbi;
- u8 addr_mode;
- int ret, idx;
-
- if (!drm_dev_enter(pipe->crtc.dev, ))
- return;
-
- DRM_DEBUG_KMS("\n");
-
- ret = mipi_dbi_poweron_conditional_reset(dbidev);
- if (ret < 0)
- goto out_exit;
- if (ret == 1)
- goto out_enable;
-
- mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
-
- mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0xc1, 0x30);
- mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81);
- mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x00, 0x78);
- mipi_dbi_command(dbi, ILI9341_PWCTRLA, 0x39, 0x2c, 0x00, 0x34, 0x02);
- mipi_dbi_command(dbi, ILI9341_PUMPCTRL, 0x20);
- mipi_dbi_command(dbi, ILI9341_DTCTRLB, 0x00, 0x00);
-
- /* Power Control */
- mipi_dbi_command(dbi, ILI9341_PWCTRL1, 0x23);
- mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x10);
- /* VCOM */
- mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x3e, 0x28);
- mipi_dbi_command(dbi, ILI9341_VMCTRL2,
