[drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Feng Tang]

Hi Thomas,

On Mon, Aug 05, 2019 at 12:22:11PM +0200, Thomas Zimmermann wrote:

	[snip] 

> >>   2019-08-03 19:29:17  ./case-anon-cow-seq-hugetlb
> >>   2019-08-03 19:29:17  ./usemem --runtime 300 -n 4 --prealloc --prefault
> >>     -O -U 815394406
> >>   917318700 bytes / 659419 usecs = 1358497 KB/s
> >>   917318700 bytes / 659658 usecs = 1358005 KB/s
> >>   917318700 bytes / 659916 usecs = 1357474 KB/s
> >>   917318700 bytes / 660168 usecs = 1356956 KB/s
> >>
> >> Rong, Feng, could you confirm this by disabling the cursor or blinking?
> > 
> > Glad to know this method restored the drop. Rong is running the case.
> > 
> > While I have another finds, as I noticed your patch changed the bpp from
> > 24 to 32, I had a patch to change it back to 24, and run the case in
> > the weekend, the -18% regrssion was reduced to about -5%. Could this
> > be related?
> 
> In the original code, the fbdev console already ran with 32 bpp [1] and
> 16 bpp was selected for low-end devices. [2][3] The patch only set the
> same values for userspace; nothing changed for the console.

I did the experiment becasue I checked the commit 

90f479ae51afa4 drm/mgag200: Replace struct mga_fbdev with generic framebuffer emulation

in which there is code:

diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index b10f726..a977333 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -162,7 +162,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
 		dev->mode_config.preferred_depth = 16;
 	else
-		dev->mode_config.preferred_depth = 24;
+		dev->mode_config.preferred_depth = 32;
 	dev->mode_config.prefer_shadow = 1;
 
My debug patch was kind of restoring of this part.

Thanks,
Feng

> 
> Best regards
> Thomas
> 
> [1]
> https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/mgag200/mgag200_fb.c?id=5d17718997367c435dbe5341a8e270d9b19478d3#n259
> [2]
> https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/mgag200/mgag200_fb.c?id=5d17718997367c435dbe5341a8e270d9b19478d3#n263
> [3]
> https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/mgag200/mgag200_fb.c?id=5d17718997367c435dbe5341a8e270d9b19478d3#n286
> 
> > 
> > commit: 
> >   f1f8555dfb9 drm/bochs: Use shadow buffer for bochs framebuffer console
> >   90f479ae51a drm/mgag200: Replace struct mga_fbdev with generic framebuffer emulation
> >   01e75fea0d5 mgag200: restore the depth back to 24
> > 
> > f1f8555dfb9a70a2 90f479ae51afa45efab97afdde9 01e75fea0d5ff39d3e588c20ec5 
> > ---------------- --------------------------- --------------------------- 
> >      43921 ±  2%     -18.3%      35884            -4.8%      41826        vm-scalability.median
> >   14889337           -17.5%   12291029            -4.1%   14278574        vm-scalability.throughput
> >  
> > commit 01e75fea0d5ff39d3e588c20ec52e7a4e6588a74
> > Author: Feng Tang <feng.tang at intel.com>
> > Date:   Fri Aug 2 15:09:19 2019 +0800
> > 
> >     mgag200: restore the depth back to 24
> >     
> >     Signed-off-by: Feng Tang <feng.tang at intel.com>
> > 
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
> > index a977333..ac8f6c9 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> > @@ -162,7 +162,7 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
> >  	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
> >  		dev->mode_config.preferred_depth = 16;
> >  	else
> > -		dev->mode_config.preferred_depth = 32;
> > +		dev->mode_config.preferred_depth = 24;>  	dev->mode_config.prefer_shadow = 1;
> >  
> >  	r = mgag200_modeset_init(mdev);
> > 
> > Thanks,
> > Feng
> > 
> >>
> >>
> >> The difference between mgag200's original fbdev support and generic
> >> fbdev emulation is generic fbdev's worker task that updates the VRAM
> >> buffer from the shadow buffer. mgag200 does this immediately, but relies
> >> on drm_can_sleep(), which is deprecated.
> >>
> >> I think that the worker task interferes with the test case, as the
> >> worker has been in fbdev emulation since forever and no performance
> >> regressions have been reported so far.
> >>
> >>
> >> So unless there's a report where this problem happens in a real-world
> >> use case, I'd like to keep code as it is. And apparently there's always
> >> the workaround of disabling the cursor blinking.
> >>
> >> Best regards
> >> Thomas
> >>
> 
> -- 
> 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)
>