[PATCHv4 0/3] Intel FPGA VIP Frame Buffer II drm driver

[PATCHv4 0/3] Intel FPGA VIP Frame Buffer II drm driver

[Hean Loong, Ong]

From: Ong Hean Loong <hean.loong.ong@intel.com>

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.


Ong Hean Loong (3):
  ARM:dt-bindings Intel FPGA Video and Image Processing Suite
  ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite
  DRM:ivip Intel FPGA Video and Image Processing Suite

 .../devicetree/bindings/display/altr,vip-fb2.txt   |  39 ++++
 arch/arm/configs/socfpga_defconfig                 |   6 +
 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              | 183 ++++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h               |  54 ++++++
 drivers/gpu/drm/ivip/intel_vip_of.c                | 204 +++++++++++++++++++++
 10 files changed, 607 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

-- 
2.7.4

[PATCHv4 1/3] ARM:dt-bindings Intel FPGA Video and Image Processing Suite

[Hean Loong, Ong]

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>
---
 .../devicetree/bindings/display/altr,vip-fb2.txt   | 39 ++++++++++++++++++++++
 1 file changed, 39 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..c4338d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
@@ -0,0 +1,39 @@
+Intel Video and Image Processing(VIP) Frame Buffer II bindings
+
+Supported hardware: Arria 10 and above with display port IP
+
+The hardware associated with this device tree is a SoC FPGA. Where there is an ARM controller
+and a FPGA device. The ARM controller would host the Linux OS while the FPGA device runs on its
+individual IP firmware. In the Intel VIP Frame Buffer II the ARM controller would be
+driving data from the Linux OS to the FPGA device programmed with the Frame Buffer II IP
+to render pixels to be streamed to the Display Port connector.
+
+The Frame Buffer II device is a simple frame buffer device. The device contains the display
+properties and the bridge or connector register. The output for this device currently
+is a dedicated to a single Display Port. Currently the max resolution supported is 1280 x 720 at
+60Hz.
+
+More information the FPGA video IP component can be acquired from
+https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_vip.pdf
+
+
+New bindings:
+=============
+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,mem-port-width = the bus width of the avalon master port on the frame reader
+
+Example:
+----------------------------
+       dp_0_frame_buf: display-controller@100000280 {
+                       compatible = "altr,vip-frame-buffer-2.0";
+                       reg = <0x00000001 0x00000280 0x00000040>;
+                       altr,max-width = <1280>;
+                       altr,max-height = <720>;
+                       altr,mem-port-width = <128>;
+       };
-- 
2.7.4

[PATCHv4 2/3] ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite

[Hean Loong, Ong]

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>
---
 arch/arm/configs/socfpga_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 2620ce7..255f553 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -111,6 +111,12 @@ 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_FB_SIMPLE=y
+CONFIG_FRAMEBUFFER_CONSOLE=m
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_DWC2=y
-- 
2.7.4

[PATCHv4 3/3] DRM:ivip Intel FPGA Video and Image Processing Suite

[Hean Loong, Ong]

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>
---
V3:
*Changes to fixing drm_simple_pipe
*Used drm_fb_cma_get_gem_addr

V2:
*Adding drm_simple_display_pipe_init
---
 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 | 183 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h  |  54 +++++++++
 drivers/gpu/drm/ivip/intel_vip_of.c   | 204 ++++++++++++++++++++++++++++++++++
 8 files changed, 562 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 83cb2a8..38a184d 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 24a066e..4162a0e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -58,6 +58,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..063d91e
--- /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_atomic_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..ea85715
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_core.c
@@ -0,0 +1,183 @@
+/*
+ * 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 void intelvipfb_enable(struct drm_simple_display_pipe *pipe,
+	struct drm_crtc_state *crtc_state)
+{
+	/*
+	 * 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;
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->base;
+	struct drm_plane_state *state = pipe->plane.state;
+	dma_addr_t addr;
+
+	addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
+
+	dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
+
+	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(struct drm_simple_display_pipe *pipe)
+{
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->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 void intelvipfb_setup_mode_config(struct drm_device *drm)
+{
+	drm_mode_config_init(drm);
+	drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
+}
+
+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 void intelvipfb_update(struct drm_simple_display_pipe *pipe,
+		struct drm_plane_state *old_state)
+{
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
+
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
+	.prepare_fb = intelvipfb_pipe_prepare_fb,
+	.update = intelvipfb_update,
+	.enable = intelvipfb_enable,
+	.disable = intelvipfb_disable
+};
+
+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;
+
+	u32 formats[] = {DRM_FORMAT_XRGB8888};
+
+	dev_set_drvdata(dev, fbpriv);
+
+	drm = fbpriv->drm;
+
+	drm->dev_private = fbpriv;
+
+	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, formats,
+		ARRAY_SIZE(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,
+		drm->mode_config.preferred_depth,
+		drm->mode_config.num_connector);
+
+	drm_mode_config_reset(drm);
+
+	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);
+
+	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..ebdbd50
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_drv.h
@@ -0,0 +1,54 @@
+/*
+ * 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 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..5a7c63b
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_of.c
@@ -0,0 +1,204 @@
+/*
+ * 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(drm_fops);
+
+static void intelvipfb_lastclose(struct drm_device *drm)
+{
+	struct intelvipfb_priv *priv = drm->dev_private;
+
+	drm_fbdev_cma_restore_mode(priv->fbcma);
+}
+
+static struct drm_driver intelvipfb_drm = {
+	.driver_features =
+		DRIVER_MODESET | DRIVER_GEM |
+		DRIVER_PRIME | 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,
+	.lastclose = intelvipfb_lastclose,
+	.name = DRIVER_NAME,
+	.date = "20170729",
+	.desc = "Intel FPGA VIP SUITE",
+	.major = 1,
+	.minor = 0,
+	.ioctls = NULL,
+	.patchlevel = 0,
+	.fops = &drm_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);
-- 
2.7.4

Re: [PATCHv4 3/3] DRM:ivip Intel FPGA Video and Image Processing Suite

[Laurent Pinchart]

Hi Hean Loong,

Thank you for the patch.

On Tuesday 01 Aug 2017 10:31:34 Hean Loong, Ong wrote:
> 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>
> ---
> V3:
> *Changes to fixing drm_simple_pipe
> *Used drm_fb_cma_get_gem_addr
> 
> V2:
> *Adding drm_simple_display_pipe_init
> ---
>  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 | 183 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/ivip/intel_vip_drv.h  |  54 +++++++++
>  drivers/gpu/drm/ivip/intel_vip_of.c   | 204 +++++++++++++++++++++++++++++++
>  8 files changed, 562 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

[snip]

> 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.

I think this should be fixed to upstream this driver.

> Note that ACPI and X_86 architecture is yet

ACPI on FPGA... I wonder if it could get any crazier than that :-)

> +	     to be supported.If M is selected the module would be called ivip.

s/.If/. If/

[snip]

> 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..ea85715
> --- /dev/null
> +++ b/drivers/gpu/drm/ivip/intel_vip_core.c
> @@ -0,0 +1,183 @@
> +/*
> + * 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 void intelvipfb_enable(struct drm_simple_display_pipe *pipe,
> +	struct drm_crtc_state *crtc_state)
> +{
> +	/*
> +	 * 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;
> +	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
> +	void __iomem *base = priv->base;
> +	struct drm_plane_state *state = pipe->plane.state;
> +	dma_addr_t addr;
> +
> +	addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> +
> +	dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
> +
> +	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(struct drm_simple_display_pipe *pipe)
> +{
> +	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
> +	void __iomem *base = priv->base;

Missing blank line.

> +	/* 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 void intelvipfb_setup_mode_config(struct drm_device *drm)
> +{
> +	drm_mode_config_init(drm);
> +	drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
> +}
> +
> +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 void intelvipfb_update(struct drm_simple_display_pipe *pipe,
> +		struct drm_plane_state *old_state)
> +{
> +	struct drm_crtc *crtc = &pipe->crtc;
> +
> +	if (crtc->state->event) {
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +		crtc->state->event = NULL;
> +	}

This is not quite right. Userspace expects the driver to perform a page flip 
here, and you just signal page flip completion without doing anything.

> +}
> +
> +static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
> +	.prepare_fb = intelvipfb_pipe_prepare_fb,
> +	.update = intelvipfb_update,
> +	.enable = intelvipfb_enable,
> +	.disable = intelvipfb_disable
> +};
> +
> +int intelvipfb_probe(struct device *dev, void __iomem *base)

You don't use the base argument, you can remove it.

> +{
> +	int retval;
> +	struct drm_device *drm;
> +	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
> +	struct drm_connector *connector;
> +

Extra blank line.

> +	u32 formats[] = {DRM_FORMAT_XRGB8888};
> +
> +	dev_set_drvdata(dev, fbpriv);

As fbpriv is obtained by a call to dev_get_drvdata(), this isn't needed.

A better option would be to pass the intelvipfb_priv pointer as an argument to 
this function.

> +	drm = fbpriv->drm;
> +
> +	drm->dev_private = fbpriv;
> +
> +	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, formats,
> +		ARRAY_SIZE(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,
> +		drm->mode_config.preferred_depth,
> +		drm->mode_config.num_connector);
> +
> +	drm_mode_config_reset(drm);
> +
> +	drm_dev_register(drm, 0);
> +
> +	return retval;
> +
> +err_mode_config:
> +
> +	drm_mode_config_cleanup(drm);
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(intelvipfb_probe);

Why do you need to export this symbol ?

> +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);
> +
> +	drm_mode_config_cleanup(drm);
> +
> +	drm_dev_unref(drm);
> +
> +	devm_kfree(dev, fbpriv);

The whole point of devm_kzalloc() is that you don't need to free the memory at 
remove time.

> +
> +	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..ebdbd50
> --- /dev/null
> +++ b/drivers/gpu/drm/ivip/intel_vip_drv.h
> @@ -0,0 +1,54 @@
> +/*
> + * 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

Missing blank line.

> +#include <linux/io.h>
> +#include <linux/fb.h>

I don't think this header is needed.

> +#define DRIVER_NAME    "intelvipfb"
> +#define BYTES_PER_PIXEL        4
> +#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..5a7c63b
> --- /dev/null
> +++ b/drivers/gpu/drm/ivip/intel_vip_of.c
> @@ -0,0 +1,204 @@
> +/*
> + * 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>

I don't think those two headers are needed.

> +#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>

Please sort these alphabetically.

> +
> +#include "intel_vip_drv.h"
> +
> +DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
> +
> +static void intelvipfb_lastclose(struct drm_device *drm)
> +{
> +	struct intelvipfb_priv *priv = drm->dev_private;
> +
> +	drm_fbdev_cma_restore_mode(priv->fbcma);
> +}
> +
> +static struct drm_driver intelvipfb_drm = {
> +	.driver_features =
> +		DRIVER_MODESET | DRIVER_GEM |
> +		DRIVER_PRIME | 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,
> +	.lastclose = intelvipfb_lastclose,
> +	.name = DRIVER_NAME,
> +	.date = "20170729",
> +	.desc = "Intel FPGA VIP SUITE",
> +	.major = 1,
> +	.minor = 0,
> +	.ioctls = NULL,
> +	.patchlevel = 0,
> +	.fops = &drm_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);

This property is not documented in patch 1/3.

> +	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;
> +}

-- 
Regards,

Laurent Pinchart

Re: [PATCHv4 1/3] ARM:dt-bindings Intel FPGA Video and Image Processing Suite

[Laurent Pinchart]

Hi Hean Loong,

Thank you for the patch.

On Tuesday 01 Aug 2017 10:31:32 Hean Loong, Ong 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>
> ---
>  .../devicetree/bindings/display/altr,vip-fb2.txt   | 39 ++++++++++++++++++
>  1 file changed, 39 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..c4338d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> @@ -0,0 +1,39 @@
> +Intel Video and Image Processing(VIP) Frame Buffer II bindings
> +
> +Supported hardware: Arria 10 and above with display port IP
> +
> +The hardware associated with this device tree is a SoC FPGA. Where there is
> an ARM controller +and a FPGA device. The ARM controller would host the
> Linux OS while the FPGA device runs on its +individual IP firmware.

DT bindings should not be OS-specific, you shouldn't mention Linux here.

> In the
> Intel VIP Frame Buffer II the ARM controller would be +driving data from
> the Linux OS to the FPGA device programmed with the Frame Buffer II IP +to
> render pixels to be streamed to the Display Port connector.
> +
> +The Frame Buffer II device is a simple frame buffer device. The device
> contains the display +properties and the bridge or connector register. The
> output for this device currently +is a dedicated to a single Display Port.
> Currently the max resolution supported is 1280 x 720 at +60Hz.
> +
> +More information the FPGA video IP component can be acquired from
> +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/
> ug/ug_vip.pdf +
> +
> +New bindings:
> +=============
> +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.

The properties hint that those are the maximum values, but the descriptions 
don't match. One of the two needs to be fixed.

> +- altr,mem-port-width = the bus width of the avalon master port on the
> frame reader
> +
> +Example:
> +----------------------------
> +       dp_0_frame_buf: display-controller@100000280 {
> +                       compatible = "altr,vip-frame-buffer-2.0";
> +                       reg = <0x00000001 0x00000280 0x00000040>;
> +                       altr,max-width = <1280>;
> +                       altr,max-height = <720>;
> +                       altr,mem-port-width = <128>;
> +       };

-- 
Regards,

Laurent Pinchart

Re: [PATCHv4 3/3] DRM:ivip Intel FPGA Video and Image Processing Suite

[Ong, Hean Loong]

On Tue, 2017-08-01 at 17:30 +0300, Laurent Pinchart wrote:
> Hi Hean Loong,
> 
> Thank you for the patch.
> 
> On Tuesday 01 Aug 2017 10:31:34 Hean Loong, Ong wrote:
> > 
> > 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>
> > ---
> > V3:
> > *Changes to fixing drm_simple_pipe
> > *Used drm_fb_cma_get_gem_addr
> > 
> > V2:
> > *Adding drm_simple_display_pipe_init
> > ---
> >  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 | 183
> > ++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/ivip/intel_vip_drv.h  |  54 +++++++++
> >  drivers/gpu/drm/ivip/intel_vip_of.c   | 204
> > +++++++++++++++++++++++++++++++
> >  8 files changed, 562 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
> [snip]
> 
> > 
> > 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.
> I think this should be fixed to upstream this driver.
> 
> > 
> > Note that ACPI and X_86 architecture is yet
> ACPI on FPGA... I wonder if it could get any crazier than that :-)
> 
The driver is meant for the product Intel FPGA Arria10 devkit. We 
do not have plans for the devkit to be shipped with ACPI and X_86 in a
forseeable future. Currently most the commercial SoC devkits are
shipped with ARM + FPGA.
> > 
> > +	     to be supported.If M is selected the module would be
> > called ivip.
> s/.If/. If/
> 
> [snip]
> 
> > 
> > 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..ea85715
> > --- /dev/null
> > +++ b/drivers/gpu/drm/ivip/intel_vip_core.c
> > @@ -0,0 +1,183 @@
> > +/*
> > + * 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 void intelvipfb_enable(struct drm_simple_display_pipe
> > *pipe,
> > +	struct drm_crtc_state *crtc_state)
> > +{
> > +	/*
> > +	 * 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;
> > +	struct intelvipfb_priv *priv = pipe->plane.dev-
> > >dev_private;
> > +	void __iomem *base = priv->base;
> > +	struct drm_plane_state *state = pipe->plane.state;
> > +	dma_addr_t addr;
> > +
> > +	addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> > +
> > +	dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
> > +
> > +	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(struct drm_simple_display_pipe
> > *pipe)
> > +{
> > +	struct intelvipfb_priv *priv = pipe->plane.dev-
> > >dev_private;
> > +	void __iomem *base = priv->base;
> Missing blank line.
> 
> > 
> > +	/* 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 void intelvipfb_setup_mode_config(struct drm_device *drm)
> > +{
> > +	drm_mode_config_init(drm);
> > +	drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
> > +}
> > +
> > +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 void intelvipfb_update(struct drm_simple_display_pipe
> > *pipe,
> > +		struct drm_plane_state *old_state)
> > +{
> > +	struct drm_crtc *crtc = &pipe->crtc;
> > +
> > +	if (crtc->state->event) {
> > +		spin_lock_irq(&crtc->dev->event_lock);
> > +		drm_crtc_send_vblank_event(crtc, crtc->state-
> > >event);
> > +		spin_unlock_irq(&crtc->dev->event_lock);
> > +		crtc->state->event = NULL;
> > +	}
> This is not quite right. Userspace expects the driver to perform a
> page flip 
> here, and you just signal page flip completion without doing
> anything.
> 
> > 
> > +}
> > +
> > +static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
> > +	.prepare_fb = intelvipfb_pipe_prepare_fb,
> > +	.update = intelvipfb_update,
> > +	.enable = intelvipfb_enable,
> > +	.disable = intelvipfb_disable
> > +};
> > +
> > +int intelvipfb_probe(struct device *dev, void __iomem *base)
> You don't use the base argument, you can remove it.
> 
> > 
> > +{
> > +	int retval;
> > +	struct drm_device *drm;
> > +	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
> > +	struct drm_connector *connector;
> > +
> Extra blank line.
> 
> > 
> > +	u32 formats[] = {DRM_FORMAT_XRGB8888};
> > +
> > +	dev_set_drvdata(dev, fbpriv);
> As fbpriv is obtained by a call to dev_get_drvdata(), this isn't
> needed.
> 
> A better option would be to pass the intelvipfb_priv pointer as an
> argument to 
> this function.
> 
> > 
> > +	drm = fbpriv->drm;
> > +
> > +	drm->dev_private = fbpriv;
> > +
> > +	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, formats,
> > +		ARRAY_SIZE(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,
> > +		drm->mode_config.preferred_depth,
> > +		drm->mode_config.num_connector);
> > +
> > +	drm_mode_config_reset(drm);
> > +
> > +	drm_dev_register(drm, 0);
> > +
> > +	return retval;
> > +
> > +err_mode_config:
> > +
> > +	drm_mode_config_cleanup(drm);
> > +	return -ENODEV;
> > +}
> > +EXPORT_SYMBOL_GPL(intelvipfb_probe);
> Why do you need to export this symbol ?
> 
> > 
> > +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);
> > +
> > +	drm_mode_config_cleanup(drm);
> > +
> > +	drm_dev_unref(drm);
> > +
> > +	devm_kfree(dev, fbpriv);
> The whole point of devm_kzalloc() is that you don't need to free the
> memory at 
> remove time.
> 
> > 
> > +
> > +	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..ebdbd50
> > --- /dev/null
> > +++ b/drivers/gpu/drm/ivip/intel_vip_drv.h
> > @@ -0,0 +1,54 @@
> > +/*
> > + * 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
> Missing blank line.
> 
> > 
> > +#include <linux/io.h>
> > +#include <linux/fb.h>
> I don't think this header is needed.
> 
> > 
> > +#define DRIVER_NAME    "intelvipfb"
> > +#define BYTES_PER_PIXEL        4
> > +#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..5a7c63b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/ivip/intel_vip_of.c
> > @@ -0,0 +1,204 @@
> > +/*
> > + * 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>
> I don't think those two headers are needed.
> 
> > 
> > +#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>
> Please sort these alphabetically.
> 
> > 
> > +
> > +#include "intel_vip_drv.h"
> > +
> > +DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
> > +
> > +static void intelvipfb_lastclose(struct drm_device *drm)
> > +{
> > +	struct intelvipfb_priv *priv = drm->dev_private;
> > +
> > +	drm_fbdev_cma_restore_mode(priv->fbcma);
> > +}
> > +
> > +static struct drm_driver intelvipfb_drm = {
> > +	.driver_features =
> > +		DRIVER_MODESET | DRIVER_GEM |
> > +		DRIVER_PRIME | 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,
> > +	.lastclose = intelvipfb_lastclose,
> > +	.name = DRIVER_NAME,
> > +	.date = "20170729",
> > +	.desc = "Intel FPGA VIP SUITE",
> > +	.major = 1,
> > +	.minor = 0,
> > +	.ioctls = NULL,
> > +	.patchlevel = 0,
> > +	.fops = &drm_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);
> This property is not documented in patch 1/3.
> 
> > 
> > +	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;
> > +}