All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU)
@ 2026-09-13 20:50 Leander Kieweg
  2026-09-13 20:50 ` [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding Leander Kieweg
  2026-09-13 20:50 ` [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU Leander Kieweg
  0 siblings, 2 replies; 8+ messages in thread
From: Leander Kieweg @ 2026-09-13 20:50 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: airlied, simona, maarten.lankhorst, mripard, tzimmermann, robh,
	krzk+dt, conor+dt, u.kleine-koenig, Leander Kieweg

This is v4 of the GlandaGPU DRM driver series [1][2][3]. It addresses
review feedback from Thomas Zimmermann and the automated review bot
on v3.

GlandaGPU is a small VHDL soft-IP 2D display controller, currently
targeting a Terasic DE10-Standard (Cyclone V SoC). This series has
been tested against a QEMU digital twin and on real hardware.

Hardware/VHDL:   https://github.com/stiangglanda/GlandaGPU
QEMU fork:       https://github.com/stiangglanda/qemu-glandagpu
Userspace tests: https://github.com/stiangglanda/GlandaGPU-userspace-tests

Changes since v3:

dt-bindings:
- Fix alphabetical ordering of the '^kieweg,.*' vendor prefix in
  vendor-prefixes.yaml.

driver core:
- Use devm_ioremap_wc() instead of devm_ioremap() for the VRAM
  mapping in both the platform and PCI probe paths, avoiding a severe
  performance regression from uncached writes (Sashiko bot).
- Only enable the hardware VSYNC interrupt when an IRQ handler is
  actually registered. enable_vblank() now returns -EINVAL when
  falling back to polling mode, instead of risking an unhandled
  interrupt storm (Sashiko bot).
- Stop unconditionally enabling the VSYNC interrupt during probe.
  Let the DRM core enable/disable it through enable_vblank()/
  disable_vblank() as needed (Thomas Zimmermann, Sashiko bot).
- Guard the PCI probe/remove code and pci_driver structure with
  #ifdef CONFIG_PCI so the driver builds with
  CONFIG_COMPILE_TEST=y && CONFIG_PCI=n (Sashiko bot).
- Use container_of_const() instead of container_of() (Thomas
  Zimmermann).
- Add glanda_plane_atomic_disable(), which blanks VRAM when the
  plane is disabled, instead of silently returning on a NULL fb
  (Thomas Zimmermann).
- Wrap direct access to the shadow-plane buffer object in
  drm_gem_fb_begin_cpu_access()/drm_gem_fb_end_cpu_access() to
  synchronize against imported buffers (Thomas Zimmermann).
- Switch atomic_update() to damage-clipped blitting via
  drm_atomic_helper_damage_iter instead of copying the whole frame
  on every update (Thomas Zimmermann).
- Always call drm_atomic_helper_check_plane_state() in
  atomic_check(), even when the plane has no CRTC yet (Thomas
  Zimmermann).
- Support panning within a larger, system-allocated framebuffer by
  calculating the correct source offset when reading pixel data 
  (Thomas Zimmermann).
- Remove glanda_connector_detect(). The default "connected" status
  is sufficient (Thomas Zimmermann).
- Wrap hardware register access in enable_vblank()/disable_vblank()
  with drm_dev_enter()/drm_dev_exit() (Thomas Zimmermann).
- Use drm_crtc_vblank_atomic_enable()/drm_crtc_vblank_atomic_disable()
  instead of custom wrapper functions (Thomas Zimmermann).
- Use drmm_mode_config_init() so the mode-config pipeline is cleaned
  up automatically (Thomas Zimmermann).
- Raise mode_config.max_width/max_height to
  DRM_SHADOW_PLANE_MAX_WIDTH/DRM_SHADOW_PLANE_MAX_HEIGHT instead of
  the fixed 640x480, so userspace can allocate larger framebuffers
  (Thomas Zimmermann).
- Call drm_plane_enable_fb_damage_clips() to enable damage clipping
  (Thomas Zimmermann).
- Move drm_vblank_init() to right before drm_mode_config_reset()
  (Thomas Zimmermann).
- Remove the manual drm_helper_probe_single_connector_modes() call
  during init. The DRM core probes modes on demand (Thomas
  Zimmermann).
- Simplify glanda_drm_fini() to just drm_dev_unplug(). The DRM core
  handles vblank/IRQ teardown after unplug (Thomas Zimmermann).
- Drop "Hardware Accelerated" from the driver description (Thomas
  Zimmermann).

Regarding the panning support: Since the physical VRAM is fixed to 
640x480, the display output itself cannot be panned. Instead, the 
panning is handled on the source side. If userspace allocates a 
larger framebuffer, the driver now calculates the correct src_x and 
src_y offsets from the plane state and copies only the requested 
sub-region into VRAM. Please let me know if this implementation 
matches what you had in mind with the sysfb reference.

[1] v1: https://lore.kernel.org/dri-devel/20260714101146.200416-1-kieweg.leander@gmail.com/T/#t
[2] v2: https://lore.kernel.org/dri-devel/20260730173643.256052-1-kieweg.leander@gmail.com/T/#t
[3] v3: https://lore.kernel.org/dri-devel/20260824195418.17707-1-kieweg.leander@gmail.com/T/#t

Leander Kieweg (2):
  dt-bindings: display: Add GlandaGPU binding
  drm/glanda: Add initial DRM driver for GlandaGPU

 .../bindings/display/kieweg,gpu.yaml          |  55 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/tiny/Kconfig                  |  11 +
 drivers/gpu/drm/tiny/Makefile                 |   1 +
 drivers/gpu/drm/tiny/glandagpu.c              | 642 ++++++++++++++++++
 6 files changed, 717 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/kieweg,gpu.yaml
 create mode 100644 drivers/gpu/drm/tiny/glandagpu.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding
  2026-09-13 20:50 [PATCH v4 0/2] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU) Leander Kieweg
@ 2026-09-13 20:50 ` Leander Kieweg
  2026-09-17  8:05   ` Krzysztof Kozlowski
  2026-09-13 20:50 ` [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU Leander Kieweg
  1 sibling, 1 reply; 8+ messages in thread
From: Leander Kieweg @ 2026-09-13 20:50 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: airlied, simona, maarten.lankhorst, mripard, tzimmermann, robh,
	krzk+dt, conor+dt, u.kleine-koenig, Leander Kieweg

Add Device Tree binding documentation for GlandaGPU, a custom
FPGA-based 2D display controller.

For hardware designs and RTL sources, see:
https://github.com/stiangglanda/GlandaGPU

Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
---
 .../bindings/display/kieweg,gpu.yaml          | 55 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/kieweg,gpu.yaml

diff --git a/Documentation/devicetree/bindings/display/kieweg,gpu.yaml b/Documentation/devicetree/bindings/display/kieweg,gpu.yaml
new file mode 100644
index 000000000..c0fda8438
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/kieweg,gpu.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/kieweg,gpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GlandaGPU 2D Hardware Accelerated Display Controller
+
+maintainers:
+  - Leander Kieweg <kieweg.leander@gmail.com>
+
+description: |
+  GlandaGPU is a custom FPGA soft-IP core providing a simple
+  2D hardware-accelerated drawing engine with a VGA-compatible
+  display output. The register window covers a combined VRAM + MMIO
+  region, with MMIO registers at a fixed offset within it.
+
+  For hardware designs and RTL sources, see:
+  https://github.com/stiangglanda/GlandaGPU
+
+properties:
+  compatible:
+    const: kieweg,gpu-1.0
+
+  reg:
+    maxItems: 1
+    description:
+      Combined VRAM + MMIO register window (VRAM at offset 0,
+      MMIO registers at offset 0x00200000 within this range).
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: Bus and pixel clock provided to the FPGA IP.
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    gpu@c0000000 {
+        compatible = "kieweg,gpu-1.0";
+        reg = <0xc0000000 0x1000000>;
+        interrupt-parent = <&intc>;
+        interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&osc1>;
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b1af9deac..3a9d15dd7 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -893,6 +893,8 @@ patternProperties:
     description: Khadas
   "^kiebackpeter,.*":
     description: Kieback & Peter GmbH
+  "^kieweg,.*":
+    description: Leander Kieweg
   "^kinetic,.*":
     description: Kinetic Technologies
   "^kingdisplay,.*":
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU
  2026-09-13 20:50 [PATCH v4 0/2] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU) Leander Kieweg
  2026-09-13 20:50 ` [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding Leander Kieweg
@ 2026-09-13 20:50 ` Leander Kieweg
  2026-09-13 21:03   ` sashiko-bot
  2026-09-18  7:34   ` Thomas Zimmermann
  1 sibling, 2 replies; 8+ messages in thread
From: Leander Kieweg @ 2026-09-13 20:50 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: airlied, simona, maarten.lankhorst, mripard, tzimmermann, robh,
	krzk+dt, conor+dt, u.kleine-koenig, Leander Kieweg

Introduce the core DRM/KMS driver for GlandaGPU. This driver
supports basic modesetting, atomic updates via shadow plane helpers,
and optional QEMU PCI probing alongside the platform driver.

Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
---
 MAINTAINERS                      |   6 +
 drivers/gpu/drm/tiny/Kconfig     |  11 +
 drivers/gpu/drm/tiny/Makefile    |   1 +
 drivers/gpu/drm/tiny/glandagpu.c | 642 +++++++++++++++++++++++++++++++
 4 files changed, 660 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/glandagpu.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6dea93a41..c16d1ed70 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8076,6 +8076,12 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	drivers/gpu/drm/gud/
 F:	include/drm/gud.h
 
+DRM DRIVER FOR GLANDAGPU
+M:	Leander Kieweg <kieweg.leander@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/kieweg,gpu.yaml
+F:	drivers/gpu/drm/tiny/glandagpu.c
+
 DRM DRIVER FOR GRAIN MEDIA GM12U320 PROJECTORS
 M:	Hans de Goede <hansg@kernel.org>
 S:	Maintained
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f0e72d4b6..267b3103d 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -56,6 +56,17 @@ config DRM_CIRRUS_QEMU
 	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
 	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
 
+config DRM_GLANDA
+	tristate "GlandaGPU DRM driver"
+	depends on DRM && MMU && (PCI || COMPILE_TEST)
+	select DRM_KMS_HELPER
+	select DRM_GEM_SHMEM_HELPER
+	help
+	  DRM/KMS driver for the GlandaGPU display controller
+	  (FPGA soft IP). This driver supports basic modesetting,
+	  dumb buffers, and atomic updates via shadow planes.
+	  It also provides PCI probing for QEMU testing.
+
 config DRM_GM12U320
 	tristate "GM12U320 driver for USB projectors"
 	depends on DRM && USB && MMU
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 48d30bf61..b4fa1554a 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_APPLETBDRM)		+= appletbdrm.o
 obj-$(CONFIG_DRM_ARCPGU)		+= arcpgu.o
 obj-$(CONFIG_DRM_BOCHS)			+= bochs.o
 obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus-qemu.o
+obj-$(CONFIG_DRM_GLANDA) 			+= glandagpu.o
 obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
 obj-$(CONFIG_DRM_PANEL_MIPI_DBI)	+= panel-mipi-dbi.o
 obj-$(CONFIG_DRM_PIXPAPER)              += pixpaper.o
diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
new file mode 100644
index 000000000..bc954eb6d
--- /dev/null
+++ b/drivers/gpu/drm/tiny/glandagpu.c
@@ -0,0 +1,642 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+#include <linux/delay.h>	/* udelay (polling) */
+#include <linux/of.h>
+#include <linux/slab.h>		/* GFP_KERNEL */
+#include <linux/interrupt.h>
+#include <linux/wait.h>
+#include <linux/mm.h>
+#include <linux/mutex.h>
+#include <linux/iosys-map.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
+
+#include <drm/drm_connector.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_print.h>
+
+/* Hardware Constants */
+#define GLANDA_WIDTH      640
+#define GLANDA_HEIGHT     480
+#define GLANDA_VRAM_SIZE  (GLANDA_WIDTH * GLANDA_HEIGHT * 4)
+#define GLANDA_MMIO_SIZE  32
+#define GLANDA_MMIO_OFFSET 0x00200000
+
+/* QEMU test device ID, from the range reserved for experimental use (docs/specs/pci-ids.rst). */
+#define PCI_DEVICE_ID_GLANDA_GPU 0x10f0
+
+/* Register Offsets */
+#define REG_STATUS  0x00
+#define REG_CTRL    0x04
+#define REG_COORD0  0x08
+#define REG_COORD1  0x0C
+#define REG_COLOR   0x10
+#define REG_ISR     0x14
+#define REG_IER     0x18
+
+/* Bit Masks */
+#define INT_DONE    BIT(0)
+#define INT_VSYNC   BIT(1)
+
+#define STATUS_BUSY BIT(0)
+#define CMD_CLEAR   (0x1)
+#define CMD_RECT    (0x2)
+#define CMD_LINE    (0x3)
+#define CTRL_START  BIT(4)
+
+struct glanda_device {
+	struct drm_device drm;
+
+	/* hw */
+	void __iomem *mmio_base;
+	void __iomem *vram_base;
+	phys_addr_t vram_phys;
+
+	int irq;
+
+	/* drm */
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
+
+#define to_glanda(dev) container_of_const(dev, struct glanda_device, drm)
+
+static const u32 glanda_plane_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static void glanda_blit_rect(struct glanda_device *gdev,
+			     const struct drm_rect *dst_clip,
+			     const struct iosys_map *src,
+			     struct drm_framebuffer *fb,
+			     int dst_off_x, int dst_off_y)
+{
+	unsigned int src_pitch = fb->pitches[0];
+	unsigned int width = drm_rect_width(dst_clip);
+	unsigned int height = drm_rect_height(dst_clip);
+	unsigned int x, y;
+
+	for (y = 0; y < height; y++) {
+		unsigned int dst_y = dst_clip->y1 + y;
+		unsigned int src_y = dst_y - dst_off_y;
+		u32 __iomem *dst = (u32 __iomem *)gdev->vram_base +
+				   (size_t)dst_y * GLANDA_WIDTH + dst_clip->x1;
+		size_t src_off = (size_t)src_y * src_pitch +
+				 (size_t)(dst_clip->x1 - dst_off_x) * sizeof(u32);
+
+		for (x = 0; x < width; x++) {
+			u32 pixel = iosys_map_rd(src, src_off + x * sizeof(u32), u32);
+			u32 packed;
+
+			pixel = le32_to_cpu((__force __le32)pixel);
+			packed = ((pixel >> 12) & 0x0F00) |
+				((pixel >> 8) & 0x00F0) |
+				((pixel >> 4) & 0x000F);
+
+			writel_relaxed(packed, &dst[x]);
+		}
+	}
+}
+
+static void glanda_plane_atomic_update(struct drm_plane *plane,
+				       struct drm_atomic_commit *state)
+{
+	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_shadow_plane_state *shadow_state = to_drm_shadow_plane_state(new_state);
+	struct drm_rect vram_clip = DRM_RECT_INIT(0, 0, GLANDA_WIDTH, GLANDA_HEIGHT);
+	struct glanda_device *gdev = to_glanda(plane->dev);
+	struct drm_framebuffer *fb = new_state->fb;
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_rect damage;
+	int dst_off_x, dst_off_y;
+	int ret, idx;
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret)
+		return;
+
+	if (!drm_dev_enter(plane->dev, &idx))
+		goto out_drm_gem_fb_end_cpu_access;
+
+	dst_off_x = new_state->dst.x1 - (new_state->src.x1 >> 16);
+	dst_off_y = new_state->dst.y1 - (new_state->src.y1 >> 16);
+
+	drm_atomic_helper_damage_iter_init(&iter, old_state, new_state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		struct drm_rect dst_clip = new_state->dst;
+
+		drm_rect_translate(&damage, dst_off_x, dst_off_y);
+
+		if (!drm_rect_intersect(&dst_clip, &damage))
+			continue;
+		if (!drm_rect_intersect(&dst_clip, &vram_clip))
+			continue;
+
+		glanda_blit_rect(gdev, &dst_clip, &shadow_state->data[0], fb,
+				 dst_off_x, dst_off_y);
+	}
+
+	drm_dev_exit(idx);
+out_drm_gem_fb_end_cpu_access:
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+}
+
+static void glanda_plane_atomic_disable(struct drm_plane *plane,
+					struct drm_atomic_commit *state)
+{
+	struct drm_device *dev = plane->dev;
+	struct glanda_device *gdev = to_glanda(dev);
+	int idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return;
+
+	memset_io(gdev->vram_base, 0, GLANDA_WIDTH * sizeof(u32) * GLANDA_HEIGHT);
+	drm_dev_exit(idx);
+}
+
+static int glanda_plane_atomic_check(struct drm_plane *plane,
+				     struct drm_atomic_commit *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_crtc_state *new_crtc_state = NULL;
+	int ret;
+
+	if (new_plane_state->crtc)
+		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
+
+	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
+						  DRM_PLANE_NO_SCALING, DRM_PLANE_NO_SCALING,
+		false,	/* can_position */
+		false); /* can_update_disabled */
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct drm_plane_helper_funcs glanda_plane_helper_funcs = {
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_update = glanda_plane_atomic_update,
+	.atomic_check = glanda_plane_atomic_check,
+	.atomic_disable = glanda_plane_atomic_disable,
+};
+
+static const struct drm_plane_funcs glanda_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static int glanda_connector_get_modes(struct drm_connector *connector)
+{
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (!mode) {
+		dev_err(connector->dev->dev, "GlandaGPU: failed to create display mode\n");
+		return 0;
+	}
+
+	/* Standard VGA timing: 640x480 @ 60 Hz. */
+	mode->hdisplay = 640;
+	mode->hsync_start = 656;
+	mode->hsync_end = 752;
+	mode->htotal = 800;
+
+	mode->vdisplay = 480;
+	mode->vsync_start = 490;
+	mode->vsync_end = 492;
+	mode->vtotal = 525;
+
+	mode->clock = 25175;	/* 25.175 MHz pixel clock */
+
+	mode->flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC;
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static int glanda_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+	struct glanda_device *gdev = to_glanda(crtc->dev);
+	u32 ier;
+	int idx;
+
+	if (gdev->irq <= 0)
+		return -EINVAL;
+
+	if (!drm_dev_enter(crtc->dev, &idx))
+		return -ENODEV;
+
+	ier = readl(gdev->mmio_base + REG_IER);
+	writel(ier | INT_VSYNC, gdev->mmio_base + REG_IER);
+
+	drm_dev_exit(idx);
+	return 0;
+}
+
+static void glanda_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+	struct glanda_device *gdev = to_glanda(crtc->dev);
+	u32 ier;
+	int idx;
+
+	if (!drm_dev_enter(crtc->dev, &idx))
+		return;
+
+	ier = readl(gdev->mmio_base + REG_IER);
+	writel(ier & ~INT_VSYNC, gdev->mmio_base + REG_IER);
+
+	drm_dev_exit(idx);
+}
+
+static void glanda_crtc_atomic_flush(struct drm_crtc *crtc,
+				     struct drm_atomic_commit *state)
+{
+	struct glanda_device *gdev = to_glanda(crtc->dev);
+	struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_pending_vblank_event *event;
+
+	if (new_state->event) {
+		event = new_state->event;
+		new_state->event = NULL;
+
+		spin_lock_irq(&crtc->dev->event_lock);
+
+		if (gdev->irq > 0 && drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+
+		spin_unlock_irq(&crtc->dev->event_lock);
+	}
+}
+
+static const struct drm_crtc_funcs glanda_crtc_funcs = {
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.reset = drm_atomic_helper_crtc_reset,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+	.enable_vblank = glanda_crtc_enable_vblank,
+	.disable_vblank = glanda_crtc_disable_vblank,
+};
+
+static const struct drm_crtc_helper_funcs glanda_crtc_helper_funcs = {
+	.atomic_enable = drm_crtc_vblank_atomic_enable,
+	.atomic_disable = drm_crtc_vblank_atomic_disable,
+	.atomic_flush = glanda_crtc_atomic_flush,
+};
+
+static const struct drm_connector_helper_funcs glanda_connector_helper_funcs = {
+	.get_modes = glanda_connector_get_modes,
+};
+
+static const struct drm_encoder_funcs glanda_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_funcs glanda_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static const struct drm_mode_config_funcs glanda_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+DEFINE_DRM_GEM_FOPS(glanda_drm_fops);
+
+static const struct drm_driver glanda_drm_driver = {
+	.driver_features =
+	    DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.name = "glandagpu",
+	.desc = "GlandaGPU DRM Driver",
+	.major = 1,
+	.minor = 0,
+	.fops = &glanda_drm_fops,
+	.dumb_create = drm_gem_shmem_dumb_create,
+};
+
+static irqreturn_t glanda_irq_handler(int irq, void *dev_id)
+{
+	struct glanda_device *gdev = dev_id;
+	u32 isr, ier;
+
+	if (!gdev || !gdev->mmio_base)
+		return IRQ_NONE;
+
+	isr = readl(gdev->mmio_base + REG_ISR);
+	ier = readl(gdev->mmio_base + REG_IER);
+
+	if (!(isr & ier))
+		return IRQ_NONE;
+
+	if (isr & INT_VSYNC)
+		drm_crtc_handle_vblank(&gdev->crtc);
+
+	/* Clear interrupt(W1C) */
+	writel(isr, gdev->mmio_base + REG_ISR);
+	return IRQ_HANDLED;
+}
+
+/* Common DRM setup once MMIO/VRAM/IRQ are known(used by both probe paths) */
+static int glanda_drm_init(struct glanda_device *gdev, int irq)
+{
+	int ret;
+
+	gdev->irq = -1;
+
+	writel(0, gdev->mmio_base + REG_IER);
+	writel(0xFFFFFFFF, gdev->mmio_base + REG_ISR);	/* clear flags */
+
+	/* DRM mode config */
+	ret = drmm_mode_config_init(&gdev->drm);
+	if (ret)
+		return ret;
+
+	gdev->drm.mode_config.min_width = 640;
+	gdev->drm.mode_config.min_height = 480;
+	gdev->drm.mode_config.max_width = DRM_SHADOW_PLANE_MAX_WIDTH;
+	gdev->drm.mode_config.max_height = DRM_SHADOW_PLANE_MAX_HEIGHT;
+	gdev->drm.mode_config.funcs = &glanda_mode_config_funcs;
+
+	ret = drm_universal_plane_init(&gdev->drm, &gdev->primary_plane, 1 << 0,
+				       &glanda_plane_funcs,
+				       glanda_plane_formats,
+				       ARRAY_SIZE(glanda_plane_formats), NULL,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize primary plane\n");
+		return ret;
+	}
+	drm_plane_helper_add(&gdev->primary_plane, &glanda_plane_helper_funcs);
+
+	drm_plane_enable_fb_damage_clips(&gdev->primary_plane);
+
+	/* CRTC init */
+	ret = drm_crtc_init_with_planes(&gdev->drm, &gdev->crtc,
+					&gdev->primary_plane, NULL,
+					&glanda_crtc_funcs, NULL);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize CRTC with planes\n");
+		return ret;
+	}
+	drm_crtc_helper_add(&gdev->crtc, &glanda_crtc_helper_funcs);
+
+	ret = drm_encoder_init(&gdev->drm, &gdev->encoder, &glanda_encoder_funcs,
+			       DRM_MODE_ENCODER_DAC, NULL);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize encoder\n");
+		return ret;
+	}
+	gdev->encoder.possible_crtcs = 1;
+
+	ret = drm_connector_init(&gdev->drm, &gdev->connector,
+				 &glanda_connector_funcs, DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize connector\n");
+		return ret;
+	}
+	drm_connector_helper_add(&gdev->connector, &glanda_connector_helper_funcs);
+
+	drm_connector_attach_encoder(&gdev->connector, &gdev->encoder);
+
+	/* VBlank init */
+	ret = drm_vblank_init(&gdev->drm, 1);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize vblank\n");
+		return ret;
+	}
+
+	drm_mode_config_reset(&gdev->drm);
+
+	if (irq > 0) {
+		gdev->irq = irq;
+		ret = devm_request_irq(gdev->drm.dev, gdev->irq, glanda_irq_handler,
+				       IRQF_SHARED, "glandagpu", gdev);
+		if (ret) {
+			drm_err(&gdev->drm, "Failed to request IRQ %d\n",
+				gdev->irq);
+			return ret;
+		}
+	} else {
+		drm_warn(&gdev->drm, "No IRQ found, falling back to polling\n");
+	}
+
+	ret = drm_dev_register(&gdev->drm, 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Shared teardown, mirrors glanda_drm_init() */
+static void glanda_drm_fini(struct glanda_device *gdev)
+{
+	drm_dev_unplug(&gdev->drm);
+}
+
+static int glandagpu_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct glanda_device *gdev;
+	int irq;
+
+	gdev = devm_drm_dev_alloc(&pdev->dev, &glanda_drm_driver, struct glanda_device, drm);
+	if (IS_ERR(gdev))
+		return PTR_ERR(gdev);
+
+	platform_set_drvdata(pdev, gdev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	if (resource_size(res) < GLANDA_MMIO_OFFSET + GLANDA_MMIO_SIZE) {
+		dev_err(&pdev->dev, "MMIO region too small: %llu bytes, need at least %u\n",
+			(unsigned long long)resource_size(res),
+			GLANDA_MMIO_OFFSET + GLANDA_MMIO_SIZE);
+		return -EINVAL;
+	}
+
+	gdev->vram_phys = res->start;
+	gdev->vram_base = devm_ioremap_wc(&pdev->dev, res->start, GLANDA_VRAM_SIZE);
+	gdev->mmio_base = devm_ioremap(&pdev->dev, res->start + GLANDA_MMIO_OFFSET,
+				       GLANDA_MMIO_SIZE);
+	if (!gdev->vram_base || !gdev->mmio_base) {
+		drm_err(&gdev->drm, "failed to ioremap\n");
+		return -ENOMEM;
+	}
+
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq == -ENXIO)
+		irq = -1;	/* no IRQ resource, fall back to polling */
+	else if (irq < 0)
+		return irq;
+
+	return glanda_drm_init(gdev, irq);
+}
+
+static void glandagpu_remove(struct platform_device *pdev)
+{
+	glanda_drm_fini(platform_get_drvdata(pdev));
+}
+
+/* Device Tree match table. */
+static const struct of_device_id glanda_of_match[] = {
+	{ .compatible = "kieweg,gpu-1.0" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, glanda_of_match);
+
+static struct platform_driver glandagpu_driver = {
+	.driver = {
+		.name = "glandagpu",
+		.of_match_table = glanda_of_match,
+	},
+	.probe = glandagpu_probe,
+	.remove = glandagpu_remove,
+};
+
+/* PCI probe path for the QEMU test device, real hardware uses platform_driver */
+#ifdef CONFIG_PCI
+static int glandagpu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct glanda_device *gdev;
+	int ret;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+	pci_set_master(pdev);
+
+	if (pci_resource_len(pdev, 0) < GLANDA_MMIO_SIZE ||
+	    pci_resource_len(pdev, 1) < GLANDA_VRAM_SIZE) {
+		dev_err(&pdev->dev, "BAR too small: BAR0=%llu (need %u), BAR1=%llu (need %u)\n",
+			(unsigned long long)pci_resource_len(pdev, 0), GLANDA_MMIO_SIZE,
+			(unsigned long long)pci_resource_len(pdev, 1), GLANDA_VRAM_SIZE);
+		return -EINVAL;
+	}
+
+	ret = pcim_iomap_regions(pdev, BIT(0), "glandagpu");
+	if (ret)
+		return ret;
+
+	ret = pcim_request_region(pdev, 1, "glandagpu");
+	if (ret)
+		return ret;
+
+	gdev = devm_drm_dev_alloc(&pdev->dev, &glanda_drm_driver, struct glanda_device, drm);
+	if (IS_ERR(gdev))
+		return PTR_ERR(gdev);
+
+	pci_set_drvdata(pdev, gdev);
+
+	gdev->mmio_base = pcim_iomap_table(pdev)[0];
+	gdev->vram_phys = pci_resource_start(pdev, 1);
+
+	gdev->vram_base = devm_ioremap_wc(&pdev->dev, gdev->vram_phys, GLANDA_VRAM_SIZE);
+	if (!gdev->vram_base)
+		return -ENOMEM;
+
+	return glanda_drm_init(gdev, pdev->irq);
+}
+
+static void glandagpu_pci_remove(struct pci_dev *pdev)
+{
+	glanda_drm_fini(pci_get_drvdata(pdev));
+}
+
+static const struct pci_device_id glanda_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_DEVICE_ID_GLANDA_GPU) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(pci, glanda_pci_ids);
+
+static struct pci_driver glandagpu_pci_driver = {
+	.name = "glandagpu-pci",
+	.id_table = glanda_pci_ids,
+	.probe = glandagpu_pci_probe,
+	.remove = glandagpu_pci_remove,
+};
+#endif /* CONFIG_PCI */
+
+static int __init glandagpu_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&glandagpu_driver);
+	if (ret) {
+		pr_err("GlandaGPU: Failed to register platform driver\n");
+		return ret;
+	}
+
+#ifdef CONFIG_PCI
+	ret = pci_register_driver(&glandagpu_pci_driver);
+	if (ret) {
+		pr_err("GlandaGPU: Failed to register PCI driver\n");
+		platform_driver_unregister(&glandagpu_driver);
+		return ret;
+	}
+#endif
+
+	return 0;
+}
+
+static void __exit glandagpu_exit(void)
+{
+#ifdef CONFIG_PCI
+	pci_unregister_driver(&glandagpu_pci_driver);
+#endif
+	platform_driver_unregister(&glandagpu_driver);
+}
+
+module_init(glandagpu_init);
+module_exit(glandagpu_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Leander Kieweg <kieweg.leander@gmail.com>");
+MODULE_DESCRIPTION("DRM driver for GlandaGPU, an FPGA-based 2D GPU with VGA output");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU
  2026-09-13 20:50 ` [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU Leander Kieweg
@ 2026-09-13 21:03   ` sashiko-bot
  2026-09-18  7:34   ` Thomas Zimmermann
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-13 21:03 UTC (permalink / raw)
  To: Leander Kieweg; +Cc: devicetree, dri-devel, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Calling a sleeping function in an atomic context (plane atomic update callback).
- [High] Missing drm_atomic_helper_shutdown() in driver removal and missing .shutdown hooks.
- [High] Infinite interrupt storm leading to a CPU hard lockup upon PCI device removal (missing 0xFFFFFFFF check).
- [High] Kernel panic via unaligned memory access in `glanda_blit_rect` due to unvalidated framebuffer pitch.
--

commit 17ab32ed9422ea6f511ee3ba92be0492625b8e06
Author: Leander Kieweg <kieweg.leander@gmail.com>

    drm/glanda: Add initial DRM driver for GlandaGPU
    
    Introduce the core DRM/KMS driver for GlandaGPU. This driver
    supports basic modesetting, atomic updates via shadow plane helpers,
    and optional QEMU PCI probing alongside the platform driver.

> diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
> new file mode 100644
> index 0000000000000..bc954eb6d4412
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/glandagpu.c

[ ... ]

> @@ -107,17 +107,17 @@ static void glanda_blit_rect(struct glanda_device *gdev,
> +		u32 __iomem *dst = (u32 __iomem *)gdev->vram_base +
> +				   (size_t)dst_y * GLANDA_WIDTH + dst_clip->x1;
> +		size_t src_off = (size_t)src_y * src_pitch +
> +				 (size_t)(dst_clip->x1 - dst_off_x) * sizeof(u32);
> +
> +		for (x = 0; x < width; x++) {
> +			u32 pixel = iosys_map_rd(src, src_off + x * sizeof(u32), u32);

[Severity: High]
Does this code risk an unaligned memory access and a kernel panic?

Userspace can specify an arbitrary framebuffer pitch via the
DRM_IOCTL_MODE_ADDFB2 ioctl, and the DRM core does not enforce byte alignment
on fb->pitches[0].

If src_pitch is not a multiple of 4, src_off could be an unaligned byte
offset. Because the iosys_map_rd() macro casts directly to a 32-bit pointer
and dereferences it, this could cause an Alignment Fault and kernel panic on
architectures with strict alignment requirements.

[ ... ]

> @@ -136,10 +136,10 @@ static void glanda_plane_atomic_update(struct drm_plane *plane,
> +	struct drm_atomic_helper_damage_iter iter;
> +	struct drm_rect damage;
> +	int dst_off_x, dst_off_y;
> +	int ret, idx;
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return;

[Severity: High]
Could this result in calling a sleeping function from an atomic context?

The glanda_plane_atomic_update() function executes in an atomic context
during non-blocking commits where sleeping is strictly forbidden.

Because drm_gem_fb_begin_cpu_access() waits on DMA fences and acquires
mutexes, it can sleep. This could lead to might_sleep() warnings, system
instability, or deadlocks. Should CPU access be prepared beforehand in
a begin_fb_access or prepare_fb hook instead?

[ ... ]

> @@ -162,6 +162,6 @@ static void glanda_plane_atomic_update(struct drm_plane *plane,
> +
> +	drm_dev_exit(idx);
> +out_drm_gem_fb_end_cpu_access:
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +}

[Severity: High]
Can this end_cpu_access call also sleep in the atomic update callback?

[ ... ]

> @@ -282,10 +282,10 @@ static const struct drm_connector_funcs glanda_connector_funcs = {
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_mode_config_funcs glanda_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create_with_dirty,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};

[Severity: High]
Does the lack of a custom validation wrapper here allow the unaligned pitch
issue in glanda_blit_rect() to occur?

Hooking fb_create directly to drm_gem_fb_create_with_dirty() without a
custom wrapper means the driver does not enforce pitch alignment during
framebuffer creation.

[ ... ]

> @@ -306,10 +306,10 @@ static irqreturn_t glanda_irq_handler(int irq, void *dev_id)
> +	struct glanda_device *gdev = dev_id;
> +	u32 isr, ier;
> +
> +	if (!gdev || !gdev->mmio_base)
> +		return IRQ_NONE;
> +
> +	isr = readl(gdev->mmio_base + REG_ISR);
> +	ier = readl(gdev->mmio_base + REG_IER);
> +
> +	if (!(isr & ier))
> +		return IRQ_NONE;

[Severity: High]
Could this cause an infinite interrupt storm and a CPU hard lockup if the PCI
device is removed?

When a PCI device is removed (e.g. hot-unplug), MMIO reads return all 1s
(0xFFFFFFFF). Since !(0xFFFFFFFF & 0xFFFFFFFF) evaluates to false, this
handler would not return IRQ_NONE.

Instead, it would process a fake VSYNC, write 0xFFFFFFFF back, and return
IRQ_HANDLED. If the IRQ line is shared and asserted by another device, the
kernel's spurious interrupt detector will not disable the broken line,
potentially causing a hard lockup.

[ ... ]

> @@ -522,10 +522,10 @@ static int glanda_drm_init(struct glanda_device *gdev, int irq)
> +	return 0;
> +}
> +
> +/* Shared teardown, mirrors glanda_drm_init() */
> +static void glanda_drm_fini(struct glanda_device *gdev)
> +{
> +	drm_dev_unplug(&gdev->drm);
> +}

[Severity: High]
Is a call to drm_atomic_helper_shutdown() missing here?

Unregistering the DRM device without shutting down the atomic helpers means
the display pipeline is not turned off. The active drm_crtc_state and
drm_plane_state would not transition to a disabled state, leaving
framebuffers pinned and resulting in memory leaks.

[ ... ]

> @@ -575,10 +575,10 @@ static const struct of_device_id glanda_of_match[] = {
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, glanda_of_match);
> +
> +static struct platform_driver glandagpu_driver = {
> +	.driver = {
> +		.name = "glandagpu",
> +		.of_match_table = glanda_of_match,
> +	},
> +	.probe = glandagpu_probe,
> +	.remove = glandagpu_remove,
> +};

[Severity: High]
Does this platform driver need a .shutdown callback?

Without a .shutdown callback, the display hardware remains active during
system halt or reboot, which can cause the device to continue scanning out
VRAM inappropriately across reboots or kexecs.

[ ... ]

> @@ -613,10 +613,10 @@ static const struct pci_device_id glanda_pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_DEVICE_ID_GLANDA_GPU) },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, glanda_pci_ids);
> +
> +static struct pci_driver glandagpu_pci_driver = {
> +	.name = "glandagpu-pci",
> +	.id_table = glanda_pci_ids,
> +	.probe = glandagpu_pci_probe,
> +	.remove = glandagpu_pci_remove,
> +};

[Severity: High]
Is the .shutdown callback also missing from the PCI driver?

Similar to the platform driver, omitting this callback prevents the display
from being gracefully disabled on shutdown.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913205007.118552-1-kieweg.leander@gmail.com?part=2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding
  2026-09-13 20:50 ` [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding Leander Kieweg
@ 2026-09-17  8:05   ` Krzysztof Kozlowski
  2026-09-17 10:45     ` Leander Kieweg
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-17  8:05 UTC (permalink / raw)
  To: Leander Kieweg
  Cc: dri-devel, devicetree, airlied, simona, maarten.lankhorst,
	mripard, tzimmermann, robh, krzk+dt, conor+dt, u.kleine-koenig

On Sun, Sep 13, 2026 at 10:50:06PM +0200, Leander Kieweg wrote:
> Add Device Tree binding documentation for GlandaGPU, a custom
> FPGA-based 2D display controller.
> 
> For hardware designs and RTL sources, see:
> https://github.com/stiangglanda/GlandaGPU
> 
> Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
> ---
>  .../bindings/display/kieweg,gpu.yaml          | 55 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/kieweg,gpu.yaml

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding
  2026-09-17  8:05   ` Krzysztof Kozlowski
@ 2026-09-17 10:45     ` Leander Kieweg
  0 siblings, 0 replies; 8+ messages in thread
From: Leander Kieweg @ 2026-09-17 10:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: dri-devel, devicetree, airlied, simona, maarten.lankhorst,
	mripard, tzimmermann, robh, krzk+dt, conor+dt, u.kleine-koenig

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Hi Krzysztof,

Thank you for the review!

Best regards,
Leander

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU
  2026-09-13 20:50 ` [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU Leander Kieweg
  2026-09-13 21:03   ` sashiko-bot
@ 2026-09-18  7:34   ` Thomas Zimmermann
  2026-09-20 19:39     ` Leander Kieweg
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Zimmermann @ 2026-09-18  7:34 UTC (permalink / raw)
  To: Leander Kieweg, dri-devel, devicetree
  Cc: airlied, simona, maarten.lankhorst, mripard, robh, krzk+dt,
	conor+dt, u.kleine-koenig

Hi,

please see the Sashiko bot's review for several details that might be 
worth addressing. I've also left a few comments below. Apart from that 
the driver looks quite nice already.

Am 13.09.26 um 22:50 schrieb Leander Kieweg:
> Introduce the core DRM/KMS driver for GlandaGPU. This driver
> supports basic modesetting, atomic updates via shadow plane helpers,
> and optional QEMU PCI probing alongside the platform driver.
>
> Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
> ---
>   MAINTAINERS                      |   6 +
>   drivers/gpu/drm/tiny/Kconfig     |  11 +
>   drivers/gpu/drm/tiny/Makefile    |   1 +
>   drivers/gpu/drm/tiny/glandagpu.c | 642 +++++++++++++++++++++++++++++++
>   4 files changed, 660 insertions(+)
>   create mode 100644 drivers/gpu/drm/tiny/glandagpu.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6dea93a41..c16d1ed70 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8076,6 +8076,12 @@ T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
>   F:	drivers/gpu/drm/gud/
>   F:	include/drm/gud.h
>   
> +DRM DRIVER FOR GLANDAGPU
> +M:	Leander Kieweg <kieweg.leander@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/display/kieweg,gpu.yaml
> +F:	drivers/gpu/drm/tiny/glandagpu.c
> +
>   DRM DRIVER FOR GRAIN MEDIA GM12U320 PROJECTORS
>   M:	Hans de Goede <hansg@kernel.org>
>   S:	Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f0e72d4b6..267b3103d 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -56,6 +56,17 @@ config DRM_CIRRUS_QEMU
>   	   - qxl (DRM_QXL, qemu -vga qxl, works best with spice)
>   	   - virtio (DRM_VIRTIO_GPU), qemu -vga virtio)
>   
> +config DRM_GLANDA
> +	tristate "GlandaGPU DRM driver"
> +	depends on DRM && MMU && (PCI || COMPILE_TEST)
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_SHMEM_HELPER
> +	help
> +	  DRM/KMS driver for the GlandaGPU display controller
> +	  (FPGA soft IP). This driver supports basic modesetting,
> +	  dumb buffers, and atomic updates via shadow planes.
> +	  It also provides PCI probing for QEMU testing.
> +
>   config DRM_GM12U320
>   	tristate "GM12U320 driver for USB projectors"
>   	depends on DRM && USB && MMU
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 48d30bf61..b4fa1554a 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_APPLETBDRM)		+= appletbdrm.o
>   obj-$(CONFIG_DRM_ARCPGU)		+= arcpgu.o
>   obj-$(CONFIG_DRM_BOCHS)			+= bochs.o
>   obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus-qemu.o
> +obj-$(CONFIG_DRM_GLANDA) 			+= glandagpu.o
>   obj-$(CONFIG_DRM_GM12U320)		+= gm12u320.o
>   obj-$(CONFIG_DRM_PANEL_MIPI_DBI)	+= panel-mipi-dbi.o
>   obj-$(CONFIG_DRM_PIXPAPER)              += pixpaper.o
> diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
> new file mode 100644
> index 000000000..bc954eb6d
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/glandagpu.c
> @@ -0,0 +1,642 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>	/* udelay (polling) */
> +#include <linux/of.h>
> +#include <linux/slab.h>		/* GFP_KERNEL */
> +#include <linux/interrupt.h>
> +#include <linux/wait.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +#include <linux/iosys-map.h>
> +
> +#include <drm/drm_drv.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_ioctl.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_vblank_helper.h>
> +
> +#include <drm/drm_connector.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_modeset_helper.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_plane.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_print.h>
> +
> +/* Hardware Constants */
> +#define GLANDA_WIDTH      640
> +#define GLANDA_HEIGHT     480
> +#define GLANDA_VRAM_SIZE  (GLANDA_WIDTH * GLANDA_HEIGHT * 4)
> +#define GLANDA_MMIO_SIZE  32
> +#define GLANDA_MMIO_OFFSET 0x00200000
> +
> +/* QEMU test device ID, from the range reserved for experimental use (docs/specs/pci-ids.rst). */
> +#define PCI_DEVICE_ID_GLANDA_GPU 0x10f0
> +
> +/* Register Offsets */
> +#define REG_STATUS  0x00
> +#define REG_CTRL    0x04
> +#define REG_COORD0  0x08
> +#define REG_COORD1  0x0C
> +#define REG_COLOR   0x10
> +#define REG_ISR     0x14
> +#define REG_IER     0x18
> +
> +/* Bit Masks */
> +#define INT_DONE    BIT(0)
> +#define INT_VSYNC   BIT(1)
> +
> +#define STATUS_BUSY BIT(0)
> +#define CMD_CLEAR   (0x1)
> +#define CMD_RECT    (0x2)
> +#define CMD_LINE    (0x3)
> +#define CTRL_START  BIT(4)
> +
> +struct glanda_device {
> +	struct drm_device drm;
> +
> +	/* hw */
> +	void __iomem *mmio_base;
> +	void __iomem *vram_base;
> +	phys_addr_t vram_phys;
> +
> +	int irq;
> +
> +	/* drm */
> +	struct drm_plane primary_plane;
> +	struct drm_crtc crtc;
> +	struct drm_encoder encoder;
> +	struct drm_connector connector;
> +};
> +
> +#define to_glanda(dev) container_of_const(dev, struct glanda_device, drm)
> +
> +static const u32 glanda_plane_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static void glanda_blit_rect(struct glanda_device *gdev,
> +			     const struct drm_rect *dst_clip,
> +			     const struct iosys_map *src,
> +			     struct drm_framebuffer *fb,
> +			     int dst_off_x, int dst_off_y)
> +{
> +	unsigned int src_pitch = fb->pitches[0];
> +	unsigned int width = drm_rect_width(dst_clip);
> +	unsigned int height = drm_rect_height(dst_clip);
> +	unsigned int x, y;
> +
> +	for (y = 0; y < height; y++) {
> +		unsigned int dst_y = dst_clip->y1 + y;
> +		unsigned int src_y = dst_y - dst_off_y;
> +		u32 __iomem *dst = (u32 __iomem *)gdev->vram_base +
> +				   (size_t)dst_y * GLANDA_WIDTH + dst_clip->x1;
> +		size_t src_off = (size_t)src_y * src_pitch +
> +				 (size_t)(dst_clip->x1 - dst_off_x) * sizeof(u32);
> +
> +		for (x = 0; x < width; x++) {
> +			u32 pixel = iosys_map_rd(src, src_off + x * sizeof(u32), u32);

Please see the review from Sashiko. You can use iosys_map_memcpy_from() 
to avoid the problem with unaligned pointers.  IIRC the _rd and _wr 
functions where rather intended for accessing HW registers with known 
alignment.

> +			u32 packed;
> +
> +			pixel = le32_to_cpu((__force __le32)pixel);
> +			packed = ((pixel >> 12) & 0x0F00) |
> +				((pixel >> 8) & 0x00F0) |
> +				((pixel >> 4) & 0x000F);
> +
> +			writel_relaxed(packed, &dst[x]);
> +		}
> +	}
> +}
> +
> +static void glanda_plane_atomic_update(struct drm_plane *plane,
> +				       struct drm_atomic_commit *state)
> +{
> +	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, plane);
> +	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_shadow_plane_state *shadow_state = to_drm_shadow_plane_state(new_state);
> +	struct drm_rect vram_clip = DRM_RECT_INIT(0, 0, GLANDA_WIDTH, GLANDA_HEIGHT);
> +	struct glanda_device *gdev = to_glanda(plane->dev);
> +	struct drm_framebuffer *fb = new_state->fb;
> +	struct drm_atomic_helper_damage_iter iter;
> +	struct drm_rect damage;
> +	int dst_off_x, dst_off_y;
> +	int ret, idx;
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return;
> +
> +	if (!drm_dev_enter(plane->dev, &idx))
> +		goto out_drm_gem_fb_end_cpu_access;
> +
> +	dst_off_x = new_state->dst.x1 - (new_state->src.x1 >> 16);
> +	dst_off_y = new_state->dst.y1 - (new_state->src.y1 >> 16);
> +
> +	drm_atomic_helper_damage_iter_init(&iter, old_state, new_state);
> +	drm_atomic_for_each_plane_damage(&iter, &damage) {
> +		struct drm_rect dst_clip = new_state->dst;
> +
> +		drm_rect_translate(&damage, dst_off_x, dst_off_y);
> +
> +		if (!drm_rect_intersect(&dst_clip, &damage))
> +			continue;
> +		if (!drm_rect_intersect(&dst_clip, &vram_clip))
> +			continue;
> +
> +		glanda_blit_rect(gdev, &dst_clip, &shadow_state->data[0], fb,
> +				 dst_off_x, dst_off_y);
> +	}
> +
> +	drm_dev_exit(idx);
> +out_drm_gem_fb_end_cpu_access:
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +}
> +
> +static void glanda_plane_atomic_disable(struct drm_plane *plane,
> +					struct drm_atomic_commit *state)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct glanda_device *gdev = to_glanda(dev);
> +	int idx;
> +
> +	if (!drm_dev_enter(dev, &idx))
> +		return;
> +
> +	memset_io(gdev->vram_base, 0, GLANDA_WIDTH * sizeof(u32) * GLANDA_HEIGHT);
> +	drm_dev_exit(idx);
> +}
> +
> +static int glanda_plane_atomic_check(struct drm_plane *plane,
> +				     struct drm_atomic_commit *state)
> +{
> +	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_crtc_state *new_crtc_state = NULL;
> +	int ret;
> +
> +	if (new_plane_state->crtc)
> +		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
> +
> +	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
> +						  DRM_PLANE_NO_SCALING, DRM_PLANE_NO_SCALING,
> +		false,	/* can_position */
> +		false); /* can_update_disabled */
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct drm_plane_helper_funcs glanda_plane_helper_funcs = {
> +	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> +	.atomic_update = glanda_plane_atomic_update,
> +	.atomic_check = glanda_plane_atomic_check,
> +	.atomic_disable = glanda_plane_atomic_disable,
> +};
> +
> +static const struct drm_plane_funcs glanda_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	DRM_GEM_SHADOW_PLANE_FUNCS,
> +};
> +
> +static int glanda_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_create(connector->dev);
> +	if (!mode) {
> +		dev_err(connector->dev->dev, "GlandaGPU: failed to create display mode\n");
> +		return 0;
> +	}
> +
> +	/* Standard VGA timing: 640x480 @ 60 Hz. */
> +	mode->hdisplay = 640;
> +	mode->hsync_start = 656;
> +	mode->hsync_end = 752;
> +	mode->htotal = 800;
> +
> +	mode->vdisplay = 480;
> +	mode->vsync_start = 490;
> +	mode->vsync_end = 492;
> +	mode->vtotal = 525;
> +
> +	mode->clock = 25175;	/* 25.175 MHz pixel clock */
> +
> +	mode->flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC;
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static int glanda_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> +	struct glanda_device *gdev = to_glanda(crtc->dev);
> +	u32 ier;
> +	int idx;
> +
> +	if (gdev->irq <= 0)
> +		return -EINVAL;
> +
> +	if (!drm_dev_enter(crtc->dev, &idx))
> +		return -ENODEV;
> +
> +	ier = readl(gdev->mmio_base + REG_IER);
> +	writel(ier | INT_VSYNC, gdev->mmio_base + REG_IER);
> +
> +	drm_dev_exit(idx);
> +	return 0;
> +}
> +
> +static void glanda_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> +	struct glanda_device *gdev = to_glanda(crtc->dev);
> +	u32 ier;
> +	int idx;
> +
> +	if (!drm_dev_enter(crtc->dev, &idx))
> +		return;
> +
> +	ier = readl(gdev->mmio_base + REG_IER);
> +	writel(ier & ~INT_VSYNC, gdev->mmio_base + REG_IER);
> +
> +	drm_dev_exit(idx);
> +}
> +
> +static void glanda_crtc_atomic_flush(struct drm_crtc *crtc,
> +				     struct drm_atomic_commit *state)
> +{
> +	struct glanda_device *gdev = to_glanda(crtc->dev);
> +	struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	struct drm_pending_vblank_event *event;
> +
> +	if (new_state->event) {
> +		event = new_state->event;
> +		new_state->event = NULL;
> +
> +		spin_lock_irq(&crtc->dev->event_lock);
> +
> +		if (gdev->irq > 0 && drm_crtc_vblank_get(crtc) == 0)
> +			drm_crtc_arm_vblank_event(crtc, event);
> +		else
> +			drm_crtc_send_vblank_event(crtc, event);
> +
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +	}
> +}
> +
> +static const struct drm_crtc_funcs glanda_crtc_funcs = {
> +	.destroy = drm_crtc_cleanup,
> +	.set_config = drm_atomic_helper_set_config,
> +	.page_flip = drm_atomic_helper_page_flip,
> +	.reset = drm_atomic_helper_crtc_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +	.enable_vblank = glanda_crtc_enable_vblank,
> +	.disable_vblank = glanda_crtc_disable_vblank,
> +};
> +
> +static const struct drm_crtc_helper_funcs glanda_crtc_helper_funcs = {
> +	.atomic_enable = drm_crtc_vblank_atomic_enable,
> +	.atomic_disable = drm_crtc_vblank_atomic_disable,
> +	.atomic_flush = glanda_crtc_atomic_flush,
> +};
> +
> +static const struct drm_connector_helper_funcs glanda_connector_helper_funcs = {
> +	.get_modes = glanda_connector_get_modes,
> +};
> +
> +static const struct drm_encoder_funcs glanda_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static const struct drm_connector_funcs glanda_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_mode_config_funcs glanda_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create_with_dirty,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +DEFINE_DRM_GEM_FOPS(glanda_drm_fops);
> +
> +static const struct drm_driver glanda_drm_driver = {
> +	.driver_features =
> +	    DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.name = "glandagpu",
> +	.desc = "GlandaGPU DRM Driver",
> +	.major = 1,
> +	.minor = 0,
> +	.fops = &glanda_drm_fops,
> +	.dumb_create = drm_gem_shmem_dumb_create,
> +};
> +
> +static irqreturn_t glanda_irq_handler(int irq, void *dev_id)
> +{
> +	struct glanda_device *gdev = dev_id;
> +	u32 isr, ier;
> +
> +	if (!gdev || !gdev->mmio_base)
> +		return IRQ_NONE;
> +
> +	isr = readl(gdev->mmio_base + REG_ISR);
> +	ier = readl(gdev->mmio_base + REG_IER);
> +
> +	if (!(isr & ier))
> +		return IRQ_NONE;
> +
> +	if (isr & INT_VSYNC)
> +		drm_crtc_handle_vblank(&gdev->crtc);
> +
> +	/* Clear interrupt(W1C) */
> +	writel(isr, gdev->mmio_base + REG_ISR);
> +	return IRQ_HANDLED;
> +}
> +
> +/* Common DRM setup once MMIO/VRAM/IRQ are known(used by both probe paths) */
> +static int glanda_drm_init(struct glanda_device *gdev, int irq)
> +{
> +	int ret;
> +
> +	gdev->irq = -1;
> +
> +	writel(0, gdev->mmio_base + REG_IER);
> +	writel(0xFFFFFFFF, gdev->mmio_base + REG_ISR);	/* clear flags */
> +
> +	/* DRM mode config */
> +	ret = drmm_mode_config_init(&gdev->drm);
> +	if (ret)
> +		return ret;
> +
> +	gdev->drm.mode_config.min_width = 640;
> +	gdev->drm.mode_config.min_height = 480;
> +	gdev->drm.mode_config.max_width = DRM_SHADOW_PLANE_MAX_WIDTH;
> +	gdev->drm.mode_config.max_height = DRM_SHADOW_PLANE_MAX_HEIGHT;
> +	gdev->drm.mode_config.funcs = &glanda_mode_config_funcs;
> +
> +	ret = drm_universal_plane_init(&gdev->drm, &gdev->primary_plane, 1 << 0,
> +				       &glanda_plane_funcs,
> +				       glanda_plane_formats,
> +				       ARRAY_SIZE(glanda_plane_formats), NULL,
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		drm_err(&gdev->drm, "Failed to initialize primary plane\n");
> +		return ret;
> +	}
> +	drm_plane_helper_add(&gdev->primary_plane, &glanda_plane_helper_funcs);
> +
> +	drm_plane_enable_fb_damage_clips(&gdev->primary_plane);
> +
> +	/* CRTC init */
> +	ret = drm_crtc_init_with_planes(&gdev->drm, &gdev->crtc,
> +					&gdev->primary_plane, NULL,
> +					&glanda_crtc_funcs, NULL);
> +	if (ret) {
> +		drm_err(&gdev->drm, "Failed to initialize CRTC with planes\n");
> +		return ret;
> +	}
> +	drm_crtc_helper_add(&gdev->crtc, &glanda_crtc_helper_funcs);
> +
> +	ret = drm_encoder_init(&gdev->drm, &gdev->encoder, &glanda_encoder_funcs,
> +			       DRM_MODE_ENCODER_DAC, NULL);
> +	if (ret) {
> +		drm_err(&gdev->drm, "Failed to initialize encoder\n");
> +		return ret;
> +	}
> +	gdev->encoder.possible_crtcs = 1;
> +
> +	ret = drm_connector_init(&gdev->drm, &gdev->connector,
> +				 &glanda_connector_funcs, DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		drm_err(&gdev->drm, "Failed to initialize connector\n");
> +		return ret;
> +	}
> +	drm_connector_helper_add(&gdev->connector, &glanda_connector_helper_funcs);
> +
> +	drm_connector_attach_encoder(&gdev->connector, &gdev->encoder);
> +
> +	/* VBlank init */
> +	ret = drm_vblank_init(&gdev->drm, 1);
> +	if (ret) {
> +		drm_err(&gdev->drm, "Failed to initialize vblank\n");
> +		return ret;
> +	}
> +
> +	drm_mode_config_reset(&gdev->drm);
> +
> +	if (irq > 0) {
> +		gdev->irq = irq;
> +		ret = devm_request_irq(gdev->drm.dev, gdev->irq, glanda_irq_handler,
> +				       IRQF_SHARED, "glandagpu", gdev);
> +		if (ret) {
> +			drm_err(&gdev->drm, "Failed to request IRQ %d\n",
> +				gdev->irq);
> +			return ret;
> +		}
> +	} else {
> +		drm_warn(&gdev->drm, "No IRQ found, falling back to polling\n");
> +	}

Dumb question: wouldn't it be easier to have a hard requirement on the 
irq. And if none is configured, just fail probing?

As it is now, your driver tries to mitigate that problem, but it 
wouldn't likely work reliably anyway.


> +
> +	ret = drm_dev_register(&gdev->drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Shared teardown, mirrors glanda_drm_init() */
> +static void glanda_drm_fini(struct glanda_device *gdev)
> +{
> +	drm_dev_unplug(&gdev->drm);

Sashiko warns about DRM _shutdown missing here. Maybe add it back for 
now so that enabled resources get cleaned up (if any).

Best regards
Thomas


> +}
> +
> +static int glandagpu_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct glanda_device *gdev;
> +	int irq;
> +
> +	gdev = devm_drm_dev_alloc(&pdev->dev, &glanda_drm_driver, struct glanda_device, drm);
> +	if (IS_ERR(gdev))
> +		return PTR_ERR(gdev);
> +
> +	platform_set_drvdata(pdev, gdev);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	if (resource_size(res) < GLANDA_MMIO_OFFSET + GLANDA_MMIO_SIZE) {
> +		dev_err(&pdev->dev, "MMIO region too small: %llu bytes, need at least %u\n",
> +			(unsigned long long)resource_size(res),
> +			GLANDA_MMIO_OFFSET + GLANDA_MMIO_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	gdev->vram_phys = res->start;
> +	gdev->vram_base = devm_ioremap_wc(&pdev->dev, res->start, GLANDA_VRAM_SIZE);
> +	gdev->mmio_base = devm_ioremap(&pdev->dev, res->start + GLANDA_MMIO_OFFSET,
> +				       GLANDA_MMIO_SIZE);
> +	if (!gdev->vram_base || !gdev->mmio_base) {
> +		drm_err(&gdev->drm, "failed to ioremap\n");
> +		return -ENOMEM;
> +	}
> +
> +	irq = platform_get_irq_optional(pdev, 0);
> +	if (irq == -ENXIO)
> +		irq = -1;	/* no IRQ resource, fall back to polling */
> +	else if (irq < 0)
> +		return irq;
> +
> +	return glanda_drm_init(gdev, irq);
> +}
> +
> +static void glandagpu_remove(struct platform_device *pdev)
> +{
> +	glanda_drm_fini(platform_get_drvdata(pdev));
> +}
> +
> +/* Device Tree match table. */
> +static const struct of_device_id glanda_of_match[] = {
> +	{ .compatible = "kieweg,gpu-1.0" },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, glanda_of_match);
> +
> +static struct platform_driver glandagpu_driver = {
> +	.driver = {
> +		.name = "glandagpu",
> +		.of_match_table = glanda_of_match,
> +	},
> +	.probe = glandagpu_probe,
> +	.remove = glandagpu_remove,
> +};
> +
> +/* PCI probe path for the QEMU test device, real hardware uses platform_driver */
> +#ifdef CONFIG_PCI
> +static int glandagpu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct glanda_device *gdev;
> +	int ret;
> +
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +	pci_set_master(pdev);
> +
> +	if (pci_resource_len(pdev, 0) < GLANDA_MMIO_SIZE ||
> +	    pci_resource_len(pdev, 1) < GLANDA_VRAM_SIZE) {
> +		dev_err(&pdev->dev, "BAR too small: BAR0=%llu (need %u), BAR1=%llu (need %u)\n",
> +			(unsigned long long)pci_resource_len(pdev, 0), GLANDA_MMIO_SIZE,
> +			(unsigned long long)pci_resource_len(pdev, 1), GLANDA_VRAM_SIZE);
> +		return -EINVAL;
> +	}
> +
> +	ret = pcim_iomap_regions(pdev, BIT(0), "glandagpu");
> +	if (ret)
> +		return ret;
> +
> +	ret = pcim_request_region(pdev, 1, "glandagpu");
> +	if (ret)
> +		return ret;
> +
> +	gdev = devm_drm_dev_alloc(&pdev->dev, &glanda_drm_driver, struct glanda_device, drm);
> +	if (IS_ERR(gdev))
> +		return PTR_ERR(gdev);
> +
> +	pci_set_drvdata(pdev, gdev);
> +
> +	gdev->mmio_base = pcim_iomap_table(pdev)[0];
> +	gdev->vram_phys = pci_resource_start(pdev, 1);
> +
> +	gdev->vram_base = devm_ioremap_wc(&pdev->dev, gdev->vram_phys, GLANDA_VRAM_SIZE);
> +	if (!gdev->vram_base)
> +		return -ENOMEM;
> +
> +	return glanda_drm_init(gdev, pdev->irq);
> +}
> +
> +static void glandagpu_pci_remove(struct pci_dev *pdev)
> +{
> +	glanda_drm_fini(pci_get_drvdata(pdev));
> +}
> +
> +static const struct pci_device_id glanda_pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_DEVICE_ID_GLANDA_GPU) },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, glanda_pci_ids);
> +
> +static struct pci_driver glandagpu_pci_driver = {
> +	.name = "glandagpu-pci",
> +	.id_table = glanda_pci_ids,
> +	.probe = glandagpu_pci_probe,
> +	.remove = glandagpu_pci_remove,
> +};
> +#endif /* CONFIG_PCI */
> +
> +static int __init glandagpu_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&glandagpu_driver);
> +	if (ret) {
> +		pr_err("GlandaGPU: Failed to register platform driver\n");
> +		return ret;
> +	}
> +
> +#ifdef CONFIG_PCI
> +	ret = pci_register_driver(&glandagpu_pci_driver);
> +	if (ret) {
> +		pr_err("GlandaGPU: Failed to register PCI driver\n");
> +		platform_driver_unregister(&glandagpu_driver);
> +		return ret;
> +	}
> +#endif
> +
> +	return 0;
> +}
> +
> +static void __exit glandagpu_exit(void)
> +{
> +#ifdef CONFIG_PCI
> +	pci_unregister_driver(&glandagpu_pci_driver);
> +#endif
> +	platform_driver_unregister(&glandagpu_driver);
> +}
> +
> +module_init(glandagpu_init);
> +module_exit(glandagpu_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Leander Kieweg <kieweg.leander@gmail.com>");
> +MODULE_DESCRIPTION("DRM driver for GlandaGPU, an FPGA-based 2D GPU with VGA output");

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU
  2026-09-18  7:34   ` Thomas Zimmermann
@ 2026-09-20 19:39     ` Leander Kieweg
  0 siblings, 0 replies; 8+ messages in thread
From: Leander Kieweg @ 2026-09-20 19:39 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: dri-devel, devicetree, airlied, simona, maarten.lankhorst,
	mripard, robh, krzk+dt, conor+dt, u.kleine-koenig

> Hi,
>
> please see the Sashiko bot's review for several details that might be
> worth addressing. I've also left a few comments below. Apart from that
> the driver looks quite nice already.

Hi Thomas,

thanks for the review. I'll address all of it (yours and the bot's)
in v5, including making the IRQ a hard requirement instead of the
polling fallback.

Best regards,
Leander

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-20 19:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-13 20:50 [PATCH v4 0/2] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU) Leander Kieweg
2026-09-13 20:50 ` [PATCH v4 1/2] dt-bindings: display: Add GlandaGPU binding Leander Kieweg
2026-09-17  8:05   ` Krzysztof Kozlowski
2026-09-17 10:45     ` Leander Kieweg
2026-09-13 20:50 ` [PATCH v4 2/2] drm/glanda: Add initial DRM driver for GlandaGPU Leander Kieweg
2026-09-13 21:03   ` sashiko-bot
2026-09-18  7:34   ` Thomas Zimmermann
2026-09-20 19:39     ` Leander Kieweg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.