Assuming this is using the firmware interface then yes it's fine. If it is using the i2c directly then it's possible to clash with the GPU driving the camera<div> </div><div>Gordon <div class="gmail_quote"><div dir="ltr">On Thu, 20 Dec 2018, 18:33 Eric Anholt, <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Nicolas Saenz Julienne <<a href="mailto:nsaenzjulienne@suse.de" target="_blank">nsaenzjulienne@suse.de</a>> writes: > This patch exposes backlight control into userspace by creating a > backlight device instead of directly accessing the PWM registers. > > The backlight driver can't go on a separate file as it shares the I2C > bus & address with the panel. I remember some concerns from Gordon about exposing the backlight PWM. Gordon, care to chime in on this? > Signed-off-by: Nicolas Saenz Julienne <<a href="mailto:nsaenzjulienne@suse.de" target="_blank">nsaenzjulienne@suse.de</a>> > --- > v1 -> v2: > - Add Kconfig dependency with BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/panel/Kconfig | 1 + > .../drm/panel/panel-raspberrypi-touchscreen.c | 33 +++++++++++++++++-- > 2 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index 6020c30a33b3..1ce35483f342 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -112,6 +112,7 @@ config DRM_PANEL_PANASONIC_VVX10F034N00 > config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN > tristate "Raspberry Pi 7-inch touchscreen panel" > depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > help > Say Y here if you want to enable support for the Raspberry > Pi 7" Touchscreen. To compile this driver as a module, > diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > index 2c9c9722734f..838b5c8767bc 100644 > --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c > @@ -52,6 +52,7 @@ > #include <linux/of_device.h> > #include <linux/of_graph.h> > #include <linux/pm.h> > +#include <linux/backlight.h> > > #include <drm/drm_panel.h> > #include <drm/drmP.h> > @@ -196,6 +197,7 @@ struct rpi_touchscreen { > struct drm_panel base; > struct mipi_dsi_device *dsi; > struct i2c_client *i2c; > + struct backlight_device *backlight; > }; > > static const struct drm_display_mode rpi_touchscreen_modes[] = { > @@ -256,7 +258,8 @@ static int rpi_touchscreen_disable(struct drm_panel *panel) > { > struct rpi_touchscreen *ts = panel_to_ts(panel); > > - rpi_touchscreen_i2c_write(ts, REG_PWM, 0); > + ts->backlight->props.brightness = 0; > + backlight_update_status(ts->backlight); > > rpi_touchscreen_i2c_write(ts, REG_POWERON, 0); > udelay(1); > @@ -300,7 +303,8 @@ static int rpi_touchscreen_enable(struct drm_panel *panel) > msleep(100); > > /* Turn on the backlight. */ > - rpi_touchscreen_i2c_write(ts, REG_PWM, 255); > + ts->backlight->props.brightness = 255; > + backlight_update_status(ts->backlight); > > /* Default to the same orientation as the closed source > * firmware used for the panel. Runtime rotation > @@ -358,12 +362,26 @@ static const struct drm_panel_funcs rpi_touchscreen_funcs = { > .get_modes = rpi_touchscreen_get_modes, > }; > > +static int raspberrypi_bl_update_status(struct backlight_device *bl) > +{ > + struct rpi_touchscreen *ts = bl_get_data(bl); > + > + rpi_touchscreen_i2c_write(ts, REG_PWM, bl->props.brightness); > + > + return 0; > +} > + > +static const struct backlight_ops raspberrypi_bl_ops = { > + .update_status = raspberrypi_bl_update_status, > +}; > + > static int rpi_touchscreen_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > struct device *dev = &i2c->dev; > struct rpi_touchscreen *ts; > struct device_node *endpoint, *dsi_host_node; > + struct backlight_properties props; > struct mipi_dsi_host *host; > int ret, ver; > struct mipi_dsi_device_info info = { > @@ -398,6 +416,17 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c, > /* Turn off at boot, so we can cleanly sequence powering on. */ > rpi_touchscreen_i2c_write(ts, REG_POWERON, 0); > > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_RAW; > + props.max_brightness = 255; > + ts->backlight = devm_backlight_device_register(dev, dev_name(dev), dev, > + ts, &raspberrypi_bl_ops, > + &props); > + if (IS_ERR(ts->backlight)) { > + dev_err(dev, "Failed to create backlight device\n"); > + return PTR_ERR(ts->backlight); > + } > + > /* Look up the DSI host. It needs to probe before we do. */ > endpoint = of_graph_get_next_endpoint(dev->of_node, NULL); > dsi_host_node = of_graph_get_remote_port_parent(endpoint); > -- > 2.19.2 </blockquote></div></div>-- <div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">-- Director of Software Engineering, Raspberry Pi (Trading) Limited t: <a href="tel:%2B44%20%280%29%201223%20322633" value="+441223322633" style="color:rgb(17,85,204);font-family:arial,sans-serif;font-size:12.8px;line-height:normal">+44 (0) 1223 322633</a> m: <a href="tel:%2B44%20%280%29%207450%20946289" value="+447450946289" style="color:rgb(17,85,204);font-family:arial,sans-serif;font-size:12.8px;line-height:normal">+44 (0) 7450 946289</a> e: <a href="mailto:gordon@raspberrypi.org" style="color:rgb(17,85,204);font-family:arial,sans-serif;font-size:12.8px;line-height:normal">gordon@raspberrypi.org</a> w: <a href="http://www.raspberrypi.org/" rel="noreferrer" target="_blank" style="color:rgb(17,85,204);font-family:arial,sans-serif;font-size:12.8px;line-height:normal">www.raspberrypi.org</a> <div> </div></div></div>