| Message ID | 20181219112033.31064-1-nsaenzjulienne@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/panel: rpi-touchscreen: Add backlight support | expand |
Nicolas Saenz Julienne <nsaenzjulienne@suse.de> 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 <nsaenzjulienne@suse.de> > --- > 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
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 Gordon On Thu, 20 Dec 2018, 18:33 Eric Anholt, <eric@anholt.net> wrote: > Nicolas Saenz Julienne <nsaenzjulienne@suse.de> 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 <nsaenzjulienne@suse.de> > > --- > > 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 >
On Thu, 2018-12-20 at 18:36 +0000, Gordon Hollingworth wrote: > 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 Well we're already in such a situation without this patch, as the panel enables the back-light though the I2C lines. This is only enabled by an overlay, it's left to the user's discretion. As commented previously by Alex Graf and given the constraints, I think that it would make sense to start considering both options as good (using FW & direct I2C access). We are defaulting to the FW based one, but providing support/overlays for the direct access doesn't seem a bad compromise to me. I do understand Gordon's concerns. Maybe it would be nice for the firmware to detect device tree nodes accessing I2C0 and disable the camera. Or providing a FW configuration option. Kind regards, Nicolas
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);
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. Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> --- 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(-)