Hi Am 01.08.19 um 08:19 schrieb Rong Chen: > Hi, > > On 7/31/19 6:21 PM, Michel Dänzer wrote: >> On 2019-07-31 11:25 a.m., Huang, Ying wrote: >>> Hi, Daniel, >>> >>> Daniel Vetter writes: >>> >>>> On Tue, Jul 30, 2019 at 10:27 PM Dave Airlie wrote: >>>>> On Wed, 31 Jul 2019 at 05:00, Daniel Vetter wrote: >>>>>> On Tue, Jul 30, 2019 at 8:50 PM Thomas Zimmermann >>>>>> wrote: >>>>>>> Hi >>>>>>> >>>>>>> Am 30.07.19 um 20:12 schrieb Daniel Vetter: >>>>>>>> On Tue, Jul 30, 2019 at 7:50 PM Thomas Zimmermann >>>>>>>> wrote: >>>>>>>>> Am 29.07.19 um 11:51 schrieb kernel test robot: >>>>>>>>>> Greeting, >>>>>>>>>> >>>>>>>>>> FYI, we noticed a -18.8% regression of vm-scalability.median >>>>>>>>>> due to commit:> >>>>>>>>>> >>>>>>>>>> commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4 >>>>>>>>>> ("drm/mgag200: Replace struct mga_fbdev with generic >>>>>>>>>> framebuffer emulation") >>>>>>>>>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git >>>>>>>>>> master >>>>>>>>> Daniel, Noralf, we may have to revert this patch. >>>>>>>>> >>>>>>>>> I expected some change in display performance, but not in VM. >>>>>>>>> Since it's >>>>>>>>> a server chipset, probably no one cares much about display >>>>>>>>> performance. >>>>>>>>> So that seemed like a good trade-off for re-using shared code. >>>>>>>>> >>>>>>>>> Part of the patch set is that the generic fb emulation now maps >>>>>>>>> and >>>>>>>>> unmaps the fbdev BO when updating the screen. I guess that's >>>>>>>>> the cause >>>>>>>>> of the performance regression. And it should be visible with other >>>>>>>>> drivers as well if they use a shadow FB for fbdev emulation. >>>>>>>> For fbcon we should need to do any maps/unamps at all, this is >>>>>>>> for the >>>>>>>> fbdev mmap support only. If the testcase mentioned here tests fbdev >>>>>>>> mmap handling it's pretty badly misnamed :-) And as long as you >>>>>>>> don't >>>>>>>> have an fbdev mmap there shouldn't be any impact at all. >>>>>>> The ast and mgag200 have only a few MiB of VRAM, so we have to >>>>>>> get the >>>>>>> fbdev BO out if it's not being displayed. If not being mapped, it >>>>>>> can be >>>>>>> evicted and make room for X, etc. >>>>>>> >>>>>>> To make this work, the BO's memory is mapped and unmapped in >>>>>>> drm_fb_helper_dirty_work() before being updated from the shadow >>>>>>> FB. [1] >>>>>>> That fbdev mapping is established on each screen update, more or >>>>>>> less. >>>>>>>  From my (yet unverified) understanding, this causes the performance >>>>>>> regression in the VM code. >>>>>>> >>>>>>> The original code in mgag200 used to kmap the fbdev BO while it's >>>>>>> being >>>>>>> displayed; [2] and the drawing code only mapped it when necessary >>>>>>> (i.e., >>>>>>> not being display). [3] >>>>>> Hm yeah, this vmap/vunmap is going to be pretty bad. We indeed should >>>>>> cache this. >>>>>> >>>>>>> I think this could be added for VRAM helpers as well, but it's >>>>>>> still a >>>>>>> workaround and non-VRAM drivers might also run into such a >>>>>>> performance >>>>>>> regression if they use the fbdev's shadow fb. >>>>>> Yeah agreed, fbdev emulation should try to cache the vmap. >>>>>> >>>>>>> Noralf mentioned that there are plans for other DRM clients >>>>>>> besides the >>>>>>> console. They would as well run into similar problems. >>>>>>> >>>>>>>>> The thing is that we'd need another generic fbdev emulation for >>>>>>>>> ast and >>>>>>>>> mgag200 that handles this issue properly. >>>>>>>> Yeah I dont think we want to jump the gun here.  If you can try to >>>>>>>> repro locally and profile where we're wasting cpu time I hope that >>>>>>>> should sched a light what's going wrong here. >>>>>>> I don't have much time ATM and I'm not even officially at work until >>>>>>> late Aug. I'd send you the revert and investigate later. I agree >>>>>>> that >>>>>>> using generic fbdev emulation would be preferable. >>>>>> Still not sure that's the right thing to do really. Yes it's a >>>>>> regression, but vm testcases shouldn run a single line of fbcon or >>>>>> drm >>>>>> code. So why this is impacted so heavily by a silly drm change is >>>>>> very >>>>>> confusing to me. We might be papering over a deeper and much more >>>>>> serious issue ... >>>>> It's a regression, the right thing is to revert first and then work >>>>> out the right thing to do. >>>> Sure, but I have no idea whether the testcase is doing something >>>> reasonable. If it's accidentally testing vm scalability of fbdev and >>>> there's no one else doing something this pointless, then it's not a >>>> real bug. Plus I think we're shooting the messenger here. >>>> >>>>> It's likely the test runs on the console and printfs stuff out >>>>> while running. >>>> But why did we not regress the world if a few prints on the console >>>> have such a huge impact? We didn't get an entire stream of mails about >>>> breaking stuff ... >>> The regression seems not related to the commit.  But we have retested >>> and confirmed the regression.  Hard to understand what happens. >> Does the regressed test cause any output on console while it's >> measuring? If so, it's probably accidentally measuring fbcon/DRM code in >> addition to the workload it's trying to measure. >> > > Sorry, I'm not familiar with DRM, we enabled the console to output logs, > and attached please find the log file. I have a patch set for fixing this problem. But I cannot reproduce the issue locally, because my machine is not for testing scalability. If I send you the patches, could you run them on the machine to test whether they solve the problem? Best regards Thomas > > "Command line: ... console=tty0 earlyprintk=ttyS0,115200 > console=ttyS0,115200 vga=normal rw" > > Best Regards, > Rong Chen > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)