2018-07-06 09:14:09

by Gerd Hoffmann

[permalink] [raw]
Subject: [PATCH] drm/cirrus: flip default to 32bpp

cirrus can handle 1024x768 (and slightly higher) with 24bpp depth.
cirrus can handle up to 800x600 with 32bpp.

The problem with 24bpp is that it is a rather unusual depth these days,
cirrus is pretty much the only relevant device still using that, and it
is a endless source of issues. Wayland doesn't support it at all. Bugs
in Xorg keep showing up.

So using 32bpp by default is the better choice IMO, even if it comes
with the drawback that the resolution is 800x600 only. But hey, better
a working 800x600 display than a broken 1024x768 display ...

Signed-off-by: Gerd Hoffmann <[email protected]>
---
drivers/gpu/drm/cirrus/cirrus_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c b/drivers/gpu/drm/cirrus/cirrus_drv.c
index 69c4e352dd..e9e5a92a36 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -16,11 +16,11 @@
#include "cirrus_drv.h"

int cirrus_modeset = -1;
-int cirrus_bpp = 24;
+int cirrus_bpp = 32;

MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
module_param_named(modeset, cirrus_modeset, int, 0400);
-MODULE_PARM_DESC(bpp, "Max bits-per-pixel (default:24)");
+MODULE_PARM_DESC(bpp, "Max bits-per-pixel (default:32)");
module_param_named(bpp, cirrus_bpp, int, 0400);

/*
--
2.9.3



2018-07-06 18:37:15

by Adam Jackson

[permalink] [raw]
Subject: Re: [PATCH] drm/cirrus: flip default to 32bpp

On Fri, 2018-07-06 at 11:12 +0200, Gerd Hoffmann wrote:
> cirrus can handle 1024x768 (and slightly higher) with 24bpp depth.
> cirrus can handle up to 800x600 with 32bpp.

16bpp is maybe a better choice? Nobody's using cirrus because they care
about color fidelity and it'll use less CPU to update. There's
precedent here, mgag200 defaults to 16bpp on sufficiently memory-
impaired devices.

- ajax

2018-07-09 07:41:50

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/cirrus: flip default to 32bpp

On Fri, Jul 06, 2018 at 02:35:07PM -0400, Adam Jackson wrote:
> On Fri, 2018-07-06 at 11:12 +0200, Gerd Hoffmann wrote:
> > cirrus can handle 1024x768 (and slightly higher) with 24bpp depth.
> > cirrus can handle up to 800x600 with 32bpp.
>
> 16bpp is maybe a better choice? Nobody's using cirrus because they care
> about color fidelity and it'll use less CPU to update. There's
> precedent here, mgag200 defaults to 16bpp on sufficiently memory-
> impaired devices.

Yeah nouveau does the same fallback to 16bpp if there's not enough vram.
So do a bunch of other drivers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2018-08-08 08:47:12

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH] drm/cirrus: flip default to 32bpp

On Mon, Jul 09, 2018 at 09:39:24AM +0200, Daniel Vetter wrote:
> On Fri, Jul 06, 2018 at 02:35:07PM -0400, Adam Jackson wrote:
> > On Fri, 2018-07-06 at 11:12 +0200, Gerd Hoffmann wrote:
> > > cirrus can handle 1024x768 (and slightly higher) with 24bpp depth.
> > > cirrus can handle up to 800x600 with 32bpp.
> >
> > 16bpp is maybe a better choice? Nobody's using cirrus because they care
> > about color fidelity and it'll use less CPU to update. There's
> > precedent here, mgag200 defaults to 16bpp on sufficiently memory-
> > impaired devices.
>
> Yeah nouveau does the same fallback to 16bpp if there's not enough vram.
> So do a bunch of other drivers.

Makes sense. But appearently 16bpp support bitrotted over time, when
just flipping the default the driver Oopses at init time. I'll look
into it.

cheers,
Gerd