Hi,

On Fri, Oct 3, 2014 at 1:14 AM, jonsmirl at gmail.com [off-list ref] wrote:

On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede [off-list ref] wrote:

quoted

Hi,

On 10/02/2014 04:41 PM, jonsmirl at gmail.com wrote:

quoted

On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede [off-list ref] wrote:

quoted

Hi,

On 10/02/2014 04:16 PM, jonsmirl at gmail.com wrote:

quoted

On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede [off-list ref] wrote:

<snip>

quoted

quoted

quoted

quoted

quoted

So there are two ways to do this...
1) modify things like earlyconsole to protect device specific resource
(I think this is a bad idea)

Why is this a bad idea? If the bootloader tells us exactly which resources
are needed, then earlyconsole can claim them, and release them on
handover to the real display driver.

Jon, can you please answer this ? I really really want to know why people
think this is such a bad idea. Understanding why people think this is a bad
idea is necessary to be able to come up with an alternative solution.

The list of resources should not be duplicated in the device tree -
once in the simplefb node and again in the real device node.

It is not duplicated, the simplefb node will list the clocks used for the
mode / output as setup by the firmware, which are often not all clocks
which the display engine supports. Where as the real device node will list
all clocks the display engine may use.

quoted

Device
tree is a hardware description and it is being twisted to solve a
software issue.

This is not true, various core devicetree developers have already said
that storing other info in the devicetree is fine, and being able to do so
is part of the original design.

quoted

This problem is not limited to clocks, same problem
exists with regulators. On SGI systems this would exist with entire
bus controllers (but they are x86 based, console is not on the root
bus). This is a very messy problem and will lead to a Frankenstein
sized driver over time.

This is a "what if ..." argument, we can discuss potential hypothetical
problems all day long, what happens if the sky falls down?

quoted

But... I think this is a red herring which is masking the real
problem. The real problem seems to be that there is no window for
loading device specific drivers before the resource clean up phase
happens. That's a real problem -- multi architecture distros are going
to have lots of loadable device specific drivers.

As Maxime pointed out to my alternative solution to fixing the clocks
problem, this is not strictly a when to do cleanup problem. If another
driver uses the same clocks, and does a clk_disable call after probing
(because the device is put in low power mode until used by userspace),
then the clk will be disabled even without any cleanup running at all.

The real problem here is simply that to work the simplefb needs certain
resources, just like any other device. And while for any other device
simply listing the needed resources is an accepted practice, for simplefb
for some reason (which I still do not understand) people all of a sudden
see listing resources as a problem.

Because you are creating two different device tree nodes describing a
single piece of hardware and that's not suppose to happen in a device
tree.  The accurate description of the hardware is being perverted to
solve a software problem.

One node describes the hardware in a format to make simplefb happy.
Another node describes the same hardware in a format to make the
device specific driver happy.

Stupid question: What about mangling an existing device node to be
simplefb compatible, and writing simplefb to be binding agnostic?

I listed some psuedo-code to do the latter earlier in the thread.

I.e. changing something like this:

vopb: vopb at ff930000 {
    compatible = "rockchip,rk3288-vop";
    reg = <0xff930000 0x19c>;
    interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
    clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
    resets = <&cru SRST_LCDC1_AXI>, <&cru SRST_LCDC1_AHB>, <&cru
SRST_LCDC1_DCLK>;
    reset-names = "axi", "ahb", "dclk";
    iommus = <&vopb_mmu>;
    vopb_out: port {
        #address-cells = <1>;
        #size-cells = <0>;
        vopb_out_edp: endpoint at 0 {
            reg = <0>;
            remote-endpoint=<&edp_in_vopb>;
        };
        vopb_out_hdmi: endpoint at 1 {
            reg = <1>;
            remote-endpoint=<&hdmi_in_vopb>;
        };
    };
};

into something like this:

vopb: vopb at ff930000 {
    compatible = "rockchip,rk3288-vop", "simple-framebuffer";
    framebuffer {
        reg = <0x1d385000 (1600 * 1200 * 2)>;
        width = <1600>;
        height = <1200>;
        stride = <(1600 * 2)>;
        format = "r5g6b5";
    };
    reg = <0xff930000 0x19c>;
    interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
    clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
    resets = <&cru SRST_LCDC1_AXI>, <&cru SRST_LCDC1_AHB>, <&cru
SRST_LCDC1_DCLK>;
    reset-names = "axi", "ahb", "dclk";
    iommus = <&vopb_mmu>;
    vopb_out: port {
        #address-cells = <1>;
        #size-cells = <0>;
        vopb_out_edp: endpoint at 0 {
            reg = <0>;
            remote-endpoint=<&edp_in_vopb>;
        };
        vopb_out_hdmi: endpoint at 1 {
            reg = <1>;
            remote-endpoint=<&hdmi_in_vopb>;
        };
    };
};

And if the clocks are output port specific, we could add some lines
like "simple-framebuffer,ignore-node" to it so simplefb doesn't enable
HDMI clocks when we're using a build-in LCD.

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/