[PATCH] drm/ipuv3/parallel: Fix an error handling path in imx_pd_probe()
If component_add() fails, we need to undo a potential previous
drm_edid_alloc() call.
Add an error handling path and the missing drm_edid_free(), as already done
in the reomve function.
Fixes: 42e08287a318 ("drm/ipuv3/parallel: convert to struct drm_edid")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/imx/ipuv3/parallel-display.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/imx/ipuv3/parallel-display.c
b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
index 91d7808a2d8d..6d51203f7f0f 100644
--- a/drivers/gpu/drm/imx/ipuv3/parallel-display.c
+++ b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
@@ -350,7 +350,15 @@ static int imx_pd_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, imxpd);
- return component_add(dev, &imx_pd_ops);
+ ret = component_add(dev, &imx_pd_ops);
+ if (ret)
+ goto free_edid;
+
+ return 0;
+
+free_edid:
+ drm_edid_free(imxpd->drm_edid);
+ return ret;
}
static void imx_pd_remove(struct platform_device *pdev)
--
2.46.0
Re: [PATCH -next] drm/imagination: Use memdup_user() helper
Le 31/08/2024 à 12:30, Jinjie Ruan a écrit :
Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
and it can simplfy code.
Signed-off-by: Jinjie Ruan
---
drivers/gpu/drm/imagination/pvr_context.c | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_context.c
b/drivers/gpu/drm/imagination/pvr_context.c
index eded5e955cc0..e75fd50a4d9f 100644
--- a/drivers/gpu/drm/imagination/pvr_context.c
+++ b/drivers/gpu/drm/imagination/pvr_context.c
@@ -69,27 +69,19 @@ process_static_context_state(struct pvr_device *pvr_dev,
const struct pvr_stream
void *stream;
int err;
- stream = kzalloc(stream_size, GFP_KERNEL);
- if (!stream)
- return -ENOMEM;
-
- if (copy_from_user(stream, u64_to_user_ptr(stream_user_ptr),
stream_size)) {
- err = -EFAULT;
- goto err_free;
- }
+ stream = memdup_user(u64_to_user_ptr(stream_user_ptr), stream_size);
+ if (IS_ERR(stream))
+ return PTR_ERR(stream);
err = pvr_stream_process(pvr_dev, cmd_defs, stream, stream_size, dest);
- if (err)
- goto err_free;
+ if (err) {
+ kfree(stream);
+ return err;
+ }
kfree(stream);
return 0;
-
-err_free:
- kfree(stream);
-
- return err;
}
It could also be:
err = pvr_stream_process(...);
kfree(stream);
return err;
as you did for drivers/gpu/drm/imagination/pvr_job.c.
CJ
static int init_render_fw_objs(struct pvr_context *ctx,
Re: [PATCH -next 2/5] drm/mediatek: Fix missing of_node_put() for mtk_drm_get_all_drm_priv()
Le 23/08/2024 à 12:46, Christophe JAILLET a écrit :
@@ -933,10 +931,8 @@ static int mtk_drm_probe(struct platform_device
*pdev)
}
ret = mtk_ddp_comp_init(node, &private->ddp_comp[comp_id],
comp_id);
- if (ret) {
- of_node_put(node);
+ if (ret)
goto err_node;
Hi,
I've seen on another thread that is was not sure that scoped versions
and gotos played well together.
It was asked to check more in details and confirm that it was safe
before applying the patch.
I've not followed the discussion, so I just point it out, in case it helps.
I'll try to give it a look in the coming days.
CJ
Hi,
looking at the generated asm file (gcc 14.2.1), everything looks fine.
# drivers/gpu/drm/mediatek/mtk_drm_drv.c:933: ret =
mtk_ddp_comp_init(node, &private->ddp_comp[comp_id], comp_id);
salq$5, %rax#, _36
movl%r14d, %edx # comp_id,
movq%rbx, %rdi # node,
leaq552(%rbp,%rax), %rsi#, _28
callmtk_ddp_comp_init #
movl%eax, %r12d # tmp205,
# drivers/gpu/drm/mediatek/mtk_drm_drv.c:934: if (ret)
testl %eax, %eax #
jne .L212 #,
...
.L212:
# ./include/linux/of.h:138: DEFINE_FREE(device_node, struct device_node
*, if (_T) of_node_put(_T))
movq%rbx, %rdi # node,
callof_node_put #
jmp .L171 #
CJ
Re: [PATCH -next 2/5] drm/mediatek: Fix missing of_node_put() for mtk_drm_get_all_drm_priv()
Le 23/08/2024 à 11:20, Jinjie Ruan a écrit :
In mtk_drm_get_all_drm_priv(), break in for_each_child_of_node() should
call of_node_put() to avoid child node resource leak, use
for_each_child_of_node_scoped() to fix it.
And avoid the need for manual cleanup of_node_put() in early exits
from the loop for another one.
Fixes: d761b9450e31 ("drm/mediatek: Add cnt checking for coverity issue")
Signed-off-by: Jinjie Ruan
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 77b50c56c124..41aff0183cbd 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -371,12 +371,11 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
struct mtk_drm_private *temp_drm_priv;
struct device_node *phandle = dev->parent->of_node;
const struct of_device_id *of_id;
- struct device_node *node;
struct device *drm_dev;
unsigned int cnt = 0;
int i, j;
- for_each_child_of_node(phandle->parent, node) {
+ for_each_child_of_node_scoped(phandle->parent, node) {
struct platform_device *pdev;
of_id = of_match_node(mtk_drm_of_ids, node);
@@ -828,7 +827,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
struct device_node *phandle = dev->parent->of_node;
const struct of_device_id *of_id;
struct mtk_drm_private *private;
- struct device_node *node;
struct component_match *match = NULL;
struct platform_device *ovl_adaptor;
int ret;
@@ -869,7 +867,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
}
/* Iterate over sibling DISP function blocks */
- for_each_child_of_node(phandle->parent, node) {
+ for_each_child_of_node_scoped(phandle->parent, node) {
const struct of_device_id *of_id;
enum mtk_ddp_comp_type comp_type;
int comp_id;
@@ -933,10 +931,8 @@ static int mtk_drm_probe(struct platform_device *pdev)
}
ret = mtk_ddp_comp_init(node, &private->ddp_comp[comp_id], comp_id);
- if (ret) {
- of_node_put(node);
+ if (ret)
goto err_node;
Hi,
I've seen on another thread that is was not sure that scoped versions
and gotos played well together.
It was asked to check more in details and confirm that it was safe
before applying the patch.
I've not followed the discussion, so I just point it out, in case it helps.
I'll try to give it a look in the coming days.
CJ
- }
}
if (!private->mutex_node) {
Re: [RESEND PATCH v4 2/2] drm/tiny: Add driver for Sharp Memory LCD
Le 19/08/2024 à 23:49, Alex Lanzano a écrit :
Add support for the monochrome Sharp Memory LCDs.
Hi,
a few nitpick below, should thre be a v5.
...
+struct sharp_memory_device {
+ struct drm_device drm;
+ struct spi_device *spi;
+
+ const struct drm_display_mode *mode;
+
+ struct drm_crtc crtc;
+ struct drm_plane plane;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+
+ struct gpio_desc *enable_gpio;
+
+ struct task_struct *sw_vcom_signal;
+ struct pwm_device *pwm_vcom_signal;
+
+ enum sharp_memory_vcom_mode vcom_mode;
+ u8 vcom;
+
+ u32 pitch;
+ u32 tx_buffer_size;
+ u8 *tx_buffer;
+
+ /* When vcom_mode == "software" a kthread is used to
+* periodically send a 'maintain display' message over
+* spi. This mutex ensures tx_buffer access and spi bus
+* usage is synchronized in this case
This comment could take only 3 lines and still be with < 80 lines.
A dot could also be added at the end of the 2nd sentence.
+*/
+ struct mutex tx_mutex;
+};
...
+static int sharp_memory_probe(struct spi_device *spi)
+{
+ int ret;
+ struct device *dev;
+ struct sharp_memory_device *smd;
+ struct drm_device *drm;
+ const char *vcom_mode_str;
+
+ ret = spi_setup(spi);
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret, "Failed to setup spi
device\n");
+
+ dev = &spi->dev;
If done earlier (when dev is declared?), it could already be used in the
dev_err_probe() just above?
+ if (!dev->coherent_dma_mask) {
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to set dma
mask\n");
+ }
+
+ smd = devm_drm_dev_alloc(dev, &sharp_memory_drm_driver,
+struct sharp_memory_device, drm);
+ if (!smd)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, smd);
+
+ smd->spi = spi;
+ drm = &smd->drm;
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to initialize drm
config\n");
+
+ smd->enable_gpio = devm_gpiod_get_optional(dev, "enable",
GPIOD_OUT_HIGH);
+ if (!smd->enable_gpio)
+ dev_warn(dev, "Enable gpio not defined\n");
+
+ /*
+* VCOM is a signal that prevents DC bias from being built up in
+* the panel resulting in pixels being forever stuck in one state.
+*
+* This driver supports three different methods to generate this
+* signal depending on EXTMODE pin:
+*
+* software (EXTMODE = L) - This mode uses a kthread to
+* periodically send a "maintain display" message to the display,
+* toggling the vcom bit on and off with each message
+*
+* external (EXTMODE = H) - This mode relies on an external
+* clock to generate the signal on the EXTCOMM pin
+*
+* pwm (EXTMODE = H) - This mode uses a pwm device to generate
+* the signal on the EXTCOMM pin
+*
+*/
+ if (device_property_read_string(&spi->dev, "sharp,vcom-mode",
&vcom_mode_str))
just dev?
+ return dev_err_probe(dev, -EINVAL,
+"Unable to find sharp,vcom-mode node in device
tree\n");
+
+ if (!strcmp("software", vcom_mode_str)) {
+ smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
+
+ } else if (!strcmp("external", vcom_mode_str)) {
+ smd->vcom_mode = SHARP_MEMORY_EXTERNAL_VCOM;
+
+ } else if (!strcmp("pwm", vcom_mode_str)) {
+ smd->vcom_mode = SHARP_MEMORY_PWM_VCOM;
+ ret = sharp_memory_init_pwm_vcom_signal(smd);
+ if (ret)
+ return dev_err_probe(dev, ret,
+"Failed to initialize external COM
signal\n");
+ } else {
+ return dev_err_probe(dev, -EINVAL, "Invalid value set for
vcom-mode\n");
+ }
+
+ drm->mode_config.funcs = &sharp_memory_mode_config_funcs;
+ smd->mode = spi_get_device_match_data(spi);
+
+ smd->pitch = (SHARP_ADDR_PERIOD + smd->mode->hdisplay +
SHARP_DUMMY_PERIOD) / 8;
+ smd->tx_buffer_size = (SHARP_MODE_PERIOD +
+ (SHARP_ADDR_PERIOD + (smd->mode->hdisplay) +
SHARP_DUMMY_PERIOD) *
+ smd->mode->vdisplay) / 8;
+
+ smd->tx_buffer = devm_kzalloc(&spi->dev, smd->tx_buffer_size,
GFP_KERNEL);
Just dev?
+ if (!smd->tx_buffer)
+ return -ENOMEM;
+
+ mutex_init(&smd->tx_mutex);
+
+ drm->mode_config.min_width = smd->mode->hdisplay;
+ drm->mode_config.max_width = smd->mode->hdisplay;
+ drm->mode_config.min_height = smd->mode->vdisplay;
+ drm->mode_config.max_height = smd-
Re: [PATCH] fbdev: omapfb: panel-sony-acx565akm: Simplify show_cabc_available_modes()
Le 09/08/2024 à 16:42, Dan Carpenter a écrit :
On Thu, Aug 08, 2024 at 11:46:11AM +0200, Christophe JAILLET wrote:
Use sysfs_emit_at() instead of snprintf() + custom logic.
Using sysfs_emit_at() is much more simple.
Also, sysfs_emit() is already used in this function, so using
sysfs_emit_at() is more consistent.
Also simplify the logic:
- always add a space after an entry
- change the last space into a '\n'
Finally it is easy to see that, given the size of cabc_modes, PAGE_SIZE
can not be reached.
So better keep everything simple (and correct).
Signed-off-by: Christophe JAILLET
---
.../omap2/omapfb/displays/panel-sony-acx565akm.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
index 71d2e015960c..fc975615d5c9 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
@@ -466,19 +466,20 @@ static ssize_t show_cabc_available_modes(struct device
*dev,
char *buf)
{
struct panel_drv_data *ddata = dev_get_drvdata(dev);
- int len;
+ int len = 0;
int i;
if (!ddata->has_cabc)
return sysfs_emit(buf, "%s\n", cabc_modes[0]);
- for (i = 0, len = 0;
-len < PAGE_SIZE && i < ARRAY_SIZE(cabc_modes); i++)
- len += snprintf(&buf[len], PAGE_SIZE - len, "%s%s%s",
- i ? " " : "", cabc_modes[i],
- i == ARRAY_SIZE(cabc_modes) - 1 ? "\n" : "");
+ for (i = 0; i < ARRAY_SIZE(cabc_modes); i++)
+ len += sysfs_emit_at(buf, len, "%s ", cabc_modes[i]);
+
+ /* Remove the trailing space */
+ if (len)
+ buf[len - 1] = '\n';
I'm uncomfortable with this line. It assumes we don't overflow PAGE_SIZE where
the original code was careful about checking. Probably easiest to do what the
original code did and say:
Hi Dan,
I don't follow you. AFAIK, it does not assume anything.
Thanks to sysfs_emit_at(), len can only be in [0..PAGE_SIZE-1] because
the trailing \0 is not counted.
So, as len != 0, len-1 is in [0..PAGE_SIZE-2].
How do you think an overflow could happen?
Also, all this code does is buildind:
"off, ui, still-image, moving-image\n"
So in any case, an overflow is impossible, and really unlikely in the
future as well.
From my PoV, my proposed patch is both much more readable and still
correct in all cases.
CJ
for (i = 0; i < ARRAY_SIZE(cabc_modes); i++)
len += sysfs_emit_at(buf, len, "%s%s", cabc_modes[i],
i == ARRAY_SIZE(cabc_modes) - 1 ? "\n" :
"");
regards,
dan carpenter
[PATCH] fbdev: omapfb: Use sysfs_emit_at() to simplify code
This file already uses sysfs_emit(). So be consistent and also use
sysfs_emit_at().
Moreover, size is always < PAGE_SIZE because scnprintf() (and now
sysfs_emit_at()) returns the number of characters written not including the
trailing '\0'. So some tests can be removed.
This slightly simplifies the code and makes it more readable.
Signed-off-by: Christophe JAILLET
---
Compile tested only.
2 spaces are added before color_caps[i].name and color_caps[i].name, but
not ctrl_caps[i].name.
I wonder if it is done on purpose or if it could be removed as well.
---
drivers/video/fbdev/omap/omapfb_main.c | 36 ++
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/video/fbdev/omap/omapfb_main.c
b/drivers/video/fbdev/omap/omapfb_main.c
index aa31c0d26e92..e12c6019a4d6 100644
--- a/drivers/video/fbdev/omap/omapfb_main.c
+++ b/drivers/video/fbdev/omap/omapfb_main.c
@@ -1241,14 +1241,13 @@ static ssize_t omapfb_show_caps_num(struct device *dev,
{
struct omapfb_device *fbdev = dev_get_drvdata(dev);
int plane;
- size_t size;
+ size_t size = 0;
struct omapfb_caps caps;
plane = 0;
- size = 0;
- while (size < PAGE_SIZE && plane < OMAPFB_PLANE_NUM) {
+ while (plane < OMAPFB_PLANE_NUM) {
omapfb_get_caps(fbdev, plane, &caps);
- size += scnprintf(&buf[size], PAGE_SIZE - size,
+ size += sysfs_emit_at(buf, size,
"plane#%d %#010x %#010x %#010x\n",
plane, caps.ctrl, caps.plane_color, caps.wnd_color);
plane++;
@@ -1263,34 +1262,27 @@ static ssize_t omapfb_show_caps_text(struct device *dev,
int i;
struct omapfb_caps caps;
int plane;
- size_t size;
+ size_t size = 0;
plane = 0;
- size = 0;
- while (size < PAGE_SIZE && plane < OMAPFB_PLANE_NUM) {
+ while (plane < OMAPFB_PLANE_NUM) {
omapfb_get_caps(fbdev, plane, &caps);
- size += scnprintf(&buf[size], PAGE_SIZE - size,
-"plane#%d:\n", plane);
- for (i = 0; i < ARRAY_SIZE(ctrl_caps) &&
-size < PAGE_SIZE; i++) {
+ size += sysfs_emit_at(buf, size, "plane#%d:\n", plane);
+ for (i = 0; i < ARRAY_SIZE(ctrl_caps); i++) {
if (ctrl_caps[i].flag & caps.ctrl)
- size += scnprintf(&buf[size], PAGE_SIZE - size,
+ size += sysfs_emit_at(buf, size,
" %s\n", ctrl_caps[i].name);
}
- size += scnprintf(&buf[size], PAGE_SIZE - size,
-" plane colors:\n");
- for (i = 0; i < ARRAY_SIZE(color_caps) &&
-size < PAGE_SIZE; i++) {
+ size += sysfs_emit_at(buf, size, " plane colors:\n");
+ for (i = 0; i < ARRAY_SIZE(color_caps); i++) {
if (color_caps[i].flag & caps.plane_color)
- size += scnprintf(&buf[size], PAGE_SIZE - size,
+ size += sysfs_emit_at(buf, size,
" %s\n", color_caps[i].name);
}
- size += scnprintf(&buf[size], PAGE_SIZE - size,
-" window colors:\n");
- for (i = 0; i < ARRAY_SIZE(color_caps) &&
-size < PAGE_SIZE; i++) {
+ size += sysfs_emit_at(buf, size, " window colors:\n");
+ for (i = 0; i < ARRAY_SIZE(color_caps); i++) {
if (color_caps[i].flag & caps.wnd_color)
- size += scnprintf(&buf[size], PAGE_SIZE - size,
+ size += sysfs_emit_at(buf, size,
" %s\n", color_caps[i].name);
}
--
2.46.0
[PATCH] fbdev: omapfb: panel-sony-acx565akm: Simplify show_cabc_available_modes()
Use sysfs_emit_at() instead of snprintf() + custom logic.
Using sysfs_emit_at() is much more simple.
Also, sysfs_emit() is already used in this function, so using
sysfs_emit_at() is more consistent.
Also simplify the logic:
- always add a space after an entry
- change the last space into a '\n'
Finally it is easy to see that, given the size of cabc_modes, PAGE_SIZE
can not be reached.
So better keep everything simple (and correct).
Signed-off-by: Christophe JAILLET
---
.../omap2/omapfb/displays/panel-sony-acx565akm.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
index 71d2e015960c..fc975615d5c9 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c
@@ -466,19 +466,20 @@ static ssize_t show_cabc_available_modes(struct device
*dev,
char *buf)
{
struct panel_drv_data *ddata = dev_get_drvdata(dev);
- int len;
+ int len = 0;
int i;
if (!ddata->has_cabc)
return sysfs_emit(buf, "%s\n", cabc_modes[0]);
- for (i = 0, len = 0;
-len < PAGE_SIZE && i < ARRAY_SIZE(cabc_modes); i++)
- len += snprintf(&buf[len], PAGE_SIZE - len, "%s%s%s",
- i ? " " : "", cabc_modes[i],
- i == ARRAY_SIZE(cabc_modes) - 1 ? "\n" : "");
+ for (i = 0; i < ARRAY_SIZE(cabc_modes); i++)
+ len += sysfs_emit_at(buf, len, "%s ", cabc_modes[i]);
+
+ /* Remove the trailing space */
+ if (len)
+ buf[len - 1] = '\n';
- return len < PAGE_SIZE ? len : PAGE_SIZE - 1;
+ return len;
}
static DEVICE_ATTR(cabc_mode, S_IRUGO | S_IWUSR,
--
2.46.0
[PATCH 2/2] drm/dp_mst: Slightly optimize drm_dp_mst_i2c_write() (2/2)
'msg' is only used with drm_dp_encode_sideband_req() which takes a
"const struct drm_dp_sideband_msg_req_body *".
So some initializations can be done only once outside of the for loop.
Signed-off-by: Christophe JAILLET
---
In case of interest, on x86_64, with allmodconfig, sizeof(*msg) is 420
bytes.
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 7bf6157eb3a3..a149ff3f70ad 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5891,10 +5891,16 @@ static int drm_dp_mst_i2c_write(struct
drm_dp_mst_branch *mstb,
ret = -ENOMEM;
goto out;
}
+
+ /*
+* 'msg' is not modified by drm_dp_encode_sideband_req(). So
+* some initializations can be done only once.
+*/
+ memset(&msg, 0, sizeof(msg));
+ msg.req_type = DP_REMOTE_I2C_WRITE;
+ msg.u.i2c_write.port_number = port->port_num;
+
for (i = 0; i < num; i++) {
- memset(&msg, 0, sizeof(msg));
- msg.req_type = DP_REMOTE_I2C_WRITE;
- msg.u.i2c_write.port_number = port->port_num;
msg.u.i2c_write.write_i2c_device_id = msgs[i].addr;
msg.u.i2c_write.num_bytes = msgs[i].len;
msg.u.i2c_write.bytes = msgs[i].buf;
--
2.45.2
[PATCH 1/2] drm/dp_mst: Slightly optimize drm_dp_mst_i2c_write() (1/2)
'txmsg' is memset()'ed in the for loop below, before usage. So we can save
another initialization txmsg when it is allocated.
Signed-off-by: Christophe JAILLET
---
In case of interest, on x86_64, with allmodconfig, sizeof(*txmsg) is 768
bytes.
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 379a449a28a2..7bf6157eb3a3 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5886,7 +5886,7 @@ static int drm_dp_mst_i2c_write(struct drm_dp_mst_branch
*mstb,
struct drm_dp_sideband_msg_tx *txmsg = NULL;
int ret;
- txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
+ txmsg = kmalloc(sizeof(*txmsg), GFP_KERNEL);
if (!txmsg) {
ret = -ENOMEM;
goto out;
--
2.45.2
[PATCH v2] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Christophe JAILLET
---
*Not* even compile tested only.
It is provided as-is
Changes in v2:
- Apply a minimal change [Helge Deller]
v1:
https://lore.kernel.org/all/dc4fe3d857849ac63131c5620f1bacf1a3d7172e.1722191367.git.christophe.jail...@wanadoo.fr/
---
drivers/video/fbdev/hpfb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
index 66fac8e5393e..a1144b150982 100644
--- a/drivers/video/fbdev/hpfb.c
+++ b/drivers/video/fbdev/hpfb.c
@@ -345,6 +345,7 @@ static int hpfb_dio_probe(struct dio_dev *d, const struct
dio_device_id *ent)
if (hpfb_init_one(paddr, vaddr)) {
if (d->scode >= DIOII_SCBASE)
iounmap((void *)vaddr);
+ release_mem_region(d->resource.start,
resource_size(&d->resource));
return -ENOMEM;
}
return 0;
--
2.45.2
Re: [PATCH v3] udmabuf: use kmem_cache to alloc udmabuf folio
Le 31/07/2024 à 09:37, Huan Yang a écrit :
The current udmabuf_folio contains a list_head and the corresponding
folio pointer, with a size of 24 bytes. udmabuf_folio uses kmalloc to
allocate memory.
However, kmalloc is a public pool, starting from 8,16,32 bytes.
Additionally, if the size is not aligned with the kmalloc size, it will
be rounded up to the corresponding size.
This means that each udmabuf_folio allocation will get 32 bytes, and
waste 8 bytes.
Considering that each udmabuf creates a folio corresponding to a
udmabuf_folio, the wasted memory can be significant in the case of
memory fragmentation.
Furthermore, if udmabuf is frequently used, the allocation and
deallocation of udmabuf_folio will also be frequent.
Therefore, this patch adds a kmem_cache dedicated to the allocation and
deallocation of udmabuf_folio.This is expected to improve the
performance of allocation and deallocation within the expected range,
while also avoiding memory waste.
Signed-off-by: Huan Yang
---
v3 -> v2: fix error description.
v2 -> v1: fix double unregister, remove unlikely.
drivers/dma-buf/udmabuf.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 047c3cd2ceff..c112c58ef09a 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -24,6 +24,8 @@ static int size_limit_mb = 64;
module_param(size_limit_mb, int, 0644);
MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is
64.");
+static struct kmem_cache *udmabuf_folio_cachep;
+
struct udmabuf {
pgoff_t pagecount;
struct folio **folios;
@@ -169,7 +171,7 @@ static void unpin_all_folios(struct list_head *unpin_list)
unpin_folio(ubuf_folio->folio);
list_del(&ubuf_folio->list);
- kfree(ubuf_folio);
+ kmem_cache_free(udmabuf_folio_cachep, ubuf_folio);
}
}
@@ -178,7 +180,7 @@ static int add_to_unpin_list(struct list_head *unpin_list,
{
struct udmabuf_folio *ubuf_folio;
- ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL);
+ ubuf_folio = kmem_cache_alloc(udmabuf_folio_cachep, GFP_KERNEL);
if (!ubuf_folio)
return -ENOMEM;
@@ -491,11 +493,20 @@ static int __init udmabuf_dev_init(void)
DMA_BIT_MASK(64));
if (ret < 0) {
pr_err("Could not setup DMA mask for udmabuf device\n");
- misc_deregister(&udmabuf_misc);
- return ret;
+ goto err;
+ }
+
+ udmabuf_folio_cachep = KMEM_CACHE(udmabuf_folio, 0);
+ if (!udmabuf_folio_cachep) {
+ ret = -ENOMEM;
+ goto err;
}
return 0;
+
+err:
+ misc_deregister(&udmabuf_misc);
+ return ret;
}
static void __exit udmabuf_dev_exit(void)
Hi,
should a kmem_cache_destroy() be also added in udmabuf_dev_exit()?
CJ
base-commit: cd19ac2f903276b820f5d0d89de0c896c27036ed
Re: [PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
Le 29/07/2024 à 22:09, Helge Deller a écrit :
On 7/29/24 17:59, Dan Carpenter wrote:
On Mon, Jul 29, 2024 at 10:13:17AM +0200, Helge Deller wrote:
On 7/28/24 20:29, Christophe JAILLET wrote:
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
True.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I think we can drop this "Fixes" tag, as it gives no real info.
If we're backporting patches then these tags really are useful. As
I've been doing more and more backporting, I've come to believe this
more firmly.
Sure, "Fixes" tags are useful, but only if they really refer
to another patch which introduced the specific issue.
But the tag 1da177e4c3f4 ("Linux-2.6.12-rc2") isn't useful, as it's
just the initial git commit. It has no relation to why release_mem_region()
might have been initially missed. See:
I agree that the description of this specific tag is not useful by itself.
But at least it means: should it be backported, it can be done up to
there. (and sometimes LWN gives some statistics on how long it took to
fix an "issue", should it be considered as such)
Without it, it is not easy to guess in which branch the patch is meaningful.
I'll sent a v2 with your suggested minimal change, but I'll keep the
Fixes tag.
Up to you to remove it or not, and to add a or a
or none of them.
Any solution is fine with me.
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 (tag: v2.6.12-rc2)
Author: Linus Torvalds
Date: Sat Apr 16 15:20:36 2005 -0700
Linux-2.6.12-rc2
Initial git repository build. I'm not bothering with the full history,
even though we have it. We can create a separate "historical" git
archive of that later if we want to, and in the meantime it's about
3.2GB when imported into git - space that would just make the early
git days unnecessarily complicated, when we don't have a lot of good
infrastructure for it.
Helge
On:
> HP300 are old HP machines with an m68k CPU.
> Not sure if someone still has such a machine 🙂
so it really was the one I found on wikipedia, LoL!
So, another way to "fix" the issue is maybe to deprecate the driver or
everything related to this old architecture?
No strong opinion about it.
CJ
[PATCH] fbdev/hpfb: Fix an error handling path in hpfb_dio_probe()
If an error occurs after request_mem_region(), a corresponding
release_mem_region() should be called, as already done in the remove
function.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Christophe JAILLET
---
*Not* even compile tested only.
I don't know on what architecture it relies on.
So it is provided as-is
---
drivers/video/fbdev/hpfb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/hpfb.c b/drivers/video/fbdev/hpfb.c
index 66fac8e5393e..87b8dcdc1cf3 100644
--- a/drivers/video/fbdev/hpfb.c
+++ b/drivers/video/fbdev/hpfb.c
@@ -342,12 +342,17 @@ static int hpfb_dio_probe(struct dio_dev *d, const struct
dio_device_id *ent)
}
printk(KERN_INFO "Topcat found at DIO select code %d "
"(secondary id %02x)\n", d->scode, (d->id >> 8) & 0xff);
- if (hpfb_init_one(paddr, vaddr)) {
- if (d->scode >= DIOII_SCBASE)
- iounmap((void *)vaddr);
- return -ENOMEM;
- }
+ if (hpfb_init_one(paddr, vaddr))
+ goto err_unmap;
+
return 0;
+
+err_unmap:
+ if (d->scode >= DIOII_SCBASE)
+ iounmap((void *)vaddr);
+ release_mem_region(d->resource.start, resource_size(&d->resource));
+
+ return -ENOMEM;
}
static void hpfb_remove_one(struct dio_dev *d)
--
2.45.2
Re: [PATCH v2 2/2] drm/tiny: Add driver for Sharp Memory LCD
Le 26/07/2024 à 21:44, Alex Lanzano a écrit :
Add support for the monochrome Sharp Memory LCDs.
Signed-off-by: Alex Lanzano
Co-developed-by: Mehdi Djait
Signed-off-by: Mehdi Djait
---
MAINTAINERS | 7 +
drivers/gpu/drm/tiny/Kconfig| 20 +
drivers/gpu/drm/tiny/Makefile | 1 +
drivers/gpu/drm/tiny/sharp-memory.c | 726
4 files changed, 754 insertions(+)
create mode 100644 drivers/gpu/drm/tiny/sharp-memory.c
Hi,
below some other tiny comments, hoping it helps.
Also "./scripts/checkpatch.pl --strict" gives some hints, should some be
of interest.
+static int sharp_memory_probe(struct spi_device *spi)
+{
+ int ret;
+ struct device *dev;
+ struct sharp_memory_device *smd;
+ enum sharp_memory_model model;
+ struct drm_device *drm;
+ const char *vcom_mode_str;
+
+ ret = spi_setup(spi);
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret, "Failed to setup spi
device\n");
+
+ dev = &spi->dev;
6 times below, &spi->dev could be replaced by dev, to slightly simplify
things.
+ if (!dev->coherent_dma_mask) {
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to set dma
mask\n");
+ }
+
+ smd = devm_drm_dev_alloc(&spi->dev, &sharp_memory_drm_driver,
+struct sharp_memory_device, drm);
Missing error handling.
+
+ spi_set_drvdata(spi, smd);
+
+ smd->spi = spi;
+ drm = &smd->drm;
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to initialize drm
config\n");
+
+ smd->enable_gpio = devm_gpiod_get_optional(dev, "enable",
GPIOD_OUT_HIGH);
+ if (smd->enable_gpio == NULL)
Nitpick: if (!smd->enable_gpio)
+ dev_warn(&spi->dev, "Enable gpio not defined\n");
+
+ /*
+* VCOM is a signal that prevents DC bias from being built up in
+* the panel resulting in pixels being forever stuck in one state.
+*
+* This driver supports three different methods to generate this
+* signal depending on EXTMODE pin:
+*
+* software (EXTMODE = L) - This mode uses a kthread to
+* periodically send a "maintain display" message to the display,
+* toggling the vcom bit on and off with each message
+*
+* external (EXTMODE = H) - This mode relies on an external
+* clock to generate the signal on the EXTCOMM pin
+*
+* pwm (EXTMODE = H) - This mode uses a pwm device to generate
+* the signal on the EXTCOMM pin
+*
+*/
+ smd->vcom = 0;
Nitpick: devm_drm_dev_alloc() already zeroes the allocated memory. So
this is useless. Unless you think it gives some useful hint, it could be
removed.
+ if (device_property_read_string(&spi->dev, "vcom-mode", &vcom_mode_str))
+ return dev_err_probe(dev, -EINVAL,
+"Unable to find vcom-mode node in device
tree\n");
+
+ if (!strcmp("software", vcom_mode_str)) {
+ smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
...
+ smd->pitch = (SHARP_ADDR_PERIOD + smd->mode->hdisplay +
SHARP_DUMMY_PERIOD) / 8;
+ smd->tx_buffer_size = (SHARP_MODE_PERIOD +
+ (SHARP_ADDR_PERIOD + (smd->mode->hdisplay) +
SHARP_DUMMY_PERIOD) *
+ smd->mode->vdisplay) / 8;
+
+ smd->tx_buffer = devm_kzalloc(&spi->dev, smd->tx_buffer_size,
GFP_KERNEL);
+ if (!smd->tx_buffer)
+ return dev_err_probe(&spi->dev, -ENOMEM, "Unable to alloc tx
buffer\n");
There is no need to log a message for memory allocation failure.
return -ENOMEM; should be just fine IMHO.
+
+ mutex_init(&smd->tx_mutex);
+
+ drm->mode_config.min_width = smd->mode->hdisplay;
+ drm->mode_config.max_width = smd->mode->hdisplay;
+ drm->mode_config.min_height = smd->mode->vdisplay;
+ drm->mode_config.max_height = smd->mode->vdisplay;
+
+ ret = sharp_memory_pipe_init(drm, smd,
+sharp_memory_formats,
Nitpick: you could save 1 LoC if this is put at the end of the previous
line.
+ARRAY_SIZE(sharp_memory_formats),
+NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to initialize display
pipeline.\n");
+
+ drm_plane_enable_fb_damage_clips(&smd->plane);
+ drm_mode_config_reset(drm);
+
+ ret = drm_dev_register(drm, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register drm
device.\n");
+
+ drm_fbdev_dma_setup(drm, 0);
+
+ return 0;
+}
...
CJ
Re: [PATCH 2/2] drm/tiny: Add driver for Sharp Memory LCD
Le 25/07/2024 à 02:47, Alex Lanzano a écrit :
Add support for the monochrome Sharp Memory LCDs.
Signed-off-by: Alex Lanzano
---
MAINTAINERS | 8 +
drivers/gpu/drm/tiny/Kconfig| 20 +
drivers/gpu/drm/tiny/Makefile | 1 +
drivers/gpu/drm/tiny/sharp-memory.c | 742
4 files changed, 771 insertions(+)
create mode 100644 drivers/gpu/drm/tiny/sharp-memory.c
Hi,
below a few comments, mostly cosmetic and 2 more interesting things
related to error handling at the end.
Hope it helps.
CJ
...
diff --git a/drivers/gpu/drm/tiny/sharp-memory.c
b/drivers/gpu/drm/tiny/sharp-memory.c
new file mode 100644
index ..5e61e348ce3a
--- /dev/null
+++ b/drivers/gpu/drm/tiny/sharp-memory.c
@@ -0,0 +1,742 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+#include
Is it really needed?
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
Nitpick: usually, alphabetical order is preferred.
+
+#include
+#include
+#include
+#include
+#include
+#include
...
+static inline void sharp_memory_set_tx_buffer_addresses(uint8_t *buffer,
+ struct drm_rect clip,
+ uint32_t pitch)
+{
+ for (uint32_t line = 0; line < clip.y2; ++line)
+ buffer[line * pitch] = line + 1;
+
Nitpick: No need for an empty line.
+}
+
+static void sharp_memory_set_tx_buffer_data(uint8_t *buffer,
+ struct drm_framebuffer *fb,
+ struct drm_rect clip,
+ uint32_t pitch,
+ struct drm_format_conv_state
*fmtcnv_state)
+{
+ int ret;
+ struct iosys_map dst, vmap;
+ struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret)
+ return;
+
+
Nitpick: No need for 2 newlines.
+ iosys_map_set_vaddr(&dst, buffer);
+ iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
+
+ drm_fb_xrgb_to_mono(&dst, &pitch, &vmap, fb, &clip, fmtcnv_state);
+
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+}
...
+static void sharp_memory_plane_atomic_update(struct drm_plane *plane,
+struct drm_atomic_state *state)
+{
+
Nitpick: No need for an empty line.
+ struct drm_plane_state *old_state =
drm_atomic_get_old_plane_state(state, plane);
+ struct drm_plane_state *plane_state = plane->state;
+ struct drm_format_conv_state fmtcnv_state = DRM_FORMAT_CONV_STATE_INIT;
+ struct sharp_memory_device *smd;
+ struct drm_rect rect;
+
+ smd = container_of(plane, struct sharp_memory_device, plane);
+ if (!smd->crtc.state->active)
+ return;
+
+
+ if (drm_atomic_helper_damage_merged(old_state, plane_state, &rect))
+ sharp_memory_fb_dirty(plane_state->fb, &rect, &fmtcnv_state);
+
+ drm_format_conv_state_release(&fmtcnv_state);
+}
...
+static void sharp_memory_crtc_enable(struct drm_crtc *crtc,
+struct drm_atomic_state *state)
+{
+ struct pwm_state pwm_state;
+ struct sharp_memory_device *smd = drm_to_sharp_memory_device(crtc->dev);
+
+ sharp_memory_clear_display(smd);
+
+ if (smd->enable_gpio)
+ gpiod_set_value(smd->enable_gpio, 1);
+
+
Nitpick: No need for 2 newlines.
+ switch (smd->vcom_mode) {
+ case SHARP_MEMORY_SOFTWARE_VCOM:
+ smd->sw_vcom_signal =
kthread_run(sharp_memory_sw_vcom_signal_thread,
+ smd, "sw_vcom_signal");
+ break;
+
+ case SHARP_MEMORY_EXTERNAL_VCOM:
+ break;
+
+ case SHARP_MEMORY_PWM_VCOM:
+ pwm_get_state(smd->pwm_vcom_signal, &pwm_state);
+ pwm_state.period =10;
+ pwm_state.duty_cycle = 1;
+ pwm_state.enabled = true;
+ pwm_apply_state(smd->pwm_vcom_signal, &pwm_state);
+ break;
+ }
+}
+
+static void sharp_memory_crtc_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct sharp_memory_device *smd = drm_to_sharp_memory_device(crtc->dev);
+
+ sharp_memory_clear_display(smd);
+
+ if (smd->enable_gpio)
+ gpiod_set_value(smd->enable_gpio, 0);
+
+
Nitpick: No need for 2 newlines.
+ switch (smd->vcom_mode) {
+ case SHARP_MEMORY_SOFTWARE_VCOM:
+ kthread_stop(smd->sw_vcom_signal);
+ break;
+
+ case SHARP_MEMORY_EXTERNAL_VCOM:
+ break;
+
+ case SHARP_MEMORY_PWM_VCOM:
+ pwm_disable(smd->
Re: [PATCH] drm/nouveau/debugfs: Simplify character output in nouveau_debugfs_vbios_image()
Le 15/07/2024 à 15:15, Ilia Mirkin a écrit : On Mon, Jul 15, 2024 at 7:49 AM Markus Elfring wrote: From: Markus Elfring Date: Mon, 15 Jul 2024 13:36:54 +0200 Single characters should be put into a sequence. Thus use the corresponding function “seq_putc” for one selected call. This issue was transformed by using the Coccinelle software. Suggested-by: Christophe Jaillet Signed-off-by: Markus Elfring --- drivers/gpu/drm/nouveau/nouveau_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c b/drivers/gpu/drm/nouveau/nouveau_debugfs.c index e83db051e851..931b62097366 100644 --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c @@ -42,7 +42,7 @@ nouveau_debugfs_vbios_image(struct seq_file *m, void *data) int i; for (i = 0; i < drm->vbios.length; i++) - seq_printf(m, "%c", drm->vbios.data[i]); + seq_putc(m, drm->vbios.data[i]); Is there some reason this whole thing isn't just seq_write(m, drm->vbios.data, drm->vbios.length) Hi, I don't know if my answer is relevant or not here but: for () seq_putc(); ==> will fill 'm' with everything that fits in and seq_write() ==> is all or nothing. So if 'm' is too small, then nothing will be appended. I've not looked at the calling tree, but I would expect 'm' to be able to have PAGE_SIZE chars, so most probably 4096. And having gpu + "vbios.rom", I would expect it to be bigger than 4096. If I'm correct, then changing for seq_write() would just show... nothing. I don't know if it can happen., but testing should be easy enough to figure it out. just my 2c. CJ return 0; } -- 2.45.2
[PATCH] accel/habanalabs/gaudi2: Constify several structures
These structures are not modified in this driver.
Constifying this structure moves about 8 ko of data to a read-only section,
so increase overall security.
On a x86_64, with allmodconfig:
Before:
==
textdata bss dec hex filename
191214 134180 456 325850 4f8da drivers/accel/habanalabs/gaudi2/gaudi2.o
After:
=
textdata bss dec hex filename
198862 126532 456 325850 4f8da drivers/accel/habanalabs/gaudi2/gaudi2.o
Signed-off-by: Christophe JAILLET
---
Compile tested-only.
A much bigger step, would be to constify gaudi2_irq_map_table.
But drivers/accel/habanalabs/include/gaudi2/gaudi2_async_ids_map_extended.h
states that this is a generated file. So I just report it here, could
someone modify the software that generates this file.
This would constify an additional 110 ko of data. Not too bad, for just 5
more letters!
textdata bss dec hex filename
310314 14884 456 325654 4f816 drivers/accel/habanalabs/gaudi2/gaudi2.o
---
drivers/accel/habanalabs/gaudi2/gaudi2.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c
b/drivers/accel/habanalabs/gaudi2/gaudi2.c
index a38b88baadf2..40aebae29ab1 100644
--- a/drivers/accel/habanalabs/gaudi2/gaudi2.c
+++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c
@@ -173,7 +173,7 @@ struct gaudi2_razwi_info {
char *eng_name;
};
-static struct gaudi2_razwi_info common_razwi_info[] = {
+static const struct gaudi2_razwi_info common_razwi_info[] = {
{RAZWI_INITIATOR_ID_X_Y(2, 4, 0), mmDCORE0_RTR0_CTRL_BASE,
GAUDI2_DCORE0_ENGINE_ID_DEC_0, "DEC0"},
{RAZWI_INITIATOR_ID_X_Y(2, 4, 4), mmDCORE0_RTR0_CTRL_BASE,
@@ -338,7 +338,7 @@ static struct gaudi2_razwi_info common_razwi_info[] = {
GAUDI2_ENGINE_ID_PSOC, "PSOC"}
};
-static struct gaudi2_razwi_info mme_razwi_info[] = {
+static const struct gaudi2_razwi_info mme_razwi_info[] = {
/* MME X high coordinate is N/A, hence using only low
coordinates */
{RAZWI_INITIATOR_ID_X_Y_LOW(7, 4), mmDCORE0_RTR5_CTRL_BASE,
GAUDI2_DCORE0_ENGINE_ID_MME, "MME0_WAP0"},
@@ -2108,7 +2108,7 @@ struct hbm_mc_error_causes {
char cause[50];
};
-static struct hl_special_block_info gaudi2_special_blocks[] =
GAUDI2_SPECIAL_BLOCKS;
+static const struct hl_special_block_info gaudi2_special_blocks[] =
GAUDI2_SPECIAL_BLOCKS;
/* Special blocks iterator is currently used to configure security protection
bits,
* and read global errors. Most HW blocks are addressable and those who aren't
(N/A)-
@@ -2116,14 +2116,14 @@ static struct hl_special_block_info
gaudi2_special_blocks[] = GAUDI2_SPECIAL_BLO
* and global error reading, since currently they both share the same settings.
* Once it changes, we must remember to use separate configurations for either
one.
*/
-static int gaudi2_iterator_skip_block_types[] = {
+static const int gaudi2_iterator_skip_block_types[] = {
GAUDI2_BLOCK_TYPE_PLL,
GAUDI2_BLOCK_TYPE_EU_BIST,
GAUDI2_BLOCK_TYPE_HBM,
GAUDI2_BLOCK_TYPE_XFT
};
-static struct range gaudi2_iterator_skip_block_ranges[] = {
+static const struct range gaudi2_iterator_skip_block_ranges[] = {
/* Skip all PSOC blocks except for PSOC_GLOBAL_CONF */
{mmPSOC_I2C_M0_BASE, mmPSOC_EFUSE_BASE},
{mmPSOC_BTL_BASE, mmPSOC_MSTR_IF_RR_SHRD_HBW_BASE},
@@ -2132,7 +2132,7 @@ static struct range gaudi2_iterator_skip_block_ranges[] =
{
{mmCPU_TIMESTAMP_BASE, mmCPU_MSTR_IF_RR_SHRD_HBW_BASE}
};
-static struct hbm_mc_error_causes hbm_mc_spi[GAUDI2_NUM_OF_HBM_MC_SPI_CAUSE] =
{
+static const struct hbm_mc_error_causes
hbm_mc_spi[GAUDI2_NUM_OF_HBM_MC_SPI_CAUSE] = {
{HBM_MC_SPI_TEMP_PIN_CHG_MASK, "temperature pins changed"},
{HBM_MC_SPI_THR_ENG_MASK, "temperature-based throttling engaged"},
{HBM_MC_SPI_THR_DIS_ENG_MASK, "temperature-based throttling
disengaged"},
@@ -8300,9 +8300,9 @@ static void gaudi2_check_if_razwi_happened(struct
hl_device *hdev)
gaudi2_ack_module_razwi_event_handler(hdev, RAZWI_ROT, mod_idx,
0, NULL);
}
-static int gaudi2_psoc_razwi_get_engines(struct gaudi2_razwi_info *razwi_info,
u32 array_size,
- u32 axuser_xy, u32 *base, u16
*eng_id,
- char *eng_name)
+static int gaudi2_psoc_razwi_get_engines(const struct gaudi2_razwi_info
*razwi_info,
+u32 array_size, u32 axuser_xy,
+u32 *base, u16 *eng_id, char *eng_name)
{
int i, num_of_eng = 0;
--
2.45.2
[PATCH] drm/exynos: Constify struct exynos_drm_ipp_funcs
'struct exynos_drm_ipp_funcs' are not modified in these drivers.
Constifying this structure moves some data to a read-only section, so
increase overall security.
On a x86_64, with allmodconfig, as an example:
Before:
==
textdata bss dec hex filename
204461746 16 2220856c0 drivers/gpu/drm/exynos/exynos_drm_fimc.o
After:
=
textdata bss dec hex filename
204461714 16 2217656a0 drivers/gpu/drm/exynos/exynos_drm_fimc.o
Signed-off-by: Christophe JAILLET
---
Compile tested-only.
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_gsc.c| 2 +-
drivers/gpu/drm/exynos/exynos_drm_scaler.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 142184c8c3bc..4d7ea65b7dd8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1125,7 +1125,7 @@ static void fimc_abort(struct exynos_drm_ipp *ipp,
}
}
-static struct exynos_drm_ipp_funcs ipp_funcs = {
+static const struct exynos_drm_ipp_funcs ipp_funcs = {
.commit = fimc_commit,
.abort = fimc_abort,
};
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 1b111e2c3347..d80b0d1eb734 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1162,7 +1162,7 @@ static void gsc_abort(struct exynos_drm_ipp *ipp,
}
}
-static struct exynos_drm_ipp_funcs ipp_funcs = {
+static const struct exynos_drm_ipp_funcs ipp_funcs = {
.commit = gsc_commit,
.abort = gsc_abort,
};
diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
index a9d469896824..2788105ac780 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
@@ -403,7 +403,7 @@ static int scaler_commit(struct exynos_drm_ipp *ipp,
return 0;
}
-static struct exynos_drm_ipp_funcs ipp_funcs = {
+static const struct exynos_drm_ipp_funcs ipp_funcs = {
.commit = scaler_commit,
};
--
2.45.2
[PATCH] drm/rockchip: Constify struct drm_encoder_helper_funcs
'struct drm_encoder_helper_funcs' is not modified in these drivers.
Constifying this structure moves some data to a read-only section, so
increase overall security.
On a x86_64, with allmodconfig:
Before:
==
textdata bss dec hex filename
7458 552 080101f4a
drivers/gpu/drm/rockchip/analogix_dp-rockchip.o
After:
=
textdata bss dec hex filename
7578 424 080021f42
drivers/gpu/drm/rockchip/analogix_dp-rockchip.o
Signed-off-by: Christophe JAILLET
---
Compile tested-only.
---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +-
drivers/gpu/drm/rockchip/inno_hdmi.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 362c7951ca4a..d3341edfe4f4 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -262,7 +262,7 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder
*encoder,
return 0;
}
-static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = {
+static const struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs
= {
.mode_fixup = rockchip_dp_drm_encoder_mode_fixup,
.mode_set = rockchip_dp_drm_encoder_mode_set,
.atomic_enable = rockchip_dp_drm_encoder_enable,
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 2241e53a2946..44ce0f581062 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -545,7 +545,7 @@ inno_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
return 0;
}
-static struct drm_encoder_helper_funcs inno_hdmi_encoder_helper_funcs = {
+static const struct drm_encoder_helper_funcs inno_hdmi_encoder_helper_funcs = {
.atomic_check = inno_hdmi_encoder_atomic_check,
.atomic_enable = inno_hdmi_encoder_enable,
.atomic_disable = inno_hdmi_encoder_disable,
--
2.45.2
[PATCH] fbdev: mmp: Constify struct mmp_overlay_ops
'struct mmp_overlay_ops' is not modified in this driver.
Constifying this structure moves some data to a read-only section, so
increase overall security.
On a x86_64, with allmodconfig, as an example:
Before:
==
textdata bss dec hex filename
11798 555 16 123693051 drivers/video/fbdev/mmp/hw/mmp_ctrl.o
After:
=
textdata bss dec hex filename
11834 507 16 123573045 drivers/video/fbdev/mmp/hw/mmp_ctrl.o
Signed-off-by: Christophe JAILLET
---
Compile tested-only
---
drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 2 +-
include/video/mmp_disp.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
index 76b50b6c98ad..a20a2c408127 100644
--- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
@@ -313,7 +313,7 @@ static void path_set_mode(struct mmp_path *path, struct
mmp_mode *mode)
mutex_unlock(&path->access_ok);
}
-static struct mmp_overlay_ops mmphw_overlay_ops = {
+static const struct mmp_overlay_ops mmphw_overlay_ops = {
.set_fetch = overlay_set_fetch,
.set_onoff = overlay_set_onoff,
.set_win = overlay_set_win,
diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
index a722dcbf5073..41354bd49895 100644
--- a/include/video/mmp_disp.h
+++ b/include/video/mmp_disp.h
@@ -156,7 +156,7 @@ struct mmp_overlay {
int status;
struct mutex access_ok;
- struct mmp_overlay_ops *ops;
+ const struct mmp_overlay_ops *ops;
};
/* panel type */
@@ -299,7 +299,7 @@ struct mmp_path_info {
int overlay_num;
void (*set_mode)(struct mmp_path *path, struct mmp_mode *mode);
void (*set_onoff)(struct mmp_path *path, int status);
- struct mmp_overlay_ops *overlay_ops;
+ const struct mmp_overlay_ops *overlay_ops;
void *plat_data;
};
--
2.45.2
Re: [PATCH 2/2] drm/panel-xinpeng-xpp055c272: add check for mipi_dsi_dcs_enter_sleep_mode
Le 20/06/2024 à 09:48, Chen Ni a écrit :
Add check for the return value of mipi_dsi_dcs_enter_sleep_mode() and
return the error if it fails in order to catch the error.
Signed-off-by: Chen Ni
---
drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
index 22a14006765e..d7c9df673f35 100644
--- a/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
+++ b/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c
@@ -139,7 +139,7 @@ static int xpp055c272_unprepare(struct drm_panel *panel)
if (ret < 0)
dev_err(ctx->dev, "failed to set display off: %d\n", ret);
- mipi_dsi_dcs_enter_sleep_mode(dsi);
+ ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
if (ret < 0) {
dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
return ret;
Reviewed-by: Christophe JAILLET
See:
https://lore.kernel.org/all/6e3a8cb3956fe94f1259c13053fddb378e7d0d82.1619878508.git.christophe.jail...@wanadoo.fr/
which never got any feedback.
CJ
Re: [PATCH 1/2] drm/panel: ltk050h3146w: add check for mipi_dsi_dcs_enter_sleep_mode
Le 20/06/2024 à 09:47, Chen Ni a écrit :
Add check for the return value of mipi_dsi_dcs_enter_sleep_mode() and
return the error if it fails in order to catch the error.
Signed-off-by: Chen Ni
---
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index 292aa26a456d..24bf05d0589f 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -526,7 +526,7 @@ static int ltk050h3146w_unprepare(struct drm_panel *panel)
return ret;
}
- mipi_dsi_dcs_enter_sleep_mode(dsi);
+ ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
if (ret < 0) {
dev_err(ctx->dev, "failed to enter sleep mode: %d\n", ret);
return ret;
Reviewed-by: Christophe JAILLET
See:
https://lore.kernel.org/all/588e8b4519487f6d33419c4b0fa7f8ea1b26cb58.1619869792.git.christophe.jail...@wanadoo.fr/
which never got any feedback.
CJ
[PATCH] drm/nouveau: Constify struct nouveau_job_ops
"struct nouveau_job_ops" is not modified in these drivers.
Constifying this structure moves some data to a read-only section, so
increase overall security.
In order to do it, "struct nouveau_job" and "struct nouveau_job_args" also
need to be adjusted to this new const qualifier.
On a x86_64, with allmodconfig:
Before:
==
textdata bss dec hex filename
5570 152 05722165a drivers/gpu/drm/nouveau/nouveau_exec.o
After:
=
textdata bss dec hex filename
5630 112 05742166e drivers/gpu/drm/nouveau/nouveau_exec.o
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/nouveau/nouveau_exec.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_sched.h | 4 ++--
drivers/gpu/drm/nouveau/nouveau_uvmm.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index e65c0ef23bc7..a0b5f1b16e8b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -188,7 +188,7 @@ nouveau_exec_job_timeout(struct nouveau_job *job)
return DRM_GPU_SCHED_STAT_NOMINAL;
}
-static struct nouveau_job_ops nouveau_exec_job_ops = {
+static const struct nouveau_job_ops nouveau_exec_job_ops = {
.submit = nouveau_exec_job_submit,
.armed_submit = nouveau_exec_job_armed_submit,
.run = nouveau_exec_job_run,
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h
b/drivers/gpu/drm/nouveau/nouveau_sched.h
index e1f01a23e6f6..20cd1da8db73 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -42,7 +42,7 @@ struct nouveau_job_args {
u32 count;
} out_sync;
- struct nouveau_job_ops *ops;
+ const struct nouveau_job_ops *ops;
};
struct nouveau_job {
@@ -73,7 +73,7 @@ struct nouveau_job {
u32 count;
} out_sync;
- struct nouveau_job_ops {
+ const struct nouveau_job_ops {
/* If .submit() returns without any error, it is guaranteed that
* armed_submit() is called.
*/
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index ee02cd833c5e..9402fa320a7e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1534,7 +1534,7 @@ nouveau_uvmm_bind_job_cleanup(struct nouveau_job *job)
nouveau_uvmm_bind_job_put(bind_job);
}
-static struct nouveau_job_ops nouveau_bind_job_ops = {
+static const struct nouveau_job_ops nouveau_bind_job_ops = {
.submit = nouveau_uvmm_bind_job_submit,
.armed_submit = nouveau_uvmm_bind_job_armed_submit,
.run = nouveau_uvmm_bind_job_run,
--
2.45.2
[PATCH v2] dma-buf/fence-array: Add flex array to struct dma_fence_array
This is an effort to get rid of all multiplications from allocation
functions in order to prevent integer overflows [1][2].
The "struct dma_fence_array" can be refactored to add a flex array in order
to have the "callback structures allocated behind the array" be more
explicit.
Do so:
- makes the code more readable and safer.
- allows using __counted_by() for additional checks
- avoids some pointer arithmetic in dma_fence_array_enable_signaling()
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
[1]
Link: https://github.com/KSPP/linux/issues/160 [2]
Signed-off-by: Christophe JAILLET
Reviewed-by: Kees Cook
---
Compile tested only.
Changes in v2:
- Name the new field 'callbacks' instead of 'cb' [Christian König]
v1:
https://lore.kernel.org/all/d3204a5b4776553455c2cfb1def72f1dae0dba25.1716054403.git.christophe.jail...@wanadoo.fr/
---
drivers/dma-buf/dma-fence-array.c | 10 --
include/linux/dma-fence-array.h | 3 +++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index 9b3ce8948351..c74ac197d5fe 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -70,7 +70,7 @@ static void dma_fence_array_cb_func(struct dma_fence *f,
static bool dma_fence_array_enable_signaling(struct dma_fence *fence)
{
struct dma_fence_array *array = to_dma_fence_array(fence);
- struct dma_fence_array_cb *cb = (void *)(&array[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(&array->lock);
dma_fence_init(&array->base, &dma_fence_array_ops, &array->lock,
context, seqno);
init_irq_work(&array->work, irq_dma_fence_array_work);
- array->num_fences = num_fences;
atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
array->fences = fences;
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index ec7f25def392..29c5650c1038 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -33,6 +33,7 @@ struct dma_fence_array_cb {
* @num_pending: fences in the array still pending
* @fences: array of the fences
* @work: internal irq_work function
+ * @callbacks: array of callback helpers
*/
struct dma_fence_array {
struct dma_fence base;
@@ -43,6 +44,8 @@ struct dma_fence_array {
struct dma_fence **fences;
struct irq_work work;
+
+ struct dma_fence_array_cb callbacks[] __counted_by(num_fences);
};
/**
--
2.45.1
[PATCH] drm/panel: lg-sw43408: Fix an error handling path in sw43408_probe()
If mipi_dsi_attach() fails, we must undo the drm_panel_add() call hidden in
sw43408_add(), as already done in the remove function.
Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Christophe JAILLET
---
Compile tested only
---
drivers/gpu/drm/panel/panel-lg-sw43408.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c
b/drivers/gpu/drm/panel/panel-lg-sw43408.c
index 115f4702d59f..27f2ad788d38 100644
--- a/drivers/gpu/drm/panel/panel-lg-sw43408.c
+++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
@@ -286,7 +286,15 @@ static int sw43408_probe(struct mipi_dsi_device *dsi)
dsi->dsc = &ctx->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(&ctx->base);
+ return ret;
}
static void sw43408_remove(struct mipi_dsi_device *dsi)
--
2.45.1
[PATCH] drm: zynqmp_dpsub: Fix an error handling path in zynqmp_dpsub_probe()
If zynqmp_dpsub_drm_init() fails, we must undo the previous
drm_bridge_add() call.
Fixes: be3f3042391d ("drm: zynqmp_dpsub: Always register bridge")
Signed-off-by: Christophe JAILLET
---
Compile tested only
---
drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index face8d6b2a6f..f5781939de9c 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -269,6 +269,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
return 0;
err_disp:
+ drm_bridge_remove(dpsub->bridge);
zynqmp_disp_remove(dpsub);
err_dp:
zynqmp_dp_remove(dpsub);
--
2.45.1
[PATCH] dma-buf/fence-array: Add flex array to struct dma_fence_array
This is an effort to get rid of all multiplications from allocation
functions in order to prevent integer overflows [1][2].
The "struct dma_fence_array" can be refactored to add a flex array in order
to have the "callback structures allocated behind the array" be more
explicit.
Do so:
- makes the code more readable and safer.
- allows using __counted_by() for additional checks
- avoids some pointer arithmetic in dma_fence_array_enable_signaling()
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
[1]
Link: https://github.com/KSPP/linux/issues/160 [2]
Signed-off-by: Christophe JAILLET
---
Compile tested only.
Also, I don't think that 'cb' is a great name and the associated kernel-doc
description could certainly be improved.
Any proposal welcomed :)
---
drivers/dma-buf/dma-fence-array.c | 10 --
include/linux/dma-fence-array.h | 3 +++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-array.c
b/drivers/dma-buf/dma-fence-array.c
index 9b3ce8948351..9c55afaca607 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -70,7 +70,7 @@ static void dma_fence_array_cb_func(struct dma_fence *f,
static bool dma_fence_array_enable_signaling(struct dma_fence *fence)
{
struct dma_fence_array *array = to_dma_fence_array(fence);
- struct dma_fence_array_cb *cb = (void *)(&array[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(&array->lock);
dma_fence_init(&array->base, &dma_fence_array_ops, &array->lock,
context, seqno);
init_irq_work(&array->work, irq_dma_fence_array_work);
- array->num_fences = num_fences;
atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
array->fences = fences;
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index ec7f25def392..a793f9d5c73b 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -33,6 +33,7 @@ struct dma_fence_array_cb {
* @num_pending: fences in the array still pending
* @fences: array of the fences
* @work: internal irq_work function
+ * @cb: array of callback helpers
*/
struct dma_fence_array {
struct dma_fence base;
@@ -43,6 +44,8 @@ struct dma_fence_array {
struct dma_fence **fences;
struct irq_work work;
+
+ struct dma_fence_array_cb cb[] __counted_by(num_fences);
};
/**
--
2.45.1
Re: [PATCH] drm/nouveau/nvif: Avoid build error due to potential integer overflows
(adding linux-harden...@vger.kernel.org)
Le 18/05/2024 à 16:37, Guenter Roeck a écrit :
Trying to build parisc:allmodconfig with gcc 12.x or later results
in the following build error.
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd':
drivers/gpu/drm/nouveau/nvif/object.c:161:9: error:
'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32
overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict]
161 | memcpy(data, args->mthd.data, size);
| ^~~
drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor':
drivers/gpu/drm/nouveau/nvif/object.c:298:17: error:
'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56
overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict]
298 | memcpy(data, args->new.data, size);
gcc assumes that 'sizeof(*args) + size' can overflow, which would result
in the problem.
The problem is not new, only it is now no longer a warning but an error since
W=1
has been enabled for the drm subsystem and since Werror is enabled for test
builds.
Rearrange arithmetic and add extra size checks to avoid the overflow.
Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
subsystem")
Cc: Javier Martinez Canillas
Cc: Jani Nikula
Cc: Thomas Zimmermann
Cc: Danilo Krummrich
Cc: Maxime Ripard
Signed-off-by: Guenter Roeck
---
checkpatch complains about the line length in the description and the
(pre-existing)
assignlemts in if conditions, but I did not want to split lines in the
description
or rearrange the code further.
I don't know why I only see the problem with parisc builds (at least so far).
drivers/gpu/drm/nouveau/nvif/object.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
b/drivers/gpu/drm/nouveau/nvif/object.c
index 4d1aaee8fe15..baf623a48874 100644
--- a/drivers/gpu/drm/nouveau/nvif/object.c
+++ b/drivers/gpu/drm/nouveau/nvif/object.c
@@ -145,8 +145,9 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void
*data, u32 size)
u8 stack[128];
int ret;
- if (sizeof(*args) + size > sizeof(stack)) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
+ if (size > sizeof(stack) - sizeof(*args)) {
+ if (size > INT_MAX ||
+ !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
Hi,
Would it be cleaner or better to use size_add(sizeof(*args), size)?
return -ENOMEM;
} else {
args = (void *)stack;
@@ -276,7 +277,8 @@ nvif_object_ctor(struct nvif_object *parent, const char
*name, u32 handle,
object->map.size = 0;
if (parent) {
- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
+ if (size > INT_MAX ||
+ !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
Same.
CJ
nvif_object_dtor(object);
return -ENOMEM;
}
Re: [PATCH v3 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 16/04/2024 à 20:30, David Wronek a écrit :
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 367 ++
3 files changed, 382 insertions(+)
...
+static int rm69380_probe(struct mipi_dsi_device *dsi)
+{
+ struct mipi_dsi_host *dsi_sec_host;
+ struct rm69380_panel *ctx;
+ struct device *dev = &dsi->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,
&info);
+ 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(&ctx->panel, dev, &rm69380_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(&ctx->panel);
+
+ for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
+ if (!ctx->dsi[i])
+ continue;
+
+ dev_dbg(&ctx->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(&ctx->panel);
+ return dev_err_probe(dev, ret,
+"Failed to attach to DSI%d\n", i);
+ }
+ }
+
+ return 0;
+}
drm_panel_add(&ctx->panel);
for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) {
if (!ctx->dsi[i])
continue;
dev_dbg(&ctx->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(&ctx->panel);
return ret;
}
Re: [PATCH v2 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 15/04/2024 à 18:10, David Wronek a écrit :
Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a
Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021.
Signed-off-by: David Wronek
---
drivers/gpu/drm/panel/Kconfig | 14 +
drivers/gpu/drm/panel/Makefile| 1 +
drivers/gpu/drm/panel/panel-raydium-rm69380.c | 366 ++
3 files changed, 381 insertions(+)
...
+static int rm69380_on(struct rm69380_panel *ctx)
+{
+ struct mipi_dsi_device *dsi = ctx->dsi[0];
+ struct device *dev = &dsi->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 = &dsi->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 = &dsi->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, &info);
+ if (IS_ERR(ctx->dsi[1])) {
+ return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]),
+"Cannot get secondary DSI node\n");
+ }
Nit: unneeded { }
+
+ dev_dbg(dev, "Second DSI name `%s`\n", ctx->dsi[1]->name);
+ mipi_dsi_set_drvdata(ctx->dsi[1], ctx);
+ } else {
+ dev_dbg(dev, "Using Single-DSI\n");
+
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 15/04/2024 à 07:37, david-vu3dztd92roxwddmvfq...@public.gmane.org a écrit : W dniu 2024-04-14 22:22, Christophe JAILLET napisał(a): Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? Yes, this is just one of the examples where this driver IC can be found. It can also be used with panels other than those from EDO. Hi, sorry if i was unclear. Is there a typo: s/combiantion/combination/ ? CJ + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF Best regards, David Wronek
Re: [PATCH 2/2] drm/panel: Add driver for EDO RM69380 OLED panel
Le 14/04/2024 à 17:22, David Wronek a écrit : Add support for the 2560x1600@90Hz OLED panel by EDO bundled with a Raydium RM69380 controller, as found on the Lenovo Xiaoxin Pad Pro 2021. Signed-off-by: David Wronek --- drivers/gpu/drm/panel/Kconfig | 14 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm69380.c | 378 ++ 3 files changed, 393 insertions(+) diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 154f5bf82980..84cbd838f57e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -542,6 +542,20 @@ config DRM_PANEL_RAYDIUM_RM692E5 Say Y here if you want to enable support for Raydium RM692E5-based display panels, such as the one found in the Fairphone 5 smartphone. +config DRM_PANEL_RAYDIUM_RM69380 + tristate "Raydium RM69380-based DSI panel" + depends on BACKLIGHT_CLASS_DEVICE + depends on DRM_DISPLAY_DP_HELPER + depends on DRM_DISPLAY_HELPER + depends on DRM_MIPI_DSI + depends on OF + help + Say Y here if you want to enable support for Raydium RM69380-based + display panels. + + This panel controller can be found in the Lenovo Xiaoxin Pad Pro 2021 + in combiantion with an EDO OLED panel. combination? + config DRM_PANEL_RONBO_RB070D30 tristate "Ronbo Electronics RB070D30 panel" depends on OF
Re: [PATCH] drm/i915/guc: Remove usage of the deprecated ida_simple_xx() API
Le 25/01/2024 à 01:04, Matthew Brost a écrit :
On Sun, Jan 14, 2024 at 04:15:34PM +0100, Christophe JAILLET wrote:
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_range() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET
Reviewed-by: Matthew Brost
Hi,
polite reminder ;-)
CJ
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a259f1118c5a..73ce21ddf682 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2156,11 +2156,10 @@ static int new_guc_id(struct intel_guc *guc, struct
intel_context *ce)
order_base_2(ce->parallel.number_children
+ 1));
else
- ret = ida_simple_get(&guc->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(&guc->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(&guc->submission_state.guc_ids,
- ce->guc_id.id);
+ ida_free(&guc->submission_state.guc_ids,
+ce->guc_id.id);
}
clr_ctx_id_mapping(guc, ce->guc_id.id);
set_context_guc_id_invalid(ce);
--
2.43.0
Re: [PATCH v2] treewide: Fix common grammar mistake "the the"
Le 11/04/2024 à 19:11, Thorsten Blum a écrit :
Use `find . -type f -exec sed -i 's/\/the/g' {} +` to find all
occurrences of "the the" and replace them with a single "the".
In arch/arm/include/asm/unwind.h replace "the the" with "to the".
Changes only comments and documentation - no code changes.
Signed-off-by: Thorsten Blum
Reviewed-by: Randy Dunlap
Reviewed-by: Tyler Hicks
...
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -244,7 +244,7 @@ enum sci_controller_states {
SCIC_INITIALIZED,
/**
-* This state indicates the the controller is in the process of becoming
maybe: that the?
+* This state indicates the controller is in the process of becoming
* ready (i.e. starting). In this state no new IO operations are
permitted.
* This state is entered from the INITIALIZED state.
*/
...
diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 3aa16e27f509..503244e8470a 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -731,7 +731,7 @@ struct io_buffer_list *io_pbuf_get_bl(struct io_ring_ctx
*ctx,
* going away, if someone is trying to be sneaky. Look it up under rcu
* so we know it's not going away, and attempt to grab a reference to
* it. If the ref is already zero, then fail the mapping. If successful,
-* the caller will call io_put_bl() to drop the the reference at at the
+* the caller will call io_put_bl() to drop the reference at at the
Not strictly related to your patch, but "at at".
* end. This may then safely free the buffer_list (and drop the pages)
* at that point, vm_insert_pages() would've already grabbed the
* necessary vma references.
...
CJ
Re: [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function
Le 23/03/2024 à 22:25, Marek Behún a écrit :
On Sat, 23 Mar 2024 22:10:40 +0100
Christophe JAILLET wrote:
...
static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev)
{
- pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+ pvt->dbgfs_dir = devm_debugfs_create_dir(dev, dev_name(dev), NULL);
+ if (IS_ERR(pvt->dbgfs_dir))
+ return PTR_ERR(pvt->dbgfs_dir);
Not sure if the test and error handling should be added here.
*If I'm correct*, functions related to debugfs already handle this case
and just do nothing. And failure in debugfs related code is not
considered as something that need to be reported and abort a probe function.
Maybe the same other (already existing) tests in this patch should be
removed as well, in a separated patch.
Functions related to debugfs maybe do, but devm_ resource management
functions may fail to allocate release structure, and those errors need
to be handled, AFAIK.
I would say no.
If this memory allocation fails, then debugfs_create_dir() will not be
called, but that's not a really big deal if the driver itself can still
run normally without it.
Up to you to leave it as-is or remove what I think is a useless error
handling.
At least, maybe it could be said in the commit log, so that maintainers
can comment on it, if they don't spot the error handling you introduce.
CJ
Marek
Re: [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function
Le 23/03/2024 à 17:43, Marek Behún a écrit :
A few drivers register a devm action to remove a debugfs directory,
implementing a one-liner function that calls debufs_remove_recursive().
Help drivers avoid this repeated implementations by adding managed
version of debugfs directory create function.
Use the new function devm_debugfs_create_dir() in the following
drivers:
drivers/crypto/caam/ctrl.c
drivers/gpu/drm/bridge/ti-sn65dsi86.c
drivers/hwmon/hp-wmi-sensors.c
drivers/hwmon/mr75203.c
drivers/hwmon/pmbus/pmbus_core.c
Also use the action function devm_debugfs_dir_recursive_drop() in
driver
drivers/gpio/gpio-mockup.c
As per Dan Williams' request [1], do not touch the driver
drivers/cxl/mem.c
[1]
https://lore.kernel.org/linux-gpio/65d7918b358a5_1ee3129...@dwillia2-mobl3.amr.corp.intel.com.notmuch/
Signed-off-by: Marek Behún
---
drivers/crypto/caam/ctrl.c| 16 +++
drivers/gpio/gpio-mockup.c| 11 ++--
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 13 ++---
drivers/hwmon/hp-wmi-sensors.c| 15 ++
drivers/hwmon/mr75203.c | 15 --
drivers/hwmon/pmbus/pmbus_core.c | 16 ---
include/linux/devm-helpers.h | 40 +++
7 files changed, 61 insertions(+), 65 deletions(-)
...
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 455eecf6380e..adbe0fe09490 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -390,13 +391,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
}
}
-static void gpio_mockup_debugfs_cleanup(void *data)
-{
- struct gpio_mockup_chip *chip = data;
-
- debugfs_remove_recursive(chip->dbg_dir);
-}
-
static void gpio_mockup_dispose_mappings(void *data)
{
struct gpio_mockup_chip *chip = data;
@@ -480,7 +474,8 @@ static int gpio_mockup_probe(struct platform_device *pdev)
gpio_mockup_debugfs_setup(dev, chip);
- return devm_add_action_or_reset(dev, gpio_mockup_debugfs_cleanup, chip);
+ return devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
+ chip->dbg_dir);
This look strange. Shouldn't the debugfs_create_dir() call in
gpio_mockup_debugfs_setup() be changed, instead?
(I've not look in the previous version if something was said about it,
so apologies if already discussed)
}
static const struct of_device_id gpio_mockup_of_match[] = {
...
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 50a8b9c3f94d..50f348fca108 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -216,17 +217,11 @@ static const struct file_operations pvt_ts_coeff_j_fops =
{
.llseek = default_llseek,
};
-static void devm_pvt_ts_dbgfs_remove(void *data)
-{
- struct pvt_device *pvt = (struct pvt_device *)data;
-
- debugfs_remove_recursive(pvt->dbgfs_dir);
- pvt->dbgfs_dir = NULL;
-}
-
static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev)
{
- pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+ pvt->dbgfs_dir = devm_debugfs_create_dir(dev, dev_name(dev), NULL);
+ if (IS_ERR(pvt->dbgfs_dir))
+ return PTR_ERR(pvt->dbgfs_dir);
Not sure if the test and error handling should be added here.
*If I'm correct*, functions related to debugfs already handle this case
and just do nothing. And failure in debugfs related code is not
considered as something that need to be reported and abort a probe function.
Maybe the same other (already existing) tests in this patch should be
removed as well, in a separated patch.
just my 2c
CJ
debugfs_create_u32("ts_coeff_h", 0644, pvt->dbgfs_dir,
&pvt->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,
&pvt_ts_coeff_j_fops);
- return devm_add_action_or_reset(dev, devm_pvt_ts_dbgfs_remove, pvt);
+ return 0;
}
...
[PATCH] drm/bridge: lt9611uxc: Fix an error handling path in lt9611uxc_probe()
If lt9611uxc_audio_init() fails, some resources still need to be released
before returning the error code.
Use the existing error handling path.
Fixes: 0cbbd5b1a012 ("drm: bridge: add support for lontium LT9611UXC bridge")
Signed-off-by: Christophe JAILLET
---
Compile tested only.
---
drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index f4f593ad8f79..d0c77630a2f9 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -965,7 +965,11 @@ static int lt9611uxc_probe(struct i2c_client *client)
}
}
- return lt9611uxc_audio_init(dev, lt9611uxc);
+ ret = lt9611uxc_audio_init(dev, lt9611uxc);
+ if (ret)
+ goto err_remove_bridge;
+
+ return 0;
err_remove_bridge:
free_irq(client->irq, lt9611uxc);
--
2.44.0
[PATCH] drm/i915/display: Save a few bytes of memory in intel_backlight_device_register()
'name' may still be "intel_backlight" when backlight_device_register() is
called.
In such a case, using kstrdup_const() saves a memory duplication when
dev_set_name() is called in backlight_device_register().
Use kfree_const() accordingly.
Signed-off-by: Christophe JAILLET
---
Compile tested only
---
drivers/gpu/drm/i915/display/intel_backlight.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
b/drivers/gpu/drm/i915/display/intel_backlight.c
index 1946d7fb3c2e..9e4a9d4f1585 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -949,7 +949,7 @@ int intel_backlight_device_register(struct intel_connector
*connector)
else
props.power = FB_BLANK_POWERDOWN;
- name = kstrdup("intel_backlight", GFP_KERNEL);
+ name = kstrdup_const("intel_backlight", GFP_KERNEL);
if (!name)
return -ENOMEM;
@@ -963,7 +963,7 @@ int intel_backlight_device_register(struct intel_connector
*connector)
* compatibility. Use unique names for subsequent backlight
devices as a
* fallback when the default name already exists.
*/
- kfree(name);
+ kfree_const(name);
name = kasprintf(GFP_KERNEL, "card%d-%s-backlight",
i915->drm.primary->index,
connector->base.name);
if (!name)
@@ -987,7 +987,7 @@ int intel_backlight_device_register(struct intel_connector
*connector)
connector->base.base.id, connector->base.name, name);
out:
- kfree(name);
+ kfree_const(name);
return ret;
}
--
2.43.2
Re: [PATCH] drm/xe/guc: Remove usage of the deprecated ida_simple_xx() API
Le 14/01/2024 à 16:09, Christophe JAILLET a écrit :
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_max() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/xe/xe_guc_submit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
b/drivers/gpu/drm/xe/xe_guc_submit.c
index 21ac68e3246f..11ffacd1dd58 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -311,7 +311,7 @@ static void __release_guc_id(struct xe_guc *guc, struct
xe_exec_queue *q, u32 xa
q->guc->id - GUC_ID_START_MLRC,
order_base_2(q->width));
else
- ida_simple_remove(&guc->submission_state.guc_ids, q->guc->id);
+ ida_free(&guc->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(&guc->submission_state.guc_ids, 0,
-GUC_ID_NUMBER_SLRC, GFP_NOWAIT);
+ ret = ida_alloc_max(&guc->submission_state.guc_ids,
+ GUC_ID_NUMBER_SLRC - 1, GFP_NOWAIT);
}
if (ret < 0)
return ret;
Hi,
gentle reminder.
All patches to remove the ida_simple API have been sent.
And Matthew Wilcox seems happy with the on going work. (see [1])
Based on next-20240220
$git grep ida_simple_get | wc -l
36
https://elixir.bootlin.com/linux/v6.8-rc3/A/ident/ida_simple_get
50
https://elixir.bootlin.com/linux/v6.7.4/A/ident/ida_simple_get
81
Thanks
CJ
[1]: https://lore.kernel.org/all/zaqrugvz734zj...@casper.infradead.org/
Re: [PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory
Le 19/02/2024 à 09:37, Dan Carpenter a écrit : On Sun, Feb 18, 2024 at 06:46:44PM +0100, Christophe JAILLET wrote: If 'list_limit' is set to a very high value, 'lsize' computation could overflow if 'head.count' is big enough. The "list_limit" is set via module parameter so if you set that high enough to lead to an integer overflow then you kind of deserve what you get. This patch is nice for kernel hardening and making the code easier to read/audit but the real world security impact is negligible. Agreed. That is what I meant by "and unlikely". Maybe the commit message could be more explicit if needed. Let me know if ok as-is or if I should try to re-word the description. CJ regards, dan carpenter
[PATCH v2] udmabuf: Fix a potential (and unlikely) access to unallocated memory
If 'list_limit' is set to a very high value, 'lsize' computation could
overflow if 'head.count' is big enough.
In such a case, udmabuf_create() would access to memory beyond 'list'.
Use memdup_array_user() which checks for overflow.
While at it, include .
Fixes: fbb0de795078 ("Add udmabuf misc device")
Signed-off-by: Christophe JAILLET
---
v2: - Use memdup_array_user() [Kees Cook]
- Use sizeof(*list) [Gustavo A. R. Silva]
- Add include
v1:
https://lore.kernel.org/all/3e37f05c7593f1016f0a46de188b3357cbbd0c0b.1695060389.git.christophe.jail...@wanadoo.fr/
Signed-off-by: Christophe JAILLET
---
drivers/dma-buf/udmabuf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..5728948ea6f2 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -314,14 +315,13 @@ static long udmabuf_ioctl_create_list(struct file *filp,
unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
- list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
+ list = memdup_array_user((void __user *)(arg + sizeof(head)),
+sizeof(*list), head.count);
if (IS_ERR(list))
return PTR_ERR(list);
--
2.43.2
Re: [PATCH V8 02/12] phy: freescale: add Samsung HDMI PHY
Le 03/02/2024 à 17:52, Adam Ford a écrit :
From: Lucas Stach
This adds the driver for the Samsung HDMI PHY found on the
i.MX8MP SoC.
Signed-off-by: Lucas Stach
Signed-off-by: Adam Ford
Tested-by: Alexander Stein
---
...
+static int fsl_samsung_hdmi_phy_probe(struct platform_device *pdev)
+{
+ struct fsl_samsung_hdmi_phy *phy;
+ int ret;
+
+ phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, phy);
+ phy->dev = &pdev->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(&pdev->dev, "register clk failed\n");
+ goto register_clk_failed;
+ }
+
+ pm_runtime_put(phy->dev);
+
+ return 0;
+
+register_clk_failed:
+ clk_disable_unprepare(phy->apbclk);
+
+ return ret;
+}
+
+static int fsl_samsung_hdmi_phy_remove(struct platform_device *pdev)
+{
+ of_clk_del_provider(pdev->dev.of_node);
A clk_disable_unprepare(phy->apbclk) call seems to be missing here.
Or maybe devm_clk_get_enabled() should be used for 'apbclk'?
CJ
+
+ return 0;
+}
...
Re: [PATCH v2 2/2] backlight: Add Kinetic KTD2801 driver
Le 18/01/2024 à 18:32, Duje Mihanović a écrit : Add driver for the Kinetic KTD2801 backlight driver. Signed-off-by: Duje Mihanović --- ... + ktd2801->gpiod = devm_gpiod_get(dev, "ctrl", GPIOD_OUT_HIGH); + if (IS_ERR(ktd2801->gpiod)) + return dev_err_probe(dev, PTR_ERR(dev), PTR_ERR(ktd2801->gpiod); ? CJ
[PATCH] drm/i915/guc: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_range() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a259f1118c5a..73ce21ddf682 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2156,11 +2156,10 @@ static int new_guc_id(struct intel_guc *guc, struct
intel_context *ce)
order_base_2(ce->parallel.number_children
+ 1));
else
- ret = ida_simple_get(&guc->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(&guc->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(&guc->submission_state.guc_ids,
- ce->guc_id.id);
+ ida_free(&guc->submission_state.guc_ids,
+ce->guc_id.id);
}
clr_ctx_id_mapping(guc, ce->guc_id.id);
set_context_guc_id_invalid(ce);
--
2.43.0
[PATCH] drm/amdgpu: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_range() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index ddd0891da116..3d7fcdeaf8cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -62,9 +62,8 @@ int amdgpu_pasid_alloc(unsigned int bits)
int pasid = -EINVAL;
for (bits = min(bits, 31U); bits > 0; bits--) {
- pasid = ida_simple_get(&amdgpu_pasid_ida,
- 1U << (bits - 1), 1U << bits,
- GFP_KERNEL);
+ pasid = ida_alloc_range(&amdgpu_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(&amdgpu_pasid_ida, pasid);
+ ida_free(&amdgpu_pasid_ida, pasid);
}
static void amdgpu_pasid_free_cb(struct dma_fence *fence,
--
2.43.0
[PATCH] drm/xe/guc: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_max() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/xe/xe_guc_submit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
b/drivers/gpu/drm/xe/xe_guc_submit.c
index 21ac68e3246f..11ffacd1dd58 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -311,7 +311,7 @@ static void __release_guc_id(struct xe_guc *guc, struct
xe_exec_queue *q, u32 xa
q->guc->id - GUC_ID_START_MLRC,
order_base_2(q->width));
else
- ida_simple_remove(&guc->submission_state.guc_ids, q->guc->id);
+ ida_free(&guc->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(&guc->submission_state.guc_ids, 0,
-GUC_ID_NUMBER_SLRC, GFP_NOWAIT);
+ ret = ida_alloc_max(&guc->submission_state.guc_ids,
+ GUC_ID_NUMBER_SLRC - 1, GFP_NOWAIT);
}
if (ret < 0)
return ret;
--
2.43.0
[PATCH] drm/amd/display: Fix a switch statement in populate_dml_output_cfg_from_stream_state()
It is likely that the statement related to 'dml_edp' is misplaced. So move
it in the correct "case SIGNAL_TYPE_EDP".
Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
index fa6a93dd9629..64d01a9cd68c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
@@ -626,8 +626,8 @@ static void
populate_dml_output_cfg_from_stream_state(struct dml_output_cfg_st *
if (is_dp2p0_output_encoder(pipe))
out->OutputEncoder[location] = dml_dp2p0;
break;
- out->OutputEncoder[location] = dml_edp;
case SIGNAL_TYPE_EDP:
+ out->OutputEncoder[location] = dml_edp;
break;
case SIGNAL_TYPE_HDMI_TYPE_A:
case SIGNAL_TYPE_DVI_SINGLE_LINK:
--
2.43.0
[PATCH] drm/stm: Fix an error handling path in stm_drm_platform_probe()
If drm_dev_register() fails, a call to drv_load() must be undone, as
already done in the remove function.
Fixes: b759012c5fa7 ("drm/stm: Add STM32 LTDC driver")
Signed-off-by: Christophe JAILLET
---
This was already sent a few years ago in [1] but it got no response.
Since, there has been some activity on this driver, so I send it again.
Note that it is untested.
[1]:
https://lore.kernel.org/all/20200501125511.132029-1-christophe.jail...@wanadoo.fr/
---
drivers/gpu/drm/stm/drv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
index e8523abef27a..4d2db079ad4f 100644
--- a/drivers/gpu/drm/stm/drv.c
+++ b/drivers/gpu/drm/stm/drv.c
@@ -203,12 +203,14 @@ static int stm_drm_platform_probe(struct platform_device
*pdev)
ret = drm_dev_register(ddev, 0);
if (ret)
- goto err_put;
+ goto err_unload;
drm_fbdev_dma_setup(ddev, 16);
return 0;
+err_unload:
+ drv_unload(ddev);
err_put:
drm_dev_put(ddev);
--
2.34.1
Re: [PATCH 1/2] drm/panel: ltk050h3146w: only print message when GPIO getting is not EPROBE_DEFER
Le 04/01/2024 à 13:41, Quentin Schulz a écrit :
From: Quentin Schulz
devm_gpiod_get_optional may return EPROBE_DEFER in case the GPIO
controller isn't yet probed when the panel driver is being probed.
In that case, a spurious and confusing error message about not being
able to get the reset GPIO is printed even though later on the device
actually manages to get probed.
Use dev_err_probe instead so that the message is only printed when it
truly matters.
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
Reviewed-by: Christophe JAILLET
---
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index 30919c872ac8d..ecfa4181c4fd9 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -646,10 +646,8 @@ static int ltk050h3146w_probe(struct mipi_dsi_device *dsi)
return -EINVAL;
ctx->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(ctx->reset_gpio)) {
- dev_err(dev, "cannot get reset gpio\n");
- return PTR_ERR(ctx->reset_gpio);
- }
+ if (IS_ERR(ctx->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "cannot get
reset gpio\n");
ctx->vci = devm_regulator_get(dev, "vci");
if (IS_ERR(ctx->vci)) {
Re: [PATCH 2/2] drm/panel: ltk050h3146w: use dev_err_probe wherever possible
Le 04/01/2024 à 13:41, Quentin Schulz a écrit :
From: Quentin Schulz
This is only a cosmetic change.
This replaces a hand-crafted EPROBE_DEFER handling for deciding to print
an error message with dev_err_probe.
A side-effect is that dev_err_probe also adds a debug message when it's
not EPROBE_DEFER, but this is seen as an improvement.
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
Reviewed-by: Christophe JAILLET
---
drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
index ecfa4181c4fd9..9d87cc1a357e3 100644
--- a/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
+++ b/drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c
@@ -650,20 +650,13 @@ static int ltk050h3146w_probe(struct mipi_dsi_device *dsi)
return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), "cannot get
reset gpio\n");
ctx->vci = devm_regulator_get(dev, "vci");
- if (IS_ERR(ctx->vci)) {
- ret = PTR_ERR(ctx->vci);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to request vci regulator: %d\n",
ret);
- return ret;
- }
+ if (IS_ERR(ctx->vci))
+ return dev_err_probe(dev, PTR_ERR(ctx->vci), "Failed to request vci
regulator\n");
ctx->iovcc = devm_regulator_get(dev, "iovcc");
- if (IS_ERR(ctx->iovcc)) {
- ret = PTR_ERR(ctx->iovcc);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to request iovcc regulator: %d\n",
ret);
- return ret;
- }
+ if (IS_ERR(ctx->iovcc))
+ return dev_err_probe(dev, PTR_ERR(ctx->iovcc),
+"Failed to request iovcc regulator\n");
mipi_dsi_set_drvdata(dsi, ctx);
Re: [PATCH linux-next] drm/amd/display: replace kzalloc and memcpy with kmemdup
Le 08/12/2023 à 03:44, yang.gua...@zte.com.cn a écrit :
From: Yang Guang
Convert kzalloc/memcpy operations to memdup makes for
cleaner code and avoids memcpy() failures
Hi,
usually, function's names are written with () in commit description.
(i.e. kzalloc()/memcpy()).
memdup should be kmemdup().
Finally the proposed change does not avoid memcpy() failures. Should it
fail (what does it mean in this context?), kmemdup() would behave
exactly the same.
Signed-off-by: Chen Haonan
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 76b47f178127..867e1a0fdef6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2264,12 +2264,10 @@ struct dc_state *dc_copy_state(struct dc_state *src_ctx)
#ifdef CONFIG_DRM_AMD_DC_FP
if (new_ctx->bw_ctx.dml2) {
- dml2 = kzalloc(sizeof(struct dml2_context), GFP_KERNEL);
- if (!dml2)
- return NULL;
-
- memcpy(dml2, src_ctx->bw_ctx.dml2, sizeof(struct dml2_context));
- new_ctx->bw_ctx.dml2 = dml2;
+ dml2 = kmemdup(src_ctx->bw_ctx.dml2, sizeof(struct
dml2_context), GFP_KERNEL);
sizeof(struct dml2_context) could be sizeof(*dlm2) to be less verbose.
CJ
+ if (!dml2)
+ return NULL;
+ new_ctx->bw_ctx.dml2 = dml2;
}
#endif
[PATCH] fbdev/offb: Simplify offb_init_fb()
Turn a strcpy()+strncat()+'\0' into an equivalent snprintf().
Signed-off-by: Christophe JAILLET
---
This patch is *not* even compile tested because cross-compiling leads to
some errors like on my machine:
cc1: error: cannot load plugin
./scripts/gcc-plugins/randomize_layout_plugin.so:
./scripts/gcc-plugins/randomize_layout_plugin.so: undefined symbol:
_ZNK6frange6acceptERK14vrange_visitor
So review with care!
---
drivers/video/fbdev/offb.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index dcb1b81d35db..b421b46d88ef 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -423,11 +423,9 @@ static void offb_init_fb(struct platform_device *parent,
const char *name,
fix = &info->fix;
var = &info->var;
- if (name) {
- strcpy(fix->id, "OFfb ");
- strncat(fix->id, name, sizeof(fix->id) - sizeof("OFfb "));
- fix->id[sizeof(fix->id) - 1] = '\0';
- } else
+ if (name)
+ snprintf(fix->id, sizeof(fix->id), "OFfb %s", name);
+ else
snprintf(fix->id, sizeof(fix->id), "OFfb %pOFn", dp);
--
2.34.1
[PATCH] drm/amd: Fix the size of a buffer in amdgpu_vcn_idle_work_handler()
In order to be sure that fw_name is not truncated, this buffer should be
at least 41 bytes long.
Let the compiler compute the correct length by itself.
When building with W=1, this fixes the following warnings:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c: In function ‘amdgpu_vcn_early_init’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:95:58: error: ‘snprintf’ output may
be truncated before the last format character [-Werror=format-truncation=]
95 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:95:9: note: ‘snprintf’ output between
12 and 41 bytes into a destination of size 40
95 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
|
^
Fixes: 69939009bde7 ("drm/amd: Load VCN microcode during early_init")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index c93f3a4c0e31..f8cd55a0d1f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -88,7 +88,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct
*work);
int amdgpu_vcn_early_init(struct amdgpu_device *adev)
{
char ucode_prefix[30];
- char fw_name[40];
+ char fw_name[sizeof(ucode_prefix) + sizeof("amdgpu/.bin") - 1];
int r;
amdgpu_ucode_ip_version_decode(adev, UVD_HWIP, ucode_prefix,
sizeof(ucode_prefix));
--
2.34.1
Re: [PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
Le 18/09/2023 à 05:10, Gustavo A. R. Silva a écrit :
On 9/18/23 12:46, Christophe JAILLET wrote:
If 'list_limit' is set to a very high value, 'lsize' computation could
overflow if 'head.count' is big enough.
In such a case, udmabuf_create() will access to memory beyond 'list'.
Use size_mul() to saturate the value, and have memdup_user() fail.
Fixes: fbb0de795078 ("Add udmabuf misc device")
Signed-off-by: Christophe JAILLET
---
drivers/dma-buf/udmabuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..fb4c4b5b3332 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -314,13 +314,13 @@ static long udmabuf_ioctl_create_list(struct
file *filp, unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
+ size_t lsize;
if (copy_from_user(&head, (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(&head, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
- list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
+ list = memdup_user((void __user *)(arg + sizeof(head)),
+ size_mul(sizeof(*list), head.count));
if (IS_ERR(list))
return PTR_ERR(list);
--
Gustavo
[PATCH] udmabuf: Fix a potential (and unlikely) access to unallocated memory
If 'list_limit' is set to a very high value, 'lsize' computation could
overflow if 'head.count' is big enough.
In such a case, udmabuf_create() will access to memory beyond 'list'.
Use size_mul() to saturate the value, and have memdup_user() fail.
Fixes: fbb0de795078 ("Add udmabuf misc device")
Signed-off-by: Christophe JAILLET
---
drivers/dma-buf/udmabuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index c40645999648..fb4c4b5b3332 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -314,13 +314,13 @@ static long udmabuf_ioctl_create_list(struct file *filp,
unsigned long arg)
struct udmabuf_create_list head;
struct udmabuf_create_item *list;
int ret = -EINVAL;
- u32 lsize;
+ size_t lsize;
if (copy_from_user(&head, (void __user *)arg, sizeof(head)))
return -EFAULT;
if (head.count > list_limit)
return -EINVAL;
- lsize = sizeof(struct udmabuf_create_item) * head.count;
+ lsize = size_mul(sizeof(struct udmabuf_create_item), head.count);
list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
if (IS_ERR(list))
return PTR_ERR(list);
--
2.34.1
Re: [PATCH] drm/simpledrm: Add support for multiple "power-domains"
Le 10/09/2023 à 18:39, Janne Grunau via B4 Relay a écrit :
From: Janne Grunau
Multiple power domains need to be handled explicitly in each driver. The
driver core can not handle it automatically since it is not aware of
power sequencing requirements the hardware might have. This is not a
problem for simpledrm since everything is expected to be powered on by
the bootloader. simpledrm has just ensure it remains powered on during
its lifetime.
This is required on Apple silicon M2 and M2 Pro/Max/Ultra desktop
systems. The HDMI output initialized by the bootloader requires keeping
the display controller and a DP phy power domain on.
Signed-off-by: Janne Grunau
---
drivers/gpu/drm/tiny/simpledrm.c | 106 +++
1 file changed, 106 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index ff86ba1ae1b8..efedede57d42 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
@@ -227,6 +228,12 @@ struct simpledrm_device {
unsigned int regulator_count;
struct regulator **regulators;
#endif
+ /* power-domains */
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+ int pwr_dom_count;
+ struct device **pwr_dom_devs;
+ struct device_link **pwr_dom_links;
+#endif
/* simplefb settings */
struct drm_display_mode mode;
@@ -468,6 +475,102 @@ static int simpledrm_device_init_regulators(struct
simpledrm_device *sdev)
}
#endif
+#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
+/*
+ * Generic power domain handling code.
+ *
+ * Here we handle the power-domains properties of our "simple-framebuffer"
+ * dt node. This is only necessary if there is more than one power-domain.
+ * A single power-domains is handled automatically by the driver core. Multiple
+ * power-domains have to be handled by drivers since the driver core can't know
+ * the correct power sequencing. Power sequencing is not an issue for simpledrm
+ * since the bootloader has put the power domains already in the correct state.
+ * simpledrm has only to ensure they remain active for its lifetime.
+ *
+ * When the driver unloads, we detach from the power-domains.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up the "simple-framebuffer" dt node, and the PM domain providers in the
+ * device tree. Chances are that there are no adverse effects, and if there
are,
+ * a clean teardown of the fb probe will not help us much either. So just
+ * complain and carry on, and hope that the user actually gets a working fb at
+ * the end of things.
+ */
+static void simpledrm_device_detach_genpd(void *res)
+{
+ int i;
+ struct simpledrm_device *sdev = /*(struct simpledrm_device *)*/res;
+
+
+ drm_err(&sdev->dev, "% power-domains count:%d\n", __func__,
sdev->pwr_dom_count);
+ if (sdev->pwr_dom_count <= 1)
+ return;
+
+ for (i = sdev->pwr_dom_count - 1; i >= 0; i--) {
+ if (!sdev->pwr_dom_links[i])
+ device_link_del(sdev->pwr_dom_links[i]);
+ if (!IS_ERR_OR_NULL(sdev->pwr_dom_devs[i]))
+ dev_pm_domain_detach(sdev->pwr_dom_devs[i], true);
+ }
+}
+
+static int simpledrm_device_attach_genpd(struct simpledrm_device *sdev)
+{
+ struct device *dev = sdev->dev.dev;
+ int i;
+
+ sdev->pwr_dom_count = of_count_phandle_with_args(dev->of_node,
"power-domains",
+"#power-domain-cells");
+ /*
+* Single power-domain devices are handled by driver core nothing to do
+* here. The same for device nodes without "power-domains" property.
+*/
+ if (sdev->pwr_dom_count <= 1)
+ return 0;
+
+ sdev->pwr_dom_devs = devm_kcalloc(dev, sdev->pwr_dom_count,
+ sizeof(*sdev->pwr_dom_devs),
+ GFP_KERNEL);
+ if (!sdev->pwr_dom_devs)
+ return -ENOMEM;
+
+ sdev->pwr_dom_links = devm_kcalloc(dev, sdev->pwr_dom_count,
+ sizeof(*sdev->pwr_dom_links),
+ GFP_KERNEL);
+ if (!sdev->pwr_dom_links)
+ return -ENOMEM;
+
+ for (i = 0; i < sdev->pwr_dom_count; i++) {
+ sdev->pwr_dom_devs[i] = dev_pm_domain_attach_by_id(dev, i);
+ if (IS_ERR(sdev->pwr_dom_devs[i])) {
+ int ret = PTR_ERR(sdev->pwr_dom_devs[i]);
+ if (ret == -EPROBE_DEFER) {
+ simpledrm_device_detach_genpd(sdev);
+ return PTR_ERR(sdev->pwr_dom_devs[i]);
+
[PATCH] accel/habanalabs/gaudi2: Fix incorrect string length computation in gaudi2_psoc_razwi_get_engines()
snprintf() returns the "number of characters which *would* be generated for
the given input", not the size *really* generated.
In order to avoid too large values for 'str_size' (and potential negative
values for "PSOC_RAZWI_ENG_STR_SIZE - str_size") use scnprintf()
instead of snprintf().
Fixes: c0e6df916050 ("accel/habanalabs: fix address decode RAZWI handling")
Signed-off-by: Christophe JAILLET
---
drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c
b/drivers/accel/habanalabs/gaudi2/gaudi2.c
index d94acec63d95..9617c062b7ca 100644
--- a/drivers/accel/habanalabs/gaudi2/gaudi2.c
+++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c
@@ -8277,11 +8277,11 @@ static int gaudi2_psoc_razwi_get_engines(struct
gaudi2_razwi_info *razwi_info, u
eng_id[num_of_eng] = razwi_info[i].eng_id;
base[num_of_eng] = razwi_info[i].rtr_ctrl;
if (!num_of_eng)
- str_size += snprintf(eng_name + str_size,
+ str_size += scnprintf(eng_name + str_size,
PSOC_RAZWI_ENG_STR_SIZE -
str_size, "%s",
razwi_info[i].eng_name);
else
- str_size += snprintf(eng_name + str_size,
+ str_size += scnprintf(eng_name + str_size,
PSOC_RAZWI_ENG_STR_SIZE -
str_size, " or %s",
razwi_info[i].eng_name);
num_of_eng++;
--
2.34.1
[PATCH] drm/rockchip: cdn-dp: Fix some error handling paths in cdn_dp_probe()
cdn_dp_audio_codec_init() can fail. So add some error handling.
If component_add() fails, the previous cdn_dp_audio_codec_init() call
should be undone, as already done in the remove function.
Fixes: 88582f564692 ("drm/rockchip: cdn-dp: Don't unregister audio dev when
unbinding")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/rockchip/cdn-dp-core.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c
b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index a29fbafce393..3793863c210e 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1177,6 +1177,7 @@ static int cdn_dp_probe(struct platform_device *pdev)
struct cdn_dp_device *dp;
struct extcon_dev *extcon;
struct phy *phy;
+ int ret;
int i;
dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
@@ -1217,9 +1218,19 @@ static int cdn_dp_probe(struct platform_device *pdev)
mutex_init(&dp->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, &cdn_dp_component_ops);
+ if (ret)
+ goto err_audio_deinit;
- return component_add(dev, &cdn_dp_component_ops);
+ return 0;
+
+err_audio_deinit:
+ platform_device_unregister(dp->audio_pdev);
+ return ret;
}
static void cdn_dp_remove(struct platform_device *pdev)
--
2.34.1
[PATCH 6/6] drm/tegra: output: Fix missing i2c_put_adapter() in the error handling paths of tegra_output_probe()
If an error occurs after a successful of_get_i2c_adapter_by_node() call, it
should be undone by a corresponding i2c_put_adapter().
Add the missing i2c_put_adapter() call.
Fixes: 9be7d864cf07 ("drm/tegra: Implement panel support")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/output.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index dc2dcb5ca1c8..d7d2389ac2f5 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -142,8 +142,10 @@ int tegra_output_probe(struct tegra_output *output)
GPIOD_IN,
"HDMI hotplug detect");
if (IS_ERR(output->hpd_gpio)) {
- if (PTR_ERR(output->hpd_gpio) != -ENOENT)
- return PTR_ERR(output->hpd_gpio);
+ if (PTR_ERR(output->hpd_gpio) != -ENOENT) {
+ err = PTR_ERR(output->hpd_gpio);
+ goto put_i2c;
+ }
output->hpd_gpio = NULL;
}
@@ -152,7 +154,7 @@ int tegra_output_probe(struct tegra_output *output)
err = gpiod_to_irq(output->hpd_gpio);
if (err < 0) {
dev_err(output->dev, "gpiod_to_irq(): %d\n", err);
- return err;
+ goto put_i2c;
}
output->hpd_irq = err;
@@ -165,7 +167,7 @@ int tegra_output_probe(struct tegra_output *output)
if (err < 0) {
dev_err(output->dev, "failed to request IRQ#%u: %d\n",
output->hpd_irq, err);
- return err;
+ goto put_i2c;
}
output->connector.polled = DRM_CONNECTOR_POLL_HPD;
@@ -179,6 +181,12 @@ int tegra_output_probe(struct tegra_output *output)
}
return 0;
+
+put_i2c:
+ if (output->ddc)
+ i2c_put_adapter(output->ddc);
+
+ return err;
}
void tegra_output_remove(struct tegra_output *output)
--
2.34.1
[PATCH 5/6] drm/tegra: rgb: Fix missing clk_put() in the error handling paths of tegra_dc_rgb_probe()
If clk_get_sys(..., "pll_d2_out0") fails, the clk_get_sys() call must be
undone.
Add the missing clk_put and a new 'put_pll_d_out0' label in the error
handling path, and use it.
Fixes: 0c921b6d4ba0 ("drm/tegra: dc: rgb: Allow changing PLLD rate on Tegra30+")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/rgb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 26d4d87b83de..e277826ab494 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -244,7 +244,7 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
if (IS_ERR(rgb->pll_d2_out0)) {
err = PTR_ERR(rgb->pll_d2_out0);
dev_err(dc->dev, "failed to get pll_d2_out0: %d\n",
err);
- goto tegra_remove;
+ goto put_pll_d_out0;
}
}
@@ -252,6 +252,8 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
return 0;
+put_pll_d_out0:
+ clk_put(rgb->pll_d_out0);
tegra_remove:
tegra_output_remove(&rgb->output);
return err;
--
2.34.1
[PATCH 4/6] drm/tegra: rgb: Fix some error handling paths in tegra_dc_rgb_probe()
If an error occurs after calling tegra_output_probe(),
tegra_output_remove() should be called as already done in the remove
function.
Fixes: 59d29c0ec93f ("drm/tegra: Allocate resources at probe time")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/rgb.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 79566c9ea8ff..26d4d87b83de 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -215,26 +215,28 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
rgb->clk = devm_clk_get(dc->dev, NULL);
if (IS_ERR(rgb->clk)) {
dev_err(dc->dev, "failed to get clock\n");
- return PTR_ERR(rgb->clk);
+ err = PTR_ERR(rgb->clk);
+ goto tegra_remove;
}
rgb->clk_parent = devm_clk_get(dc->dev, "parent");
if (IS_ERR(rgb->clk_parent)) {
dev_err(dc->dev, "failed to get parent clock\n");
- return PTR_ERR(rgb->clk_parent);
+ err = PTR_ERR(rgb->clk_parent);
+ goto tegra_remove;
}
err = clk_set_parent(rgb->clk, rgb->clk_parent);
if (err < 0) {
dev_err(dc->dev, "failed to set parent clock: %d\n", err);
- return err;
+ goto tegra_remove;
}
rgb->pll_d_out0 = clk_get_sys(NULL, "pll_d_out0");
if (IS_ERR(rgb->pll_d_out0)) {
err = PTR_ERR(rgb->pll_d_out0);
dev_err(dc->dev, "failed to get pll_d_out0: %d\n", err);
- return err;
+ goto tegra_remove;
}
if (dc->soc->has_pll_d2_out0) {
@@ -242,13 +244,17 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
if (IS_ERR(rgb->pll_d2_out0)) {
err = PTR_ERR(rgb->pll_d2_out0);
dev_err(dc->dev, "failed to get pll_d2_out0: %d\n",
err);
- return err;
+ goto tegra_remove;
}
}
dc->rgb = &rgb->output;
return 0;
+
+tegra_remove:
+ tegra_output_remove(&rgb->output);
+ return err;
}
void tegra_dc_rgb_remove(struct tegra_dc *dc)
--
2.34.1
[PATCH 3/6] drm/tegra: hdmi: Fix some error handling paths in tegra_hdmi_probe()
If an error occurs after calling tegra_output_probe(),
tegra_output_remove() should be called as already done in the remove
function.
Fixes: 59d29c0ec93f ("drm/tegra: Allocate resources at probe time")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/hdmi.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 80c760986d9e..c73e7fb1877e 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1854,12 +1854,14 @@ static int tegra_hdmi_probe(struct platform_device
*pdev)
return err;
hdmi->regs = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(hdmi->regs))
- return PTR_ERR(hdmi->regs);
+ if (IS_ERR(hdmi->regs)) {
+ err = PTR_ERR(hdmi->regs);
+ goto tegra_remove;
+ }
err = platform_get_irq(pdev, 0);
if (err < 0)
- return err;
+ goto tegra_remove;
hdmi->irq = err;
@@ -1868,18 +1870,18 @@ static int tegra_hdmi_probe(struct platform_device
*pdev)
if (err < 0) {
dev_err(&pdev->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(&pdev->dev);
if (err)
- return err;
+ goto tegra_remove;
err = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
if (err)
- return err;
+ goto tegra_remove;
INIT_LIST_HEAD(&hdmi->client.list);
hdmi->client.ops = &hdmi_client_ops;
@@ -1889,10 +1891,14 @@ static int tegra_hdmi_probe(struct platform_device
*pdev)
if (err < 0) {
dev_err(&pdev->dev, "failed to register host1x client: %d\n",
err);
- return err;
+ goto tegra_remove;
}
return 0;
+
+tegra_remove:
+ tegra_output_remove(&hdmi->output);
+ return err;
}
static void tegra_hdmi_remove(struct platform_device *pdev)
--
2.34.1
[PATCH 2/6] drm/tegra: dsi: Fix missing pm_runtime_disable() in the error handling path of tegra_dsi_probe()
If an error occurs after calling pm_runtime_enable(), pm_runtime_disable()
should be called as already done in the remove function.
Fixes: ef8187d75265 ("drm/tegra: dsi: Implement runtime PM")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/dsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 70a77c75bfe1..0cf379e20d89 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -1672,6 +1672,7 @@ static int tegra_dsi_probe(struct platform_device *pdev)
return 0;
unregister:
+ pm_runtime_disable(&pdev->dev);
mipi_dsi_host_unregister(&dsi->host);
mipi_free:
tegra_mipi_free(dsi->mipi);
--
2.34.1
[PATCH 0/6] drm/tegra: Fix some error handling paths
Most of the patches are retated to tegra_output_probe() and missing tegra_output_remove(). Others are things spotted while writting the serie. Patches 1, 3, 4 are verbose, but some functions called in the probe can return -EPROBE_DEFER, so it is nice to correctly release resources. Maybe moving the tegra_output_probe() call would minimize the changes, but I'm always reluctant to move code, because of possible side-effects. Christophe JAILLET (6): drm/tegra: dsi: Fix some error handling paths in tegra_dsi_probe() drm/tegra: dsi: Fix missing pm_runtime_disable() in the error handling path of tegra_dsi_probe() drm/tegra: dsi: Fix some error handling paths in tegra_hdmi_probe() drm/tegra: rgb: Fix some error handling paths in tegra_dc_rgb_probe() drm/tegra: rgb: Fix missing clk_put() in the error handling paths of tegra_dc_rgb_probe() drm/tegra: output: Fix missing i2c_put_adapter() in the error handling paths of tegra_output_probe() drivers/gpu/drm/tegra/dsi.c| 55 ++ drivers/gpu/drm/tegra/hdmi.c | 20 - drivers/gpu/drm/tegra/output.c | 16 +++--- drivers/gpu/drm/tegra/rgb.c| 18 +++ 4 files changed, 74 insertions(+), 35 deletions(-) -- 2.34.1
[PATCH 1/6] drm/tegra: dsi: Fix some error handling paths in tegra_dsi_probe()
If an error occurs after calling tegra_output_probe(),
tegra_output_remove() should be called as already done in the remove
function.
Fixes: dec727399a4b ("drm/tegra: Add DSI support")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/tegra/dsi.c | 54 -
1 file changed, 35 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index a9870c828374..70a77c75bfe1 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -1593,44 +1593,58 @@ static int tegra_dsi_probe(struct platform_device *pdev)
if (!pdev->dev.pm_domain) {
dsi->rst = devm_reset_control_get(&pdev->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(&pdev->dev, NULL);
- if (IS_ERR(dsi->clk))
- return dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk),
-"cannot get DSI clock\n");
+ if (IS_ERR(dsi->clk)) {
+ err = dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk),
+ "cannot get DSI clock\n");
+ goto tegra_remove;
+ }
dsi->clk_lp = devm_clk_get(&pdev->dev, "lp");
- if (IS_ERR(dsi->clk_lp))
- return dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk_lp),
-"cannot get low-power clock\n");
+ if (IS_ERR(dsi->clk_lp)) {
+ err = dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk_lp),
+ "cannot get low-power clock\n");
+ goto tegra_remove;
+ }
dsi->clk_parent = devm_clk_get(&pdev->dev, "parent");
- if (IS_ERR(dsi->clk_parent))
- return dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk_parent),
-"cannot get parent clock\n");
+ if (IS_ERR(dsi->clk_parent)) {
+ err = dev_err_probe(&pdev->dev, PTR_ERR(dsi->clk_parent),
+ "cannot get parent clock\n");
+ goto tegra_remove;
+ }
dsi->vdd = devm_regulator_get(&pdev->dev, "avdd-dsi-csi");
- if (IS_ERR(dsi->vdd))
- return dev_err_probe(&pdev->dev, PTR_ERR(dsi->vdd),
-"cannot get VDD supply\n");
+ if (IS_ERR(dsi->vdd)) {
+ err = dev_err_probe(&pdev->dev, PTR_ERR(dsi->vdd),
+ "cannot get VDD supply\n");
+ goto tegra_remove;
+ }
err = tegra_dsi_setup_clocks(dsi);
if (err < 0) {
dev_err(&pdev->dev, "cannot setup clocks\n");
- return err;
+ goto tegra_remove;
}
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
dsi->regs = devm_ioremap_resource(&pdev->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(&pdev->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 = &tegra_dsi_host_ops;
dsi->host.dev = &pdev->dev;
@@ -1661,6 +1675,8 @@ static int tegra_dsi_probe(struct platform_device *pdev)
mipi_dsi_host_unregister(&dsi->host);
mipi_free:
tegra_mipi_free(dsi->mipi);
+tegra_remove:
+ tegra_output_remove(&dsi->output);
return err;
}
--
2.34.1
Re: [PATCH] drm/ast: Avoid possible NULL dereference
Le 21/08/2023 à 08:22, Su Hui a écrit :
smatch error:
drivers/gpu/drm/ast/ast_dp501.c:227 ast_launch_m68k() error:
we previously assumed 'ast->dp501_fw' could be null (see line 223)
when "!ast->dp501_fw" and "ast_load_dp501_microcode(dev) >= 0" is true,
there will be a NULL dereference of 'ast->dp501_fw'.
Fixes: 12f8030e05c6 ("drm/ast: Actually load DP501 firmware when required")
Signed-off-by: Su Hui
---
drivers/gpu/drm/ast/ast_dp501.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 1bc35a992369..d9f3a0786a6f 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -224,8 +224,10 @@ static bool ast_launch_m68k(struct drm_device *dev)
ast_load_dp501_microcode(dev) < 0)
return false;
- fw_addr = (u8 *)ast->dp501_fw->data;
- len = ast->dp501_fw->size;
+ if (ast->dp501_fw) {
+ fw_addr = (u8 *)ast->dp501_fw->data;
+ len = ast->dp501_fw->size;
+ }
}
/* Get BootAddress */
ast_moutdwm(ast, 0x1e6e2000, 0x1688a8a8);
Hi,
this looks like a false positive.
If "!ast->dp501_fw", and ast_load_dp501_microcode()>=0, then
ast_load_dp501_microcode() will set a valid value in ast->dp501_fw.
CJ
Re: [PATCH] drm/amdgpu: Avoid possible buffer overflow
Le 21/08/2023 à 08:19, Su Hui a écrit :
smatch error:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1257
amdgpu_discovery_reg_base_init() error:
testing array offset 'adev->vcn.num_vcn_inst' after use.
change the assignment order to avoid buffer overflow.
Fixes: c40bdfb2ffa4 ("drm/amdgpu: fix incorrect VCN revision in SRIOV")
Signed-off-by: Su Hui
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 8e1cfc87122d..ba95526c3d45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1250,11 +1250,12 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
* 0b10 : encode is disabled
* 0b01 : decode is disabled
*/
- adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
- ip->revision & 0xc0;
+
ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+
adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+ ip->revision & 0xc0;
Hi,
I don't think that the patch is correct.
Because of the "ip->revision &= ~0xc0" which is now above it,
"ip->revision & 0xc0" should now always be 0.
Maybe both lines should be moved within the "if"?
CJ
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver
Le 20/08/2023 à 11:41, Julius Zint a écrit :
The HID spec defines the following Usage IDs (p. 345 ff):
- Monitor Page (0x80) -> Monitor Control (0x01)
- VESA Virtual Controls Page (0x82) -> Brightness (0x10)
Apple made use of them in their Apple Studio Display and most likely on
other external displays (LG UltraFine 5k, Pro Display XDR).
The driver will work for any HID device with a report, where the
application matches the Monitor Control Usage ID and:
1. An Input field in this report with the Brightness Usage ID (to get
the current brightness)
2. A Feature field in this report with the Brightness Usage ID (to
set the current brightness)
This driver has been developed and tested with the Apple Studio Display.
Here is a small excerpt from the decoded HID descriptor showing the
feature field for setting the brightness:
Usage Page (Monitor VESA VCP), ; Monitor VESA VPC (82h, monitor page)
Usage (10h, Brightness),
Logical Minimum (400),
Logical Maximum (6),
Unit (Centimeter^-2 * Candela),
Unit Exponent (14),
Report Size (32),
Report Count (1),
Feature (Variable, Null State),
The full HID descriptor dump is available as a comment in the source
code.
Signed-off-by: Julius Zint
---
[...]
+static void hid_bl_remove(struct hid_device *hdev)
+{
+ struct backlight_device *bl;
+ struct hid_bl_data *data;
+
+ hid_dbg(hdev, "remove\n");
+ bl = hid_get_drvdata(hdev);
+ data = bl_get_data(bl);
+
+ devm_backlight_device_unregister(&hdev->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(&hdev->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(&hdev->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(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = data->max_brightness - data->min_brightness;
+
+ bl = devm_backlight_device_register(&hdev->dev, "vesa_vcp",
+ &hdev->dev, data,
+
[PATCH 4/4] drm/amdgpu: Use kvzalloc() to simplify code
kvzalloc() can be used instead of kvmalloc() + memset() + explicit NULL
assignments.
It is less verbose and more future proof.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 6ea9ff22c281..6f5b641b631e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -78,17 +78,13 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev,
struct drm_file *filp,
unsigned i;
int r;
- list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL);
+ list = kvzalloc(struct_size(list, entries, num_entries), GFP_KERNEL);
if (!list)
return -ENOMEM;
kref_init(&list->refcount);
- list->gds_obj = NULL;
- list->gws_obj = NULL;
- list->oa_obj = NULL;
array = list->entries;
- memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry));
for (i = 0; i < num_entries; ++i) {
struct amdgpu_bo_list_entry *entry;
--
2.34.1
[PATCH 2/4] drm/amdgpu: Remove a redundant sanity check
The case where 'num_entries' is too big, is already handled by struct_size(), because kvmalloc() would fail. It will return -ENOMEM instead of -EINVAL, but it is only related to a unlikely to happen sanity check. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c index 571fed04eb7a..c8f59a044286 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c @@ -78,10 +78,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file *filp, unsigned i; int r; - if (num_entries > (SIZE_MAX - sizeof(struct amdgpu_bo_list)) - / sizeof(struct amdgpu_bo_list_entry)) - return -EINVAL; - list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL); if (!list) return -ENOMEM; -- 2.34.1
[PATCH 3/4] drm/amdgpu: Remove amdgpu_bo_list_array_entry()
Now that there is an explicit flexible array at the end of 'struct
amdgpu_bo_list', it can be used to remove amdgpu_bo_list_array_entry() and
simplify some macro.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 16
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index c8f59a044286..6ea9ff22c281 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -87,7 +87,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct
drm_file *filp,
list->gws_obj = NULL;
list->oa_obj = NULL;
- array = amdgpu_bo_list_array_entry(list, 0);
+ array = list->entries;
memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry));
for (i = 0; i < num_entries; ++i) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
index 368e50b30160..6a703be45d04 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
@@ -71,22 +71,14 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev,
size_t num_entries,
struct amdgpu_bo_list **list);
-static inline struct amdgpu_bo_list_entry *
-amdgpu_bo_list_array_entry(struct amdgpu_bo_list *list, unsigned index)
-{
- struct amdgpu_bo_list_entry *array = (void *)&list[1];
-
- return &array[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 != &list->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 = &list->entries[list->first_userptr]; \
+e != &list->entries[list->num_entries]; \
++e)
#endif
--
2.34.1
[PATCH 1/4] drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list'
'struct amdgpu_bo_list' is really used as if it was ended by a flex array.
So make it more explicit and add a 'struct amdgpu_bo_list_entry entries[]'
field at the end of the structure.
This way, struct_size() can be used when it is allocated.
It is less verbose.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 2 ++
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index b6298e901cbd..571fed04eb7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -75,7 +75,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct
drm_file *filp,
struct amdgpu_bo_list_entry *array;
struct amdgpu_bo_list *list;
uint64_t total_size = 0;
- size_t size;
unsigned i;
int r;
@@ -83,9 +82,7 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, struct
drm_file *filp,
/ sizeof(struct amdgpu_bo_list_entry))
return -EINVAL;
- size = sizeof(struct amdgpu_bo_list);
- size += num_entries * sizeof(struct amdgpu_bo_list_entry);
- list = kvmalloc(size, GFP_KERNEL);
+ list = kvmalloc(struct_size(list, entries, num_entries), GFP_KERNEL);
if (!list)
return -ENOMEM;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
index 26c01cb131f2..368e50b30160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
@@ -55,6 +55,8 @@ struct amdgpu_bo_list {
/* Protect access during command submission.
*/
struct mutex bo_list_mutex;
+
+ struct amdgpu_bo_list_entry entries[];
};
int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id,
--
2.34.1
[PATCH 0/4] drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list' and simplify amdgpu_bo_list_create()
This serie simplifies amdgpu_bo_list_create() and usage of the 'struct amdgpu_bo_list'. It is compile tested only. Christophe JAILLET (4): drm/amdgpu: Explicitly add a flexible array at the end of 'struct amdgpu_bo_list' drm/amdgpu: Remove a redundant sanity check drm/amdgpu: Remove amdgpu_bo_list_array_entry() drm/amdgpu: Use kvzalloc() to simplify code drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 15 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 18 ++ 2 files changed, 8 insertions(+), 25 deletions(-) -- 2.34.1
Re: [PATCH 1/1] gpu: drm: aspeed: fix value check in aspeed_gfx_load()
Le 27/07/2023 à 19:03, Yuanjun Gong a écrit :
in aspeed_gfx_load(), check the return value of clk_prepare_enable()
and return the error code if clk_prepare_enable() returns an
unexpected value.
Fixes: 4f2a8f5898ec ("drm: Add ASPEED GFX driver")
Signed-off-by: Yuanjun Gong
---
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index c8c7f8215155..3bfa39bc4f7e 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -199,7 +199,11 @@ static int aspeed_gfx_load(struct drm_device *drm)
"missing or invalid clk device tree entry");
return PTR_ERR(priv->clk);
}
- clk_prepare_enable(priv->clk);
+ ret = clk_prepare_enable(priv->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to enable clock\n");
+ return ret;
+ }
/* Sanitize control registers */
writel(0, priv->base + CRT_CTRL1);
Hi,
the code also lacks a clk_disable_unprepare() in aspeed_gfx_unload().
So using devm_clk_get_enabled() a few lines above should fix both issue.
CJ
[PATCH] drm/msm: Slightly simplify memory allocation in submit_lookup_cmds()
If 'sz' is SIZE_MAX, kmalloc() will fail. So there is no need to explicitly
check for an hypothetical overflow.
Remove the check to save a few lines of code.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/msm/msm_gem_submit.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 3f1aa4de3b87..6ca8f8cbb6e2 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -211,11 +211,7 @@ static int submit_lookup_cmds(struct msm_gem_submit
*submit,
sz = array_size(submit_cmd.nr_relocs,
sizeof(struct drm_msm_gem_submit_reloc));
- /* check for overflow: */
- if (sz == SIZE_MAX) {
- ret = -ENOMEM;
- goto out;
- }
+
submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL);
if (!submit->cmd[i].relocs) {
ret = -ENOMEM;
--
2.34.1
[PATCH] drm/i915: Fix an error handling path in igt_write_huge()
All error handling paths go to 'out', except this one. Be consistent and
also branch to 'out' here.
Fixes: c10a652e239e ("drm/i915/selftests: Rework context handling in hugepages
selftests")
Signed-off-by: Christophe JAILLET
---
/!\ Speculative /!\
This patch is based on analysis of the surrounding code and should be
reviewed with care !
If the patch is wrong, maybe a comment in the code could explain why.
/!\ Speculative /!\
---
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index df6c9a84252c..6b9f6cf50bf6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1246,8 +1246,10 @@ static int igt_write_huge(struct drm_i915_private *i915,
* times in succession a possibility by enlarging the permutation array.
*/
order = i915_random_order(count * count, &prng);
- if (!order)
- return -ENOMEM;
+ if (!order) {
+ err = -ENOMEM;
+ goto out;
+ }
max_page_size = rounddown_pow_of_two(obj->mm.page_sizes.sg);
max = div_u64(max - size, max_page_size);
--
2.34.1
[PATCH v3] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.
This simplifies the code and avoids the need of a dedicated function used
with devm_add_action_or_reset().
Signed-off-by: Christophe JAILLET
Reviewed-by: Andrzej Hajda
---
Change in v3:
- Rebase with latest -next
Change in v2:
- Convert to dev_err_probe()[Andrzej Hajda]
- Update the error message[Andrzej Hajda]
- Add R-b tag[Andrzej Hajda]
https://lore.kernel.org/all/208546ce4e01973da1eb9ad7bc0f9241f650b3af.1672415956.git.christophe.jail...@wanadoo.fr/
v1:
https://lore.kernel.org/all/4f855984ea895e1488169e77935fa6e044912ac2.1672414073.git.christophe.jail...@wanadoo.fr/
---
drivers/gpu/drm/bridge/tc358767.c | 25 -
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 2a58eb271f70..99f3d5ca7257 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2215,13 +2215,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc)
return -EINVAL;
}
-static void tc_clk_disable(void *data)
-{
- struct clk *refclk = data;
-
- clk_disable_unprepare(refclk);
-}
-
static int tc_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -2238,20 +2231,10 @@ static int tc_probe(struct i2c_client *client, const
struct i2c_device_id *id)
if (ret)
return ret;
- tc->refclk = devm_clk_get(dev, "ref");
- if (IS_ERR(tc->refclk)) {
- ret = PTR_ERR(tc->refclk);
- dev_err(dev, "Failed to get refclk: %d\n", ret);
- return ret;
- }
-
- ret = clk_prepare_enable(tc->refclk);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk);
- if (ret)
- return ret;
+ tc->refclk = devm_clk_get_enabled(dev, "ref");
+ if (IS_ERR(tc->refclk))
+ return dev_err_probe(dev, PTR_ERR(tc->refclk),
+"Failed to get and enable the ref clk\n");
/* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */
usleep_range(10, 15);
--
2.34.1
[PATCH] video/hdmi: Reorder fields in 'struct hdmi_avi_infoframe'
Group some variables based on their sizes to reduce hole and avoid padding.
On x86_64, this shrinks the size of 'struct hdmi_avi_infoframe'
from 68 to 60 bytes.
It saves a few bytes of memory and is more cache-line friendly.
This also reduces the union hdmi_infoframe the same way.
Signed-off-by: Christophe JAILLET
---
Using pahole
Before:
==
struct hdmi_avi_infoframe {
enum hdmi_infoframe_type type; /* 0 4 */
unsigned char version; /* 4 1 */
unsigned char length; /* 5 1 */
/* XXX 2 bytes hole, try to pack */
enum hdmi_colorspace colorspace; /* 8 4 */
enum hdmi_scan_modescan_mode;/*12 4 */
enum hdmi_colorimetry colorimetry; /*16 4 */
enum hdmi_picture_aspect picture_aspect; /*20 4 */
enum hdmi_active_aspectactive_aspect;/*24 4 */
bool itc; /*28 1 */
/* XXX 3 bytes hole, try to pack */
enum hdmi_extended_colorimetry extended_colorimetry; /*32 4 */
enum hdmi_quantization_range quantization_range; /*36 4 */
enum hdmi_nups nups; /*40 4 */
unsigned char video_code; /*44 1 */
/* XXX 3 bytes hole, try to pack */
enum hdmi_ycc_quantization_range ycc_quantization_range; /*48 4
*/
enum hdmi_content_type content_type; /*52 4 */
unsigned char pixel_repeat; /*56 1 */
/* XXX 1 byte hole, try to pack */
short unsigned int top_bar; /*58 2 */
short unsigned int bottom_bar; /*60 2 */
short unsigned int left_bar; /*62 2 */
/* --- cacheline 1 boundary (64 bytes) --- */
short unsigned int right_bar;/*64 2 */
/* size: 68, cachelines: 2, members: 20 */
/* sum members: 57, holes: 4, sum holes: 9 */
/* padding: 2 */
/* last cacheline: 4 bytes */
};
After:
=
struct hdmi_avi_infoframe {
enum hdmi_infoframe_type type; /* 0 4 */
unsigned char version; /* 4 1 */
unsigned char length; /* 5 1 */
bool itc; /* 6 1 */
unsigned char pixel_repeat; /* 7 1 */
enum hdmi_colorspace colorspace; /* 8 4 */
enum hdmi_scan_modescan_mode;/*12 4 */
enum hdmi_colorimetry colorimetry; /*16 4 */
enum hdmi_picture_aspect picture_aspect; /*20 4 */
enum hdmi_active_aspectactive_aspect;/*24 4 */
enum hdmi_extended_colorimetry extended_colorimetry; /*28 4 */
enum hdmi_quantization_range quantization_range; /*32 4 */
enum hdmi_nups nups; /*36 4 */
unsigned char video_code; /*40 1 */
/* XXX 3 bytes hole, try to pack */
enum hdmi_ycc_quantization_range ycc_quantization_range; /*44 4
*/
enum hdmi_content_type content_type; /*48 4 */
short unsigned int top_bar; /*52 2 */
short unsigned int bottom_bar; /*54 2 */
short unsigned int left_bar; /*56 2 */
short unsigned int right_bar;/*58 2 */
/* size: 60, cachelines: 1, members: 20 */
/* sum members: 57, holes: 1, sum holes: 3 */
/* last cacheline: 60 bytes */
};
---
include/linux/hdmi.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index 2f4dcc8d060e..3bb87bf6bc65 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -170,19 +170,19 @@ struct hdmi_avi_infoframe {
enum hdmi_infoframe_type type;
unsigned char version;
unsigned char length;
+ bool itc;
+ unsigned char pixel_repeat;
enum hdmi_colorspace colorspace;
enum hdmi_scan_mode scan_mode;
enum hdmi_colorimetry colorimetry;
enum hdmi_picture_aspect picture_aspect;
enum hdmi_active_aspect active_aspect;
- bool itc;
enum hdmi_extended_colorimetry extended_colorimetry;
enum hdmi_quantization_range quantization_range;
enum hdmi_nups nups;
unsigned char video_code;
enum hdmi_ycc_quantization_range ycc_quantization_range;
Re: [PATCH v4 1/4] Input: ads7846 - Convert to use software nodes
Le 08/05/2023 à 23:20, Linus Walleij a écrit :
The Nokia 770 is using GPIOs from the global numberspace on the
CBUS node to pass down to the LCD controller. This regresses when we
let the OMAP GPIO driver use dynamic GPIO base.
The Nokia 770 now has dynamic allocation of IRQ numbers, so this
needs to be fixed for it to work.
As this is the only user of LCD MIPID we can easily augment the
driver to use a GPIO descriptor instead and resolve the issue.
The platform data .shutdown() callback wasn't even used in the
code, but we encode a shutdown asserting RESET in the remove()
callback for completeness sake.
The CBUS also has the ADS7846 touchscreen attached.
Populate the devices on the Nokia 770 CBUS I2C using software
nodes instead of platform data quirks. This includes the LCD
and the ADS7846 touchscreen so the conversion just brings the LCD
along with it as software nodes is an all-or-nothing design
pattern.
The ADS7846 has some limited support for using GPIO descriptors,
let's convert it over completely to using device properties and then
fix all remaining boardfile users to provide all platform data using
software nodes.
Dump the of includes and of_match_ptr() in the ADS7846 driver as part
of the job.
Since we have to move ADS7846 over to obtaining the GPIOs it is
using exclusively from descriptors, we provide descriptor tables
for the two remaining in-kernel boardfiles using ADS7846:
- PXA Spitz
- MIPS Alchemy DB1000 development board
It was too hard for me to include software node conversion of
these two remaining users at this time: the spitz is using a
hscync callback in the platform data that would require further
GPIO descriptor conversion of the Spitz, and moving the hsync
callback down into the driver: it will just become too big of
a job, but it can be done separately.
The MIPS Alchemy DB1000 is simply something I cannot test, so take
the easier approach of just providing some GPIO descriptors in
this case as I don't want the patch to grow too intrusive.
As we see that several device trees have incorrect polarity flags
and just expect to bypass the gpiolib polarity handling, fix up
all device trees too, in a separate patch.
Suggested-by: Dmitry Torokhov
Fixes: 92bf78b33b0b ("gpio: omap: use dynamic allocation of base")
Signed-off-by: Linus Walleij
---
[...]
diff --git a/drivers/video/fbdev/omap/lcd_mipid.c
b/drivers/video/fbdev/omap/lcd_mipid.c
index 03cff39d392d..e4a7f0b824ff 100644
--- a/drivers/video/fbdev/omap/lcd_mipid.c
+++ b/drivers/video/fbdev/omap/lcd_mipid.c
@@ -7,6 +7,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -41,6 +42,7 @@ struct mipid_device {
when we can issue the
next sleep in/out command */
unsigned long hw_guard_wait; /* max guard time in jiffies */
+ struct gpio_desc*reset;
struct omapfb_device *fbdev;
struct spi_device *spi;
@@ -556,6 +558,12 @@ static int mipid_spi_probe(struct spi_device *spi)
return -ENOMEM;
}
+ /* This will de-assert RESET if active */
+ md->reset = gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(md->reset))
+ return dev_err_probe(&spi->dev, PTR_ERR(md->reset),
+"no reset GPIO line\n");
+
spi->mode = SPI_MODE_0;
md->spi = spi;
dev_set_drvdata(&spi->dev, md);
@@ -574,6 +582,8 @@ static void mipid_spi_remove(struct spi_device *spi)
{
struct mipid_device *md = dev_get_drvdata(&spi->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(&md->panel);
kfree(md);
}
[...]
[PATCH] video: fbdev: omapfb: lcd_mipid: Fix an error handling path in mipid_spi_probe()
If 'mipid_detect()' fails, we must free 'md' to avoid a memory leak.
Fixes: 66d2f99d0bb5 ("omapfb: add support for MIPI-DCS compatible LCDs")
Signed-off-by: Christophe JAILLET
---
drivers/video/fbdev/omap/lcd_mipid.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap/lcd_mipid.c
b/drivers/video/fbdev/omap/lcd_mipid.c
index e4a7f0b824ff..a0fc4570403b 100644
--- a/drivers/video/fbdev/omap/lcd_mipid.c
+++ b/drivers/video/fbdev/omap/lcd_mipid.c
@@ -571,11 +571,15 @@ static int mipid_spi_probe(struct spi_device *spi)
r = mipid_detect(md);
if (r < 0)
- return r;
+ goto free_md;
omapfb_register_panel(&md->panel);
return 0;
+
+free_md:
+ kfree(md);
+ return r;
}
static void mipid_spi_remove(struct spi_device *spi)
--
2.34.1
[PATCH] accel/ivpu: Use struct_size()
Use struct_size() instead of hand-writing it. It is less verbose, more
robust and more informative.
Signed-off-by: Christophe JAILLET
---
drivers/accel/ivpu/ivpu_job.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 3c6f1e16cf2f..0a09bba8da24 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -289,15 +289,13 @@ ivpu_create_job(struct ivpu_file_priv *file_priv, u32
engine_idx, u32 bo_count)
{
struct ivpu_device *vdev = file_priv->vdev;
struct ivpu_job *job;
- size_t buf_size;
int ret;
ret = ivpu_rpm_get(vdev);
if (ret < 0)
return NULL;
- buf_size = sizeof(*job) + bo_count * sizeof(struct ivpu_bo *);
- job = kzalloc(buf_size, GFP_KERNEL);
+ job = kzalloc(struct_size(job, bos, bo_count), GFP_KERNEL);
if (!job)
goto err_rpm_put;
--
2.34.1
[PATCH 3/3] drm/amd/display: Use USEC_PER_SEC
Use USEC_PER_SEC instead of defining an equivalent local 'us_in_sec'. Signed-off-by: Christophe JAILLET --- NOT compile tested. Because of some BROKEN in KConfig files. Some header may be missing for USEC_PER_SEC! --- drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c index eafe8561e55e..9b82ee3e06d0 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c @@ -678,7 +678,6 @@ static uint32_t get_dmif_switch_time_us( uint32_t pixels_per_second; uint32_t pixels_per_frame; uint32_t refresh_rate; - const uint32_t us_in_sec = 100; const uint32_t min_single_frame_time_us = 3; /*return double of frame time*/ const uint32_t single_frame_time_multiplier = 2; @@ -691,8 +690,7 @@ static uint32_t get_dmif_switch_time_us( pixels_per_frame = h_total * v_total; refresh_rate = pixels_per_second / pixels_per_frame; - - frame_time = us_in_sec / refresh_rate; + frame_time = USEC_PER_SEC / refresh_rate; if (frame_time < min_single_frame_time_us) frame_time = min_single_frame_time_us; -- 2.34.1
[PATCH 2/3] drm/amd/display: Simplify get_dmif_switch_time_us()
Thanks to the sanity check a few lines above:
if (!h_total || !v_total || !pix_clk_khz)
and the computation done afterwards on these non 0 values, we know that
'pixels_per_second', 'pixels_per_frame' and 'refresh_rate' are not 0
The code can be simplified accordingly.
Signed-off-by: Christophe JAILLET
---
NOT compile tested. Because of some BROKEN in KConfig files.
---
drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
index 091f0d68a045..eafe8561e55e 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
@@ -690,21 +690,8 @@ static uint32_t get_dmif_switch_time_us(
pixels_per_second = pix_clk_khz * 1000;
pixels_per_frame = h_total * v_total;
- if (!pixels_per_second || !pixels_per_frame) {
- /* avoid division by zero */
- ASSERT(pixels_per_frame);
- ASSERT(pixels_per_second);
- return single_frame_time_multiplier * min_single_frame_time_us;
- }
-
refresh_rate = pixels_per_second / pixels_per_frame;
- if (!refresh_rate) {
- /* avoid division by zero*/
- ASSERT(refresh_rate);
- return single_frame_time_multiplier * min_single_frame_time_us;
- }
-
frame_time = us_in_sec / refresh_rate;
if (frame_time < min_single_frame_time_us)
--
2.34.1
[PATCH 1/3] drm/amd/display: Fix an erroneous sanity check in get_dmif_switch_time_us()
It is likely that there is a typo in the sanity check for 'v_total'.
If it is 0, then 'pixels_per_frame' will also be 0, and in this case,
we also return 'single_frame_time_multiplier * min_single_frame_time_us'.
So test for !v_total which looks much more logical.
Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
index 4cdd4dacb761..091f0d68a045 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
@@ -683,7 +683,7 @@ static uint32_t get_dmif_switch_time_us(
/*return double of frame time*/
const uint32_t single_frame_time_multiplier = 2;
- if (!h_total || v_total || !pix_clk_khz)
+ if (!h_total || !v_total || !pix_clk_khz)
return single_frame_time_multiplier * min_single_frame_time_us;
/*TODO: should we use pixel format normalized pixel clock here?*/
--
2.34.1
Re: [PATCH 1/3] drm/amd/display: drop redundant memset() in get_available_dsc_slices()
Le 17/05/2023 à 20:33, Hamza Mahfooz a écrit :
get_available_dsc_slices() returns the number of indices set, and all of
the users of get_available_dsc_slices() don't cross the returned bound
when iterating over available_slices[]. So, the memset() in
get_available_dsc_slices() is redundant and can be dropped.
Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)")
Reported-by: Christophe JAILLET
Signed-off-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
index b9a05bb025db..58dd62cce4bb 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
@@ -645,8 +645,6 @@ static int get_available_dsc_slices(union
dsc_enc_slice_caps slice_caps, int *av
{
int idx = 0;
- memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE);
-
if (slice_caps.bits.NUM_SLICES_1)
available_slices[idx++] = 1;
Thanks for it, it went off my radar.
FWIW:
Reviewed-by: Christophe JAILLET
Re: [PATCH v2 net-next 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
Le 26/04/2023 à 20:54, Justin Chen a écrit :
Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.
This patch supports:
- Wake-on-LAN using magic packets
- basic ethtool operations (link, counters, message level)
- MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
Signed-off-by: Florian Fainelli
Signed-off-by: Justin Chen
---
[...]
+void bcmasp_disable_all_filters(struct bcmasp_intf *intf)
+{
+ struct bcmasp_priv *priv = intf->parent;
+ unsigned int i;
Hi,
Nit: Some loop index are unsigned int, but most are int.
This could be done consistantly.
+
+ /* Disable all filters held by this port */
+ for (i = ASP_RX_FILT_MDA_RES_COUNT(intf); i < NUM_MDA_FILTERS; i++) {
+ if (priv->mda_filters[i].en &&
+ priv->mda_filters[i].port == intf->port)
+ bcmasp_en_mda_filter(intf, 0, i);
+ }
+}
[...]
+static int bcmasp_probe(struct platform_device *pdev)
+{
+ struct device_node *ports_node, *intf_node;
+ const struct bcmasp_plat_data *pdata;
+ struct device *dev = &pdev->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(&pdev->dev, DMA_BIT_MASK(40));
+ if (ret)
+ ret = dma_set_mask_and_coherent(&pdev->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(&pdev->dev, "unable to set DMA mask: %d\n", ret);
+ return ret;
+ }
+
[...]
+static int __maybe_unused bcmasp_suspend(struct device *d)
+{
+ struct bcmasp_priv *priv = dev_get_drvdata(d);
+ struct bcmasp_intf *intf;
+ unsigned int i;
Same
+ int ret = 0;
no need to initialize, but it is mostmy a matter of taste.
+
+ for (i = 0; i < priv->intf_count; i++) {
+ intf = priv->intfs[i];
+ if (!intf)
+ continue;
+
+ ret = bcmasp_interface_suspend(intf);
+ if (ret)
+ break;
+ }
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret)
+ return ret;
+
+ /* Whether Wake-on-LAN is enabled or not, we can always disable
+* the shared TX clock
+*/
+ bcmasp_core_clock_set(priv, 0, ASP_CTRL_CLOCK_CTRL_ASP_TX_DISABLE);
+
+ bcmasp_core_clock_select(priv, true);
+
+ clk_disable_unprepare(priv->clk);
+
+ return ret;
+}
+
+static int __maybe_unused bcmasp_resume(struct device *d)
+{
+ struct bcmasp_priv *priv = dev_get_drvdata(d);
+ struct bcmasp_intf *intf;
+ unsigned int i;
same
+ int ret = 0;
no need to initialize, but it is mostmy a matter of taste.
Just my 2c,
CJ
[PATCH] drm/amd/display: Correctly initialize some memory in get_available_dsc_slices()
The intent here is to clear the 'available_slices' buffer before setting
some values in it.
This is an array of int, so in order to fully initialize it, we must clear
MIN_AVAILABLE_SLICES_SIZE * sizeof(int) bytes.
Compute the right length of the buffer when calling memset().
Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)")
Signed-off-by: Christophe JAILLET
---
NOT even compile-tested.
make -j7 drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.o
on my setup, it fails with:
CC drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.o
drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c:27:10: fatal error: dc_hw_types.h:
Aucun fichier ou dossier de ce type
27 | #include "dc_hw_types.h"
| ^~~
I've not investigated why.
---
drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
index b9a05bb025db..1d7384b2be28 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
@@ -645,7 +645,7 @@ static int get_available_dsc_slices(union
dsc_enc_slice_caps slice_caps, int *av
{
int idx = 0;
- memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE);
+ memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE *
sizeof(*available_slices));
if (slice_caps.bits.NUM_SLICES_1)
available_slices[idx++] = 1;
--
2.34.1
Re: [PATCH] video: fbdev: mmp: Fix deferred clk handling in mmphw_probe()
Le 19/04/2023 à 06:59, Dan Carpenter a écrit :
On Sat, Apr 15, 2023 at 04:09:03PM +0300, Dan Carpenter wrote:
On Thu, Apr 13, 2023 at 09:33:17PM +0200, Christophe JAILLET wrote:
When dev_err_probe() is called, 'ret' holds the value of the previous
successful devm_request_irq() call.
'ret' should be assigned with a meaningful value before being used in
dev_err_probe().
While at it, use and return "PTR_ERR(ctrl->clk)" instead of a hard-coded
"-ENOENT" so that -EPROBE_DEFER is handled and propagated correctly.
Fixes: 81b63420564d ("video: fbdev: mmp: Make use of the helper function
dev_err_probe()")
Signed-off-by: Christophe JAILLET
---
Presumably you already wrote a Coccinelle script for this but I've added
it to Smatch as well.
No I haven't.
I've spotted it while looking at some devm_clk_get_optional() candidate
with grep.
git grep -A5 devm_clk_get | grep -B5 ENOENT
Not perfect, but sometimes this kind of approach can find interesting
things coccinelle would miss.
As an example, the bitmap_alloc patch on sh4 was found this way, with grep.
So nice to have it in smatch, ;-)
CJ
Here is this warning:
drivers/video/fbdev/mmp/hw/mmp_ctrl.c:518 mmphw_probe() warn: passing zero to
'dev_err_probe'
Other warnings. All five are interesting.
drivers/power/supply/rt9467-charger.c:1026 rt9467_request_interrupt() warn:
passing zero to 'dev_err_probe'
drivers/pci/controller/dwc/pcie-bt1.c:601 bt1_pcie_add_port() warn: passing
zero to 'dev_err_probe'
drivers/spi/spi-sprd-adi.c:570 sprd_adi_probe() warn: passing zero to
'dev_err_probe'
drivers/soc/qcom/icc-bwmon.c:776 bwmon_probe() warn: passing zero to
'dev_err_probe'
drivers/soc/qcom/icc-bwmon.c:781 bwmon_probe() warn: passing zero to
'dev_err_probe'
regards,
dan carpenter
[PATCH] drm/amd/display: Fix a test dml32_rq_dlg_get_rq_reg()
It is likely p1_min_meta_chunk_bytes was expected here, instead of
min_meta_chunk_bytes.
Test the correct variable.
Fixes: dda4fb85e433 ("drm/amd/display: DML changes for DCN32/321")
Signed-off-by: Christophe JAILLET
---
.../gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c
index 395ae8761980..9ba6cb67655f 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_rq_dlg_calc_32.c
@@ -116,7 +116,7 @@ void dml32_rq_dlg_get_rq_reg(display_rq_regs_st *rq_regs,
else
rq_regs->rq_regs_l.min_meta_chunk_size =
dml_log2(min_meta_chunk_bytes) - 6 + 1;
- if (min_meta_chunk_bytes == 0)
+ if (p1_min_meta_chunk_bytes == 0)
rq_regs->rq_regs_c.min_meta_chunk_size = 0;
else
rq_regs->rq_regs_c.min_meta_chunk_size =
dml_log2(p1_min_meta_chunk_bytes) - 6 + 1;
--
2.34.1
[PATCH] drm/amd/display: Fix a test CalculatePrefetchSchedule()
It is likely Height was expected here, instead of Width.
Test the correct variable.
Fixes: 17529ea2acfa ("drm/amd/display: Optimizations for DML math")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
index b7c2844d0cbe..f294f2f8c75b 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_mode_vba_21.c
@@ -810,7 +810,7 @@ static bool CalculatePrefetchSchedule(
*swath_width_chroma_ub = dml_ceil(SwathWidthY / 2 - 1,
myPipe->BlockWidth256BytesC) + myPipe->BlockWidth256BytesC;
} else {
*swath_width_luma_ub = dml_ceil(SwathWidthY - 1,
myPipe->BlockHeight256BytesY) + myPipe->BlockHeight256BytesY;
- if (myPipe->BlockWidth256BytesC > 0)
+ if (myPipe->BlockHeight256BytesC > 0)
*swath_width_chroma_ub = dml_ceil(SwathWidthY / 2 - 1,
myPipe->BlockHeight256BytesC) + myPipe->BlockHeight256BytesC;
}
--
2.34.1
[PATCH] video: fbdev: mmp: Fix deferred clk handling in mmphw_probe()
When dev_err_probe() is called, 'ret' holds the value of the previous
successful devm_request_irq() call.
'ret' should be assigned with a meaningful value before being used in
dev_err_probe().
While at it, use and return "PTR_ERR(ctrl->clk)" instead of a hard-coded
"-ENOENT" so that -EPROBE_DEFER is handled and propagated correctly.
Fixes: 81b63420564d ("video: fbdev: mmp: Make use of the helper function
dev_err_probe()")
Signed-off-by: Christophe JAILLET
---
drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
index a9df8ee79810..51fbf02a0343 100644
--- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
@@ -514,9 +514,9 @@ static int mmphw_probe(struct platform_device *pdev)
/* get clock */
ctrl->clk = devm_clk_get(ctrl->dev, mi->clk_name);
if (IS_ERR(ctrl->clk)) {
+ ret = PTR_ERR(ctrl->clk);
dev_err_probe(ctrl->dev, ret,
"unable to get clk %s\n", mi->clk_name);
- ret = -ENOENT;
goto failed;
}
clk_prepare_enable(ctrl->clk);
--
2.34.1
Re: [PATCH] drm/armada: Fix a potential double free in an error handling path
Le 26/12/2021 à 17:34, Christophe JAILLET a écrit :
'priv' is a managed resource, so there is no need to free it explicitly or
there will be a double free().
Fixes: 90ad200b4cbc ("drm/armada: Use devm_drm_dev_alloc")
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/armada/armada_drv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index 8e3e98f13db4..54168134d9b9 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -99,7 +99,6 @@ static int armada_drm_bind(struct device *dev)
if (ret) {
dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n",
__func__, ret);
- kfree(priv);
return ret;
}
Hi,
polite reminder. (I've updated the mails according to the output of
get_maintainer.pl)
The patch still LGTM and should apply cleanly.
CJ
[PATCH] drm/amd/display: Slightly optimize dm_dmub_outbox1_low_irq()
A kzalloc()+memcpy() can be optimized in a single kmemdup().
This saves a few cycles because some memory doesn't need to be zeroed.
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5bac5781a06b..57a5fbdab890 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -820,15 +820,14 @@ static void dm_dmub_outbox1_low_irq(void
*interrupt_params)
DRM_ERROR("Failed to allocate
dmub_hpd_wrk");
return;
}
- dmub_hpd_wrk->dmub_notify =
kzalloc(sizeof(struct dmub_notification), GFP_ATOMIC);
+ dmub_hpd_wrk->dmub_notify = kmemdup(¬ify,
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(&dmub_hpd_wrk->handle_hpd_work,
dm_handle_hpd_work);
- if (dmub_hpd_wrk->dmub_notify)
- memcpy(dmub_hpd_wrk->dmub_notify,
¬ify, sizeof(struct dmub_notification));
dmub_hpd_wrk->adev = adev;
if (notify.type == DMUB_NOTIFICATION_HPD) {
plink =
adev->dm.dc->links[notify.link_index];
--
2.32.0
Re: [PATCH] drm/amdkfd: Fix an illegal memory access
Le 21/02/2023 à 17:26, Felix Kuehling a écrit :
On 2023-02-21 06:35, qu.huang-fxuvxftifdnyg1zeobx...@public.gmane.org
wrote:
From: Qu Huang
In the kfd_wait_on_events() function, the kfd_event_waiter structure is
allocated by alloc_event_waiters(), but the event field of the waiter
structure is not initialized; When copy_from_user() fails in the
kfd_wait_on_events() function, it will enter exception handling to
release the previously allocated memory of the waiter structure;
Due to the event field of the waiters structure being accessed
in the free_waiters() function, this results in illegal memory access
and system crash, here is the crash log:
localhost kernel: RIP: 0010:native_queued_spin_lock_slowpath+0x185/0x1e0
localhost kernel: RSP: 0018:aa53c362bd60 EFLAGS: 00010082
localhost kernel: RAX: ff3d3d6bff4007cb RBX: 0282 RCX:
002c
localhost kernel: RDX: 9e855eeacb80 RSI: 279c RDI:
e7088f6a21d0
localhost kernel: RBP: e7088f6a21d0 R08: 002c R09:
aa53c362be64
localhost kernel: R10: aa53c362bbd8 R11: 0001 R12:
0002
localhost kernel: R13: 9e7ead15d600 R14: R15:
9e7ead15d698
localhost kernel: FS: 152a3d111700()
GS:9e855ee8() knlGS:
localhost kernel: CS: 0010 DS: ES: CR0: 80050033
localhost kernel: CR2: 15293810 CR3: 00044d7a4000 CR4:
003506e0
localhost kernel: Call Trace:
localhost kernel: _raw_spin_lock_irqsave+0x30/0x40
localhost kernel: remove_wait_queue+0x12/0x50
localhost kernel: kfd_wait_on_events+0x1b6/0x490 [hydcu]
localhost kernel: ? ftrace_graph_caller+0xa0/0xa0
localhost kernel: kfd_ioctl+0x38c/0x4a0 [hydcu]
localhost kernel: ? kfd_ioctl_set_trap_handler+0x70/0x70 [hydcu]
localhost kernel: ? kfd_ioctl_create_queue+0x5a0/0x5a0 [hydcu]
localhost kernel: ? ftrace_graph_caller+0xa0/0xa0
localhost kernel: __x64_sys_ioctl+0x8e/0xd0
localhost kernel: ? syscall_trace_enter.isra.18+0x143/0x1b0
localhost kernel: do_syscall_64+0x33/0x80
localhost kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9
localhost kernel: RIP: 0033:0x152a4dff68d7
Signed-off-by: Qu Huang
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index 729d26d..e5faaad 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -787,6 +787,7 @@ static struct kfd_event_waiter
*alloc_event_waiters(uint32_t num_events)
for (i = 0; (event_waiters) && (i < num_events) ; i++) {
init_wait(&event_waiters[i].wait);
event_waiters[i].activated = false;
+ event_waiters[i].event = NULL;
Thank you for catching this. We're often lazy about initializing things
to NULL or 0 because most of our data structures are allocated with
kzalloc or similar. I'm not sure why we're not doing this here. If we
allocated event_waiters with kcalloc, we could also remove the
initialization of activated. I think that would be the cleaner and safer
solution.
Hi,
I think that the '(event_waiters) &&' in the 'for' can also be removed.
'event_waiters' is already NULL tested a few lines above
Just my 2c.
CJ
Regards,
Felix
}
return event_waiters;
--
1.8.3.1
[PATCH] drm/amd: Optimize some memory initializations
Instead of zeroing some memory and then copying data in part or all of it, use memcpy_and_pad(). This avoids writing some memory twice and should save a few cycles. Signed-off-by: Christophe JAILLET --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 11 --- drivers/gpu/drm/amd/amdgpu/psp_v13_0.c | 8 ++-- drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c | 8 ++-- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index a8391f269cd0..5e69693a5cc4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -613,9 +613,8 @@ psp_cmd_submit_buf(struct psp_context *psp, if (!drm_dev_enter(adev_to_drm(psp->adev), &idx)) 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(&psp->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), &idx)) return; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - memcpy(psp->fw_pri_buf, start_addr, bin_size); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, start_addr, bin_size, 0); drm_dev_exit(idx); } diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c index d62fcc77af95..79733ec4ffab 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0.c @@ -168,10 +168,8 @@ static int psp_v13_0_bootloader_load_component(struct psp_context *psp, if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy PSP KDB binary to memory */ - memcpy(psp->fw_pri_buf, bin_desc->start_addr, bin_desc->size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, bin_desc->start_addr, bin_desc->size_bytes, 0); /* Provide the PSP KDB to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, @@ -237,10 +235,8 @@ static int psp_v13_0_bootloader_load_sos(struct psp_context *psp) if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy Secure OS binary to PSP memory */ - memcpy(psp->fw_pri_buf, psp->sos.start_addr, psp->sos.size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, psp->sos.start_addr, psp->sos.size_bytes, 0); /* Provide the PSP secure OS to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c b/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c index d5ba58eba3e2..c73415b09e85 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v13_0_4.c @@ -107,10 +107,8 @@ static int psp_v13_0_4_bootloader_load_component(struct psp_context*psp, if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy PSP KDB binary to memory */ - memcpy(psp->fw_pri_buf, bin_desc->start_addr, bin_desc->size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, bin_desc->start_addr, bin_desc->size_bytes, 0); /* Provide the PSP KDB to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, @@ -170,10 +168,8 @@ static int psp_v13_0_4_bootloader_load_sos(struct psp_context *psp) if (ret) return ret; - memset(psp->fw_pri_buf, 0, PSP_1_MEG); - /* Copy Secure OS binary to PSP memory */ - memcpy(psp->fw_pri_buf, psp->sos.start_addr, psp->sos.size_bytes); + memcpy_and_pad(psp->fw_pri_buf, PSP_1_MEG, psp->sos.start_addr, psp->sos.size_bytes, 0); /* Provide the PSP secure OS to bootloader */ WREG32_SOC15(MP0, 0, regMP0_SMN_C2PMSG_36, -- 2.34.1
Re: [PATCH] agp/amd64: Fix full name of the GPL
Le 22/01/2023 à 19:16, Diederik de Haas a écrit : Signed-off-by: Diederik de Haas --- drivers/char/agp/amd64-agp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c index ce8651436609..3020fd92fd00 100644 --- a/drivers/char/agp/amd64-agp.c +++ b/drivers/char/agp/amd64-agp.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* * Copyright 2001-2003 SuSE Labs. - * Distributed under the GNU public license, v2. + * Distributed under the GNU General Public License, v2. * * This is a GART driver for the AMD Opteron/Athlon64 on-CPU northbridge. * It also includes support for the AMD 8151 AGP bridge, Hi, maybe the line can just be removed. There is already a SPDX-License-Identifier above. CJ
[PATCH v2] video: fbdev: omapfb: Use kstrtobool() instead of strtobool()
strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET --- This patch was already sent as a part of a serie ([1]) that axed all usages of strtobool(). Most of the patches have been merged in -next. I synch'ed with latest -next and re-send the remaining ones as individual patches. Changes in v2: - No change [1]: https://lore.kernel.org/all/cover.1667336095.git.christophe.jail...@wanadoo.fr/ --- drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/manager-sysfs.c | 7 --- drivers/video/fbdev/omap2/omapfb/dss/overlay-sysfs.c | 3 ++- drivers/video/fbdev/omap2/omapfb/omapfb-sysfs.c | 3 ++- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c index bc5a44c2a144..ae937854403b 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/display-sysfs.c @@ -10,6 +10,7 @@ #define DSS_SUBSYS_NAME "DISPLAY" #include +#include #include #include #include @@ -36,7 +37,7 @@ static ssize_t display_enabled_store(struct omap_dss_device *dssdev, int r; bool enable; - r = strtobool(buf, &enable); + r = kstrtobool(buf, &enable); 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, &te); + r = kstrtobool(buf, &te); 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, &mirror); + r = kstrtobool(buf, &mirror); 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, &enable); + r = kstrtobool(buf, &enable); 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, &enable); + r = kstrtobool(buf, &enable); 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, &enable); + r = kstrtobool(buf, &enable); 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, &enable); + r = kstrtobool(buf, &enable); 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, &mirror); + r = kstrtobool(buf, &mirror); if (r) return r; -- 2.34.1
[PATCH v2] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.
This simplifies the code and avoids the need of a dedicated function used
with devm_add_action_or_reset().
Signed-off-by: Christophe JAILLET
Reviewed-by: Andrzej Hajda
---
Change in v2:
- Convert to dev_err_probe()[Andrzej Hajda]
- Update the error message[Andrzej Hajda]
- Add R-b tag[Andrzej Hajda]
v1:
https://lore.kernel.org/all/4f855984ea895e1488169e77935fa6e044912ac2.1672414073.git.christophe.jail...@wanadoo.fr/
---
drivers/gpu/drm/bridge/tc358767.c | 25 -
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 2a58eb271f70..99f3d5ca7257 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2022,13 +2022,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc)
return -EINVAL;
}
-static void tc_clk_disable(void *data)
-{
- struct clk *refclk = data;
-
- clk_disable_unprepare(refclk);
-}
-
static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
struct device *dev = &client->dev;
@@ -2045,20 +2038,10 @@ static int tc_probe(struct i2c_client *client, const
struct i2c_device_id *id)
if (ret)
return ret;
- tc->refclk = devm_clk_get(dev, "ref");
- if (IS_ERR(tc->refclk)) {
- ret = PTR_ERR(tc->refclk);
- dev_err(dev, "Failed to get refclk: %d\n", ret);
- return ret;
- }
-
- ret = clk_prepare_enable(tc->refclk);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk);
- if (ret)
- return ret;
+ tc->refclk = devm_clk_get_enabled(dev, "ref");
+ if (IS_ERR(tc->refclk))
+ return dev_err_probe(dev, PTR_ERR(tc->refclk),
+"Failed to get and enable the ref clk\n");
/* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */
usleep_range(10, 15);
--
2.34.1
[PATCH] drm/bridge: tc358767: Use devm_clk_get_enabled() helper
The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.
This simplifies the code and avoids the need of a dedicated function used
with devm_add_action_or_reset().
Signed-off-by: Christophe JAILLET
---
drivers/gpu/drm/bridge/tc358767.c | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 2a58eb271f70..b95c36ca660d 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2022,13 +2022,6 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc)
return -EINVAL;
}
-static void tc_clk_disable(void *data)
-{
- struct clk *refclk = data;
-
- clk_disable_unprepare(refclk);
-}
-
static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
struct device *dev = &client->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
