2020-08-12 07:46:08

by Tian Tao

[permalink] [raw]
Subject: [PATCH drm/hisilicon v3 0/2] hibmc clean up and code refactoring

patch #1 and #3 is clean up, patch #2 is for code refactoring

Changes since v1:
- Rewrite the commits messages and patch name in #1
- Rewrite the commits message in #2.
- Add the new patch #3

Changes since v2:
- merge patch #3 into patch #2

Tian Tao (2):
drm/hisilicon: Remove the unused include statements
drm/hisilicon: Code refactoring for hibmc_drv_de

drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 58 ++++++------------------
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 --
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 -
4 files changed, 15 insertions(+), 52 deletions(-)

--
2.7.4


2020-08-12 07:46:34

by Tian Tao

[permalink] [raw]
Subject: [PATCH drm/hisilicon v3 2/2] drm/hisilicon: Code refactoring for hibmc_drv_de

The memory used to be allocated with devres helpers and released
automatically. In rare circumstances, the memory's release could
have happened before the DRM device got released, which would have
caused memory corruption of some kind. Now we're embedding the data
structures in struct hibmc_drm_private. The whole release problem
has been resolved, because struct hibmc_drm_private is allocated
with drmm_kzalloc and always released with the DRM device.

Signed-off-by: Tian Tao <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 55 ++++++-------------------
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
2 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index 66132eb..d9062a3 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -157,37 +157,6 @@ static const struct drm_plane_helper_funcs hibmc_plane_helper_funcs = {
.atomic_update = hibmc_plane_atomic_update,
};

-static struct drm_plane *hibmc_plane_init(struct hibmc_drm_private *priv)
-{
- struct drm_device *dev = priv->dev;
- struct drm_plane *plane;
- int ret = 0;
-
- plane = devm_kzalloc(dev->dev, sizeof(*plane), GFP_KERNEL);
- if (!plane) {
- DRM_ERROR("failed to alloc memory when init plane\n");
- return ERR_PTR(-ENOMEM);
- }
- /*
- * plane init
- * TODO: Now only support primary plane, overlay planes
- * need to do.
- */
- ret = drm_universal_plane_init(dev, plane, 1, &hibmc_plane_funcs,
- channel_formats1,
- ARRAY_SIZE(channel_formats1),
- NULL,
- DRM_PLANE_TYPE_PRIMARY,
- NULL);
- if (ret) {
- DRM_ERROR("failed to init plane: %d\n", ret);
- return ERR_PTR(ret);
- }
-
- drm_plane_helper_add(plane, &hibmc_plane_helper_funcs);
- return plane;
-}
-
static void hibmc_crtc_dpms(struct drm_crtc *crtc, int dpms)
{
struct hibmc_drm_private *priv = crtc->dev->dev_private;
@@ -534,22 +503,24 @@ static const struct drm_crtc_helper_funcs hibmc_crtc_helper_funcs = {
int hibmc_de_init(struct hibmc_drm_private *priv)
{
struct drm_device *dev = priv->dev;
- struct drm_crtc *crtc;
- struct drm_plane *plane;
+ struct drm_crtc *crtc = &priv->crtc;
+ struct drm_plane *plane = &priv->primary_plane;
int ret;

- plane = hibmc_plane_init(priv);
- if (IS_ERR(plane)) {
- DRM_ERROR("failed to create plane: %ld\n", PTR_ERR(plane));
- return PTR_ERR(plane);
- }
+ ret = drm_universal_plane_init(dev, plane, 1, &hibmc_plane_funcs,
+ channel_formats1,
+ ARRAY_SIZE(channel_formats1),
+ NULL,
+ DRM_PLANE_TYPE_PRIMARY,
+ NULL);

- crtc = devm_kzalloc(dev->dev, sizeof(*crtc), GFP_KERNEL);
- if (!crtc) {
- DRM_ERROR("failed to alloc memory when init crtc\n");
- return -ENOMEM;
+ if (ret) {
+ DRM_ERROR("failed to init plane: %d\n", ret);
+ return ret;
}

+ drm_plane_helper_add(plane, &hibmc_plane_helper_funcs);
+
ret = drm_crtc_init_with_planes(dev, crtc, plane,
NULL, &hibmc_crtc_funcs, NULL);
if (ret) {
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index a683763..197485e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -28,6 +28,8 @@ struct hibmc_drm_private {

/* drm */
struct drm_device *dev;
+ struct drm_plane primary_plane;
+ struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
bool mode_config_initialized;
--
2.7.4

2020-08-12 07:47:09

by Tian Tao

[permalink] [raw]
Subject: [PATCH drm/hisilicon v3 1/2] drm/hisilicon: Remove the unused include statements

Remove some unused include statements.

Signed-off-by: Tian Tao <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 3 ---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 -----
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 --
3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index cc70e83..66132eb 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -17,9 +17,6 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_gem_vram_helper.h>
-#include <drm/drm_plane_helper.h>
-#include <drm/drm_print.h>
-#include <drm/drm_probe_helper.h>
#include <drm/drm_vblank.h>

#include "hibmc_drm_drv.h"
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index b8d839a..54f6144 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -11,18 +11,13 @@
* Jianhua Li <[email protected]>
*/

-#include <linux/console.h>
-#include <linux/module.h>
#include <linux/pci.h>

#include <drm/drm_atomic_helper.h>
#include <drm/drm_drv.h>
-#include <drm/drm_fb_helper.h>
#include <drm/drm_gem_vram_helper.h>
#include <drm/drm_irq.h>
#include <drm/drm_managed.h>
-#include <drm/drm_print.h>
-#include <drm/drm_probe_helper.h>
#include <drm/drm_vblank.h>

#include "hibmc_drm_drv.h"
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 2ca69c3..ed12f61 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -11,10 +11,8 @@
* Jianhua Li <[email protected]>
*/

-#include <drm/drm_gem_vram_helper.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_crtc_helper.h>
#include <drm/drm_print.h>

#include "hibmc_drm_drv.h"
--
2.7.4

2020-08-12 08:34:50

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH drm/hisilicon v3 0/2] hibmc clean up and code refactoring

Merged into drm-misc-next

Am 12.08.20 um 09:42 schrieb Tian Tao:
> patch #1 and #3 is clean up, patch #2 is for code refactoring
>
> Changes since v1:
> - Rewrite the commits messages and patch name in #1
> - Rewrite the commits message in #2.
> - Add the new patch #3
>
> Changes since v2:
> - merge patch #3 into patch #2
>
> Tian Tao (2):
> drm/hisilicon: Remove the unused include statements
> drm/hisilicon: Code refactoring for hibmc_drv_de
>
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 58 ++++++------------------
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 5 --
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 -
> 4 files changed, 15 insertions(+), 52 deletions(-)
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


Attachments:
signature.asc (527.00 B)
OpenPGP digital signature