Re: [PATCH v3 2/2] drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel

[Sean Paul]

On Tue, Dec 18, 2018 at 05:06:38PM +0530, Jagan Teki wrote:
> On Sat, Dec 15, 2018 at 3:32 AM Sean Paul <sean@xxxxxxxxxx> wrote:
> >
> > On Sat, Dec 15, 2018 at 02:11:01AM +0530, Jagan Teki wrote:
> > > Feiyang FY07024DI26A30-D is 1024x600, 4-lane MIPI-DSI LCD panel.
> > >
> > > Add panel driver for it.
> > >
> > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > > ---
> > > Changes for v2:
> > > - use simple structure for command init
> > > - update proper comments on power, reset delay sequnce
> > > - fix to use set_display_off in disable function
> > > - move mode type to structure
> > > - drop refres rate value, let drm compute
> > >
> > >  MAINTAINERS                                   |   6 +
> > >  drivers/gpu/drm/panel/Kconfig                 |   9 +
> > >  drivers/gpu/drm/panel/Makefile                |   1 +
> > >  .../drm/panel/panel-feiyang-fy07024di26a30d.c | 296 ++++++++++++++++++
> > >  4 files changed, 312 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index d2928a4d2847..e643238855ea 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -4732,6 +4732,12 @@ T:     git git://anongit.freedesktop.org/drm/drm-misc
> > >  S:   Maintained
> > >  F:   drivers/gpu/drm/tve200/
> > >
> > > +DRM DRIVER FOR FEIYANG FY07024DI26A30-D MIPI-DSI LCD PANELS
> > > +M:   Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > > +S:   Maintained
> > > +F:   drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > > +F:   Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.txt
> > > +
> > >  DRM DRIVER FOR ILITEK ILI9225 PANELS
> > >  M:   David Lechner <david@xxxxxxxxxxxxxx>
> > >  S:   Maintained
> >
> > As I mentioned on IRC, this will be drm-misc maintained with the other panels,
> > no need to have a dedicated MAINTAINERS entry. You'll get pulled from
> > get_maintainers.pl as a majority commit signer
> 
> This I missed it on IRC, sorry will drop.
> 
> >
> > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > > index d93b2ba709bc..cb8ca93550cf 100644
> > > --- a/drivers/gpu/drm/panel/Kconfig
> > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > @@ -38,6 +38,15 @@ config DRM_PANEL_SIMPLE
> > >         that it can be automatically turned off when the panel goes into a
> > >         low power state.
> > >
> > > +config DRM_PANEL_FEIYANG_FY07024DI26A30D
> > > +     tristate "Feiyang FY07024DI26A30-D MIPI-DSI LCD panel"
> > > +     depends on OF
> > > +     depends on DRM_MIPI_DSI
> > > +     depends on BACKLIGHT_CLASS_DEVICE
> > > +     help
> > > +       Say Y if you want to enable support for panels based on the
> > > +       Feiyang FY07024DI26A30-D MIPI-DSI interface.
> > > +
> > >  config DRM_PANEL_ILITEK_IL9322
> > >       tristate "Ilitek ILI9322 320x240 QVGA panels"
> > >       depends on OF && SPI
> > > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > > index 6a9b4cec1891..0fa0ef69e109 100644
> > > --- a/drivers/gpu/drm/panel/Makefile
> > > +++ b/drivers/gpu/drm/panel/Makefile
> > > @@ -2,6 +2,7 @@
> > >  obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o
> > >  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_ILI9881C) += panel-ilitek-ili9881c.o
> > >  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
> > > diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > > new file mode 100644
> > > index 000000000000..4abccbf62c3c
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> > > @@ -0,0 +1,296 @@

/snip

> > > +static const struct drm_display_mode feiyang_default_mode = {
> > > +     .clock = 55000,
> > > +
> > > +     .hdisplay = 1024,
> > > +     .hsync_start = 1024 + 396,
> > > +     .hsync_end = 1024 + 396 + 20,
> > > +     .htotal = 1024 + 396 + 20 + 100,
> > > +
> > > +     .vdisplay = 600,
> > > +     .vsync_start = 600 + 12,
> > > +     .vsync_end = 600 + 12 + 2,
> > > +     .vtotal = 600 + 12 + 2 + 21,
> >
> > These timings are still incorrect, they'll give a 56.2Hz refresh rate. Is that
> > really what you want?
> 
> I would like to go with same rate as of now since BSP is using the
> same, since I don't have any information from chip vendor(even I wrote
> it and waiting for response). I will update accordingly once I get the
> information from vendor.
> 
> Let me know your inputs.

Not totally on board with this, tbh. It seems wrong, so I'm not sure why we'd
apply code that seems wrong. We're not in a rush, we've already cut 4.21, so I
think we should just wait for vendor confirmation on these values.

Sean

-- 
Sean Paul, Software Engineer, Google / Chromium OS