Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs

From: Thomas Zimmermann
Date: Thu Feb 20 2025 - 05:16:24 EST

Next message: Lorenzo Stoakes: "Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings"
Previous message: Hans de Goede: "Re: [PATCH] leds: Fix LED_OFF brightness race"
In reply to: Aditya Garg: "Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs"
Next in thread: Aditya Garg: "Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi

Am 20.02.25 um 11:11 schrieb Aditya Garg:

Hi

+ ret = drm_dev_register(drm, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register DRM device\n");

This call does not belong to the mode-setting pipeline and belongs into appletbdrm_probe().

+
+ return 0;
+}
+
+static int appletbdrm_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+ struct device *dev = &intf->dev;
+ struct appletbdrm_device *adev;
+ int ret;
+
+ ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to find bulk endpoints\n");
+
+ adev = devm_drm_dev_alloc(dev, &appletbdrm_drm_driver, struct appletbdrm_device, drm);
+ if (IS_ERR(adev))
+ return PTR_ERR(adev);
+
+ adev->dev = dev;
+ adev->in_ep = bulk_in->bEndpointAddress;
+ adev->out_ep = bulk_out->bEndpointAddress;
+
+ usb_set_intfdata(intf, adev);

Rather set the DRM device here and fetch it in the callbacks below. At some point, we might be able to share some of those helpers among drivers.

FWIW

Moving register drm device here results in these errors in journalctl:

Feb 20 15:02:46 MacBook kernel: appletbdrm: loading out-of-tree module taints kernel.
Feb 20 15:02:46 MacBook kernel: appletbdrm: module verification failed: signature and/or required key missing - tainting kernel
Feb 20 15:02:46 MacBook kernel: BUG: kernel NULL pointer dereference, address: 0000000000000030
Feb 20 15:02:46 MacBook kernel: #PF: supervisor read access in kernel mode
Feb 20 15:02:46 MacBook kernel: #PF: error_code(0x0000) - not-present page
Feb 20 15:02:46 MacBook kernel: PGD 0 P4D 0
Feb 20 15:02:46 MacBook kernel: Oops: Oops: 0000 [#1] PREEMPT SMP PTI
Feb 20 15:02:46 MacBook kernel: CPU: 10 UID: 0 PID: 3530 Comm: modprobe Tainted: G C OE 6.13.3-1-t2-noble #1
Feb 20 15:02:46 MacBook kernel: Tainted: [C]=CRAP, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Feb 20 15:02:46 MacBook kernel: Hardware name: Apple Inc. MacBookPro16,1/Mac-E1008331FDC96864, BIOS 2069.80.3.0.0 (iBridge: 22.16.13051.0.0,0) 01/07/2025
Feb 20 15:02:46 MacBook kernel: RIP: 0010:drm_dev_register+0x1c/0x290
Feb 20 15:02:46 MacBook kernel: Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 49 89 f5 41 54 53 48 89 fb 48 83 ec 08 <4c> 8b 77 30 49 83 3e 00 0f 84 09 02 00 00 48 83 7b 20 00 0f 84 0e
Feb 20 15:02:46 MacBook kernel: RSP: 0018:ffffbf4344cbb670 EFLAGS: 00010282
Feb 20 15:02:46 MacBook kernel: RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
Feb 20 15:02:46 MacBook kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
Feb 20 15:02:46 MacBook kernel: RBP: ffffbf4344cbb6a0 R08: 0000000000000000 R09: 0000000000000000
Feb 20 15:02:46 MacBook kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff992a8f114020
Feb 20 15:02:46 MacBook kernel: R13: 0000000000000000 R14: ffff992a8f115ad8 R15: ffff992a8f114000
Feb 20 15:02:46 MacBook kernel: FS: 000073572877c080(0000) GS:ffff992deed00000(0000) knlGS:0000000000000000
Feb 20 15:02:46 MacBook kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Feb 20 15:02:46 MacBook kernel: CR2: 0000000000000030 CR3: 000000011dd12003 CR4: 00000000003726f0
Feb 20 15:02:46 MacBook kernel: Call Trace:
Feb 20 15:02:46 MacBook kernel: <TASK>
Feb 20 15:02:46 MacBook kernel: ? show_regs+0x6c/0x80
Feb 20 15:02:46 MacBook kernel: ? __die+0x24/0x80
Feb 20 15:02:46 MacBook kernel: ? page_fault_oops+0x175/0x5d0
Feb 20 15:02:46 MacBook kernel: ? do_user_addr_fault+0x4b2/0x870
Feb 20 15:02:46 MacBook kernel: ? exc_page_fault+0x85/0x1c0
Feb 20 15:02:46 MacBook kernel: ? asm_exc_page_fault+0x27/0x30
Feb 20 15:02:46 MacBook kernel: ? drm_dev_register+0x1c/0x290
Feb 20 15:02:46 MacBook kernel: appletbdrm_probe+0x4eb/0x5f0 [appletbdrm]
Feb 20 15:02:46 MacBook kernel: usb_probe_interface+0x168/0x3d0
Feb 20 15:02:46 MacBook kernel: really_probe+0xee/0x3c0
Feb 20 15:02:46 MacBook kernel: __driver_probe_device+0x8c/0x180
Feb 20 15:02:46 MacBook kernel: driver_probe_device+0x24/0xd0
Feb 20 15:02:46 MacBook kernel: __driver_attach+0x10b/0x210
Feb 20 15:02:46 MacBook kernel: ? __pfx___driver_attach+0x10/0x10
Feb 20 15:02:46 MacBook kernel: bus_for_each_dev+0x8a/0xf0
Feb 20 15:02:46 MacBook kernel: driver_attach+0x1e/0x30
Feb 20 15:02:46 MacBook kernel: bus_add_driver+0x14e/0x290
Feb 20 15:02:46 MacBook kernel: driver_register+0x5e/0x130
Feb 20 15:02:46 MacBook kernel: usb_register_driver+0x87/0x170
Feb 20 15:02:46 MacBook kernel: ? __pfx_appletbdrm_usb_driver_init+0x10/0x10 [appletbdrm]
Feb 20 15:02:46 MacBook kernel: appletbdrm_usb_driver_init+0x23/0xff0 [appletbdrm]
Feb 20 15:02:46 MacBook kernel: do_one_initcall+0x5b/0x340
Feb 20 15:02:46 MacBook kernel: do_init_module+0x97/0x2a0
Feb 20 15:02:46 MacBook kernel: load_module+0x2293/0x25c0
Feb 20 15:02:46 MacBook kernel: init_module_from_file+0x97/0xe0
Feb 20 15:02:46 MacBook kernel: ? init_module_from_file+0x97/0xe0
Feb 20 15:02:46 MacBook kernel: idempotent_init_module+0x110/0x300
Feb 20 15:02:46 MacBook kernel: __x64_sys_finit_module+0x77/0x100
Feb 20 15:02:46 MacBook kernel: x64_sys_call+0x1f37/0x2650
Feb 20 15:02:46 MacBook kernel: do_syscall_64+0x7e/0x170
Feb 20 15:02:46 MacBook kernel: ? ksys_read+0x72/0xf0
Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 20 15:02:46 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 20 15:02:46 MacBook kernel: ? __do_sys_newfstatat+0x44/0x90
Feb 20 15:02:46 MacBook kernel: ? ext4_llseek+0xc0/0x120
Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 20 15:02:46 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 20 15:02:46 MacBook kernel: ? count_memcg_events.constprop.0+0x2a/0x50
Feb 20 15:02:46 MacBook kernel: ? handle_mm_fault+0xaf/0x2e0
Feb 20 15:02:46 MacBook kernel: ? do_user_addr_fault+0x5d5/0x870
Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 20 15:02:46 MacBook kernel: ? irqentry_exit_to_user_mode+0x2d/0x1d0
Feb 20 15:02:46 MacBook kernel: ? irqentry_exit+0x43/0x50
Feb 20 15:02:46 MacBook kernel: ? exc_page_fault+0x96/0x1c0
Feb 20 15:02:46 MacBook kernel: entry_SYSCALL_64_after_hwframe+0x76/0x7e
Feb 20 15:02:46 MacBook kernel: RIP: 0033:0x735727f2725d
Feb 20 15:02:46 MacBook kernel: Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89 01 48
Feb 20 15:02:46 MacBook kernel: RSP: 002b:00007fffd9f88d18 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
Feb 20 15:02:46 MacBook kernel: RAX: ffffffffffffffda RBX: 000062610c6eb8e0 RCX: 0000735727f2725d
Feb 20 15:02:46 MacBook kernel: RDX: 0000000000000000 RSI: 00006260e7b3be52 RDI: 0000000000000003
Feb 20 15:02:46 MacBook kernel: RBP: 00007fffd9f88dd0 R08: 0000000000000040 R09: 00007fffd9f88e50
Feb 20 15:02:46 MacBook kernel: R10: 0000735728003b20 R11: 0000000000000246 R12: 00006260e7b3be52
Feb 20 15:02:46 MacBook kernel: R13: 0000000000040000 R14: 000062610c6e4920 R15: 0000000000000000
Feb 20 15:02:46 MacBook kernel: </TASK>

The following change was done:

@@ -13,6 +13,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_client_setup.h>
#include <drm/drm_crtc.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
@@ -596,7 +597,6 @@ static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)
* as the height is actually the width of the framebuffer and vice
* versa
*/
-
drm->mode_config.min_width = 0;
drm->mode_config.min_height = 0;
drm->mode_config.max_width = max(adev->height, DRM_SHADOW_PLANE_MAX_WIDTH);
@@ -635,10 +635,6 @@ static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)
drm_mode_config_reset(drm);
- ret = drm_dev_register(drm, 0);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to register DRM device\n");
-
return 0;
}
@@ -648,6 +644,7 @@ static int appletbdrm_probe(struct usb_interface *intf,
struct usb_endpoint_descriptor *bulk_in, *bulk_out;
struct device *dev = &intf->dev;
struct appletbdrm_device *adev;
+ struct drm_device *drm;

Because you apparently never initialize this variable.

int ret;
ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
@@ -676,7 +673,17 @@ static int appletbdrm_probe(struct usb_interface *intf,
if (ret)
return dev_err_probe(dev, ret, "Failed to clear display\n");
- return appletbdrm_setup_mode_config(adev);
+ ret = appletbdrm_setup_mode_config(adev);
+ if (ret)
+ return ret;
+
+ ret = drm_dev_register(drm, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register DRM device\n");
+
+ drm_client_setup(drm, NULL);

You won't need a DRM client on the touch bar. Just clearing the display should be enough.

Best regards
Thomas

+
+ return 0;
}
static void appletbdrm_disconnect(struct usb_interface *intf)

+
+ ret = appletbdrm_get_information(adev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get display information\n");
+
+ ret = appletbdrm_signal_readiness(adev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to signal readiness\n");
+
+ ret = appletbdrm_clear_display(adev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to clear display\n");

Clearing the display is not something usually done in probe. But I guess there's no better place. I'd do this as the final call in probe; after registering the device. That way, it acts a bit like an in-kernel DRM client.

Best regards
Thomas

+
+ return appletbdrm_setup_mode_config(adev);
+}
+
+static void appletbdrm_disconnect(struct usb_interface *intf)
+{
+ struct appletbdrm_device *adev = usb_get_intfdata(intf);
+ struct drm_device *drm = &adev->drm;
+
+ drm_dev_unplug(drm);
+ drm_atomic_helper_shutdown(drm);
+}
+
+static void appletbdrm_shutdown(struct usb_interface *intf)
+{
+ struct appletbdrm_device *adev = usb_get_intfdata(intf);
+
+ /*
+ * The framebuffer needs to be cleared on shutdown since its content
+ * persists across boots
+ */
+ drm_atomic_helper_shutdown(&adev->drm);
+}
+
+static const struct usb_device_id appletbdrm_usb_id_table[] = {
+ { USB_DEVICE_INTERFACE_CLASS(0x05ac, 0x8302, USB_CLASS_AUDIO_VIDEO) },
+ {}
+};
+MODULE_DEVICE_TABLE(usb, appletbdrm_usb_id_table);
+
+static struct usb_driver appletbdrm_usb_driver = {
+ .name = "appletbdrm",
+ .probe = appletbdrm_probe,
+ .disconnect = appletbdrm_disconnect,
+ .shutdown = appletbdrm_shutdown,
+ .id_table = appletbdrm_usb_id_table,
+};
+module_usb_driver(appletbdrm_usb_driver);
+
+MODULE_AUTHOR("Kerem Karabay <kekrby@xxxxxxxxx>");
+MODULE_DESCRIPTION("Apple Touch Bar DRM Driver");
+MODULE_LICENSE("GPL");

Regards
Aditya

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Next message: Lorenzo Stoakes: "Re: [PATCH 0/4] mm: permit guard regions for file-backed/shmem mappings"
Previous message: Hans de Goede: "Re: [PATCH] leds: Fix LED_OFF brightness race"
In reply to: Aditya Garg: "Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs"
Next in thread: Aditya Garg: "Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]