linux-kernel - Re: [PATCH] drm/panel: samsung: s6e8aa0: Add backlight control support

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: <08a83744-ed02-d641-e2a4-db2247ae53de@iki.fi>
Date:   Mon, 23 Sep 2019 20:57:17 +0300
From:   Joonas Kylmälä <joonas.kylmala@....fi>
To:     Andrzej Hajda <a.hajda@...sung.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Cc:     paul.kocialkowski@...tlin.com, GNUtoo@...erdimension.org
Subject: Re: [PATCH] drm/panel: samsung: s6e8aa0: Add backlight control
 support

Hi,

thanks a lot for the review, Andrzej!

Andrzej Hajda:
>> +static const struct backlight_ops s6e8aa0_backlight_ops = {
>> +	.update_status	= s6e8aa0_set_brightness,
> 
> 
> This is racy, update_status can be called in any time between probe and
> remove, particularly:
> 
> a) before panel enable,
> 
> b) during panel enable,
> 
> c) when panel is enabled,
> 
> d) during panel disable,
> 
> e) after panel disable,
> 
> 
> b and d are racy for sure - backlight and drm callbacks are async.
> 
> IMO the best solution would be to register backlight after attaching
> panel to drm, but for this drm_panel_funcs should have attach/detach
> callbacks (like drm_bridge_funcs),
> 
> then update_status callback should take some drm_connector lock to
> synchronize with drm, and write to hw only when pipe is enabled.

I will start reading the kernel DRM KMS documentation in order to learn
how the attach callback would fit in the picture and do what you suggest
but it might take a few weeks or months.

Also, as a lot of this code was mimicked from
drivers/gpu/drm/panel/panel-samsung-s6e63m0.c it looks like that is also
having this race condition. Unfortunately, I don't think I have the
s6e63m0 panel so for that I probably cannot fix this issue.

Joonas