[PATCH v2] dma-buf/fence-array: Add flex array to struct dma_fence_array

2024-05-25 Thread Christophe JAILLET
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()

2024-05-20 Thread Christophe JAILLET
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()

2024-05-20 Thread Christophe JAILLET
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

2024-05-18 Thread Christophe JAILLET
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

2024-05-18 Thread Christophe JAILLET

(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

2024-04-16 Thread Christophe JAILLET

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

2024-04-15 Thread Christophe JAILLET

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

2024-04-14 Thread Christophe JAILLET
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

2024-04-14 Thread Christophe JAILLET

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

2024-04-14 Thread Christophe JAILLET

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"

2024-04-11 Thread Christophe JAILLET

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

2024-03-24 Thread Christophe JAILLET

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

2024-03-23 Thread Christophe JAILLET

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()

2024-03-16 Thread Christophe JAILLET
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()

2024-02-23 Thread Christophe JAILLET
'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

2024-02-20 Thread Christophe JAILLET

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

2024-02-19 Thread Christophe JAILLET

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

2024-02-18 Thread Christophe JAILLET
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

2024-02-03 Thread Christophe JAILLET

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

2024-01-19 Thread Christophe JAILLET

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

2024-01-14 Thread Christophe JAILLET
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

2024-01-14 Thread Christophe JAILLET
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

2024-01-14 Thread Christophe JAILLET
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()

2024-01-13 Thread Christophe JAILLET
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()

2024-01-06 Thread Christophe JAILLET
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

2024-01-04 Thread Christophe JAILLET

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

2024-01-04 Thread Christophe JAILLET

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

2023-12-08 Thread Christophe JAILLET

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()

2023-10-21 Thread Christophe JAILLET
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()

2023-09-22 Thread Christophe JAILLET
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

2023-09-18 Thread Christophe JAILLET

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

2023-09-18 Thread Christophe JAILLET
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"

2023-09-10 Thread Christophe JAILLET

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()

2023-09-04 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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

2023-09-02 Thread Christophe JAILLET
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()

2023-09-02 Thread Christophe JAILLET
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

2023-08-21 Thread Christophe JAILLET

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

2023-08-21 Thread Christophe JAILLET

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

2023-08-20 Thread Christophe JAILLET

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

2023-08-20 Thread Christophe JAILLET
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

2023-08-20 Thread Christophe JAILLET
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()

2023-08-20 Thread Christophe JAILLET
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'

2023-08-20 Thread Christophe JAILLET
'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()

2023-08-20 Thread Christophe JAILLET
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()

2023-07-27 Thread Christophe JAILLET

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()

2023-07-23 Thread Christophe JAILLET
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()

2023-07-17 Thread Christophe JAILLET
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

2023-07-08 Thread Christophe JAILLET
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'

2023-06-18 Thread Christophe JAILLET
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

2023-06-04 Thread Christophe JAILLET

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()

2023-06-04 Thread Christophe JAILLET
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()

2023-05-29 Thread Christophe JAILLET
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

2023-05-29 Thread Christophe JAILLET
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()

2023-05-29 Thread Christophe JAILLET
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()

2023-05-29 Thread Christophe JAILLET
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()

2023-05-18 Thread Marion &amp; Christophe JAILLET



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

2023-05-02 Thread Christophe JAILLET

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()

2023-04-25 Thread Christophe JAILLET
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()

2023-04-18 Thread Christophe JAILLET

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()

2023-04-17 Thread Christophe JAILLET
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()

2023-04-17 Thread Christophe JAILLET
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()

2023-04-13 Thread Christophe JAILLET
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

2023-04-11 Thread Christophe JAILLET

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()

2023-03-21 Thread Christophe JAILLET
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

2023-02-21 Thread Christophe JAILLET

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

2023-01-29 Thread Christophe JAILLET
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

2023-01-22 Thread Christophe JAILLET

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()

2023-01-14 Thread Christophe JAILLET
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

2022-12-30 Thread Christophe JAILLET
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

2022-12-30 Thread Christophe JAILLET
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

2022-12-15 Thread Marion &amp; Christophe JAILLET




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

2022-12-15 Thread Christophe JAILLET

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()

2022-12-12 Thread Christophe JAILLET
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()

2022-12-10 Thread Christophe JAILLET
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()

2022-12-10 Thread Christophe JAILLET
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()

2022-12-10 Thread Christophe JAILLET
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()

2022-11-01 Thread Christophe JAILLET
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'

2022-10-08 Thread Christophe JAILLET

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/

2022-09-25 Thread Christophe JAILLET
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()

2022-08-15 Thread Christophe JAILLET

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

2022-07-08 Thread Christophe JAILLET
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

2022-07-04 Thread Christophe JAILLET
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()

2022-06-18 Thread Christophe JAILLET
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()

2022-06-18 Thread Christophe JAILLET
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

2022-06-16 Thread Christophe JAILLET

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

2022-06-01 Thread Christophe JAILLET

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

2022-04-26 Thread Christophe JAILLET

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

2022-03-22 Thread Christophe JAILLET
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()

2022-02-12 Thread Christophe JAILLET
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()

2022-02-12 Thread Christophe JAILLET
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()

2022-02-08 Thread Christophe JAILLET
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

2022-02-05 Thread Christophe JAILLET
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

2022-02-04 Thread Christophe JAILLET
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()

2022-01-29 Thread Christophe JAILLET
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



  1   2   3   >