Re: [PATCH] of: Add simple panel device tree binding

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2560 bytes --]

On Tue, Nov 26, 2013 at 02:54:41AM +0100, Laurent Pinchart wrote:
> Hi Thierry,
> 
> Thank you for the patch.
> 
> On Friday 22 November 2013 19:41:54 Thierry Reding wrote:
> > This binding specifies a set of common properties for display panels. It
> > can be used as a basis by bindings for specific panels.
> > 
> > Bindings for three specific panels are provided to show how the simple
> > panel binding can be used.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > This binding has already been discussed earlier. Both Laurent and Tomi
> > seemed to be generally fine with this.
> 
> That's correct, I'm generally fine with your approach, but there's still one 
> point I'd like to see addressed.
> 
> As I mentioned previously, I would like to avoid adding zillions of compatible 
> properties to the driver, when we could use a single property in the DT 
> bindings that would specify the timings instead. This would lower the amount 
> of changes made to the simple panel driver, while keeping DT simple enough (at 
> least in my opinion).
> 
> Specifying the full timings in every DT source file would be pretty verbose 
> and could be error-prone. However, using a single property to specify one of 
> the standard display timings, as orinally proposed by Hans de Goede (CC'ed) 
> during a chat at the kernel summit would in my opinion be a good middle-ground 
> solution.
> 
> Would you consider adding such a property to the simple panel bindings, and 
> define a common compatible string ? Each panel should of course list its own 
> compatibility string in addition to the common compatible string in case the 
> need for extensions arises in the future.

My gripe with this is that it duplicates information. By definition a
simple panel supports a limited number of display modes (typically only
a single one), so once you know the compatible value (which needs to be
there anyway) you can derive the mode from it. Adding a property that
specifies the display mode is redundant.

Dave Airlie proposed something else, namely to keep a list of common
modes within the driver and have each device reference that mode
instead. I think that could work similarily well. It still requires the
driver to be touched for each new panel, but the changes will be very
small. They'll be more of the "add a new table entry" sort of change
that we have in drivers like the 8250/16550-compatible serial. Most of
that could probably be wrapped into a macro to make it concise.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel