linux-kernel - Re: [PATCH] drm/ssd130x: Init display before the SSD130X_DISPLAY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82ad8c89-9d6a-ffa8-e2ad-7b53ac27cafb@suse.de>
Date:   Wed, 25 Jan 2023 20:56:17 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Javier Martinez Canillas <javierm@...hat.com>,
        linux-kernel@...r.kernel.org
Cc:     Maxime Ripard <maxime@...no.tech>, Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...il.com>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/ssd130x: Init display before the SSD130X_DISPLAY_ON
 command



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@...hat.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@...e.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

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)