From 8456b99c16d193c4c3b7df305cf431e027f0189c Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:40:55 +0200 Subject: udl-kms: change down_interruptible to down If we leave urbs around, it causes not only leak, but also memory corruption. This patch fixes the function udl_free_urb_list, so that it always waits for all urbs that are in progress. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_main.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index d518de8f496b..1239decd46ff 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -170,18 +170,13 @@ static void udl_free_urb_list(struct drm_device *dev) struct list_head *node; struct urb_node *unode; struct urb *urb; - int ret; unsigned long flags; DRM_DEBUG("Waiting for completes and freeing all render urbs\n"); /* keep waiting and freeing, until we've got 'em all */ while (count--) { - - /* Getting interrupted means a leak, but ok at shutdown*/ - ret = down_interruptible(&udl->urbs.limit_sem); - if (ret) - break; + down(&udl->urbs.limit_sem); spin_lock_irqsave(&udl->urbs.lock, flags); -- cgit v1.2.3 From 542bb9788a1f485eb1a2229178f665d8ea166156 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:40:56 +0200 Subject: udl-kms: handle allocation failure Allocations larger than PAGE_ALLOC_COSTLY_ORDER are unreliable and they may fail anytime. This patch fixes the udl kms driver so that when a large alloactions fails, it tries to do multiple smaller allocations. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_main.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 1239decd46ff..7e9ad926926a 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -200,17 +200,22 @@ static void udl_free_urb_list(struct drm_device *dev) static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size) { struct udl_device *udl = dev->dev_private; - int i = 0; struct urb *urb; struct urb_node *unode; char *buf; + size_t wanted_size = count * size; spin_lock_init(&udl->urbs.lock); +retry: udl->urbs.size = size; INIT_LIST_HEAD(&udl->urbs.list); - while (i < count) { + sema_init(&udl->urbs.limit_sem, 0); + udl->urbs.count = 0; + udl->urbs.available = 0; + + while (udl->urbs.count * size < wanted_size) { unode = kzalloc(sizeof(struct urb_node), GFP_KERNEL); if (!unode) break; @@ -226,11 +231,16 @@ static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size) } unode->urb = urb; - buf = usb_alloc_coherent(udl->udev, MAX_TRANSFER, GFP_KERNEL, + buf = usb_alloc_coherent(udl->udev, size, GFP_KERNEL, &urb->transfer_dma); if (!buf) { kfree(unode); usb_free_urb(urb); + if (size > PAGE_SIZE) { + size /= 2; + udl_free_urb_list(dev); + goto retry; + } break; } @@ -241,16 +251,14 @@ static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size) list_add_tail(&unode->entry, &udl->urbs.list); - i++; + up(&udl->urbs.limit_sem); + udl->urbs.count++; + udl->urbs.available++; } - sema_init(&udl->urbs.limit_sem, i); - udl->urbs.count = i; - udl->urbs.available = i; - - DRM_DEBUG("allocated %d %d byte urbs\n", i, (int) size); + DRM_DEBUG("allocated %d %d byte urbs\n", udl->urbs.count, (int) size); - return i; + return udl->urbs.count; } struct urb *udl_get_urb(struct drm_device *dev) -- cgit v1.2.3 From 09a00abe3a9941c2715ca83eb88172cd2f54d8fd Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:40:57 +0200 Subject: udl-kms: fix crash due to uninitialized memory We must use kzalloc when allocating the fb_deferred_io structure. Otherwise, the field first_io is undefined and it causes a crash. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index d5583190f3e4..c3f5867dac91 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -221,7 +221,7 @@ static int udl_fb_open(struct fb_info *info, int user) struct fb_deferred_io *fbdefio; - fbdefio = kmalloc(sizeof(struct fb_deferred_io), GFP_KERNEL); + fbdefio = kzalloc(sizeof(struct fb_deferred_io), GFP_KERNEL); if (fbdefio) { fbdefio->delay = DL_DEFIO_WRITE_DELAY; -- cgit v1.2.3 From 91ba11fb7d7ca0a3bbe8a512e65e666e2ec1e889 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:41:00 +0200 Subject: udl-kms: avoid division Division is slow, so it shouldn't be done by the pixel generating code. The driver supports only 2 or 4 bytes per pixel, so we can replace division with a shift. Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_drv.h | 2 +- drivers/gpu/drm/udl/udl_fb.c | 15 +++++++++------ drivers/gpu/drm/udl/udl_transfer.c | 39 +++++++++++++++++++------------------- 3 files changed, 30 insertions(+), 26 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index 55c0cc309198..7588a9eb0ee0 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -112,7 +112,7 @@ udl_fb_user_fb_create(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd); -int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, +int udl_render_hline(struct drm_device *dev, 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 *ident_ptr, int *sent_ptr); diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index c3f5867dac91..8746eeeec44d 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -90,7 +90,10 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, int bytes_identical = 0; struct urb *urb; int aligned_x; - int bpp = fb->base.format->cpp[0]; + int log_bpp; + + BUG_ON(!is_power_of_2(fb->base.format->cpp[0])); + log_bpp = __ffs(fb->base.format->cpp[0]); if (!fb->active_16) return 0; @@ -125,12 +128,12 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, for (i = y; i < y + height ; i++) { const int line_offset = fb->base.pitches[0] * i; - const int byte_offset = line_offset + (x * bpp); - const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp); - if (udl_render_hline(dev, bpp, &urb, + const int byte_offset = line_offset + (x << log_bpp); + const int dev_byte_offset = (fb->base.width * i + x) << log_bpp; + if (udl_render_hline(dev, log_bpp, &urb, (char *) fb->obj->vmapping, &cmd, byte_offset, dev_byte_offset, - width * bpp, + width << log_bpp, &bytes_identical, &bytes_sent)) goto error; } @@ -149,7 +152,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, error: atomic_add(bytes_sent, &udl->bytes_sent); atomic_add(bytes_identical, &udl->bytes_identical); - atomic_add(width*height*bpp, &udl->bytes_rendered); + atomic_add((width * height) << log_bpp, &udl->bytes_rendered); end_cycles = get_cycles(); atomic_add(((unsigned int) ((end_cycles - start_cycles) >> 10)), /* Kcycles */ diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c index b992644c17e6..f3331d33547a 100644 --- a/drivers/gpu/drm/udl/udl_transfer.c +++ b/drivers/gpu/drm/udl/udl_transfer.c @@ -83,12 +83,12 @@ static inline u16 pixel32_to_be16(const uint32_t pixel) ((pixel >> 8) & 0xf800)); } -static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp) +static inline u16 get_pixel_val16(const uint8_t *pixel, int log_bpp) { - u16 pixel_val16 = 0; - if (bpp == 2) + u16 pixel_val16; + if (log_bpp == 1) pixel_val16 = *(const uint16_t *)pixel; - else if (bpp == 4) + else pixel_val16 = pixel32_to_be16(*(const uint32_t *)pixel); return pixel_val16; } @@ -125,8 +125,9 @@ static void udl_compress_hline16( const u8 *const pixel_end, uint32_t *device_address_ptr, uint8_t **command_buffer_ptr, - const uint8_t *const cmd_buffer_end, int bpp) + const uint8_t *const cmd_buffer_end, int log_bpp) { + const int bpp = 1 << log_bpp; const u8 *pixel = *pixel_start_ptr; uint32_t dev_addr = *device_address_ptr; uint8_t *cmd = *command_buffer_ptr; @@ -153,12 +154,12 @@ static void udl_compress_hline16( raw_pixels_count_byte = cmd++; /* we'll know this later */ raw_pixel_start = pixel; - cmd_pixel_end = pixel + min3(MAX_CMD_PIXELS + 1UL, - (unsigned long)(pixel_end - pixel) / bpp, - (unsigned long)(cmd_buffer_end - 1 - cmd) / 2) * bpp; + cmd_pixel_end = pixel + (min3(MAX_CMD_PIXELS + 1UL, + (unsigned long)(pixel_end - pixel) >> log_bpp, + (unsigned long)(cmd_buffer_end - 1 - cmd) / 2) << log_bpp); prefetch_range((void *) pixel, cmd_pixel_end - pixel); - pixel_val16 = get_pixel_val16(pixel, bpp); + pixel_val16 = get_pixel_val16(pixel, log_bpp); while (pixel < cmd_pixel_end) { const u8 *const start = pixel; @@ -170,7 +171,7 @@ static void udl_compress_hline16( pixel += bpp; while (pixel < cmd_pixel_end) { - pixel_val16 = get_pixel_val16(pixel, bpp); + pixel_val16 = get_pixel_val16(pixel, log_bpp); if (pixel_val16 != repeating_pixel_val16) break; pixel += bpp; @@ -179,10 +180,10 @@ static void udl_compress_hline16( if (unlikely(pixel > start + bpp)) { /* go back and fill in raw pixel count */ *raw_pixels_count_byte = (((start - - raw_pixel_start) / bpp) + 1) & 0xFF; + raw_pixel_start) >> log_bpp) + 1) & 0xFF; /* immediately after raw data is repeat byte */ - *cmd++ = (((pixel - start) / bpp) - 1) & 0xFF; + *cmd++ = (((pixel - start) >> log_bpp) - 1) & 0xFF; /* Then start another raw pixel span */ raw_pixel_start = pixel; @@ -192,14 +193,14 @@ static void udl_compress_hline16( if (pixel > raw_pixel_start) { /* finalize last RAW span */ - *raw_pixels_count_byte = ((pixel-raw_pixel_start) / bpp) & 0xFF; + *raw_pixels_count_byte = ((pixel - raw_pixel_start) >> log_bpp) & 0xFF; } else { /* undo unused byte */ cmd--; } - *cmd_pixels_count_byte = ((pixel - cmd_pixel_start) / bpp) & 0xFF; - dev_addr += ((pixel - cmd_pixel_start) / bpp) * 2; + *cmd_pixels_count_byte = ((pixel - cmd_pixel_start) >> log_bpp) & 0xFF; + dev_addr += ((pixel - cmd_pixel_start) >> log_bpp) * 2; } if (cmd_buffer_end <= MIN_RLX_CMD_BYTES + cmd) { @@ -222,19 +223,19 @@ 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 bpp, struct urb **urb_ptr, +int udl_render_hline(struct drm_device *dev, 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 *ident_ptr, int *sent_ptr) { const u8 *line_start, *line_end, *next_pixel; - u32 base16 = 0 + (device_byte_offset / bpp) * 2; + u32 base16 = 0 + (device_byte_offset >> log_bpp) * 2; struct urb *urb = *urb_ptr; u8 *cmd = *urb_buf_ptr; u8 *cmd_end = (u8 *) urb->transfer_buffer + urb->transfer_buffer_length; - BUG_ON(!(bpp == 2 || bpp == 4)); + BUG_ON(!(log_bpp == 1 || log_bpp == 2)); line_start = (u8 *) (front + byte_offset); next_pixel = line_start; @@ -244,7 +245,7 @@ int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, udl_compress_hline16(&next_pixel, line_end, &base16, - (u8 **) &cmd, (u8 *) cmd_end, bpp); + (u8 **) &cmd, (u8 *) cmd_end, log_bpp); if (cmd >= cmd_end) { int len = cmd - (u8 *) urb->transfer_buffer; -- cgit v1.2.3 From 58cba7c222e8c87bb6b61fcba21d1536145221b0 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:41:01 +0200 Subject: udl-kms: avoid prefetch Modern processors can detect linear memory accesses and prefetch data automatically, so there's no need to use prefetch. Signed-off-by: Mikulas Patocka Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_transfer.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c index f3331d33547a..ce87661e544f 100644 --- a/drivers/gpu/drm/udl/udl_transfer.c +++ b/drivers/gpu/drm/udl/udl_transfer.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -51,9 +50,6 @@ static int udl_trim_hline(const u8 *bback, const u8 **bfront, int *width_bytes) int start = width; int end = width; - prefetch((void *) front); - prefetch((void *) back); - for (j = 0; j < width; j++) { if (back[j] != front[j]) { start = j; @@ -140,8 +136,6 @@ static void udl_compress_hline16( const u8 *cmd_pixel_start, *cmd_pixel_end = NULL; uint16_t pixel_val16; - prefetchw((void *) cmd); /* pull in one cache line at least */ - *cmd++ = 0xaf; *cmd++ = 0x6b; *cmd++ = (uint8_t) ((dev_addr >> 16) & 0xFF); @@ -158,7 +152,6 @@ static void udl_compress_hline16( (unsigned long)(pixel_end - pixel) >> log_bpp, (unsigned long)(cmd_buffer_end - 1 - cmd) / 2) << log_bpp); - prefetch_range((void *) pixel, cmd_pixel_end - pixel); pixel_val16 = get_pixel_val16(pixel, log_bpp); while (pixel < cmd_pixel_end) { -- cgit v1.2.3 From c2f53119b410047e5d97de2a5ec74157d2e58986 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:41:02 +0200 Subject: udl-kms: use spin_lock_irq instead of spin_lock_irqsave spin_lock_irqsave and spin_unlock_irqrestore is inteded to be called from a context where it is unknown if interrupts are enabled or disabled (such as interrupt handlers). From a process context, we should call spin_lock_irq and spin_unlock_irq, that avoids the costly pushf and popf instructions. Signed-off-by: Mikulas Patocka Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_main.c | 10 ++++------ drivers/gpu/drm/udl/udl_modeset.c | 5 ++--- 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 7e9ad926926a..f455f095a146 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -170,7 +170,6 @@ static void udl_free_urb_list(struct drm_device *dev) struct list_head *node; struct urb_node *unode; struct urb *urb; - unsigned long flags; DRM_DEBUG("Waiting for completes and freeing all render urbs\n"); @@ -178,12 +177,12 @@ static void udl_free_urb_list(struct drm_device *dev) while (count--) { down(&udl->urbs.limit_sem); - spin_lock_irqsave(&udl->urbs.lock, flags); + spin_lock_irq(&udl->urbs.lock); node = udl->urbs.list.next; /* have reserved one with sem */ list_del_init(node); - spin_unlock_irqrestore(&udl->urbs.lock, flags); + spin_unlock_irq(&udl->urbs.lock); unode = list_entry(node, struct urb_node, entry); urb = unode->urb; @@ -268,7 +267,6 @@ struct urb *udl_get_urb(struct drm_device *dev) struct list_head *entry; struct urb_node *unode; struct urb *urb = NULL; - unsigned long flags; /* Wait for an in-flight buffer to complete and get re-queued */ ret = down_timeout(&udl->urbs.limit_sem, GET_URB_TIMEOUT); @@ -279,14 +277,14 @@ struct urb *udl_get_urb(struct drm_device *dev) goto error; } - spin_lock_irqsave(&udl->urbs.lock, flags); + spin_lock_irq(&udl->urbs.lock); BUG_ON(list_empty(&udl->urbs.list)); /* reserved one with limit_sem */ entry = udl->urbs.list.next; list_del_init(entry); udl->urbs.available--; - spin_unlock_irqrestore(&udl->urbs.lock, flags); + spin_unlock_irq(&udl->urbs.lock); unode = list_entry(entry, struct urb_node, entry); urb = unode->urb; diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index 5bcae7649795..98c6a58bcb07 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -366,7 +366,6 @@ static int udl_crtc_page_flip(struct drm_crtc *crtc, { struct udl_framebuffer *ufb = to_udl_fb(fb); struct drm_device *dev = crtc->dev; - unsigned long flags; struct drm_framebuffer *old_fb = crtc->primary->fb; if (old_fb) { @@ -377,10 +376,10 @@ static int udl_crtc_page_flip(struct drm_crtc *crtc, udl_handle_damage(ufb, 0, 0, fb->width, fb->height); - spin_lock_irqsave(&dev->event_lock, flags); + spin_lock_irq(&dev->event_lock); if (event) drm_crtc_send_vblank_event(crtc, event); - spin_unlock_irqrestore(&dev->event_lock, flags); + spin_unlock_irq(&dev->event_lock); crtc->primary->fb = fb; return 0; -- cgit v1.2.3 From 90991209837ab619555a46a97a88dead7a960d2d Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Sun, 3 Jun 2018 16:41:03 +0200 Subject: udl-kms: dont spam the syslog with debug messages The udl kms driver writes messages to the syslog whenever some application opens or closes /dev/fb0 and whenever the user switches between the Xserver and the console. This patch changes the priority of these messages to debug. Signed-off-by: Mikulas Patocka Signed-off-by: Dave Airlie --- drivers/gpu/drm/udl/udl_fb.c | 6 +++--- drivers/gpu/drm/udl/udl_modeset.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/udl') diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 8746eeeec44d..dbb62f6eb48a 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -178,7 +178,7 @@ static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) pos = (unsigned long)info->fix.smem_start + offset; - pr_notice("mmap() framebuffer addr:%lu size:%lu\n", + pr_debug("mmap() framebuffer addr:%lu size:%lu\n", pos, size); /* We don't want the framebuffer to be mapped encrypted */ @@ -236,7 +236,7 @@ static int udl_fb_open(struct fb_info *info, int user) } #endif - pr_notice("open /dev/fb%d user=%d fb_info=%p count=%d\n", + pr_debug("open /dev/fb%d user=%d fb_info=%p count=%d\n", info->node, user, info, ufbdev->fb_count); return 0; @@ -261,7 +261,7 @@ static int udl_fb_release(struct fb_info *info, int user) } #endif - pr_warn("released /dev/fb%d user=%d count=%d\n", + pr_debug("released /dev/fb%d user=%d count=%d\n", info->node, user, ufbdev->fb_count); return 0; diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index 98c6a58bcb07..7e37765cf5ac 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -243,7 +243,7 @@ static int udl_crtc_write_mode_to_hw(struct drm_crtc *crtc) memcpy(buf, udl->mode_buf, udl->mode_buf_len); retval = udl_submit_urb(dev, urb, udl->mode_buf_len); - DRM_INFO("write mode info %d\n", udl->mode_buf_len); + DRM_DEBUG("write mode info %d\n", udl->mode_buf_len); return retval; } -- cgit v1.2.3