[PATCH] drm/panel: Add driver for Seiko 43WVF1G panel

[Fabio Estevam]

Hi Rob,

On Wed, Jun 14, 2017 at 2:49 PM, Thierry Reding
<thierry.reding at 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 at 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