From 3367aa7d74d240261de2543ddb35531ccad9d884 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 11 May 2022 13:30:39 +0200 Subject: fbdev: Restart conflicting fb removal loop when unregistering devices Drivers that want to remove registered conflicting framebuffers prior to register their own framebuffer, call to remove_conflicting_framebuffers(). This function takes the registration_lock mutex, to prevent a race when drivers register framebuffer devices. But if a conflicting framebuffer device is found, the underlaying platform device is unregistered and this will lead to the platform driver .remove callback to be called. Which in turn will call to unregister_framebuffer() that takes the same lock. To prevent this, a struct fb_info.forced_out field was used as indication to unregister_framebuffer() whether the mutex has to be grabbed or not. But this could be unsafe, since the fbdev core is making assumptions about what drivers may or may not do in their .remove callbacks. Allowing to run these callbacks with the registration_lock held can cause deadlocks, since the fbdev core has no control over what drivers do in their removal path. A better solution is to drop the lock before platform_device_unregister(), so unregister_framebuffer() can take it when called from the fbdev driver. The lock is acquired again after the device has been unregistered and at this point the removal loop can be restarted. Since the conflicting framebuffer device has already been removed, the loop would just finish when no more conflicting framebuffers are found. Suggested-by: Daniel Vetter Signed-off-by: Javier Martinez Canillas Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20220511113039.1252432-1-javierm@redhat.com --- include/linux/fb.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/linux/fb.h b/include/linux/fb.h index 69c67c70fa78..bbe1e4571899 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -511,7 +511,6 @@ struct fb_info { } *apertures; bool skip_vt_switch; /* no VT switch on suspend/resume required */ - bool forced_out; /* set when being removed by another driver */ }; static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { -- cgit v1.2.3 From d9ba1b4cbb2989c919198832f4ebd6eb8ba94da0 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 9 May 2022 15:03:23 +0300 Subject: drm/displayid: convert to drm_edid We'll need to propagate drm_edid everywhere. v2: Rebase Signed-off-by: Jani Nikula Reviewed-by: Ankit Nautiyal Link: https://patchwork.freedesktop.org/patch/msgid/a52a6882e87a4bb6b1670918f3aba13f9b52f6de.1652097712.git.jani.nikula@intel.com --- drivers/gpu/drm/drm_displayid.c | 16 ++++++++-------- drivers/gpu/drm/drm_edid.c | 17 ++++++++++------- include/drm/drm_displayid.h | 6 +++--- include/drm/drm_edid.h | 6 ++++-- 4 files changed, 25 insertions(+), 20 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_displayid.c b/drivers/gpu/drm/drm_displayid.c index 32da557b960f..38ea8203df45 100644 --- a/drivers/gpu/drm/drm_displayid.c +++ b/drivers/gpu/drm/drm_displayid.c @@ -33,11 +33,11 @@ static int validate_displayid(const u8 *displayid, int length, int idx) return 0; } -static const u8 *drm_find_displayid_extension(const struct edid *edid, +static const u8 *drm_find_displayid_extension(const struct drm_edid *drm_edid, int *length, int *idx, int *ext_index) { - const u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index); + const u8 *displayid = drm_find_edid_extension(drm_edid, DISPLAYID_EXT, ext_index); const struct displayid_header *base; int ret; @@ -58,12 +58,12 @@ static const u8 *drm_find_displayid_extension(const struct edid *edid, return displayid; } -void displayid_iter_edid_begin(const struct edid *edid, +void displayid_iter_edid_begin(const struct drm_edid *drm_edid, struct displayid_iter *iter) { memset(iter, 0, sizeof(*iter)); - iter->edid = edid; + iter->drm_edid = drm_edid; } static const struct displayid_block * @@ -88,7 +88,7 @@ __displayid_iter_next(struct displayid_iter *iter) { const struct displayid_block *block; - if (!iter->edid) + if (!iter->drm_edid) return NULL; if (iter->section) { @@ -96,7 +96,7 @@ __displayid_iter_next(struct displayid_iter *iter) block = displayid_iter_block(iter); if (WARN_ON(!block)) { iter->section = NULL; - iter->edid = NULL; + iter->drm_edid = NULL; return NULL; } @@ -109,12 +109,12 @@ __displayid_iter_next(struct displayid_iter *iter) } for (;;) { - iter->section = drm_find_displayid_extension(iter->edid, + iter->section = drm_find_displayid_extension(iter->drm_edid, &iter->length, &iter->idx, &iter->ext_index); if (!iter->section) { - iter->edid = NULL; + iter->drm_edid = NULL; return NULL; } diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 30c33fdc52fc..99d9eec27ca8 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3563,10 +3563,13 @@ static int add_detailed_modes(struct drm_connector *connector, /* * Search EDID for CEA extension block. + * + * FIXME: Prefer not returning pointers to raw EDID data. */ -const u8 *drm_find_edid_extension(const struct edid *edid, +const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, int ext_id, int *ext_index) { + const struct edid *edid = drm_edid ? drm_edid->edid : NULL; const u8 *edid_ext = NULL; int i; @@ -3598,11 +3601,11 @@ static bool drm_edid_has_cta_extension(const struct drm_edid *drm_edid) bool found = false; /* Look for a top level CEA extension block */ - if (drm_find_edid_extension(drm_edid->edid, CEA_EXT, &ext_index)) + if (drm_find_edid_extension(drm_edid, CEA_EXT, &ext_index)) return true; /* CEA blocks can also be found embedded in a DisplayID block */ - displayid_iter_edid_begin(drm_edid->edid, &iter); + displayid_iter_edid_begin(drm_edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_CTA) { found = true; @@ -4454,7 +4457,7 @@ static void cea_db_iter_edid_begin(const struct drm_edid *drm_edid, memset(iter, 0, sizeof(*iter)); drm_edid_iter_begin(drm_edid, &iter->edid_iter); - displayid_iter_edid_begin(drm_edid ? drm_edid->edid : NULL, &iter->displayid_iter); + displayid_iter_edid_begin(drm_edid, &iter->displayid_iter); } static const struct cea_db * @@ -5657,7 +5660,7 @@ static void drm_update_mso(struct drm_connector *connector, const struct displayid_block *block; struct displayid_iter iter; - displayid_iter_edid_begin(drm_edid->edid, &iter); + displayid_iter_edid_begin(drm_edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_2_VENDOR_SPECIFIC) drm_parse_vesa_mso_data(connector, block); @@ -5872,7 +5875,7 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, struct displayid_iter iter; int num_modes = 0; - displayid_iter_edid_begin(drm_edid->edid, &iter); + displayid_iter_edid_begin(drm_edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_TYPE_1_DETAILED_TIMING || block->tag == DATA_BLOCK_2_TYPE_7_DETAILED_TIMING) @@ -6372,7 +6375,7 @@ static void _drm_update_tile_info(struct drm_connector *connector, connector->has_tile = false; - displayid_iter_edid_begin(drm_edid ? drm_edid->edid : NULL, &iter); + displayid_iter_edid_begin(drm_edid, &iter); displayid_iter_for_each(block, &iter) { if (block->tag == DATA_BLOCK_TILED_DISPLAY) drm_parse_tiled_block(connector, block); diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 7ffbd9f7bfc7..49649eb8447e 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -25,7 +25,7 @@ #include #include -struct edid; +struct drm_edid; #define VESA_IEEE_OUI 0x3a0292 @@ -141,7 +141,7 @@ struct displayid_vesa_vendor_specific_block { /* DisplayID iteration */ struct displayid_iter { - const struct edid *edid; + const struct drm_edid *drm_edid; const u8 *section; int length; @@ -149,7 +149,7 @@ struct displayid_iter { int ext_index; }; -void displayid_iter_edid_begin(const struct edid *edid, +void displayid_iter_edid_begin(const struct drm_edid *drm_edid, struct displayid_iter *iter); const struct displayid_block * __displayid_iter_next(struct displayid_iter *iter); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index c3204a58fb09..c61e75ab8f63 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -28,6 +28,7 @@ #include struct drm_device; +struct drm_edid; struct i2c_adapter; #define EDID_LENGTH 128 @@ -578,8 +579,9 @@ struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, struct drm_display_mode * drm_display_mode_from_cea_vic(struct drm_device *dev, u8 video_code); -const u8 *drm_find_edid_extension(const struct edid *edid, - int ext_id, int *ext_index); +/* Interface based on struct drm_edid */ +const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, + int ext_id, int *ext_index); #endif /* __DRM_EDID_H__ */ -- cgit v1.2.3 From 7d64c40a7d96190d9d06e240305389e025295916 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Tue, 12 Apr 2022 01:15:36 +0300 Subject: drm/scheduler: Don't kill jobs in interrupt context Interrupt context can't sleep. Drivers like Panfrost and MSM are taking mutex when job is released, and thus, that code can sleep. This results into "BUG: scheduling while atomic" if locks are contented while job is freed. There is no good reason for releasing scheduler's jobs in IRQ context, hence use normal context to fix the trouble. Cc: stable@vger.kernel.org Fixes: 542cff7893a3 ("drm/sched: Avoid lockdep spalt on killing a processes") Signed-off-by: Dmitry Osipenko Signed-off-by: Andrey Grodzovsky Link: https://patchwork.freedesktop.org/patch/msgid/20220411221536.283312-1-dmitry.osipenko@collabora.com --- drivers/gpu/drm/scheduler/sched_entity.c | 6 +++--- include/drm/gpu_scheduler.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 191c56064f19..6b25b2f4f5a3 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -190,7 +190,7 @@ long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout) } EXPORT_SYMBOL(drm_sched_entity_flush); -static void drm_sched_entity_kill_jobs_irq_work(struct irq_work *wrk) +static void drm_sched_entity_kill_jobs_work(struct work_struct *wrk) { struct drm_sched_job *job = container_of(wrk, typeof(*job), work); @@ -207,8 +207,8 @@ static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f, struct drm_sched_job *job = container_of(cb, struct drm_sched_job, finish_cb); - init_irq_work(&job->work, drm_sched_entity_kill_jobs_irq_work); - irq_work_queue(&job->work); + INIT_WORK(&job->work, drm_sched_entity_kill_jobs_work); + schedule_work(&job->work); } static struct dma_fence * diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 0fca8f38bee4..addb135eeea6 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -28,7 +28,7 @@ #include #include #include -#include +#include #define MAX_WAIT_SCHED_ENTITY_Q_EMPTY msecs_to_jiffies(1000) @@ -295,7 +295,7 @@ struct drm_sched_job { */ union { struct dma_fence_cb finish_cb; - struct irq_work work; + struct work_struct work; }; uint64_t id; -- cgit v1.2.3 From 16f1456466c269ecda32b88c3f8fdd76f8ec370c Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 16 May 2022 15:43:39 +0200 Subject: drm/mgag200: Implement connector's get_modes with helper Provide drm_connector_helper_get_modes_from_ddc() to implement the connector's get_modes callback. The new helper updates the connector from DDC-provided EDID data. v2: * clear property if EDID is NULL in helper Signed-off-by: Thomas Zimmermann Reviewed-by: Jocelyn Falempe Tested-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20220516134343.6085-4-tzimmermann@suse.de --- drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 17 ++++------------ include/drm/drm_probe_helper.h | 2 ++ 3 files changed, 42 insertions(+), 13 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 682359512996..d77f17867195 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -964,3 +964,39 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) return changed; } EXPORT_SYMBOL(drm_helper_hpd_irq_event); + +/** + * drm_connector_helper_get_modes_from_ddc - Updates the connector's EDID + * property from the connector's + * DDC channel + * @connector: The connector + * + * Returns: + * The number of detected display modes. + * + * Uses a connector's DDC channel to retrieve EDID data and update the + * connector's EDID property and display modes. Drivers can use this + * function to implement struct &drm_connector_helper_funcs.get_modes + * for connectors with a DDC channel. + */ +int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector) +{ + struct edid *edid; + int count = 0; + + if (!connector->ddc) + return 0; + + edid = drm_get_edid(connector, connector->ddc); + + // clears property if EDID is NULL + drm_connector_update_edid_property(connector, edid); + + if (edid) { + count = drm_add_edid_modes(connector, edid); + kfree(edid); + } + + return count; +} +EXPORT_SYMBOL(drm_connector_helper_get_modes_from_ddc); diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b227891d01ec..4c0680dd1a78 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -689,26 +689,17 @@ static void mgag200_disable_display(struct mga_device *mdev) * Connector */ -static int mga_vga_get_modes(struct drm_connector *connector) +static int mgag200_vga_connector_helper_get_modes(struct drm_connector *connector) { struct mga_device *mdev = to_mga_device(connector->dev); - struct mga_connector *mga_connector = to_mga_connector(connector); - struct edid *edid; - int ret = 0; + int ret; /* * Protect access to I/O registers from concurrent modesetting * by acquiring the I/O-register lock. */ mutex_lock(&mdev->rmmio_lock); - - edid = drm_get_edid(connector, &mga_connector->i2c->adapter); - if (edid) { - drm_connector_update_edid_property(connector, edid); - ret = drm_add_edid_modes(connector, edid); - kfree(edid); - } - + ret = drm_connector_helper_get_modes_from_ddc(connector); mutex_unlock(&mdev->rmmio_lock); return ret; @@ -828,7 +819,7 @@ static void mga_connector_destroy(struct drm_connector *connector) } static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = { - .get_modes = mga_vga_get_modes, + .get_modes = mgag200_vga_connector_helper_get_modes, .mode_valid = mga_vga_mode_valid, }; diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h index 48300aa6ca71..c80cab7a53b7 100644 --- a/include/drm/drm_probe_helper.h +++ b/include/drm/drm_probe_helper.h @@ -26,4 +26,6 @@ void drm_kms_helper_poll_disable(struct drm_device *dev); void drm_kms_helper_poll_enable(struct drm_device *dev); bool drm_kms_helper_is_poll_worker(void); +int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector); + #endif -- cgit v1.2.3 From 746b9c62cc8614fa59c23f3332682b5e9e1d801c Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Tue, 17 May 2022 13:33:24 +0200 Subject: drm/gem: Ignore color planes that are unused by framebuffer format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only handle color planes that exist in a framebuffer's color format. Ignore non-existing planes. So far, several helpers assumed that all 4 planes are available and silently ignored non-existing planes. This lead to subtil bugs with uninitialized data in instances of struct iosys_map. [1] Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Tested-by: Noralf Trønnes Acked-by: Christian König Link: https://lore.kernel.org/dri-devel/20210730183511.20080-1-tzimmermann@suse.de/T/#md0172b10bb588d8f20f4f456e304f08d2a4505f7 # 1 Link: https://patchwork.freedesktop.org/patch/msgid/20220517113327.26919-3-tzimmermann@suse.de --- drivers/gpu/drm/drm_gem_atomic_helper.c | 6 +++-- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 37 +++++++++++++++------------- include/drm/drm_gem_framebuffer_helper.h | 10 +++----- 3 files changed, 27 insertions(+), 26 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c index a5026f617739..f16d60217c6c 100644 --- a/drivers/gpu/drm/drm_gem_atomic_helper.c +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c @@ -169,8 +169,10 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_gem_object *obj = drm_gem_fb_get_obj(state->fb, i); struct dma_fence *new; - if (WARN_ON_ONCE(!obj)) - continue; + if (!obj) { + ret = -EINVAL; + goto error; + } ret = dma_resv_get_singleton(obj->resv, usage, &new); if (ret) diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index 931828784dfe..6d3c26efdeeb 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -92,9 +92,9 @@ drm_gem_fb_init(struct drm_device *dev, */ void drm_gem_fb_destroy(struct drm_framebuffer *fb) { - size_t i; + unsigned int i; - for (i = 0; i < ARRAY_SIZE(fb->obj); i++) + for (i = 0; i < fb->format->num_planes; i++) drm_gem_object_put(fb->obj[i]); drm_framebuffer_cleanup(fb); @@ -329,24 +329,26 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty); * The argument returns the addresses of the data stored in each BO. This * is different from @map if the framebuffer's offsets field is non-zero. * + * Both, @map and @data, must each refer to arrays with at least + * fb->format->num_planes elements. + * * See drm_gem_fb_vunmap() for unmapping. * * Returns: * 0 on success, or a negative errno code otherwise. */ -int drm_gem_fb_vmap(struct drm_framebuffer *fb, - struct iosys_map map[static DRM_FORMAT_MAX_PLANES], - struct iosys_map data[DRM_FORMAT_MAX_PLANES]) +int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, + struct iosys_map *data) { struct drm_gem_object *obj; unsigned int i; int ret; - for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) { + for (i = 0; i < fb->format->num_planes; ++i) { obj = drm_gem_fb_get_obj(fb, i); if (!obj) { - iosys_map_clear(&map[i]); - continue; + ret = -EINVAL; + goto err_drm_gem_vunmap; } ret = drm_gem_vmap(obj, &map[i]); if (ret) @@ -354,7 +356,7 @@ int drm_gem_fb_vmap(struct drm_framebuffer *fb, } if (data) { - for (i = 0; i < DRM_FORMAT_MAX_PLANES; ++i) { + for (i = 0; i < fb->format->num_planes; ++i) { memcpy(&data[i], &map[i], sizeof(data[i])); if (iosys_map_is_null(&data[i])) continue; @@ -385,10 +387,9 @@ EXPORT_SYMBOL(drm_gem_fb_vmap); * * See drm_gem_fb_vmap() for more information. */ -void drm_gem_fb_vunmap(struct drm_framebuffer *fb, - struct iosys_map map[static DRM_FORMAT_MAX_PLANES]) +void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct iosys_map *map) { - unsigned int i = DRM_FORMAT_MAX_PLANES; + unsigned int i = fb->format->num_planes; struct drm_gem_object *obj; while (i) { @@ -443,13 +444,15 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direct { struct dma_buf_attachment *import_attach; struct drm_gem_object *obj; - size_t i; + unsigned int i; int ret; - for (i = 0; i < ARRAY_SIZE(fb->obj); ++i) { + for (i = 0; i < fb->format->num_planes; ++i) { obj = drm_gem_fb_get_obj(fb, i); - if (!obj) - continue; + if (!obj) { + ret = -EINVAL; + goto err___drm_gem_fb_end_cpu_access; + } import_attach = obj->import_attach; if (!import_attach) continue; @@ -479,7 +482,7 @@ EXPORT_SYMBOL(drm_gem_fb_begin_cpu_access); */ void drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir) { - __drm_gem_fb_end_cpu_access(fb, dir, ARRAY_SIZE(fb->obj)); + __drm_gem_fb_end_cpu_access(fb, dir, fb->format->num_planes); } EXPORT_SYMBOL(drm_gem_fb_end_cpu_access); diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h index 1091e4fa08cb..d302521f3dd4 100644 --- a/include/drm/drm_gem_framebuffer_helper.h +++ b/include/drm/drm_gem_framebuffer_helper.h @@ -4,8 +4,6 @@ #include #include -#include - struct drm_afbc_framebuffer; struct drm_device; struct drm_fb_helper_surface_size; @@ -39,11 +37,9 @@ struct drm_framebuffer * drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file, const struct drm_mode_fb_cmd2 *mode_cmd); -int drm_gem_fb_vmap(struct drm_framebuffer *fb, - struct iosys_map map[static DRM_FORMAT_MAX_PLANES], - struct iosys_map data[DRM_FORMAT_MAX_PLANES]); -void drm_gem_fb_vunmap(struct drm_framebuffer *fb, - struct iosys_map map[static DRM_FORMAT_MAX_PLANES]); +int drm_gem_fb_vmap(struct drm_framebuffer *fb, struct iosys_map *map, + struct iosys_map *data); +void drm_gem_fb_vunmap(struct drm_framebuffer *fb, struct iosys_map *map); int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir); void drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir); -- cgit v1.2.3 From 69ef4a192bba0d76216198ec6d5fe82375337903 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 9 May 2022 16:18:09 -0700 Subject: drm: Document the power requirements for DP AUX transfers When doing DP AUX transfers there are two actors that need to be powered in order for the DP AUX transfer to work: the DP source and the DP sink. Commit bacbab58f09d ("drm: Mention the power state requirement on side-channel operations") added some documentation saying that the DP source is required to power itself up (if needed) to do AUX transfers. However, that commit doesn't talk anything about the DP sink. For full fledged DP the sink isn't really a problem. It's expected that if an external DP monitor isn't plugged in that attempting to do AUX transfers won't work. It's also expected that if a DP monitor is plugged in (and thus asserting HPD) then AUX transfers will work. When we're looking at eDP, however, things are less obvious. Let's add some documentation about expectations. Here's what we'll say: 1. We don't expect the DP AUX transfer function to power on an eDP panel. If an eDP panel is physically connected but powered off then it makes sense for the transfer to fail. 2. We'll document that the official way to power on a panel is via the bridge chain, specifically by making sure that the panel's prepare function has been called (which is called by panel_bridge_pre_enable()). It's already specified in the kernel doc of drm_panel_prepare() that this is the way to power the panel on and also that after this call "it is possible to communicate with any integrated circuitry via a command bus." 3. We'll also document that for code running in the panel driver itself that it is legal for the panel driver to power itself up however it wants (it doesn't need to officially call drm_panel_pre_enable()) and then it can do AUX bus transfers. This is currently the way that edp-panel works when it's running atop the DP AUX bus. NOTE: there was much discussion of all of this in response to v1 [1] of this patch. A summary of that is: * With the Intel i195 driver, apparently eDP panels do get powered up. We won't forbid this but it is expected that code that wants to run on a variety of platforms should ensure that the drm_panel's prepare() function has been called. * There is at least a reasonable amount of agreement that the transfer() functions itself shouldn't be responsible for powering the panel. It's proposed that if we need the DP AUX dev nodes to be robust for eDP that the code handling the DP AUX dev nodes could handle powering the panel by ensuring that the panel's prepare() call was made. Potentially drm_dp_aux_dev_get_by_minor() could be a good place to do this. This is left as a future exercise. Until that's fixed the DP AUX dev nodes for eDP are probably best just used for debugging. * If a panel could be in PSR and DP AUX via the dev node needs to be reliable then we need to be able to pull the panel out of PSR. On i915 this is also apparently handled as part of the transfer() function. [1] https://lore.kernel.org/r/20220503162033.1.Ia8651894026707e4fa61267da944ff739610d180@changeid Signed-off-by: Douglas Anderson Reviewed-by: Dmitry Baryshkov Reviewed-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20220509161733.v2.1.Ia8651894026707e4fa61267da944ff739610d180@changeid --- include/drm/display/drm_dp_helper.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index dca40a045dd6..dc3c02225fcf 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -370,9 +370,19 @@ struct drm_dp_aux { * helpers assume this is the case. * * Also note that this callback can be called no matter the - * state @dev is in. Drivers that need that device to be powered - * to perform this operation will first need to make sure it's - * been properly enabled. + * state @dev is in and also no matter what state the panel is + * in. It's expected: + * - If the @dev providing the AUX bus is currently unpowered then + * it will power itself up for the transfer. + * - If we're on eDP (using a drm_panel) and the panel is not in a + * state where it can respond (it's not powered or it's in a + * low power state) then this function may return an error, but + * not crash. It's up to the caller of this code to make sure that + * the panel is powered on if getting an error back is not OK. If a + * drm_panel driver is initiating a DP AUX transfer it may power + * itself up however it wants. All other code should ensure that + * the pre_enable() bridge chain (which eventually calls the + * drm_panel prepare function) has powered the panel. */ ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg); -- cgit v1.2.3 From ade1fc91eb99614c7155fec762ad5761bb470e06 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Tue, 10 May 2022 13:42:41 +0300 Subject: drm/edid: Extract drm_edid_decode_mfg_id() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the PNPID decoding available for other users. Cc: dri-devel@lists.freedesktop.org Reviewed-by: Jani Nikula Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20220510104242.6099-15-ville.syrjala@linux.intel.com --- include/drm/drm_edid.h | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index c61e75ab8f63..1d5950b5b407 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -497,6 +497,22 @@ static inline u8 drm_eld_get_conn_type(const uint8_t *eld) return eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK; } +/** + * drm_edid_decode_mfg_id - Decode the manufacturer ID + * @mfg_id: The manufacturer ID + * @vend: A 4-byte buffer to store the 3-letter vendor string plus a '\0' + * termination + */ +static inline const char *drm_edid_decode_mfg_id(u16 mfg_id, char vend[4]) +{ + vend[0] = '@' + ((mfg_id >> 10) & 0x1f); + vend[1] = '@' + ((mfg_id >> 5) & 0x1f); + vend[2] = '@' + ((mfg_id >> 0) & 0x1f); + vend[3] = '\0'; + + return vend; +} + /** * drm_edid_encode_panel_id - Encode an ID for matching against drm_edid_get_panel_id() * @vend_chr_0: First character of the vendor string. @@ -537,10 +553,7 @@ static inline u8 drm_eld_get_conn_type(const uint8_t *eld) static inline void drm_edid_decode_panel_id(u32 panel_id, char vend[4], u16 *product_id) { *product_id = (u16)(panel_id & 0xffff); - vend[0] = '@' + ((panel_id >> 26) & 0x1f); - vend[1] = '@' + ((panel_id >> 21) & 0x1f); - vend[2] = '@' + ((panel_id >> 16) & 0x1f); - vend[3] = '\0'; + drm_edid_decode_mfg_id(panel_id >> 16, vend); } bool drm_probe_ddc(struct i2c_adapter *adapter); -- cgit v1.2.3 From 01357a5a45ed8eb9543183f5c9c6713ae60fc1f3 Mon Sep 17 00:00:00 2001 From: Christian König Date: Sun, 24 Apr 2022 16:55:14 +0200 Subject: dma-buf: cleanup dma_fence_unwrap implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the code from the inline functions into exported functions. Signed-off-by: Christian König Acked-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20220518135844.3338-3-christian.koenig@amd.com --- drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/dma-fence-unwrap.c | 59 ++++++++++++++++++++++++++++++++++++++ include/linux/dma-fence-unwrap.h | 52 +++------------------------------ 3 files changed, 64 insertions(+), 49 deletions(-) create mode 100644 drivers/dma-buf/dma-fence-unwrap.c (limited to 'include') diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 4c9eb53ba3f8..70ec901edf2c 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ - dma-resv.o + dma-fence-unwrap.o dma-resv.o obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o obj-$(CONFIG_DMABUF_HEAPS) += heaps/ obj-$(CONFIG_SYNC_FILE) += sync_file.o diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c new file mode 100644 index 000000000000..711be125428c --- /dev/null +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * dma-fence-util: misc functions for dma_fence objects + * + * Copyright (C) 2022 Advanced Micro Devices, Inc. + * Authors: + * Christian König + */ + +#include +#include +#include +#include + +/* Internal helper to start new array iteration, don't use directly */ +static struct dma_fence * +__dma_fence_unwrap_array(struct dma_fence_unwrap *cursor) +{ + cursor->array = dma_fence_chain_contained(cursor->chain); + cursor->index = 0; + return dma_fence_array_first(cursor->array); +} + +/** + * dma_fence_unwrap_first - return the first fence from fence containers + * @head: the entrypoint into the containers + * @cursor: current position inside the containers + * + * Unwraps potential dma_fence_chain/dma_fence_array containers and return the + * first fence. + */ +struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head, + struct dma_fence_unwrap *cursor) +{ + cursor->chain = dma_fence_get(head); + return __dma_fence_unwrap_array(cursor); +} +EXPORT_SYMBOL_GPL(dma_fence_unwrap_first); + +/** + * dma_fence_unwrap_next - return the next fence from a fence containers + * @cursor: current position inside the containers + * + * Continue unwrapping the dma_fence_chain/dma_fence_array containers and return + * the next fence from them. + */ +struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor) +{ + struct dma_fence *tmp; + + ++cursor->index; + tmp = dma_fence_array_next(cursor->array, cursor->index); + if (tmp) + return tmp; + + cursor->chain = dma_fence_chain_walk(cursor->chain); + return __dma_fence_unwrap_array(cursor); +} +EXPORT_SYMBOL_GPL(dma_fence_unwrap_next); diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h index 77e335a1bcac..e7c219da4ed7 100644 --- a/include/linux/dma-fence-unwrap.h +++ b/include/linux/dma-fence-unwrap.h @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * fence-chain: chain fences together in a timeline - * * Copyright (C) 2022 Advanced Micro Devices, Inc. * Authors: * Christian König @@ -10,8 +8,7 @@ #ifndef __LINUX_DMA_FENCE_UNWRAP_H #define __LINUX_DMA_FENCE_UNWRAP_H -#include -#include +struct dma_fence; /** * struct dma_fence_unwrap - cursor into the container structure @@ -33,50 +30,9 @@ struct dma_fence_unwrap { unsigned int index; }; -/* Internal helper to start new array iteration, don't use directly */ -static inline struct dma_fence * -__dma_fence_unwrap_array(struct dma_fence_unwrap * cursor) -{ - cursor->array = dma_fence_chain_contained(cursor->chain); - cursor->index = 0; - return dma_fence_array_first(cursor->array); -} - -/** - * dma_fence_unwrap_first - return the first fence from fence containers - * @head: the entrypoint into the containers - * @cursor: current position inside the containers - * - * Unwraps potential dma_fence_chain/dma_fence_array containers and return the - * first fence. - */ -static inline struct dma_fence * -dma_fence_unwrap_first(struct dma_fence *head, struct dma_fence_unwrap *cursor) -{ - cursor->chain = dma_fence_get(head); - return __dma_fence_unwrap_array(cursor); -} - -/** - * dma_fence_unwrap_next - return the next fence from a fence containers - * @cursor: current position inside the containers - * - * Continue unwrapping the dma_fence_chain/dma_fence_array containers and return - * the next fence from them. - */ -static inline struct dma_fence * -dma_fence_unwrap_next(struct dma_fence_unwrap *cursor) -{ - struct dma_fence *tmp; - - ++cursor->index; - tmp = dma_fence_array_next(cursor->array, cursor->index); - if (tmp) - return tmp; - - cursor->chain = dma_fence_chain_walk(cursor->chain); - return __dma_fence_unwrap_array(cursor); -} +struct dma_fence *dma_fence_unwrap_first(struct dma_fence *head, + struct dma_fence_unwrap *cursor); +struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor); /** * dma_fence_unwrap_for_each - iterate over all fences in containers -- cgit v1.2.3 From 8f61973718485f3e89bc4f408f929048b7b47c83 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 4 May 2022 13:01:29 +0200 Subject: dma-buf: return only unsignaled fences in dma_fence_unwrap_for_each v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dma_fence_chain containers cleanup signaled fences automatically, so filter those out from arrays as well. v2: fix missing walk over the array v3: massively simplify the patch and actually update the description. Signed-off-by: Christian König Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20220518135844.3338-4-christian.koenig@amd.com --- include/linux/dma-fence-unwrap.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h index e7c219da4ed7..a4d342fef8e0 100644 --- a/include/linux/dma-fence-unwrap.h +++ b/include/linux/dma-fence-unwrap.h @@ -43,9 +43,13 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor); * Unwrap dma_fence_chain and dma_fence_array containers and deep dive into all * potential fences in them. If @head is just a normal fence only that one is * returned. + * + * Note that signalled fences are opportunistically filtered out, which + * means the iteration is potentially over no fence at all. */ #define dma_fence_unwrap_for_each(fence, cursor, head) \ for (fence = dma_fence_unwrap_first(head, cursor); fence; \ - fence = dma_fence_unwrap_next(cursor)) + fence = dma_fence_unwrap_next(cursor)) \ + if (!dma_fence_is_signaled(fence)) #endif -- cgit v1.2.3 From 245a4a7b531cffb41233a716497c25b06835cf4b Mon Sep 17 00:00:00 2001 From: Christian König Date: Mon, 25 Apr 2022 14:22:12 +0200 Subject: dma-buf: generalize dma_fence unwrap & merging v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a dma_fence_unwrap_merge() macro which allows to unwrap fences which potentially can be containers as well and then merge them back together into a flat dma_fence_array. v2: rename the function, add some more comments about how the wrapper is used, move filtering of signaled fences into the unwrap iterator, add complex selftest which covers more cases. v3: fix signaled fence filtering once more Signed-off-by: Christian König Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20220518135844.3338-5-christian.koenig@amd.com --- drivers/dma-buf/dma-fence-unwrap.c | 103 +++++++++++++++++++++++++++++ drivers/dma-buf/st-dma-fence-unwrap.c | 109 +++++++++++++++++++++++++++++++ drivers/dma-buf/sync_file.c | 119 ++-------------------------------- include/linux/dma-fence-unwrap.h | 24 +++++++ 4 files changed, 242 insertions(+), 113 deletions(-) (limited to 'include') diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index 711be125428c..502a65ea6d44 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -11,6 +11,7 @@ #include #include #include +#include /* Internal helper to start new array iteration, don't use directly */ static struct dma_fence * @@ -57,3 +58,105 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor) return __dma_fence_unwrap_array(cursor); } EXPORT_SYMBOL_GPL(dma_fence_unwrap_next); + +/* Implementation for the dma_fence_merge() marco, don't use directly */ +struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, + struct dma_fence **fences, + struct dma_fence_unwrap *iter) +{ + struct dma_fence_array *result; + struct dma_fence *tmp, **array; + unsigned int i; + size_t count; + + count = 0; + for (i = 0; i < num_fences; ++i) { + dma_fence_unwrap_for_each(tmp, &iter[i], fences[i]) + ++count; + } + + if (count == 0) + return dma_fence_get_stub(); + + array = kmalloc_array(count, sizeof(*array), GFP_KERNEL); + if (!array) + return NULL; + + /* + * This trashes the input fence array and uses it as position for the + * following merge loop. This works because the dma_fence_merge() + * wrapper macro is creating this temporary array on the stack together + * with the iterators. + */ + for (i = 0; i < num_fences; ++i) + fences[i] = dma_fence_unwrap_first(fences[i], &iter[i]); + + count = 0; + do { + unsigned int sel; + +restart: + tmp = NULL; + for (i = 0; i < num_fences; ++i) { + struct dma_fence *next; + + while (fences[i] && dma_fence_is_signaled(fences[i])) + fences[i] = dma_fence_unwrap_next(&iter[i]); + + next = fences[i]; + if (!next) + continue; + + /* + * We can't guarantee that inpute fences are ordered by + * context, but it is still quite likely when this + * function is used multiple times. So attempt to order + * the fences by context as we pass over them and merge + * fences with the same context. + */ + if (!tmp || tmp->context > next->context) { + tmp = next; + sel = i; + + } else if (tmp->context < next->context) { + continue; + + } else if (dma_fence_is_later(tmp, next)) { + fences[i] = dma_fence_unwrap_next(&iter[i]); + goto restart; + } else { + fences[sel] = dma_fence_unwrap_next(&iter[sel]); + goto restart; + } + } + + if (tmp) { + array[count++] = dma_fence_get(tmp); + fences[sel] = dma_fence_unwrap_next(&iter[sel]); + } + } while (tmp); + + if (count == 0) { + tmp = dma_fence_get_stub(); + goto return_tmp; + } + + if (count == 1) { + tmp = array[0]; + goto return_tmp; + } + + result = dma_fence_array_create(count, array, + dma_fence_context_alloc(1), + 1, false); + if (!result) { + tmp = NULL; + goto return_tmp; + } + return &result->base; + +return_tmp: + kfree(array); + return tmp; +} +EXPORT_SYMBOL_GPL(__dma_fence_unwrap_merge); diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c index e20c5a7dcfe4..4105d5ea8dde 100644 --- a/drivers/dma-buf/st-dma-fence-unwrap.c +++ b/drivers/dma-buf/st-dma-fence-unwrap.c @@ -238,6 +238,113 @@ static int unwrap_chain_array(void *arg) return err; } +static int unwrap_merge(void *arg) +{ + struct dma_fence *fence, *f1, *f2, *f3; + struct dma_fence_unwrap iter; + int err = 0; + + f1 = mock_fence(); + if (!f1) + return -ENOMEM; + + f2 = mock_fence(); + if (!f2) { + err = -ENOMEM; + goto error_put_f1; + } + + f3 = dma_fence_unwrap_merge(f1, f2); + if (!f3) { + err = -ENOMEM; + goto error_put_f2; + } + + dma_fence_unwrap_for_each(fence, &iter, f3) { + if (fence == f1) { + dma_fence_put(f1); + f1 = NULL; + } else if (fence == f2) { + dma_fence_put(f2); + f2 = NULL; + } else { + pr_err("Unexpected fence!\n"); + err = -EINVAL; + } + } + + if (f1 || f2) { + pr_err("Not all fences seen!\n"); + err = -EINVAL; + } + + dma_fence_put(f3); +error_put_f2: + dma_fence_put(f2); +error_put_f1: + dma_fence_put(f1); + return err; +} + +static int unwrap_merge_complex(void *arg) +{ + struct dma_fence *fence, *f1, *f2, *f3, *f4, *f5; + struct dma_fence_unwrap iter; + int err = -ENOMEM; + + f1 = mock_fence(); + if (!f1) + return -ENOMEM; + + f2 = mock_fence(); + if (!f2) + goto error_put_f1; + + f3 = dma_fence_unwrap_merge(f1, f2); + if (!f3) + goto error_put_f2; + + /* The resulting array has the fences in reverse */ + f4 = dma_fence_unwrap_merge(f2, f1); + if (!f4) + goto error_put_f3; + + /* Signaled fences should be filtered, the two arrays merged. */ + f5 = dma_fence_unwrap_merge(f3, f4, dma_fence_get_stub()); + if (!f5) + goto error_put_f4; + + err = 0; + dma_fence_unwrap_for_each(fence, &iter, f5) { + if (fence == f1) { + dma_fence_put(f1); + f1 = NULL; + } else if (fence == f2) { + dma_fence_put(f2); + f2 = NULL; + } else { + pr_err("Unexpected fence!\n"); + err = -EINVAL; + } + } + + if (f1 || f2) { + pr_err("Not all fences seen!\n"); + err = -EINVAL; + } + + dma_fence_put(f5); +error_put_f4: + dma_fence_put(f4); +error_put_f3: + dma_fence_put(f3); +error_put_f2: + dma_fence_put(f2); +error_put_f1: + dma_fence_put(f1); + return err; +} + int dma_fence_unwrap(void) { static const struct subtest tests[] = { @@ -245,6 +352,8 @@ int dma_fence_unwrap(void) SUBTEST(unwrap_array), SUBTEST(unwrap_chain), SUBTEST(unwrap_chain_array), + SUBTEST(unwrap_merge), + SUBTEST(unwrap_merge_complex), }; return subtests(tests, NULL); diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 0fe564539166..3ebec19a8e02 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -146,50 +146,6 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) return buf; } -static int sync_file_set_fence(struct sync_file *sync_file, - struct dma_fence **fences, int num_fences) -{ - struct dma_fence_array *array; - - /* - * The reference for the fences in the new sync_file and held - * in add_fence() during the merge procedure, so for num_fences == 1 - * we already own a new reference to the fence. For num_fence > 1 - * we own the reference of the dma_fence_array creation. - */ - - if (num_fences == 0) { - sync_file->fence = dma_fence_get_stub(); - kfree(fences); - - } else if (num_fences == 1) { - sync_file->fence = fences[0]; - kfree(fences); - - } else { - array = dma_fence_array_create(num_fences, fences, - dma_fence_context_alloc(1), - 1, false); - if (!array) - return -ENOMEM; - - sync_file->fence = &array->base; - } - - return 0; -} - -static void add_fence(struct dma_fence **fences, - int *i, struct dma_fence *fence) -{ - fences[*i] = fence; - - if (!dma_fence_is_signaled(fence)) { - dma_fence_get(fence); - (*i)++; - } -} - /** * sync_file_merge() - merge two sync_files * @name: name of new fence @@ -203,84 +159,21 @@ static void add_fence(struct dma_fence **fences, static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b) { - struct dma_fence *a_fence, *b_fence, **fences; - struct dma_fence_unwrap a_iter, b_iter; - unsigned int index, num_fences; struct sync_file *sync_file; + struct dma_fence *fence; sync_file = sync_file_alloc(); if (!sync_file) return NULL; - num_fences = 0; - dma_fence_unwrap_for_each(a_fence, &a_iter, a->fence) - ++num_fences; - dma_fence_unwrap_for_each(b_fence, &b_iter, b->fence) - ++num_fences; - - if (num_fences > INT_MAX) - goto err_free_sync_file; - - fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); - if (!fences) - goto err_free_sync_file; - - /* - * We can't guarantee that fences in both a and b are ordered, but it is - * still quite likely. - * - * So attempt to order the fences as we pass over them and merge fences - * with the same context. - */ - - index = 0; - for (a_fence = dma_fence_unwrap_first(a->fence, &a_iter), - b_fence = dma_fence_unwrap_first(b->fence, &b_iter); - a_fence || b_fence; ) { - - if (!b_fence) { - add_fence(fences, &index, a_fence); - a_fence = dma_fence_unwrap_next(&a_iter); - - } else if (!a_fence) { - add_fence(fences, &index, b_fence); - b_fence = dma_fence_unwrap_next(&b_iter); - - } else if (a_fence->context < b_fence->context) { - add_fence(fences, &index, a_fence); - a_fence = dma_fence_unwrap_next(&a_iter); - - } else if (b_fence->context < a_fence->context) { - add_fence(fences, &index, b_fence); - b_fence = dma_fence_unwrap_next(&b_iter); - - } else if (__dma_fence_is_later(a_fence->seqno, b_fence->seqno, - a_fence->ops)) { - add_fence(fences, &index, a_fence); - a_fence = dma_fence_unwrap_next(&a_iter); - b_fence = dma_fence_unwrap_next(&b_iter); - - } else { - add_fence(fences, &index, b_fence); - a_fence = dma_fence_unwrap_next(&a_iter); - b_fence = dma_fence_unwrap_next(&b_iter); - } + fence = dma_fence_unwrap_merge(a->fence, b->fence); + if (!fence) { + fput(sync_file->file); + return NULL; } - - if (sync_file_set_fence(sync_file, fences, index) < 0) - goto err_put_fences; - + sync_file->fence = fence; strlcpy(sync_file->user_name, name, sizeof(sync_file->user_name)); return sync_file; - -err_put_fences: - while (index) - dma_fence_put(fences[--index]); - kfree(fences); - -err_free_sync_file: - fput(sync_file->file); - return NULL; } static int sync_file_release(struct inode *inode, struct file *file) diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h index a4d342fef8e0..390de1ee9d35 100644 --- a/include/linux/dma-fence-unwrap.h +++ b/include/linux/dma-fence-unwrap.h @@ -52,4 +52,28 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor); fence = dma_fence_unwrap_next(cursor)) \ if (!dma_fence_is_signaled(fence)) +struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, + struct dma_fence **fences, + struct dma_fence_unwrap *cursors); + +/** + * dma_fence_unwrap_merge - unwrap and merge fences + * + * All fences given as parameters are unwrapped and merged back together as flat + * dma_fence_array. Useful if multiple containers need to be merged together. + * + * Implemented as a macro to allocate the necessary arrays on the stack and + * account the stack frame size to the caller. + * + * Returns NULL on memory allocation failure, a dma_fence object representing + * all the given fences otherwise. + */ +#define dma_fence_unwrap_merge(...) \ + ({ \ + struct dma_fence *__f[] = { __VA_ARGS__ }; \ + struct dma_fence_unwrap __c[ARRAY_SIZE(__f)]; \ + \ + __dma_fence_unwrap_merge(ARRAY_SIZE(__f), __f, __c); \ + }) + #endif -- cgit v1.2.3 From 3800b1710946f7db3cb3a29cb2e218cf5df999d0 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Tue, 10 May 2022 12:29:42 -0700 Subject: drm/dp: Add callbacks to make using DP AUX bus properly easier As talked about in this patch in the kerneldoc of of_dp_aux_populate_ep_device() and also in the past in commit a1e3667a9835 ("drm/bridge: ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), it can be difficult for eDP controller drivers to know when the panel has finished probing when they're using of_dp_aux_populate_ep_devices(). The ti-sn65dsi86 driver managed to solve this because it was already broken up into a bunch of sub-drivers. That means we could solve the problem there by adding a new sub-driver to get the panel. We could use the traditional -EPROBE_DEFER retry mechansim to handle the case where the panel hadn't probed yet. In parade-ps8640 we didn't really solve this. The code just expects the panel to be ready right away. While reviewing the code originally I had managed to convince myself it was fine to just expect the panel right away, but additional testing has shown that not to be the case. We could fix parade-ps8640 like we did ti-sn65dsi86 but it's pretty cumbersome (since we're not already broken into multiple drivers) and requires a bunch of boilerplate code. After discussion [1] it seems like the best solution for most people is: - Accept that there's always at most one device that will probe as a result of the DP AUX bus (it may have sub-devices, but there will be one device _directly_ probed). - When that device finishes probing, we can just have a call back. This patch implements that idea. We'll now take a callback as an argument to the populate function. To make this easier to land in pieces, we'll make wrappers for the old functions. The functions with the new name (which make it clear that we only have one child) will take the callback and the functions with the old name will temporarily wrap. [1] https://lore.kernel.org/r/CAD=FV=Ur3afHhsXe7a3baWEnD=MFKFeKRbhFU+bt3P67G0MVzQ@mail.gmail.com Signed-off-by: Douglas Anderson Reviewed-by: Dmitry Baryshkov Link: https://patchwork.freedesktop.org/patch/msgid/20220510122726.v3.2.I4182ae27e00792842cb86f1433990a0ef9c0a073@changeid --- drivers/gpu/drm/display/drm_dp_aux_bus.c | 209 ++++++++++++++++++++----------- include/drm/display/drm_dp_aux_bus.h | 34 ++++- 2 files changed, 168 insertions(+), 75 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/display/drm_dp_aux_bus.c b/drivers/gpu/drm/display/drm_dp_aux_bus.c index 552f949cff59..f5741b45ca07 100644 --- a/drivers/gpu/drm/display/drm_dp_aux_bus.c +++ b/drivers/gpu/drm/display/drm_dp_aux_bus.c @@ -3,10 +3,10 @@ * Copyright 2021 Google Inc. * * The DP AUX bus is used for devices that are connected over a DisplayPort - * AUX bus. The devices on the far side of the bus are referred to as - * endpoints in this code. + * AUX bus. The device on the far side of the bus is referred to as an + * endpoint in this code. * - * Commonly there is only one device connected to the DP AUX bus: a panel. + * There is only one device connected to the DP AUX bus: an eDP panel. * Though historically panels (even DP panels) have been modeled as simple * platform devices, putting them under the DP AUX bus allows the panel driver * to perform transactions on that bus. @@ -22,6 +22,11 @@ #include #include +struct dp_aux_ep_device_with_data { + struct dp_aux_ep_device aux_ep; + int (*done_probing)(struct drm_dp_aux *aux); +}; + /** * dp_aux_ep_match() - The match function for the dp_aux_bus. * @dev: The device to match. @@ -48,6 +53,8 @@ static int dp_aux_ep_probe(struct device *dev) { struct dp_aux_ep_driver *aux_ep_drv = to_dp_aux_ep_drv(dev->driver); struct dp_aux_ep_device *aux_ep = to_dp_aux_ep_dev(dev); + struct dp_aux_ep_device_with_data *aux_ep_with_data = + container_of(aux_ep, struct dp_aux_ep_device_with_data, aux_ep); int ret; ret = dev_pm_domain_attach(dev, true); @@ -56,7 +63,32 @@ static int dp_aux_ep_probe(struct device *dev) ret = aux_ep_drv->probe(aux_ep); if (ret) - dev_pm_domain_detach(dev, true); + goto err_attached; + + if (aux_ep_with_data->done_probing) { + ret = aux_ep_with_data->done_probing(aux_ep->aux); + if (ret) { + /* + * The done_probing() callback should not return + * -EPROBE_DEFER to us. If it does, we treat it as an + * error. Passing it on as-is would cause the _panel_ + * to defer. + */ + if (ret == -EPROBE_DEFER) { + dev_err(dev, + "DP AUX done_probing() can't defer\n"); + ret = -EINVAL; + } + goto err_probed; + } + } + + return 0; +err_probed: + if (aux_ep_drv->remove) + aux_ep_drv->remove(aux_ep); +err_attached: + dev_pm_domain_detach(dev, true); return ret; } @@ -122,7 +154,11 @@ ATTRIBUTE_GROUPS(dp_aux_ep_dev); */ static void dp_aux_ep_dev_release(struct device *dev) { - kfree(to_dp_aux_ep_dev(dev)); + struct dp_aux_ep_device *aux_ep = to_dp_aux_ep_dev(dev); + struct dp_aux_ep_device_with_data *aux_ep_with_data = + container_of(aux_ep, struct dp_aux_ep_device_with_data, aux_ep); + + kfree(aux_ep_with_data); } static struct device_type dp_aux_device_type_type = { @@ -136,12 +172,14 @@ static struct device_type dp_aux_device_type_type = { * @dev: The device to destroy. * @data: Not used * - * This is just used as a callback by of_dp_aux_depopulate_ep_devices() and + * This is just used as a callback by of_dp_aux_depopulate_bus() and * is called for _all_ of the child devices of the device providing the AUX bus. * We'll only act on those that are of type "dp_aux_bus_type". * - * This function is effectively an inverse of what's in the loop - * in of_dp_aux_populate_ep_devices(). + * This function is effectively an inverse of what's in + * of_dp_aux_populate_bus(). NOTE: since we only populate one child + * then it's expected that only one device will match all the "if" tests in + * this function and get to the device_unregister(). * * Return: 0 if no error or negative error code. */ @@ -164,123 +202,150 @@ static int of_dp_aux_ep_destroy(struct device *dev, void *data) } /** - * of_dp_aux_depopulate_ep_devices() - Undo of_dp_aux_populate_ep_devices - * @aux: The AUX channel whose devices we want to depopulate + * of_dp_aux_depopulate_bus() - Undo of_dp_aux_populate_bus + * @aux: The AUX channel whose device we want to depopulate * - * This will destroy all devices that were created - * by of_dp_aux_populate_ep_devices(). + * This will destroy the device that was created + * by of_dp_aux_populate_bus(). */ -void of_dp_aux_depopulate_ep_devices(struct drm_dp_aux *aux) +void of_dp_aux_depopulate_bus(struct drm_dp_aux *aux) { device_for_each_child_reverse(aux->dev, NULL, of_dp_aux_ep_destroy); } -EXPORT_SYMBOL_GPL(of_dp_aux_depopulate_ep_devices); +EXPORT_SYMBOL_GPL(of_dp_aux_depopulate_bus); /** - * of_dp_aux_populate_ep_devices() - Populate the endpoint devices on the DP AUX - * @aux: The AUX channel whose devices we want to populate. It is required that + * of_dp_aux_populate_bus() - Populate the endpoint device on the DP AUX + * @aux: The AUX channel whose device we want to populate. It is required that * drm_dp_aux_init() has already been called for this AUX channel. + * @done_probing: Callback functions to call after EP device finishes probing. + * Will not be called if there are no EP devices and this + * function will return -ENODEV. * - * This will populate all the devices under the "aux-bus" node of the device - * providing the AUX channel (AKA aux->dev). + * This will populate the device (expected to be an eDP panel) under the + * "aux-bus" node of the device providing the AUX channel (AKA aux->dev). * * When this function finishes, it is _possible_ (but not guaranteed) that - * our sub-devices will have finished probing. It should be noted that if our - * sub-devices return -EPROBE_DEFER that we will not return any error codes - * ourselves but our sub-devices will _not_ have actually probed successfully - * yet. There may be other cases (maybe added in the future?) where sub-devices - * won't have been probed yet when this function returns, so it's best not to - * rely on that. + * our sub-device will have finished probing. It should be noted that if our + * sub-device returns -EPROBE_DEFER or is probing asynchronously for some + * reason that we will not return any error codes ourselves but our + * sub-device will _not_ have actually probed successfully yet. + * + * In many cases it's important for the caller of this function to be notified + * when our sub device finishes probing. Our sub device is expected to be an + * eDP panel and the caller is expected to be an eDP controller. The eDP + * controller needs to be able to get a reference to the panel when it finishes + * probing. For this reason the caller can pass in a function pointer that + * will be called when our sub-device finishes probing. * * If this function succeeds you should later make sure you call - * of_dp_aux_depopulate_ep_devices() to undo it, or just use the devm version + * of_dp_aux_depopulate_bus() to undo it, or just use the devm version * of this function. * - * Return: 0 if no error or negative error code. + * Return: 0 if no error or negative error code; returns -ENODEV if there are + * no children. The done_probing() function won't be called in that + * case. */ -int of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux) +int of_dp_aux_populate_bus(struct drm_dp_aux *aux, + int (*done_probing)(struct drm_dp_aux *aux)) { - struct device_node *bus, *np; + struct device_node *bus = NULL, *np = NULL; struct dp_aux_ep_device *aux_ep; + struct dp_aux_ep_device_with_data *aux_ep_with_data; int ret; /* drm_dp_aux_init() should have been called already; warn if not */ WARN_ON_ONCE(!aux->ddc.algo); if (!aux->dev->of_node) - return 0; - + return -ENODEV; bus = of_get_child_by_name(aux->dev->of_node, "aux-bus"); if (!bus) - return 0; + return -ENODEV; - for_each_available_child_of_node(bus, np) { - if (of_node_test_and_set_flag(np, OF_POPULATED)) - continue; + np = of_get_next_available_child(bus, NULL); + of_node_put(bus); + if (!np) + return -ENODEV; - aux_ep = kzalloc(sizeof(*aux_ep), GFP_KERNEL); - if (!aux_ep) - continue; - aux_ep->aux = aux; + if (of_node_test_and_set_flag(np, OF_POPULATED)) { + dev_err(aux->dev, "DP AUX EP device already populated\n"); + ret = -EINVAL; + goto err_did_get_np; + } - aux_ep->dev.parent = aux->dev; - aux_ep->dev.bus = &dp_aux_bus_type; - aux_ep->dev.type = &dp_aux_device_type_type; - aux_ep->dev.of_node = of_node_get(np); - dev_set_name(&aux_ep->dev, "aux-%s", dev_name(aux->dev)); + aux_ep_with_data = kzalloc(sizeof(*aux_ep_with_data), GFP_KERNEL); + if (!aux_ep_with_data) { + ret = -ENOMEM; + goto err_did_set_populated; + } - ret = device_register(&aux_ep->dev); - if (ret) { - dev_err(aux->dev, "Failed to create AUX EP for %pOF: %d\n", np, ret); - of_node_clear_flag(np, OF_POPULATED); - of_node_put(np); + aux_ep_with_data->done_probing = done_probing; - /* - * As per docs of device_register(), call this instead - * of kfree() directly for error cases. - */ - put_device(&aux_ep->dev); + aux_ep = &aux_ep_with_data->aux_ep; + aux_ep->aux = aux; + aux_ep->dev.parent = aux->dev; + aux_ep->dev.bus = &dp_aux_bus_type; + aux_ep->dev.type = &dp_aux_device_type_type; + aux_ep->dev.of_node = of_node_get(np); + dev_set_name(&aux_ep->dev, "aux-%s", dev_name(aux->dev)); - /* - * Following in the footsteps of of_i2c_register_devices(), - * we won't fail the whole function here--we'll just - * continue registering any other devices we find. - */ - } - } + ret = device_register(&aux_ep->dev); + if (ret) { + dev_err(aux->dev, "Failed to create AUX EP for %pOF: %d\n", np, ret); - of_node_put(bus); + /* + * As per docs of device_register(), call this instead + * of kfree() directly for error cases. + */ + put_device(&aux_ep->dev); + + goto err_did_set_populated; + } return 0; + +err_did_set_populated: + of_node_clear_flag(np, OF_POPULATED); + +err_did_get_np: + of_node_put(np); + + return ret; } -EXPORT_SYMBOL_GPL(of_dp_aux_populate_ep_devices); +EXPORT_SYMBOL_GPL(of_dp_aux_populate_bus); -static void of_dp_aux_depopulate_ep_devices_void(void *data) +static void of_dp_aux_depopulate_bus_void(void *data) { - of_dp_aux_depopulate_ep_devices(data); + of_dp_aux_depopulate_bus(data); } /** - * devm_of_dp_aux_populate_ep_devices() - devm wrapper for of_dp_aux_populate_ep_devices() - * @aux: The AUX channel whose devices we want to populate + * devm_of_dp_aux_populate_bus() - devm wrapper for of_dp_aux_populate_bus() + * @aux: The AUX channel whose device we want to populate + * @done_probing: Callback functions to call after EP device finishes probing. + * Will not be called if there are no EP devices and this + * function will return -ENODEV. * * Handles freeing w/ devm on the device "aux->dev". * - * Return: 0 if no error or negative error code. + * Return: 0 if no error or negative error code; returns -ENODEV if there are + * no children. The done_probing() function won't be called in that + * case. */ -int devm_of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux) +int devm_of_dp_aux_populate_bus(struct drm_dp_aux *aux, + int (*done_probing)(struct drm_dp_aux *aux)) { int ret; - ret = of_dp_aux_populate_ep_devices(aux); + ret = of_dp_aux_populate_bus(aux, done_probing); if (ret) return ret; return devm_add_action_or_reset(aux->dev, - of_dp_aux_depopulate_ep_devices_void, - aux); + of_dp_aux_depopulate_bus_void, aux); } -EXPORT_SYMBOL_GPL(devm_of_dp_aux_populate_ep_devices); +EXPORT_SYMBOL_GPL(devm_of_dp_aux_populate_bus); int __dp_aux_dp_driver_register(struct dp_aux_ep_driver *drv, struct module *owner) { diff --git a/include/drm/display/drm_dp_aux_bus.h b/include/drm/display/drm_dp_aux_bus.h index 4f19b20b1dd6..8a0a486383c5 100644 --- a/include/drm/display/drm_dp_aux_bus.h +++ b/include/drm/display/drm_dp_aux_bus.h @@ -44,9 +44,37 @@ static inline struct dp_aux_ep_driver *to_dp_aux_ep_drv(struct device_driver *dr return container_of(drv, struct dp_aux_ep_driver, driver); } -int of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux); -void of_dp_aux_depopulate_ep_devices(struct drm_dp_aux *aux); -int devm_of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux); +int of_dp_aux_populate_bus(struct drm_dp_aux *aux, + int (*done_probing)(struct drm_dp_aux *aux)); +void of_dp_aux_depopulate_bus(struct drm_dp_aux *aux); +int devm_of_dp_aux_populate_bus(struct drm_dp_aux *aux, + int (*done_probing)(struct drm_dp_aux *aux)); + +/* Deprecated versions of the above functions. To be removed when no callers. */ +static inline int of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux) +{ + int ret; + + ret = of_dp_aux_populate_bus(aux, NULL); + + /* New API returns -ENODEV for no child case; adapt to old assumption */ + return (ret != -ENODEV) ? ret : 0; +} + +static inline int devm_of_dp_aux_populate_ep_devices(struct drm_dp_aux *aux) +{ + int ret; + + ret = devm_of_dp_aux_populate_bus(aux, NULL); + + /* New API returns -ENODEV for no child case; adapt to old assumption */ + return (ret != -ENODEV) ? ret : 0; +} + +static inline void of_dp_aux_depopulate_ep_devices(struct drm_dp_aux *aux) +{ + of_dp_aux_depopulate_bus(aux); +} #define dp_aux_dp_driver_register(aux_ep_drv) \ __dp_aux_dp_driver_register(aux_ep_drv, THIS_MODULE) -- cgit v1.2.3 From 50e156bd8a9d0910ac4bae5fcff00ddb798db967 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Tue, 10 May 2022 12:29:43 -0700 Subject: drm/bridge: Add devm_drm_bridge_add() This adds a devm managed version of drm_bridge_add(). Like other "devm" function listed in drm_bridge.h, this function takes an explicit "dev" to use for the lifetime management. A few notes: * In general we have a "struct device" for bridges that makes a good candidate for where the lifetime matches exactly what we want. * The "bridge->dev->dev" device appears to be the encoder device. That's not the right device to use for lifetime management. Suggested-by: Dmitry Baryshkov Signed-off-by: Douglas Anderson Reviewed-by: Dmitry Baryshkov Link: https://patchwork.freedesktop.org/patch/msgid/20220510122726.v3.3.Iba4b9bf6c7a1ee5ea2835ad7bd5eaf84d7688520@changeid --- drivers/gpu/drm/drm_bridge.c | 23 +++++++++++++++++++++++ include/drm/drm_bridge.h | 1 + 2 files changed, 24 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c96847fc0ebc..e275b4ca344b 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -170,6 +170,29 @@ void drm_bridge_add(struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_add); +static void drm_bridge_remove_void(void *bridge) +{ + drm_bridge_remove(bridge); +} + +/** + * devm_drm_bridge_add - devm managed version of drm_bridge_add() + * + * @dev: device to tie the bridge lifetime to + * @bridge: bridge control structure + * + * This is the managed version of drm_bridge_add() which automatically + * calls drm_bridge_remove() when @dev is unbound. + * + * Return: 0 if no error or negative error code. + */ +int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge) +{ + drm_bridge_add(bridge); + return devm_add_action_or_reset(dev, drm_bridge_remove_void, bridge); +} +EXPORT_SYMBOL(devm_drm_bridge_add); + /** * drm_bridge_remove - remove the given bridge from the global bridge list * diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index f27b4060faa2..42aec8612f37 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -796,6 +796,7 @@ drm_priv_to_bridge(struct drm_private_obj *priv) } void drm_bridge_add(struct drm_bridge *bridge); +int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge); void drm_bridge_remove(struct drm_bridge *bridge); int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, struct drm_bridge *previous, -- cgit v1.2.3