2017-12-01 14:03:34

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels

On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <[email protected]> wrote:

> This adds a new driver for display panels based on the Ilitek ILI9225
> controller.
>
> This was developed for a no-name panel with a red PCB that is commonly
> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>
> I really did try very hard to find a make and model for this panel, but
> there doesn't seem to be one, so the best I can do is offer the picture
> in the link above for identification.
>
> Signed-off-by: David Lechner <[email protected]>

Can you explain why tinydrm is not putting its panel drivers in
drivers/gpu/drm/panel?

I guess everybody knows except me, it's usually like that :(

I am anyways working on a driver for Ilitek 9322 that I want
to land in drivers/gpu/drm/panel. Here is the last iteration:
https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
Yeah I got sidetracked. OK I will get to it now.

There are some similarities with the code I'm seeing here
but I believe they are essentially different. But it will be hard
to share code if you put the driver in the tinydrm framework.

I guess you have also seen:
drivers/video/backlight/ili922x.c
?

Stefano Babic who wrote the backlight driver is available for
reviewing, so includ him in follow-ups (added to To: line).

I'm putting you on CC as I'm rewriting it a bit after the DT
maintainers review, will try to repost ASAP.

Yours,
Linus Walleij


2017-12-01 18:27:45

by Noralf Trønnes

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels

(cc: Thierry)

Den 01.12.2017 15.03, skrev Linus Walleij:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <[email protected]> wrote:
>
>> This adds a new driver for display panels based on the Ilitek ILI9225
>> controller.
>>
>> This was developed for a no-name panel with a red PCB that is commonly
>> marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
>>
>> I really did try very hard to find a make and model for this panel, but
>> there doesn't seem to be one, so the best I can do is offer the picture
>> in the link above for identification.
>>
>> Signed-off-by: David Lechner <[email protected]>
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?

The short answer is that tinydrm sends pixel data over the panel
controller's control interface whereas drm/panel leaves that to a
dedicated pixel interface fed by another driver.

tinydrm is used with controllers that have onboard GRAM which is
scanned out by the panel controller itself. Many of these controllers
support multiple interfaces, like MIPI DSI/DPI/DBI. A MIPI DPI
compatible controller has a control bus, usually SPI, to operate the
panel. This same control bus can be used in MIPI DBI mode on some
controllers to push pixels.

The MIPI standard documents isn't open, but some are available if you
do a search. We also have MIPI DCS which is the command set shared by
the MIPI compatible controllers.

So how do we deal with controllers that can operate in many modes?
I raised the question when a panel driver was reviewed earlier this
year, but nothing really came out of it.

I suppose that displays that are used with DSI/DPI doesn't have a
controller with onboard GRAM, since that would just increase the price.
The MIPI standard defines different controller types which has no,
partial or full framebuffer. So in reality it's not that likely that
we will see the same controller used both in tinydrm and drm/panel.

But I'm hardly an expert on these matters, I've only used the DBI mode.
I've cc'ed the drm/panel maintainer, maybe he can shed some more light.

Noralf.

> I guess everybody knows except me, it's usually like that :(
>
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
>
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
>
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
>
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
>
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.
>
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

2017-12-04 07:45:31

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] drm/tinydrm: add driver for ILI9225 panels

On Fri, Dec 01, 2017 at 03:03:30PM +0100, Linus Walleij wrote:
> On Wed, Nov 8, 2017 at 4:52 AM, David Lechner <[email protected]> wrote:
>
> > This adds a new driver for display panels based on the Ilitek ILI9225
> > controller.
> >
> > This was developed for a no-name panel with a red PCB that is commonly
> > marketed for Arduino. See <https://github.com/Nkawu/TFT_22_ILI9225>.
> >
> > I really did try very hard to find a make and model for this panel, but
> > there doesn't seem to be one, so the best I can do is offer the picture
> > in the link above for identification.
> >
> > Signed-off-by: David Lechner <[email protected]>
>
> Can you explain why tinydrm is not putting its panel drivers in
> drivers/gpu/drm/panel?
>
> I guess everybody knows except me, it's usually like that :(
>
> I am anyways working on a driver for Ilitek 9322 that I want
> to land in drivers/gpu/drm/panel. Here is the last iteration:
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150205.html
> Yeah I got sidetracked. OK I will get to it now.
>
> There are some similarities with the code I'm seeing here
> but I believe they are essentially different. But it will be hard
> to share code if you put the driver in the tinydrm framework.
>
> I guess you have also seen:
> drivers/video/backlight/ili922x.c
> ?
>
> Stefano Babic who wrote the backlight driver is available for
> reviewing, so includ him in follow-ups (added to To: line).
>
> I'm putting you on CC as I'm rewriting it a bit after the DT
> maintainers review, will try to repost ASAP.

Bit more historical context: We tried using drm_panel in tinydrm, but that
didn't really fit to well (as Noralf explains, tinydrm is kinda more for
stand-alone panels). But tinydrm is also a bit too much midlayer-y still,
so there's a bunch of todo items capture in Documentation/gpu/todo.rst. In
the end we shouldn't need a special tinydrm driver, that should be covered
by the usual drm helpers.

Might be worth it to at least capture/summarize some of the reasons for
why tinydrm doesn't use drm_panel, and what it would take to better share
code (or maybe that's just a silly idea, not the first duplicated driver
in drm).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch