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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 01/17] drm/cirrus: Compute blit destination offset in single location
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 02/17] drm/cirrus: Replace cpp value with format
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 03/17] drm/cirrus: Use drm_fb_blit() to update scanout buffer
Cirrus' blit helper reimplements code from the shared blit helper drm_fb_blit(). Use the helper instead. Signed-off-by: Thomas Zimmermann <tzimmermann at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 04/17] drm/cirrus: Move drm_dev_{enter, exit}() into DRM helpers
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 05/17] drm/cirrus: Split cirrus_mode_set() into smaller functions
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 06/17] drm/cirrus: Integrate connector into pipeline code
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 07/17] drm/cirrus: Move primary-plane format arrays
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 08/17] drm/cirrus: Convert to regular atomic helpers
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 09/17] drm/cirrus: Enable damage clipping on primary plane
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 10/17] drm/cirrus: Inline cirrus_fb_blit_rect()
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 11/17] drm/cirrus: Remove format test from cirrus_fb_create()
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 12/17] drm/cirrus: Remove size test from cirrus_fb_create()
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 13/17] drm/cirrus: Test mode against video-memory size in device-wide mode_valid
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 14/17] drm/cirrus: Inline cirrus_check_size() into primary-plane atomic_check
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 15/17] drm/cirrus: Introduce struct cirrus_primary_plane_state
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 16/17] drm/cirrus: Store HW format/pitch in primary-plane state
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 at 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
Thomas Zimmermann
2023-Feb-15 16:15 UTC
[PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank
Set the VGA bit for unblanking with macro constants instead of magic values. No functional changes. Signed-off-by: Thomas Zimmermann <tzimmermann at 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
Possibly Parallel Threads
- [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank
- [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank
- [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank
- [PATCH 17/17] drm/cirrus: Use VGA macro constants to unblank
- [PATCH 00/17] cirrus: Modernize the cirrus driver