Message-ID:
 <PN3PR01MB95974D5EB5A25386A8BF6FDAB8C02@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Mon, 24 Feb 2025 16:58:48 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
	"mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
	<tzimmermann@...e.de>, "airlied@...il.com" <airlied@...il.com>,
	"simona@...ll.ch" <simona@...ll.ch>, "andriy.shevchenko@...ux.intel.com"
	<andriy.shevchenko@...ux.intel.com>
CC: Kerem Karabay <kekrby@...il.com>, Atharva Tiwari <evepolonium@...il.com>,
	Aun-Ali Zaidi <admin@...eit.net>, Linux Kernel Mailing List
	<linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86
 Macs



> On 24 Feb 2025, at 7:10 PM, Aditya Garg <gargaditya08@...e.com> wrote:
> 
> From: Kerem Karabay <kekrby@...il.com>
> 
> The Touch Bars found on x86 Macs support two USB configurations: one
> where the device presents itself as a HID keyboard and can display
> predefined sets of keys, and one where the operating system has full
> control over what is displayed.
> 
> This commit adds support for the display functionality of the second
> configuration. Functionality for the first configuration has been
> merged in the HID tree.
> 
> Note that this driver has only been tested on T2 Macs, and only includes
> the USB device ID for these devices. Testing on T1 Macs would be
> appreciated.
> 
> Credit goes to Ben (Bingxing) Wang on GitHub for reverse engineering
> most of the protocol.
> 
> Also, as requested by Andy, I would like to clarify the use of __packed
> structs in this driver:
> 
> - All the packed structs are aligned except for appletbdrm_msg_information.
> - We have to pack appletbdrm_msg_information since it is requirement of
>  the protocol.
> - We compared binaries compiled by keeping the rest structs __packed and
>  not __packed using bloat-o-meter, and __packed was not affecting code
>  generation.
> - To maintain consistency, rest structs have been kept __packed.
> 
> Link: https://github.com/imbushuo/DFRDisplayKm
> Signed-off-by: Kerem Karabay <kekrby@...il.com>
> Co-developed-by: Atharva Tiwari <evepolonium@...il.com>
> Signed-off-by: Atharva Tiwari <evepolonium@...il.com>
> Co-developed-by: Aditya Garg <gargaditya08@...e.com>
> Signed-off-by: Aditya Garg <gargaditya08@...e.com>
> Signed-off-by: Aun-Ali Zaidi <admin@...eit.net>
> ---
> v2 ->
> - Add the driver to MAINTAINERS.
> - Allocate memory for request and response in plane's atomic-check helper
> - Void the use of drm_fb_blit()
> - Implement atomic_disable
> - Make PRIME work
> - Remove the date field from struct drm_driver
> - intersect damage with dst_clip
> - Register DRM device in appletbdrm_probe
> - Clear the display as the final call in probe
> - Select hid_multitouch as well in kconfig
> 
> v3 ->
> - Change commit message to credit Ben (Bingxing) Wang
> 
> v4 ->
> - Use "Link:" in commit message
> - Specify why __packed has been used in commit message
> - Use %p4cc instead of %p4ch for errors
> - Add myself to Maintainers
> - Remove #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> - Add missing header files
> - Use return 0; instead of return ret; in static int appletbdrm_send_request
> - Better errno types used in appletbdrm_read_response
> - Use BITS_TO_BYTES() to convert APPLETBDRM_BITS_PER_PIXEL to bytes
> - Remove redundant else in plane_helper_atomic_check
> - Remove unnecessary use of 0xff in request->msg_id = timestamp & 0xff;
> - Remove duplicated struct device in struct appletbdrm_device
> - clear frames_size and request_size as well in primary_plane_helper_atomic_update
> - Allocate using kzalloc instead of kmemdup
> - Use drm_err() instead of dev_err_probe()
> - Avoid clearing drm->mode_config.min_width and height to 0
> - Use put_device() to release the DMA device in appletbdrm_disconnect
> 
> MAINTAINERS                       |   8 +
> drivers/gpu/drm/tiny/Kconfig      |  14 +
> drivers/gpu/drm/tiny/Makefile     |   1 +
> drivers/gpu/drm/tiny/appletbdrm.c | 835 ++++++++++++++++++++++++++++++
> 4 files changed, 858 insertions(+)
> create mode 100644 drivers/gpu/drm/tiny/appletbdrm.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index efee40ea5..f7c97eb24 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7148,6 +7148,14 @@ S:    Supported
> T:    git https://gitlab.freedesktop.org/drm/misc/kernel.git
> F:    drivers/gpu/drm/sun4i/sun8i*
> 
> +DRM DRIVER FOR APPLE TOUCH BARS
> +M:    Aun-Ali Zaidi <admin@...eit.net>
> +M:    Aditya Garg <gargaditya08@...e.com>
> +L:    dri-devel@...ts.freedesktop.org
> +S:    Maintained
> +T:    git https://gitlab.freedesktop.org/drm/misc/kernel.git
> +F:    drivers/gpu/drm/tiny/appletbdrm.c
> +
> DRM DRIVER FOR ARM PL111 CLCD
> M:    Linus Walleij <linus.walleij@...aro.org>
> S:    Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 94cbdb133..25471791c 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -1,5 +1,19 @@
> # SPDX-License-Identifier: GPL-2.0-only
> 
> +config DRM_APPLETBDRM
> +    tristate "DRM support for Apple Touch Bars"
> +    depends on DRM && USB && MMU
> +    select DRM_GEM_SHMEM_HELPER
> +    select DRM_KMS_HELPER
> +    select HID_APPLETB_BL

Btw I have a doubt regarding this dependency. While hid-appletb-bl has made into the linux-next tree, it has still not been merged into Linus' tree, and neither the drm tree I assume. It potentially could cause issues?

> +    select HID_MULTITOUCH
> +    help
> +      Say Y here if you want support for the display of Touch Bars on x86
> +      MacBook Pros.
> +
> +      To compile this driver as a module, choose M here: the
> +      module will be called appletbdrm.
> +

Powered by blists - more mailing lists