From b756b0cbcb8574fc1b6c8448b63bc5b9b8c38f90 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:40 +0100 Subject: [PATCH 01/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_fake_vblank() drm_atomic_helper_fake_vblank() fake a vblank event if needed when a new commit is being applied. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-21-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ece056ab07b4..2ee7df6aca54 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2472,7 +2472,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies); /** * drm_atomic_helper_fake_vblank - fake VBLANK events if needed - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * * This function walks all CRTCs and fakes VBLANK events on those with * &drm_crtc_state.no_vblank set to true and &drm_crtc_state.event != NULL. @@ -2488,25 +2488,25 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_dependencies); * This is part of the atomic helper support for nonblocking commits, see * drm_atomic_helper_setup_commit() for an overview. */ -void drm_atomic_helper_fake_vblank(struct drm_atomic_state *old_state) +void drm_atomic_helper_fake_vblank(struct drm_atomic_state *state) { struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; int i; - for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { unsigned long flags; if (!new_crtc_state->no_vblank) continue; - spin_lock_irqsave(&old_state->dev->event_lock, flags); + spin_lock_irqsave(&state->dev->event_lock, flags); if (new_crtc_state->event) { drm_crtc_send_vblank_event(crtc, new_crtc_state->event); new_crtc_state->event = NULL; } - spin_unlock_irqrestore(&old_state->dev->event_lock, flags); + spin_unlock_irqrestore(&state->dev->event_lock, flags); } } EXPORT_SYMBOL(drm_atomic_helper_fake_vblank); -- 2.51.0 From 3fae6d20e329816e4f0b32b765eaa4e48b02ec66 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:41 +0100 Subject: [PATCH 02/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_commit_hw_done() drm_atomic_helper_commit_hw_done() signals hardware completion of a given commit. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-22-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2ee7df6aca54..15937860e406 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2513,7 +2513,7 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank); /** * drm_atomic_helper_commit_hw_done - setup possible nonblocking commit - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * * This function is used to signal completion of the hardware commit step. After * this step the driver is not allowed to read or change any permanent software @@ -2526,14 +2526,14 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank); * This is part of the atomic helper support for nonblocking commits, see * drm_atomic_helper_setup_commit() for an overview. */ -void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state) +void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *state) { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc_commit *commit; int i; - for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { commit = new_crtc_state->commit; if (!commit) continue; @@ -2553,9 +2553,9 @@ void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state) complete_all(&commit->hw_done); } - if (old_state->fake_commit) { - complete_all(&old_state->fake_commit->hw_done); - complete_all(&old_state->fake_commit->flip_done); + if (state->fake_commit) { + complete_all(&state->fake_commit->hw_done); + complete_all(&state->fake_commit->flip_done); } } EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done); -- 2.51.0 From e64834b5094ff4ef2cfd960b82f69c2c26345501 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:42 +0100 Subject: [PATCH 03/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_wait_for_vblanks() drm_atomic_helper_wait_for_vblanks() waits for vblank events on all the CRTCs affected by a commit. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-23-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 15937860e406..8f290535c861 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1668,7 +1668,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_fences); /** * drm_atomic_helper_wait_for_vblanks - wait for vblank on CRTCs * @dev: DRM device - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * * Helper to, after atomic commit, wait for vblanks on all affected * CRTCs (ie. before cleaning up old framebuffers using @@ -1682,7 +1682,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_fences); */ void drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, - struct drm_atomic_state *old_state) + struct drm_atomic_state *state) { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; @@ -1693,10 +1693,10 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, * Legacy cursor ioctls are completely unsynced, and userspace * relies on that (by doing tons of cursor updates). */ - if (old_state->legacy_cursor_update) + if (state->legacy_cursor_update) return; - for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { if (!new_crtc_state->active) continue; @@ -1705,17 +1705,17 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, continue; crtc_mask |= drm_crtc_mask(crtc); - old_state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc); + state->crtcs[i].last_vblank_count = drm_crtc_vblank_count(crtc); } - for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { if (!(crtc_mask & drm_crtc_mask(crtc))) continue; ret = wait_event_timeout(dev->vblank[i].queue, - old_state->crtcs[i].last_vblank_count != - drm_crtc_vblank_count(crtc), - msecs_to_jiffies(100)); + state->crtcs[i].last_vblank_count != + drm_crtc_vblank_count(crtc), + msecs_to_jiffies(100)); WARN(!ret, "[CRTC:%d:%s] vblank wait timed out\n", crtc->base.id, crtc->name); -- 2.51.0 From 6280e96f8a5dcee3125d8963bc80773c99536429 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:43 +0100 Subject: [PATCH 04/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_cleanup_planes() drm_atomic_helper_cleanup_planes() is one of the final part of a commit, and will free up all plane resources used in the previous commit. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-24-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 8f290535c861..9ec332360d08 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2989,10 +2989,10 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc); /** * drm_atomic_helper_cleanup_planes - cleanup plane resources after commit * @dev: DRM device - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * * This function cleans up plane state, specifically framebuffers, from the old - * configuration. Hence the old configuration must be perserved in @old_state to + * configuration. Hence the old configuration must be perserved in @state to * be able to call this function. * * This function may not be called on the new state when the atomic update @@ -3000,13 +3000,13 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc); * drm_atomic_helper_unprepare_planes() in this case. */ void drm_atomic_helper_cleanup_planes(struct drm_device *dev, - struct drm_atomic_state *old_state) + struct drm_atomic_state *state) { struct drm_plane *plane; struct drm_plane_state *old_plane_state; int i; - for_each_old_plane_in_state(old_state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { const struct drm_plane_helper_funcs *funcs = plane->helper_private; if (funcs->cleanup_fb) -- 2.51.0 From bc8ab44023c1a4193ef2c5d2a152955722b33a8e Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:44 +0100 Subject: [PATCH 05/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_commit_cleanup_done() drm_atomic_helper_wait_for_dependencies() is the final part of a commit and signals it completion. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-25-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9ec332360d08..c429b0248e81 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2562,23 +2562,23 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_hw_done); /** * drm_atomic_helper_commit_cleanup_done - signal completion of commit - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * - * This signals completion of the atomic update @old_state, including any + * This signals completion of the atomic update @state, including any * cleanup work. If used, it must be called right before calling * drm_atomic_state_put(). * * This is part of the atomic helper support for nonblocking commits, see * drm_atomic_helper_setup_commit() for an overview. */ -void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state) +void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *state) { struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; struct drm_crtc_commit *commit; int i; - for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { commit = old_crtc_state->commit; if (WARN_ON(!commit)) continue; @@ -2591,9 +2591,9 @@ void drm_atomic_helper_commit_cleanup_done(struct drm_atomic_state *old_state) spin_unlock(&crtc->commit_lock); } - if (old_state->fake_commit) { - complete_all(&old_state->fake_commit->cleanup_done); - WARN_ON(!try_wait_for_completion(&old_state->fake_commit->hw_done)); + if (state->fake_commit) { + complete_all(&state->fake_commit->cleanup_done); + WARN_ON(!try_wait_for_completion(&state->fake_commit->hw_done)); } } EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done); -- 2.51.0 From f56b6db3e5e432fd0c82ab231572f92a159b5d34 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 13 Feb 2025 15:43:45 +0100 Subject: [PATCH 06/16] drm/atomic-helper: Change parameter name of drm_atomic_helper_wait_for_flip_done() drm_atomic_helper_wait_for_flip_done() will wait for pages flips on all CRTCs affected by a given commit. It takes the drm_atomic_state being committed as a parameter. However, that parameter name is called (and documented) as old_state, which is pretty confusing. Let's rename that variable as state. Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250213-bridge-connector-v3-26-e71598f49c8f@kernel.org Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index c429b0248e81..7a25e70694ba 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1728,7 +1728,7 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); /** * drm_atomic_helper_wait_for_flip_done - wait for all page flips to be done * @dev: DRM device - * @old_state: atomic state object with old state structures + * @state: atomic state object being committed * * Helper to, after atomic commit, wait for page flips on all affected * crtcs (ie. before cleaning up old framebuffers using @@ -1741,16 +1741,16 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); * initialized using drm_atomic_helper_setup_commit(). */ void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, - struct drm_atomic_state *old_state) + struct drm_atomic_state *state) { struct drm_crtc *crtc; int i; for (i = 0; i < dev->mode_config.num_crtc; i++) { - struct drm_crtc_commit *commit = old_state->crtcs[i].commit; + struct drm_crtc_commit *commit = state->crtcs[i].commit; int ret; - crtc = old_state->crtcs[i].ptr; + crtc = state->crtcs[i].ptr; if (!crtc || !commit) continue; @@ -1761,8 +1761,8 @@ void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, crtc->base.id, crtc->name); } - if (old_state->fake_commit) - complete_all(&old_state->fake_commit->flip_done); + if (state->fake_commit) + complete_all(&state->fake_commit->flip_done); } EXPORT_SYMBOL(drm_atomic_helper_wait_for_flip_done); -- 2.51.0 From 6ec054a52d92fd172560996e3b2a4234a31f5265 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Feb 2025 13:22:04 +0100 Subject: [PATCH 07/16] drm/ast: cursor: Calculate checksum in helper Setting the cursor image requires a 32-bit checksum of the cursor image data. The current cursor code converts the image to ARGB4444 format and computes the checksum in a single step. Moving the checksum calculation into a separate helper will allow to move the format conversion into a shared helper. v2: - don't loop for checksum'ing final pixel (Jocelyn) - fix typo in commit message (Jocelyn) Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250217122336.230067-2-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_drv.h | 13 +++++++- drivers/gpu/drm/ast/ast_mode.c | 61 +++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 022a8c070c1b..d3115b31b032 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -142,6 +142,17 @@ static inline struct ast_plane *to_ast_plane(struct drm_plane *plane) return container_of(plane, struct ast_plane, base); } +struct ast_cursor_plane { + struct ast_plane base; + + u8 argb4444[AST_HWC_SIZE]; +}; + +static inline struct ast_cursor_plane *to_ast_cursor_plane(struct drm_plane *plane) +{ + return container_of(to_ast_plane(plane), struct ast_cursor_plane, base); +} + /* * Connector */ @@ -186,7 +197,7 @@ struct ast_device { enum ast_tx_chip tx_chip; struct ast_plane primary_plane; - struct ast_plane cursor_plane; + struct ast_cursor_plane cursor_plane; struct drm_crtc crtc; union { struct { diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index bd781293b6d9..d25852b0ce03 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -632,7 +632,34 @@ static int ast_primary_plane_init(struct ast_device *ast) * Cursor plane */ -static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, int width, int height) +static u32 ast_cursor_calculate_checksum(const void *src, unsigned int width, unsigned int height) +{ + u32 csum = 0; + unsigned int one_pixel_copy = width & BIT(0); + unsigned int two_pixel_copy = width - one_pixel_copy; + unsigned int trailing_bytes = (AST_MAX_HWC_WIDTH - width) * sizeof(u16); + unsigned int x, y; + + for (y = 0; y < height; y++) { + for (x = 0; x < two_pixel_copy; x += 2) { + const u32 *src32 = (const u32 *)src; + + csum += *src32; + src += SZ_4; + } + if (one_pixel_copy) { + const u16 *src16 = (const u16 *)src; + + csum += *src16; + src += SZ_2; + } + src += trailing_bytes; + } + + return csum; +} + +static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, u8 *tmp, int width, int height) { union { u32 ul; @@ -642,9 +669,9 @@ static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, int width, i u16 us; u8 b[2]; } data16; - u32 csum = 0; + u32 csum; s32 alpha_dst_delta, last_alpha_dst_delta; - u8 __iomem *dstxor; + u8 *dstxor; const u8 *srcxor; int i, j; u32 per_pixel_copy, two_pixel_copy; @@ -653,7 +680,7 @@ static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, int width, i last_alpha_dst_delta = alpha_dst_delta - (width << 1); srcxor = src; - dstxor = (u8 *)dst + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - height) * alpha_dst_delta; + dstxor = tmp + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - height) * alpha_dst_delta; per_pixel_copy = width & 1; two_pixel_copy = width >> 1; @@ -665,21 +692,17 @@ static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, int width, i data32.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4); data32.b[2] = srcdata32[1].b[1] | (srcdata32[1].b[0] >> 4); data32.b[3] = srcdata32[1].b[3] | (srcdata32[1].b[2] >> 4); - - writel(data32.ul, dstxor); - csum += data32.ul; + memcpy(dstxor, &data32, 4); dstxor += 4; srcxor += 8; - } for (i = 0; i < per_pixel_copy; i++) { srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0; data16.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4); data16.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4); - writew(data16.us, dstxor); - csum += (u32)data16.us; + memcpy(dstxor, &data16, 2); dstxor += 2; srcxor += 4; @@ -687,6 +710,11 @@ static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, int width, i dstxor += last_alpha_dst_delta; } + csum = ast_cursor_calculate_checksum(tmp, width, height); + + /* write pixel data */ + memcpy_toio(dst, tmp, AST_HWC_SIZE); + /* write checksum + signature */ dst += AST_HWC_SIZE; writel(csum, dst); @@ -767,6 +795,7 @@ static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) { + struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane); struct ast_plane *ast_plane = to_ast_plane(plane); 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); @@ -789,7 +818,8 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, */ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) { - ast_update_cursor_image(dst, src, fb->width, fb->height); + ast_update_cursor_image(dst, src, ast_cursor_plane->argb4444, + fb->width, fb->height); ast_set_cursor_base(ast, dst_off); } @@ -849,8 +879,9 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = { static int ast_cursor_plane_init(struct ast_device *ast) { struct drm_device *dev = &ast->base; - struct ast_plane *ast_cursor_plane = &ast->cursor_plane; - struct drm_plane *cursor_plane = &ast_cursor_plane->base; + struct ast_cursor_plane *ast_cursor_plane = &ast->cursor_plane; + struct ast_plane *ast_plane = &ast_cursor_plane->base; + struct drm_plane *cursor_plane = &ast_plane->base; size_t size; void __iomem *vaddr; u64 offset; @@ -869,7 +900,7 @@ static int ast_cursor_plane_init(struct ast_device *ast) vaddr = ast->vram + ast->vram_fb_available - size; offset = ast->vram_fb_available - size; - ret = ast_plane_init(dev, ast_cursor_plane, vaddr, offset, size, + ret = ast_plane_init(dev, ast_plane, vaddr, offset, size, 0x01, &ast_cursor_plane_funcs, ast_cursor_plane_formats, ARRAY_SIZE(ast_cursor_plane_formats), NULL, DRM_PLANE_TYPE_CURSOR); @@ -1156,7 +1187,7 @@ static int ast_crtc_init(struct ast_device *ast) int ret; ret = drm_crtc_init_with_planes(dev, crtc, &ast->primary_plane.base, - &ast->cursor_plane.base, &ast_crtc_funcs, + &ast->cursor_plane.base.base, &ast_crtc_funcs, NULL); if (ret) return ret; -- 2.51.0 From 966a0d49d1cd57165ad3e6232cf9de6fe43ecc63 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Feb 2025 13:22:05 +0100 Subject: [PATCH 08/16] drm/ast: cursor: Move format conversion to shared helper User-space cursor-image data is encoded in ARBG8888, while hardware supports ARGB4444. Implement the format conversion as part of the format-helper framework, so that other drivers can benefit. This allows to respect the damage area of the cursor update. In previous code, all cursor image data had to be converted on each update. Now, only the changed areas require an update. The hardware image is always updated completely, as it is required for the checksum update. The format-conversion helper still contains the old implementation's optimization of writing 2 output pixels at the same time. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250217122336.230067-3-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_drv.h | 4 +- drivers/gpu/drm/ast/ast_mode.c | 71 +++++++---------------------- drivers/gpu/drm/drm_format_helper.c | 69 ++++++++++++++++++++++++++++ include/drm/drm_format_helper.h | 3 ++ 4 files changed, 92 insertions(+), 55 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index d3115b31b032..973abd0cbd42 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -113,7 +113,9 @@ enum ast_config_mode { #define AST_MAX_HWC_WIDTH 64 #define AST_MAX_HWC_HEIGHT 64 -#define AST_HWC_SIZE (AST_MAX_HWC_WIDTH * AST_MAX_HWC_HEIGHT * 2) +#define AST_HWC_PITCH (AST_MAX_HWC_WIDTH * SZ_2) +#define AST_HWC_SIZE (AST_MAX_HWC_HEIGHT * AST_HWC_PITCH) + #define AST_HWC_SIGNATURE_SIZE 32 /* define for signature structure */ diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index d25852b0ce03..8ab006e6bc32 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -659,61 +659,16 @@ static u32 ast_cursor_calculate_checksum(const void *src, unsigned int width, un return csum; } -static void ast_update_cursor_image(u8 __iomem *dst, const u8 *src, u8 *tmp, int width, int height) +static void ast_set_cursor_image(struct ast_device *ast, const u8 *src, + unsigned int width, unsigned int height) { - union { - u32 ul; - u8 b[4]; - } srcdata32[2], data32; - union { - u16 us; - u8 b[2]; - } data16; + u8 __iomem *dst = ast->cursor_plane.base.vaddr; u32 csum; - s32 alpha_dst_delta, last_alpha_dst_delta; - u8 *dstxor; - const u8 *srcxor; - int i, j; - u32 per_pixel_copy, two_pixel_copy; - - alpha_dst_delta = AST_MAX_HWC_WIDTH << 1; - last_alpha_dst_delta = alpha_dst_delta - (width << 1); - - srcxor = src; - dstxor = tmp + last_alpha_dst_delta + (AST_MAX_HWC_HEIGHT - height) * alpha_dst_delta; - per_pixel_copy = width & 1; - two_pixel_copy = width >> 1; - - for (j = 0; j < height; j++) { - for (i = 0; i < two_pixel_copy; i++) { - srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0; - srcdata32[1].ul = *((u32 *)(srcxor + 4)) & 0xf0f0f0f0; - data32.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4); - data32.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4); - data32.b[2] = srcdata32[1].b[1] | (srcdata32[1].b[0] >> 4); - data32.b[3] = srcdata32[1].b[3] | (srcdata32[1].b[2] >> 4); - memcpy(dstxor, &data32, 4); - - dstxor += 4; - srcxor += 8; - } - - for (i = 0; i < per_pixel_copy; i++) { - srcdata32[0].ul = *((u32 *)srcxor) & 0xf0f0f0f0; - data16.b[0] = srcdata32[0].b[1] | (srcdata32[0].b[0] >> 4); - data16.b[1] = srcdata32[0].b[3] | (srcdata32[0].b[2] >> 4); - memcpy(dstxor, &data16, 2); - - dstxor += 2; - srcxor += 4; - } - dstxor += last_alpha_dst_delta; - } - csum = ast_cursor_calculate_checksum(tmp, width, height); + csum = ast_cursor_calculate_checksum(src, width, height); /* write pixel data */ - memcpy_toio(dst, tmp, AST_HWC_SIZE); + memcpy_toio(dst, src, AST_HWC_SIZE); /* write checksum + signature */ dst += AST_HWC_SIZE; @@ -802,9 +757,7 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, struct drm_framebuffer *fb = plane_state->fb; struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane); struct ast_device *ast = to_ast_device(plane->dev); - struct iosys_map src_map = shadow_plane_state->data[0]; struct drm_rect damage; - const u8 *src = src_map.vaddr; /* TODO: Use mapping abstraction properly */ u64 dst_off = ast_plane->offset; u8 __iomem *dst = ast_plane->vaddr; /* TODO: Use mapping abstraction properly */ u8 __iomem *sig = dst + AST_HWC_SIZE; /* TODO: Use mapping abstraction properly */ @@ -818,8 +771,18 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, */ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) { - ast_update_cursor_image(dst, src, ast_cursor_plane->argb4444, - fb->width, fb->height); + u8 *argb4444 = ast_cursor_plane->argb4444; + struct iosys_map argb4444_dst[DRM_FORMAT_MAX_PLANES] = { + IOSYS_MAP_INIT_VADDR(argb4444), + }; + unsigned int argb4444_dst_pitch[DRM_FORMAT_MAX_PLANES] = { + AST_HWC_PITCH, + }; + + drm_fb_argb8888_to_argb4444(argb4444_dst, argb4444_dst_pitch, + shadow_plane_state->data, fb, &damage, + &shadow_plane_state->fmtcnv_state); + ast_set_cursor_image(ast, argb4444, fb->width, fb->height); ast_set_cursor_base(ast, dst_off); } diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index b1be458ed4dd..ecb278b63e8c 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -978,6 +978,75 @@ void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pit } EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray8); +static void drm_fb_argb8888_to_argb4444_line(void *dbuf, const void *sbuf, unsigned int pixels) +{ + unsigned int pixels2 = pixels & ~GENMASK_ULL(0, 0); + __le32 *dbuf32 = dbuf; + __le16 *dbuf16 = dbuf + pixels2 * sizeof(*dbuf16); + const __le32 *sbuf32 = sbuf; + unsigned int x; + u32 val32; + u16 val16; + u32 pix[2]; + + for (x = 0; x < pixels2; x += 2, ++dbuf32) { + pix[0] = le32_to_cpu(sbuf32[x]); + pix[1] = le32_to_cpu(sbuf32[x + 1]); + val32 = ((pix[0] & 0xf0000000) >> 16) | + ((pix[0] & 0x00f00000) >> 12) | + ((pix[0] & 0x0000f000) >> 8) | + ((pix[0] & 0x000000f0) >> 4) | + ((pix[1] & 0xf0000000) >> 0) | + ((pix[1] & 0x00f00000) << 4) | + ((pix[1] & 0x0000f000) << 8) | + ((pix[1] & 0x000000f0) << 12); + *dbuf32 = cpu_to_le32(val32); + } + for (; x < pixels; x++) { + pix[0] = le32_to_cpu(sbuf32[x]); + val16 = ((pix[0] & 0xf0000000) >> 16) | + ((pix[0] & 0x00f00000) >> 12) | + ((pix[0] & 0x0000f000) >> 8) | + ((pix[0] & 0x000000f0) >> 4); + dbuf16[x] = cpu_to_le16(val16); + } +} + +/** + * drm_fb_argb8888_to_argb4444 - Convert ARGB8888 to ARGB4444 clip buffer + * @dst: Array of ARGB4444 destination buffers + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines + * within @dst; can be NULL if scanlines are stored next to each other. + * @src: Array of ARGB8888 source buffer + * @fb: DRM framebuffer + * @clip: Clip rectangle area to copy + * @state: Transform and conversion state + * + * This function copies parts of a framebuffer to display memory and converts + * the color format during the process. The parameters @dst, @dst_pitch and + * @src refer to arrays. Each array must have at least as many entries as + * there are planes in @fb's format. Each entry stores the value for the + * format's respective color plane at the same index. + * + * This function does not apply clipping on @dst (i.e. the destination is at the + * top-left corner). + * + * Drivers can use this function for ARGB4444 devices that don't support + * ARGB8888 natively. + */ +void drm_fb_argb8888_to_argb4444(struct iosys_map *dst, const unsigned int *dst_pitch, + const struct iosys_map *src, const struct drm_framebuffer *fb, + const struct drm_rect *clip, struct drm_format_conv_state *state) +{ + static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = { + 2, + }; + + drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, state, + drm_fb_argb8888_to_argb4444_line); +} +EXPORT_SYMBOL(drm_fb_argb8888_to_argb4444); + /** * drm_fb_blit - Copy parts of a framebuffer to display memory * @dst: Array of display-memory addresses to copy to diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 428d81afe215..a1347e47e9d5 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -110,6 +110,9 @@ void drm_fb_xrgb8888_to_argb2101010(struct iosys_map *dst, const unsigned int *d void drm_fb_xrgb8888_to_gray8(struct iosys_map *dst, const unsigned int *dst_pitch, const struct iosys_map *src, const struct drm_framebuffer *fb, const struct drm_rect *clip, struct drm_format_conv_state *state); +void drm_fb_argb8888_to_argb4444(struct iosys_map *dst, const unsigned int *dst_pitch, + const struct iosys_map *src, const struct drm_framebuffer *fb, + const struct drm_rect *clip, struct drm_format_conv_state *state); int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t dst_format, const struct iosys_map *src, const struct drm_framebuffer *fb, -- 2.51.0 From 19f4da84b695fa84b42de017ebe6018a0195425d Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Feb 2025 13:22:06 +0100 Subject: [PATCH 09/16] drm/ast: cursor: Add support for ARGB4444 Add support for cursor image data in ARGB4444 format. This is the hardware's native format and requires no conversion. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250217122336.230067-4-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 8ab006e6bc32..91da54a4d8a6 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -720,6 +720,7 @@ static void ast_set_cursor_enabled(struct ast_device *ast, bool enabled) } static const uint32_t ast_cursor_plane_formats[] = { + DRM_FORMAT_ARGB4444, DRM_FORMAT_ARGB8888, }; @@ -771,17 +772,28 @@ static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, */ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) { - u8 *argb4444 = ast_cursor_plane->argb4444; - struct iosys_map argb4444_dst[DRM_FORMAT_MAX_PLANES] = { - IOSYS_MAP_INIT_VADDR(argb4444), - }; - unsigned int argb4444_dst_pitch[DRM_FORMAT_MAX_PLANES] = { - AST_HWC_PITCH, - }; - - drm_fb_argb8888_to_argb4444(argb4444_dst, argb4444_dst_pitch, - shadow_plane_state->data, fb, &damage, - &shadow_plane_state->fmtcnv_state); + u8 *argb4444; + + switch (fb->format->format) { + case DRM_FORMAT_ARGB4444: + argb4444 = shadow_plane_state->data[0].vaddr; + break; + default: + argb4444 = ast_cursor_plane->argb4444; + { + struct iosys_map argb4444_dst[DRM_FORMAT_MAX_PLANES] = { + IOSYS_MAP_INIT_VADDR(argb4444), + }; + unsigned int argb4444_dst_pitch[DRM_FORMAT_MAX_PLANES] = { + AST_HWC_PITCH, + }; + + drm_fb_argb8888_to_argb4444(argb4444_dst, argb4444_dst_pitch, + shadow_plane_state->data, fb, &damage, + &shadow_plane_state->fmtcnv_state); + } + break; + } ast_set_cursor_image(ast, argb4444, fb->width, fb->height); ast_set_cursor_base(ast, dst_off); } -- 2.51.0 From e82e1a0c22d841f379b1c768469dcdaae650e443 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 17 Feb 2025 13:22:07 +0100 Subject: [PATCH 10/16] drm/ast: cursor: Move implementation to separate source file Move the cursor code into a separate source file for readability. No functional changes. v2: - include Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250217122336.230067-5-tzimmermann@suse.de --- drivers/gpu/drm/ast/Makefile | 1 + drivers/gpu/drm/ast/ast_cursor.c | 309 +++++++++++++++++++++++++++++++ drivers/gpu/drm/ast/ast_drv.h | 24 +-- drivers/gpu/drm/ast/ast_mode.c | 277 +-------------------------- 4 files changed, 330 insertions(+), 281 deletions(-) create mode 100644 drivers/gpu/drm/ast/ast_cursor.c diff --git a/drivers/gpu/drm/ast/Makefile b/drivers/gpu/drm/ast/Makefile index 3107ea9c7bf5..8d09ba5d5889 100644 --- a/drivers/gpu/drm/ast/Makefile +++ b/drivers/gpu/drm/ast/Makefile @@ -4,6 +4,7 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. ast-y := \ + ast_cursor.o \ ast_ddc.o \ ast_dp501.o \ ast_dp.o \ diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c new file mode 100644 index 000000000000..139ab00dee8f --- /dev/null +++ b/drivers/gpu/drm/ast/ast_cursor.c @@ -0,0 +1,309 @@ +// SPDX-License-Identifier: MIT +/* + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sub license, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE + * USE OR OTHER DEALINGS IN THE SOFTWARE. + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + */ + +#include +#include + +#include +#include +#include +#include +#include + +#include "ast_drv.h" + +/* + * Hardware cursor + */ + +/* define for signature structure */ +#define AST_HWC_SIGNATURE_CHECKSUM 0x00 +#define AST_HWC_SIGNATURE_SizeX 0x04 +#define AST_HWC_SIGNATURE_SizeY 0x08 +#define AST_HWC_SIGNATURE_X 0x0C +#define AST_HWC_SIGNATURE_Y 0x10 +#define AST_HWC_SIGNATURE_HOTSPOTX 0x14 +#define AST_HWC_SIGNATURE_HOTSPOTY 0x18 + +static u32 ast_cursor_calculate_checksum(const void *src, unsigned int width, unsigned int height) +{ + u32 csum = 0; + unsigned int one_pixel_copy = width & BIT(0); + unsigned int two_pixel_copy = width - one_pixel_copy; + unsigned int trailing_bytes = (AST_MAX_HWC_WIDTH - width) * sizeof(u16); + unsigned int x, y; + + for (y = 0; y < height; y++) { + for (x = 0; x < two_pixel_copy; x += 2) { + const u32 *src32 = (const u32 *)src; + + csum += *src32; + src += SZ_4; + } + if (one_pixel_copy) { + const u16 *src16 = (const u16 *)src; + + csum += *src16; + src += SZ_2; + } + src += trailing_bytes; + } + + return csum; +} + +static void ast_set_cursor_image(struct ast_device *ast, const u8 *src, + unsigned int width, unsigned int height) +{ + u8 __iomem *dst = ast->cursor_plane.base.vaddr; + u32 csum; + + csum = ast_cursor_calculate_checksum(src, width, height); + + /* write pixel data */ + memcpy_toio(dst, src, AST_HWC_SIZE); + + /* write checksum + signature */ + dst += AST_HWC_SIZE; + writel(csum, dst); + writel(width, dst + AST_HWC_SIGNATURE_SizeX); + writel(height, dst + AST_HWC_SIGNATURE_SizeY); + writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX); + writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY); +} + +static void ast_set_cursor_base(struct ast_device *ast, u64 address) +{ + u8 addr0 = (address >> 3) & 0xff; + u8 addr1 = (address >> 11) & 0xff; + u8 addr2 = (address >> 19) & 0xff; + + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc8, addr0); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc9, addr1); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xca, addr2); +} + +static void ast_set_cursor_location(struct ast_device *ast, u16 x, u16 y, + u8 x_offset, u8 y_offset) +{ + u8 x0 = (x & 0x00ff); + u8 x1 = (x & 0x0f00) >> 8; + u8 y0 = (y & 0x00ff); + u8 y1 = (y & 0x0700) >> 8; + + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc2, x_offset); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc3, y_offset); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc4, x0); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc5, x1); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc6, y0); + ast_set_index_reg(ast, AST_IO_VGACRI, 0xc7, y1); +} + +static void ast_set_cursor_enabled(struct ast_device *ast, bool enabled) +{ + static const u8 mask = (u8)~(AST_IO_VGACRCB_HWC_16BPP | + AST_IO_VGACRCB_HWC_ENABLED); + + u8 vgacrcb = AST_IO_VGACRCB_HWC_16BPP; + + if (enabled) + vgacrcb |= AST_IO_VGACRCB_HWC_ENABLED; + + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xcb, mask, vgacrcb); +} + +/* + * Cursor plane + */ + +static const uint32_t ast_cursor_plane_formats[] = { + DRM_FORMAT_ARGB4444, + DRM_FORMAT_ARGB8888, +}; + +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, + struct drm_atomic_state *state) +{ + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); + struct drm_framebuffer *new_fb = new_plane_state->fb; + struct drm_crtc_state *new_crtc_state = NULL; + int ret; + + if (new_plane_state->crtc) + new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc); + + ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state, + DRM_PLANE_NO_SCALING, + DRM_PLANE_NO_SCALING, + true, true); + if (ret || !new_plane_state->visible) + return ret; + + if (new_fb->width > AST_MAX_HWC_WIDTH || new_fb->height > AST_MAX_HWC_HEIGHT) + return -EINVAL; + + return 0; +} + +static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, + struct drm_atomic_state *state) +{ + struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane); + struct ast_plane *ast_plane = to_ast_plane(plane); + 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 ast_device *ast = to_ast_device(plane->dev); + struct drm_rect damage; + u64 dst_off = ast_plane->offset; + u8 __iomem *dst = ast_plane->vaddr; /* TODO: Use mapping abstraction properly */ + u8 __iomem *sig = dst + AST_HWC_SIZE; /* TODO: Use mapping abstraction properly */ + unsigned int offset_x, offset_y; + u16 x, y; + u8 x_offset, y_offset; + + /* + * Do data transfer to hardware buffer and point the scanout + * engine to the offset. + */ + + if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) { + u8 *argb4444; + + switch (fb->format->format) { + case DRM_FORMAT_ARGB4444: + argb4444 = shadow_plane_state->data[0].vaddr; + break; + default: + argb4444 = ast_cursor_plane->argb4444; + { + struct iosys_map argb4444_dst[DRM_FORMAT_MAX_PLANES] = { + IOSYS_MAP_INIT_VADDR(argb4444), + }; + unsigned int argb4444_dst_pitch[DRM_FORMAT_MAX_PLANES] = { + AST_HWC_PITCH, + }; + + drm_fb_argb8888_to_argb4444(argb4444_dst, argb4444_dst_pitch, + shadow_plane_state->data, fb, &damage, + &shadow_plane_state->fmtcnv_state); + } + break; + } + ast_set_cursor_image(ast, argb4444, fb->width, fb->height); + ast_set_cursor_base(ast, dst_off); + } + + /* + * Update location in HWC signature and registers. + */ + + writel(plane_state->crtc_x, sig + AST_HWC_SIGNATURE_X); + writel(plane_state->crtc_y, sig + AST_HWC_SIGNATURE_Y); + + offset_x = AST_MAX_HWC_WIDTH - fb->width; + offset_y = AST_MAX_HWC_HEIGHT - fb->height; + + if (plane_state->crtc_x < 0) { + x_offset = (-plane_state->crtc_x) + offset_x; + x = 0; + } else { + x_offset = offset_x; + x = plane_state->crtc_x; + } + if (plane_state->crtc_y < 0) { + y_offset = (-plane_state->crtc_y) + offset_y; + y = 0; + } else { + y_offset = offset_y; + y = plane_state->crtc_y; + } + + ast_set_cursor_location(ast, x, y, x_offset, y_offset); + + /* Dummy write to enable HWC and make the HW pick-up the changes. */ + ast_set_cursor_enabled(ast, true); +} + +static void ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane, + struct drm_atomic_state *state) +{ + struct ast_device *ast = to_ast_device(plane->dev); + + ast_set_cursor_enabled(ast, false); +} + +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = { + DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, + .atomic_check = ast_cursor_plane_helper_atomic_check, + .atomic_update = ast_cursor_plane_helper_atomic_update, + .atomic_disable = ast_cursor_plane_helper_atomic_disable, +}; + +static const struct drm_plane_funcs ast_cursor_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, +}; + +int ast_cursor_plane_init(struct ast_device *ast) +{ + struct drm_device *dev = &ast->base; + struct ast_cursor_plane *ast_cursor_plane = &ast->cursor_plane; + struct ast_plane *ast_plane = &ast_cursor_plane->base; + struct drm_plane *cursor_plane = &ast_plane->base; + size_t size; + void __iomem *vaddr; + u64 offset; + int ret; + + /* + * Allocate backing storage for cursors. The BOs are permanently + * pinned to the top end of the VRAM. + */ + + size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE); + + if (ast->vram_fb_available < size) + return -ENOMEM; + + vaddr = ast->vram + ast->vram_fb_available - size; + offset = ast->vram_fb_available - size; + + ret = ast_plane_init(dev, ast_plane, vaddr, offset, size, + 0x01, &ast_cursor_plane_funcs, + ast_cursor_plane_formats, ARRAY_SIZE(ast_cursor_plane_formats), + NULL, DRM_PLANE_TYPE_CURSOR); + if (ret) { + drm_err(dev, "ast_plane_init() failed: %d\n", ret); + return ret; + } + drm_plane_helper_add(cursor_plane, &ast_cursor_plane_helper_funcs); + drm_plane_enable_fb_damage_clips(cursor_plane); + + ast->vram_fb_available -= size; + + return 0; +} diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 973abd0cbd42..d2c2605d2728 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -118,15 +118,6 @@ enum ast_config_mode { #define AST_HWC_SIGNATURE_SIZE 32 -/* define for signature structure */ -#define AST_HWC_SIGNATURE_CHECKSUM 0x00 -#define AST_HWC_SIGNATURE_SizeX 0x04 -#define AST_HWC_SIGNATURE_SizeY 0x08 -#define AST_HWC_SIGNATURE_X 0x0C -#define AST_HWC_SIGNATURE_Y 0x10 -#define AST_HWC_SIGNATURE_HOTSPOTX 0x14 -#define AST_HWC_SIGNATURE_HOTSPOTY 0x18 - /* * Planes */ @@ -383,8 +374,6 @@ struct ast_crtc_state { #define to_ast_crtc_state(state) container_of(state, struct ast_crtc_state, base) -int ast_mode_config_init(struct ast_device *ast); - #define AST_MM_ALIGN_SHIFT 4 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1) @@ -450,6 +439,9 @@ void ast_patch_ahb_2500(void __iomem *regs); int ast_vga_output_init(struct ast_device *ast); int ast_sil164_output_init(struct ast_device *ast); +/* ast_cursor.c */ +int ast_cursor_plane_init(struct ast_device *ast); + /* ast dp501 */ bool ast_backup_fw(struct ast_device *ast, u8 *addr, u32 size); void ast_init_3rdtx(struct ast_device *ast); @@ -459,4 +451,14 @@ int ast_dp501_output_init(struct ast_device *ast); int ast_dp_launch(struct ast_device *ast); int ast_astdp_output_init(struct ast_device *ast); +/* ast_mode.c */ +int ast_mode_config_init(struct ast_device *ast); +int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane, + void __iomem *vaddr, u64 offset, unsigned long size, + uint32_t possible_crtcs, + const struct drm_plane_funcs *funcs, + const uint32_t *formats, unsigned int format_count, + const uint64_t *format_modifiers, + enum drm_plane_type type); + #endif diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 91da54a4d8a6..c3b950675485 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -438,13 +438,13 @@ static void ast_wait_for_vretrace(struct ast_device *ast) * Planes */ -static int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane, - void __iomem *vaddr, u64 offset, unsigned long size, - uint32_t possible_crtcs, - const struct drm_plane_funcs *funcs, - const uint32_t *formats, unsigned int format_count, - const uint64_t *format_modifiers, - enum drm_plane_type type) +int ast_plane_init(struct drm_device *dev, struct ast_plane *ast_plane, + void __iomem *vaddr, u64 offset, unsigned long size, + uint32_t possible_crtcs, + const struct drm_plane_funcs *funcs, + const uint32_t *formats, unsigned int format_count, + const uint64_t *format_modifiers, + enum drm_plane_type type) { struct drm_plane *plane = &ast_plane->base; @@ -628,269 +628,6 @@ static int ast_primary_plane_init(struct ast_device *ast) return 0; } -/* - * Cursor plane - */ - -static u32 ast_cursor_calculate_checksum(const void *src, unsigned int width, unsigned int height) -{ - u32 csum = 0; - unsigned int one_pixel_copy = width & BIT(0); - unsigned int two_pixel_copy = width - one_pixel_copy; - unsigned int trailing_bytes = (AST_MAX_HWC_WIDTH - width) * sizeof(u16); - unsigned int x, y; - - for (y = 0; y < height; y++) { - for (x = 0; x < two_pixel_copy; x += 2) { - const u32 *src32 = (const u32 *)src; - - csum += *src32; - src += SZ_4; - } - if (one_pixel_copy) { - const u16 *src16 = (const u16 *)src; - - csum += *src16; - src += SZ_2; - } - src += trailing_bytes; - } - - return csum; -} - -static void ast_set_cursor_image(struct ast_device *ast, const u8 *src, - unsigned int width, unsigned int height) -{ - u8 __iomem *dst = ast->cursor_plane.base.vaddr; - u32 csum; - - csum = ast_cursor_calculate_checksum(src, width, height); - - /* write pixel data */ - memcpy_toio(dst, src, AST_HWC_SIZE); - - /* write checksum + signature */ - dst += AST_HWC_SIZE; - writel(csum, dst); - writel(width, dst + AST_HWC_SIGNATURE_SizeX); - writel(height, dst + AST_HWC_SIGNATURE_SizeY); - writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX); - writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY); -} - -static void ast_set_cursor_base(struct ast_device *ast, u64 address) -{ - u8 addr0 = (address >> 3) & 0xff; - u8 addr1 = (address >> 11) & 0xff; - u8 addr2 = (address >> 19) & 0xff; - - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc8, addr0); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc9, addr1); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xca, addr2); -} - -static void ast_set_cursor_location(struct ast_device *ast, u16 x, u16 y, - u8 x_offset, u8 y_offset) -{ - u8 x0 = (x & 0x00ff); - u8 x1 = (x & 0x0f00) >> 8; - u8 y0 = (y & 0x00ff); - u8 y1 = (y & 0x0700) >> 8; - - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc2, x_offset); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc3, y_offset); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc4, x0); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc5, x1); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc6, y0); - ast_set_index_reg(ast, AST_IO_VGACRI, 0xc7, y1); -} - -static void ast_set_cursor_enabled(struct ast_device *ast, bool enabled) -{ - static const u8 mask = (u8)~(AST_IO_VGACRCB_HWC_16BPP | - AST_IO_VGACRCB_HWC_ENABLED); - - u8 vgacrcb = AST_IO_VGACRCB_HWC_16BPP; - - if (enabled) - vgacrcb |= AST_IO_VGACRCB_HWC_ENABLED; - - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xcb, mask, vgacrcb); -} - -static const uint32_t ast_cursor_plane_formats[] = { - DRM_FORMAT_ARGB4444, - DRM_FORMAT_ARGB8888, -}; - -static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, - struct drm_atomic_state *state) -{ - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); - struct drm_framebuffer *new_fb = new_plane_state->fb; - struct drm_crtc_state *new_crtc_state = NULL; - int ret; - - if (new_plane_state->crtc) - new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc); - - ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state, - DRM_PLANE_NO_SCALING, - DRM_PLANE_NO_SCALING, - true, true); - if (ret || !new_plane_state->visible) - return ret; - - if (new_fb->width > AST_MAX_HWC_WIDTH || new_fb->height > AST_MAX_HWC_HEIGHT) - return -EINVAL; - - return 0; -} - -static void ast_cursor_plane_helper_atomic_update(struct drm_plane *plane, - struct drm_atomic_state *state) -{ - struct ast_cursor_plane *ast_cursor_plane = to_ast_cursor_plane(plane); - struct ast_plane *ast_plane = to_ast_plane(plane); - 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 ast_device *ast = to_ast_device(plane->dev); - struct drm_rect damage; - u64 dst_off = ast_plane->offset; - u8 __iomem *dst = ast_plane->vaddr; /* TODO: Use mapping abstraction properly */ - u8 __iomem *sig = dst + AST_HWC_SIZE; /* TODO: Use mapping abstraction properly */ - unsigned int offset_x, offset_y; - u16 x, y; - u8 x_offset, y_offset; - - /* - * Do data transfer to hardware buffer and point the scanout - * engine to the offset. - */ - - if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &damage)) { - u8 *argb4444; - - switch (fb->format->format) { - case DRM_FORMAT_ARGB4444: - argb4444 = shadow_plane_state->data[0].vaddr; - break; - default: - argb4444 = ast_cursor_plane->argb4444; - { - struct iosys_map argb4444_dst[DRM_FORMAT_MAX_PLANES] = { - IOSYS_MAP_INIT_VADDR(argb4444), - }; - unsigned int argb4444_dst_pitch[DRM_FORMAT_MAX_PLANES] = { - AST_HWC_PITCH, - }; - - drm_fb_argb8888_to_argb4444(argb4444_dst, argb4444_dst_pitch, - shadow_plane_state->data, fb, &damage, - &shadow_plane_state->fmtcnv_state); - } - break; - } - ast_set_cursor_image(ast, argb4444, fb->width, fb->height); - ast_set_cursor_base(ast, dst_off); - } - - /* - * Update location in HWC signature and registers. - */ - - writel(plane_state->crtc_x, sig + AST_HWC_SIGNATURE_X); - writel(plane_state->crtc_y, sig + AST_HWC_SIGNATURE_Y); - - offset_x = AST_MAX_HWC_WIDTH - fb->width; - offset_y = AST_MAX_HWC_HEIGHT - fb->height; - - if (plane_state->crtc_x < 0) { - x_offset = (-plane_state->crtc_x) + offset_x; - x = 0; - } else { - x_offset = offset_x; - x = plane_state->crtc_x; - } - if (plane_state->crtc_y < 0) { - y_offset = (-plane_state->crtc_y) + offset_y; - y = 0; - } else { - y_offset = offset_y; - y = plane_state->crtc_y; - } - - ast_set_cursor_location(ast, x, y, x_offset, y_offset); - - /* Dummy write to enable HWC and make the HW pick-up the changes. */ - ast_set_cursor_enabled(ast, true); -} - -static void ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane, - struct drm_atomic_state *state) -{ - struct ast_device *ast = to_ast_device(plane->dev); - - ast_set_cursor_enabled(ast, false); -} - -static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = { - DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, - .atomic_check = ast_cursor_plane_helper_atomic_check, - .atomic_update = ast_cursor_plane_helper_atomic_update, - .atomic_disable = ast_cursor_plane_helper_atomic_disable, -}; - -static const struct drm_plane_funcs ast_cursor_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 int ast_cursor_plane_init(struct ast_device *ast) -{ - struct drm_device *dev = &ast->base; - struct ast_cursor_plane *ast_cursor_plane = &ast->cursor_plane; - struct ast_plane *ast_plane = &ast_cursor_plane->base; - struct drm_plane *cursor_plane = &ast_plane->base; - size_t size; - void __iomem *vaddr; - u64 offset; - int ret; - - /* - * Allocate backing storage for cursors. The BOs are permanently - * pinned to the top end of the VRAM. - */ - - size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE); - - if (ast->vram_fb_available < size) - return -ENOMEM; - - vaddr = ast->vram + ast->vram_fb_available - size; - offset = ast->vram_fb_available - size; - - ret = ast_plane_init(dev, ast_plane, vaddr, offset, size, - 0x01, &ast_cursor_plane_funcs, - ast_cursor_plane_formats, ARRAY_SIZE(ast_cursor_plane_formats), - NULL, DRM_PLANE_TYPE_CURSOR); - if (ret) { - drm_err(dev, "ast_plane_init() failed: %d\n", ret); - return ret; - } - drm_plane_helper_add(cursor_plane, &ast_cursor_plane_helper_funcs); - drm_plane_enable_fb_damage_clips(cursor_plane); - - ast->vram_fb_available -= size; - - return 0; -} - /* * CRTC */ -- 2.51.0 From ddd147d91d509c9d9fc6159efc5b56f61440bb9a Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Fri, 7 Feb 2025 18:35:22 +0100 Subject: [PATCH 11/16] drm: writeback: Fix kernel doc name During the creation of drmm_ variants for writeback connector, one function was renamed, but not the kernel doc. To remove the warning, use the proper name in kernel doc. Fixes: 135d8fc7af44 ("drm: writeback: Create an helper for drm_writeback_connector initialization") Reported-by: Stephen Rothwell Closes: https://lore.kernel.org/all/20250207142201.550ce870@canb.auug.org.au/ Reviewed-by: Dmitry Baryshkov Link: https://patchwork.freedesktop.org/patch/msgid/20250207-b4-fix-warning-v1-1-b4964beb60a3@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index f139b49af4c9..edbeab88ff2b 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -213,7 +213,7 @@ static void delete_writeback_properties(struct drm_device *dev) } /** - * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with + * __drm_writeback_connector_init - Initialize a writeback connector with * a custom encoder * * @dev: DRM device -- 2.51.0 From 4ce2c7e201c265df1c62a9190a98a98803208b8f Mon Sep 17 00:00:00 2001 From: John Keeping Date: Mon, 17 Feb 2025 12:04:28 +0000 Subject: [PATCH 12/16] drm/panel: ilitek-ili9882t: fix GPIO name in error message This driver uses the enable-gpios property and it is confusing that the error message refers to reset-gpios. Use the correct name when the enable GPIO is not found. Fixes: e2450d32e5fb5 ("drm/panel: ili9882t: Break out as separate driver") Signed-off-by: John Keeping Signed-off-by: Linus Walleij Link: https://patchwork.freedesktop.org/patch/msgid/20250217120428.3779197-1-jkeeping@inmusicbrands.com --- drivers/gpu/drm/panel/panel-ilitek-ili9882t.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c index 266a087fe14c..3c24a63b6be8 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c @@ -607,7 +607,7 @@ static int ili9882t_add(struct ili9882t *ili) ili->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW); if (IS_ERR(ili->enable_gpio)) { - dev_err(dev, "cannot get reset-gpios %ld\n", + dev_err(dev, "cannot get enable-gpios %ld\n", PTR_ERR(ili->enable_gpio)); return PTR_ERR(ili->enable_gpio); } -- 2.51.0 From 60341a6d79aa5e18a9c4ad8d7193e1ec6f8741b0 Mon Sep 17 00:00:00 2001 From: Herve Codina Date: Thu, 20 Feb 2025 15:04:06 +0100 Subject: [PATCH 13/16] drm/atomic-helper: Add a note in drm_atomic_helper_reset_crtc() kernel-doc As suggested in [0], add a note indicating that drm_atomic_helper_reset_crtc() can be a no-op in some cases. [0]:https://lore.kernel.org/all/Z7XfnPGDYspwG42y@phenom.ffwll.local/ Signed-off-by: Herve Codina Reviewed-by: Simona Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20250220140406.593314-1-herve.codina@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 7a25e70694ba..5302ab324898 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3409,6 +3409,10 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all); * This implies a reset of all active components available between the CRTC and * connectors. * + * NOTE: This relies on resetting &drm_crtc_state.connectors_changed. + * For drivers which optimize out unnecessary modesets this will result in + * a no-op commit, achieving nothing. + * * Returns: * 0 on success or a negative error code on failure. */ -- 2.51.0 From 27e21f22db9993769b5c983113ccffcac65d772a Mon Sep 17 00:00:00 2001 From: Jeff Hugo Date: Wed, 19 Feb 2025 14:41:12 -0700 Subject: [PATCH 14/16] MAINTAINERS: Update my email address Qualcomm is migrating away from quicinc.com email addresses towards ones with *.qualcomm.com. Signed-off-by: Jeff Hugo Reviewed-by: Bjorn Andersson Link: https://patchwork.freedesktop.org/patch/msgid/20250219214112.2168604-1-jeff.hugo@oss.qualcomm.com --- .mailmap | 3 ++- MAINTAINERS | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.mailmap b/.mailmap index ae0adc499f4a..f4b927e48ad1 100644 --- a/.mailmap +++ b/.mailmap @@ -320,7 +320,8 @@ Jeff Garzik Jeff Layton Jeff Layton Jeff Layton -Jeffrey Hugo +Jeff Hugo +Jeff Hugo Jens Axboe Jens Axboe Jens Axboe diff --git a/MAINTAINERS b/MAINTAINERS index 950e8b7c0805..815a28c7e6fc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19426,7 +19426,7 @@ F: drivers/clk/qcom/ F: include/dt-bindings/clock/qcom,* QUALCOMM CLOUD AI (QAIC) DRIVER -M: Jeffrey Hugo +M: Jeff Hugo R: Carl Vanderlip L: linux-arm-msm@vger.kernel.org L: dri-devel@lists.freedesktop.org -- 2.51.0 From acf3256160bdabcb5c07032f3bf6eb5a21f5b95f Mon Sep 17 00:00:00 2001 From: Jeffrey Hugo Date: Fri, 14 Feb 2025 09:21:09 -0700 Subject: [PATCH 15/16] bus: mhi: host: Avoid possible uninitialized fw_load_type If mhi_fw_load_handler() bails out early because the EE is not capable of loading firmware, we may reference fw_load_type in cleanup which is uninitialized at this point. The cleanup code checks fw_load_type as a proxy for knowing if fbc_image was allocated and needs to be freed, but we can directly test for that. This avoids the possible uninitialized access and appears to be clearer code. Reported-by: Dan Carpenter Closes: https://lore.kernel.org/all/e3148ac4-7bb8-422d-ae0f-18a8eb15e269@stanley.mountain/ Fixes: f88f1d0998ea ("bus: mhi: host: Add a policy to enable image transfer via BHIe in PBL") Signed-off-by: Jeffrey Hugo Acked-by: Manivannan Sadhasivam Reviewed-by: Carl Vanderlip Signed-off-by: Jeff Hugo Link: https://patchwork.freedesktop.org/patch/msgid/20250214162109.3555300-1-quic_jhugo@quicinc.com --- drivers/bus/mhi/host/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c index c8e48f621a8c..efa3b6dddf4d 100644 --- a/drivers/bus/mhi/host/boot.c +++ b/drivers/bus/mhi/host/boot.c @@ -608,7 +608,7 @@ fw_load_ready_state: return; error_ready_state: - if (fw_load_type == MHI_FW_LOAD_FBC) { + if (mhi_cntrl->fbc_image) { mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image); mhi_cntrl->fbc_image = NULL; } -- 2.51.0 From b6eb664d89e7ed1e3369fe2860fea31e6dc45e34 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Fri, 21 Feb 2025 10:50:33 +0000 Subject: [PATCH 16/16] drm/sched: Add internal job peek/pop API MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Idea is to add helpers for peeking and popping jobs from entities with the goal of decoupling the hidden assumption in the code that queue_node is the first element in struct drm_sched_job. That assumption usually comes in the form of: while ((job = to_drm_sched_job(spsc_queue_pop(&entity->job_queue)))) Which breaks if the queue_node is re-positioned due to_drm_sched_job being implemented with a container_of. This also allows us to remove duplicate definitions of to_drm_sched_job. Signed-off-by: Tvrtko Ursulin Cc: Christian König Cc: Danilo Krummrich Cc: Matthew Brost Cc: Philipp Stanner Signed-off-by: Philipp Stanner Link: https://patchwork.freedesktop.org/patch/msgid/20250221105038.79665-2-tvrtko.ursulin@igalia.com --- drivers/gpu/drm/scheduler/sched_entity.c | 11 +++-- drivers/gpu/drm/scheduler/sched_internal.h | 48 ++++++++++++++++++++++ drivers/gpu/drm/scheduler/sched_main.c | 7 ++-- 3 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 drivers/gpu/drm/scheduler/sched_internal.h diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 69bcf0e99d57..a171f05ad761 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -28,10 +28,9 @@ #include #include -#include "gpu_scheduler_trace.h" +#include "sched_internal.h" -#define to_drm_sched_job(sched_job) \ - container_of((sched_job), struct drm_sched_job, queue_node) +#include "gpu_scheduler_trace.h" /** * drm_sched_entity_init - Init a context entity used by scheduler when @@ -255,7 +254,7 @@ static void drm_sched_entity_kill(struct drm_sched_entity *entity) /* The entity is guaranteed to not be used by the scheduler */ prev = rcu_dereference_check(entity->last_scheduled, true); dma_fence_get(prev); - while ((job = to_drm_sched_job(spsc_queue_pop(&entity->job_queue)))) { + while ((job = drm_sched_entity_queue_pop(entity))) { struct drm_sched_fence *s_fence = job->s_fence; dma_fence_get(&s_fence->finished); @@ -477,7 +476,7 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) { struct drm_sched_job *sched_job; - sched_job = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); + sched_job = drm_sched_entity_queue_peek(entity); if (!sched_job) return NULL; @@ -513,7 +512,7 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) { struct drm_sched_job *next; - next = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); + next = drm_sched_entity_queue_peek(entity); if (next) { struct drm_sched_rq *rq; diff --git a/drivers/gpu/drm/scheduler/sched_internal.h b/drivers/gpu/drm/scheduler/sched_internal.h new file mode 100644 index 000000000000..bd34898911d7 --- /dev/null +++ b/drivers/gpu/drm/scheduler/sched_internal.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: MIT */ + +#ifndef _DRM_GPU_SCHEDULER_INTERNAL_H_ +#define _DRM_GPU_SCHEDULER_INTERNAL_H_ + +/** + * drm_sched_entity_queue_pop - Low level helper for popping queued jobs + * + * @entity: scheduler entity + * + * Low level helper for popping queued jobs. + * + * Returns: The job dequeued or NULL. + */ +static inline struct drm_sched_job * +drm_sched_entity_queue_pop(struct drm_sched_entity *entity) +{ + struct spsc_node *node; + + node = spsc_queue_pop(&entity->job_queue); + if (!node) + return NULL; + + return container_of(node, struct drm_sched_job, queue_node); +} + +/** + * drm_sched_entity_queue_peek - Low level helper for peeking at the job queue + * + * @entity: scheduler entity + * + * Low level helper for peeking at the job queue + * + * Returns: The job at the head of the queue or NULL. + */ +static inline struct drm_sched_job * +drm_sched_entity_queue_peek(struct drm_sched_entity *entity) +{ + struct spsc_node *node; + + node = spsc_queue_peek(&entity->job_queue); + if (!node) + return NULL; + + return container_of(node, struct drm_sched_job, queue_node); +} + +#endif diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 8c36a59afb72..c634993f1346 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -78,6 +78,8 @@ #include #include +#include "sched_internal.h" + #define CREATE_TRACE_POINTS #include "gpu_scheduler_trace.h" @@ -87,9 +89,6 @@ static struct lockdep_map drm_sched_lockdep_map = { }; #endif -#define to_drm_sched_job(sched_job) \ - container_of((sched_job), struct drm_sched_job, queue_node) - int drm_sched_policy = DRM_SCHED_POLICY_FIFO; /** @@ -123,7 +122,7 @@ static bool drm_sched_can_queue(struct drm_gpu_scheduler *sched, { struct drm_sched_job *s_job; - s_job = to_drm_sched_job(spsc_queue_peek(&entity->job_queue)); + s_job = drm_sched_entity_queue_peek(entity); if (!s_job) return false; -- 2.51.0