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 <gargadity...@live.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?
