[PATCH 00/17] cirrus: Modernize the cirrus driver

[PATCH 00/17] cirrus: Modernize the cirrus driver

[Thomas Zimmermann]

Update the cirrus driver to follow current best practices. While the
driver's hardware is obsolete, the cirrus driver is still one of the
go-to modules to learn about writing a DRM driver. So keep it in good
shape.

Patches 1 to 3 simplify blitting and convert it to the DRM's current
helpers.

Patches 4 to 8 replace simple-KMS helpers with DRM's regular atomic
helpers. The former are midlayers on top of the latter, and should
be replaced entirely.

Patches 9 and 10 further improve blitting. This enables damage clipping
for userspace and the console. Until now, cirrus' mandatory fullscreen
updates have added unnecessary overhead to every screen update.

Patches 11 to 14 simplify mode and framebuffer tests. With the use
of regular helpers, these tests can now be implemented in the places
they belong.

Patches 15 and 16 move hardware color format and pitch into plane
state of the primary plane. These fields have been kept in the device
structure itself, where thy don't belong.

Patch 17 replaces two magic values by macro constants. There are
more such cases within cirrus, but those two values stuck out as
specifically hard to interpret.

Tested with qemu's cirrus emulation.

Thomas Zimmermann (17):
  drm/cirrus: Compute blit destination offset in single location
  drm/cirrus: Replace cpp value with format
  drm/cirrus: Use drm_fb_blit() to update scanout buffer
  drm/cirrus: Move drm_dev_{enter,exit}() into DRM helpers
  drm/cirrus: Split cirrus_mode_set() into smaller functions
  drm/cirrus: Integrate connector into pipeline code
  drm/cirrus: Move primary-plane format arrays
  drm/cirrus: Convert to regular atomic helpers
  drm/cirrus: Enable damage clipping on primary plane
  drm/cirrus: Inline cirrus_fb_blit_rect()
  drm/cirrus: Remove format test from cirrus_fb_create()
  drm/cirrus: Remove size test from cirrus_fb_create()
  drm/cirrus: Test mode against video-memory size in device-wide
    mode_valid
  drm/cirrus: Inline cirrus_check_size() into primary-plane atomic_check
  drm/cirrus: Introduce struct cirrus_primary_plane_state
  drm/cirrus: Store HW format/pitch in primary-plane state
  drm/cirrus: Use VGA macro constants to unblank

 drivers/gpu/drm/tiny/cirrus.c | 499 +++++++++++++++++++++-------------
 1 file changed, 305 insertions(+), 194 deletions(-)

-- 
2.39.1

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

[PATCH 01/17] drm/cirrus: Compute blit destination offset in single location

[Thomas Zimmermann]

The calculation for the scanout-buffer blit offset is independent
from the color format. In the one case where the current code uses
fb->pitches[0] instead of cirrus->pitch, their values are identical.
Hence merge all into a single line.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index cf35b6090503..7fb21db8416d 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -327,17 +327,15 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 		return -ENODEV;
 
 	iosys_map_set_vaddr_iomem(&dst, cirrus->vram);
+	iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 
 	if (cirrus->cpp == fb->format->cpp[0]) {
-		iosys_map_incr(&dst, drm_fb_clip_offset(fb->pitches[0], fb->format, rect));
 		drm_fb_memcpy(&dst, fb->pitches, vmap, fb, rect);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
-		iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 		drm_fb_xrgb8888_to_rgb565(&dst, &cirrus->pitch, vmap, fb, rect, false);
 
 	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
-		iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 		drm_fb_xrgb8888_to_rgb888(&dst, &cirrus->pitch, vmap, fb, rect);
 
 	} else {
-- 
2.39.1

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

[PATCH 02/17] drm/cirrus: Replace cpp value with format

[Thomas Zimmermann]

Using components per pixel to describe a color format is obsolete.
Use the format info and 4CC value instead.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 7fb21db8416d..67e83fa42a32 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -58,7 +58,7 @@ struct cirrus_device {
 	struct drm_device	       dev;
 	struct drm_simple_display_pipe pipe;
 	struct drm_connector	       conn;
-	unsigned int		       cpp;
+	const struct drm_format_info   *format;
 	unsigned int		       pitch;
 	void __iomem		       *vram;
 	void __iomem		       *mmio;
@@ -126,34 +126,34 @@ static void wreg_hdr(struct cirrus_device *cirrus, u8 val)
 	iowrite8(val, cirrus->mmio + VGA_DAC_MASK);
 }
 
-static int cirrus_convert_to(struct drm_framebuffer *fb)
+static const struct drm_format_info *cirrus_convert_to(struct drm_framebuffer *fb)
 {
-	if (fb->format->cpp[0] == 4 && fb->pitches[0] > CIRRUS_MAX_PITCH) {
+	if (fb->format->format == DRM_FORMAT_XRGB8888 && fb->pitches[0] > CIRRUS_MAX_PITCH) {
 		if (fb->width * 3 <= CIRRUS_MAX_PITCH)
 			/* convert from XR24 to RG24 */
-			return 3;
+			return drm_format_info(DRM_FORMAT_RGB888);
 		else
 			/* convert from XR24 to RG16 */
-			return 2;
+			return drm_format_info(DRM_FORMAT_RGB565);
 	}
-	return 0;
+	return NULL;
 }
 
-static int cirrus_cpp(struct drm_framebuffer *fb)
+static const struct drm_format_info *cirrus_format(struct drm_framebuffer *fb)
 {
-	int convert_cpp = cirrus_convert_to(fb);
+	const struct drm_format_info *format = cirrus_convert_to(fb);
 
-	if (convert_cpp)
-		return convert_cpp;
-	return fb->format->cpp[0];
+	if (format)
+		return format;
+	return fb->format;
 }
 
 static int cirrus_pitch(struct drm_framebuffer *fb)
 {
-	int convert_cpp = cirrus_convert_to(fb);
+	const struct drm_format_info *format = cirrus_convert_to(fb);
 
-	if (convert_cpp)
-		return convert_cpp * fb->width;
+	if (format)
+		return drm_format_info_min_pitch(format, 0, fb->width);
 	return fb->pitches[0];
 }
 
@@ -263,20 +263,20 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 	sr07 &= 0xe0;
 	hdr = 0;
 
-	cirrus->cpp = cirrus_cpp(fb);
-	switch (cirrus->cpp * 8) {
-	case 8:
+	cirrus->format = cirrus_format(fb);
+	switch (cirrus->format->format) {
+	case DRM_FORMAT_C8:
 		sr07 |= 0x11;
 		break;
-	case 16:
+	case DRM_FORMAT_RGB565:
 		sr07 |= 0x17;
 		hdr = 0xc1;
 		break;
-	case 24:
+	case DRM_FORMAT_RGB888:
 		sr07 |= 0x15;
 		hdr = 0xc5;
 		break;
-	case 32:
+	case DRM_FORMAT_XRGB8888:
 		sr07 |= 0x19;
 		hdr = 0xc5;
 		break;
@@ -329,13 +329,15 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 	iosys_map_set_vaddr_iomem(&dst, cirrus->vram);
 	iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 
-	if (cirrus->cpp == fb->format->cpp[0]) {
+	if (cirrus->format == fb->format) {
 		drm_fb_memcpy(&dst, fb->pitches, vmap, fb, rect);
 
-	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2) {
+	} else if (fb->format->format == DRM_FORMAT_XRGB8888 &&
+		   cirrus->format->format == DRM_FORMAT_RGB565) {
 		drm_fb_xrgb8888_to_rgb565(&dst, &cirrus->pitch, vmap, fb, rect, false);
 
-	} else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3) {
+	} else if (fb->format->format == DRM_FORMAT_XRGB8888 &&
+		   cirrus->format->format == DRM_FORMAT_RGB565) {
 		drm_fb_xrgb8888_to_rgb888(&dst, &cirrus->pitch, vmap, fb, rect);
 
 	} else {
@@ -450,7 +452,7 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 
-	if (state->fb && cirrus->cpp != cirrus_cpp(state->fb))
+	if (state->fb && cirrus->format != cirrus_format(state->fb))
 		cirrus_mode_set(cirrus, &crtc->mode, state->fb);
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-- 
2.39.1

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

[PATCH 03/17] drm/cirrus: Use drm_fb_blit() to update scanout buffer

[Thomas Zimmermann]

Cirrus' blit helper reimplements code from the shared blit helper
drm_fb_blit(). Use the helper instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/cirrus.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 67e83fa42a32..71fa07535298 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -329,20 +329,7 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 	iosys_map_set_vaddr_iomem(&dst, cirrus->vram);
 	iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 
-	if (cirrus->format == fb->format) {
-		drm_fb_memcpy(&dst, fb->pitches, vmap, fb, rect);
-
-	} else if (fb->format->format == DRM_FORMAT_XRGB8888 &&
-		   cirrus->format->format == DRM_FORMAT_RGB565) {
-		drm_fb_xrgb8888_to_rgb565(&dst, &cirrus->pitch, vmap, fb, rect, false);
-
-	} else if (fb->format->format == DRM_FORMAT_XRGB8888 &&
-		   cirrus->format->format == DRM_FORMAT_RGB565) {
-		drm_fb_xrgb8888_to_rgb888(&dst, &cirrus->pitch, vmap, fb, rect);
-
-	} else {
-		WARN_ON_ONCE("cpp mismatch");
-	}
+	drm_fb_blit(&dst, &cirrus->pitch, cirrus->format->format, vmap, fb, rect);
 
 	drm_dev_exit(idx);
 
-- 
2.39.1

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

[PATCH 04/17] drm/cirrus: Move drm_dev_{enter, exit}() into DRM helpers

[Thomas Zimmermann]

Call drm_dev_enter() and drm_dev_exit() immediately after entering
cirrus' DRM helper functions. Remove these calls from other functions.
Each enter/exit block in the DRM helpers covers the full hardware
update. No functional changes.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 71fa07535298..0b02244bd9f1 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -159,13 +159,9 @@ static int cirrus_pitch(struct drm_framebuffer *fb)
 
 static void cirrus_set_start_address(struct cirrus_device *cirrus, u32 offset)
 {
-	int idx;
 	u32 addr;
 	u8 tmp;
 
-	if (!drm_dev_enter(&cirrus->dev, &idx))
-		return;
-
 	addr = offset >> 2;
 	wreg_crt(cirrus, 0x0c, (u8)((addr >> 8) & 0xff));
 	wreg_crt(cirrus, 0x0d, (u8)(addr & 0xff));
@@ -180,8 +176,6 @@ static void cirrus_set_start_address(struct cirrus_device *cirrus, u32 offset)
 	tmp &= 0x7f;
 	tmp |= (addr >> 12) & 0x80;
 	wreg_crt(cirrus, 0x1d, tmp);
-
-	drm_dev_exit(idx);
 }
 
 static int cirrus_mode_set(struct cirrus_device *cirrus,
@@ -190,12 +184,9 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 {
 	int hsyncstart, hsyncend, htotal, hdispend;
 	int vtotal, vdispend;
-	int tmp, idx;
+	int tmp;
 	int sr07 = 0, hdr = 0;
 
-	if (!drm_dev_enter(&cirrus->dev, &idx))
-		return -1;
-
 	htotal = mode->htotal / 8;
 	hsyncend = mode->hsync_end / 8;
 	hsyncstart = mode->hsync_start / 8;
@@ -281,7 +272,6 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 		hdr = 0xc5;
 		break;
 	default:
-		drm_dev_exit(idx);
 		return -1;
 	}
 
@@ -311,7 +301,6 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
 	outb(0x20, 0x3c0);
 
-	drm_dev_exit(idx);
 	return 0;
 }
 
@@ -321,18 +310,12 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 {
 	struct cirrus_device *cirrus = to_cirrus(fb->dev);
 	struct iosys_map dst;
-	int idx;
-
-	if (!drm_dev_enter(&cirrus->dev, &idx))
-		return -ENODEV;
 
 	iosys_map_set_vaddr_iomem(&dst, cirrus->vram);
 	iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
 
 	drm_fb_blit(&dst, &cirrus->pitch, cirrus->format->format, vmap, fb, rect);
 
-	drm_dev_exit(idx);
-
 	return 0;
 }
 
@@ -425,9 +408,15 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe,
 {
 	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	int idx;
+
+	if (!drm_dev_enter(&cirrus->dev, &idx))
+		return;
 
 	cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
 	cirrus_fb_blit_fullscreen(plane_state->fb, &shadow_plane_state->data[0]);
+
+	drm_dev_exit(idx);
 }
 
 static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -438,12 +427,18 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
+	int idx;
+
+	if (!drm_dev_enter(&cirrus->dev, &idx))
+		return;
 
 	if (state->fb && cirrus->format != cirrus_format(state->fb))
 		cirrus_mode_set(cirrus, &crtc->mode, state->fb);
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		cirrus_fb_blit_rect(state->fb, &shadow_plane_state->data[0], &rect);
+
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
-- 
2.39.1

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

[PATCH 05/17] drm/cirrus: Split cirrus_mode_set() into smaller functions

[Thomas Zimmermann]

Split cirrus_mode_set() into smaller functions that set the display
mode, color format and scnaline pitch individually. Better reflects
the design of the DRM modesetting pipeline.

Done in preparation of converting cirrus to regular atomic helpers.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 0b02244bd9f1..60488e49bdb5 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -178,14 +178,12 @@ static void cirrus_set_start_address(struct cirrus_device *cirrus, u32 offset)
 	wreg_crt(cirrus, 0x1d, tmp);
 }
 
-static int cirrus_mode_set(struct cirrus_device *cirrus,
-			   struct drm_display_mode *mode,
-			   struct drm_framebuffer *fb)
+static void cirrus_mode_set(struct cirrus_device *cirrus,
+			    struct drm_display_mode *mode)
 {
 	int hsyncstart, hsyncend, htotal, hdispend;
 	int vtotal, vdispend;
 	int tmp;
-	int sr07 = 0, hdr = 0;
 
 	htotal = mode->htotal / 8;
 	hsyncend = mode->hsync_end / 8;
@@ -249,15 +247,21 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 
 	/* Disable Hercules/CGA compatibility */
 	wreg_crt(cirrus, VGA_CRTC_MODE, 0x03);
+}
+
+static void cirrus_format_set(struct cirrus_device *cirrus,
+			      struct drm_framebuffer *fb)
+{
+	u8 sr07, hdr;
 
 	sr07 = rreg_seq(cirrus, 0x07);
 	sr07 &= 0xe0;
-	hdr = 0;
 
 	cirrus->format = cirrus_format(fb);
 	switch (cirrus->format->format) {
 	case DRM_FORMAT_C8:
 		sr07 |= 0x11;
+		hdr = 0x00;
 		break;
 	case DRM_FORMAT_RGB565:
 		sr07 |= 0x17;
@@ -272,22 +276,11 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 		hdr = 0xc5;
 		break;
 	default:
-		return -1;
+		return;
 	}
 
 	wreg_seq(cirrus, 0x7, sr07);
 
-	/* Program the pitch */
-	cirrus->pitch = cirrus_pitch(fb);
-	tmp = cirrus->pitch / 8;
-	wreg_crt(cirrus, 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(cirrus, 0x1b, tmp);
-
 	/* Enable high-colour modes */
 	wreg_gfx(cirrus, VGA_GFX_MODE, 0x40);
 
@@ -295,13 +288,25 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
 	wreg_gfx(cirrus, VGA_GFX_MISC, 0x01);
 
 	wreg_hdr(cirrus, hdr);
+}
 
-	cirrus_set_start_address(cirrus, 0);
+static void cirrus_pitch_set(struct cirrus_device *cirrus,
+			     struct drm_framebuffer *fb)
+{
+	u8 cr13, cr1b;
 
-	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
-	outb(0x20, 0x3c0);
+	/* Program the pitch */
+	cirrus->pitch = cirrus_pitch(fb);
+	cr13 = cirrus->pitch / 8;
+	wreg_crt(cirrus, VGA_CRTC_OFFSET, cr13);
 
-	return 0;
+	/* Enable extended blanking and pitch bits, and enable full memory */
+	cr1b = 0x22;
+	cr1b |= (cirrus->pitch >> 7) & 0x10;
+	cr1b |= (cirrus->pitch >> 6) & 0x40;
+	wreg_crt(cirrus, 0x1b, cr1b);
+
+	cirrus_set_start_address(cirrus, 0);
 }
 
 static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
@@ -413,9 +418,14 @@ static void cirrus_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return;
 
-	cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
+	cirrus_mode_set(cirrus, &crtc_state->mode);
+	cirrus_format_set(cirrus, plane_state->fb);
+	cirrus_pitch_set(cirrus, plane_state->fb);
 	cirrus_fb_blit_fullscreen(plane_state->fb, &shadow_plane_state->data[0]);
 
+	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
+	outb(0x20, 0x3c0);
+
 	drm_dev_exit(idx);
 }
 
@@ -425,15 +435,18 @@ static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
 	struct drm_plane_state *state = pipe->plane.state;
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
-	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_rect rect;
 	int idx;
 
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return;
 
-	if (state->fb && cirrus->format != cirrus_format(state->fb))
-		cirrus_mode_set(cirrus, &crtc->mode, state->fb);
+	if (state->fb) {
+		if (cirrus->format != cirrus_format(state->fb))
+			cirrus_format_set(cirrus, state->fb);
+		if (cirrus->pitch != cirrus_pitch(state->fb))
+			cirrus_pitch_set(cirrus, state->fb);
+	}
 
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		cirrus_fb_blit_rect(state->fb, &shadow_plane_state->data[0], &rect);
-- 
2.39.1

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

[PATCH 06/17] drm/cirrus: Integrate connector into pipeline code

[Thomas Zimmermann]

Integrate the connector with the rest of the pipeline setup code.
Move some helpers within the file and adapt naming slightly. No
functional changes.

Done in preparation of converting cirrus to regular atomic helpers.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 60488e49bdb5..cc1d45ea1f62 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -57,7 +57,7 @@
 struct cirrus_device {
 	struct drm_device	       dev;
 	struct drm_simple_display_pipe pipe;
-	struct drm_connector	       conn;
+	struct drm_connector	       connector;
 	const struct drm_format_info   *format;
 	unsigned int		       pitch;
 	void __iomem		       *vram;
@@ -352,41 +352,7 @@ static int cirrus_check_size(int width, int height,
 }
 
 /* ------------------------------------------------------------------ */
-/* 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					      */
+/* cirrus display pipe						      */
 
 static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
 						   const struct drm_display_mode *mode)
@@ -473,15 +439,49 @@ static const uint64_t cirrus_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
+static int cirrus_connector_helper_get_modes(struct drm_connector *connector)
+{
+	int count;
+
+	count = drm_add_modes_noedid(connector,
+				     connector->dev->mode_config.max_width,
+				     connector->dev->mode_config.max_height);
+	drm_set_preferred_mode(connector, 1024, 768);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs cirrus_connector_helper_funcs = {
+	.get_modes = cirrus_connector_helper_get_modes,
+};
+
+static const struct drm_connector_funcs cirrus_connector_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_pipe_init(struct cirrus_device *cirrus)
 {
-	return drm_simple_display_pipe_init(&cirrus->dev,
+	struct drm_device *dev = &cirrus->dev;
+	struct drm_connector *connector;
+	int ret;
+
+	connector = &cirrus->connector;
+	ret = drm_connector_init(&cirrus->dev, connector, &cirrus_connector_funcs,
+				 DRM_MODE_CONNECTOR_VGA);
+	if (ret)
+		return ret;
+	drm_connector_helper_add(connector, &cirrus_connector_helper_funcs);
+
+	return drm_simple_display_pipe_init(dev,
 					    &cirrus->pipe,
 					    &cirrus_pipe_funcs,
 					    cirrus_formats,
 					    ARRAY_SIZE(cirrus_formats),
 					    cirrus_modifiers,
-					    &cirrus->conn);
+					    connector);
 }
 
 /* ------------------------------------------------------------------ */
@@ -584,10 +584,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	ret = cirrus_conn_init(cirrus);
-	if (ret < 0)
-		return ret;
-
 	ret = cirrus_pipe_init(cirrus);
 	if (ret < 0)
 		return ret;
-- 
2.39.1

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

[PATCH 07/17] drm/cirrus: Move primary-plane format arrays

[Thomas Zimmermann]

Move the primary plane's format and modifier arrays within the
source file and adapt naming slightly. No functional changes.

Done in preparation of converting cirrus to regular atomic helpers.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index cc1d45ea1f62..7ca6a897a2b2 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -354,6 +354,17 @@ static int cirrus_check_size(int width, int height,
 /* ------------------------------------------------------------------ */
 /* cirrus display pipe						      */
 
+static const uint32_t cirrus_primary_plane_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
+	DRM_FORMAT_XRGB8888,
+};
+
+static const uint64_t cirrus_primary_plane_format_modifiers[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
 static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
 						   const struct drm_display_mode *mode)
 {
@@ -428,17 +439,6 @@ static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
 	DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
 };
 
-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_connector_helper_get_modes(struct drm_connector *connector)
 {
 	int count;
@@ -478,9 +478,9 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
 	return drm_simple_display_pipe_init(dev,
 					    &cirrus->pipe,
 					    &cirrus_pipe_funcs,
-					    cirrus_formats,
-					    ARRAY_SIZE(cirrus_formats),
-					    cirrus_modifiers,
+					    cirrus_primary_plane_formats,
+					    ARRAY_SIZE(cirrus_primary_plane_formats),
+					    cirrus_primary_plane_format_modifiers,
 					    connector);
 }
 
-- 
2.39.1

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

[PATCH 08/17] drm/cirrus: Convert to regular atomic helpers

[Thomas Zimmermann]

Replace simple-KMS helpers with DRM's regular helpers for atomic
modesetting. Avoids the mid-layer and the additional wrappers around
GEM's shadow-plane helpers.

Most of the simple-KMS code is just wrappers around regular atomic
helpers. The conversion is therefore equivalent to pulling the
simple-KMS helpers into cirrus and removing all the intermediate
code and data structures between the driver and the atomic helpers.
As the simple-KMS helpers lump primary plan, CRTC and encoder into a
single data structure, the conversion to regular helpers allows to
split modesetting from plane updates and handle each individually.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 7ca6a897a2b2..af26de9ef329 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -24,6 +24,7 @@
 #include <video/vga.h>
 
 #include <drm/drm_aperture.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_connector.h>
@@ -43,7 +44,6 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_module.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #define DRIVER_NAME "cirrus"
 #define DRIVER_DESC "qemu cirrus vga"
@@ -56,10 +56,18 @@
 
 struct cirrus_device {
 	struct drm_device	       dev;
-	struct drm_simple_display_pipe pipe;
+
+	/* modesetting pipeline */
+	struct drm_plane	       primary_plane;
+	struct drm_crtc		       crtc;
+	struct drm_encoder	       encoder;
 	struct drm_connector	       connector;
+
+	/* HW scanout buffer */
 	const struct drm_format_info   *format;
 	unsigned int		       pitch;
+
+	/* HW resources */
 	void __iomem		       *vram;
 	void __iomem		       *mmio;
 };
@@ -324,18 +332,6 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
 	return 0;
 }
 
-static int cirrus_fb_blit_fullscreen(struct drm_framebuffer *fb,
-				     const struct iosys_map *map)
-{
-	struct drm_rect fullscreen = {
-		.x1 = 0,
-		.x2 = fb->width,
-		.y1 = 0,
-		.y2 = fb->height,
-	};
-	return cirrus_fb_blit_rect(fb, map, &fullscreen);
-}
-
 static int cirrus_check_size(int width, int height,
 			     struct drm_framebuffer *fb)
 {
@@ -365,78 +361,130 @@ static const uint64_t cirrus_primary_plane_format_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
-static enum drm_mode_status cirrus_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
-						   const struct drm_display_mode *mode)
+static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
+						    struct drm_atomic_state *state)
 {
-	if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
-		return MODE_BAD;
-	return MODE_OK;
-}
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_framebuffer *fb = new_plane_state->fb;
+	struct drm_crtc *new_crtc = new_plane_state->crtc;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	int ret;
 
-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 (new_crtc)
+		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
 
-	if (!fb)
+	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 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)
+static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
+						      struct drm_atomic_state *state)
 {
-	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
+	struct cirrus_device *cirrus = to_cirrus(plane->dev);
+	struct drm_plane_state *plane_state = drm_atomic_get_new_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_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_rect rect;
 	int idx;
 
+	if (!fb)
+		return;
+
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return;
 
-	cirrus_mode_set(cirrus, &crtc_state->mode);
-	cirrus_format_set(cirrus, plane_state->fb);
-	cirrus_pitch_set(cirrus, plane_state->fb);
-	cirrus_fb_blit_fullscreen(plane_state->fb, &shadow_plane_state->data[0]);
+	if (cirrus->format != cirrus_format(fb))
+		cirrus_format_set(cirrus, fb);
+	if (cirrus->pitch != cirrus_pitch(fb))
+		cirrus_pitch_set(cirrus, fb);
 
-	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
-	outb(0x20, 0x3c0);
+	if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
+		cirrus_fb_blit_rect(fb, &shadow_plane_state->data[0], &rect);
 
 	drm_dev_exit(idx);
 }
 
-static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
-			       struct drm_plane_state *old_state)
+static const struct drm_plane_helper_funcs cirrus_primary_plane_helper_funcs = {
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_check = cirrus_primary_plane_helper_atomic_check,
+	.atomic_update = cirrus_primary_plane_helper_atomic_update,
+};
+
+static const struct drm_plane_funcs cirrus_primary_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static enum drm_mode_status cirrus_crtc_helper_mode_valid(struct drm_crtc *crtc,
+							  const struct drm_display_mode *mode)
 {
-	struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
-	struct drm_plane_state *state = pipe->plane.state;
-	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
-	struct drm_rect rect;
+	if (cirrus_check_size(mode->hdisplay, mode->vdisplay, NULL) < 0)
+		return MODE_BAD;
+
+	return MODE_OK;
+}
+
+static int cirrus_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
+{
+	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	int ret;
+
+	if (!crtc_state->enable)
+		return 0;
+
+	ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void cirrus_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+					     struct drm_atomic_state *state)
+{
+	struct cirrus_device *cirrus = to_cirrus(crtc->dev);
+	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	int idx;
 
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return;
 
-	if (state->fb) {
-		if (cirrus->format != cirrus_format(state->fb))
-			cirrus_format_set(cirrus, state->fb);
-		if (cirrus->pitch != cirrus_pitch(state->fb))
-			cirrus_pitch_set(cirrus, state->fb);
-	}
+	cirrus_mode_set(cirrus, &crtc_state->mode);
 
-	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		cirrus_fb_blit_rect(state->fb, &shadow_plane_state->data[0], &rect);
+	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
+	outb(0x20, 0x3c0);
 
 	drm_dev_exit(idx);
 }
 
-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,
-	DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
+static const struct drm_crtc_helper_funcs cirrus_crtc_helper_funcs = {
+	.mode_valid = cirrus_crtc_helper_mode_valid,
+	.atomic_check = cirrus_crtc_helper_atomic_check,
+	.atomic_enable = cirrus_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs cirrus_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 cirrus_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
 };
 
 static int cirrus_connector_helper_get_modes(struct drm_connector *connector)
@@ -465,23 +513,49 @@ static const struct drm_connector_funcs cirrus_connector_funcs = {
 static int cirrus_pipe_init(struct cirrus_device *cirrus)
 {
 	struct drm_device *dev = &cirrus->dev;
+	struct drm_plane *primary_plane;
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 	int ret;
 
+	primary_plane = &cirrus->primary_plane;
+	ret = drm_universal_plane_init(dev, primary_plane, 0,
+				       &cirrus_primary_plane_funcs,
+				       cirrus_primary_plane_formats,
+				       ARRAY_SIZE(cirrus_primary_plane_formats),
+				       cirrus_primary_plane_format_modifiers,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ret;
+	drm_plane_helper_add(primary_plane, &cirrus_primary_plane_helper_funcs);
+
+	crtc = &cirrus->crtc;
+	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
+					&cirrus_crtc_funcs, NULL);
+	if (ret)
+		return ret;
+	drm_crtc_helper_add(crtc, &cirrus_crtc_helper_funcs);
+
+	encoder = &cirrus->encoder;
+	ret = drm_encoder_init(dev, encoder, &cirrus_encoder_funcs,
+			       DRM_MODE_ENCODER_DAC, NULL);
+	if (ret)
+		return ret;
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
 	connector = &cirrus->connector;
-	ret = drm_connector_init(&cirrus->dev, connector, &cirrus_connector_funcs,
+	ret = drm_connector_init(dev, connector, &cirrus_connector_funcs,
 				 DRM_MODE_CONNECTOR_VGA);
 	if (ret)
 		return ret;
 	drm_connector_helper_add(connector, &cirrus_connector_helper_funcs);
 
-	return drm_simple_display_pipe_init(dev,
-					    &cirrus->pipe,
-					    &cirrus_pipe_funcs,
-					    cirrus_primary_plane_formats,
-					    ARRAY_SIZE(cirrus_primary_plane_formats),
-					    cirrus_primary_plane_format_modifiers,
-					    connector);
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /* ------------------------------------------------------------------ */
-- 
2.39.1

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

[PATCH 09/17] drm/cirrus: Enable damage clipping on primary plane

[Thomas Zimmermann]

Enable damage clipping on the primary plane and iterate over small
areas of reported framebuffer damage. Avoid the overhead of permanent
full-screen updates that cirrus currently implements.

This problem is indicated by the warning

  drm_plane_enable_fb_damage_clips() not called

in the kernel's log. Without damage clipping, drivers do full updates
of the screen area. This is costly as many screen updates, such as
cursor movement or command-line input, only change a small portion
of the output. Damage clipping allows renderers to inform drivers about
the changed areas.

With the damage information known, cirrus now iterates over a list of
change areas and only flushes those to the hardware's scanout buffer.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index af26de9ef329..46c6aa34ba79 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -393,7 +393,8 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *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_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
-	struct drm_rect rect;
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_rect damage;
 	int idx;
 
 	if (!fb)
@@ -407,8 +408,10 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	if (cirrus->pitch != cirrus_pitch(fb))
 		cirrus_pitch_set(cirrus, fb);
 
-	if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
-		cirrus_fb_blit_rect(fb, &shadow_plane_state->data[0], &rect);
+	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		cirrus_fb_blit_rect(fb, &shadow_plane_state->data[0], &damage);
+	}
 
 	drm_dev_exit(idx);
 }
@@ -529,6 +532,7 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
 	if (ret)
 		return ret;
 	drm_plane_helper_add(primary_plane, &cirrus_primary_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(primary_plane);
 
 	crtc = &cirrus->crtc;
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
-- 
2.39.1

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

[PATCH 10/17] drm/cirrus: Inline cirrus_fb_blit_rect()

[Thomas Zimmermann]

Inline cirrus_fb_blit_rect into its only caller. While at it, update
the code to use IOSYS_MAP_INIT_OFFSET(), which is the ideomatic way
of initializing struct iosys_map with an offset.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 46c6aa34ba79..a483abc2e6ba 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -317,21 +317,6 @@ static void cirrus_pitch_set(struct cirrus_device *cirrus,
 	cirrus_set_start_address(cirrus, 0);
 }
 
-static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
-			       const struct iosys_map *vmap,
-			       struct drm_rect *rect)
-{
-	struct cirrus_device *cirrus = to_cirrus(fb->dev);
-	struct iosys_map dst;
-
-	iosys_map_set_vaddr_iomem(&dst, cirrus->vram);
-	iosys_map_incr(&dst, drm_fb_clip_offset(cirrus->pitch, fb->format, rect));
-
-	drm_fb_blit(&dst, &cirrus->pitch, cirrus->format->format, vmap, fb, rect);
-
-	return 0;
-}
-
 static int cirrus_check_size(int width, int height,
 			     struct drm_framebuffer *fb)
 {
@@ -393,6 +378,7 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *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_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct iosys_map vaddr = IOSYS_MAP_INIT_VADDR_IOMEM(cirrus->vram);
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 	int idx;
@@ -410,7 +396,11 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
 	drm_atomic_for_each_plane_damage(&iter, &damage) {
-		cirrus_fb_blit_rect(fb, &shadow_plane_state->data[0], &damage);
+		unsigned int offset = drm_fb_clip_offset(cirrus->pitch, fb->format, &damage);
+		struct iosys_map dst = IOSYS_MAP_INIT_OFFSET(&vaddr, offset);
+
+		drm_fb_blit(&dst, &cirrus->pitch, cirrus->format->format,
+			    &shadow_plane_state->data[0], fb, &damage);
 	}
 
 	drm_dev_exit(idx);
-- 
2.39.1

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

[PATCH 11/17] drm/cirrus: Remove format test from cirrus_fb_create()

[Thomas Zimmermann]

The DRM core implements a format check when setting a framebuffer
for a plane. [1] Remove the unnecessary test from cirrus_fb_create().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://elixir.bootlin.com/linux/v6.1/source/drivers/gpu/drm/drm_atomic.c#L629 # [1]
---
 drivers/gpu/drm/tiny/cirrus.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index a483abc2e6ba..c1ffbbe1d545 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -559,10 +559,6 @@ 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);
-- 
2.39.1

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

[PATCH 12/17] drm/cirrus: Remove size test from cirrus_fb_create()

[Thomas Zimmermann]

The DRM core implements a size check against the mode config's
limits when creating a framebuffer. [1] Remove the unnecessary
test from cirrus_fb_create() and remove the now-empty function.
Create framebuffers with drm_gem_fb_create_with_dirty().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://elixir.bootlin.com/linux/v6.1/source/drivers/gpu/drm/drm_framebuffer.c#L287 # [1]
---
 drivers/gpu/drm/tiny/cirrus.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index c1ffbbe1d545..c2d7bb775629 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -555,17 +555,8 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
 /* ------------------------------------------------------------------ */
 /* 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 (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,
+	.fb_create = drm_gem_fb_create_with_dirty,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.39.1

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

[PATCH 13/17] drm/cirrus: Test mode against video-memory size in device-wide mode_valid

[Thomas Zimmermann]

Test a display mode against the available amount of video memory in
struct drm_mode_config_funcs.mode_valid, which cirrus implements in
cirrus_mode_config_mode_valid(). This helper tests display modes against
device-wide limits. Remove the now-obsolete per-CRTC test.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index c2d7bb775629..6c2be39d79a5 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -419,15 +419,6 @@ static const struct drm_plane_funcs cirrus_primary_plane_funcs = {
 	DRM_GEM_SHADOW_PLANE_FUNCS,
 };
 
-static enum drm_mode_status cirrus_crtc_helper_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_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
 {
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
@@ -462,7 +453,6 @@ static void cirrus_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs cirrus_crtc_helper_funcs = {
-	.mode_valid = cirrus_crtc_helper_mode_valid,
 	.atomic_check = cirrus_crtc_helper_atomic_check,
 	.atomic_enable = cirrus_crtc_helper_atomic_enable,
 };
@@ -555,8 +545,21 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
 /* ------------------------------------------------------------------ */
 /* cirrus framebuffers & mode config				      */
 
+static enum drm_mode_status cirrus_mode_config_mode_valid(struct drm_device *dev,
+							  const struct drm_display_mode *mode)
+{
+	const struct drm_format_info *format = drm_format_info(DRM_FORMAT_XRGB8888);
+	uint64_t pitch = drm_format_info_min_pitch(format, 0, mode->hdisplay);
+
+	if (pitch * mode->vdisplay > CIRRUS_VRAM_SIZE)
+		return MODE_MEM;
+
+	return MODE_OK;
+}
+
 static const struct drm_mode_config_funcs cirrus_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create_with_dirty,
+	.mode_valid = cirrus_mode_config_mode_valid,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.39.1

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

[PATCH 14/17] drm/cirrus: Inline cirrus_check_size() into primary-plane atomic_check

[Thomas Zimmermann]

Inline the framebuffer size check into the primary plane's atomic_check
cirrus_primary_plane_atomic_check(). No functional changes.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 6c2be39d79a5..8a1ae94d9106 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -317,21 +317,6 @@ static void cirrus_pitch_set(struct cirrus_device *cirrus,
 	cirrus_set_start_address(cirrus, 0);
 }
 
-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 display pipe						      */
 
@@ -354,6 +339,7 @@ static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
 	struct drm_crtc *new_crtc = new_plane_state->crtc;
 	struct drm_crtc_state *new_crtc_state = NULL;
 	int ret;
+	unsigned int pitch;
 
 	if (new_crtc)
 		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
@@ -367,7 +353,15 @@ static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
 	else if (!new_plane_state->visible)
 		return 0;
 
-	return cirrus_check_size(fb->width, fb->height, fb);
+	pitch = cirrus_pitch(fb);
+
+	/* validate size constraints */
+	if (pitch > CIRRUS_MAX_PITCH)
+		return -EINVAL;
+	else if (pitch * fb->height > CIRRUS_VRAM_SIZE)
+		return -EINVAL;
+
+	return 0;
 }
 
 static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
-- 
2.39.1

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

[PATCH 15/17] drm/cirrus: Introduce struct cirrus_primary_plane_state

[Thomas Zimmermann]

The cirrus driver maintains plane state, format and pitch, in it's
device structure. Introduce a plane state for the primary plane to
store the values.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/cirrus.c | 59 ++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 8a1ae94d9106..ec6b918dce7b 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -74,6 +74,16 @@ struct cirrus_device {
 
 #define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev)
 
+struct cirrus_primary_plane_state {
+	struct drm_shadow_plane_state base;
+};
+
+static inline struct cirrus_primary_plane_state *
+to_cirrus_primary_plane_state(struct drm_plane_state *plane_state)
+{
+	return container_of(plane_state, struct cirrus_primary_plane_state, base.base);
+};
+
 /* ------------------------------------------------------------------ */
 /*
  * The meat of this driver. The core passes us a mode and we have to program
@@ -406,11 +416,58 @@ static const struct drm_plane_helper_funcs cirrus_primary_plane_helper_funcs = {
 	.atomic_update = cirrus_primary_plane_helper_atomic_update,
 };
 
+static struct drm_plane_state *
+cirrus_primary_plane_atomic_duplicate_state(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state = plane->state;
+	struct cirrus_primary_plane_state *new_primary_plane_state;
+	struct drm_shadow_plane_state *new_shadow_plane_state;
+
+	if (!plane_state)
+		return NULL;
+
+	new_primary_plane_state = kzalloc(sizeof(*new_primary_plane_state), GFP_KERNEL);
+	if (!new_primary_plane_state)
+		return NULL;
+	new_shadow_plane_state = &new_primary_plane_state->base;
+
+	__drm_gem_duplicate_shadow_plane_state(plane, new_shadow_plane_state);
+
+	return &new_shadow_plane_state->base;
+}
+
+static void cirrus_primary_plane_atomic_destroy_state(struct drm_plane *plane,
+						      struct drm_plane_state *plane_state)
+{
+	struct cirrus_primary_plane_state *primary_plane_state =
+		to_cirrus_primary_plane_state(plane_state);
+
+	__drm_gem_destroy_shadow_plane_state(&primary_plane_state->base);
+	kfree(primary_plane_state);
+}
+
+static void cirrus_reset_primary_plane(struct drm_plane *plane)
+{
+	struct cirrus_primary_plane_state *primary_plane_state;
+
+	if (plane->state) {
+		cirrus_primary_plane_atomic_destroy_state(plane, plane->state);
+		plane->state = NULL; /* must be set to NULL here */
+	}
+
+	primary_plane_state = kzalloc(sizeof(*primary_plane_state), GFP_KERNEL);
+	if (!primary_plane_state)
+		return;
+	__drm_gem_reset_shadow_plane(plane, &primary_plane_state->base);
+}
+
 static const struct drm_plane_funcs cirrus_primary_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = drm_plane_cleanup,
-	DRM_GEM_SHADOW_PLANE_FUNCS,
+	.reset = cirrus_reset_primary_plane,
+	.atomic_duplicate_state = cirrus_primary_plane_atomic_duplicate_state,
+	.atomic_destroy_state = cirrus_primary_plane_atomic_destroy_state,
 };
 
 static int cirrus_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
-- 
2.39.1

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

[PATCH 16/17] drm/cirrus: Store HW format/pitch in primary-plane state

[Thomas Zimmermann]

The hardware settings for color format and pitch are state of the
primary plane. Store the values in the primary plane's state structure
struct cirrus_primary_plane_state. Adapt all callers.

All fields in struct cirrus_device are now considered immutable after
initialization. Plane updates consider the difference between the old
and the new plane state before updating format or pitch.

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

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index ec6b918dce7b..ad67fb895213 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -63,10 +63,6 @@ struct cirrus_device {
 	struct drm_encoder	       encoder;
 	struct drm_connector	       connector;
 
-	/* HW scanout buffer */
-	const struct drm_format_info   *format;
-	unsigned int		       pitch;
-
 	/* HW resources */
 	void __iomem		       *vram;
 	void __iomem		       *mmio;
@@ -76,6 +72,10 @@ struct cirrus_device {
 
 struct cirrus_primary_plane_state {
 	struct drm_shadow_plane_state base;
+
+	/* HW scanout buffer */
+	const struct drm_format_info   *format;
+	unsigned int		       pitch;
 };
 
 static inline struct cirrus_primary_plane_state *
@@ -268,15 +268,14 @@ static void cirrus_mode_set(struct cirrus_device *cirrus,
 }
 
 static void cirrus_format_set(struct cirrus_device *cirrus,
-			      struct drm_framebuffer *fb)
+			      const struct drm_format_info *format)
 {
 	u8 sr07, hdr;
 
 	sr07 = rreg_seq(cirrus, 0x07);
 	sr07 &= 0xe0;
 
-	cirrus->format = cirrus_format(fb);
-	switch (cirrus->format->format) {
+	switch (format->format) {
 	case DRM_FORMAT_C8:
 		sr07 |= 0x11;
 		hdr = 0x00;
@@ -308,20 +307,18 @@ static void cirrus_format_set(struct cirrus_device *cirrus,
 	wreg_hdr(cirrus, hdr);
 }
 
-static void cirrus_pitch_set(struct cirrus_device *cirrus,
-			     struct drm_framebuffer *fb)
+static void cirrus_pitch_set(struct cirrus_device *cirrus, unsigned int pitch)
 {
 	u8 cr13, cr1b;
 
 	/* Program the pitch */
-	cirrus->pitch = cirrus_pitch(fb);
-	cr13 = cirrus->pitch / 8;
+	cr13 = pitch / 8;
 	wreg_crt(cirrus, VGA_CRTC_OFFSET, cr13);
 
 	/* Enable extended blanking and pitch bits, and enable full memory */
 	cr1b = 0x22;
-	cr1b |= (cirrus->pitch >> 7) & 0x10;
-	cr1b |= (cirrus->pitch >> 6) & 0x40;
+	cr1b |= (pitch >> 7) & 0x10;
+	cr1b |= (pitch >> 6) & 0x40;
 	wreg_crt(cirrus, 0x1b, cr1b);
 
 	cirrus_set_start_address(cirrus, 0);
@@ -345,6 +342,8 @@ static int cirrus_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 cirrus_primary_plane_state *new_primary_plane_state =
+		to_cirrus_primary_plane_state(new_plane_state);
 	struct drm_framebuffer *fb = new_plane_state->fb;
 	struct drm_crtc *new_crtc = new_plane_state->crtc;
 	struct drm_crtc_state *new_crtc_state = NULL;
@@ -371,6 +370,9 @@ static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
 	else if (pitch * fb->height > CIRRUS_VRAM_SIZE)
 		return -EINVAL;
 
+	new_primary_plane_state->format = cirrus_format(fb);
+	new_primary_plane_state->pitch = pitch;
+
 	return 0;
 }
 
@@ -379,9 +381,15 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 {
 	struct cirrus_device *cirrus = to_cirrus(plane->dev);
 	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct cirrus_primary_plane_state *primary_plane_state =
+		to_cirrus_primary_plane_state(plane_state);
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
+	const struct drm_format_info *format = primary_plane_state->format;
+	unsigned int pitch = primary_plane_state->pitch;
 	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct cirrus_primary_plane_state *old_primary_plane_state =
+		to_cirrus_primary_plane_state(old_plane_state);
 	struct iosys_map vaddr = IOSYS_MAP_INIT_VADDR_IOMEM(cirrus->vram);
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
@@ -393,18 +401,17 @@ static void cirrus_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	if (!drm_dev_enter(&cirrus->dev, &idx))
 		return;
 
-	if (cirrus->format != cirrus_format(fb))
-		cirrus_format_set(cirrus, fb);
-	if (cirrus->pitch != cirrus_pitch(fb))
-		cirrus_pitch_set(cirrus, fb);
+	if (old_primary_plane_state->format != format)
+		cirrus_format_set(cirrus, format);
+	if (old_primary_plane_state->pitch != pitch)
+		cirrus_pitch_set(cirrus, pitch);
 
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
 	drm_atomic_for_each_plane_damage(&iter, &damage) {
-		unsigned int offset = drm_fb_clip_offset(cirrus->pitch, fb->format, &damage);
+		unsigned int offset = drm_fb_clip_offset(pitch, format, &damage);
 		struct iosys_map dst = IOSYS_MAP_INIT_OFFSET(&vaddr, offset);
 
-		drm_fb_blit(&dst, &cirrus->pitch, cirrus->format->format,
-			    &shadow_plane_state->data[0], fb, &damage);
+		drm_fb_blit(&dst, &pitch, format->format, shadow_plane_state->data, fb, &damage);
 	}
 
 	drm_dev_exit(idx);
@@ -420,6 +427,8 @@ static struct drm_plane_state *
 cirrus_primary_plane_atomic_duplicate_state(struct drm_plane *plane)
 {
 	struct drm_plane_state *plane_state = plane->state;
+	struct cirrus_primary_plane_state *primary_plane_state =
+		to_cirrus_primary_plane_state(plane_state);
 	struct cirrus_primary_plane_state *new_primary_plane_state;
 	struct drm_shadow_plane_state *new_shadow_plane_state;
 
@@ -432,6 +441,8 @@ cirrus_primary_plane_atomic_duplicate_state(struct drm_plane *plane)
 	new_shadow_plane_state = &new_primary_plane_state->base;
 
 	__drm_gem_duplicate_shadow_plane_state(plane, new_shadow_plane_state);
+	new_primary_plane_state->format = primary_plane_state->format;
+	new_primary_plane_state->pitch = primary_plane_state->pitch;
 
 	return &new_shadow_plane_state->base;
 }
-- 
2.39.1

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

[PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Thomas Zimmermann]

Set the VGA bit for unblanking with macro constants instead of magic
values. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/tiny/cirrus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index ad67fb895213..594bc472862f 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -509,7 +509,7 @@ static void cirrus_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	cirrus_mode_set(cirrus, &crtc_state->mode);
 
 	/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
-	outb(0x20, 0x3c0);
+	outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
 
 	drm_dev_exit(idx);
 }
-- 
2.39.1

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Gerd Hoffmann]

On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> Set the VGA bit for unblanking with macro constants instead of magic
> values. No functional changes.

blank/unblank should work simliar to bochs (see commit 250e743915d4),
that is maybe a nice thing to add of you modernize the driver anyway.

take care,
  Gerd

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1126 bytes --]

Hi,

thanks for taking a look at the patches.

Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
>> Set the VGA bit for unblanking with macro constants instead of magic
>> values. No functional changes.
> 
> blank/unblank should work simliar to bochs (see commit 250e743915d4),
> that is maybe a nice thing to add of you modernize the driver anyway.
Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS 
controls palette access, but blanking is sounds more like DPMS.

The PAS setting is actually part of the primary plane, so it's current 
location in the CRTC code is misleading. I didn't want to change the 
driver logic too much, but I guess I'll fix that in the next iteration.

Best regards
Thomas

[1] 
https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/attrreg.htm#3C0

> 
> take care,
>    Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Gerd Hoffmann]

On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
> Hi,
> 
> thanks for taking a look at the patches.
> 
> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> > On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> > > Set the VGA bit for unblanking with macro constants instead of magic
> > > values. No functional changes.
> > 
> > blank/unblank should work simliar to bochs (see commit 250e743915d4),
> > that is maybe a nice thing to add of you modernize the driver anyway.
> Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS
> controls palette access, but blanking is sounds more like DPMS.

Yes, strictly speaking it is not the same thing. DPMS blank will send
the monitor into suspend mode which this does not.  On virtual hardware
there isn't much of a difference though ;)

take care,
  Gerd

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Ville Syrjälä]

On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
> Hi,
> 
> thanks for taking a look at the patches.
> 
> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> > On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> >> Set the VGA bit for unblanking with macro constants instead of magic
> >> values. No functional changes.
> > 
> > blank/unblank should work simliar to bochs (see commit 250e743915d4),
> > that is maybe a nice thing to add of you modernize the driver anyway.
> Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS 
> controls palette access, but blanking is sounds more like DPMS.

Why aren't people just using the normal way of flipping the
screen off bit in sequencer register 01?


> 
> The PAS setting is actually part of the primary plane, so it's current 
> location in the CRTC code is misleading. I didn't want to change the 
> driver logic too much, but I guess I'll fix that in the next iteration.
> 
> Best regards
> Thomas
> 
> [1] 
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/attrreg.htm#3C0
> 
> > 
> > take care,
> >    Gerd
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev




-- 
Ville Syrjälä
Intel
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Gerd Hoffmann]

On Thu, Feb 16, 2023 at 02:52:51PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
> > Hi,
> > 
> > thanks for taking a look at the patches.
> > 
> > Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> > > On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> > >> Set the VGA bit for unblanking with macro constants instead of magic
> > >> values. No functional changes.
> > > 
> > > blank/unblank should work simliar to bochs (see commit 250e743915d4),
> > > that is maybe a nice thing to add of you modernize the driver anyway.
> > Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS 
> > controls palette access, but blanking is sounds more like DPMS.
> 
> Why aren't people just using the normal way of flipping the
> screen off bit in sequencer register 01?

qemu vga emulation doesn't check that bit ...

take care,
  Gerd

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1969 bytes --]

Hi

Am 16.02.23 um 13:52 schrieb Ville Syrjälä:
> On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
>> Hi,
>>
>> thanks for taking a look at the patches.
>>
>> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
>>> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
>>>> Set the VGA bit for unblanking with macro constants instead of magic
>>>> values. No functional changes.
>>>
>>> blank/unblank should work simliar to bochs (see commit 250e743915d4),
>>> that is maybe a nice thing to add of you modernize the driver anyway.
>> Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS
>> controls palette access, but blanking is sounds more like DPMS.
> 
> Why aren't people just using the normal way of flipping the
> screen off bit in sequencer register 01?

Setting the SD bit in SR01 isn't a bad idea. We can do this as part of 
enabling/disabling the plane.

But for PAS, we don't have a choice. It's one of the bazillion obscure 
VGA settings and (according to a comment in the source code) we need to 
update it for compatibility.

Best regards
Thomas

> 
> 
>>
>> The PAS setting is actually part of the primary plane, so it's current
>> location in the CRTC code is misleading. I didn't want to change the
>> driver logic too much, but I guess I'll fix that in the next iteration.
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/attrreg.htm#3C0
>>
>>>
>>> take care,
>>>     Gerd
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
> 
> 
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Ville Syrjälä]

On Thu, Feb 16, 2023 at 02:21:43PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 16.02.23 um 13:52 schrieb Ville Syrjälä:
> > On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
> >> Hi,
> >>
> >> thanks for taking a look at the patches.
> >>
> >> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> >>> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> >>>> Set the VGA bit for unblanking with macro constants instead of magic
> >>>> values. No functional changes.
> >>>
> >>> blank/unblank should work simliar to bochs (see commit 250e743915d4),
> >>> that is maybe a nice thing to add of you modernize the driver anyway.
> >> Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS
> >> controls palette access, but blanking is sounds more like DPMS.
> > 
> > Why aren't people just using the normal way of flipping the
> > screen off bit in sequencer register 01?
> 
> Setting the SD bit in SR01 isn't a bad idea. We can do this as part of 
> enabling/disabling the plane.
> 
> But for PAS, we don't have a choice. It's one of the bazillion obscure 
> VGA settings and (according to a comment in the source code) we need to 
> update it for compatibility.

Well, you do need to enable the palette to see something
other that border color. Not sure tha't a very obscure thing :P

On a related note, the code looks pretty sketchy. It just
blindly writes to 0x3c0 assuming it is the attribute controller
index register. But unless you explicitly reset the flip-flop
it could actually be the data write register instead. That could
easily happen if the previous access to the attribute controller
was a read since reads do not toggle the register role.

-- 
Ville Syrjälä
Intel
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2198 bytes --]

Hi

Am 16.02.23 um 18:20 schrieb Ville Syrjälä:
> On Thu, Feb 16, 2023 at 02:21:43PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 16.02.23 um 13:52 schrieb Ville Syrjälä:
>>> On Thu, Feb 16, 2023 at 01:03:02PM +0100, Thomas Zimmermann wrote:
>>>> Hi,
>>>>
>>>> thanks for taking a look at the patches.
>>>>
>>>> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
>>>>> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
>>>>>> Set the VGA bit for unblanking with macro constants instead of magic
>>>>>> values. No functional changes.
>>>>>
>>>>> blank/unblank should work simliar to bochs (see commit 250e743915d4),
>>>>> that is maybe a nice thing to add of you modernize the driver anyway.
>>>> Yeah, it's the VGA PAS field. [1] But is it really called blanking? PAS
>>>> controls palette access, but blanking is sounds more like DPMS.
>>>
>>> Why aren't people just using the normal way of flipping the
>>> screen off bit in sequencer register 01?
>>
>> Setting the SD bit in SR01 isn't a bad idea. We can do this as part of
>> enabling/disabling the plane.
>>
>> But for PAS, we don't have a choice. It's one of the bazillion obscure
>> VGA settings and (according to a comment in the source code) we need to
>> update it for compatibility.
> 
> Well, you do need to enable the palette to see something
> other that border color. Not sure tha't a very obscure thing :P

Pun intended? :D

> 
> On a related note, the code looks pretty sketchy. It just
> blindly writes to 0x3c0 assuming it is the attribute controller
> index register. But unless you explicitly reset the flip-flop
> it could actually be the data write register instead. That could
> easily happen if the previous access to the attribute controller
> was a read since reads do not toggle the register role.

Yeah, the attribute controller is *fun* to work with. In the next 
iteration, I'll probably add a helper to do all this; similar to bochs.

Best regards
Thomas

> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 671 bytes --]

Hi

Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
>> Set the VGA bit for unblanking with macro constants instead of magic
>> values. No functional changes.
> 
> blank/unblank should work simliar to bochs (see commit 250e743915d4),
> that is maybe a nice thing to add of you modernize the driver anyway.
> 
> take care,
>    Gerd
> 

Do you have comments on the other patches?

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

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

Re: [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank

[Gerd Hoffmann]

On Mon, Feb 20, 2023 at 03:22:03PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 16.02.23 um 12:33 schrieb Gerd Hoffmann:
> > On Wed, Feb 15, 2023 at 05:15:17PM +0100, Thomas Zimmermann wrote:
> > > Set the VGA bit for unblanking with macro constants instead of magic
> > > values. No functional changes.
> > 
> > blank/unblank should work simliar to bochs (see commit 250e743915d4),
> > that is maybe a nice thing to add of you modernize the driver anyway.
> > 
> > take care,
> >    Gerd
> > 
> 
> Do you have comments on the other patches?

Checked briefly only, looked sane overall.  Seems the blit and format
conversions helpers improved alot since I've added them initially (don't
follow drm that closely any more, busy with other stuff), nice to see
cirrus being updated to that and getting dirty tracking support.

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

take care,
  Gerd

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