[RFC PATCH 0/3] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU)

[RFC PATCH 0/3] drm: Add DRM driver for GlandaGPU (VHDL soft-IP GPU)

[Leander Kieweg]

Hi everyone,

This series adds a DRM driver for GlandaGPU, a custom open-source 2D
graphics core I designed in VHDL. It currently runs on a Cyclone V
SoC (Terasic DE10-Standard), with VGA output at 640x480@60Hz. I also
built a QEMU fork that emulates the same MMIO/VRAM interface as a
digital twin, so I can develop and test without needing the FPGA
board.

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

I'm sending this as an RFC because I'd like feedback on the
following points before cleaning up the series further:

1) Mainline viability of custom FPGA hardware

   GlandaGPU isn't a commercial chip, it's an open-source soft GPU
   core. The DE10-Standard itself is a regular purchasable dev
   board, and the RTL is public, so anyone can reproduce the exact
   hardware. Testing doesn't require the FPGA either, since the
   QEMU twin models the same interface. I know this is unusual
   compared to typical mainline DRM hardware. I'd like to know
   whether this is workable, or whether it's a dealbreaker for
   mainline.

2) UAPI / ioctls / userspace

   The driver currently exposes three fixed ioctls (CLEAR,
   DRAW_RECT, DRAW_LINE), mapping directly onto the current
   hardware command set. I have two related questions here:

   a) I plan to keep developing the hardware further, which will
      likely mean more ioctls over time (for example, polygon/3D
      rendering is one direction I'm considering). Is it acceptable
      to keep adding a new, separate ioctl for each drawing
      primitive like this, or should I move to a generic
      command-buffer submission model instead, similar to
      DRM_IOCTL_VIRTGPU_EXECBUFFER in virtio_gpu, before this is
      treated as stable?

   b) If I do end up with an ioctl-based acceleration UAPI, is
      writing a Mesa/Gallium3D driver the expected way to make it
      usable from userspace, or is there a lighter-weight option
      that makes more sense for a project this size?

3) x86 QEMU platform test device

   To let reviewers try the driver against the QEMU twin without
   cross-compiling an ARM kernel and rootfs, I registered a
   platform device at a fixed address on x86. I'm aware this
   doesn't belong in the driver itself, so I isolated it into patch
   3/3 and marked it "NOT FOR MERGE". Let me know if you'd rather
   see it dropped entirely, relying only on documentation of the
   QEMU-on-ARM testing path instead.

4) Pixel format conversion

   glanda_pipe_update() currently does a per-pixel software
   conversion from XRGB8888 into the hardware's native packed
   format on every flip. I'm aware this is a known bottleneck. I'm
   planning to extend the VHDL to accept XRGB8888 natively so I can
   drop this conversion entirely. I'm flagging it here as a known
   limitation rather than blocking on it, since it's a hardware-side
   change and doesn't affect the UAPI.

5) drm_simple_display_pipe vs. manual plane/CRTC/encoder

   Since the driver only has a single plane, CRTC, and encoder, I
   tried converting it to use drm_simple_display_pipe instead of the
   manual setup. It compiled cleanly, but my userspace tests didn't
   behave the way I expected. I haven't figured out why yet, so I
   kept the manual setup for this RFC. I'm open to revisiting this
   if that's the preferred direction.

Testing Status:

The driver has been tested and verified on both the QEMU fork (x86)
and physical FPGA hardware (ARM) using:
  - A custom static userspace test: https://github.com/stiangglanda/GlandaGPU-userspace-tests
  - `modetest -M glandagpu -s 36:640x480 -v` (which successfully
    displays the test pattern)

Thanks for any feedback,
Leander Kieweg

Leander Kieweg (3):
  dt-bindings: display: Add GlandaGPU binding
  drm/glanda: Add initial DRM driver for GlandaGPU
  NOT FOR MERGE: drm/glanda: Add x86 platform test device

 .../bindings/display/glanda,gpu.yaml          |  49 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/tiny/Kconfig                  |  25 +
 drivers/gpu/drm/tiny/Makefile                 |   1 +
 drivers/gpu/drm/tiny/glandagpu.c              | 808 ++++++++++++++++++
 include/uapi/drm/glanda_drm.h                 |  40 +
 7 files changed, 932 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/glanda,gpu.yaml
 create mode 100644 drivers/gpu/drm/tiny/glandagpu.c
 create mode 100644 include/uapi/drm/glanda_drm.h

-- 
2.43.0

[RFC PATCH 1/3] dt-bindings: display: Add GlandaGPU binding

[Leander Kieweg]

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

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

diff --git a/Documentation/devicetree/bindings/display/glanda,gpu.yaml b/Documentation/devicetree/bindings/display/glanda,gpu.yaml
new file mode 100644
index 000000000..40304e773
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/glanda,gpu.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/glanda,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 (clear/rect/line) with a
+  VGA-compatible display output. The register window covers a
+  combined VRAM + MMIO region, with MMIO registers at a fixed
+  offset within it.
+
+properties:
+  compatible:
+    const: glanda,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
+
+additionalProperties: false
+
+examples:
+  - |
+    gpu@c0000000 {
+        compatible = "glanda,gpu-1.0";
+        reg = <0xc0000000 0x1000000>;
+        interrupts = <0 43 4>;
+        clocks = <&osc1>;
+    };
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b1af9deac..b8b7a5401 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -666,6 +666,8 @@ patternProperties:
     description: Giantplus Technology Co., Ltd.
   "^gira,.*":
     description: Gira Giersiepen GmbH & Co. KG
+  "^glanda,.*":
+    description: GlandaGPU
   "^glinet,.*":
     description: GL Intelligence, Inc.
   "^globalscale,.*":
-- 
2.43.0

Re: [RFC PATCH 1/3] dt-bindings: display: Add GlandaGPU binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing newline at the end of the new YAML binding file.
--

--- Patch [1]: [RFC PATCH 1/3] dt-bindings: display: Add GlandaGPU binding ---
commit 13be4be8578f4fe27e2424184082468464c6c35a
Author: Leander Kieweg <kieweg.leander@gmail.com>

dt-bindings: display: Add GlandaGPU binding

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

> diff --git a/Documentation/devicetree/bindings/display/glanda,gpu.yaml b/Documentation/devicetree/bindings/display/glanda,gpu.yaml
> new file mode 100644
> index 0000000000000..40304e77382a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/glanda,gpu.yaml

[ ... ]

> +examples:
> +  - |
> +    gpu@c0000000 {
> +        compatible = "glanda,gpu-1.0";
> +        reg = <0xc0000000 0x1000000>;
> +        interrupts = <0 43 4>;
> +        clocks = <&osc1>;
> +    };
> \ No newline at end of file

[Severity: Low]
This isn't a bug, but should this new YAML file end with a newline? 

Missing newlines can trigger styling warnings during yamllint checks when
running make dt_binding_check.

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

[RFC PATCH 2/3] drm/glanda: Add initial DRM driver for GlandaGPU

[Leander Kieweg]

Introduce the core DRM/KMS driver for GlandaGPU. This driver
supports basic modesetting, atomic updates, and custom 2D hardware
acceleration IOCTLs.

Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
---
 MAINTAINERS                      |   7 +
 drivers/gpu/drm/tiny/Kconfig     |  11 +
 drivers/gpu/drm/tiny/Makefile    |   1 +
 drivers/gpu/drm/tiny/glandagpu.c | 769 +++++++++++++++++++++++++++++++
 include/uapi/drm/glanda_drm.h    |  40 ++
 5 files changed, 828 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/glandagpu.c
 create mode 100644 include/uapi/drm/glanda_drm.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6dea93a41..49bbb5c43 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8076,6 +8076,13 @@ 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/glanda,gpu.yaml
+F:	drivers/gpu/drm/tiny/glandagpu.c
+F:	include/uapi/drm/glanda_drm.h
+
 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..7a15bf95a 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
+    select DRM_KMS_HELPER
+    select DRM_GEM_SHMEM_HELPER
+	help
+	  DRM/KMS driver for the GlandaGPU hardware-accelerated 2D
+	  display controller (FPGA soft IP). This driver supports
+	  basic modesetting, dumb buffers, and simple 2D drawing
+	  acceleration via custom hardware IOCTLs.
+
 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..8f87ae096
--- /dev/null
+++ b/drivers/gpu/drm/tiny/glandagpu.c
@@ -0,0 +1,769 @@
+// 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/io.h>
+#include <linux/delay.h>	/* udelay (polling) */
+#include <linux/mod_devicetable.h>	/* Device Tree parsing */
+#include <linux/of.h>
+#include <linux/slab.h>		/* GFP_KERNEL */
+#include <linux/fs.h>
+#include <linux/cdev.h>
+#include <linux/uaccess.h>
+#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_framebuffer.h>
+#include <drm/drm_vblank.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_simple_kms_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>
+
+#include <uapi/drm/glanda_drm.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
+
+/* Base addresses used by the x86 test device. */
+#define BRIDGE_BASE       0xC0000000
+#define GLANDA_VRAM_BASE  (BRIDGE_BASE + 0x00000000)
+#define GLANDA_MMIO_BASE  (BRIDGE_BASE + GLANDA_MMIO_OFFSET)
+#define GLANDA_BASE_SIZE  (BRIDGE_BASE + 0x01000000 - 1)
+
+/* 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;
+	struct device *dev;
+	phys_addr_t vram_phys;
+
+	int irq;
+	wait_queue_head_t cmd_wq;
+	bool cmd_done;
+
+	struct mutex lock; /* for every ineration with the hardware */
+
+	/* drm */
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
+
+#define to_glanda(dev) container_of(dev, struct glanda_device, drm)
+
+static const u32 glanda_plane_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static int glanda_wait_idle(struct glanda_device *gdev)
+{
+	int ret;
+	unsigned int status;
+
+	status = readl(gdev->mmio_base + REG_STATUS);
+	if (!(status & STATUS_BUSY))
+		return 0;
+
+	if (gdev->irq < 0) {
+		int timeout = 10000;
+
+		do {
+			status = readl(gdev->mmio_base + REG_STATUS);
+			if (!(status & STATUS_BUSY))
+				return 0;
+			udelay(1);
+		} while (--timeout > 0);
+
+		dev_err(gdev->dev, "GlandaGPU: polling wait_idle timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	gdev->cmd_done = false;
+
+	ret = wait_event_interruptible_timeout(gdev->cmd_wq, gdev->cmd_done ||
+					       !(readl(gdev->mmio_base + REG_STATUS) & STATUS_BUSY),
+					       msecs_to_jiffies(500));
+
+	if (ret == 0) {
+		dev_err(gdev->dev, "GlandaGPU: IRQ wait_idle timeout\n");
+		return -ETIMEDOUT;
+	} else if (ret < 0) {
+		return ret;
+	}
+
+	return 0;
+}
+
+static int glanda_hw_clear(struct glanda_device *gdev, int color)
+{
+	u32 ctrl;
+	int ret;
+
+	if (mutex_lock_interruptible(&gdev->lock))
+		return -ERESTARTSYS;
+
+	ret = glanda_wait_idle(gdev);
+	if (ret) {
+		mutex_unlock(&gdev->lock);
+		return ret;
+	}
+
+	writel(color, gdev->mmio_base + REG_COLOR);
+	ctrl = CTRL_START | CMD_CLEAR;
+	writel(ctrl, gdev->mmio_base + REG_CTRL);
+
+	mutex_unlock(&gdev->lock);
+	return 0;
+}
+
+static int glanda_hw_draw_rect(struct glanda_device *gdev,
+			       int x, int y, int w, int h, int color)
+{
+	u32 coord0, coord1, ctrl;
+	int ret;
+
+	if (mutex_lock_interruptible(&gdev->lock))
+		return -ERESTARTSYS;
+
+	ret = glanda_wait_idle(gdev);
+	if (ret) {
+		mutex_unlock(&gdev->lock);
+		return ret;
+	}
+
+	coord0 = (y << 16) | (x & 0x3FF);
+	coord1 = (h << 16) | (w & 0x3FF);
+
+	writel(coord0, gdev->mmio_base + REG_COORD0);
+	writel(coord1, gdev->mmio_base + REG_COORD1);
+	writel(color, gdev->mmio_base + REG_COLOR);
+
+	ctrl = CTRL_START | CMD_RECT;
+	writel(ctrl, gdev->mmio_base + REG_CTRL);
+
+	mutex_unlock(&gdev->lock);
+	return 0;
+}
+
+static int glanda_hw_draw_line(struct glanda_device *gdev,
+			       int x1, int y1, int x2, int y2, int color)
+{
+	u32 coord0, coord1, ctrl;
+	int ret;
+
+	if (mutex_lock_interruptible(&gdev->lock))
+		return -ERESTARTSYS;
+
+	ret = glanda_wait_idle(gdev);
+	if (ret) {
+		mutex_unlock(&gdev->lock);
+		return ret;
+	}
+
+	coord0 = (y1 << 16) | (x1 & 0x3FF);
+	coord1 = (y2 << 16) | (x2 & 0x3FF);
+
+	writel(coord0, gdev->mmio_base + REG_COORD0);
+	writel(coord1, gdev->mmio_base + REG_COORD1);
+	writel(color, gdev->mmio_base + REG_COLOR);
+
+	ctrl = CTRL_START | CMD_LINE;
+	writel(ctrl, gdev->mmio_base + REG_CTRL);
+
+	mutex_unlock(&gdev->lock);
+	return 0;
+}
+
+static int glanda_drm_ioctl_clear(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv)
+{
+	struct glanda_device *gdev = to_glanda(dev);
+	struct glanda_clear_cmd *cmd = data;
+
+	return glanda_hw_clear(gdev, cmd->color);
+}
+
+static bool glanda_rect_cmd_is_valid(const struct glanda_draw_rect_cmd *cmd)
+{
+	if (cmd->x >= GLANDA_WIDTH || cmd->y >= GLANDA_HEIGHT)
+		return false;
+	if (cmd->w > GLANDA_WIDTH || cmd->h > GLANDA_HEIGHT)
+		return false;
+	if (cmd->x + cmd->w > GLANDA_WIDTH)
+		return false;
+	if (cmd->y + cmd->h > GLANDA_HEIGHT)
+		return false;
+
+	return true;
+}
+
+static int glanda_drm_ioctl_draw_rect(struct drm_device *dev, void *data,
+				      struct drm_file *file_priv)
+{
+	struct glanda_device *gdev = to_glanda(dev);
+	struct glanda_draw_rect_cmd *cmd = data;
+
+	if (!glanda_rect_cmd_is_valid(cmd))
+		return -EINVAL;
+
+	return glanda_hw_draw_rect(gdev, cmd->x, cmd->y, cmd->w, cmd->h,
+				   cmd->color);
+}
+
+static bool glanda_line_cmd_is_valid(const struct glanda_draw_line_cmd *cmd)
+{
+	if (cmd->x0 >= GLANDA_WIDTH || cmd->y0 >= GLANDA_HEIGHT)
+		return false;
+	if (cmd->x1 >= GLANDA_WIDTH || cmd->y1 >= GLANDA_HEIGHT)
+		return false;
+
+	return true;
+}
+
+static int glanda_drm_ioctl_draw_line(struct drm_device *dev, void *data,
+				      struct drm_file *file_priv)
+{
+	struct glanda_device *gdev = to_glanda(dev);
+	struct glanda_draw_line_cmd *cmd = data;
+
+	if (!glanda_line_cmd_is_valid(cmd))
+		return -EINVAL;
+
+	return glanda_hw_draw_line(gdev, cmd->x0, cmd->y0, cmd->x1, cmd->y1,
+				   cmd->color);
+}
+
+static void glanda_plane_atomic_update(struct drm_plane *plane,
+				       struct drm_atomic_commit *state)
+{
+	struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_framebuffer *fb = new_state->fb;
+	struct glanda_device *gdev = to_glanda(plane->dev);
+	struct drm_gem_shmem_object *shmem;
+	struct iosys_map map;
+	u32 src_pitch;
+	u32 width;
+	u32 height;
+	int ret;
+
+	if (!fb)
+		return;
+
+	shmem = to_drm_gem_shmem_obj(fb->obj[0]);
+	if (!shmem)  {
+		drm_err(&gdev->drm, "GlandaGPU: framebuffer is not a shmem GEM object\n");
+		return;
+	}
+
+	dma_resv_lock(shmem->base.resv, NULL);
+	ret = drm_gem_shmem_vmap_locked(shmem, &map);
+	if (ret) {
+		dma_resv_unlock(shmem->base.resv);
+		drm_err(&gdev->drm,
+			"GlandaGPU: failed to vmap GEM shmem object\n");
+		return;
+	}
+
+	mutex_lock(&gdev->lock);
+
+	ret = glanda_wait_idle(gdev);
+	if (ret) {
+		drm_err(&gdev->drm, "GlandaGPU: timed out waiting for idle\n");
+		mutex_unlock(&gdev->lock);
+		drm_gem_shmem_vunmap_locked(shmem, &map);
+	dma_resv_unlock(shmem->base.resv);
+		return;
+	}
+
+	src_pitch = fb->pitches[0];
+	width = min_t(u32, fb->width, GLANDA_WIDTH);
+	height = min_t(u32, fb->height, GLANDA_HEIGHT);
+
+	u8 __iomem *dst_base = gdev->vram_base;
+	u8 *src_base = map.vaddr;
+	u32 y;
+
+	for (y = 0; y < height; y++) {
+		u32 *src = (u32 *)(src_base + y * src_pitch);
+		u32 __iomem *dst = (u32 __iomem *)(dst_base + y * GLANDA_WIDTH * sizeof(u32));
+		u32 x;
+
+		for (x = 0; x < width; x++) {
+			u32 pixel = src[x];
+			u32 packed = ((pixel >> 12) & 0x0F00) |
+				((pixel >> 8) & 0x00F0) |
+				((pixel >> 4) & 0x000F);
+
+			writel_relaxed(packed, &dst[x]);
+		}
+	}
+
+	mutex_unlock(&gdev->lock);
+	drm_gem_shmem_vunmap_locked(shmem, &map);
+	dma_resv_unlock(shmem->base.resv);
+}
+
+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 *crtc_state;
+
+	if (!new_plane_state->crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
+
+	return drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
+		DRM_PLANE_NO_SCALING, DRM_PLANE_NO_SCALING,
+		false,	/* can_position */
+		false	/* can_update_disabled */);
+}
+
+static const struct drm_plane_helper_funcs glanda_plane_helper_funcs = {
+	.atomic_update = glanda_plane_atomic_update,
+	.atomic_check = glanda_plane_atomic_check,
+};
+
+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,
+	.reset = drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+};
+
+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 enum drm_connector_status glanda_connector_detect(struct drm_connector
+							 *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static int glanda_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+	struct glanda_device *gdev = to_glanda(crtc->dev);
+	u32 ier;
+
+	ier = readl(gdev->mmio_base + REG_IER);
+	writel(ier | INT_VSYNC, gdev->mmio_base + REG_IER);
+
+	return 0;
+}
+
+static void glanda_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+	struct glanda_device *gdev = to_glanda(crtc->dev);
+	u32 ier = readl(gdev->mmio_base + REG_IER);
+
+	writel(ier & ~INT_VSYNC, gdev->mmio_base + REG_IER);
+}
+
+static void glanda_crtc_atomic_enable(struct drm_crtc *crtc,
+				      struct drm_atomic_commit *state)
+{
+	drm_crtc_vblank_on(crtc);
+}
+
+static void glanda_crtc_atomic_disable(struct drm_crtc *crtc,
+				       struct drm_atomic_commit *state)
+{
+	drm_crtc_vblank_off(crtc);
+}
+
+static void glanda_crtc_atomic_flush(struct drm_crtc *crtc,
+				     struct drm_atomic_commit *state)
+{
+	struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_pending_vblank_event *event;
+
+	if (new_state && new_state->event) {
+		event = new_state->event;
+
+		new_state->event = NULL;
+
+		spin_lock_irq(&crtc->dev->event_lock);
+
+		if (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 = glanda_crtc_atomic_enable,
+	.atomic_disable = glanda_crtc_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_connector_funcs glanda_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.detect = glanda_connector_detect,
+	.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_framebuffer_funcs glanda_fb_funcs = {
+	.destroy = drm_gem_fb_destroy,
+	.create_handle = drm_gem_fb_create_handle,
+	.dirty = drm_atomic_helper_dirtyfb,
+};
+
+static struct drm_framebuffer *glanda_fb_create(struct drm_device *dev,
+						struct drm_file *file,
+						const struct drm_format_info *info,
+						const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	return drm_gem_fb_create_with_funcs(dev, file, info, mode_cmd, &glanda_fb_funcs);
+}
+
+static const struct drm_mode_config_funcs glanda_mode_config_funcs = {
+	.fb_create = glanda_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+/*
+ * RFC NOTE: These three fixed-function ioctls (clear/rect/line) are a
+ * minimal placeholder UAPI to demonstrate the hardware's 2D drawing
+ * capability end-to-end. Given plans to add polygon/3D rendering support
+ * in the future, feedback is explicitly requested on whether a generic
+ * command-buffer submission ioctl (similar to virtio_gpu) would
+ * be a better long-term UAPI direction before this is treated as stable.
+ */
+static const struct drm_ioctl_desc glanda_ioctls[] = {
+	DRM_IOCTL_DEF_DRV(GLANDA_CLEAR, glanda_drm_ioctl_clear,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(GLANDA_DRAW_RECT, glanda_drm_ioctl_draw_rect,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(GLANDA_DRAW_LINE, glanda_drm_ioctl_draw_line,
+			  DRM_AUTH | DRM_RENDER_ALLOW),
+};
+
+DEFINE_DRM_GEM_FOPS(glanda_drm_fops);
+
+static const struct drm_driver glanda_drm_driver = {
+	.driver_features =
+	    DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_RENDER,
+	.name = "glandagpu",
+	.desc = "GlandaGPU Hardware Accelerated DRM Driver",
+	.major = 1,
+	.minor = 0,
+	.fops = &glanda_drm_fops,
+	.dumb_create = drm_gem_shmem_dumb_create,
+	.ioctls = glanda_ioctls,
+	.num_ioctls = ARRAY_SIZE(glanda_ioctls),
+};
+
+static irqreturn_t glanda_irq_handler(int irq, void *dev_id)
+{
+	struct glanda_device *gdev = dev_id;
+
+	if (!gdev || !gdev->mmio_base)
+		return IRQ_NONE;
+
+	u32 isr = readl(gdev->mmio_base + REG_ISR);
+
+	if (!isr)
+		return IRQ_NONE;
+
+	if (isr & INT_DONE) {
+		gdev->cmd_done = true;
+		wake_up_interruptible(&gdev->cmd_wq);
+	}
+
+	if (isr & INT_VSYNC)
+		drm_crtc_handle_vblank(&gdev->crtc);
+
+	/* Clear interrupt(W1C) */
+	writel(isr, gdev->mmio_base + REG_ISR);
+	return IRQ_HANDLED;
+}
+
+static int glandagpu_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct glanda_device *gdev;
+	int ret;
+
+	dev_info(&pdev->dev, "GlandaGPU Probe started\n");
+
+	gdev = devm_drm_dev_alloc(&pdev->dev, &glanda_drm_driver, struct glanda_device, drm);
+	if (IS_ERR(gdev))
+		return PTR_ERR(gdev);
+
+	gdev->dev = &pdev->dev;
+	platform_set_drvdata(pdev, gdev);
+
+	mutex_init(&gdev->lock);
+	/* Interrupt setup */
+	init_waitqueue_head(&gdev->cmd_wq);
+	gdev->irq = -1;
+	/* Map VRAM */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	gdev->vram_phys = res->start;
+	gdev->vram_base = devm_ioremap(&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;
+	}
+
+	writel(0, gdev->mmio_base + REG_IER);
+	writel(0xFFFFFFFF, gdev->mmio_base + REG_ISR);	/* clear flags */
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret > 0) {
+		gdev->irq = ret;
+		ret = devm_request_irq(&pdev->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;
+		}
+
+		writel(INT_DONE, gdev->mmio_base + REG_IER);
+		drm_info(&gdev->drm, "IRQ %d requested and enabled\n", gdev->irq);
+	} else {
+		drm_warn(&gdev->drm, "No IRQ found, falling back to polling\n");
+	}
+
+	/* DRM mode config */
+	drm_mode_config_init(&gdev->drm);
+	gdev->drm.mode_config.min_width = 640;
+	gdev->drm.mode_config.min_height = 480;
+	gdev->drm.mode_config.max_width = 640;
+	gdev->drm.mode_config.max_height = 480;
+	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");
+		goto err_mode_cleanup;
+	}
+	drm_plane_helper_add(&gdev->primary_plane, &glanda_plane_helper_funcs);
+
+	/* VBlank init */
+	ret = drm_vblank_init(&gdev->drm, 1);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize vblank\n");
+		goto err_mode_cleanup;
+	}
+
+	/* 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");
+		goto err_mode_cleanup;
+	}
+	drm_crtc_helper_add(&gdev->crtc, &glanda_crtc_helper_funcs);
+
+	ret = drm_simple_encoder_init(&gdev->drm, &gdev->encoder, DRM_MODE_ENCODER_DAC);
+	if (ret) {
+		drm_err(&gdev->drm, "Failed to initialize encoder\n");
+		goto err_mode_cleanup;
+	}
+	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");
+		goto err_mode_cleanup;
+	}
+	drm_connector_helper_add(&gdev->connector, &glanda_connector_helper_funcs);
+
+	drm_connector_attach_encoder(&gdev->connector, &gdev->encoder);
+
+	/* Populate connector state early so userspace can enumerate modes. */
+	mutex_lock(&gdev->drm.mode_config.mutex);
+	drm_helper_probe_single_connector_modes(&gdev->connector, 1024, 768);
+	mutex_unlock(&gdev->drm.mode_config.mutex);
+
+	drm_mode_config_reset(&gdev->drm);
+
+	ret = drm_dev_register(&gdev->drm, 0);
+	if (ret)
+		goto err_mode_cleanup;
+
+	drm_info(&gdev->drm, "GlandaGPU DRM Initialized (/dev/dri/cardX created)\n");
+	return 0;
+
+err_mode_cleanup:
+	drm_mode_config_cleanup(&gdev->drm);
+	return ret;
+}
+
+static void glandagpu_remove(struct platform_device *pdev)
+{
+	struct glanda_device *gdev = platform_get_drvdata(pdev);
+
+	/* Disable interrupts first so no new IRQ work can race the teardown
+	 * below, and wake up anyone still blocked in glanda_wait_idle().
+	 */
+	writel(0, gdev->mmio_base + REG_IER);
+	gdev->cmd_done = true;
+	wake_up_interruptible(&gdev->cmd_wq);
+
+	drm_info(&gdev->drm, "GlandaGPU DRM Driver removed\n");
+	drm_dev_unregister(&gdev->drm);
+	drm_mode_config_cleanup(&gdev->drm);
+}
+
+/* Device Tree match table. */
+static const struct of_device_id glanda_of_match[] = {
+	{.compatible = "glanda,gpu-1.0", },
+	{ /* end of table */  }
+};
+
+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,
+};
+
+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;
+	}
+
+	pr_info("GlandaGPU: Module loaded successfully\n");
+	return 0;
+}
+
+static void __exit glandagpu_exit(void)
+{
+	platform_driver_unregister(&glandagpu_driver);
+	pr_info("GlandaGPU: Module unloaded\n");
+}
+
+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");
diff --git a/include/uapi/drm/glanda_drm.h b/include/uapi/drm/glanda_drm.h
new file mode 100644
index 000000000..35d25ba83
--- /dev/null
+++ b/include/uapi/drm/glanda_drm.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef _GLANDA_DRM_H_
+#define _GLANDA_DRM_H_
+
+#include <linux/types.h>
+#include <drm/drm.h>
+
+struct glanda_clear_cmd {
+	__u32 color;
+};
+
+struct glanda_draw_rect_cmd {
+	__u16 x;
+	__u16 y;
+	__u16 w;
+	__u16 h;
+	__u32 color;
+};
+
+struct glanda_draw_line_cmd {
+	__u16 x0;
+	__u16 y0;
+	__u16 x1;
+	__u16 y1;
+	__u32 color;
+};
+
+#define DRM_GLANDA_CLEAR     0x00
+#define DRM_GLANDA_DRAW_RECT 0x01
+#define DRM_GLANDA_DRAW_LINE 0x02
+
+#define DRM_IOCTL_GLANDA_CLEAR \
+	DRM_IOW(DRM_COMMAND_BASE + DRM_GLANDA_CLEAR, struct glanda_clear_cmd)
+#define DRM_IOCTL_GLANDA_DRAW_RECT \
+	DRM_IOW(DRM_COMMAND_BASE + DRM_GLANDA_DRAW_RECT, struct glanda_draw_rect_cmd)
+#define DRM_IOCTL_GLANDA_DRAW_LINE \
+	DRM_IOW(DRM_COMMAND_BASE + DRM_GLANDA_DRAW_LINE, struct glanda_draw_line_cmd)
+
+#endif
-- 
2.43.0

Re: [RFC PATCH 2/3] drm/glanda: Add initial DRM driver for GlandaGPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Plane atomic update callback calls sleeping functions, violating DRM atomic context rules.
- [High] Manual call to drm_mode_config_cleanup causes a double-free.
- [High] Missing drm_atomic_helper_shutdown() during driver removal.
- [High] Teardown logic creates a race condition causing ioctl threads to write to unmapped MMIO memory.
- [High] IRQ handler is registered before the CRTC is initialized, creating a window for a NULL pointer dereference.
- [Medium] Direct pointer dereference of `iosys_map` memory bypasses abstraction and breaks on I/O memory.
- [Medium] Mishandling of platform_get_irq return value breaks probe deferral.
--

--- Patch [2]: [RFC PATCH 2/3] drm/glanda: Add initial DRM driver for GlandaGPU ---
commit ee638d3912e8ed264bf2c9ee4b6fd9350bc3726b
Author: Leander Kieweg <kieweg.leander@gmail.com>
Subject: drm/glanda: Add initial DRM driver for GlandaGPU

This commit introduces the core DRM/KMS driver for GlandaGPU. This driver
supports basic modesetting, atomic updates, and custom 2D hardware
acceleration IOCTLs.

> diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
> new file mode 100644
> index 0000000000000..8f87ae096a2e3
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/glandagpu.c
> @@ -0,0 +1,769 @@
[ ... ]
> +static void glanda_plane_atomic_update(struct drm_plane *plane,
> +				       struct drm_atomic_commit *state)
> +{
[ ... ]
> +	dma_resv_lock(shmem->base.resv, NULL);
> +	ret = drm_gem_shmem_vmap_locked(shmem, &map);
> +	if (ret) {
> +		dma_resv_unlock(shmem->base.resv);
> +		drm_err(&gdev->drm,
> +			"GlandaGPU: failed to vmap GEM shmem object\n");
> +		return;
> +	}
> +
> +	mutex_lock(&gdev->lock);

[Severity: High]
DRM atomic commit callbacks execute in an atomic context and cannot sleep. 
Is it safe to call dma_resv_lock, drm_gem_shmem_vmap_locked, and mutex_lock 
here? These functions can sleep and might trigger a schedule while atomic
oops or deadlocks when this path is reached via non-blocking atomic commits.

> +
> +	ret = glanda_wait_idle(gdev);
> +	if (ret) {
> +		drm_err(&gdev->drm, "GlandaGPU: timed out waiting for idle\n");
> +		mutex_unlock(&gdev->lock);
> +		drm_gem_shmem_vunmap_locked(shmem, &map);
> +	dma_resv_unlock(shmem->base.resv);
> +		return;
> +	}
> +
> +	src_pitch = fb->pitches[0];
> +	width = min_t(u32, fb->width, GLANDA_WIDTH);
> +	height = min_t(u32, fb->height, GLANDA_HEIGHT);
> +
> +	u8 __iomem *dst_base = gdev->vram_base;
> +	u8 *src_base = map.vaddr;
> +	u32 y;
> +
> +	for (y = 0; y < height; y++) {
> +		u32 *src = (u32 *)(src_base + y * src_pitch);

[Severity: Medium]
Does directly dereferencing map.vaddr break abstraction on architectures 
that do not support direct memory dereferencing for I/O memory? If the 
mapped memory is in I/O space (like an imported dma-buf), this might cause 
a panic. Should this use iosys_map helpers like iosys_map_rd instead?

[ ... ]
> +static int glandagpu_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	writel(0, gdev->mmio_base + REG_IER);
> +	writel(0xFFFFFFFF, gdev->mmio_base + REG_ISR);	/* clear flags */
> +
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret > 0) {
> +		gdev->irq = ret;
> +		ret = devm_request_irq(&pdev->dev, gdev->irq, glanda_irq_handler,
> +				       IRQF_SHARED, "glandagpu", gdev);

[Severity: Medium]
What happens if platform_get_irq returns -EPROBE_DEFER or another negative 
error code? The check for ret > 0 seems to treat all negative error codes 
as a missing IRQ, falling back to polling permanently rather than returning 
the error to the driver core to retry.

[Severity: High]
Since devm_request_irq is called with IRQF_SHARED here, could the IRQ 
handler fire immediately before the CRTC is initialized later in probe? 
If the shared interrupt fires early and the INT_VSYNC bit is set, 
glanda_irq_handler would call drm_crtc_handle_vblank on an uninitialized 
crtc object, which could lead to a NULL pointer dereference.

[ ... ]
> +err_mode_cleanup:
> +	drm_mode_config_cleanup(&gdev->drm);
> +	return ret;
> +}

[Severity: High]
The mode config is initialized using drm_mode_config_init, which wraps 
drmm_mode_config_init and automatically registers a devres action for 
cleanup. Does calling drm_mode_config_cleanup manually here cause a 
double-free on probe failure?

> +
> +static void glandagpu_remove(struct platform_device *pdev)
> +{
> +	struct glanda_device *gdev = platform_get_drvdata(pdev);
> +
> +	/* Disable interrupts first so no new IRQ work can race the teardown
> +	 * below, and wake up anyone still blocked in glanda_wait_idle().
> +	 */
> +	writel(0, gdev->mmio_base + REG_IER);
> +	gdev->cmd_done = true;
> +	wake_up_interruptible(&gdev->cmd_wq);

[Severity: High]
By faking a successful completion instead of signaling an abort condition, 
any ioctl threads sleeping in glanda_wait_idle will wake up and proceed 
to write to mmio_base. Without proper synchronization or a call to 
drm_dev_enter/exit to ensure threads finish, can these awakened threads 
end up writing to unmapped MMIO memory as the devres teardown proceeds?

> +
> +	drm_info(&gdev->drm, "GlandaGPU DRM Driver removed\n");
> +	drm_dev_unregister(&gdev->drm);

[Severity: High]
Should this removal sequence include a call to drm_atomic_helper_shutdown 
to quiesce the display hardware? Without it, hardware pipelines might remain 
active after the module is unloaded, which could result in memory corruption 
or resource leaks.

> +	drm_mode_config_cleanup(&gdev->drm);
> +}

[Severity: High]
Similar to the probe error path, does this manual call to 
drm_mode_config_cleanup cause a double-free on module unload, given that 
drm_mode_config_init already registered a devres cleanup handler?

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

Re: [RFC PATCH 2/3] drm/glanda: Add initial DRM driver for GlandaGPU

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Hello,

On Tue, Jul 14, 2026 at 12:11:44PM +0200, Leander Kieweg wrote:
> +#include <linux/mod_devicetable.h>	/* Device Tree parsing */

Don't include this header, I plan to get rid of it soon. of_device_id is
provided by <linux/of.h>, so you can just drop this #include.

> +/* Device Tree match table. */
> +static const struct of_device_id glanda_of_match[] = {
> +	{.compatible = "glanda,gpu-1.0", },

Make this:

	{ .compatible = "glanda,gpu-1.0" },

please. (That is, a space after the opening { and no comma before the closing }).

Thanks
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[RFC PATCH 3/3] NOT FOR MERGE: drm/glanda: Add x86 platform test device

[Leander Kieweg]

Register a mock platform device on x86 architectures to allow
testing the GlandaGPU driver inside QEMU without cross-compiling.

Signed-off-by: Leander Kieweg <kieweg.leander@gmail.com>
---
 drivers/gpu/drm/tiny/Kconfig     | 14 ++++++++++++
 drivers/gpu/drm/tiny/glandagpu.c | 39 ++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 7a15bf95a..156713746 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -67,6 +67,20 @@ config DRM_GLANDA
 	  basic modesetting, dumb buffers, and simple 2D drawing
 	  acceleration via custom hardware IOCTLs.
 
+config DRM_GLANDA_X86_TEST
+    bool "Register a fixed-address test device on x86 (QEMU only)"
+    depends on DRM_GLANDA && X86
+    default n
+    help
+      Registers a platform device at a hardcoded physical address
+      (0xC0000000) for testing GlandaGPU against a QEMU-based digital
+      twin without needing a devicetree.
+
+      WARNING: this blindly ioremaps a fixed physical address range.
+      Do NOT enable this on real x86 hardware. This exists solely to
+      let reviewers test the driver in QEMU without cross-compiling
+      an ARM kernel/rootfs. Leave disabled otherwise.
+
 config DRM_GM12U320
 	tristate "GM12U320 driver for USB projectors"
 	depends on DRM && USB && MMU
diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
index 8f87ae096..c1e9370f2 100644
--- a/drivers/gpu/drm/tiny/glandagpu.c
+++ b/drivers/gpu/drm/tiny/glandagpu.c
@@ -741,6 +741,34 @@ static struct platform_driver glandagpu_driver = {
 	.remove = glandagpu_remove,
 };
 
+#ifdef CONFIG_DRM_GLANDA_X86_TEST
+static struct platform_device *pdev_x86;
+
+static struct resource glandagpu_resources[] = {
+	[0] = {			/* Single resource covering VRAM and MMIO. */
+	       .start = BRIDGE_BASE,
+	       .end = GLANDA_BASE_SIZE,
+	       .flags = IORESOURCE_MEM,},
+	[1] = {			/* IRQ */
+	       .start = 11,
+	       .end = 11,
+	       .flags = IORESOURCE_IRQ,},
+};
+
+static int glandagpu_register_x86_test_device(void)
+{
+	pdev_x86 = platform_device_register_simple("glandagpu", -1,
+						   glandagpu_resources,
+						   ARRAY_SIZE(glandagpu_resources));
+	if (IS_ERR(pdev_x86)) {
+		pr_err("GlandaGPU: Failed to register platform device\n");
+		return PTR_ERR(pdev_x86);
+	}
+
+	return 0;
+}
+#endif
+
 static int __init glandagpu_init(void)
 {
 	int ret;
@@ -750,6 +778,13 @@ static int __init glandagpu_init(void)
 		pr_err("GlandaGPU: Failed to register platform driver\n");
 		return ret;
 	}
+#ifdef CONFIG_DRM_GLANDA_X86_TEST
+	ret = glandagpu_register_x86_test_device();
+	if (ret) {
+		platform_driver_unregister(&glandagpu_driver);
+		return ret;
+	}
+#endif
 
 	pr_info("GlandaGPU: Module loaded successfully\n");
 	return 0;
@@ -757,6 +792,10 @@ static int __init glandagpu_init(void)
 
 static void __exit glandagpu_exit(void)
 {
+#ifdef CONFIG_DRM_GLANDA_X86_TEST
+	if (pdev_x86)
+		platform_device_unregister(pdev_x86);
+#endif
 	platform_driver_unregister(&glandagpu_driver);
 	pr_info("GlandaGPU: Module unloaded\n");
 }
-- 
2.43.0

Re: [RFC PATCH 3/3] NOT FOR MERGE: drm/glanda: Add x86 platform test device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing DRM unplug protection in IOCTL handlers allows Use-After-Unmap of MMIO resources.
- [High] The plane atomic update callback calls multiple sleeping functions while executing in atomic context.
- [High] Premature shared IRQ registration causes a NULL pointer dereference if the interrupt fires before CRTC initialization.
--

--- Patch [3]: [RFC PATCH 3/3] NOT FOR MERGE: drm/glanda: Add x86 platform test device ---
commit 6108d4cd0cea176f9e750373bc9ba705d3d2a6c7
Author: Leander Kieweg <kieweg.leander@gmail.com>

NOT FOR MERGE: drm/glanda: Add x86 platform test device

Register a mock platform device on x86 architectures to allow
testing the GlandaGPU driver inside QEMU without cross-compiling.

> diff --git a/drivers/gpu/drm/tiny/glandagpu.c b/drivers/gpu/drm/tiny/glandagpu.c
> index 8f87ae096a2e3..c1e9370f2a4b1 100644
> --- a/drivers/gpu/drm/tiny/glandagpu.c
> +++ b/drivers/gpu/drm/tiny/glandagpu.c

[Severity: High]
This is a pre-existing issue, but is it possible for a concurrent ioctl to
access unmapped MMIO memory during device removal?

Looking at glandagpu_remove(), it wakes up blocked ioctls:

drivers/gpu/drm/tiny/glandagpu.c:glandagpu_remove() {
    ...
	gdev->cmd_done = true;
	wake_up_interruptible(&gdev->cmd_wq);
    ...
}

However, it lacks drm_dev_unplug() and drm_dev_enter()/drm_dev_exit()
synchronization. When glandagpu_remove() returns, the devres framework
automatically unmaps gdev->mmio_base. The forcibly awakened ioctl could
then resume and write directly to the unmapped MMIO memory:

drivers/gpu/drm/tiny/glandagpu.c:glanda_hw_clear() {
    ...
	writel(color, gdev->mmio_base + REG_COLOR);
    ...
}

Could this cause a kernel panic due to a use-after-unmap page fault?

[Severity: High]
This isn't a bug introduced by this patch, but does the plane atomic update
callback sleep while in atomic context?

Userspace can initiate a non-blocking DRM atomic commit, which executes
glanda_plane_atomic_update() in an atomic context. However, it appears to
call multiple sleeping functions:

drivers/gpu/drm/tiny/glandagpu.c:glanda_plane_atomic_update() {
    ...
	dma_resv_lock(shmem->base.resv, NULL);
	ret = drm_gem_shmem_vmap_locked(shmem, &map);
    ...
	mutex_lock(&gdev->lock);

	ret = glanda_wait_idle(gdev);
    ...
}

Since glanda_wait_idle() internally uses wait_event_interruptible_timeout(),
could these calls cause system instability, scheduling-while-atomic warnings,
or deadlocks in the DRM commit worker?

[Severity: High]
This is a pre-existing issue, but could premature shared IRQ registration
cause a NULL pointer dereference during probe?

In glandagpu_probe(), devm_request_irq() is called with IRQF_SHARED before
the DRM CRTC is initialized by drm_crtc_init_with_planes():

drivers/gpu/drm/tiny/glandagpu.c:glandagpu_probe() {
    ...
		ret = devm_request_irq(&pdev->dev, gdev->irq, glanda_irq_handler,
				       IRQF_SHARED, "glandagpu", gdev);
    ...
	ret = drm_crtc_init_with_planes(&gdev->drm, &gdev->crtc,
    ...
}

If another device on the same shared line triggers an interrupt immediately,
the handler reads the raw REG_ISR without masking it against the interrupt
enable register (REG_IER). If the INT_VSYNC bit is set, it proceeds to
handle the vblank:

drivers/gpu/drm/tiny/glandagpu.c:glanda_irq_handler() {
    ...
	if (isr & INT_VSYNC)
		drm_crtc_handle_vblank(&gdev->crtc);
    ...
}

Since gdev->crtc is still zero-initialized at this point, could this
dereference a NULL crtc->dev pointer and lead to a kernel panic?

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