[1/2] drm/panel: Add DT bindings for Sony ACX424AKP
diff mbox series

Message ID 20190902090633.24239-1-linus.walleij@linaro.org
State Changes Requested
Headers show
Series
  • [1/2] drm/panel: Add DT bindings for Sony ACX424AKP
Related show

Checks

Context Check Description
robh/dt-meta-schema fail build log
robh/checkpatch success

Commit Message

Linus Walleij Sept. 2, 2019, 9:06 a.m. UTC
This adds device tree bindings for the Sony ACX424AKP panel.
Let's use YAML.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../display/panel/sony,acx424akp.yaml         | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml

Comments

Thierry Reding Sept. 2, 2019, 9:35 a.m. UTC | #1
On Mon, Sep 02, 2019 at 11:06:32AM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Sony ACX424AKP panel.
> Let's use YAML.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../display/panel/sony,acx424akp.yaml         | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> new file mode 100644
> index 000000000000..29f41fee1b69
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony ACX424AKP 4" 480x864 AMOLED panel
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: sony,acx424akp
> +  reg: true
> +  port: true
> +  reset-gpios: true
> +  vddi-supply:
> +     description: regulator that supplies the vddi voltage
> +  dsi-command-mode:
> +     type: boolean
> +     description:
> +       If this is specified, the panel will be used in command
> +       mode instead of video mode.

I'm not sure there's concensus on this one yet. I think so far the
driver decides which mode to use the panel in. Technically this falls
into the category of configuration, so it doesn't really belong in the
DT.

I vaguely recall from discussions I've had on this subject that there's
usually no reason to do video mode if you can do command mode because
command mode is more power efficient. This was a long time ago, so I may
be misremembering. Perhaps you have different information on this?

Thierry

> +
> +required:
> +  - compatible
> +  - reg
> +  - port
> +  - reset-gpios
> +  - power-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    panel@0 {
> +        compatible = "sony,acx424akp";
> +        reg = <0>;
> +        vddi-supply = <&foo>;
> +        reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
> +
> +        port {
> +            panel_in: endpoint {
> +                remote-endpoint = <&dsi_out>;
> +            };
> +        };
> +    };
> +
> +...
> \ No newline at end of file
> -- 
> 2.21.0
>
Linus Walleij Sept. 2, 2019, 11:44 a.m. UTC | #2
On Mon, Sep 2, 2019 at 11:35 AM Thierry Reding <thierry.reding@gmail.com> wrote:

> > +  dsi-command-mode:
> > +     type: boolean
> > +     description:
> > +       If this is specified, the panel will be used in command
> > +       mode instead of video mode.
>
> I'm not sure there's concensus on this one yet. I think so far the
> driver decides which mode to use the panel in. Technically this falls
> into the category of configuration, so it doesn't really belong in the
> DT.

The way we've used DT is for a bit of both hardware description
and configuration I'd say, but I'm no authority on the subject.

> I vaguely recall from discussions I've had on this subject that there's
> usually no reason to do video mode if you can do command mode because
> command mode is more power efficient. This was a long time ago, so I may
> be misremembering. Perhaps you have different information on this?

No idea. I was under the impression that video mode was preferred
but I have no idea why.

Yours,
Linus Walleij
Thierry Reding Sept. 2, 2019, 2:40 p.m. UTC | #3
On Mon, Sep 02, 2019 at 01:44:38PM +0200, Linus Walleij wrote:
> On Mon, Sep 2, 2019 at 11:35 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > > +  dsi-command-mode:
> > > +     type: boolean
> > > +     description:
> > > +       If this is specified, the panel will be used in command
> > > +       mode instead of video mode.
> >
> > I'm not sure there's concensus on this one yet. I think so far the
> > driver decides which mode to use the panel in. Technically this falls
> > into the category of configuration, so it doesn't really belong in the
> > DT.
> 
> The way we've used DT is for a bit of both hardware description
> and configuration I'd say, but I'm no authority on the subject.
> 
> > I vaguely recall from discussions I've had on this subject that there's
> > usually no reason to do video mode if you can do command mode because
> > command mode is more power efficient. This was a long time ago, so I may
> > be misremembering. Perhaps you have different information on this?
> 
> No idea. I was under the impression that video mode was preferred
> but I have no idea why.

Hm... my recollection is that command mode is only supported on "smart"
panels that have an internal framebuffer. So the commands actually
instruct the panel to update their internal framebuffer, which means you
can technically switch off the display engine when there are no updates.

Under those circumstances I think it'd make sense to default to command
mode if both the panel and the host support it and stick with video mode
if for example the host can't do command mode.

Or perhaps this is something that could be set from some userspace
policy maker via a connector property? A compositor for instance would
have a pretty good idea of what kind of activity is going on, so it
could at some point decide to switch between video mode and command mode
if one of them is more appropriate for the given workload.

Command mode can also be used to do partial updates, if I remember
correctly, which again would make it possible for a compositor to send
only a subset of a screen update.

Thierry
Rob Herring Sept. 2, 2019, 3:31 p.m. UTC | #4
On Mon, Sep 02, 2019 at 04:40:06PM +0200, Thierry Reding wrote:
> On Mon, Sep 02, 2019 at 01:44:38PM +0200, Linus Walleij wrote:
> > On Mon, Sep 2, 2019 at 11:35 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> > 
> > > > +  dsi-command-mode:
> > > > +     type: boolean
> > > > +     description:
> > > > +       If this is specified, the panel will be used in command
> > > > +       mode instead of video mode.
> > >
> > > I'm not sure there's concensus on this one yet. I think so far the
> > > driver decides which mode to use the panel in. Technically this falls
> > > into the category of configuration, so it doesn't really belong in the
> > > DT.
> > 
> > The way we've used DT is for a bit of both hardware description
> > and configuration I'd say, but I'm no authority on the subject.

I'm okay with this if there's consensus, but it should be in a common 
doc. We probably need a dsi-commom schema with this, reg, ??.

> > 
> > > I vaguely recall from discussions I've had on this subject that there's
> > > usually no reason to do video mode if you can do command mode because
> > > command mode is more power efficient. This was a long time ago, so I may
> > > be misremembering. Perhaps you have different information on this?

30 or 60fps updates tend to be impossible because you have less b/w and 
it's async to the refresh.

I think most panels that can do both, always need command mode too for 
initialization.

> > No idea. I was under the impression that video mode was preferred
> > but I have no idea why.
> 
> Hm... my recollection is that command mode is only supported on "smart"
> panels that have an internal framebuffer. So the commands actually
> instruct the panel to update their internal framebuffer, which means you
> can technically switch off the display engine when there are no updates.
>
> Under those circumstances I think it'd make sense to default to command
> mode if both the panel and the host support it and stick with video mode
> if for example the host can't do command mode.
> 
> Or perhaps this is something that could be set from some userspace
> policy maker via a connector property? A compositor for instance would
> have a pretty good idea of what kind of activity is going on, so it
> could at some point decide to switch between video mode and command mode
> if one of them is more appropriate for the given workload.
> 
> Command mode can also be used to do partial updates, if I remember
> correctly, which again would make it possible for a compositor to send
> only a subset of a screen update.

All makes sense to me.

Rob
Linus Walleij Sept. 2, 2019, 5:25 p.m. UTC | #5
On Mon, Sep 2, 2019 at 4:40 PM Thierry Reding <thierry.reding@gmail.com> wrote:

> Hm... my recollection is that command mode is only supported on "smart"
> panels that have an internal framebuffer. So the commands actually
> instruct the panel to update their internal framebuffer, which means you
> can technically switch off the display engine when there are no updates.

That is my understanding as well.

> Under those circumstances I think it'd make sense to default to command
> mode if both the panel and the host support it and stick with video mode
> if for example the host can't do command mode.

Makes sense to me.

Maybe we should rather have a generic setting like:
dsi-enforce-video-mode;
and the default always being command mode, if
we have consensus that command mode should always
be preferred.

> Or perhaps this is something that could be set from some userspace
> policy maker via a connector property? A compositor for instance would
> have a pretty good idea of what kind of activity is going on, so it
> could at some point decide to switch between video mode and command mode
> if one of them is more appropriate for the given workload.

It's probably more that userspace should be made aware of the
fact that we have partial updates, and if I understand correctly that
is done by dirtyrect/damage API in
drm_damage_helper.c

(Very nice overview doc at the top of the file!)

The driver enables damage by calling
drm_plane_enable_fb_damage_clips()
then in the .update() callback uses drm_atomic_helper_damage_merged()
to figure out damaged rectangles and update those with special
commands.

tinydrm/ili9225.c is a pretty clean example of a driver
actually doing this. There are not many of them.

> Command mode can also be used to do partial updates, if I remember
> correctly, which again would make it possible for a compositor to send
> only a subset of a screen update.

Indeed, who needs a blitter when you can just update the
dirtyrects.

It is a bit terse but intuitively I feel that the damage interface is what
userspace should use, then DRM should be able to infer that a
damaged rectangle can be updated on the display, and the
display controller need to announce that it can handle these
partial updates.

This requires that the display controller can generate such
complex command streams in response to
drm_atomic_helper_damage_merged()
of course, from a DSI protocol
level it is supported, but we are not writing these command
sequences with software, they are generated by hardware.

So the display controller DSI portions must be able to send
individual rectangles as well.

But this is all science fiction. What all DSI display controllers
in mailine do today (I think) is to simply scan out the whole
framebuffer continously with a vblank TE IRQ to avoid tearing.

Andrzej knows for sure what is out there I think.

Yours,
Linus Walleij

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
new file mode 100644
index 000000000000..29f41fee1b69
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
@@ -0,0 +1,53 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony ACX424AKP 4" 480x864 AMOLED panel
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: sony,acx424akp
+  reg: true
+  port: true
+  reset-gpios: true
+  vddi-supply:
+     description: regulator that supplies the vddi voltage
+  dsi-command-mode:
+     type: boolean
+     description:
+       If this is specified, the panel will be used in command
+       mode instead of video mode.
+
+required:
+  - compatible
+  - reg
+  - port
+  - reset-gpios
+  - power-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    panel@0 {
+        compatible = "sony,acx424akp";
+        reg = <0>;
+        vddi-supply = <&foo>;
+        reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
+
+        port {
+            panel_in: endpoint {
+                remote-endpoint = <&dsi_out>;
+            };
+        };
+    };
+
+...
\ No newline at end of file