Linux virtualization list
 help / color / mirror / Atom feed
* Re: [PATCH RFC 1/4] vsock/virtio: reduce credit update messages
From: Stefan Hajnoczi @ 2019-04-04 19:15 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-2-sgarzare@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 1177 bytes --]

On Thu, Apr 04, 2019 at 12:58:35PM +0200, Stefano Garzarella wrote:
> @@ -256,6 +257,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
>  	struct virtio_vsock_sock *vvs = vsk->trans;
>  	struct virtio_vsock_pkt *pkt;
>  	size_t bytes, total = 0;
> +	s64 free_space;

Why s64?  buf_alloc, fwd_cnt, and last_fwd_cnt are all u32.  fwd_cnt -
last_fwd_cnt <= buf_alloc is always true.

>  	int err = -EFAULT;
>  
>  	spin_lock_bh(&vvs->rx_lock);
> @@ -288,9 +290,15 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
>  	}
>  	spin_unlock_bh(&vvs->rx_lock);
>  
> -	/* Send a credit pkt to peer */
> -	virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM,
> -					    NULL);
> +	/* We send a credit update only when the space available seen
> +	 * by the transmitter is less than VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
> +	 */
> +	free_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt);

Locking?  These fields should be accessed under tx_lock.

> +	if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) {
> +		virtio_transport_send_credit_update(vsk,
> +						    VIRTIO_VSOCK_TYPE_STREAM,
> +						    NULL);
> +	}

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Michael S. Tsirkin @ 2019-04-04 18:04 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: kvm, netdev, linux-kernel, virtualization, Stefan Hajnoczi,
	David S. Miller
In-Reply-To: <20190404164715.sycigtccwq2rziuz@steredhat>

On Thu, Apr 04, 2019 at 06:47:15PM +0200, Stefano Garzarella wrote:
> On Thu, Apr 04, 2019 at 11:52:46AM -0400, Michael S. Tsirkin wrote:
> > I simply love it that you have analysed the individual impact of
> > each patch! Great job!
> 
> Thanks! I followed Stefan's suggestions!
> 
> > 
> > For comparison's sake, it could be IMHO benefitial to add a column
> > with virtio-net+vhost-net performance.
> > 
> > This will both give us an idea about whether the vsock layer introduces
> > inefficiencies, and whether the virtio-net idea has merit.
> > 
> 
> Sure, I already did TCP tests on virtio-net + vhost, starting qemu in
> this way:
>   $ qemu-system-x86_64 ... \
>       -netdev tap,id=net0,vhost=on,ifname=tap0,script=no,downscript=no \
>       -device virtio-net-pci,netdev=net0
> 
> I did also a test using TCP_NODELAY, just to be fair, because VSOCK
> doesn't implement something like this.

Why not?

> In both cases I set the MTU to the maximum allowed (65520).
> 
>                         VSOCK                        TCP + virtio-net + vhost
>                   host -> guest [Gbps]                 host -> guest [Gbps]
> pkt_size  before opt. patch 1 patches 2+3 patch 4     TCP_NODELAY
>   64          0.060     0.102     0.102     0.096         0.16        0.15
>   256         0.22      0.40      0.40      0.36          0.32        0.57
>   512         0.42      0.82      0.85      0.74          1.2         1.2
>   1K          0.7       1.6       1.6       1.5           2.1         2.1
>   2K          1.5       3.0       3.1       2.9           3.5         3.4
>   4K          2.5       5.2       5.3       5.3           5.5         5.3
>   8K          3.9       8.4       8.6       8.8           8.0         7.9
>   16K         6.6      11.1      11.3      12.8           9.8        10.2
>   32K         9.9      15.8      15.8      18.1          11.8        10.7
>   64K        13.5      17.4      17.7      21.4          11.4        11.3
>   128K       17.9      19.0      19.0      23.6          11.2        11.0
>   256K       18.0      19.4      19.8      24.4          11.1        11.0
>   512K       18.4      19.6      20.1      25.3          10.1        10.7
> 
> For small packet size (< 4K) I think we should implement some kind of
> batching/merging, that could be for free if we use virtio-net as a transport.
> 
> Note: Maybe I have something miss configured because TCP on virtio-net
> for host -> guest case doesn't exceed 11 Gbps.
> 
>                         VSOCK                        TCP + virtio-net + vhost
>                   guest -> host [Gbps]                 guest -> host [Gbps]
> pkt_size  before opt. patch 1 patches 2+3             TCP_NODELAY
>   64          0.088     0.100     0.101                   0.24        0.24
>   256         0.35      0.36      0.41                    0.36        1.03
>   512         0.70      0.74      0.73                    0.69        1.6
>   1K          1.1       1.3       1.3                     1.1         3.0
>   2K          2.4       2.4       2.6                     2.1         5.5
>   4K          4.3       4.3       4.5                     3.8         8.8
>   8K          7.3       7.4       7.6                     6.6        20.0
>   16K         9.2       9.6      11.1                    12.3        29.4
>   32K         8.3       8.9      18.1                    19.3        28.2
>   64K         8.3       8.9      25.4                    20.6        28.7
>   128K        7.2       8.7      26.7                    23.1        27.9
>   256K        7.7       8.4      24.9                    28.5        29.4
>   512K        7.7       8.5      25.0                    28.3        29.3
> 
> For guest -> host I think is important the TCP_NODELAY test, because TCP
> buffering increases a lot the throughput.
> 
> > One other comment: it makes sense to test with disabling smap
> > mitigations (boot host and guest with nosmap).  No problem with also
> > testing the default smap path, but I think you will discover that the
> > performance impact of smap hardening being enabled is often severe for
> > such benchmarks.
> 
> Thanks for this valuable suggestion, I'll redo all the tests with nosmap!
> 
> Cheers,
> Stefano

^ permalink raw reply

* Re: [PATCH v2 6/6] drm/cirrus: rewrite and modernize driver.
From: Sam Ravnborg @ 2019-04-04 17:41 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: David Airlie, Dave Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
In-Reply-To: <20190404152430.8263-7-kraxel@redhat.com>

Hi Gerd.

Very nice diffstat - good work indeed!

> I think it'll still be alot easier to review than a
> longish baby-step patch series.
Agreed.

Some random nits below.
With the relevant parts addressed you can add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

> new file mode 100644
> index 000000000000..5060e3d854d3
> --- /dev/null
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -0,0 +1,621 @@
> +/*
> + * Copyright 2012-2019 Red Hat
> + *
> + * This file is subject to the terms and conditions of the GNU General
> + * Public License version 2. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Authors: Matthew Garrett
> + *	    Dave Airlie
> + *	    Gerd Hoffmann
> + *
> + * Portions of this code derived from cirrusfb.c:
> + * drivers/video/cirrusfb.c - driver for Cirrus Logic chipsets
> + *
> + * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
> + */
Can we introduce an SPDX identifier?
(I am not good at the license stuff, so I cannot tell)

> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/console.h>
> +
> +#include <video/vga.h>
> +#include <video/cirrus.h>
> +
> +#include <drm/drm_drv.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_ioctl.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_connector.h>
> +
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_atomic_state_helper.h>
Please sort again, you added a few new includes since last time

> +struct cirrus_device {
> +	struct drm_device	       dev;
> +	struct drm_simple_display_pipe pipe;
> +	struct drm_connector	       conn;
> +	unsigned int		       cpp;
> +	unsigned int		       pitch;
> +	void __iomem		       *vram;
> +	void __iomem		       *mmio;
> +};
> +
> +/* ------------------------------------------------------------------ */
> +/*
> + * The meat of this driver. The core passes us a mode and we have to program
> + * it. The modesetting here is the bare minimum required to satisfy the qemu
> + * emulation of this hardware, and running this against a real device is
> + * likely to result in an inadequately programmed mode. We've already had
> + * the opportunity to modify the mode, so whatever we receive here should
> + * be something that can be correctly programmed and displayed
> + */
> +
> +#define RREG8(reg) ioread8(((void __iomem *)cirrus->mmio) + (reg))
> +#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cirrus->mmio) + (reg))
> +#define RREG32(reg) ioread32(((void __iomem *)cirrus->mmio) + (reg))
> +#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cirrus->mmio) + (reg))

The cast of cirrus->mmio to (void __iomem *) should not be necessary as
this is the type is has in struct cirrus_device

There is not reason to use a define, use can use a static inline function

> +
> +#define SEQ_INDEX 4
> +#define SEQ_DATA 5
> +
> +#define WREG_SEQ(reg, v)					\
> +	do {							\
> +		WREG8(SEQ_INDEX, reg);				\
> +		WREG8(SEQ_DATA, v);				\
> +	} while (0)						\
This is only used once, drop the define?


> +#define CRT_INDEX 0x14
> +#define CRT_DATA 0x15
> +
> +#define WREG_CRT(reg, v)					\
> +	do {							\
> +		WREG8(CRT_INDEX, reg);				\
> +		WREG8(CRT_DATA, v);				\
> +	} while (0)						\
static inline?

> +#define GFX_INDEX 0xe
> +#define GFX_DATA 0xf
> +
> +#define WREG_GFX(reg, v)					\
> +	do {							\
> +		WREG8(GFX_INDEX, reg);				\
> +		WREG8(GFX_DATA, v);				\
> +	} while (0)						\
Used twice - drop?
> +
> +#define VGA_DAC_MASK 0x6
> +
> +#define WREG_HDR(v)						\
> +	do {							\
> +		RREG8(VGA_DAC_MASK);				\
> +		RREG8(VGA_DAC_MASK);				\
> +		RREG8(VGA_DAC_MASK);				\
> +		RREG8(VGA_DAC_MASK);				\
> +		WREG8(VGA_DAC_MASK, v);				\
> +	} while (0)						\
Used once, drop?

> +static int cirrus_convert_to(struct drm_framebuffer *fb)
> +{
> +	if (fb->format->cpp[0] == 4 && fb->pitches[0] > CIRRUS_MAX_PITCH) {
> +		if (fb->width * 3 <= CIRRUS_MAX_PITCH)
> +			/* convert from XR24 to RG24 */
> +			return 3;
> +		else
> +			/* convert from XR24 to RG16 */
> +			return 2;
> +	}
> +	return 0;
> +}
> +
> +static int cirrus_cpp(struct drm_framebuffer *fb)
> +{
> +	int convert_cpp = cirrus_convert_to(fb);
> +
> +	if (convert_cpp)
> +		return convert_cpp;
> +	return fb->format->cpp[0];
> +}
> +
> +static int cirrus_pitch(struct drm_framebuffer *fb)
> +{
> +	int convert_cpp = cirrus_convert_to(fb);
> +
> +	if (convert_cpp)
> +		return convert_cpp * fb->width;
> +	return fb->pitches[0];
> +}
> +
> +static int cirrus_mode_set(struct cirrus_device *cirrus,
> +			   struct drm_display_mode *mode,
> +			   struct drm_framebuffer *fb)
> +{
> +	int hsyncstart, hsyncend, htotal, hdispend;
> +	int vtotal, vdispend;
> +	int tmp;
> +	int sr07 = 0, hdr = 0;
> +
> +	htotal = mode->htotal / 8;
> +	hsyncend = mode->hsync_end / 8;
> +	hsyncstart = mode->hsync_start / 8;
> +	hdispend = mode->hdisplay / 8;
> +
> +	vtotal = mode->vtotal;
> +	vdispend = mode->vdisplay;
> +
> +	vdispend -= 1;
> +	vtotal -= 2;
> +
> +	htotal -= 5;
> +	hdispend -= 1;
> +	hsyncstart += 1;
> +	hsyncend += 1;
> +
> +	WREG_CRT(VGA_CRTC_V_SYNC_END, 0x20);
> +	WREG_CRT(VGA_CRTC_H_TOTAL, htotal);
> +	WREG_CRT(VGA_CRTC_H_DISP, hdispend);
> +	WREG_CRT(VGA_CRTC_H_SYNC_START, hsyncstart);
> +	WREG_CRT(VGA_CRTC_H_SYNC_END, hsyncend);
> +	WREG_CRT(VGA_CRTC_V_TOTAL, vtotal & 0xff);
> +	WREG_CRT(VGA_CRTC_V_DISP_END, vdispend & 0xff);
> +
> +	tmp = 0x40;
> +	if ((vdispend + 1) & 512)
> +		tmp |= 0x20;
> +	WREG_CRT(VGA_CRTC_MAX_SCAN, tmp);
> +
> +	/*
> +	 * Overflow bits for values that don't fit in the standard registers
> +	 */
> +	tmp = 16;
> +	if (vtotal & 256)
> +		tmp |= 1;
> +	if (vdispend & 256)
> +		tmp |= 2;
> +	if ((vdispend + 1) & 256)
> +		tmp |= 8;
> +	if (vtotal & 512)
> +		tmp |= 32;
> +	if (vdispend & 512)
> +		tmp |= 64;
> +	WREG_CRT(VGA_CRTC_OVERFLOW, tmp);
> +
> +	tmp = 0;
> +
> +	/* More overflow bits */
> +
> +	if ((htotal + 5) & 64)
> +		tmp |= 16;
> +	if ((htotal + 5) & 128)
> +		tmp |= 32;
> +	if (vtotal & 256)
> +		tmp |= 64;
> +	if (vtotal & 512)
> +		tmp |= 128;
For bit operations / numbers above consider to hexadecimal numbers to increase readability.

> +
> +	WREG_CRT(CL_CRT1A, tmp);
> +
> +	/* Disable Hercules/CGA compatibility */
> +	WREG_CRT(VGA_CRTC_MODE, 0x03);
> +
> +	WREG8(SEQ_INDEX, 0x7);
> +	sr07 = RREG8(SEQ_DATA);
> +	sr07 &= 0xe0;
> +	hdr = 0;
> +
> +	cirrus->cpp = cirrus_cpp(fb);
> +	switch (cirrus->cpp * 8) {
> +	case 8:
> +		sr07 |= 0x11;
> +		break;
> +	case 16:
> +		sr07 |= 0x17;
> +		hdr = 0xc1;
> +		break;
> +	case 24:
> +		sr07 |= 0x15;
> +		hdr = 0xc5;
> +		break;
> +	case 32:
> +		sr07 |= 0x19;
> +		hdr = 0xc5;
> +		break;
> +	default:
> +		return -1;
> +	}
> +
> +	WREG_SEQ(0x7, sr07);
> +
> +	/* Program the pitch */
> +	cirrus->pitch = cirrus_pitch(fb);
> +	tmp = cirrus->pitch / 8;
> +	WREG_CRT(VGA_CRTC_OFFSET, tmp);
> +
> +	/* Enable extended blanking and pitch bits, and enable full memory */
> +	tmp = 0x22;
> +	tmp |= (cirrus->pitch >> 7) & 0x10;
> +	tmp |= (cirrus->pitch >> 6) & 0x40;
> +	WREG_CRT(0x1b, tmp);
> +
> +	/* Enable high-colour modes */
> +	WREG_GFX(VGA_GFX_MODE, 0x40);
> +
> +	/* And set graphics mode */
> +	WREG_GFX(VGA_GFX_MISC, 0x01);
> +
> +	WREG_HDR(hdr);
> +	/* cirrus_crtc_do_set_base(crtc, old_fb, x, y, 0); */
> +
> +	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
> +	outb(0x20, 0x3c0);
> +	return 0;
> +}
> +
> +static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
> +			       struct drm_plane_state *old_state)
> +{
> +	struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_crtc *crtc = &pipe->crtc;
> +	struct drm_rect rect;
> +
> +	if (pipe->plane.state->fb &&
> +	    cirrus->cpp != cirrus_cpp(pipe->plane.state->fb))
> +		cirrus_mode_set(cirrus, &crtc->mode,
> +				pipe->plane.state->fb);
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		cirrus_fb_blit_rect(pipe->plane.state->fb, &rect);
> +
> +	if (crtc->state->event) {
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +		crtc->state->event = NULL;
Should you set crtc->state->event = NULL; before spin_unlock_irq()?

> +
> +/* only bind to the cirrus chip in qemu */
> +static const struct pci_device_id pciidlist[] = {
> +	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
> +	  PCI_SUBVENDOR_ID_REDHAT_QUMRANET, PCI_SUBDEVICE_ID_QEMU,
> +	  0, 0, 0 },

	PCI_DEVICE_SUB(PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
		       PCI_SUBVENDOR_ID_REDHAT_QUMRANET, PCI_SUBDEVICE_ID_QEMU)
????
Hmm, looks like an alternative way to say the same, that is hardly much improvement?!?


> +	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
> +	  PCI_VENDOR_ID_XEN, 0x0001,
> +	  0, 0, 0 },
> +	{ /* end if list */}
Add space before final } 

^ permalink raw reply

* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Stefano Garzarella @ 2019-04-04 16:47 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kvm, netdev, linux-kernel, virtualization, Stefan Hajnoczi,
	David S. Miller
In-Reply-To: <20190404114643-mutt-send-email-mst@kernel.org>

On Thu, Apr 04, 2019 at 11:52:46AM -0400, Michael S. Tsirkin wrote:
> I simply love it that you have analysed the individual impact of
> each patch! Great job!

Thanks! I followed Stefan's suggestions!

> 
> For comparison's sake, it could be IMHO benefitial to add a column
> with virtio-net+vhost-net performance.
> 
> This will both give us an idea about whether the vsock layer introduces
> inefficiencies, and whether the virtio-net idea has merit.
> 

Sure, I already did TCP tests on virtio-net + vhost, starting qemu in
this way:
  $ qemu-system-x86_64 ... \
      -netdev tap,id=net0,vhost=on,ifname=tap0,script=no,downscript=no \
      -device virtio-net-pci,netdev=net0

I did also a test using TCP_NODELAY, just to be fair, because VSOCK
doesn't implement something like this.
In both cases I set the MTU to the maximum allowed (65520).

                        VSOCK                        TCP + virtio-net + vhost
                  host -> guest [Gbps]                 host -> guest [Gbps]
pkt_size  before opt. patch 1 patches 2+3 patch 4     TCP_NODELAY
  64          0.060     0.102     0.102     0.096         0.16        0.15
  256         0.22      0.40      0.40      0.36          0.32        0.57
  512         0.42      0.82      0.85      0.74          1.2         1.2
  1K          0.7       1.6       1.6       1.5           2.1         2.1
  2K          1.5       3.0       3.1       2.9           3.5         3.4
  4K          2.5       5.2       5.3       5.3           5.5         5.3
  8K          3.9       8.4       8.6       8.8           8.0         7.9
  16K         6.6      11.1      11.3      12.8           9.8        10.2
  32K         9.9      15.8      15.8      18.1          11.8        10.7
  64K        13.5      17.4      17.7      21.4          11.4        11.3
  128K       17.9      19.0      19.0      23.6          11.2        11.0
  256K       18.0      19.4      19.8      24.4          11.1        11.0
  512K       18.4      19.6      20.1      25.3          10.1        10.7

For small packet size (< 4K) I think we should implement some kind of
batching/merging, that could be for free if we use virtio-net as a transport.

Note: Maybe I have something miss configured because TCP on virtio-net
for host -> guest case doesn't exceed 11 Gbps.

                        VSOCK                        TCP + virtio-net + vhost
                  guest -> host [Gbps]                 guest -> host [Gbps]
pkt_size  before opt. patch 1 patches 2+3             TCP_NODELAY
  64          0.088     0.100     0.101                   0.24        0.24
  256         0.35      0.36      0.41                    0.36        1.03
  512         0.70      0.74      0.73                    0.69        1.6
  1K          1.1       1.3       1.3                     1.1         3.0
  2K          2.4       2.4       2.6                     2.1         5.5
  4K          4.3       4.3       4.5                     3.8         8.8
  8K          7.3       7.4       7.6                     6.6        20.0
  16K         9.2       9.6      11.1                    12.3        29.4
  32K         8.3       8.9      18.1                    19.3        28.2
  64K         8.3       8.9      25.4                    20.6        28.7
  128K        7.2       8.7      26.7                    23.1        27.9
  256K        7.7       8.4      24.9                    28.5        29.4
  512K        7.7       8.5      25.0                    28.3        29.3

For guest -> host I think is important the TCP_NODELAY test, because TCP
buffering increases a lot the throughput.

> One other comment: it makes sense to test with disabling smap
> mitigations (boot host and guest with nosmap).  No problem with also
> testing the default smap path, but I think you will discover that the
> performance impact of smap hardening being enabled is often severe for
> such benchmarks.

Thanks for this valuable suggestion, I'll redo all the tests with nosmap!

Cheers,
Stefano

^ permalink raw reply

* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Michael S. Tsirkin @ 2019-04-04 15:52 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: kvm, netdev, linux-kernel, virtualization, Stefan Hajnoczi,
	David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>

On Thu, Apr 04, 2019 at 12:58:34PM +0200, Stefano Garzarella wrote:
> This series tries to increase the throughput of virtio-vsock with slight
> changes:
>  - patch 1/4: reduces the number of credit update messages sent to the
>               transmitter
>  - patch 2/4: allows the host to split packets on multiple buffers,
>               in this way, we can remove the packet size limit to
>               VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE
>  - patch 3/4: uses VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size
>               allowed
>  - patch 4/4: increases RX buffer size to 64 KiB (affects only host->guest)
> 
> RFC:
>  - maybe patch 4 can be replaced with multiple queues with different
>    buffer sizes or using EWMA to adapt the buffer size to the traffic
> 
>  - as Jason suggested in a previous thread [1] I'll evaluate to use
>    virtio-net as transport, but I need to understand better how to
>    interface with it, maybe introducing sk_buff in virtio-vsock.
> 
> Any suggestions?
> 
> Here some benchmarks step by step. I used iperf3 [2] modified with VSOCK
> support:
> 
>                         host -> guest [Gbps]
> pkt_size    before opt.   patch 1   patches 2+3   patch 4
>   64            0.060       0.102       0.102       0.096
>   256           0.22        0.40        0.40        0.36
>   512           0.42        0.82        0.85        0.74
>   1K            0.7         1.6         1.6         1.5
>   2K            1.5         3.0         3.1         2.9
>   4K            2.5         5.2         5.3         5.3
>   8K            3.9         8.4         8.6         8.8
>   16K           6.6        11.1        11.3        12.8
>   32K           9.9        15.8        15.8        18.1
>   64K          13.5        17.4        17.7        21.4
>   128K         17.9        19.0        19.0        23.6
>   256K         18.0        19.4        19.8        24.4
>   512K         18.4        19.6        20.1        25.3
> 
>                         guest -> host [Gbps]
> pkt_size    before opt.   patch 1   patches 2+3
>   64            0.088       0.100       0.101
>   256           0.35        0.36        0.41
>   512           0.70        0.74        0.73
>   1K            1.1         1.3         1.3
>   2K            2.4         2.4         2.6
>   4K            4.3         4.3         4.5
>   8K            7.3         7.4         7.6
>   16K           9.2         9.6        11.1
>   32K           8.3         8.9        18.1
>   64K           8.3         8.9        25.4
>   128K          7.2         8.7        26.7
>   256K          7.7         8.4        24.9
>   512K          7.7         8.5        25.0
> 
> Thanks,
> Stefano

I simply love it that you have analysed the individual impact of
each patch! Great job!

For comparison's sake, it could be IMHO benefitial to add a column
with virtio-net+vhost-net performance.

This will both give us an idea about whether the vsock layer introduces
inefficiencies, and whether the virtio-net idea has merit.

One other comment: it makes sense to test with disabling smap
mitigations (boot host and guest with nosmap).  No problem with also
testing the default smap path, but I think you will discover that the
performance impact of smap hardening being enabled is often severe for
such benchmarks.


> [1] https://www.spinics.net/lists/netdev/msg531783.html
> [2] https://github.com/stefano-garzarella/iperf/
> 
> Stefano Garzarella (4):
>   vsock/virtio: reduce credit update messages
>   vhost/vsock: split packets to send using multiple buffers
>   vsock/virtio: change the maximum packet size allowed
>   vsock/virtio: increase RX buffer size to 64 KiB
> 
>  drivers/vhost/vsock.c                   | 35 ++++++++++++++++++++-----
>  include/linux/virtio_vsock.h            |  3 ++-
>  net/vmw_vsock/virtio_transport_common.c | 18 +++++++++----
>  3 files changed, 44 insertions(+), 12 deletions(-)
> 
> -- 
> 2.20.1

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Pankaj Gupta @ 2019-04-04 15:50 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Jan Kara, kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, cohuck, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <20190404150025.GN5147@magnolia>


> > > 
> > > So I would put the helper in include/linux/dax.h and have it like:
> > > 
> > > bool daxdev_mapping_supported(struct vm_area_struct *vma,
> 
> Should this be static inline if you're putting it in the header file?

yes. Thanks.

> 
> A comment ought to be added to describe what this predicate function
> does.

Sure, will add a comment describing the function.

> 
> > > 			      struct dax_device *dax_dev)
> > > {
> > > 	if (!(vma->vm_flags & VM_SYNC))
> > > 		return true;
> > > 	if (!IS_DAX(file_inode(vma->vm_file)))
> > > 		return false;
> > > 	return dax_synchronous(dax_dev);
> > > }
> > 
> > Sure. This is much better. I was also not sure what to name the helper
> > function.
> > I will go ahead with this unless 'Dave' & 'Darrick' have anything to add.
> 
> Jan's approach (modulo that one comment) looks good to me.

Sure. Thank you.

Best regards,
Pankaj

> 
> --D
> 

^ permalink raw reply

* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Stefano Garzarella @ 2019-04-04 15:44 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404141410.GA25152@stefanha-x1.localdomain>

On Thu, Apr 04, 2019 at 03:14:10PM +0100, Stefan Hajnoczi wrote:
> On Thu, Apr 04, 2019 at 12:58:34PM +0200, Stefano Garzarella wrote:
> > This series tries to increase the throughput of virtio-vsock with slight
> > changes:
> >  - patch 1/4: reduces the number of credit update messages sent to the
> >               transmitter
> >  - patch 2/4: allows the host to split packets on multiple buffers,
> >               in this way, we can remove the packet size limit to
> >               VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE
> >  - patch 3/4: uses VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size
> >               allowed
> >  - patch 4/4: increases RX buffer size to 64 KiB (affects only host->guest)
> > 
> > RFC:
> >  - maybe patch 4 can be replaced with multiple queues with different
> >    buffer sizes or using EWMA to adapt the buffer size to the traffic
> > 
> >  - as Jason suggested in a previous thread [1] I'll evaluate to use
> >    virtio-net as transport, but I need to understand better how to
> >    interface with it, maybe introducing sk_buff in virtio-vsock.
> > 
> > Any suggestions?
> 
> Great performance results, nice job!

:)

> 
> Please include efficiency numbers (bandwidth / CPU utilization) in the
> future.  Due to the nature of these optimizations it's unlikely that
> efficiency has decreased, so I'm not too worried about it this time.

Thanks for the suggestion! I'll measure also the efficiency for future
optimizations.

Cheers,
Stefano

^ permalink raw reply

* [PATCH v2 6/6] drm/cirrus: rewrite and modernize driver.
From: Gerd Hoffmann @ 2019-04-04 15:24 UTC (permalink / raw)
  To: dri-devel
  Cc: David Airlie, open list,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE, Daniel Vetter,
	Dave Airlie
In-Reply-To: <20190404152430.8263-1-kraxel@redhat.com>

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.
 - All gem objects are stored in main memory and are manged using the
   new shmem helpers.  ttm is out.
 - It supports RG16, RG24 and XR24 formats.  XR24 gets converted to RG24
   or RG16 at blit time if needed, to avoid the pitch becoming larger
   than what the cirrus hardware can handle.
 - The simple display pipeline is used.
 - The generic fbdev emulation is used.
 - It's a atomic driver now.
 - It runs wayland.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/cirrus/cirrus_drv.h   | 251 -----------
 drivers/gpu/drm/cirrus/cirrus.c       | 621 ++++++++++++++++++++++++++
 drivers/gpu/drm/cirrus/cirrus_drv.c   | 161 -------
 drivers/gpu/drm/cirrus/cirrus_fbdev.c | 309 -------------
 drivers/gpu/drm/cirrus/cirrus_main.c  | 328 --------------
 drivers/gpu/drm/cirrus/cirrus_mode.c  | 617 -------------------------
 drivers/gpu/drm/cirrus/cirrus_ttm.c   | 343 --------------
 drivers/gpu/drm/cirrus/Kconfig        |   2 +-
 drivers/gpu/drm/cirrus/Makefile       |   3 -
 9 files changed, 622 insertions(+), 2013 deletions(-)
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.h
 create mode 100644 drivers/gpu/drm/cirrus/cirrus.c
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_drv.c
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_fbdev.c
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_main.c
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_mode.c
 delete mode 100644 drivers/gpu/drm/cirrus/cirrus_ttm.c

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h
deleted file mode 100644
index 828b150cdb20..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ /dev/null
@@ -1,251 +0,0 @@
-/*
- * Copyright 2012 Red Hat
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- *          Dave Airlie
- */
-#ifndef __CIRRUS_DRV_H__
-#define __CIRRUS_DRV_H__
-
-#include <video/vga.h>
-
-#include <drm/drm_encoder.h>
-#include <drm/drm_fb_helper.h>
-
-#include <drm/ttm/ttm_bo_api.h>
-#include <drm/ttm/ttm_bo_driver.h>
-#include <drm/ttm/ttm_placement.h>
-#include <drm/ttm/ttm_memory.h>
-#include <drm/ttm/ttm_module.h>
-
-#include <drm/drm_gem.h>
-
-#define DRIVER_AUTHOR		"Matthew Garrett"
-
-#define DRIVER_NAME		"cirrus"
-#define DRIVER_DESC		"qemu Cirrus emulation"
-#define DRIVER_DATE		"20110418"
-
-#define DRIVER_MAJOR		1
-#define DRIVER_MINOR		0
-#define DRIVER_PATCHLEVEL	0
-
-#define CIRRUSFB_CONN_LIMIT 1
-
-#define RREG8(reg) ioread8(((void __iomem *)cdev->rmmio) + (reg))
-#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cdev->rmmio) + (reg))
-#define RREG32(reg) ioread32(((void __iomem *)cdev->rmmio) + (reg))
-#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cdev->rmmio) + (reg))
-
-#define SEQ_INDEX 4
-#define SEQ_DATA 5
-
-#define WREG_SEQ(reg, v)					\
-	do {							\
-		WREG8(SEQ_INDEX, reg);				\
-		WREG8(SEQ_DATA, v);				\
-	} while (0)						\
-
-#define CRT_INDEX 0x14
-#define CRT_DATA 0x15
-
-#define WREG_CRT(reg, v)					\
-	do {							\
-		WREG8(CRT_INDEX, reg);				\
-		WREG8(CRT_DATA, v);				\
-	} while (0)						\
-
-#define GFX_INDEX 0xe
-#define GFX_DATA 0xf
-
-#define WREG_GFX(reg, v)					\
-	do {							\
-		WREG8(GFX_INDEX, reg);				\
-		WREG8(GFX_DATA, v);				\
-	} while (0)						\
-
-/*
- * Cirrus has a "hidden" DAC register that can be accessed by writing to
- * the pixel mask register to reset the state, then reading from the register
- * four times. The next write will then pass to the DAC
- */
-#define VGA_DAC_MASK 0x6
-
-#define WREG_HDR(v)						\
-	do {							\
-		RREG8(VGA_DAC_MASK);					\
-		RREG8(VGA_DAC_MASK);					\
-		RREG8(VGA_DAC_MASK);					\
-		RREG8(VGA_DAC_MASK);					\
-		WREG8(VGA_DAC_MASK, v);					\
-	} while (0)						\
-
-
-#define CIRRUS_MAX_FB_HEIGHT 4096
-#define CIRRUS_MAX_FB_WIDTH 4096
-
-#define CIRRUS_DPMS_CLEARED (-1)
-
-#define to_cirrus_crtc(x) container_of(x, struct cirrus_crtc, base)
-#define to_cirrus_encoder(x) container_of(x, struct cirrus_encoder, base)
-
-struct cirrus_crtc {
-	struct drm_crtc			base;
-	int				last_dpms;
-	bool				enabled;
-};
-
-struct cirrus_fbdev;
-struct cirrus_mode_info {
-	struct cirrus_crtc		*crtc;
-	/* pointer to fbdev info structure */
-	struct cirrus_fbdev		*gfbdev;
-};
-
-struct cirrus_encoder {
-	struct drm_encoder		base;
-	int				last_dpms;
-};
-
-struct cirrus_connector {
-	struct drm_connector		base;
-};
-
-struct cirrus_mc {
-	resource_size_t			vram_size;
-	resource_size_t			vram_base;
-};
-
-struct cirrus_device {
-	struct drm_device		*dev;
-	unsigned long			flags;
-
-	resource_size_t			rmmio_base;
-	resource_size_t			rmmio_size;
-	void __iomem			*rmmio;
-
-	struct cirrus_mc			mc;
-	struct cirrus_mode_info		mode_info;
-
-	int				num_crtc;
-	int fb_mtrr;
-
-	struct {
-		struct ttm_bo_device bdev;
-	} ttm;
-	bool mm_inited;
-};
-
-
-struct cirrus_fbdev {
-	struct drm_fb_helper helper; /* must be first */
-	struct drm_framebuffer *gfb;
-	void *sysram;
-	int size;
-	int x1, y1, x2, y2; /* dirty rect */
-	spinlock_t dirty_lock;
-};
-
-struct cirrus_bo {
-	struct ttm_buffer_object bo;
-	struct ttm_placement placement;
-	struct ttm_bo_kmap_obj kmap;
-	struct drm_gem_object gem;
-	struct ttm_place placements[3];
-	int pin_count;
-};
-#define gem_to_cirrus_bo(gobj) container_of((gobj), struct cirrus_bo, gem)
-
-static inline struct cirrus_bo *
-cirrus_bo(struct ttm_buffer_object *bo)
-{
-	return container_of(bo, struct cirrus_bo, bo);
-}
-
-
-#define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
-				/* cirrus_main.c */
-int cirrus_device_init(struct cirrus_device *cdev,
-		      struct drm_device *ddev,
-		      struct pci_dev *pdev,
-		      uint32_t flags);
-void cirrus_device_fini(struct cirrus_device *cdev);
-void cirrus_gem_free_object(struct drm_gem_object *obj);
-int cirrus_dumb_mmap_offset(struct drm_file *file,
-			    struct drm_device *dev,
-			    uint32_t handle,
-			    uint64_t *offset);
-int cirrus_gem_create(struct drm_device *dev,
-		   u32 size, bool iskernel,
-		      struct drm_gem_object **obj);
-int cirrus_dumb_create(struct drm_file *file,
-		    struct drm_device *dev,
-		       struct drm_mode_create_dumb *args);
-
-int cirrus_framebuffer_init(struct drm_device *dev,
-			    struct drm_framebuffer *gfb,
-			    const struct drm_mode_fb_cmd2 *mode_cmd,
-			    struct drm_gem_object *obj);
-
-bool cirrus_check_framebuffer(struct cirrus_device *cdev, int width, int height,
-			      int bpp, int pitch);
-
-				/* cirrus_display.c */
-int cirrus_modeset_init(struct cirrus_device *cdev);
-void cirrus_modeset_fini(struct cirrus_device *cdev);
-
-				/* cirrus_fbdev.c */
-int cirrus_fbdev_init(struct cirrus_device *cdev);
-void cirrus_fbdev_fini(struct cirrus_device *cdev);
-
-
-
-				/* cirrus_irq.c */
-void cirrus_driver_irq_preinstall(struct drm_device *dev);
-int cirrus_driver_irq_postinstall(struct drm_device *dev);
-void cirrus_driver_irq_uninstall(struct drm_device *dev);
-irqreturn_t cirrus_driver_irq_handler(int irq, void *arg);
-
-				/* cirrus_kms.c */
-int cirrus_driver_load(struct drm_device *dev, unsigned long flags);
-void cirrus_driver_unload(struct drm_device *dev);
-extern struct drm_ioctl_desc cirrus_ioctls[];
-extern int cirrus_max_ioctl;
-
-int cirrus_mm_init(struct cirrus_device *cirrus);
-void cirrus_mm_fini(struct cirrus_device *cirrus);
-void cirrus_ttm_placement(struct cirrus_bo *bo, int domain);
-int cirrus_bo_create(struct drm_device *dev, int size, int align,
-		     uint32_t flags, struct cirrus_bo **pcirrusbo);
-int cirrus_mmap(struct file *filp, struct vm_area_struct *vma);
-
-static inline int cirrus_bo_reserve(struct cirrus_bo *bo, bool no_wait)
-{
-	int ret;
-
-	ret = ttm_bo_reserve(&bo->bo, true, no_wait, NULL);
-	if (ret) {
-		if (ret != -ERESTARTSYS && ret != -EBUSY)
-			DRM_ERROR("reserve failed %p\n", bo);
-		return ret;
-	}
-	return 0;
-}
-
-static inline void cirrus_bo_unreserve(struct cirrus_bo *bo)
-{
-	ttm_bo_unreserve(&bo->bo);
-}
-
-int cirrus_bo_push_sysram(struct cirrus_bo *bo);
-int cirrus_bo_pin(struct cirrus_bo *bo, u32 pl_flag, u64 *gpu_addr);
-
-extern int cirrus_bpp;
-
-#endif				/* __CIRRUS_DRV_H__ */
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
new file mode 100644
index 000000000000..5060e3d854d3
--- /dev/null
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -0,0 +1,621 @@
+/*
+ * Copyright 2012-2019 Red Hat
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License version 2. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * Authors: Matthew Garrett
+ *	    Dave Airlie
+ *	    Gerd Hoffmann
+ *
+ * Portions of this code derived from cirrusfb.c:
+ * drivers/video/cirrusfb.c - driver for Cirrus Logic chipsets
+ *
+ * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/console.h>
+
+#include <video/vga.h>
+#include <video/cirrus.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_file.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_connector.h>
+
+#include <drm/drm_fourcc.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_state_helper.h>
+
+#define DRIVER_NAME "cirrus"
+#define DRIVER_DESC "qemu cirrus vga"
+#define DRIVER_DATE "2019"
+#define DRIVER_MAJOR 2
+#define DRIVER_MINOR 0
+
+#define CIRRUS_MAX_PITCH (0x1FF << 3)      /* (4096 - 1) & ~111b bytes */
+#define CIRRUS_VRAM_SIZE (4 * 1024 * 1024) /* 4 MB */
+
+struct cirrus_device {
+	struct drm_device	       dev;
+	struct drm_simple_display_pipe pipe;
+	struct drm_connector	       conn;
+	unsigned int		       cpp;
+	unsigned int		       pitch;
+	void __iomem		       *vram;
+	void __iomem		       *mmio;
+};
+
+/* ------------------------------------------------------------------ */
+/*
+ * The meat of this driver. The core passes us a mode and we have to program
+ * it. The modesetting here is the bare minimum required to satisfy the qemu
+ * emulation of this hardware, and running this against a real device is
+ * likely to result in an inadequately programmed mode. We've already had
+ * the opportunity to modify the mode, so whatever we receive here should
+ * be something that can be correctly programmed and displayed
+ */
+
+#define RREG8(reg) ioread8(((void __iomem *)cirrus->mmio) + (reg))
+#define WREG8(reg, v) iowrite8(v, ((void __iomem *)cirrus->mmio) + (reg))
+#define RREG32(reg) ioread32(((void __iomem *)cirrus->mmio) + (reg))
+#define WREG32(reg, v) iowrite32(v, ((void __iomem *)cirrus->mmio) + (reg))
+
+#define SEQ_INDEX 4
+#define SEQ_DATA 5
+
+#define WREG_SEQ(reg, v)					\
+	do {							\
+		WREG8(SEQ_INDEX, reg);				\
+		WREG8(SEQ_DATA, v);				\
+	} while (0)						\
+
+#define CRT_INDEX 0x14
+#define CRT_DATA 0x15
+
+#define WREG_CRT(reg, v)					\
+	do {							\
+		WREG8(CRT_INDEX, reg);				\
+		WREG8(CRT_DATA, v);				\
+	} while (0)						\
+
+#define GFX_INDEX 0xe
+#define GFX_DATA 0xf
+
+#define WREG_GFX(reg, v)					\
+	do {							\
+		WREG8(GFX_INDEX, reg);				\
+		WREG8(GFX_DATA, v);				\
+	} while (0)						\
+
+#define VGA_DAC_MASK 0x6
+
+#define WREG_HDR(v)						\
+	do {							\
+		RREG8(VGA_DAC_MASK);				\
+		RREG8(VGA_DAC_MASK);				\
+		RREG8(VGA_DAC_MASK);				\
+		RREG8(VGA_DAC_MASK);				\
+		WREG8(VGA_DAC_MASK, v);				\
+	} while (0)						\
+
+
+static int cirrus_convert_to(struct drm_framebuffer *fb)
+{
+	if (fb->format->cpp[0] == 4 && fb->pitches[0] > CIRRUS_MAX_PITCH) {
+		if (fb->width * 3 <= CIRRUS_MAX_PITCH)
+			/* convert from XR24 to RG24 */
+			return 3;
+		else
+			/* convert from XR24 to RG16 */
+			return 2;
+	}
+	return 0;
+}
+
+static int cirrus_cpp(struct drm_framebuffer *fb)
+{
+	int convert_cpp = cirrus_convert_to(fb);
+
+	if (convert_cpp)
+		return convert_cpp;
+	return fb->format->cpp[0];
+}
+
+static int cirrus_pitch(struct drm_framebuffer *fb)
+{
+	int convert_cpp = cirrus_convert_to(fb);
+
+	if (convert_cpp)
+		return convert_cpp * fb->width;
+	return fb->pitches[0];
+}
+
+static int cirrus_mode_set(struct cirrus_device *cirrus,
+			   struct drm_display_mode *mode,
+			   struct drm_framebuffer *fb)
+{
+	int hsyncstart, hsyncend, htotal, hdispend;
+	int vtotal, vdispend;
+	int tmp;
+	int sr07 = 0, hdr = 0;
+
+	htotal = mode->htotal / 8;
+	hsyncend = mode->hsync_end / 8;
+	hsyncstart = mode->hsync_start / 8;
+	hdispend = mode->hdisplay / 8;
+
+	vtotal = mode->vtotal;
+	vdispend = mode->vdisplay;
+
+	vdispend -= 1;
+	vtotal -= 2;
+
+	htotal -= 5;
+	hdispend -= 1;
+	hsyncstart += 1;
+	hsyncend += 1;
+
+	WREG_CRT(VGA_CRTC_V_SYNC_END, 0x20);
+	WREG_CRT(VGA_CRTC_H_TOTAL, htotal);
+	WREG_CRT(VGA_CRTC_H_DISP, hdispend);
+	WREG_CRT(VGA_CRTC_H_SYNC_START, hsyncstart);
+	WREG_CRT(VGA_CRTC_H_SYNC_END, hsyncend);
+	WREG_CRT(VGA_CRTC_V_TOTAL, vtotal & 0xff);
+	WREG_CRT(VGA_CRTC_V_DISP_END, vdispend & 0xff);
+
+	tmp = 0x40;
+	if ((vdispend + 1) & 512)
+		tmp |= 0x20;
+	WREG_CRT(VGA_CRTC_MAX_SCAN, tmp);
+
+	/*
+	 * Overflow bits for values that don't fit in the standard registers
+	 */
+	tmp = 16;
+	if (vtotal & 256)
+		tmp |= 1;
+	if (vdispend & 256)
+		tmp |= 2;
+	if ((vdispend + 1) & 256)
+		tmp |= 8;
+	if (vtotal & 512)
+		tmp |= 32;
+	if (vdispend & 512)
+		tmp |= 64;
+	WREG_CRT(VGA_CRTC_OVERFLOW, tmp);
+
+	tmp = 0;
+
+	/* More overflow bits */
+
+	if ((htotal + 5) & 64)
+		tmp |= 16;
+	if ((htotal + 5) & 128)
+		tmp |= 32;
+	if (vtotal & 256)
+		tmp |= 64;
+	if (vtotal & 512)
+		tmp |= 128;
+
+	WREG_CRT(CL_CRT1A, tmp);
+
+	/* Disable Hercules/CGA compatibility */
+	WREG_CRT(VGA_CRTC_MODE, 0x03);
+
+	WREG8(SEQ_INDEX, 0x7);
+	sr07 = RREG8(SEQ_DATA);
+	sr07 &= 0xe0;
+	hdr = 0;
+
+	cirrus->cpp = cirrus_cpp(fb);
+	switch (cirrus->cpp * 8) {
+	case 8:
+		sr07 |= 0x11;
+		break;
+	case 16:
+		sr07 |= 0x17;
+		hdr = 0xc1;
+		break;
+	case 24:
+		sr07 |= 0x15;
+		hdr = 0xc5;
+		break;
+	case 32:
+		sr07 |= 0x19;
+		hdr = 0xc5;
+		break;
+	default:
+		return -1;
+	}
+
+	WREG_SEQ(0x7, sr07);
+
+	/* Program the pitch */
+	cirrus->pitch = cirrus_pitch(fb);
+	tmp = cirrus->pitch / 8;
+	WREG_CRT(VGA_CRTC_OFFSET, tmp);
+
+	/* Enable extended blanking and pitch bits, and enable full memory */
+	tmp = 0x22;
+	tmp |= (cirrus->pitch >> 7) & 0x10;
+	tmp |= (cirrus->pitch >> 6) & 0x40;
+	WREG_CRT(0x1b, tmp);
+
+	/* Enable high-colour modes */
+	WREG_GFX(VGA_GFX_MODE, 0x40);
+
+	/* And set graphics mode */
+	WREG_GFX(VGA_GFX_MISC, 0x01);
+
+	WREG_HDR(hdr);
+	/* cirrus_crtc_do_set_base(crtc, old_fb, x, y, 0); */
+
+	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
+	outb(0x20, 0x3c0);
+	return 0;
+}
+
+static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
+			       struct drm_rect *rect)
+{
+	struct cirrus_device *cirrus = fb->dev->dev_private;
+	void *vmap;
+
+	vmap = drm_gem_shmem_vmap(fb->obj[0]);
+	if (!vmap)
+		return -ENOMEM;
+
+	if (cirrus->cpp == fb->format->cpp[0])
+		drm_fb_memcpy(__io_virt(cirrus->vram),
+			      vmap, fb, rect, true);
+
+	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2)
+		drm_fb_xrgb8888_to_rgb565(__io_virt(cirrus->vram),
+					  vmap, fb, rect, false, true);
+
+	else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3)
+		drm_fb_xrgb8888_to_rgb888(__io_virt(cirrus->vram),
+					  vmap, fb, rect, true);
+
+	else
+		WARN_ON_ONCE("cpp mismatch");
+
+	drm_gem_shmem_vunmap(fb->obj[0], vmap);
+	return 0;
+}
+
+static int cirrus_fb_blit_fullscreen(struct drm_framebuffer *fb)
+{
+	struct drm_rect fullscreen = {
+		.x1 = 0,
+		.x2 = fb->width,
+		.y1 = 0,
+		.y2 = fb->height,
+	};
+	return cirrus_fb_blit_rect(fb, &fullscreen);
+}
+
+static int cirrus_check_size(int width, int height,
+			     struct drm_framebuffer *fb)
+{
+	int pitch = width * 2;
+
+	if (fb)
+		pitch = cirrus_pitch(fb);
+
+	if (pitch > CIRRUS_MAX_PITCH)
+		return -EINVAL;
+	if (pitch * height > CIRRUS_VRAM_SIZE)
+		return -EINVAL;
+	return 0;
+}
+
+/* ------------------------------------------------------------------ */
+/* cirrus connector						      */
+
+static int cirrus_conn_get_modes(struct drm_connector *conn)
+{
+	int count;
+
+	count = drm_add_modes_noedid(conn,
+				     conn->dev->mode_config.max_width,
+				     conn->dev->mode_config.max_height);
+	drm_set_preferred_mode(conn, 1024, 768);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs cirrus_conn_helper_funcs = {
+	.get_modes = cirrus_conn_get_modes,
+};
+
+static const struct drm_connector_funcs cirrus_conn_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int cirrus_conn_init(struct cirrus_device *cirrus)
+{
+	drm_connector_helper_add(&cirrus->conn, &cirrus_conn_helper_funcs);
+	return drm_connector_init(&cirrus->dev, &cirrus->conn,
+				  &cirrus_conn_funcs, DRM_MODE_CONNECTOR_VGA);
+
+}
+
+/* ------------------------------------------------------------------ */
+/* cirrus (simple) display pipe					      */
+
+static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_crtc *crtc,
+						   const struct drm_display_mode *mode)
+{
+	if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
+		return MODE_BAD;
+	return MODE_OK;
+}
+
+static int cirrus_pipe_check(struct drm_simple_display_pipe *pipe,
+			     struct drm_plane_state *plane_state,
+			     struct drm_crtc_state *crtc_state)
+{
+	struct drm_framebuffer *fb = plane_state->fb;
+
+	if (!fb)
+		return 0;
+	return cirrus_check_size(fb->width, fb->height, fb);
+}
+
+static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe,
+			       struct drm_crtc_state *crtc_state,
+			       struct drm_plane_state *plane_state)
+{
+	struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+
+	cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
+	cirrus_fb_blit_fullscreen(plane_state->fb);
+}
+
+static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
+			       struct drm_plane_state *old_state)
+{
+	struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_rect rect;
+
+	if (pipe->plane.state->fb &&
+	    cirrus->cpp != cirrus_cpp(pipe->plane.state->fb))
+		cirrus_mode_set(cirrus, &crtc->mode,
+				pipe->plane.state->fb);
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		cirrus_fb_blit_rect(pipe->plane.state->fb, &rect);
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
+
+static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
+	.mode_valid = cirrus_pipe_mode_valid,
+	.check	    = cirrus_pipe_check,
+	.enable	    = cirrus_pipe_enable,
+	.update	    = cirrus_pipe_update,
+};
+
+static const uint32_t cirrus_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const uint64_t cirrus_modifiers[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+static int cirrus_pipe_init(struct cirrus_device *cirrus)
+{
+	return drm_simple_display_pipe_init(&cirrus->dev,
+					    &cirrus->pipe,
+					    &cirrus_pipe_funcs,
+					    cirrus_formats,
+					    ARRAY_SIZE(cirrus_formats),
+					    cirrus_modifiers,
+					    &cirrus->conn);
+}
+
+/* ------------------------------------------------------------------ */
+/* cirrus framebuffers & mode config				      */
+
+static struct drm_framebuffer*
+cirrus_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+		 const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	if (mode_cmd->pixel_format != DRM_FORMAT_RGB565 &&
+	    mode_cmd->pixel_format != DRM_FORMAT_RGB888 &&
+	    mode_cmd->pixel_format != DRM_FORMAT_XRGB8888)
+		return ERR_PTR(-EINVAL);
+	if (cirrus_check_size(mode_cmd->width, mode_cmd->height, NULL) < 0)
+		return ERR_PTR(-EINVAL);
+	return drm_gem_fb_create_with_dirty(dev, file_priv, mode_cmd);
+}
+
+static const struct drm_mode_config_funcs cirrus_mode_config_funcs = {
+	.fb_create = cirrus_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static void cirrus_mode_config_init(struct cirrus_device *cirrus)
+{
+	struct drm_device *dev = &cirrus->dev;
+
+	drm_mode_config_init(dev);
+	dev->mode_config.min_width = 0;
+	dev->mode_config.min_height = 0;
+	dev->mode_config.max_width = CIRRUS_MAX_PITCH / 2;
+	dev->mode_config.max_height = 1024;
+	dev->mode_config.preferred_depth = 16;
+	dev->mode_config.prefer_shadow = 0;
+	dev->mode_config.funcs = &cirrus_mode_config_funcs;
+}
+
+/* ------------------------------------------------------------------ */
+
+DEFINE_DRM_GEM_SHMEM_FOPS(cirrus_fops);
+
+static struct drm_driver cirrus_driver = {
+	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC | DRIVER_PRIME,
+
+	.name		 = DRIVER_NAME,
+	.desc		 = DRIVER_DESC,
+	.date		 = DRIVER_DATE,
+	.major		 = DRIVER_MAJOR,
+	.minor		 = DRIVER_MINOR,
+
+	.fops		 = &cirrus_fops,
+	DRM_GEM_SHMEM_DRIVER_OPS,
+};
+
+static int cirrus_pci_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *ent)
+{
+	struct drm_device *dev;
+	struct cirrus_device *cirrus;
+	int ret;
+
+	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "cirrusdrmfb");
+	if (ret)
+		return ret;
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	ret = pci_request_regions(pdev, DRIVER_NAME);
+	if (ret)
+		return ret;
+
+	ret = -ENOMEM;
+	cirrus = kzalloc(sizeof(*cirrus), GFP_KERNEL);
+	if (cirrus == NULL)
+		goto err_pci_release;
+
+	dev = &cirrus->dev;
+	ret = drm_dev_init(dev, &cirrus_driver, &pdev->dev);
+	if (ret)
+		goto err_free_cirrus;
+	dev->dev_private = cirrus;
+
+	ret = -ENOMEM;
+	cirrus->vram = ioremap(pci_resource_start(pdev, 0),
+			       pci_resource_len(pdev, 0));
+	if (cirrus->vram == NULL)
+		goto err_dev_put;
+
+	cirrus->mmio = ioremap(pci_resource_start(pdev, 1),
+			       pci_resource_len(pdev, 1));
+	if (cirrus->mmio == NULL)
+		goto err_unmap_vram;
+
+	cirrus_mode_config_init(cirrus);
+
+	ret = cirrus_conn_init(cirrus);
+	if (ret < 0)
+		goto err_cleanup;
+
+	ret = cirrus_pipe_init(cirrus);
+	if (ret < 0)
+		goto err_cleanup;
+
+	drm_mode_config_reset(dev);
+
+	dev->pdev = pdev;
+	pci_set_drvdata(pdev, dev);
+	ret = drm_dev_register(dev, 0);
+	if (ret)
+		goto err_cleanup;
+
+	drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth);
+	return 0;
+
+err_cleanup:
+	drm_mode_config_cleanup(dev);
+	iounmap(cirrus->mmio);
+err_unmap_vram:
+	iounmap(cirrus->vram);
+err_dev_put:
+	drm_dev_put(dev);
+err_free_cirrus:
+	kfree(cirrus);
+err_pci_release:
+	pci_release_regions(pdev);
+	return ret;
+}
+
+static void cirrus_pci_remove(struct pci_dev *pdev)
+{
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct cirrus_device *cirrus = dev->dev_private;
+
+	drm_dev_unregister(dev);
+	drm_mode_config_cleanup(dev);
+	iounmap(cirrus->mmio);
+	iounmap(cirrus->vram);
+	drm_dev_put(dev);
+	kfree(cirrus);
+	pci_release_regions(pdev);
+}
+
+/* only bind to the cirrus chip in qemu */
+static const struct pci_device_id pciidlist[] = {
+	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
+	  PCI_SUBVENDOR_ID_REDHAT_QUMRANET, PCI_SUBDEVICE_ID_QEMU,
+	  0, 0, 0 },
+	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
+	  PCI_VENDOR_ID_XEN, 0x0001,
+	  0, 0, 0 },
+	{ /* end if list */}
+};
+static struct pci_driver cirrus_pci_driver = {
+	.name = DRIVER_NAME,
+	.id_table = pciidlist,
+	.probe = cirrus_pci_probe,
+	.remove = cirrus_pci_remove,
+};
+
+static int __init cirrus_init(void)
+{
+	if (vgacon_text_force())
+		return -EINVAL;
+	return pci_register_driver(&cirrus_pci_driver);
+}
+
+static void __exit cirrus_exit(void)
+{
+	pci_unregister_driver(&cirrus_pci_driver);
+}
+
+module_init(cirrus_init);
+module_exit(cirrus_exit);
+
+MODULE_DEVICE_TABLE(pci, pciidlist);
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c b/drivers/gpu/drm/cirrus/cirrus_drv.c
deleted file mode 100644
index 8ec880f3a322..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- * Copyright 2012 Red Hat <mjg@redhat.com>
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- *          Dave Airlie
- */
-#include <linux/module.h>
-#include <linux/console.h>
-#include <drm/drmP.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_probe_helper.h>
-
-#include "cirrus_drv.h"
-
-int cirrus_modeset = -1;
-int cirrus_bpp = 16;
-
-MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
-module_param_named(modeset, cirrus_modeset, int, 0400);
-MODULE_PARM_DESC(bpp, "Max bits-per-pixel (default:16)");
-module_param_named(bpp, cirrus_bpp, int, 0400);
-
-/*
- * This is the generic driver code. This binds the driver to the drm core,
- * which then performs further device association and calls our graphics init
- * functions
- */
-
-static struct drm_driver driver;
-
-/* only bind to the cirrus chip in qemu */
-static const struct pci_device_id pciidlist[] = {
-	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446,
-	  PCI_SUBVENDOR_ID_REDHAT_QUMRANET, PCI_SUBDEVICE_ID_QEMU,
-	  0, 0, 0 },
-	{ PCI_VENDOR_ID_CIRRUS, PCI_DEVICE_ID_CIRRUS_5446, PCI_VENDOR_ID_XEN,
-	  0x0001, 0, 0, 0 },
-	{0,}
-};
-
-
-static int cirrus_pci_probe(struct pci_dev *pdev,
-			    const struct pci_device_id *ent)
-{
-	int ret;
-
-	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "cirrusdrmfb");
-	if (ret)
-		return ret;
-
-	return drm_get_pci_dev(pdev, ent, &driver);
-}
-
-static void cirrus_pci_remove(struct pci_dev *pdev)
-{
-	struct drm_device *dev = pci_get_drvdata(pdev);
-
-	drm_put_dev(dev);
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int cirrus_pm_suspend(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	struct cirrus_device *cdev = drm_dev->dev_private;
-
-	drm_kms_helper_poll_disable(drm_dev);
-
-	if (cdev->mode_info.gfbdev) {
-		console_lock();
-		drm_fb_helper_set_suspend(&cdev->mode_info.gfbdev->helper, 1);
-		console_unlock();
-	}
-
-	return 0;
-}
-
-static int cirrus_pm_resume(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	struct cirrus_device *cdev = drm_dev->dev_private;
-
-	drm_helper_resume_force_mode(drm_dev);
-
-	if (cdev->mode_info.gfbdev) {
-		console_lock();
-		drm_fb_helper_set_suspend(&cdev->mode_info.gfbdev->helper, 0);
-		console_unlock();
-	}
-
-	drm_kms_helper_poll_enable(drm_dev);
-	return 0;
-}
-#endif
-
-static const struct file_operations cirrus_driver_fops = {
-	.owner = THIS_MODULE,
-	.open = drm_open,
-	.release = drm_release,
-	.unlocked_ioctl = drm_ioctl,
-	.mmap = cirrus_mmap,
-	.poll = drm_poll,
-	.compat_ioctl = drm_compat_ioctl,
-};
-static struct drm_driver driver = {
-	.driver_features = DRIVER_MODESET | DRIVER_GEM,
-	.load = cirrus_driver_load,
-	.unload = cirrus_driver_unload,
-	.fops = &cirrus_driver_fops,
-	.name = DRIVER_NAME,
-	.desc = DRIVER_DESC,
-	.date = DRIVER_DATE,
-	.major = DRIVER_MAJOR,
-	.minor = DRIVER_MINOR,
-	.patchlevel = DRIVER_PATCHLEVEL,
-	.gem_free_object_unlocked = cirrus_gem_free_object,
-	.dumb_create = cirrus_dumb_create,
-	.dumb_map_offset = cirrus_dumb_mmap_offset,
-};
-
-static const struct dev_pm_ops cirrus_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(cirrus_pm_suspend,
-				cirrus_pm_resume)
-};
-
-static struct pci_driver cirrus_pci_driver = {
-	.name = DRIVER_NAME,
-	.id_table = pciidlist,
-	.probe = cirrus_pci_probe,
-	.remove = cirrus_pci_remove,
-	.driver.pm = &cirrus_pm_ops,
-};
-
-static int __init cirrus_init(void)
-{
-	if (vgacon_text_force() && cirrus_modeset == -1)
-		return -EINVAL;
-
-	if (cirrus_modeset == 0)
-		return -EINVAL;
-	return pci_register_driver(&cirrus_pci_driver);
-}
-
-static void __exit cirrus_exit(void)
-{
-	pci_unregister_driver(&cirrus_pci_driver);
-}
-
-module_init(cirrus_init);
-module_exit(cirrus_exit);
-
-MODULE_DEVICE_TABLE(pci, pciidlist);
-MODULE_AUTHOR(DRIVER_AUTHOR);
-MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
deleted file mode 100644
index 2e6128069fc3..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ /dev/null
@@ -1,309 +0,0 @@
-/*
- * Copyright 2012 Red Hat
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- *          Dave Airlie
- */
-#include <linux/module.h>
-#include <drm/drmP.h>
-#include <drm/drm_util.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_crtc_helper.h>
-
-#include "cirrus_drv.h"
-
-static void cirrus_dirty_update(struct cirrus_fbdev *afbdev,
-			     int x, int y, int width, int height)
-{
-	int i;
-	struct drm_gem_object *obj;
-	struct cirrus_bo *bo;
-	int src_offset, dst_offset;
-	int bpp = afbdev->gfb->format->cpp[0];
-	int ret = -EBUSY;
-	bool unmap = false;
-	bool store_for_later = false;
-	int x2, y2;
-	unsigned long flags;
-
-	obj = afbdev->gfb->obj[0];
-	bo = gem_to_cirrus_bo(obj);
-
-	/*
-	 * try and reserve the BO, if we fail with busy
-	 * then the BO is being moved and we should
-	 * store up the damage until later.
-	 */
-	if (drm_can_sleep())
-		ret = cirrus_bo_reserve(bo, true);
-	if (ret) {
-		if (ret != -EBUSY)
-			return;
-		store_for_later = true;
-	}
-
-	x2 = x + width - 1;
-	y2 = y + height - 1;
-	spin_lock_irqsave(&afbdev->dirty_lock, flags);
-
-	if (afbdev->y1 < y)
-		y = afbdev->y1;
-	if (afbdev->y2 > y2)
-		y2 = afbdev->y2;
-	if (afbdev->x1 < x)
-		x = afbdev->x1;
-	if (afbdev->x2 > x2)
-		x2 = afbdev->x2;
-
-	if (store_for_later) {
-		afbdev->x1 = x;
-		afbdev->x2 = x2;
-		afbdev->y1 = y;
-		afbdev->y2 = y2;
-		spin_unlock_irqrestore(&afbdev->dirty_lock, flags);
-		return;
-	}
-
-	afbdev->x1 = afbdev->y1 = INT_MAX;
-	afbdev->x2 = afbdev->y2 = 0;
-	spin_unlock_irqrestore(&afbdev->dirty_lock, flags);
-
-	if (!bo->kmap.virtual) {
-		ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
-		if (ret) {
-			DRM_ERROR("failed to kmap fb updates\n");
-			cirrus_bo_unreserve(bo);
-			return;
-		}
-		unmap = true;
-	}
-	for (i = y; i < y + height; i++) {
-		/* assume equal stride for now */
-		src_offset = dst_offset = i * afbdev->gfb->pitches[0] + (x * bpp);
-		memcpy_toio(bo->kmap.virtual + src_offset, afbdev->sysram + src_offset, width * bpp);
-
-	}
-	if (unmap)
-		ttm_bo_kunmap(&bo->kmap);
-
-	cirrus_bo_unreserve(bo);
-}
-
-static void cirrus_fillrect(struct fb_info *info,
-			 const struct fb_fillrect *rect)
-{
-	struct cirrus_fbdev *afbdev = info->par;
-	drm_fb_helper_sys_fillrect(info, rect);
-	cirrus_dirty_update(afbdev, rect->dx, rect->dy, rect->width,
-			 rect->height);
-}
-
-static void cirrus_copyarea(struct fb_info *info,
-			 const struct fb_copyarea *area)
-{
-	struct cirrus_fbdev *afbdev = info->par;
-	drm_fb_helper_sys_copyarea(info, area);
-	cirrus_dirty_update(afbdev, area->dx, area->dy, area->width,
-			 area->height);
-}
-
-static void cirrus_imageblit(struct fb_info *info,
-			  const struct fb_image *image)
-{
-	struct cirrus_fbdev *afbdev = info->par;
-	drm_fb_helper_sys_imageblit(info, image);
-	cirrus_dirty_update(afbdev, image->dx, image->dy, image->width,
-			 image->height);
-}
-
-
-static struct fb_ops cirrusfb_ops = {
-	.owner = THIS_MODULE,
-	.fb_check_var = drm_fb_helper_check_var,
-	.fb_set_par = drm_fb_helper_set_par,
-	.fb_fillrect = cirrus_fillrect,
-	.fb_copyarea = cirrus_copyarea,
-	.fb_imageblit = cirrus_imageblit,
-	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
-	.fb_setcmap = drm_fb_helper_setcmap,
-};
-
-static int cirrusfb_create_object(struct cirrus_fbdev *afbdev,
-			       const struct drm_mode_fb_cmd2 *mode_cmd,
-			       struct drm_gem_object **gobj_p)
-{
-	struct drm_device *dev = afbdev->helper.dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	u32 bpp;
-	u32 size;
-	struct drm_gem_object *gobj;
-	int ret = 0;
-
-	bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;
-
-	if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
-				      bpp, mode_cmd->pitches[0]))
-		return -EINVAL;
-
-	size = mode_cmd->pitches[0] * mode_cmd->height;
-	ret = cirrus_gem_create(dev, size, true, &gobj);
-	if (ret)
-		return ret;
-
-	*gobj_p = gobj;
-	return ret;
-}
-
-static int cirrusfb_create(struct drm_fb_helper *helper,
-			   struct drm_fb_helper_surface_size *sizes)
-{
-	struct cirrus_fbdev *gfbdev =
-		container_of(helper, struct cirrus_fbdev, helper);
-	struct cirrus_device *cdev = gfbdev->helper.dev->dev_private;
-	struct fb_info *info;
-	struct drm_framebuffer *fb;
-	struct drm_mode_fb_cmd2 mode_cmd;
-	void *sysram;
-	struct drm_gem_object *gobj = NULL;
-	int size, ret;
-
-	mode_cmd.width = sizes->surface_width;
-	mode_cmd.height = sizes->surface_height;
-	mode_cmd.pitches[0] = mode_cmd.width * ((sizes->surface_bpp + 7) / 8);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
-	size = mode_cmd.pitches[0] * mode_cmd.height;
-
-	ret = cirrusfb_create_object(gfbdev, &mode_cmd, &gobj);
-	if (ret) {
-		DRM_ERROR("failed to create fbcon backing object %d\n", ret);
-		return ret;
-	}
-
-	sysram = vmalloc(size);
-	if (!sysram)
-		return -ENOMEM;
-
-	info = drm_fb_helper_alloc_fbi(helper);
-	if (IS_ERR(info)) {
-		ret = PTR_ERR(info);
-		goto err_vfree;
-	}
-
-	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
-	if (!fb) {
-		ret = -ENOMEM;
-		goto err_drm_gem_object_put_unlocked;
-	}
-
-	ret = cirrus_framebuffer_init(cdev->dev, fb, &mode_cmd, gobj);
-	if (ret)
-		goto err_kfree;
-
-	gfbdev->sysram = sysram;
-	gfbdev->size = size;
-	gfbdev->gfb = fb;
-
-	/* setup helper */
-	gfbdev->helper.fb = fb;
-
-	info->fbops = &cirrusfb_ops;
-
-	drm_fb_helper_fill_info(info, &gfbdev->helper, sizes);
-
-	/* setup aperture base/size for vesafb takeover */
-	info->apertures->ranges[0].base = cdev->dev->mode_config.fb_base;
-	info->apertures->ranges[0].size = cdev->mc.vram_size;
-
-	info->fix.smem_start = cdev->dev->mode_config.fb_base;
-	info->fix.smem_len = cdev->mc.vram_size;
-
-	info->screen_base = sysram;
-	info->screen_size = size;
-
-	info->fix.mmio_start = 0;
-	info->fix.mmio_len = 0;
-
-	DRM_INFO("fb mappable at 0x%lX\n", info->fix.smem_start);
-	DRM_INFO("vram aper at 0x%lX\n", (unsigned long)info->fix.smem_start);
-	DRM_INFO("size %lu\n", (unsigned long)info->fix.smem_len);
-	DRM_INFO("fb depth is %d\n", fb->format->depth);
-	DRM_INFO("   pitch is %d\n", fb->pitches[0]);
-
-	return 0;
-
-err_kfree:
-	kfree(fb);
-err_drm_gem_object_put_unlocked:
-	drm_gem_object_put_unlocked(gobj);
-err_vfree:
-	vfree(sysram);
-	return ret;
-}
-
-static int cirrus_fbdev_destroy(struct drm_device *dev,
-				struct cirrus_fbdev *gfbdev)
-{
-	struct drm_framebuffer *gfb = gfbdev->gfb;
-
-	drm_helper_force_disable_all(dev);
-
-	drm_fb_helper_unregister_fbi(&gfbdev->helper);
-
-	vfree(gfbdev->sysram);
-	drm_fb_helper_fini(&gfbdev->helper);
-	if (gfb)
-		drm_framebuffer_put(gfb);
-
-	return 0;
-}
-
-static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
-	.fb_probe = cirrusfb_create,
-};
-
-int cirrus_fbdev_init(struct cirrus_device *cdev)
-{
-	struct cirrus_fbdev *gfbdev;
-	int ret;
-
-	/*bpp_sel = 8;*/
-	gfbdev = kzalloc(sizeof(struct cirrus_fbdev), GFP_KERNEL);
-	if (!gfbdev)
-		return -ENOMEM;
-
-	cdev->mode_info.gfbdev = gfbdev;
-	spin_lock_init(&gfbdev->dirty_lock);
-
-	drm_fb_helper_prepare(cdev->dev, &gfbdev->helper,
-			      &cirrus_fb_helper_funcs);
-
-	ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper,
-				 CIRRUSFB_CONN_LIMIT);
-	if (ret)
-		return ret;
-
-	ret = drm_fb_helper_single_add_all_connectors(&gfbdev->helper);
-	if (ret)
-		return ret;
-
-	/* disable all the possible outputs/crtcs before entering KMS mode */
-	drm_helper_disable_unused_functions(cdev->dev);
-
-	return drm_fb_helper_initial_config(&gfbdev->helper, cirrus_bpp);
-}
-
-void cirrus_fbdev_fini(struct cirrus_device *cdev)
-{
-	if (!cdev->mode_info.gfbdev)
-		return;
-
-	cirrus_fbdev_destroy(cdev->dev, cdev->mode_info.gfbdev);
-	kfree(cdev->mode_info.gfbdev);
-	cdev->mode_info.gfbdev = NULL;
-}
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
deleted file mode 100644
index 57f8fe6d020b..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ /dev/null
@@ -1,328 +0,0 @@
-/*
- * Copyright 2012 Red Hat
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- *          Dave Airlie
- */
-#include <drm/drmP.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_gem_framebuffer_helper.h>
-
-#include "cirrus_drv.h"
-
-static const struct drm_framebuffer_funcs cirrus_fb_funcs = {
-	.create_handle = drm_gem_fb_create_handle,
-	.destroy = drm_gem_fb_destroy,
-};
-
-int cirrus_framebuffer_init(struct drm_device *dev,
-			    struct drm_framebuffer *gfb,
-			    const struct drm_mode_fb_cmd2 *mode_cmd,
-			    struct drm_gem_object *obj)
-{
-	int ret;
-
-	drm_helper_mode_fill_fb_struct(dev, gfb, mode_cmd);
-	gfb->obj[0] = obj;
-	ret = drm_framebuffer_init(dev, gfb, &cirrus_fb_funcs);
-	if (ret) {
-		DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
-		return ret;
-	}
-	return 0;
-}
-
-static struct drm_framebuffer *
-cirrus_user_framebuffer_create(struct drm_device *dev,
-			       struct drm_file *filp,
-			       const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	struct cirrus_device *cdev = dev->dev_private;
-	struct drm_gem_object *obj;
-	struct drm_framebuffer *fb;
-	u32 bpp;
-	int ret;
-
-	bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;
-
-	if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
-				      bpp, mode_cmd->pitches[0]))
-		return ERR_PTR(-EINVAL);
-
-	obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
-	if (obj == NULL)
-		return ERR_PTR(-ENOENT);
-
-	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
-	if (!fb) {
-		drm_gem_object_put_unlocked(obj);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = cirrus_framebuffer_init(dev, fb, mode_cmd, obj);
-	if (ret) {
-		drm_gem_object_put_unlocked(obj);
-		kfree(fb);
-		return ERR_PTR(ret);
-	}
-	return fb;
-}
-
-static const struct drm_mode_config_funcs cirrus_mode_funcs = {
-	.fb_create = cirrus_user_framebuffer_create,
-};
-
-/* Unmap the framebuffer from the core and release the memory */
-static void cirrus_vram_fini(struct cirrus_device *cdev)
-{
-	iounmap(cdev->rmmio);
-	cdev->rmmio = NULL;
-	if (cdev->mc.vram_base)
-		release_mem_region(cdev->mc.vram_base, cdev->mc.vram_size);
-}
-
-/* Map the framebuffer from the card and configure the core */
-static int cirrus_vram_init(struct cirrus_device *cdev)
-{
-	/* BAR 0 is VRAM */
-	cdev->mc.vram_base = pci_resource_start(cdev->dev->pdev, 0);
-	cdev->mc.vram_size = pci_resource_len(cdev->dev->pdev, 0);
-
-	if (!request_mem_region(cdev->mc.vram_base, cdev->mc.vram_size,
-				"cirrusdrmfb_vram")) {
-		DRM_ERROR("can't reserve VRAM\n");
-		return -ENXIO;
-	}
-
-	return 0;
-}
-
-/*
- * Our emulated hardware has two sets of memory. One is video RAM and can
- * simply be used as a linear framebuffer - the other provides mmio access
- * to the display registers. The latter can also be accessed via IO port
- * access, but we map the range and use mmio to program them instead
- */
-
-int cirrus_device_init(struct cirrus_device *cdev,
-		       struct drm_device *ddev,
-		       struct pci_dev *pdev, uint32_t flags)
-{
-	int ret;
-
-	cdev->dev = ddev;
-	cdev->flags = flags;
-
-	/* Hardcode the number of CRTCs to 1 */
-	cdev->num_crtc = 1;
-
-	/* BAR 0 is the framebuffer, BAR 1 contains registers */
-	cdev->rmmio_base = pci_resource_start(cdev->dev->pdev, 1);
-	cdev->rmmio_size = pci_resource_len(cdev->dev->pdev, 1);
-
-	if (!request_mem_region(cdev->rmmio_base, cdev->rmmio_size,
-				"cirrusdrmfb_mmio")) {
-		DRM_ERROR("can't reserve mmio registers\n");
-		return -ENOMEM;
-	}
-
-	cdev->rmmio = ioremap(cdev->rmmio_base, cdev->rmmio_size);
-
-	if (cdev->rmmio == NULL)
-		return -ENOMEM;
-
-	ret = cirrus_vram_init(cdev);
-	if (ret) {
-		release_mem_region(cdev->rmmio_base, cdev->rmmio_size);
-		return ret;
-	}
-
-	return 0;
-}
-
-void cirrus_device_fini(struct cirrus_device *cdev)
-{
-	release_mem_region(cdev->rmmio_base, cdev->rmmio_size);
-	cirrus_vram_fini(cdev);
-}
-
-/*
- * Functions here will be called by the core once it's bound the driver to
- * a PCI device
- */
-
-int cirrus_driver_load(struct drm_device *dev, unsigned long flags)
-{
-	struct cirrus_device *cdev;
-	int r;
-
-	cdev = kzalloc(sizeof(struct cirrus_device), GFP_KERNEL);
-	if (cdev == NULL)
-		return -ENOMEM;
-	dev->dev_private = (void *)cdev;
-
-	r = cirrus_device_init(cdev, dev, dev->pdev, flags);
-	if (r) {
-		dev_err(&dev->pdev->dev, "Fatal error during GPU init: %d\n", r);
-		goto out;
-	}
-
-	r = cirrus_mm_init(cdev);
-	if (r) {
-		dev_err(&dev->pdev->dev, "fatal err on mm init\n");
-		goto out;
-	}
-
-	/*
-	 * cirrus_modeset_init() is initializing/registering the emulated fbdev
-	 * and DRM internals can access/test some of the fields in
-	 * mode_config->funcs as part of the fbdev registration process.
-	 * Make sure dev->mode_config.funcs is properly set to avoid
-	 * dereferencing a NULL pointer.
-	 * FIXME: mode_config.funcs assignment should probably be done in
-	 * cirrus_modeset_init() (that's a common pattern seen in other DRM
-	 * drivers).
-	 */
-	dev->mode_config.funcs = &cirrus_mode_funcs;
-	r = cirrus_modeset_init(cdev);
-	if (r) {
-		dev_err(&dev->pdev->dev, "Fatal error during modeset init: %d\n", r);
-		goto out;
-	}
-
-	return 0;
-out:
-	cirrus_driver_unload(dev);
-	return r;
-}
-
-void cirrus_driver_unload(struct drm_device *dev)
-{
-	struct cirrus_device *cdev = dev->dev_private;
-
-	if (cdev == NULL)
-		return;
-	cirrus_modeset_fini(cdev);
-	cirrus_mm_fini(cdev);
-	cirrus_device_fini(cdev);
-	kfree(cdev);
-	dev->dev_private = NULL;
-}
-
-int cirrus_gem_create(struct drm_device *dev,
-		   u32 size, bool iskernel,
-		   struct drm_gem_object **obj)
-{
-	struct cirrus_bo *cirrusbo;
-	int ret;
-
-	*obj = NULL;
-
-	size = roundup(size, PAGE_SIZE);
-	if (size == 0)
-		return -EINVAL;
-
-	ret = cirrus_bo_create(dev, size, 0, 0, &cirrusbo);
-	if (ret) {
-		if (ret != -ERESTARTSYS)
-			DRM_ERROR("failed to allocate GEM object\n");
-		return ret;
-	}
-	*obj = &cirrusbo->gem;
-	return 0;
-}
-
-int cirrus_dumb_create(struct drm_file *file,
-		    struct drm_device *dev,
-		    struct drm_mode_create_dumb *args)
-{
-	int ret;
-	struct drm_gem_object *gobj;
-	u32 handle;
-
-	args->pitch = args->width * ((args->bpp + 7) / 8);
-	args->size = args->pitch * args->height;
-
-	ret = cirrus_gem_create(dev, args->size, false,
-			     &gobj);
-	if (ret)
-		return ret;
-
-	ret = drm_gem_handle_create(file, gobj, &handle);
-	drm_gem_object_put_unlocked(gobj);
-	if (ret)
-		return ret;
-
-	args->handle = handle;
-	return 0;
-}
-
-static void cirrus_bo_unref(struct cirrus_bo **bo)
-{
-	struct ttm_buffer_object *tbo;
-
-	if ((*bo) == NULL)
-		return;
-
-	tbo = &((*bo)->bo);
-	ttm_bo_put(tbo);
-	*bo = NULL;
-}
-
-void cirrus_gem_free_object(struct drm_gem_object *obj)
-{
-	struct cirrus_bo *cirrus_bo = gem_to_cirrus_bo(obj);
-
-	cirrus_bo_unref(&cirrus_bo);
-}
-
-
-static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo)
-{
-	return drm_vma_node_offset_addr(&bo->bo.vma_node);
-}
-
-int
-cirrus_dumb_mmap_offset(struct drm_file *file,
-		     struct drm_device *dev,
-		     uint32_t handle,
-		     uint64_t *offset)
-{
-	struct drm_gem_object *obj;
-	struct cirrus_bo *bo;
-
-	obj = drm_gem_object_lookup(file, handle);
-	if (obj == NULL)
-		return -ENOENT;
-
-	bo = gem_to_cirrus_bo(obj);
-	*offset = cirrus_bo_mmap_offset(bo);
-
-	drm_gem_object_put_unlocked(obj);
-
-	return 0;
-}
-
-bool cirrus_check_framebuffer(struct cirrus_device *cdev, int width, int height,
-			      int bpp, int pitch)
-{
-	const int max_pitch = 0x1FF << 3; /* (4096 - 1) & ~111b bytes */
-	const int max_size = cdev->mc.vram_size;
-
-	if (bpp > cirrus_bpp)
-		return false;
-	if (bpp > 32)
-		return false;
-
-	if (pitch > max_pitch)
-		return false;
-
-	if (pitch * height > max_size)
-		return false;
-
-	return true;
-}
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
deleted file mode 100644
index b109cd71426f..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ /dev/null
@@ -1,617 +0,0 @@
-
-/*
- * Copyright 2012 Red Hat
- *
- * This file is subject to the terms and conditions of the GNU General
- * Public License version 2. See the file COPYING in the main
- * directory of this archive for more details.
- *
- * Authors: Matthew Garrett
- *          Dave Airlie
- *
- * Portions of this code derived from cirrusfb.c:
- * drivers/video/cirrusfb.c - driver for Cirrus Logic chipsets
- *
- * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
- */
-#include <drm/drmP.h>
-#include <drm/drm_crtc_helper.h>
-#include <drm/drm_plane_helper.h>
-#include <drm/drm_probe_helper.h>
-
-#include <video/cirrus.h>
-
-#include "cirrus_drv.h"
-
-#define CIRRUS_LUT_SIZE 256
-
-#define PALETTE_INDEX 0x8
-#define PALETTE_DATA 0x9
-
-/*
- * This file contains setup code for the CRTC.
- */
-
-/*
- * The DRM core requires DPMS functions, but they make little sense in our
- * case and so are just stubs
- */
-
-static void cirrus_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	u8 sr01, gr0e;
-
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		sr01 = 0x00;
-		gr0e = 0x00;
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-		sr01 = 0x20;
-		gr0e = 0x02;
-		break;
-	case DRM_MODE_DPMS_SUSPEND:
-		sr01 = 0x20;
-		gr0e = 0x04;
-		break;
-	case DRM_MODE_DPMS_OFF:
-		sr01 = 0x20;
-		gr0e = 0x06;
-		break;
-	default:
-		return;
-	}
-
-	WREG8(SEQ_INDEX, 0x1);
-	sr01 |= RREG8(SEQ_DATA) & ~0x20;
-	WREG_SEQ(0x1, sr01);
-
-	WREG8(GFX_INDEX, 0xe);
-	gr0e |= RREG8(GFX_DATA) & ~0x06;
-	WREG_GFX(0xe, gr0e);
-}
-
-static void cirrus_set_start_address(struct drm_crtc *crtc, unsigned offset)
-{
-	struct cirrus_device *cdev = crtc->dev->dev_private;
-	u32 addr;
-	u8 tmp;
-
-	addr = offset >> 2;
-	WREG_CRT(0x0c, (u8)((addr >> 8) & 0xff));
-	WREG_CRT(0x0d, (u8)(addr & 0xff));
-
-	WREG8(CRT_INDEX, 0x1b);
-	tmp = RREG8(CRT_DATA);
-	tmp &= 0xf2;
-	tmp |= (addr >> 16) & 0x01;
-	tmp |= (addr >> 15) & 0x0c;
-	WREG_CRT(0x1b, tmp);
-	WREG8(CRT_INDEX, 0x1d);
-	tmp = RREG8(CRT_DATA);
-	tmp &= 0x7f;
-	tmp |= (addr >> 12) & 0x80;
-	WREG_CRT(0x1d, tmp);
-}
-
-/* cirrus is different - we will force move buffers out of VRAM */
-static int cirrus_crtc_do_set_base(struct drm_crtc *crtc,
-				struct drm_framebuffer *fb,
-				int x, int y, int atomic)
-{
-	struct cirrus_device *cdev = crtc->dev->dev_private;
-	struct cirrus_bo *bo;
-	int ret;
-	u64 gpu_addr;
-
-	/* push the previous fb to system ram */
-	if (!atomic && fb) {
-		bo = gem_to_cirrus_bo(fb->obj[0]);
-		ret = cirrus_bo_reserve(bo, false);
-		if (ret)
-			return ret;
-		cirrus_bo_push_sysram(bo);
-		cirrus_bo_unreserve(bo);
-	}
-
-	bo = gem_to_cirrus_bo(crtc->primary->fb->obj[0]);
-
-	ret = cirrus_bo_reserve(bo, false);
-	if (ret)
-		return ret;
-
-	ret = cirrus_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
-	if (ret) {
-		cirrus_bo_unreserve(bo);
-		return ret;
-	}
-
-	if (cdev->mode_info.gfbdev->gfb == crtc->primary->fb) {
-		/* if pushing console in kmap it */
-		ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
-		if (ret)
-			DRM_ERROR("failed to kmap fbcon\n");
-	}
-	cirrus_bo_unreserve(bo);
-
-	cirrus_set_start_address(crtc, (u32)gpu_addr);
-	return 0;
-}
-
-static int cirrus_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-			     struct drm_framebuffer *old_fb)
-{
-	return cirrus_crtc_do_set_base(crtc, old_fb, x, y, 0);
-}
-
-/*
- * The meat of this driver. The core passes us a mode and we have to program
- * it. The modesetting here is the bare minimum required to satisfy the qemu
- * emulation of this hardware, and running this against a real device is
- * likely to result in an inadequately programmed mode. We've already had
- * the opportunity to modify the mode, so whatever we receive here should
- * be something that can be correctly programmed and displayed
- */
-static int cirrus_crtc_mode_set(struct drm_crtc *crtc,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode,
-				int x, int y, struct drm_framebuffer *old_fb)
-{
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	const struct drm_framebuffer *fb = crtc->primary->fb;
-	int hsyncstart, hsyncend, htotal, hdispend;
-	int vtotal, vdispend;
-	int tmp;
-	int sr07 = 0, hdr = 0;
-
-	htotal = mode->htotal / 8;
-	hsyncend = mode->hsync_end / 8;
-	hsyncstart = mode->hsync_start / 8;
-	hdispend = mode->hdisplay / 8;
-
-	vtotal = mode->vtotal;
-	vdispend = mode->vdisplay;
-
-	vdispend -= 1;
-	vtotal -= 2;
-
-	htotal -= 5;
-	hdispend -= 1;
-	hsyncstart += 1;
-	hsyncend += 1;
-
-	WREG_CRT(VGA_CRTC_V_SYNC_END, 0x20);
-	WREG_CRT(VGA_CRTC_H_TOTAL, htotal);
-	WREG_CRT(VGA_CRTC_H_DISP, hdispend);
-	WREG_CRT(VGA_CRTC_H_SYNC_START, hsyncstart);
-	WREG_CRT(VGA_CRTC_H_SYNC_END, hsyncend);
-	WREG_CRT(VGA_CRTC_V_TOTAL, vtotal & 0xff);
-	WREG_CRT(VGA_CRTC_V_DISP_END, vdispend & 0xff);
-
-	tmp = 0x40;
-	if ((vdispend + 1) & 512)
-		tmp |= 0x20;
-	WREG_CRT(VGA_CRTC_MAX_SCAN, tmp);
-
-	/*
-	 * Overflow bits for values that don't fit in the standard registers
-	 */
-	tmp = 16;
-	if (vtotal & 256)
-		tmp |= 1;
-	if (vdispend & 256)
-		tmp |= 2;
-	if ((vdispend + 1) & 256)
-		tmp |= 8;
-	if (vtotal & 512)
-		tmp |= 32;
-	if (vdispend & 512)
-		tmp |= 64;
-	WREG_CRT(VGA_CRTC_OVERFLOW, tmp);
-
-	tmp = 0;
-
-	/* More overflow bits */
-
-	if ((htotal + 5) & 64)
-		tmp |= 16;
-	if ((htotal + 5) & 128)
-		tmp |= 32;
-	if (vtotal & 256)
-		tmp |= 64;
-	if (vtotal & 512)
-		tmp |= 128;
-
-	WREG_CRT(CL_CRT1A, tmp);
-
-	/* Disable Hercules/CGA compatibility */
-	WREG_CRT(VGA_CRTC_MODE, 0x03);
-
-	WREG8(SEQ_INDEX, 0x7);
-	sr07 = RREG8(SEQ_DATA);
-	sr07 &= 0xe0;
-	hdr = 0;
-	switch (fb->format->cpp[0] * 8) {
-	case 8:
-		sr07 |= 0x11;
-		break;
-	case 16:
-		sr07 |= 0x17;
-		hdr = 0xc1;
-		break;
-	case 24:
-		sr07 |= 0x15;
-		hdr = 0xc5;
-		break;
-	case 32:
-		sr07 |= 0x19;
-		hdr = 0xc5;
-		break;
-	default:
-		return -1;
-	}
-
-	WREG_SEQ(0x7, sr07);
-
-	/* Program the pitch */
-	tmp = fb->pitches[0] / 8;
-	WREG_CRT(VGA_CRTC_OFFSET, tmp);
-
-	/* Enable extended blanking and pitch bits, and enable full memory */
-	tmp = 0x22;
-	tmp |= (fb->pitches[0] >> 7) & 0x10;
-	tmp |= (fb->pitches[0] >> 6) & 0x40;
-	WREG_CRT(0x1b, tmp);
-
-	/* Enable high-colour modes */
-	WREG_GFX(VGA_GFX_MODE, 0x40);
-
-	/* And set graphics mode */
-	WREG_GFX(VGA_GFX_MISC, 0x01);
-
-	WREG_HDR(hdr);
-	cirrus_crtc_do_set_base(crtc, old_fb, x, y, 0);
-
-	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
-	outb(0x20, 0x3c0);
-	return 0;
-}
-
-/*
- * This is called before a mode is programmed. A typical use might be to
- * enable DPMS during the programming to avoid seeing intermediate stages,
- * but that's not relevant to us
- */
-static void cirrus_crtc_prepare(struct drm_crtc *crtc)
-{
-}
-
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct cirrus_device *cdev = dev->dev_private;
-	u16 *r, *g, *b;
-	int i;
-
-	if (!crtc->enabled)
-		return;
-
-	r = crtc->gamma_store;
-	g = r + crtc->gamma_size;
-	b = g + crtc->gamma_size;
-
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-		/* VGA registers */
-		WREG8(PALETTE_INDEX, i);
-		WREG8(PALETTE_DATA, *r++ >> 8);
-		WREG8(PALETTE_DATA, *g++ >> 8);
-		WREG8(PALETTE_DATA, *b++ >> 8);
-	}
-}
-
-/*
- * This is called after a mode is programmed. It should reverse anything done
- * by the prepare function
- */
-static void cirrus_crtc_commit(struct drm_crtc *crtc)
-{
-	cirrus_crtc_load_lut(crtc);
-}
-
-/*
- * The core can pass us a set of gamma values to program. We actually only
- * use this for 8-bit mode so can't perform smooth fades on deeper modes,
- * but it's a requirement that we provide the function
- */
-static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				 u16 *blue, uint32_t size,
-				 struct drm_modeset_acquire_ctx *ctx)
-{
-	cirrus_crtc_load_lut(crtc);
-
-	return 0;
-}
-
-/* Simple cleanup function */
-static void cirrus_crtc_destroy(struct drm_crtc *crtc)
-{
-	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
-	drm_crtc_cleanup(crtc);
-	kfree(cirrus_crtc);
-}
-
-/* These provide the minimum set of functions required to handle a CRTC */
-static const struct drm_crtc_funcs cirrus_crtc_funcs = {
-	.gamma_set = cirrus_crtc_gamma_set,
-	.set_config = drm_crtc_helper_set_config,
-	.destroy = cirrus_crtc_destroy,
-};
-
-static const struct drm_crtc_helper_funcs cirrus_helper_funcs = {
-	.dpms = cirrus_crtc_dpms,
-	.mode_set = cirrus_crtc_mode_set,
-	.mode_set_base = cirrus_crtc_mode_set_base,
-	.prepare = cirrus_crtc_prepare,
-	.commit = cirrus_crtc_commit,
-};
-
-/* CRTC setup */
-static const uint32_t cirrus_formats_16[] = {
-	DRM_FORMAT_RGB565,
-};
-
-static const uint32_t cirrus_formats_24[] = {
-	DRM_FORMAT_RGB888,
-	DRM_FORMAT_RGB565,
-};
-
-static const uint32_t cirrus_formats_32[] = {
-	DRM_FORMAT_XRGB8888,
-	DRM_FORMAT_ARGB8888,
-	DRM_FORMAT_RGB888,
-	DRM_FORMAT_RGB565,
-};
-
-static struct drm_plane *cirrus_primary_plane(struct drm_device *dev)
-{
-	const uint32_t *formats;
-	uint32_t nformats;
-	struct drm_plane *primary;
-	int ret;
-
-	switch (cirrus_bpp) {
-	case 16:
-		formats = cirrus_formats_16;
-		nformats = ARRAY_SIZE(cirrus_formats_16);
-		break;
-	case 24:
-		formats = cirrus_formats_24;
-		nformats = ARRAY_SIZE(cirrus_formats_24);
-		break;
-	case 32:
-		formats = cirrus_formats_32;
-		nformats = ARRAY_SIZE(cirrus_formats_32);
-		break;
-	default:
-		return NULL;
-	}
-
-	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-	if (primary == NULL) {
-		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
-		return NULL;
-	}
-
-	ret = drm_universal_plane_init(dev, primary, 0,
-				       &drm_primary_helper_funcs,
-				       formats, nformats,
-				       NULL,
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		kfree(primary);
-		primary = NULL;
-	}
-
-	return primary;
-}
-
-static void cirrus_crtc_init(struct drm_device *dev)
-{
-	struct cirrus_device *cdev = dev->dev_private;
-	struct cirrus_crtc *cirrus_crtc;
-	struct drm_plane *primary;
-
-	cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
-			      (CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)),
-			      GFP_KERNEL);
-
-	if (cirrus_crtc == NULL)
-		return;
-
-	primary = cirrus_primary_plane(dev);
-	if (primary == NULL) {
-		kfree(cirrus_crtc);
-		return;
-	}
-
-	drm_crtc_init_with_planes(dev, &cirrus_crtc->base,
-				  primary, NULL,
-				  &cirrus_crtc_funcs, NULL);
-
-	drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
-	cdev->mode_info.crtc = cirrus_crtc;
-
-	drm_crtc_helper_add(&cirrus_crtc->base, &cirrus_helper_funcs);
-}
-
-static void cirrus_encoder_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
-{
-}
-
-static void cirrus_encoder_dpms(struct drm_encoder *encoder, int state)
-{
-	return;
-}
-
-static void cirrus_encoder_prepare(struct drm_encoder *encoder)
-{
-}
-
-static void cirrus_encoder_commit(struct drm_encoder *encoder)
-{
-}
-
-static void cirrus_encoder_destroy(struct drm_encoder *encoder)
-{
-	struct cirrus_encoder *cirrus_encoder = to_cirrus_encoder(encoder);
-	drm_encoder_cleanup(encoder);
-	kfree(cirrus_encoder);
-}
-
-static const struct drm_encoder_helper_funcs cirrus_encoder_helper_funcs = {
-	.dpms = cirrus_encoder_dpms,
-	.mode_set = cirrus_encoder_mode_set,
-	.prepare = cirrus_encoder_prepare,
-	.commit = cirrus_encoder_commit,
-};
-
-static const struct drm_encoder_funcs cirrus_encoder_encoder_funcs = {
-	.destroy = cirrus_encoder_destroy,
-};
-
-static struct drm_encoder *cirrus_encoder_init(struct drm_device *dev)
-{
-	struct drm_encoder *encoder;
-	struct cirrus_encoder *cirrus_encoder;
-
-	cirrus_encoder = kzalloc(sizeof(struct cirrus_encoder), GFP_KERNEL);
-	if (!cirrus_encoder)
-		return NULL;
-
-	encoder = &cirrus_encoder->base;
-	encoder->possible_crtcs = 0x1;
-
-	drm_encoder_init(dev, encoder, &cirrus_encoder_encoder_funcs,
-			 DRM_MODE_ENCODER_DAC, NULL);
-	drm_encoder_helper_add(encoder, &cirrus_encoder_helper_funcs);
-
-	return encoder;
-}
-
-
-static int cirrus_vga_get_modes(struct drm_connector *connector)
-{
-	int count;
-
-	/* Just add a static list of modes */
-	if (cirrus_bpp <= 24) {
-		count = drm_add_modes_noedid(connector, 1280, 1024);
-		drm_set_preferred_mode(connector, 1024, 768);
-	} else {
-		count = drm_add_modes_noedid(connector, 800, 600);
-		drm_set_preferred_mode(connector, 800, 600);
-	}
-	return count;
-}
-
-static struct drm_encoder *cirrus_connector_best_encoder(struct drm_connector
-						  *connector)
-{
-	int enc_id = connector->encoder_ids[0];
-	/* pick the encoder ids */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
-	return NULL;
-}
-
-static void cirrus_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-	kfree(connector);
-}
-
-static const struct drm_connector_helper_funcs cirrus_vga_connector_helper_funcs = {
-	.get_modes = cirrus_vga_get_modes,
-	.best_encoder = cirrus_connector_best_encoder,
-};
-
-static const struct drm_connector_funcs cirrus_vga_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = cirrus_connector_destroy,
-};
-
-static struct drm_connector *cirrus_vga_init(struct drm_device *dev)
-{
-	struct drm_connector *connector;
-	struct cirrus_connector *cirrus_connector;
-
-	cirrus_connector = kzalloc(sizeof(struct cirrus_connector), GFP_KERNEL);
-	if (!cirrus_connector)
-		return NULL;
-
-	connector = &cirrus_connector->base;
-
-	drm_connector_init(dev, connector,
-			   &cirrus_vga_connector_funcs, DRM_MODE_CONNECTOR_VGA);
-
-	drm_connector_helper_add(connector, &cirrus_vga_connector_helper_funcs);
-
-	drm_connector_register(connector);
-	return connector;
-}
-
-
-int cirrus_modeset_init(struct cirrus_device *cdev)
-{
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
-	int ret;
-
-	drm_mode_config_init(cdev->dev);
-
-	cdev->dev->mode_config.max_width = CIRRUS_MAX_FB_WIDTH;
-	cdev->dev->mode_config.max_height = CIRRUS_MAX_FB_HEIGHT;
-
-	cdev->dev->mode_config.fb_base = cdev->mc.vram_base;
-	cdev->dev->mode_config.preferred_depth = cirrus_bpp;
-	/* don't prefer a shadow on virt GPU */
-	cdev->dev->mode_config.prefer_shadow = 0;
-
-	cirrus_crtc_init(cdev->dev);
-
-	encoder = cirrus_encoder_init(cdev->dev);
-	if (!encoder) {
-		DRM_ERROR("cirrus_encoder_init failed\n");
-		return -1;
-	}
-
-	connector = cirrus_vga_init(cdev->dev);
-	if (!connector) {
-		DRM_ERROR("cirrus_vga_init failed\n");
-		return -1;
-	}
-
-	drm_connector_attach_encoder(connector, encoder);
-
-	ret = cirrus_fbdev_init(cdev);
-	if (ret) {
-		DRM_ERROR("cirrus_fbdev_init failed\n");
-		return ret;
-	}
-
-	return 0;
-}
-
-void cirrus_modeset_fini(struct cirrus_device *cdev)
-{
-	cirrus_fbdev_fini(cdev);
-	drm_helper_force_disable_all(cdev->dev);
-	drm_mode_config_cleanup(cdev->dev);
-}
diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
deleted file mode 100644
index e075810b4bd4..000000000000
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ /dev/null
@@ -1,343 +0,0 @@
-/*
- * Copyright 2012 Red Hat Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- */
-/*
- * Authors: Dave Airlie <airlied@redhat.com>
- */
-#include <drm/drmP.h>
-#include <drm/ttm/ttm_page_alloc.h>
-
-#include "cirrus_drv.h"
-
-static inline struct cirrus_device *
-cirrus_bdev(struct ttm_bo_device *bd)
-{
-	return container_of(bd, struct cirrus_device, ttm.bdev);
-}
-
-static void cirrus_bo_ttm_destroy(struct ttm_buffer_object *tbo)
-{
-	struct cirrus_bo *bo;
-
-	bo = container_of(tbo, struct cirrus_bo, bo);
-
-	drm_gem_object_release(&bo->gem);
-	kfree(bo);
-}
-
-static bool cirrus_ttm_bo_is_cirrus_bo(struct ttm_buffer_object *bo)
-{
-	if (bo->destroy == &cirrus_bo_ttm_destroy)
-		return true;
-	return false;
-}
-
-static int
-cirrus_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
-		     struct ttm_mem_type_manager *man)
-{
-	switch (type) {
-	case TTM_PL_SYSTEM:
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
-		break;
-	case TTM_PL_VRAM:
-		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			TTM_MEMTYPE_FLAG_MAPPABLE;
-		man->available_caching = TTM_PL_FLAG_UNCACHED |
-			TTM_PL_FLAG_WC;
-		man->default_caching = TTM_PL_FLAG_WC;
-		break;
-	default:
-		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static void
-cirrus_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
-{
-	struct cirrus_bo *cirrusbo = cirrus_bo(bo);
-
-	if (!cirrus_ttm_bo_is_cirrus_bo(bo))
-		return;
-
-	cirrus_ttm_placement(cirrusbo, TTM_PL_FLAG_SYSTEM);
-	*pl = cirrusbo->placement;
-}
-
-static int cirrus_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct cirrus_bo *cirrusbo = cirrus_bo(bo);
-
-	return drm_vma_node_verify_access(&cirrusbo->gem.vma_node,
-					  filp->private_data);
-}
-
-static int cirrus_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
-				  struct ttm_mem_reg *mem)
-{
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-	struct cirrus_device *cirrus = cirrus_bdev(bdev);
-
-	mem->bus.addr = NULL;
-	mem->bus.offset = 0;
-	mem->bus.size = mem->num_pages << PAGE_SHIFT;
-	mem->bus.base = 0;
-	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
-	switch (mem->mem_type) {
-	case TTM_PL_SYSTEM:
-		/* system memory */
-		return 0;
-	case TTM_PL_VRAM:
-		mem->bus.offset = mem->start << PAGE_SHIFT;
-		mem->bus.base = pci_resource_start(cirrus->dev->pdev, 0);
-		mem->bus.is_iomem = true;
-		break;
-	default:
-		return -EINVAL;
-		break;
-	}
-	return 0;
-}
-
-static void cirrus_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
-{
-}
-
-static void cirrus_ttm_backend_destroy(struct ttm_tt *tt)
-{
-	ttm_tt_fini(tt);
-	kfree(tt);
-}
-
-static struct ttm_backend_func cirrus_tt_backend_func = {
-	.destroy = &cirrus_ttm_backend_destroy,
-};
-
-
-static struct ttm_tt *cirrus_ttm_tt_create(struct ttm_buffer_object *bo,
-					   uint32_t page_flags)
-{
-	struct ttm_tt *tt;
-
-	tt = kzalloc(sizeof(struct ttm_tt), GFP_KERNEL);
-	if (tt == NULL)
-		return NULL;
-	tt->func = &cirrus_tt_backend_func;
-	if (ttm_tt_init(tt, bo, page_flags)) {
-		kfree(tt);
-		return NULL;
-	}
-	return tt;
-}
-
-struct ttm_bo_driver cirrus_bo_driver = {
-	.ttm_tt_create = cirrus_ttm_tt_create,
-	.init_mem_type = cirrus_bo_init_mem_type,
-	.eviction_valuable = ttm_bo_eviction_valuable,
-	.evict_flags = cirrus_bo_evict_flags,
-	.move = NULL,
-	.verify_access = cirrus_bo_verify_access,
-	.io_mem_reserve = &cirrus_ttm_io_mem_reserve,
-	.io_mem_free = &cirrus_ttm_io_mem_free,
-};
-
-int cirrus_mm_init(struct cirrus_device *cirrus)
-{
-	int ret;
-	struct drm_device *dev = cirrus->dev;
-	struct ttm_bo_device *bdev = &cirrus->ttm.bdev;
-
-	ret = ttm_bo_device_init(&cirrus->ttm.bdev,
-				 &cirrus_bo_driver,
-				 dev->anon_inode->i_mapping,
-				 DRM_FILE_PAGE_OFFSET,
-				 true);
-	if (ret) {
-		DRM_ERROR("Error initialising bo driver; %d\n", ret);
-		return ret;
-	}
-
-	ret = ttm_bo_init_mm(bdev, TTM_PL_VRAM,
-			     cirrus->mc.vram_size >> PAGE_SHIFT);
-	if (ret) {
-		DRM_ERROR("Failed ttm VRAM init: %d\n", ret);
-		return ret;
-	}
-
-	arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0),
-				   pci_resource_len(dev->pdev, 0));
-
-	cirrus->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
-					   pci_resource_len(dev->pdev, 0));
-
-	cirrus->mm_inited = true;
-	return 0;
-}
-
-void cirrus_mm_fini(struct cirrus_device *cirrus)
-{
-	struct drm_device *dev = cirrus->dev;
-
-	if (!cirrus->mm_inited)
-		return;
-
-	ttm_bo_device_release(&cirrus->ttm.bdev);
-
-	arch_phys_wc_del(cirrus->fb_mtrr);
-	cirrus->fb_mtrr = 0;
-	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
-				pci_resource_len(dev->pdev, 0));
-}
-
-void cirrus_ttm_placement(struct cirrus_bo *bo, int domain)
-{
-	u32 c = 0;
-	unsigned i;
-	bo->placement.placement = bo->placements;
-	bo->placement.busy_placement = bo->placements;
-	if (domain & TTM_PL_FLAG_VRAM)
-		bo->placements[c++].flags = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_VRAM;
-	if (domain & TTM_PL_FLAG_SYSTEM)
-		bo->placements[c++].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
-	if (!c)
-		bo->placements[c++].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
-	bo->placement.num_placement = c;
-	bo->placement.num_busy_placement = c;
-	for (i = 0; i < c; ++i) {
-		bo->placements[i].fpfn = 0;
-		bo->placements[i].lpfn = 0;
-	}
-}
-
-int cirrus_bo_create(struct drm_device *dev, int size, int align,
-		  uint32_t flags, struct cirrus_bo **pcirrusbo)
-{
-	struct cirrus_device *cirrus = dev->dev_private;
-	struct cirrus_bo *cirrusbo;
-	size_t acc_size;
-	int ret;
-
-	cirrusbo = kzalloc(sizeof(struct cirrus_bo), GFP_KERNEL);
-	if (!cirrusbo)
-		return -ENOMEM;
-
-	ret = drm_gem_object_init(dev, &cirrusbo->gem, size);
-	if (ret) {
-		kfree(cirrusbo);
-		return ret;
-	}
-
-	cirrusbo->bo.bdev = &cirrus->ttm.bdev;
-
-	cirrus_ttm_placement(cirrusbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
-
-	acc_size = ttm_bo_dma_acc_size(&cirrus->ttm.bdev, size,
-				       sizeof(struct cirrus_bo));
-
-	ret = ttm_bo_init(&cirrus->ttm.bdev, &cirrusbo->bo, size,
-			  ttm_bo_type_device, &cirrusbo->placement,
-			  align >> PAGE_SHIFT, false, acc_size,
-			  NULL, NULL, cirrus_bo_ttm_destroy);
-	if (ret)
-		return ret;
-
-	*pcirrusbo = cirrusbo;
-	return 0;
-}
-
-static inline u64 cirrus_bo_gpu_offset(struct cirrus_bo *bo)
-{
-	return bo->bo.offset;
-}
-
-int cirrus_bo_pin(struct cirrus_bo *bo, u32 pl_flag, u64 *gpu_addr)
-{
-	struct ttm_operation_ctx ctx = { false, false };
-	int i, ret;
-
-	if (bo->pin_count) {
-		bo->pin_count++;
-		if (gpu_addr)
-			*gpu_addr = cirrus_bo_gpu_offset(bo);
-	}
-
-	cirrus_ttm_placement(bo, pl_flag);
-	for (i = 0; i < bo->placement.num_placement; i++)
-		bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
-	ret = ttm_bo_validate(&bo->bo, &bo->placement, &ctx);
-	if (ret)
-		return ret;
-
-	bo->pin_count = 1;
-	if (gpu_addr)
-		*gpu_addr = cirrus_bo_gpu_offset(bo);
-	return 0;
-}
-
-int cirrus_bo_push_sysram(struct cirrus_bo *bo)
-{
-	struct ttm_operation_ctx ctx = { false, false };
-	int i, ret;
-	if (!bo->pin_count) {
-		DRM_ERROR("unpin bad %p\n", bo);
-		return 0;
-	}
-	bo->pin_count--;
-	if (bo->pin_count)
-		return 0;
-
-	if (bo->kmap.virtual)
-		ttm_bo_kunmap(&bo->kmap);
-
-	cirrus_ttm_placement(bo, TTM_PL_FLAG_SYSTEM);
-	for (i = 0; i < bo->placement.num_placement ; i++)
-		bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
-
-	ret = ttm_bo_validate(&bo->bo, &bo->placement, &ctx);
-	if (ret) {
-		DRM_ERROR("pushing to VRAM failed\n");
-		return ret;
-	}
-	return 0;
-}
-
-int cirrus_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	struct drm_file *file_priv;
-	struct cirrus_device *cirrus;
-
-	if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET))
-		return -EINVAL;
-
-	file_priv = filp->private_data;
-	cirrus = file_priv->minor->dev->dev_private;
-	return ttm_bo_mmap(filp, vma, &cirrus->ttm.bdev);
-}
diff --git a/drivers/gpu/drm/cirrus/Kconfig b/drivers/gpu/drm/cirrus/Kconfig
index fc78c90ee931..dd4f52a0bc1c 100644
--- a/drivers/gpu/drm/cirrus/Kconfig
+++ b/drivers/gpu/drm/cirrus/Kconfig
@@ -2,7 +2,7 @@ config DRM_CIRRUS_QEMU
 	tristate "Cirrus driver for QEMU emulated device"
 	depends on DRM && PCI && MMU
 	select DRM_KMS_HELPER
-	select DRM_TTM
+	select DRM_GEM_SHMEM_HELPER
 	help
 	 This is a KMS driver for emulated cirrus device in qemu.
 	 It is *NOT* intended for real cirrus devices. This requires
diff --git a/drivers/gpu/drm/cirrus/Makefile b/drivers/gpu/drm/cirrus/Makefile
index 919c0a336c97..acf8971d37a1 100644
--- a/drivers/gpu/drm/cirrus/Makefile
+++ b/drivers/gpu/drm/cirrus/Makefile
@@ -1,4 +1 @@
-cirrus-y  := cirrus_main.o cirrus_mode.o \
-	cirrus_drv.o cirrus_fbdev.o cirrus_ttm.o
-
 obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
-- 
2.18.1

^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Darrick J. Wong @ 2019-04-04 15:00 UTC (permalink / raw)
  To: Pankaj Gupta
  Cc: Jan Kara, kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, cohuck, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <527416483.17415178.1554372524196.JavaMail.zimbra@redhat.com>

On Thu, Apr 04, 2019 at 06:08:44AM -0400, Pankaj Gupta wrote:
> 
> > On Thu 04-04-19 05:09:10, Pankaj Gupta wrote:
> > > 
> > > > > On Thu, Apr 04, 2019 at 09:09:12AM +1100, Dave Chinner wrote:
> > > > > > On Wed, Apr 03, 2019 at 04:10:18PM +0530, Pankaj Gupta wrote:
> > > > > > > Virtio pmem provides asynchronous host page cache flush
> > > > > > > mechanism. we don't support 'MAP_SYNC' with virtio pmem
> > > > > > > and xfs.
> > > > > > > 
> > > > > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > > > > > ---
> > > > > > >  fs/xfs/xfs_file.c | 8 ++++++++
> > > > > > >  1 file changed, 8 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > > > > > index 1f2e2845eb76..dced2eb8c91a 100644
> > > > > > > --- a/fs/xfs/xfs_file.c
> > > > > > > +++ b/fs/xfs/xfs_file.c
> > > > > > > @@ -1203,6 +1203,14 @@ xfs_file_mmap(
> > > > > > >  	if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > > > > > >  		return -EOPNOTSUPP;
> > > > > > >  
> > > > > > > +	/* We don't support synchronous mappings with DAX files if
> > > > > > > +	 * dax_device is not synchronous.
> > > > > > > +	 */
> > > > > > > +	if (IS_DAX(file_inode(filp)) && !dax_synchronous(
> > > > > > > +		xfs_find_daxdev_for_inode(file_inode(filp))) &&
> > > > > > > +					(vma->vm_flags & VM_SYNC))
> > > > > > > +		return -EOPNOTSUPP;
> > > > > > > +
> > > > > > >  	file_accessed(filp);
> > > > > > >  	vma->vm_ops = &xfs_file_vm_ops;
> > > > > > >  	if (IS_DAX(file_inode(filp)))
> > > > > > 
> > > > > > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > > > > > 
> > > > > > xfs_file_mmap(
> > > > > > ....
> > > > > > {
> > > > > > 	struct inode	*inode = file_inode(filp);
> > > > > > 
> > > > > > 	if (vma->vm_flags & VM_SYNC) {
> > > > > > 		if (!IS_DAX(inode))
> > > > > > 			return -EOPNOTSUPP;
> > > > > > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > > > > > 			return -EOPNOTSUPP;
> > > > > > 	}
> > > > > > 
> > > > > > 	file_accessed(filp);
> > > > > > 	vma->vm_ops = &xfs_file_vm_ops;
> > > > > > 	if (IS_DAX(inode))
> > > > > > 		vma->vm_flags |= VM_HUGEPAGE;
> > > > > > 	return 0;
> > > > > > }
> > > > > > 
> > > > > > 
> > > > > > Even better, factor out all the "MAP_SYNC supported" checks into a
> > > > > > helper so that the filesystem code just doesn't have to care about
> > > > > > the details of checking for DAX+MAP_SYNC support....
> > > > > 
> > > > > Seconded, since ext4 has nearly the same flag validation logic.
> > > > 
> > > 
> > > Only issue with this I see is we need the helper function only for
> > > supported
> > > filesystems ext4 & xfs (right now). If I create the function in "fs.h" it
> > > will be compiled for every filesystem, even for those don't need it.
> > > 
> > > Sample patch below, does below patch is near to what you have in mind?
> > 
> > So I would put the helper in include/linux/dax.h and have it like:
> > 
> > bool daxdev_mapping_supported(struct vm_area_struct *vma,

Should this be static inline if you're putting it in the header file?

A comment ought to be added to describe what this predicate function
does.

> > 			      struct dax_device *dax_dev)
> > {
> > 	if (!(vma->vm_flags & VM_SYNC))
> > 		return true;
> > 	if (!IS_DAX(file_inode(vma->vm_file)))
> > 		return false;
> > 	return dax_synchronous(dax_dev);
> > }
> 
> Sure. This is much better. I was also not sure what to name the helper function.
> I will go ahead with this unless 'Dave' & 'Darrick' have anything to add.

Jan's approach (modulo that one comment) looks good to me.

--D

> Thank you very much.
> 
> Best regards,
> Pankaj 
> 
> > 
> > 								Honza
> > > 
> > > =================
> > > 
> > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > index 1f2e2845eb76..614995170cac 100644
> > > --- a/fs/xfs/xfs_file.c
> > > +++ b/fs/xfs/xfs_file.c
> > > @@ -1196,12 +1196,17 @@ xfs_file_mmap(
> > >         struct file     *filp,
> > >         struct vm_area_struct *vma)
> > >  {
> > > +       struct dax_device *dax_dev =
> > > xfs_find_daxdev_for_inode(file_inode(filp));
> > > +
> > >         /*
> > > -        * We don't support synchronous mappings for non-DAX files. At
> > > least
> > > -        * until someone comes with a sensible use case.
> > > +        * We don't support synchronous mappings for non-DAX files and
> > > +        * for DAX files if underneath dax_device is not synchronous.
> > >          */
> > > -       if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > > -               return -EOPNOTSUPP;
> > > +       if (vma->vm_flags & VM_SYNC) {
> > > +               int err = is_synchronous(filp, dax_dev);
> > > +               if (err)
> > > +                       return err;
> > > +       }
> > >  
> > >         file_accessed(filp);
> > >         vma->vm_ops = &xfs_file_vm_ops;
> > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > index 8b42df09b04c..add017de3dd7 100644
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -2162,6 +2162,20 @@ static inline void file_accessed(struct file *file)
> > >                 touch_atime(&file->f_path);
> > >  }
> > >  
> > > +struct dax_device;
> > > +extern bool dax_synchronous(struct dax_device *dax_dev);
> > > +static inline int is_synchronous(struct file *filp, struct dax_device
> > > *dax_dev)
> > > +{
> > > +       struct inode *inode = file_inode(filp);
> > > +
> > > +       if (!IS_DAX(inode))
> > > +               return -EOPNOTSUPP;
> > > +       if (!dax_synchronous(dax_dev))
> > > +               return -EOPNOTSUPP;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  int sync_inode(struct inode *inode, struct writeback_control *wbc);
> > >  int sync_inode_metadata(struct inode *inode, int wait);
> > >  
> > > ---------
> > > 
> > > Thanks,
> > > Pankaj
> > > 
> > > 
> > > 
> > --
> > Jan Kara <jack@suse.com>
> > SUSE Labs, CR
> > 
> > 

^ permalink raw reply

* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Stefan Hajnoczi @ 2019-04-04 14:14 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 1277 bytes --]

On Thu, Apr 04, 2019 at 12:58:34PM +0200, Stefano Garzarella wrote:
> This series tries to increase the throughput of virtio-vsock with slight
> changes:
>  - patch 1/4: reduces the number of credit update messages sent to the
>               transmitter
>  - patch 2/4: allows the host to split packets on multiple buffers,
>               in this way, we can remove the packet size limit to
>               VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE
>  - patch 3/4: uses VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size
>               allowed
>  - patch 4/4: increases RX buffer size to 64 KiB (affects only host->guest)
> 
> RFC:
>  - maybe patch 4 can be replaced with multiple queues with different
>    buffer sizes or using EWMA to adapt the buffer size to the traffic
> 
>  - as Jason suggested in a previous thread [1] I'll evaluate to use
>    virtio-net as transport, but I need to understand better how to
>    interface with it, maybe introducing sk_buff in virtio-vsock.
> 
> Any suggestions?

Great performance results, nice job!

Please include efficiency numbers (bandwidth / CPU utilization) in the
future.  Due to the nature of these optimizations it's unlikely that
efficiency has decreased, so I'm not too worried about it this time.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH] drm/cirrus: rewrite and modernize driver.
From: Gerd Hoffmann @ 2019-04-04 11:45 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: David Airlie, David Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
In-Reply-To: <20190404102722.w6c7aoxzcrhn5xhi@sirius.home.kraxel.org>

  Hi,

> So I guess I have to add a dest_clip bool parameter when moving them.
> /me looks for a good place.  drm_fb_helpers.c I think.

https://git.kraxel.org/cgit/linux/log/?h=drm-rewrite-cirrus updated.

v2 series will follow later today or tomorrow.

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH] drm/cirrus: rewrite and modernize driver.
From: Noralf Trønnes @ 2019-04-04 11:45 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: David Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE, Daniel Vetter,
	David Airlie
In-Reply-To: <20190404102722.w6c7aoxzcrhn5xhi@sirius.home.kraxel.org>



Den 04.04.2019 12.27, skrev Gerd Hoffmann:
>   Hi,
> 
>>> tinydrm_xrgb8888_to_*
>>>
>>> imo these could be put into some drm_format_helpers.c to be shared.
>>
>> I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got
>> there yet.
>>
>> Gerd, if you end up using some of those functions, feel free to move
>> just those you need and I can do the rest later. But if you have time to
>> spare I wouldn't mind getting all of them moved ;-)
> 
> For now I just promoted cirrus to be a tinydrm driver ;)
> 
> Noticed that those helpers (including tinydrm_memcpy for the
> non-converting case) apply clipping on the source but not on
> the destination.  So, for fullscreen updates that works ok,
> but for updating sub-rectangles it doesn't ...

Ah yes, these MIPI type controllers support setting the destination
window in controller RAM for the incoming buffer so there has been no
need for clipping on the destination buffer.

> 
> So I guess I have to add a dest_clip bool parameter when moving them.
> /me looks for a good place.  drm_fb_helpers.c I think.
> 
> cheers,
>   Gerd
> 

^ permalink raw reply

* [PATCH RFC 4/4] vsock/virtio: increase RX buffer size to 64 KiB
From: Stefano Garzarella @ 2019-04-04 10:58 UTC (permalink / raw)
  To: netdev
  Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>

In order to increase host -> guest throughput with large packets,
we can use 64 KiB RX buffers.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/virtio_vsock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 6d7a22cc20bf..43cce304408e 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -10,7 +10,7 @@
 #define VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE	128
 #define VIRTIO_VSOCK_DEFAULT_BUF_SIZE		(1024 * 256)
 #define VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE	(1024 * 256)
-#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 4)
+#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE	(1024 * 64)
 #define VIRTIO_VSOCK_MAX_BUF_SIZE		0xFFFFFFFFUL
 #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE		(1024 * 64)
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH RFC 3/4] vsock/virtio: change the maximum packet size allowed
From: Stefano Garzarella @ 2019-04-04 10:58 UTC (permalink / raw)
  To: netdev
  Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>

Since now we are able to split packets, we can avoid limiting
their sizes to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE.
Instead, we can use VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max
packet size.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f32301d823f5..822e5d07a4ec 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -167,8 +167,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 	vvs = vsk->trans;
 
 	/* we can send less than pkt_len bytes */
-	if (pkt_len > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE)
-		pkt_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+	if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
+		pkt_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
 
 	/* virtio_transport_get_credit might return less than pkt_len credit */
 	pkt_len = virtio_transport_get_credit(vvs, pkt_len);
-- 
2.20.1

^ permalink raw reply related

* [PATCH RFC 2/4] vhost/vsock: split packets to send using multiple buffers
From: Stefano Garzarella @ 2019-04-04 10:58 UTC (permalink / raw)
  To: netdev
  Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>

If the packets to sent to the guest are bigger than the buffer
available, we can split them, using multiple buffers and fixing
the length in the packet header.
This is safe since virtio-vsock supports only stream sockets.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vhost/vsock.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index bb5fc0e9fbc2..9951b7e661f6 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -94,7 +94,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 		struct iov_iter iov_iter;
 		unsigned out, in;
 		size_t nbytes;
-		size_t len;
+		size_t iov_len, payload_len;
 		int head;
 
 		spin_lock_bh(&vsock->send_pkt_list_lock);
@@ -139,8 +139,18 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 			break;
 		}
 
-		len = iov_length(&vq->iov[out], in);
-		iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
+		payload_len = pkt->len - pkt->off;
+		iov_len = iov_length(&vq->iov[out], in);
+		iov_iter_init(&iov_iter, READ, &vq->iov[out], in, iov_len);
+
+		/* If the packet is greater than the space available in the
+		 * buffer, we split it using multiple buffers.
+		 */
+		if (payload_len > iov_len - sizeof(pkt->hdr))
+			payload_len = iov_len - sizeof(pkt->hdr);
+
+		/* Set the correct length in the header */
+		pkt->hdr.len = cpu_to_le32(payload_len);
 
 		nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
 		if (nbytes != sizeof(pkt->hdr)) {
@@ -149,16 +159,29 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 			break;
 		}
 
-		nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
-		if (nbytes != pkt->len) {
+		nbytes = copy_to_iter(pkt->buf + pkt->off, payload_len,
+				      &iov_iter);
+		if (nbytes != payload_len) {
 			virtio_transport_free_pkt(pkt);
 			vq_err(vq, "Faulted on copying pkt buf\n");
 			break;
 		}
 
-		vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+		vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
 		added = true;
 
+		pkt->off += payload_len;
+
+		/* If we didn't send all the payload we can requeue the packet
+		 * to send it with the next available buffer.
+		 */
+		if (pkt->off < pkt->len) {
+			spin_lock_bh(&vsock->send_pkt_list_lock);
+			list_add(&pkt->list, &vsock->send_pkt_list);
+			spin_unlock_bh(&vsock->send_pkt_list_lock);
+			continue;
+		}
+
 		if (pkt->reply) {
 			int val;
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH RFC 1/4] vsock/virtio: reduce credit update messages
From: Stefano Garzarella @ 2019-04-04 10:58 UTC (permalink / raw)
  To: netdev
  Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190404105838.101559-1-sgarzare@redhat.com>

In order to reduce the number of credit update messages,
we send them only when the space available seen by the
transmitter is less than VIRTIO_VSOCK_MAX_PKT_BUF_SIZE.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/virtio_vsock.h            |  1 +
 net/vmw_vsock/virtio_transport_common.c | 14 +++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index e223e2632edd..6d7a22cc20bf 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -37,6 +37,7 @@ struct virtio_vsock_sock {
 	u32 tx_cnt;
 	u32 buf_alloc;
 	u32 peer_fwd_cnt;
+	u32 last_fwd_cnt;
 	u32 peer_buf_alloc;
 
 	/* Protected by rx_lock */
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 602715fc9a75..f32301d823f5 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -206,6 +206,7 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs,
 void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt)
 {
 	spin_lock_bh(&vvs->tx_lock);
+	vvs->last_fwd_cnt = vvs->fwd_cnt;
 	pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt);
 	pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc);
 	spin_unlock_bh(&vvs->tx_lock);
@@ -256,6 +257,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
 	struct virtio_vsock_sock *vvs = vsk->trans;
 	struct virtio_vsock_pkt *pkt;
 	size_t bytes, total = 0;
+	s64 free_space;
 	int err = -EFAULT;
 
 	spin_lock_bh(&vvs->rx_lock);
@@ -288,9 +290,15 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
 	}
 	spin_unlock_bh(&vvs->rx_lock);
 
-	/* Send a credit pkt to peer */
-	virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM,
-					    NULL);
+	/* We send a credit update only when the space available seen
+	 * by the transmitter is less than VIRTIO_VSOCK_MAX_PKT_BUF_SIZE
+	 */
+	free_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt);
+	if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) {
+		virtio_transport_send_credit_update(vsk,
+						    VIRTIO_VSOCK_TYPE_STREAM,
+						    NULL);
+	}
 
 	return total;
 
-- 
2.20.1

^ permalink raw reply related

* [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Stefano Garzarella @ 2019-04-04 10:58 UTC (permalink / raw)
  To: netdev
  Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization,
	Stefan Hajnoczi, David S. Miller

This series tries to increase the throughput of virtio-vsock with slight
changes:
 - patch 1/4: reduces the number of credit update messages sent to the
              transmitter
 - patch 2/4: allows the host to split packets on multiple buffers,
              in this way, we can remove the packet size limit to
              VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE
 - patch 3/4: uses VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size
              allowed
 - patch 4/4: increases RX buffer size to 64 KiB (affects only host->guest)

RFC:
 - maybe patch 4 can be replaced with multiple queues with different
   buffer sizes or using EWMA to adapt the buffer size to the traffic

 - as Jason suggested in a previous thread [1] I'll evaluate to use
   virtio-net as transport, but I need to understand better how to
   interface with it, maybe introducing sk_buff in virtio-vsock.

Any suggestions?

Here some benchmarks step by step. I used iperf3 [2] modified with VSOCK
support:

                        host -> guest [Gbps]
pkt_size    before opt.   patch 1   patches 2+3   patch 4
  64            0.060       0.102       0.102       0.096
  256           0.22        0.40        0.40        0.36
  512           0.42        0.82        0.85        0.74
  1K            0.7         1.6         1.6         1.5
  2K            1.5         3.0         3.1         2.9
  4K            2.5         5.2         5.3         5.3
  8K            3.9         8.4         8.6         8.8
  16K           6.6        11.1        11.3        12.8
  32K           9.9        15.8        15.8        18.1
  64K          13.5        17.4        17.7        21.4
  128K         17.9        19.0        19.0        23.6
  256K         18.0        19.4        19.8        24.4
  512K         18.4        19.6        20.1        25.3

                        guest -> host [Gbps]
pkt_size    before opt.   patch 1   patches 2+3
  64            0.088       0.100       0.101
  256           0.35        0.36        0.41
  512           0.70        0.74        0.73
  1K            1.1         1.3         1.3
  2K            2.4         2.4         2.6
  4K            4.3         4.3         4.5
  8K            7.3         7.4         7.6
  16K           9.2         9.6        11.1
  32K           8.3         8.9        18.1
  64K           8.3         8.9        25.4
  128K          7.2         8.7        26.7
  256K          7.7         8.4        24.9
  512K          7.7         8.5        25.0

Thanks,
Stefano

[1] https://www.spinics.net/lists/netdev/msg531783.html
[2] https://github.com/stefano-garzarella/iperf/

Stefano Garzarella (4):
  vsock/virtio: reduce credit update messages
  vhost/vsock: split packets to send using multiple buffers
  vsock/virtio: change the maximum packet size allowed
  vsock/virtio: increase RX buffer size to 64 KiB

 drivers/vhost/vsock.c                   | 35 ++++++++++++++++++++-----
 include/linux/virtio_vsock.h            |  3 ++-
 net/vmw_vsock/virtio_transport_common.c | 18 +++++++++----
 3 files changed, 44 insertions(+), 12 deletions(-)

-- 
2.20.1

^ permalink raw reply

* Re: [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Pankaj Gupta @ 2019-04-04 10:52 UTC (permalink / raw)
  To: Adam Borowski
  Cc: cohuck, jack, kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, darrick wong, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <20190404095649.GB8738@angband.pl>


> 
> On Thu, Apr 04, 2019 at 02:12:30AM -0400, Pankaj Gupta wrote:
> > > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > > 
> > > xfs_file_mmap(
> > > ....
> > > {
> > > 	struct inode	*inode = file_inode(filp);
> > > 
> > > 	if (vma->vm_flags & VM_SYNC) {
> > > 		if (!IS_DAX(inode))
> > > 			return -EOPNOTSUPP;
> > > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > > 			return -EOPNOTSUPP;
> > > 	}
> > > 
> > > 	file_accessed(filp);
> > > 	vma->vm_ops = &xfs_file_vm_ops;
> > > 	if (IS_DAX(inode))
> > > 		vma->vm_flags |= VM_HUGEPAGE;
> > > 	return 0;
> > > }
> > 
> > Sure, this is better.
> 
> > > Even better, factor out all the "MAP_SYNC supported" checks into a
> > > helper so that the filesystem code just doesn't have to care about
> > > the details of checking for DAX+MAP_SYNC support....
> > 
> > o.k. Will add one common helper function for both ext4 & xfs filesystems.
> 
> Note this pending patch for Goldwyn Rodrigues' patchset for btrfs:
> 
> https://lore.kernel.org/linux-btrfs/20190328102418.5466-1-kilobyte@angband.pl/
> 
> We might want to coordinate.

Sure. Good to know.

Thanks for the pointer. Will have a look.

Best regards,
Pankaj 

> 
> 
> Meow!
> --
> ⢀⣴⠾⠻⢶⣦⠀
> ⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
> ⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
> ⠈⠳⣄⠀⠀⠀⠀
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH] drm/cirrus: rewrite and modernize driver.
From: Gerd Hoffmann @ 2019-04-04 10:27 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: David Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE, Daniel Vetter,
	David Airlie
In-Reply-To: <2866e37e-8563-1f1e-8f16-1119f70d52e1@tronnes.org>

  Hi,

> > tinydrm_xrgb8888_to_*
> > 
> > imo these could be put into some drm_format_helpers.c to be shared.
> 
> I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got
> there yet.
> 
> Gerd, if you end up using some of those functions, feel free to move
> just those you need and I can do the rest later. But if you have time to
> spare I wouldn't mind getting all of them moved ;-)

For now I just promoted cirrus to be a tinydrm driver ;)

Noticed that those helpers (including tinydrm_memcpy for the
non-converting case) apply clipping on the source but not on
the destination.  So, for fullscreen updates that works ok,
but for updating sub-rectangles it doesn't ...

So I guess I have to add a dest_clip bool parameter when moving them.
/me looks for a good place.  drm_fb_helpers.c I think.

cheers,
  Gerd

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Pankaj Gupta @ 2019-04-04 10:08 UTC (permalink / raw)
  To: Jan Kara, Darrick J. Wong
  Cc: kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, cohuck, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <20190404094029.GD16131@quack2.suse.cz>


> On Thu 04-04-19 05:09:10, Pankaj Gupta wrote:
> > 
> > > > On Thu, Apr 04, 2019 at 09:09:12AM +1100, Dave Chinner wrote:
> > > > > On Wed, Apr 03, 2019 at 04:10:18PM +0530, Pankaj Gupta wrote:
> > > > > > Virtio pmem provides asynchronous host page cache flush
> > > > > > mechanism. we don't support 'MAP_SYNC' with virtio pmem
> > > > > > and xfs.
> > > > > > 
> > > > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > > > > ---
> > > > > >  fs/xfs/xfs_file.c | 8 ++++++++
> > > > > >  1 file changed, 8 insertions(+)
> > > > > > 
> > > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > > > > index 1f2e2845eb76..dced2eb8c91a 100644
> > > > > > --- a/fs/xfs/xfs_file.c
> > > > > > +++ b/fs/xfs/xfs_file.c
> > > > > > @@ -1203,6 +1203,14 @@ xfs_file_mmap(
> > > > > >  	if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > > > > >  		return -EOPNOTSUPP;
> > > > > >  
> > > > > > +	/* We don't support synchronous mappings with DAX files if
> > > > > > +	 * dax_device is not synchronous.
> > > > > > +	 */
> > > > > > +	if (IS_DAX(file_inode(filp)) && !dax_synchronous(
> > > > > > +		xfs_find_daxdev_for_inode(file_inode(filp))) &&
> > > > > > +					(vma->vm_flags & VM_SYNC))
> > > > > > +		return -EOPNOTSUPP;
> > > > > > +
> > > > > >  	file_accessed(filp);
> > > > > >  	vma->vm_ops = &xfs_file_vm_ops;
> > > > > >  	if (IS_DAX(file_inode(filp)))
> > > > > 
> > > > > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > > > > 
> > > > > xfs_file_mmap(
> > > > > ....
> > > > > {
> > > > > 	struct inode	*inode = file_inode(filp);
> > > > > 
> > > > > 	if (vma->vm_flags & VM_SYNC) {
> > > > > 		if (!IS_DAX(inode))
> > > > > 			return -EOPNOTSUPP;
> > > > > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > > > > 			return -EOPNOTSUPP;
> > > > > 	}
> > > > > 
> > > > > 	file_accessed(filp);
> > > > > 	vma->vm_ops = &xfs_file_vm_ops;
> > > > > 	if (IS_DAX(inode))
> > > > > 		vma->vm_flags |= VM_HUGEPAGE;
> > > > > 	return 0;
> > > > > }
> > > > > 
> > > > > 
> > > > > Even better, factor out all the "MAP_SYNC supported" checks into a
> > > > > helper so that the filesystem code just doesn't have to care about
> > > > > the details of checking for DAX+MAP_SYNC support....
> > > > 
> > > > Seconded, since ext4 has nearly the same flag validation logic.
> > > 
> > 
> > Only issue with this I see is we need the helper function only for
> > supported
> > filesystems ext4 & xfs (right now). If I create the function in "fs.h" it
> > will be compiled for every filesystem, even for those don't need it.
> > 
> > Sample patch below, does below patch is near to what you have in mind?
> 
> So I would put the helper in include/linux/dax.h and have it like:
> 
> bool daxdev_mapping_supported(struct vm_area_struct *vma,
> 			      struct dax_device *dax_dev)
> {
> 	if (!(vma->vm_flags & VM_SYNC))
> 		return true;
> 	if (!IS_DAX(file_inode(vma->vm_file)))
> 		return false;
> 	return dax_synchronous(dax_dev);
> }

Sure. This is much better. I was also not sure what to name the helper function.
I will go ahead with this unless 'Dave' & 'Darrick' have anything to add.

Thank you very much.

Best regards,
Pankaj 

> 
> 								Honza
> > 
> > =================
> > 
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 1f2e2845eb76..614995170cac 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -1196,12 +1196,17 @@ xfs_file_mmap(
> >         struct file     *filp,
> >         struct vm_area_struct *vma)
> >  {
> > +       struct dax_device *dax_dev =
> > xfs_find_daxdev_for_inode(file_inode(filp));
> > +
> >         /*
> > -        * We don't support synchronous mappings for non-DAX files. At
> > least
> > -        * until someone comes with a sensible use case.
> > +        * We don't support synchronous mappings for non-DAX files and
> > +        * for DAX files if underneath dax_device is not synchronous.
> >          */
> > -       if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > -               return -EOPNOTSUPP;
> > +       if (vma->vm_flags & VM_SYNC) {
> > +               int err = is_synchronous(filp, dax_dev);
> > +               if (err)
> > +                       return err;
> > +       }
> >  
> >         file_accessed(filp);
> >         vma->vm_ops = &xfs_file_vm_ops;
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 8b42df09b04c..add017de3dd7 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2162,6 +2162,20 @@ static inline void file_accessed(struct file *file)
> >                 touch_atime(&file->f_path);
> >  }
> >  
> > +struct dax_device;
> > +extern bool dax_synchronous(struct dax_device *dax_dev);
> > +static inline int is_synchronous(struct file *filp, struct dax_device
> > *dax_dev)
> > +{
> > +       struct inode *inode = file_inode(filp);
> > +
> > +       if (!IS_DAX(inode))
> > +               return -EOPNOTSUPP;
> > +       if (!dax_synchronous(dax_dev))
> > +               return -EOPNOTSUPP;
> > +
> > +       return 0;
> > +}
> > +
> >  int sync_inode(struct inode *inode, struct writeback_control *wbc);
> >  int sync_inode_metadata(struct inode *inode, int wait);
> >  
> > ---------
> > 
> > Thanks,
> > Pankaj
> > 
> > 
> > 
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> 
> 

^ permalink raw reply

* Re: [PATCH] drm/cirrus: rewrite and modernize driver.
From: Noralf Trønnes @ 2019-04-04 10:06 UTC (permalink / raw)
  To: Daniel Vetter, Gerd Hoffmann
  Cc: David Airlie, David Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
In-Reply-To: <CAKMK7uHiWu-kOa6SP0ZRstnrwdYR0MHgtHdHZPLWLXxWaQVPrQ@mail.gmail.com>



Den 04.04.2019 10.52, skrev Daniel Vetter:
> On Thu, Apr 4, 2019 at 10:30 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>>   Hi,
>>
>>>> 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.
>>
>> Have a pointer to these helpers?  grepping around in drm didn't turn up
>> anything so far ...
> 
> tinydrm_xrgb8888_to_*
> 
> imo these could be put into some drm_format_helpers.c to be shared.

I agree, my long term goal is to get rid of tinydrm.ko. Just haven't got
there yet.

Gerd, if you end up using some of those functions, feel free to move
just those you need and I can do the rest later. But if you have time to
spare I wouldn't mind getting all of them moved ;-)

Noralf.

> From a quick look the xrgb8888_to_rgb888 is missing, but for a quick
> hack you can just use rgb565 to get going.
> -Daniel
> 

^ permalink raw reply

* Re: [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Adam Borowski @ 2019-04-04  9:56 UTC (permalink / raw)
  To: Pankaj Gupta
  Cc: cohuck, jack, kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, darrick wong, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <1518295599.17367903.1554358350929.JavaMail.zimbra@redhat.com>

On Thu, Apr 04, 2019 at 02:12:30AM -0400, Pankaj Gupta wrote:
> > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > 
> > xfs_file_mmap(
> > ....
> > {
> > 	struct inode	*inode = file_inode(filp);
> > 
> > 	if (vma->vm_flags & VM_SYNC) {
> > 		if (!IS_DAX(inode))
> > 			return -EOPNOTSUPP;
> > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > 			return -EOPNOTSUPP;
> > 	}
> > 
> > 	file_accessed(filp);
> > 	vma->vm_ops = &xfs_file_vm_ops;
> > 	if (IS_DAX(inode))
> > 		vma->vm_flags |= VM_HUGEPAGE;
> > 	return 0;
> > }
> 
> Sure, this is better.

> > Even better, factor out all the "MAP_SYNC supported" checks into a
> > helper so that the filesystem code just doesn't have to care about
> > the details of checking for DAX+MAP_SYNC support....
> 
> o.k. Will add one common helper function for both ext4 & xfs filesystems.

Note this pending patch for Goldwyn Rodrigues' patchset for btrfs:

https://lore.kernel.org/linux-btrfs/20190328102418.5466-1-kilobyte@angband.pl/

We might want to coordinate.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
⠈⠳⣄⠀⠀⠀⠀
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Jan Kara @ 2019-04-04  9:40 UTC (permalink / raw)
  To: Pankaj Gupta
  Cc: cohuck, jack, kvm, mst, Dave Chinner, qemu-devel, virtualization,
	adilger kernel, zwisler, aarcange, dave jiang, linux-nvdimm,
	vishal l verma, willy, hch, linux-acpi, jmoyer, linux-ext4, lenb,
	riel, stefanha, dan j williams, lcapitulino, nilal, tytso,
	xiaoguangrong eric, Darrick J. Wong, rjw, linux-kernel, linux-xfs,
	linux-fsdevel, imammedo
In-Reply-To: <651986267.17402634.1554368950178.JavaMail.zimbra@redhat.com>

On Thu 04-04-19 05:09:10, Pankaj Gupta wrote:
> 
> > > On Thu, Apr 04, 2019 at 09:09:12AM +1100, Dave Chinner wrote:
> > > > On Wed, Apr 03, 2019 at 04:10:18PM +0530, Pankaj Gupta wrote:
> > > > > Virtio pmem provides asynchronous host page cache flush
> > > > > mechanism. we don't support 'MAP_SYNC' with virtio pmem
> > > > > and xfs.
> > > > > 
> > > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > > > ---
> > > > >  fs/xfs/xfs_file.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > > > index 1f2e2845eb76..dced2eb8c91a 100644
> > > > > --- a/fs/xfs/xfs_file.c
> > > > > +++ b/fs/xfs/xfs_file.c
> > > > > @@ -1203,6 +1203,14 @@ xfs_file_mmap(
> > > > >  	if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > > > >  		return -EOPNOTSUPP;
> > > > >  
> > > > > +	/* We don't support synchronous mappings with DAX files if
> > > > > +	 * dax_device is not synchronous.
> > > > > +	 */
> > > > > +	if (IS_DAX(file_inode(filp)) && !dax_synchronous(
> > > > > +		xfs_find_daxdev_for_inode(file_inode(filp))) &&
> > > > > +					(vma->vm_flags & VM_SYNC))
> > > > > +		return -EOPNOTSUPP;
> > > > > +
> > > > >  	file_accessed(filp);
> > > > >  	vma->vm_ops = &xfs_file_vm_ops;
> > > > >  	if (IS_DAX(file_inode(filp)))
> > > > 
> > > > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > > > 
> > > > xfs_file_mmap(
> > > > ....
> > > > {
> > > > 	struct inode	*inode = file_inode(filp);
> > > > 
> > > > 	if (vma->vm_flags & VM_SYNC) {
> > > > 		if (!IS_DAX(inode))
> > > > 			return -EOPNOTSUPP;
> > > > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > > > 			return -EOPNOTSUPP;
> > > > 	}
> > > > 
> > > > 	file_accessed(filp);
> > > > 	vma->vm_ops = &xfs_file_vm_ops;
> > > > 	if (IS_DAX(inode))
> > > > 		vma->vm_flags |= VM_HUGEPAGE;
> > > > 	return 0;
> > > > }
> > > > 
> > > > 
> > > > Even better, factor out all the "MAP_SYNC supported" checks into a
> > > > helper so that the filesystem code just doesn't have to care about
> > > > the details of checking for DAX+MAP_SYNC support....
> > > 
> > > Seconded, since ext4 has nearly the same flag validation logic.
> > 
> 
> Only issue with this I see is we need the helper function only for supported
> filesystems ext4 & xfs (right now). If I create the function in "fs.h" it 
> will be compiled for every filesystem, even for those don't need it.  
> 
> Sample patch below, does below patch is near to what you have in mind?

So I would put the helper in include/linux/dax.h and have it like:

bool daxdev_mapping_supported(struct vm_area_struct *vma,
			      struct dax_device *dax_dev)
{
	if (!(vma->vm_flags & VM_SYNC))
		return true;
	if (!IS_DAX(file_inode(vma->vm_file)))
		return false;
	return dax_synchronous(dax_dev);
}

								Honza
> 
> =================
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 1f2e2845eb76..614995170cac 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1196,12 +1196,17 @@ xfs_file_mmap(
>         struct file     *filp,
>         struct vm_area_struct *vma)
>  {
> +       struct dax_device *dax_dev = xfs_find_daxdev_for_inode(file_inode(filp));
> +
>         /*
> -        * We don't support synchronous mappings for non-DAX files. At least
> -        * until someone comes with a sensible use case.
> +        * We don't support synchronous mappings for non-DAX files and
> +        * for DAX files if underneath dax_device is not synchronous.
>          */
> -       if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> -               return -EOPNOTSUPP;
> +       if (vma->vm_flags & VM_SYNC) {
> +               int err = is_synchronous(filp, dax_dev);
> +               if (err)
> +                       return err;
> +       }
>  
>         file_accessed(filp);
>         vma->vm_ops = &xfs_file_vm_ops;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8b42df09b04c..add017de3dd7 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2162,6 +2162,20 @@ static inline void file_accessed(struct file *file)
>                 touch_atime(&file->f_path);
>  }
>  
> +struct dax_device;
> +extern bool dax_synchronous(struct dax_device *dax_dev);
> +static inline int is_synchronous(struct file *filp, struct dax_device *dax_dev)
> +{
> +       struct inode *inode = file_inode(filp);
> +
> +       if (!IS_DAX(inode))
> +               return -EOPNOTSUPP;
> +       if (!dax_synchronous(dax_dev))
> +               return -EOPNOTSUPP;
> +
> +       return 0;
> +}
> +
>  int sync_inode(struct inode *inode, struct writeback_control *wbc);
>  int sync_inode_metadata(struct inode *inode, int wait);
>  
> ---------
> 
> Thanks,
> Pankaj
> 
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH v4 5/5] xfs: disable map_sync for async flush
From: Pankaj Gupta @ 2019-04-04  9:09 UTC (permalink / raw)
  To: Darrick J. Wong, Dave Chinner
  Cc: jack, kvm, mst, qemu-devel, virtualization, adilger kernel,
	zwisler, aarcange, dave jiang, linux-nvdimm, vishal l verma,
	willy, hch, linux-acpi, jmoyer, linux-ext4, lenb, riel, stefanha,
	dan j williams, lcapitulino, nilal, tytso, xiaoguangrong eric,
	cohuck, rjw, linux-kernel, linux-xfs, linux-fsdevel, imammedo
In-Reply-To: <1508883801.17367965.1554358418550.JavaMail.zimbra@redhat.com>


> > On Thu, Apr 04, 2019 at 09:09:12AM +1100, Dave Chinner wrote:
> > > On Wed, Apr 03, 2019 at 04:10:18PM +0530, Pankaj Gupta wrote:
> > > > Virtio pmem provides asynchronous host page cache flush
> > > > mechanism. we don't support 'MAP_SYNC' with virtio pmem
> > > > and xfs.
> > > > 
> > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > > ---
> > > >  fs/xfs/xfs_file.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > > > index 1f2e2845eb76..dced2eb8c91a 100644
> > > > --- a/fs/xfs/xfs_file.c
> > > > +++ b/fs/xfs/xfs_file.c
> > > > @@ -1203,6 +1203,14 @@ xfs_file_mmap(
> > > >  	if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > > >  		return -EOPNOTSUPP;
> > > >  
> > > > +	/* We don't support synchronous mappings with DAX files if
> > > > +	 * dax_device is not synchronous.
> > > > +	 */
> > > > +	if (IS_DAX(file_inode(filp)) && !dax_synchronous(
> > > > +		xfs_find_daxdev_for_inode(file_inode(filp))) &&
> > > > +					(vma->vm_flags & VM_SYNC))
> > > > +		return -EOPNOTSUPP;
> > > > +
> > > >  	file_accessed(filp);
> > > >  	vma->vm_ops = &xfs_file_vm_ops;
> > > >  	if (IS_DAX(file_inode(filp)))
> > > 
> > > All this ad hoc IS_DAX conditional logic is getting pretty nasty.
> > > 
> > > xfs_file_mmap(
> > > ....
> > > {
> > > 	struct inode	*inode = file_inode(filp);
> > > 
> > > 	if (vma->vm_flags & VM_SYNC) {
> > > 		if (!IS_DAX(inode))
> > > 			return -EOPNOTSUPP;
> > > 		if (!dax_synchronous(xfs_find_daxdev_for_inode(inode))
> > > 			return -EOPNOTSUPP;
> > > 	}
> > > 
> > > 	file_accessed(filp);
> > > 	vma->vm_ops = &xfs_file_vm_ops;
> > > 	if (IS_DAX(inode))
> > > 		vma->vm_flags |= VM_HUGEPAGE;
> > > 	return 0;
> > > }
> > > 
> > > 
> > > Even better, factor out all the "MAP_SYNC supported" checks into a
> > > helper so that the filesystem code just doesn't have to care about
> > > the details of checking for DAX+MAP_SYNC support....
> > 
> > Seconded, since ext4 has nearly the same flag validation logic.
> 

Only issue with this I see is we need the helper function only for supported
filesystems ext4 & xfs (right now). If I create the function in "fs.h" it 
will be compiled for every filesystem, even for those don't need it.  

Sample patch below, does below patch is near to what you have in mind?

=================

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 1f2e2845eb76..614995170cac 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1196,12 +1196,17 @@ xfs_file_mmap(
        struct file     *filp,
        struct vm_area_struct *vma)
 {
+       struct dax_device *dax_dev = xfs_find_daxdev_for_inode(file_inode(filp));
+
        /*
-        * We don't support synchronous mappings for non-DAX files. At least
-        * until someone comes with a sensible use case.
+        * We don't support synchronous mappings for non-DAX files and
+        * for DAX files if underneath dax_device is not synchronous.
         */
-       if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
-               return -EOPNOTSUPP;
+       if (vma->vm_flags & VM_SYNC) {
+               int err = is_synchronous(filp, dax_dev);
+               if (err)
+                       return err;
+       }
 
        file_accessed(filp);
        vma->vm_ops = &xfs_file_vm_ops;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8b42df09b04c..add017de3dd7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2162,6 +2162,20 @@ static inline void file_accessed(struct file *file)
                touch_atime(&file->f_path);
 }
 
+struct dax_device;
+extern bool dax_synchronous(struct dax_device *dax_dev);
+static inline int is_synchronous(struct file *filp, struct dax_device *dax_dev)
+{
+       struct inode *inode = file_inode(filp);
+
+       if (!IS_DAX(inode))
+               return -EOPNOTSUPP;
+       if (!dax_synchronous(dax_dev))
+               return -EOPNOTSUPP;
+
+       return 0;
+}
+
 int sync_inode(struct inode *inode, struct writeback_control *wbc);
 int sync_inode_metadata(struct inode *inode, int wait);
 
---------

Thanks,
Pankaj

^ permalink raw reply related

* Re: [PATCH] drm/cirrus: rewrite and modernize driver.
From: Daniel Vetter @ 2019-04-04  8:52 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: David Airlie, David Airlie, open list, dri-devel,
	open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE
In-Reply-To: <20190404083034.f7vrvukzqx5v7qju@sirius.home.kraxel.org>

On Thu, Apr 4, 2019 at 10:30 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > 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.
>
> Have a pointer to these helpers?  grepping around in drm didn't turn up
> anything so far ...

tinydrm_xrgb8888_to_*

imo these could be put into some drm_format_helpers.c to be shared.
From a quick look the xrgb8888_to_rgb888 is missing, but for a quick
hack you can just use rgb565 to get going.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox