Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs
From: Thomas Zimmermann
Date: Thu Feb 20 2025 - 05:20:22 EST
Hi,
to be honest: you are just throwing patches and errors at me and want me
to fix your driver for you. If you want to maintain a kernel driver, you
need to be able to debug at least such basic problems. I suggest you
start to debug these issues by yourself and try to find their causes.
(printk() will be helpful.)
Best regards
Thomas
Am 19.02.25 um 11:19 schrieb Aditya Garg:
Hi
On 19 Feb 2025, at 3:19 PM, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
Hi
Am 19.02.25 um 10:37 schrieb Aditya Garg:
Hi
I’ve tried these changes, seem to be breaking the driver:
—>8—
From 16c920cabf65ec664663ebe1611c0ccf6e81de4a Mon Sep 17 00:00:00 2001
From: Aditya Garg <gargaditya08@xxxxxxxx>
Date: Tue, 18 Feb 2025 18:54:10 +0530
Subject: [PATCH] better error handling
---
.../apple-touchbar-advanced-0.1/appletbdrm.c | 68 +++++++++++++------
1 file changed, 46 insertions(+), 22 deletions(-)
diff --git a/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c b/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c
index f2d9113..cb13b36 100644
--- a/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c
+++ b/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c
@@ -133,6 +133,17 @@ struct appletbdrm_device {
struct drm_encoder encoder;
};
+struct appletbdrm_plane_state {
+ struct drm_shadow_plane_state base;
+ u8 *request_buffer;
+ u8 *response_buffer;
+};
+
+static inline struct appletbdrm_plane_state *to_appletbdrm_plane_state(struct drm_plane_state *state)
+{
+ return container_of(state, struct appletbdrm_plane_state, base.base);
+}
+
static int appletbdrm_send_request(struct appletbdrm_device *adev,
struct appletbdrm_msg_request_header *request, size_t size)
{
@@ -311,24 +322,6 @@ static int appletbdrm_flush_damage(struct appletbdrm_device *adev,
if (!frames_size)
return 0;
- request_size = ALIGN(sizeof(*request) + frames_size + sizeof(*footer), 16);
-
- request = kzalloc(request_size, GFP_KERNEL);
- if (!request)
- return -ENOMEM;
-
- response = kzalloc(sizeof(*response), GFP_KERNEL);
- if (!response) {
- ret = -ENOMEM;
- goto free_request;
- }
-
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret) {
- drm_err(drm, "Failed to start CPU framebuffer access (%d)\n", ret);
- goto free_response;
- }
-
request->header.unk_00 = cpu_to_le16(2);
request->header.unk_02 = cpu_to_le16(0x12);
request->header.unk_04 = cpu_to_le32(9);
@@ -389,10 +382,6 @@ static int appletbdrm_flush_damage(struct appletbdrm_device *adev,
end_fb_cpu_access:
drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-free_response:
- kfree(response);
-free_request:
- kfree(request);
return ret;
}
@@ -415,6 +404,15 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
struct drm_crtc *new_crtc = new_plane_state->crtc;
struct drm_crtc_state *new_crtc_state = NULL;
+ struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(new_plane_state);
+ struct drm_device *drm = plane->dev;
+ struct drm_plane_state *plane_state = plane->state;
+ struct appletbdrm_fb_request_response *response;
+ struct appletbdrm_fb_request_footer *footer;
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct appletbdrm_fb_request *request;
+ size_t frames_size = 0;
+ size_t request_size;
int ret;
if (new_crtc)
@@ -429,6 +427,22 @@ static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
else if (!new_plane_state->visible)
return 0;
+ request_size = ALIGN(sizeof(*request) + frames_size + sizeof(*footer), 16);
+
+ appletbdrm_state->request_buffer = kzalloc(request_size, GFP_KERNEL);
+ if (!request)
+ return -ENOMEM;
+
+ appletbdrm_state->response_buffer = kzalloc(sizeof(*response), GFP_KERNEL);
+ if (!response) {
+ ret = -ENOMEM;
+ }
+
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret) {
+ drm_err(drm, "Failed to start CPU framebuffer access (%d)\n", ret);
+ }
+
return 0;
}
@@ -464,6 +478,15 @@ static void appletbdrm_primary_plane_helper_atomic_disable(struct drm_plane *pla
drm_dev_exit(idx);
}
+static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
+
+ kfree(appletbdrm_state->request_buffer);
+ kfree(appletbdrm_state->response_buffer);
+}
+
static const struct drm_plane_helper_funcs appletbdrm_primary_plane_helper_funcs = {
DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
.atomic_check = appletbdrm_primary_plane_helper_atomic_check,
@@ -474,6 +497,7 @@ static const struct drm_plane_helper_funcs appletbdrm_primary_plane_helper_funcs
static const struct drm_plane_funcs appletbdrm_primary_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
+ .atomic_destroy_state = appletbdrm_primary_plane_destroy_state,
.destroy = drm_plane_cleanup,
DRM_GEM_SHADOW_PLANE_FUNCS,
You don't allocate struct appletbdrm_plane_state. Instead of this macro, you also have to set your own helpers for the plane's .reset and .atomic_duplicate_state There's again example code in the ssd130x driver.
Any attempt make to allocate request and response outside appletdrm_flush_damage seems to be breaking the driver.
If I understand correctly, you want me to allocate them outside appletdrm_flush_damage, in appletbdrm_primary_plane_helper_atomic_check, return -ENOMEM if they fail. After that add kfree(return) and kfree(response) in appletbdrm_primary_plane_destroy_state.
The ssd130x driver example isn’t really helping me. Could you please help me out here?
What's the exact error message?
I'm not getting any error message. Modprobing the compiled driver simply gets stuck, or shows killed.
For the patch I sent yesterday, I got this in journalctl:
Feb 18 18:36:33 MacBook kernel: Call Trace:
Feb 18 18:36:33 MacBook kernel: <TASK>
Feb 18 18:36:33 MacBook kernel: ? show_regs+0x6c/0x80
Feb 18 18:36:33 MacBook kernel: ? __die+0x24/0x80
Feb 18 18:36:33 MacBook kernel: ? page_fault_oops+0x175/0x5c0
Feb 18 18:36:33 MacBook kernel: ? do_user_addr_fault+0x4b2/0x870
Feb 18 18:36:33 MacBook kernel: ? exc_page_fault+0x85/0x1c0
Feb 18 18:36:33 MacBook kernel: ? asm_exc_page_fault+0x27/0x30
Feb 18 18:36:33 MacBook kernel: ? drm_gem_fb_begin_cpu_access+0x5/0xc0
Feb 18 18:36:33 MacBook kernel: ? appletbdrm_primary_plane_helper_atomic_check+0x158/0x1b0 [appletbdrm]
Feb 18 18:36:33 MacBook kernel: drm_atomic_helper_check_planes+0xf6/0x250
Feb 18 18:36:33 MacBook kernel: drm_atomic_helper_check+0x51/0xa0
Feb 18 18:36:33 MacBook kernel: drm_atomic_check_only+0x688/0xb00
Feb 18 18:36:33 MacBook kernel: drm_atomic_commit+0x6f/0xf0
Feb 18 18:36:33 MacBook kernel: ? __pfx___drm_printfn_info+0x10/0x10
Feb 18 18:36:33 MacBook kernel: drm_mode_atomic_ioctl+0xc01/0xe60
Feb 18 18:36:33 MacBook kernel: ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
Feb 18 18:36:33 MacBook kernel: drm_ioctl_kernel+0xb6/0x120
Feb 18 18:36:33 MacBook kernel: drm_ioctl+0x2f3/0x5b0
Feb 18 18:36:33 MacBook kernel: ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
Feb 18 18:36:33 MacBook kernel: __x64_sys_ioctl+0xa4/0xe0
Feb 18 18:36:33 MacBook kernel: x64_sys_call+0x131e/0x2650
Feb 18 18:36:33 MacBook kernel: do_syscall_64+0x7e/0x170
Feb 18 18:36:33 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 18 18:36:33 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
Feb 18 18:36:33 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 18 18:36:33 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 18 18:36:33 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
Feb 18 18:36:33 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 18 18:36:33 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
Feb 18 18:36:33 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
Feb 18 18:36:33 MacBook kernel: ? do_syscall_64+0x8a/0x170
Feb 18 18:36:33 MacBook kernel: entry_SYSCALL_64_after_hwframe+0x76/0x7e
Feb 18 18:36:33 MacBook kernel: RIP: 0033:0x5e684e58801b
Feb 18 18:36:33 MacBook kernel: Code: ff ff ff 4c 89 f7 e8 44 d0 f4 ff ff 15 1e a6 48 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 89 ff 89 f6 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f 9d c1 48 85 c0 0f 98 c2 20 ca 89 c1 c1 e1 10
Feb 18 18:36:33 MacBook kernel: RSP: 002b:00007fff91689798 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
Feb 18 18:36:33 MacBook kernel: RAX: ffffffffffffffda RBX: 00007fff91689850 RCX: 00005e684e58801b
Feb 18 18:36:33 MacBook kernel: RDX: 00007fff916897a0 RSI: 00000000c03864bc RDI: 0000000000000004
Feb 18 18:36:33 MacBook kernel: RBP: 00005e68545f9570 R08: 00005e68545f9590 R09: 00005e68545f9750
Feb 18 18:36:33 MacBook kernel: R10: 00005e68545eb560 R11: 0000000000000206 R12: 00005e68545f9750
Feb 18 18:36:33 MacBook kernel: R13: 00005e68545f9590 R14: 00005e68545f97a0 R15: 0000000000000001
Feb 18 18:36:33 MacBook kernel: </TASK>
Btw, the *working* source code with changes approved by you is available over here:
https://github.com/AdityaGarg8/apple-touchbar-drv/blob/dfr/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c
Best regards
Thomas
--
--
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)
--
--
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)