[PATCH 00/10] rm/bochs: Modernize driver

[PATCH 00/10] rm/bochs: Modernize driver

[Thomas Zimmermann]

Bochs is lagging behind the overall state of DRM. This series gives
the driver an update.

Patch 1 removes duplicated functionality that is already handled
by the DRM core.

Patches 2 and 3 streamlines driver cleanup. Patch 2 reworks EDID
handling to follow current best practices. All buffers with EDID
data will now be cleaned up automatically. Patch 3 adds managed
cleanups for I/O resources. No fini function is needed.

Patches 4 to 6 embed struct drm_device in struct bochs_device and
remove all uses of dev_private.

Patch 7 replaces simple display helpers with regular atomic helpers.
The former are a mid-layer that is more often 'in the way' than helping.
Regular atomic helpers are composable with each other. Simple-pipe
is not.

Patch 8 replaces GEM VRAM with GEM SHMEM. The new memory manager
is more reliable and allows for larger resolutions. Display updates
were so slow that Gnome was unmanageable with a flickering cursor and
single FPS. The new memory management makes Gnome at least useable.

Patch 9 implements display-mode validation against the available video
memory. Modes should now be useable iff they passed mode_valid.

Patch 10 removes code from GEM VRAM helpers that is now no longer in
use.

Tested with qemu emulation.

Thomas Zimmermann (10):
  drm/bochs: Remove manual format test from fb_create
  drm/bochs: Use helpers for struct drm_edid
  drm/bochs: Do managed resource cleanup
  drm/bochs: Pass bochs device to various functions
  drm/bochs: Upcast with to_bochs_device()
  drm/bochs: Allocate DRM device in struct bochs_device
  drm/bochs: Use regular atomic helpers
  drm/bochs: Use GEM SHMEM helpers for memory management
  drm/bochs: Validate display modes against available video memory
  drm/gem-vram: Remove support for simple display pipelines

 drivers/gpu/drm/drm_gem_vram_helper.c |  45 ---
 drivers/gpu/drm/tiny/Kconfig          |   4 +-
 drivers/gpu/drm/tiny/bochs.c          | 384 +++++++++++++++-----------
 include/drm/drm_gem_vram_helper.h     |  13 -
 4 files changed, 224 insertions(+), 222 deletions(-)

-- 
2.46.0

[PATCH 01/10] drm/bochs: Remove manual format test from fb_create

[Thomas Zimmermann]

An updated implementation of drm_gem_fb_create() already tests the
driver's planes for supported formats. [1] No need to duplicate this
test in bochs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://elixir.bootlin.com/linux/v6.9/source/drivers/gpu/drm/drm_gem_framebuffer_helper.c#L169 # 1
---
 drivers/gpu/drm/tiny/bochs.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 31fc5d839e10..47a45a14306c 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -511,19 +511,8 @@ static void bochs_connector_init(struct drm_device *dev)
 	}
 }
 
-static struct drm_framebuffer *
-bochs_gem_fb_create(struct drm_device *dev, struct drm_file *file,
-		    const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	if (mode_cmd->pixel_format != DRM_FORMAT_XRGB8888 &&
-	    mode_cmd->pixel_format != DRM_FORMAT_BGRX8888)
-		return ERR_PTR(-EINVAL);
-
-	return drm_gem_fb_create(dev, file, mode_cmd);
-}
-
 static const struct drm_mode_config_funcs bochs_mode_funcs = {
-	.fb_create = bochs_gem_fb_create,
+	.fb_create = drm_gem_fb_create,
 	.mode_valid = drm_vram_helper_mode_valid,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.46.0

[PATCH 02/10] drm/bochs: Use helpers for struct drm_edid

[Thomas Zimmermann]

Implement a read function for struct drm_edid and read the EDID data
with drm_edit_read_custom(). Update the connector data accordingly.

The EDID data comes from the emulator itself and the connector stores
a copy in its EDID property. The drm_edid field in struct bochs_device
is therefore not required. Remove it.

If qemu provides no EDID data, install default display modes as before.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 48 +++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 47a45a14306c..197fc00b373f 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -85,7 +85,6 @@ struct bochs_device {
 	u16 yres_virtual;
 	u32 stride;
 	u32 bpp;
-	const struct drm_edid *drm_edid;
 
 	/* drm */
 	struct drm_device *dev;
@@ -172,12 +171,14 @@ static void bochs_hw_set_little_endian(struct bochs_device *bochs)
 #define bochs_hw_set_native_endian(_b) bochs_hw_set_little_endian(_b)
 #endif
 
-static int bochs_get_edid_block(void *data, u8 *buf,
-				unsigned int block, size_t len)
+static int bochs_get_edid_block(void *data, u8 *buf, unsigned int block, size_t len)
 {
 	struct bochs_device *bochs = data;
 	size_t i, start = block * EDID_LENGTH;
 
+	if (!bochs->mmio)
+		return -1;
+
 	if (start + len > 0x400 /* vga register offset */)
 		return -1;
 
@@ -187,25 +188,20 @@ static int bochs_get_edid_block(void *data, u8 *buf,
 	return 0;
 }
 
-static int bochs_hw_load_edid(struct bochs_device *bochs)
+static const struct drm_edid *bochs_hw_read_edid(struct drm_connector *connector)
 {
+	struct drm_device *dev = connector->dev;
+	struct bochs_device *bochs = dev->dev_private;
 	u8 header[8];
 
-	if (!bochs->mmio)
-		return -1;
-
 	/* check header to detect whenever edid support is enabled in qemu */
 	bochs_get_edid_block(bochs, header, 0, ARRAY_SIZE(header));
 	if (drm_edid_header_is_valid(header) != 8)
-		return -1;
+		return NULL;
 
-	drm_edid_free(bochs->drm_edid);
-	bochs->drm_edid = drm_edid_read_custom(&bochs->connector,
-					       bochs_get_edid_block, bochs);
-	if (!bochs->drm_edid)
-		return -1;
+	drm_dbg(dev, "Found EDID data blob.\n");
 
-	return 0;
+	return drm_edid_read_custom(connector, bochs_get_edid_block, bochs);
 }
 
 static int bochs_hw_init(struct drm_device *dev)
@@ -303,7 +299,6 @@ static void bochs_hw_fini(struct drm_device *dev)
 	if (bochs->fb_map)
 		iounmap(bochs->fb_map);
 	pci_release_regions(to_pci_dev(dev->dev));
-	drm_edid_free(bochs->drm_edid);
 }
 
 static void bochs_hw_blank(struct bochs_device *bochs, bool blank)
@@ -469,21 +464,28 @@ static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
 	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
 };
 
-static int bochs_connector_get_modes(struct drm_connector *connector)
+static int bochs_connector_helper_get_modes(struct drm_connector *connector)
 {
+	const struct drm_edid *edid;
 	int count;
 
-	count = drm_edid_connector_add_modes(connector);
+	edid = bochs_hw_read_edid(connector);
 
-	if (!count) {
+	if (edid) {
+		drm_edid_connector_update(connector, edid);
+		count = drm_edid_connector_add_modes(connector);
+		drm_edid_free(edid);
+	} else {
+		drm_edid_connector_update(connector, NULL);
 		count = drm_add_modes_noedid(connector, 8192, 8192);
 		drm_set_preferred_mode(connector, defx, defy);
 	}
+
 	return count;
 }
 
 static const struct drm_connector_helper_funcs bochs_connector_connector_helper_funcs = {
-	.get_modes = bochs_connector_get_modes,
+	.get_modes = bochs_connector_helper_get_modes,
 };
 
 static const struct drm_connector_funcs bochs_connector_connector_funcs = {
@@ -501,14 +503,8 @@ static void bochs_connector_init(struct drm_device *dev)
 
 	drm_connector_init(dev, connector, &bochs_connector_connector_funcs,
 			   DRM_MODE_CONNECTOR_VIRTUAL);
+	drm_connector_attach_edid_property(connector);
 	drm_connector_helper_add(connector, &bochs_connector_connector_helper_funcs);
-
-	bochs_hw_load_edid(bochs);
-	if (bochs->drm_edid) {
-		DRM_INFO("Found EDID data blob.\n");
-		drm_connector_attach_edid_property(connector);
-		drm_edid_connector_update(&bochs->connector, bochs->drm_edid);
-	}
 }
 
 static const struct drm_mode_config_funcs bochs_mode_funcs = {
-- 
2.46.0

[PATCH 03/10] drm/bochs: Do managed resource cleanup

[Thomas Zimmermann]

Do managed cleanup of all PCI resources. Remove the now-unused cleanup
helper bochs_hw_fini().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 42 +++++++++---------------------------
 1 file changed, 10 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 197fc00b373f..5d09b4cb28ed 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -212,14 +212,14 @@ static int bochs_hw_init(struct drm_device *dev)
 	u16 id;
 
 	if (pdev->resource[2].flags & IORESOURCE_MEM) {
+		ioaddr = pci_resource_start(pdev, 2);
+		iosize = pci_resource_len(pdev, 2);
 		/* mmio bar with vga and bochs registers present */
-		if (pci_request_region(pdev, 2, "bochs-drm") != 0) {
+		if (!devm_request_mem_region(&pdev->dev, ioaddr, iosize, "bochs-drm")) {
 			DRM_ERROR("Cannot request mmio region\n");
 			return -EBUSY;
 		}
-		ioaddr = pci_resource_start(pdev, 2);
-		iosize = pci_resource_len(pdev, 2);
-		bochs->mmio = ioremap(ioaddr, iosize);
+		bochs->mmio = devm_ioremap(&pdev->dev, ioaddr, iosize);
 		if (bochs->mmio == NULL) {
 			DRM_ERROR("Cannot map mmio region\n");
 			return -ENOMEM;
@@ -227,7 +227,7 @@ static int bochs_hw_init(struct drm_device *dev)
 	} else {
 		ioaddr = VBE_DISPI_IOPORT_INDEX;
 		iosize = 2;
-		if (!request_region(ioaddr, iosize, "bochs-drm")) {
+		if (!devm_request_region(&pdev->dev, ioaddr, iosize, "bochs-drm")) {
 			DRM_ERROR("Cannot request ioports\n");
 			return -EBUSY;
 		}
@@ -254,10 +254,10 @@ static int bochs_hw_init(struct drm_device *dev)
 		size = min(size, mem);
 	}
 
-	if (pci_request_region(pdev, 0, "bochs-drm") != 0)
+	if (!devm_request_mem_region(&pdev->dev, addr, size, "bochs-drm"))
 		DRM_WARN("Cannot request framebuffer, boot fb still active?\n");
 
-	bochs->fb_map = ioremap(addr, size);
+	bochs->fb_map = devm_ioremap(&pdev->dev, addr, size);
 	if (bochs->fb_map == NULL) {
 		DRM_ERROR("Cannot map framebuffer\n");
 		return -ENOMEM;
@@ -286,21 +286,6 @@ static int bochs_hw_init(struct drm_device *dev)
 	return 0;
 }
 
-static void bochs_hw_fini(struct drm_device *dev)
-{
-	struct bochs_device *bochs = dev->dev_private;
-
-	/* TODO: shot down existing vram mappings */
-
-	if (bochs->mmio)
-		iounmap(bochs->mmio);
-	if (bochs->ioports)
-		release_region(VBE_DISPI_IOPORT_INDEX, 2);
-	if (bochs->fb_map)
-		iounmap(bochs->fb_map);
-	pci_release_regions(to_pci_dev(dev->dev));
-}
-
 static void bochs_hw_blank(struct bochs_device *bochs, bool blank)
 {
 	DRM_DEBUG_DRIVER("hw_blank %d\n", blank);
@@ -565,17 +550,13 @@ static int bochs_load(struct drm_device *dev)
 
 	ret = drmm_vram_helper_init(dev, bochs->fb_base, bochs->fb_size);
 	if (ret)
-		goto err_hw_fini;
+		return ret;
 
 	ret = bochs_kms_init(bochs);
 	if (ret)
-		goto err_hw_fini;
+		return ret;
 
 	return 0;
-
-err_hw_fini:
-	bochs_hw_fini(dev);
-	return ret;
 }
 
 DEFINE_DRM_GEM_FOPS(bochs_fops);
@@ -650,13 +631,11 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 
 	ret = drm_dev_register(dev, 0);
 	if (ret)
-		goto err_hw_fini;
+		goto err_free_dev;
 
 	drm_fbdev_ttm_setup(dev, 32);
 	return ret;
 
-err_hw_fini:
-	bochs_hw_fini(dev);
 err_free_dev:
 	drm_dev_put(dev);
 	return ret;
@@ -668,7 +647,6 @@ static void bochs_pci_remove(struct pci_dev *pdev)
 
 	drm_dev_unplug(dev);
 	drm_atomic_helper_shutdown(dev);
-	bochs_hw_fini(dev);
 	drm_dev_put(dev);
 }
 
-- 
2.46.0

[PATCH 04/10] drm/bochs: Pass bochs device to various functions

[Thomas Zimmermann]

Avoid upcasting from struct drm_device by passing the bochs device
directly to functions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 5d09b4cb28ed..acb2652577d4 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -204,9 +204,9 @@ static const struct drm_edid *bochs_hw_read_edid(struct drm_connector *connector
 	return drm_edid_read_custom(connector, bochs_get_edid_block, bochs);
 }
 
-static int bochs_hw_init(struct drm_device *dev)
+static int bochs_hw_init(struct bochs_device *bochs)
 {
-	struct bochs_device *bochs = dev->dev_private;
+	struct drm_device *dev = bochs->dev;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	unsigned long addr, size, mem, ioaddr, iosize;
 	u16 id;
@@ -481,9 +481,9 @@ static const struct drm_connector_funcs bochs_connector_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static void bochs_connector_init(struct drm_device *dev)
+static void bochs_connector_init(struct bochs_device *bochs)
 {
-	struct bochs_device *bochs = dev->dev_private;
+	struct drm_device *dev = bochs->dev;
 	struct drm_connector *connector = &bochs->connector;
 
 	drm_connector_init(dev, connector, &bochs_connector_connector_funcs,
@@ -516,7 +516,7 @@ static int bochs_kms_init(struct bochs_device *bochs)
 
 	bochs->dev->mode_config.funcs = &bochs_mode_funcs;
 
-	bochs_connector_init(bochs->dev);
+	bochs_connector_init(bochs);
 	drm_simple_display_pipe_init(bochs->dev,
 				     &bochs->pipe,
 				     &bochs_pipe_funcs,
@@ -544,7 +544,7 @@ static int bochs_load(struct drm_device *dev)
 	dev->dev_private = bochs;
 	bochs->dev = dev;
 
-	ret = bochs_hw_init(dev);
+	ret = bochs_hw_init(bochs);
 	if (ret)
 		return ret;
 
-- 
2.46.0

[PATCH 05/10] drm/bochs: Upcast with to_bochs_device()

[Thomas Zimmermann]

The dev_private field in struct drm_device is deprecated. Limit its
use by moving it into a helper function. A later change will remove
it entirely.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index acb2652577d4..61b2b7aa03cb 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -92,6 +92,11 @@ struct bochs_device {
 	struct drm_connector connector;
 };
 
+static struct bochs_device *to_bochs_device(const struct drm_device *dev)
+{
+	return (struct bochs_device *)dev->dev_private;
+}
+
 /* ---------------------------------------------------------------------- */
 
 static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
@@ -191,7 +196,7 @@ static int bochs_get_edid_block(void *data, u8 *buf, unsigned int block, size_t
 static const struct drm_edid *bochs_hw_read_edid(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
-	struct bochs_device *bochs = dev->dev_private;
+	struct bochs_device *bochs = to_bochs_device(dev);
 	u8 header[8];
 
 	/* check header to detect whenever edid support is enabled in qemu */
@@ -420,7 +425,7 @@ static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
 			      struct drm_crtc_state *crtc_state,
 			      struct drm_plane_state *plane_state)
 {
-	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
+	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
 
 	bochs_hw_setmode(bochs, &crtc_state->mode);
 	bochs_plane_update(bochs, plane_state);
@@ -428,7 +433,7 @@ static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
+	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
 
 	bochs_hw_blank(bochs, true);
 }
@@ -436,7 +441,7 @@ static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
 static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 			      struct drm_plane_state *old_state)
 {
-	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
+	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
 
 	bochs_plane_update(bochs, pipe->plane.state);
 }
-- 
2.46.0

[PATCH 06/10] drm/bochs: Allocate DRM device in struct bochs_device

[Thomas Zimmermann]

Allocate an instance of struct drm_device in struct bochs_device. Also
remove all uses of dev_private from bochs and upcast from the embedded
instance if necessary.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 52 +++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 61b2b7aa03cb..5679f1b090af 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -71,6 +71,8 @@ enum bochs_types {
 };
 
 struct bochs_device {
+	struct drm_device dev;
+
 	/* hw */
 	void __iomem   *mmio;
 	int            ioports;
@@ -87,14 +89,13 @@ struct bochs_device {
 	u32 bpp;
 
 	/* drm */
-	struct drm_device *dev;
 	struct drm_simple_display_pipe pipe;
 	struct drm_connector connector;
 };
 
 static struct bochs_device *to_bochs_device(const struct drm_device *dev)
 {
-	return (struct bochs_device *)dev->dev_private;
+	return container_of(dev, struct bochs_device, dev);
 }
 
 /* ---------------------------------------------------------------------- */
@@ -211,7 +212,7 @@ static const struct drm_edid *bochs_hw_read_edid(struct drm_connector *connector
 
 static int bochs_hw_init(struct bochs_device *bochs)
 {
-	struct drm_device *dev = bochs->dev;
+	struct drm_device *dev = &bochs->dev;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	unsigned long addr, size, mem, ioaddr, iosize;
 	u16 id;
@@ -306,7 +307,7 @@ static void bochs_hw_setmode(struct bochs_device *bochs, struct drm_display_mode
 {
 	int idx;
 
-	if (!drm_dev_enter(bochs->dev, &idx))
+	if (!drm_dev_enter(&bochs->dev, &idx))
 		return;
 
 	bochs->xres = mode->hdisplay;
@@ -342,7 +343,7 @@ static void bochs_hw_setformat(struct bochs_device *bochs, const struct drm_form
 {
 	int idx;
 
-	if (!drm_dev_enter(bochs->dev, &idx))
+	if (!drm_dev_enter(&bochs->dev, &idx))
 		return;
 
 	DRM_DEBUG_DRIVER("format %c%c%c%c\n",
@@ -373,7 +374,7 @@ static void bochs_hw_setbase(struct bochs_device *bochs, int x, int y, int strid
 	unsigned long offset;
 	unsigned int vx, vy, vwidth, idx;
 
-	if (!drm_dev_enter(bochs->dev, &idx))
+	if (!drm_dev_enter(&bochs->dev, &idx))
 		return;
 
 	bochs->stride = stride;
@@ -488,7 +489,7 @@ static const struct drm_connector_funcs bochs_connector_connector_funcs = {
 
 static void bochs_connector_init(struct bochs_device *bochs)
 {
-	struct drm_device *dev = bochs->dev;
+	struct drm_device *dev = &bochs->dev;
 	struct drm_connector *connector = &bochs->connector;
 
 	drm_connector_init(dev, connector, &bochs_connector_connector_funcs,
@@ -506,23 +507,24 @@ static const struct drm_mode_config_funcs bochs_mode_funcs = {
 
 static int bochs_kms_init(struct bochs_device *bochs)
 {
+	struct drm_device *dev = &bochs->dev;
 	int ret;
 
-	ret = drmm_mode_config_init(bochs->dev);
+	ret = drmm_mode_config_init(dev);
 	if (ret)
 		return ret;
 
-	bochs->dev->mode_config.max_width = 8192;
-	bochs->dev->mode_config.max_height = 8192;
+	dev->mode_config.max_width = 8192;
+	dev->mode_config.max_height = 8192;
 
-	bochs->dev->mode_config.preferred_depth = 24;
-	bochs->dev->mode_config.prefer_shadow = 0;
-	bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
+	dev->mode_config.preferred_depth = 24;
+	dev->mode_config.prefer_shadow = 0;
+	dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
 
-	bochs->dev->mode_config.funcs = &bochs_mode_funcs;
+	dev->mode_config.funcs = &bochs_mode_funcs;
 
 	bochs_connector_init(bochs);
-	drm_simple_display_pipe_init(bochs->dev,
+	drm_simple_display_pipe_init(dev,
 				     &bochs->pipe,
 				     &bochs_pipe_funcs,
 				     bochs_formats,
@@ -530,7 +532,7 @@ static int bochs_kms_init(struct bochs_device *bochs)
 				     NULL,
 				     &bochs->connector);
 
-	drm_mode_config_reset(bochs->dev);
+	drm_mode_config_reset(dev);
 
 	return 0;
 }
@@ -538,17 +540,11 @@ static int bochs_kms_init(struct bochs_device *bochs)
 /* ---------------------------------------------------------------------- */
 /* drm interface                                                          */
 
-static int bochs_load(struct drm_device *dev)
+static int bochs_load(struct bochs_device *bochs)
 {
-	struct bochs_device *bochs;
+	struct drm_device *dev = &bochs->dev;
 	int ret;
 
-	bochs = drmm_kzalloc(dev, sizeof(*bochs), GFP_KERNEL);
-	if (bochs == NULL)
-		return -ENOMEM;
-	dev->dev_private = bochs;
-	bochs->dev = dev;
-
 	ret = bochs_hw_init(bochs);
 	if (ret)
 		return ret;
@@ -606,6 +602,7 @@ static const struct dev_pm_ops bochs_pm_ops = {
 
 static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
+	struct bochs_device *bochs;
 	struct drm_device *dev;
 	unsigned long fbsize;
 	int ret;
@@ -620,9 +617,10 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 	if (ret)
 		return ret;
 
-	dev = drm_dev_alloc(&bochs_driver, &pdev->dev);
-	if (IS_ERR(dev))
+	bochs = devm_drm_dev_alloc(&pdev->dev, &bochs_driver, struct bochs_device, dev);
+	if (IS_ERR(bochs))
 		return PTR_ERR(dev);
+	dev = &bochs->dev;
 
 	ret = pcim_enable_device(pdev);
 	if (ret)
@@ -630,7 +628,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 
 	pci_set_drvdata(pdev, dev);
 
-	ret = bochs_load(dev);
+	ret = bochs_load(bochs);
 	if (ret)
 		goto err_free_dev;
 
-- 
2.46.0

[PATCH 07/10] drm/bochs: Use regular atomic helpers

[Thomas Zimmermann]

Remove the simple display pipeline in favor of the regular atomic
helpers in bochs. The simple-pipe helpers are considered deprecated
in DRM.

This effectivly inlines the simple-pipe code for plane and CRTC
support. Instead of a single update helper, there's now a mode-set
helper for the CRTC and an update helper for the plane. The encoder
changes type from NONE ot VIRTUAL.

Removing simple-pipe helpers from bochs will allow for related
cleanups in GEM VRAM helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 177 ++++++++++++++++++++++++++---------
 1 file changed, 132 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 5679f1b090af..76e29950a807 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -4,6 +4,7 @@
 #include <linux/pci.h>
 
 #include <drm/drm_aperture.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_edid.h>
@@ -14,8 +15,8 @@
 #include <drm/drm_gem_vram_helper.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_module.h>
+#include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include <video/vga.h>
 
@@ -89,7 +90,9 @@ struct bochs_device {
 	u32 bpp;
 
 	/* drm */
-	struct drm_simple_display_pipe pipe;
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
 	struct drm_connector connector;
 };
 
@@ -396,11 +399,34 @@ static void bochs_hw_setbase(struct bochs_device *bochs, int x, int y, int strid
 
 /* ---------------------------------------------------------------------- */
 
-static const uint32_t bochs_formats[] = {
+static const uint32_t bochs_primary_plane_formats[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_BGRX8888,
 };
 
+static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
+						   struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_crtc *new_crtc = new_plane_state->crtc;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	int ret;
+
+	if (new_crtc)
+		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
+
+	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
+						  DRM_PLANE_NO_SCALING,
+						  DRM_PLANE_NO_SCALING,
+						  false, false);
+	if (ret)
+		return ret;
+	else if (!new_plane_state->visible)
+		return 0;
+
+	return 0;
+}
+
 static void bochs_plane_update(struct bochs_device *bochs, struct drm_plane_state *state)
 {
 	struct drm_gem_vram_object *gbo;
@@ -422,37 +448,80 @@ static void bochs_plane_update(struct bochs_device *bochs, struct drm_plane_stat
 	bochs_hw_setformat(bochs, state->fb->format);
 }
 
-static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
-			      struct drm_crtc_state *crtc_state,
-			      struct drm_plane_state *plane_state)
+static void bochs_primary_plane_helper_atomic_update(struct drm_plane *plane,
+						     struct drm_atomic_state *state)
 {
-	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
+	struct bochs_device *bochs = to_bochs_device(plane->dev);
+	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
 
-	bochs_hw_setmode(bochs, &crtc_state->mode);
 	bochs_plane_update(bochs, plane_state);
 }
 
-static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
+static const struct drm_plane_helper_funcs bochs_primary_plane_helper_funcs = {
+	DRM_GEM_VRAM_PLANE_HELPER_FUNCS,
+	.atomic_check = bochs_primary_plane_helper_atomic_check,
+	.atomic_update = bochs_primary_plane_helper_atomic_update,
+};
+
+static const struct drm_plane_funcs bochs_primary_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= drm_plane_cleanup,
+	.reset			= drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+};
+
+static void bochs_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
+{
+	struct bochs_device *bochs = to_bochs_device(crtc->dev);
+	struct drm_crtc_state *crtc_state = crtc->state;
+
+	bochs_hw_setmode(bochs, &crtc_state->mode);
+}
+
+static int bochs_crtc_helper_atomic_check(struct drm_crtc *crtc,
+					  struct drm_atomic_state *state)
 {
-	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
+	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
-	bochs_hw_blank(bochs, true);
+	if (!crtc_state->enable)
+		return 0;
+
+	return drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+}
+
+static void bochs_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+					    struct drm_atomic_state *state)
+{
 }
 
-static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
-			      struct drm_plane_state *old_state)
+static void bochs_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+					     struct drm_atomic_state *crtc_state)
 {
-	struct bochs_device *bochs = to_bochs_device(pipe->crtc.dev);
+	struct bochs_device *bochs = to_bochs_device(crtc->dev);
 
-	bochs_plane_update(bochs, pipe->plane.state);
+	bochs_hw_blank(bochs, true);
 }
 
-static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
-	.enable	    = bochs_pipe_enable,
-	.disable    = bochs_pipe_disable,
-	.update	    = bochs_pipe_update,
-	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
-	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
+static const struct drm_crtc_helper_funcs bochs_crtc_helper_funcs = {
+	.mode_set_nofb = bochs_crtc_helper_mode_set_nofb,
+	.atomic_check = bochs_crtc_helper_atomic_check,
+	.atomic_enable = bochs_crtc_helper_atomic_enable,
+	.atomic_disable = bochs_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs bochs_crtc_funcs = {
+	.reset = drm_atomic_helper_crtc_reset,
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_encoder_funcs bochs_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
 };
 
 static int bochs_connector_helper_get_modes(struct drm_connector *connector)
@@ -475,11 +544,11 @@ static int bochs_connector_helper_get_modes(struct drm_connector *connector)
 	return count;
 }
 
-static const struct drm_connector_helper_funcs bochs_connector_connector_helper_funcs = {
+static const struct drm_connector_helper_funcs bochs_connector_helper_funcs = {
 	.get_modes = bochs_connector_helper_get_modes,
 };
 
-static const struct drm_connector_funcs bochs_connector_connector_funcs = {
+static const struct drm_connector_funcs bochs_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = drm_connector_cleanup,
 	.reset = drm_atomic_helper_connector_reset,
@@ -487,18 +556,7 @@ static const struct drm_connector_funcs bochs_connector_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static void bochs_connector_init(struct bochs_device *bochs)
-{
-	struct drm_device *dev = &bochs->dev;
-	struct drm_connector *connector = &bochs->connector;
-
-	drm_connector_init(dev, connector, &bochs_connector_connector_funcs,
-			   DRM_MODE_CONNECTOR_VIRTUAL);
-	drm_connector_attach_edid_property(connector);
-	drm_connector_helper_add(connector, &bochs_connector_connector_helper_funcs);
-}
-
-static const struct drm_mode_config_funcs bochs_mode_funcs = {
+static const struct drm_mode_config_funcs bochs_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
 	.mode_valid = drm_vram_helper_mode_valid,
 	.atomic_check = drm_atomic_helper_check,
@@ -508,6 +566,10 @@ static const struct drm_mode_config_funcs bochs_mode_funcs = {
 static int bochs_kms_init(struct bochs_device *bochs)
 {
 	struct drm_device *dev = &bochs->dev;
+	struct drm_plane *primary_plane;
+	struct drm_crtc *crtc;
+	struct drm_connector *connector;
+	struct drm_encoder *encoder;
 	int ret;
 
 	ret = drmm_mode_config_init(dev);
@@ -518,19 +580,44 @@ static int bochs_kms_init(struct bochs_device *bochs)
 	dev->mode_config.max_height = 8192;
 
 	dev->mode_config.preferred_depth = 24;
-	dev->mode_config.prefer_shadow = 0;
 	dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
 
-	dev->mode_config.funcs = &bochs_mode_funcs;
+	dev->mode_config.funcs = &bochs_mode_config_funcs;
 
-	bochs_connector_init(bochs);
-	drm_simple_display_pipe_init(dev,
-				     &bochs->pipe,
-				     &bochs_pipe_funcs,
-				     bochs_formats,
-				     ARRAY_SIZE(bochs_formats),
-				     NULL,
-				     &bochs->connector);
+	primary_plane = &bochs->primary_plane;
+	ret = drm_universal_plane_init(dev, primary_plane, 0,
+				       &bochs_primary_plane_funcs,
+				       bochs_primary_plane_formats,
+				       ARRAY_SIZE(bochs_primary_plane_formats),
+				       NULL,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ret;
+	drm_plane_helper_add(primary_plane, &bochs_primary_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(primary_plane);
+
+	crtc = &bochs->crtc;
+	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
+					&bochs_crtc_funcs, NULL);
+	if (ret)
+		return ret;
+	drm_crtc_helper_add(crtc, &bochs_crtc_helper_funcs);
+
+	encoder = &bochs->encoder;
+	ret = drm_encoder_init(dev, encoder, &bochs_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret)
+		return ret;
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+	connector = &bochs->connector;
+	ret = drm_connector_init(dev, connector, &bochs_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret)
+		return ret;
+	drm_connector_helper_add(connector, &bochs_connector_helper_funcs);
+	drm_connector_attach_edid_property(connector);
+	drm_connector_attach_encoder(connector, encoder);
 
 	drm_mode_config_reset(dev);
 
-- 
2.46.0

[PATCH 08/10] drm/bochs: Use GEM SHMEM helpers for memory management

[Thomas Zimmermann]

Replace GEM VRAM with GEM SHMEM in bochs. The new memory manager
stores buffer objects in system memory. Makes the driver's memory
management more reliably.

Most of the changes are hidden in external helpers that allocate
buffers. Replacing DRM_GEM_VRAM_DRIVER with DRM_GEM_SHMEM_DRIVER_OPS
swaps these. With GEM VRAM, the video memory was updated directly by
the DRM client. The biggest change within bochs is in atomic_update,
which now updates video memory via memcpy() from the BO in system
memory. Shadow-plane helpers maintaining the pointers to the buffer's
data, so bochs doesn't have to. The update is triggered by each page
flip's call to the framebuffer's dirty helper. The driver supports
damage clipping to minimize memcpy() overhead.

The advantage of GEM SHMEM is that it makes memory management
more reliable. Given DRM's double buffering during page flips, the
minimum amount of video memory is three times the maximum consumption
in some pathological cases. For example, if the maximum size of a GEM
buffer is 1920x1080-32 (i.e., 32-bit FullHD), the buffer size is
8 MiB. Display hardware has to provide at lease 24 MiB to reliably
page flip such configurations. This cannot always be guaranteed and
bochs already contains code to rule out <4 MiB configurations. With
GEM SHMEM, only 8 MiB of video memory are required for the given
example. Unsupported modes can be sorted out easily.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/Kconfig |  4 +-
 drivers/gpu/drm/tiny/bochs.c | 78 +++++++++++++++++-------------------
 2 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..8f206c6387ec 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -13,10 +13,8 @@ config DRM_ARCPGU
 config DRM_BOCHS
 	tristate "DRM Support for bochs dispi vga interface (qemu stdvga)"
 	depends on DRM && PCI && MMU
+	select DRM_GEM_SHMEM_HELPER
 	select DRM_KMS_HELPER
-	select DRM_VRAM_HELPER
-	select DRM_TTM
-	select DRM_TTM_HELPER
 	help
 	  This is a KMS driver for qemu's stdvga output. Choose this option
 	  for qemu.
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 76e29950a807..bde70a6075ec 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -6,13 +6,15 @@
 #include <drm/drm_aperture.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_edid.h>
-#include <drm/drm_fbdev_ttm.h>
+#include <drm/drm_fbdev_shmem.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_module.h>
 #include <drm/drm_plane_helper.h>
@@ -427,49 +429,49 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
 	return 0;
 }
 
-static void bochs_plane_update(struct bochs_device *bochs, struct drm_plane_state *state)
+static void bochs_primary_plane_helper_atomic_update(struct drm_plane *plane,
+						     struct drm_atomic_state *state)
 {
-	struct drm_gem_vram_object *gbo;
-	s64 gpu_addr;
-
-	if (!state->fb || !bochs->stride)
+	struct drm_device *dev = plane->dev;
+	struct bochs_device *bochs = to_bochs_device(dev);
+	struct drm_plane_state *plane_state = plane->state;
+	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_rect damage;
+
+	if (!fb || !bochs->stride)
 		return;
 
-	gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (WARN_ON_ONCE(gpu_addr < 0))
-		return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
+	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		struct iosys_map dst = IOSYS_MAP_INIT_VADDR_IOMEM(bochs->fb_map);
 
-	bochs_hw_setbase(bochs,
-			 state->crtc_x,
-			 state->crtc_y,
-			 state->fb->pitches[0],
-			 state->fb->offsets[0] + gpu_addr);
-	bochs_hw_setformat(bochs, state->fb->format);
-}
-
-static void bochs_primary_plane_helper_atomic_update(struct drm_plane *plane,
-						     struct drm_atomic_state *state)
-{
-	struct bochs_device *bochs = to_bochs_device(plane->dev);
-	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
+		iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, &damage));
+		drm_fb_memcpy(&dst, fb->pitches, shadow_plane_state->data, fb, &damage);
+	}
 
-	bochs_plane_update(bochs, plane_state);
+	/* Always scanout image at VRAM offset 0 */
+	bochs_hw_setbase(bochs,
+			 plane_state->crtc_x,
+			 plane_state->crtc_y,
+			 fb->pitches[0],
+			 0);
+	bochs_hw_setformat(bochs, fb->format);
 }
 
 static const struct drm_plane_helper_funcs bochs_primary_plane_helper_funcs = {
-	DRM_GEM_VRAM_PLANE_HELPER_FUNCS,
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
 	.atomic_check = bochs_primary_plane_helper_atomic_check,
 	.atomic_update = bochs_primary_plane_helper_atomic_update,
 };
 
 static const struct drm_plane_funcs bochs_primary_plane_funcs = {
-	.update_plane		= drm_atomic_helper_update_plane,
-	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= drm_plane_cleanup,
-	.reset			= drm_atomic_helper_plane_reset,
-	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
-	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	DRM_GEM_SHADOW_PLANE_FUNCS
 };
 
 static void bochs_crtc_helper_mode_set_nofb(struct drm_crtc *crtc)
@@ -557,8 +559,7 @@ static const struct drm_connector_funcs bochs_connector_funcs = {
 };
 
 static const struct drm_mode_config_funcs bochs_mode_config_funcs = {
-	.fb_create = drm_gem_fb_create,
-	.mode_valid = drm_vram_helper_mode_valid,
+	.fb_create = drm_gem_fb_create_with_dirty,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -629,17 +630,12 @@ static int bochs_kms_init(struct bochs_device *bochs)
 
 static int bochs_load(struct bochs_device *bochs)
 {
-	struct drm_device *dev = &bochs->dev;
 	int ret;
 
 	ret = bochs_hw_init(bochs);
 	if (ret)
 		return ret;
 
-	ret = drmm_vram_helper_init(dev, bochs->fb_base, bochs->fb_size);
-	if (ret)
-		return ret;
-
 	ret = bochs_kms_init(bochs);
 	if (ret)
 		return ret;
@@ -657,7 +653,7 @@ static const struct drm_driver bochs_driver = {
 	.date			= "20130925",
 	.major			= 1,
 	.minor			= 0,
-	DRM_GEM_VRAM_DRIVER,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 };
 
 /* ---------------------------------------------------------------------- */
@@ -723,7 +719,7 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 	if (ret)
 		goto err_free_dev;
 
-	drm_fbdev_ttm_setup(dev, 32);
+	drm_fbdev_shmem_setup(dev, 32);
 	return ret;
 
 err_free_dev:
-- 
2.46.0

[PATCH 09/10] drm/bochs: Validate display modes against available video memory

[Thomas Zimmermann]

For each mode, test the required memory against the available video
memory. Filters out modes that do not fit into display memory.

Also remove the old test against the 4 MiB limit. It is now obsolete
and did not necessarily produce correct results.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/bochs.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index bde70a6075ec..b09c6c76923b 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -558,8 +558,28 @@ static const struct drm_connector_funcs bochs_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
+static enum drm_mode_status bochs_mode_config_mode_valid(struct drm_device *dev,
+							 const struct drm_display_mode *mode)
+{
+	struct bochs_device *bochs = to_bochs_device(dev);
+	const struct drm_format_info *format = drm_format_info(DRM_FORMAT_XRGB8888);
+	uint64_t pitch;
+
+	if (drm_WARN_ON(dev, !format))
+		return MODE_ERROR;
+
+	pitch = drm_format_info_min_pitch(format, 0, mode->vdisplay);
+	if (!pitch)
+		return MODE_BAD_WIDTH;
+	if (bochs->fb_size / pitch < mode->hdisplay)
+		return MODE_MEM;
+
+	return MODE_OK;
+}
+
 static const struct drm_mode_config_funcs bochs_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create_with_dirty,
+	.mode_valid = bochs_mode_config_mode_valid,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -687,15 +707,8 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent
 {
 	struct bochs_device *bochs;
 	struct drm_device *dev;
-	unsigned long fbsize;
 	int ret;
 
-	fbsize = pci_resource_len(pdev, 0);
-	if (fbsize < 4 * 1024 * 1024) {
-		DRM_ERROR("less than 4 MB video memory, ignoring device\n");
-		return -ENOMEM;
-	}
-
 	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &bochs_driver);
 	if (ret)
 		return ret;
-- 
2.46.0

Re: [PATCH 09/10] drm/bochs: Validate display modes against available video memory

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.11-rc5 next-20240829]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-bochs-Remove-manual-format-test-from-fb_create/20240826-131701
base:   git://anongit.freedesktop.org/drm/drm drm-next
patch link:    https://lore.kernel.org/r/20240823124422.286989-10-tzimmermann%40suse.de
patch subject: [PATCH 09/10] drm/bochs: Validate display modes against available video memory
config: i386-randconfig-006-20240829 (https://download.01.org/0day-ci/archive/20240830/202408300420.qcGOiIM6-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240830/202408300420.qcGOiIM6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408300420.qcGOiIM6-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/gpu/drm/tiny/bochs.o: in function `bochs_mode_config_mode_valid':
>> drivers/gpu/drm/tiny/bochs.c:574: undefined reference to `__udivdi3'


vim +574 drivers/gpu/drm/tiny/bochs.c

   560	
   561	static enum drm_mode_status bochs_mode_config_mode_valid(struct drm_device *dev,
   562								 const struct drm_display_mode *mode)
   563	{
   564		struct bochs_device *bochs = to_bochs_device(dev);
   565		const struct drm_format_info *format = drm_format_info(DRM_FORMAT_XRGB8888);
   566		uint64_t pitch;
   567	
   568		if (drm_WARN_ON(dev, !format))
   569			return MODE_ERROR;
   570	
   571		pitch = drm_format_info_min_pitch(format, 0, mode->vdisplay);
   572		if (!pitch)
   573			return MODE_BAD_WIDTH;
 > 574		if (bochs->fb_size / pitch < mode->hdisplay)
   575			return MODE_MEM;
   576	
   577		return MODE_OK;
   578	}
   579	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 10/10] drm/gem-vram: Remove support for simple display pipelines

[Thomas Zimmermann]

There are no more drivers that use GEM VRAM helpers with a simple
display pipeline. Remove the respective code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 45 ---------------------------
 include/drm/drm_gem_vram_helper.h     | 13 --------
 2 files changed, 58 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 6027584406af..22b1fe9c03b8 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -16,7 +16,6 @@
 #include <drm/drm_mode.h>
 #include <drm/drm_plane.h>
 #include <drm/drm_prime.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include <drm/ttm/ttm_range_manager.h>
 #include <drm/ttm/ttm_tt.h>
@@ -686,50 +685,6 @@ drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
 }
 EXPORT_SYMBOL(drm_gem_vram_plane_helper_cleanup_fb);
 
-/*
- * Helpers for struct drm_simple_display_pipe_funcs
- */
-
-/**
- * drm_gem_vram_simple_display_pipe_prepare_fb() - Implements &struct
- *				   drm_simple_display_pipe_funcs.prepare_fb
- * @pipe:	a simple display pipe
- * @new_state:	the plane's new state
- *
- * During plane updates, this function pins the GEM VRAM
- * objects of the plane's new framebuffer to VRAM. Call
- * drm_gem_vram_simple_display_pipe_cleanup_fb() to unpin them.
- *
- * Returns:
- *	0 on success, or
- *	a negative errno code otherwise.
- */
-int drm_gem_vram_simple_display_pipe_prepare_fb(
-	struct drm_simple_display_pipe *pipe,
-	struct drm_plane_state *new_state)
-{
-	return drm_gem_vram_plane_helper_prepare_fb(&pipe->plane, new_state);
-}
-EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_prepare_fb);
-
-/**
- * drm_gem_vram_simple_display_pipe_cleanup_fb() - Implements &struct
- *						   drm_simple_display_pipe_funcs.cleanup_fb
- * @pipe:	a simple display pipe
- * @old_state:	the plane's old state
- *
- * During plane updates, this function unpins the GEM VRAM
- * objects of the plane's old framebuffer from VRAM. Complements
- * drm_gem_vram_simple_display_pipe_prepare_fb().
- */
-void drm_gem_vram_simple_display_pipe_cleanup_fb(
-	struct drm_simple_display_pipe *pipe,
-	struct drm_plane_state *old_state)
-{
-	drm_gem_vram_plane_helper_cleanup_fb(&pipe->plane, old_state);
-}
-EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_cleanup_fb);
-
 /*
  * PRIME helpers
  */
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 9a73f786f4ad..00830b49a3ff 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -17,7 +17,6 @@
 struct drm_mode_create_dumb;
 struct drm_plane;
 struct drm_plane_state;
-struct drm_simple_display_pipe;
 struct filp;
 struct vm_area_struct;
 
@@ -137,18 +136,6 @@ drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
 	.prepare_fb = drm_gem_vram_plane_helper_prepare_fb, \
 	.cleanup_fb = drm_gem_vram_plane_helper_cleanup_fb
 
-/*
- * Helpers for struct drm_simple_display_pipe_funcs
- */
-
-int drm_gem_vram_simple_display_pipe_prepare_fb(
-	struct drm_simple_display_pipe *pipe,
-	struct drm_plane_state *new_state);
-
-void drm_gem_vram_simple_display_pipe_cleanup_fb(
-	struct drm_simple_display_pipe *pipe,
-	struct drm_plane_state *old_state);
-
 /**
  * define DRM_GEM_VRAM_DRIVER - default callback functions for
  *				&struct drm_driver
-- 
2.46.0

Re: [PATCH 00/10] rm/bochs: Modernize driver

[Gerd Hoffmann]

  Hi,

> Patch 8 replaces GEM VRAM with GEM SHMEM. The new memory manager
> is more reliable and allows for larger resolutions.

Valid point.

> Display updates were so slow that Gnome was unmanageable with a
> flickering cursor and single FPS. The new memory management makes
> Gnome at least useable.

Hmm?  I'm wondering where this huge improvement comes from?

With enough video memory VRAM performance should be ok.

If video memory is tight and ttm is forced to shuffle around
framebuffers between vram and system memory on each page flip (touching
much of vram along the way which causes additional overhead on the qemu
side), that is obviously very bad for performance.  One of the reasons
why cirrus uses SHMEM + shadowing since years.

Shadow buffering comes with some overhead too, so the switch isn't an
obvious win (assuming enough vram).  Hiding the page flips from qemu
might reduce the work qemu has to do though, especially if the shadowing
uses dirty tracking and only touches the vram pages which have actually
changed content.  So there is a fair chance that this outweighs the
shadowing overhead and ends up being a net win.  I don't expect the
difference being very big though.  Also different display usage patterns
might yield different results (fbcon vs. gnome for example).

So this probably makes sense, but I'd like to see a bit more background
information ...

On vram sizes:  The default qemu vram size (16M) should be fine for the
default display resolution (1280x800).  For FullHD vram size should be
doubled (-device VGA,vgamem_mb=32).


Skimmed the other patches, looks sane overall, but I don't follow drm
close enough any more to do an full review.  So I leave this here:

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

Re: [PATCH 00/10] rm/bochs: Modernize driver

[Thomas Zimmermann]

Hi

Am 23.08.24 um 16:34 schrieb Gerd Hoffmann:
>    Hi,
>
>> Patch 8 replaces GEM VRAM with GEM SHMEM. The new memory manager
>> is more reliable and allows for larger resolutions.
> Valid point.
>
>> Display updates were so slow that Gnome was unmanageable with a
>> flickering cursor and single FPS. The new memory management makes
>> Gnome at least useable.
> Hmm?  I'm wondering where this huge improvement comes from?
>
> With enough video memory VRAM performance should be ok.
>
> If video memory is tight and ttm is forced to shuffle around
> framebuffers between vram and system memory on each page flip (touching
> much of vram along the way which causes additional overhead on the qemu
> side), that is obviously very bad for performance.  One of the reasons
> why cirrus uses SHMEM + shadowing since years.
>
> Shadow buffering comes with some overhead too, so the switch isn't an
> obvious win (assuming enough vram).  Hiding the page flips from qemu
> might reduce the work qemu has to do though, especially if the shadowing
> uses dirty tracking and only touches the vram pages which have actually
> changed content.  So there is a fair chance that this outweighs the
> shadowing overhead and ends up being a net win.  I don't expect the
> difference being very big though.  Also different display usage patterns
> might yield different results (fbcon vs. gnome for example).
>
> So this probably makes sense, but I'd like to see a bit more background
> information ...

The difference is in damage handling.

The old code had two BOs in video memory and flipped between them. IDK 
the details of the old rendering, but from the massive flickering of the 
cursor, I assume that X11's internal either copies a full buffer during 
each redraw, or doesn't really handle damage well. It could also happen 
that X didn't use a shadow buffer for rendering. Bochs didn't request 
one. Without, drawing to I/O memory is really slow. If that applies to 
virtual I/O memory as well IDK.

The new driver code only copies areas that have been changed from 
rendering. The flickering is gone and the overall update performance is 
acceptable.

>
> On vram sizes:  The default qemu vram size (16M) should be fine for the
> default display resolution (1280x800).  For FullHD vram size should be
> doubled (-device VGA,vgamem_mb=32).

Right. Bochs never really tested that. So I saw something like 5k by 3k 
resolutions on my test setup with 16 MiB. Now that video-memory 
requirements for each mode can be calculated easily, we can sort out the 
invalid modes.

>
>
> Skimmed the other patches, looks sane overall, but I don't follow drm
> close enough any more to do an full review.  So I leave this here:
>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>

Thank you so much.

Best regards
Thomas

>
> take care,
>    Gerd
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 00/10] rm/bochs: Modernize driver

[Gerd Hoffmann]

  Hi,

> > So this probably makes sense, but I'd like to see a bit more background
> > information ...
> 
> The difference is in damage handling.
> 
> The old code had two BOs in video memory and flipped between them. IDK the
> details of the old rendering, but from the massive flickering of the cursor,
> I assume that X11's internal either copies a full buffer during each redraw,
> or doesn't really handle damage well. It could also happen that X didn't use
> a shadow buffer for rendering. Bochs didn't request one. Without, drawing to
> I/O memory is really slow. If that applies to virtual I/O memory as well
> IDK.
> 
> The new driver code only copies areas that have been changed from rendering.
> The flickering is gone and the overall update performance is acceptable.

Thanks.

Have you tried wayland and fbcon too?

> > On vram sizes:  The default qemu vram size (16M) should be fine for the
> > default display resolution (1280x800).  For FullHD vram size should be
> > doubled (-device VGA,vgamem_mb=32).
> 
> Right. Bochs never really tested that.  So I saw something like 5k by
> 3k resolutions on my test setup with 16 MiB.

IIRC there used to be a check in the past, limiting resolutions to
buffer sizes which fit into vram twice (to allow for double buffering).

Crude heuristic.  I do not remember when and why it went away.  Also
I've seen wayland use three not two buffers ...

> Now that video-memory requirements for each mode can be calculated
> easily, we can sort out the invalid modes.

Yes.  Also I think trading higher main memory (shmem) usage for lower
vram usage is good overall.  Main memory can be uses for something else
if not needed whereas vram sits around unused.

take care,
  Gerd

Re: [PATCH 00/10] rm/bochs: Modernize driver

[Thomas Zimmermann]

Hi

Am 26.08.24 um 09:32 schrieb Gerd Hoffmann:
>    Hi,
>
>>> So this probably makes sense, but I'd like to see a bit more background
>>> information ...
>> The difference is in damage handling.
>>
>> The old code had two BOs in video memory and flipped between them. IDK the
>> details of the old rendering, but from the massive flickering of the cursor,
>> I assume that X11's internal either copies a full buffer during each redraw,
>> or doesn't really handle damage well. It could also happen that X didn't use
>> a shadow buffer for rendering. Bochs didn't request one. Without, drawing to
>> I/O memory is really slow. If that applies to virtual I/O memory as well
>> IDK.
>>
>> The new driver code only copies areas that have been changed from rendering.
>> The flickering is gone and the overall update performance is acceptable.
> Thanks.
>
> Have you tried wayland and fbcon too?

Fbcon is ok for text at least. I've yet to try wayland.

>
>>> On vram sizes:  The default qemu vram size (16M) should be fine for the
>>> default display resolution (1280x800).  For FullHD vram size should be
>>> doubled (-device VGA,vgamem_mb=32).
>> Right. Bochs never really tested that.  So I saw something like 5k by
>> 3k resolutions on my test setup with 16 MiB.
> IIRC there used to be a check in the past, limiting resolutions to
> buffer sizes which fit into vram twice (to allow for double buffering).
>
> Crude heuristic.  I do not remember when and why it went away.  Also
> I've seen wayland use three not two buffers ...

IDK where that test went.

The problem with TTM pinning/unpinning and eviction is that it requires 
3 times the maximum consumption of physical video memory, because there 
is fragmentation in some corner cases. So the 16 MiB default size seem 
really low.

Best regards
Thomas

>
>> Now that video-memory requirements for each mode can be calculated
>> easily, we can sort out the invalid modes.
> Yes.  Also I think trading higher main memory (shmem) usage for lower
> vram usage is good overall.  Main memory can be uses for something else
> if not needed whereas vram sits around unused.
>
> take care,
>    Gerd
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)