Hi Rob,
On Wed, Jun 14, 2017 at 2:49 PM, Thierry Reding thierry.reding@gmail.com wrote:
On Tue, Apr 25, 2017 at 01:18:35PM -0300, Marco Franchi wrote:
Add driver for Seiko Instruments Inc. 4.3" WVGA (800 x RGB x 480) TFT with Touch-Panel.
Datasheet available at: http://www.glyn.de/data/glyn/media/doc/43wvf1g-0.pdf
Seiko 43WVF1G panel has two power supplies: AVDD and DVDD and they require a specific power on/down sequence. For this reason the simple panel driver cannot be used to drive this panel, so create a new one heavily based on simple panel.
Based on initial patch submission from Breno Lima.
Signed-off-by: Marco Franchi marco.franchi@nxp.com
.../bindings/display/panel/seiko,43wvf1g.txt | 23 ++ drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 372 +++++++++++++++++++++ 4 files changed, 405 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt create mode 100644 drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
diff --git a/Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt b/Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt new file mode 100644 index 0000000..cd1eeda --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/seiko,43wvf1g.txt @@ -0,0 +1,23 @@ +Seiko Instruments Inc. 4.3" WVGA (800 x RGB x 480) TFT with Touch-Panel
+Required properties: +- compatible: should be "sii,43wvf1g". +- "DVDD-supply": 3v3 digital regulator. +- "AVDD-supply": 5v analog regulator.
I don't think we should be using all-uppercase for supply names. So the above should be "dvdd-supply" and "avdd-supply". No need to resend only for that change, I can fix it up when applying.
Rob, can I have your Acked-by with the above change?
Any comment, please? It would be nice if we can get this into 4.13, if possible.
Just two minor comments below.
diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c
[...]
+struct seiko_panel_desc {
const struct drm_display_mode *modes;unsigned int num_modes;const struct display_timing *timings;unsigned int num_timings;unsigned int bpc;/*** @width: width (in millimeters) of the panel's active display area* @height: height (in millimeters) of the panel's active display area*/struct {unsigned int width;unsigned int height;} size;u32 bus_format;u32 bus_flags;+};
It's somewhat unfortunate how this has to duplicate a lot of the panel-simple driver just because it uses two regulators. However, with some of the work going on to make panel-simple code more reusable this might improve in the future. For now I think this doesn't hurt.
+struct seiko_panel {
struct drm_panel base;bool prepared;bool enabled;const struct seiko_panel_desc *desc;struct backlight_device *backlight;struct regulator *DVDD;struct regulator *AVDD;+};
Same as for the binding: we don't use all-uppercase names for variables, so I'll fix those up to be lowercase when applying.
Thierry