[PATCH v2] dma-buf/fence-array: Add flex array to struct dma_fence_array
This is an effort to get rid of all multiplications from allocation functions in order to prevent integer overflows [1][2]. The "struct dma_fence_array" can be refactored to add a flex array in order to have the "callback structures allocated behind the array" be more explicit. Do so: - makes the code more readable and safer. - allows using __counted_by() for additional checks - avoids some pointer arithmetic in dma_fence_array_enable_signaling() Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1] Link: https://github.com/KSPP/linux/issues/160 [2] Signed-off-by: Christophe JAILLET Reviewed-by: Kees Cook --- Compile tested only. Changes in v2: - Name the new field 'callbacks' instead of 'cb' [Christian König] v1: https://lore.kernel.org/all/d3204a5b4776553455c2cfb1def72f1dae0dba25.1716054403.git.christophe.jail...@wanadoo.fr/ --- drivers/dma-buf/dma-fence-array.c | 10 -- include/linux/dma-fence-array.h | 3 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 9b3ce8948351..c74ac197d5fe 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -70,7 +70,7 @@ static void dma_fence_array_cb_func(struct dma_fence *f, static bool dma_fence_array_enable_signaling(struct dma_fence *fence) { struct dma_fence_array *array = to_dma_fence_array(fence); - struct dma_fence_array_cb *cb = (void *)([1]); + struct dma_fence_array_cb *cb = array->callbacks; unsigned i; for (i = 0; i < array->num_fences; ++i) { @@ -168,22 +168,20 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, bool signal_on_any) { struct dma_fence_array *array; - size_t size = sizeof(*array); WARN_ON(!num_fences || !fences); - /* Allocate the callback structures behind the array. */ - size += num_fences * sizeof(struct dma_fence_array_cb); - array = kzalloc(size, GFP_KERNEL); + array = kzalloc(struct_size(array, callbacks, num_fences), GFP_KERNEL); if (!array) return NULL; + array->num_fences = num_fences; + spin_lock_init(>lock); dma_fence_init(>base, _fence_array_ops, >lock, context, seqno); init_irq_work(>work, irq_dma_fence_array_work); - array->num_fences = num_fences; atomic_set(>num_pending, signal_on_any ? 1 : num_fences); array->fences = fences; diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index ec7f25def392..29c5650c1038 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -33,6 +33,7 @@ struct dma_fence_array_cb { * @num_pending: fences in the array still pending * @fences: array of the fences * @work: internal irq_work function + * @callbacks: array of callback helpers */ struct dma_fence_array { struct dma_fence base; @@ -43,6 +44,8 @@ struct dma_fence_array { struct dma_fence **fences; struct irq_work work; + + struct dma_fence_array_cb callbacks[] __counted_by(num_fences); }; /** -- 2.45.1
[PATCH] drm/panel: lg-sw43408: Fix an error handling path in sw43408_probe()
If mipi_dsi_attach() fails, we must undo the drm_panel_add() call hidden in sw43408_add(), as already done in the remove function. Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver") Signed-off-by: Christophe JAILLET --- Compile tested only --- drivers/gpu/drm/panel/panel-lg-sw43408.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c index 115f4702d59f..27f2ad788d38 100644 --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c @@ -286,7 +286,15 @@ static int sw43408_probe(struct mipi_dsi_device *dsi) dsi->dsc = >dsc; - return mipi_dsi_attach(dsi); + ret = mipi_dsi_attach(dsi); + if (ret < 0) + goto err_remove_panel; + + return 0; + +err_remove_panel: + drm_panel_remove(>base); + return ret; } static void sw43408_remove(struct mipi_dsi_device *dsi) -- 2.45.1
[PATCH] drm: zynqmp_dpsub: Fix an error handling path in zynqmp_dpsub_probe()
If zynqmp_dpsub_drm_init() fails, we must undo the previous drm_bridge_add() call. Fixes: be3f3042391d ("drm: zynqmp_dpsub: Always register bridge") Signed-off-by: Christophe JAILLET --- Compile tested only --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index face8d6b2a6f..f5781939de9c 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -269,6 +269,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev) return 0; err_disp: + drm_bridge_remove(dpsub->bridge); zynqmp_disp_remove(dpsub); err_dp: zynqmp_dp_remove(dpsub); -- 2.45.1
[PATCH] dma-buf/fence-array: Add flex array to struct dma_fence_array
This is an effort to get rid of all multiplications from allocation functions in order to prevent integer overflows [1][2]. The "struct dma_fence_array" can be refactored to add a flex array in order to have the "callback structures allocated behind the array" be more explicit. Do so: - makes the code more readable and safer. - allows using __counted_by() for additional checks - avoids some pointer arithmetic in dma_fence_array_enable_signaling() Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1] Link: https://github.com/KSPP/linux/issues/160 [2] Signed-off-by: Christophe JAILLET --- Compile tested only. Also, I don't think that 'cb' is a great name and the associated kernel-doc description could certainly be improved. Any proposal welcomed :) --- drivers/dma-buf/dma-fence-array.c | 10 -- include/linux/dma-fence-array.h | 3 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 9b3ce8948351..9c55afaca607 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -70,7 +70,7 @@ static void dma_fence_array_cb_func(struct dma_fence *f, static bool dma_fence_array_enable_signaling(struct dma_fence *fence) { struct dma_fence_array *array = to_dma_fence_array(fence); - struct dma_fence_array_cb *cb = (void *)([1]); + struct dma_fence_array_cb *cb = array->cb; unsigned i; for (i = 0; i < array->num_fences; ++i) { @@ -168,22 +168,20 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, bool signal_on_any) { struct dma_fence_array *array; - size_t size = sizeof(*array); WARN_ON(!num_fences || !fences); - /* Allocate the callback structures behind the array. */ - size += num_fences * sizeof(struct dma_fence_array_cb); - array = kzalloc(size, GFP_KERNEL); + array = kzalloc(struct_size(array, cb, num_fences), GFP_KERNEL); if (!array) return NULL; + array->num_fences = num_fences; + spin_lock_init(>lock); dma_fence_init(>base, _fence_array_ops, >lock, context, seqno); init_irq_work(>work, irq_dma_fence_array_work); - array->num_fences = num_fences; atomic_set(>num_pending, signal_on_any ? 1 : num_fences); array->fences = fences; diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index ec7f25def392..a793f9d5c73b 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -33,6 +33,7 @@ struct dma_fence_array_cb { * @num_pending: fences in the array still pending * @fences: array of the fences * @work: internal irq_work function + * @cb: array of callback helpers */ struct dma_fence_array { struct dma_fence base; @@ -43,6 +44,8 @@ struct dma_fence_array { struct dma_fence **fences; struct irq_work work; + + struct dma_fence_array_cb cb[] __counted_by(num_fences); }; /** -- 2.45.1
Re: [PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
(adding linux-harden...@vger.kernel.org) Le 18/05/2024 à 16:37, Guenter Roeck a écrit : Trying to build parisc:allmodconfig with gcc 12.x or later results in the following build error. drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd': drivers/gpu/drm/nouveau/nvif/object.c:161:9: error: 'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32 overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict] 161 | memcpy(data, args->mthd.data, size); | ^~~ drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor': drivers/gpu/drm/nouveau/nvif/object.c:298:17: error: 'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56 overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict] 298 | memcpy(data, args->new.data, size); gcc assumes that 'sizeof(*args) + size' can overflow, which would result in the problem. The problem is not new, only it is now no longer a warning but an error since W=1 has been enabled for the drm subsystem and since Werror is enabled for test builds. Rearrange arithmetic and add extra size checks to avoid the overflow. Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the subsystem") Cc: Javier Martinez Canillas Cc: Jani Nikula Cc: Thomas Zimmermann Cc: Danilo Krummrich Cc: Maxime Ripard Signed-off-by: Guenter Roeck --- checkpatch complains about the line length in the description and the (pre-existing) assignlemts in if conditions, but I did not want to split lines in the description or rearrange the code further. I don't know why I only see the problem with parisc builds (at least so far). drivers/gpu/drm/nouveau/nvif/object.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c index 4d1aaee8fe15..baf623a48874 100644 --- a/drivers/gpu/drm/nouveau/nvif/object.c +++ b/drivers/gpu/drm/nouveau/nvif/object.c @@ -145,8 +145,9 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void *data, u32 size) u8 stack[128]; int ret; - if (sizeof(*args) + size > sizeof(stack)) { - if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) + if (size > sizeof(stack) - sizeof(*args)) { + if (size > INT_MAX || + !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) Hi, Would it be cleaner or better to use size_add(sizeof(*args), size)? return -ENOMEM; } else { args = (void *)stack; @@ -276,7 +277,8 @@ nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle, object->map.size = 0; if (parent) { - if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) { + if (size > INT_MAX || + !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) { Same. CJ nvif_object_dtor(object); return -ENOMEM; }
Re: [PATCH v3 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 16/04/2024 à 20:30, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 367 ++ 3 files changed, 382 insertions(+) ... +static int rm69380_probe(struct mipi_dsi_device *dsi) +{ + struct mipi_dsi_host *dsi_sec_host; + struct rm69380_panel *ctx; + struct device *dev = >dev; + struct device_node *dsi_sec; + int ret, i; + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->supplies[0].supply = "vddio"; + ctx->supplies[1].supply = "avdd"; + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies), + ctx->supplies); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to get regulators\n"); + + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(ctx->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), +"Failed to get reset-gpios\n"); + + dsi_sec = of_graph_get_remote_node(dsi->dev.of_node, 1, -1); + + if (dsi_sec) { + const struct mipi_dsi_device_info info = { "RM69380", 0, + dsi_sec }; + + dev_dbg(dev, "Using Dual-DSI: found `%s`\n", dsi_sec->name); + + dsi_sec_host = of_find_mipi_dsi_host_by_node(dsi_sec); + of_node_put(dsi_sec); + if (!dsi_sec_host) + return dev_err_probe(dev, -EPROBE_DEFER, +"Cannot get secondary DSI host\n"); + + ctx->dsi[1] = + devm_mipi_dsi_device_register_full(dev, dsi_sec_host, ); + if (IS_ERR(ctx->dsi[1])) + return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]), +"Cannot get secondary DSI node\n"); + + dev_dbg(dev, "Second DSI name `%s`\n", ctx->dsi[1]->name); + mipi_dsi_set_drvdata(ctx->dsi[1], ctx); + } else { + dev_dbg(dev, "Using Single-DSI\n"); + } + + ctx->dsi[0] = dsi; + mipi_dsi_set_drvdata(dsi, ctx); + + drm_panel_init(>panel, dev, _panel_funcs, + DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; + + ctx->panel.backlight = rm69380_create_backlight(dsi); + if (IS_ERR(ctx->panel.backlight)) + return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight), +"Failed to create backlight\n"); + + drm_panel_add(>panel); + + for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) { + if (!ctx->dsi[i]) + continue; + + dev_dbg(>dsi[i]->dev, "Binding DSI %d\n", i); + + ctx->dsi[i]->lanes = 4; + ctx->dsi[i]->format = MIPI_DSI_FMT_RGB888; + ctx->dsi[i]->mode_flags = MIPI_DSI_MODE_VIDEO_BURST | + MIPI_DSI_CLOCK_NON_CONTINUOUS; + + ret = mipi_dsi_attach(ctx->dsi[i]); + if (ret < 0) { + mipi_dsi_detach(ctx->dsi[i]); I don't think this works. if 'i' fails, you have to detach 0...i-1 maybe something like below (complety untested). + drm_panel_remove(>panel); + return dev_err_probe(dev, ret, +"Failed to attach to DSI%d\n", i); + } + } + + return 0; +} drm_panel_add(>panel); for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) { if (!ctx->dsi[i]) continue; dev_dbg(>dsi[i]->dev, "Binding DSI %d\n", i); ctx->dsi[i]->lanes = 4; ctx->dsi[i]->format = MIPI_DSI_FMT_RGB888; ctx->dsi[i]->mode_flags = MIPI_DSI_MODE_VIDEO_BURST | MIPI_DSI_CLOCK_NON_CONTINUOUS; ret = mipi_dsi_attach(ctx->dsi[i]); if (ret < 0) { dev_err_probe(dev, ret, "Failed to attach to DSI%d\n", i); goto err_detach_dsi; } } return 0; err_detach_dsi: while (--i >= 0) if (ctx->dsi[i]) mipi_dsi_detach(ctx->dsi[i]); drm_panel_remove(>panel); return ret; }
Re: [PATCH v2 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 15/04/2024 à 18:10, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366 ++ 3 files changed, 381 insertions(+) ... +static int rm69380_on(struct rm69380_panel *ctx) +{ + struct mipi_dsi_device *dsi = ctx->dsi[0]; + struct device *dev = >dev; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + if (ctx->dsi[1]) + ctx->dsi[1]->mode_flags |= MIPI_DSI_MODE_LPM; + + mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd4); + mipi_dsi_dcs_write_seq(dsi, 0x00, 0x80); + mipi_dsi_dcs_write_seq(dsi, 0xfe, 0xd0); + mipi_dsi_dcs_write_seq(dsi, 0x48, 0x00); + mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x26); + mipi_dsi_dcs_write_seq(dsi, 0x75, 0x3f); + mipi_dsi_dcs_write_seq(dsi, 0x1d, 0x1a); + mipi_dsi_dcs_write_seq(dsi, 0xfe, 0x00); + mipi_dsi_dcs_write_seq(dsi, 0x53, 0x28); + mipi_dsi_dcs_write_seq(dsi, 0xc2, 0x08); + mipi_dsi_dcs_write_seq(dsi, 0x35, 0x00); + mipi_dsi_dcs_write_seq(dsi, 0x51, 0x07, 0xff); + + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); + if (ret < 0) { + dev_err(dev, "Failed to exit sleep mode: %d\n", ret); + return ret; + } + msleep(20); + + ret = mipi_dsi_dcs_set_display_on(dsi); + if (ret < 0) { + dev_err(dev, "Failed to set display on: %d\n", ret); + return ret; + } + msleep(36); 36 and 35 below are un-usual values for msleep. Why 2 different values? Would using a #define for this(these) value(s) make sense here? + + return 0; +} + +static int rm69380_off(struct rm69380_panel *ctx) +{ + struct mipi_dsi_device *dsi = ctx->dsi[0]; + struct device *dev = >dev; + int ret; + + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + if (ctx->dsi[1]) + ctx->dsi[1]->mode_flags &= ~MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_set_display_off(dsi); + if (ret < 0) { + dev_err(dev, "Failed to set display off: %d\n", ret); + return ret; + } + msleep(35); (here) + + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + if (ret < 0) { + dev_err(dev, "Failed to enter sleep mode: %d\n", ret); + return ret; + } + msleep(20); + + return 0; +} ... +static int rm69380_probe(struct mipi_dsi_device *dsi) +{ + struct mipi_dsi_host *dsi_sec_host; + struct rm69380_panel *ctx; + struct device *dev = >dev; + struct device_node *dsi_sec; + int ret, i; + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->supplies[0].supply = "vddio"; + ctx->supplies[1].supply = "avdd"; + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies), + ctx->supplies); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to get regulators\n"); + + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(ctx->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), +"Failed to get reset-gpios\n"); + + dsi_sec = of_graph_get_remote_node(dsi->dev.of_node, 1, -1); + + if (dsi_sec) { + dev_dbg(dev, "Using Dual-DSI\n"); This should be after de 'info' variable below, so... + + const struct mipi_dsi_device_info info = { "RM69380", 0, + dsi_sec }; + + dev_dbg(dev, "Found second DSI `%s`\n", dsi_sec->name); ... maybe merge the 2 messages into something like: dev_dbg(dev, "Using Dual-DSI: found `%s`\n", dsi_sec->name); + + dsi_sec_host = of_find_mipi_dsi_host_by_node(dsi_sec); + of_node_put(dsi_sec); + if (!dsi_sec_host) { + return dev_err_probe(dev, -EPROBE_DEFER, +"Cannot get secondary DSI host\n"); + } + Nit: unneeded { } + ctx->dsi[1] = + mipi_dsi_device_register_full(dsi_sec_host, ); + if (IS_ERR(ctx->dsi[1])) { + return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]), +"Cannot get secondary DSI node\n"); + } Nit: unneeded { } + + dev_dbg(dev, "Second DSI name `%s`\n", ctx->dsi[1]->name); + mipi_dsi_set_drvdata(ctx->dsi[1], ctx); + } else { + dev_dbg(dev, "Using Single-DSI\n"); + } + +
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 15/04/2024 à 07:37, david-vu3dztd92roxwddmvfq...@public.gmane.org a écrit : W dniu 2024-04-14 22:22, Christophe JAILLET napisał(a): Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? Yes, this is just one of the examples where this driver IC can be found. It can also be used with panels other than those from EDO. Hi, sorry if i was unclear. Is there a typo: s/combiantion/combination/ ? CJ + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF Best regards, David Wronek
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF
Re: [PATCH] drm/i915/guc: Remove usage of the deprecated ida_simple_xx() API
Le 25/01/2024 à 01:04, Matthew Brost a écrit : On Sun, Jan 14, 2024 at 04:15:34PM +0100, Christophe JAILLET wrote: ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_range() is inclusive. So a -1 has been added when needed. Signed-off-by: Christophe JAILLET Reviewed-by: Matthew Brost Hi, polite reminder ;-) CJ --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index a259f1118c5a..73ce21ddf682 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2156,11 +2156,10 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) order_base_2(ce->parallel.number_children + 1)); else - ret = ida_simple_get(>submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID(guc), -guc->submission_state.num_guc_ids, -GFP_KERNEL | __GFP_RETRY_MAYFAIL | -__GFP_NOWARN); + ret = ida_alloc_range(>submission_state.guc_ids, + NUMBER_MULTI_LRC_GUC_ID(guc), + guc->submission_state.num_guc_ids - 1, + GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) return ret; @@ -2183,8 +2182,8 @@ static void __release_guc_id(struct intel_guc *guc, struct intel_context *ce) + 1)); } else { --guc->submission_state.guc_ids_in_use; - ida_simple_remove(>submission_state.guc_ids, - ce->guc_id.id); + ida_free(>submission_state.guc_ids, +ce->guc_id.id); } clr_ctx_id_mapping(guc, ce->guc_id.id); set_context_guc_id_invalid(ce); -- 2.43.0
Re: [PATCH v2] treewide: Fix common grammar mistake "the the"
Le 11/04/2024 à 19:11, Thorsten Blum a écrit : Use `find . -type f -exec sed -i 's/\/the/g' {} +` to find all occurrences of "the the" and replace them with a single "the". In arch/arm/include/asm/unwind.h replace "the the" with "to the". Changes only comments and documentation - no code changes. Signed-off-by: Thorsten Blum Reviewed-by: Randy Dunlap Reviewed-by: Tyler Hicks ... --- a/drivers/scsi/isci/host.h +++ b/drivers/scsi/isci/host.h @@ -244,7 +244,7 @@ enum sci_controller_states { SCIC_INITIALIZED, /** -* This state indicates the the controller is in the process of becoming maybe: that the? +* This state indicates the controller is in the process of becoming * ready (i.e. starting). In this state no new IO operations are permitted. * This state is entered from the INITIALIZED state. */ ... diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c index 3aa16e27f509..503244e8470a 100644 --- a/io_uring/kbuf.c +++ b/io_uring/kbuf.c @@ -731,7 +731,7 @@ struct io_buffer_list *io_pbuf_get_bl(struct io_ring_ctx *ctx, * going away, if someone is trying to be sneaky. Look it up under rcu * so we know it's not going away, and attempt to grab a reference to * it. If the ref is already zero, then fail the mapping. If successful, -* the caller will call io_put_bl() to drop the the reference at at the +* the caller will call io_put_bl() to drop the reference at at the Not strictly related to your patch, but "at at". * end. This may then safely free the buffer_list (and drop the pages) * at that point, vm_insert_pages() would've already grabbed the * necessary vma references. ... CJ
Re: [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function
Le 23/03/2024 à 22:25, Marek Behún a écrit : On Sat, 23 Mar 2024 22:10:40 +0100 Christophe JAILLET wrote: ... static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev) { - pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL); + pvt->dbgfs_dir = devm_debugfs_create_dir(dev, dev_name(dev), NULL); + if (IS_ERR(pvt->dbgfs_dir)) + return PTR_ERR(pvt->dbgfs_dir); Not sure if the test and error handling should be added here. *If I'm correct*, functions related to debugfs already handle this case and just do nothing. And failure in debugfs related code is not considered as something that need to be reported and abort a probe function. Maybe the same other (already existing) tests in this patch should be removed as well, in a separated patch. Functions related to debugfs maybe do, but devm_ resource management functions may fail to allocate release structure, and those errors need to be handled, AFAIK. I would say no. If this memory allocation fails, then debugfs_create_dir() will not be called, but that's not a really big deal if the driver itself can still run normally without it. Up to you to leave it as-is or remove what I think is a useless error handling. At least, maybe it could be said in the commit log, so that maintainers can comment on it, if they don't spot the error handling you introduce. CJ Marek
Re: [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function
Le 23/03/2024 à 17:43, Marek Behún a écrit : A few drivers register a devm action to remove a debugfs directory, implementing a one-liner function that calls debufs_remove_recursive(). Help drivers avoid this repeated implementations by adding managed version of debugfs directory create function. Use the new function devm_debugfs_create_dir() in the following drivers: drivers/crypto/caam/ctrl.c drivers/gpu/drm/bridge/ti-sn65dsi86.c drivers/hwmon/hp-wmi-sensors.c drivers/hwmon/mr75203.c drivers/hwmon/pmbus/pmbus_core.c Also use the action function devm_debugfs_dir_recursive_drop() in driver drivers/gpio/gpio-mockup.c As per Dan Williams' request [1], do not touch the driver drivers/cxl/mem.c [1] https://lore.kernel.org/linux-gpio/65d7918b358a5_1ee3129...@dwillia2-mobl3.amr.corp.intel.com.notmuch/ Signed-off-by: Marek Behún --- drivers/crypto/caam/ctrl.c| 16 +++ drivers/gpio/gpio-mockup.c| 11 ++-- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 13 ++--- drivers/hwmon/hp-wmi-sensors.c| 15 ++ drivers/hwmon/mr75203.c | 15 -- drivers/hwmon/pmbus/pmbus_core.c | 16 --- include/linux/devm-helpers.h | 40 +++ 7 files changed, 61 insertions(+), 65 deletions(-) ... diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c index 455eecf6380e..adbe0fe09490 100644 --- a/drivers/gpio/gpio-mockup.c +++ b/drivers/gpio/gpio-mockup.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -390,13 +391,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev, } } -static void gpio_mockup_debugfs_cleanup(void *data) -{ - struct gpio_mockup_chip *chip = data; - - debugfs_remove_recursive(chip->dbg_dir); -} - static void gpio_mockup_dispose_mappings(void *data) { struct gpio_mockup_chip *chip = data; @@ -480,7 +474,8 @@ static int gpio_mockup_probe(struct platform_device *pdev) gpio_mockup_debugfs_setup(dev, chip); - return devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip); + return devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, + chip->dbg_dir); This look strange. Shouldn't the debugfs_create_dir() call in gpio_mockup_debugfs_setup() be changed, instead? (I've not look in the previous version if something was said about it, so apologies if already discussed) } static const struct of_device_id gpio_mockup_of_match[] = { ... diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c index 50a8b9c3f94d..50f348fca108 100644 --- a/drivers/hwmon/mr75203.c +++ b/drivers/hwmon/mr75203.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -216,17 +217,11 @@ static const struct file_operations pvt_ts_coeff_j_fops = { .llseek = default_llseek, }; -static void devm_pvt_ts_dbgfs_remove(void *data) -{ - struct pvt_device *pvt = (struct pvt_device *)data; - - debugfs_remove_recursive(pvt->dbgfs_dir); - pvt->dbgfs_dir = NULL; -} - static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev) { - pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL); + pvt->dbgfs_dir = devm_debugfs_create_dir(dev, dev_name(dev), NULL); + if (IS_ERR(pvt->dbgfs_dir)) + return PTR_ERR(pvt->dbgfs_dir); Not sure if the test and error handling should be added here. *If I'm correct*, functions related to debugfs already handle this case and just do nothing. And failure in debugfs related code is not considered as something that need to be reported and abort a probe function. Maybe the same other (already existing) tests in this patch should be removed as well, in a separated patch. just my 2c CJ debugfs_create_u32("ts_coeff_h", 0644, pvt->dbgfs_dir, >ts_coeff.h); @@ -237,7 +232,7 @@ static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev) debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt, _ts_coeff_j_fops); - return devm_add_action_or_reset(dev, devm_pvt_ts_dbgfs_remove, pvt); + return 0; } ...
[PATCH] drm/bridge: lt9611uxc: Fix an error handling path in lt9611uxc_probe()
If lt9611uxc_audio_init() fails, some resources still need to be released before returning the error code. Use the existing error handling path. Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge") Signed-off-by: Christophe JAILLET --- Compile tested only. --- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index f4f593ad8f79..d0c77630a2f9 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -965,7 +965,11 @@ static int lt9611uxc_probe(struct i2c_client *client) } } - return lt9611uxc_audio_init(dev, lt9611uxc); + ret = lt9611uxc_audio_init(dev, lt9611uxc); + if (ret) + goto err_remove_bridge; + + return 0; err_remove_bridge: free_irq(client->irq, lt9611uxc); -- 2.44.0
[PATCH] drm/i915/display: Save a few bytes of memory in intel_backlight_device_register()
'name' may still be "intel_backlight" when backlight_device_register() is called. In such a case, using kstrdup_const() saves a memory duplication when dev_set_name() is called in backlight_device_register(). Use kfree_const() accordingly. Signed-off-by: Christophe JAILLET --- Compile tested only --- drivers/gpu/drm/i915/display/intel_backlight.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c index 1946d7fb3c2e..9e4a9d4f1585 100644 --- a/drivers/gpu/drm/i915/display/intel_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_backlight.c @@ -949,7 +949,7 @@ int intel_backlight_device_register(struct intel_connector *connector) else props.power = FB_BLANK_POWERDOWN; - name = kstrdup("intel_backlight", GFP_KERNEL); + name = kstrdup_const("intel_backlight", GFP_KERNEL); if (!name) return -ENOMEM; @@ -963,7 +963,7 @@ int intel_backlight_device_register(struct intel_connector *connector) * compatibility. Use unique names for subsequent backlight devices as a * fallback when the default name already exists. */ - kfree(name); + kfree_const(name); name = kasprintf(GFP_KERNEL, "card%d-%s-backlight", i915->drm.primary->index, connector->base.name); if (!name) @@ -987,7 +987,7 @@ int intel_backlight_device_register(struct intel_connector *connector) connector->base.base.id, connector->base.name, name); out: - kfree(name); + kfree_const(name); return ret; } -- 2.43.2
Re: [PATCH] drm/xe/guc: Remove usage of the deprecated ida_simple_xx() API
Le 14/01/2024 à 16:09, Christophe JAILLET a écrit : ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_max() is inclusive. So a -1 has been added when needed. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/xe/xe_guc_submit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index 21ac68e3246f..11ffacd1dd58 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -311,7 +311,7 @@ static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q, u32 xa q->guc->id - GUC_ID_START_MLRC, order_base_2(q->width)); else - ida_simple_remove(>submission_state.guc_ids, q->guc->id); + ida_free(>submission_state.guc_ids, q->guc->id); } static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) @@ -335,8 +335,8 @@ static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) ret = bitmap_find_free_region(bitmap, GUC_ID_NUMBER_MLRC, order_base_2(q->width)); } else { - ret = ida_simple_get(>submission_state.guc_ids, 0, -GUC_ID_NUMBER_SLRC, GFP_NOWAIT); + ret = ida_alloc_max(>submission_state.guc_ids, + GUC_ID_NUMBER_SLRC - 1, GFP_NOWAIT); } if (ret < 0) return ret; Hi, gentle reminder. All patches to remove the ida_simple API have been sent. And Matthew Wilcox seems happy with the on going work. (see [1]) Based on next-20240220 $git grep ida_simple_get | wc -l 36 https://elixir.bootlin.com/linux/v6.8-rc3/A/ident/ida_simple_get 50 https://elixir.bootlin.com/linux/v6.7.4/A/ident/ida_simple_get 81 Thanks CJ [1]: https://lore.kernel.org/all/zaqrugvz734zj...@casper.infradead.org/
Re: [PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory
Le 19/02/2024 à 09:37, Dan Carpenter a écrit : On Sun, Feb 18, 2024 at 06:46:44PM +0100, Christophe JAILLET wrote: If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. The "list_limit" is set via module parameter so if you set that high enough to lead to an integer overflow then you kind of deserve what you get. This patch is nice for kernel hardening and making the code easier to read/audit but the real world security impact is negligible. Agreed. That is what I meant by "and unlikely". Maybe the commit message could be more explicit if needed. Let me know if ok as-is or if I should try to re-word the description. CJ regards, dan carpenter
[PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory
If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. In such a case, udmabuf_create() would access to memory beyond 'list'. Use memdup_array_user() which checks for overflow. While at it, include . Fixes: fbb0de795078 ("Add udmabuf misc device") Signed-off-by: Christophe JAILLET --- v2: - Use memdup_array_user() [Kees Cook] - Use sizeof(*list) [Gustavo A. R. Silva] - Add include v1: https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.1695060389.git.christophe.jail...@wanadoo.fr/ Signed-off-by: Christophe JAILLET --- drivers/dma-buf/udmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..5728948ea6f2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; if (copy_from_user(, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; - list = memdup_user((void __user *)(arg + sizeof(head)), lsize); + list = memdup_array_user((void __user *)(arg + sizeof(head)), +sizeof(*list), head.count); if (IS_ERR(list)) return PTR_ERR(list); -- 2.43.2
Re: [PATCH V8 02/12] phy: freescale: add Samsung HDMI PHY
Le 03/02/2024 à 17:52, Adam Ford a écrit : From: Lucas Stach This adds the driver for the Samsung HDMI PHY found on the i.MX8MP SoC. Signed-off-by: Lucas Stach Signed-off-by: Adam Ford Tested-by: Alexander Stein --- ... +static int fsl_samsung_hdmi_phy_probe(struct platform_device *pdev) +{ + struct fsl_samsung_hdmi_phy *phy; + int ret; + + phy = devm_kzalloc(>dev, sizeof(*phy), GFP_KERNEL); + if (!phy) + return -ENOMEM; + + platform_set_drvdata(pdev, phy); + phy->dev = >dev; + + phy->regs = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(phy->regs)) + return PTR_ERR(phy->regs); + + phy->apbclk = devm_clk_get(phy->dev, "apb"); + if (IS_ERR(phy->apbclk)) + return dev_err_probe(phy->dev, PTR_ERR(phy->apbclk), +"failed to get apb clk\n"); + + phy->refclk = devm_clk_get(phy->dev, "ref"); + if (IS_ERR(phy->refclk)) + return dev_err_probe(phy->dev, PTR_ERR(phy->refclk), +"failed to get ref clk\n"); + + ret = clk_prepare_enable(phy->apbclk); + if (ret) { + dev_err(phy->dev, "failed to enable apbclk\n"); + return ret; + } + + pm_runtime_get_noresume(phy->dev); + pm_runtime_set_active(phy->dev); + pm_runtime_enable(phy->dev); + + ret = phy_clk_register(phy); + if (ret) { + dev_err(>dev, "register clk failed\n"); + goto register_clk_failed; + } + + pm_runtime_put(phy->dev); + + return 0; + +register_clk_failed: + clk_disable_unprepare(phy->apbclk); + + return ret; +} + +static int fsl_samsung_hdmi_phy_remove(struct platform_device *pdev) +{ + of_clk_del_provider(pdev->dev.of_node); A clk_disable_unprepare(phy->apbclk) call seems to be missing here. Or maybe devm_clk_get_enabled() should be used for 'apbclk'? CJ + + return 0; +} ...
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
Le 18/01/2024 à 18:32, Duje Mihanović a écrit : Add driver for the Kinetic KTD2801 backlight driver. Signed-off-by: Duje Mihanović --- ... + ktd2801->gpiod = devm_gpiod_get(dev, "ctrl", GPIOD_OUT_HIGH); + if (IS_ERR(ktd2801->gpiod)) + return dev_err_probe(dev, PTR_ERR(dev), PTR_ERR(ktd2801->gpiod); ? CJ
[PATCH] drm/i915/guc: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_range() is inclusive. So a -1 has been added when needed. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index a259f1118c5a..73ce21ddf682 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2156,11 +2156,10 @@ static int new_guc_id(struct intel_guc *guc, struct intel_context *ce) order_base_2(ce->parallel.number_children + 1)); else - ret = ida_simple_get(>submission_state.guc_ids, -NUMBER_MULTI_LRC_GUC_ID(guc), -guc->submission_state.num_guc_ids, -GFP_KERNEL | __GFP_RETRY_MAYFAIL | -__GFP_NOWARN); + ret = ida_alloc_range(>submission_state.guc_ids, + NUMBER_MULTI_LRC_GUC_ID(guc), + guc->submission_state.num_guc_ids - 1, + GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); if (unlikely(ret < 0)) return ret; @@ -2183,8 +2182,8 @@ static void __release_guc_id(struct intel_guc *guc, struct intel_context *ce) + 1)); } else { --guc->submission_state.guc_ids_in_use; - ida_simple_remove(>submission_state.guc_ids, - ce->guc_id.id); + ida_free(>submission_state.guc_ids, +ce->guc_id.id); } clr_ctx_id_mapping(guc, ce->guc_id.id); set_context_guc_id_invalid(ce); -- 2.43.0
[PATCH] drm/amdgpu: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_range() is inclusive. So a -1 has been added when needed. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index ddd0891da116..3d7fcdeaf8cf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -62,9 +62,8 @@ int amdgpu_pasid_alloc(unsigned int bits) int pasid = -EINVAL; for (bits = min(bits, 31U); bits > 0; bits--) { - pasid = ida_simple_get(_pasid_ida, - 1U << (bits - 1), 1U << bits, - GFP_KERNEL); + pasid = ida_alloc_range(_pasid_ida, 1U << (bits - 1), + (1U << bits) - 1, GFP_KERNEL); if (pasid != -ENOSPC) break; } @@ -82,7 +81,7 @@ int amdgpu_pasid_alloc(unsigned int bits) void amdgpu_pasid_free(u32 pasid) { trace_amdgpu_pasid_freed(pasid); - ida_simple_remove(_pasid_ida, pasid); + ida_free(_pasid_ida, pasid); } static void amdgpu_pasid_free_cb(struct dma_fence *fence, -- 2.43.0
[PATCH] drm/xe/guc: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated ida_simple_get() and ida_simple_remove(). Note that the upper limit of ida_simple_get() is exclusive, but the one of ida_alloc_max() is inclusive. So a -1 has been added when needed. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/xe/xe_guc_submit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index 21ac68e3246f..11ffacd1dd58 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -311,7 +311,7 @@ static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q, u32 xa q->guc->id - GUC_ID_START_MLRC, order_base_2(q->width)); else - ida_simple_remove(>submission_state.guc_ids, q->guc->id); + ida_free(>submission_state.guc_ids, q->guc->id); } static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) @@ -335,8 +335,8 @@ static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) ret = bitmap_find_free_region(bitmap, GUC_ID_NUMBER_MLRC, order_base_2(q->width)); } else { - ret = ida_simple_get(>submission_state.guc_ids, 0, -GUC_ID_NUMBER_SLRC, GFP_NOWAIT); + ret = ida_alloc_max(>submission_state.guc_ids, + GUC_ID_NUMBER_SLRC - 1, GFP_NOWAIT); } if (ret < 0) return ret; -- 2.43.0
[PATCH] drm/amd/display: Fix a switch statement in populate_dml_output_cfg_from_stream_state()
It is likely that the statement related to 'dml_edp' is misplaced. So move it in the correct "case SIGNAL_TYPE_EDP". Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c index fa6a93dd9629..64d01a9cd68c 100644 --- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c +++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c @@ -626,8 +626,8 @@ static void populate_dml_output_cfg_from_stream_state(struct dml_output_cfg_st * if (is_dp2p0_output_encoder(pipe)) out->OutputEncoder[location] = dml_dp2p0; break; - out->OutputEncoder[location] = dml_edp; case SIGNAL_TYPE_EDP: + out->OutputEncoder[location] = dml_edp; break; case SIGNAL_TYPE_HDMI_TYPE_A: case SIGNAL_TYPE_DVI_SINGLE_LINK: -- 2.43.0
[PATCH] drm/stm: Fix an error handling path in stm_drm_platform_probe()
If drm_dev_register() fails, a call to drv_load() must be undone, as already done in the remove function. Fixes: b759012c5fa7 ("drm/stm: Add STM32 LTDC driver") Signed-off-by: Christophe JAILLET --- This was already sent a few years ago in [1] but it got no response. Since, there has been some activity on this driver, so I send it again. Note that it is untested. [1]: https://lore.kernel.org/all/20200501125511.132029-1-christophe.jail...@wanadoo.fr/ --- drivers/gpu/drm/stm/drv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index e8523abef27a..4d2db079ad4f 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -203,12 +203,14 @@ static int stm_drm_platform_probe(struct platform_device *pdev) ret = drm_dev_register(ddev, 0); if (ret) - goto err_put; + goto err_unload; drm_fbdev_dma_setup(ddev, 16); return 0; +err_unload: + drv_unload(ddev); err_put: drm_dev_put(ddev); -- 2.34.1
Re: [PATCH 1/2] drm/panel: ltk050h3146w: only print message when GPIO getting is not EPROBE_DEFER
Le 04/01/2024 à 13:41, Quentin Schulz a écrit : From: Quentin Schulz devm_gpiod_get_optional may return EPROBE_DEFER in case the GPIO controller isn't yet probed when the panel driver is being probed. In that case, a spurious and confusing error message about not being able to get the reset GPIO is printed even though later on the device actually manages to get probed. Use dev_err_probe instead so that the message is only printed when it truly matters. Cc: Quentin Schulz Signed-off-by: Quentin Schulz Reviewed-by: Christophe JAILLET --- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c index 30919c872ac8d..ecfa4181c4fd9 100644 --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c @@ -646,10 +646,8 @@ static int ltk050h3146w_probe(struct mipi_dsi_device *dsi) return -EINVAL; ctx->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); - if (IS_ERR(ctx->reset_gpio)) { - dev_err(dev, "cannot get reset gpio\n"); - return PTR_ERR(ctx->reset_gpio); - } + if (IS_ERR(ctx->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "cannot get reset gpio\n"); ctx->vci = devm_regulator_get(dev, "vci"); if (IS_ERR(ctx->vci)) {
Re: [PATCH 2/2] drm/panel: ltk050h3146w: use dev_err_probe wherever possible
Le 04/01/2024 à 13:41, Quentin Schulz a écrit : From: Quentin Schulz This is only a cosmetic change. This replaces a hand-crafted EPROBE_DEFER handling for deciding to print an error message with dev_err_probe. A side-effect is that dev_err_probe also adds a debug message when it's not EPROBE_DEFER, but this is seen as an improvement. Cc: Quentin Schulz Signed-off-by: Quentin Schulz Reviewed-by: Christophe JAILLET --- drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c index ecfa4181c4fd9..9d87cc1a357e3 100644 --- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c +++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c @@ -650,20 +650,13 @@ static int ltk050h3146w_probe(struct mipi_dsi_device *dsi) return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "cannot get reset gpio\n"); ctx->vci = devm_regulator_get(dev, "vci"); - if (IS_ERR(ctx->vci)) { - ret = PTR_ERR(ctx->vci); - if (ret != -EPROBE_DEFER) - dev_err(dev, "Failed to request vci regulator: %d\n", ret); - return ret; - } + if (IS_ERR(ctx->vci)) + return dev_err_probe(dev, PTR_ERR(ctx->vci), "Failed to request vci regulator\n"); ctx->iovcc = devm_regulator_get(dev, "iovcc"); - if (IS_ERR(ctx->iovcc)) { - ret = PTR_ERR(ctx->iovcc); - if (ret != -EPROBE_DEFER) - dev_err(dev, "Failed to request iovcc regulator: %d\n", ret); - return ret; - } + if (IS_ERR(ctx->iovcc)) + return dev_err_probe(dev, PTR_ERR(ctx->iovcc), +"Failed to request iovcc regulator\n"); mipi_dsi_set_drvdata(dsi, ctx);
Re: [PATCH linux-next] drm/amd/display: replace kzalloc and memcpy with kmemdup
Le 08/12/2023 à 03:44, yang.gua...@zte.com.cn a écrit : From: Yang Guang Convert kzalloc/memcpy operations to memdup makes for cleaner code and avoids memcpy() failures Hi, usually, function's names are written with () in commit description. (i.e. kzalloc()/memcpy()). memdup should be kmemdup(). Finally the proposed change does not avoid memcpy() failures. Should it fail (what does it mean in this context?), kmemdup() would behave exactly the same. Signed-off-by: Chen Haonan --- drivers/gpu/drm/amd/display/dc/core/dc.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 76b47f178127..867e1a0fdef6 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -2264,12 +2264,10 @@ struct dc_state *dc_copy_state(struct dc_state *src_ctx) #ifdef CONFIG_DRM_AMD_DC_FP if (new_ctx->bw_ctx.dml2) { - dml2 = kzalloc(sizeof(struct dml2_context), GFP_KERNEL); - if (!dml2) - return NULL; - - memcpy(dml2, src_ctx->bw_ctx.dml2, sizeof(struct dml2_context)); - new_ctx->bw_ctx.dml2 = dml2; + dml2 = kmemdup(src_ctx->bw_ctx.dml2, sizeof(struct dml2_context), GFP_KERNEL); sizeof(struct dml2_context) could be sizeof(*dlm2) to be less verbose. CJ + if (!dml2) + return NULL; + new_ctx->bw_ctx.dml2 = dml2; } #endif
[PATCH] fbdev/offb: Simplify offb_init_fb()
Turn a strcpy()+strncat()+'\0' into an equivalent snprintf(). Signed-off-by: Christophe JAILLET --- This patch is *not* even compile tested because cross-compiling leads to some errors like on my machine: cc1: error: cannot load plugin ./scripts/gcc-plugins/randomize_layout_plugin.so: ./scripts/gcc-plugins/randomize_layout_plugin.so: undefined symbol: _ZNK6frange6acceptERK14vrange_visitor So review with care! --- drivers/video/fbdev/offb.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c index dcb1b81d35db..b421b46d88ef 100644 --- a/drivers/video/fbdev/offb.c +++ b/drivers/video/fbdev/offb.c @@ -423,11 +423,9 @@ static void offb_init_fb(struct platform_device *parent, const char *name, fix = >fix; var = >var; - if (name) { - strcpy(fix->id, "OFfb "); - strncat(fix->id, name, sizeof(fix->id) - sizeof("OFfb ")); - fix->id[sizeof(fix->id) - 1] = '\0'; - } else + if (name) + snprintf(fix->id, sizeof(fix->id), "OFfb %s", name); + else snprintf(fix->id, sizeof(fix->id), "OFfb %pOFn", dp); -- 2.34.1
[PATCH] drm/amd: Fix the size of a buffer in amdgpu_vcn_idle_work_handler()
In order to be sure that fw_name is not truncated, this buffer should be at least 41 bytes long. Let the compiler compute the correct length by itself. When building with W=1, this fixes the following warnings: drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:95:58: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] 95 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix); | ^ drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:95:9: note: ‘snprintf’ output between 12 and 41 bytes into a destination of size 40 95 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix); | ^ Fixes: 69939009bde7 ("drm/amd: Load VCN microcode during early_init") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c index c93f3a4c0e31..f8cd55a0d1f0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c @@ -88,7 +88,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work); int amdgpu_vcn_early_init(struct amdgpu_device *adev) { char ucode_prefix[30]; - char fw_name[40]; + char fw_name[sizeof(ucode_prefix) + sizeof("amdgpu/.bin") - 1]; int r; amdgpu_ucode_ip_version_decode(adev, UVD_HWIP, ucode_prefix, sizeof(ucode_prefix)); -- 2.34.1
Re: [PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
Le 18/09/2023 à 05:10, Gustavo A. R. Silva a écrit : On 9/18/23 12:46, Christophe JAILLET wrote: If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. In such a case, udmabuf_create() will access to memory beyond 'list'. Use size_mul() to saturate the value, and have memdup_user() fail. Fixes: fbb0de795078 ("Add udmabuf misc device") Signed-off-by: Christophe JAILLET --- drivers/dma-buf/udmabuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..fb4c4b5b3332 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -314,13 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; + size_t lsize; if (copy_from_user(, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; + lsize = size_mul(sizeof(struct udmabuf_create_item), head.count); list = memdup_user((void __user *)(arg + sizeof(head)), lsize); if (IS_ERR(list)) return PTR_ERR(list); How about this, and we get rid of `lsize`: Keeping or removing lsize is mostly a matter of taste, I think. Using sizeof(*list) is better. Let see if there are some other comments, and I'll send a v2. Thanks for the feed-back. CJ diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..5cf9d849aaa8 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -314,14 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; if (copy_from_user(, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; - list = memdup_user((void __user *)(arg + sizeof(head)), lsize); + list = memdup_user((void __user *)(arg + sizeof(head)), + size_mul(sizeof(*list), head.count)); if (IS_ERR(list)) return PTR_ERR(list); -- Gustavo
[PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. In such a case, udmabuf_create() will access to memory beyond 'list'. Use size_mul() to saturate the value, and have memdup_user() fail. Fixes: fbb0de795078 ("Add udmabuf misc device") Signed-off-by: Christophe JAILLET --- drivers/dma-buf/udmabuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..fb4c4b5b3332 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -314,13 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) struct udmabuf_create_list head; struct udmabuf_create_item *list; int ret = -EINVAL; - u32 lsize; + size_t lsize; if (copy_from_user(, (void __user *)arg, sizeof(head))) return -EFAULT; if (head.count > list_limit) return -EINVAL; - lsize = sizeof(struct udmabuf_create_item) * head.count; + lsize = size_mul(sizeof(struct udmabuf_create_item), head.count); list = memdup_user((void __user *)(arg + sizeof(head)), lsize); if (IS_ERR(list)) return PTR_ERR(list); -- 2.34.1
Re: [PATCH] drm/simpledrm: Add support for multiple "power-domains"
Le 10/09/2023 à 18:39, Janne Grunau via B4 Relay a écrit : From: Janne Grunau Multiple power domains need to be handled explicitly in each driver. The driver core can not handle it automatically since it is not aware of power sequencing requirements the hardware might have. This is not a problem for simpledrm since everything is expected to be powered on by the bootloader. simpledrm has just ensure it remains powered on during its lifetime. This is required on Apple silicon M2 and M2 Pro/Max/Ultra desktop systems. The HDMI output initialized by the bootloader requires keeping the display controller and a DP phy power domain on. Signed-off-by: Janne Grunau --- drivers/gpu/drm/tiny/simpledrm.c | 106 +++ 1 file changed, 106 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index ff86ba1ae1b8..efedede57d42 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -227,6 +228,12 @@ struct simpledrm_device { unsigned int regulator_count; struct regulator **regulators; #endif + /* power-domains */ +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS + int pwr_dom_count; + struct device **pwr_dom_devs; + struct device_link **pwr_dom_links; +#endif /* simplefb settings */ struct drm_display_mode mode; @@ -468,6 +475,102 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev) } #endif +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS +/* + * Generic power domain handling code. + * + * Here we handle the power-domains properties of our "simple-framebuffer" + * dt node. This is only necessary if there is more than one power-domain. + * A single power-domains is handled automatically by the driver core. Multiple + * power-domains have to be handled by drivers since the driver core can't know + * the correct power sequencing. Power sequencing is not an issue for simpledrm + * since the bootloader has put the power domains already in the correct state. + * simpledrm has only to ensure they remain active for its lifetime. + * + * When the driver unloads, we detach from the power-domains. + * + * We only complain about errors here, no action is taken as the most likely + * error can only happen due to a mismatch between the bootloader which set + * up the "simple-framebuffer" dt node, and the PM domain providers in the + * device tree. Chances are that there are no adverse effects, and if there are, + * a clean teardown of the fb probe will not help us much either. So just + * complain and carry on, and hope that the user actually gets a working fb at + * the end of things. + */ +static void simpledrm_device_detach_genpd(void *res) +{ + int i; + struct simpledrm_device *sdev = /*(struct simpledrm_device *)*/res; + + + drm_err(>dev, "% power-domains count:%d\n", __func__, sdev->pwr_dom_count); + if (sdev->pwr_dom_count <= 1) + return; + + for (i = sdev->pwr_dom_count - 1; i >= 0; i--) { + if (!sdev->pwr_dom_links[i]) + device_link_del(sdev->pwr_dom_links[i]); + if (!IS_ERR_OR_NULL(sdev->pwr_dom_devs[i])) + dev_pm_domain_detach(sdev->pwr_dom_devs[i], true); + } +} + +static int simpledrm_device_attach_genpd(struct simpledrm_device *sdev) +{ + struct device *dev = sdev->dev.dev; + int i; + + sdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node, "power-domains", +"#power-domain-cells"); + /* +* Single power-domain devices are handled by driver core nothing to do +* here. The same for device nodes without "power-domains" property. +*/ + if (sdev->pwr_dom_count <= 1) + return 0; + + sdev->pwr_dom_devs = devm_kcalloc(dev, sdev->pwr_dom_count, + sizeof(*sdev->pwr_dom_devs), + GFP_KERNEL); + if (!sdev->pwr_dom_devs) + return -ENOMEM; + + sdev->pwr_dom_links = devm_kcalloc(dev, sdev->pwr_dom_count, + sizeof(*sdev->pwr_dom_links), + GFP_KERNEL); + if (!sdev->pwr_dom_links) + return -ENOMEM; + + for (i = 0; i < sdev->pwr_dom_count; i++) { + sdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i); + if (IS_ERR(sdev->pwr_dom_devs[i])) { + int ret = PTR_ERR(sdev->pwr_dom_devs[i]); + if (ret == -EPROBE_DEFER) { + simpledrm_device_detach_genpd(sdev); + return PTR_ERR(sdev->pwr_dom_devs[i]); +
[PATCH] accel/habanalabs/gaudi2: Fix incorrect string length computation in gaudi2_psoc_razwi_get_engines()
snprintf() returns the "number of characters which *would* be generated for the given input", not the size *really* generated. In order to avoid too large values for 'str_size' (and potential negative values for "PSOC_RAZWI_ENG_STR_SIZE - str_size") use scnprintf() instead of snprintf(). Fixes: c0e6df916050 ("accel/habanalabs: fix address decode RAZWI handling") Signed-off-by: Christophe JAILLET --- drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c b/drivers/accel/habanalabs/gaudi2/gaudi2.c index d94acec63d95..9617c062b7ca 100644 --- a/drivers/accel/habanalabs/gaudi2/gaudi2.c +++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c @@ -8277,11 +8277,11 @@ static int gaudi2_psoc_razwi_get_engines(struct gaudi2_razwi_info *razwi_info, u eng_id[num_of_eng] = razwi_info[i].eng_id; base[num_of_eng] = razwi_info[i].rtr_ctrl; if (!num_of_eng) - str_size += snprintf(eng_name + str_size, + str_size += scnprintf(eng_name + str_size, PSOC_RAZWI_ENG_STR_SIZE - str_size, "%s", razwi_info[i].eng_name); else - str_size += snprintf(eng_name + str_size, + str_size += scnprintf(eng_name + str_size, PSOC_RAZWI_ENG_STR_SIZE - str_size, " or %s", razwi_info[i].eng_name); num_of_eng++; -- 2.34.1
[PATCH] drm/rockchip: cdn-dp: Fix some error handling paths in cdn_dp_probe()
cdn_dp_audio_codec_init() can fail. So add some error handling. If component_add() fails, the previous cdn_dp_audio_codec_init() call should be undone, as already done in the remove function. Fixes: 88582f564692 ("drm/rockchip: cdn-dp: Don't unregister audio dev when unbinding") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/rockchip/cdn-dp-core.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index a29fbafce393..3793863c210e 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -1177,6 +1177,7 @@ static int cdn_dp_probe(struct platform_device *pdev) struct cdn_dp_device *dp; struct extcon_dev *extcon; struct phy *phy; + int ret; int i; dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); @@ -1217,9 +1218,19 @@ static int cdn_dp_probe(struct platform_device *pdev) mutex_init(>lock); dev_set_drvdata(dev, dp); - cdn_dp_audio_codec_init(dp, dev); + ret = cdn_dp_audio_codec_init(dp, dev); + if (ret) + return ret; + + ret = component_add(dev, _dp_component_ops); + if (ret) + goto err_audio_deinit; - return component_add(dev, _dp_component_ops); + return 0; + +err_audio_deinit: + platform_device_unregister(dp->audio_pdev); + return ret; } static void cdn_dp_remove(struct platform_device *pdev) -- 2.34.1
[PATCH 6/6] drm/tegra: output: Fix missing i2c_put_adapter() in the error handling paths of tegra_output_probe()
If an error occurs after a successful of_get_i2c_adapter_by_node() call, it should be undone by a corresponding i2c_put_adapter(). Add the missing i2c_put_adapter() call. Fixes: 9be7d864cf07 ("drm/tegra: Implement panel support") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/output.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c index dc2dcb5ca1c8..d7d2389ac2f5 100644 --- a/drivers/gpu/drm/tegra/output.c +++ b/drivers/gpu/drm/tegra/output.c @@ -142,8 +142,10 @@ int tegra_output_probe(struct tegra_output *output) GPIOD_IN, "HDMI hotplug detect"); if (IS_ERR(output->hpd_gpio)) { - if (PTR_ERR(output->hpd_gpio) != -ENOENT) - return PTR_ERR(output->hpd_gpio); + if (PTR_ERR(output->hpd_gpio) != -ENOENT) { + err = PTR_ERR(output->hpd_gpio); + goto put_i2c; + } output->hpd_gpio = NULL; } @@ -152,7 +154,7 @@ int tegra_output_probe(struct tegra_output *output) err = gpiod_to_irq(output->hpd_gpio); if (err < 0) { dev_err(output->dev, "gpiod_to_irq(): %d\n", err); - return err; + goto put_i2c; } output->hpd_irq = err; @@ -165,7 +167,7 @@ int tegra_output_probe(struct tegra_output *output) if (err < 0) { dev_err(output->dev, "failed to request IRQ#%u: %d\n", output->hpd_irq, err); - return err; + goto put_i2c; } output->connector.polled = DRM_CONNECTOR_POLL_HPD; @@ -179,6 +181,12 @@ int tegra_output_probe(struct tegra_output *output) } return 0; + +put_i2c: + if (output->ddc) + i2c_put_adapter(output->ddc); + + return err; } void tegra_output_remove(struct tegra_output *output) -- 2.34.1
[PATCH 5/6] drm/tegra: rgb: Fix missing clk_put() in the error handling paths of tegra_dc_rgb_probe()
If clk_get_sys(..., "pll_d2_out0") fails, the clk_get_sys() call must be undone. Add the missing clk_put and a new 'put_pll_d_out0' label in the error handling path, and use it. Fixes: 0c921b6d4ba0 ("drm/tegra: dc: rgb: Allow changing PLLD rate on Tegra30+") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/rgb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 26d4d87b83de..e277826ab494 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -244,7 +244,7 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) if (IS_ERR(rgb->pll_d2_out0)) { err = PTR_ERR(rgb->pll_d2_out0); dev_err(dc->dev, "failed to get pll_d2_out0: %d\n", err); - goto tegra_remove; + goto put_pll_d_out0; } } @@ -252,6 +252,8 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) return 0; +put_pll_d_out0: + clk_put(rgb->pll_d_out0); tegra_remove: tegra_output_remove(>output); return err; -- 2.34.1
[PATCH 4/6] drm/tegra: rgb: Fix some error handling paths in tegra_dc_rgb_probe()
If an error occurs after calling tegra_output_probe(), tegra_output_remove() should be called as already done in the remove function. Fixes: 59d29c0ec93f ("drm/tegra: Allocate resources at probe time") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/rgb.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c index 79566c9ea8ff..26d4d87b83de 100644 --- a/drivers/gpu/drm/tegra/rgb.c +++ b/drivers/gpu/drm/tegra/rgb.c @@ -215,26 +215,28 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) rgb->clk = devm_clk_get(dc->dev, NULL); if (IS_ERR(rgb->clk)) { dev_err(dc->dev, "failed to get clock\n"); - return PTR_ERR(rgb->clk); + err = PTR_ERR(rgb->clk); + goto tegra_remove; } rgb->clk_parent = devm_clk_get(dc->dev, "parent"); if (IS_ERR(rgb->clk_parent)) { dev_err(dc->dev, "failed to get parent clock\n"); - return PTR_ERR(rgb->clk_parent); + err = PTR_ERR(rgb->clk_parent); + goto tegra_remove; } err = clk_set_parent(rgb->clk, rgb->clk_parent); if (err < 0) { dev_err(dc->dev, "failed to set parent clock: %d\n", err); - return err; + goto tegra_remove; } rgb->pll_d_out0 = clk_get_sys(NULL, "pll_d_out0"); if (IS_ERR(rgb->pll_d_out0)) { err = PTR_ERR(rgb->pll_d_out0); dev_err(dc->dev, "failed to get pll_d_out0: %d\n", err); - return err; + goto tegra_remove; } if (dc->soc->has_pll_d2_out0) { @@ -242,13 +244,17 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc) if (IS_ERR(rgb->pll_d2_out0)) { err = PTR_ERR(rgb->pll_d2_out0); dev_err(dc->dev, "failed to get pll_d2_out0: %d\n", err); - return err; + goto tegra_remove; } } dc->rgb = >output; return 0; + +tegra_remove: + tegra_output_remove(>output); + return err; } void tegra_dc_rgb_remove(struct tegra_dc *dc) -- 2.34.1
[PATCH 3/6] drm/tegra: hdmi: Fix some error handling paths in tegra_hdmi_probe()
If an error occurs after calling tegra_output_probe(), tegra_output_remove() should be called as already done in the remove function. Fixes: 59d29c0ec93f ("drm/tegra: Allocate resources at probe time") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/hdmi.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index 80c760986d9e..c73e7fb1877e 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -1854,12 +1854,14 @@ static int tegra_hdmi_probe(struct platform_device *pdev) return err; hdmi->regs = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(hdmi->regs)) - return PTR_ERR(hdmi->regs); + if (IS_ERR(hdmi->regs)) { + err = PTR_ERR(hdmi->regs); + goto tegra_remove; + } err = platform_get_irq(pdev, 0); if (err < 0) - return err; + goto tegra_remove; hdmi->irq = err; @@ -1868,18 +1870,18 @@ static int tegra_hdmi_probe(struct platform_device *pdev) if (err < 0) { dev_err(>dev, "failed to request IRQ#%u: %d\n", hdmi->irq, err); - return err; + goto tegra_remove; } platform_set_drvdata(pdev, hdmi); err = devm_pm_runtime_enable(>dev); if (err) - return err; + goto tegra_remove; err = devm_tegra_core_dev_init_opp_table_common(>dev); if (err) - return err; + goto tegra_remove; INIT_LIST_HEAD(>client.list); hdmi->client.ops = _client_ops; @@ -1889,10 +1891,14 @@ static int tegra_hdmi_probe(struct platform_device *pdev) if (err < 0) { dev_err(>dev, "failed to register host1x client: %d\n", err); - return err; + goto tegra_remove; } return 0; + +tegra_remove: + tegra_output_remove(>output); + return err; } static void tegra_hdmi_remove(struct platform_device *pdev) -- 2.34.1
[PATCH 2/6] drm/tegra: dsi: Fix missing pm_runtime_disable() in the error handling path of tegra_dsi_probe()
If an error occurs after calling pm_runtime_enable(), pm_runtime_disable() should be called as already done in the remove function. Fixes: ef8187d75265 ("drm/tegra: dsi: Implement runtime PM") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/dsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 70a77c75bfe1..0cf379e20d89 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -1672,6 +1672,7 @@ static int tegra_dsi_probe(struct platform_device *pdev) return 0; unregister: + pm_runtime_disable(>dev); mipi_dsi_host_unregister(>host); mipi_free: tegra_mipi_free(dsi->mipi); -- 2.34.1
[PATCH 0/6] drm/tegra: Fix some error handling paths
Most of the patches are retated to tegra_output_probe() and missing tegra_output_remove(). Others are things spotted while writting the serie. Patches 1, 3, 4 are verbose, but some functions called in the probe can return -EPROBE_DEFER, so it is nice to correctly release resources. Maybe moving the tegra_output_probe() call would minimize the changes, but I'm always reluctant to move code, because of possible side-effects. Christophe JAILLET (6): drm/tegra: dsi: Fix some error handling paths in tegra_dsi_probe() drm/tegra: dsi: Fix missing pm_runtime_disable() in the error handling path of tegra_dsi_probe() drm/tegra: dsi: Fix some error handling paths in tegra_hdmi_probe() drm/tegra: rgb: Fix some error handling paths in tegra_dc_rgb_probe() drm/tegra: rgb: Fix missing clk_put() in the error handling paths of tegra_dc_rgb_probe() drm/tegra: output: Fix missing i2c_put_adapter() in the error handling paths of tegra_output_probe() drivers/gpu/drm/tegra/dsi.c| 55 ++ drivers/gpu/drm/tegra/hdmi.c | 20 - drivers/gpu/drm/tegra/output.c | 16 +++--- drivers/gpu/drm/tegra/rgb.c| 18 +++ 4 files changed, 74 insertions(+), 35 deletions(-) -- 2.34.1
[PATCH 1/6] drm/tegra: dsi: Fix some error handling paths in tegra_dsi_probe()
If an error occurs after calling tegra_output_probe(), tegra_output_remove() should be called as already done in the remove function. Fixes: dec727399a4b ("drm/tegra: Add DSI support") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/tegra/dsi.c | 54 - 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index a9870c828374..70a77c75bfe1 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -1593,44 +1593,58 @@ static int tegra_dsi_probe(struct platform_device *pdev) if (!pdev->dev.pm_domain) { dsi->rst = devm_reset_control_get(>dev, "dsi"); - if (IS_ERR(dsi->rst)) - return PTR_ERR(dsi->rst); + if (IS_ERR(dsi->rst)) { + err = PTR_ERR(dsi->rst); + goto tegra_remove; + } } dsi->clk = devm_clk_get(>dev, NULL); - if (IS_ERR(dsi->clk)) - return dev_err_probe(>dev, PTR_ERR(dsi->clk), -"cannot get DSI clock\n"); + if (IS_ERR(dsi->clk)) { + err = dev_err_probe(>dev, PTR_ERR(dsi->clk), + "cannot get DSI clock\n"); + goto tegra_remove; + } dsi->clk_lp = devm_clk_get(>dev, "lp"); - if (IS_ERR(dsi->clk_lp)) - return dev_err_probe(>dev, PTR_ERR(dsi->clk_lp), -"cannot get low-power clock\n"); + if (IS_ERR(dsi->clk_lp)) { + err = dev_err_probe(>dev, PTR_ERR(dsi->clk_lp), + "cannot get low-power clock\n"); + goto tegra_remove; + } dsi->clk_parent = devm_clk_get(>dev, "parent"); - if (IS_ERR(dsi->clk_parent)) - return dev_err_probe(>dev, PTR_ERR(dsi->clk_parent), -"cannot get parent clock\n"); + if (IS_ERR(dsi->clk_parent)) { + err = dev_err_probe(>dev, PTR_ERR(dsi->clk_parent), + "cannot get parent clock\n"); + goto tegra_remove; + } dsi->vdd = devm_regulator_get(>dev, "avdd-dsi-csi"); - if (IS_ERR(dsi->vdd)) - return dev_err_probe(>dev, PTR_ERR(dsi->vdd), -"cannot get VDD supply\n"); + if (IS_ERR(dsi->vdd)) { + err = dev_err_probe(>dev, PTR_ERR(dsi->vdd), + "cannot get VDD supply\n"); + goto tegra_remove; + } err = tegra_dsi_setup_clocks(dsi); if (err < 0) { dev_err(>dev, "cannot setup clocks\n"); - return err; + goto tegra_remove; } regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); dsi->regs = devm_ioremap_resource(>dev, regs); - if (IS_ERR(dsi->regs)) - return PTR_ERR(dsi->regs); + if (IS_ERR(dsi->regs)) { + err = PTR_ERR(dsi->regs); + goto tegra_remove; + } dsi->mipi = tegra_mipi_request(>dev, pdev->dev.of_node); - if (IS_ERR(dsi->mipi)) - return PTR_ERR(dsi->mipi); + if (IS_ERR(dsi->mipi)) { + err = PTR_ERR(dsi->mipi); + goto tegra_remove; + } dsi->host.ops = _dsi_host_ops; dsi->host.dev = >dev; @@ -1661,6 +1675,8 @@ static int tegra_dsi_probe(struct platform_device *pdev) mipi_dsi_host_unregister(>host); mipi_free: tegra_mipi_free(dsi->mipi); +tegra_remove: + tegra_output_remove(>output); return err; } -- 2.34.1
Re: [PATCH] drm/ast: Avoid possible NULL dereference
Le 21/08/2023 à 08:22, Su Hui a écrit : smatch error: drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error: we previously assumed 'ast->dp501_fw' could be null (see line 223) when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true, there will be a NULL dereference of 'ast->dp501_fw'. Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when required") Signed-off-by: Su Hui --- drivers/gpu/drm/ast/ast_dp501.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c index 1bc35a992369..d9f3a0786a6f 100644 --- a/drivers/gpu/drm/ast/ast_dp501.c +++ b/drivers/gpu/drm/ast/ast_dp501.c @@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev) ast_load_dp501_microcode(dev) < 0) return false; - fw_addr = (u8 *)ast->dp501_fw->data; - len = ast->dp501_fw->size; + if (ast->dp501_fw) { + fw_addr = (u8 *)ast->dp501_fw->data; + len = ast->dp501_fw->size; + } } /* Get BootAddress */ ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8); Hi, this looks like a false positive. If "!ast->dp501_fw", and ast_load_dp501_microcode()>=0, then ast_load_dp501_microcode() will set a valid value in ast->dp501_fw. CJ
Re: [PATCH] drm/amdgpu: Avoid possible buffer overflow
Le 21/08/2023 à 08:19, Su Hui a écrit : smatch error: drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257 amdgpu_discovery_reg_base_init() error: testing array offset 'adev->vcn.num_vcn_inst' after use. change the assignment order to avoid buffer overflow. Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV") Signed-off-by: Su Hui --- drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c index 8e1cfc87122d..ba95526c3d45 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c @@ -1250,11 +1250,12 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) * 0b10 : encode is disabled * 0b01 : decode is disabled */ - adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = - ip->revision & 0xc0; + ip->revision &= ~0xc0; if (adev->vcn.num_vcn_inst < AMDGPU_MAX_VCN_INSTANCES) { + adev->vcn.vcn_config[adev->vcn.num_vcn_inst] = + ip->revision & 0xc0; Hi, I don't think that the patch is correct. Because of the "ip->revision &= ~0xc0" which is now above it, "ip->revision & 0xc0" should now always be 0. Maybe both lines should be moved within the "if"? CJ adev->vcn.num_vcn_inst++; adev->vcn.inst_mask |= (1U << ip->instance_number);
Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
Le 20/08/2023 à 11:41, Julius Zint a écrit : The HID spec defines the following Usage IDs (p. 345 ff): - Monitor Page (0x80) -> Monitor Control (0x01) - VESA Virtual Controls Page (0x82) -> Brightness (0x10) Apple made use of them in their Apple Studio Display and most likely on other external displays (LG UltraFine 5k, Pro Display XDR). The driver will work for any HID device with a report, where the application matches the Monitor Control Usage ID and: 1. An Input field in this report with the Brightness Usage ID (to get the current brightness) 2. A Feature field in this report with the Brightness Usage ID (to set the current brightness) This driver has been developed and tested with the Apple Studio Display. Here is a small excerpt from the decoded HID descriptor showing the feature field for setting the brightness: Usage Page (Monitor VESA VCP), ; Monitor VESA VPC (82h, monitor page) Usage (10h, Brightness), Logical Minimum (400), Logical Maximum (6), Unit (Centimeter^-2 * Candela), Unit Exponent (14), Report Size (32), Report Count (1), Feature (Variable, Null State), The full HID descriptor dump is available as a comment in the source code. Signed-off-by: Julius Zint --- [...] +static void hid_bl_remove(struct hid_device *hdev) +{ + struct backlight_device *bl; + struct hid_bl_data *data; + + hid_dbg(hdev, "remove\n"); + bl = hid_get_drvdata(hdev); + data = bl_get_data(bl); + + devm_backlight_device_unregister(>dev, bl); Hi, If this need to be called here, before the hid_() calls below, there seems to be no real point in using devm_backlight_device_register() / devm_backlight_device_unregister(). The non-devm_ version should be enough. + hid_hw_close(hdev); + hid_hw_stop(hdev); + hid_set_drvdata(hdev, NULL); + devm_kfree(>dev, data); 'data' is devm_kzalloc()'ed. It is likely that this explicit devm_kfree() could be saved. It will be freed by the framework. +} [...] +static int hid_bl_probe(struct hid_device *hdev, const struct hid_device_id *id) +{ + int ret; + struct hid_field *input_field; + struct hid_field *feature_field; + struct hid_bl_data *data; + struct backlight_properties props; + struct backlight_device *bl; + + hid_dbg(hdev, "probe\n"); + + ret = hid_parse(hdev); + if (ret) { + hid_err(hdev, "parse failed: %d\n", ret); + return ret; + } + + ret = hid_hw_start(hdev, HID_CONNECT_DRIVER); + if (ret) { + hid_err(hdev, "hw start failed: %d\n", ret); + return ret; + } + + input_field = hid_get_usage_field(hdev, HID_INPUT_REPORT, + HID_USAGE_MONITOR_CTRL, + HID_USAGE_VESA_VCP_BRIGHTNESS); + if (input_field == NULL) { + ret = -ENODEV; + goto exit_stop; + } + + feature_field = hid_get_usage_field(hdev, HID_FEATURE_REPORT, + HID_USAGE_MONITOR_CTRL, + HID_USAGE_VESA_VCP_BRIGHTNESS); + if (feature_field == NULL) { + ret = -ENODEV; + goto exit_stop; + } + + if (input_field->logical_minimum != feature_field->logical_minimum) { + hid_warn(hdev, "minimums do not match: %d / %d\n", +input_field->logical_minimum, +feature_field->logical_minimum); + ret = -ENODEV; + goto exit_stop; + } + + if (input_field->logical_maximum != feature_field->logical_maximum) { + hid_warn(hdev, "maximums do not match: %d / %d\n", +input_field->logical_maximum, +feature_field->logical_maximum); + ret = -ENODEV; + goto exit_stop; + } + + hid_dbg(hdev, "Monitor VESA VCP with brightness control\n"); + + ret = hid_hw_open(hdev); + if (ret) { + hid_err(hdev, "hw open failed: %d\n", ret); + goto exit_stop; + } + + data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL); + if (data == NULL) { + ret = -ENOMEM; + goto exit_stop; exit_free? in order to undo the hid_hw_open() just above. + } + data->hdev = hdev; + data->min_brightness = input_field->logical_minimum; + data->max_brightness = input_field->logical_maximum; + data->input_field = input_field; + data->feature_field = feature_field; + + memset(, 0, sizeof(props)); + props.type = BACKLIGHT_RAW; + props.max_brightness = data->max_brightness - data->min_brightness; + + bl = devm_backlight_device_register(>dev, "vesa_vcp", + >dev, data, +
[PATCH 4/4] drm/amdgpu: Use kvzalloc() to simplify code
kvzalloc() can be used instead of kvmalloc() + memset() + explicit NULL assignments. It is less verbose and more future proof. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 6ea9ff22c281..6f5b641b631e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -78,17 +78,13 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, unsigned i; int r; - list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL); + list = kvzalloc(struct_size(list, entries, num_entries), GFP_KERNEL); if (!list) return -ENOMEM; kref_init(>refcount); - list->gds_obj = NULL; - list->gws_obj = NULL; - list->oa_obj = NULL; array = list->entries; - memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry)); for (i = 0; i < num_entries; ++i) { struct amdgpu_bo_list_entry *entry; -- 2.34.1
[PATCH 2/4] drm/amdgpu: Remove a redundant sanity check
The case where 'num_entries' is too big, is already handled by struct_size(), because kvmalloc() would fail. It will return -ENOMEM instead of -EINVAL, but it is only related to a unlikely to happen sanity check. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 571fed04eb7a..c8f59a044286 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -78,10 +78,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, unsigned i; int r; - if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) - / sizeof(struct amdgpu_bo_list_entry)) - return -EINVAL; - list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL); if (!list) return -ENOMEM; -- 2.34.1
[PATCH 3/4] drm/amdgpu: Remove amdgpu_bo_list_array_entry()
Now that there is an explicit flexible array at the end of 'struct amdgpu_bo_list', it can be used to remove amdgpu_bo_list_array_entry() and simplify some macro. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 16 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index c8f59a044286..6ea9ff22c281 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -87,7 +87,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, list->gws_obj = NULL; list->oa_obj = NULL; - array = amdgpu_bo_list_array_entry(list, 0); + array = list->entries; memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry)); for (i = 0; i < num_entries; ++i) { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h index 368e50b30160..6a703be45d04 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h @@ -71,22 +71,14 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, size_t num_entries, struct amdgpu_bo_list **list); -static inline struct amdgpu_bo_list_entry * -amdgpu_bo_list_array_entry(struct amdgpu_bo_list *list, unsigned index) -{ - struct amdgpu_bo_list_entry *array = (void *)[1]; - - return [index]; -} - #define amdgpu_bo_list_for_each_entry(e, list) \ - for (e = amdgpu_bo_list_array_entry(list, 0); \ -e != amdgpu_bo_list_array_entry(list, (list)->num_entries); \ + for (e = list->entries; \ +e != >entries[list->num_entries]; \ ++e) #define amdgpu_bo_list_for_each_userptr_entry(e, list) \ - for (e = amdgpu_bo_list_array_entry(list, (list)->first_userptr); \ -e != amdgpu_bo_list_array_entry(list, (list)->num_entries); \ + for (e = >entries[list->first_userptr]; \ +e != >entries[list->num_entries]; \ ++e) #endif -- 2.34.1
[PATCH 1/4] drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list'
'struct amdgpu_bo_list' is really used as if it was ended by a flex array. So make it more explicit and add a 'struct amdgpu_bo_list_entry entries[]' field at the end of the structure. This way, struct_size() can be used when it is allocated. It is less verbose. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index b6298e901cbd..571fed04eb7a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -75,7 +75,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, struct amdgpu_bo_list_entry *array; struct amdgpu_bo_list *list; uint64_t total_size = 0; - size_t size; unsigned i; int r; @@ -83,9 +82,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, / sizeof(struct amdgpu_bo_list_entry)) return -EINVAL; - size = sizeof(struct amdgpu_bo_list); - size += num_entries * sizeof(struct amdgpu_bo_list_entry); - list = kvmalloc(size, GFP_KERNEL); + list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL); if (!list) return -ENOMEM; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h index 26c01cb131f2..368e50b30160 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h @@ -55,6 +55,8 @@ struct amdgpu_bo_list { /* Protect access during command submission. */ struct mutex bo_list_mutex; + + struct amdgpu_bo_list_entry entries[]; }; int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id, -- 2.34.1
[PATCH 0/4] drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list' and simplify amdgpu_bo_list_create()
This serie simplifies amdgpu_bo_list_create() and usage of the 'struct amdgpu_bo_list'. It is compile tested only. Christophe JAILLET (4): drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list' drm/amdgpu: Remove a redundant sanity check drm/amdgpu: Remove amdgpu_bo_list_array_entry() drm/amdgpu: Use kvzalloc() to simplify code drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 15 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 18 ++ 2 files changed, 8 insertions(+), 25 deletions(-) -- 2.34.1
Re: [PATCH 1/1] gpu: drm: aspeed: fix value check in aspeed_gfx_load()
Le 27/07/2023 à 19:03, Yuanjun Gong a écrit : in aspeed_gfx_load(), check the return value of clk_prepare_enable() and return the error code if clk_prepare_enable() returns an unexpected value. Fixes: 4f2a8f5898ec ("drm: Add ASPEED GFX driver") Signed-off-by: Yuanjun Gong --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index c8c7f8215155..3bfa39bc4f7e 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -199,7 +199,11 @@ static int aspeed_gfx_load(struct drm_device *drm) "missing or invalid clk device tree entry"); return PTR_ERR(priv->clk); } - clk_prepare_enable(priv->clk); + ret = clk_prepare_enable(priv->clk); + if (ret) { + dev_err(>dev, "Failed to enable clock\n"); + return ret; + } /* Sanitize control registers */ writel(0, priv->base + CRT_CTRL1); Hi, the code also lacks a clk_disable_unprepare() in aspeed_gfx_unload(). So using devm_clk_get_enabled() a few lines above should fix both issue. CJ
[PATCH] drm/msm: Slightly simplify memory allocation in submit_lookup_cmds()
If 'sz' is SIZE_MAX, kmalloc() will fail. So there is no need to explicitly check for an hypothetical overflow. Remove the check to save a few lines of code. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/msm/msm_gem_submit.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 3f1aa4de3b87..6ca8f8cbb6e2 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -211,11 +211,7 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit, sz = array_size(submit_cmd.nr_relocs, sizeof(struct drm_msm_gem_submit_reloc)); - /* check for overflow: */ - if (sz == SIZE_MAX) { - ret = -ENOMEM; - goto out; - } + submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL); if (!submit->cmd[i].relocs) { ret = -ENOMEM; -- 2.34.1
[PATCH] drm/i915: Fix an error handling path in igt_write_huge()
All error handling paths go to 'out', except this one. Be consistent and also branch to 'out' here. Fixes: c10a652e239e ("drm/i915/selftests: Rework context handling in hugepages selftests") Signed-off-by: Christophe JAILLET --- /!\ Speculative /!\ This patch is based on analysis of the surrounding code and should be reviewed with care ! If the patch is wrong, maybe a comment in the code could explain why. /!\ Speculative /!\ --- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index df6c9a84252c..6b9f6cf50bf6 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -1246,8 +1246,10 @@ static int igt_write_huge(struct drm_i915_private *i915, * times in succession a possibility by enlarging the permutation array. */ order = i915_random_order(count * count, ); - if (!order) - return -ENOMEM; + if (!order) { + err = -ENOMEM; + goto out; + } max_page_size = rounddown_pow_of_two(obj->mm.page_sizes.sg); max = div_u64(max - size, max_page_size); -- 2.34.1
[PATCH v3] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper: - calls devm_clk_get() - calls clk_prepare_enable() and registers what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the need of a dedicated function used with devm_add_action_or_reset(). Signed-off-by: Christophe JAILLET Reviewed-by: Andrzej Hajda --- Change in v3: - Rebase with latest -next Change in v2: - Convert to dev_err_probe()[Andrzej Hajda] - Update the error message[Andrzej Hajda] - Add R-b tag[Andrzej Hajda] https://lore.kernel.org/all/208546ce4e01973da1eb9ad7bc0f9241f650b3af.1672415956.git.christophe.jail...@wanadoo.fr/ v1: https://lore.kernel.org/all/4f855984ea895e1488169e77935fa6e044912ac2.1672414073.git.christophe.jail...@wanadoo.fr/ --- drivers/gpu/drm/bridge/tc358767.c | 25 - 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 2a58eb271f70..99f3d5ca7257 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -2215,13 +2215,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) return -EINVAL; } -static void tc_clk_disable(void *data) -{ - struct clk *refclk = data; - - clk_disable_unprepare(refclk); -} - static int tc_probe(struct i2c_client *client) { struct device *dev = >dev; @@ -2238,20 +2231,10 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) if (ret) return ret; - tc->refclk = devm_clk_get(dev, "ref"); - if (IS_ERR(tc->refclk)) { - ret = PTR_ERR(tc->refclk); - dev_err(dev, "Failed to get refclk: %d\n", ret); - return ret; - } - - ret = clk_prepare_enable(tc->refclk); - if (ret) - return ret; - - ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); - if (ret) - return ret; + tc->refclk = devm_clk_get_enabled(dev, "ref"); + if (IS_ERR(tc->refclk)) + return dev_err_probe(dev, PTR_ERR(tc->refclk), +"Failed to get and enable the ref clk\n"); /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */ usleep_range(10, 15); -- 2.34.1
[PATCH] video/hdmi: Reorder fields in 'struct hdmi_avi_infoframe'
Group some variables based on their sizes to reduce hole and avoid padding. On x86_64, this shrinks the size of 'struct hdmi_avi_infoframe' from 68 to 60 bytes. It saves a few bytes of memory and is more cache-line friendly. This also reduces the union hdmi_infoframe the same way. Signed-off-by: Christophe JAILLET --- Using pahole Before: == struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; /* 0 4 */ unsigned char version; /* 4 1 */ unsigned char length; /* 5 1 */ /* XXX 2 bytes hole, try to pack */ enum hdmi_colorspace colorspace; /* 8 4 */ enum hdmi_scan_modescan_mode;/*12 4 */ enum hdmi_colorimetry colorimetry; /*16 4 */ enum hdmi_picture_aspect picture_aspect; /*20 4 */ enum hdmi_active_aspectactive_aspect;/*24 4 */ bool itc; /*28 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_extended_colorimetry extended_colorimetry; /*32 4 */ enum hdmi_quantization_range quantization_range; /*36 4 */ enum hdmi_nups nups; /*40 4 */ unsigned char video_code; /*44 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_ycc_quantization_range ycc_quantization_range; /*48 4 */ enum hdmi_content_type content_type; /*52 4 */ unsigned char pixel_repeat; /*56 1 */ /* XXX 1 byte hole, try to pack */ short unsigned int top_bar; /*58 2 */ short unsigned int bottom_bar; /*60 2 */ short unsigned int left_bar; /*62 2 */ /* --- cacheline 1 boundary (64 bytes) --- */ short unsigned int right_bar;/*64 2 */ /* size: 68, cachelines: 2, members: 20 */ /* sum members: 57, holes: 4, sum holes: 9 */ /* padding: 2 */ /* last cacheline: 4 bytes */ }; After: = struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; /* 0 4 */ unsigned char version; /* 4 1 */ unsigned char length; /* 5 1 */ bool itc; /* 6 1 */ unsigned char pixel_repeat; /* 7 1 */ enum hdmi_colorspace colorspace; /* 8 4 */ enum hdmi_scan_modescan_mode;/*12 4 */ enum hdmi_colorimetry colorimetry; /*16 4 */ enum hdmi_picture_aspect picture_aspect; /*20 4 */ enum hdmi_active_aspectactive_aspect;/*24 4 */ enum hdmi_extended_colorimetry extended_colorimetry; /*28 4 */ enum hdmi_quantization_range quantization_range; /*32 4 */ enum hdmi_nups nups; /*36 4 */ unsigned char video_code; /*40 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_ycc_quantization_range ycc_quantization_range; /*44 4 */ enum hdmi_content_type content_type; /*48 4 */ short unsigned int top_bar; /*52 2 */ short unsigned int bottom_bar; /*54 2 */ short unsigned int left_bar; /*56 2 */ short unsigned int right_bar;/*58 2 */ /* size: 60, cachelines: 1, members: 20 */ /* sum members: 57, holes: 1, sum holes: 3 */ /* last cacheline: 60 bytes */ }; --- include/linux/hdmi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 2f4dcc8d060e..3bb87bf6bc65 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -170,19 +170,19 @@ struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; unsigned char version; unsigned char length; + bool itc; + unsigned char pixel_repeat; enum hdmi_colorspace colorspace; enum hdmi_scan_mode scan_mode; enum hdmi_colorimetry colorimetry; enum hdmi_picture_aspect picture_aspect; enum hdmi_active_aspect active_aspect; - bool itc; enum hdmi_extended_colorimetry extended_colorimetry; enum hdmi_quantization_range quantization_range; enum hdmi_nups nups; unsigned char video_code; enum hdmi_ycc_quantization_range ycc_quantization_range; enum
Re: [PATCH v4 1/4] Input: ads7846 - Convert to use software nodes
Le 08/05/2023 à 23:20, Linus Walleij a écrit : The Nokia 770 is using GPIOs from the global numberspace on the CBUS node to pass down to the LCD controller. This regresses when we let the OMAP GPIO driver use dynamic GPIO base. The Nokia 770 now has dynamic allocation of IRQ numbers, so this needs to be fixed for it to work. As this is the only user of LCD MIPID we can easily augment the driver to use a GPIO descriptor instead and resolve the issue. The platform data .shutdown() callback wasn't even used in the code, but we encode a shutdown asserting RESET in the remove() callback for completeness sake. The CBUS also has the ADS7846 touchscreen attached. Populate the devices on the Nokia 770 CBUS I2C using software nodes instead of platform data quirks. This includes the LCD and the ADS7846 touchscreen so the conversion just brings the LCD along with it as software nodes is an all-or-nothing design pattern. The ADS7846 has some limited support for using GPIO descriptors, let's convert it over completely to using device properties and then fix all remaining boardfile users to provide all platform data using software nodes. Dump the of includes and of_match_ptr() in the ADS7846 driver as part of the job. Since we have to move ADS7846 over to obtaining the GPIOs it is using exclusively from descriptors, we provide descriptor tables for the two remaining in-kernel boardfiles using ADS7846: - PXA Spitz - MIPS Alchemy DB1000 development board It was too hard for me to include software node conversion of these two remaining users at this time: the spitz is using a hscync callback in the platform data that would require further GPIO descriptor conversion of the Spitz, and moving the hsync callback down into the driver: it will just become too big of a job, but it can be done separately. The MIPS Alchemy DB1000 is simply something I cannot test, so take the easier approach of just providing some GPIO descriptors in this case as I don't want the patch to grow too intrusive. As we see that several device trees have incorrect polarity flags and just expect to bypass the gpiolib polarity handling, fix up all device trees too, in a separate patch. Suggested-by: Dmitry Torokhov Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base") Signed-off-by: Linus Walleij --- [...] diff --git a/drivers/video/fbdev/omap/lcd_mipid.c b/drivers/video/fbdev/omap/lcd_mipid.c index 03cff39d392d..e4a7f0b824ff 100644 --- a/drivers/video/fbdev/omap/lcd_mipid.c +++ b/drivers/video/fbdev/omap/lcd_mipid.c @@ -7,6 +7,7 @@ */ #include #include +#include #include #include #include @@ -41,6 +42,7 @@ struct mipid_device { when we can issue the next sleep in/out command */ unsigned long hw_guard_wait; /* max guard time in jiffies */ + struct gpio_desc*reset; struct omapfb_device *fbdev; struct spi_device *spi; @@ -556,6 +558,12 @@ static int mipid_spi_probe(struct spi_device *spi) return -ENOMEM; } + /* This will de-assert RESET if active */ + md->reset = gpiod_get(>dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(md->reset)) + return dev_err_probe(>dev, PTR_ERR(md->reset), +"no reset GPIO line\n"); + spi->mode = SPI_MODE_0; md->spi = spi; dev_set_drvdata(>dev, md); @@ -574,6 +582,8 @@ static void mipid_spi_remove(struct spi_device *spi) { struct mipid_device *md = dev_get_drvdata(>dev); + /* Asserts RESET */ + gpiod_set_value(md->reset, 1); Hi, should this also be done in the probe if mipid_detect() fails? If yes, please also look at [1], that I've just sent, which introduces an error handling path in the probe. CJ [1]: https://lore.kernel.org/all/8b82e34724755b69f34f15dddb288cd373080390.1620505229.git.christophe.jail...@wanadoo.fr/ mipid_disable(>panel); kfree(md); } [...]
[PATCH] video: fbdev: omapfb: lcd_mipid: Fix an error handling path in mipid_spi_probe()
If 'mipid_detect()' fails, we must free 'md' to avoid a memory leak. Fixes: 66d2f99d0bb5 ("omapfb: add support for MIPI-DCS compatible LCDs") Signed-off-by: Christophe JAILLET --- drivers/video/fbdev/omap/lcd_mipid.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/omap/lcd_mipid.c b/drivers/video/fbdev/omap/lcd_mipid.c index e4a7f0b824ff..a0fc4570403b 100644 --- a/drivers/video/fbdev/omap/lcd_mipid.c +++ b/drivers/video/fbdev/omap/lcd_mipid.c @@ -571,11 +571,15 @@ static int mipid_spi_probe(struct spi_device *spi) r = mipid_detect(md); if (r < 0) - return r; + goto free_md; omapfb_register_panel(>panel); return 0; + +free_md: + kfree(md); + return r; } static void mipid_spi_remove(struct spi_device *spi) -- 2.34.1
[PATCH] accel/ivpu: Use struct_size()
Use struct_size() instead of hand-writing it. It is less verbose, more robust and more informative. Signed-off-by: Christophe JAILLET --- drivers/accel/ivpu/ivpu_job.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c index 3c6f1e16cf2f..0a09bba8da24 100644 --- a/drivers/accel/ivpu/ivpu_job.c +++ b/drivers/accel/ivpu/ivpu_job.c @@ -289,15 +289,13 @@ ivpu_create_job(struct ivpu_file_priv *file_priv, u32 engine_idx, u32 bo_count) { struct ivpu_device *vdev = file_priv->vdev; struct ivpu_job *job; - size_t buf_size; int ret; ret = ivpu_rpm_get(vdev); if (ret < 0) return NULL; - buf_size = sizeof(*job) + bo_count * sizeof(struct ivpu_bo *); - job = kzalloc(buf_size, GFP_KERNEL); + job = kzalloc(struct_size(job, bos, bo_count), GFP_KERNEL); if (!job) goto err_rpm_put; -- 2.34.1
[PATCH 3/3] drm/amd/display: Use USEC_PER_SEC
Use USEC_PER_SEC instead of defining an equivalent local 'us_in_sec'. Signed-off-by: Christophe JAILLET --- NOT compile tested. Because of some BROKEN in KConfig files. Some header may be missing for USEC_PER_SEC! --- drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c index eafe8561e55e..9b82ee3e06d0 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c @@ -678,7 +678,6 @@ static uint32_t get_dmif_switch_time_us( uint32_t pixels_per_second; uint32_t pixels_per_frame; uint32_t refresh_rate; - const uint32_t us_in_sec = 100; const uint32_t min_single_frame_time_us = 3; /*return double of frame time*/ const uint32_t single_frame_time_multiplier = 2; @@ -691,8 +690,7 @@ static uint32_t get_dmif_switch_time_us( pixels_per_frame = h_total * v_total; refresh_rate = pixels_per_second / pixels_per_frame; - - frame_time = us_in_sec / refresh_rate; + frame_time = USEC_PER_SEC / refresh_rate; if (frame_time < min_single_frame_time_us) frame_time = min_single_frame_time_us; -- 2.34.1
[PATCH 2/3] drm/amd/display: Simplify get_dmif_switch_time_us()
Thanks to the sanity check a few lines above: if (!h_total || !v_total || !pix_clk_khz) and the computation done afterwards on these non 0 values, we know that 'pixels_per_second', 'pixels_per_frame' and 'refresh_rate' are not 0 The code can be simplified accordingly. Signed-off-by: Christophe JAILLET --- NOT compile tested. Because of some BROKEN in KConfig files. --- drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 13 - 1 file changed, 13 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c index 091f0d68a045..eafe8561e55e 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c @@ -690,21 +690,8 @@ static uint32_t get_dmif_switch_time_us( pixels_per_second = pix_clk_khz * 1000; pixels_per_frame = h_total * v_total; - if (!pixels_per_second || !pixels_per_frame) { - /* avoid division by zero */ - ASSERT(pixels_per_frame); - ASSERT(pixels_per_second); - return single_frame_time_multiplier * min_single_frame_time_us; - } - refresh_rate = pixels_per_second / pixels_per_frame; - if (!refresh_rate) { - /* avoid division by zero*/ - ASSERT(refresh_rate); - return single_frame_time_multiplier * min_single_frame_time_us; - } - frame_time = us_in_sec / refresh_rate; if (frame_time < min_single_frame_time_us) -- 2.34.1
[PATCH 1/3] drm/amd/display: Fix an erroneous sanity check in get_dmif_switch_time_us()
It is likely that there is a typo in the sanity check for 'v_total'. If it is 0, then 'pixels_per_frame' will also be 0, and in this case, we also return 'single_frame_time_multiplier * min_single_frame_time_us'. So test for !v_total which looks much more logical. Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c index 4cdd4dacb761..091f0d68a045 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c @@ -683,7 +683,7 @@ static uint32_t get_dmif_switch_time_us( /*return double of frame time*/ const uint32_t single_frame_time_multiplier = 2; - if (!h_total || v_total || !pix_clk_khz) + if (!h_total || !v_total || !pix_clk_khz) return single_frame_time_multiplier * min_single_frame_time_us; /*TODO: should we use pixel format normalized pixel clock here?*/ -- 2.34.1
Re: [PATCH 1/3] drm/amd/display: drop redundant memset() in get_available_dsc_slices()
Le 17/05/2023 à 20:33, Hamza Mahfooz a écrit : get_available_dsc_slices() returns the number of indices set, and all of the users of get_available_dsc_slices() don't cross the returned bound when iterating over available_slices[]. So, the memset() in get_available_dsc_slices() is redundant and can be dropped. Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)") Reported-by: Christophe JAILLET Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c index b9a05bb025db..58dd62cce4bb 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c @@ -645,8 +645,6 @@ static int get_available_dsc_slices(union dsc_enc_slice_caps slice_caps, int *av { int idx = 0; - memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE); - if (slice_caps.bits.NUM_SLICES_1) available_slices[idx++] = 1; Thanks for it, it went off my radar. FWIW: Reviewed-by: Christophe JAILLET
Re: [PATCH v2 net-next 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
Le 26/04/2023 à 20:54, Justin Chen a écrit : Add support for the Broadcom ASP 2.0 Ethernet controller which is first introduced with 72165. This controller features two distinct Ethernet ports that can be independently operated. This patch supports: - Wake-on-LAN using magic packets - basic ethtool operations (link, counters, message level) - MAC destination address filtering (promiscuous, ALL_MULTI, etc.) Signed-off-by: Florian Fainelli Signed-off-by: Justin Chen --- [...] +void bcmasp_disable_all_filters(struct bcmasp_intf *intf) +{ + struct bcmasp_priv *priv = intf->parent; + unsigned int i; Hi, Nit: Some loop index are unsigned int, but most are int. This could be done consistantly. + + /* Disable all filters held by this port */ + for (i = ASP_RX_FILT_MDA_RES_COUNT(intf); i < NUM_MDA_FILTERS; i++) { + if (priv->mda_filters[i].en && + priv->mda_filters[i].port == intf->port) + bcmasp_en_mda_filter(intf, 0, i); + } +} [...] +static int bcmasp_probe(struct platform_device *pdev) +{ + struct device_node *ports_node, *intf_node; + const struct bcmasp_plat_data *pdata; + struct device *dev = >dev; + int ret, i, count = 0, port; + struct bcmasp_priv *priv; + struct bcmasp_intf *intf; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->irq = platform_get_irq(pdev, 0); + if (priv->irq <= 0) { + dev_err(dev, "invalid interrupt\n"); + return -EINVAL; + } + + priv->clk = devm_clk_get_optional_enabled(dev, "sw_asp"); + if (IS_ERR(priv->clk)) { + dev_err(dev, "failed to request clock\n"); + return PTR_ERR(priv->clk); + } + + /* Base from parent node */ + priv->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(priv->base)) { + dev_err(dev, "failed to iomap\n"); + return PTR_ERR(priv->base); + } + + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(40)); + if (ret) + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32)); I don't think that this fallback is needed. See [1]. More over, using dev_err_probe() would slighly simplify the probe function. (saves a few LoC, logs the error code in a human reading format) [1]: https://lore.kernel.org/lkml/86bf852e-4220-52d4-259d-3455bc24d...@wanadoo.fr/T/#m022abc0051ede3ba1feeb06cefd59e2a8a5c7864 + if (ret) { + dev_err(>dev, "unable to set DMA mask: %d\n", ret); + return ret; + } + [...] +static int __maybe_unused bcmasp_suspend(struct device *d) +{ + struct bcmasp_priv *priv = dev_get_drvdata(d); + struct bcmasp_intf *intf; + unsigned int i; Same + int ret = 0; no need to initialize, but it is mostmy a matter of taste. + + for (i = 0; i < priv->intf_count; i++) { + intf = priv->intfs[i]; + if (!intf) + continue; + + ret = bcmasp_interface_suspend(intf); + if (ret) + break; + } + + ret = clk_prepare_enable(priv->clk); + if (ret) + return ret; + + /* Whether Wake-on-LAN is enabled or not, we can always disable +* the shared TX clock +*/ + bcmasp_core_clock_set(priv, 0, ASP_CTRL_CLOCK_CTRL_ASP_TX_DISABLE); + + bcmasp_core_clock_select(priv, true); + + clk_disable_unprepare(priv->clk); + + return ret; +} + +static int __maybe_unused bcmasp_resume(struct device *d) +{ + struct bcmasp_priv *priv = dev_get_drvdata(d); + struct bcmasp_intf *intf; + unsigned int i; same + int ret = 0; no need to initialize, but it is mostmy a matter of taste. Just my 2c, CJ
[PATCH] drm/amd/display: Correctly initialize some memory in get_available_dsc_slices()
The intent here is to clear the 'available_slices' buffer before setting some values in it. This is an array of int, so in order to fully initialize it, we must clear MIN_AVAILABLE_SLICES_SIZE * sizeof(int) bytes. Compute the right length of the buffer when calling memset(). Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)") Signed-off-by: Christophe JAILLET --- NOT even compile-tested. make -j7 drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.o on my setup, it fails with: CC drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.o drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c:27:10: fatal error: dc_hw_types.h: Aucun fichier ou dossier de ce type 27 | #include "dc_hw_types.h" | ^~~ I've not investigated why. --- drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c index b9a05bb025db..1d7384b2be28 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c @@ -645,7 +645,7 @@ static int get_available_dsc_slices(union dsc_enc_slice_caps slice_caps, int *av { int idx = 0; - memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE); + memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE * sizeof(*available_slices)); if (slice_caps.bits.NUM_SLICES_1) available_slices[idx++] = 1; -- 2.34.1
Re: [PATCH] video: fbdev: mmp: Fix deferred clk handling in mmphw_probe()
Le 19/04/2023 à 06:59, Dan Carpenter a écrit : On Sat, Apr 15, 2023 at 04:09:03PM +0300, Dan Carpenter wrote: On Thu, Apr 13, 2023 at 09:33:17PM +0200, Christophe JAILLET wrote: When dev_err_probe() is called, 'ret' holds the value of the previous successful devm_request_irq() call. 'ret' should be assigned with a meaningful value before being used in dev_err_probe(). While at it, use and return "PTR_ERR(ctrl->clk)" instead of a hard-coded "-ENOENT" so that -EPROBE_DEFER is handled and propagated correctly. Fixes: 81b63420564d ("video: fbdev: mmp: Make use of the helper function dev_err_probe()") Signed-off-by: Christophe JAILLET --- Presumably you already wrote a Coccinelle script for this but I've added it to Smatch as well. No I haven't. I've spotted it while looking at some devm_clk_get_optional() candidate with grep. git grep -A5 devm_clk_get | grep -B5 ENOENT Not perfect, but sometimes this kind of approach can find interesting things coccinelle would miss. As an example, the bitmap_alloc patch on sh4 was found this way, with grep. So nice to have it in smatch, ;-) CJ Here is this warning: drivers/video/fbdev/mmp/hw/mmp_ctrl.c:518 mmphw_probe() warn: passing zero to 'dev_err_probe' Other warnings. All five are interesting. drivers/power/supply/rt9467-charger.c:1026 rt9467_request_interrupt() warn: passing zero to 'dev_err_probe' drivers/pci/controller/dwc/pcie-bt1.c:601 bt1_pcie_add_port() warn: passing zero to 'dev_err_probe' drivers/spi/spi-sprd-adi.c:570 sprd_adi_probe() warn: passing zero to 'dev_err_probe' drivers/soc/qcom/icc-bwmon.c:776 bwmon_probe() warn: passing zero to 'dev_err_probe' drivers/soc/qcom/icc-bwmon.c:781 bwmon_probe() warn: passing zero to 'dev_err_probe' regards, dan carpenter
[PATCH] drm/amd/display: Fix a test dml32_rq_dlg_get_rq_reg()
It is likely p1_min_meta_chunk_bytes was expected here, instead of min_meta_chunk_bytes. Test the correct variable. Fixes: dda4fb85e433 ("drm/amd/display: DML changes for DCN32/321") Signed-off-by: Christophe JAILLET --- .../gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c index 395ae8761980..9ba6cb67655f 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c @@ -116,7 +116,7 @@ void dml32_rq_dlg_get_rq_reg(display_rq_regs_st *rq_regs, else rq_regs->rq_regs_l.min_meta_chunk_size = dml_log2(min_meta_chunk_bytes) - 6 + 1; - if (min_meta_chunk_bytes == 0) + if (p1_min_meta_chunk_bytes == 0) rq_regs->rq_regs_c.min_meta_chunk_size = 0; else rq_regs->rq_regs_c.min_meta_chunk_size = dml_log2(p1_min_meta_chunk_bytes) - 6 + 1; -- 2.34.1
[PATCH] drm/amd/display: Fix a test CalculatePrefetchSchedule()
It is likely Height was expected here, instead of Width. Test the correct variable. Fixes: 17529ea2acfa ("drm/amd/display: Optimizations for DML math") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c index b7c2844d0cbe..f294f2f8c75b 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c @@ -810,7 +810,7 @@ static bool CalculatePrefetchSchedule( *swath_width_chroma_ub = dml_ceil(SwathWidthY / 2 - 1, myPipe->BlockWidth256BytesC) + myPipe->BlockWidth256BytesC; } else { *swath_width_luma_ub = dml_ceil(SwathWidthY - 1, myPipe->BlockHeight256BytesY) + myPipe->BlockHeight256BytesY; - if (myPipe->BlockWidth256BytesC > 0) + if (myPipe->BlockHeight256BytesC > 0) *swath_width_chroma_ub = dml_ceil(SwathWidthY / 2 - 1, myPipe->BlockHeight256BytesC) + myPipe->BlockHeight256BytesC; } -- 2.34.1
[PATCH] video: fbdev: mmp: Fix deferred clk handling in mmphw_probe()
When dev_err_probe() is called, 'ret' holds the value of the previous successful devm_request_irq() call. 'ret' should be assigned with a meaningful value before being used in dev_err_probe(). While at it, use and return "PTR_ERR(ctrl->clk)" instead of a hard-coded "-ENOENT" so that -EPROBE_DEFER is handled and propagated correctly. Fixes: 81b63420564d ("video: fbdev: mmp: Make use of the helper function dev_err_probe()") Signed-off-by: Christophe JAILLET --- drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c index a9df8ee79810..51fbf02a0343 100644 --- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c +++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c @@ -514,9 +514,9 @@ static int mmphw_probe(struct platform_device *pdev) /* get clock */ ctrl->clk = devm_clk_get(ctrl->dev, mi->clk_name); if (IS_ERR(ctrl->clk)) { + ret = PTR_ERR(ctrl->clk); dev_err_probe(ctrl->dev, ret, "unable to get clk %s\n", mi->clk_name); - ret = -ENOENT; goto failed; } clk_prepare_enable(ctrl->clk); -- 2.34.1
Re: [PATCH] drm/armada: Fix a potential double free in an error handling path
Le 26/12/2021 à 17:34, Christophe JAILLET a écrit : 'priv' is a managed resource, so there is no need to free it explicitly or there will be a double free(). Fixes: 90ad200b4cbc ("drm/armada: Use devm_drm_dev_alloc") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/armada/armada_drv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index 8e3e98f13db4..54168134d9b9 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -99,7 +99,6 @@ static int armada_drm_bind(struct device *dev) if (ret) { dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n", __func__, ret); - kfree(priv); return ret; } Hi, polite reminder. (I've updated the mails according to the output of get_maintainer.pl) The patch still LGTM and should apply cleanly. CJ
[PATCH] drm/amd/display: Slightly optimize dm_dmub_outbox1_low_irq()
A kzalloc()+memcpy() can be optimized in a single kmemdup(). This saves a few cycles because some memory doesn't need to be zeroed. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 5bac5781a06b..57a5fbdab890 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -820,15 +820,14 @@ static void dm_dmub_outbox1_low_irq(void *interrupt_params) DRM_ERROR("Failed to allocate dmub_hpd_wrk"); return; } - dmub_hpd_wrk->dmub_notify = kzalloc(sizeof(struct dmub_notification), GFP_ATOMIC); + dmub_hpd_wrk->dmub_notify = kmemdup(, sizeof(struct dmub_notification), + GFP_ATOMIC); if (!dmub_hpd_wrk->dmub_notify) { kfree(dmub_hpd_wrk); DRM_ERROR("Failed to allocate dmub_hpd_wrk->dmub_notify"); return; } INIT_WORK(_hpd_wrk->handle_hpd_work, dm_handle_hpd_work); - if (dmub_hpd_wrk->dmub_notify) - memcpy(dmub_hpd_wrk->dmub_notify, , sizeof(struct dmub_notification)); dmub_hpd_wrk->adev = adev; if (notify.type == DMUB_NOTIFICATION_HPD) { plink = adev->dm.dc->links[notify.link_index]; -- 2.32.0
Re: [PATCH] drm/amdkfd: Fix an illegal memory access
Le 21/02/2023 à 17:26, Felix Kuehling a écrit : On 2023-02-21 06:35, qu.huang-fxuvxftifdnyg1zeobx...@public.gmane.org wrote: From: Qu Huang In the kfd_wait_on_events() function, the kfd_event_waiter structure is allocated by alloc_event_waiters(), but the event field of the waiter structure is not initialized; When copy_from_user() fails in the kfd_wait_on_events() function, it will enter exception handling to release the previously allocated memory of the waiter structure; Due to the event field of the waiters structure being accessed in the free_waiters() function, this results in illegal memory access and system crash, here is the crash log: localhost kernel: RIP: 0010:native_queued_spin_lock_slowpath+0x185/0x1e0 localhost kernel: RSP: 0018:aa53c362bd60 EFLAGS: 00010082 localhost kernel: RAX: ff3d3d6bff4007cb RBX: 0282 RCX: 002c localhost kernel: RDX: 9e855eeacb80 RSI: 279c RDI: e7088f6a21d0 localhost kernel: RBP: e7088f6a21d0 R08: 002c R09: aa53c362be64 localhost kernel: R10: aa53c362bbd8 R11: 0001 R12: 0002 localhost kernel: R13: 9e7ead15d600 R14: R15: 9e7ead15d698 localhost kernel: FS: 152a3d111700() GS:9e855ee8() knlGS: localhost kernel: CS: 0010 DS: ES: CR0: 80050033 localhost kernel: CR2: 15293810 CR3: 00044d7a4000 CR4: 003506e0 localhost kernel: Call Trace: localhost kernel: _raw_spin_lock_irqsave+0x30/0x40 localhost kernel: remove_wait_queue+0x12/0x50 localhost kernel: kfd_wait_on_events+0x1b6/0x490 [hydcu] localhost kernel: ? ftrace_graph_caller+0xa0/0xa0 localhost kernel: kfd_ioctl+0x38c/0x4a0 [hydcu] localhost kernel: ? kfd_ioctl_set_trap_handler+0x70/0x70 [hydcu] localhost kernel: ? kfd_ioctl_create_queue+0x5a0/0x5a0 [hydcu] localhost kernel: ? ftrace_graph_caller+0xa0/0xa0 localhost kernel: __x64_sys_ioctl+0x8e/0xd0 localhost kernel: ? syscall_trace_enter.isra.18+0x143/0x1b0 localhost kernel: do_syscall_64+0x33/0x80 localhost kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9 localhost kernel: RIP: 0033:0x152a4dff68d7 Signed-off-by: Qu Huang --- drivers/gpu/drm/amd/amdkfd/kfd_events.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_events.c index 729d26d..e5faaad 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c @@ -787,6 +787,7 @@ static struct kfd_event_waiter *alloc_event_waiters(uint32_t num_events) for (i = 0; (event_waiters) && (i < num_events) ; i++) { init_wait(_waiters[i].wait); event_waiters[i].activated = false; + event_waiters[i].event = NULL; Thank you for catching this. We're often lazy about initializing things to NULL or 0 because most of our data structures are allocated with kzalloc or similar. I'm not sure why we're not doing this here. If we allocated event_waiters with kcalloc, we could also remove the initialization of activated. I think that would be the cleaner and safer solution. Hi, I think that the '(event_waiters) &&' in the 'for' can also be removed. 'event_waiters' is already NULL tested a few lines above Just my 2c. CJ Regards, Felix } return event_waiters; -- 1.8.3.1
[PATCH] drm/amd: Optimize some memory initializations
Instead of zeroing some memory and then copying data in part or all of it, use memcpy_and_pad(). This avoids writing some memory twice and should save a few cycles. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 11 --- drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c | 8 ++-- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index a8391f269cd0..5e69693a5cc4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -613,9 +613,8 @@ psp_cmd_submit_buf(struct psp_context *psp, if (!drm_dev_enter(adev_to_drm(psp->adev), )) return 0; - memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE); - - memcpy(psp->cmd_buf_mem, cmd, sizeof(struct psp_gfx_cmd_resp)); + memcpy_and_pad(psp->cmd_buf_mem, PSP_CMD_BUFFER_SIZE, cmd, + sizeof(struct psp_gfx_cmd_resp), 0); index = atomic_inc_return(>fence_value); ret = psp_ring_cmd_submit(psp, psp->cmd_buf_mc_addr, fence_mc_addr, index); @@ -947,8 +946,7 @@ static int psp_rl_load(struct amdgpu_device *adev) cmd = acquire_psp_cmd_buf(psp); - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - memcpy(psp->fw_pri_buf, psp->rl.start_addr, psp->rl.size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, psp->rl.start_addr, psp->rl.size_bytes, 0); cmd->cmd_id = GFX_CMD_ID_LOAD_IP_FW; cmd->cmd.cmd_load_ip_fw.fw_phy_addr_lo = lower_32_bits(psp->fw_pri_mc_addr); @@ -3479,8 +3477,7 @@ void psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t bin_size if (!drm_dev_enter(adev_to_drm(psp->adev), )) return; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - memcpy(psp->fw_pri_buf, start_addr, bin_size); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, start_addr, bin_size, 0); drm_dev_exit(idx); } diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c index d62fcc77af95..79733ec4ffab 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c @@ -168,10 +168,8 @@ static int psp_v13_0_bootloader_load_component(struct psp_context *psp, if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy PSP KDB binary to memory */ - memcpy(psp->fw_pri_buf, bin_desc->start_addr, bin_desc->size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, bin_desc->start_addr, bin_desc->size_bytes, 0); /* Provide the PSP KDB to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, @@ -237,10 +235,8 @@ static int psp_v13_0_bootloader_load_sos(struct psp_context *psp) if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy Secure OS binary to PSP memory */ - memcpy(psp->fw_pri_buf, psp->sos.start_addr, psp->sos.size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, psp->sos.start_addr, psp->sos.size_bytes, 0); /* Provide the PSP secure OS to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c index d5ba58eba3e2..c73415b09e85 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c @@ -107,10 +107,8 @@ static int psp_v13_0_4_bootloader_load_component(struct psp_context*psp, if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy PSP KDB binary to memory */ - memcpy(psp->fw_pri_buf, bin_desc->start_addr, bin_desc->size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, bin_desc->start_addr, bin_desc->size_bytes, 0); /* Provide the PSP KDB to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, @@ -170,10 +168,8 @@ static int psp_v13_0_4_bootloader_load_sos(struct psp_context *psp) if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy Secure OS binary to PSP memory */ - memcpy(psp->fw_pri_buf, psp->sos.start_addr, psp->sos.size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, psp->sos.start_addr, psp->sos.size_bytes, 0); /* Provide the PSP secure OS to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, -- 2.34.1
Re: [PATCH] agp/amd64: Fix full name of the GPL
Le 22/01/2023 à 19:16, Diederik de Haas a écrit : Signed-off-by: Diederik de Haas --- drivers/char/agp/amd64-agp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c index ce8651436609..3020fd92fd00 100644 --- a/drivers/char/agp/amd64-agp.c +++ b/drivers/char/agp/amd64-agp.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright 2001-2003 SuSE Labs. - * Distributed under the GNU public license, v2. + * Distributed under the GNU General Public License, v2. * * This is a GART driver for the AMD Opteron/Athlon64 on-CPU northbridge. * It also includes support for the AMD 8151 AGP bridge, Hi, maybe the line can just be removed. There is already a SPDX-License-Identifier above. CJ
[PATCH v2] video: fbdev: omapfb: Use kstrtobool() instead of strtobool()
strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET --- This patch was already sent as a part of a serie ([1]) that axed all usages of strtobool(). Most of the patches have been merged in -next. I synch'ed with latest -next and re-send the remaining ones as individual patches. Changes in v2: - No change [1]: https://lore.kernel.org/all/cover.1667336095.git.christophe.jail...@wanadoo.fr/ --- drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c | 3 ++- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c index bc5a44c2a144..ae937854403b 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c @@ -10,6 +10,7 @@ #define DSS_SUBSYS_NAME "DISPLAY" #include +#include #include #include #include @@ -36,7 +37,7 @@ static ssize_t display_enabled_store(struct omap_dss_device *dssdev, int r; bool enable; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -73,7 +74,7 @@ static ssize_t display_tear_store(struct omap_dss_device *dssdev, if (!dssdev->driver->enable_te || !dssdev->driver->get_te) return -ENOENT; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -183,7 +184,7 @@ static ssize_t display_mirror_store(struct omap_dss_device *dssdev, if (!dssdev->driver->set_mirror || !dssdev->driver->get_mirror) return -ENOENT; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c index ba21c4a2633d..1b644be5fe2e 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c @@ -10,6 +10,7 @@ #define DSS_SUBSYS_NAME "MANAGER" #include +#include #include #include #include @@ -246,7 +247,7 @@ static ssize_t manager_trans_key_enabled_store(struct omap_overlay_manager *mgr, bool enable; int r; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -290,7 +291,7 @@ static ssize_t manager_alpha_blending_enabled_store( if(!dss_has_feature(FEAT_ALPHA_FIXED_ZORDER)) return -ENODEV; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -329,7 +330,7 @@ static ssize_t manager_cpr_enable_store(struct omap_overlay_manager *mgr, if (!dss_has_feature(FEAT_CPR)) return -ENODEV; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c index 601c0beb6de9..1da4fb1c77b4 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -210,7 +211,7 @@ static ssize_t overlay_enabled_store(struct omap_overlay *ovl, const char *buf, int r; bool enable; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c index 06dc41aa0354..831b2c2fbdf9 100644 --- a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -96,7 +97,7 @@ static ssize_t store_mirror(struct device *dev, int r; struct fb_var_screeninfo new_var; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; -- 2.34.1
[PATCH v2] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper: - calls devm_clk_get() - calls clk_prepare_enable() and registers what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the need of a dedicated function used with devm_add_action_or_reset(). Signed-off-by: Christophe JAILLET Reviewed-by: Andrzej Hajda --- Change in v2: - Convert to dev_err_probe()[Andrzej Hajda] - Update the error message[Andrzej Hajda] - Add R-b tag[Andrzej Hajda] v1: https://lore.kernel.org/all/4f855984ea895e1488169e77935fa6e044912ac2.1672414073.git.christophe.jail...@wanadoo.fr/ --- drivers/gpu/drm/bridge/tc358767.c | 25 - 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 2a58eb271f70..99f3d5ca7257 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -2022,13 +2022,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) return -EINVAL; } -static void tc_clk_disable(void *data) -{ - struct clk *refclk = data; - - clk_disable_unprepare(refclk); -} - static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct device *dev = >dev; @@ -2045,20 +2038,10 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) if (ret) return ret; - tc->refclk = devm_clk_get(dev, "ref"); - if (IS_ERR(tc->refclk)) { - ret = PTR_ERR(tc->refclk); - dev_err(dev, "Failed to get refclk: %d\n", ret); - return ret; - } - - ret = clk_prepare_enable(tc->refclk); - if (ret) - return ret; - - ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); - if (ret) - return ret; + tc->refclk = devm_clk_get_enabled(dev, "ref"); + if (IS_ERR(tc->refclk)) + return dev_err_probe(dev, PTR_ERR(tc->refclk), +"Failed to get and enable the ref clk\n"); /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */ usleep_range(10, 15); -- 2.34.1
[PATCH] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper: - calls devm_clk_get() - calls clk_prepare_enable() and registers what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the need of a dedicated function used with devm_add_action_or_reset(). Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/bridge/tc358767.c | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 2a58eb271f70..b95c36ca660d 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -2022,13 +2022,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) return -EINVAL; } -static void tc_clk_disable(void *data) -{ - struct clk *refclk = data; - - clk_disable_unprepare(refclk); -} - static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct device *dev = >dev; @@ -2045,21 +2038,13 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) if (ret) return ret; - tc->refclk = devm_clk_get(dev, "ref"); + tc->refclk = devm_clk_get_enabled(dev, "ref"); if (IS_ERR(tc->refclk)) { ret = PTR_ERR(tc->refclk); dev_err(dev, "Failed to get refclk: %d\n", ret); return ret; } - ret = clk_prepare_enable(tc->refclk); - if (ret) - return ret; - - ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); - if (ret) - return ret; - /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */ usleep_range(10, 15); -- 2.34.1
Re: [PATCH] drm/amd/pm: avoid large variable on kernel stack
Le 15/12/2022 à 20:46, Christophe JAILLET a écrit : Le 15/12/2022 à 17:36, Arnd Bergmann a écrit : From: Arnd Bergmann The activity_monitor_external[] array is too big to fit on the kernel stack, resulting in this warning with clang: drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1438:12: error: stack frame size (1040) exceeds limit (1024) in 'smu_v13_0_7_get_power_profile_mode' [-Werror,-Wframe-larger-than] Use dynamic allocation instead. It should also be possible to have single element here instead of the array, but this seems easier. Fixes: 334682ae8151 ("drm/amd/pm: enable workload type change on smu_v13_0_7") Signed-off-by: Arnd Bergmann --- .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c index c270f94a1b86..7eba854e09ec 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c @@ -1439,7 +1439,7 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu, static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf) { - DpmActivityMonitorCoeffIntExternal_t activity_monitor_external[PP_SMC_POWER_PROFILE_COUNT]; + DpmActivityMonitorCoeffIntExternal_t *activity_monitor_external; uint32_t i, j, size = 0; int16_t workload_type = 0; int result = 0; @@ -1447,6 +1447,12 @@ static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf if (!buf) return -EINVAL; + activity_monitor_external = kcalloc(sizeof(activity_monitor_external), Hi, Before, 'activity_monitor_external' was not initialized. Maybe kcalloc() is enough? Read kmalloc_array() sizeof(*activity_monitor_external)? + PP_SMC_POWER_PROFILE_COUNT, + GFP_KERNEL); + if (!activity_monitor_external) + return -ENOMEM; + size += sysfs_emit_at(buf, size, " "); for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++) Unrelated, but wouldn't it be more straightforward with "< PP_SMC_POWER_PROFILE_COUNT"? size += sysfs_emit_at(buf, size, "%-14s%s", amdgpu_pp_profile_name[i], @@ -1459,15 +1465,17 @@ static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf workload_type = smu_cmn_to_asic_specific_index(smu, CMN2ASIC_MAPPING_WORKLOAD, i); - if (workload_type < 0) - return -EINVAL; + if (workload_type < 0) { + result = -EINVAL; + goto out; + } result = smu_cmn_update_table(smu, SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type, (void *)(_monitor_external[i]), false); if (result) { dev_err(smu->adev->dev, "[%s] Failed to get activity monitor!", __func__); - return result; + goto out; } } @@ -1495,7 +1503,10 @@ do { \ PRINT_DPM_MONITOR(Fclk_BoosterFreq); #undef PRINT_DPM_MONITOR - return size; + result = size; +out: + kfree(activity_monitor_external); + return result; } static int smu_v13_0_7_set_power_profile_mode(struct smu_context *smu, long *input, uint32_t size)
Re: [PATCH] drm/amd/pm: avoid large variable on kernel stack
Le 15/12/2022 à 17:36, Arnd Bergmann a écrit : From: Arnd Bergmann The activity_monitor_external[] array is too big to fit on the kernel stack, resulting in this warning with clang: drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1438:12: error: stack frame size (1040) exceeds limit (1024) in 'smu_v13_0_7_get_power_profile_mode' [-Werror,-Wframe-larger-than] Use dynamic allocation instead. It should also be possible to have single element here instead of the array, but this seems easier. Fixes: 334682ae8151 ("drm/amd/pm: enable workload type change on smu_v13_0_7") Signed-off-by: Arnd Bergmann --- .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c index c270f94a1b86..7eba854e09ec 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c @@ -1439,7 +1439,7 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu, static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf) { - DpmActivityMonitorCoeffIntExternal_t activity_monitor_external[PP_SMC_POWER_PROFILE_COUNT]; + DpmActivityMonitorCoeffIntExternal_t *activity_monitor_external; uint32_t i, j, size = 0; int16_t workload_type = 0; int result = 0; @@ -1447,6 +1447,12 @@ static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf if (!buf) return -EINVAL; + activity_monitor_external = kcalloc(sizeof(activity_monitor_external), Hi, Before, 'activity_monitor_external' was not initialized. Maybe kcalloc() is enough? sizeof(*activity_monitor_external)? + PP_SMC_POWER_PROFILE_COUNT, + GFP_KERNEL); + if (!activity_monitor_external) + return -ENOMEM; + size += sysfs_emit_at(buf, size, " "); for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++) Unrelated, but wouldn't it be more straightforward with "< PP_SMC_POWER_PROFILE_COUNT"? size += sysfs_emit_at(buf, size, "%-14s%s", amdgpu_pp_profile_name[i], @@ -1459,15 +1465,17 @@ static int smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf workload_type = smu_cmn_to_asic_specific_index(smu, CMN2ASIC_MAPPING_WORKLOAD, i); - if (workload_type < 0) - return -EINVAL; + if (workload_type < 0) { + result = -EINVAL; + goto out; + } result = smu_cmn_update_table(smu, SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type, (void *)(_monitor_external[i]), false); if (result) { dev_err(smu->adev->dev, "[%s] Failed to get activity monitor!", __func__); - return result; + goto out; } } @@ -1495,7 +1503,10 @@ do { \ PRINT_DPM_MONITOR(Fclk_BoosterFreq); #undef PRINT_DPM_MONITOR - return size; + result = size; +out: + kfree(activity_monitor_external); + return result; } static int smu_v13_0_7_set_power_profile_mode(struct smu_context *smu, long *input, uint32_t size)
[PATCH v2] drm/msm/hdmi: Fix the error handling path of msm_hdmi_dev_probe()
If an error occurs after a successful msm_hdmi_get_phy() call, it must be undone by a corresponding msm_hdmi_put_phy(), as already done in the remove function. Fixes: 437365464043 ("drm/msm/hdmi: move msm_hdmi_get_phy() to msm_hdmi_dev_probe()") Signed-off-by: Christophe JAILLET Reviewed-by: Abhinav Kumar --- Change in v2: - Fix a typo in the prefix of the subject line[Abhinav Kumar] - Add R-b tag[Abhinav Kumar] v1: https://lore.kernel.org/all/b3d9dac978f1e2e42a40ec61f58aa98c44c85dfd.1670741386.git.christophe.jail...@wanadoo.fr/ --- drivers/gpu/drm/msm/hdmi/hdmi.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 4d3fdc806bef..97372bb241d8 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -532,11 +532,19 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev) ret = devm_pm_runtime_enable(>dev); if (ret) - return ret; + goto err_put_phy; platform_set_drvdata(pdev, hdmi); - return component_add(>dev, _hdmi_ops); + ret = component_add(>dev, _hdmi_ops); + if (ret) + goto err_put_phy; + + return 0; + +err_put_phy: + msm_hdmi_put_phy(hdmi); + return ret; } static int msm_hdmi_dev_remove(struct platform_device *pdev) -- 2.34.1
[PATCH] drm/msm/hdm: Fix the error handling path of msm_hdmi_dev_probe()
If an error occurs after a successful msm_hdmi_get_phy() call, it must be undone by a corresponding msm_hdmi_put_phy(), as already done in the remove function. Fixes: 437365464043 ("drm/msm/hdmi: move msm_hdmi_get_phy() to msm_hdmi_dev_probe()") Signed-off-by: Christophe JAILLET --- Not sure if the Fixes tag is correct. At least it is when the probe needs to be fixed but the issue was maybe there elsewhere before. --- drivers/gpu/drm/msm/hdmi/hdmi.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 4d3fdc806bef..97372bb241d8 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -532,11 +532,19 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev) ret = devm_pm_runtime_enable(>dev); if (ret) - return ret; + goto err_put_phy; platform_set_drvdata(pdev, hdmi); - return component_add(>dev, _hdmi_ops); + ret = component_add(>dev, _hdmi_ops); + if (ret) + goto err_put_phy; + + return 0; + +err_put_phy: + msm_hdmi_put_phy(hdmi); + return ret; } static int msm_hdmi_dev_remove(struct platform_device *pdev) -- 2.34.1
[PATCH v2 2/2] video: fbdev: uvesafb: Simplify uvesafb_remove()
When the remove() function is called, we know that the probe() function has successfully been executed. So 'info' is known to be not NULL. Simplify the code accordingly. Signed-off-by: Christophe JAILLET --- Change in v2: - new patch --- drivers/video/fbdev/uvesafb.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index 0e3cabbec4b4..2bb95c35ab2a 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -1777,25 +1777,23 @@ static int uvesafb_probe(struct platform_device *dev) static int uvesafb_remove(struct platform_device *dev) { struct fb_info *info = platform_get_drvdata(dev); + struct uvesafb_par *par = info->par; - if (info) { - struct uvesafb_par *par = info->par; + sysfs_remove_group(>dev.kobj, _dev_attgrp); + unregister_framebuffer(info); + release_region(0x3c0, 32); + iounmap(info->screen_base); + arch_phys_wc_del(par->mtrr_handle); + release_mem_region(info->fix.smem_start, info->fix.smem_len); + fb_destroy_modedb(info->monspecs.modedb); + fb_dealloc_cmap(>cmap); - sysfs_remove_group(>dev.kobj, _dev_attgrp); - unregister_framebuffer(info); - release_region(0x3c0, 32); - iounmap(info->screen_base); - arch_phys_wc_del(par->mtrr_handle); - release_mem_region(info->fix.smem_start, info->fix.smem_len); - fb_destroy_modedb(info->monspecs.modedb); - fb_dealloc_cmap(>cmap); + kfree(par->vbe_modes); + kfree(par->vbe_state_orig); + kfree(par->vbe_state_saved); - kfree(par->vbe_modes); - kfree(par->vbe_state_orig); - kfree(par->vbe_state_saved); + framebuffer_release(info); - framebuffer_release(info); - } return 0; } -- 2.34.1
[PATCH v2 1/2] video: fbdev: uvesafb: Fixes an error handling path in uvesafb_probe()
If an error occurs after a successful uvesafb_init_mtrr() call, it must be undone by a corresponding arch_phys_wc_del() call, as already done in the remove function. This has been added in the remove function in commit 63e28a7a5ffc ("uvesafb: Clean up MTRR code") Fixes: 8bdb3a2d7df4 ("uvesafb: the driver core") Signed-off-by: Christophe JAILLET --- Unsure about the Fixes tag, maybe it is 63e28a7a5ffc Change in v2: - add arch_phys_wc_del() at the right place in the error handling path v1 (a long time ago!): https://lore.kernel.org/all/dd2a4806d3a570ab84947806f38a494454fd0245.1622994310.git.christophe.jail...@wanadoo.fr/ --- drivers/video/fbdev/uvesafb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index 00d789b6c0fa..0e3cabbec4b4 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -1758,6 +1758,7 @@ static int uvesafb_probe(struct platform_device *dev) out_unmap: iounmap(info->screen_base); out_mem: + arch_phys_wc_del(par->mtrr_handle); release_mem_region(info->fix.smem_start, info->fix.smem_len); out_reg: release_region(0x3c0, 32); -- 2.34.1
[PATCH 15/30] video: fbdev: omapfb: Use kstrtobool() instead of strtobool()
strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET --- This patch is part of a serie that axes all usages of strtobool(). Each patch can be applied independently from the other ones. The last patch of the serie removes the definition of strtobool(). You may not be in copy of the cover letter. So, if needed, it is available at [1]. [1]: https://lore.kernel.org/all/cover.1667336095.git.christophe.jail...@wanadoo.fr/ --- drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c | 3 ++- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c index bc5a44c2a144..ae937854403b 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c @@ -10,6 +10,7 @@ #define DSS_SUBSYS_NAME "DISPLAY" #include +#include #include #include #include @@ -36,7 +37,7 @@ static ssize_t display_enabled_store(struct omap_dss_device *dssdev, int r; bool enable; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -73,7 +74,7 @@ static ssize_t display_tear_store(struct omap_dss_device *dssdev, if (!dssdev->driver->enable_te || !dssdev->driver->get_te) return -ENOENT; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -183,7 +184,7 @@ static ssize_t display_mirror_store(struct omap_dss_device *dssdev, if (!dssdev->driver->set_mirror || !dssdev->driver->get_mirror) return -ENOENT; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c index ba21c4a2633d..1b644be5fe2e 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c @@ -10,6 +10,7 @@ #define DSS_SUBSYS_NAME "MANAGER" #include +#include #include #include #include @@ -246,7 +247,7 @@ static ssize_t manager_trans_key_enabled_store(struct omap_overlay_manager *mgr, bool enable; int r; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -290,7 +291,7 @@ static ssize_t manager_alpha_blending_enabled_store( if(!dss_has_feature(FEAT_ALPHA_FIXED_ZORDER)) return -ENODEV; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; @@ -329,7 +330,7 @@ static ssize_t manager_cpr_enable_store(struct omap_overlay_manager *mgr, if (!dss_has_feature(FEAT_CPR)) return -ENODEV; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c index 601c0beb6de9..1da4fb1c77b4 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -210,7 +211,7 @@ static ssize_t overlay_enabled_store(struct omap_overlay *ovl, const char *buf, int r; bool enable; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c index 06dc41aa0354..831b2c2fbdf9 100644 --- a/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -96,7 +97,7 @@ static ssize_t store_mirror(struct device *dev, int r; struct fb_var_screeninfo new_var; - r = strtobool(buf, ); + r = kstrtobool(buf, ); if (r) return r; -- 2.34.1
Re: [PATCH] drm/i915/perf: remove redundant variable 'taken'
Le 07/10/2022 à 21:53, Colin Ian King a écrit : The assignment to variable taken is redundant and so it can be removed as well as the variable too. Cleans up clang-scan build warnings: warning: Although the value stored to 'taken' is used in the enclosing expression, the value is never actually read from 'taken' [deadcode.DeadStores] Hi, #define OA_TAKEN(tail, head)((tail - head) & (OA_BUFFER_SIZE - 1)) So if the result is not used, maybe calling OA_TAKEN() can be removed as well? It looks like a no-op in such a case. CJ Signed-off-by: Colin Ian King --- drivers/gpu/drm/i915/i915_perf.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 0defbb43ceea..15816df916c7 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -656,7 +656,6 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream, size_t start_offset = *offset; unsigned long flags; u32 head, tail; - u32 taken; int ret = 0; if (drm_WARN_ON(>i915->drm, !stream->enabled)) @@ -692,7 +691,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream, for (/* none */; -(taken = OA_TAKEN(tail, head)); +OA_TAKEN(tail, head); head = (head + report_size) & mask) { u8 *report = oa_buf_base + head; u32 *report32 = (void *)report; @@ -950,7 +949,6 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, size_t start_offset = *offset; unsigned long flags; u32 head, tail; - u32 taken; int ret = 0; if (drm_WARN_ON(>i915->drm, !stream->enabled)) @@ -984,7 +982,7 @@ static int gen7_append_oa_reports(struct i915_perf_stream *stream, for (/* none */; -(taken = OA_TAKEN(tail, head)); +OA_TAKEN(tail, head); head = (head + report_size) & mask) { u8 *report = oa_buf_base + head; u32 *report32 = (void *)report;
[PATCH] headers: Remove some left-over license text in include/uapi/drm/
There is already a SPDX-License-Identifier tag, so the corresponding license text can be removed. Signed-off-by: Christophe JAILLET --- include/uapi/drm/armada_drm.h | 4 include/uapi/drm/etnaviv_drm.h | 12 include/uapi/drm/exynos_drm.h | 5 - include/uapi/drm/omap_drm.h| 12 4 files changed, 33 deletions(-) diff --git a/include/uapi/drm/armada_drm.h b/include/uapi/drm/armada_drm.h index af1c14c837c5..f711e63a9758 100644 --- a/include/uapi/drm/armada_drm.h +++ b/include/uapi/drm/armada_drm.h @@ -2,10 +2,6 @@ /* * Copyright (C) 2012 Russell King * With inspiration from the i915 driver - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #ifndef DRM_ARMADA_IOCTL_H #define DRM_ARMADA_IOCTL_H diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h index af024d90453d..13dd1d1a9d41 100644 --- a/include/uapi/drm/etnaviv_drm.h +++ b/include/uapi/drm/etnaviv_drm.h @@ -1,18 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ /* * Copyright (C) 2015 Etnaviv Project - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 as published by - * the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along with - * this program. If not, see <http://www.gnu.org/licenses/>. */ #ifndef __ETNAVIV_DRM_H__ diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h index a51aa1c618c1..a96fa566433c 100644 --- a/include/uapi/drm/exynos_drm.h +++ b/include/uapi/drm/exynos_drm.h @@ -6,11 +6,6 @@ * Inki Dae * Joonyoung Shim * Seung-Woo Kim - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the - * Free Software Foundation; either version 2 of the License, or (at your - * option) any later version. */ #ifndef _UAPI_EXYNOS_DRM_H_ diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h index 5a142fad473c..b51dad32122d 100644 --- a/include/uapi/drm/omap_drm.h +++ b/include/uapi/drm/omap_drm.h @@ -4,18 +4,6 @@ * * Copyright (C) 2011 Texas Instruments * Author: Rob Clark - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 as published by - * the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along with - * this program. If not, see <http://www.gnu.org/licenses/>. */ #ifndef __OMAP_DRM_H__ -- 2.34.1
Re: [PATCH] drm/hyperv: Fix an error handling path in hyperv_vmbus_probe()
Le 15/08/2022 à 17:56, Wei Liu a écrit : All that said, the fix looks good, so Reviewed-by: Michael Kelley I made the two changes listed above and applied this patch to hyperv-fixes. Thanks a lot, that saves me a v2. CJ Thanks, Wei.
[PATCH] drm/vc4: Use the bitmap API to allocate bitmaps
Use bitmap_zalloc()/bitmap_free() instead of hand-writing them. It is less verbose and it improves the semantic. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/vc4/vc4_validate_shaders.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_validate_shaders.c b/drivers/gpu/drm/vc4/vc4_validate_shaders.c index e315aeb5fef5..d074d2014be4 100644 --- a/drivers/gpu/drm/vc4/vc4_validate_shaders.c +++ b/drivers/gpu/drm/vc4/vc4_validate_shaders.c @@ -795,9 +795,8 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj) reset_validation_state(_state); - validation_state.branch_targets = - kcalloc(BITS_TO_LONGS(validation_state.max_ip), - sizeof(unsigned long), GFP_KERNEL); + validation_state.branch_targets = bitmap_zalloc(validation_state.max_ip, + GFP_KERNEL); if (!validation_state.branch_targets) goto fail; @@ -939,12 +938,12 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj) (validated_shader->uniforms_size + 4 * validated_shader->num_texture_samples); - kfree(validation_state.branch_targets); + bitmap_free(validation_state.branch_targets); return validated_shader; fail: - kfree(validation_state.branch_targets); + bitmap_free(validation_state.branch_targets); if (validated_shader) { kfree(validated_shader->uniform_addr_offsets); kfree(validated_shader->texture_samples); -- 2.34.1
[PATCH] gpu: host1x: Use the bitmap API to allocate bitmaps
Use bitmap_zalloc()/bitmap_free() instead of hand-writing them. It is less verbose and it improves the semantic. While at it, remove a useless bitmap_zero() call. The bitmap is already zero'ed when allocated. Signed-off-by: Christophe JAILLET --- drivers/gpu/host1x/channel.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c index 2a9a3a8d5931..2d0051d6314c 100644 --- a/drivers/gpu/host1x/channel.c +++ b/drivers/gpu/host1x/channel.c @@ -21,22 +21,18 @@ int host1x_channel_list_init(struct host1x_channel_list *chlist, if (!chlist->channels) return -ENOMEM; - chlist->allocated_channels = - kcalloc(BITS_TO_LONGS(num_channels), sizeof(unsigned long), - GFP_KERNEL); + chlist->allocated_channels = bitmap_zalloc(num_channels, GFP_KERNEL); if (!chlist->allocated_channels) { kfree(chlist->channels); return -ENOMEM; } - bitmap_zero(chlist->allocated_channels, num_channels); - return 0; } void host1x_channel_list_free(struct host1x_channel_list *chlist) { - kfree(chlist->allocated_channels); + bitmap_free(chlist->allocated_channels); kfree(chlist->channels); } -- 2.34.1
[PATCH] drm/rockchip: Fix an error handling path rockchip_dp_probe()
Should component_add() fail, we should call analogix_dp_remove() in the error handling path, as already done in the remove function. Fixes: 152cce0006ab ("drm/bridge: analogix_dp: Split bind() into probe() and real bind()") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 70be64ca0a00..ad2d3ae7e621 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -408,7 +408,15 @@ static int rockchip_dp_probe(struct platform_device *pdev) if (IS_ERR(dp->adp)) return PTR_ERR(dp->adp); - return component_add(dev, _dp_component_ops); + ret = component_add(dev, _dp_component_ops); + if (ret) + goto err_dp_remove; + + return 0; + +err_dp_remove: + analogix_dp_remove(dp->adp); + return ret; } static int rockchip_dp_remove(struct platform_device *pdev) -- 2.34.1
[PATCH] drm/bochs: Fix some error handling paths in bochs_pci_probe()
The remove() function calls bochs_hw_fini() but this function is not called in the error handling of the probe. This call releases the resources allocated by bochs_hw_init() used in bochs_load(). Update the probe and bochs_load() to call bochs_hw_fini() if an error occurs after a successful bochs_hw_init() call. Signed-off-by: Christophe JAILLET --- I've not been able to find a Fixes: tag because of code re-arrangement done in 796c3e35ac16. --- drivers/gpu/drm/tiny/bochs.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index 4f8bf86633df..d7a34ed4be3e 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -581,13 +581,17 @@ static int bochs_load(struct drm_device *dev) ret = drmm_vram_helper_init(dev, bochs->fb_base, bochs->fb_size); if (ret) - return ret; + goto err_hw_fini; ret = bochs_kms_init(bochs); if (ret) - return ret; + goto err_hw_fini; return 0; + +err_hw_fini: + bochs_hw_fini(dev); + return ret; } DEFINE_DRM_GEM_FOPS(bochs_fops); @@ -662,11 +666,13 @@ static int bochs_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent ret = drm_dev_register(dev, 0); if (ret) - goto err_free_dev; + goto err_hw_fini; drm_fbdev_generic_setup(dev, 32); return ret; +err_hw_fini: + bochs_hw_fini(dev); err_free_dev: drm_dev_put(dev); return ret; -- 2.34.1
Re: [PATCH] drm/connector: Remove usage of the deprecated ida_simple_xxx API
Le 16/06/2022 à 07:18, Bo Liu a écrit : Use ida_alloc_xxx()/ida_free() instead of ida_simple_get()/ida_simple_remove(). The latter is deprecated and more verbose. Signed-off-by: Bo Liu --- drivers/gpu/drm/drm_connector.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1c48d162c77e..e3484b115ae6 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -250,7 +250,7 @@ int drm_connector_init(struct drm_device *dev, connector->funcs = funcs; /* connector index is used with 32bit bitmasks */ - ret = ida_simple_get(>connector_ida, 0, 32, GFP_KERNEL); + ret = ida_alloc_max(>connector_ida, 31, GFP_KERNEL); if (ret < 0) { DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n", drm_connector_enum_list[connector_type].name, @@ -262,7 +262,7 @@ int drm_connector_init(struct drm_device *dev, connector->connector_type = connector_type; connector->connector_type_id = - ida_simple_get(connector_ida, 1, 0, GFP_KERNEL); + ida_alloc_min(connector_ida, 1, GFP_KERNEL); if (connector->connector_type_id < 0) { ret = connector->connector_type_id; goto out_put_id; @@ -322,10 +322,10 @@ int drm_connector_init(struct drm_device *dev, connector->debugfs_entry = NULL; out_put_type_id: if (ret) - ida_simple_remove(connector_ida, connector->connector_type_id); + ida_free(connector_ida, connector->connector_type_id); out_put_id: if (ret) - ida_simple_remove(>connector_ida, connector->index); + ida_free(>connector_ida, connector->index); out_put: if (ret) drm_mode_object_unregister(dev, >base); @@ -479,10 +479,10 @@ void drm_connector_cleanup(struct drm_connector *connector) list_for_each_entry_safe(mode, t, >modes, head) drm_mode_remove(connector, mode); - ida_simple_remove(_connector_enum_list[connector->connector_type].ida, + ida_free(_connector_enum_list[connector->connector_type].ida, connector->connector_type_id); Hi, Nitpick: the code should be aligned with "_connector_enum_list" now - ida_simple_remove(>mode_config.connector_ida, + ida_free(>mode_config.connector_ida, connector->index); Same here, but I guess that it fits one one line now. kfree(connector->display_info.bus_formats);
Re: [PATCH v10 14/21] drm/mediatek: dpi: Add dpintf support
Le 23/05/2022 à 12:47, Guillaume Ranquet a écrit : dpintf is the displayport interface hardware unit. This unit is similar to dpi and can reuse most of the code. This patch adds support for mt8195-dpintf to this dpi driver. Main differences are: - Some features/functional components are not available for dpintf which are now excluded from code execution once is_dpintf is set - dpintf can and needs to choose between different clockdividers based on the clockspeed. This is done by choosing a different clock parent. - There are two additional clocks that need to be managed. These are only set for dpintf and will be set to NULL if not supplied. The clk_* calls handle these as normal clocks then. - Some register contents differ slightly between the two components. To work around this I added register bits/masks with a DPINTF_ prefix and use them where different. Based on a separate driver for dpintf created by Jason-JH.Lin . Signed-off-by: Markus Schneider-Pargmann Signed-off-by: Guillaume Ranquet --- drivers/gpu/drm/mediatek/mtk_dpi.c | 126 +--- drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 35 ++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 ++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- include/linux/soc/mediatek/mtk-mmsys.h | 4 +- 6 files changed, 159 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index eb969c5c5c2e..763bfb700135 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -71,6 +71,7 @@ struct mtk_dpi { void __iomem *regs; struct device *dev; struct clk *engine_clk; + struct clk *dpi_ck_cg; struct clk *pixel_clk; struct clk *tvd_clk; int irq; @@ -126,6 +127,7 @@ struct mtk_dpi_conf { const u32 *output_fmts; u32 num_output_fmts; bool is_ck_de_pol; + bool is_dpintf; bool swap_input_support; /* Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) */ u32 dimension_mask; @@ -438,6 +440,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) mtk_dpi_disable(dpi); clk_disable_unprepare(dpi->pixel_clk); clk_disable_unprepare(dpi->engine_clk); + clk_disable_unprepare(dpi->dpi_ck_cg); + clk_disable_unprepare(dpi->tvd_clk); } static int mtk_dpi_power_on(struct mtk_dpi *dpi) @@ -447,12 +451,24 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) if (++dpi->refcount != 1) return 0; Hi, belwo the error handling path looks odd. (both where we goto, and the order of the clk_disable_unprepare() in the error handling path. just my 2c, CJ + ret = clk_prepare_enable(dpi->tvd_clk); + if (ret) { + dev_err(dpi->dev, "Failed to enable tvd pll: %d\n", ret); + goto err_pixel; + } + ret = clk_prepare_enable(dpi->engine_clk); if (ret) { dev_err(dpi->dev, "Failed to enable engine clock: %d\n", ret); goto err_refcount; } + ret = clk_prepare_enable(dpi->dpi_ck_cg); + if (ret) { + dev_err(dpi->dev, "Failed to enable dpi_ck_cg clock: %d\n", ret); + goto err_ck_cg; + } + ret = clk_prepare_enable(dpi->pixel_clk); if (ret) { dev_err(dpi->dev, "Failed to enable pixel clock: %d\n", ret); @@ -466,6 +482,8 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) return 0; err_pixel: + clk_disable_unprepare(dpi->dpi_ck_cg); +err_ck_cg: clk_disable_unprepare(dpi->engine_clk); err_refcount: dpi->refcount--; [...]
Re: [PATCH 1/2] drm/amdkfd: Use bitmap_zalloc() when applicable
Le 26/04/2022 à 20:01, Felix Kuehling a écrit : Hi Christophe, I just stumbled over this patch series while cleaning up my inbox. Sorry for dropping it back in November. I'm about to apply it but I noticed that patch 1 is missing a Signed-off-by. Is it OK to add that in your name? Hi, No problem for me if you can add it. Thanks. But if you prefer a v2, it is also fine for me. BTW sorry for missing the SoB tag. This definitively means that I forgot the checkpatch.pl step for this patch, which is bad. CJ Thanks, Felix Am 2021-11-28 um 11:45 schrieb Christophe JAILLET: 'kfd->gtt_sa_bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify code, improve the semantic and avoid some open-coded arithmetic in allocator arguments. Also change the corresponding 'kfree()' into 'bitmap_free()' to keep consistency. --- drivers/gpu/drm/amd/amdkfd/kfd_device.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c index e1294fba0c26..c5a0ce44a295 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c @@ -1252,8 +1252,6 @@ int kgd2kfd_schedule_evict_and_restore_process(struct mm_struct *mm, static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size, unsigned int chunk_size) { - unsigned int num_of_longs; - if (WARN_ON(buf_size < chunk_size)) return -EINVAL; if (WARN_ON(buf_size == 0)) @@ -1264,11 +1262,8 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size, kfd->gtt_sa_chunk_size = chunk_size; kfd->gtt_sa_num_of_chunks = buf_size / chunk_size; - num_of_longs = (kfd->gtt_sa_num_of_chunks + BITS_PER_LONG - 1) / - BITS_PER_LONG; - - kfd->gtt_sa_bitmap = kcalloc(num_of_longs, sizeof(long), GFP_KERNEL); - + kfd->gtt_sa_bitmap = bitmap_zalloc(kfd->gtt_sa_num_of_chunks, + GFP_KERNEL); if (!kfd->gtt_sa_bitmap) return -ENOMEM; @@ -1278,13 +1273,12 @@ static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size, mutex_init(>gtt_sa_lock); return 0; - } static void kfd_gtt_sa_fini(struct kfd_dev *kfd) { mutex_destroy(>gtt_sa_lock); - kfree(kfd->gtt_sa_bitmap); + bitmap_free(kfd->gtt_sa_bitmap); } static inline uint64_t kfd_gtt_sa_calc_gpu_addr(uint64_t start_addr,
[PATCH] video: of: display_timing: Remove a redundant zeroing of memory
of_parse_display_timing() already call memset(0) on its 2nd argument, so there is no need to clear it explicitly before calling this function. Use kmalloc() instead of kzalloc() to save a few cycles. Signed-off-by: Christophe JAILLET --- drivers/video/of_display_timing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c index f93b6abbe258..bebd371c6b93 100644 --- a/drivers/video/of_display_timing.c +++ b/drivers/video/of_display_timing.c @@ -199,7 +199,7 @@ struct display_timings *of_get_display_timings(const struct device_node *np) struct display_timing *dt; int r; - dt = kzalloc(sizeof(*dt), GFP_KERNEL); + dt = kmalloc(sizeof(*dt), GFP_KERNEL); if (!dt) { pr_err("%pOF: could not allocate display_timing struct\n", np); -- 2.32.0
[PATCH] tda9950: Slightly simplify tda9950_devm_glue_init()
Use devm_add_action_or_reset() instead of hand writing it. This is more straightforward and saves a few lines of code. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/i2c/tda9950.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c index 5b03fdd1eaa4..526774594510 100644 --- a/drivers/gpu/drm/i2c/tda9950.c +++ b/drivers/gpu/drm/i2c/tda9950.c @@ -361,9 +361,7 @@ static int tda9950_devm_glue_init(struct device *dev, struct tda9950_glue *glue) return ret; } - ret = devm_add_action(dev, tda9950_devm_glue_exit, glue); - if (ret) - tda9950_devm_glue_exit(glue); + ret = devm_add_action_or_reset(dev, tda9950_devm_glue_exit, glue); return ret; } -- 2.32.0
[PATCH] backlight: backlight: Slighly simplify devm_of_find_backlight()
Use devm_add_action_or_reset() instead of devm_add_action()+hand writing what is done in the release function, should an error occur. This is more straightforward and saves a few lines of code. While at it, remove a useless test in devm_backlight_release(). 'data' is known to be not NULL when this function is called. Signed-off-by: Christophe JAILLET --- drivers/video/backlight/backlight.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 4ae6fae94ac2..b788ff3d0f45 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -710,8 +710,7 @@ static void devm_backlight_release(void *data) { struct backlight_device *bd = data; - if (bd) - put_device(>dev); + put_device(>dev); } /** @@ -737,11 +736,10 @@ struct backlight_device *devm_of_find_backlight(struct device *dev) bd = of_find_backlight(dev); if (IS_ERR_OR_NULL(bd)) return bd; - ret = devm_add_action(dev, devm_backlight_release, bd); - if (ret) { - put_device(>dev); + ret = devm_add_action_or_reset(dev, devm_backlight_release, bd); + if (ret) return ERR_PTR(ret); - } + return bd; } EXPORT_SYMBOL(devm_of_find_backlight); -- 2.32.0
[PATCH] drm/nouveau: Fix a potential theorical leak in nouveau_get_backlight_name()
If successful ida_simple_get() calls are not undone when needed, some additional memory may be allocated and wasted. Here, an ID between 0 and MAX_INT is required. If this ID is >=100, it is not taken into account and is wasted. It should be released. Instead of calling ida_simple_remove(), take advantage of the 'max' parameter to require the ID not to be too big. Should it be too big, it is not allocated and don't need to be freed. While at it, use ida_alloc_xxx()/ida_free() instead to ida_simple_get()/ida_simple_remove(). The latter is deprecated and more verbose. Fixes: db1a0ae21461 ("drm/nouveau/bl: Assign different names to interfaces") Signed-off-by: Christophe JAILLET --- This patch is more a clean-up than a fix. It is unlikely than >= 100 backlight devices will be registered, and the over allocation would occur even much later when the underlying xarray is full. I also think that the 'if (bl->id >= 0)' before the ida_simple_remove() calls are useless. We don't store the id if a negative (i.e. error) is returned by ida_simple_get(). Finally, having a '#define BL_MAX_MINORS 99' could be better than a magic number in the code. --- drivers/gpu/drm/nouveau/nouveau_backlight.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c index ae2f2abc8f5a..ccd080ba30bf 100644 --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c @@ -46,8 +46,8 @@ static bool nouveau_get_backlight_name(char backlight_name[BL_NAME_SIZE], struct nouveau_backlight *bl) { - const int nb = ida_simple_get(_ida, 0, 0, GFP_KERNEL); - if (nb < 0 || nb >= 100) + const int nb = ida_alloc_max(_ida, 99, GFP_KERNEL); + if (nb < 0) return false; if (nb > 0) snprintf(backlight_name, BL_NAME_SIZE, "nv_backlight%d", nb); @@ -414,7 +414,7 @@ nouveau_backlight_init(struct drm_connector *connector) nv_encoder, ops, ); if (IS_ERR(bl->dev)) { if (bl->id >= 0) - ida_simple_remove(_ida, bl->id); + ida_free(_ida, bl->id); ret = PTR_ERR(bl->dev); goto fail_alloc; } @@ -442,7 +442,7 @@ nouveau_backlight_fini(struct drm_connector *connector) return; if (bl->id >= 0) - ida_simple_remove(_ida, bl->id); + ida_free(_ida, bl->id); backlight_device_unregister(bl->dev); nv_conn->backlight = NULL; -- 2.32.0
[PATCH] drm/radeon: Avoid open coded arithmetic in memory allocation
kmalloc_array()/kcalloc() should be used to avoid potential overflow when a multiplication is needed to compute the size of the requested memory. So turn a kzalloc()+explicit size computation into an equivalent kcalloc(). Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/radeon/radeon_atombios.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c index 28c4413f4dc8..7b9cc7a9f42f 100644 --- a/drivers/gpu/drm/radeon/radeon_atombios.c +++ b/drivers/gpu/drm/radeon/radeon_atombios.c @@ -897,13 +897,13 @@ bool radeon_get_atom_connector_info_from_supported_devices_table(struct union atom_supported_devices *supported_devices; int i, j, max_device; struct bios_connector *bios_connectors; - size_t bc_size = sizeof(*bios_connectors) * ATOM_MAX_SUPPORTED_DEVICE; struct radeon_router router; router.ddc_valid = false; router.cd_valid = false; - bios_connectors = kzalloc(bc_size, GFP_KERNEL); + bios_connectors = kcalloc(ATOM_MAX_SUPPORTED_DEVICE, + sizeof(*bios_connectors), GFP_KERNEL); if (!bios_connectors) return false; -- 2.32.0
[PATCH] backlight: pwm_bl: Avoid open coded arithmetic in memory allocation
kmalloc_array()/kcalloc() should be used to avoid potential overflow when a multiplication is needed to compute the size of the requested memory. So turn a kzalloc()+explicit size computation into an equivalent kcalloc(). Signed-off-by: Christophe JAILLET --- drivers/video/backlight/pwm_bl.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 8d8959a70e44..c0523a0269ee 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -263,9 +263,8 @@ static int pwm_backlight_parse_dt(struct device *dev, /* read brightness levels from DT property */ if (num_levels > 0) { - size_t size = sizeof(*data->levels) * num_levels; - - data->levels = devm_kzalloc(dev, size, GFP_KERNEL); + data->levels = devm_kcalloc(dev, num_levels, + sizeof(*data->levels), GFP_KERNEL); if (!data->levels) return -ENOMEM; @@ -320,8 +319,8 @@ static int pwm_backlight_parse_dt(struct device *dev, * Create a new table of brightness levels with all the * interpolated steps. */ - size = sizeof(*table) * num_levels; - table = devm_kzalloc(dev, size, GFP_KERNEL); + table = devm_kcalloc(dev, num_levels, sizeof(*table), +GFP_KERNEL); if (!table) return -ENOMEM; /* -- 2.32.0
[PATCH] drm/bridge: lt9611: Fix an error handling path in lt9611_probe()
If lt9611_audio_init() fails, some resources still need to be released before returning an error code. Add the missing goto the error handling path. Fixes: 23278bf54afe ("drm/bridge: Introduce LT9611 DSI to HDMI bridge") Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/bridge/lontium-lt9611.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c index dafb1b47c15f..00597eb54661 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c @@ -1164,7 +1164,11 @@ static int lt9611_probe(struct i2c_client *client, lt9611_enable_hpd_interrupts(lt9611); - return lt9611_audio_init(dev, lt9611); + ret = lt9611_audio_init(dev, lt9611); + if (ret) + goto err_remove_bridge; + + return 0; err_remove_bridge: drm_bridge_remove(>bridge); -- 2.32.0