From 0b4346d6898490d03b2d93070103db73ae864f86 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:00 +0200 Subject: [PATCH 01/16] drm/udl: Improve type safety when using struct udl_device Push upcasts from struct drm_device to struct udl_device outwards in the call chain; cast earlier and call functions with the upcasted value. Improves type safety. Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-4-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_drv.c | 6 ++++-- drivers/gpu/drm/udl/udl_drv.h | 12 ++++++------ drivers/gpu/drm/udl/udl_main.c | 28 ++++++++++++---------------- drivers/gpu/drm/udl/udl_modeset.c | 21 ++++++++++++--------- drivers/gpu/drm/udl/udl_transfer.c | 6 +++--- 5 files changed, 37 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index d1bc3f165b27..1922988625eb 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -22,13 +22,14 @@ static int udl_usb_suspend(struct usb_interface *interface, pm_message_t message) { struct drm_device *dev = usb_get_intfdata(interface); + struct udl_device *udl = to_udl(dev); int ret; ret = drm_mode_config_helper_suspend(dev); if (ret) return ret; - udl_sync_pending_urbs(dev); + udl_sync_pending_urbs(udl); return 0; } @@ -109,9 +110,10 @@ static int udl_usb_probe(struct usb_interface *interface, static void udl_usb_disconnect(struct usb_interface *interface) { struct drm_device *dev = usb_get_intfdata(interface); + struct udl_device *udl = to_udl(dev); drm_dev_unplug(dev); - udl_drop_usb(dev); + udl_drop_usb(udl); } /* diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index 1204319fc8e3..918738e549d6 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -69,22 +69,22 @@ static inline struct usb_device *udl_to_usb_device(struct udl_device *udl) } /* modeset */ -int udl_modeset_init(struct drm_device *dev); +int udl_modeset_init(struct udl_device *udl); struct drm_connector *udl_connector_init(struct drm_device *dev); -struct urb *udl_get_urb(struct drm_device *dev); +struct urb *udl_get_urb(struct udl_device *udl); -int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len); -void udl_sync_pending_urbs(struct drm_device *dev); +int udl_submit_urb(struct udl_device *udl, struct urb *urb, size_t len); +void udl_sync_pending_urbs(struct udl_device *udl); void udl_urb_completion(struct urb *urb); int udl_init(struct udl_device *udl); -int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr, +int udl_render_hline(struct udl_device *udl, int log_bpp, struct urb **urb_ptr, const char *front, char **urb_buf_ptr, u32 byte_offset, u32 device_byte_offset, u32 byte_width); -int udl_drop_usb(struct drm_device *dev); +int udl_drop_usb(struct udl_device *udl); int udl_select_std_channel(struct udl_device *udl); #endif diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index f1ffa928d5d9..47fb6c34bfde 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -145,9 +145,8 @@ void udl_urb_completion(struct urb *urb) wake_up(&udl->urbs.sleep); } -static void udl_free_urb_list(struct drm_device *dev) +static void udl_free_urb_list(struct udl_device *udl) { - struct udl_device *udl = to_udl(dev); struct urb_node *unode; struct urb *urb; @@ -172,9 +171,8 @@ static void udl_free_urb_list(struct drm_device *dev) wake_up_all(&udl->urbs.sleep); } -static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size) +static int udl_alloc_urb_list(struct udl_device *udl, int count, size_t size) { - struct udl_device *udl = to_udl(dev); struct urb *urb; struct urb_node *unode; char *buf; @@ -210,7 +208,7 @@ retry: usb_free_urb(urb); if (size > PAGE_SIZE) { size /= 2; - udl_free_urb_list(dev); + udl_free_urb_list(udl); goto retry; } break; @@ -259,9 +257,8 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout) } #define GET_URB_TIMEOUT HZ -struct urb *udl_get_urb(struct drm_device *dev) +struct urb *udl_get_urb(struct udl_device *udl) { - struct udl_device *udl = to_udl(dev); struct urb *urb; spin_lock_irq(&udl->urbs.lock); @@ -270,9 +267,8 @@ struct urb *udl_get_urb(struct drm_device *dev) return urb; } -int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len) +int udl_submit_urb(struct udl_device *udl, struct urb *urb, size_t len) { - struct udl_device *udl = to_udl(dev); int ret; if (WARN_ON(len > udl->urbs.size)) { @@ -290,9 +286,9 @@ int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len) } /* wait until all pending URBs have been processed */ -void udl_sync_pending_urbs(struct drm_device *dev) +void udl_sync_pending_urbs(struct udl_device *udl) { - struct udl_device *udl = to_udl(dev); + struct drm_device *dev = &udl->drm; spin_lock_irq(&udl->urbs.lock); /* 2 seconds as a sane timeout */ @@ -329,13 +325,13 @@ int udl_init(struct udl_device *udl) if (udl_select_std_channel(udl)) DRM_ERROR("Selecting channel failed\n"); - if (!udl_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { + if (!udl_alloc_urb_list(udl, WRITES_IN_FLIGHT, MAX_TRANSFER)) { DRM_ERROR("udl_alloc_urb_list failed\n"); goto err; } DRM_DEBUG("\n"); - ret = udl_modeset_init(dev); + ret = udl_modeset_init(udl); if (ret) goto err; @@ -343,14 +339,14 @@ int udl_init(struct udl_device *udl) err: if (udl->urbs.count) - udl_free_urb_list(dev); + udl_free_urb_list(udl); DRM_ERROR("%d\n", ret); return ret; } -int udl_drop_usb(struct drm_device *dev) +int udl_drop_usb(struct udl_device *udl) { - udl_free_urb_list(dev); + udl_free_urb_list(udl); return 0; } diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index 3b65e93ea0ae..231e829bd709 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -205,6 +205,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb, const struct drm_rect *clip) { struct drm_device *dev = fb->dev; + struct udl_device *udl = to_udl(dev); void *vaddr = map->vaddr; /* TODO: Use mapping abstraction properly */ int i, ret; char *cmd; @@ -216,7 +217,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb, return ret; log_bpp = ret; - urb = udl_get_urb(dev); + urb = udl_get_urb(udl); if (!urb) return -ENOMEM; cmd = urb->transfer_buffer; @@ -226,7 +227,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb, const int byte_offset = line_offset + (clip->x1 << log_bpp); const int dev_byte_offset = (fb->width * i + clip->x1) << log_bpp; const int byte_width = drm_rect_width(clip) << log_bpp; - ret = udl_render_hline(dev, log_bpp, &urb, (char *)vaddr, + ret = udl_render_hline(udl, log_bpp, &urb, (char *)vaddr, &cmd, byte_offset, dev_byte_offset, byte_width); if (ret) @@ -239,7 +240,7 @@ static int udl_handle_damage(struct drm_framebuffer *fb, if (cmd < (char *)urb->transfer_buffer + urb->transfer_buffer_length) *cmd++ = UDL_MSG_BULK; len = cmd - (char *)urb->transfer_buffer; - ret = udl_submit_urb(dev, urb, len); + ret = udl_submit_urb(udl, urb, len); } else { udl_urb_completion(urb); } @@ -330,6 +331,7 @@ static const struct drm_plane_funcs udl_primary_plane_funcs = { static void udl_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { struct drm_device *dev = crtc->dev; + struct udl_device *udl = to_udl(dev); struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); struct drm_display_mode *mode = &crtc_state->mode; struct urb *urb; @@ -339,7 +341,7 @@ static void udl_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atom if (!drm_dev_enter(dev, &idx)) return; - urb = udl_get_urb(dev); + urb = udl_get_urb(udl); if (!urb) goto out; @@ -355,7 +357,7 @@ static void udl_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atom buf = udl_vidreg_unlock(buf); buf = udl_dummy_render(buf); - udl_submit_urb(dev, urb, buf - (char *)urb->transfer_buffer); + udl_submit_urb(udl, urb, buf - (char *)urb->transfer_buffer); out: drm_dev_exit(idx); @@ -364,6 +366,7 @@ out: static void udl_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state) { struct drm_device *dev = crtc->dev; + struct udl_device *udl = to_udl(dev); struct urb *urb; char *buf; int idx; @@ -371,7 +374,7 @@ static void udl_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato if (!drm_dev_enter(dev, &idx)) return; - urb = udl_get_urb(dev); + urb = udl_get_urb(udl); if (!urb) goto out; @@ -381,7 +384,7 @@ static void udl_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_ato buf = udl_vidreg_unlock(buf); buf = udl_dummy_render(buf); - udl_submit_urb(dev, urb, buf - (char *)urb->transfer_buffer); + udl_submit_urb(udl, urb, buf - (char *)urb->transfer_buffer); out: drm_dev_exit(idx); @@ -476,9 +479,9 @@ static const struct drm_mode_config_funcs udl_mode_config_funcs = { .atomic_commit = drm_atomic_helper_commit, }; -int udl_modeset_init(struct drm_device *dev) +int udl_modeset_init(struct udl_device *udl) { - struct udl_device *udl = to_udl(dev); + struct drm_device *dev = &udl->drm; struct drm_plane *primary_plane; struct drm_crtc *crtc; struct drm_encoder *encoder; diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c index 62224992988f..7d670b3a5293 100644 --- a/drivers/gpu/drm/udl/udl_transfer.c +++ b/drivers/gpu/drm/udl/udl_transfer.c @@ -170,7 +170,7 @@ static void udl_compress_hline16( * (that we can only write to, slowly, and can never read), and (optionally) * our shadow copy that tracks what's been sent to that hardware buffer. */ -int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr, +int udl_render_hline(struct udl_device *udl, int log_bpp, struct urb **urb_ptr, const char *front, char **urb_buf_ptr, u32 byte_offset, u32 device_byte_offset, u32 byte_width) @@ -199,10 +199,10 @@ int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr, if (cmd >= cmd_end) { int len = cmd - (u8 *) urb->transfer_buffer; - int ret = udl_submit_urb(dev, urb, len); + int ret = udl_submit_urb(udl, urb, len); if (ret) return ret; - urb = udl_get_urb(dev); + urb = udl_get_urb(udl); if (!urb) return -EAGAIN; *urb_ptr = urb; -- 2.50.1 From 89323678d8280f4f8e5971124d3aef8a6dfece0a Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:01 +0200 Subject: [PATCH 02/16] drm/udl: The number of pixels is always positive Store sku_pixel_limit as type unsigned long instead of int. The number of pixels available is always positive. Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-5-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index 918738e549d6..145bb95ccc48 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -51,13 +51,13 @@ struct urb_list { struct udl_device { struct drm_device drm; + unsigned long sku_pixel_limit; + struct drm_plane primary_plane; struct drm_crtc crtc; struct drm_encoder encoder; struct drm_connector connector; - int sku_pixel_limit; - struct urb_list urbs; }; -- 2.50.1 From 895452ae48c1e179df509b6184227a5ea40b4f38 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:02 +0200 Subject: [PATCH 03/16] drm/udl: Handle errors from usb_get_descriptor() Reading the vendor descriptor from the udl device can fail with an error, which the current code fails to capture. Store the return value in an integer and test for the error. Abort parsing on errors or treat the value as length on success. Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-6-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_main.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 47fb6c34bfde..4291ddb7158c 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -31,28 +31,32 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) char *desc; char *buf; char *desc_end; - - u8 total_len = 0; + int ret; + unsigned int len; buf = kzalloc(MAX_VENDOR_DESCRIPTOR_SIZE, GFP_KERNEL); if (!buf) return false; desc = buf; - total_len = usb_get_descriptor(udev, 0x5f, /* vendor specific */ - 0, desc, MAX_VENDOR_DESCRIPTOR_SIZE); - if (total_len > 5) { - DRM_INFO("vendor descriptor length:%x data:%11ph\n", - total_len, desc); + ret = usb_get_descriptor(udev, 0x5f, /* vendor specific */ + 0, desc, MAX_VENDOR_DESCRIPTOR_SIZE); + if (ret < 0) + goto unrecognized; + len = ret; + + if (len > 5) { + DRM_INFO("vendor descriptor length: %u data:%11ph\n", + len, desc); - if ((desc[0] != total_len) || /* descriptor length */ + if ((desc[0] != len) || /* descriptor length */ (desc[1] != 0x5f) || /* vendor descriptor type */ (desc[2] != 0x01) || /* version (2 bytes) */ (desc[3] != 0x00) || - (desc[4] != total_len - 2)) /* length after type */ + (desc[4] != len - 2)) /* length after type */ goto unrecognized; - desc_end = desc + total_len; + desc_end = desc + len; desc += 5; /* the fixed header we've already parsed */ while (desc < desc_end) { -- 2.50.1 From 1fad33f04ed1fd14049cda6a35faa4abc0886201 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:03 +0200 Subject: [PATCH 04/16] drm/udl: Return error if vendor descriptor is too short There need to be least 5 bytes in the vendor descriptor. Return an error otherwise. Also change the branching to early-out on the error. Adjust indention of the rest of the parser function. The original length test expected more than 5 bytes in the vendor descriptor. As a descriptor with no key-value pairs has exactly 5 bytes and is still valid, change the test to accept this case as well. v2 - clarify changes to length test in commit description (Patrik) Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-7-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_main.c | 72 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 4291ddb7158c..58d6065589d3 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -45,43 +45,43 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) goto unrecognized; len = ret; - if (len > 5) { - DRM_INFO("vendor descriptor length: %u data:%11ph\n", - len, desc); - - if ((desc[0] != len) || /* descriptor length */ - (desc[1] != 0x5f) || /* vendor descriptor type */ - (desc[2] != 0x01) || /* version (2 bytes) */ - (desc[3] != 0x00) || - (desc[4] != len - 2)) /* length after type */ - goto unrecognized; - - desc_end = desc + len; - desc += 5; /* the fixed header we've already parsed */ - - while (desc < desc_end) { - u8 length; - u16 key; - - key = le16_to_cpu(*((u16 *) desc)); - desc += sizeof(u16); - length = *desc; - desc++; - - switch (key) { - case 0x0200: { /* max_area */ - u32 max_area; - max_area = le32_to_cpu(*((u32 *)desc)); - DRM_DEBUG("DL chip limited to %d pixel modes\n", - max_area); - udl->sku_pixel_limit = max_area; - break; - } - default: - break; - } - desc += length; + if (len < 5) + goto unrecognized; + + DRM_INFO("vendor descriptor length: %u data:%11ph\n", len, desc); + + if ((desc[0] != len) || /* descriptor length */ + (desc[1] != 0x5f) || /* vendor descriptor type */ + (desc[2] != 0x01) || /* version (2 bytes) */ + (desc[3] != 0x00) || + (desc[4] != len - 2)) /* length after type */ + goto unrecognized; + + desc_end = desc + len; + desc += 5; /* the fixed header we've already parsed */ + + while (desc < desc_end) { + u8 length; + u16 key; + + key = le16_to_cpu(*((u16 *)desc)); + desc += sizeof(u16); + length = *desc; + desc++; + + switch (key) { + case 0x0200: { /* max_area */ + u32 max_area = le32_to_cpu(*((u32 *)desc)); + + DRM_DEBUG("DL chip limited to %d pixel modes\n", + max_area); + udl->sku_pixel_limit = max_area; + break; + } + default: + break; } + desc += length; } goto success; -- 2.50.1 From f4fce9ea8473ab8f91c0aa0855b9d51313b8000e Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:04 +0200 Subject: [PATCH 05/16] drm/udl: Treat vendor descriptor as u8 The vendor descriptor is an array of unsigned bytes. It is raw data that is not to be modified. Declare it as 'const u8'. Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-8-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_main.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 58d6065589d3..d3a04bcb65d2 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -28,19 +28,18 @@ static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout); static int udl_parse_vendor_descriptor(struct udl_device *udl) { struct usb_device *udev = udl_to_usb_device(udl); - char *desc; - char *buf; - char *desc_end; + void *buf; int ret; unsigned int len; + const u8 *desc; + const u8 *desc_end; buf = kzalloc(MAX_VENDOR_DESCRIPTOR_SIZE, GFP_KERNEL); if (!buf) return false; - desc = buf; ret = usb_get_descriptor(udev, 0x5f, /* vendor specific */ - 0, desc, MAX_VENDOR_DESCRIPTOR_SIZE); + 0, buf, MAX_VENDOR_DESCRIPTOR_SIZE); if (ret < 0) goto unrecognized; len = ret; @@ -48,6 +47,9 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) if (len < 5) goto unrecognized; + desc = buf; + desc_end = desc + len; + DRM_INFO("vendor descriptor length: %u data:%11ph\n", len, desc); if ((desc[0] != len) || /* descriptor length */ @@ -56,9 +58,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) (desc[3] != 0x00) || (desc[4] != len - 2)) /* length after type */ goto unrecognized; - - desc_end = desc + len; - desc += 5; /* the fixed header we've already parsed */ + desc += 5; while (desc < desc_end) { u8 length; -- 2.50.1 From df6dc12e5e6ddbf2a59d5283fcf6ccd0b3d0334d Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:05 +0200 Subject: [PATCH 06/16] drm/udl: Validate length in vendor-descriptor parser Rewrite the parser for the vendor firmware descriptor with the following improvements. - Validate the key-value length given in a vendor descriptor against the length of the descriptor. The current code fails to do this and might read more bytes than available. This can lead to out-of-bounds reads of the allocated buffer. - Read raw data with helpers for unaligned data. This allows the code to run on platforms that do now support unaligned memory access by default. - Validate the pixel limit against a default value. The default comes from real-world devices. If the reported number of pixels is significantly above the limit, it is likely invalid. - Drop the obsolete print macros. There is still a warning about invalid firmware descriptors. The rest of the output is bogus. Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-9-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_main.c | 77 ++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index d3a04bcb65d2..b5a6b254a202 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -8,6 +8,8 @@ * Copyright (C) 2009 Bernie Thompson */ +#include + #include #include #include @@ -23,10 +25,56 @@ #define WRITES_IN_FLIGHT (20) #define MAX_VENDOR_DESCRIPTOR_SIZE 256 +#define UDL_SKU_PIXEL_LIMIT_DEFAULT 2080000 + static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout); +/* + * Try to make sense of whatever we parse. Therefore return @end on + * errors, but don't fail hard. + */ +static const u8 *udl_parse_key_value_pair(struct udl_device *udl, const u8 *pos, const u8 *end) +{ + u16 key; + u8 len; + + /* read key */ + if (pos >= end - 2) + return end; + key = get_unaligned_le16(pos); + pos += 2; + + /* read value length */ + if (pos >= end - 1) + return end; + len = *pos++; + + /* read value */ + if (pos >= end - len) + return end; + switch (key) { + case 0x0200: { /* maximum number of pixels */ + unsigned int sku_pixel_limit; + + if (len < sizeof(__le32)) + break; + sku_pixel_limit = get_unaligned_le32(pos); + if (sku_pixel_limit >= 16 * UDL_SKU_PIXEL_LIMIT_DEFAULT) + break; /* almost 100 MiB, so probably bogus */ + udl->sku_pixel_limit = sku_pixel_limit; + break; + } + default: + break; + } + pos += len; + + return pos; +} + static int udl_parse_vendor_descriptor(struct udl_device *udl) { + struct drm_device *dev = &udl->drm; struct usb_device *udev = udl_to_usb_device(udl); void *buf; int ret; @@ -50,8 +98,6 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) desc = buf; desc_end = desc + len; - DRM_INFO("vendor descriptor length: %u data:%11ph\n", len, desc); - if ((desc[0] != len) || /* descriptor length */ (desc[1] != 0x5f) || /* vendor descriptor type */ (desc[2] != 0x01) || /* version (2 bytes) */ @@ -60,35 +106,14 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) goto unrecognized; desc += 5; - while (desc < desc_end) { - u8 length; - u16 key; - - key = le16_to_cpu(*((u16 *)desc)); - desc += sizeof(u16); - length = *desc; - desc++; - - switch (key) { - case 0x0200: { /* max_area */ - u32 max_area = le32_to_cpu(*((u32 *)desc)); - - DRM_DEBUG("DL chip limited to %d pixel modes\n", - max_area); - udl->sku_pixel_limit = max_area; - break; - } - default: - break; - } - desc += length; - } + while (desc < desc_end) + desc = udl_parse_key_value_pair(udl, desc, desc_end); goto success; unrecognized: /* allow udlfb to load for now even if firmware unrecognized */ - DRM_ERROR("Unrecognized vendor firmware descriptor\n"); + drm_warn(dev, "Unrecognized vendor firmware descriptor\n"); success: kfree(buf); -- 2.50.1 From fb10144ba426b88080c6abcede5f1332bebc3c60 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Thu, 10 Apr 2025 12:59:06 +0200 Subject: [PATCH 07/16] drm/udl: Support adapters without firmware descriptor Set default limit on the number of pixels for adapters without vendor firmware descriptor. The devices work as expected, they just don't provide any description. If parsing the vendor firmware descriptor fails, the device falls back to the given default limits. Failing to allocate memory is still an error. v2: - fix typo in constant (Patrik) Signed-off-by: Thomas Zimmermann Reviewed-by: Patrik Jakobsson Link: https://lore.kernel.org/r/20250410105948.25463-10-tzimmermann@suse.de --- drivers/gpu/drm/udl/udl_main.c | 37 +++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index b5a6b254a202..3dfeb88cf918 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -76,6 +76,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) { struct drm_device *dev = &udl->drm; struct usb_device *udev = udl_to_usb_device(udl); + bool detected = false; void *buf; int ret; unsigned int len; @@ -84,16 +85,16 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) buf = kzalloc(MAX_VENDOR_DESCRIPTOR_SIZE, GFP_KERNEL); if (!buf) - return false; + return -ENOMEM; ret = usb_get_descriptor(udev, 0x5f, /* vendor specific */ 0, buf, MAX_VENDOR_DESCRIPTOR_SIZE); if (ret < 0) - goto unrecognized; + goto out; len = ret; if (len < 5) - goto unrecognized; + goto out; desc = buf; desc_end = desc + len; @@ -103,21 +104,20 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl) (desc[2] != 0x01) || /* version (2 bytes) */ (desc[3] != 0x00) || (desc[4] != len - 2)) /* length after type */ - goto unrecognized; + goto out; desc += 5; + detected = true; + while (desc < desc_end) desc = udl_parse_key_value_pair(udl, desc, desc_end); - goto success; - -unrecognized: - /* allow udlfb to load for now even if firmware unrecognized */ - drm_warn(dev, "Unrecognized vendor firmware descriptor\n"); - -success: +out: + if (!detected) + drm_warn(dev, "Unrecognized vendor firmware descriptor\n"); kfree(buf); - return true; + + return 0; } /* @@ -345,11 +345,16 @@ int udl_init(struct udl_device *udl) drm_warn(dev, "buffer sharing not supported"); /* not an error */ } - if (!udl_parse_vendor_descriptor(udl)) { - ret = -ENODEV; - DRM_ERROR("firmware not recognized. Assume incompatible device\n"); + /* + * Not all devices provide vendor descriptors with device + * information. Initialize to default values of real-world + * devices. It is just enough memory for FullHD. + */ + udl->sku_pixel_limit = UDL_SKU_PIXEL_LIMIT_DEFAULT; + + ret = udl_parse_vendor_descriptor(udl); + if (ret) goto err; - } if (udl_select_std_channel(udl)) DRM_ERROR("Selecting channel failed\n"); -- 2.50.1 From 970003de19a9cf7caa68a136b3bfa7334c8e7132 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:40 +0100 Subject: [PATCH 08/16] drm/etnaviv: Test for imported buffers with drm_gem_is_imported() Instead of testing import_attach for imported GEM buffers, invoke drm_gem_is_imported() to do the test. The helper tests the dma_buf itself while import_attach is just an artifact of the import. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Lucas Stach Cc: Russell King Cc: Christian Gmeiner Cc: etnaviv@lists.freedesktop.org Reviewed-by: Lucas Stach Link: https://lore.kernel.org/r/20250317131923.238374-3-tzimmermann@suse.de --- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 42e57d142554..40a50c60dfff 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -39,7 +39,7 @@ int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map) int etnaviv_gem_prime_pin(struct drm_gem_object *obj) { - if (!obj->import_attach) { + if (!drm_gem_is_imported(obj)) { struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); mutex_lock(&etnaviv_obj->lock); @@ -51,7 +51,7 @@ int etnaviv_gem_prime_pin(struct drm_gem_object *obj) void etnaviv_gem_prime_unpin(struct drm_gem_object *obj) { - if (!obj->import_attach) { + if (!drm_gem_is_imported(obj)) { struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); mutex_lock(&etnaviv_obj->lock); -- 2.50.1 From e91eb3ae415472b28211d7fed07fa283845b311e Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:41 +0100 Subject: [PATCH 09/16] drm/etnaviv: Use dma_buf from GEM object instance Avoid dereferencing struct drm_gem_object.import_attach for the imported dma-buf. The dma_buf field in the GEM object instance refers to the same buffer. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Lucas Stach Cc: Russell King Cc: Christian Gmeiner Cc: etnaviv@lists.freedesktop.org Reviewed-by: Lucas Stach Link: https://lore.kernel.org/r/20250317131923.238374-4-tzimmermann@suse.de --- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 40a50c60dfff..917ad527c961 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj) struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr); if (etnaviv_obj->vaddr) - dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map); + dma_buf_vunmap_unlocked(etnaviv_obj->base.dma_buf, &map); /* Don't drop the pages for imported dmabuf, as they are not * ours, just free the array we allocated: @@ -82,7 +82,7 @@ static void *etnaviv_gem_prime_vmap_impl(struct etnaviv_gem_object *etnaviv_obj) lockdep_assert_held(&etnaviv_obj->lock); - ret = dma_buf_vmap(etnaviv_obj->base.import_attach->dmabuf, &map); + ret = dma_buf_vmap(etnaviv_obj->base.dma_buf, &map); if (ret) return NULL; return map.vaddr; -- 2.50.1 From 55eba86e11381af782830760774bc22868af20a0 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:44 +0100 Subject: [PATCH 10/16] drm/msm: Test for imported buffers with drm_gem_is_imported() Instead of testing import_attach for imported GEM buffers, invoke drm_gem_is_imported() to do the test. The helper tests the dma_buf itself while import_attach is just an artifact of the import. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Cc: Marijn Suijten Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Acked-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250317131923.238374-7-tzimmermann@suse.de --- drivers/gpu/drm/msm/msm_drv.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 4 ++-- drivers/gpu/drm/msm/msm_gem.h | 2 +- drivers/gpu/drm/msm/msm_gem_prime.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index c3588dc9e537..f316e6776f67 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -671,7 +671,7 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, ret = msm_ioctl_gem_info_set_iova(dev, file, obj, args->value); break; case MSM_INFO_GET_FLAGS: - if (obj->import_attach) { + if (drm_gem_is_imported(obj)) { ret = -EINVAL; break; } diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index ebc9ba66efb8..2995e80fec3b 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -735,7 +735,7 @@ static void *get_vaddr(struct drm_gem_object *obj, unsigned madv) msm_gem_assert_locked(obj); - if (obj->import_attach) + if (drm_gem_is_imported(obj)) return ERR_PTR(-ENODEV); pages = msm_gem_get_pages_locked(obj, madv); @@ -1074,7 +1074,7 @@ static void msm_gem_free_object(struct drm_gem_object *obj) put_iova_spaces(obj, true); - if (obj->import_attach) { + if (drm_gem_is_imported(obj)) { GEM_WARN_ON(msm_obj->vaddr); /* Don't drop the pages for imported dmabuf, as they are not diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 85f0257e83da..ba5c4ff76292 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -224,7 +224,7 @@ msm_gem_assert_locked(struct drm_gem_object *obj) /* imported/exported objects are not purgeable: */ static inline bool is_unpurgeable(struct msm_gem_object *msm_obj) { - return msm_obj->base.import_attach || msm_obj->pin_count; + return drm_gem_is_imported(&msm_obj->base) || msm_obj->pin_count; } static inline bool is_purgeable(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c b/drivers/gpu/drm/msm/msm_gem_prime.c index ee267490c935..2e37913d5a6a 100644 --- a/drivers/gpu/drm/msm/msm_gem_prime.c +++ b/drivers/gpu/drm/msm/msm_gem_prime.c @@ -50,7 +50,7 @@ int msm_gem_prime_pin(struct drm_gem_object *obj) struct page **pages; int ret = 0; - if (obj->import_attach) + if (drm_gem_is_imported(obj)) return 0; pages = msm_gem_pin_pages_locked(obj); @@ -62,7 +62,7 @@ int msm_gem_prime_pin(struct drm_gem_object *obj) void msm_gem_prime_unpin(struct drm_gem_object *obj) { - if (obj->import_attach) + if (drm_gem_is_imported(obj)) return; msm_gem_unpin_pages_locked(obj); -- 2.50.1 From 64e21b80ded95e919060a2043c52f58ecb281aac Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:46 +0100 Subject: [PATCH 11/16] drm/panfrost: Test for imported buffers with drm_gem_is_imported() Instead of testing import_attach for imported GEM buffers, invoke drm_gem_is_imported() to do the test. The helper tests the dma_buf itself while import_attach is just an artifact of the import. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Boris Brezillon Cc: Rob Herring Cc: Steven Price Reviewed-by: Steven Price Link: https://lore.kernel.org/r/20250317131923.238374-9-tzimmermann@suse.de --- drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index 8e0ff3efede7..963f04ba2de6 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -200,7 +200,7 @@ static enum drm_gem_object_status panfrost_gem_status(struct drm_gem_object *obj struct panfrost_gem_object *bo = to_panfrost_bo(obj); enum drm_gem_object_status res = 0; - if (bo->base.base.import_attach || bo->base.pages) + if (drm_gem_is_imported(&bo->base.base) || bo->base.pages) res |= DRM_GEM_OBJECT_RESIDENT; if (bo->base.madv == PANFROST_MADV_DONTNEED) -- 2.50.1 From 266ab86ac1f5c05a91bd1a73929c590bfeb3c491 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:47 +0100 Subject: [PATCH 12/16] drm/panthor: Test for imported buffers with drm_gem_is_imported() Instead of testing import_attach for imported GEM buffers, invoke drm_gem_is_imported() to do the test. The helper tests the dma_buf itself while import_attach is just an artifact of the import. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Boris Brezillon Cc: Steven Price Cc: Liviu Dudau Reviewed-by: Liviu Dudau Reviewed-by: Steven Price Link: https://lore.kernel.org/r/20250317131923.238374-10-tzimmermann@suse.de --- drivers/gpu/drm/panthor/panthor_gem.c | 2 +- drivers/gpu/drm/panthor/panthor_mmu.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c index 8244a4e6c2a2..fd014ccc3bfc 100644 --- a/drivers/gpu/drm/panthor/panthor_gem.c +++ b/drivers/gpu/drm/panthor/panthor_gem.c @@ -155,7 +155,7 @@ static enum drm_gem_object_status panthor_gem_status(struct drm_gem_object *obj) struct panthor_gem_object *bo = to_panthor_bo(obj); enum drm_gem_object_status res = 0; - if (bo->base.base.import_attach || bo->base.pages) + if (drm_gem_is_imported(&bo->base.base) || bo->base.pages) res |= DRM_GEM_OBJECT_RESIDENT; return res; diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c index dc173c6edde0..6ca9a2642a4e 100644 --- a/drivers/gpu/drm/panthor/panthor_mmu.c +++ b/drivers/gpu/drm/panthor/panthor_mmu.c @@ -1104,7 +1104,7 @@ static void panthor_vm_bo_put(struct drm_gpuvm_bo *vm_bo) /* If the vm_bo object was destroyed, release the pin reference that * was hold by this object. */ - if (unpin && !bo->base.base.import_attach) + if (unpin && !drm_gem_is_imported(&bo->base.base)) drm_gem_shmem_unpin(&bo->base); drm_gpuvm_put(vm); @@ -1235,7 +1235,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx, if (ret) goto err_cleanup; - if (!bo->base.base.import_attach) { + if (!drm_gem_is_imported(&bo->base.base)) { /* Pre-reserve the BO pages, so the map operation doesn't have to * allocate. */ @@ -1246,7 +1246,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx, sgt = drm_gem_shmem_get_pages_sgt(&bo->base); if (IS_ERR(sgt)) { - if (!bo->base.base.import_attach) + if (!drm_gem_is_imported(&bo->base.base)) drm_gem_shmem_unpin(&bo->base); ret = PTR_ERR(sgt); @@ -1257,7 +1257,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx, preallocated_vm_bo = drm_gpuvm_bo_create(&vm->base, &bo->base.base); if (!preallocated_vm_bo) { - if (!bo->base.base.import_attach) + if (!drm_gem_is_imported(&bo->base.base)) drm_gem_shmem_unpin(&bo->base); ret = -ENOMEM; @@ -1283,7 +1283,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx, * which will be released in panthor_vm_bo_put(). */ if (preallocated_vm_bo != op_ctx->map.vm_bo && - !bo->base.base.import_attach) + !drm_gem_is_imported(&bo->base.base)) drm_gem_shmem_unpin(&bo->base); op_ctx->map.bo_offset = offset; -- 2.50.1 From 5a80b00d0d10b920397b9cbd4102b1c07ba6583a Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:51 +0100 Subject: [PATCH 13/16] drm/vmwgfx: Test for imported buffers with drm_gem_is_imported() Instead of testing import_attach for imported GEM buffers, invoke drm_gem_is_imported() to do the test. The helper tests the dma_buf itself while import_attach is just an artifact of the import. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Zack Rusin Cc: Broadcom internal kernel review list Reviewed-by: Zack Rusin Link: https://lore.kernel.org/r/20250317131923.238374-14-tzimmermann@suse.de --- drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c index 64bd7d74854e..fa5841fda659 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -429,7 +429,7 @@ static void *map_external(struct vmw_bo *bo, struct iosys_map *map) void *ptr = NULL; int ret; - if (bo->tbo.base.import_attach) { + if (drm_gem_is_imported(&bo->tbo.base)) { ret = dma_buf_vmap(bo->tbo.base.dma_buf, map); if (ret) { drm_dbg_driver(&vmw->drm, @@ -447,7 +447,7 @@ out: static void unmap_external(struct vmw_bo *bo, struct iosys_map *map) { - if (bo->tbo.base.import_attach) + if (drm_gem_is_imported(&bo->tbo.base)) dma_buf_vunmap(bo->tbo.base.dma_buf, map); else vmw_bo_unmap(bo); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index 026c9b699604..e417921af584 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -84,7 +84,7 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(obj); int ret; - if (obj->import_attach) { + if (drm_gem_is_imported(obj)) { ret = dma_buf_vmap(obj->import_attach->dmabuf, map); if (!ret) { if (drm_WARN_ON(obj->dev, map->is_iomem)) { @@ -101,7 +101,7 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) static void vmw_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) { - if (obj->import_attach) + if (drm_gem_is_imported(obj)) dma_buf_vunmap(obj->import_attach->dmabuf, map); else drm_gem_ttm_vunmap(obj, map); @@ -111,7 +111,7 @@ static int vmw_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) { int ret; - if (obj->import_attach) { + if (drm_gem_is_imported(obj)) { /* * Reset both vm_ops and vm_private_data, so we don't end up with * vm_ops pointing to our implementation if the dma-buf backend -- 2.50.1 From aec8a40228acb385d60feec59b54573d307e60f3 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Mar 2025 14:06:52 +0100 Subject: [PATCH 14/16] drm/vmwgfx: Use dma_buf from GEM object instance Avoid dereferencing struct drm_gem_object.import_attach for the imported dma-buf. The dma_buf field in the GEM object instance refers to the same buffer. Prepares to make import_attach optional. Signed-off-by: Thomas Zimmermann Cc: Zack Rusin Cc: Broadcom internal kernel review list Reviewed-by: Zack Rusin Link: https://lore.kernel.org/r/20250317131923.238374-15-tzimmermann@suse.de --- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index e417921af584..c55382167c1b 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -85,10 +85,10 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) int ret; if (drm_gem_is_imported(obj)) { - ret = dma_buf_vmap(obj->import_attach->dmabuf, map); + ret = dma_buf_vmap(obj->dma_buf, map); if (!ret) { if (drm_WARN_ON(obj->dev, map->is_iomem)) { - dma_buf_vunmap(obj->import_attach->dmabuf, map); + dma_buf_vunmap(obj->dma_buf, map); return -EIO; } } @@ -102,7 +102,7 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map) static void vmw_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map) { if (drm_gem_is_imported(obj)) - dma_buf_vunmap(obj->import_attach->dmabuf, map); + dma_buf_vunmap(obj->dma_buf, map); else drm_gem_ttm_vunmap(obj, map); } -- 2.50.1 From f8cc87024d1a1261f71dd2413951b87875b3422f Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 9 Apr 2025 17:38:13 +0800 Subject: [PATCH 15/16] drm/bridge: anx7625: Use devm_pm_runtime_enable() The anx7625 driver is open coding what devm_pm_runtime_enable() does. Switch to the common helper instead. Reviewed-by: Dmitry Baryshkov Link: https://patch.msgid.link/20250409093814.3977025-1-wenst@chromium.org Signed-off-by: Chen-Yu Tsai --- drivers/gpu/drm/bridge/analogix/anx7625.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 866806e908cd..8bfe477c476c 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -2569,12 +2569,6 @@ static const struct dev_pm_ops anx7625_pm_ops = { anx7625_runtime_pm_resume, NULL) }; -static void anx7625_runtime_disable(void *data) -{ - pm_runtime_dont_use_autosuspend(data); - pm_runtime_disable(data); -} - static int anx7625_link_bridge(struct drm_dp_aux *aux) { struct anx7625_data *platform = container_of(aux, struct anx7625_data, aux); @@ -2708,11 +2702,10 @@ static int anx7625_i2c_probe(struct i2c_client *client) goto free_wq; } - pm_runtime_enable(dev); pm_runtime_set_autosuspend_delay(dev, 1000); pm_runtime_use_autosuspend(dev); pm_suspend_ignore_children(dev, true); - ret = devm_add_action_or_reset(dev, anx7625_runtime_disable, dev); + ret = devm_pm_runtime_enable(dev); if (ret) goto free_wq; -- 2.50.1 From fec450ca15af63649e219060f37a8ec673333726 Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Tue, 8 Apr 2025 16:54:25 +0300 Subject: [PATCH 16/16] drm/display: hdmi: provide central data authority for ACR params HDMI standard defines recommended N and CTS values for Audio Clock Regeneration. Currently each driver implements those, frequently in somewhat unique way. Provide a generic helper for getting those values to be used by the HDMI drivers. The helper is added to drm_hdmi_helper.c rather than drm_hdmi_audio.c since HDMI drivers can be using this helper function even without switching to DRM HDMI Audio helpers. Note: currently this only handles the values per HDMI 1.4b Section 7.2 and HDMI 2.0 Section 9.2.1. Later the table can be expanded to accommodate for Deep Color TMDS char rates per HDMI 1.4 Appendix D and/or HDMI 2.0 / 2.1 Appendix C). Reviewed-by: Maxime Ripard Link: https://lore.kernel.org/r/20250408-drm-hdmi-acr-v2-1-dee7298ab1af@oss.qualcomm.com Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/display/drm_hdmi_helper.c | 168 ++++++++++++++++++++++ include/drm/display/drm_hdmi_helper.h | 6 + 2 files changed, 174 insertions(+) diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c index 74dd4d01dd9b..855cb02b827d 100644 --- a/drivers/gpu/drm/display/drm_hdmi_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c @@ -256,3 +256,171 @@ drm_hdmi_compute_mode_clock(const struct drm_display_mode *mode, return DIV_ROUND_CLOSEST_ULL(clock * bpc, 8); } EXPORT_SYMBOL(drm_hdmi_compute_mode_clock); + +struct drm_hdmi_acr_n_cts_entry { + unsigned int n; + unsigned int cts; +}; + +struct drm_hdmi_acr_data { + unsigned long tmds_clock_khz; + struct drm_hdmi_acr_n_cts_entry n_cts_32k, + n_cts_44k1, + n_cts_48k; +}; + +static const struct drm_hdmi_acr_data hdmi_acr_n_cts[] = { + { + /* "Other" entry */ + .n_cts_32k = { .n = 4096, }, + .n_cts_44k1 = { .n = 6272, }, + .n_cts_48k = { .n = 6144, }, + }, { + .tmds_clock_khz = 25175, + .n_cts_32k = { .n = 4576, .cts = 28125, }, + .n_cts_44k1 = { .n = 7007, .cts = 31250, }, + .n_cts_48k = { .n = 6864, .cts = 28125, }, + }, { + .tmds_clock_khz = 25200, + .n_cts_32k = { .n = 4096, .cts = 25200, }, + .n_cts_44k1 = { .n = 6272, .cts = 28000, }, + .n_cts_48k = { .n = 6144, .cts = 25200, }, + }, { + .tmds_clock_khz = 27000, + .n_cts_32k = { .n = 4096, .cts = 27000, }, + .n_cts_44k1 = { .n = 6272, .cts = 30000, }, + .n_cts_48k = { .n = 6144, .cts = 27000, }, + }, { + .tmds_clock_khz = 27027, + .n_cts_32k = { .n = 4096, .cts = 27027, }, + .n_cts_44k1 = { .n = 6272, .cts = 30030, }, + .n_cts_48k = { .n = 6144, .cts = 27027, }, + }, { + .tmds_clock_khz = 54000, + .n_cts_32k = { .n = 4096, .cts = 54000, }, + .n_cts_44k1 = { .n = 6272, .cts = 60000, }, + .n_cts_48k = { .n = 6144, .cts = 54000, }, + }, { + .tmds_clock_khz = 54054, + .n_cts_32k = { .n = 4096, .cts = 54054, }, + .n_cts_44k1 = { .n = 6272, .cts = 60060, }, + .n_cts_48k = { .n = 6144, .cts = 54054, }, + }, { + .tmds_clock_khz = 74176, + .n_cts_32k = { .n = 11648, .cts = 210937, }, /* and 210938 */ + .n_cts_44k1 = { .n = 17836, .cts = 234375, }, + .n_cts_48k = { .n = 11648, .cts = 140625, }, + }, { + .tmds_clock_khz = 74250, + .n_cts_32k = { .n = 4096, .cts = 74250, }, + .n_cts_44k1 = { .n = 6272, .cts = 82500, }, + .n_cts_48k = { .n = 6144, .cts = 74250, }, + }, { + .tmds_clock_khz = 148352, + .n_cts_32k = { .n = 11648, .cts = 421875, }, + .n_cts_44k1 = { .n = 8918, .cts = 234375, }, + .n_cts_48k = { .n = 5824, .cts = 140625, }, + }, { + .tmds_clock_khz = 148500, + .n_cts_32k = { .n = 4096, .cts = 148500, }, + .n_cts_44k1 = { .n = 6272, .cts = 165000, }, + .n_cts_48k = { .n = 6144, .cts = 148500, }, + }, { + .tmds_clock_khz = 296703, + .n_cts_32k = { .n = 5824, .cts = 421875, }, + .n_cts_44k1 = { .n = 4459, .cts = 234375, }, + .n_cts_48k = { .n = 5824, .cts = 281250, }, + }, { + .tmds_clock_khz = 297000, + .n_cts_32k = { .n = 3072, .cts = 222750, }, + .n_cts_44k1 = { .n = 4704, .cts = 247500, }, + .n_cts_48k = { .n = 5120, .cts = 247500, }, + }, { + .tmds_clock_khz = 593407, + .n_cts_32k = { .n = 5824, .cts = 843750, }, + .n_cts_44k1 = { .n = 8918, .cts = 937500, }, + .n_cts_48k = { .n = 5824, .cts = 562500, }, + }, { + .tmds_clock_khz = 594000, + .n_cts_32k = { .n = 3072, .cts = 445500, }, + .n_cts_44k1 = { .n = 9408, .cts = 990000, }, + .n_cts_48k = { .n = 6144, .cts = 594000, }, + }, +}; + +static int drm_hdmi_acr_find_tmds_entry(unsigned long tmds_clock_khz) +{ + int i; + + /* skip the "other" entry */ + for (i = 1; i < ARRAY_SIZE(hdmi_acr_n_cts); i++) { + if (hdmi_acr_n_cts[i].tmds_clock_khz == tmds_clock_khz) + return i; + } + + return 0; +} + +/** + * drm_hdmi_acr_get_n_cts() - get N and CTS values for Audio Clock Regeneration + * + * @tmds_char_rate: TMDS clock (char rate) as used by the HDMI connector + * @sample_rate: audio sample rate + * @out_n: a pointer to write the N value + * @out_cts: a pointer to write the CTS value + * + * Get the N and CTS values (either by calculating them or by returning data + * from the tables. This follows the HDMI 1.4b Section 7.2 "Audio Sample Clock + * Capture and Regeneration". + * + * Note, @sample_rate corresponds to the Fs value, see sections 7.2.4 - 7.2.6 + * on how to select Fs for non-L-PCM formats. + */ +void +drm_hdmi_acr_get_n_cts(unsigned long long tmds_char_rate, + unsigned int sample_rate, + unsigned int *out_n, + unsigned int *out_cts) +{ + /* be a bit more tolerant, especially for the 1.001 entries */ + unsigned long tmds_clock_khz = DIV_ROUND_CLOSEST_ULL(tmds_char_rate, 1000); + const struct drm_hdmi_acr_n_cts_entry *entry; + unsigned int n, cts, mult; + int tmds_idx; + + tmds_idx = drm_hdmi_acr_find_tmds_entry(tmds_clock_khz); + + /* + * Don't change the order, 192 kHz is divisible by 48k and 32k, but it + * should use 48k entry. + */ + if (sample_rate % 48000 == 0) { + entry = &hdmi_acr_n_cts[tmds_idx].n_cts_48k; + mult = sample_rate / 48000; + } else if (sample_rate % 44100 == 0) { + entry = &hdmi_acr_n_cts[tmds_idx].n_cts_44k1; + mult = sample_rate / 44100; + } else if (sample_rate % 32000 == 0) { + entry = &hdmi_acr_n_cts[tmds_idx].n_cts_32k; + mult = sample_rate / 32000; + } else { + entry = NULL; + } + + if (entry) { + n = entry->n * mult; + cts = entry->cts; + } else { + /* Recommended optimal value, HDMI 1.4b, Section 7.2.1 */ + n = 128 * sample_rate / 1000; + cts = 0; + } + + if (!cts) + cts = DIV_ROUND_CLOSEST_ULL(tmds_char_rate * n, + 128 * sample_rate); + + *out_n = n; + *out_cts = cts; +} +EXPORT_SYMBOL(drm_hdmi_acr_get_n_cts); diff --git a/include/drm/display/drm_hdmi_helper.h b/include/drm/display/drm_hdmi_helper.h index 57e3b18c15ec..09145c9ee9fc 100644 --- a/include/drm/display/drm_hdmi_helper.h +++ b/include/drm/display/drm_hdmi_helper.h @@ -28,4 +28,10 @@ unsigned long long drm_hdmi_compute_mode_clock(const struct drm_display_mode *mode, unsigned int bpc, enum hdmi_colorspace fmt); +void +drm_hdmi_acr_get_n_cts(unsigned long long tmds_char_rate, + unsigned int sample_rate, + unsigned int *out_n, + unsigned int *out_cts); + #endif -- 2.50.1