Re: [PATCH v4 28/41] drm/atmel-hlcdc: Drop spurious csc_init call from reset

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 08.09.26 um 16:46 schrieb Maxime Ripard:
> The lcdc_csc_init callback was introduced in commit aa71584b323a ("drm:
> atmel-hlcdc: add driver ops to differentiate HLCDC and XLCDC IP") and
> called only once, at init time, from atmel_hlcdc_plane_init_properties().
>
> commit 81af99cbd9e4 ("drm/atmel-hlcdc: destroy properly the plane state
> in the reset callback") then reworked the reset hook to use
> atmel_hlcdc_plane_atomic_destroy_state() instead of duplicating the
> code. However, it also introduced a new call to lcdc_csc_init in the
> reset path that wasn't there before and wasn't mentioned in the commit
> log.
>
> Since CSC coefficients are hardware constants that only need to be
> written once at init time. This also prevents the conversion to
> atomic_create_state, which must not have any hardware side-effect.
>
> Fixes: 81af99cbd9e4 ("drm/atmel-hlcdc: destroy properly the plane state in the reset callback")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

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

This looks reasonable, but I don't dare giving it a full r-b.

>
> ---
> Cc: Dharma Balasubiramani <dharma.b@microchip.com>
> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
> Cc: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index f17a832351e9..70a1cef24388 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -1184,12 +1184,10 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
>   }
>   
>   static void atmel_hlcdc_plane_reset(struct drm_plane *p)
>   {
>   	struct atmel_hlcdc_plane_state *state;
> -	struct atmel_hlcdc_dc *dc = p->dev->dev_private;
> -	struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
>   
>   	if (p->state) {
>   		atmel_hlcdc_plane_atomic_destroy_state(p, p->state);
>   		p->state = NULL;
>   	}
> @@ -1202,13 +1200,10 @@ static void atmel_hlcdc_plane_reset(struct drm_plane *p)
>   				"Failed to allocate initial plane state\n");
>   			return;
>   		}
>   		__drm_atomic_helper_plane_reset(p, &state->base);
>   	}
> -
> -	if (plane->layer.desc->layout.csc)
> -		dc->desc->ops->lcdc_csc_init(plane, plane->layer.desc);
>   }
>   
>   static const struct drm_plane_funcs layer_plane_funcs = {
>   	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)