Re: [PATCH v3 5/5] drm/panel: Add ilitek ili9341 driver

[dillon min <dillon.minfei@gmail.com>]

Hi Linus,

Linus Walleij <linus.walleij@linaro.org> 于2020年5月14日周四 下午10:08写道：
>
> On Thu, May 14, 2020 at 12:22 PM dillon min <dillon.minfei@gmail.com> wrote:
>
> > > > +       /* Gamma */
> > > > +       mipi_dbi_command(dbi, ILI9341_3GAMMA_EN, 0x00);
> > > > +       mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> > > > +       mipi_dbi_command(dbi, ILI9341_PGAMMA,
> > > > +                        0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> > > > +                        0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> > > > +       mipi_dbi_command(dbi, ILI9341_NGAMMA,
> > > > +                        0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> > > > +                        0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
> > >
> > > It seems to be copies of the stuff above, but why is there a different
> > > gamma if you use DBI?
>
> > for dbi interface, currently i just copy the code from tiny/ili9341.c.
> > as so many boards use this driver now, like raspberry pi, etc
> > i'm afraid it's will not work after modification. so, just leave the
> > original code there.
>
> OK if you move it to ili9341_config it will be clear which panels
> need this gamma and which panels need another gamma.
>
> I think there should be one ili9341_config for the new st,*
> variant and one for the old DBI variant.
>
Okay, it's a good idea to setup two different configs.  ili9341_dbi_config,
ili9341_dpi_config. first for old dbi variant, second for dpi variant.

> > anther question:
> > is there any panel driver have dbi & dpi or dpi & dsi supported? which
> > i mean support
> > two different panel interface in one driver.
> > thanks
>
> Usually you split the driver in three files becuase a driver can
> only list one initcall, and also it makes it modularized.
>
> There is nothing in-tree but look at my branch here:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/log/?h=ux500-skomer-v5.7-rc1
>
> You see how I split up the s6e63m0 driver in one SPI part
> and one DSI part:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/commit/?h=ux500-skomer-v5.7-rc1&id=6c0e1fb5df5fa8fa857dee65f6c0f54d06d158a7
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git/commit/?h=ux500-skomer-v5.7-rc1&id=cfbf562cc709b53b62f5fbc7fedf176ffa17b088
>
> The overall idea should work the same with DBI.
>
> Yours,
> Linus Walleij
>
> Yours,
> Linus Walleij

Thanks for share, but i thinks it's not suitable to my case. the difference is
for panel s6e63m0
1 spi only for panel register configuration
2 dpi for rgb video data transfer (drm_panel_init(..., DRM_MODE_CONNECTOR_DPI);)

my case:
1 support ili9341 by drm mipi dbi (only spi bus,  "3/4 line serial
interface", pdf chap 7.6.1), Ie, raspberry pi + ili9341 + spi
2 support ili9341 by spi & dpi (spi for register set, dpi for rgb
data, "6/16/18 bit parallel rgb interface", pdf chap 7.6.8)
   Ie, stm32f429+ili9341+ltdc+spi
these two communication type has no dependency with each other.

DBI has much more difference than DPI & SPI, or DSI & SPI in drm.
   drm_mipi_dbi support both panel register configuration and video
data transfer via spi
   with or without dc pins.
   also the registration to drm is difference, for dbi is
mipi_dbi_dev_init and drm_fbdev_generic_setup
   for panel driver is drm_panel_init and drm_panel_add
at soc's view, we can drive ili9341 to work just by drm mipi dbi
(actually it's dbi -> spi bus) without host's lcd or dsi controller
but for some panel can't transfer video data by spi, and with dpi or
dsi interface, must connect to host's lcd or dsi controller.

so, for ilitek-ili9341 , it's use different dts binding drive to
register different client type (dbi, or dpi & spi) to drm.
actually, there is a driver tiny/ili9341.c support this panel by spi
bus only already. but, from Sam's suggestion, should extend this
driver
to support dpi & spi interface, this is the background of panel-ilitek-ili9341.c

thanks.

best regards.

dillon