Re: [PATCH] ARM: dts: sun4i: Add simplefb node with de_fe0-de_be0-lcd0-hdmi pipeline

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3935 bytes --]

On Mon, Jan 19, 2015 at 02:46:00PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 19-01-15 14:35, Hans de Goede wrote:
> >Hi,
> >
> >On 19-01-15 14:20, Maxime Ripard wrote:
> >>Hi Hans,
> >>
> >>On Mon, Jan 19, 2015 at 02:01:17PM +0100, Hans de Goede wrote:
> >>>Testing has shown that on sun4i the display backend engine does not have
> >>>deep enough fifo-s causing flickering / tearing in full-hd mode due to
> >>>fifo underruns. This can be avoided by letting the display frontend engine
> >>>do the dma from memory, and then letting it feed the data directly into
> >>>the backend unmodified, as the frontend does have deep enough fifo-s.
> >>>
> >>>Note since u-boot-v2015.01 has been released using the de_be0-lcd0-hdmi
> >>>pipeline on sun4i, we need to keep that one around too (unfortunately).
> >>>
> >>>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>>---
> >>>  arch/arm/boot/dts/sun4i-a10.dtsi | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>>diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>>index f5e35b5..ccd60e3 100644
> >>>--- a/arch/arm/boot/dts/sun4i-a10.dtsi
> >>>+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>>@@ -44,6 +44,14 @@
> >>>                   <&ahb_gates 44>;
> >>>              status = "disabled";
> >>>          };
> >>>+
> >>>+        framebuffer@1 {
> >>>+            compatible = "allwinner,simple-framebuffer", "simple-framebuffer";
> >>>+            allwinner,pipeline = "de_fe0-de_be0-lcd0-hdmi";
> >>>+            clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
> >>>+                 <&ahb_gates 44>, <&ahb_gates 46>;
> >>>+            status = "disabled";
> >>>+        };
> >>
> >>Thanks for this.
> >>
> >>I do have a question though, maybe I'm missing something, but what
> >>would prevent us from using the existing node already? The output
> >>pipeline seems to be the same, at least for which output we
> >>use. Wether we use or not the display frontend looks like an
> >>implementation detail.
> >
> >The purpose of having a node per pipeline is to have the rights clocks
> >listed for the pipeline chosen. So the "de_fe0-de_be0-lcd0-hdmi" node
> >has one ahb_gates entry more so as to make sure the kernel does not gate
> >off the frontend.
> >
> >The idea of having different nodes per pipeline is that when we get
> >support for more module clocks in place, as a preparation for kms
> >support, we can add the module clocks to the relevant nodes, to also
> >ensure the module clocks will not get turned off.
> >
> >By having this info in the dtb, rather then in u-boot we can update
> >the clocks list as we add new clock nodes in the dtb, ensuring that
> >an older u-boot will keep working as we add support for module
> >clocks, etc.
> 
> Hmm, thinking more about this, and about how u-boot searches for
> the simplefb node, I think that having a single node like this:
> 
>         framebuffer@0 {
>             compatible = "allwinner,simple-framebuffer", "simple-framebuffer";
>             /* This node is for de_fe0-de_be0-lcd0-hdmi, but we also declare de_be0-lcd0-hdmi
>                for compatibility with u-boot-v2015.01 which does not use the frontend */
>             allwinner,pipeline = "de_fe0-de_be0-lcd0-hdmi", "de_be0-lcd0-hdmi";
>             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
>                  <&ahb_gates 44>, <&ahb_gates 46>;
>             status = "disabled";
>         };
> 
> Should work, but this will cause simplefb to enable the frontend ahb gate
> even if it is not used (this will only impact u-boot-v2015.01 users), and as
> such feels wrong, so I would prefer to go with my patch as is.

That's actually what I had in mind. It didn't feel that wrong, since
it pretty much mimics the compatible behaviour, but it's your call.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com