[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 --]

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

[Leander Kieweg]

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

Will drop it, thanks.

> Make this:
>
>         { .compatible = "glanda,gpu-1.0" },
>
> please. (That is, a space after the opening { and no comma before the closing }).

Will fix in v2.

Thanks for the review!
Leander

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

[Thomas Zimmermann]

Hi,

you can find a quick review below.


Am 14.07.26 um 12:11 schrieb 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;

Please use drm_device.dev instead.

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

Synchronization happens with drm_gem_fb_begin_cpu_access(). No need to 
take the lock here.

> +	ret = drm_gem_shmem_vmap_locked(shmem, &map);

This vmap should not happen here, but in begin_fb_access.  We have 
helpers that set up the mapping for your driver. Take a look at 
DRM_GEM_SHADOW_PLANE_HELPER_FUNCS and DRM_GEM_SHADOW_PLANE_FUNCS and the 
drivers that use them.

> +	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]);

That's confusing. Each pixel is 32 bit. But each color channel is only 4 
bit. So there's a 20-bit hole in each pixel, right?   Why don't you use 
16 bit per pixel?

> +		}
> +	}
> +
> +	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) {

I don't think it is possible that new_state is NULL.


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

No need for this function. drm_gem_fb_create_with_dirty() is exactly 
what you're looking for.

> +{
> +	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),
> +};

Drop these ioctls entirely. As I outlined in my reply to the cover 
letter, these will not work with DRM as they are now. Especially since 
you're using gem-shmem as a shadow buffer. All your rendering happens in 
system memory in user space in software.


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

This function is going away. You can open-code it instead.

Best regards
Thomas

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

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

[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

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

[Thomas Zimmermann]

Hi

Am 14.07.26 um 12:11 schrieb 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.

That is a pretty cool project.

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

We already have a driver for a RasPi-based USB display that someone made 
in their spare time. So being a hobbyist project is not a problem per se.

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

2d primitives are likely not very useful for DRM. The canonical 
reference of why is at [1]. The tl;dr is that there's no standard API, 
and GPU-CPU transfers and setup costs are too high to make it 
significantly faster than software rendering.

Various people have proposed to add some form of 2d pipeline to DRM, but 
nothing concrete has ever emerged.

Conceptually, DRM doesn't really render anything. It composes the screen 
from already-rendered buffers. Rendering to these buffers is mostly done 
by Mesa drivers with some help from DRM's kernel drivers. If you want 
hardware rendering, you'd need memory management for off-screen 
rendering that Mesa can use independently from display output. Therefore 
these simple draw and clear primitives aren't that useful. You need to 
design a full rendering pipeline instead.

[1] https://blog.ffwll.ch/2018/08/no-2d-in-drm.html


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

Using a single ioctl per command will kill performance. So, if anything, 
you'd want the command-buffer model.  To make it fast for 2d primitives, 
you'd likely have to model it like a 3d pipeline: have all 2d graphics 
buffers in the display memory already and submit a large batch of 
rendering commands that generate the entire screen at once.


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

Rule of thumb is that you need a working user-space side for ioctls. 
Mesa would be the premier target for 3d.

I'm not much involved in Mesa, but I think Mesa is quickly moving 
towards programmable pipelines. Getting drivers for fixed-function 
hardware merged might be hard.

How complicated is it to model a stream processor (i.e. GPU core) in VHDL?


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

Can you use a PCI device for that and let the kernel do all the work? 
See [2] for how to get a PCI device id.

[2] https://www.qemu.org/docs/master/specs/pci-ids.html


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

That's indeed a good thing to have in hardware.

I mentioned that the 2d/3d rendering is probably complicated to get 
done. If I may suggest an alternative, you could implement additional 
features of the mode-setting pipeline. Besides the primary plane that 
your hardware already supports, you could add a cursor plane. Or you 
could add overlay planes for displaying YUV formats (i.e., video 
frames). Or you could implement existing DRM properties, such as 
scaling, background colors, or HDR.  These features are already 
supported by user space. Your device would be usable immediately.


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

Glad to hear it didn't work. drm_simple_display_pipe is obsolete and on 
its way out. Please don't use it.

Best regards
Thomas


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

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