[PATCH] drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call

[PATCH] drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call

[Lucas Stach]

Old and new state parameters are swapped, so the old state was cleared
instead of the new duplicated state.

Fixes: 903674588a48 ("drm/atomic-helper: Add format-conversion state to shadow-plane state")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Leonard Göhrs <l.goehrs@pengutronix.de>
---
 drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e440f458b663..93337543aac3 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -224,8 +224,8 @@ __drm_gem_duplicate_shadow_plane_state(struct drm_plane *plane,
 
 	__drm_atomic_helper_plane_duplicate_state(plane, &new_shadow_plane_state->base);
 
-	drm_format_conv_state_copy(&shadow_plane_state->fmtcnv_state,
-				   &new_shadow_plane_state->fmtcnv_state);
+	drm_format_conv_state_copy(&new_shadow_plane_state->fmtcnv_state,
+				   &shadow_plane_state->fmtcnv_state);
 }
 EXPORT_SYMBOL(__drm_gem_duplicate_shadow_plane_state);
 
-- 
2.39.2

Re: [PATCH] drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call

[Lucas Stach]

Am Donnerstag, dem 04.04.2024 um 10:17 +0200 schrieb Lucas Stach:
> Old and new state parameters are swapped, so the old state was cleared
> instead of the new duplicated state.
> 
To be clear, as the commit message may do a poor job at conveying the
consequences: this fixes a major memory leak when a temporary buffer is
used for the format conversion, as clearing the wrong state lets us
forget about the existence of the temporary buffer on each atomic
commit. So each commit allocates a new temp buffer while the old one is
never freed.

As such I would appreciate if this commit is added to the next round of
-fixes.

Regards,
Lucas

> Fixes: 903674588a48 ("drm/atomic-helper: Add format-conversion state to shadow-plane state")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Tested-by: Leonard Göhrs <l.goehrs@pengutronix.de>
> ---
>  drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
> index e440f458b663..93337543aac3 100644
> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
> @@ -224,8 +224,8 @@ __drm_gem_duplicate_shadow_plane_state(struct drm_plane *plane,
>  
>  	__drm_atomic_helper_plane_duplicate_state(plane, &new_shadow_plane_state->base);
>  
> -	drm_format_conv_state_copy(&shadow_plane_state->fmtcnv_state,
> -				   &new_shadow_plane_state->fmtcnv_state);
> +	drm_format_conv_state_copy(&new_shadow_plane_state->fmtcnv_state,
> +				   &shadow_plane_state->fmtcnv_state);
>  }
>  EXPORT_SYMBOL(__drm_gem_duplicate_shadow_plane_state);
>  

Re: [PATCH] drm/atomic-helper: fix parameter order in drm_format_conv_state_copy() call

[Thomas Zimmermann]

Hi,

thanks for this fix.

Am 04.04.24 um 10:17 schrieb Lucas Stach:
> Old and new state parameters are swapped, so the old state was cleared
> instead of the new duplicated state.
>
> Fixes: 903674588a48 ("drm/atomic-helper: Add format-conversion state to shadow-plane state")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Tested-by: Leonard Göhrs <l.goehrs@pengutronix.de>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Please also add

Cc: <stable@vger.kernel.org> # v6.8+

Best regards
Thomas

> ---
>   drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
> index e440f458b663..93337543aac3 100644
> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
> @@ -224,8 +224,8 @@ __drm_gem_duplicate_shadow_plane_state(struct drm_plane *plane,
>   
>   	__drm_atomic_helper_plane_duplicate_state(plane, &new_shadow_plane_state->base);
>   
> -	drm_format_conv_state_copy(&shadow_plane_state->fmtcnv_state,
> -				   &new_shadow_plane_state->fmtcnv_state);
> +	drm_format_conv_state_copy(&new_shadow_plane_state->fmtcnv_state,
> +				   &shadow_plane_state->fmtcnv_state);
>   }
>   EXPORT_SYMBOL(__drm_gem_duplicate_shadow_plane_state);
>   

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)