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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 19.02.25 um 08:57 schrieb Thomas Zimmermann:
Hi

Am 18.02.25 um 21:12 schrieb Aditya Garg:
Hi

In continuation to my previous mail.

+
+static int appletbdrm_send_msg(struct appletbdrm_device *adev, u32 msg)
+{
+ struct appletbdrm_msg_simple_request *request;
+ int ret;
+
+ request = kzalloc(sizeof(*request), GFP_KERNEL);
+ if (!request)
+ return -ENOMEM;
+
+ request->header.unk_00 = cpu_to_le16(2);
+ request->header.unk_02 = cpu_to_le16(0x1512);
+ request->header.size = cpu_to_le32(sizeof(*request) - sizeof(request->header));
+ request->msg = msg;
+ request->size = request->header.size;
+
+ ret = appletbdrm_send_request(adev, &request->header, sizeof(*request));
+
+ kfree(request);
This is temporary data for the send operation and save to free here?
Probably yes. If I understand correctly, it’s needed to make the touchbar go into the display mode, from the hid keyboard mode.

We here are doing the same as the Windows driver [1] for this does.

[1] https://github.com/imbushuo/DFRDisplayKm/blob/master/src/DFRDisplayKm/include/Dfr.h#L3

Yeah. My concern was that request is being freed while the USB send operation is still using it. But in the USB code, it doesn't look like that.

[...]
Can we void the use of drm_fb_blit()? Since you know all formats in advance, just do

switch (format)
case XRGB8888: drm_fb_xrgb888_to_bgr888() break default:
    drm_fb_memcpy() break }We use blit in simpledrm and ofdrm, where we don't know the formats and output buffers in advance. But it's really not so great in other drivers, I think.
I think you mean this:

#include <drm/drm_framebuffer.h>

        switch (fb->format->format) {
        case DRM_FORMAT_XRGB8888:
            drm_fb_xrgb8888_to_bgr888(&dst, NULL, &shadow_plane_state->data[0], fb, &damage, &shadow_plane_state->fmtcnv_state);
            break;
        default:
            drm_fb_memcpy(&dst, NULL, &shadow_plane_state->data[0], fb, &damage);
            break;
        }

Yes.

[...]
For USB devices, we need special wiring to make PRIME work. The PRIME device must support DMA, but a USB device doesn't. So we pass the USB controller device instead. See [2] for what udl does and how it obtains dmadev.

[2] https://elixir.bootlin.com/linux/v6.14-rc3/source/drivers/gpu/drm/udl/udl_drv.c#L76
Disregard my previous reply for this. I believe you meant by this?:

—>8—
 From b6fda730995b7f28374c1ff38778a6f3e6da65da Mon Sep 17 00:00:00 2001
From: Aditya Garg <gargaditya08@xxxxxxxx>
Date: Tue, 18 Feb 2025 22:47:44 +0530
Subject: [PATCH] prime

---
drivers/gpu/drm/tiny/appletbdrm.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
index f2d911325..b835063c2 100644
--- a/drivers/gpu/drm/tiny/appletbdrm.c
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -118,6 +118,7 @@ struct appletbdrm_fb_request_response {

struct appletbdrm_device {
    struct device *dev;
+    struct device *dmadev;

    unsigned int in_ep;
    unsigned int out_ep;
@@ -521,10 +522,22 @@ static const struct drm_encoder_funcs appletbdrm_encoder_funcs = {
    .destroy = drm_encoder_cleanup,
};

+static struct drm_gem_object *appletbdrm_driver_gem_prime_import(struct drm_device *dev,
+                                 struct dma_buf *dma_buf)
+{
+    struct appletbdrm_device *adev = drm_to_adev(dev);
+
+    if (!adev->dmadev)
+        return ERR_PTR(-ENODEV);
+
+    return drm_gem_prime_import_dev(dev, dma_buf, adev->dmadev);
+}
+
DEFINE_DRM_GEM_FOPS(appletbdrm_drm_fops);

static const struct drm_driver appletbdrm_drm_driver = {
    DRM_GEM_SHMEM_DRIVER_OPS,
+    .gem_prime_import    = appletbdrm_driver_gem_prime_import,

Exactly. The TODO item for this problem is at [1], but there's quite a bit of change involved to fix it. Setting a dedicated DMA device is the next best thing.

[1] https://elixir.bootlin.com/linux/v6.13.3/source/Documentation/gpu/todo.rst#L615

To add to this: you also have to set dmadev. See [2] for the respective code in udl. And please git-grep udl for dmadev and look for the places were it handles device ref-counting.

[2] https://elixir.bootlin.com/linux/v6.13.2/source/drivers/gpu/drm/udl/udl_main.c#L314

Best regards
Thomas


Best regards
Thomas

    .name            = "appletbdrm",
    .desc            = "Apple Touch Bar DRM Driver",
    .major            = 1,


--
--
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)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux