Hi Joonas.
On Sat, Apr 04, 2020 at 04:27:02PM +0300, Joonas Kylmälä wrote:
Hi,
addressing this email to you all since there might be widespread race condition issue in the DRM panel drivers that are using MIPI DSI. See below for my message.
Andrzej Hajda:
+static int s6e8aa0_set_brightness(struct backlight_device *bd) +{
- struct s6e8aa0 *ctx = bl_get_data(bd);
- const u8 *gamma;
- if (ctx->error)
return;- gamma = ctx->variant->gamma_tables[bd->props.brightness];
- if (ctx->version >= 142)
s6e8aa0_elvss_nvm_set(ctx);- s6e8aa0_dcs_write(ctx, gamma, GAMMA_TABLE_LEN);
- /* update gamma table. */
- s6e8aa0_dcs_write_seq_static(ctx, 0xf7, 0x03);
- return s6e8aa0_clear_error(ctx);
+}
+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 have done now research and if I understand right one issue here might be with setting the backlight brightness if the DSI device is not attached before calling update_status() since calling it would call subsequently s6e8aa0_set_brightness() -> s6e8aa0_dcs_write() -> mipi_dsi_dcs_write_buffer(), which then requires DSI to be attached.
Not directly related to your comments above. But I have looked at the backlight support for the various samsung panels.
None of them are good examples to follow. Please have a look at for example panel-novatek-nt35510.c which is a good example how to have a local backligth and tie it into the general way it is used by drm_panel.
I have typed patches to fix all three samsung panels, will post patches later when I get more time.
If we are concerned with set_brightness() being called while not ready, this can be checked in the set_brightness() function and return error if not OK.
As the the race concerns see Daniel's reply.
Sam