This series adds drm_panel support to drm_mipi_dbi. This avoids having 2 drivers for the same controller, one per interface DPI/DBI. Some display controllers like the ILI9341 support both interfaces and some displays/panels make both these interfaces available.
Some info about why MIPI DBI is special: MIPI DSI (Display Serial Interface) and MIPI DPI (Display Pixel Interface) needs specialiced hardware and thus they have interface drivers for this. MIPI DBI (Display Bus Interface) on the other hand reuses existing busses like SPI/8080/6800 and in the case of SPI just reuses the exising SPI subsystem. All the panels I have seen that have onboard RAM and can receive pixels over DBI needs to initialize the controller as well. This means that both the panel driver and the interface driver needs access to the same bus device.
Changes since RFC[1]: - Instead of starting with Josef's driver[2], I've started with moving the existing ili9341 driver into drm/panel and then move over mi0283qt as well since it has a ili9341 controller. Josef can then add support for his panel to this driver. His panel can support both interface modes. - Emil mentioned that the DRM driver name is ABI so I've honoured that. - The DRM driver structure is now defined in the driver using a macro instead of doing it in drm_mipi_dbi. This is necessary for file open to take a ref on the driver module.
Another thing: Looking at drm_panel.c I couldn't find anything that pins the panel driver module while the panel is attached to the connector. Shouldn't struct drm_panel_funcs have a module owner field?
Noralf.
[1] https://patchwork.freedesktop.org/series/64398/ [2] https://patchwork.freedesktop.org/patch/316528/
Noralf Trønnes (4): drm/mipi-dbi: Support command mode panel drivers drm/tiny/ili9341: Move driver to drm/panel drm/tiny/mi0283qt: Move driver to panel-ilitek-ili9341 drm/panel/ili9341: Support DPI panels
MAINTAINERS | 16 +- drivers/gpu/drm/drm_mipi_dbi.c | 92 ++++ drivers/gpu/drm/panel/Kconfig | 13 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 469 +++++++++++++++++++ drivers/gpu/drm/tiny/Kconfig | 24 - drivers/gpu/drm/tiny/Makefile | 2 - drivers/gpu/drm/tiny/ili9341.c | 268 ----------- drivers/gpu/drm/tiny/mi0283qt.c | 294 ------------ include/drm/drm_mipi_dbi.h | 34 ++ 10 files changed, 618 insertions(+), 595 deletions(-) create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c delete mode 100644 drivers/gpu/drm/tiny/ili9341.c delete mode 100644 drivers/gpu/drm/tiny/mi0283qt.c
This adds a function that registers a DRM driver for use with MIPI DBI panels in command mode. That is, framebuffer upload over DBI.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/drm_mipi_dbi.c | 92 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dbi.h | 34 +++++++++++++ 2 files changed, 126 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index 1961f713aaab..797a20e3a017 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -17,11 +17,13 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_mipi_dbi.h> #include <drm/drm_modes.h> +#include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> #include <drm/drm_rect.h> #include <drm/drm_vblank.h> @@ -597,6 +599,96 @@ void mipi_dbi_release(struct drm_device *drm) } EXPORT_SYMBOL(mipi_dbi_release);
+static void drm_mipi_dbi_panel_pipe_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 drm_panel *panel = dbidev->panel; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + + DRM_DEBUG_KMS("\n"); + + ret = drm_panel_prepare(panel); + if (ret) + goto out_exit; + + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); + + drm_panel_enable(panel); +out_exit: + drm_dev_exit(idx); +} + +static void drm_mipi_dbi_panel_pipe_disable(struct drm_simple_display_pipe *pipe) +{ + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + struct drm_panel *panel = dbidev->panel; + + if (!dbidev->enabled) + return; + + DRM_DEBUG_KMS("\n"); + + dbidev->enabled = false; + drm_panel_disable(panel); + drm_panel_unprepare(panel); +} + +static const struct drm_simple_display_pipe_funcs drm_mipi_dbi_pipe_funcs = { + .enable = drm_mipi_dbi_panel_pipe_enable, + .disable = drm_mipi_dbi_panel_pipe_disable, + .update = mipi_dbi_pipe_update, + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, +}; + +/** + * drm_mipi_dbi_panel_register - Register a MIPI DBI DRM driver + * @panel: DRM Panel + * @dbidev: MIPI DBI device structure to initialize + * @mode: Display mode + * + * This function registeres a DRM driver with @panel attached. + * + * Returns: + * Zero on success, negative error code on failure. + */ +int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev, + struct drm_driver *driver, const struct drm_display_mode *mode, + u32 rotation) +{ + struct device *dev = panel->dev; + struct drm_device *drm; + int ret; + + dbidev->panel = panel; + + drm = &dbidev->drm; + ret = devm_drm_dev_init(dev, drm, driver); + if (ret) { + kfree(dbidev); + return ret; + } + + drm_mode_config_init(drm); + + ret = mipi_dbi_dev_init(dbidev, &drm_mipi_dbi_pipe_funcs, mode, rotation); + + drm_mode_config_reset(drm); + + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + drm_fbdev_generic_setup(drm, 16); + + return 0; +} +EXPORT_SYMBOL(drm_mipi_dbi_panel_register); + /** * mipi_dbi_hw_reset - Hardware reset of controller * @dbi: MIPI DBI structure diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h index 67c66f5ee591..f41ee0d31871 100644 --- a/include/drm/drm_mipi_dbi.h +++ b/include/drm/drm_mipi_dbi.h @@ -12,6 +12,7 @@ #include <drm/drm_device.h> #include <drm/drm_simple_kms_helper.h>
+struct drm_panel; struct drm_rect; struct spi_device; struct gpio_desc; @@ -123,6 +124,11 @@ struct mipi_dbi_dev { * @dbi: MIPI DBI interface */ struct mipi_dbi dbi; + + /** + * @panel: Attached DRM panel. See drm_mipi_dbi_panel_register(). + */ + struct drm_panel *panel; };
static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm) @@ -140,6 +146,34 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev, int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_simple_display_pipe_funcs *funcs, const struct drm_display_mode *mode, unsigned int rotation); + +/** + * DEFINE_DRM_MIPI_DBI_PANEL_DRIVER - Define a DRM driver structure + * @_name: Name + * @_desc: Description + * @_date: Date + * + * This macro defines a &drm_driver for MIPI DBI panel drivers. + */ +#define DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(_name, _desc, _date) \ + DEFINE_DRM_GEM_CMA_FOPS(_name ## _fops); \ + static struct drm_driver _name ## _drm_driver = { \ + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, \ + .fops = & _name ## _fops, \ + .release = mipi_dbi_release, \ + DRM_GEM_CMA_VMAP_DRIVER_OPS, \ + .debugfs_init = mipi_dbi_debugfs_init, \ + .name = #_name, \ + .desc = _desc, \ + .date = _date, \ + .major = 1, \ + .minor = 0, \ + } + +int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev, + struct drm_driver *driver, const struct drm_display_mode *mode, + u32 rotation); + void mipi_dbi_release(struct drm_device *drm); void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_plane_state *old_state);
Hi Noralf.
I really like how this allows us to have a single file for all the uses of a driver IC. And this patch-set is much easier to grasp than the first RFC.
General things:
- The current design have a tight connection between the display controller and the panel. Will this hurt in the long run? In other words, should we try to add a panel_bridge in-between? For now I think this would just make something simple more complicated. So this note was to say - no I think we should not use panel_bridge.
- drm_panel has proper support for modes. This is today duplicated in mipi_dbi. Could we make it so that when a panel is used then the panel has the mode info - as we then use the panel more in the way we do in other cases? As it is now the mode is specified in mipi_dbi_dev_init() The drm_connector would then, if a panel is used, ask the panel for the mode. I did not really think to the end of this, but it seems wrong that we introduce drm_panel and then keep modes in mipi_dbi.
- For backlight support please move this to drm_panel. I have patches that makes it simple to use backlight with drm_panel, and this will then benefit from it. The drm_panel backlight supports requires that a backlight phandle is specified in the DT node of the panel.
Some more specific comments in the following.
Sam
On Thu, Aug 01, 2019 at 03:52:46PM +0200, Noralf Trønnes wrote:
This adds a function that registers a DRM driver for use with MIPI DBI panels in command mode. That is, framebuffer upload over DBI.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/drm_mipi_dbi.c | 92 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dbi.h | 34 +++++++++++++ 2 files changed, 126 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index 1961f713aaab..797a20e3a017 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -17,11 +17,13 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_mipi_dbi.h> #include <drm/drm_modes.h> +#include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> #include <drm/drm_rect.h> #include <drm/drm_vblank.h> @@ -597,6 +599,96 @@ void mipi_dbi_release(struct drm_device *drm) } EXPORT_SYMBOL(mipi_dbi_release);
+static void drm_mipi_dbi_panel_pipe_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 drm_panel *panel = dbidev->panel;
- int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
return;- DRM_DEBUG_KMS("\n");
Still usefull?
- ret = drm_panel_prepare(panel);
- if (ret)
goto out_exit;- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
- drm_panel_enable(panel);
+out_exit:
nit - blank line above label?
- drm_dev_exit(idx);
+}
+static void drm_mipi_dbi_panel_pipe_disable(struct drm_simple_display_pipe *pipe) +{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct drm_panel *panel = dbidev->panel;
- if (!dbidev->enabled)
return;- DRM_DEBUG_KMS("\n");
Still usefull?
- dbidev->enabled = false;
- drm_panel_disable(panel);
- drm_panel_unprepare(panel);
+}
+static const struct drm_simple_display_pipe_funcs drm_mipi_dbi_pipe_funcs = {
- .enable = drm_mipi_dbi_panel_pipe_enable,
- .disable = drm_mipi_dbi_panel_pipe_disable,
- .update = mipi_dbi_pipe_update,
- .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+/**
- drm_mipi_dbi_panel_register - Register a MIPI DBI DRM driver
- @panel: DRM Panel
- @dbidev: MIPI DBI device structure to initialize
- @mode: Display mode
- This function registeres a DRM driver with @panel attached.
- Returns:
- Zero on success, negative error code on failure.
- */
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
struct drm_driver *driver, const struct drm_display_mode *mode,u32 rotation)
Can we make this use enum drm_panel_orientation - so we can use of_drm_get_panel_orientation() in the callers? of_drm_get_panel_orientation() is not merged yet, but we could do so if this patch-set needs it.
I know that this would require mipi_dbi_dev_init() and all users to be updated. But it is a simpler interface so worth it.
+{
- struct device *dev = panel->dev;
- struct drm_device *drm;
- int ret;
- dbidev->panel = panel;
- drm = &dbidev->drm;
- ret = devm_drm_dev_init(dev, drm, driver);
- if (ret) {
kfree(dbidev);return ret;- }
- drm_mode_config_init(drm);
- ret = mipi_dbi_dev_init(dbidev, &drm_mipi_dbi_pipe_funcs, mode, rotation);
- drm_mode_config_reset(drm);
- ret = drm_dev_register(drm, 0);
- if (ret)
return ret;- drm_fbdev_generic_setup(drm, 16);
- return 0;
+} +EXPORT_SYMBOL(drm_mipi_dbi_panel_register);
/**
- mipi_dbi_hw_reset - Hardware reset of controller
- @dbi: MIPI DBI structure
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h index 67c66f5ee591..f41ee0d31871 100644 --- a/include/drm/drm_mipi_dbi.h +++ b/include/drm/drm_mipi_dbi.h @@ -12,6 +12,7 @@ #include <drm/drm_device.h> #include <drm/drm_simple_kms_helper.h>
+struct drm_panel; struct drm_rect; struct spi_device; struct gpio_desc; @@ -123,6 +124,11 @@ struct mipi_dbi_dev { * @dbi: MIPI DBI interface */ struct mipi_dbi dbi;
- /**
* @panel: Attached DRM panel. See drm_mipi_dbi_panel_register().*/- struct drm_panel *panel;
};
static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm) @@ -140,6 +146,34 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev, int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_simple_display_pipe_funcs *funcs, const struct drm_display_mode *mode, unsigned int rotation);
+/**
- DEFINE_DRM_MIPI_DBI_PANEL_DRIVER - Define a DRM driver structure
- @_name: Name
- @_desc: Description
- @_date: Date
- This macro defines a &drm_driver for MIPI DBI panel drivers.
- */
+#define DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(_name, _desc, _date) \
- DEFINE_DRM_GEM_CMA_FOPS(_name ## _fops); \
- static struct drm_driver _name ## _drm_driver = { \
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, \.fops = & _name ## _fops, \.release = mipi_dbi_release, \DRM_GEM_CMA_VMAP_DRIVER_OPS, \.debugfs_init = mipi_dbi_debugfs_init, \.name = #_name, \.desc = _desc, \.date = _date, \.major = 1, \.minor = 0, \- }
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
struct drm_driver *driver, const struct drm_display_mode *mode,u32 rotation);void mipi_dbi_release(struct drm_device *drm); void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_plane_state *old_state); -- 2.20.1
Den 11.08.2019 16.16, skrev Sam Ravnborg:
Hi Noralf.
I really like how this allows us to have a single file for all the uses of a driver IC. And this patch-set is much easier to grasp than the first RFC.
General things:
- The current design have a tight connection between the display controller and the panel. Will this hurt in the long run? In other words, should we try to add a panel_bridge in-between? For now I think this would just make something simple more complicated. So this note was to say - no I think we should not use panel_bridge.
I did look at panel_bridge but it didn't give me anything I needed, it would only be a 1:1 passthrough layer.
- drm_panel has proper support for modes. This is today duplicated in mipi_dbi. Could we make it so that when a panel is used then the panel has the mode info - as we then use the panel more in the way we do in other cases? As it is now the mode is specified in mipi_dbi_dev_init() The drm_connector would then, if a panel is used, ask the panel for the mode. I did not really think to the end of this, but it seems wrong that we introduce drm_panel and then keep modes in mipi_dbi.
I considered that, but it would would just generate duplicate code for the connector. It would make sense to refactor this when/if all mipi dbi drivers are turned into panel drivers.
- For backlight support please move this to drm_panel. I have patches that makes it simple to use backlight with drm_panel, and this will then benefit from it. The drm_panel backlight supports requires that a backlight phandle is specified in the DT node of the panel.
I can fix that when I respin if those patches have landed by then.
Some more specific comments in the following.
Sam
On Thu, Aug 01, 2019 at 03:52:46PM +0200, Noralf Trønnes wrote:
This adds a function that registers a DRM driver for use with MIPI DBI panels in command mode. That is, framebuffer upload over DBI.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/drm_mipi_dbi.c | 92 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dbi.h | 34 +++++++++++++ 2 files changed, 126 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index 1961f713aaab..797a20e3a017 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -17,11 +17,13 @@ #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> +#include <drm/drm_fb_helper.h> #include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_mipi_dbi.h> #include <drm/drm_modes.h> +#include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> #include <drm/drm_rect.h> #include <drm/drm_vblank.h> @@ -597,6 +599,96 @@ void mipi_dbi_release(struct drm_device *drm) } EXPORT_SYMBOL(mipi_dbi_release);
+static void drm_mipi_dbi_panel_pipe_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 drm_panel *panel = dbidev->panel;
- int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
return;- DRM_DEBUG_KMS("\n");
Still usefull?
No I think this can go (it was in the code I copied from the driver).
- ret = drm_panel_prepare(panel);
- if (ret)
goto out_exit;- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
- drm_panel_enable(panel);
+out_exit:
nit - blank line above label?
- drm_dev_exit(idx);
+}
+static void drm_mipi_dbi_panel_pipe_disable(struct drm_simple_display_pipe *pipe) +{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct drm_panel *panel = dbidev->panel;
- if (!dbidev->enabled)
return;- DRM_DEBUG_KMS("\n");
Still usefull?
- dbidev->enabled = false;
- drm_panel_disable(panel);
- drm_panel_unprepare(panel);
+}
+static const struct drm_simple_display_pipe_funcs drm_mipi_dbi_pipe_funcs = {
- .enable = drm_mipi_dbi_panel_pipe_enable,
- .disable = drm_mipi_dbi_panel_pipe_disable,
- .update = mipi_dbi_pipe_update,
- .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+/**
- drm_mipi_dbi_panel_register - Register a MIPI DBI DRM driver
- @panel: DRM Panel
- @dbidev: MIPI DBI device structure to initialize
- @mode: Display mode
- This function registeres a DRM driver with @panel attached.
- Returns:
- Zero on success, negative error code on failure.
- */
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
struct drm_driver *driver, const struct drm_display_mode *mode,u32 rotation)Can we make this use enum drm_panel_orientation - so we can use of_drm_get_panel_orientation() in the callers? of_drm_get_panel_orientation() is not merged yet, but we could do so if this patch-set needs it.
I know that this would require mipi_dbi_dev_init() and all users to be updated. But it is a simpler interface so worth it.
That would break rotation on userspace that doesn't know about the panel orientation property which is a recent addition. These panels are mostly used in the embedded world not desktop. It also would break fbdev 90/270 rotation (see drm_client_rotation()).
Noralf.
+{
- struct device *dev = panel->dev;
- struct drm_device *drm;
- int ret;
- dbidev->panel = panel;
- drm = &dbidev->drm;
- ret = devm_drm_dev_init(dev, drm, driver);
- if (ret) {
kfree(dbidev);return ret;- }
- drm_mode_config_init(drm);
- ret = mipi_dbi_dev_init(dbidev, &drm_mipi_dbi_pipe_funcs, mode, rotation);
- drm_mode_config_reset(drm);
- ret = drm_dev_register(drm, 0);
- if (ret)
return ret;- drm_fbdev_generic_setup(drm, 16);
- return 0;
+} +EXPORT_SYMBOL(drm_mipi_dbi_panel_register);
/**
- mipi_dbi_hw_reset - Hardware reset of controller
- @dbi: MIPI DBI structure
Hi Noralf.
- drm_panel has proper support for modes. This is today duplicated in mipi_dbi. Could we make it so that when a panel is used then the panel has the mode info - as we then use the panel more in the way we do in other cases? As it is now the mode is specified in mipi_dbi_dev_init() The drm_connector would then, if a panel is used, ask the panel for the mode. I did not really think to the end of this, but it seems wrong that we introduce drm_panel and then keep modes in mipi_dbi.
I considered that, but it would would just generate duplicate code for the connector. It would make sense to refactor this when/if all mipi dbi drivers are turned into panel drivers.
The objective should be that all mipi dbi drivers could be refactored. And so it makes sense to do it right from the beginning. It will be some duplicated code until all are migrated. But as the number of mipi dbi drivers are low it is doable if a few people throw some time after it. I volunteer to assist.
In drm_mipi_dbi.c we could just add:
static int mipi_dbi_connector_get_modes(struct drm_connector *connector) { ... if (dbidev->panel) return drm_panel_get_modes(dbidev->panel);
Then if there is a panel we would use the mode specified by the panel. To make this work we would need a drm_panel_attach() in drm_mipi_dbi_panel_register() to give the panel access to the connector. I have patches that makes connector an argument to drm_panel_get_modes() but they need a bit more baking, so you cannot benefit from them yet.
Maybe this is the same argument as backlight? We can introduce this when the drm_panel core is better prepared.
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
struct drm_driver *driver, const struct drm_display_mode *mode,u32 rotation)Can we make this use enum drm_panel_orientation - so we can use of_drm_get_panel_orientation() in the callers? of_drm_get_panel_orientation() is not merged yet, but we could do so if this patch-set needs it.
I know that this would require mipi_dbi_dev_init() and all users to be updated. But it is a simpler interface so worth it.
That would break rotation on userspace that doesn't know about the panel orientation property which is a recent addition. These panels are mostly used in the embedded world not desktop. It also would break fbdev 90/270 rotation (see drm_client_rotation()).
I think it is possible to move most of drm over to one way to spicify rotation. But let's wait with that battle until another day. It should not hinder this series.
Sam
Den 12.08.2019 20.49, skrev Sam Ravnborg:
Hi Noralf.
- drm_panel has proper support for modes. This is today duplicated in mipi_dbi. Could we make it so that when a panel is used then the panel has the mode info - as we then use the panel more in the way we do in other cases? As it is now the mode is specified in mipi_dbi_dev_init() The drm_connector would then, if a panel is used, ask the panel for the mode. I did not really think to the end of this, but it seems wrong that we introduce drm_panel and then keep modes in mipi_dbi.
I considered that, but it would would just generate duplicate code for the connector. It would make sense to refactor this when/if all mipi dbi drivers are turned into panel drivers.
The objective should be that all mipi dbi drivers could be refactored.
Not all can be converted, easily at least. ili9225 and st7586 are not true MIPI DBI controllers, they are just similar enough to benefit from the helper.
And so it makes sense to do it right from the beginning. It will be some duplicated code until all are migrated. But as the number of mipi dbi drivers are low it is doable if a few people throw some time after it. I volunteer to assist.
In drm_mipi_dbi.c we could just add:
static int mipi_dbi_connector_get_modes(struct drm_connector *connector) { ... if (dbidev->panel) return drm_panel_get_modes(dbidev->panel);
This would make sense if the display mode was used as-is in the panel driver. But the mode can be rotated so that would neeed to be taken care of somehow.
I have started to work on a driver now so I can spend time on complex refactorings. I'm not against changing this, but maybe a change like this should happen after drivers have been converted and the picture is more clear.
And Thierry hasn't voiced his opinion on this either, so no point in spending more time on this if he disagrees with this "panel driver as full drm driver" move.
Noralf.
Then if there is a panel we would use the mode specified by the panel. To make this work we would need a drm_panel_attach() in drm_mipi_dbi_panel_register() to give the panel access to the connector. I have patches that makes connector an argument to drm_panel_get_modes() but they need a bit more baking, so you cannot benefit from them yet.
Maybe this is the same argument as backlight? We can introduce this when the drm_panel core is better prepared.
+int drm_mipi_dbi_panel_register(struct drm_panel *panel, struct mipi_dbi_dev *dbidev,
struct drm_driver *driver, const struct drm_display_mode *mode,u32 rotation)Can we make this use enum drm_panel_orientation - so we can use of_drm_get_panel_orientation() in the callers? of_drm_get_panel_orientation() is not merged yet, but we could do so if this patch-set needs it.
I know that this would require mipi_dbi_dev_init() and all users to be updated. But it is a simpler interface so worth it.
That would break rotation on userspace that doesn't know about the panel orientation property which is a recent addition. These panels are mostly used in the embedded world not desktop. It also would break fbdev 90/270 rotation (see drm_client_rotation()).
I think it is possible to move most of drm over to one way to spicify rotation. But let's wait with that battle until another day. It should not hinder this series.
Sam
Move the driver to drm/panel and take advantage of the new panel support in drm_mipi_dbi. Change the file name to match the naming standard in drm/panel. The DRM driver name is kept since it is ABI.
Add missing MAINTAINERS entry.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- MAINTAINERS | 7 + drivers/gpu/drm/panel/Kconfig | 12 ++ drivers/gpu/drm/panel/Makefile | 1 + .../panel-ilitek-ili9341.c} | 174 ++++++++++-------- drivers/gpu/drm/tiny/Kconfig | 13 -- drivers/gpu/drm/tiny/Makefile | 1 - 6 files changed, 113 insertions(+), 95 deletions(-) rename drivers/gpu/drm/{tiny/ili9341.c => panel/panel-ilitek-ili9341.c} (66%)
diff --git a/MAINTAINERS b/MAINTAINERS index 6fe3462a1f7a..66b3893a100f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5108,6 +5108,13 @@ S: Maintained F: drivers/gpu/drm/tiny/ili9225.c F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
+DRM DRIVER FOR ILITEK ILI9341 PANELS +M: David Lechner david@lechnology.com +T: git git://anongit.freedesktop.org/drm/drm-misc +S: Maintained +F: drivers/gpu/drm/panel/panel-ilitek-ili9341.c +F: Documentation/devicetree/bindings/display/ilitek,ili9341.txt + DRM DRIVER FOR HX8357D PANELS M: Eric Anholt eric@anholt.net T: git git://anongit.freedesktop.org/drm/drm-misc diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index eaecd40cc32e..a24953ec2d40 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -56,6 +56,18 @@ 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_ILI9341 + tristate "Ilitek ILI9341 display panels" + depends on SPI + depends on BACKLIGHT_CLASS_DEVICE + select DRM_KMS_CMA_HELPER + select DRM_MIPI_DBI + 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 panel-ilitek-ili9341. + 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 62dae45f8f74..ba4a303c1a66 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o +obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c similarity index 66% rename from drivers/gpu/drm/tiny/ili9341.c rename to drivers/gpu/drm/panel/panel-ilitek-ili9341.c index 33b51dc7faa8..f8df737018d3 100644 --- a/drivers/gpu/drm/tiny/ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -12,17 +12,17 @@ #include <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/property.h> #include <linux/spi/spi.h>
+#include <video/mipi_display.h> + #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> -#include <drm/drm_fb_helper.h> #include <drm/drm_gem_cma_helper.h> -#include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_mipi_dbi.h> -#include <drm/drm_modeset_helper.h> -#include <video/mipi_display.h> +#include <drm/drm_panel.h>
#define ILI9341_FRMCTR1 0xb1 #define ILI9341_DISCTRL 0xb6 @@ -49,23 +49,48 @@ #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 ili9341_config { + const struct drm_panel_funcs *funcs; + const struct drm_display_mode *mode; +}; + +struct ili9341 { + struct mipi_dbi_dev dbidev; /* This must be the first entry */ + struct drm_panel panel; + struct backlight_device *backlight; + const struct ili9341_config *conf; +}; + +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) { - struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + return container_of(panel, struct ili9341, panel); +} + +static int ili9341_enable(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + + return backlight_enable(ili->backlight); +} + +static int ili9341_disable(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + + return backlight_disable(ili->backlight); +} + +static int yx240qv29_prepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + struct mipi_dbi_dev *dbidev = &ili->dbidev; struct mipi_dbi *dbi = &dbidev->dbi; u8 addr_mode; - int ret, idx; - - if (!drm_dev_enter(pipe->crtc.dev, &idx)) - return; - - DRM_DEBUG_KMS("\n"); + int ret;
ret = mipi_dbi_poweron_conditional_reset(dbidev); if (ret < 0) - goto out_exit; + return ret; if (ret == 1) goto out_enable;
@@ -130,72 +155,54 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe, } addr_mode |= ILI9341_MADCTL_BGR; mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); - mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); -out_exit: - drm_dev_exit(idx); + + return 0; }
-static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = { - .enable = yx240qv29_enable, - .disable = mipi_dbi_pipe_disable, - .update = mipi_dbi_pipe_update, - .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, +static const struct drm_panel_funcs yx240qv29_funcs = { + .disable = ili9341_disable, + .prepare = yx240qv29_prepare, + .enable = ili9341_enable, };
static const struct drm_display_mode yx240qv29_mode = { DRM_SIMPLE_MODE(240, 320, 37, 49), };
-DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops); - -static struct drm_driver ili9341_driver = { - .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .fops = &ili9341_fops, - .release = mipi_dbi_release, - DRM_GEM_CMA_VMAP_DRIVER_OPS, - .debugfs_init = mipi_dbi_debugfs_init, - .name = "ili9341", - .desc = "Ilitek ILI9341", - .date = "20180514", - .major = 1, - .minor = 0, +static const struct ili9341_config yx240qv29_data = { + .funcs = &yx240qv29_funcs, + .mode = &yx240qv29_mode, };
-static const struct of_device_id ili9341_of_match[] = { - { .compatible = "adafruit,yx240qv29" }, - { } -}; -MODULE_DEVICE_TABLE(of, ili9341_of_match); - -static const struct spi_device_id ili9341_id[] = { - { "yx240qv29", 0 }, - { } -}; -MODULE_DEVICE_TABLE(spi, ili9341_id); +DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(ili9341, "Ilitek ILI9341", "20180514");
static int ili9341_probe(struct spi_device *spi) { + const struct spi_device_id *spi_id; struct device *dev = &spi->dev; - struct mipi_dbi_dev *dbidev; - struct drm_device *drm; struct mipi_dbi *dbi; struct gpio_desc *dc; + struct ili9341 *ili; u32 rotation = 0; int ret;
- dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL); - if (!dbidev) + ili = kzalloc(sizeof(*ili), GFP_KERNEL); + if (!ili) return -ENOMEM;
- dbi = &dbidev->dbi; - drm = &dbidev->drm; - ret = devm_drm_dev_init(dev, drm, &ili9341_driver); - if (ret) { - kfree(dbidev); - return ret; + ili->conf = of_device_get_match_data(dev); + if (!ili->conf) { + spi_id = spi_get_device_id(spi); + if (spi_id) + ili->conf = (struct ili9341_config *)spi_id->driver_data; }
- drm_mode_config_init(drm); + if (!ili->conf) + return -ENODEV; + + spi_set_drvdata(spi, ili); + + dbi = &ili->dbidev.dbi;
dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(dbi->reset)) { @@ -209,9 +216,9 @@ static int ili9341_probe(struct spi_device *spi) return PTR_ERR(dc); }
- dbidev->backlight = devm_of_find_backlight(dev); - if (IS_ERR(dbidev->backlight)) - return PTR_ERR(dbidev->backlight); + ili->backlight = devm_of_find_backlight(dev); + if (IS_ERR(ili->backlight)) + return PTR_ERR(ili->backlight);
device_property_read_u32(dev, "rotation", &rotation);
@@ -219,41 +226,46 @@ static int ili9341_probe(struct spi_device *spi) if (ret) return ret;
- ret = mipi_dbi_dev_init(dbidev, &ili9341_pipe_funcs, &yx240qv29_mode, rotation); - if (ret) - return ret; + drm_panel_init(&ili->panel); + ili->panel.dev = dev; + ili->panel.funcs = ili->conf->funcs;
- drm_mode_config_reset(drm); - - ret = drm_dev_register(drm, 0); - if (ret) - return ret; - - spi_set_drvdata(spi, drm); - - drm_fbdev_generic_setup(drm, 0); - - return 0; + return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, &ili9341_drm_driver, + ili->conf->mode, rotation); }
static int ili9341_remove(struct spi_device *spi) { - struct drm_device *drm = spi_get_drvdata(spi); + struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(drm); - drm_atomic_helper_shutdown(drm); + drm_dev_unplug(&ili->dbidev.drm); + drm_atomic_helper_shutdown(&ili->dbidev.drm);
return 0; }
static void ili9341_shutdown(struct spi_device *spi) { - drm_atomic_helper_shutdown(spi_get_drvdata(spi)); + struct ili9341 *ili = spi_get_drvdata(spi); + + drm_atomic_helper_shutdown(&ili->dbidev.drm); }
+static const struct of_device_id ili9341_of_match[] = { + { .compatible = "adafruit,yx240qv29", .data = &yx240qv29_data }, + { } +}; +MODULE_DEVICE_TABLE(of, ili9341_of_match); + +static const struct spi_device_id ili9341_id[] = { + { "yx240qv29", (unsigned long)&yx240qv29_data }, + { } +}; +MODULE_DEVICE_TABLE(spi, ili9341_id); + static struct spi_driver ili9341_spi_driver = { .driver = { - .name = "ili9341", + .name = "panel-ilitek-ili9341", .of_match_table = ili9341_of_match, }, .id_table = ili9341_id, @@ -263,6 +275,6 @@ static struct spi_driver ili9341_spi_driver = { }; module_spi_driver(ili9341_spi_driver);
-MODULE_DESCRIPTION("Ilitek ILI9341 DRM driver"); +MODULE_DESCRIPTION("Ilitek ILI9341 panel driver"); MODULE_AUTHOR("David Lechner david@lechnology.com"); MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 504763423d46..3e63e93fbeac 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -34,19 +34,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_KMS_CMA_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_MI0283QT tristate "DRM support for MI0283QT" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 896cf31132d3..c140962f5c7e 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -3,7 +3,6 @@ obj-$(CONFIG_DRM_GM12U320) += gm12u320.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o -obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o obj-$(CONFIG_TINYDRM_ST7586) += st7586.o
On 8/1/19 8:52 AM, Noralf Trønnes wrote:
Move the driver to drm/panel and take advantage of the new panel support in drm_mipi_dbi. Change the file name to match the naming standard in drm/panel. The DRM driver name is kept since it is ABI.
Add missing MAINTAINERS entry.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
Reviewed-by: David Lechner david@lechnology.com
Although, I will say that the way the diff came out, it makes it a bit hard to follow the patch, so more more details in the commit message about the specific changes could be helpful.
Den 01.08.2019 21.43, skrev David Lechner:
On 8/1/19 8:52 AM, Noralf Trønnes wrote:
Move the driver to drm/panel and take advantage of the new panel support in drm_mipi_dbi. Change the file name to match the naming standard in drm/panel. The DRM driver name is kept since it is ABI.
Add missing MAINTAINERS entry.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
Reviewed-by: David Lechner david@lechnology.com
Although, I will say that the way the diff came out, it makes it a bit hard to follow the patch, so more more details in the commit message about the specific changes could be helpful.
Oh, I actually liked how the diff came out, since I found it easy to see how the drivers differed. But then again, I have have the code fresh in my brain cache so that might make a difference in how it looks.
I can expand the commit message.
I also see that I have moved the device tables, maybe I should move them back where they where originally so they are closer to the configs they refer to.
Noralf.
Hi Noralf.
Most feedback on this driver was covered in comment to 1/4. Only a few things caught my eye.
On Thu, Aug 01, 2019 at 03:52:47PM +0200, Noralf Trønnes wrote:
Move the driver to drm/panel and take advantage of the new panel support in drm_mipi_dbi. Change the file name to match the naming standard in drm/panel. The DRM driver name is kept since it is ABI.
Add missing MAINTAINERS entry.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
MAINTAINERS | 7 + drivers/gpu/drm/panel/Kconfig | 12 ++ drivers/gpu/drm/panel/Makefile | 1 + .../panel-ilitek-ili9341.c} | 174 ++++++++++-------- drivers/gpu/drm/tiny/Kconfig | 13 -- drivers/gpu/drm/tiny/Makefile | 1 - 6 files changed, 113 insertions(+), 95 deletions(-) rename drivers/gpu/drm/{tiny/ili9341.c => panel/panel-ilitek-ili9341.c} (66%)
+struct ili9341 {
- struct mipi_dbi_dev dbidev; /* This must be the first entry */
Can we avoid the need for this to be the first entry?
-static struct drm_driver ili9341_driver = {
- .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- .fops = &ili9341_fops,
- .release = mipi_dbi_release,
- DRM_GEM_CMA_VMAP_DRIVER_OPS,
- .debugfs_init = mipi_dbi_debugfs_init,
- .name = "ili9341",
- .desc = "Ilitek ILI9341",
- .date = "20180514",
- .major = 1,
- .minor = 0,
+DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(ili9341, "Ilitek ILI9341", "20180514");
Update the date. This is a major change so let it be refelcted in the date.
Sam
Den 11.08.2019 17.24, skrev Sam Ravnborg:
Hi Noralf.
Most feedback on this driver was covered in comment to 1/4. Only a few things caught my eye.
On Thu, Aug 01, 2019 at 03:52:47PM +0200, Noralf Trønnes wrote:
Move the driver to drm/panel and take advantage of the new panel support in drm_mipi_dbi. Change the file name to match the naming standard in drm/panel. The DRM driver name is kept since it is ABI.
Add missing MAINTAINERS entry.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
MAINTAINERS | 7 + drivers/gpu/drm/panel/Kconfig | 12 ++ drivers/gpu/drm/panel/Makefile | 1 + .../panel-ilitek-ili9341.c} | 174 ++++++++++-------- drivers/gpu/drm/tiny/Kconfig | 13 -- drivers/gpu/drm/tiny/Makefile | 1 - 6 files changed, 113 insertions(+), 95 deletions(-) rename drivers/gpu/drm/{tiny/ili9341.c => panel/panel-ilitek-ili9341.c} (66%)
+struct ili9341 {
- struct mipi_dbi_dev dbidev; /* This must be the first entry */
Can we avoid the need for this to be the first entry?
That would require this driver to have a custom drm_driver->release and pass that into the DEFINE_DRM_MIPI_DBI_PANEL_DRIVER macro.
Having it as the first entry, mipi_dbi_release() will suffice. Simplifying things.
-static struct drm_driver ili9341_driver = {
- .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- .fops = &ili9341_fops,
- .release = mipi_dbi_release,
- DRM_GEM_CMA_VMAP_DRIVER_OPS,
- .debugfs_init = mipi_dbi_debugfs_init,
- .name = "ili9341",
- .desc = "Ilitek ILI9341",
- .date = "20180514",
- .major = 1,
- .minor = 0,
+DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(ili9341, "Ilitek ILI9341", "20180514");
Update the date. This is a major change so let it be refelcted in the date.
I guess that makes sense. I don't know what this date field is used for though.
Noralf.
The MI0283QT panels use a ILI9341 controller so it makes sense to merge it with the other ili9341 code.
The DRM driver name is ABI, so that is retained.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- MAINTAINERS | 9 +- drivers/gpu/drm/panel/Kconfig | 1 + drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 161 +++++++++- drivers/gpu/drm/tiny/Kconfig | 11 - drivers/gpu/drm/tiny/Makefile | 1 - drivers/gpu/drm/tiny/mi0283qt.c | 294 ------------------- 6 files changed, 161 insertions(+), 316 deletions(-) delete mode 100644 drivers/gpu/drm/tiny/mi0283qt.c
diff --git a/MAINTAINERS b/MAINTAINERS index 66b3893a100f..cd7b8bdb3780 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5110,10 +5110,12 @@ F: Documentation/devicetree/bindings/display/ilitek,ili9225.txt
DRM DRIVER FOR ILITEK ILI9341 PANELS M: David Lechner david@lechnology.com +M: Noralf Trønnes noralf@tronnes.org T: git git://anongit.freedesktop.org/drm/drm-misc S: Maintained F: drivers/gpu/drm/panel/panel-ilitek-ili9341.c F: Documentation/devicetree/bindings/display/ilitek,ili9341.txt +F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt
DRM DRIVER FOR HX8357D PANELS M: Eric Anholt eric@anholt.net @@ -5137,13 +5139,6 @@ M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/
-DRM DRIVER FOR MI0283QT -M: Noralf Trønnes noralf@tronnes.org -T: git git://anongit.freedesktop.org/drm/drm-misc -S: Maintained -F: drivers/gpu/drm/tiny/mi0283qt.c -F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt - DRM DRIVER FOR MSM ADRENO GPU M: Rob Clark robdclark@gmail.com M: Sean Paul sean@poorly.run diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index a24953ec2d40..fbb5a723eeb5 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -65,6 +65,7 @@ config DRM_PANEL_ILITEK_ILI9341 help DRM driver for the following Ilitek ILI9341 panels: * YX240QV29-T 2.4" 240x320 TFT (Adafruit 2.4") + * Multi-Inno MI0283QT
If M is selected the module will be called panel-ilitek-ili9341.
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f8df737018d3..f6082fa2a389 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -3,8 +3,6 @@ * DRM driver for Ilitek ILI9341 panels * * Copyright 2018 David Lechner david@lechnology.com - * - * Based on mi0283qt.c: * Copyright 2016 Noralf Trønnes */
@@ -13,7 +11,9 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/pm.h> #include <linux/property.h> +#include <linux/regulator/consumer.h> #include <linux/spi/spi.h>
#include <video/mipi_display.h> @@ -22,6 +22,7 @@ #include <drm/drm_drv.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_mipi_dbi.h> +#include <drm/drm_modeset_helper.h> #include <drm/drm_panel.h>
#define ILI9341_FRMCTR1 0xb1 @@ -57,6 +58,7 @@ struct ili9341_config { struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel; + struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf; }; @@ -174,12 +176,133 @@ static const struct ili9341_config yx240qv29_data = { .mode = &yx240qv29_mode, };
+static int mi0283qt_prepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + struct mipi_dbi_dev *dbidev = &ili->dbidev; + struct mipi_dbi *dbi = &dbidev->dbi; + u8 addr_mode; + int ret; + + if (ili->regulator) + regulator_enable(ili->regulator); + + ret = mipi_dbi_poweron_conditional_reset(dbidev); + if (ret < 0) { + if (ili->regulator) + regulator_disable(ili->regulator); + return ret; + } + if (ret == 1) + goto out_enable; + + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF); + + mipi_dbi_command(dbi, ILI9341_PWCTRLB, 0x00, 0x83, 0x30); + mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81); + mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79); + 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, 0x26); + mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11); + /* VCOM */ + mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e); + mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe); + + /* Memory Access Control */ + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT); + + /* Frame Rate */ + mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b); + + /* Gamma */ + mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08); + mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); + mipi_dbi_command(dbi, ILI9341_PGAMCTRL, + 0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87, + 0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00); + mipi_dbi_command(dbi, ILI9341_NGAMCTRL, + 0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78, + 0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f); + + /* DDRAM */ + mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07); + + /* Display */ + mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00); + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); + msleep(100); + + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON); + msleep(100); + +out_enable: + /* The PiTFT (ili9340) has a hardware reset circuit that + * resets only on power-on and not on each reboot through + * a gpio like the rpi-display does. + * As a result, we need to always apply the rotation value + * regardless of the display "on/off" state. + */ + switch (dbidev->rotation) { + default: + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY | + ILI9341_MADCTL_MX; + break; + case 90: + addr_mode = ILI9341_MADCTL_MY; + break; + case 180: + addr_mode = ILI9341_MADCTL_MV; + break; + case 270: + addr_mode = ILI9341_MADCTL_MX; + break; + } + addr_mode |= ILI9341_MADCTL_BGR; + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); + + return 0; +} + +static int mi0283qt_unprepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + + if (ili->regulator) + regulator_disable(ili->regulator); + + return 0; +} + +static const struct drm_panel_funcs mi0283qt_drm_funcs = { + .prepare = mi0283qt_prepare, + .enable = ili9341_enable, + .disable = ili9341_disable, + .unprepare = mi0283qt_unprepare, +}; + +static const struct drm_display_mode mi0283qt_mode = { + DRM_SIMPLE_MODE(320, 240, 58, 43), +}; + +static const struct ili9341_config mi0283qt_data = { + .funcs = &mi0283qt_drm_funcs, + .mode = &mi0283qt_mode, +}; + +/* Legacy, DRM driver name is ABI */ +DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(mi0283qt, "Multi-Inno MI0283QT", "20160614"); + DEFINE_DRM_MIPI_DBI_PANEL_DRIVER(ili9341, "Ilitek ILI9341", "20180514");
static int ili9341_probe(struct spi_device *spi) { const struct spi_device_id *spi_id; struct device *dev = &spi->dev; + struct drm_driver *driver; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili; @@ -216,6 +339,10 @@ static int ili9341_probe(struct spi_device *spi) return PTR_ERR(dc); }
+ ili->regulator = devm_regulator_get(dev, "power"); + if (IS_ERR(ili->regulator)) + return PTR_ERR(ili->regulator); + ili->backlight = devm_of_find_backlight(dev); if (IS_ERR(ili->backlight)) return PTR_ERR(ili->backlight); @@ -230,7 +357,12 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
- return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, &ili9341_drm_driver, + if (ili->conf == &mi0283qt_data) + driver = &mi0283qt_drm_driver; + else + driver = &ili9341_drm_driver; + + return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, ili->conf->mode, rotation); }
@@ -251,14 +383,36 @@ static void ili9341_shutdown(struct spi_device *spi) drm_atomic_helper_shutdown(&ili->dbidev.drm); }
+static int __maybe_unused ili9341_pm_suspend(struct device *dev) +{ + struct ili9341 *ili = dev_get_drvdata(dev); + + return drm_mode_config_helper_suspend(&ili->dbidev.drm); +} + +static int __maybe_unused ili9341_pm_resume(struct device *dev) +{ + struct ili9341 *ili = dev_get_drvdata(dev); + + drm_mode_config_helper_resume(&ili->dbidev.drm); + + return 0; +} + +static const struct dev_pm_ops ili9341_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(ili9341_pm_suspend, ili9341_pm_resume) +}; + static const struct of_device_id ili9341_of_match[] = { { .compatible = "adafruit,yx240qv29", .data = &yx240qv29_data }, + { .compatible = "multi-inno,mi0283qt", .data = &mi0283qt_data }, { } }; MODULE_DEVICE_TABLE(of, ili9341_of_match);
static const struct spi_device_id ili9341_id[] = { { "yx240qv29", (unsigned long)&yx240qv29_data }, + { "mi0283qt", (unsigned long)&mi0283qt_data }, { } }; MODULE_DEVICE_TABLE(spi, ili9341_id); @@ -267,6 +421,7 @@ static struct spi_driver ili9341_spi_driver = { .driver = { .name = "panel-ilitek-ili9341", .of_match_table = ili9341_of_match, + .pm = &ili9341_pm_ops, }, .id_table = ili9341_id, .probe = ili9341_probe, diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 3e63e93fbeac..4fae3e50147d 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -34,17 +34,6 @@ config TINYDRM_ILI9225
If M is selected the module will be called ili9225.
-config TINYDRM_MI0283QT - tristate "DRM support for MI0283QT" - depends on DRM && SPI - select DRM_KMS_HELPER - select DRM_KMS_CMA_HELPER - select DRM_MIPI_DBI - select BACKLIGHT_CLASS_DEVICE - help - DRM driver for the Multi-Inno MI0283QT display panel - If M is selected the module will be called mi0283qt. - config TINYDRM_REPAPER tristate "DRM support for Pervasive Displays RePaper panels (V231)" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index c140962f5c7e..05029c4b1916 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -3,7 +3,6 @@ obj-$(CONFIG_DRM_GM12U320) += gm12u320.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o -obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o obj-$(CONFIG_TINYDRM_ST7586) += st7586.o obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c deleted file mode 100644 index e2cfd9a17143..000000000000 --- a/drivers/gpu/drm/tiny/mi0283qt.c +++ /dev/null @@ -1,294 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * DRM driver for Multi-Inno MI0283QT panels - * - * Copyright 2016 Noralf Trønnes - */ - -#include <linux/backlight.h> -#include <linux/delay.h> -#include <linux/gpio/consumer.h> -#include <linux/module.h> -#include <linux/property.h> -#include <linux/regulator/consumer.h> -#include <linux/spi/spi.h> - -#include <drm/drm_atomic_helper.h> -#include <drm/drm_drv.h> -#include <drm/drm_fb_helper.h> -#include <drm/drm_gem_cma_helper.h> -#include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_mipi_dbi.h> -#include <drm/drm_modeset_helper.h> -#include <video/mipi_display.h> - -#define ILI9341_FRMCTR1 0xb1 -#define ILI9341_DISCTRL 0xb6 -#define ILI9341_ETMOD 0xb7 - -#define ILI9341_PWCTRL1 0xc0 -#define ILI9341_PWCTRL2 0xc1 -#define ILI9341_VMCTRL1 0xc5 -#define ILI9341_VMCTRL2 0xc7 -#define ILI9341_PWCTRLA 0xcb -#define ILI9341_PWCTRLB 0xcf - -#define ILI9341_PGAMCTRL 0xe0 -#define ILI9341_NGAMCTRL 0xe1 -#define ILI9341_DTCTRLA 0xe8 -#define ILI9341_DTCTRLB 0xea -#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 mi0283qt_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 = &dbidev->dbi; - u8 addr_mode; - int ret, idx; - - if (!drm_dev_enter(pipe->crtc.dev, &idx)) - 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, 0x83, 0x30); - mipi_dbi_command(dbi, ILI9341_PWRSEQ, 0x64, 0x03, 0x12, 0x81); - mipi_dbi_command(dbi, ILI9341_DTCTRLA, 0x85, 0x01, 0x79); - 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, 0x26); - mipi_dbi_command(dbi, ILI9341_PWCTRL2, 0x11); - /* VCOM */ - mipi_dbi_command(dbi, ILI9341_VMCTRL1, 0x35, 0x3e); - mipi_dbi_command(dbi, ILI9341_VMCTRL2, 0xbe); - - /* Memory Access Control */ - mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, MIPI_DCS_PIXEL_FMT_16BIT); - - /* Frame Rate */ - mipi_dbi_command(dbi, ILI9341_FRMCTR1, 0x00, 0x1b); - - /* Gamma */ - mipi_dbi_command(dbi, ILI9341_EN3GAM, 0x08); - mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01); - mipi_dbi_command(dbi, ILI9341_PGAMCTRL, - 0x1f, 0x1a, 0x18, 0x0a, 0x0f, 0x06, 0x45, 0x87, - 0x32, 0x0a, 0x07, 0x02, 0x07, 0x05, 0x00); - mipi_dbi_command(dbi, ILI9341_NGAMCTRL, - 0x00, 0x25, 0x27, 0x05, 0x10, 0x09, 0x3a, 0x78, - 0x4d, 0x05, 0x18, 0x0d, 0x38, 0x3a, 0x1f); - - /* DDRAM */ - mipi_dbi_command(dbi, ILI9341_ETMOD, 0x07); - - /* Display */ - mipi_dbi_command(dbi, ILI9341_DISCTRL, 0x0a, 0x82, 0x27, 0x00); - mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); - msleep(100); - - mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON); - msleep(100); - -out_enable: - /* The PiTFT (ili9340) has a hardware reset circuit that - * resets only on power-on and not on each reboot through - * a gpio like the rpi-display does. - * As a result, we need to always apply the rotation value - * regardless of the display "on/off" state. - */ - switch (dbidev->rotation) { - default: - addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY | - ILI9341_MADCTL_MX; - break; - case 90: - addr_mode = ILI9341_MADCTL_MY; - break; - case 180: - addr_mode = ILI9341_MADCTL_MV; - break; - case 270: - addr_mode = ILI9341_MADCTL_MX; - break; - } - addr_mode |= ILI9341_MADCTL_BGR; - mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode); - mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); -out_exit: - drm_dev_exit(idx); -} - -static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = { - .enable = mi0283qt_enable, - .disable = mipi_dbi_pipe_disable, - .update = mipi_dbi_pipe_update, - .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, -}; - -static const struct drm_display_mode mi0283qt_mode = { - DRM_SIMPLE_MODE(320, 240, 58, 43), -}; - -DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops); - -static struct drm_driver mi0283qt_driver = { - .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, - .fops = &mi0283qt_fops, - .release = mipi_dbi_release, - DRM_GEM_CMA_VMAP_DRIVER_OPS, - .debugfs_init = mipi_dbi_debugfs_init, - .name = "mi0283qt", - .desc = "Multi-Inno MI0283QT", - .date = "20160614", - .major = 1, - .minor = 0, -}; - -static const struct of_device_id mi0283qt_of_match[] = { - { .compatible = "multi-inno,mi0283qt" }, - {}, -}; -MODULE_DEVICE_TABLE(of, mi0283qt_of_match); - -static const struct spi_device_id mi0283qt_id[] = { - { "mi0283qt", 0 }, - { }, -}; -MODULE_DEVICE_TABLE(spi, mi0283qt_id); - -static int mi0283qt_probe(struct spi_device *spi) -{ - struct device *dev = &spi->dev; - struct mipi_dbi_dev *dbidev; - struct drm_device *drm; - struct mipi_dbi *dbi; - struct gpio_desc *dc; - u32 rotation = 0; - int ret; - - dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL); - if (!dbidev) - return -ENOMEM; - - dbi = &dbidev->dbi; - drm = &dbidev->drm; - ret = devm_drm_dev_init(dev, drm, &mi0283qt_driver); - if (ret) { - kfree(dbidev); - return ret; - } - - drm_mode_config_init(drm); - - dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(dbi->reset)) { - DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n"); - return PTR_ERR(dbi->reset); - } - - dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); - if (IS_ERR(dc)) { - DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n"); - return PTR_ERR(dc); - } - - dbidev->regulator = devm_regulator_get(dev, "power"); - if (IS_ERR(dbidev->regulator)) - return PTR_ERR(dbidev->regulator); - - dbidev->backlight = devm_of_find_backlight(dev); - if (IS_ERR(dbidev->backlight)) - return PTR_ERR(dbidev->backlight); - - device_property_read_u32(dev, "rotation", &rotation); - - ret = mipi_dbi_spi_init(spi, dbi, dc); - if (ret) - return ret; - - ret = mipi_dbi_dev_init(dbidev, &mi0283qt_pipe_funcs, &mi0283qt_mode, rotation); - if (ret) - return ret; - - drm_mode_config_reset(drm); - - ret = drm_dev_register(drm, 0); - if (ret) - return ret; - - spi_set_drvdata(spi, drm); - - drm_fbdev_generic_setup(drm, 0); - - return 0; -} - -static int mi0283qt_remove(struct spi_device *spi) -{ - struct drm_device *drm = spi_get_drvdata(spi); - - drm_dev_unplug(drm); - drm_atomic_helper_shutdown(drm); - - return 0; -} - -static void mi0283qt_shutdown(struct spi_device *spi) -{ - drm_atomic_helper_shutdown(spi_get_drvdata(spi)); -} - -static int __maybe_unused mi0283qt_pm_suspend(struct device *dev) -{ - return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); -} - -static int __maybe_unused mi0283qt_pm_resume(struct device *dev) -{ - drm_mode_config_helper_resume(dev_get_drvdata(dev)); - - return 0; -} - -static const struct dev_pm_ops mi0283qt_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(mi0283qt_pm_suspend, mi0283qt_pm_resume) -}; - -static struct spi_driver mi0283qt_spi_driver = { - .driver = { - .name = "mi0283qt", - .owner = THIS_MODULE, - .of_match_table = mi0283qt_of_match, - .pm = &mi0283qt_pm_ops, - }, - .id_table = mi0283qt_id, - .probe = mi0283qt_probe, - .remove = mi0283qt_remove, - .shutdown = mi0283qt_shutdown, -}; -module_spi_driver(mi0283qt_spi_driver); - -MODULE_DESCRIPTION("Multi-Inno MI0283QT DRM driver"); -MODULE_AUTHOR("Noralf Trønnes"); -MODULE_LICENSE("GPL");
On 8/1/19 8:52 AM, Noralf Trønnes wrote:
The MI0283QT panels use a ILI9341 controller so it makes sense to merge it with the other ili9341 code.
The DRM driver name is ABI, so that is retained.
Cc: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
...
@@ -216,6 +339,10 @@ static int ili9341_probe(struct spi_device *spi) return PTR_ERR(dc); }
- ili->regulator = devm_regulator_get(dev, "power");
- if (IS_ERR(ili->regulator))
return PTR_ERR(ili->regulator);- ili->backlight = devm_of_find_backlight(dev); if (IS_ERR(ili->backlight)) return PTR_ERR(ili->backlight);
@@ -230,7 +357,12 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
Should probably add a comment here that this is for backwards compatibility of the driver name so that no one is tempted to to add more driver structs when adding new panels.
- return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, &ili9341_drm_driver,
- if (ili->conf == &mi0283qt_data)
driver = &mi0283qt_drm_driver;- else
driver = &ili9341_drm_driver;- return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, ili->conf->mode, rotation); }
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode; + bool no_dpi; };
struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel; + bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf; @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode, + .no_dpi = true, };
static int mi0283qt_prepare(struct drm_panel *panel) @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode, + .no_dpi = true, };
/* Legacy, DRM driver name is ABI */ @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver; + struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili; @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
- if (ili->conf == &mi0283qt_data) - driver = &mi0283qt_drm_driver; - else - driver = &ili9341_drm_driver;
- return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, - ili->conf->mode, rotation); + port = of_get_child_by_name(dev->of_node, "port"); + if (port) { + of_node_put(port); + ili->use_dpi = true; + } + + if (ili->conf->no_dpi) + ili->use_dpi = false; + + if (ili->use_dpi) { + ret = drm_panel_add(&ili->panel); + } else { + if (ili->conf == &mi0283qt_data) + driver = &mi0283qt_drm_driver; + else + driver = &ili9341_drm_driver; + + ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, + ili->conf->mode, rotation); + } + + return ret; }
static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(&ili->dbidev.drm); - drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (ili->use_dpi) { + drm_panel_remove(&ili->panel); + drm_panel_disable(&ili->panel); + drm_panel_unprepare(&ili->panel); + kfree(ili); + } else { + drm_dev_unplug(&ili->dbidev.drm); + drm_atomic_helper_shutdown(&ili->dbidev.drm); + }
return 0; } @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_atomic_helper_shutdown(&ili->dbidev.drm); }
static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- return drm_mode_config_helper_suspend(&ili->dbidev.drm); + if (!ili->use_dpi) + return drm_mode_config_helper_suspend(&ili->dbidev.drm); + + return 0; }
static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- drm_mode_config_helper_resume(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_mode_config_helper_resume(&ili->dbidev.drm);
return 0; }
On 8/1/19 8:52 AM, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode;
- bool no_dpi;
Can we invert this to use positive logic? i.e. use_dpi instead of no_dpi.
};
struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel;
- bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf;
@@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode,
.no_dpi = true, };
static int mi0283qt_prepare(struct drm_panel *panel)
@@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode,
.no_dpi = true, };
/* Legacy, DRM driver name is ABI */
@@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver;
- struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili;
@@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
if (ili->conf == &mi0283qt_data)
driver = &mi0283qt_drm_driver;else
driver = &ili9341_drm_driver;return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,
ili->conf->mode, rotation);
- port = of_get_child_by_name(dev->of_node, "port");
- if (port) {
of_node_put(port);ili->use_dpi = true;- }
- if (ili->conf->no_dpi)
ili->use_dpi = false;
then this can just be ili->use_dpi = ili->conf->use_dpi.
if (ili->use_dpi) {
ret = drm_panel_add(&ili->panel);} else {
if (ili->conf == &mi0283qt_data)driver = &mi0283qt_drm_driver;elsedriver = &ili9341_drm_driver;ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,ili->conf->mode, rotation);}
return ret; }
static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(&ili->dbidev.drm);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
if (ili->use_dpi) {
drm_panel_remove(&ili->panel);drm_panel_disable(&ili->panel);drm_panel_unprepare(&ili->panel);kfree(ili);} else {
drm_dev_unplug(&ili->dbidev.drm);drm_atomic_helper_shutdown(&ili->dbidev.drm);}
return 0; }
@@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
if (!ili->use_dpi)
drm_atomic_helper_shutdown(&ili->dbidev.drm);}
static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- return drm_mode_config_helper_suspend(&ili->dbidev.drm);
if (!ili->use_dpi)
return drm_mode_config_helper_suspend(&ili->dbidev.drm);return 0; }
static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- drm_mode_config_helper_resume(&ili->dbidev.drm);
if (!ili->use_dpi)
drm_mode_config_helper_resume(&ili->dbidev.drm);return 0; }
Den 01.08.2019 21.10, skrev David Lechner:
On 8/1/19 8:52 AM, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode; + bool no_dpi;
Can we invert this to use positive logic? i.e. use_dpi instead of no_dpi.
This is a capability flag. I could call it has_dpi and add a comment about what it does.
Noralf.
}; struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel; + bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf; @@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode, + .no_dpi = true, }; static int mi0283qt_prepare(struct drm_panel *panel) @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode, + .no_dpi = true, }; /* Legacy, DRM driver name is ABI */ @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver; + struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili; @@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs; - if (ili->conf == &mi0283qt_data) - driver = &mi0283qt_drm_driver; - else - driver = &ili9341_drm_driver; - return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, - ili->conf->mode, rotation); + port = of_get_child_by_name(dev->of_node, "port"); + if (port) { + of_node_put(port); + ili->use_dpi = true; + }
+ if (ili->conf->no_dpi) + ili->use_dpi = false;
then this can just be ili->use_dpi = ili->conf->use_dpi.
+ if (ili->use_dpi) { + ret = drm_panel_add(&ili->panel); + } else { + if (ili->conf == &mi0283qt_data) + driver = &mi0283qt_drm_driver; + else + driver = &ili9341_drm_driver;
+ ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver, + ili->conf->mode, rotation); + }
+ return ret; } static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi); - drm_dev_unplug(&ili->dbidev.drm); - drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (ili->use_dpi) { + drm_panel_remove(&ili->panel); + drm_panel_disable(&ili->panel); + drm_panel_unprepare(&ili->panel); + kfree(ili); + } else { + drm_dev_unplug(&ili->dbidev.drm); + drm_atomic_helper_shutdown(&ili->dbidev.drm); + } return 0; } @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi); - drm_atomic_helper_shutdown(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_atomic_helper_shutdown(&ili->dbidev.drm); } static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev); - return drm_mode_config_helper_suspend(&ili->dbidev.drm); + if (!ili->use_dpi) + return drm_mode_config_helper_suspend(&ili->dbidev.drm);
+ return 0; } static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev); - drm_mode_config_helper_resume(&ili->dbidev.drm); + if (!ili->use_dpi) + drm_mode_config_helper_resume(&ili->dbidev.drm); return 0; }
Hi Noralf.
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Have you consiered if the compatible could be used to determine the use of a panel? Then it is more explicit how the HW is described in DT.
Sam
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode;
- bool no_dpi;
};
struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel;
- bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf;
@@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode,
- .no_dpi = true,
};
static int mi0283qt_prepare(struct drm_panel *panel) @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode,
- .no_dpi = true,
};
/* Legacy, DRM driver name is ABI */ @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver;
- struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili;
@@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
if (ili->conf == &mi0283qt_data)
driver = &mi0283qt_drm_driver;else
driver = &ili9341_drm_driver;return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,
ili->conf->mode, rotation);
- port = of_get_child_by_name(dev->of_node, "port");
- if (port) {
of_node_put(port);ili->use_dpi = true;- }
- if (ili->conf->no_dpi)
ili->use_dpi = false;- if (ili->use_dpi) {
ret = drm_panel_add(&ili->panel);- } else {
if (ili->conf == &mi0283qt_data)driver = &mi0283qt_drm_driver;elsedriver = &ili9341_drm_driver;ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,ili->conf->mode, rotation);- }
- return ret;
}
static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(&ili->dbidev.drm);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
if (ili->use_dpi) {
drm_panel_remove(&ili->panel);drm_panel_disable(&ili->panel);drm_panel_unprepare(&ili->panel);kfree(ili);} else {
drm_dev_unplug(&ili->dbidev.drm);drm_atomic_helper_shutdown(&ili->dbidev.drm);}
return 0;
} @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
- if (!ili->use_dpi)
drm_atomic_helper_shutdown(&ili->dbidev.drm);}
static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- return drm_mode_config_helper_suspend(&ili->dbidev.drm);
- if (!ili->use_dpi)
return drm_mode_config_helper_suspend(&ili->dbidev.drm);- return 0;
}
static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- drm_mode_config_helper_resume(&ili->dbidev.drm);
if (!ili->use_dpi)
drm_mode_config_helper_resume(&ili->dbidev.drm);return 0;
}
2.20.1
Den 11.08.2019 18.41, skrev Sam Ravnborg:
Hi Noralf.
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Have you consiered if the compatible could be used to determine the use of a panel? Then it is more explicit how the HW is described in DT.
Is that common to use the compatible to tell which interface to use? I don't know what best practice is here.
Noralf.
On Mon, Aug 12, 2019 at 02:13:54PM +0200, Noralf Trønnes wrote:
Den 11.08.2019 18.41, skrev Sam Ravnborg:
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Have you consiered if the compatible could be used to determine the use of a panel? Then it is more explicit how the HW is described in DT.
Is that common to use the compatible to tell which interface to use? I don't know what best practice is here.
Usually the compatible identifies the device, not the interface. Additional properties are preferred to describe the interface.
Hi Laurent/Noralf.
On Mon, Aug 12, 2019 at 06:35:42PM +0300, Laurent Pinchart wrote:
On Mon, Aug 12, 2019 at 02:13:54PM +0200, Noralf Trønnes wrote:
Den 11.08.2019 18.41, skrev Sam Ravnborg:
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Have you consiered if the compatible could be used to determine the use of a panel? Then it is more explicit how the HW is described in DT.
Is that common to use the compatible to tell which interface to use? I don't know what best practice is here.
Usually the compatible identifies the device, not the interface. Additional properties are preferred to describe the interface.
Thanks Laurent. Then the ports idea as implmented by the patch seems to fly.
Sam
Hi Noralf.
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c index f6082fa2a389..7cbfd739c7fd 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -11,6 +11,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> @@ -53,11 +54,13 @@ struct ili9341_config { const struct drm_panel_funcs *funcs; const struct drm_display_mode *mode;
- bool no_dpi;
};
struct ili9341 { struct mipi_dbi_dev dbidev; /* This must be the first entry */ struct drm_panel panel;
- bool use_dpi; struct regulator *regulator; struct backlight_device *backlight; const struct ili9341_config *conf;
@@ -174,6 +177,7 @@ static const struct drm_display_mode yx240qv29_mode = { static const struct ili9341_config yx240qv29_data = { .funcs = &yx240qv29_funcs, .mode = &yx240qv29_mode,
- .no_dpi = true,
};
static int mi0283qt_prepare(struct drm_panel *panel) @@ -291,6 +295,7 @@ static const struct drm_display_mode mi0283qt_mode = { static const struct ili9341_config mi0283qt_data = { .funcs = &mi0283qt_drm_funcs, .mode = &mi0283qt_mode,
- .no_dpi = true,
};
/* Legacy, DRM driver name is ABI */ @@ -303,6 +308,7 @@ static int ili9341_probe(struct spi_device *spi) const struct spi_device_id *spi_id; struct device *dev = &spi->dev; struct drm_driver *driver;
- struct device_node *port; struct mipi_dbi *dbi; struct gpio_desc *dc; struct ili9341 *ili;
@@ -357,21 +363,44 @@ static int ili9341_probe(struct spi_device *spi) ili->panel.dev = dev; ili->panel.funcs = ili->conf->funcs;
if (ili->conf == &mi0283qt_data)
driver = &mi0283qt_drm_driver;else
driver = &ili9341_drm_driver;return drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,
ili->conf->mode, rotation);
- port = of_get_child_by_name(dev->of_node, "port");
- if (port) {
of_node_put(port);ili->use_dpi = true;- }
- if (ili->conf->no_dpi)
ili->use_dpi = false;- if (ili->use_dpi) {
ret = drm_panel_add(&ili->panel);- } else {
if (ili->conf == &mi0283qt_data)driver = &mi0283qt_drm_driver;elsedriver = &ili9341_drm_driver;ret = drm_mipi_dbi_panel_register(&ili->panel, &ili->dbidev, driver,ili->conf->mode, rotation);- }
- return ret;
}
static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(&ili->dbidev.drm);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
- if (ili->use_dpi) {
drm_panel_remove(&ili->panel);drm_panel_disable(&ili->panel);drm_panel_unprepare(&ili->panel);kfree(ili);
At first I thought - order is wrong. But drm_panel_remove() prevents display drivers from using the driver. And this will not invalidate the other calls. Maybe add a short comment?
Sam
} else {
drm_dev_unplug(&ili->dbidev.drm);drm_atomic_helper_shutdown(&ili->dbidev.drm);}
return 0;
} @@ -380,21 +409,26 @@ static void ili9341_shutdown(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
- if (!ili->use_dpi)
drm_atomic_helper_shutdown(&ili->dbidev.drm);}
static int __maybe_unused ili9341_pm_suspend(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- return drm_mode_config_helper_suspend(&ili->dbidev.drm);
- if (!ili->use_dpi)
return drm_mode_config_helper_suspend(&ili->dbidev.drm);- return 0;
}
static int __maybe_unused ili9341_pm_resume(struct device *dev) { struct ili9341 *ili = dev_get_drvdata(dev);
- drm_mode_config_helper_resume(&ili->dbidev.drm);
if (!ili->use_dpi)
drm_mode_config_helper_resume(&ili->dbidev.drm);return 0;
}
2.20.1
Den 11.08.2019 19.02, skrev Sam Ravnborg:
Hi Noralf.
On Thu, Aug 01, 2019 at 03:52:49PM +0200, Noralf Trønnes wrote:
Add support for panels that use the DPI interface. ILI9341 has onboard RAM so the assumption made here is that all such panels support pixel upload over DBI.
The presence/absense of the Device Tree 'port' node decides which interface is used for pixel transfer.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 56 ++++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
<snip>
static int ili9341_remove(struct spi_device *spi) { struct ili9341 *ili = spi_get_drvdata(spi);
- drm_dev_unplug(&ili->dbidev.drm);
- drm_atomic_helper_shutdown(&ili->dbidev.drm);
- if (ili->use_dpi) {
drm_panel_remove(&ili->panel);drm_panel_disable(&ili->panel);drm_panel_unprepare(&ili->panel);kfree(ili);At first I thought - order is wrong. But drm_panel_remove() prevents display drivers from using the driver. And this will not invalidate the other calls. Maybe add a short comment?
I just copied this code from Josef's driver, didn't actually look that close at it. Isn't there a common pattern for this in the panel drivers? I would assume that everyone would have to do more or less the same on driver unbind.
Noralf.
Sam
} else {
drm_dev_unplug(&ili->dbidev.drm);drm_atomic_helper_shutdown(&ili->dbidev.drm);}
return 0;
}
dri-devel@lists.freedesktop.org
-
David Lechner -
Laurent Pinchart -
Noralf Trønnes -
Sam Ravnborg