Re: [PATCH] drm/ssd130x: Init display before the SSD130X_DISPLAY_ON command

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2912 bytes --]



Am 25.01.23 um 19:42 schrieb Javier Martinez Canillas:
> Commit 622113b9f11f ("drm/ssd130x: Replace simple display helpers with the
> atomic helpers") changed the driver to just use the atomic helpers instead
> of the simple KMS abstraction layer.
> 
> But the commit also made a subtle change on the display power sequence and
> initialization order, by moving the ssd130x_power_on() call to the encoder
> .atomic_enable handler and the ssd130x_init() call to CRTC .reset handler.
> 
> Before this change, both ssd130x_power_on() and ssd130x_init() were called
> in the simple display pipeline .enable handler, so the display was already
> initialized by the time the SSD130X_DISPLAY_ON command was sent.
> 
> For some reasons, it only made the ssd130x SPI driver to fail but the I2C
> was still working. That is the reason why the bug was not noticed before.
> 
> To revert to the old driver behavior, move the ssd130x_init() call to the
> encoder .atomic_enable as well. Besides fixing the panel not being turned
> on when using SPI, it also gets rid of the custom CRTC .reset callback.
> 
> Fixes: 622113b9f11f ("drm/ssd130x: Replace simple display helpers with the atomic helpers")
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

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

> ---
> 
>   drivers/gpu/drm/solomon/ssd130x.c | 18 +++++++-----------
>   1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index b16330a8b624..8cbf5aa66e19 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -663,18 +663,8 @@ static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs = {
>   	.atomic_check = drm_crtc_helper_atomic_check,
>   };
>   
> -static void ssd130x_crtc_reset(struct drm_crtc *crtc)
> -{
> -	struct drm_device *drm = crtc->dev;
> -	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
> -
> -	ssd130x_init(ssd130x);
> -
> -	drm_atomic_helper_crtc_reset(crtc);
> -}
> -
>   static const struct drm_crtc_funcs ssd130x_crtc_funcs = {
> -	.reset = ssd130x_crtc_reset,
> +	.reset = drm_atomic_helper_crtc_reset,
>   	.destroy = drm_crtc_cleanup,
>   	.set_config = drm_atomic_helper_set_config,
>   	.page_flip = drm_atomic_helper_page_flip,
> @@ -693,6 +683,12 @@ static void ssd130x_encoder_helper_atomic_enable(struct drm_encoder *encoder,
>   	if (ret)
>   		return;
>   
> +	ret = ssd130x_init(ssd130x);
> +	if (ret) {
> +		ssd130x_power_off(ssd130x);
> +		return;
> +	}
> +
>   	ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_ON);
>   
>   	backlight_enable(ssd130x->bl_dev);

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]