Re: [RFR 2/2] drm/panel: Add simple panel support

Hi Thierry,

On Thursday 17 October 2013 12:48:57 Thierry Reding wrote:

On Thu, Oct 17, 2013 at 12:52:40PM +0300, Tomi Valkeinen wrote:

quoted

On 17/10/13 12:16, Thierry Reding wrote:

quoted

On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:

quoted

Just my opinion, but yes, DT is that inflexible. DT bindings are like a
syscall. Once they are there, you need to support them forever. That
support can, of course, include some kind of DT data converters or
such, that try to convert old bindings to new bindings at kernel boot.

That's not entirely true. DT bindings are allowed to change, the only
restriction being that they don't break backwards compatibility. So if

True, but afaik the same goes for syscalls. But yes, it's probably
easier to add, say, new properties to DT bindings than adding new flags
to a syscall.

quoted

Am I the only one refusing to accept that we can't provide even the most
basic functionality simply because we haven't been able to come up with
the ultimately "perfect" DT binding yet? By definition it's not very
likely that any binding will ever be perfect.

With "perfect" I meant without any obvious features missing. I agree
that DT bindings will never be perfect, but I at least would like them
to be good enough to cover all the cases we know of.

As far as the simple panel device tree binding is concerned, it covers
all the cases I know of. I can't seriously be expected to do any better
than that. These panels are dumb. They only come with a power supply
that needs to be switched on and an additional GPIO to enable it once
powered up. All three panels just work with that set of properties. So
anything that doesn't can arguably be covered by some other, not-so-
simple binding.

quoted

quoted

I don't think we're doing ourselves any good by letting DT actively
hinder us in merging any of these features. I would personally prefer
having to support several bindings rather than not be able to provide
the functionality at all.

I'd say the driver writer is of course free to create basic bindings for
the device in question as long as the bindings are sane, and he agrees
to later create any needed converters. I don't see any problem with
having a driver supporting several versions of the bindings.

But I don't think it's fair to push a driver with basic bindings,
knowing that the bindings won't be enough in the future, and leave all
the converter-mess to other people (not saying that's the case here).

As a fbdev maintainer, I'm ok with adding DT bindings to device specific
fb drivers, as those can be just left as they are. Even when CDF is
merged, the specific fb drivers just work with the old bindings. If the
driver maintainer wants to use CDF, it's up to him to create any needed
binding-converters.

But a generic driver for panels is more problematic, as that one should
be maintained and refreshed to new features like CDF. If such was
proposed to be merged to fbdev, I'd be very reluctant to merge it if
there were any concerns about it.

I'm still missing the point here. For one, I don't see any reason that
this driver would ever need to gain CDF support. At the same time, CDF
could easily be supported by just adding a few required properties and
it should be easy to support any non-CDF cases just as well to remain
backwards-compatible.

quoted

I guess one option there would be to implement a new driver for the same
panel devices, one that supports CDF, and leave the old one as is. Not
so nice option, though.

As I said, anything that really needs a CDF binding to work likely isn't
"simple" anymore, therefore a separate driver can easily be justified.

The system as a whole would be more complex, but the panel could be the same. 
We can't have two drivers for the same piece of hardware in the DT world, as 
there will be a single compatible string and no way to choose between the 
drivers (unlike the board code world that could set device names to "foo-
encoder-v4l2" or "foo-encoder-drm" and live happily with that ever after).

quoted

quoted

For crying out loud, we can use the 3D engine to render to a framebuffer
but we can't look at the result because we can't make the bloody panel
light up! Seriously!?

Ah, but do you have DT bindings for the 3D engine yet, which represent
how different internal blocks in the 3D engine are connected? If not,
better start designing it! Then the problem is solved, you won't even
have working 3D engine. ;)

But seriously speaking, I'm with you. I don't get this DT stuff.

And the funny thing is, DT should just represent the hardware, it
doesn't matter what kind of SW drivers there are, so in a sense talking
about DT problems with a SW framework like CDF is a bit silly. How hard
can it be to describe the hardware properly?

Very, obviously =).

Indeed. The fundamental point here seems to be that we don't represent
things in DT which we don't need. In the same way that we don't add device
nodes for enumerable devices, why would we need to explicitly add
information about connections between devices that cannot be controlled
anyway. If the board connects an RGB/LVDS output to a panel directly, it is
only required that the output knows what it is connected to because there is
no other way besides DT to obtain any information about the panel. Therefore
a unidirectional connection is more than enough. Using that the output can
access the panel and query it for information such as the mode timings as
well as switch it on or off as required.

Please note that the idea of putting the modes in DT in no way implies that 
the device controller should parse the modes from the panel node. It must 
instead query the panel at runtime through whatever API is provided. Going 
directly to the DT data would be a layering violation. The reason why I 
propose to add modes to the panel DT node is to make the geenric panel driver 
more generic and simpler.

-- 
Regards,

Laurent Pinchart

On 10/17/2013 12:07 PM, Laurent Pinchart wrote:
...

quoted

As I said, anything that really needs a CDF binding to work
likely isn't "simple" anymore, therefore a separate driver can
easily be justified.

The system as a whole would be more complex, but the panel could be
the same. We can't have two drivers for the same piece of hardware
in the DT world, as there will be a single compatible string and no
way to choose between the drivers (unlike the board code world that
could set device names to "foo- encoder-v4l2" or "foo-encoder-drm"
and live happily with that ever after).

That's not true. We can certainly define two different compatible
values for a piece of HW if we have to. We can easily control whether
they are handled by the same or different drivers in the OS.

Now, we should try to avoid this, because then that means that the
original binding wasn't fully describing the HW. However, at least in
the case of these simple LCD panels, it's hard to see that there is
anything more than what's already in Thierry's binding. Remember, the
binding is a description of the HW, not any Linux-internal details of
how e.g. a CDF or DRM subsystem is supposed to use the HW; that had
better be embodied into the driver or subsystem code, which aren't
ABIs, and hence can be easily modified/enhanced.