From e8941ac97f281ffa06d6ed058b3bba676f3f8183 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ma=C3=ADra=20Canal?= Date: Fri, 20 Dec 2024 10:37:09 -0300 Subject: [PATCH 01/16] drm/vc4: Remove BOs seqnos MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit `bo->seqno`, `bo->write_seqno`, and `exec->bin_dep_seqno` are leftovers from a time when VC4 didn't support DMA Reservation Objects. Before DMA Resv was introduced, tracking the correspondence between BOs and jobs through the job's seqno made sense. However, this is no longer needed, as VC4 now supports DMA Reservation Objects and attaches the "job done" fence to the BOs. Therefore, remove the BOs seqnos in favor of using DMA Resv Objects. Signed-off-by: Maíra Canal Reviewed-by: Maxime Ripard Reviewed-by: Melissa Wen Link: https://patchwork.freedesktop.org/patch/msgid/20241220134204.634577-4-mcanal@igalia.com --- drivers/gpu/drm/vc4/vc4_crtc.c | 33 ++++++++--------- drivers/gpu/drm/vc4/vc4_drv.h | 27 -------------- drivers/gpu/drm/vc4/vc4_gem.c | 59 +----------------------------- drivers/gpu/drm/vc4/vc4_validate.c | 11 ------ 4 files changed, 17 insertions(+), 113 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index cf40a53ad42e..2a48038abe7a 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -884,11 +884,7 @@ struct vc4_async_flip_state { struct drm_framebuffer *fb; struct drm_framebuffer *old_fb; struct drm_pending_vblank_event *event; - - union { - struct dma_fence_cb fence; - struct vc4_seqno_cb seqno; - } cb; + struct dma_fence_cb cb; }; /* Called when the V3D execution for the BO being flipped to is done, so that @@ -919,10 +915,11 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state) kfree(flip_state); } -static void vc4_async_page_flip_seqno_complete(struct vc4_seqno_cb *cb) +static void vc4_async_page_flip_complete_with_cleanup(struct dma_fence *fence, + struct dma_fence_cb *cb) { struct vc4_async_flip_state *flip_state = - container_of(cb, struct vc4_async_flip_state, cb.seqno); + container_of(cb, struct vc4_async_flip_state, cb); struct vc4_bo *bo = NULL; if (flip_state->old_fb) { @@ -932,6 +929,7 @@ static void vc4_async_page_flip_seqno_complete(struct vc4_seqno_cb *cb) } vc4_async_page_flip_complete(flip_state); + dma_fence_put(fence); /* * Decrement the BO usecnt in order to keep the inc/dec @@ -950,7 +948,7 @@ static void vc4_async_page_flip_fence_complete(struct dma_fence *fence, struct dma_fence_cb *cb) { struct vc4_async_flip_state *flip_state = - container_of(cb, struct vc4_async_flip_state, cb.fence); + container_of(cb, struct vc4_async_flip_state, cb); vc4_async_page_flip_complete(flip_state); dma_fence_put(fence); @@ -961,16 +959,15 @@ static int vc4_async_set_fence_cb(struct drm_device *dev, { struct drm_framebuffer *fb = flip_state->fb; struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0); + dma_fence_func_t async_page_flip_complete_function; struct vc4_dev *vc4 = to_vc4_dev(dev); struct dma_fence *fence; int ret; - if (vc4->gen == VC4_GEN_4) { - struct vc4_bo *bo = to_vc4_bo(&dma_bo->base); - - return vc4_queue_seqno_cb(dev, &flip_state->cb.seqno, bo->seqno, - vc4_async_page_flip_seqno_complete); - } + if (vc4->gen == VC4_GEN_4) + async_page_flip_complete_function = vc4_async_page_flip_complete_with_cleanup; + else + async_page_flip_complete_function = vc4_async_page_flip_fence_complete; ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence); if (ret) @@ -978,14 +975,14 @@ static int vc4_async_set_fence_cb(struct drm_device *dev, /* If there's no fence, complete the page flip immediately */ if (!fence) { - vc4_async_page_flip_fence_complete(fence, &flip_state->cb.fence); + async_page_flip_complete_function(fence, &flip_state->cb); return 0; } /* If the fence has already been completed, complete the page flip */ - if (dma_fence_add_callback(fence, &flip_state->cb.fence, - vc4_async_page_flip_fence_complete)) - vc4_async_page_flip_fence_complete(fence, &flip_state->cb.fence); + if (dma_fence_add_callback(fence, &flip_state->cb, + async_page_flip_complete_function)) + async_page_flip_complete_function(fence, &flip_state->cb); return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 4a078ffd9f82..221d8e01d539 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -186,11 +186,6 @@ struct vc4_dev { */ struct vc4_perfmon *active_perfmon; - /* List of struct vc4_seqno_cb for callbacks to be made from a - * workqueue when the given seqno is passed. - */ - struct list_head seqno_cb_list; - /* The memory used for storing binner tile alloc, tile state, * and overflow memory allocations. This is freed when V3D * powers down. @@ -247,16 +242,6 @@ struct vc4_dev { struct vc4_bo { struct drm_gem_dma_object base; - /* seqno of the last job to render using this BO. */ - uint64_t seqno; - - /* seqno of the last job to use the RCL to write to this BO. - * - * Note that this doesn't include binner overflow memory - * writes. - */ - uint64_t write_seqno; - bool t_format; /* List entry for the BO's position in either @@ -304,12 +289,6 @@ struct vc4_fence { #define to_vc4_fence(_fence) \ container_of_const(_fence, struct vc4_fence, base) -struct vc4_seqno_cb { - struct work_struct work; - uint64_t seqno; - void (*func)(struct vc4_seqno_cb *cb); -}; - struct vc4_v3d { struct vc4_dev *vc4; struct platform_device *pdev; @@ -695,9 +674,6 @@ struct vc4_exec_info { /* Sequence number for this bin/render job. */ uint64_t seqno; - /* Latest write_seqno of any BO that binning depends on. */ - uint64_t bin_dep_seqno; - struct dma_fence *fence; /* Last current addresses the hardware was processing when the @@ -1025,9 +1001,6 @@ void vc4_move_job_to_render(struct drm_device *dev, struct vc4_exec_info *exec); int vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, bool interruptible); void vc4_job_handle_completed(struct vc4_dev *vc4); -int vc4_queue_seqno_cb(struct drm_device *dev, - struct vc4_seqno_cb *cb, uint64_t seqno, - void (*func)(struct vc4_seqno_cb *cb)); int vc4_gem_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index de7be9942c13..8125f87edc60 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -553,27 +553,19 @@ vc4_move_job_to_render(struct drm_device *dev, struct vc4_exec_info *exec) } static void -vc4_update_bo_seqnos(struct vc4_exec_info *exec, uint64_t seqno) +vc4_attach_fences(struct vc4_exec_info *exec) { struct vc4_bo *bo; unsigned i; for (i = 0; i < exec->bo_count; i++) { bo = to_vc4_bo(exec->bo[i]); - bo->seqno = seqno; - dma_resv_add_fence(bo->base.base.resv, exec->fence, DMA_RESV_USAGE_READ); } - list_for_each_entry(bo, &exec->unref_list, unref_head) { - bo->seqno = seqno; - } - for (i = 0; i < exec->rcl_write_bo_count; i++) { bo = to_vc4_bo(&exec->rcl_write_bo[i]->base); - bo->write_seqno = seqno; - dma_resv_add_fence(bo->base.base.resv, exec->fence, DMA_RESV_USAGE_WRITE); } @@ -647,7 +639,7 @@ vc4_queue_submit(struct drm_device *dev, struct vc4_exec_info *exec, if (out_sync) drm_syncobj_replace_fence(out_sync, exec->fence); - vc4_update_bo_seqnos(exec, seqno); + vc4_attach_fences(exec); drm_exec_fini(exec_ctx); @@ -845,12 +837,6 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec) goto fail; } - /* Block waiting on any previous rendering into the CS's VBO, - * IB, or textures, so that pixels are actually written by the - * time we try to read them. - */ - ret = vc4_wait_for_seqno(dev, exec->bin_dep_seqno, ~0ull, true); - fail: kvfree(temp); return ret; @@ -909,7 +895,6 @@ void vc4_job_handle_completed(struct vc4_dev *vc4) { unsigned long irqflags; - struct vc4_seqno_cb *cb, *cb_temp; if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4)) return; @@ -926,48 +911,9 @@ vc4_job_handle_completed(struct vc4_dev *vc4) spin_lock_irqsave(&vc4->job_lock, irqflags); } - list_for_each_entry_safe(cb, cb_temp, &vc4->seqno_cb_list, work.entry) { - if (cb->seqno <= vc4->finished_seqno) { - list_del_init(&cb->work.entry); - schedule_work(&cb->work); - } - } - spin_unlock_irqrestore(&vc4->job_lock, irqflags); } -static void vc4_seqno_cb_work(struct work_struct *work) -{ - struct vc4_seqno_cb *cb = container_of(work, struct vc4_seqno_cb, work); - - cb->func(cb); -} - -int vc4_queue_seqno_cb(struct drm_device *dev, - struct vc4_seqno_cb *cb, uint64_t seqno, - void (*func)(struct vc4_seqno_cb *cb)) -{ - struct vc4_dev *vc4 = to_vc4_dev(dev); - unsigned long irqflags; - - if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4)) - return -ENODEV; - - cb->func = func; - INIT_WORK(&cb->work, vc4_seqno_cb_work); - - spin_lock_irqsave(&vc4->job_lock, irqflags); - if (seqno > vc4->finished_seqno) { - cb->seqno = seqno; - list_add_tail(&cb->work.entry, &vc4->seqno_cb_list); - } else { - schedule_work(&cb->work); - } - spin_unlock_irqrestore(&vc4->job_lock, irqflags); - - return 0; -} - /* Scheduled when any job has been completed, this walks the list of * jobs that had completed and unrefs their BOs and frees their exec * structs. @@ -1221,7 +1167,6 @@ int vc4_gem_init(struct drm_device *dev) INIT_LIST_HEAD(&vc4->bin_job_list); INIT_LIST_HEAD(&vc4->render_job_list); INIT_LIST_HEAD(&vc4->job_done_list); - INIT_LIST_HEAD(&vc4->seqno_cb_list); spin_lock_init(&vc4->job_lock); INIT_WORK(&vc4->hangcheck.reset_work, vc4_reset_work); diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c index 5bf134968ade..1e7bdda55698 100644 --- a/drivers/gpu/drm/vc4/vc4_validate.c +++ b/drivers/gpu/drm/vc4/vc4_validate.c @@ -284,9 +284,6 @@ validate_indexed_prim_list(VALIDATE_ARGS) if (!ib) return -EINVAL; - exec->bin_dep_seqno = max(exec->bin_dep_seqno, - to_vc4_bo(&ib->base)->write_seqno); - if (offset > ib->base.size || (ib->base.size - offset) / index_size < length) { DRM_DEBUG("IB access overflow (%d + %d*%d > %zd)\n", @@ -738,11 +735,6 @@ reloc_tex(struct vc4_exec_info *exec, *validated_p0 = tex->dma_addr + p0; - if (is_cs) { - exec->bin_dep_seqno = max(exec->bin_dep_seqno, - to_vc4_bo(&tex->base)->write_seqno); - } - return true; fail: DRM_INFO("Texture p0 at %d: 0x%08x\n", sample->p_offset[0], p0); @@ -904,9 +896,6 @@ validate_gl_shader_rec(struct drm_device *dev, uint32_t stride = *(uint8_t *)(pkt_u + o + 5); uint32_t max_index; - exec->bin_dep_seqno = max(exec->bin_dep_seqno, - to_vc4_bo(&vbo->base)->write_seqno); - if (state->addr & 0x8) stride |= (*(uint32_t *)(pkt_u + 100 + i * 4)) & ~0xff; -- 2.51.0 From 51678bb9a7fb25e44f38a4f0b1bd283fec809917 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Mon, 13 Jan 2025 10:33:40 +0000 Subject: [PATCH 02/16] drm/sched: Add helper to check job dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Lets isolate scheduler internals from drivers such as pvr which currently walks the dependency array to look for fences. Signed-off-by: Tvrtko Ursulin Cc: Christian König Cc: Danilo Krummrich Cc: Matthew Brost Cc: Philipp Stanner Reviewed-by: Matt Coster Acked-by: Danilo Krummrich Signed-off-by: Philipp Stanner Link: https://patchwork.freedesktop.org/patch/msgid/20250113103341.43914-1-tvrtko.ursulin@igalia.com --- drivers/gpu/drm/scheduler/sched_main.c | 23 +++++++++++++++++++++++ include/drm/gpu_scheduler.h | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index d172e3158880..a48be16ab84f 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -984,6 +984,29 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, } EXPORT_SYMBOL(drm_sched_job_add_implicit_dependencies); +/** + * drm_sched_job_has_dependency - check whether fence is the job's dependency + * @job: scheduler job to check + * @fence: fence to look for + * + * Returns: + * True if @fence is found within the job's dependencies, or otherwise false. + */ +bool drm_sched_job_has_dependency(struct drm_sched_job *job, + struct dma_fence *fence) +{ + struct dma_fence *f; + unsigned long index; + + xa_for_each(&job->dependencies, index, f) { + if (f == fence) + return true; + } + + return false; +} +EXPORT_SYMBOL(drm_sched_job_has_dependency); + /** * drm_sched_job_cleanup - clean up scheduler job resources * @job: scheduler job to clean up diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index e2e6af8849c6..a0ff08123f07 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -564,7 +564,8 @@ int drm_sched_job_add_resv_dependencies(struct drm_sched_job *job, int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, struct drm_gem_object *obj, bool write); - +bool drm_sched_job_has_dependency(struct drm_sched_job *job, + struct dma_fence *fence); void drm_sched_entity_modify_sched(struct drm_sched_entity *entity, struct drm_gpu_scheduler **sched_list, -- 2.51.0 From cdb73451bc675392c9c76779e2fec720edafd7ab Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Mon, 13 Jan 2025 10:33:41 +0000 Subject: [PATCH 03/16] drm/imagination: Use the drm_sched_job_has_dependency helper MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Instead of manually peeking into the DRM scheduler implementation details lets use the previously added helper. Signed-off-by: Tvrtko Ursulin Cc: Christian König Cc: Danilo Krummrich Cc: Matthew Brost Cc: Philipp Stanner Cc: Frank Binns Cc: Matt Coster Reviewed-by: Matt Coster Signed-off-by: Philipp Stanner Link: https://patchwork.freedesktop.org/patch/msgid/20250113103341.43914-2-tvrtko.ursulin@igalia.com --- drivers/gpu/drm/imagination/pvr_job.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/imagination/pvr_job.c b/drivers/gpu/drm/imagination/pvr_job.c index 618503a212a7..1cdb3cfd058d 100644 --- a/drivers/gpu/drm/imagination/pvr_job.c +++ b/drivers/gpu/drm/imagination/pvr_job.c @@ -597,8 +597,6 @@ update_job_resvs_for_each(struct pvr_job_data *job_data, u32 job_count) static bool can_combine_jobs(struct pvr_job *a, struct pvr_job *b) { struct pvr_job *geom_job = a, *frag_job = b; - struct dma_fence *fence; - unsigned long index; /* Geometry and fragment jobs can be combined if they are queued to the * same context and targeting the same HWRT. @@ -609,13 +607,9 @@ static bool can_combine_jobs(struct pvr_job *a, struct pvr_job *b) a->hwrt != b->hwrt) return false; - xa_for_each(&frag_job->base.dependencies, index, fence) { - /* We combine when we see an explicit geom -> frag dep. */ - if (&geom_job->base.s_fence->scheduled == fence) - return true; - } - - return false; + /* We combine when we see an explicit geom -> frag dep. */ + return drm_sched_job_has_dependency(&frag_job->base, + &geom_job->base.s_fence->scheduled); } static struct dma_fence * -- 2.51.0 From 0a3f3f7c5da8e45e1d4adf4ed6f4e2b05912785a Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:13 +0100 Subject: [PATCH 04/16] drm/vkms: Switch to managed for connector MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The current VKMS driver uses non-managed function to create connectors. It is not an issue yet, but in order to support multiple devices easily, convert this code to use drm and device managed helpers. Reviewed-by: Maxime Ripard Reviewed-by: Maíra Canal Reviewed-by: Thomas Zimmermann Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-1-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_output.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 8f4bd5aef087..570823ecb28f 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -3,11 +3,11 @@ #include "vkms_drv.h" #include #include +#include #include static const struct drm_connector_funcs vkms_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, @@ -75,8 +75,8 @@ int vkms_output_init(struct vkms_device *vkmsdev) } } - ret = drm_connector_init(dev, connector, &vkms_connector_funcs, - DRM_MODE_CONNECTOR_VIRTUAL); + ret = drmm_connector_init(dev, connector, &vkms_connector_funcs, + DRM_MODE_CONNECTOR_VIRTUAL, NULL); if (ret) { DRM_ERROR("Failed to init connector\n"); return ret; @@ -88,7 +88,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) { DRM_ERROR("Failed to init encoder\n"); - goto err_encoder; + return ret; } encoder->possible_crtcs = drm_crtc_mask(crtc); @@ -110,9 +110,5 @@ int vkms_output_init(struct vkms_device *vkmsdev) err_attach: drm_encoder_cleanup(encoder); - -err_encoder: - drm_connector_cleanup(connector); - return ret; } -- 2.51.0 From 16d22ba2debda22907b3eda4cc8ec1229136f424 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:14 +0100 Subject: [PATCH 05/16] drm/vkms: Switch to managed for encoder MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The current VKMS driver uses non-managed function to create encoders. It is not an issue yet, but in order to support multiple devices easily, convert this code to use drm and device managed helpers. Reviewed-by: Maxime Ripard Reviewed-by: Maíra Canal Reviewed-by: Thomas Zimmermann Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-2-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_output.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 570823ecb28f..ab9affa75b66 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -13,10 +13,6 @@ static const struct drm_connector_funcs vkms_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -static const struct drm_encoder_funcs vkms_encoder_funcs = { - .destroy = drm_encoder_cleanup, -}; - static int vkms_conn_get_modes(struct drm_connector *connector) { int count; @@ -84,8 +80,8 @@ int vkms_output_init(struct vkms_device *vkmsdev) drm_connector_helper_add(connector, &vkms_conn_helper_funcs); - ret = drm_encoder_init(dev, encoder, &vkms_encoder_funcs, - DRM_MODE_ENCODER_VIRTUAL, NULL); + ret = drmm_encoder_init(dev, encoder, NULL, + DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) { DRM_ERROR("Failed to init encoder\n"); return ret; @@ -95,7 +91,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) ret = drm_connector_attach_encoder(connector, encoder); if (ret) { DRM_ERROR("Failed to attach connector to encoder\n"); - goto err_attach; + return ret; } if (vkmsdev->config->writeback) { @@ -108,7 +104,5 @@ int vkms_output_init(struct vkms_device *vkmsdev) return 0; -err_attach: - drm_encoder_cleanup(encoder); return ret; } -- 2.51.0 From c367b772e6d89d8c7b560c7df7e3803ce6b8bcea Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:15 +0100 Subject: [PATCH 06/16] drm/managed: Add DRM-managed alloc_ordered_workqueue Add drmm_alloc_ordered_workqueue(), a helper that provides managed ordered workqueue cleanup. The workqueue will be destroyed with the final reference of the DRM device. Reviewed-by: Thomas Zimmermann Reviewed-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-3-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_managed.c | 8 ++++++++ include/drm/drm_managed.h | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c index 79ce86a5bd67..cc4c463daae7 100644 --- a/drivers/gpu/drm/drm_managed.c +++ b/drivers/gpu/drm/drm_managed.c @@ -310,3 +310,11 @@ void __drmm_mutex_release(struct drm_device *dev, void *res) mutex_destroy(lock); } EXPORT_SYMBOL(__drmm_mutex_release); + +void __drmm_workqueue_release(struct drm_device *device, void *res) +{ + struct workqueue_struct *wq = res; + + destroy_workqueue(wq); +} +EXPORT_SYMBOL(__drmm_workqueue_release); diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h index f547b09ca023..53017cc609ac 100644 --- a/include/drm/drm_managed.h +++ b/include/drm/drm_managed.h @@ -127,4 +127,16 @@ void __drmm_mutex_release(struct drm_device *dev, void *res); drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ }) \ +void __drmm_workqueue_release(struct drm_device *device, void *wq); + +#define drmm_alloc_ordered_workqueue(dev, fmt, flags, args...) \ + ({ \ + struct workqueue_struct *wq = alloc_ordered_workqueue(fmt, flags, ##args); \ + wq ? ({ \ + int ret = drmm_add_action_or_reset(dev, __drmm_workqueue_release, wq); \ + ret ? ERR_PTR(ret) : wq; \ + }) : \ + wq; \ + }) + #endif -- 2.51.0 From 8dd92e6eee590179818ba155cf02dc172e0c88c9 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:16 +0100 Subject: [PATCH 07/16] drm/vkms: Switch to managed for crtc MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The current VKMS driver uses managed function to create crtc, but don't use it to properly clean the crtc workqueue. It is not an issue yet, but in order to support multiple devices easily, convert this code to use drm and device managed helpers. Acked-by: Maxime Ripard Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-4-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_crtc.c | 5 ++++- drivers/gpu/drm/vkms/vkms_drv.c | 9 --------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 28a57ae109fc..434c35d5e947 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -296,7 +297,9 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->lock); spin_lock_init(&vkms_out->composer_lock); - vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0); + vkms_out->composer_workq = drmm_alloc_ordered_workqueue(dev, "vkms_composer", 0); + if (IS_ERR(vkms_out->composer_workq)) + return PTR_ERR(vkms_out->composer_workq); if (!vkms_out->composer_workq) return -ENOMEM; diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index e0409aba9349..7c142bfc3bd9 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -53,14 +53,6 @@ MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support"); DEFINE_DRM_GEM_FOPS(vkms_driver_fops); -static void vkms_release(struct drm_device *dev) -{ - struct vkms_device *vkms = drm_device_to_vkms_device(dev); - - if (vkms->output.composer_workq) - destroy_workqueue(vkms->output.composer_workq); -} - static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; @@ -108,7 +100,6 @@ static const struct drm_debugfs_info vkms_config_debugfs_list[] = { static const struct drm_driver vkms_driver = { .driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM, - .release = vkms_release, .fops = &vkms_driver_fops, DRM_GEM_SHMEM_DRIVER_OPS, DRM_FBDEV_SHMEM_DRIVER_OPS, -- 2.51.0 From 135d8fc7af44c52083e18ccb24d56383d301f741 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:17 +0100 Subject: [PATCH 08/16] drm: writeback: Create an helper for drm_writeback_connector initialization As the old drm and the new drmm variants of drm_writeback_connector requires almost the same initialization, create an internal helper to do most of the initialization work. Currently there is no cleanup function for writeback connectors. To allows implementation of drmm variant of writeback connector, create a cleanup function that can be used to properly remove all the writeback-specific properties and allocations. This also introduce an helper to cleanup only the drm_writeback_connector properties, so it can be used during initialization to cleanup in case of failure. Reviewed-by: Maxime Ripard Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-5-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_writeback.c | 87 +++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 33a3c98a962d..057af96dafeb 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -202,7 +203,6 @@ EXPORT_SYMBOL(drm_writeback_connector_init); * @dev: DRM device * @wb_connector: Writeback connector to initialize * @enc: handle to the already initialized drm encoder - * @con_funcs: Connector funcs vtable * @formats: Array of supported pixel formats for the writeback engine * @n_formats: Length of the formats array * @@ -218,41 +218,31 @@ EXPORT_SYMBOL(drm_writeback_connector_init); * assigning the encoder helper functions, possible_crtcs and any other encoder * specific operation. * - * Drivers should always use this function instead of drm_connector_init() to - * set up writeback connectors if they want to manage themselves the lifetime of the - * associated encoder. - * * Returns: 0 on success, or a negative error code */ -int drm_writeback_connector_init_with_encoder(struct drm_device *dev, - struct drm_writeback_connector *wb_connector, struct drm_encoder *enc, - const struct drm_connector_funcs *con_funcs, const u32 *formats, - int n_formats) +static int __drm_writeback_connector_init(struct drm_device *dev, + struct drm_writeback_connector *wb_connector, + struct drm_encoder *enc, const u32 *formats, + int n_formats) { - struct drm_property_blob *blob; struct drm_connector *connector = &wb_connector->base; struct drm_mode_config *config = &dev->mode_config; + struct drm_property_blob *blob; int ret = create_writeback_properties(dev); if (ret != 0) return ret; - blob = drm_property_create_blob(dev, n_formats * sizeof(*formats), - formats); - if (IS_ERR(blob)) - return PTR_ERR(blob); - - connector->interlace_allowed = 0; - ret = drm_connector_init(dev, connector, con_funcs, - DRM_MODE_CONNECTOR_WRITEBACK); - if (ret) - goto connector_fail; - ret = drm_connector_attach_encoder(connector, enc); if (ret) - goto attach_fail; + return ret; + + blob = drm_property_create_blob(dev, n_formats * sizeof(*formats), + formats); + if (IS_ERR(blob)) + return PTR_ERR(blob); INIT_LIST_HEAD(&wb_connector->job_queue); spin_lock_init(&wb_connector->job_lock); @@ -275,11 +265,56 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev, wb_connector->pixel_formats_blob_ptr = blob; return 0; +} + +/** + * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with + * a custom encoder + * + * @dev: DRM device + * @wb_connector: Writeback connector to initialize + * @enc: handle to the already initialized drm encoder + * @con_funcs: Connector funcs vtable + * @formats: Array of supported pixel formats for the writeback engine + * @n_formats: Length of the formats array + * + * This function creates the writeback-connector-specific properties if they + * have not been already created, initializes the connector as + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property + * values. + * + * This function assumes that the drm_writeback_connector's encoder has already been + * created and initialized before invoking this function. + * + * In addition, this function also assumes that callers of this API will manage + * assigning the encoder helper functions, possible_crtcs and any other encoder + * specific operation. + * + * Drivers should always use this function instead of drm_connector_init() to + * set up writeback connectors if they want to manage themselves the lifetime of the + * associated encoder. + * + * Returns: 0 on success, or a negative error code + */ +int drm_writeback_connector_init_with_encoder(struct drm_device *dev, + struct drm_writeback_connector *wb_connector, + struct drm_encoder *enc, + const struct drm_connector_funcs *con_funcs, + const u32 *formats, int n_formats) +{ + struct drm_connector *connector = &wb_connector->base; + int ret; + + ret = drm_connector_init(dev, connector, con_funcs, + DRM_MODE_CONNECTOR_WRITEBACK); + if (ret) + return ret; + + ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats, + n_formats); + if (ret) + drm_connector_cleanup(connector); -attach_fail: - drm_connector_cleanup(connector); -connector_fail: - drm_property_blob_put(blob); return ret; } EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder); -- 2.51.0 From 2f3f4a73631b160e44ab13d497f7be62264d47ac Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:18 +0100 Subject: [PATCH 09/16] drm: writeback: Add missing cleanup in case of initialization failure The current implementation of drm_writeback_connector initialization does not properly clean up all resources in case of failure (allocated properties and possible_encoders). Add this cleaning in case of failure. Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-6-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_writeback.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 057af96dafeb..4cdc6cdcf76b 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -196,6 +196,22 @@ int drm_writeback_connector_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_writeback_connector_init); +static void delete_writeback_properties(struct drm_device *dev) +{ + if (dev->mode_config.writeback_pixel_formats_property) { + drm_property_destroy(dev, dev->mode_config.writeback_pixel_formats_property); + dev->mode_config.writeback_pixel_formats_property = NULL; + } + if (dev->mode_config.writeback_out_fence_ptr_property) { + drm_property_destroy(dev, dev->mode_config.writeback_out_fence_ptr_property); + dev->mode_config.writeback_out_fence_ptr_property = NULL; + } + if (dev->mode_config.writeback_fb_id_property) { + drm_property_destroy(dev, dev->mode_config.writeback_fb_id_property); + dev->mode_config.writeback_fb_id_property = NULL; + } +} + /** * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with * a custom encoder @@ -231,18 +247,20 @@ static int __drm_writeback_connector_init(struct drm_device *dev, int ret = create_writeback_properties(dev); if (ret != 0) - return ret; + goto failed_properties; connector->interlace_allowed = 0; ret = drm_connector_attach_encoder(connector, enc); if (ret) - return ret; + goto failed_properties; blob = drm_property_create_blob(dev, n_formats * sizeof(*formats), formats); - if (IS_ERR(blob)) - return PTR_ERR(blob); + if (IS_ERR(blob)) { + ret = PTR_ERR(blob); + goto failed_properties; + } INIT_LIST_HEAD(&wb_connector->job_queue); spin_lock_init(&wb_connector->job_lock); @@ -265,6 +283,9 @@ static int __drm_writeback_connector_init(struct drm_device *dev, wb_connector->pixel_formats_blob_ptr = blob; return 0; +failed_properties: + delete_writeback_properties(dev); + return ret; } /** -- 2.51.0 From 1914ba2b91ea8eff674e2369f610bb6bb9056745 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:19 +0100 Subject: [PATCH 10/16] drm: writeback: Create drmm variants for drm_writeback_connector initialization To allows driver to only use drmm objects, add helper to create drm_writeback_connectors with automated lifetime management. Acked-by: Thomas Zimmermann Acked-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-7-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/drm_writeback.c | 74 +++++++++++++++++++++++++++++++++ include/drm/drm_writeback.h | 6 +++ 2 files changed, 80 insertions(+) diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 4cdc6cdcf76b..3628fbef7752 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -340,6 +340,80 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev, } EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder); +/** + * drm_writeback_connector_cleanup - Cleanup the writeback connector + * @dev: DRM device + * @wb_connector: Pointer to the writeback connector to clean up + * + * This will decrement the reference counter of blobs and destroy properties. It + * will also clean the remaining jobs in this writeback connector. Caution: This helper will not + * clean up the attached encoder and the drm_connector. + */ +static void drm_writeback_connector_cleanup(struct drm_device *dev, + struct drm_writeback_connector *wb_connector) +{ + unsigned long flags; + struct drm_writeback_job *pos, *n; + + delete_writeback_properties(dev); + drm_property_blob_put(wb_connector->pixel_formats_blob_ptr); + + spin_lock_irqsave(&wb_connector->job_lock, flags); + list_for_each_entry_safe(pos, n, &wb_connector->job_queue, list_entry) { + drm_writeback_cleanup_job(pos); + list_del(&pos->list_entry); + } + spin_unlock_irqrestore(&wb_connector->job_lock, flags); +} + +/** + * drmm_writeback_connector_init - Initialize a writeback connector with + * a custom encoder + * + * @dev: DRM device + * @wb_connector: Writeback connector to initialize + * @con_funcs: Connector funcs vtable + * @enc: Encoder to connect this writeback connector + * @formats: Array of supported pixel formats for the writeback engine + * @n_formats: Length of the formats array + * + * This function initialize a writeback connector and register its cleanup. + * + * This function creates the writeback-connector-specific properties if they + * have not been already created, initializes the connector as + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property + * values. + * + * Returns: 0 on success, or a negative error code + */ +int drmm_writeback_connector_init(struct drm_device *dev, + struct drm_writeback_connector *wb_connector, + const struct drm_connector_funcs *con_funcs, + struct drm_encoder *enc, + const u32 *formats, int n_formats) +{ + struct drm_connector *connector = &wb_connector->base; + int ret; + + ret = drmm_connector_init(dev, connector, con_funcs, + DRM_MODE_CONNECTOR_WRITEBACK, NULL); + if (ret) + return ret; + + ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats, + n_formats); + if (ret) + return ret; + + ret = drmm_add_action_or_reset(dev, (void *)drm_writeback_connector_cleanup, + wb_connector); + if (ret) + return ret; + + return 0; +} +EXPORT_SYMBOL(drmm_writeback_connector_init); + int drm_writeback_set_fb(struct drm_connector_state *conn_state, struct drm_framebuffer *fb) { diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 17e576c80169..c380a7b8f55a 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -161,6 +161,12 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev, const struct drm_connector_funcs *con_funcs, const u32 *formats, int n_formats); +int drmm_writeback_connector_init(struct drm_device *dev, + struct drm_writeback_connector *wb_connector, + const struct drm_connector_funcs *con_funcs, + struct drm_encoder *enc, + const u32 *formats, int n_formats); + int drm_writeback_set_fb(struct drm_connector_state *conn_state, struct drm_framebuffer *fb); -- 2.51.0 From 23fdf4308988b8aee2bb7cf8b77153f822d1fb3a Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Thu, 16 Jan 2025 18:47:20 +0100 Subject: [PATCH 11/16] drm/vkms: Switch to managed for writeback connector MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The current VKMS driver uses non-managed function to create writeback connectors. It is not an issue yet, but in order to support multiple devices easily, convert this code to use drm and device managed helpers. Acked-by: Thomas Zimmermann Acked-by: Maxime Ripard Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250116-google-vkms-managed-v9-8-3e4ae1bd05a0@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_drv.h | 3 ++- drivers/gpu/drm/vkms/vkms_output.c | 2 +- drivers/gpu/drm/vkms/vkms_writeback.c | 21 +++++++++++++-------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 00541eff3d1b..46ac36aebb27 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -179,6 +179,7 @@ struct vkms_output { struct drm_encoder encoder; struct drm_connector connector; struct drm_writeback_connector wb_connector; + struct drm_encoder wb_encoder; struct hrtimer vblank_hrtimer; ktime_t period_ns; struct workqueue_struct *composer_workq; @@ -275,6 +276,6 @@ void vkms_set_composer(struct vkms_output *out, bool enabled); void vkms_writeback_row(struct vkms_writeback_job *wb, const struct line_buffer *src_buffer, int y); /* Writeback */ -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev); +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc); #endif /* _VKMS_DRV_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index ab9affa75b66..de817e279486 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -95,7 +95,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) } if (vkmsdev->config->writeback) { - writeback = vkms_enable_writeback_connector(vkmsdev); + writeback = vkms_enable_writeback_connector(vkmsdev, crtc); if (writeback) DRM_ERROR("Failed to init writeback connector\n"); } diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c index 79918b44fedd..981975c2b0a0 100644 --- a/drivers/gpu/drm/vkms/vkms_writeback.c +++ b/drivers/gpu/drm/vkms/vkms_writeback.c @@ -24,7 +24,6 @@ static const u32 vkms_wb_formats[] = { static const struct drm_connector_funcs vkms_wb_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, @@ -163,16 +162,22 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = { .atomic_check = vkms_wb_atomic_check, }; -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev) +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc) { struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector; + int ret; + + ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder, + NULL, DRM_MODE_ENCODER_VIRTUAL, NULL); + if (ret) + return ret; + vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc); drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs); - return drm_writeback_connector_init(&vkmsdev->drm, wb, - &vkms_wb_connector_funcs, - NULL, - vkms_wb_formats, - ARRAY_SIZE(vkms_wb_formats), - 1); + return drmm_writeback_connector_init(&vkmsdev->drm, wb, + &vkms_wb_connector_funcs, + &vkmsdev->output.wb_encoder, + vkms_wb_formats, + ARRAY_SIZE(vkms_wb_formats)); } -- 2.51.0 From b0a76faea6b1492e480a69ef1a6cd19e30e7d60d Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Fri, 17 Jan 2025 10:04:27 +0100 Subject: [PATCH 12/16] drm/vkms: Switch to dynamic allocation for connector MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A specific allocation for the connector is not strictly necessary at this point, but in order to implement dynamic configuration of VKMS (configFS), it will be easier to have one allocation per connector. Reviewed-by: Maxime Ripard Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250117-b4-vkms-allocated-v4-1-8ec8fd21aaf6@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_drv.h | 1 - drivers/gpu/drm/vkms/vkms_output.c | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 46ac36aebb27..afa625457b61 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -177,7 +177,6 @@ struct vkms_crtc_state { struct vkms_output { struct drm_crtc crtc; struct drm_encoder encoder; - struct drm_connector connector; struct drm_writeback_connector wb_connector; struct drm_encoder wb_encoder; struct hrtimer vblank_hrtimer; diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index de817e279486..a41d7a29a377 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -32,7 +32,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) { struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; - struct drm_connector *connector = &output->connector; + struct drm_connector *connector; struct drm_encoder *encoder = &output->encoder; struct drm_crtc *crtc = &output->crtc; struct vkms_plane *primary, *overlay, *cursor = NULL; @@ -71,6 +71,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) } } + connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL); + if (!connector) { + DRM_ERROR("Failed to allocate connector\n"); + return -ENOMEM; + } + ret = drmm_connector_init(dev, connector, &vkms_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL, NULL); if (ret) { -- 2.51.0 From 45a4778415736fe4649a9fac2323091cdd710d86 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Fri, 17 Jan 2025 10:04:28 +0100 Subject: [PATCH 13/16] drm/vkms: Switch to dynamic allocation for encoder MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A specific allocation for the encoder is not strictly necessary at this point, but in order to implement dynamic configuration of VKMS (configFS), it will be easier to have one allocation per encoder. Reviewed-by: Maxime Ripard Reviewed-by: José Expósito Link: https://patchwork.freedesktop.org/patch/msgid/20250117-b4-vkms-allocated-v4-2-8ec8fd21aaf6@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_drv.h | 1 - drivers/gpu/drm/vkms/vkms_output.c | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index afa625457b61..333983bcf8d4 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -176,7 +176,6 @@ struct vkms_crtc_state { */ struct vkms_output { struct drm_crtc crtc; - struct drm_encoder encoder; struct drm_writeback_connector wb_connector; struct drm_encoder wb_encoder; struct hrtimer vblank_hrtimer; diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index a41d7a29a377..21ca975e424d 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -33,7 +33,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; struct drm_connector *connector; - struct drm_encoder *encoder = &output->encoder; + struct drm_encoder *encoder; struct drm_crtc *crtc = &output->crtc; struct vkms_plane *primary, *overlay, *cursor = NULL; int ret; @@ -86,6 +86,11 @@ int vkms_output_init(struct vkms_device *vkmsdev) drm_connector_helper_add(connector, &vkms_conn_helper_funcs); + encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL); + if (!encoder) { + DRM_ERROR("Failed to allocate encoder\n"); + return -ENOMEM; + } ret = drmm_encoder_init(dev, encoder, NULL, DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) { -- 2.51.0 From 49a167c393b0ceb592b9d2e65cc4f46bcc707108 Mon Sep 17 00:00:00 2001 From: Louis Chauvet Date: Fri, 17 Jan 2025 10:04:29 +0100 Subject: [PATCH 14/16] drm/vkms: Switch to dynamic allocation for CRTC A specific allocation for the CRTC is not strictly necessary at this point, but in order to implement dynamic configuration of VKMS (configFS), it will be easier to have one allocation per CRTC. Reviewed-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20250117-b4-vkms-allocated-v4-3-8ec8fd21aaf6@bootlin.com Signed-off-by: Louis Chauvet --- drivers/gpu/drm/vkms/vkms_crtc.c | 32 ++++++++++++++------------- drivers/gpu/drm/vkms/vkms_drv.h | 8 +++---- drivers/gpu/drm/vkms/vkms_output.c | 22 +++++++++--------- drivers/gpu/drm/vkms/vkms_writeback.c | 23 ++++++++++--------- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 434c35d5e947..cf229aec71c3 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -84,9 +84,7 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc, int *max_error, ktime_t *vblank_time, bool in_vblank_irq) { - struct drm_device *dev = crtc->dev; - struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev); - struct vkms_output *output = &vkmsdev->output; + struct vkms_output *output = drm_crtc_to_vkms_output(crtc); struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc); if (!READ_ONCE(vblank->enabled)) { @@ -271,25 +269,29 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = { .atomic_disable = vkms_crtc_atomic_disable, }; -int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, - struct drm_plane *primary, struct drm_plane *cursor) +struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary, + struct drm_plane *cursor) { - struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc); + struct vkms_output *vkms_out; + struct drm_crtc *crtc; int ret; - ret = drmm_crtc_init_with_planes(dev, crtc, primary, cursor, - &vkms_crtc_funcs, NULL); - if (ret) { - DRM_ERROR("Failed to init CRTC\n"); - return ret; + vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc, + primary, cursor, + &vkms_crtc_funcs, NULL); + if (IS_ERR(vkms_out)) { + DRM_DEV_ERROR(dev->dev, "Failed to init CRTC\n"); + return vkms_out; } + crtc = &vkms_out->crtc; + drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs); ret = drm_mode_crtc_set_gamma_size(crtc, VKMS_LUT_SIZE); if (ret) { DRM_ERROR("Failed to set gamma size\n"); - return ret; + return ERR_PTR(ret); } drm_crtc_enable_color_mgmt(crtc, 0, false, VKMS_LUT_SIZE); @@ -299,9 +301,9 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, vkms_out->composer_workq = drmm_alloc_ordered_workqueue(dev, "vkms_composer", 0); if (IS_ERR(vkms_out->composer_workq)) - return PTR_ERR(vkms_out->composer_workq); + return ERR_CAST(vkms_out->composer_workq); if (!vkms_out->composer_workq) - return -ENOMEM; + return ERR_PTR(-ENOMEM); - return ret; + return vkms_out; } diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 333983bcf8d4..abbb652be2b5 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -215,7 +215,6 @@ struct vkms_config { struct vkms_device { struct drm_device drm; struct platform_device *platform; - struct vkms_output output; const struct vkms_config *config; }; @@ -242,8 +241,9 @@ struct vkms_device { * @primary: primary plane to attach to the CRTC * @cursor: plane to attach to the CRTC */ -int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, - struct drm_plane *primary, struct drm_plane *cursor); +struct vkms_output *vkms_crtc_init(struct drm_device *dev, + struct drm_plane *primary, + struct drm_plane *cursor); /** * vkms_output_init() - Initialize all sub-components needed for a VKMS device. @@ -274,6 +274,6 @@ void vkms_set_composer(struct vkms_output *out, bool enabled); void vkms_writeback_row(struct vkms_writeback_job *wb, const struct line_buffer *src_buffer, int y); /* Writeback */ -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc); +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct vkms_output *vkms_out); #endif /* _VKMS_DRV_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 21ca975e424d..22f0d678af3a 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -30,11 +30,10 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { int vkms_output_init(struct vkms_device *vkmsdev) { - struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; struct drm_connector *connector; struct drm_encoder *encoder; - struct drm_crtc *crtc = &output->crtc; + struct vkms_output *output; struct vkms_plane *primary, *overlay, *cursor = NULL; int ret; int writeback; @@ -56,9 +55,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) return PTR_ERR(cursor); } - ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base); - if (ret) - return ret; + output = vkms_crtc_init(dev, &primary->base, + cursor ? &cursor->base : NULL); + if (IS_ERR(output)) { + DRM_ERROR("Failed to allocate CRTC\n"); + return PTR_ERR(output); + } if (vkmsdev->config->overlay) { for (n = 0; n < NUM_OVERLAY_PLANES; n++) { @@ -67,7 +69,7 @@ int vkms_output_init(struct vkms_device *vkmsdev) DRM_DEV_ERROR(dev->dev, "Failed to init vkms plane\n"); return PTR_ERR(overlay); } - overlay->base.possible_crtcs = drm_crtc_mask(crtc); + overlay->base.possible_crtcs = drm_crtc_mask(&output->crtc); } } @@ -97,23 +99,23 @@ int vkms_output_init(struct vkms_device *vkmsdev) DRM_ERROR("Failed to init encoder\n"); return ret; } - encoder->possible_crtcs = drm_crtc_mask(crtc); + encoder->possible_crtcs = drm_crtc_mask(&output->crtc); + /* Attach the encoder and the connector */ ret = drm_connector_attach_encoder(connector, encoder); if (ret) { DRM_ERROR("Failed to attach connector to encoder\n"); return ret; } + /* Initialize the writeback component */ if (vkmsdev->config->writeback) { - writeback = vkms_enable_writeback_connector(vkmsdev, crtc); + writeback = vkms_enable_writeback_connector(vkmsdev, output); if (writeback) DRM_ERROR("Failed to init writeback connector\n"); } drm_mode_config_reset(dev); - return 0; - return ret; } diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c index 981975c2b0a0..e9b5c74d7c58 100644 --- a/drivers/gpu/drm/vkms/vkms_writeback.c +++ b/drivers/gpu/drm/vkms/vkms_writeback.c @@ -105,7 +105,9 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector, struct drm_writeback_job *job) { struct vkms_writeback_job *vkmsjob = job->priv; - struct vkms_device *vkmsdev; + struct vkms_output *vkms_output = container_of(connector, + struct vkms_output, + wb_connector); if (!job->fb) return; @@ -114,8 +116,7 @@ static void vkms_wb_cleanup_job(struct drm_writeback_connector *connector, drm_framebuffer_put(vkmsjob->wb_frame_info.fb); - vkmsdev = drm_device_to_vkms_device(job->fb->dev); - vkms_set_composer(&vkmsdev->output, false); + vkms_set_composer(vkms_output, false); kfree(vkmsjob); } @@ -124,8 +125,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn, { struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state, conn); - struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev); - struct vkms_output *output = &vkmsdev->output; + struct vkms_output *output = drm_crtc_to_vkms_output(connector_state->crtc); struct drm_writeback_connector *wb_conn = &output->wb_connector; struct drm_connector_state *conn_state = wb_conn->base.state; struct vkms_crtc_state *crtc_state = output->composer_state; @@ -139,7 +139,7 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn, if (!conn_state) return; - vkms_set_composer(&vkmsdev->output, true); + vkms_set_composer(output, true); active_wb = conn_state->writeback_job->priv; wb_frame_info = &active_wb->wb_frame_info; @@ -162,22 +162,23 @@ static const struct drm_connector_helper_funcs vkms_wb_conn_helper_funcs = { .atomic_check = vkms_wb_atomic_check, }; -int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, struct drm_crtc *crtc) +int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, + struct vkms_output *vkms_output) { - struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector; + struct drm_writeback_connector *wb = &vkms_output->wb_connector; int ret; - ret = drmm_encoder_init(&vkmsdev->drm, &vkmsdev->output.wb_encoder, + ret = drmm_encoder_init(&vkmsdev->drm, &vkms_output->wb_encoder, NULL, DRM_MODE_ENCODER_VIRTUAL, NULL); if (ret) return ret; - vkmsdev->output.wb_encoder.possible_crtcs |= drm_crtc_mask(crtc); + vkms_output->wb_encoder.possible_crtcs |= drm_crtc_mask(&vkms_output->crtc); drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs); return drmm_writeback_connector_init(&vkmsdev->drm, wb, &vkms_wb_connector_funcs, - &vkmsdev->output.wb_encoder, + &vkms_output->wb_encoder, vkms_wb_formats, ARRAY_SIZE(vkms_wb_formats)); } -- 2.51.0 From 3d09b2718969f6db5b9e50daa9a033a78f065522 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Fri, 17 Jan 2025 11:29:06 +0100 Subject: [PATCH 15/16] drm/ast: Detect wide-screen support before creating modeset pipeline Wide-screen support is relevant for mode validation. Do not detect it before setting up the mode-setting pipeline. Gets the function call out of the way of other initialization code. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250117103450.28692-2-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index bc37c65305d4..037d389ab630 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -290,7 +290,6 @@ struct drm_device *ast_device_create(struct pci_dev *pdev, ast->regs = regs; ast->ioregs = ioregs; - ast_detect_widescreen(ast); ast_detect_tx_chip(ast, need_post); ret = ast_get_dram_info(ast); @@ -315,6 +314,8 @@ struct drm_device *ast_device_create(struct pci_dev *pdev, drm_info(dev, "failed to map reserved buffer!\n"); } + ast_detect_widescreen(ast); + ret = ast_mode_config_init(ast); if (ret) return ERR_PTR(ret); -- 2.51.0 From be1c00b180f1c580c93e585058e64df51fcfd4c2 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Fri, 17 Jan 2025 11:29:07 +0100 Subject: [PATCH 16/16] drm/ast: Detect DRAM before TX-chip Move DRAM detection before TX-chip detection. Both steps are independent from each other. Detection of the TX-chip is now next to posting those chips, which can be done in a single step. Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20250117103450.28692-3-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 037d389ab630..456230bef273 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -290,15 +290,13 @@ struct drm_device *ast_device_create(struct pci_dev *pdev, ast->regs = regs; ast->ioregs = ioregs; - ast_detect_tx_chip(ast, need_post); - ret = ast_get_dram_info(ast); if (ret) return ERR_PTR(ret); - drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n", ast->mclk, ast->dram_type, ast->dram_bus_width); + ast_detect_tx_chip(ast, need_post); if (need_post) ast_post_gpu(ast); -- 2.51.0