devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-renesas-soc@vger.kernel.org,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
Date: Tue, 22 Nov 2016 15:14:08 +0200	[thread overview]
Message-ID: <2645572.hCCJeODTBK@avalon> (raw)
In-Reply-To: <20161122110548.GB22735@ulmo.ba.sec>

Hi Thierry,

On Tuesday 22 Nov 2016 12:05:48 Thierry Reding wrote:
> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> > Document properties common to several display panels in a central
> > location that can be referenced by the panel device tree bindings.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/panel/panel-common.txt        | 91 +++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/panel/panel-common.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > new file mode 100644
> > index 000000000000..ec52c472c845
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> > @@ -0,0 +1,91 @@
> > +Common Properties for Display Panel
> > +===================================
> > +
> > +This document defines device tree properties common to several classes of
> > +display panels. It doesn't constitue a device tree binding specification
> > by
> > +itself but is meant to be referenced by device tree bindings.
> > +
> > +When referenced from panel device tree bindings the properties defined in
> > this
> > +document are defined as follows. The panel device tree bindings are
> > +responsible for defining whether each property is required or optional.
> > +
> > +
> > +Descriptive Properties
> > +----------------------
> > +
> > +- width-mm,
> > +- height-mm: The width-mm and height-mm specify the width and height of
> > the
> > +  physical area where images are displayed. These properties are
> > expressed in
> > +  millimeters and rounded to the closest unit.
> 
> Erm... this is already implied by the compatible string. Having this in
> device tree is completely redundant.

Nothing new under the sun here, we already have plenty of properties that 
could be implied by compatible strings. For instance for SoC IP cores many 
vendors use both an SoC-specific compatible string and a generic compatible 
string (e.g. "renesas,gpio-r8a7795" and "renesas,gpio-rcar"). The SoC-
compatible string implies register addresses, clocks and interrupts, but we 
still describe them in DT.

At the end of the day information about devices and their integration in the 
system needs to be available, either from DT or from C code. DT bindings 
should be designed to strike a good balance there, avoiding redundant 
information in DT (and thus keeping the bindings simple) while still providing 
enough information to allow for a reasonable level of genericity in OS 
implementations.

> > +- label: The label property specifies a symbolic name for the panel as a
> > +  string suitable for use by humans. It typically contains a name
> > inscribed on
> > +  the system (e.g. as an affixed label) or specified in the system's
> > +  documentation (e.g. in the user's manual).
> > +
> > +  If no such name exists, and unless the property is mandatory according
> > to
> > +  device tree bindings, it shall rather be omitted than constructed of
> > +  non-descriptive information. For instance an LCD panel in a system that
> > +  contains a single panel shall not be labelled "LCD" if that name is not
> > +  inscribed on the system or used in a descriptive fashion in system
> > +  documentation.
> > +
> > +
> > +Display Timings
> > +---------------
> > +
> > +- panel-timing: Most display panels are restricted to a single resolution
> > and
> > +  require specific display timings. The panel-timing subnode expresses
> > those
> > +  timings as specified in the timing subnode section of the display
> > timing
> > +  bindings defined in
> > +  Documentation/devicetree/bindings/display/display-timing.txt.
> 
> Why? That's also implied by the compatible string. Honestly, I thought
> by now we had been over this often enough...

Same argument as above. I won't try to change your mind and fix the simple 
panel driver, but I still stand firm on my belief that expressing the size and 
timings in DT is the right solution in a wide variety of cases (and yes I've 
read http://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, and while I agree with the title, I still believe size and 
timings in DT are not wrong).

-- 
Regards,

Laurent Pinchart

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

  reply	other threads:[~2016-11-22 13:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-19  3:28 [PATCH v2 00/13] R-Car DU: Add support for LVDS mode selection Laurent Pinchart
2016-11-19  3:28 ` [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties Laurent Pinchart
2016-11-21 16:48   ` Rob Herring
2016-11-22  9:36     ` Laurent Pinchart
2016-11-29  8:27       ` Laurent Pinchart
2016-11-29 15:14         ` Rob Herring
2016-11-29 18:23           ` Laurent Pinchart
2016-12-18 20:54             ` Laurent Pinchart
2016-12-19 15:38               ` Rob Herring
2016-12-19 16:54                 ` Laurent Pinchart
2017-01-03 22:33                   ` Rob Herring
     [not found]   ` <1479526093-7014-2-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2016-11-22 11:05     ` Thierry Reding
2016-11-22 13:14       ` Laurent Pinchart [this message]
2016-11-22 21:10       ` Rob Herring
2017-04-09 11:47     ` Emil Velikov
2017-04-11  5:12       ` Laurent Pinchart
2016-11-19  3:28 ` [PATCH v2 02/13] devicetree/bindings: display: Add bindings for LVDS panels Laurent Pinchart
     [not found]   ` <1479526093-7014-3-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2016-11-21 16:48     ` Rob Herring
2016-11-22 11:02     ` Thierry Reding
2016-11-22 13:21       ` Laurent Pinchart
     [not found] ` <1479526093-7014-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2016-11-19  3:28   ` [PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels Laurent Pinchart
2016-11-21 16:49     ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2645572.hCCJeODTBK@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).