[PATCH v2] drm/panel: rpi-touchscreen: Add backlight support

[PATCH v2] drm/panel: rpi-touchscreen: Add backlight support

[Nicolas Saenz Julienne]

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(-)

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

Re: [PATCH v2] drm/panel: rpi-touchscreen: Add backlight support

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 4514 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v2] drm/panel: rpi-touchscreen: Add backlight support

[Gordon Hollingworth]

[-- Attachment #1.1: Type: text/plain, Size: 5342 bytes --]

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
>
-- 
--
Director of Software Engineering, Raspberry Pi (Trading) Limited
t:  +44 (0) 1223 322633
m: +44 (0) 7450 946289
e: gordon@raspberrypi.org
w: www.raspberrypi.org

[-- Attachment #1.2: Type: text/html, Size: 8944 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/panel: rpi-touchscreen: Add backlight support

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]