From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Date: Thu, 21 Apr 2016 18:54:45 +0000 Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support Message-Id: <571921F5.2080007@tronnes.org> List-Id: References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> In-Reply-To: <1461165929-11344-5-git-send-email-noralf@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org Cc: tomi.valkeinen@ti.com, laurent.pinchart@ideasonboard.com, linux-kernel@vger.kernel.org Den 20.04.2016 17:25, skrev Noralf Trønnes: > This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > Accumulated fbdev framebuffer changes are signaled using the callback > (struct drm_framebuffer_funcs *)->dirty() > > The drm_fb_helper_sys_*() functions will accumulate changes and > schedule fb_info.deferred_work _if_ fb_info.fbdefio is set. > This worker is used by the deferred io mmap code to signal that it > has been collecting page faults. The page faults and/or other changes > are then merged into a drm_clip_rect and passed to the framebuffer > dirty() function. > > The driver is responsible for setting up the fb_info.fbdefio structure > and calling fb_deferred_io_init() using the provided callback: > (struct fb_deferred_io).deferred_io = drm_fb_helper_deferred_io; > > Signed-off-by: Noralf Trønnes > --- > drivers/gpu/drm/drm_fb_helper.c | 119 +++++++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 15 +++++ > 2 files changed, 133 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c [...] > +#ifdef CONFIG_FB_DEFERRED_IO > +/** > + * drm_fb_helper_deferred_io() - (struct fb_deferred_io *)->deferred_io callback > + * function > + * > + * This function always runs in process context (struct delayed_work) and calls > + * the (struct drm_framebuffer_funcs)->dirty function with the collected > + * damage. There's no need to worry about the possibility that the fb_sys_*() > + * functions could be running in atomic context. They only schedule the > + * delayed worker which then calls this deferred_io callback. > + */ > +void drm_fb_helper_deferred_io(struct fb_info *info, > + struct list_head *pagelist) > +{ > + struct drm_fb_helper *helper = info->par; > + unsigned long start, end, min, max; > + struct drm_clip_rect clip; > + unsigned long flags; > + struct page *page; > + > + if (!helper->fb->funcs->dirty) > + return; > + > + spin_lock_irqsave(&helper->dirty_lock, flags); > + clip = helper->dirty_clip; > + drm_clip_rect_reset(&helper->dirty_clip); > + spin_unlock_irqrestore(&helper->dirty_lock, flags); > + > + min = ULONG_MAX; > + max = 0; > + list_for_each_entry(page, pagelist, lru) { > + start = page->index << PAGE_SHIFT; > + end = start + PAGE_SIZE - 1; > + min = min(min, start); > + max = max(max, end); > + } > + > + if (min < max) { > + struct drm_clip_rect mmap_clip; > + > + mmap_clip.x1 = 0; > + mmap_clip.x2 = info->var.xres; > + mmap_clip.y1 = min / info->fix.line_length; > + mmap_clip.y2 = min_t(u32, max / info->fix.line_length, > + info->var.yres); > + drm_clip_rect_merge(&clip, &mmap_clip, 1, 0, 0, 0); > + } > + > + if (!drm_clip_rect_is_empty(&clip)) > + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); > +} > +EXPORT_SYMBOL(drm_fb_helper_deferred_io); There is one thing I have wondered about when it comes to deferred io and long run times for the defio worker with my displays: Userspace writes to some pages then the deferred io worker kicks off and runs for 100ms holding the page list mutex. While this is happening, userspace writes to a new page triggering a page_mkwrite. Now this function has to wait for the mutex to be released. Who is actually waiting here and is there a problem that this can last for 100ms? Excerpt from drivers/video/fbdev/core/fb_defio.c: /* vm_ops->page_mkwrite handler */ static int fb_deferred_io_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) { ... /* this is a callback we get when userspace first tries to write to the page. we schedule a workqueue. that workqueue will eventually mkclean the touched pages and execute the deferred framebuffer IO. then if userspace touches a page again, we repeat the same scheme */ ... /* protect against the workqueue changing the page list */ mutex_lock(&fbdefio->lock); ... /* * We want the page to remain locked from ->page_mkwrite until * the PTE is marked dirty to avoid page_mkclean() being called * before the PTE is updated, which would leave the page ignored * by defio. * Do this by locking the page here and informing the caller * about it with VM_FAULT_LOCKED. */ lock_page(page); /* we loop through the pagelist before adding in order to keep the pagelist sorted */ list_for_each_entry(cur, &fbdefio->pagelist, lru) { /* this check is to catch the case where a new process could start writing to the same page through a new pte. this new access can cause the mkwrite even when the original ps's pte is marked writable */ if (unlikely(cur = page)) goto page_already_added; else if (cur->index > page->index) break; } list_add_tail(&page->lru, &cur->lru); page_already_added: mutex_unlock(&fbdefio->lock); /* come back after delay to process the deferred IO */ schedule_delayed_work(&info->deferred_work, fbdefio->delay); return VM_FAULT_LOCKED; } static int fb_deferred_io_set_page_dirty(struct page *page) { if (!PageDirty(page)) SetPageDirty(page); return 0; } /* workqueue callback */ static void fb_deferred_io_work(struct work_struct *work) { ... /* here we mkclean the pages, then do all deferred IO */ mutex_lock(&fbdefio->lock); list_for_each_entry(cur, &fbdefio->pagelist, lru) { lock_page(cur); page_mkclean(cur); unlock_page(cur); } /* driver's callback with pagelist */ fbdefio->deferred_io(info, &fbdefio->pagelist); ... mutex_unlock(&fbdefio->lock); } Noralf. From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support Date: Thu, 21 Apr 2016 20:54:45 +0200 Message-ID: <571921F5.2080007@tronnes.org> References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from smtp.domeneshop.no (smtp.domeneshop.no [IPv6:2a01:5b40:0:3005::1]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9EF286ED73 for ; Thu, 21 Apr 2016 18:54:52 +0000 (UTC) In-Reply-To: <1461165929-11344-5-git-send-email-noralf@tronnes.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org Cc: tomi.valkeinen@ti.com, laurent.pinchart@ideasonboard.com, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org CkRlbiAyMC4wNC4yMDE2IDE3OjI1LCBza3JldiBOb3JhbGYgVHLDuG5uZXM6Cj4gVGhpcyBhZGRz IGRlZmVycmVkIGlvIHN1cHBvcnQgaWYgQ09ORklHX0ZCX0RFRkVSUkVEX0lPIGlzIGVuYWJsZWQu Cj4gQWNjdW11bGF0ZWQgZmJkZXYgZnJhbWVidWZmZXIgY2hhbmdlcyBhcmUgc2lnbmFsZWQgdXNp bmcgdGhlIGNhbGxiYWNrCj4gKHN0cnVjdCBkcm1fZnJhbWVidWZmZXJfZnVuY3MgKiktPmRpcnR5 KCkKPgo+IFRoZSBkcm1fZmJfaGVscGVyX3N5c18qKCkgZnVuY3Rpb25zIHdpbGwgYWNjdW11bGF0 ZSBjaGFuZ2VzIGFuZAo+IHNjaGVkdWxlIGZiX2luZm8uZGVmZXJyZWRfd29yayBfaWZfIGZiX2lu Zm8uZmJkZWZpbyBpcyBzZXQuCj4gVGhpcyB3b3JrZXIgaXMgdXNlZCBieSB0aGUgZGVmZXJyZWQg aW8gbW1hcCBjb2RlIHRvIHNpZ25hbCB0aGF0IGl0Cj4gaGFzIGJlZW4gY29sbGVjdGluZyBwYWdl IGZhdWx0cy4gVGhlIHBhZ2UgZmF1bHRzIGFuZC9vciBvdGhlciBjaGFuZ2VzCj4gYXJlIHRoZW4g bWVyZ2VkIGludG8gYSBkcm1fY2xpcF9yZWN0IGFuZCBwYXNzZWQgdG8gdGhlIGZyYW1lYnVmZmVy Cj4gZGlydHkoKSBmdW5jdGlvbi4KPgo+IFRoZSBkcml2ZXIgaXMgcmVzcG9uc2libGUgZm9yIHNl dHRpbmcgdXAgdGhlIGZiX2luZm8uZmJkZWZpbyBzdHJ1Y3R1cmUKPiBhbmQgY2FsbGluZyBmYl9k ZWZlcnJlZF9pb19pbml0KCkgdXNpbmcgdGhlIHByb3ZpZGVkIGNhbGxiYWNrOgo+IChzdHJ1Y3Qg ZmJfZGVmZXJyZWRfaW8pLmRlZmVycmVkX2lvID0gZHJtX2ZiX2hlbHBlcl9kZWZlcnJlZF9pbzsK Pgo+IFNpZ25lZC1vZmYtYnk6IE5vcmFsZiBUcsO4bm5lcyA8bm9yYWxmQHRyb25uZXMub3JnPgo+ IC0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL2RybV9mYl9oZWxwZXIuYyB8IDExOSArKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKystCj4gICBpbmNsdWRlL2RybS9kcm1fZmJfaGVs cGVyLmggICAgIHwgIDE1ICsrKysrCj4gICAyIGZpbGVzIGNoYW5nZWQsIDEzMyBpbnNlcnRpb25z KCspLCAxIGRlbGV0aW9uKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9m Yl9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJfaGVscGVyLmMKClsuLi5dCgo+ICsj aWZkZWYgQ09ORklHX0ZCX0RFRkVSUkVEX0lPCj4gKy8qKgo+ICsgKiBkcm1fZmJfaGVscGVyX2Rl ZmVycmVkX2lvKCkgLSAoc3RydWN0IGZiX2RlZmVycmVkX2lvICopLT5kZWZlcnJlZF9pbyBjYWxs YmFjawo+ICsgKiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBmdW5jdGlvbgo+ICsgKgo+ ICsgKiBUaGlzIGZ1bmN0aW9uIGFsd2F5cyBydW5zIGluIHByb2Nlc3MgY29udGV4dCAoc3RydWN0 IGRlbGF5ZWRfd29yaykgYW5kIGNhbGxzCj4gKyAqIHRoZSAoc3RydWN0IGRybV9mcmFtZWJ1ZmZl cl9mdW5jcyktPmRpcnR5IGZ1bmN0aW9uIHdpdGggdGhlIGNvbGxlY3RlZAo+ICsgKiBkYW1hZ2Uu IFRoZXJlJ3Mgbm8gbmVlZCB0byB3b3JyeSBhYm91dCB0aGUgcG9zc2liaWxpdHkgdGhhdCB0aGUg ZmJfc3lzXyooKQo+ICsgKiBmdW5jdGlvbnMgY291bGQgYmUgcnVubmluZyBpbiBhdG9taWMgY29u dGV4dC4gVGhleSBvbmx5IHNjaGVkdWxlIHRoZQo+ICsgKiBkZWxheWVkIHdvcmtlciB3aGljaCB0 aGVuIGNhbGxzIHRoaXMgZGVmZXJyZWRfaW8gY2FsbGJhY2suCj4gKyAqLwo+ICt2b2lkIGRybV9m Yl9oZWxwZXJfZGVmZXJyZWRfaW8oc3RydWN0IGZiX2luZm8gKmluZm8sCj4gKwkJCSAgICAgICBz dHJ1Y3QgbGlzdF9oZWFkICpwYWdlbGlzdCkKPiArewo+ICsJc3RydWN0IGRybV9mYl9oZWxwZXIg KmhlbHBlciA9IGluZm8tPnBhcjsKPiArCXVuc2lnbmVkIGxvbmcgc3RhcnQsIGVuZCwgbWluLCBt YXg7Cj4gKwlzdHJ1Y3QgZHJtX2NsaXBfcmVjdCBjbGlwOwo+ICsJdW5zaWduZWQgbG9uZyBmbGFn czsKPiArCXN0cnVjdCBwYWdlICpwYWdlOwo+ICsKPiArCWlmICghaGVscGVyLT5mYi0+ZnVuY3Mt PmRpcnR5KQo+ICsJCXJldHVybjsKPiArCj4gKwlzcGluX2xvY2tfaXJxc2F2ZSgmaGVscGVyLT5k aXJ0eV9sb2NrLCBmbGFncyk7Cj4gKwljbGlwID0gaGVscGVyLT5kaXJ0eV9jbGlwOwo+ICsJZHJt X2NsaXBfcmVjdF9yZXNldCgmaGVscGVyLT5kaXJ0eV9jbGlwKTsKPiArCXNwaW5fdW5sb2NrX2ly cXJlc3RvcmUoJmhlbHBlci0+ZGlydHlfbG9jaywgZmxhZ3MpOwo+ICsKPiArCW1pbiA9IFVMT05H X01BWDsKPiArCW1heCA9IDA7Cj4gKwlsaXN0X2Zvcl9lYWNoX2VudHJ5KHBhZ2UsIHBhZ2VsaXN0 LCBscnUpIHsKPiArCQlzdGFydCA9IHBhZ2UtPmluZGV4IDw8IFBBR0VfU0hJRlQ7Cj4gKwkJZW5k ID0gc3RhcnQgKyBQQUdFX1NJWkUgLSAxOwo+ICsJCW1pbiA9IG1pbihtaW4sIHN0YXJ0KTsKPiAr CQltYXggPSBtYXgobWF4LCBlbmQpOwo+ICsJfQo+ICsKPiArCWlmIChtaW4gPCBtYXgpIHsKPiAr CQlzdHJ1Y3QgZHJtX2NsaXBfcmVjdCBtbWFwX2NsaXA7Cj4gKwo+ICsJCW1tYXBfY2xpcC54MSA9 IDA7Cj4gKwkJbW1hcF9jbGlwLngyID0gaW5mby0+dmFyLnhyZXM7Cj4gKwkJbW1hcF9jbGlwLnkx ID0gbWluIC8gaW5mby0+Zml4LmxpbmVfbGVuZ3RoOwo+ICsJCW1tYXBfY2xpcC55MiA9IG1pbl90 KHUzMiwgbWF4IC8gaW5mby0+Zml4LmxpbmVfbGVuZ3RoLAo+ICsJCQkJICAgICBpbmZvLT52YXIu eXJlcyk7Cj4gKwkJZHJtX2NsaXBfcmVjdF9tZXJnZSgmY2xpcCwgJm1tYXBfY2xpcCwgMSwgMCwg MCwgMCk7Cj4gKwl9Cj4gKwo+ICsJaWYgKCFkcm1fY2xpcF9yZWN0X2lzX2VtcHR5KCZjbGlwKSkK PiArCQloZWxwZXItPmZiLT5mdW5jcy0+ZGlydHkoaGVscGVyLT5mYiwgTlVMTCwgMCwgMCwgJmNs aXAsIDEpOwo+ICt9Cj4gK0VYUE9SVF9TWU1CT0woZHJtX2ZiX2hlbHBlcl9kZWZlcnJlZF9pbyk7 CgpUaGVyZSBpcyBvbmUgdGhpbmcgSSBoYXZlIHdvbmRlcmVkIGFib3V0IHdoZW4gaXQgY29tZXMg dG8gZGVmZXJyZWQgaW8gYW5kCmxvbmcgcnVuIHRpbWVzIGZvciB0aGUgZGVmaW8gd29ya2VyIHdp dGggbXkgZGlzcGxheXM6CgpVc2Vyc3BhY2Ugd3JpdGVzIHRvIHNvbWUgcGFnZXMgdGhlbiB0aGUg ZGVmZXJyZWQgaW8gd29ya2VyIGtpY2tzIG9mZiBhbmQKcnVucyBmb3IgMTAwbXMgaG9sZGluZyB0 aGUgcGFnZSBsaXN0IG11dGV4LiBXaGlsZSB0aGlzIGlzIGhhcHBlbmluZywKdXNlcnNwYWNlIHdy aXRlcyB0byBhIG5ldyBwYWdlIHRyaWdnZXJpbmcgYSBwYWdlX21rd3JpdGUuIE5vdyB0aGlzCmZ1 bmN0aW9uIGhhcyB0byB3YWl0IGZvciB0aGUgbXV0ZXggdG8gYmUgcmVsZWFzZWQuCgpXaG8gaXMg YWN0dWFsbHkgd2FpdGluZyBoZXJlIGFuZCBpcyB0aGVyZSBhIHByb2JsZW0gdGhhdCB0aGlzIGNh biBsYXN0CmZvciAxMDBtcz8KCkV4Y2VycHQgZnJvbSBkcml2ZXJzL3ZpZGVvL2ZiZGV2L2NvcmUv ZmJfZGVmaW8uYzoKCi8qIHZtX29wcy0+cGFnZV9ta3dyaXRlIGhhbmRsZXIgKi8Kc3RhdGljIGlu dCBmYl9kZWZlcnJlZF9pb19ta3dyaXRlKHN0cnVjdCB2bV9hcmVhX3N0cnVjdCAqdm1hLAogICAg ICAgICAgICAgICAgICAgc3RydWN0IHZtX2ZhdWx0ICp2bWYpCnsKLi4uCiAgICAgLyogdGhpcyBp cyBhIGNhbGxiYWNrIHdlIGdldCB3aGVuIHVzZXJzcGFjZSBmaXJzdCB0cmllcyB0bwogICAgIHdy aXRlIHRvIHRoZSBwYWdlLiB3ZSBzY2hlZHVsZSBhIHdvcmtxdWV1ZS4gdGhhdCB3b3JrcXVldWUK ICAgICB3aWxsIGV2ZW50dWFsbHkgbWtjbGVhbiB0aGUgdG91Y2hlZCBwYWdlcyBhbmQgZXhlY3V0 ZSB0aGUKICAgICBkZWZlcnJlZCBmcmFtZWJ1ZmZlciBJTy4gdGhlbiBpZiB1c2Vyc3BhY2UgdG91 Y2hlcyBhIHBhZ2UKICAgICBhZ2Fpbiwgd2UgcmVwZWF0IHRoZSBzYW1lIHNjaGVtZSAqLwouLi4K ICAgICAvKiBwcm90ZWN0IGFnYWluc3QgdGhlIHdvcmtxdWV1ZSBjaGFuZ2luZyB0aGUgcGFnZSBs aXN0ICovCiAgICAgbXV0ZXhfbG9jaygmZmJkZWZpby0+bG9jayk7Ci4uLgogICAgIC8qCiAgICAg ICogV2Ugd2FudCB0aGUgcGFnZSB0byByZW1haW4gbG9ja2VkIGZyb20gLT5wYWdlX21rd3JpdGUg dW50aWwKICAgICAgKiB0aGUgUFRFIGlzIG1hcmtlZCBkaXJ0eSB0byBhdm9pZCBwYWdlX21rY2xl YW4oKSBiZWluZyBjYWxsZWQKICAgICAgKiBiZWZvcmUgdGhlIFBURSBpcyB1cGRhdGVkLCB3aGlj aCB3b3VsZCBsZWF2ZSB0aGUgcGFnZSBpZ25vcmVkCiAgICAgICogYnkgZGVmaW8uCiAgICAgICog RG8gdGhpcyBieSBsb2NraW5nIHRoZSBwYWdlIGhlcmUgYW5kIGluZm9ybWluZyB0aGUgY2FsbGVy CiAgICAgICogYWJvdXQgaXQgd2l0aCBWTV9GQVVMVF9MT0NLRUQuCiAgICAgICovCiAgICAgbG9j a19wYWdlKHBhZ2UpOwoKICAgICAvKiB3ZSBsb29wIHRocm91Z2ggdGhlIHBhZ2VsaXN0IGJlZm9y ZSBhZGRpbmcgaW4gb3JkZXIKICAgICB0byBrZWVwIHRoZSBwYWdlbGlzdCBzb3J0ZWQgKi8KICAg ICBsaXN0X2Zvcl9lYWNoX2VudHJ5KGN1ciwgJmZiZGVmaW8tPnBhZ2VsaXN0LCBscnUpIHsKICAg ICAgICAgLyogdGhpcyBjaGVjayBpcyB0byBjYXRjaCB0aGUgY2FzZSB3aGVyZSBhIG5ldwogICAg ICAgICBwcm9jZXNzIGNvdWxkIHN0YXJ0IHdyaXRpbmcgdG8gdGhlIHNhbWUgcGFnZQogICAgICAg ICB0aHJvdWdoIGEgbmV3IHB0ZS4gdGhpcyBuZXcgYWNjZXNzIGNhbiBjYXVzZSB0aGUKICAgICAg ICAgbWt3cml0ZSBldmVuIHdoZW4gdGhlIG9yaWdpbmFsIHBzJ3MgcHRlIGlzIG1hcmtlZAogICAg ICAgICB3cml0YWJsZSAqLwogICAgICAgICBpZiAodW5saWtlbHkoY3VyID09IHBhZ2UpKQogICAg ICAgICAgICAgZ290byBwYWdlX2FscmVhZHlfYWRkZWQ7CiAgICAgICAgIGVsc2UgaWYgKGN1ci0+ aW5kZXggPiBwYWdlLT5pbmRleCkKICAgICAgICAgICAgIGJyZWFrOwogICAgIH0KCiAgICAgbGlz dF9hZGRfdGFpbCgmcGFnZS0+bHJ1LCAmY3VyLT5scnUpOwoKcGFnZV9hbHJlYWR5X2FkZGVkOgog ICAgIG11dGV4X3VubG9jaygmZmJkZWZpby0+bG9jayk7CgogICAgIC8qIGNvbWUgYmFjayBhZnRl ciBkZWxheSB0byBwcm9jZXNzIHRoZSBkZWZlcnJlZCBJTyAqLwogICAgIHNjaGVkdWxlX2RlbGF5 ZWRfd29yaygmaW5mby0+ZGVmZXJyZWRfd29yaywgZmJkZWZpby0+ZGVsYXkpOwogICAgIHJldHVy biBWTV9GQVVMVF9MT0NLRUQ7Cn0KCnN0YXRpYyBpbnQgZmJfZGVmZXJyZWRfaW9fc2V0X3BhZ2Vf ZGlydHkoc3RydWN0IHBhZ2UgKnBhZ2UpCnsKICAgICBpZiAoIVBhZ2VEaXJ0eShwYWdlKSkKICAg ICAgICAgU2V0UGFnZURpcnR5KHBhZ2UpOwogICAgIHJldHVybiAwOwp9CgovKiB3b3JrcXVldWUg Y2FsbGJhY2sgKi8Kc3RhdGljIHZvaWQgZmJfZGVmZXJyZWRfaW9fd29yayhzdHJ1Y3Qgd29ya19z dHJ1Y3QgKndvcmspCnsKLi4uCiAgICAgLyogaGVyZSB3ZSBta2NsZWFuIHRoZSBwYWdlcywgdGhl biBkbyBhbGwgZGVmZXJyZWQgSU8gKi8KICAgICBtdXRleF9sb2NrKCZmYmRlZmlvLT5sb2NrKTsK ICAgICBsaXN0X2Zvcl9lYWNoX2VudHJ5KGN1ciwgJmZiZGVmaW8tPnBhZ2VsaXN0LCBscnUpIHsK ICAgICAgICAgbG9ja19wYWdlKGN1cik7CiAgICAgICAgIHBhZ2VfbWtjbGVhbihjdXIpOwogICAg ICAgICB1bmxvY2tfcGFnZShjdXIpOwogICAgIH0KCiAgICAgLyogZHJpdmVyJ3MgY2FsbGJhY2sg d2l0aCBwYWdlbGlzdCAqLwogICAgIGZiZGVmaW8tPmRlZmVycmVkX2lvKGluZm8sICZmYmRlZmlv LT5wYWdlbGlzdCk7Ci4uLgogICAgIG11dGV4X3VubG9jaygmZmJkZWZpby0+bG9jayk7Cn0KCgpO b3JhbGYuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpk cmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753328AbcDUSyy (ORCPT ); Thu, 21 Apr 2016 14:54:54 -0400 Received: from smtp.domeneshop.no ([194.63.252.55]:33789 "EHLO smtp.domeneshop.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751720AbcDUSyx (ORCPT ); Thu, 21 Apr 2016 14:54:53 -0400 Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support To: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> Cc: daniel@ffwll.ch, laurent.pinchart@ideasonboard.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Message-ID: <571921F5.2080007@tronnes.org> Date: Thu, 21 Apr 2016 20:54:45 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1461165929-11344-5-git-send-email-noralf@tronnes.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Den 20.04.2016 17:25, skrev Noralf Trønnes: > This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > Accumulated fbdev framebuffer changes are signaled using the callback > (struct drm_framebuffer_funcs *)->dirty() > > The drm_fb_helper_sys_*() functions will accumulate changes and > schedule fb_info.deferred_work _if_ fb_info.fbdefio is set. > This worker is used by the deferred io mmap code to signal that it > has been collecting page faults. The page faults and/or other changes > are then merged into a drm_clip_rect and passed to the framebuffer > dirty() function. > > The driver is responsible for setting up the fb_info.fbdefio structure > and calling fb_deferred_io_init() using the provided callback: > (struct fb_deferred_io).deferred_io = drm_fb_helper_deferred_io; > > Signed-off-by: Noralf Trønnes > --- > drivers/gpu/drm/drm_fb_helper.c | 119 +++++++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 15 +++++ > 2 files changed, 133 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c [...] > +#ifdef CONFIG_FB_DEFERRED_IO > +/** > + * drm_fb_helper_deferred_io() - (struct fb_deferred_io *)->deferred_io callback > + * function > + * > + * This function always runs in process context (struct delayed_work) and calls > + * the (struct drm_framebuffer_funcs)->dirty function with the collected > + * damage. There's no need to worry about the possibility that the fb_sys_*() > + * functions could be running in atomic context. They only schedule the > + * delayed worker which then calls this deferred_io callback. > + */ > +void drm_fb_helper_deferred_io(struct fb_info *info, > + struct list_head *pagelist) > +{ > + struct drm_fb_helper *helper = info->par; > + unsigned long start, end, min, max; > + struct drm_clip_rect clip; > + unsigned long flags; > + struct page *page; > + > + if (!helper->fb->funcs->dirty) > + return; > + > + spin_lock_irqsave(&helper->dirty_lock, flags); > + clip = helper->dirty_clip; > + drm_clip_rect_reset(&helper->dirty_clip); > + spin_unlock_irqrestore(&helper->dirty_lock, flags); > + > + min = ULONG_MAX; > + max = 0; > + list_for_each_entry(page, pagelist, lru) { > + start = page->index << PAGE_SHIFT; > + end = start + PAGE_SIZE - 1; > + min = min(min, start); > + max = max(max, end); > + } > + > + if (min < max) { > + struct drm_clip_rect mmap_clip; > + > + mmap_clip.x1 = 0; > + mmap_clip.x2 = info->var.xres; > + mmap_clip.y1 = min / info->fix.line_length; > + mmap_clip.y2 = min_t(u32, max / info->fix.line_length, > + info->var.yres); > + drm_clip_rect_merge(&clip, &mmap_clip, 1, 0, 0, 0); > + } > + > + if (!drm_clip_rect_is_empty(&clip)) > + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); > +} > +EXPORT_SYMBOL(drm_fb_helper_deferred_io); There is one thing I have wondered about when it comes to deferred io and long run times for the defio worker with my displays: Userspace writes to some pages then the deferred io worker kicks off and runs for 100ms holding the page list mutex. While this is happening, userspace writes to a new page triggering a page_mkwrite. Now this function has to wait for the mutex to be released. Who is actually waiting here and is there a problem that this can last for 100ms? Excerpt from drivers/video/fbdev/core/fb_defio.c: /* vm_ops->page_mkwrite handler */ static int fb_deferred_io_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) { ... /* this is a callback we get when userspace first tries to write to the page. we schedule a workqueue. that workqueue will eventually mkclean the touched pages and execute the deferred framebuffer IO. then if userspace touches a page again, we repeat the same scheme */ ... /* protect against the workqueue changing the page list */ mutex_lock(&fbdefio->lock); ... /* * We want the page to remain locked from ->page_mkwrite until * the PTE is marked dirty to avoid page_mkclean() being called * before the PTE is updated, which would leave the page ignored * by defio. * Do this by locking the page here and informing the caller * about it with VM_FAULT_LOCKED. */ lock_page(page); /* we loop through the pagelist before adding in order to keep the pagelist sorted */ list_for_each_entry(cur, &fbdefio->pagelist, lru) { /* this check is to catch the case where a new process could start writing to the same page through a new pte. this new access can cause the mkwrite even when the original ps's pte is marked writable */ if (unlikely(cur == page)) goto page_already_added; else if (cur->index > page->index) break; } list_add_tail(&page->lru, &cur->lru); page_already_added: mutex_unlock(&fbdefio->lock); /* come back after delay to process the deferred IO */ schedule_delayed_work(&info->deferred_work, fbdefio->delay); return VM_FAULT_LOCKED; } static int fb_deferred_io_set_page_dirty(struct page *page) { if (!PageDirty(page)) SetPageDirty(page); return 0; } /* workqueue callback */ static void fb_deferred_io_work(struct work_struct *work) { ... /* here we mkclean the pages, then do all deferred IO */ mutex_lock(&fbdefio->lock); list_for_each_entry(cur, &fbdefio->pagelist, lru) { lock_page(cur); page_mkclean(cur); unlock_page(cur); } /* driver's callback with pagelist */ fbdefio->deferred_io(info, &fbdefio->pagelist); ... mutex_unlock(&fbdefio->lock); } Noralf.