The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs
@ 2023-06-20  6:47 Jonas Karlman
  2023-06-20  6:47 ` [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs Jonas Karlman
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jonas Karlman @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter
  Cc: Mark Yao, Andy Yan, Sascha Hauer, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel, Jonas Karlman

This series fixes a reset of state in duplicate state crtc funcs for VOP
driver, a possible crash and ensure crtc reset helper is called in VOP2
driver.

Patch 1 use kmemdup instead of kzalloc to duplicate the crtc state.
Patch 2 change to use crtc and plane cleanup helpers directly.
Patch 3 adds a crash guard for empty crtc state.
Patch 4 adds a missing call to crtc reset helper.

This is the next part of an ongoing effort to upstream HDMI 2.0 support
used in LibreELEC for the past 3+ years.

This series is also available at [1].

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20230619-duplicate-state

Jonas Karlman (4):
  drm/rockchip: vop: Fix crtc duplicate state
  drm/rockchip: vop: User cleanup helpers directly as destroy ops
  drm/rockchip: vop2: Don't crash for invalid duplicate_state
  drm/rockchip: vop2: Add missing call to crtc reset helper

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c  | 19 +++--------
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 36 +++++++++-----------
 2 files changed, 21 insertions(+), 34 deletions(-)

-- 
2.41.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs
  2023-06-20  6:47 [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs Jonas Karlman
@ 2023-06-20  6:47 ` Jonas Karlman
  2023-06-21  8:05   ` Sascha Hauer
  2023-06-20  6:47 ` [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs Jonas Karlman
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Mark Yao
  Cc: Andy Yan, Sascha Hauer, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel, Jonas Karlman

struct rockchip_crtc_state members such as output_type, output_bpc and
enable_afbc is always reset to zero in the atomic_duplicate_state crtc
funcs.

Fix this by using kmemdup on the subclass rockchip_crtc_state struct.

Fixes: 4e257d9eee23 ("drm/rockchip: get rid of rockchip_drm_crtc_mode_config")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a530ecc4d207..60b23636a3fe 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1614,7 +1614,8 @@ static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
 	if (WARN_ON(!crtc->state))
 		return NULL;
 
-	rockchip_state = kzalloc(sizeof(*rockchip_state), GFP_KERNEL);
+	rockchip_state = kmemdup(to_rockchip_crtc_state(crtc->state),
+				 sizeof(*rockchip_state), GFP_KERNEL);
 	if (!rockchip_state)
 		return NULL;
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs
  2023-06-20  6:47 [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs Jonas Karlman
  2023-06-20  6:47 ` [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs Jonas Karlman
@ 2023-06-20  6:47 ` Jonas Karlman
  2023-06-21  8:05   ` Sascha Hauer
  2023-06-20  6:47 ` [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state Jonas Karlman
  2023-06-20  6:47 ` [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper Jonas Karlman
  3 siblings, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter
  Cc: Mark Yao, Andy Yan, Sascha Hauer, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel, Jonas Karlman

vop_plane_destroy and vop_crtc_destroy are plain wrappers around
drm_plane_cleanup and drm_crtc_cleanup. Use them directly as plane and
crtc funcs to closer match VOP2 driver.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 60b23636a3fe..25c873d4ff53 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -766,11 +766,6 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 	}
 }
 
-static void vop_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_cleanup(plane);
-}
-
 static inline bool rockchip_afbc(u64 modifier)
 {
 	return modifier == ROCKCHIP_AFBC_MOD;
@@ -1131,7 +1126,7 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = {
 static const struct drm_plane_funcs vop_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
-	.destroy = vop_plane_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
@@ -1602,11 +1597,6 @@ static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
 	.atomic_disable = vop_crtc_atomic_disable,
 };
 
-static void vop_crtc_destroy(struct drm_crtc *crtc)
-{
-	drm_crtc_cleanup(crtc);
-}
-
 static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct rockchip_crtc_state *rockchip_state;
@@ -1711,7 +1701,7 @@ vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 static const struct drm_crtc_funcs vop_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
-	.destroy = vop_crtc_destroy,
+	.destroy = drm_crtc_cleanup,
 	.reset = vop_crtc_reset,
 	.atomic_duplicate_state = vop_crtc_duplicate_state,
 	.atomic_destroy_state = vop_crtc_destroy_state,
@@ -1962,7 +1952,7 @@ static void vop_destroy_crtc(struct vop *vop)
 	 */
 	list_for_each_entry_safe(plane, tmp, &drm_dev->mode_config.plane_list,
 				 head)
-		vop_plane_destroy(plane);
+		drm_plane_cleanup(plane);
 
 	/*
 	 * Destroy CRTC after vop_plane_destroy() since vop_disable_plane()
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state
  2023-06-20  6:47 [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs Jonas Karlman
  2023-06-20  6:47 ` [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs Jonas Karlman
  2023-06-20  6:47 ` [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs Jonas Karlman
@ 2023-06-20  6:47 ` Jonas Karlman
  2023-06-21  8:06   ` Sascha Hauer
  2023-06-20  6:47 ` [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper Jonas Karlman
  3 siblings, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Andy Yan, Sascha Hauer, Rob Herring
  Cc: Mark Yao, dri-devel, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman

It's possible for users to try to duplicate the CRTC state even when the
state doesn't exist. drm_atomic_helper_crtc_duplicate_state() (and other
users of __drm_atomic_helper_crtc_duplicate_state()) already guard this
with a WARN_ON() instead of crashing, so let's do that here too.

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index ca73b8ccc29f..f725487d02ef 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2099,11 +2099,13 @@ static void vop2_crtc_reset(struct drm_crtc *crtc)
 
 static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc)
 {
-	struct rockchip_crtc_state *vcstate, *old_vcstate;
+	struct rockchip_crtc_state *vcstate;
 
-	old_vcstate = to_rockchip_crtc_state(crtc->state);
+	if (WARN_ON(!crtc->state))
+		return NULL;
 
-	vcstate = kmemdup(old_vcstate, sizeof(*old_vcstate), GFP_KERNEL);
+	vcstate = kmemdup(to_rockchip_crtc_state(crtc->state),
+			  sizeof(*vcstate), GFP_KERNEL);
 	if (!vcstate)
 		return NULL;
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper
  2023-06-20  6:47 [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs Jonas Karlman
                   ` (2 preceding siblings ...)
  2023-06-20  6:47 ` [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state Jonas Karlman
@ 2023-06-20  6:47 ` Jonas Karlman
  2023-06-21  8:11   ` Sascha Hauer
  3 siblings, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Rob Herring, Andy Yan, Sascha Hauer
  Cc: Mark Yao, dri-devel, linux-arm-kernel, linux-rockchip,
	linux-kernel, Jonas Karlman

Add missing call to crtc reset helper to properly vblank reset.

Also move vop2_crtc_reset and call vop2_crtc_destroy_state to simplify
and remove duplicated code.

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 28 ++++++++------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index f725487d02ef..1be84fe0208f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2080,23 +2080,6 @@ static const struct drm_crtc_helper_funcs vop2_crtc_helper_funcs = {
 	.atomic_disable = vop2_crtc_atomic_disable,
 };
 
-static void vop2_crtc_reset(struct drm_crtc *crtc)
-{
-	struct rockchip_crtc_state *vcstate = to_rockchip_crtc_state(crtc->state);
-
-	if (crtc->state) {
-		__drm_atomic_helper_crtc_destroy_state(crtc->state);
-		kfree(vcstate);
-	}
-
-	vcstate = kzalloc(sizeof(*vcstate), GFP_KERNEL);
-	if (!vcstate)
-		return;
-
-	crtc->state = &vcstate->base;
-	crtc->state->crtc = crtc;
-}
-
 static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct rockchip_crtc_state *vcstate;
@@ -2123,6 +2106,17 @@ static void vop2_crtc_destroy_state(struct drm_crtc *crtc,
 	kfree(vcstate);
 }
 
+static void vop2_crtc_reset(struct drm_crtc *crtc)
+{
+	struct rockchip_crtc_state *vcstate =
+		kzalloc(sizeof(*vcstate), GFP_KERNEL);
+
+	if (crtc->state)
+		vop2_crtc_destroy_state(crtc, crtc->state);
+
+	__drm_atomic_helper_crtc_reset(crtc, &vcstate->base);
+}
+
 static const struct drm_crtc_funcs vop2_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs
  2023-06-20  6:47 ` [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs Jonas Karlman
@ 2023-06-21  8:05   ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2023-06-21  8:05 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Mark Yao, Andy Yan, dri-devel, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Tue, Jun 20, 2023 at 06:47:36AM +0000, Jonas Karlman wrote:
> struct rockchip_crtc_state members such as output_type, output_bpc and
> enable_afbc is always reset to zero in the atomic_duplicate_state crtc
> funcs.
> 
> Fix this by using kmemdup on the subclass rockchip_crtc_state struct.
> 
> Fixes: 4e257d9eee23 ("drm/rockchip: get rid of rockchip_drm_crtc_mode_config")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index a530ecc4d207..60b23636a3fe 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1614,7 +1614,8 @@ static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!crtc->state))
>  		return NULL;
>  
> -	rockchip_state = kzalloc(sizeof(*rockchip_state), GFP_KERNEL);
> +	rockchip_state = kmemdup(to_rockchip_crtc_state(crtc->state),
> +				 sizeof(*rockchip_state), GFP_KERNEL);
>  	if (!rockchip_state)
>  		return NULL;
>  
> -- 
> 2.41.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs
  2023-06-20  6:47 ` [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs Jonas Karlman
@ 2023-06-21  8:05   ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2023-06-21  8:05 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Mark Yao, Andy Yan, dri-devel, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Tue, Jun 20, 2023 at 06:47:37AM +0000, Jonas Karlman wrote:
> vop_plane_destroy and vop_crtc_destroy are plain wrappers around
> drm_plane_cleanup and drm_crtc_cleanup. Use them directly as plane and
> crtc funcs to closer match VOP2 driver.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 60b23636a3fe..25c873d4ff53 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -766,11 +766,6 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
>  	}
>  }
>  
> -static void vop_plane_destroy(struct drm_plane *plane)
> -{
> -	drm_plane_cleanup(plane);
> -}
> -
>  static inline bool rockchip_afbc(u64 modifier)
>  {
>  	return modifier == ROCKCHIP_AFBC_MOD;
> @@ -1131,7 +1126,7 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = {
>  static const struct drm_plane_funcs vop_plane_funcs = {
>  	.update_plane	= drm_atomic_helper_update_plane,
>  	.disable_plane	= drm_atomic_helper_disable_plane,
> -	.destroy = vop_plane_destroy,
> +	.destroy = drm_plane_cleanup,
>  	.reset = drm_atomic_helper_plane_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> @@ -1602,11 +1597,6 @@ static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
>  	.atomic_disable = vop_crtc_atomic_disable,
>  };
>  
> -static void vop_crtc_destroy(struct drm_crtc *crtc)
> -{
> -	drm_crtc_cleanup(crtc);
> -}
> -
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>  	struct rockchip_crtc_state *rockchip_state;
> @@ -1711,7 +1701,7 @@ vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
>  static const struct drm_crtc_funcs vop_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> -	.destroy = vop_crtc_destroy,
> +	.destroy = drm_crtc_cleanup,
>  	.reset = vop_crtc_reset,
>  	.atomic_duplicate_state = vop_crtc_duplicate_state,
>  	.atomic_destroy_state = vop_crtc_destroy_state,
> @@ -1962,7 +1952,7 @@ static void vop_destroy_crtc(struct vop *vop)
>  	 */
>  	list_for_each_entry_safe(plane, tmp, &drm_dev->mode_config.plane_list,
>  				 head)
> -		vop_plane_destroy(plane);
> +		drm_plane_cleanup(plane);
>  
>  	/*
>  	 * Destroy CRTC after vop_plane_destroy() since vop_disable_plane()
> -- 
> 2.41.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state
  2023-06-20  6:47 ` [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state Jonas Karlman
@ 2023-06-21  8:06   ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2023-06-21  8:06 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Andy Yan, Rob Herring, Mark Yao, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel

On Tue, Jun 20, 2023 at 06:47:38AM +0000, Jonas Karlman wrote:
> It's possible for users to try to duplicate the CRTC state even when the
> state doesn't exist. drm_atomic_helper_crtc_duplicate_state() (and other
> users of __drm_atomic_helper_crtc_duplicate_state()) already guard this
> with a WARN_ON() instead of crashing, so let's do that here too.
> 
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index ca73b8ccc29f..f725487d02ef 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -2099,11 +2099,13 @@ static void vop2_crtc_reset(struct drm_crtc *crtc)
>  
>  static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
> -	struct rockchip_crtc_state *vcstate, *old_vcstate;
> +	struct rockchip_crtc_state *vcstate;
>  
> -	old_vcstate = to_rockchip_crtc_state(crtc->state);
> +	if (WARN_ON(!crtc->state))
> +		return NULL;
>  
> -	vcstate = kmemdup(old_vcstate, sizeof(*old_vcstate), GFP_KERNEL);
> +	vcstate = kmemdup(to_rockchip_crtc_state(crtc->state),
> +			  sizeof(*vcstate), GFP_KERNEL);
>  	if (!vcstate)
>  		return NULL;
>  
> -- 
> 2.41.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper
  2023-06-20  6:47 ` [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper Jonas Karlman
@ 2023-06-21  8:11   ` Sascha Hauer
  2023-06-21 15:41     ` Jonas Karlman
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Hauer @ 2023-06-21  8:11 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Rob Herring, Andy Yan, Mark Yao, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel

On Tue, Jun 20, 2023 at 06:47:39AM +0000, Jonas Karlman wrote:
> Add missing call to crtc reset helper to properly vblank reset.
> 
> Also move vop2_crtc_reset and call vop2_crtc_destroy_state to simplify
> and remove duplicated code.
> 
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 28 ++++++++------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index f725487d02ef..1be84fe0208f 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -2080,23 +2080,6 @@ static const struct drm_crtc_helper_funcs vop2_crtc_helper_funcs = {
>  	.atomic_disable = vop2_crtc_atomic_disable,
>  };
>  
> -static void vop2_crtc_reset(struct drm_crtc *crtc)
> -{
> -	struct rockchip_crtc_state *vcstate = to_rockchip_crtc_state(crtc->state);
> -
> -	if (crtc->state) {
> -		__drm_atomic_helper_crtc_destroy_state(crtc->state);
> -		kfree(vcstate);
> -	}
> -
> -	vcstate = kzalloc(sizeof(*vcstate), GFP_KERNEL);
> -	if (!vcstate)
> -		return;
> -
> -	crtc->state = &vcstate->base;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>  	struct rockchip_crtc_state *vcstate;
> @@ -2123,6 +2106,17 @@ static void vop2_crtc_destroy_state(struct drm_crtc *crtc,
>  	kfree(vcstate);
>  }
>  
> +static void vop2_crtc_reset(struct drm_crtc *crtc)
> +{
> +	struct rockchip_crtc_state *vcstate =
> +		kzalloc(sizeof(*vcstate), GFP_KERNEL);
> +
> +	if (crtc->state)
> +		vop2_crtc_destroy_state(crtc, crtc->state);
> +
> +	__drm_atomic_helper_crtc_reset(crtc, &vcstate->base);
> +}

You missed to check for allocation failures before using vcstate.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper
  2023-06-21  8:11   ` Sascha Hauer
@ 2023-06-21 15:41     ` Jonas Karlman
  0 siblings, 0 replies; 10+ messages in thread
From: Jonas Karlman @ 2023-06-21 15:41 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Sandy Huang, Heiko Stuebner, David Airlie, Daniel Vetter,
	Rob Herring, Andy Yan, Mark Yao, dri-devel, linux-arm-kernel,
	linux-rockchip, linux-kernel

On 2023-06-21 10:11, Sascha Hauer wrote:
> On Tue, Jun 20, 2023 at 06:47:39AM +0000, Jonas Karlman wrote:
>> Add missing call to crtc reset helper to properly vblank reset.
>>
>> Also move vop2_crtc_reset and call vop2_crtc_destroy_state to simplify
>> and remove duplicated code.
>>
>> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 28 ++++++++------------
>>  1 file changed, 11 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index f725487d02ef..1be84fe0208f 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -2080,23 +2080,6 @@ static const struct drm_crtc_helper_funcs vop2_crtc_helper_funcs = {
>>  	.atomic_disable = vop2_crtc_atomic_disable,
>>  };
>>  
>> -static void vop2_crtc_reset(struct drm_crtc *crtc)
>> -{
>> -	struct rockchip_crtc_state *vcstate = to_rockchip_crtc_state(crtc->state);
>> -
>> -	if (crtc->state) {
>> -		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>> -		kfree(vcstate);
>> -	}
>> -
>> -	vcstate = kzalloc(sizeof(*vcstate), GFP_KERNEL);
>> -	if (!vcstate)
>> -		return;
>> -
>> -	crtc->state = &vcstate->base;
>> -	crtc->state->crtc = crtc;
>> -}
>> -
>>  static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc)
>>  {
>>  	struct rockchip_crtc_state *vcstate;
>> @@ -2123,6 +2106,17 @@ static void vop2_crtc_destroy_state(struct drm_crtc *crtc,
>>  	kfree(vcstate);
>>  }
>>  
>> +static void vop2_crtc_reset(struct drm_crtc *crtc)
>> +{
>> +	struct rockchip_crtc_state *vcstate =
>> +		kzalloc(sizeof(*vcstate), GFP_KERNEL);
>> +
>> +	if (crtc->state)
>> +		vop2_crtc_destroy_state(crtc, crtc->state);
>> +
>> +	__drm_atomic_helper_crtc_reset(crtc, &vcstate->base);
>> +}
> 
> You missed to check for allocation failures before using vcstate.

Good catch, I will fix for both vop and vop2 driver in v2.

Regards,
Jonas

> 
> Sascha
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-06-21 15:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-20  6:47 [PATCH 0/4] drm/rockchip: Fix crtc duplicate state and crtc reset funcs Jonas Karlman
2023-06-20  6:47 ` [PATCH 1/4] drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs Jonas Karlman
2023-06-21  8:05   ` Sascha Hauer
2023-06-20  6:47 ` [PATCH 2/4] drm/rockchip: vop: Use cleanup helper directly as destroy funcs Jonas Karlman
2023-06-21  8:05   ` Sascha Hauer
2023-06-20  6:47 ` [PATCH 3/4] drm/rockchip: vop2: Don't crash for invalid duplicate_state Jonas Karlman
2023-06-21  8:06   ` Sascha Hauer
2023-06-20  6:47 ` [PATCH 4/4] drm/rockchip: vop2: Add missing call to crtc reset helper Jonas Karlman
2023-06-21  8:11   ` Sascha Hauer
2023-06-21 15:41     ` Jonas Karlman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox