From ad81e23426a651eb89a4b306e1c4169e6308c124 Mon Sep 17 00:00:00 2001 From: Jocelyn Falempe Date: Wed, 10 May 2023 15:10:34 +0200 Subject: drm/mgag200: Fix gamma lut not initialized. When mgag200 switched from simple KMS to regular atomic helpers, the initialization of the gamma settings was lost. This leads to a black screen, if the bios/uefi doesn't use the same pixel color depth. v2: rebase on top of drm-misc-fixes, and add Cc stable tag. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2171155 Fixes: 1baf9127c482 ("drm/mgag200: Replace simple-KMS with regular atomic helpers") Cc: Tested-by: Phil Oester Reviewed-by: Thomas Zimmermann Signed-off-by: Jocelyn Falempe Link: https://patchwork.freedesktop.org/patch/msgid/20230510131034.284078-1-jfalempe@redhat.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 0a5aaf78172a..576c4c838a33 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -640,6 +640,11 @@ void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_ if (funcs->pixpllc_atomic_update) funcs->pixpllc_atomic_update(crtc, old_state); + if (crtc_state->gamma_lut) + mgag200_crtc_set_gamma(mdev, format, crtc_state->gamma_lut->data); + else + mgag200_crtc_set_gamma_linear(mdev, format); + mgag200_enable_display(mdev); if (funcs->enable_vidrst) -- cgit From 140809738d969376f26f13264b16669703956f6c Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 15 May 2023 11:29:43 +0200 Subject: drm/pl111: Fix FB depth on IMPD-1 framebuffer The last argument to the function drm_fbdev_dma_setup() was changed from desired BPP to desired depth. In our case the desired depth was 15 but BPP was 16, so we specified 16 as BPP and we relied on the FB emulation core to select a format with a suitable depth for the limited bandwidth and end up with e.g. XRGB1555 like in the past: [drm] Initialized pl111 1.0.0 20170317 for c1000000.display on minor 0 drm-clcd-pl111 c1000000.display: [drm] requested bpp 16, scaled depth down to 15 drm-clcd-pl111 c1000000.display: enable IM-PD1 CLCD connectors Console: switching to colour frame buffer device 80x30 drm-clcd-pl111 c1000000.display: [drm] fb0: pl111drmfb frame buffer device However the current code will fail at that: [drm] Initialized pl111 1.0.0 20170317 for c1000000.display on minor 0 drm-clcd-pl111 c1000000.display: [drm] bpp/depth value of 16/16 not supported drm-clcd-pl111 c1000000.display: [drm] No compatible format found drm-clcd-pl111 c1000000.display: [drm] *ERROR* fbdev: Failed to setup generic emulation (ret=-12) Fix this by passing the desired depth of 15 for the IM/PD-1 display instead of 16 to drm_fbdev_dma_setup(). The desired depth is however in turn used for bandwidth limiting calculations and that was done with a simple / integer division, whereas we now have to modify that to use DIV_ROUND_UP() so that we get DIV_ROUND_UP(15, 2) = 2 not 15/2 = 1. After this the display works again on the Integrator/AP IM/PD-1. Cc: Emma Anholt Cc: stable@vger.kernel.org Suggested-by: Thomas Zimmermann Fixes: 37c90d589dc0 ("drm/fb-helper: Fix single-probe color-format selection") Link: https://lore.kernel.org/dri-devel/20230102112927.26565-1-tzimmermann@suse.de/ Reviewed-by: Thomas Zimmermann Signed-off-by: Linus Walleij Link: https://patchwork.freedesktop.org/patch/msgid/20230515092943.1401558-1-linus.walleij@linaro.org --- drivers/gpu/drm/pl111/pl111_display.c | 2 +- drivers/gpu/drm/pl111/pl111_drm.h | 4 ++-- drivers/gpu/drm/pl111/pl111_drv.c | 8 ++++---- drivers/gpu/drm/pl111/pl111_versatile.c | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index 6afdf260a4e2..b9fe926a49e8 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -53,7 +53,7 @@ pl111_mode_valid(struct drm_simple_display_pipe *pipe, { struct drm_device *drm = pipe->crtc.dev; struct pl111_drm_dev_private *priv = drm->dev_private; - u32 cpp = priv->variant->fb_bpp / 8; + u32 cpp = DIV_ROUND_UP(priv->variant->fb_depth, 8); u64 bw; /* diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h index 2a46b5bd8576..d1fe756444ee 100644 --- a/drivers/gpu/drm/pl111/pl111_drm.h +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -114,7 +114,7 @@ struct drm_minor; * extensions to the control register * @formats: array of supported pixel formats on this variant * @nformats: the length of the array of supported pixel formats - * @fb_bpp: desired bits per pixel on the default framebuffer + * @fb_depth: desired depth per pixel on the default framebuffer */ struct pl111_variant_data { const char *name; @@ -126,7 +126,7 @@ struct pl111_variant_data { bool st_bitmux_control; const u32 *formats; unsigned int nformats; - unsigned int fb_bpp; + unsigned int fb_depth; }; struct pl111_drm_dev_private { diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 4b2a9e9753f6..43049c8028b2 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -308,7 +308,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev, if (ret < 0) goto dev_put; - drm_fbdev_dma_setup(drm, priv->variant->fb_bpp); + drm_fbdev_dma_setup(drm, priv->variant->fb_depth); return 0; @@ -351,7 +351,7 @@ static const struct pl111_variant_data pl110_variant = { .is_pl110 = true, .formats = pl110_pixel_formats, .nformats = ARRAY_SIZE(pl110_pixel_formats), - .fb_bpp = 16, + .fb_depth = 16, }; /* RealView, Versatile Express etc use this modern variant */ @@ -376,7 +376,7 @@ static const struct pl111_variant_data pl111_variant = { .name = "PL111", .formats = pl111_pixel_formats, .nformats = ARRAY_SIZE(pl111_pixel_formats), - .fb_bpp = 32, + .fb_depth = 32, }; static const u32 pl110_nomadik_pixel_formats[] = { @@ -405,7 +405,7 @@ static const struct pl111_variant_data pl110_nomadik_variant = { .is_lcdc = true, .st_bitmux_control = true, .broken_vblank = true, - .fb_bpp = 16, + .fb_depth = 16, }; static const struct amba_id pl111_id_table[] = { diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 1b436b75fd39..00c3ebd32359 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -316,7 +316,7 @@ static const struct pl111_variant_data pl110_integrator = { .broken_vblank = true, .formats = pl110_integrator_pixel_formats, .nformats = ARRAY_SIZE(pl110_integrator_pixel_formats), - .fb_bpp = 16, + .fb_depth = 16, }; /* @@ -330,7 +330,7 @@ static const struct pl111_variant_data pl110_impd1 = { .broken_vblank = true, .formats = pl110_integrator_pixel_formats, .nformats = ARRAY_SIZE(pl110_integrator_pixel_formats), - .fb_bpp = 16, + .fb_depth = 15, }; /* @@ -343,7 +343,7 @@ static const struct pl111_variant_data pl110_versatile = { .external_bgr = true, .formats = pl110_versatile_pixel_formats, .nformats = ARRAY_SIZE(pl110_versatile_pixel_formats), - .fb_bpp = 16, + .fb_depth = 16, }; /* @@ -355,7 +355,7 @@ static const struct pl111_variant_data pl111_realview = { .name = "PL111 RealView", .formats = pl111_realview_pixel_formats, .nformats = ARRAY_SIZE(pl111_realview_pixel_formats), - .fb_bpp = 16, + .fb_depth = 16, }; /* @@ -367,7 +367,7 @@ static const struct pl111_variant_data pl111_vexpress = { .name = "PL111 Versatile Express", .formats = pl111_realview_pixel_formats, .nformats = ARRAY_SIZE(pl111_realview_pixel_formats), - .fb_bpp = 16, + .fb_depth = 16, .broken_clockdivider = true, }; -- cgit From 2d2f5f1e8fd91fe848aacde28f738fb8e8d95a27 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 3 May 2023 13:41:27 +0300 Subject: accel/qaic: silence some uninitialized variable warnings Smatch complains that these are not initialized if get_cntl_version() fails but we still print them in the debug message. Not the end of the world, but true enough. Let's just initialize them to a dummy value to make the checker happy. Fixes: c501ca23a6a3 ("accel/qaic: Add uapi and core driver file") Signed-off-by: Dan Carpenter Reviewed-by: Jeffrey Hugo Reviewed-by: Carl Vanderlip Reviewed-by: Pranjal Ramajor Asha Kanojiya [jhugo: Add fixes and reorder varable declarations for style] Signed-off-by: Jeffrey Hugo Link: https://patchwork.freedesktop.org/patch/msgid/d11ee378-7b06-4b5e-b56f-d66174be1ab3@kili.mountain --- drivers/accel/qaic/qaic_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c index ff80eb571729..2d0828db28d8 100644 --- a/drivers/accel/qaic/qaic_drv.c +++ b/drivers/accel/qaic/qaic_drv.c @@ -262,8 +262,8 @@ static void qaic_destroy_drm_device(struct qaic_device *qdev, s32 partition_id) static int qaic_mhi_probe(struct mhi_device *mhi_dev, const struct mhi_device_id *id) { + u16 major = -1, minor = -1; struct qaic_device *qdev; - u16 major, minor; int ret; /* -- cgit From 1aa7f416175619e0286fddc5fc44e968b06bf2aa Mon Sep 17 00:00:00 2001 From: Maya Matuszczyk Date: Mon, 15 May 2023 20:48:43 +0200 Subject: drm: panel-orientation-quirks: Change Air's quirk to support Air Plus It turned out that Aya Neo Air Plus had a different board name than expected. This patch changes Aya Neo Air's quirk to account for that, as both devices share "Air" in DMI product name. Tested on Air claiming to be an Air Pro, and on Air Plus. Signed-off-by: Maya Matuszczyk Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede Link: https://patchwork.freedesktop.org/patch/msgid/20230515184843.1552612-1-maccraft123mc@gmail.com --- drivers/gpu/drm/drm_panel_orientation_quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c b/drivers/gpu/drm/drm_panel_orientation_quirks.c index b1a38e6ce2f8..0cb646cb04ee 100644 --- a/drivers/gpu/drm/drm_panel_orientation_quirks.c +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c @@ -179,7 +179,7 @@ static const struct dmi_system_id orientation_data[] = { }, { /* AYA NEO AIR */ .matches = { DMI_EXACT_MATCH(DMI_SYS_VENDOR, "AYANEO"), - DMI_MATCH(DMI_BOARD_NAME, "AIR"), + DMI_MATCH(DMI_PRODUCT_NAME, "AIR"), }, .driver_data = (void *)&lcd1080x1920_leftside_up, }, { /* AYA NEO NEXT */ -- cgit From aa8bf93101a185b49f83c9137453571a08be6e76 Mon Sep 17 00:00:00 2001 From: Vladislav Efanov Date: Wed, 17 May 2023 15:52:47 +0300 Subject: drm/sched: Remove redundant check The rq pointer points inside the drm_gpu_scheduler structure. Thus it can't be NULL. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: c61cdbdbffc1 ("drm/scheduler: Fix hang when sched_entity released") Signed-off-by: Vladislav Efanov Link: https://lore.kernel.org/r/20230517125247.434103-1-VEfanov@ispras.ru Reviewed-by: Luben Tuikov Signed-off-by: Luben Tuikov --- drivers/gpu/drm/scheduler/sched_main.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 8c183639603e..aea5a90ff98b 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -1141,9 +1141,6 @@ void drm_sched_fini(struct drm_gpu_scheduler *sched) for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN; i--) { struct drm_sched_rq *rq = &sched->sched_rq[i]; - if (!rq) - continue; - spin_lock(&rq->lock); list_for_each_entry(s_entity, &rq->entities, list) /* -- cgit From c21f11d182c2180d8b90eaff84f574cfa845b250 Mon Sep 17 00:00:00 2001 From: Matthew Auld Date: Fri, 19 May 2023 10:07:33 +0100 Subject: drm: fix drmm_mutex_init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In mutex_init() lockdep identifies a lock by defining a special static key for each lock class. However if we wrap the macro in a function, like in drmm_mutex_init(), we end up generating: int drmm_mutex_init(struct drm_device *dev, struct mutex *lock) { static struct lock_class_key __key; __mutex_init((lock), "lock", &__key); .... } The static __key here is what lockdep uses to identify the lock class, however since this is just a normal function the key here will be created once, where all callers then use the same key. In effect the mutex->depmap.key will be the same pointer for different drmm_mutex_init() callers. This then results in impossible lockdep splats since lockdep thinks completely unrelated locks are the same lock class. To fix this turn drmm_mutex_init() into a macro such that it generates a different "static struct lock_class_key __key" for each invocation, which looks to be inline with what mutex_init() wants. v2: - Revamp the commit message with clearer explanation of the issue. - Rather export __drmm_mutex_release() than static inline. Reported-by: Thomas Hellström Reported-by: Sarah Walker Fixes: e13f13e039dc ("drm: Add DRM-managed mutex_init()") Cc: Stanislaw Gruszka Cc: Boris Brezillon Cc: Thomas Zimmermann Cc: Jocelyn Falempe Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matthew Auld Reviewed-by: Boris Brezillon Reviewed-by: Stanislaw Gruszka Reviewed-by: Lucas De Marchi Acked-by: Thomas Zimmermann Signed-off-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20230519090733.489019-1-matthew.auld@intel.com --- drivers/gpu/drm/drm_managed.c | 22 ++-------------------- include/drm/drm_managed.h | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c index 4cf214de50c4..c21c3f623033 100644 --- a/drivers/gpu/drm/drm_managed.c +++ b/drivers/gpu/drm/drm_managed.c @@ -264,28 +264,10 @@ void drmm_kfree(struct drm_device *dev, void *data) } EXPORT_SYMBOL(drmm_kfree); -static void drmm_mutex_release(struct drm_device *dev, void *res) +void __drmm_mutex_release(struct drm_device *dev, void *res) { struct mutex *lock = res; mutex_destroy(lock); } - -/** - * drmm_mutex_init - &drm_device-managed mutex_init() - * @dev: DRM device - * @lock: lock to be initialized - * - * Returns: - * 0 on success, or a negative errno code otherwise. - * - * This is a &drm_device-managed version of mutex_init(). The initialized - * lock is automatically destroyed on the final drm_dev_put(). - */ -int drmm_mutex_init(struct drm_device *dev, struct mutex *lock) -{ - mutex_init(lock); - - return drmm_add_action_or_reset(dev, drmm_mutex_release, lock); -} -EXPORT_SYMBOL(drmm_mutex_init); +EXPORT_SYMBOL(__drmm_mutex_release); diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h index 359883942612..ad08f834af40 100644 --- a/include/drm/drm_managed.h +++ b/include/drm/drm_managed.h @@ -105,6 +105,22 @@ char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp); void drmm_kfree(struct drm_device *dev, void *data); -int drmm_mutex_init(struct drm_device *dev, struct mutex *lock); +void __drmm_mutex_release(struct drm_device *dev, void *res); + +/** + * drmm_mutex_init - &drm_device-managed mutex_init() + * @dev: DRM device + * @lock: lock to be initialized + * + * Returns: + * 0 on success, or a negative errno code otherwise. + * + * This is a &drm_device-managed version of mutex_init(). The initialized + * lock is automatically destroyed on the final drm_dev_put(). + */ +#define drmm_mutex_init(dev, lock) ({ \ + mutex_init(lock); \ + drmm_add_action_or_reset(dev, __drmm_mutex_release, lock); \ +}) \ #endif -- cgit From 0e163e54c34c12369ccf6562e74e8f0a800f4aad Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Wed, 17 May 2023 10:56:05 -0600 Subject: accel/qaic: initialize ret variable to 0 clang static analysis reports drivers/accel/qaic/qaic_data.c:610:2: warning: Undefined or garbage value returned to caller [core.uninitialized.UndefReturn] return ret; ^~~~~~~~~~ From a code analysis of the function, the ret variable is only set some of the time but is always returned. This suggests ret can return uninitialized garbage. However BO allocation will ensure ret is always set in reality. Initialize ret to 0 to silence the warning. Fixes: ff13be830333 ("accel/qaic: Add datapath") Signed-off-by: Tom Rix [jhugo: Reword commit text] Signed-off-by: Jeffrey Hugo Reviewed-by: Carl Vanderlip Reviewed-by: Pranjal Ramajor Asha Kanojiya Link: https://patchwork.freedesktop.org/patch/msgid/20230517165605.16770-1-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index c0a574cd1b35..b46a16fb3080 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -591,7 +591,7 @@ static int qaic_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struc struct qaic_bo *bo = to_qaic_bo(obj); unsigned long offset = 0; struct scatterlist *sg; - int ret; + int ret = 0; if (obj->import_attach) return -EINVAL; -- cgit From d3b277b7aa1c74a65c84019b8fbe7856f841841a Mon Sep 17 00:00:00 2001 From: Pranjal Ramajor Asha Kanojiya Date: Wed, 17 May 2023 13:35:36 -0600 Subject: accel/qaic: Validate user data before grabbing any lock Validating user data does not need to be protected by any lock and it is safe to move it out of critical region. Fixes: ff13be830333 ("accel/qaic: Add datapath") Fixes: 129776ac2e38 ("accel/qaic: Add control path") Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Link: https://patchwork.freedesktop.org/patch/msgid/20230517193540.14323-2-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_control.c | 12 +++----- drivers/accel/qaic/qaic_data.c | 61 +++++++++++++++------------------------ 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c index 9f216eb6f76e..9e39b1a324f7 100644 --- a/drivers/accel/qaic/qaic_control.c +++ b/drivers/accel/qaic/qaic_control.c @@ -1249,7 +1249,7 @@ dma_cont_failed: int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { - struct qaic_manage_msg *user_msg; + struct qaic_manage_msg *user_msg = data; struct qaic_device *qdev; struct manage_msg *msg; struct qaic_user *usr; @@ -1258,6 +1258,9 @@ int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_ int usr_rcu_id; int ret; + if (user_msg->len > QAIC_MANAGE_MAX_MSG_LENGTH) + return -EINVAL; + usr = file_priv->driver_priv; usr_rcu_id = srcu_read_lock(&usr->qddev_lock); @@ -1275,13 +1278,6 @@ int qaic_manage_ioctl(struct drm_device *dev, void *data, struct drm_file *file_ return -ENODEV; } - user_msg = data; - - if (user_msg->len > QAIC_MANAGE_MAX_MSG_LENGTH) { - ret = -EINVAL; - goto out; - } - msg = kzalloc(QAIC_MANAGE_MAX_MSG_LENGTH + sizeof(*msg), GFP_KERNEL); if (!msg) { ret = -ENOMEM; diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index b46a16fb3080..5f71d76dd3a6 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -663,6 +663,10 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi if (args->pad) return -EINVAL; + size = PAGE_ALIGN(args->size); + if (size == 0) + return -EINVAL; + usr = file_priv->driver_priv; usr_rcu_id = srcu_read_lock(&usr->qddev_lock); if (!usr->qddev) { @@ -677,12 +681,6 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi goto unlock_dev_srcu; } - size = PAGE_ALIGN(args->size); - if (size == 0) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - bo = qaic_alloc_init_bo(); if (IS_ERR(bo)) { ret = PTR_ERR(bo); @@ -936,6 +934,22 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi struct qaic_bo *bo; int ret; + if (args->hdr.count == 0) + return -EINVAL; + + arg_size = args->hdr.count * sizeof(*slice_ent); + if (arg_size / args->hdr.count != sizeof(*slice_ent)) + return -EINVAL; + + if (args->hdr.size == 0) + return -EINVAL; + + if (!(args->hdr.dir == DMA_TO_DEVICE || args->hdr.dir == DMA_FROM_DEVICE)) + return -EINVAL; + + if (args->data == 0) + return -EINVAL; + usr = file_priv->driver_priv; usr_rcu_id = srcu_read_lock(&usr->qddev_lock); if (!usr->qddev) { @@ -950,43 +964,17 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi goto unlock_dev_srcu; } - if (args->hdr.count == 0) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - - arg_size = args->hdr.count * sizeof(*slice_ent); - if (arg_size / args->hdr.count != sizeof(*slice_ent)) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - if (args->hdr.dbc_id >= qdev->num_dbc) { ret = -EINVAL; goto unlock_dev_srcu; } - if (args->hdr.size == 0) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - - if (!(args->hdr.dir == DMA_TO_DEVICE || args->hdr.dir == DMA_FROM_DEVICE)) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - dbc = &qdev->dbc[args->hdr.dbc_id]; if (dbc->usr != usr) { ret = -EINVAL; goto unlock_dev_srcu; } - if (args->data == 0) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - user_data = u64_to_user_ptr(args->data); slice_ent = kzalloc(arg_size, GFP_KERNEL); @@ -1316,7 +1304,6 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr received_ts = ktime_get_ns(); size = is_partial ? sizeof(*pexec) : sizeof(*exec); - n = (unsigned long)size * args->hdr.count; if (args->hdr.count == 0 || n / args->hdr.count != size) return -EINVAL; @@ -1665,6 +1652,9 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file int rcu_id; int ret; + if (args->pad != 0) + return -EINVAL; + usr = file_priv->driver_priv; usr_rcu_id = srcu_read_lock(&usr->qddev_lock); if (!usr->qddev) { @@ -1679,11 +1669,6 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file goto unlock_dev_srcu; } - if (args->pad != 0) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - if (args->dbc_id >= qdev->num_dbc) { ret = -EINVAL; goto unlock_dev_srcu; -- cgit From 2e0904efc945ed5b04a9b251b9cefdd6754f5e15 Mon Sep 17 00:00:00 2001 From: Pranjal Ramajor Asha Kanojiya Date: Wed, 17 May 2023 13:35:37 -0600 Subject: accel/qaic: Validate if BO is sliced before slicing QAIC_ATTACH_SLICE_BO attaches slicing configuration to a BO. Validate if given BO is already sliced. An already sliced BO cannot be sliced again. Fixes: ff13be830333 ("accel/qaic: Add datapath") Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Link: https://patchwork.freedesktop.org/patch/msgid/20230517193540.14323-3-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_data.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index 5f71d76dd3a6..e672dd244703 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -1001,6 +1001,11 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi bo = to_qaic_bo(obj); + if (bo->sliced) { + ret = -EINVAL; + goto put_bo; + } + ret = qaic_prepare_bo(qdev, bo, &args->hdr); if (ret) goto put_bo; -- cgit From faa7c4eee412ca14dd6a107cb6122d31c4277cbe Mon Sep 17 00:00:00 2001 From: Pranjal Ramajor Asha Kanojiya Date: Wed, 17 May 2023 13:35:38 -0600 Subject: accel/qaic: Flush the transfer list again Before calling synchronize_srcu() we clear the transfer list, this is to allow all the QAIC_WAIT_BO callers to exit otherwise the system could deadlock. There could be a corner case where more elements get added to transfer list after we have flushed it. Re-flush the transfer list once all the holders of dbc->ch_lock have completed execution i.e. synchronize_srcu() is complete. Fixes: ff13be830333 ("accel/qaic: Add datapath") Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Link: https://patchwork.freedesktop.org/patch/msgid/20230517193540.14323-4-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_data.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index e672dd244703..c1af99cfd2d1 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -1845,6 +1845,11 @@ void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id) dbc->usr = NULL; empty_xfer_list(qdev, dbc); synchronize_srcu(&dbc->ch_lock); + /* + * Threads holding channel lock, may add more elements in the xfer_list. + * Flush out these elements from xfer_list. + */ + empty_xfer_list(qdev, dbc); } void release_dbc(struct qaic_device *qdev, u32 dbc_id) -- cgit From 75af0a585af93183ba68bb1b45d0d7a61e963712 Mon Sep 17 00:00:00 2001 From: Pranjal Ramajor Asha Kanojiya Date: Wed, 17 May 2023 13:35:39 -0600 Subject: accel/qaic: Grab ch_lock during QAIC_ATTACH_SLICE_BO During QAIC_ATTACH_SLICE_BO, we associate a BO to its DBC. We need to grab the dbc->ch_lock to make sure that DBC does not goes away while QAIC_ATTACH_SLICE_BO is still running. Fixes: ff13be830333 ("accel/qaic: Add datapath") Signed-off-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip Reviewed-by: Jeffrey Hugo Signed-off-by: Jeffrey Hugo Link: https://patchwork.freedesktop.org/patch/msgid/20230517193540.14323-5-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_data.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c index c1af99cfd2d1..e42c1f9ffff8 100644 --- a/drivers/accel/qaic/qaic_data.c +++ b/drivers/accel/qaic/qaic_data.c @@ -924,8 +924,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi { struct qaic_attach_slice_entry *slice_ent; struct qaic_attach_slice *args = data; + int rcu_id, usr_rcu_id, qdev_rcu_id; struct dma_bridge_chan *dbc; - int usr_rcu_id, qdev_rcu_id; struct drm_gem_object *obj; struct qaic_device *qdev; unsigned long arg_size; @@ -969,12 +969,6 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi goto unlock_dev_srcu; } - dbc = &qdev->dbc[args->hdr.dbc_id]; - if (dbc->usr != usr) { - ret = -EINVAL; - goto unlock_dev_srcu; - } - user_data = u64_to_user_ptr(args->data); slice_ent = kzalloc(arg_size, GFP_KERNEL); @@ -1006,9 +1000,16 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi goto put_bo; } + dbc = &qdev->dbc[args->hdr.dbc_id]; + rcu_id = srcu_read_lock(&dbc->ch_lock); + if (dbc->usr != usr) { + ret = -EINVAL; + goto unlock_ch_srcu; + } + ret = qaic_prepare_bo(qdev, bo, &args->hdr); if (ret) - goto put_bo; + goto unlock_ch_srcu; ret = qaic_attach_slicing_bo(qdev, bo, &args->hdr, slice_ent); if (ret) @@ -1018,6 +1019,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, args->hdr.dir); bo->dbc = dbc; + srcu_read_unlock(&dbc->ch_lock, rcu_id); drm_gem_object_put(obj); srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id); srcu_read_unlock(&usr->qddev_lock, usr_rcu_id); @@ -1026,6 +1028,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi unprepare_bo: qaic_unprepare_bo(qdev, bo); +unlock_ch_srcu: + srcu_read_unlock(&dbc->ch_lock, rcu_id); put_bo: drm_gem_object_put(obj); free_slice_ent: -- cgit From e997c218ad736fd6f524d73a987bad9d94128d3d Mon Sep 17 00:00:00 2001 From: Jeffrey Hugo Date: Wed, 17 May 2023 13:35:40 -0600 Subject: accel/qaic: Fix NNC message corruption If msg_xfer() is unable to queue part of a NNC message because the MHI ring is full, it will attempt to give the QSM some time to drain the queue. However, if QSM fails to make any room, msg_xfer() will fail and tell the caller to try again. This is problematic because part of the message may have been committed to the ring and there is no mechanism to revoke that content. This will cause QSM to receive a corrupt message. The better way to do this is to check if the ring has enough space for the entire message before committing any of the message. Since msg_xfer() is under the cntl_mutex no one else can come in and consume the space. Fixes: 129776ac2e38 ("accel/qaic: Add control path") Signed-off-by: Jeffrey Hugo Reviewed-by: Pranjal Ramajor Asha Kanojiya Reviewed-by: Carl Vanderlip Link: https://patchwork.freedesktop.org/patch/msgid/20230517193540.14323-6-quic_jhugo@quicinc.com --- drivers/accel/qaic/qaic_control.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c index 9e39b1a324f7..5c57f7b4494e 100644 --- a/drivers/accel/qaic/qaic_control.c +++ b/drivers/accel/qaic/qaic_control.c @@ -997,14 +997,34 @@ static void *msg_xfer(struct qaic_device *qdev, struct wrapper_list *wrappers, u struct xfer_queue_elem elem; struct wire_msg *out_buf; struct wrapper_msg *w; + long ret = -EAGAIN; + int xfer_count = 0; int retry_count; - long ret; if (qdev->in_reset) { mutex_unlock(&qdev->cntl_mutex); return ERR_PTR(-ENODEV); } + /* Attempt to avoid a partial commit of a message */ + list_for_each_entry(w, &wrappers->list, list) + xfer_count++; + + for (retry_count = 0; retry_count < QAIC_MHI_RETRY_MAX; retry_count++) { + if (xfer_count <= mhi_get_free_desc_count(qdev->cntl_ch, DMA_TO_DEVICE)) { + ret = 0; + break; + } + msleep_interruptible(QAIC_MHI_RETRY_WAIT_MS); + if (signal_pending(current)) + break; + } + + if (ret) { + mutex_unlock(&qdev->cntl_mutex); + return ERR_PTR(ret); + } + elem.seq_num = seq_num; elem.buf = NULL; init_completion(&elem.xfer_done); @@ -1038,16 +1058,9 @@ static void *msg_xfer(struct qaic_device *qdev, struct wrapper_list *wrappers, u list_for_each_entry(w, &wrappers->list, list) { kref_get(&w->ref_count); retry_count = 0; -retry: ret = mhi_queue_buf(qdev->cntl_ch, DMA_TO_DEVICE, &w->msg, w->len, list_is_last(&w->list, &wrappers->list) ? MHI_EOT : MHI_CHAIN); if (ret) { - if (ret == -EAGAIN && retry_count++ < QAIC_MHI_RETRY_MAX) { - msleep_interruptible(QAIC_MHI_RETRY_WAIT_MS); - if (!signal_pending(current)) - goto retry; - } - qdev->cntl_lost_buf = true; kref_put(&w->ref_count, free_wrapper); mutex_unlock(&qdev->cntl_mutex); -- cgit