From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
Further information can be obtained from https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/u...
Ong, Hean Loong (3): dt-bindings: display: Intel FPGA VIP drm driver Devicetree bindings ARM: drm: Intel FPGA VIP Frame Buffer II drm driver ARM: socfpga: drm driver updates in socfpga_defconfig
.../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++ arch/arm/configs/socfpga_defconfig | 4 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ivip/Kconfig | 13 ++ drivers/gpu/drm/ivip/Makefile | 9 + drivers/gpu/drm/ivip/intel_vip_conn.c | 96 ++++++++++ drivers/gpu/drm/ivip/intel_vip_core.c | 171 ++++++++++++++++++ drivers/gpu/drm/ivip/intel_vip_drv.h | 55 ++++++ drivers/gpu/drm/ivip/intel_vip_of.c | 195 +++++++++++++++++++++ 10 files changed, 576 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt create mode 100644 drivers/gpu/drm/ivip/Kconfig create mode 100644 drivers/gpu/drm/ivip/Makefile create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel Arria10 devkit and its variants. Vendor name retained as altr.
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com --- v2: * Moved Device Tree bindings to Documentation/devicetree/bindings/display/ * Added vendor name altr, to description --- .../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt new file mode 100644 index 0000000..bdffefb --- /dev/null +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt @@ -0,0 +1,30 @@ +Intel Video and Image Processing(VIP) Frame Buffer II bindings + +Supported hardware: Arria 10 and above with display port IP + +The drm driver for the Arria 10 devkit would require the display resolution +and pixel information to be included as these values are generated based +on the FPGA design that drives the video connector attached to the drm driver +Information the FPGA video IP component can be acquired from +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/u... + +Required properties: + +- compatible: "altr,vip-frame-buffer-2.0" +- reg: Physical base address and length of the framebuffer controller's + registers. +- altr,max-width: The width of the framebuffer in pixels. +- altr,max-height: The height of the framebuffer in pixels. +- altr,bits-per-symbol: only "8" is currently supported +- altr,mem-port-width = the bus width of the avalon master port on the frame reader + +Example: + + dp_0_frame_buf: vip@100000280 { + compatible = "altr,vip-frame-buffer-2.0"; + reg = <0x00000001 0x00000280 0x00000040>; + altr,max-width = <1280>; + altr,max-height = <720>; + altr,bits-per-symbol = <8>; + altr,mem-port-width = <128>; + };
On Tue, Apr 25, 2017 at 10:06:44AM +0800, hean.loong.ong@intel.com wrote:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel Arria10 devkit and its variants. Vendor name retained as altr.
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
v2:
- Moved Device Tree bindings to Documentation/devicetree/bindings/display/
- Added vendor name altr, to description
.../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt new file mode 100644 index 0000000..bdffefb --- /dev/null +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt @@ -0,0 +1,30 @@ +Intel Video and Image Processing(VIP) Frame Buffer II bindings
+Supported hardware: Arria 10 and above with display port IP
+The drm driver for the Arria 10 devkit would require the display resolution
Bindings describe h/w. DRM driver is a Linux term.
+and pixel information to be included as these values are generated based +on the FPGA design that drives the video connector attached to the drm driver +Information the FPGA video IP component can be acquired from +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/u...
+Required properties:
+- compatible: "altr,vip-frame-buffer-2.0" +- reg: Physical base address and length of the framebuffer controller's
- registers.
+- altr,max-width: The width of the framebuffer in pixels. +- altr,max-height: The height of the framebuffer in pixels. +- altr,bits-per-symbol: only "8" is currently supported
Supported in the driver or IP? The former isn't relevant to the binding. In the latter case, you don't need it if that's the only thing supported.
+- altr,mem-port-width = the bus width of the avalon master port on the frame reader
In bits or bytes?
+Example:
- dp_0_frame_buf: vip@100000280 {
compatible = "altr,vip-frame-buffer-2.0";reg = <0x00000001 0x00000280 0x00000040>;altr,max-width = <1280>;altr,max-height = <720>;altr,bits-per-symbol = <8>;altr,mem-port-width = <128>;- };
-- 2.7.4
On Fri, 2017-04-28 at 13:32 -0500, Rob Herring wrote:
On Tue, Apr 25, 2017 at 10:06:44AM +0800, hean.loong.ong@intel.com wrote:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel Arria10 devkit and its variants. Vendor name retained as altr.
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
v2:
- Moved Device Tree bindings to
Documentation/devicetree/bindings/display/
- Added vendor name altr, to description
.../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
diff --git a/Documentation/devicetree/bindings/display/altr,vip- fb2.txt b/Documentation/devicetree/bindings/display/altr,vip- fb2.txt new file mode 100644 index 0000000..bdffefb --- /dev/null +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt @@ -0,0 +1,30 @@ +Intel Video and Image Processing(VIP) Frame Buffer II bindings
+Supported hardware: Arria 10 and above with display port IP
+The drm driver for the Arria 10 devkit would require the display resolution
Bindings describe h/w. DRM driver is a Linux term.
Noted.
+and pixel information to be included as these values are generated based +on the FPGA design that drives the video connector attached to the drm driver +Information the FPGA video IP component can be acquired from +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/li terature/ug/ug_vip.pdf
+Required properties:
+- compatible: "altr,vip-frame-buffer-2.0" +- reg: Physical base address and length of the framebuffer controller's + registers. +- altr,max-width: The width of the framebuffer in pixels. +- altr,max-height: The height of the framebuffer in pixels. +- altr,bits-per-symbol: only "8" is currently supported
Supported in the driver or IP? The former isn't relevant to the binding. In the latter case, you don't need it if that's the only thing supported.
Since the device is an FPGA the values here are based on how the FPGA HW design is created or programmed. The values here are the optimal reference design proposed for the Intel Arria10 devkit. However anyone that uses the Intel Arria10 devkit could create a device that runs with a different resolution that has varying values but with the condition that they need to fill these values accordingly with Intel Quartus Programmer tools. Once programmed the parameters could not be changed at runtime and the HW rerence designs currently only support 1 type of resolution per design. Therefore the driver needs to support a hardware with varying parameters programmed specifically into the FPGA.
+- altr,mem-port-width = the bus width of the avalon master port on the frame reader
In bits or bytes?
+Example:
- dp_0_frame_buf: vip@100000280 {
compatible = "altr,vip-frame-buffer-2.0";reg = <0x00000001 0x00000280 0x00000040>;altr,max-width = <1280>;altr,max-height = <720>;altr,bits-per-symbol = <8>;altr,mem-port-width = <128>;- };
On 04/25/2017 04:06 AM, hean.loong.ong@intel.com wrote:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel Arria10 devkit and its variants. Vendor name retained as altr.
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
v2:
- Moved Device Tree bindings to Documentation/devicetree/bindings/display/
- Added vendor name altr, to description
.../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt new file mode 100644 index 0000000..bdffefb --- /dev/null +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt @@ -0,0 +1,30 @@ +Intel Video and Image Processing(VIP) Frame Buffer II bindings
+Supported hardware: Arria 10 and above with display port IP
Hi,
+The drm driver for the Arria 10 devkit would require the display resolution +and pixel information to be included as these values are generated based +on the FPGA design that drives the video connector attached to the drm driver +Information the FPGA video IP component can be acquired from +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/u...
The bindings should not reference the driver, but only the hardware and the way it is configured and interconnected with the system. Please explicit that this is an IP component that can be configured with explicit limits on the display widths and heights and memory parameters.
But you should also indicate over what this IP is connected and add a port connected to a connector node in case this ip is not used on a specific board like in [1] or [2] :
""" Required nodes:
The connections to the DU output video ports are modeled using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
The following table lists for each supported model the port number corresponding to each DU output.
Port 0 Port1 Port2 Port3 ----------------------------------------------------------------------------- R8A7779 (H1) DPAD 0 DPAD 1 - - """
You may also need to add a "Display Port" connector binding aswell along the HDMI, VGA, ....
I know this targets an FPGA system, so you should explicit that in the description.
+Required properties:
+- compatible: "altr,vip-frame-buffer-2.0"
You should also add model specific compatible strings for each supported FPGA devices.
+- reg: Physical base address and length of the framebuffer controller's
- registers.
+- altr,max-width: The width of the framebuffer in pixels. +- altr,max-height: The height of the framebuffer in pixels. +- altr,bits-per-symbol: only "8" is currently supported +- altr,mem-port-width = the bus width of the avalon master port on the frame reader
What is the avalon master port ? Can you add a schema to explicit how this IP is connected to the system ?
+Example:
- dp_0_frame_buf: vip@100000280 {
compatible = "altr,vip-frame-buffer-2.0";reg = <0x00000001 0x00000280 0x00000040>;altr,max-width = <1280>;altr,max-height = <720>;altr,bits-per-symbol = <8>;altr,mem-port-width = <128>;- };
[1] Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt [2] Documentation/devicetree/bindings/display/renesas,du.txt
Thanks,
Neil
Thanks Neil.
On Thu, 2017-05-04 at 09:55 +0200, Neil Armstrong wrote:
On 04/25/2017 04:06 AM, hean.loong.ong@intel.com wrote:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel Arria10 devkit and its variants. Vendor name retained as altr.
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
v2:
- Moved Device Tree bindings to
Documentation/devicetree/bindings/display/
- Added vendor name altr, to description
.../devicetree/bindings/display/altr,vip-fb2.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
diff --git a/Documentation/devicetree/bindings/display/altr,vip- fb2.txt b/Documentation/devicetree/bindings/display/altr,vip- fb2.txt new file mode 100644 index 0000000..bdffefb --- /dev/null +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt @@ -0,0 +1,30 @@ +Intel Video and Image Processing(VIP) Frame Buffer II bindings
+Supported hardware: Arria 10 and above with display port IP
Hi,
+The drm driver for the Arria 10 devkit would require the display resolution +and pixel information to be included as these values are generated based +on the FPGA design that drives the video connector attached to the drm driver +Information the FPGA video IP component can be acquired from +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/li terature/ug/ug_vip.pdf
The bindings should not reference the driver, but only the hardware and the way it is configured and interconnected with the system. Please explicit that this is an IP component that can be configured with explicit limits on the display widths and heights and memory parameters.
But you should also indicate over what this IP is connected and add a port connected to a connector node in case this ip is not used on a specific board like in [1] or [2] :
""" Required nodes:
The connections to the DU output video ports are modeled using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
The following table lists for each supported model the port number corresponding to each DU output.
Port 0 Port1 Port2Port3
R8A7779 (H1) DPAD 0 DPAD 1 -
"""
You may also need to add a "Display Port" connector binding aswell along the HDMI, VGA, ....
I know this targets an FPGA system, so you should explicit that in the description.
+Required properties:
+- compatible: "altr,vip-frame-buffer-2.0"
You should also add model specific compatible strings for each supported FPGA devices.
+- reg: Physical base address and length of the framebuffer controller's + registers. +- altr,max-width: The width of the framebuffer in pixels. +- altr,max-height: The height of the framebuffer in pixels. +- altr,bits-per-symbol: only "8" is currently supported +- altr,mem-port-width = the bus width of the avalon master port on the frame reader
What is the avalon master port ?
A memory bus that on the FPGA that interfaces with the ARM processor
Can you add a schema to explicit how this IP is connected to the system ?
I could like to include a more detailed schema for the next updated patch. In the mean time the details of the IP is provided on the links below: https://www.altera.com/products/intellectual-property/ip/interface-prot ocols/m-alt-displayport-megacore.html
https://www.altera.com/documentation/yru1480906794402.html
but basically the idea is: ARM/Linux -->FPGA(Avalon-MM interface)-->DisplayPort Connector
+Example:
- dp_0_frame_buf: vip@100000280 {
compatible = "altr,vip-frame-buffer-2.0";reg = <0x00000001 0x00000280 0x00000040>;altr,max-width = <1280>;altr,max-height = <720>;altr,bits-per-symbol = <8>;altr,mem-port-width = <128>;- };
[1] Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt [2] Documentation/devicetree/bindings/display/renesas,du.txt
Thanks,
Neil
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II. The driver only supports the Intel Arria10 devkit and its variants. This driver can be either loaded staticlly or in modules. The OF device tree binding is located at: Documentation/devicetree/bindings/display/altr,vip-fb2.txt
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com --- v2: * Simplify the driver by using drm_simple_display_pipe_init. * Cleaned up some unused codes with no-ops * Clean up Kconfig to use only DRM_IVIP * Use DRM_GEM_CMA_FOPS for file operations * Removed the use of fb_info to populate DT information --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/ivip/Kconfig | 13 +++ drivers/gpu/drm/ivip/Makefile | 9 ++ drivers/gpu/drm/ivip/intel_vip_conn.c | 96 +++++++++++++++++ drivers/gpu/drm/ivip/intel_vip_core.c | 171 +++++++++++++++++++++++++++++ drivers/gpu/drm/ivip/intel_vip_drv.h | 55 ++++++++++ drivers/gpu/drm/ivip/intel_vip_of.c | 195 ++++++++++++++++++++++++++++++++++ 8 files changed, 542 insertions(+) create mode 100644 drivers/gpu/drm/ivip/Kconfig create mode 100644 drivers/gpu/drm/ivip/Makefile create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 78d7fc0..bc03c938 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -195,6 +195,8 @@ source "drivers/gpu/drm/nouveau/Kconfig"
source "drivers/gpu/drm/i915/Kconfig"
+source "drivers/gpu/drm/ivip/Kconfig" + config DRM_VGEM tristate "Virtual GEM provider" depends on DRM diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 59f0f9b..7cfe899 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_DRM_AMDGPU)+= amd/amdgpu/ obj-$(CONFIG_DRM_MGA) += mga/ obj-$(CONFIG_DRM_I810) += i810/ obj-$(CONFIG_DRM_I915) += i915/ +obj-$(CONFIG_DRM_IVIP) += ivip/ obj-$(CONFIG_DRM_MGAG200) += mgag200/ obj-$(CONFIG_DRM_VC4) += vc4/ obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/ diff --git a/drivers/gpu/drm/ivip/Kconfig b/drivers/gpu/drm/ivip/Kconfig new file mode 100644 index 0000000..9a8c5ce --- /dev/null +++ b/drivers/gpu/drm/ivip/Kconfig @@ -0,0 +1,13 @@ +config DRM_IVIP + tristate "Intel FGPA Video and Image Processing" + depends on DRM && OF + select DRM_GEM_CMA_HELPER + select DRM_KMS_HELPER + select DRM_KMS_FB_HELPER + select DRM_KMS_CMA_HELPER + help + Choose this option if you have a Intel FPGA Arria 10 system + and above with a Display Port IP. This does not support legacy + Intel FPGA Cyclone V display port. Currently only single frame + buffer is supported. Note that ACPI and X_86 architecture is yet + to be supported.If M is selected the module would be called ivip. diff --git a/drivers/gpu/drm/ivip/Makefile b/drivers/gpu/drm/ivip/Makefile new file mode 100644 index 0000000..95291c6 --- /dev/null +++ b/drivers/gpu/drm/ivip/Makefile @@ -0,0 +1,9 @@ +# +# Makefile for the drm device driver. This driver provides support for the +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. + +ccflags-y := -Iinclude/drm + +obj-$(CONFIG_DRM_IVIP) += ivip.o +ivip-objs := intel_vip_of.o intel_vip_core.o \ +intel_vip_conn.o diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c new file mode 100644 index 0000000..499d3b4 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_conn.c @@ -0,0 +1,96 @@ +/* + * intel_vip_conn.c -- Intel Video and Image Processing(VIP) + * Frame Buffer II driver + * + * This driver supports the Intel VIP Frame Reader component. + * More info on the hardware can be found in the Intel Video + * and Image Processing Suite User Guide at this address + * http://www.altera.com/literature/ug/ug_vip.pdf. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * Authors: + * Ong, Hean-Loong hean.loong.ong@intel.com + * + */ + +#include <linux/init.h> +#include <linux/kernel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_plane_helper.h> + +static enum drm_connector_status +intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force) +{ + return connector_status_connected; +} + +static void intelvipfb_drm_connector_destroy(struct drm_connector *connector) +{ + drm_connector_unregister(connector); + drm_connector_cleanup(connector); +} + +static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = { + .dpms = drm_helper_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .detect = intelvipfb_drm_connector_detect, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = intelvipfb_drm_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector) +{ + struct drm_device *drm = connector->dev; + int count; + + count = drm_add_modes_noedid(connector, drm->mode_config.max_width, + drm->mode_config.max_height); + drm_set_preferred_mode(connector, drm->mode_config.max_width, + drm->mode_config.max_height); + return count; +} + +static const struct drm_connector_helper_funcs +intelvipfb_drm_connector_helper_funcs = { + .get_modes = intelvipfb_drm_connector_get_modes, +}; + +struct drm_connector * +intelvipfb_conn_setup(struct drm_device *drm) +{ + struct drm_connector *conn; + int ret; + + conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL); + if (IS_ERR(conn)) + return NULL; + + ret = drm_connector_init(drm, conn, &intelvipfb_drm_connector_funcs, + DRM_MODE_CONNECTOR_DisplayPort); + if (ret < 0) { + dev_err(drm->dev, "failed to initialize drm connector\n"); + ret = -ENOMEM; + goto error_connector_cleanup; + } + + drm_connector_helper_add(conn, &intelvipfb_drm_connector_helper_funcs); + + return conn; + +error_connector_cleanup: + drm_connector_cleanup(conn); + + return NULL; +} diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c new file mode 100644 index 0000000..4e83343 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_core.c @@ -0,0 +1,171 @@ +/* + * intel_vip_core.c -- Intel Video and Image Processing(VIP) + * Frame Buffer II driver + * + * This driver supports the Intel VIP Frame Reader component. + * More info on the hardware can be found in the Intel Video + * and Image Processing Suite User Guide at this address + * http://www.altera.com/literature/ug/ug_vip.pdf. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * Authors: + * Ong, Hean-Loong hean.loong.ong@intel.com + * + */ + +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h> + +#include "intel_vip_drv.h" + +static const u32 fbpriv_formats[] = { + DRM_FORMAT_XRGB8888, + DRM_FORMAT_RGB565 +}; + +static void intelvipfb_start_hw(void __iomem *base, resource_size_t addr) +{ + /* + * The frameinfo variable has to correspond to the size of the VIP Suite + * Frame Reader register 7 which will determine the maximum size used + * in this frameinfo + */ + + u32 frameinfo; + + frameinfo = + readl(base + INTELVIPFB_FRAME_READER) & 0x00ffffff; + writel(frameinfo, base + INTELVIPFB_FRAME_INFO); + writel(addr, base + INTELVIPFB_FRAME_START); + /* Finally set the control register to 1 to start streaming */ + writel(1, base + INTELVIPFB_CONTROL); +} + +static void intelvipfb_disable_hw(void __iomem *base) +{ + /* set the control register to 0 to stop streaming */ + writel(0, base + INTELVIPFB_CONTROL); +} + +static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = { + .fb_create = drm_fb_cma_create, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +static struct drm_mode_config_helper_funcs intelvipfb_mode_config_helpers = { + .atomic_commit_tail = drm_atomic_helper_commit_tail, +}; + +static void intelvipfb_setup_mode_config(struct drm_device *drm) +{ + drm_mode_config_init(drm); + drm->mode_config.funcs = &intelvipfb_mode_config_funcs; + drm->mode_config.helper_private = &intelvipfb_mode_config_helpers; +} + +static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *plane_state) +{ + return drm_fb_cma_prepare_fb(&pipe->plane, plane_state); +} + +static struct drm_simple_display_pipe_funcs fbpriv_funcs = { + .prepare_fb = intelvipfb_pipe_prepare_fb, +}; + +int intelvipfb_probe(struct device *dev, void __iomem *base) +{ + int retval; + struct drm_device *drm; + struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev); + struct drm_connector *connector; + + dev_set_drvdata(dev, fbpriv); + + drm = fbpriv->drm; + + intelvipfb_setup_mode_config(drm); + + connector = intelvipfb_conn_setup(drm); + if (!connector) { + dev_err(drm->dev, "Connector setup failed\n"); + goto err_mode_config; + } + + retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe, + &fbpriv_funcs, + fbpriv_formats, + ARRAY_SIZE(fbpriv_formats), + connector); + if (retval < 0) { + dev_err(drm->dev, "Cannot setup simple display pipe\n"); + goto err_mode_config; + } + + fbpriv->fbcma = drm_fbdev_cma_init(drm, PREF_BPP, + drm->mode_config.num_connector); + if (!fbpriv->fbcma) { + fbpriv->fbcma = NULL; + dev_err(drm->dev, "Failed to init FB CMA area\n"); + goto err_mode_config; + } + + drm_mode_config_reset(drm); + + intelvipfb_start_hw(base, drm->mode_config.fb_base); + + drm_dev_register(drm, 0); + + return retval; + +err_mode_config: + + drm_mode_config_cleanup(drm); + return -ENODEV; +} +EXPORT_SYMBOL_GPL(intelvipfb_probe); + +int intelvipfb_remove(struct device *dev) +{ + struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev); + struct drm_device *drm = fbpriv->drm; + + drm_dev_unregister(drm); + + if (fbpriv->fbcma) + drm_fbdev_cma_fini(fbpriv->fbcma); + + intelvipfb_disable_hw(fbpriv->base); + drm_mode_config_cleanup(drm); + + drm_dev_unref(drm); + + devm_kfree(dev, fbpriv); + + return 0; +} +EXPORT_SYMBOL_GPL(intelvipfb_remove); + +MODULE_AUTHOR("Ong, Hean-Loong hean.loong.ong@intel.com"); +MODULE_DESCRIPTION("Intel VIP Frame Buffer II driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/gpu/drm/ivip/intel_vip_drv.h b/drivers/gpu/drm/ivip/intel_vip_drv.h new file mode 100644 index 0000000..8ef83f59 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_drv.h @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2017 Intel Corporation. + * + * Intel Video and Image Processing(VIP) Frame Buffer II driver. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see http://www.gnu.org/licenses/. + * + * Authors: + * Ong, Hean-Loong hean.loong.ong@intel.com + * + */ +#ifndef _INTEL_VIP_DRV_H +#define _INTEL_VIP_DRV_H +#include <linux/io.h> +#include <linux/fb.h> + +#define DRIVER_NAME "intelvipfb" +#define BYTES_PER_PIXEL 4 +#define PREF_BPP 32 +#define CRTC_NUM 1 +#define CONN_NUM 1 + +/* control registers */ +#define INTELVIPFB_CONTROL 0 +#define INTELVIPFB_STATUS 0x4 +#define INTELVIPFB_INTERRUPT 0x8 +#define INTELVIPFB_FRAME_COUNTER 0xC +#define INTELVIPFB_FRAME_DROP 0x10 +#define INTELVIPFB_FRAME_INFO 0x14 +#define INTELVIPFB_FRAME_START 0x18 +#define INTELVIPFB_FRAME_READER 0x1C + +int intelvipfb_probe(struct device *dev, void __iomem *base); +int intelvipfb_remove(struct device *dev); +int intelvipfb_setup_crtc(struct drm_device *drm); +struct drm_connector *intelvipfb_conn_setup(struct drm_device *drm); + +struct intelvipfb_priv { + struct drm_simple_display_pipe pipe; + struct drm_fbdev_cma *fbcma; + struct drm_device *drm; + void __iomem *base; +}; + +#endif diff --git a/drivers/gpu/drm/ivip/intel_vip_of.c b/drivers/gpu/drm/ivip/intel_vip_of.c new file mode 100644 index 0000000..7e3dc39 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_of.c @@ -0,0 +1,195 @@ +/* + * intel_vip_of.c -- Intel Video and Image Processing(VIP) + * Frame Buffer II driver + * + * This driver supports the Intel VIP Frame Reader component. + * More info on the hardware can be found in the Intel Video + * and Image Processing Suite User Guide at this address + * http://www.altera.com/literature/ug/ug_vip.pdf. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * Authors: + * Ong, Hean-Loong hean.loong.ong@intel.com + * + */ + +#include <linux/component.h> +#include <linux/fb.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> + +#include <drm/drm_fb_helper.h> +#include <drm/drm_of.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_simple_kms_helper.h> + +#include "intel_vip_drv.h" + +DEFINE_DRM_GEM_CMA_FOPS(fops); + +static struct drm_driver intelvipfb_drm = { + .driver_features = + DRIVER_GEM | DRIVER_PRIME | + DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC, + .gem_free_object_unlocked = drm_gem_cma_free_object, + .gem_vm_ops = &drm_gem_cma_vm_ops, + .dumb_create = drm_gem_cma_dumb_create, + .dumb_map_offset = drm_gem_cma_dumb_map_offset, + .dumb_destroy = drm_gem_dumb_destroy, + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_prime_export = drm_gem_prime_export, + .gem_prime_import = drm_gem_prime_import, + .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, + .gem_prime_vmap = drm_gem_cma_prime_vmap, + .gem_prime_vunmap = drm_gem_cma_prime_vunmap, + .gem_prime_mmap = drm_gem_cma_prime_mmap, + .name = DRIVER_NAME, + .date = "20170329", + .desc = "Intel FPGA VIP SUITE", + .major = 1, + .minor = 0, + .patchlevel = 0, + .fops = &fops, +}; + +/* + * Setting up information derived from OF Device Tree Nodes + * max-width, max-height, bits per pixel, memory port width + */ + +static int intelvipfb_drm_setup(struct device *dev, + struct intelvipfb_priv *fbpriv) +{ + struct drm_device *drm = fbpriv->drm; + struct device_node *np = dev->of_node; + int mem_word_width; + int max_h, max_w; + int bpp; + int ret; + + ret = of_property_read_u32(np, "altr,max-width", &max_w); + if (ret) { + dev_err(dev, + "Missing required parameter 'altr,max-width'"); + return ret; + } + + ret = of_property_read_u32(np, "altr,max-height", &max_h); + if (ret) { + dev_err(dev, + "Missing required parameter 'altr,max-height'"); + return ret; + } + + ret = of_property_read_u32(np, "altr,bits-per-symbol", &bpp); + if (ret) { + dev_err(dev, + "Missing required parameter 'altr,bits-per-symbol'"); + return ret; + } + + ret = of_property_read_u32(np, "altr,mem-port-width", &mem_word_width); + if (ret) { + dev_err(dev, "Missing required parameter 'altr,mem-port-width '"); + return ret; + } + + if (!(mem_word_width >= 32 && mem_word_width % 32 == 0)) { + dev_err(dev, + "mem-word-width is set to %i. must be >= 32 and multiple of 32.", + mem_word_width); + return -ENODEV; + } + + drm->mode_config.min_width = 640; + drm->mode_config.min_height = 480; + drm->mode_config.max_width = max_w; + drm->mode_config.max_height = max_h; + drm->mode_config.preferred_depth = bpp * BYTES_PER_PIXEL; + + return 0; +} + +static int intelvipfb_of_probe(struct platform_device *pdev) +{ + int retval; + struct resource *reg_res; + struct intelvipfb_priv *fbpriv; + struct device *dev = &pdev->dev; + struct drm_device *drm; + + fbpriv = devm_kzalloc(dev, sizeof(*fbpriv), GFP_KERNEL); + if (!fbpriv) + return -ENOMEM; + + /*setup DRM */ + drm = drm_dev_alloc(&intelvipfb_drm, dev); + if (IS_ERR(drm)) + return PTR_ERR(drm); + + retval = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32)); + if (retval) + return -ENODEV; + + fbpriv->drm = drm; + + reg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!reg_res) + return -ENOMEM; + + fbpriv->base = devm_ioremap_resource(dev, reg_res); + + if (IS_ERR(fbpriv->base)) { + dev_err(dev, "devm_ioremap_resource failed\n"); + retval = PTR_ERR(fbpriv->base); + return -ENOMEM; + } + + intelvipfb_drm_setup(dev, fbpriv); + + dev_set_drvdata(dev, fbpriv); + + return intelvipfb_probe(dev, fbpriv->base); +} + +static int intelvipfb_of_remove(struct platform_device *pdev) +{ + return intelvipfb_remove(&pdev->dev); +} + +/* + * The name vip-frame-buffer-2.0 is derived from + * http://www.altera.com/literature/ug/ug_vip.pdf + * frame buffer IP cores section 14 + */ + +static const struct of_device_id intelvipfb_of_match[] = { + { .compatible = "altr,vip-frame-buffer-2.0" }, + {}, +}; + +MODULE_DEVICE_TABLE(of, intelvipfb_of_match); + +static struct platform_driver intelvipfb_driver = { + .probe = intelvipfb_of_probe, + .remove = intelvipfb_of_remove, + .driver = { + .name = DRIVER_NAME, + .of_match_table = intelvipfb_of_match, + }, +}; + +module_platform_driver(intelvipfb_driver);
On Tue, 25 Apr 2017, hean.loong.ong@intel.com wrote:
+++ b/drivers/gpu/drm/ivip/Makefile @@ -0,0 +1,9 @@ +# +# Makefile for the drm device driver. This driver provides support for the +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+ccflags-y := -Iinclude/drm
Just a drive-by observation, there are patches on the list removing such ccflags from existing drivers. You shouldn't need this. Just make sure all your #includes begin with <drm/.
BR, Jani.
+obj-$(CONFIG_DRM_IVIP) += ivip.o +ivip-objs := intel_vip_of.o intel_vip_core.o \ +intel_vip_conn.o
hean.loong.ong@intel.com writes:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II. The driver only supports the Intel Arria10 devkit and its variants. This driver can be either loaded staticlly or in modules. The OF device tree binding is located at: Documentation/devicetree/bindings/display/altr,vip-fb2.txt
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
I'm not sure if this driver is going to make sense as-is, if it doesn't actually do display of planes from system memory. But in case I was wrong, here's my review:
diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c new file mode 100644 index 0000000..499d3b4 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_conn.c @@ -0,0 +1,96 @@ +/*
- intel_vip_conn.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or modify it
- under the terms and conditions of the GNU General Public License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but WITHOUT
- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_plane_helper.h>
+static enum drm_connector_status +intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force) +{
- return connector_status_connected;
+}
+static void intelvipfb_drm_connector_destroy(struct drm_connector *connector) +{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
+}
+static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .detect = intelvipfb_drm_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = intelvipfb_drm_connector_destroy,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector) +{
- struct drm_device *drm = connector->dev;
- int count;
- count = drm_add_modes_noedid(connector, drm->mode_config.max_width,
drm->mode_config.max_height);- drm_set_preferred_mode(connector, drm->mode_config.max_width,
drm->mode_config.max_height);- return count;
+}
You're adding a bunch of modes here, but I don't see anywhere in the driver that you change the resolution being scanned out.
If you can't change resolution, then I'd probably use drm_gtf_mode() or something to generate just the one mode (do you have a vrefresh for it?). Also, in the simple display pipe's atomic_check, make sure that the mode chosen is the width/height you can support.
+static const struct drm_connector_helper_funcs +intelvipfb_drm_connector_helper_funcs = {
- .get_modes = intelvipfb_drm_connector_get_modes,
+};
+struct drm_connector * +intelvipfb_conn_setup(struct drm_device *drm) +{
- struct drm_connector *conn;
- int ret;
- conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL);
- if (IS_ERR(conn))
return NULL;- ret = drm_connector_init(drm, conn, &intelvipfb_drm_connector_funcs,
DRM_MODE_CONNECTOR_DisplayPort);
Are you actually outputting DisplayPort (https://en.wikipedia.org/wiki/DisplayPort)?
You probably want something else from the DRM_MODE_CONNECTOR list, or maybe just DRM_MODE_CONNECTOR_Unknown.
- if (ret < 0) {
dev_err(drm->dev, "failed to initialize drm connector\n");ret = -ENOMEM;goto error_connector_cleanup;- }
- drm_connector_helper_add(conn, &intelvipfb_drm_connector_helper_funcs);
- return conn;
+error_connector_cleanup:
- drm_connector_cleanup(conn);
- return NULL;
+} diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c new file mode 100644 index 0000000..4e83343 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_core.c @@ -0,0 +1,171 @@ +/*
- intel_vip_core.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or modify it
- under the terms and conditions of the GNU General Public License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but WITHOUT
- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h>
+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h>
+#include "intel_vip_drv.h"
+static const u32 fbpriv_formats[] = {
- DRM_FORMAT_XRGB8888,
- DRM_FORMAT_RGB565
+};
You're registering that you support this set of formats, but I don't see anything programming the hardware differently based on the format of the plane.
+static void intelvipfb_start_hw(void __iomem *base, resource_size_t addr) +{
- /*
* The frameinfo variable has to correspond to the size of the VIP Suite* Frame Reader register 7 which will determine the maximum size used* in this frameinfo*/- u32 frameinfo;
- frameinfo =
readl(base + INTELVIPFB_FRAME_READER) & 0x00ffffff;- writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
- writel(addr, base + INTELVIPFB_FRAME_START);
- /* Finally set the control register to 1 to start streaming */
- writel(1, base + INTELVIPFB_CONTROL);
+}
The addr you're passing in here is from dev->mode_config.fb_base, which is this weird sideband value from drm_fbdev_cma. It's the wrong value to use if anything else uses the KMS interfaces to change the plane -- you should be using drm_fb_cma_get_gem_addr(drm_simple_display_pipe->plane->state, 0) instead.
+static void intelvipfb_disable_hw(void __iomem *base) +{
- /* set the control register to 0 to stop streaming */
- writel(0, base + INTELVIPFB_CONTROL);
+}
These two functions should be be called from fbpriv_funcs.enable() and .disable(), not device load/unload.
+static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = {
- .fb_create = drm_fb_cma_create,
- .atomic_check = drm_atomic_helper_check,
- .atomic_commit = drm_atomic_helper_commit,
+};
+static struct drm_mode_config_helper_funcs intelvipfb_mode_config_helpers = {
- .atomic_commit_tail = drm_atomic_helper_commit_tail,
+};
+static void intelvipfb_setup_mode_config(struct drm_device *drm) +{
- drm_mode_config_init(drm);
- drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
- drm->mode_config.helper_private = &intelvipfb_mode_config_helpers;
+}
+static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state)+{
- return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+}
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
- .prepare_fb = intelvipfb_pipe_prepare_fb,
+};
+int intelvipfb_probe(struct device *dev, void __iomem *base) +{
- int retval;
- struct drm_device *drm;
- struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
- struct drm_connector *connector;
- dev_set_drvdata(dev, fbpriv);
- drm = fbpriv->drm;
- intelvipfb_setup_mode_config(drm);
- connector = intelvipfb_conn_setup(drm);
- if (!connector) {
dev_err(drm->dev, "Connector setup failed\n");goto err_mode_config;- }
- retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe,
&fbpriv_funcs,fbpriv_formats,ARRAY_SIZE(fbpriv_formats),connector);- if (retval < 0) {
dev_err(drm->dev, "Cannot setup simple display pipe\n");goto err_mode_config;- }
- fbpriv->fbcma = drm_fbdev_cma_init(drm, PREF_BPP,
drm->mode_config.num_connector);- if (!fbpriv->fbcma) {
fbpriv->fbcma = NULL;dev_err(drm->dev, "Failed to init FB CMA area\n");goto err_mode_config;- }
On failure, drm_fbdev_cma_init() returns an ERR_PTR, not NULL.
Also, you're passing this PREF_BPP value here, ignoring the altr,bits-per-symbol property. That seems wrong.
On Wed, 2017-05-03 at 13:34 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II. The driver only supports the Intel Arria10 devkit and its variants. This driver can be either loaded staticlly or in modules. The OF device tree binding is located at: Documentation/devicetree/bindings/display/altr,vip-fb2.txt
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
I'm not sure if this driver is going to make sense as-is, if it doesn't actually do display of planes from system memory. But in case I was wrong, here's my review:
diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c new file mode 100644 index 0000000..499d3b4 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_conn.c @@ -0,0 +1,96 @@ +/*
- intel_vip_conn.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or
modify it
- under the terms and conditions of the GNU General Public
License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but
WITHOUT
- ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_plane_helper.h>
+static enum drm_connector_status +intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force) +{
- return connector_status_connected;
+}
+static void intelvipfb_drm_connector_destroy(struct drm_connector *connector) +{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
+}
+static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .detect = intelvipfb_drm_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = intelvipfb_drm_connector_destroy,
- .atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state =
drm_atomic_helper_connector_destroy_state, +};
+static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector) +{
- struct drm_device *drm = connector->dev;
- int count;
- count = drm_add_modes_noedid(connector, drm-
mode_config.max_width,
drm->mode_config.max_height);- drm_set_preferred_mode(connector, drm-
mode_config.max_width,
drm->mode_config.max_height);- return count;
+}
You're adding a bunch of modes here, but I don't see anywhere in the driver that you change the resolution being scanned out.
If you can't change resolution, then I'd probably use drm_gtf_mode() or something to generate just the one mode (do you have a vrefresh for it?). Also, in the simple display pipe's atomic_check, make sure that the mode chosen is the width/height you can support.
From the drivers perpective the resolution is dependant on how the FPGA Display Port Framebuffer hardwware IP is designed. I would take a look into how drm_gtf_mode() works compared to this.
+static const struct drm_connector_helper_funcs +intelvipfb_drm_connector_helper_funcs = {
- .get_modes = intelvipfb_drm_connector_get_modes,
+};
+struct drm_connector * +intelvipfb_conn_setup(struct drm_device *drm) +{
- struct drm_connector *conn;
- int ret;
- conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL);
- if (IS_ERR(conn))
return NULL;- ret = drm_connector_init(drm, conn,
&intelvipfb_drm_connector_funcs,
DRM_MODE_CONNECTOR_DisplayPort);Are you actually outputting DisplayPort (https://en.wikipedia.org/wiki/DisplayPort)?
You probably want something else from the DRM_MODE_CONNECTOR list, or maybe just DRM_MODE_CONNECTOR_Unknown.
The Physical Connector is a Display Port connector. A simple introduction of the product is found here. https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literat ure/an/an793.pdf
- if (ret < 0) {
dev_err(drm->dev, "failed to initialize drmconnector\n");
ret = -ENOMEM;goto error_connector_cleanup;- }
- drm_connector_helper_add(conn,
&intelvipfb_drm_connector_helper_funcs);
- return conn;
+error_connector_cleanup:
- drm_connector_cleanup(conn);
- return NULL;
+} diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c new file mode 100644 index 0000000..4e83343 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_core.c @@ -0,0 +1,171 @@ +/*
- intel_vip_core.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or
modify it
- under the terms and conditions of the GNU General Public
License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but
WITHOUT
- ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h>
+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h>
+#include "intel_vip_drv.h"
+static const u32 fbpriv_formats[] = {
- DRM_FORMAT_XRGB8888,
- DRM_FORMAT_RGB565
+};
You're registering that you support this set of formats, but I don't see anything programming the hardware differently based on the format of the plane.
+static void intelvipfb_start_hw(void __iomem *base, resource_size_t addr) +{
- /*
- * The frameinfo variable has to correspond to the size of
the VIP Suite
- * Frame Reader register 7 which will determine the
maximum size used
- * in this frameinfo
- */
- u32 frameinfo;
- frameinfo =
readl(base + INTELVIPFB_FRAME_READER) &0x00ffffff;
- writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
- writel(addr, base + INTELVIPFB_FRAME_START);
- /* Finally set the control register to 1 to start
streaming */
- writel(1, base + INTELVIPFB_CONTROL);
+}
The addr you're passing in here is from dev->mode_config.fb_base, which is this weird sideband value from drm_fbdev_cma. It's the wrong value to use if anything else uses the KMS interfaces to change the plane -- you should be using drm_fb_cma_get_gem_addr(drm_simple_display_pipe->plane->state, 0) instead.
Thank you for highlighting this. Will look into using drm_fb_cma_get_gem_addr(drm_simple_display_pipe->plane->state, 0)
+static void intelvipfb_disable_hw(void __iomem *base) +{
- /* set the control register to 0 to stop streaming */
- writel(0, base + INTELVIPFB_CONTROL);
+}
These two functions should be be called from fbpriv_funcs.enable() and .disable(), not device load/unload.
Since I am not supporting hotplugging here would it make a difference?
+static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = {
- .fb_create = drm_fb_cma_create,
- .atomic_check = drm_atomic_helper_check,
- .atomic_commit = drm_atomic_helper_commit,
+};
+static struct drm_mode_config_helper_funcs intelvipfb_mode_config_helpers = {
- .atomic_commit_tail = drm_atomic_helper_commit_tail,
+};
+static void intelvipfb_setup_mode_config(struct drm_device *drm) +{
- drm_mode_config_init(drm);
- drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
- drm->mode_config.helper_private =
&intelvipfb_mode_config_helpers; +}
+static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
struct drm_plane_state*plane_state) +{
- return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+}
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
- .prepare_fb = intelvipfb_pipe_prepare_fb,
+};
+int intelvipfb_probe(struct device *dev, void __iomem *base) +{
- int retval;
- struct drm_device *drm;
- struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
- struct drm_connector *connector;
- dev_set_drvdata(dev, fbpriv);
- drm = fbpriv->drm;
- intelvipfb_setup_mode_config(drm);
- connector = intelvipfb_conn_setup(drm);
- if (!connector) {
dev_err(drm->dev, "Connector setup failed\n");goto err_mode_config;- }
- retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe,
&fbpriv_funcs,fbpriv_formats,ARRAY_SIZE(fbpriv_formats),
connector);- if (retval < 0) {
dev_err(drm->dev, "Cannot setup simple displaypipe\n");
goto err_mode_config;- }
- fbpriv->fbcma = drm_fbdev_cma_init(drm, PREF_BPP,
drm-mode_config.num_connector);
- if (!fbpriv->fbcma) {
fbpriv->fbcma = NULL;dev_err(drm->dev, "Failed to init FB CMA area\n");goto err_mode_config;- }
On failure, drm_fbdev_cma_init() returns an ERR_PTR, not NULL.
Also, you're passing this PREF_BPP value here, ignoring the altr,bits-per-symbol property. That seems wrong.
Noted thanks for highlighting.
On Wed, 2017-05-03 at 13:34 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II. The driver only supports the Intel Arria10 devkit and its variants. This driver can be either loaded staticlly or in modules. The OF device tree binding is located at: Documentation/devicetree/bindings/display/altr,vip-fb2.txt
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com
I'm not sure if this driver is going to make sense as-is, if it doesn't actually do display of planes from system memory. But in case I was wrong, here's my review:
diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c new file mode 100644 index 0000000..499d3b4 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_conn.c @@ -0,0 +1,96 @@ +/*
- intel_vip_conn.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or
modify it
- under the terms and conditions of the GNU General Public
License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but
WITHOUT
- ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_plane_helper.h>
+static enum drm_connector_status +intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force) +{
- return connector_status_connected;
+}
+static void intelvipfb_drm_connector_destroy(struct drm_connector *connector) +{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
+}
+static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .detect = intelvipfb_drm_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = intelvipfb_drm_connector_destroy,
- .atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state =
drm_atomic_helper_connector_destroy_state, +};
+static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector) +{
- struct drm_device *drm = connector->dev;
- int count;
- count = drm_add_modes_noedid(connector, drm-
mode_config.max_width,
drm->mode_config.max_height);- drm_set_preferred_mode(connector, drm-
mode_config.max_width,
drm->mode_config.max_height);- return count;
+}
You're adding a bunch of modes here, but I don't see anywhere in the driver that you change the resolution being scanned out.
If you can't change resolution, then I'd probably use drm_gtf_mode() or something to generate just the one mode (do you have a vrefresh for it?). Also, in the simple display pipe's atomic_check, make sure that the mode chosen is the width/height you can support.
I can see the point in the proposed approach.Based on the comment for get_modes in drm_connector_helper_funcs the recommended approach was using drm_add_modes_noedid and drm_set_preferred_mode. Not many drivers uses the drm_gtf_mode directly therefore I am not inclined to this approach.
+static const struct drm_connector_helper_funcs +intelvipfb_drm_connector_helper_funcs = {
- .get_modes = intelvipfb_drm_connector_get_modes,
+};
+struct drm_connector * +intelvipfb_conn_setup(struct drm_device *drm) +{
- struct drm_connector *conn;
- int ret;
- conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL);
- if (IS_ERR(conn))
return NULL;- ret = drm_connector_init(drm, conn,
&intelvipfb_drm_connector_funcs,
DRM_MODE_CONNECTOR_DisplayPort);Are you actually outputting DisplayPort (https://en.wikipedia.org/wiki/DisplayPort)?
You probably want something else from the DRM_MODE_CONNECTOR list, or maybe just DRM_MODE_CONNECTOR_Unknown.
- if (ret < 0) {
dev_err(drm->dev, "failed to initialize drmconnector\n");
ret = -ENOMEM;goto error_connector_cleanup;- }
- drm_connector_helper_add(conn,
&intelvipfb_drm_connector_helper_funcs);
- return conn;
+error_connector_cleanup:
- drm_connector_cleanup(conn);
- return NULL;
+} diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c new file mode 100644 index 0000000..4e83343 --- /dev/null +++ b/drivers/gpu/drm/ivip/intel_vip_core.c @@ -0,0 +1,171 @@ +/*
- intel_vip_core.c -- Intel Video and Image Processing(VIP)
- Frame Buffer II driver
- This driver supports the Intel VIP Frame Reader component.
- More info on the hardware can be found in the Intel Video
- and Image Processing Suite User Guide at this address
- This program is free software; you can redistribute it and/or
modify it
- under the terms and conditions of the GNU General Public
License,
- version 2, as published by the Free Software Foundation.
- This program is distributed in the hope it will be useful, but
WITHOUT
- ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
License for
- more details.
- Authors:
- Ong, Hean-Loong hean.loong.ong@intel.com
- */
+#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h>
+#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <drm/drm_simple_kms_helper.h>
+#include "intel_vip_drv.h"
+static const u32 fbpriv_formats[] = {
- DRM_FORMAT_XRGB8888,
- DRM_FORMAT_RGB565
+};
You're registering that you support this set of formats, but I don't see anything programming the hardware differently based on the format of the plane.
+static void intelvipfb_start_hw(void __iomem *base, resource_size_t addr) +{
- /*
- * The frameinfo variable has to correspond to the size of
the VIP Suite
- * Frame Reader register 7 which will determine the
maximum size used
- * in this frameinfo
- */
- u32 frameinfo;
- frameinfo =
readl(base + INTELVIPFB_FRAME_READER) &0x00ffffff;
- writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
- writel(addr, base + INTELVIPFB_FRAME_START);
- /* Finally set the control register to 1 to start
streaming */
- writel(1, base + INTELVIPFB_CONTROL);
+}
The addr you're passing in here is from dev->mode_config.fb_base, which is this weird sideband value from drm_fbdev_cma. It's the wrong value to use if anything else uses the KMS interfaces to change the plane -- you should be using drm_fb_cma_get_gem_addr(drm_simple_display_pipe->plane->state, 0) instead.
The function drm_fb_cma_get_gem_addr(drm_simple_display_pipe, drm_simple_display_pipe->plane->state, 0) is causing an exception in git tag next-20170526. I have confirmed that the pipe is okay. What other configuration should I be looking into before using this function
+static void intelvipfb_disable_hw(void __iomem *base) +{
- /* set the control register to 0 to stop streaming */
- writel(0, base + INTELVIPFB_CONTROL);
+}
These two functions should be be called from fbpriv_funcs.enable() and .disable(), not device load/unload.
I agree to this. However in my recent test (git tag next-20170526) it does not seem to be triggered when the driver is loaded
+static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = {
- .fb_create = drm_fb_cma_create,
- .atomic_check = drm_atomic_helper_check,
- .atomic_commit = drm_atomic_helper_commit,
+};
+static struct drm_mode_config_helper_funcs intelvipfb_mode_config_helpers = {
- .atomic_commit_tail = drm_atomic_helper_commit_tail,
+};
+static void intelvipfb_setup_mode_config(struct drm_device *drm) +{
- drm_mode_config_init(drm);
- drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
- drm->mode_config.helper_private =
&intelvipfb_mode_config_helpers; +}
+static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
struct drm_plane_state*plane_state) +{
- return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+}
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
- .prepare_fb = intelvipfb_pipe_prepare_fb,
+};
+int intelvipfb_probe(struct device *dev, void __iomem *base) +{
- int retval;
- struct drm_device *drm;
- struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
- struct drm_connector *connector;
- dev_set_drvdata(dev, fbpriv);
- drm = fbpriv->drm;
- intelvipfb_setup_mode_config(drm);
- connector = intelvipfb_conn_setup(drm);
- if (!connector) {
dev_err(drm->dev, "Connector setup failed\n");goto err_mode_config;- }
- retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe,
&fbpriv_funcs,fbpriv_formats,ARRAY_SIZE(fbpriv_formats),
connector);- if (retval < 0) {
dev_err(drm->dev, "Cannot setup simple displaypipe\n");
goto err_mode_config;- }
- fbpriv->fbcma = drm_fbdev_cma_init(drm, PREF_BPP,
drm-mode_config.num_connector);
- if (!fbpriv->fbcma) {
fbpriv->fbcma = NULL;dev_err(drm->dev, "Failed to init FB CMA area\n");goto err_mode_config;- }
On failure, drm_fbdev_cma_init() returns an ERR_PTR, not NULL.
Also, you're passing this PREF_BPP value here, ignoring the altr,bits-per-symbol property. That seems wrong.
From: "Ong, Hean Loong" hean.loong.ong@intel.com
Intel FPGA Video and Image Processing Suite Frame Buffer II driver config for Arria 10 devkit and its variants
Signed-off-by: Ong, Hean Loong hean.loong.ong@intel.com --- v2: * Added drm frame bufferr II module support for Arria10 --- arch/arm/configs/socfpga_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig index 030264c..49ae269 100644 --- a/arch/arm/configs/socfpga_defconfig +++ b/arch/arm/configs/socfpga_defconfig @@ -110,6 +110,10 @@ CONFIG_MFD_ALTERA_A10SR=y CONFIG_MFD_STMPE=y CONFIG_REGULATOR=y CONFIG_REGULATOR_FIXED_VOLTAGE=y +CONFIG_DRM=m +CONFIG_DRM_IVIP=m +CONFIG_DRM_IVIP_OF=m +CONFIG_FB=y CONFIG_USB=y CONFIG_USB_STORAGE=y CONFIG_USB_DWC2=y
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
The docs I've read for the Frame Buffer IP don't fit into any current DRM component, since it's what we're currently calling a "writeback connector".
I don't understand your system enough to answer. You didn't answer if the "DMA" block you diagrammed was the frame reader IP (or maybe the frame reader IP comes after). I also don't see how the frame buffer IP could possibly be connected directly to a physical display connector.
On Mon, 2017-05-08 at 09:03 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
The docs I've read for the Frame Buffer IP don't fit into any current DRM component, since it's what we're currently calling a "writeback connector".
While running my own test (since the intel-gpu-tools doesn't seems applicable.) The drm_simple_display_pipe_init seems to be simple enough for displaying a matchbox console. The use case for this driver is only part of the enablemennt of the reference design board so that users of the Arria10 devkit can use this as base for their own designs.
I don't understand your system enough to answer. You didn't answer if the "DMA" block you diagrammed was the frame reader IP (or maybe the frame reader IP comes after). I also don't see how the frame buffer IP could possibly be connected directly to a physical display connector.
The FPGA FrameBuffer 2 soft IP reads the information provided to it from the Linux Kernel via the platform device register provided in the DT. The Linux driver here allocates a chunk of coherent memory that directly maps to the SDRAM. The FPGA soft IP would stream the display out to the Display Port based on the information that was written to the SDRAM.
The FrameBuffer 2 IP or previously used Frame Reader IP are soft IP is programmed as part of the FPGA which is connected to the Display Port via a native physical transceiver. The Soft IP are driven by the NIOS 2 soft core which is also the soft processor.
Note however whatever that goes beyond the the memory bus (between ARM and FPGA) known as Avalon is no longer under the control of the Linux Driver (as we only establish connection on the ARM side)
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Mon, 2017-05-08 at 09:03 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote:
hean.loong.ong@intel.com writes:
> > > > From: Ong Hean Loong hean.loong.ong@intel.com > > Hi, > > The new Intel Arria10 SOC FPGA devkit has a Display Port IP > component > which requires a new driver. This is a virtual driver in > which > the > FGPA hardware would enable the Display Port based on the > information > and data provided from the DRM frame buffer from the OS. > Basically > all > all information with reagrds to resolution and bits per > pixel > are > pre-configured on the FPGA design and these information are > fed > to > the driver via the device tree information as part of the > hardware > information. I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
The docs I've read for the Frame Buffer IP don't fit into any current DRM component, since it's what we're currently calling a "writeback connector".
While running my own test (since the intel-gpu-tools doesn't seems applicable.) The drm_simple_display_pipe_init seems to be simple enough for displaying a matchbox console. The use case for this driver is only part of the enablemennt of the reference design board so that users of the Arria10 devkit can use this as base for their own designs.
I don't understand your system enough to answer. You didn't answer if the "DMA" block you diagrammed was the frame reader IP (or maybe the frame reader IP comes after). I also don't see how the frame buffer IP could possibly be connected directly to a physical display connector.
The FPGA FrameBuffer 2 soft IP reads the information provided to it from the Linux Kernel via the platform device register provided in the DT. The Linux driver here allocates a chunk of coherent memory that directly maps to the SDRAM. The FPGA soft IP would stream the display out to the Display Port based on the information that was written to the SDRAM.
Your "FPGA soft IP" that's reading pixels from an area of memory (a DRM plane does this) and outputting that to a display port (a DRM CRTC and encoder does this) would make a lot of sense as a DRM driver.
However, this piece of hardware that takes in pixels in a stream from elsewhere and stores it to memory doesn't make sense to me as a DRM driver.
I can't really offer more advice until I understand what's generating the pixels that the FrameBuffer IP is storing.
On Tue, 2017-05-09 at 09:40 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Mon, 2017-05-08 at 09:03 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote: > > > > hean.loong.ong@intel.com writes: > > > > > > > > > > > From: Ong Hean Loong hean.loong.ong@intel.com > > > > Hi, > > > > The new Intel Arria10 SOC FPGA devkit has a Display > > Port IP > > component > > which requires a new driver. This is a virtual driver > > in > > which > > the > > FGPA hardware would enable the Display Port based on > > the > > information > > and data provided from the DRM frame buffer from the > > OS. > > Basically > > all > > all information with reagrds to resolution and bits per > > pixel > > are > > pre-configured on the FPGA design and these information > > are > > fed > > to > > the driver via the device tree information as part of > > the > > hardware > > information. > I started reviewing the code, but I want to make sure I > understand > what's going on: > > This IP core isn't displaying contents from system memory > on > some > sort > of actual physical display, right? It's a core that > takes > some > input > video stream (not described in the DT or in this driver) > and > stores > it > to memory? If the IP Core you are referring to is some form of GPU then in this case we are using the Intel FPGA Display Port Framebuffer IP. It does display contents streamed from the ARM/Linux system to the Display Port of the physical Monitor.
Below a simple illustration of the system:
ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP | | Physical Connection Display Port
The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
The docs I've read for the Frame Buffer IP don't fit into any current DRM component, since it's what we're currently calling a "writeback connector".
While running my own test (since the intel-gpu-tools doesn't seems applicable.) The drm_simple_display_pipe_init seems to be simple enough for displaying a matchbox console. The use case for this driver is only part of the enablemennt of the reference design board so that users of the Arria10 devkit can use this as base for their own designs.
I don't understand your system enough to answer. You didn't answer if the "DMA" block you diagrammed was the frame reader IP (or maybe the frame reader IP comes after). I also don't see how the frame buffer IP could possibly be connected directly to a physical display connector.
The FPGA FrameBuffer 2 soft IP reads the information provided to it from the Linux Kernel via the platform device register provided in the DT. The Linux driver here allocates a chunk of coherent memory that directly maps to the SDRAM. The FPGA soft IP would stream the display out to the Display Port based on the information that was written to the SDRAM.
Your "FPGA soft IP" that's reading pixels from an area of memory (a DRM plane does this) and outputting that to a display port (a DRM CRTC and encoder does this) would make a lot of sense as a DRM driver.
However, this piece of hardware that takes in pixels in a stream from elsewhere and stores it to memory doesn't make sense to me as a DRM driver.
I can't really offer more advice until I understand what's generating the pixels that the FrameBuffer IP is storing.
My previous explanation might have been confusing here. I apologize for that. To put it in simpler terms the FPGA FrameBuffer Soft IP could be seen as the GPU and the DRM driver patch here is allocating memory for information to be streamed from the ARM/Linux to the display port. Basically the driver just wraps the information such as the pixels to be drawn by the FPGA FrameBuffer 2.
The piece of hardware in discussion is the SoC FPGA where Linux runs on the ARM chip and the FGPA is driven by its NIOS soft core with its own proprietary firmware.
For example the application from the ARM Linux would have to write information on the /dev/fb0 with the information stored in the SDRAM to be fetched by the FPGA framebuffer IP and displayed on the Display Port Monitor.
On Thu, May 11, 2017 at 02:50:41AM +0000, Ong, Hean Loong wrote:
On Tue, 2017-05-09 at 09:40 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Mon, 2017-05-08 at 09:03 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
On Thu, 2017-05-04 at 10:11 -0700, Eric Anholt wrote:
"Ong, Hean Loong" hean.loong.ong@intel.com writes:
> > > > On Wed, 2017-05-03 at 13:28 -0700, Eric Anholt wrote: > > > > > > > > hean.loong.ong@intel.com writes: > > > > > > > > > > > > > > > > > From: Ong Hean Loong hean.loong.ong@intel.com > > > > > > Hi, > > > > > > The new Intel Arria10 SOC FPGA devkit has a Display > > > Port IP > > > component > > > which requires a new driver. This is a virtual driver > > > in > > > which > > > the > > > FGPA hardware would enable the Display Port based on > > > the > > > information > > > and data provided from the DRM frame buffer from the > > > OS. > > > Basically > > > all > > > all information with reagrds to resolution and bits per > > > pixel > > > are > > > pre-configured on the FPGA design and these information > > > are > > > fed > > > to > > > the driver via the device tree information as part of > > > the > > > hardware > > > information. > > I started reviewing the code, but I want to make sure I > > understand > > what's going on: > > > > This IP core isn't displaying contents from system memory > > on > > some > > sort > > of actual physical display, right? It's a core that > > takes > > some > > input > > video stream (not described in the DT or in this driver) > > and > > stores > > it > > to memory? > If the IP Core you are referring to is some form of GPU > then in > this > case we are using the Intel FPGA Display Port Framebuffer > IP. > It > does > display contents streamed from the ARM/Linux system to the > Display > Port > of the physical Monitor. > > Below a simple illustration of the system: > > ARM/Linux --DMA-->Intel FPGA Display Port Framebuffer IP > | > | > Physical Connection > Display Port The "DMA" in this diagram is the frame reader IP, right? The frame reader, as described in the spec, sounds approximately like a DRM plane, so if you have that in your system then that needs to be part of this DRM driver (otherwise you won't be putting the right things on the screen!).
Would the drm_simple_display_pipe_init be able to handle this ? It seems to be displaying the proper images on screen, based on my current changes. There were recommendations to use the drm_simple_display_pipe_init instead of creating the CRTC and planes myself
The docs I've read for the Frame Buffer IP don't fit into any current DRM component, since it's what we're currently calling a "writeback connector".
While running my own test (since the intel-gpu-tools doesn't seems applicable.) The drm_simple_display_pipe_init seems to be simple enough for displaying a matchbox console. The use case for this driver is only part of the enablemennt of the reference design board so that users of the Arria10 devkit can use this as base for their own designs.
I don't understand your system enough to answer. You didn't answer if the "DMA" block you diagrammed was the frame reader IP (or maybe the frame reader IP comes after). I also don't see how the frame buffer IP could possibly be connected directly to a physical display connector.
The FPGA FrameBuffer 2 soft IP reads the information provided to it from the Linux Kernel via the platform device register provided in the DT. The Linux driver here allocates a chunk of coherent memory that directly maps to the SDRAM. The FPGA soft IP would stream the display out to the Display Port based on the information that was written to the SDRAM.
Your "FPGA soft IP" that's reading pixels from an area of memory (a DRM plane does this) and outputting that to a display port (a DRM CRTC and encoder does this) would make a lot of sense as a DRM driver.
However, this piece of hardware that takes in pixels in a stream from elsewhere and stores it to memory doesn't make sense to me as a DRM driver.
I can't really offer more advice until I understand what's generating the pixels that the FrameBuffer IP is storing.
My previous explanation might have been confusing here. I apologize for that. To put it in simpler terms the FPGA FrameBuffer Soft IP could be seen as the GPU and the DRM driver patch here is allocating memory for information to be streamed from the ARM/Linux to the display port. Basically the driver just wraps the information such as the pixels to be drawn by the FPGA FrameBuffer 2.
The piece of hardware in discussion is the SoC FPGA where Linux runs on the ARM chip and the FGPA is driven by its NIOS soft core with its own proprietary firmware.
For example the application from the ARM Linux would have to write information on the /dev/fb0 with the information stored in the SDRAM to be fetched by the FPGA framebuffer IP and displayed on the Display Port Monitor.
Ah ok, this sounds like it's indeed suitable for drm. Thanks for clearing up the confusion ... -Daniel
On Wed, May 3, 2017 at 10:28 PM, Eric Anholt eric@anholt.net wrote:
From: Ong Hean Loong hean.loong.ong@intel.com
Hi,
The new Intel Arria10 SOC FPGA devkit has a Display Port IP component which requires a new driver. This is a virtual driver in which the FGPA hardware would enable the Display Port based on the information and data provided from the DRM frame buffer from the OS. Basically all all information with reagrds to resolution and bits per pixel are pre-configured on the FPGA design and these information are fed to the driver via the device tree information as part of the hardware information.
I started reviewing the code, but I want to make sure I understand what's going on:
This IP core isn't displaying contents from system memory on some sort of actual physical display, right? It's a core that takes some input video stream (not described in the DT or in this driver) and stores it to memory?
I assumed it's the input IP core in the linked pdf, i.e. the CVI (clocked video input). There's also a CVO (clocked video output), and that would indeed be a misfit for drm. Definitely need to clarify this (in the DT description). -Daniel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Eric Anholt -
hean.loong.ong＠intel.com -
Jani Nikula -
Neil Armstrong -
Ong, Hean Loong
-
Rob Herring