USB DisplayLink patches

Hi

Here I'm sending bug fixes and performance improvements for the USB DisplayLink framebuffer and modesetting drivers for this merge window.

Mikulas

The displaylink hardware has such a peculiarity that it doesn't render a command until next command is received. This produces occasional corruption, such as when setting 22x11 font on the console, only the first line of the cursor will be blinking if the cursor is located at some specific columns.

When we end up with a repeating pixel, the driver has a bug that it leaves one uninitialized byte after the command (and this byte is enough to flush the command and render it - thus it fixes the screen corruption), however whe we end up with a non-repeating pixel, there is no byte appended and this results in temporary screen corruption.

This patch fixes the screen corruption by always appending a byte 0xAF at the end of URB. It also removes the uninitialized byte.

---
 drivers/gpu/drm/udl/udl_fb.c       |    5 ++++-
 drivers/gpu/drm/udl/udl_transfer.c |   11 +++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31 14:47:07.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31 14:53:31.000000000 +0200
@@ -153,11 +153,11 @@ static void udl_compress_hline16(
raw_pixels_count_byte = cmd++; /* we'll know this later */ raw_pixel_start = pixel;

- cmd_pixel_end = pixel + (min(MAX_CMD_PIXELS + 1,
- min((int)(pixel_end - pixel) / bpp,
- (int)(cmd_buffer_end - cmd) / 2))) * bpp;
+ cmd_pixel_end = pixel + min3(MAX_CMD_PIXELS + 1UL,
+ (unsigned long)(pixel_end - pixel) / bpp,
+ (unsigned long)(cmd_buffer_end - 1 - cmd) / 2) * bpp;

- prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+ prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
@@ -193,6 +193,9 @@ static void udl_compress_hline16(
if (pixel > raw_pixel_start) {
/* finalize last RAW span */ *raw_pixels_count_byte = ((pixel-raw_pixel_start) / bpp) & 0xFF;
+ } else {
+ /* undo unused byte */
+ cmd--;
}

*cmd_pixels_count_byte = ((pixel - cmd_pixel_start) / bpp) & 0xFF;
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_fb.c 2018-05-31 14:47:07.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c 2018-05-31 14:53:32.000000000 +0200
@@ -137,7 +137,10 @@ int udl_handle_damage(struct udl_framebu

if (cmd > (char *) urb->transfer_buffer) {
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ int len;
+ if (cmd < (char *) urb->transfer_buffer + urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
+ len = cmd - (char *) urb->transfer_buffer;
ret = udl_submit_urb(dev, urb, len); bytes_sent += len;
} else

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.

---
 drivers/gpu/drm/udl/udl_main.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31 10:23:42.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c 2018-05-31 10:28:11.000000000 +0200
@@ -170,18 +170,13 @@ static void udl_free_urb_list(struct drm
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);

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.

---
 drivers/gpu/drm/udl/udl_main.c |   28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31 11:16:15.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c 2018-05-31 11:16:15.000000000 +0200
@@ -200,17 +200,22 @@ static void udl_free_urb_list(struct drm
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
}
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

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)

We must use kzalloc when allocating the fb_deferred_io structure. Otherwise, the field first_io is undefined and it causes a crash.

---
 drivers/gpu/drm/udl/udl_fb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_fb.c 2018-05-29 17:55:39.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c 2018-05-29 17:55:39.000000000 +0200
@@ -221,7 +221,7 @@ static int udl_fb_open(struct fb_info *i

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;

The udl driver crashes when fbdefio is used. The crash can be reproduced with this sequence:
1. echo 1 >/sys/module/udl/parameters/fb_defio
2. run some program that maps the framebuffer, such as 'links -g' or 'fbi'
3. allocate memory to the point where the machine starts swapping

The reason for the crash is that udl_gem_get_pages calls drm_gem_get_pages and drm_gem_get_pages allocates the pages using shmem_read_mapping_page. The shmem pages are kept on the memory management lists using the page->lru entry.

However, fbdefio reuses the page->lru entry for the list of pages that were modified, so the memory management lists are corrupted and the machine crashes when vmscan starts to scan memory.

I fixed this crash by allocating pages with "alloc_page" instead. The pages allocated with "alloc_page" have page->lru unused, and thus the system doesn't crash when fbdefio uses it.

Unable to handle kernel paging request at virtual address dead000000000200

Mem abort info:
ESR = 0x96000044
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000044
CM = 0, WnR = 1
[dead000000000200] address between user and kernel address ranges

Internal error: Oops: 96000044 [#1] PREEMPT SMP
Modules linked in: ip6table_filter ip6_tables iptable_filter ip_tables x_tables af_packet autofs4 udl drm_kms_helper cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea fb font drm drm_panel_orientation_quirks mousedev hid_generic usbhid hid binfmt_misc snd_usb_audio snd_hwdep snd_usbmidi_lib snd_rawmidi snd_pcm snd_timer snd soundcore ipv6 aes_ce_blk crypto_simd cryptd aes_ce_cipher crc32_ce ghash_ce gf128mul aes_arm64 sha2_ce sha256_arm64 sha1_ce sha1_generic xhci_plat_hcd xhci_hcd sd_mod usbcore usb_common mvpp2 unix
CPU: 0 PID: 39 Comm: kswapd0 Not tainted 4.16.12 #3
Hardware name: Marvell 8040 MACHIATOBin (DT)
pstate: 00000085 (nzcv daIf -PAN -UAO)
pc : isolate_lru_pages.isra.16+0x23c/0x2b0
lr : isolate_lru_pages.isra.16+0x104/0x2b0

sp : ffffffc13a897ac0
x29: ffffffc13a897ac0 x28: 0000000000000003
x27: 0000000000000003 x26: 0000000000000004
x25: ffffff80087e84a0 x24: ffffffc13a897b68
x23: ffffffbf04cefc60 x22: ffffffc13a897e44
x21: 0000000000000009 x20: ffffffc13a897c00
x19: ffffffc13a897b40 x18: ffffffbf04d39000
x17: 00000000fffffff8 x16: ffffffbf00000000
x15: 0000000000000006 x14: 0000000000000000
x13: 00000000000001aa x12: 400000000004001c
x11: 0000000000000001 x10: 0000000000000001
x9 : 00000000000ee3ac x8 : 00000000000004a0
x7 : ffffff80087e84a0 x6 : 0000000000000000
x5 : 0000000000000000 x4 : 0000000000000002
x3 : ffffff80087e84a0 x2 : 0000000000000200
x1 : dead000000000200 x0 : 400000000004001c Process kswapd0 (pid: 39, stack limit = 0x0000000097f25571)

Call trace:
 isolate_lru_pages.isra.16+0x23c/0x2b0
 shrink_inactive_list+0xe4/0x3b0
 shrink_node_memcg.constprop.19+0x374/0x630
 shrink_node+0x64/0x1c8
 kswapd+0x340/0x568
 kthread+0x118/0x120
 ret_from_fork+0x10/0x18
Code: d2804002 f85e02e0 f85e02ec f9000461 (f9000023)
---[ end trace f9f3ad3856cb2ef3 ]---

note: kswapd0[39] exited with preempt_count 1

---
 drivers/gpu/drm/udl/udl_gem.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_gem.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_gem.c 2018-01-10 09:31:23.000000000 +0100
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_gem.c 2018-05-29 17:46:10.000000000 +0200
@@ -130,28 +130,51 @@ int udl_gem_fault(struct vm_fault *vmf)
int udl_gem_get_pages(struct udl_gem_object *obj) {
struct page **pages;
+ int npages, i;

if (obj->pages)
return 0;

- pages = drm_gem_get_pages(&obj->base);
- if (IS_ERR(pages))
- return PTR_ERR(pages);
+ npages = obj->base.size >> PAGE_SHIFT;
+
+ pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
+ if (!pages)
+ return -ENOMEM;
+
+ for (i = 0; i < npages; i++) {
+ struct page *p = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!p)
+ goto fail;
+ pages[i] = p;
+ }

obj->pages = pages;

return 0;
+
+fail:
+ while (i--)
+ put_page(pages[i]);
+ kvfree(pages);
+ return -ENOMEM;
}

void udl_gem_put_pages(struct udl_gem_object *obj) {
+ int npages, i;
+
if (obj->base.import_attach) {
kvfree(obj->pages); obj->pages = NULL;
return;
}

- drm_gem_put_pages(&obj->base, obj->pages, false, false);
+ npages = obj->base.size >> PAGE_SHIFT;
+
+ for (i = 0; i < npages; i++)
+ put_page(obj->pages[i]);
+
+ kvfree(obj->pages);
obj->pages = NULL;
}

The defio subsystem overwrites the method fb_osp->mmap. That method is stored in module's static data - and that means that if we have multiple diplaylink adapters, they will over write each other's method.

In order to avoid interference between multiple adapters, we copy the fb_ops structure to a device-local memory.

---
 drivers/gpu/drm/udl/udl_fb.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:05:20.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:08:03.000000000 +0200
@@ -34,6 +34,7 @@ module_param(fb_defio, int, S_IWUSR | S_
struct udl_fbdev {
struct drm_fb_helper helper;
struct udl_framebuffer ufb;
+ struct fb_ops fb_ops;
int fb_count;
};

@@ -405,7 +406,8 @@ static int udlfb_create(struct drm_fb_he
info->fix.smem_len = size;
info->fix.smem_start = (unsigned long)ufbdev->ufb.obj->vmapping;

- info->fbops = &udlfb_ops;
+ ufbdev->fb_ops = udlfb_ops;
+ info->fbops = &ufbdev->fb_ops;
drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth); drm_fb_helper_fill_var(info, &ufbdev->helper, sizes->fb_width, sizes->fb_height);

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.

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

Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_drv.h
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_drv.h 2018-06-03 13:15:01.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_drv.h 2018-06-03 13:15:01.000000000 +0200
@@ -110,7 +110,7 @@ udl_fb_user_fb_create(struct drm_device
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);
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:15:01.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:15:01.000000000 +0200
@@ -91,7 +91,10 @@ int udl_handle_damage(struct udl_framebu
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;
@@ -126,12 +129,12 @@ int udl_handle_damage(struct udl_framebu

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;
}
@@ -150,7 +153,7 @@ int udl_handle_damage(struct udl_framebu
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 */
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_transfer.c
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-06-03 13:15:01.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_transfer.c 2018-06-03 13:15:01.000000000 +0200
@@ -83,12 +83,12 @@ static inline u16 pixel32_to_be16(const
((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 *

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;

Modern processors can detect linear memory accesses and prefetch data automatically, so there's no need to use prefetch.

---
 drivers/gpu/drm/udl/udl_transfer.c |    7 -------
 1 file changed, 7 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31 14:48:12.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31 14:48:12.000000000 +0200
@@ -13,7 +13,6 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/fb.h>
-#include <linux/prefetch.h>
#include <asm/unaligned.h>

#include <drm/drmP.h>
@@ -51,9 +50,6 @@ static int udl_trim_hline(const u8 *bbac
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) {

SGkgTWlrdWxhcywNCg0KT24gU3VuLCAyMDE4LTA2LTAzIGF0IDE2OjQxICswMjAwLCBNaWt1bGFz IFBhdG9ja2Egd3JvdGU6DQo+IE1vZGVybiBwcm9jZXNzb3JzIGNhbiBkZXRlY3QgbGluZWFyIG1l bW9yeSBhY2Nlc3NlcyBhbmQgcHJlZmV0Y2ggZGF0YQ0KPiBhdXRvbWF0aWNhbGx5LCBzbyB0aGVy ZSdzIG5vIG5lZWQgdG8gdXNlIHByZWZldGNoLg0KDQpOb3QgZWFjaCBhbmQgZXZlcnkgQ1BVIHRo YXQncyBjYXBhYmxlIG9mIHJ1bm5pbmcgTGludXggaGFzIHByZWZldGNoDQpmdW5jdGlvbmFsaXR5 IDopDQoNClN0aWxsIHJlYWQtb24uLi4NCg0KPiBTaWduZWQtb2ZmLWJ5OiBNaWt1bGFzIFBhdG9j a2EgPG1wYXRvY2thQHJlZGhhdC5jb20+DQo+IA0KPiAtLS0NCj4gIGRyaXZlcnMvZ3B1L2RybS91 ZGwvdWRsX3RyYW5zZmVyLmMgfCAgICA3IC0tLS0tLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCA3IGRl bGV0aW9ucygtKQ0KPiANCj4gSW5kZXg6IGxpbnV4LTQuMTYuMTIvZHJpdmVycy9ncHUvZHJtL3Vk bC91ZGxfdHJhbnNmZXIuYw0KPiA9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQo+IC0tLSBsaW51eC00LjE2LjEyLm9yaWcv ZHJpdmVycy9ncHUvZHJtL3VkbC91ZGxfdHJhbnNmZXIuYwkyMDE4LTA1LTMxIDE0OjQ4OjEyLjAw MDAwMDAwMCArMDIwMA0KPiArKysgbGludXgtNC4xNi4xMi9kcml2ZXJzL2dwdS9kcm0vdWRsL3Vk bF90cmFuc2Zlci5jCTIwMTgtMDUtMzEgMTQ6NDg6MTIuMDAwMDAwMDAwICswMjAwDQo+IEBAIC0x Myw3ICsxMyw2IEBADQo+ICAjaW5jbHVkZSA8bGludXgvbW9kdWxlLmg+DQo+ICAjaW5jbHVkZSA8 bGludXgvc2xhYi5oPg0KPiAgI2luY2x1ZGUgPGxpbnV4L2ZiLmg+DQo+IC0jaW5jbHVkZSA8bGlu dXgvcHJlZmV0Y2guaD4NCj4gICNpbmNsdWRlIDxhc20vdW5hbGlnbmVkLmg+DQo+ICANCj4gICNp bmNsdWRlIDxkcm0vZHJtUC5oPg0KPiBAQCAtNTEsOSArNTAsNiBAQCBzdGF0aWMgaW50IHVkbF90 cmltX2hsaW5lKGNvbnN0IHU4ICpiYmFjDQo+ICAJaW50IHN0YXJ0ID0gd2lkdGg7DQo+ICAJaW50 IGVuZCA9IHdpZHRoOw0KPiAgDQo+IC0JcHJlZmV0Y2goKHZvaWQgKikgZnJvbnQpOw0KPiAtCXBy ZWZldGNoKCh2b2lkICopIGJhY2spOw0KDQpBRkFJSyBwcmVmZXRjaGVyIGZldGNoZXMgbmV3IGRh dGEgYWNjb3JkaW5nIHRvIGEga25vd24gaGlzdG9yeS4uLiBpLmUuIGJhc2VkIG9uIHByZXZpb3Vz bHkNCnVzZWQgcGF0dGVybiB3ZSdsbCB0cnlpbmcgdG8gZ2V0IHRoZSBuZXh0IGJhdGNoIG9mIGRh dGEuDQoNCkJ1dCB0aGUgY29kZSBhYm92ZSBpcyBpbiB0aGUgdmVyeSBiZWdpbm5pbmcgb2YgdGhl IGRhdGEgcHJvY2Vzc2luZyByb3V0aW5lIHdoZXJlDQpwcmVmZXRjaGVyIGRvZXNuJ3QgeWV0IGhh dmUgYW55IGhpc3RvcnkgdG8ga25vdyB3aGF0IGFuZCB3aGVyZSB0byBwcmVmZXRjaC4NCg0KU28g SSdkIHNheSB0aGlzIHBhcnRpY3VsYXIgdXNhZ2UgaXMgZ29vZC4NCkF0IGxlYXN0IHRob3NlIHBy ZWZldGNoZXMgc2hvdWxkbid0IGh1cnQgYmVjYXVzZSB0eXBpY2FsbHkgaXQNCndvdWxkIGJlIGp1 c3QgMSBpbnN0cnVjdGlvbiBpZiB0aG9zZSBleGlzdCBvciBub3RoaW5nIGlmIENQVS9jb21waWxl ciBkb2Vzbid0DQpzdXBwb3J0IGl0Lg0KDQo+ICAJZm9yIChqID0gMDsgaiA8IHdpZHRoOyBqKysp IHsNCj4gIAkJaWYgKGJhY2tbal0gIT0gZnJvbnRbal0pIHsNCj4gIAkJCXN0YXJ0ID0gajsNCj4g QEAgLTE0MCw4ICsxMzYsNiBAQCBzdGF0aWMgdm9pZCB1ZGxfY29tcHJlc3NfaGxpbmUxNigNCj4g IAkJY29uc3QgdTggKmNtZF9waXhlbF9zdGFydCwgKmNtZF9waXhlbF9lbmQgPSBOVUxMOw0KPiAg CQl1aW50MTZfdCBwaXhlbF92YWwxNjsNCj4gIA0KPiAtCQlwcmVmZXRjaHcoKHZvaWQgKikgY21k KTsgLyogcHVsbCBpbiBvbmUgY2FjaGUgbGluZSBhdCBsZWFzdCAqLw0KPiAtDQoNClByZXR0eSBt dWNoIHRoZSBzYW1lIGlzIGhlcmUuIE9idmlvdXNseSBvbiB0aGUgZmlyc3QgaXRlcmF0aW9uIHBy ZWZldGNoZXIgZG9zbid0DQprbm93IHdoZXJlIHRvIGdldCAiY21kIi4gT24gdGhlIG5leHQgaXRl cmF0aW9ucyBpdCBtaWdodCBiZSBiZXR0ZXIgYnV0IGdpdmVuIGFtb3VudA0Kb2Ygb3BlcmF0aW9u IGhhcHBlbnMgZnVydGhlciBpbiB0aGUgY3ljbGUgKGFuZCBpbiBpbm5lciBjeWNsZXMpIEkgd29u J3QgYmUgY29tcGxldGVseQ0Kc3VyZSB0aGF0IGVhY2ggYW5kIGV2ZXJ5IHByZWZldGNoZXIgd2ls bCBzdGlsbCBrZWVwIHRyYWNrIG9mICJjbWQiLg0KDQo+ICAJCSpjbWQrKyA9IDB4YWY7DQo+ICAJ CSpjbWQrKyA9IDB4NmI7DQo+ICAJCSpjbWQrKyA9ICh1aW50OF90KSAoKGRldl9hZGRyID4+IDE2 KSAmIDB4RkYpOw0KPiBAQCAtMTU4LDcgKzE1Miw2IEBAIHN0YXRpYyB2b2lkIHVkbF9jb21wcmVz c19obGluZTE2KA0KPiAgCQkJCQkodW5zaWduZWQgbG9uZykocGl4ZWxfZW5kIC0gcGl4ZWwpID4+ IGxvZ19icHAsDQo+ICAJCQkJCSh1bnNpZ25lZCBsb25nKShjbWRfYnVmZmVyX2VuZCAtIDEgLSBj bWQpIC8gMikgPDwgbG9nX2JwcCk7DQo+ICANCj4gLQkJcHJlZmV0Y2hfcmFuZ2UoKHZvaWQgKikg cGl4ZWwsIGNtZF9waXhlbF9lbmQgLSBwaXhlbCk7DQoNCkFnYWluIEknbSBub3Qgc3VyZSB3aGF0 IHdlIGdhaW4gcmVtb3ZpbmcgdGhhdCBjb2RlIGluIGNvbXBhcmlzb24gb2YgcG9zc2libGUNCnBl cmZvcm1hbmNlIGRlZ3JhZGF0aW9uIG9uIHNpbXBsZXIgQ1BVcy4NCg0KQW5kIGVzc2VudGlhbGx5 IGFsbCB0aGUgc2FtZSBpcyBhcHBsaWNhYmxlIHRvIFVETEZCIHBhdGNoLg0KDQotQWxleGV5

Tested this one on AT91SAM9G20 SoC, but couldn't get any measurable difference. Part of problem is probably that full speed USB is the bottleneck here. However, the same applies on OMAP3630 based board with high speed USB.

As a side note, I didn't experience any problem those paches are fixing, so perhaps testcases could be described briefly, preferably with some numbers (I'm not running console on udlfb, but try to give it a try next week).

Thank you,
ladis

See this post https://lwn.net/Articles/444336/ where they measured that prefetch hurts performance. Prefetch shouldn't be used unless you have a proof that it improves performance.

The problem is that the prefetch instruction causes stalls in the pipeline when it encounters TLB miss and the automatic prefetcher doesn't.

Mikulas

SGkgTWlrdWxhcywNCg0KT24gVHVlLCAyMDE4LTA2LTA1IGF0IDExOjMwIC0wNDAwLCBNaWt1bGFz IFBhdG9ja2Egd3JvdGU6DQo+IA0KPiBPbiBUdWUsIDUgSnVuIDIwMTgsIEFsZXhleSBCcm9ka2lu IHdyb3RlOg0KPiANCj4gPiBIaSBNaWt1bGFzLA0KPiA+IA0KPiA+IE9uIFN1biwgMjAxOC0wNi0w MyBhdCAxNjo0MSArMDIwMCwgTWlrdWxhcyBQYXRvY2thIHdyb3RlOg0KPiA+ID4gTW9kZXJuIHBy b2Nlc3NvcnMgY2FuIGRldGVjdCBsaW5lYXIgbWVtb3J5IGFjY2Vzc2VzIGFuZCBwcmVmZXRjaCBk YXRhDQo+ID4gPiBhdXRvbWF0aWNhbGx5LCBzbyB0aGVyZSdzIG5vIG5lZWQgdG8gdXNlIHByZWZl dGNoLg0KPiA+IA0KPiA+IE5vdCBlYWNoIGFuZCBldmVyeSBDUFUgdGhhdCdzIGNhcGFibGUgb2Yg cnVubmluZyBMaW51eCBoYXMgcHJlZmV0Y2gNCj4gPiBmdW5jdGlvbmFsaXR5IDopDQo+ID4gDQo+ ID4gU3RpbGwgcmVhZC1vbi4uLg0KPiA+IA0KPiA+ID4gU2lnbmVkLW9mZi1ieTogTWlrdWxhcyBQ YXRvY2thIDxtcGF0b2NrYUByZWRoYXQuY29tPg0KPiA+ID4gDQo+ID4gPiAtLS0NCj4gPiA+ICBk cml2ZXJzL2dwdS9kcm0vdWRsL3VkbF90cmFuc2Zlci5jIHwgICAgNyAtLS0tLS0tDQo+ID4gPiAg MSBmaWxlIGNoYW5nZWQsIDcgZGVsZXRpb25zKC0pDQo+ID4gPiANCj4gPiA+IEluZGV4OiBsaW51 eC00LjE2LjEyL2RyaXZlcnMvZ3B1L2RybS91ZGwvdWRsX3RyYW5zZmVyLmMNCj4gPiA+ID09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0NCj4gPiA+IC0tLSBsaW51eC00LjE2LjEyLm9yaWcvZHJpdmVycy9ncHUvZHJtL3VkbC91 ZGxfdHJhbnNmZXIuYwkyMDE4LTA1LTMxIDE0OjQ4OjEyLjAwMDAwMDAwMCArMDIwMA0KPiA+ID4g KysrIGxpbnV4LTQuMTYuMTIvZHJpdmVycy9ncHUvZHJtL3VkbC91ZGxfdHJhbnNmZXIuYwkyMDE4 LTA1LTMxIDE0OjQ4OjEyLjAwMDAwMDAwMCArMDIwMA0KPiA+ID4gQEAgLTEzLDcgKzEzLDYgQEAN Cj4gPiA+ICAjaW5jbHVkZSA8bGludXgvbW9kdWxlLmg+DQo+ID4gPiAgI2luY2x1ZGUgPGxpbnV4 L3NsYWIuaD4NCj4gPiA+ICAjaW5jbHVkZSA8bGludXgvZmIuaD4NCj4gPiA+IC0jaW5jbHVkZSA8 bGludXgvcHJlZmV0Y2guaD4NCj4gPiA+ICAjaW5jbHVkZSA8YXNtL3VuYWxpZ25lZC5oPg0KPiA+ ID4gIA0KPiA+ID4gICNpbmNsdWRlIDxkcm0vZHJtUC5oPg0KPiA+ID4gQEAgLTUxLDkgKzUwLDYg QEAgc3RhdGljIGludCB1ZGxfdHJpbV9obGluZShjb25zdCB1OCAqYmJhYw0KPiA+ID4gIAlpbnQg c3RhcnQgPSB3aWR0aDsNCj4gPiA+ICAJaW50IGVuZCA9IHdpZHRoOw0KPiA+ID4gIA0KPiA+ID4g LQlwcmVmZXRjaCgodm9pZCAqKSBmcm9udCk7DQo+ID4gPiAtCXByZWZldGNoKCh2b2lkICopIGJh Y2spOw0KPiA+IA0KPiA+IEFGQUlLIHByZWZldGNoZXIgZmV0Y2hlcyBuZXcgZGF0YSBhY2NvcmRp bmcgdG8gYSBrbm93biBoaXN0b3J5Li4uIGkuZS4gYmFzZWQgb24gcHJldmlvdXNseQ0KPiA+IHVz ZWQgcGF0dGVybiB3ZSdsbCB0cnlpbmcgdG8gZ2V0IHRoZSBuZXh0IGJhdGNoIG9mIGRhdGEuDQo+ ID4gDQo+ID4gQnV0IHRoZSBjb2RlIGFib3ZlIGlzIGluIHRoZSB2ZXJ5IGJlZ2lubmluZyBvZiB0 aGUgZGF0YSBwcm9jZXNzaW5nIHJvdXRpbmUgd2hlcmUNCj4gPiBwcmVmZXRjaGVyIGRvZXNuJ3Qg eWV0IGhhdmUgYW55IGhpc3RvcnkgdG8ga25vdyB3aGF0IGFuZCB3aGVyZSB0byBwcmVmZXRjaC4N Cj4gPiANCj4gPiBTbyBJJ2Qgc2F5IHRoaXMgcGFydGljdWxhciB1c2FnZSBpcyBnb29kLg0KPiA+ IEF0IGxlYXN0IHRob3NlIHByZWZldGNoZXMgc2hvdWxkbid0IGh1cnQgYmVjYXVzZSB0eXBpY2Fs bHkgaXQNCj4gPiB3b3VsZCBiZSBqdXN0IDEgaW5zdHJ1Y3Rpb24gaWYgdGhvc2UgZXhpc3Qgb3Ig bm90aGluZyBpZiBDUFUvY29tcGlsZXIgZG9lc24ndA0KPiA+IHN1cHBvcnQgaXQuDQo+IA0KPiBT ZWUgdGhpcyBwb3N0IGh0dHBzOi8vdXJsZGVmZW5zZS5wcm9vZnBvaW50LmNvbS92Mi91cmw/dT1o dHRwcy0zQV9fbHduLm5ldF9BcnRpY2xlc180NDQzMzZfJmQ9RHdJQkFnJmM9RFBMNl9YXzZKa1hG eDdBWFdxQjB0ZyZyPWxxZGVlU1NFZXMwR0ZERGw2NTZlDQo+IFZpWE83YnJlUzU1eXRXa2hwazVS ODFJJm09YTVSYXFKdHZhakZrTTFoTDdiT0tENWpWN2NwRmZUdkcyWTFjWUNkQlBkMCZzPXcwVzh3 RnRBZ0VOcDhURTZSemRQR2hkS1Jhc0pjX290SW4wOFYwRWtnclkmZT0gd2hlcmUgdGhleSBtZWFz dXJlZCB0aGF0IA0KPiBwcmVmZXRjaCBodXJ0cyBwZXJmb3JtYW5jZS4gUHJlZmV0Y2ggc2hvdWxk bid0IGJlIHVzZWQgdW5sZXNzIHlvdSBoYXZlIGEgDQo+IHByb29mIHRoYXQgaXQgaW1wcm92ZXMg cGVyZm9ybWFuY2UuDQo+IA0KPiBUaGUgcHJvYmxlbSBpcyB0aGF0IHRoZSBwcmVmZXRjaCBpbnN0 cnVjdGlvbiBjYXVzZXMgc3RhbGxzIGluIHRoZSBwaXBlbGluZSANCj4gd2hlbiBpdCBlbmNvdW50 ZXJzIFRMQiBtaXNzIGFuZCB0aGUgYXV0b21hdGljIHByZWZldGNoZXIgZG9lc24ndC4NCg0KV293 LCB0aGFua3MgZm9yIHRoZSBsaW5rLg0KSSBkaWRuJ3Qga25vdyBhYm91dCB0aGF0IHN1YnRsZSBp c3N1ZSB3aXRoIHByZWZldGNoIGluc3RydWN0aW9ucyBvbiBBUk0gYW5kIHg4Ni4NCg0KU28gT0sg aW4gY2FzZSBvZiBVREwgdGhlc2UgcHJlZmV0Y2hlcyBhbnl3YXlzIG1ha2Ugbm90IG5vdCBtdWNo IHNlbnNlIEkgZ3Vlc3MgYW5kIHRoZXJlJ3MNCnNvbWV0aGluZyB3b3JzZSBzdGlsbCwgc2VlIHdo YXQgSSd2ZSBnb3QgZnJvbSBXYW5kQm9hcmQgUXVhZCBydW5uaW5nIGttc2N1YmUgWzFdIGFwcGxp Y2F0aW9uDQp3aXRoIGhlbHAgb2YgcGVyZiB1dGlsaXR5Og0KLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tPjgtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQojIE92ZXJoZWFkICBDb21tYW5kICBT aGFyZWQgT2JqZWN0ICAgICAgICAgICAgU3ltYm9sIA0KIyAuLi4uLi4uLiAgLi4uLi4uLiAgLi4u Li4uLi4uLi4uLi4uLi4uLi4uLi4gIC4uLi4uLi4uLi4uLi4uLi4uLi4uLi4uLi4uLi4uLi4uLi4u Li4uLi4NCiMNCiAgICA5Mi45MyUgIGttc2N1YmUgIFtrZXJuZWwua2FsbHN5bXNdICAgICAgICBb a10gdWRsX3JlbmRlcl9obGluZQ0KICAgICAyLjUxJSAga21zY3ViZSAgW2tlcm5lbC5rYWxsc3lt c10gICAgICAgIFtrXSBfX2RpdnNpMw0KICAgICAwLjMzJSAga21zY3ViZSAgW2tlcm5lbC5rYWxs c3ltc10gICAgICAgIFtrXSBfcmF3X3NwaW5fdW5sb2NrX2lycXJlc3RvcmUNCiAgICAgMC4yMiUg IGttc2N1YmUgIFtrZXJuZWwua2FsbHN5bXNdICAgICAgICBba10gbG9ja19hY3F1aXJlDQogICAg IDAuMTklICBrbXNjdWJlICBba2VybmVsLmthbGxzeW1zXSAgICAgICAgW2tdIF9yYXdfc3Bpbl91 bmxvY2tfaXJxDQogICAgIDAuMTclICBrbXNjdWJlICBba2VybmVsLmthbGxzeW1zXSAgICAgICAg W2tdIHVkbF9oYW5kbGVfZGFtYWdlDQogICAgIDAuMTIlICBrbXNjdWJlICBba2VybmVsLmthbGxz eW1zXSAgICAgICAgW2tdIHY3X2RtYV9jbGVhbl9yYW5nZQ0KICAgICAwLjExJSAga21zY3ViZSAg W2tlcm5lbC5rYWxsc3ltc10gICAgICAgIFtrXSBsMmMyMTBfY2xlYW5fcmFuZ2UNCiAgICAgMC4w NiUgIGttc2N1YmUgIFtrZXJuZWwua2FsbHN5bXNdICAgICAgICBba10gX19tZW16ZXJvDQotLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0+OC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCg0KVGhh dCBzYWlkIGl0J3Mgbm90IGV2ZW4gVVNCIDIuMCB3aGljaCBpcyBhIGJvdHRsZS1uZWNrIGJ1dA0K Y29tcHV0YXRpb25zIGluIHRoZSB1ZGxfcmVuZGVyX2hsaW5lKCkuDQoNCg0KWzFdIGh0dHBzOi8v Y2dpdC5mcmVlZGVza3RvcC5vcmcvbWVzYS9rbXNjdWJlLw0KDQotQWxleGV5

Try this patch
http://people.redhat.com/~mpatocka/patches/kernel/udl/udlkms-avoid-division.patch

It is doing a lot of divisions - and WandBoard has Cortex-A9, that doesn't have division instruction.

BTW. the framebuffer UDL driver (not the modesetting driver) has performance counters in sysfs. Their location depends on the system, you can find them with find /sys -name "*metrics*"

The file "metrics_reset" resets the counters, so you can measure if the prefetch instructions improve performance or not.

Mikulas

Hi Mikulas,

Looks like that patch makes not that much of a difference.

Below are results of perf.

Without patch:
----------------------------->8-------------------------------------------

 91.46% kmscube  [kernel.kallsyms]        [k] udl_render_hline
 2.15%  kmscube  [kernel.kallsyms]        [k] __divsi3
 1.09%  kmscube  [kernel.kallsyms]        [k] mmioset
 0.48%  kmscube  [kernel.kallsyms]        [k] _raw_spin_unlock_irq
 0.48%  kmscube  [kernel.kallsyms]        [k] v7_dma_clean_range
 0.34%  kmscube  [kernel.kallsyms]        [k] _raw_spin_unlock_irqrestore
 0.28%  kmscube  [kernel.kallsyms]        [k] l2c210_clean_range
 0.25%  kmscube  [kernel.kallsyms]        [k] shmem_getpage_gfp.constprop.4
 0.18%  kmscube  [kernel.kallsyms]        [k] lock_acquire

With patch:
----------------------------->8-------------------------------------------

 94.81% kmscube  [kernel.kallsyms]        [k] udl_render_hline
 0.88%  kmscube  [kernel.kallsyms]        [k] mmioset
 0.44%  kmscube  [kernel.kallsyms]        [k] _raw_spin_unlock_irq
 0.38%  kmscube  [kernel.kallsyms]        [k] v7_dma_clean_range
 0.33%  kmscube  [kernel.kallsyms]        [k] _raw_spin_unlock_irqrestore
 0.22%  kmscube  [kernel.kallsyms]        [k] l2c210_clean_range
 0.16%  kmscube  [kernel.kallsyms]        [k] shmem_getpage_gfp.constprop.4
 0.16%  kmscube  [kernel.kallsyms]        [k] udl_handle_damage
 0.16%  kmscube  [kernel.kallsyms]        [k] lock_acquire

----------------------------->8-------------------------------------------

There is no mode __divsi3 function in perf results, so patch works. But this function was just ~2% of overall overhead.

-Alexey

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.

---
 drivers/gpu/drm/udl/udl_main.c    |   10 ++++------
 drivers/gpu/drm/udl/udl_modeset.c |    5 ++---
 2 files changed, 6 insertions(+), 9 deletions(-)

Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31 11:17:01.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c 2018-05-31 11:17:01.000000000 +0200
@@ -170,7 +170,6 @@ static void udl_free_urb_list(struct drm
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
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_devic
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_devic
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;
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_modeset.c
=================================--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_modeset.c 2018-05-31 11:17:01.000000000 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_modeset.c 2018-05-31 11:17:01.000000000 +0200
@@ -366,7 +366,6 @@ static int udl_crtc_page_flip(struct drm
{
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

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;

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.

---
 drivers/gpu/drm/udl/udl_fb.c      |    6 +++---
 drivers/gpu/drm/udl/udl_modeset.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:17:58.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:42:48.000000000 +0200
@@ -179,7 +179,7 @@ static int udl_fb_mmap(struct fb_info *i

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 */
@@ -237,7 +237,7 @@ static int udl_fb_open(struct fb_info *i
}
#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;
@@ -262,7 +262,7 @@ static int udl_fb_release(struct fb_info
}
#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;
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_modeset.c
=================================--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_modeset.c 2018-06-03 13:17:58.000000000 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_modeset.c 2018-06-03 13:41:56.000000000 +0200
@@ -243,7 +243,7 @@ static int udl_crtc_write_mode_to_hw(str

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;
}

I observed that the performance of the udl fb driver degrades over time. On a freshly booted machine, it takes 6 seconds to do "ls -la /usr/bin"; after some time of use, the same operation takes 14 seconds.

The reason is that the value of "limit_sem" decays over time.

The udl driver uses a semaphore "limit_set" to specify how many free urbs are there on dlfb->urbs.list. If the count is zero, the "down" operation will sleep until some urbs are added to the freelist.

In order to avoid some hypothetical deadlock, the driver will not call "up" immediatelly, but it will offload it to a workqueue. The problem is that if we call "schedule_delayed_work" on the same work item multiple times, the work item may only be executed once.

This is happening:
* some urb completes
* dlfb_urb_completion adds it to the free list
* dlfb_urb_completion calls schedule_delayed_work to schedule the function dlfb_release_urb_work to increase the semaphore count
* as the urb is on the free list, some other task grabs it and submits it
* the submitted urb completes, dlfb_urb_completion is called again
* dlfb_urb_completion calls schedule_delayed_work, but the work is already scheduled, so it does nothing
* finally, dlfb_release_urb_work is called, it increases the semaphore count by 1, although it should increase it by 2

So, the semaphore count is decreasing over time, and this causes gradual performance degradation.

Note that in the current kernel, the "up" function may be called from interrupt and it may race with the "down" function called by another thread, so we don't have to offload the call of "up" to a workqueue at all. This patch removes the workqueue code. The patch also changes "down_interruptible" to "down" in dlfb_free_urb_list, so that we will clean up the driver properly even if a signal arrives.

With this patch, the performance of udlfb no longer degrades.

---
 drivers/video/fbdev/udlfb.c |   27 ++-------------------------
 include/video/udlfb.h       |    1 -
 2 files changed, 2 insertions(+), 26 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 12:31:04.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 12:31:04.000000000 +0200
@@ -922,14 +922,6 @@ static void dlfb_free(struct kref *kref)
kfree(dlfb);
}

-static void dlfb_release_urb_work(struct work_struct *work) -{
- struct urb_node *unode = container_of(work, struct urb_node,
- release_urb_work.work);
-
- up(&unode->dlfb->urbs.limit_sem);
-}
-
static void dlfb_free_framebuffer(struct dlfb_data *dlfb) {
struct fb_info *info = dlfb->info;
@@ -1789,14 +1781,7 @@ static void dlfb_urb_completion(struct u
dlfb->urbs.available++; spin_unlock_irqrestore(&dlfb->urbs.lock, flags);

- /*
- * When using fb_defio, we deadlock if up() is called
- * while another is waiting. So queue to another process.
- */
- if (fb_defio)
- schedule_delayed_work(&unode->release_urb_work, 0);
- else
- up(&dlfb->urbs.limit_sem);
+ up(&dlfb->urbs.limit_sem);
}

static void dlfb_free_urb_list(struct dlfb_data *dlfb)
@@ -1805,16 +1790,11 @@ static void dlfb_free_urb_list(struct dl
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- int ret;
unsigned long flags;

/* keep waiting and freeing, until we've got 'em all */ while (count--) {
-
- /* Getting interrupted means a leak, but ok at disconnect */
- ret = down_interruptible(&dlfb->urbs.limit_sem);
- if (ret)
- break;
+ down(&dlfb->urbs.limit_sem);

spin_lock_irqsave(&dlfb->urbs.lock, flags);

@@ -1854,9 +1834,6 @@ static int dlfb_alloc_urb_list(struct dl
break;
unode->dlfb = dlfb;

- INIT_DELAYED_WORK(&unode->release_urb_work,
- dlfb_release_urb_work);
-
urb = usb_alloc_urb(0, GFP_KERNEL); if (!urb) {
kfree(unode);
Index: linux-4.17-rc7/include/video/udlfb.h
=================================--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-05-31 12:31:04.000000000 +0200
+++ linux-4.17-rc7/include/video/udlfb.h 2018-05-31 12:31:04.000000000 +0200
@@ -20,7 +20,6 @@ struct dloarea {
struct urb_node {
struct list_head entry;
struct dlfb_data *dlfb;
- struct delayed_work release_urb_work;
struct urb *urb;
};

The displaylink hardware has such a peculiarity that it doesn't render a command until next command is received. This produces occasional corruption, such as when setting 22x11 font on the console, only the first line of the cursor will be blinking if the cursor is located at some specific columns.

When we end up with a repeating pixel, the driver has a bug that it leaves one uninitialized byte after the command (and this byte is enough to flush the command and render it - thus it fixes the screen corruption), however whe we end up with a non-repeating pixel, there is no byte appended and this results in temporary screen corruption.

This patch fixes the screen corruption by always appending a byte 0xAF at the end of URB. It also removes the uninitialized byte.

---
 drivers/video/fbdev/udlfb.c |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 14:43:13.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:46:03.000000000 +0200
@@ -27,6 +27,7 @@
#include <linux/slab.h>
#include <linux/prefetch.h>
#include <linux/delay.h>
+#include <asm/unaligned.h>
#include <video/udlfb.h>
#include "edid.h"

@@ -450,17 +451,17 @@ static void dlfb_compress_hline(
raw_pixels_count_byte = cmd++; /* we'll know this later */ raw_pixel_start = pixel;

- cmd_pixel_end = pixel + min(MAX_CMD_PIXELS + 1,
- min((int)(pixel_end - pixel),
- (int)(cmd_buffer_end - cmd) / BPP));
+ cmd_pixel_end = pixel + min3(MAX_CMD_PIXELS + 1UL,
+ (unsigned long)(pixel_end - pixel),
+ (unsigned long)(cmd_buffer_end - 1 - cmd) / BPP);

- prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * BPP);
+ prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8 *)pixel);

while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel;

- *cmd++ = *pixel >> 8;
- *cmd++ = *pixel;
+ put_unaligned_be16(*pixel, cmd);
+ cmd += 2;
pixel++;

if (unlikely((pixel < cmd_pixel_end) &&
@@ -486,13 +487,16 @@ static void dlfb_compress_hline(
if (pixel > raw_pixel_start) {
/* finalize last RAW span */ *raw_pixels_count_byte = (pixel-raw_pixel_start) & 0xFF;
+ } else {
+ /* undo unused byte */
+ cmd--;
}

*cmd_pixels_count_byte = (pixel - cmd_pixel_start) & 0xFF;
- dev_addr += (pixel - cmd_pixel_start) * BPP;
+ dev_addr += (u8 *)pixel - (u8 *)cmd_pixel_start;
}

- if (cmd_buffer_end <= MIN_RLX_CMD_BYTES + cmd) {
+ if (cmd_buffer_end - MIN_RLX_CMD_BYTES <= cmd) {
/* Fill leftover bytes with no-ops */ if (cmd_buffer_end > cmd)
memset(cmd, 0xAF, cmd_buffer_end - cmd);
@@ -610,8 +614,11 @@ static int dlfb_handle_damage(struct dlf
}

if (cmd > (char *) urb->transfer_buffer) {
+ int len;
+ if (cmd < (char *) urb->transfer_buffer + urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ len = cmd - (char *) urb->transfer_buffer;
ret = dlfb_submit_urb(dlfb, urb, len); bytes_sent += len;
} else
@@ -735,8 +742,11 @@ static void dlfb_dpy_deferred_io(struct
}

if (cmd > (char *) urb->transfer_buffer) {
+ int len;
+ if (cmd < (char *) urb->transfer_buffer + urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ len = cmd - (char *) urb->transfer_buffer;
dlfb_submit_urb(dlfb, urb, len); bytes_sent += len;
} else

The udlfb driver reprograms the hardware everytime the user switches the console, that makes quite unusable when working on the console.

This patch makes the driver remember the videomode we are in and avoid reprogramming the hardware if we switch to the same videomode.

We mask the "activate" field and the "FB_VMODE_SMOOTH_XPAN" flag when comparing the videomode, because they cause spurious switches when switching to and from the Xserver.

---
 drivers/video/fbdev/udlfb.c |   18 ++++++++++++++++--
 include/video/udlfb.h       |    1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 14:49:52.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:49:52.000000000 +0200
@@ -1041,10 +1041,24 @@ static int dlfb_ops_set_par(struct fb_in
int result;
u16 *pix_framebuffer;
int i;
+ struct fb_var_screeninfo fvs;
+
+ /* clear the activate field because it causes spurious miscompares */
+ fvs = info->var;
+ fvs.activate = 0;
+ fvs.vmode &= ~FB_VMODE_SMOOTH_XPAN;
+
+ if (!memcmp(&dlfb->current_mode, &fvs, sizeof(struct fb_var_screeninfo)))
+ return 0;

result = dlfb_set_video_mode(dlfb, &info->var);

- if ((result = 0) && (dlfb->fb_count = 0)) {
+ if (result)
+ return result;
+
+ dlfb->current_mode = fvs;
+
+ if (dlfb->fb_count = 0) {

/* paint greenscreen */

@@ -1056,7 +1070,7 @@ static int dlfb_ops_set_par(struct fb_in
info->screen_base);
}

- return result;
+ return 0;
}

/* To fonzi the jukebox (e.g. make blanking changes take effect) */
Index: linux-4.17-rc7/include/video/udlfb.h
=================================--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-05-31 14:49:52.000000000 +0200
+++ linux-4.17-rc7/include/video/udlfb.h 2018-05-31 14:49:52.000000000 +0200
@@ -56,6 +56,7 @@ struct dlfb_data {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */ atomic_t bytes_sent; /* to usb, after compression including overhead */ atomic_t cpu_kcycles_used; /* transpired during pixel processing */
+ struct fb_var_screeninfo current_mode;
};

#define NR_USB_REQUEST_I2C_SUB_IO 0x02

The defio subsystem overwrites the method fb_osp->mmap. That method is stored in module's static data - and that means that if we have multiple diplaylink adapters, they will over write each other's method.

In order to avoid interference between multiple adapters, we copy the fb_ops structure to a device-local memory.

---
 drivers/video/fbdev/udlfb.c |    3 ++-
 include/video/udlfb.h       |    1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:33.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:33.000000000 +0200
@@ -1665,7 +1665,8 @@ static void dlfb_init_framebuffer_work(s
dlfb->info = info;
info->par = dlfb;
info->pseudo_palette = dlfb->pseudo_palette;
- info->fbops = &dlfb_ops;
+ dlfb->ops = dlfb_ops;
+ info->fbops = &dlfb->ops;

retval = fb_alloc_cmap(&info->cmap, 256, 0);
if (retval < 0) {
Index: linux-4.17-rc7/include/video/udlfb.h
=================================--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-06-03 13:17:33.000000000 +0200
+++ linux-4.17-rc7/include/video/udlfb.h 2018-06-03 13:17:33.000000000 +0200
@@ -51,6 +51,7 @@ struct dlfb_data {
int base8;
u32 pseudo_palette[256];
int blank_mode; /*one of FB_BLANK_ */
+ struct fb_ops ops;
/* blit-only rendering path metrics, exposed through sysfs */ atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */ atomic_t bytes_identical; /* saved effort with backbuffer comparison */

The default delay 5 jiffies is too much when the kernel is compiled with HZ0 - it results in jumpy cursor in Xwindow.

In order to find out the optimal delay, I benchmarked the driver on 1280x720x30fps video. I found out that with HZ00, 10ms is acceptable, but with HZ%0 or HZ00, we need 4ms, so that the video is played without any frame skips.

This patch changes the delay to this value.

---
 include/video/udlfb.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-4.17-rc7/include/video/udlfb.h
=================================--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-06-03 13:17:37.000000000 +0200
+++ linux-4.17-rc7/include/video/udlfb.h 2018-06-03 13:17:37.000000000 +0200
@@ -88,7 +88,7 @@ struct dlfb_data {
#define MIN_RAW_PIX_BYTES 2
#define MIN_RAW_CMD_BYTES (RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)

-#define DL_DEFIO_WRITE_DELAY    5 /* fb_deferred_io.delay in jiffies */
+#define DL_DEFIO_WRITE_DELAY    msecs_to_jiffies(HZ <= 300 ? 4 : 10) /* optimal value for 720p video */
 #define DL_DEFIO_WRITE_DISABLE  (HZ*60) /* "disable" with long delay */

/* remove these once align.h patch is taken into kernel */

Allocations larger than PAGE_ALLOC_COSTLY_ORDER are unreliable and they may fail anytime. This patch fixes the udlfb driver so that when a large alloactions fails, it tries to do multiple smaller allocations.

---
 drivers/video/fbdev/udlfb.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:38.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:38.000000000 +0200
@@ -1843,17 +1843,22 @@ static void dlfb_free_urb_list(struct dl

static int dlfb_alloc_urb_list(struct dlfb_data *dlfb, int count, size_t size) {
- int i = 0;
struct urb *urb;
struct urb_node *unode;
char *buf;
+ size_t wanted_size = count * size;

spin_lock_init(&dlfb->urbs.lock);

+retry:
dlfb->urbs.size = size;
INIT_LIST_HEAD(&dlfb->urbs.list);

- while (i < count) {
+ sema_init(&dlfb->urbs.limit_sem, 0);
+ dlfb->urbs.count = 0;
+ dlfb->urbs.available = 0;
+
+ while (dlfb->urbs.count * size < wanted_size) {
unode = kzalloc(sizeof(*unode), GFP_KERNEL); if (!unode)
break;
@@ -1866,11 +1871,16 @@ static int dlfb_alloc_urb_list(struct dl
}
unode->urb = urb;

- buf = usb_alloc_coherent(dlfb->udev, MAX_TRANSFER, GFP_KERNEL,
+ buf = usb_alloc_coherent(dlfb->udev, size, GFP_KERNEL,
&urb->transfer_dma);
if (!buf) {
kfree(unode); usb_free_urb(urb);
+ if (size > PAGE_SIZE) {
+ size /= 2;
+ dlfb_free_urb_list(dlfb);
+ goto retry;
+ }
break;
}

@@ -1881,14 +1891,12 @@ static int dlfb_alloc_urb_list(struct dl

list_add_tail(&unode->entry, &dlfb->urbs.list);

- i++;
+ up(&dlfb->urbs.limit_sem);
+ dlfb->urbs.count++;
+ dlfb->urbs.available++;
}

- sema_init(&dlfb->urbs.limit_sem, i);
- dlfb->urbs.count = i;
- dlfb->urbs.available = i;
-
- return i;
+ return dlfb->urbs.count;
}

static struct urb *dlfb_get_urb(struct dlfb_data *dlfb)

Set the variable "line_length" in the function dlfb_ops_set_par. Without this, we get garbage if we select different videomode with fbset.

---
 drivers/video/fbdev/udlfb.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 14:50:00.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:50:00.000000000 +0200
@@ -1057,6 +1057,7 @@ static int dlfb_ops_set_par(struct fb_in
return result;

dlfb->current_mode = fvs;
+ info->fix.line_length = info->var.xres * (info->var.bits_per_pixel / 8);

if (dlfb->fb_count = 0) {

This patch changes udlfb so that it may reallocate the framebuffer when setting higher-resolution mode. If we boot the system without monitor attached, udlfb creates a framebuffer with the size 800x600. This patch makes it possible to select higher videomode with the fbset command when a monitor is attached.

Note that there is no reliable way to prevent the system from touching the old framebuffer, so we must not free it. We add it to the list dlfb->deferred_free and free it when the driver is unloaded.

---
 drivers/video/fbdev/udlfb.c |   70 +++++++++++++++++++++++++++++---------------
 include/video/udlfb.h       |    1 
 2 files changed, 48 insertions(+), 23 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:41.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:41.000000000 +0200
@@ -73,6 +73,13 @@ static bool fb_defio = 1; /* Detect mma
static bool shadow = 1; /* Optionally disable shadow framebuffer */ static int pixel_limit; /* Optionally force a pixel resolution limit */

+struct dlfb_deferred_free {
+ struct list_head list;
+ void *mem;
+};
+
+static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info *info, u32 new_len); +
/* dlfb keeps a list of urbs for efficient bulk transfers */
static void dlfb_urb_completion(struct urb *urb);
static struct urb *dlfb_get_urb(struct dlfb_data *dlfb);
@@ -927,6 +934,12 @@ static void dlfb_free(struct kref *kref)
{
struct dlfb_data *dlfb = container_of(kref, struct dlfb_data, kref);

+ while (!list_empty(&dlfb->deferred_free)) {
+ struct dlfb_deferred_free *d = list_entry(dlfb->deferred_free.next, struct dlfb_deferred_free, list);
+ list_del(&d->list);
+ vfree(d->mem);
+ kfree(d);
+ }
vfree(dlfb->backing_buffer); kfree(dlfb->edid);
kfree(dlfb);
@@ -1020,10 +1033,6 @@ static int dlfb_ops_check_var(struct fb_
struct fb_videomode mode;
struct dlfb_data *dlfb = info->par;

- /* TODO: support dynamically changing framebuffer size */
- if ((var->xres * var->yres * 2) > info->fix.smem_len)
- return -EINVAL;
-
/* set device-specific elements of var unrelated to mode */ dlfb_var_color_format(var);

@@ -1042,6 +1051,7 @@ static int dlfb_ops_set_par(struct fb_in
u16 *pix_framebuffer;
int i;
struct fb_var_screeninfo fvs;
+ u32 line_length = info->var.xres * (info->var.bits_per_pixel / 8);

/* clear the activate field because it causes spurious miscompares */ fvs = info->var;
@@ -1051,13 +1061,17 @@ static int dlfb_ops_set_par(struct fb_in
if (!memcmp(&dlfb->current_mode, &fvs, sizeof(struct fb_var_screeninfo)))
return 0;

+ result = dlfb_realloc_framebuffer(dlfb, info, info->var.yres * line_length);
+ if (result)
+ return result;
+
result = dlfb_set_video_mode(dlfb, &info->var);

if (result)
return result;

dlfb->current_mode = fvs;
- info->fix.line_length = info->var.xres * (info->var.bits_per_pixel / 8);
+ info->fix.line_length = line_length;

if (dlfb->fb_count = 0) {

@@ -1066,11 +1080,11 @@ static int dlfb_ops_set_par(struct fb_in
pix_framebuffer = (u16 *) info->screen_base; for (i = 0; i < info->fix.smem_len / 2; i++)
pix_framebuffer[i] = 0x37e6;
-
- dlfb_handle_damage(dlfb, 0, 0, info->var.xres, info->var.yres,
- info->screen_base);
}

+ dlfb_handle_damage(dlfb, 0, 0, info->var.xres, info->var.yres,
+ info->screen_base);
+
return 0;
}

@@ -1146,21 +1160,29 @@ static struct fb_ops dlfb_ops = {
};

+static void dlfb_deferred_vfree(struct dlfb_data *dlfb, void *mem)
+{
+ struct dlfb_deferred_free *d = kmalloc(sizeof(struct dlfb_deferred_free), GFP_KERNEL);
+ if (!d)
+ return;
+ d->mem = mem;
+ list_add(&d->list, &dlfb->deferred_free);
+}
+
/*
* Assumes &info->lock held by caller
* Assumes no active clients have framebuffer open
*/
-static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info *info) +static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info *info, u32 new_len)
{
- int old_len = info->fix.smem_len;
- int new_len;
+ u32 old_len = info->fix.smem_len;
unsigned char *old_fb = info->screen_base;
unsigned char *new_fb;
unsigned char *new_back = NULL;

- new_len = info->fix.line_length * info->var.yres;
+ new_len = PAGE_ALIGN(new_len);

- if (PAGE_ALIGN(new_len) > old_len) {
+ if (new_len > old_len) {
/*
* Alloc system memory for virtual framebuffer
*/
@@ -1169,14 +1191,15 @@ static int dlfb_realloc_framebuffer(stru
dev_err(info->dev, "Virtual framebuffer alloc failed\n"); return -ENOMEM;
}
+ memset(new_fb, 0xff, new_len);

if (info->screen_base) {
memcpy(new_fb, old_fb, old_len);
- vfree(info->screen_base);
+ dlfb_deferred_vfree(dlfb, info->screen_base);
}

info->screen_base = new_fb;
- info->fix.smem_len = PAGE_ALIGN(new_len);
+ info->fix.smem_len = new_len;
info->fix.smem_start = (unsigned long) new_fb; info->flags = udlfb_info_flags;

@@ -1192,7 +1215,7 @@ static int dlfb_realloc_framebuffer(stru
dev_info(info->dev,
"No shadow/backing buffer allocated\n");
else {
- vfree(dlfb->backing_buffer);
+ dlfb_deferred_vfree(dlfb, dlfb->backing_buffer);
dlfb->backing_buffer = new_back;
}
}
@@ -1344,11 +1367,6 @@ static int dlfb_setup_modes(struct dlfb_
* with mode size info, we can now alloc our framebuffer. */
memcpy(&info->fix, &dlfb_fix, sizeof(dlfb_fix));
- info->fix.line_length = info->var.xres *
- (info->var.bits_per_pixel / 8);
-
- result = dlfb_realloc_framebuffer(dlfb, info);
-
} else
result = -EINVAL;

@@ -1436,7 +1454,10 @@ static ssize_t edid_store(
if (!dlfb->edid || memcmp(src, dlfb->edid, src_size))
return -EINVAL;

- dlfb_ops_set_par(fb_info);
+ ret = dlfb_ops_set_par(fb_info);
+ if (ret)
+ return ret;
+
return src_size;
}

@@ -1596,6 +1617,7 @@ static int dlfb_usb_probe(struct usb_int
}

kref_init(&dlfb->kref); /* matching kref_put in usb .disconnect fn */
+ INIT_LIST_HEAD(&dlfb->deferred_free);

dlfb->udev = usbdev;
usb_set_intfdata(intf, dlfb);
@@ -1693,7 +1715,9 @@ static void dlfb_init_framebuffer_work(s
dlfb_select_std_channel(dlfb);

dlfb_ops_check_var(&info->var, info);
- dlfb_ops_set_par(info);
+ retval = dlfb_ops_set_par(info);
+ if (retval)
+ goto error;

retval = register_framebuffer(info);
if (retval < 0) {
Index: linux-4.17-rc7/include/video/udlfb.h
=================================--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-06-03 13:17:41.000000000 +0200
+++ linux-4.17-rc7/include/video/udlfb.h 2018-06-03 13:17:41.000000000 +0200
@@ -58,6 +58,7 @@ struct dlfb_data {
atomic_t bytes_sent; /* to usb, after compression including overhead */ atomic_t cpu_kcycles_used; /* transpired during pixel processing */ struct fb_var_screeninfo current_mode;
+ struct list_head deferred_free;
};

#define NR_USB_REQUEST_I2C_SUB_IO 0x02

What is it really complaining about? That URL shows 404 Not Found and this email has no warnings at all.

screen_base in struct fb_info is annotated with __iomem tag:

...
char __iomem *screen_base; /* Virtual address */
...

and this tag should be preserved (or explicitly casted).

You should be able to reproduce the issue with the above sequence.

[...]

[...]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Currently, the udlfb driver only tests for identical bytes at the beginning or at the end of a page and renders anything between the first and last mismatching pixel. But pages are not the same as lines, so this is quite suboptimal - if there is something modified at the beginning of a page and at the end of a page, the whole page is rendered, even if most of the page is not modified.

This patch makes it test for identical pixels at the beginning and end of each rendering command. This patch improves identical byte detection by 41% when playing video in a window.

This patch also fixes a possible screen corruption if the user is writing to the framebuffer while dlfb_render_hline is in progress - the pixel data that is copied to the backbuffer with memcpy may be different from the pixel data that is actually rendered to the hardware (because the content of the framebuffer may change between memcpy and the rendering command). We must make sure that we copy exactly the same pixel as the pixel that is being rendered.

---
 drivers/video/fbdev/udlfb.c |   45 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 14:51:43.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:51:43.000000000 +0200
@@ -431,7 +431,9 @@ static void dlfb_compress_hline(
const uint16_t *const pixel_end,
uint32_t *device_address_ptr,
uint8_t **command_buffer_ptr,
- const uint8_t *const cmd_buffer_end)
+ const uint8_t *const cmd_buffer_end,
+ unsigned long back_buffer_offset,
+ int *ident_ptr)
{
const uint16_t *pixel = *pixel_start_ptr;
uint32_t dev_addr = *device_address_ptr;
@@ -444,6 +446,14 @@ static void dlfb_compress_hline(
const uint16_t *raw_pixel_start = NULL; const uint16_t *cmd_pixel_start, *cmd_pixel_end = NULL;

+ if (back_buffer_offset &&
+ *pixel = *(u16 *)((u8 *)pixel + back_buffer_offset)) {
+ pixel++;
+ dev_addr += BPP;
+ (*ident_ptr)++;
+ continue;
+ }
+
prefetchw((void *) cmd); /* pull in one cache line at least */

*cmd++ = 0xAF;
@@ -462,25 +472,37 @@ static void dlfb_compress_hline(
(unsigned long)(pixel_end - pixel), (unsigned long)(cmd_buffer_end - 1 - cmd) / BPP);

+ if (back_buffer_offset) {
+ /* note: the framebuffer may change under us, so we must test for underflow */
+ while (cmd_pixel_end - 1 > pixel &&
+ *(cmd_pixel_end - 1) = *(u16 *)((u8 *)(cmd_pixel_end - 1) + back_buffer_offset))
+ cmd_pixel_end--;
+ }
+
prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8 *)pixel);

while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel;
+ u16 pixel_value = *pixel;

- put_unaligned_be16(*pixel, cmd);
+ put_unaligned_be16(pixel_value, cmd);
+ if (back_buffer_offset)
+ *(u16 *)((u8 *)pixel + back_buffer_offset) = pixel_value;
cmd += 2;
pixel++;

if (unlikely((pixel < cmd_pixel_end) &&
- (*pixel = *repeating_pixel))) {
+ (*pixel = pixel_value))) {
/* go back and fill in raw pixel count */ *raw_pixels_count_byte = ((repeating_pixel -
raw_pixel_start) + 1) & 0xFF;

- while ((pixel < cmd_pixel_end)
- && (*pixel = *repeating_pixel)) {
- pixel++;
- }
+ do {
+ if (back_buffer_offset)
+ *(u16 *)((u8 *)pixel + back_buffer_offset) = pixel_value;
+ pixel++;
+ } while ((pixel < cmd_pixel_end) &&
+ (*pixel = pixel_value));

/* immediately after raw data is repeat byte */ *cmd++ = ((pixel - repeating_pixel) - 1) & 0xFF;
@@ -531,6 +553,7 @@ static int dlfb_render_hline(struct dlfb
struct urb *urb = *urb_ptr;
u8 *cmd = *urb_buf_ptr;
u8 *cmd_end = (u8 *) urb->transfer_buffer + urb->transfer_buffer_length;
+ unsigned long back_buffer_offset = 0;

line_start = (u8 *) (front + byte_offset); next_pixel = line_start;
@@ -541,6 +564,8 @@ static int dlfb_render_hline(struct dlfb
const u8 *back_start = (u8 *) (dlfb->backing_buffer
+ byte_offset);

+ back_buffer_offset = (unsigned long)back_start - (unsigned long)line_start;
+
*ident_ptr += dlfb_trim_hline(back_start, &next_pixel,
&byte_width);

@@ -549,16 +574,14 @@ static int dlfb_render_hline(struct dlfb
dev_addr += offset;
back_start += offset;
line_start += offset;
-
- memcpy((char *)back_start, (char *) line_start,
- byte_width);
}

while (next_pixel < line_end) {

dlfb_compress_hline((const uint16_t **) &next_pixel,
(const uint16_t *) line_end, &dev_addr,
- (u8 **) &cmd, (u8 *) cmd_end);
+ (u8 **) &cmd, (u8 *) cmd_end, back_buffer_offset,
+ ident_ptr);

if (cmd >= cmd_end) {
int len = cmd - (u8 *) urb->transfer_buffer;

Modern processors can detect linear memory accesses and prefetch data automatically, so there's no need to use prefetch.

---
 drivers/video/fbdev/udlfb.c |    8 --------
 1 file changed, 8 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31 12:48:35.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 12:48:35.000000000 +0200
@@ -25,7 +25,6 @@
#include <linux/fb.h>
#include <linux/vmalloc.h>
#include <linux/slab.h>
-#include <linux/prefetch.h>
#include <linux/delay.h>
#include <asm/unaligned.h>
#include <video/udlfb.h>
@@ -375,9 +374,6 @@ static int dlfb_trim_hline(const u8 *bba
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;
@@ -454,8 +450,6 @@ static void dlfb_compress_hline(
continue;
}

- prefetchw((void *) cmd); /* pull in one cache line at least */
-
*cmd++ = 0xAF;
*cmd++ = 0x6B;
*cmd++ = dev_addr >> 16;
@@ -479,8 +473,6 @@ static void dlfb_compress_hline(
cmd_pixel_end--;
}

- prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8 *)pixel);
-
while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel; u16 pixel_value = *pixel;

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.

---
 drivers/video/fbdev/udlfb.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
=================================--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:46.000000000 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:46.000000000 +0200
@@ -1855,18 +1855,17 @@ static void dlfb_free_urb_list(struct dl
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- unsigned long flags;

/* keep waiting and freeing, until we've got 'em all */ while (count--) {
down(&dlfb->urbs.limit_sem);

- spin_lock_irqsave(&dlfb->urbs.lock, flags);
+ spin_lock_irq(&dlfb->urbs.lock);

node = dlfb->urbs.list.next; /* have reserved one with sem */ list_del_init(node);

- spin_unlock_irqrestore(&dlfb->urbs.lock, flags);
+ spin_unlock_irq(&dlfb->urbs.lock);

unode = list_entry(node, struct urb_node, entry); urb = unode->urb;
@@ -1944,7 +1943,6 @@ static struct urb *dlfb_get_urb(struct d
int ret;
struct list_head *entry;
struct urb_node *unode;
- unsigned long flags;

/* Wait for an in-flight buffer to complete and get re-queued */ ret = down_timeout(&dlfb->urbs.limit_sem, GET_URB_TIMEOUT);
@@ -1956,14 +1954,14 @@ static struct urb *dlfb_get_urb(struct d
return NULL;
}

- spin_lock_irqsave(&dlfb->urbs.lock, flags);
+ spin_lock_irq(&dlfb->urbs.lock);

BUG_ON(list_empty(&dlfb->urbs.list)); /* reserved one with limit_sem */ entry = dlfb->urbs.list.next;
list_del_init(entry);
dlfb->urbs.available--;

- spin_unlock_irqrestore(&dlfb->urbs.lock, flags);
+ spin_unlock_irq(&dlfb->urbs.lock);

unode = list_entry(entry, struct urb_node, entry); return unode->urb;

Hi,

You probably want to split these up into separate series for the kms and fbdev drivers.

Otherwise at least for drm you've missed this merge window, since it closes around rc6 of the previous kernel, did you use git send-email for these patches, at least some of them viewed funny on my phone, I'll try and look over the kms ones soon. Do you have any numbers for improvements to the kms ones?

Dave.

Could you apply at least the fbdefio patches (without them, fbdefio is unusable due to crashes) and the display corruption of the last line (because most people will hit it)?

I used the command "quilt mail". I use quilt, not git, for management of my patches.

I measured performance improvement on the framebuffer patches. The kms driver already performs well, there's not much to do.

I'd like to as you if you could review the patch "udl-kms: fix a linked-list corruption when using fbdefio" - for me it fixes the crashes, but I am not expert in modesetting drivers and I don't know if some other part of the kernel assumes that the framebuffer pages must be allocated with drm_gem_get_pages.

BTW. When I unplug the USB adapter while using the modesetting driver, I get this warning. Do you have an idea how to fix it?

WARNING: CPU: 0 PID: 61 at drivers/gpu/drm/drm_mode_config.c:439 drm_mode_config_cleanup+0x250/0x2b8 [drm]
Modules linked in: udlfb hid_generic usbhid hid tun bridge stp llc autofs4 binfmt_misc ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables ipt_REJECT nf_reject_ipv4 xt_conntrack xt_multiport iptable_filter iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_nat xt_tcpudp iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 ip_tables x_tables pppoe pppox af_packet ppp_generic slhc udl drm_kms_helper cfbfillrect cfbimgblt cfbcopyarea drm drm_panel_orientation_quirks syscopyarea sysfillrect sysimgblt fb_sys_fops fb font snd_usb_audio snd_hwdep snd_usbmidi_lib snd_rawmidi snd_pcm snd_timer snd soundcore nf_nat_ftp nf_conntrack_ftp nf_nat nf_conntrack sd_mod ipv6 aes_ce_blk crypto_simd cryptd aes_ce_cipher crc32_ce ghash_ce gf128mul aes_arm64 sha2_ce
 sha256_arm64 sha1_ce xhci_plat_hcd xhci_hcd sha1_generic usbcore usb_common ahci_platform libahci_platform libahci mvpp2 unix
CPU: 0 PID: 61 Comm: kworker/0:2 Not tainted 4.17.0-rc7 #1
Hardware name: Marvell 8040 MACCHIATOBin (DT)
Workqueue: usb_hub_wq hub_event [usbcore]
pstate: 80000005 (Nzcv daif -PAN -UAO)
pc : drm_mode_config_cleanup+0x250/0x2b8 [drm]
lr : drm_mode_config_cleanup+0x88/0x2b8 [drm]

sp : ffffffc13a643920
x29: ffffffc13a643920 x28: ffffffc13a63ac00
x27: ffffffc1380962c0 x26: ffffffc11b95f898
x25: ffffff8000afd1d8 x24: ffffffc11b95f800
x23: 0000000000000060 x22: ffffff8000afd240
x21: ffffffc11b95eb38 x20: ffffffc11b95e800
x19: ffffffc11b95eb30 x18: ffffffc11b95ea7c
x17: 0000007fa3591b60 x16: ffffff80081dc178
x15: ffffffc11b95ea78 x14: 0000000000000000
x13: ffffffc12b3fc000 x12: ffffffc12b3fc028
x11: ffffffc12b3fc119 x10: 000000000000001f
x9 : 0000000000000028 x8 : ffffff8000a84000
x7 : 0000000000000000 x6 : 0000000000000001
x5 : 0000000000000002 x4 : 0000000000000001
x3 : 0000000000000002 x2 : 000000000000002f
x1 : ffffffc11b95eaf8 x0 : ffffffc11b978818

Call trace:
 drm_mode_config_cleanup+0x250/0x2b8 [drm]
 udl_modeset_cleanup+0xc/0x18 [udl]
 udl_driver_unload+0x30/0x50 [udl]
 drm_dev_unregister+0x3c/0xe8 [drm]
 drm_dev_unplug+0x18/0x70 [drm]
 udl_usb_disconnect+0x30/0x40 [udl]
 usb_unbind_interface+0x6c/0x290 [usbcore]
 device_release_driver_internal+0x170/0x200
 device_release_driver+0x14/0x20
 bus_remove_device+0x118/0x128
 device_del+0x110/0x308
 usb_disable_device+0x8c/0x1f8 [usbcore]
 usb_disconnect+0xb4/0x218 [usbcore]
 usb_disconnect+0x9c/0x218 [usbcore]
 usb_disconnect+0x9c/0x218 [usbcore]
 hub_event+0xf20/0x1020 [usbcore]
 process_one_work+0x1c8/0x310
 worker_thread+0x44/0x450
 kthread+0x118/0x120
 ret_from_fork+0x10/0x18
---[ end trace 978a27ff198f1268 ]---

[drm:drm_mode_config_cleanup [drm]] *ERROR* connector DVI-I-1 leaked!

Mikulas

Probably the driver is missing a proper shutdown call (for atomic drivers this would be drm_atomic_helper_shutdown), leaving the connector active, which leaves it's reference count elevated. Or something like that.

Aside: Should we maintain uld as part of drm-misc under the small drivers topic? Or is this patch pile here more a one-shot effort? See

https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html#small-drivers

Cheers, Daniel

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

SGkgTWlrdWxhcywNCg0KT24gU3VuLCAyMDE4LTA2LTAzIGF0IDE2OjQwICswMjAwLCBNaWt1bGFz IFBhdG9ja2Egd3JvdGU6DQo+IEhpDQo+IA0KPiBIZXJlIEknbSBzZW5kaW5nIGJ1ZyBmaXhlcyBh bmQgcGVyZm9ybWFuY2UgaW1wcm92ZW1lbnRzIGZvciB0aGUgVVNCDQo+IERpc3BsYXlMaW5rIGZy YW1lYnVmZmVyIGFuZCBtb2Rlc2V0dGluZyBkcml2ZXJzIGZvciB0aGlzIG1lcmdlIHdpbmRvdy4N Cg0KRm9yIHN1Y2ggYSBsb25nIHNlcmllcyBpdCB3b3VsZCBiZSB2ZXJ5IG5pY2UgdG8gcG9zdCBh IGxpbmsgdG8geW91ciBnaXQNCnRyZWUgc28gcGVvcGxlIG1heSBwbGF5IHdpdGggeW91ciBjaGFu Z2VzIGVhc2lseS4NCg0KQ291bGQgeW91IHBsZWFzZSBwcmVwYXJlIG9uZT8NCg0KLUFsZXhleQ=

I don't use git to manage my patches, I use quilt.

I uploaded the patches from quilt here:
http://people.redhat.com/~mpatocka/patches/kernel/udl/series.html

Mikulas

I've queued fbdev ones for 4.19 (w/ some minor fixes), thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics