2022-10-26 16:02:21

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 0/5] drm/arm/malidp: use drm managed resources

Hi,

This patch series converts the driver to use drm managed resources to prevent
potential use-after-free issues on driver unbind/rebind and to get rid of the
usage of deprecated APIs.

Changes in v2:
- While protecting critical sections with drm_dev_{enter,exit} I forgot to
handle alternate return paths within the read-side critical sections, hence
fix them.
- Add a patch to remove explicit calls to drm_mode_config_cleanup() and switch
to drmm_mode_config_init() explicitly.

Changes in v3:
- Remove patches to protect platform device bound resources with
drm_dev_{enter,exit}, since this would leave the hardware enabled when
regularly unloading the driver e.g. via rmmod.
Instead do this in a later series, once we got drm_dev_unplug() in place
to deal with a regular driver shutdown.

Danilo Krummrich (5):
drm/arm/malidp: use drmm_* to allocate driver structures
drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
drm/arm/malidp: plane: use drm managed resources
drm/arm/malidp: remove calls to drm_mode_config_cleanup()

drivers/gpu/drm/arm/malidp_crtc.c | 7 ++-
drivers/gpu/drm/arm/malidp_drv.c | 69 +++++++++++------------------
drivers/gpu/drm/arm/malidp_drv.h | 2 +
drivers/gpu/drm/arm/malidp_hw.c | 10 ++---
drivers/gpu/drm/arm/malidp_mw.c | 6 +--
drivers/gpu/drm/arm/malidp_planes.c | 32 ++++---------
6 files changed, 48 insertions(+), 78 deletions(-)


base-commit: e1e7bc481d49c3e3ada11029ce0d9b85a0a539d7
--
2.37.3



2022-10-26 16:02:31

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 3/5] drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()

Use drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
to get rid of the explicit destroy hook in struct drm_plane_funcs.

Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/arm/malidp_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 34ad7e1cd2b8..dc01c43f6193 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -514,7 +514,6 @@ static void malidp_crtc_disable_vblank(struct drm_crtc *crtc)
}

static const struct drm_crtc_funcs malidp_crtc_funcs = {
- .destroy = drm_crtc_cleanup,
.set_config = drm_atomic_helper_set_config,
.page_flip = drm_atomic_helper_page_flip,
.reset = malidp_crtc_reset,
@@ -548,8 +547,8 @@ int malidp_crtc_init(struct drm_device *drm)
return -EINVAL;
}

- ret = drm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL,
- &malidp_crtc_funcs, NULL);
+ ret = drmm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL,
+ &malidp_crtc_funcs, NULL);
if (ret)
return ret;

--
2.37.3


2022-10-26 16:32:22

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 5/5] drm/arm/malidp: remove calls to drm_mode_config_cleanup()

drm_mode_config_init() simply calls drmm_mode_config_init(), hence
cleanup is automatically handled through registering
drm_mode_config_cleanup() with drmm_add_action_or_reset().

While at it, get rid of the deprecated drm_mode_config_init() and
replace it with drmm_mode_config_init() directly.

Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 678c5b0d8014..bebaa5a07e27 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -396,7 +396,9 @@ static int malidp_init(struct drm_device *drm)
struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ goto out;

drm->mode_config.min_width = hwdev->min_line_size;
drm->mode_config.min_height = hwdev->min_line_size;
@@ -407,24 +409,16 @@ static int malidp_init(struct drm_device *drm)

ret = malidp_crtc_init(drm);
if (ret)
- goto crtc_fail;
+ goto out;

ret = malidp_mw_connector_init(drm);
if (ret)
- goto crtc_fail;
-
- return 0;
+ goto out;

-crtc_fail:
- drm_mode_config_cleanup(drm);
+out:
return ret;
}

-static void malidp_fini(struct drm_device *drm)
-{
- drm_mode_config_cleanup(drm);
-}
-
static int malidp_irq_init(struct platform_device *pdev)
{
int irq_de, irq_se, ret = 0;
@@ -874,7 +868,6 @@ static int malidp_bind(struct device *dev)
bind_fail:
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
- malidp_fini(drm);
query_hw_fail:
pm_runtime_put(dev);
if (pm_runtime_enabled(dev))
@@ -902,7 +895,6 @@ static void malidp_unbind(struct device *dev)
component_unbind_all(dev, drm);
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
- malidp_fini(drm);
pm_runtime_put(dev);
if (pm_runtime_enabled(dev))
pm_runtime_disable(dev);
--
2.37.3


2022-10-26 16:32:35

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 1/5] drm/arm/malidp: use drmm_* to allocate driver structures

Use drm managed resources to allocate driver structures and get rid of
the deprecated drm_dev_alloc() call and replace it with
devm_drm_dev_alloc().

This also serves as preparation to get rid of drm_device->dev_private
and to fix use-after-free issues on driver unload.

Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.c | 20 +++++++-------------
drivers/gpu/drm/arm/malidp_drv.h | 1 +
2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 1d0b0c54ccc7..41c80e905991 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -23,6 +23,7 @@
#include <drm/drm_fourcc.h>
#include <drm/drm_gem_dma_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_managed.h>
#include <drm/drm_modeset_helper.h>
#include <drm/drm_module.h>
#include <drm/drm_of.h>
@@ -716,11 +717,13 @@ static int malidp_bind(struct device *dev)
int ret = 0, i;
u32 version, out_depth = 0;

- malidp = devm_kzalloc(dev, sizeof(*malidp), GFP_KERNEL);
- if (!malidp)
- return -ENOMEM;
+ malidp = devm_drm_dev_alloc(dev, &malidp_driver, typeof(*malidp), base);
+ if (IS_ERR(malidp))
+ return PTR_ERR(malidp);
+
+ drm = &malidp->base;

- hwdev = devm_kzalloc(dev, sizeof(*hwdev), GFP_KERNEL);
+ hwdev = drmm_kzalloc(drm, sizeof(*hwdev), GFP_KERNEL);
if (!hwdev)
return -ENOMEM;

@@ -753,12 +756,6 @@ static int malidp_bind(struct device *dev)
if (ret && ret != -ENODEV)
return ret;

- drm = drm_dev_alloc(&malidp_driver, dev);
- if (IS_ERR(drm)) {
- ret = PTR_ERR(drm);
- goto alloc_fail;
- }
-
drm->dev_private = malidp;
dev_set_drvdata(dev, drm);

@@ -887,8 +884,6 @@ static int malidp_bind(struct device *dev)
malidp_runtime_pm_suspend(dev);
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
- drm_dev_put(drm);
-alloc_fail:
of_reserved_mem_device_release(dev);

return ret;
@@ -917,7 +912,6 @@ static void malidp_unbind(struct device *dev)
malidp_runtime_pm_suspend(dev);
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
- drm_dev_put(drm);
of_reserved_mem_device_release(dev);
}

diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index cdfddfabf2d1..00be369b28f1 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -29,6 +29,7 @@ struct malidp_error_stats {
};

struct malidp_drm {
+ struct drm_device base;
struct malidp_hw_device *dev;
struct drm_crtc crtc;
struct drm_writeback_connector mw_connector;
--
2.37.3


2022-10-26 16:33:07

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 4/5] drm/arm/malidp: plane: use drm managed resources

Use drm managed resource allocation (drmm_universal_plane_alloc()) in
order to get rid of the explicit destroy hook in struct drm_plane_funcs.

Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/arm/malidp_planes.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 815d9199752f..34547edf1ee3 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -68,14 +68,6 @@
/* readahead for partial-frame prefetch */
#define MALIDP_MMU_PREFETCH_READAHEAD 8

-static void malidp_de_plane_destroy(struct drm_plane *plane)
-{
- struct malidp_plane *mp = to_malidp_plane(plane);
-
- drm_plane_cleanup(plane);
- kfree(mp);
-}
-
/*
* Replicate what the default ->reset hook does: free the state pointer and
* allocate a new empty object. We just need enough space to store
@@ -260,7 +252,6 @@ static bool malidp_format_mod_supported_per_plane(struct drm_plane *plane,
static const struct drm_plane_funcs malidp_de_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
- .destroy = malidp_de_plane_destroy,
.reset = malidp_plane_reset,
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
@@ -972,12 +963,6 @@ int malidp_de_planes_init(struct drm_device *drm)
for (i = 0; i < map->n_layers; i++) {
u8 id = map->layers[i].id;

- plane = kzalloc(sizeof(*plane), GFP_KERNEL);
- if (!plane) {
- ret = -ENOMEM;
- goto cleanup;
- }
-
/* build the list of DRM supported formats based on the map */
for (n = 0, j = 0; j < map->n_pixel_formats; j++) {
if ((map->pixel_formats[j].layer & id) == id)
@@ -990,13 +975,14 @@ int malidp_de_planes_init(struct drm_device *drm)
/*
* All the layers except smart layer supports AFBC modifiers.
*/
- ret = drm_universal_plane_init(drm, &plane->base, crtcs,
- &malidp_de_plane_funcs, formats, n,
- (id == DE_SMART) ? linear_only_modifiers : modifiers,
- plane_type, NULL);
-
- if (ret < 0)
+ plane = drmm_universal_plane_alloc(drm, struct malidp_plane, base,
+ crtcs, &malidp_de_plane_funcs, formats, n,
+ (id == DE_SMART) ? linear_only_modifiers :
+ modifiers, plane_type, NULL);
+ if (IS_ERR(plane)) {
+ ret = PTR_ERR(plane);
goto cleanup;
+ }

drm_plane_helper_add(&plane->base,
&malidp_de_plane_helper_funcs);
--
2.37.3


2022-10-26 16:52:02

by Danilo Krummrich

[permalink] [raw]
Subject: [PATCH drm-misc-next v3 2/5] drm/arm/malidp: replace drm->dev_private with drm_to_malidp()

Using drm_device->dev_private is deprecated. Since we've switched to
devm_drm_dev_alloc(), struct drm_device is now embedded in struct
malidp_drm, hence we can use container_of() to get the struct drm_device
instance instead.

Signed-off-by: Danilo Krummrich <[email protected]>
---
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_drv.c | 29 +++++++++++++----------------
drivers/gpu/drm/arm/malidp_drv.h | 1 +
drivers/gpu/drm/arm/malidp_hw.c | 10 +++++-----
drivers/gpu/drm/arm/malidp_mw.c | 6 +++---
drivers/gpu/drm/arm/malidp_planes.c | 4 ++--
6 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 962730772b2f..34ad7e1cd2b8 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -526,7 +526,7 @@ static const struct drm_crtc_funcs malidp_crtc_funcs = {

int malidp_crtc_init(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_plane *primary = NULL, *plane;
int ret;

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 41c80e905991..678c5b0d8014 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -169,7 +169,7 @@ static void malidp_atomic_commit_se_config(struct drm_crtc *crtc,
*/
static int malidp_set_and_wait_config_valid(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
int ret;

@@ -190,7 +190,7 @@ static int malidp_set_and_wait_config_valid(struct drm_device *drm)
static void malidp_atomic_commit_hw_done(struct drm_atomic_state *state)
{
struct drm_device *drm = state->dev;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
int loop = 5;

malidp->event = malidp->crtc.state->event;
@@ -231,7 +231,7 @@ static void malidp_atomic_commit_hw_done(struct drm_atomic_state *state)
static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *drm = state->dev;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
int i;
@@ -393,7 +393,7 @@ static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
static int malidp_init(struct drm_device *drm)
{
int ret;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

drm_mode_config_init(drm);
@@ -429,7 +429,7 @@ static int malidp_irq_init(struct platform_device *pdev)
{
int irq_de, irq_se, ret = 0;
struct drm_device *drm = dev_get_drvdata(&pdev->dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

/* fetch the interrupts from DT */
@@ -463,7 +463,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
struct drm_device *drm,
struct drm_mode_create_dumb *args)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
/* allocate for the worst case scenario, i.e. rotated buffers */
u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);

@@ -509,7 +509,7 @@ static void malidp_error_stats_dump(const char *prefix,
static int malidp_show_stats(struct seq_file *m, void *arg)
{
struct drm_device *drm = m->private;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
unsigned long irqflags;
struct malidp_error_stats de_errors, se_errors;

@@ -532,7 +532,7 @@ static ssize_t malidp_debugfs_write(struct file *file, const char __user *ubuf,
{
struct seq_file *m = file->private_data;
struct drm_device *drm = m->private;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
unsigned long irqflags;

spin_lock_irqsave(&malidp->errors_lock, irqflags);
@@ -553,7 +553,7 @@ static const struct file_operations malidp_debugfs_fops = {

static void malidp_debugfs_init(struct drm_minor *minor)
{
- struct malidp_drm *malidp = minor->dev->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(minor->dev);

malidp_error_stats_init(&malidp->de_errors);
malidp_error_stats_init(&malidp->se_errors);
@@ -653,7 +653,7 @@ static ssize_t core_id_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct drm_device *drm = dev_get_drvdata(dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);

return snprintf(buf, PAGE_SIZE, "%08x\n", malidp->core_id);
}
@@ -671,7 +671,7 @@ ATTRIBUTE_GROUPS(mali_dp);
static int malidp_runtime_pm_suspend(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

/* we can only suspend if the hardware is in config mode */
@@ -690,7 +690,7 @@ static int malidp_runtime_pm_suspend(struct device *dev)
static int malidp_runtime_pm_resume(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

clk_prepare_enable(hwdev->pclk);
@@ -756,7 +756,6 @@ static int malidp_bind(struct device *dev)
if (ret && ret != -ENODEV)
return ret;

- drm->dev_private = malidp;
dev_set_drvdata(dev, drm);

/* Enable power management */
@@ -882,7 +881,6 @@ static int malidp_bind(struct device *dev)
pm_runtime_disable(dev);
else
malidp_runtime_pm_suspend(dev);
- drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
of_reserved_mem_device_release(dev);

@@ -892,7 +890,7 @@ static int malidp_bind(struct device *dev)
static void malidp_unbind(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;

drm_dev_unregister(drm);
@@ -910,7 +908,6 @@ static void malidp_unbind(struct device *dev)
pm_runtime_disable(dev);
else
malidp_runtime_pm_suspend(dev);
- drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
of_reserved_mem_device_release(dev);
}
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 00be369b28f1..bc0387876dea 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -45,6 +45,7 @@ struct malidp_drm {
#endif
};

+#define drm_to_malidp(x) container_of(x, struct malidp_drm, base)
#define crtc_to_malidp_device(x) container_of(x, struct malidp_drm, crtc)

struct malidp_plane {
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index e9de542f9b7c..9b845d3f34e1 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -1168,7 +1168,7 @@ static void malidp_hw_clear_irq(struct malidp_hw_device *hwdev, u8 block, u32 ir
static irqreturn_t malidp_de_irq(int irq, void *arg)
{
struct drm_device *drm = arg;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev;
struct malidp_hw *hw;
const struct malidp_irq_map *de;
@@ -1226,7 +1226,7 @@ static irqreturn_t malidp_de_irq(int irq, void *arg)
static irqreturn_t malidp_de_irq_thread_handler(int irq, void *arg)
{
struct drm_device *drm = arg;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);

wake_up(&malidp->wq);

@@ -1252,7 +1252,7 @@ void malidp_de_irq_hw_init(struct malidp_hw_device *hwdev)

int malidp_de_irq_init(struct drm_device *drm, int irq)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
int ret;

@@ -1286,7 +1286,7 @@ void malidp_de_irq_fini(struct malidp_hw_device *hwdev)
static irqreturn_t malidp_se_irq(int irq, void *arg)
{
struct drm_device *drm = arg;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
struct malidp_hw *hw = hwdev->hw;
const struct malidp_irq_map *se = &hw->map.se_irq_map;
@@ -1363,7 +1363,7 @@ static irqreturn_t malidp_se_irq_thread_handler(int irq, void *arg)

int malidp_se_irq_init(struct drm_device *drm, int irq)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
int ret;

diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index ef76d0e6ee2f..626709bec6f5 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -129,7 +129,7 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_connector_state *conn_state)
{
struct malidp_mw_connector_state *mw_state = to_mw_state(conn_state);
- struct malidp_drm *malidp = encoder->dev->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(encoder->dev);
struct drm_framebuffer *fb;
int i, n_planes;

@@ -207,7 +207,7 @@ static u32 *get_writeback_formats(struct malidp_drm *malidp, int *n_formats)

int malidp_mw_connector_init(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
u32 *formats;
int ret, n_formats;

@@ -236,7 +236,7 @@ int malidp_mw_connector_init(struct drm_device *drm)
void malidp_mw_atomic_commit(struct drm_device *drm,
struct drm_atomic_state *old_state)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_writeback_connector *mw_conn = &malidp->mw_connector;
struct drm_connector_state *conn_state = mw_conn->base.state;
struct malidp_hw_device *hwdev = malidp->dev;
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 45f5e35e7f24..815d9199752f 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -151,7 +151,7 @@ bool malidp_format_mod_supported(struct drm_device *drm,
{
const struct drm_format_info *info;
const u64 *modifiers;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
const struct malidp_hw_regmap *map = &malidp->dev->hw->map;

if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
@@ -931,7 +931,7 @@ static const uint64_t linear_only_modifiers[] = {

int malidp_de_planes_init(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
const struct malidp_hw_regmap *map = &malidp->dev->hw->map;
struct malidp_plane *plane = NULL;
enum drm_plane_type plane_type;
--
2.37.3


2022-11-16 11:44:54

by Liviu Dudau

[permalink] [raw]
Subject: Re: [PATCH drm-misc-next v3 0/5] drm/arm/malidp: use drm managed resources

On Wed, Oct 26, 2022 at 05:59:29PM +0200, Danilo Krummrich wrote:
> Hi,

Hi Danilo,

Sorry for the additional delay in reviewing and testing this series. I've now managed
to get enough of both to be happy with the series.

For the whole series: Reviewed-by: Liviu Dudau <[email protected]>

I will push the series today to drm-misc-next.

Best regards,
Liviu

>
> This patch series converts the driver to use drm managed resources to prevent
> potential use-after-free issues on driver unbind/rebind and to get rid of the
> usage of deprecated APIs.
>
> Changes in v2:
> - While protecting critical sections with drm_dev_{enter,exit} I forgot to
> handle alternate return paths within the read-side critical sections, hence
> fix them.
> - Add a patch to remove explicit calls to drm_mode_config_cleanup() and switch
> to drmm_mode_config_init() explicitly.
>
> Changes in v3:
> - Remove patches to protect platform device bound resources with
> drm_dev_{enter,exit}, since this would leave the hardware enabled when
> regularly unloading the driver e.g. via rmmod.
> Instead do this in a later series, once we got drm_dev_unplug() in place
> to deal with a regular driver shutdown.
>
> Danilo Krummrich (5):
> drm/arm/malidp: use drmm_* to allocate driver structures
> drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
> drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
> drm/arm/malidp: plane: use drm managed resources
> drm/arm/malidp: remove calls to drm_mode_config_cleanup()
>
> drivers/gpu/drm/arm/malidp_crtc.c | 7 ++-
> drivers/gpu/drm/arm/malidp_drv.c | 69 +++++++++++------------------
> drivers/gpu/drm/arm/malidp_drv.h | 2 +
> drivers/gpu/drm/arm/malidp_hw.c | 10 ++---
> drivers/gpu/drm/arm/malidp_mw.c | 6 +--
> drivers/gpu/drm/arm/malidp_planes.c | 32 ++++---------
> 6 files changed, 48 insertions(+), 78 deletions(-)
>
>
> base-commit: e1e7bc481d49c3e3ada11029ce0d9b85a0a539d7
> --
> 2.37.3
>

--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯

2022-11-18 18:53:28

by Danilo Krummrich

[permalink] [raw]
Subject: Re: [PATCH drm-misc-next v3 0/5] drm/arm/malidp: use drm managed resources

Hi Liviu, hi Daniel,

Thanks for submitting the patch series.

Unfortunately, I wasn't able to finish the work to make drm_dev_unplug()
deal properly with non-hotunplug cases before my vacation, since I was
working on another series. I'll finalize and submit it once I'm back in
two weeks.

- Danilo