On Mon, Sep 23, 2019 at 10:33 AM Paul Kocialkowski paul.kocialkowski@bootlin.com wrote:
Hi,
On Fri 13 Sep 19, 20:16, Rob Herring wrote:
On Fri, Sep 13, 2019 at 4:58 PM Paul Kocialkowski paul.kocialkowski@bootlin.com wrote:
Hi Rob and thanks for the review!
On Fri 13 Sep 19, 15:35, Rob Herring wrote:
On Tue, Sep 10, 2019 at 05:34:08PM +0200, Paul Kocialkowski wrote:
The Xylon LogiCVC is a display controller implemented as programmable logic in Xilinx FPGAs.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com
.../bindings/display/xylon,logicvc.txt | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xylon,logicvc.txt
Consider converting this to DT schema format. See Documentation/devicetree/writing-schema.rst (.md in 5.3).
Oh right, that would certainly be much more future-proof!
diff --git a/Documentation/devicetree/bindings/display/xylon,logicvc.txt b/Documentation/devicetree/bindings/display/xylon,logicvc.txt new file mode 100644 index 000000000000..eb4b1553888a --- /dev/null +++ b/Documentation/devicetree/bindings/display/xylon,logicvc.txt @@ -0,0 +1,188 @@ +Xylon LogiCVC display controller
+The Xylon LogiCVC is a display controller that supports multiple layers. +It is usually implemented as programmable logic and was optimized for use +with Xilinx Zynq-7000 SoCs and Xilinx FPGAs.
+Because the controller is intended for use in a FPGA, most of the configuration +of the controller takes place at logic configuration bitstream synthesis time. +As a result, many of the device-tree bindings are meant to reflect the +synthesis configuration. These do not allow configuring the controller +differently than synthesis configuration.
+Layers are declared in the "layers" sub-node and have dedicated configuration. +In version 3 of the controller, each layer has fixed memory offset and address +starting from the video memory base address for its framebuffer. With version 4, +framebuffers are configured with a direct memory address instead.
+Matching synthesis parameters are provided when applicable.
+Required properties: +- compatible: Should be one of:
- "xylon,logicvc-3.02.a-display"
- "xylon,logicvc-4.01.a-display"
+- reg: Physical base address and size for the controller registers. +- clocks: List of phandle and clock-specifier pairs, one for each entry
- in 'clock-names'
+- clock-names: List of clock names that should at least contain:
- "vclk": The VCLK video clock input.
+- interrupts: The interrupt to use for VBLANK signaling. +- xylon,display-interface: Display interface in use, should be one of:
- "lvds-4bits": 4-bit LVDS interface (C_DISPLAY_INTERFACE == 4).
+- xylon,display-colorspace: Display output colorspace in use, should be one of:
- "rgb": RGB colorspace (C_DISPLAY_COLOR_SPACE == 0).
+- xylon,display-depth: Display output depth in use (C_PIXEL_DATA_WIDTH). +- xylon,row-stride: Fixed number of pixels in a framebuffer row (C_ROW_STRIDE). +- xylon,layers-count: The number of available layers (C_NUM_OF_LAYERS).
Presumably some of this is determined by the display attached. Isn't it safe to assume the IP was configured correctly for the intended display and you can just get this from the panel?
Layers are what corresponds to DRM planes, which are not actually indicated by the panel but are a charasteristic of the display controller. In our case, this is directly selected at bitstream synthesis time for the controller.
So I'm afraid there is no way we can auto-detect this from the driver.
Sorry, I referring to the set of properties above. In particular, xylon,display-interface and xylon,display-colorspace, though I don't know if the latter is talking in memory format or on the wire format.
Both of these are about the wire format, which is also "hardcoded" at synthesis time with no way to be detected afterwards, as far as I know. Memory format is described in the layer sub-nodes.
You have to attach the controller to something at the other end of the wire. A panel is only going to support 1 or a few wire formats, so you do likely know because the panel knows. In the case that a panel supports multiple wire formats, we do have some standard properties there. See the LVDS panel binding.
Actually for xylon,layers-count, You should just count the child nodes of 'layers'.
Oh that's a good point, thanks!
+Optional properties: +- memory-region: phandle to a node describing memory, as specified in:
- Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+- clock-names: List of clock names that can optionally contain:
- "vclk2": The VCLK2 doubled-rate video clock input.
- "lvdsclk": The LVDS clock.
- "lvdsclkn": The LVDS clock inverted.
How are these really optional?
Well, the controller currently only supports LVDS, but more interfaces may be added later, so the lvdsclk clock will be optional when another interface is used instead. Maybe I'm mistaken about how to categorize them though.
My understanding is that the need for vclk2 and lvdsclkn depend on the target FPGA family. I've developped the driver without the need for them, but the datasheet states that they may be needed (but doesn't provide significant details about their role though).
Not sure what to tell you then. You'll see it becomes a bit messy to describe in schema. Ideally we define the exact number, order, and values possible (or sets of those).
I'll try to do my best.
+- xylon,syscon: Syscon phandle representing the logicvc instance. +- xylon,dithering: Dithering module is enabled (C_XCOLOR). +- xylon,background-layer: The last layer is used to display a black background
- (C_USE_BACKGROUND). It must still be registered.
+- xylon,layers-configurable: Configuration of layers' size, position and offset
- is enabled (C_USE_SIZE_POSITION).
I would think this will effectively have to be enabled to make this usable with DRM. I'm not sure if a "standard" userspace would use any of the layers if all this is fixed.
I was going with the same assumption, but drm_atomic_helper_check_plane_state has a can_position parameter, which will check that the plane covers the whole CRTC if set to false. So I guess it is somewhat expected that this can be the case and some drivers (e.g. arm/hdlcd_crtc.c) also set this to false.
Certainly atomic can fail on anything not supported. My question is more whether userspace has some minimum requirements. A cursor couldn't deal with can_position=false for example.
Right, so I suppose that using an overlay plane as cursor wouldn't work in this situation. Well, I haven't found any formal definition of what minimal requirements are expected from overlay planes. I would expect userspace that tries to use an overlay plane as a cursor to have a software fallback as soon as something goes wrong. My feeling is that overlay planes are provided on a "best-effort" basis, though contradiction is welcome here.
For sure, there's always a software fallback. While we shouldn't let a specific OS's requirements dictate DT bindings, I just wonder if some of the configuration ends up always having to be set a certain way. Clearly, you could be writing the whole software stack and do a fixed configuration, but would you still be using DT at that point?
Rob