| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <81731c1c-b74d-4b60-8c01-cda9a92d9c1b@suse.de>
Date: Wed, 19 Feb 2025 10:49:37 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Aditya Garg <gargaditya08@...e.com>
Cc: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"mripard@...nel.org" <mripard@...nel.org>,
"airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch"
<simona@...ll.ch>, Kerem Karabay <kekrby@...il.com>,
Atharva Tiwari <evepolonium@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs
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@...e.com>
>>> 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?
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)
Powered by blists - more mailing lists