From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Date: Fri, 22 Apr 2016 08:27:19 +0000 Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support Message-Id: <20160422082719.GH2510@phenom.ffwll.local> List-Id: References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> <571921F5.2080007@tronnes.org> In-Reply-To: <571921F5.2080007@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com, laurent.pinchart@ideasonboard.com On Thu, Apr 21, 2016 at 08:54:45PM +0200, Noralf Tr=F8nnes wrote: >=20 > Den 20.04.2016 17:25, skrev Noralf Tr=F8nnes: > >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 =3D drm_fb_helper_deferred_io; > > > >Signed-off-by: Noralf Tr=F8nnes > >--- > > 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_he= lper.c >=20 > [...] >=20 > >+#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) a= nd calls > >+ * the (struct drm_framebuffer_funcs)->dirty function with the collected > >+ * damage. There's no need to worry about the possibility that the fb_s= ys_*() > >+ * 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 =3D 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 =3D helper->dirty_clip; > >+ drm_clip_rect_reset(&helper->dirty_clip); > >+ spin_unlock_irqrestore(&helper->dirty_lock, flags); > >+ > >+ min =3D ULONG_MAX; > >+ max =3D 0; > >+ list_for_each_entry(page, pagelist, lru) { > >+ start =3D page->index << PAGE_SHIFT; > >+ end =3D start + PAGE_SIZE - 1; > >+ min =3D min(min, start); > >+ max =3D max(max, end); > >+ } > >+ > >+ if (min < max) { > >+ struct drm_clip_rect mmap_clip; > >+ > >+ mmap_clip.x1 =3D 0; > >+ mmap_clip.x2 =3D info->var.xres; > >+ mmap_clip.y1 =3D min / info->fix.line_length; > >+ mmap_clip.y2 =3D 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); >=20 > 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: >=20 > 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. >=20 > Who is actually waiting here and is there a problem that this can last > for 100ms? No idea at all - I haven't looked that closely at fbdev defio. But one reason we have an explicit ioctl in drm to flush out frontbuffer rendering is exactly that flushing could take some time, and should only be done once userspace has completed some rendering. Not right in the middle of an op. I guess fix up your userspace to use dumb drm fb + drm dirtyfb ioctl? Otherwise you'll get to improve fbdev defio, and fbdev is deprecated subsystem and a real horror show. I highly recommend against touching it ;-) Cheers, Daniel >=20 > Excerpt from drivers/video/fbdev/core/fb_defio.c: >=20 > /* 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); >=20 > /* 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 =3D page)) > goto page_already_added; > else if (cur->index > page->index) > break; > } >=20 > list_add_tail(&page->lru, &cur->lru); >=20 > page_already_added: > mutex_unlock(&fbdefio->lock); >=20 > /* come back after delay to process the deferred IO */ > schedule_delayed_work(&info->deferred_work, fbdefio->delay); > return VM_FAULT_LOCKED; > } >=20 > static int fb_deferred_io_set_page_dirty(struct page *page) > { > if (!PageDirty(page)) > SetPageDirty(page); > return 0; > } >=20 > /* 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); > } >=20 > /* driver's callback with pagelist */ > fbdefio->deferred_io(info, &fbdefio->pagelist); > ... > mutex_unlock(&fbdefio->lock); > } >=20 >=20 > Noralf. >=20 --=20 Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support Date: Fri, 22 Apr 2016 10:27:19 +0200 Message-ID: <20160422082719.GH2510@phenom.ffwll.local> References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> <571921F5.2080007@tronnes.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 402D76E2A3 for ; Fri, 22 Apr 2016 08:27:24 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id n3so2592585wmn.1 for ; Fri, 22 Apr 2016 01:27:24 -0700 (PDT) Content-Disposition: inline In-Reply-To: <571921F5.2080007@tronnes.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com, laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBBcHIgMjEsIDIwMTYgYXQgMDg6NTQ6NDVQTSArMDIwMCwgTm9yYWxmIFRyw7hubmVz IHdyb3RlOgo+IAo+IERlbiAyMC4wNC4yMDE2IDE3OjI1LCBza3JldiBOb3JhbGYgVHLDuG5uZXM6 Cj4gPlRoaXMgYWRkcyBkZWZlcnJlZCBpbyBzdXBwb3J0IGlmIENPTkZJR19GQl9ERUZFUlJFRF9J TyBpcyBlbmFibGVkLgo+ID5BY2N1bXVsYXRlZCBmYmRldiBmcmFtZWJ1ZmZlciBjaGFuZ2VzIGFy ZSBzaWduYWxlZCB1c2luZyB0aGUgY2FsbGJhY2sKPiA+KHN0cnVjdCBkcm1fZnJhbWVidWZmZXJf ZnVuY3MgKiktPmRpcnR5KCkKPiA+Cj4gPlRoZSBkcm1fZmJfaGVscGVyX3N5c18qKCkgZnVuY3Rp b25zIHdpbGwgYWNjdW11bGF0ZSBjaGFuZ2VzIGFuZAo+ID5zY2hlZHVsZSBmYl9pbmZvLmRlZmVy cmVkX3dvcmsgX2lmXyBmYl9pbmZvLmZiZGVmaW8gaXMgc2V0Lgo+ID5UaGlzIHdvcmtlciBpcyB1 c2VkIGJ5IHRoZSBkZWZlcnJlZCBpbyBtbWFwIGNvZGUgdG8gc2lnbmFsIHRoYXQgaXQKPiA+aGFz IGJlZW4gY29sbGVjdGluZyBwYWdlIGZhdWx0cy4gVGhlIHBhZ2UgZmF1bHRzIGFuZC9vciBvdGhl ciBjaGFuZ2VzCj4gPmFyZSB0aGVuIG1lcmdlZCBpbnRvIGEgZHJtX2NsaXBfcmVjdCBhbmQgcGFz c2VkIHRvIHRoZSBmcmFtZWJ1ZmZlcgo+ID5kaXJ0eSgpIGZ1bmN0aW9uLgo+ID4KPiA+VGhlIGRy aXZlciBpcyByZXNwb25zaWJsZSBmb3Igc2V0dGluZyB1cCB0aGUgZmJfaW5mby5mYmRlZmlvIHN0 cnVjdHVyZQo+ID5hbmQgY2FsbGluZyBmYl9kZWZlcnJlZF9pb19pbml0KCkgdXNpbmcgdGhlIHBy b3ZpZGVkIGNhbGxiYWNrOgo+ID4oc3RydWN0IGZiX2RlZmVycmVkX2lvKS5kZWZlcnJlZF9pbyA9 IGRybV9mYl9oZWxwZXJfZGVmZXJyZWRfaW87Cj4gPgo+ID5TaWduZWQtb2ZmLWJ5OiBOb3JhbGYg VHLDuG5uZXMgPG5vcmFsZkB0cm9ubmVzLm9yZz4KPiA+LS0tCj4gPiAgZHJpdmVycy9ncHUvZHJt L2RybV9mYl9oZWxwZXIuYyB8IDExOSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKystCj4gPiAgaW5jbHVkZS9kcm0vZHJtX2ZiX2hlbHBlci5oICAgICB8ICAxNSArKysrKwo+ ID4gIDIgZmlsZXMgY2hhbmdlZCwgMTMzIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiA+ Cj4gPmRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2ZiX2hlbHBlci5jIGIvZHJpdmVy cy9ncHUvZHJtL2RybV9mYl9oZWxwZXIuYwo+IAo+IFsuLi5dCj4gCj4gPisjaWZkZWYgQ09ORklH X0ZCX0RFRkVSUkVEX0lPCj4gPisvKioKPiA+KyAqIGRybV9mYl9oZWxwZXJfZGVmZXJyZWRfaW8o KSAtIChzdHJ1Y3QgZmJfZGVmZXJyZWRfaW8gKiktPmRlZmVycmVkX2lvIGNhbGxiYWNrCj4gPisg KiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBmdW5jdGlvbgo+ID4rICoKPiA+KyAqIFRo aXMgZnVuY3Rpb24gYWx3YXlzIHJ1bnMgaW4gcHJvY2VzcyBjb250ZXh0IChzdHJ1Y3QgZGVsYXll ZF93b3JrKSBhbmQgY2FsbHMKPiA+KyAqIHRoZSAoc3RydWN0IGRybV9mcmFtZWJ1ZmZlcl9mdW5j cyktPmRpcnR5IGZ1bmN0aW9uIHdpdGggdGhlIGNvbGxlY3RlZAo+ID4rICogZGFtYWdlLiBUaGVy ZSdzIG5vIG5lZWQgdG8gd29ycnkgYWJvdXQgdGhlIHBvc3NpYmlsaXR5IHRoYXQgdGhlIGZiX3N5 c18qKCkKPiA+KyAqIGZ1bmN0aW9ucyBjb3VsZCBiZSBydW5uaW5nIGluIGF0b21pYyBjb250ZXh0 LiBUaGV5IG9ubHkgc2NoZWR1bGUgdGhlCj4gPisgKiBkZWxheWVkIHdvcmtlciB3aGljaCB0aGVu IGNhbGxzIHRoaXMgZGVmZXJyZWRfaW8gY2FsbGJhY2suCj4gPisgKi8KPiA+K3ZvaWQgZHJtX2Zi X2hlbHBlcl9kZWZlcnJlZF9pbyhzdHJ1Y3QgZmJfaW5mbyAqaW5mbywKPiA+KwkJCSAgICAgICBz dHJ1Y3QgbGlzdF9oZWFkICpwYWdlbGlzdCkKPiA+K3sKPiA+KwlzdHJ1Y3QgZHJtX2ZiX2hlbHBl ciAqaGVscGVyID0gaW5mby0+cGFyOwo+ID4rCXVuc2lnbmVkIGxvbmcgc3RhcnQsIGVuZCwgbWlu LCBtYXg7Cj4gPisJc3RydWN0IGRybV9jbGlwX3JlY3QgY2xpcDsKPiA+Kwl1bnNpZ25lZCBsb25n IGZsYWdzOwo+ID4rCXN0cnVjdCBwYWdlICpwYWdlOwo+ID4rCj4gPisJaWYgKCFoZWxwZXItPmZi LT5mdW5jcy0+ZGlydHkpCj4gPisJCXJldHVybjsKPiA+Kwo+ID4rCXNwaW5fbG9ja19pcnFzYXZl KCZoZWxwZXItPmRpcnR5X2xvY2ssIGZsYWdzKTsKPiA+KwljbGlwID0gaGVscGVyLT5kaXJ0eV9j bGlwOwo+ID4rCWRybV9jbGlwX3JlY3RfcmVzZXQoJmhlbHBlci0+ZGlydHlfY2xpcCk7Cj4gPisJ c3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmaGVscGVyLT5kaXJ0eV9sb2NrLCBmbGFncyk7Cj4gPisK PiA+KwltaW4gPSBVTE9OR19NQVg7Cj4gPisJbWF4ID0gMDsKPiA+KwlsaXN0X2Zvcl9lYWNoX2Vu dHJ5KHBhZ2UsIHBhZ2VsaXN0LCBscnUpIHsKPiA+KwkJc3RhcnQgPSBwYWdlLT5pbmRleCA8PCBQ QUdFX1NISUZUOwo+ID4rCQllbmQgPSBzdGFydCArIFBBR0VfU0laRSAtIDE7Cj4gPisJCW1pbiA9 IG1pbihtaW4sIHN0YXJ0KTsKPiA+KwkJbWF4ID0gbWF4KG1heCwgZW5kKTsKPiA+Kwl9Cj4gPisK PiA+KwlpZiAobWluIDwgbWF4KSB7Cj4gPisJCXN0cnVjdCBkcm1fY2xpcF9yZWN0IG1tYXBfY2xp cDsKPiA+Kwo+ID4rCQltbWFwX2NsaXAueDEgPSAwOwo+ID4rCQltbWFwX2NsaXAueDIgPSBpbmZv LT52YXIueHJlczsKPiA+KwkJbW1hcF9jbGlwLnkxID0gbWluIC8gaW5mby0+Zml4LmxpbmVfbGVu Z3RoOwo+ID4rCQltbWFwX2NsaXAueTIgPSBtaW5fdCh1MzIsIG1heCAvIGluZm8tPmZpeC5saW5l X2xlbmd0aCwKPiA+KwkJCQkgICAgIGluZm8tPnZhci55cmVzKTsKPiA+KwkJZHJtX2NsaXBfcmVj dF9tZXJnZSgmY2xpcCwgJm1tYXBfY2xpcCwgMSwgMCwgMCwgMCk7Cj4gPisJfQo+ID4rCj4gPisJ aWYgKCFkcm1fY2xpcF9yZWN0X2lzX2VtcHR5KCZjbGlwKSkKPiA+KwkJaGVscGVyLT5mYi0+ZnVu Y3MtPmRpcnR5KGhlbHBlci0+ZmIsIE5VTEwsIDAsIDAsICZjbGlwLCAxKTsKPiA+K30KPiA+K0VY UE9SVF9TWU1CT0woZHJtX2ZiX2hlbHBlcl9kZWZlcnJlZF9pbyk7Cj4gCj4gVGhlcmUgaXMgb25l IHRoaW5nIEkgaGF2ZSB3b25kZXJlZCBhYm91dCB3aGVuIGl0IGNvbWVzIHRvIGRlZmVycmVkIGlv IGFuZAo+IGxvbmcgcnVuIHRpbWVzIGZvciB0aGUgZGVmaW8gd29ya2VyIHdpdGggbXkgZGlzcGxh eXM6Cj4gCj4gVXNlcnNwYWNlIHdyaXRlcyB0byBzb21lIHBhZ2VzIHRoZW4gdGhlIGRlZmVycmVk IGlvIHdvcmtlciBraWNrcyBvZmYgYW5kCj4gcnVucyBmb3IgMTAwbXMgaG9sZGluZyB0aGUgcGFn ZSBsaXN0IG11dGV4LiBXaGlsZSB0aGlzIGlzIGhhcHBlbmluZywKPiB1c2Vyc3BhY2Ugd3JpdGVz IHRvIGEgbmV3IHBhZ2UgdHJpZ2dlcmluZyBhIHBhZ2VfbWt3cml0ZS4gTm93IHRoaXMKPiBmdW5j dGlvbiBoYXMgdG8gd2FpdCBmb3IgdGhlIG11dGV4IHRvIGJlIHJlbGVhc2VkLgo+IAo+IFdobyBp cyBhY3R1YWxseSB3YWl0aW5nIGhlcmUgYW5kIGlzIHRoZXJlIGEgcHJvYmxlbSB0aGF0IHRoaXMg Y2FuIGxhc3QKPiBmb3IgMTAwbXM/CgpObyBpZGVhIGF0IGFsbCAtIEkgaGF2ZW4ndCBsb29rZWQg dGhhdCBjbG9zZWx5IGF0ICBmYmRldiBkZWZpby4gQnV0IG9uZQpyZWFzb24gd2UgaGF2ZSBhbiBl eHBsaWNpdCBpb2N0bCBpbiBkcm0gdG8gZmx1c2ggb3V0IGZyb250YnVmZmVyIHJlbmRlcmluZwpp cyBleGFjdGx5IHRoYXQgZmx1c2hpbmcgY291bGQgdGFrZSBzb21lIHRpbWUsIGFuZCBzaG91bGQg b25seSBiZSBkb25lCm9uY2UgdXNlcnNwYWNlIGhhcyBjb21wbGV0ZWQgc29tZSByZW5kZXJpbmcu IE5vdCByaWdodCBpbiB0aGUgbWlkZGxlIG9mIGFuCm9wLgoKSSBndWVzcyBmaXggdXAgeW91ciB1 c2Vyc3BhY2UgdG8gdXNlIGR1bWIgZHJtIGZiICsgZHJtIGRpcnR5ZmIgaW9jdGw/Ck90aGVyd2lz ZSB5b3UnbGwgZ2V0IHRvIGltcHJvdmUgZmJkZXYgZGVmaW8sIGFuZCBmYmRldiBpcyBkZXByZWNh dGVkCnN1YnN5c3RlbSBhbmQgYSByZWFsIGhvcnJvciBzaG93LiBJIGhpZ2hseSByZWNvbW1lbmQg YWdhaW5zdCB0b3VjaGluZyBpdAo7LSkKCkNoZWVycywgRGFuaWVsCgo+IAo+IEV4Y2VycHQgZnJv bSBkcml2ZXJzL3ZpZGVvL2ZiZGV2L2NvcmUvZmJfZGVmaW8uYzoKPiAKPiAvKiB2bV9vcHMtPnBh Z2VfbWt3cml0ZSBoYW5kbGVyICovCj4gc3RhdGljIGludCBmYl9kZWZlcnJlZF9pb19ta3dyaXRl KHN0cnVjdCB2bV9hcmVhX3N0cnVjdCAqdm1hLAo+ICAgICAgICAgICAgICAgICAgIHN0cnVjdCB2 bV9mYXVsdCAqdm1mKQo+IHsKPiAuLi4KPiAgICAgLyogdGhpcyBpcyBhIGNhbGxiYWNrIHdlIGdl dCB3aGVuIHVzZXJzcGFjZSBmaXJzdCB0cmllcyB0bwo+ICAgICB3cml0ZSB0byB0aGUgcGFnZS4g d2Ugc2NoZWR1bGUgYSB3b3JrcXVldWUuIHRoYXQgd29ya3F1ZXVlCj4gICAgIHdpbGwgZXZlbnR1 YWxseSBta2NsZWFuIHRoZSB0b3VjaGVkIHBhZ2VzIGFuZCBleGVjdXRlIHRoZQo+ICAgICBkZWZl cnJlZCBmcmFtZWJ1ZmZlciBJTy4gdGhlbiBpZiB1c2Vyc3BhY2UgdG91Y2hlcyBhIHBhZ2UKPiAg ICAgYWdhaW4sIHdlIHJlcGVhdCB0aGUgc2FtZSBzY2hlbWUgKi8KPiAuLi4KPiAgICAgLyogcHJv dGVjdCBhZ2FpbnN0IHRoZSB3b3JrcXVldWUgY2hhbmdpbmcgdGhlIHBhZ2UgbGlzdCAqLwo+ICAg ICBtdXRleF9sb2NrKCZmYmRlZmlvLT5sb2NrKTsKPiAuLi4KPiAgICAgLyoKPiAgICAgICogV2Ug d2FudCB0aGUgcGFnZSB0byByZW1haW4gbG9ja2VkIGZyb20gLT5wYWdlX21rd3JpdGUgdW50aWwK PiAgICAgICogdGhlIFBURSBpcyBtYXJrZWQgZGlydHkgdG8gYXZvaWQgcGFnZV9ta2NsZWFuKCkg YmVpbmcgY2FsbGVkCj4gICAgICAqIGJlZm9yZSB0aGUgUFRFIGlzIHVwZGF0ZWQsIHdoaWNoIHdv dWxkIGxlYXZlIHRoZSBwYWdlIGlnbm9yZWQKPiAgICAgICogYnkgZGVmaW8uCj4gICAgICAqIERv IHRoaXMgYnkgbG9ja2luZyB0aGUgcGFnZSBoZXJlIGFuZCBpbmZvcm1pbmcgdGhlIGNhbGxlcgo+ ICAgICAgKiBhYm91dCBpdCB3aXRoIFZNX0ZBVUxUX0xPQ0tFRC4KPiAgICAgICovCj4gICAgIGxv Y2tfcGFnZShwYWdlKTsKPiAKPiAgICAgLyogd2UgbG9vcCB0aHJvdWdoIHRoZSBwYWdlbGlzdCBi ZWZvcmUgYWRkaW5nIGluIG9yZGVyCj4gICAgIHRvIGtlZXAgdGhlIHBhZ2VsaXN0IHNvcnRlZCAq Lwo+ICAgICBsaXN0X2Zvcl9lYWNoX2VudHJ5KGN1ciwgJmZiZGVmaW8tPnBhZ2VsaXN0LCBscnUp IHsKPiAgICAgICAgIC8qIHRoaXMgY2hlY2sgaXMgdG8gY2F0Y2ggdGhlIGNhc2Ugd2hlcmUgYSBu ZXcKPiAgICAgICAgIHByb2Nlc3MgY291bGQgc3RhcnQgd3JpdGluZyB0byB0aGUgc2FtZSBwYWdl Cj4gICAgICAgICB0aHJvdWdoIGEgbmV3IHB0ZS4gdGhpcyBuZXcgYWNjZXNzIGNhbiBjYXVzZSB0 aGUKPiAgICAgICAgIG1rd3JpdGUgZXZlbiB3aGVuIHRoZSBvcmlnaW5hbCBwcydzIHB0ZSBpcyBt YXJrZWQKPiAgICAgICAgIHdyaXRhYmxlICovCj4gICAgICAgICBpZiAodW5saWtlbHkoY3VyID09 IHBhZ2UpKQo+ICAgICAgICAgICAgIGdvdG8gcGFnZV9hbHJlYWR5X2FkZGVkOwo+ICAgICAgICAg ZWxzZSBpZiAoY3VyLT5pbmRleCA+IHBhZ2UtPmluZGV4KQo+ICAgICAgICAgICAgIGJyZWFrOwo+ ICAgICB9Cj4gCj4gICAgIGxpc3RfYWRkX3RhaWwoJnBhZ2UtPmxydSwgJmN1ci0+bHJ1KTsKPiAK PiBwYWdlX2FscmVhZHlfYWRkZWQ6Cj4gICAgIG11dGV4X3VubG9jaygmZmJkZWZpby0+bG9jayk7 Cj4gCj4gICAgIC8qIGNvbWUgYmFjayBhZnRlciBkZWxheSB0byBwcm9jZXNzIHRoZSBkZWZlcnJl ZCBJTyAqLwo+ICAgICBzY2hlZHVsZV9kZWxheWVkX3dvcmsoJmluZm8tPmRlZmVycmVkX3dvcmss IGZiZGVmaW8tPmRlbGF5KTsKPiAgICAgcmV0dXJuIFZNX0ZBVUxUX0xPQ0tFRDsKPiB9Cj4gCj4g c3RhdGljIGludCBmYl9kZWZlcnJlZF9pb19zZXRfcGFnZV9kaXJ0eShzdHJ1Y3QgcGFnZSAqcGFn ZSkKPiB7Cj4gICAgIGlmICghUGFnZURpcnR5KHBhZ2UpKQo+ICAgICAgICAgU2V0UGFnZURpcnR5 KHBhZ2UpOwo+ICAgICByZXR1cm4gMDsKPiB9Cj4gCj4gLyogd29ya3F1ZXVlIGNhbGxiYWNrICov Cj4gc3RhdGljIHZvaWQgZmJfZGVmZXJyZWRfaW9fd29yayhzdHJ1Y3Qgd29ya19zdHJ1Y3QgKndv cmspCj4gewo+IC4uLgo+ICAgICAvKiBoZXJlIHdlIG1rY2xlYW4gdGhlIHBhZ2VzLCB0aGVuIGRv IGFsbCBkZWZlcnJlZCBJTyAqLwo+ICAgICBtdXRleF9sb2NrKCZmYmRlZmlvLT5sb2NrKTsKPiAg ICAgbGlzdF9mb3JfZWFjaF9lbnRyeShjdXIsICZmYmRlZmlvLT5wYWdlbGlzdCwgbHJ1KSB7Cj4g ICAgICAgICBsb2NrX3BhZ2UoY3VyKTsKPiAgICAgICAgIHBhZ2VfbWtjbGVhbihjdXIpOwo+ICAg ICAgICAgdW5sb2NrX3BhZ2UoY3VyKTsKPiAgICAgfQo+IAo+ICAgICAvKiBkcml2ZXIncyBjYWxs YmFjayB3aXRoIHBhZ2VsaXN0ICovCj4gICAgIGZiZGVmaW8tPmRlZmVycmVkX2lvKGluZm8sICZm YmRlZmlvLT5wYWdlbGlzdCk7Cj4gLi4uCj4gICAgIG11dGV4X3VubG9jaygmZmJkZWZpby0+bG9j ayk7Cj4gfQo+IAo+IAo+IE5vcmFsZi4KPiAKCi0tIApEYW5pZWwgVmV0dGVyClNvZnR3YXJlIEVu Z2luZWVyLCBJbnRlbCBDb3Jwb3JhdGlvbgpodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBs aXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753978AbcDVI12 (ORCPT ); Fri, 22 Apr 2016 04:27:28 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36631 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753620AbcDVI1Y (ORCPT ); Fri, 22 Apr 2016 04:27:24 -0400 Date: Fri, 22 Apr 2016 10:27:19 +0200 From: Daniel Vetter To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, daniel@ffwll.ch, laurent.pinchart@ideasonboard.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/8] drm/fb-helper: Add fb_deferred_io support Message-ID: <20160422082719.GH2510@phenom.ffwll.local> Mail-Followup-To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= , dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org, laurent.pinchart@ideasonboard.com, tomi.valkeinen@ti.com, linux-kernel@vger.kernel.org References: <1461165929-11344-1-git-send-email-noralf@tronnes.org> <1461165929-11344-5-git-send-email-noralf@tronnes.org> <571921F5.2080007@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <571921F5.2080007@tronnes.org> X-Operating-System: Linux phenom 4.4.0-1-amd64 User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 21, 2016 at 08:54:45PM +0200, Noralf Trønnes wrote: > > 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? No idea at all - I haven't looked that closely at fbdev defio. But one reason we have an explicit ioctl in drm to flush out frontbuffer rendering is exactly that flushing could take some time, and should only be done once userspace has completed some rendering. Not right in the middle of an op. I guess fix up your userspace to use dumb drm fb + drm dirtyfb ioctl? Otherwise you'll get to improve fbdev defio, and fbdev is deprecated subsystem and a real horror show. I highly recommend against touching it ;-) Cheers, Daniel > > 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. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch