Hi Rob, On Wed, Jun 14, 2017 at 2:49 PM, Thierry Reding <thierry.reding@xxxxxxxxx> 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@xxxxxxx> >> --- >> .../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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
