On Thu, Apr 4, 2019 at 7:51 AM Gerd Hoffmann <kraxel at redhat.com>
wrote:>
> On Thu, Apr 04, 2019 at 12:58:09PM +1000, David Airlie wrote:
> > On Wed, Apr 3, 2019 at 5:23 PM Gerd Hoffmann <kraxel at
redhat.com> wrote:
> > >
> > > Time to kill some bad sample code people are copying from ;)
> > >
> > > This is a complete rewrite of the cirrus driver. The
cirrus_mode_set()
> > > function is pretty much the only function which is carried over
largely
> > > unmodified. Everything else is upside down.
> > >
> > > It is a single monster patch. But given that it does some pretty
> > > fundamental changes to the drivers workflow and also reduces the
code
> > > size by roughly 70% I think it'll still be alot easier to
review than a
> > > longish baby-step patch series.
> > >
> > > Changes summary:
> > > - Given the small amout of video memory (4 MB) the cirrus device
has
> > > the rewritten driver doesn't try to manage buffers there.
Instead
> > > it will blit (memcpy) the active framebuffer to video memory.
> >
> > Does it get any slower, with TTM I just wrote it to migrate just the
> > frontbuffer in/out of VRAM on modeset, won't we end up with more
> > copies now?
>
> I didn't benchmark it, but if you care about performance you should not
> be using cirrus anyway ...
>
> For fbcon it probably doesn't make much of a difference, fbcon used a
> shadow framebuffer before (for fbdev mmap and dirty tracking).
>
> xorg probably ends up with more copying.
>
> Anything doing display updates with page flips (i.e wayland) should end
> up with less copying, because you have one copy (blit) instead of two
> copies (migrate old frontbuffer out of vram, migrate new frontbuffer
> into vram) on pageflip.
>
> Speaking of wayland: Seems at least gnome-shell insists on using XR24.
Yeah XR24 is pretty much mandatory. Noralf added a few helpers to
convert XR24 to other formats, for display not supporting anything
else. Because userspace.
> > > - Only DRM_FORMAT_RGB565 (depth 16) is supported. The old
driver does
> > > that too by default. There was a module parameter which
enables 24/32
> > > bpp support and disables higher resolutions (due to cirrus
hardware
> > > constrains). That parameter wasn't reimplemented.
>
> > This might be the big sticking point, this is a userspace regression
> > for a feature that was explicitly added a few years ago, can we really
> > get away without it?
>
> Well, I can reintroduce the module option. I don't see any other
> reasonable way to support 32bpp. If the driver reports XR24 as
> supported and also adds the higher resolutions (which work with RG16
> only) to the mode list userspace will of course try the higher
> resolutions with XR24 and struggle ...
Maybe atomic userspace is better (it should be, it can do TEST_ONLY),
but I'm not so sure that exposing all modes for atomic clients would
work. Also, currently not possible with our probe helpers (we don't
refilter the list per client).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch