| Message ID | 20200714071305.18492-2-wens@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: sunxi: Add support for MSI Primo73 tablet | expand |
On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > From: Chen-Yu Tsai <wens@csie.org> > > Some LCD panels do not support 24-bit true color, or 8bits per channel > RGB. Many low end ones only support up to 6 bits per channel natively. > > Add a device tree property to describe the native bit depth of the > panel. This is separate from the bus width or format of the connection > to the display output. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > .../devicetree/bindings/display/panel/panel-dpi.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > index 0cd74c8dab42..8eb013fb1969 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > @@ -26,6 +26,9 @@ properties: > height-mm: true > label: true > panel-timing: true > + bits-per-color: > + description: > + Shall contain an integer describing the number of bits per color. You should specify its type (u32), range (1-8 I guess?) and default value (which seems to be 8). Also, it's not unusual to have a different number of bits per color, like for 16 bits panels where we usually use RGB565. I guess we could make that an array? Maxime
On Thu, Jul 16, 2020 at 4:00 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > > From: Chen-Yu Tsai <wens@csie.org> > > > > Some LCD panels do not support 24-bit true color, or 8bits per channel > > RGB. Many low end ones only support up to 6 bits per channel natively. > > > > Add a device tree property to describe the native bit depth of the > > panel. This is separate from the bus width or format of the connection > > to the display output. > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > --- > > .../devicetree/bindings/display/panel/panel-dpi.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > > index 0cd74c8dab42..8eb013fb1969 100644 > > --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > > +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > > @@ -26,6 +26,9 @@ properties: > > height-mm: true > > label: true > > panel-timing: true > > + bits-per-color: > > + description: > > + Shall contain an integer describing the number of bits per color. > > You should specify its type (u32), range (1-8 I guess?) and default > value (which seems to be 8). Ok. > Also, it's not unusual to have a different number of bits per color, > like for 16 bits panels where we usually use RGB565. I guess we could > make that an array? So for different number of bits per color, I'm not sure whether that's a function of the panel itself, or the bus format. I don't really have any 16-bit panels on hand. As it stands DRM only handles a common color depth. ChenYu
On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > From: Chen-Yu Tsai <wens@csie.org> > > Some LCD panels do not support 24-bit true color, or 8bits per channel > RGB. Many low end ones only support up to 6 bits per channel natively. This should be implied by the panel's compatible property. > Add a device tree property to describe the native bit depth of the > panel. This is separate from the bus width or format of the connection > to the display output. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > .../devicetree/bindings/display/panel/panel-dpi.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > index 0cd74c8dab42..8eb013fb1969 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml > @@ -26,6 +26,9 @@ properties: > height-mm: true > label: true > panel-timing: true > + bits-per-color: > + description: > + Shall contain an integer describing the number of bits per color. > port: true > power-supply: true > reset-gpios: true > @@ -42,6 +45,7 @@ examples: > panel { > compatible = "osddisplays,osd057T0559-34ts", "panel-dpi"; > label = "osddisplay"; > + bits-per-color = <8>; > power-supply = <&vcc_supply>; > backlight = <&backlight>; > > -- > 2.27.0 >
On Mon, Jul 20, 2020 at 08:10:26PM -0600, Rob Herring wrote: > On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > > From: Chen-Yu Tsai <wens@csie.org> > > > > Some LCD panels do not support 24-bit true color, or 8bits per channel > > RGB. Many low end ones only support up to 6 bits per channel natively. > > This should be implied by the panel's compatible property. I'm not sure it should, or at least it's not sufficient. Some panels while 24 bits capable might only have the higher bits connected to save off a couple of pins per color, in which case we should probably describe that somehow. Maxime
On Tue, Jul 21, 2020 at 5:23 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Mon, Jul 20, 2020 at 08:10:26PM -0600, Rob Herring wrote: > > On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > > > From: Chen-Yu Tsai <wens@csie.org> > > > > > > Some LCD panels do not support 24-bit true color, or 8bits per channel > > > RGB. Many low end ones only support up to 6 bits per channel natively. > > > > This should be implied by the panel's compatible property. > > I'm not sure it should, or at least it's not sufficient. Some panels > while 24 bits capable might only have the higher bits connected to save > off a couple of pins per color, in which case we should probably > describe that somehow. The bus format stuff that was added then removed might be better suited for what you have in mind. Right now that's put in the simple panel driver, but it likely doesn't belong there, since the bus format is also related to the hardware integration, signal routing as you mentioned, and not just a property of the panel itself. Nevertheless, what I'm looking for can be achieved using bus format as well. Given that I have no datasheet for the panel in the device I'm upstreaming, and only a vague part number to go with, describing it as a bus format modifier rather than a property of the panel might be safer. So I'll just drop the two patches regarding bit depth for now. ChenYu
On Tue, Jul 21, 2020 at 3:23 AM Maxime Ripard <maxime@cerno.tech> wrote: > > On Mon, Jul 20, 2020 at 08:10:26PM -0600, Rob Herring wrote: > > On Tue, Jul 14, 2020 at 03:13:01PM +0800, Chen-Yu Tsai wrote: > > > From: Chen-Yu Tsai <wens@csie.org> > > > > > > Some LCD panels do not support 24-bit true color, or 8bits per channel > > > RGB. Many low end ones only support up to 6 bits per channel natively. > > > > This should be implied by the panel's compatible property. > > I'm not sure it should, or at least it's not sufficient. Some panels > while 24 bits capable might only have the higher bits connected to save > off a couple of pins per color, in which case we should probably > describe that somehow. That's the bus/interface format which the 2nd paragraph said this was not for. If it was, then just bits per component is not enough. Rob
diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml index 0cd74c8dab42..8eb013fb1969 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml @@ -26,6 +26,9 @@ properties: height-mm: true label: true panel-timing: true + bits-per-color: + description: + Shall contain an integer describing the number of bits per color. port: true power-supply: true reset-gpios: true @@ -42,6 +45,7 @@ examples: panel { compatible = "osddisplays,osd057T0559-34ts", "panel-dpi"; label = "osddisplay"; + bits-per-color = <8>; power-supply = <&vcc_supply>; backlight = <&backlight>;