[PATCH v16 00/10] Add backlight helper functions

[PATCH v16 00/10] Add backlight helper functions

[Meghana Madhyastha]

Move drm helper functions from tinydrm-helpers to linux/backlight for
ease of use by callers in other drivers.

Changes in v16:
-Add a comment about setting brightness = max_brightness in of_find_backlight
-Add dri-devel and linux-kernel mailing lists

Meghana Madhyastha (10):
  video: backlight: Add helpers to enable and disable backlight
  drm/tinydrm: Convert tinydrm_enable/disable_backlight to
    backlight_enable/disable
  video: backlight: Add of_find_backlight helper in backlight.c
  drm/tinydrm: Replace tinydrm_of_find_backlight with of_find_backlight
  video: backlight: Add devres versions of of_find_backlight
  drm/tinydrm: Call devres version of of_find_backlight
  drm/panel: Use backlight_enable/disable helpers
  drm/omapdrm: Use backlight_enable/disable helpers
  drm/panel: Use of_find_backlight helper
  drm/omapdrm: Use of_find_backlight helper

 drivers/gpu/drm/omapdrm/displays/panel-dpi.c    | 22 ++----
 drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 16 ++---
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c  |  6 +-
 drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 22 ++----
 drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 22 ++----
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c  | 95 -------------------------
 drivers/gpu/drm/tinydrm/mi0283qt.c              |  3 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c              |  4 +-
 drivers/video/backlight/backlight.c             | 69 ++++++++++++++++++
 include/drm/tinydrm/tinydrm-helpers.h           |  4 --
 include/linux/backlight.h                       | 56 +++++++++++++++
 11 files changed, 157 insertions(+), 162 deletions(-)

-- 
2.11.0

[PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Meghana Madhyastha]

Add helper functions backlight_enable and backlight_disable to
enable/disable a backlight device. These helper functions can
then be used by different drm and tinydrm drivers to avoid
repetition of code and also to enforce a uniform and consistent
way to enable/disable a backlight device.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index af7003548..7b6a9a2a3 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -130,6 +130,36 @@ static inline int backlight_update_status(struct backlight_device *bd)
 	return ret;
 }
 
+/**
+  * backlight_enable - Enable backlight
+  * @bd: the backlight device to enable
+  */
+static inline int backlight_enable(struct backlight_device *bd)
+{
+	if (!bd)
+		return 0;
+
+	bd->props.power = FB_BLANK_UNBLANK;
+	bd->props.state &= ~BL_CORE_FBBLANK;
+
+	return backlight_update_status(bd);
+}
+
+/**
+  * backlight_disable - Disable backlight
+  * @bd: the backlight device to disable
+  */
+static inline int backlight_disable(struct backlight_device *bd)
+{
+	if (!bd)
+		return 0;
+		
+	bd->props.power = FB_BLANK_POWERDOWN;
+	bd->props.state |= BL_CORE_FBBLANK;
+
+	return backlight_update_status(bd);
+}
+
 extern struct backlight_device *backlight_device_register(const char *name,
 	struct device *dev, void *devdata, const struct backlight_ops *ops,
 	const struct backlight_properties *props);
-- 
2.11.0

[PATCH v16 02/10] drm/tinydrm: Convert tinydrm_enable/disable_backlight to backlight_enable/disable

[Meghana Madhyastha]

Remove tinydrm_enable/disable_backlight and let the callers call the
more generic backlight_enable/disable helpers

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 55 --------------------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c             |  4 +-
 include/drm/tinydrm/tinydrm-helpers.h          |  2 -
 3 files changed, 2 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bf96072d1..7326e1758 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -276,61 +276,6 @@ struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
 }
 EXPORT_SYMBOL(tinydrm_of_find_backlight);
 
-/**
- * tinydrm_enable_backlight - Enable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_enable_backlight(struct backlight_device *backlight)
-{
-	unsigned int old_state;
-	int ret;
-
-	if (!backlight)
-		return 0;
-
-	old_state = backlight->props.state;
-	backlight->props.state &= ~BL_CORE_FBBLANK;
-	DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
-		      backlight->props.state);
-
-	ret = backlight_update_status(backlight);
-	if (ret)
-		DRM_ERROR("Failed to enable backlight %d\n", ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(tinydrm_enable_backlight);
-
-/**
- * tinydrm_disable_backlight - Disable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_disable_backlight(struct backlight_device *backlight)
-{
-	unsigned int old_state;
-	int ret;
-
-	if (!backlight)
-		return 0;
-
-	old_state = backlight->props.state;
-	backlight->props.state |= BL_CORE_FBBLANK;
-	DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
-		      backlight->props.state);
-	ret = backlight_update_status(backlight);
-	if (ret)
-		DRM_ERROR("Failed to disable backlight %d\n", ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(tinydrm_disable_backlight);
-
 #if IS_ENABLED(CONFIG_SPI)
 
 /**
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index aa6b6ce56..8c2cb1cf2 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -291,7 +291,7 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (fb)
 		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
 
-	tinydrm_enable_backlight(mipi->backlight);
+	backlight_enable(mipi->backlight);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_enable);
 
@@ -330,7 +330,7 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 	mipi->enabled = false;
 
 	if (mipi->backlight)
-		tinydrm_disable_backlight(mipi->backlight);
+		backlight_disable(mipi->backlight);
 	else
 		mipi_dbi_blank(mipi);
 }
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index d554ded60..f54fae03e 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -47,8 +47,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
 			       struct drm_clip_rect *clip);
 
 struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
-int tinydrm_enable_backlight(struct backlight_device *backlight);
-int tinydrm_disable_backlight(struct backlight_device *backlight);
 
 size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
 bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
-- 
2.11.0

[PATCH v16 03/10] video: backlight: Add of_find_backlight helper in backlight.c

[Meghana Madhyastha]

Add of_find_backlight, a helper function which is a generic version
of tinydrm_of_find_backlight that can be used by other drivers to avoid
repetition of code and simplify things.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
Changes in v16:
-Add comment about brightness in of_find_backlight

 drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
 include/linux/backlight.h           | 19 ++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 8049e7656..7e4a5d77d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -580,6 +580,46 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
 EXPORT_SYMBOL(of_find_backlight_by_node);
 #endif
 
+/**
+  * of_find_backlight - Get backlight device
+  * @dev: Device
+  *
+  * This function looks for a property named 'backlight' on the DT node
+  * connected to @dev and looks up the backlight device.
+  *
+  * Call backlight_put() to drop the reference on the backlight device.
+  * gpio_backlight uses brightness as power state during probe.
+  *
+  * Returns:
+  * A pointer to the backlight device if found.
+  * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
+  * device is found.
+  * NULL if there's no backlight property.
+  */
+struct backlight_device *of_find_backlight(struct device *dev)
+{
+	struct backlight_device *bd = NULL;
+	struct device_node *np;
+
+	if (!dev)
+		return NULL;
+
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		np = of_parse_phandle(dev->of_node, "backlight", 0);
+		if (np) {
+			bd = of_find_backlight_by_node(np);
+			of_node_put(np);
+			if (!bd)
+				return ERR_PTR(-EPROBE_DEFER);
+			if (!bd->props.brightness)
+				bd->props.brightness = bd->props.max_brightness;
+		}
+	}
+
+	return bd;
+}
+EXPORT_SYMBOL(of_find_backlight);
+
 static void __exit backlight_class_exit(void)
 {
 	class_destroy(backlight_class);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 7b6a9a2a3..32ea510da 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -160,6 +160,16 @@ static inline int backlight_disable(struct backlight_device *bd)
 	return backlight_update_status(bd);
 }
 
+/**
+  * backlight_put - Drop backlight reference
+  * @bd: the backlight device to put
+  */
+static inline void backlight_put(struct backlight_device *bd)
+{
+	if (bd)
+		put_device(&bd->dev);
+}
+
 extern struct backlight_device *backlight_device_register(const char *name,
 	struct device *dev, void *devdata, const struct backlight_ops *ops,
 	const struct backlight_properties *props);
@@ -203,4 +213,13 @@ of_find_backlight_by_node(struct device_node *node)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+struct backlight_device *of_find_backlight(struct device *dev);
+#else
+static inline struct backlight_device *of_find_backlight(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 #endif
-- 
2.11.0

[PATCH v16 04/10] drm/tinydrm: Replace tinydrm_of_find_backlight with of_find_backlight

[Meghana Madhyastha]

Remove tinydrm_of_find_backlight from tinydrm-helpers.c. We now have
a generic of_find_backlight defined in backlight.c. Let the callers
of tinydrm_of_find_backlight call of_find_backlight.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --------------------------
 drivers/gpu/drm/tinydrm/mi0283qt.c             |  3 +-
 include/drm/tinydrm/tinydrm-helpers.h          |  2 --
 3 files changed, 2 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index 7326e1758..d1c3ce9ab 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -236,46 +236,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
 
-/**
- * tinydrm_of_find_backlight - Find backlight device in device-tree
- * @dev: Device
- *
- * This function looks for a DT node pointed to by a property named 'backlight'
- * and uses of_find_backlight_by_node() to get the backlight device.
- * Additionally if the brightness property is zero, it is set to
- * max_brightness.
- *
- * Returns:
- * NULL if there's no backlight property.
- * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
- * is found.
- * If the backlight device is found, a pointer to the structure is returned.
- */
-struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
-{
-	struct backlight_device *backlight;
-	struct device_node *np;
-
-	np = of_parse_phandle(dev->of_node, "backlight", 0);
-	if (!np)
-		return NULL;
-
-	backlight = of_find_backlight_by_node(np);
-	of_node_put(np);
-
-	if (!backlight)
-		return ERR_PTR(-EPROBE_DEFER);
-
-	if (!backlight->props.brightness) {
-		backlight->props.brightness = backlight->props.max_brightness;
-		DRM_DEBUG_KMS("Backlight brightness set to %d\n",
-			      backlight->props.brightness);
-	}
-
-	return backlight;
-}
-EXPORT_SYMBOL(tinydrm_of_find_backlight);
-
 #if IS_ENABLED(CONFIG_SPI)
 
 /**
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 674d40764..3f9065fb5 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -14,6 +14,7 @@
 #include <drm/tinydrm/ili9341.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
+#include <linux/backlight.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -190,7 +191,7 @@ static int mi0283qt_probe(struct spi_device *spi)
 	if (IS_ERR(mipi->regulator))
 		return PTR_ERR(mipi->regulator);
 
-	mipi->backlight = tinydrm_of_find_backlight(dev);
+	mipi->backlight = of_find_backlight(dev);
 	if (IS_ERR(mipi->backlight))
 		return PTR_ERR(mipi->backlight);
 
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index f54fae03e..0a4ddbc04 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -46,8 +46,6 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
 void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
 			       struct drm_clip_rect *clip);
 
-struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
-
 size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
 bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
 int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
-- 
2.11.0

[PATCH v16 05/10] video: backlight: Add devres versions of of_find_backlight

[Meghana Madhyastha]

Add devm_of_find_backlight and the corresponding release
function because some drivers use devres versions of functions
for acquiring device resources.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
 include/linux/backlight.h           |  7 +++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 7e4a5d77d..b3f76945f 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -620,6 +620,35 @@ struct backlight_device *of_find_backlight(struct device *dev)
 }
 EXPORT_SYMBOL(of_find_backlight);
 
+static void devm_backlight_put(void *data)
+{
+	backlight_put(data);
+}
+
+/**
+  * devm_of_find_backlight - Resource-managed of_find_backlight()
+  * @dev: Device
+  *
+  * Device managed version of of_find_backlight(). The reference on the backlight
+  * device is automatically dropped on driver detach.
+  */
+struct backlight_device *devm_of_find_backlight(struct device *dev)
+{
+	struct backlight_device *bd;
+	int ret;
+
+	bd = of_find_backlight(dev);
+	if (IS_ERR_OR_NULL(bd))
+		return bd;
+	ret = devm_add_action(dev, devm_backlight_put, bd);
+	if (ret) {
+		backlight_put(bd);
+		return ERR_PTR(ret);
+	}
+	return bd;
+}
+EXPORT_SYMBOL(devm_of_find_backlight);
+
 static void __exit backlight_class_exit(void)
 {
 	class_destroy(backlight_class);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 32ea510da..1d373f5a6 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -215,11 +215,18 @@ of_find_backlight_by_node(struct device_node *node)
 
 #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
 struct backlight_device *of_find_backlight(struct device *dev);
+struct backlight_device *devm_of_find_backlight(struct device *dev);
 #else
 static inline struct backlight_device *of_find_backlight(struct device *dev)
 {
 	return NULL;
 }
+
+static inline struct backlight_device *
+devm_of_find_backlight(struct device *dev)
+{
+	return NULL;
+}
 #endif
 
 #endif
-- 
2.11.0

[PATCH v16 06/10] drm/tinydrm: Call devres version of of_find_backlight

[Meghana Madhyastha]

Call devm_of_find_backlight (the devres version) instead of
of_find_backlight.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 3f9065fb5..4d8650bdc 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -191,7 +191,7 @@ static int mi0283qt_probe(struct spi_device *spi)
 	if (IS_ERR(mipi->regulator))
 		return PTR_ERR(mipi->regulator);
 
-	mipi->backlight = of_find_backlight(dev);
+	mipi->backlight = devm_of_find_backlight(dev);
 	if (IS_ERR(mipi->backlight))
 		return PTR_ERR(mipi->backlight);
 
-- 
2.11.0

[PATCH v16 07/10] drm/panel: Use backlight_enable/disable helpers

[Meghana Madhyastha]

Use backlight_enable/disable helpers instead of changing
the property and calling backlight_update_status for cleaner
and simpler code and also to avoid repetitions.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/panel/panel-innolux-p079zca.c   |  6 ++----
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c  |  6 ++----
 drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 12 ++++--------
 drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 12 ++++--------
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 6ba93449f..4c1b29eec 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -45,8 +45,7 @@ static int innolux_panel_disable(struct drm_panel *panel)
 	if (!innolux->enabled)
 		return 0;
 
-	innolux->backlight->props.power = FB_BLANK_POWERDOWN;
-	backlight_update_status(innolux->backlight);
+	backlight_disable(innolux->backlight);
 
 	err = mipi_dsi_dcs_set_display_off(innolux->link);
 	if (err < 0)
@@ -151,8 +150,7 @@ static int innolux_panel_enable(struct drm_panel *panel)
 	if (innolux->enabled)
 		return 0;
 
-	innolux->backlight->props.power = FB_BLANK_UNBLANK;
-	ret = backlight_update_status(innolux->backlight);
+	ret = backlight_enable(innolux->backlight);
 	if (ret) {
 		DRM_DEV_ERROR(panel->drm->dev,
 			      "Failed to enable backlight %d\n", ret);
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
index 5b2340ef7..0a94ab79a 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -192,8 +192,7 @@ static int jdi_panel_disable(struct drm_panel *panel)
 	if (!jdi->enabled)
 		return 0;
 
-	jdi->backlight->props.power = FB_BLANK_POWERDOWN;
-	backlight_update_status(jdi->backlight);
+	backlight_disable(jdi->backlight);
 
 	jdi->enabled = false;
 
@@ -289,8 +288,7 @@ static int jdi_panel_enable(struct drm_panel *panel)
 	if (jdi->enabled)
 		return 0;
 
-	jdi->backlight->props.power = FB_BLANK_UNBLANK;
-	backlight_update_status(jdi->backlight);
+	backlight_enable(jdi->backlight);
 
 	jdi->enabled = true;
 
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 3cce3ca19..1512ec4f3 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -96,10 +96,8 @@ static int sharp_panel_disable(struct drm_panel *panel)
 	if (!sharp->enabled)
 		return 0;
 
-	if (sharp->backlight) {
-		sharp->backlight->props.power = FB_BLANK_POWERDOWN;
-		backlight_update_status(sharp->backlight);
-	}
+	if (sharp->backlight)
+		backlight_disable(sharp->backlight);
 
 	sharp->enabled = false;
 
@@ -263,10 +261,8 @@ static int sharp_panel_enable(struct drm_panel *panel)
 	if (sharp->enabled)
 		return 0;
 
-	if (sharp->backlight) {
-		sharp->backlight->props.power = FB_BLANK_UNBLANK;
-		backlight_update_status(sharp->backlight);
-	}
+	if (sharp->backlight)
+		backlight_enable(sharp->backlight);
 
 	sharp->enabled = true;
 
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index 3aeb0bda4..a6af3257f 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -117,10 +117,8 @@ static int sharp_nt_panel_disable(struct drm_panel *panel)
 	if (!sharp_nt->enabled)
 		return 0;
 
-	if (sharp_nt->backlight) {
-		sharp_nt->backlight->props.power = FB_BLANK_POWERDOWN;
-		backlight_update_status(sharp_nt->backlight);
-	}
+	if (sharp_nt->backlight)
+		backlight_disable(sharp_nt->backlight);
 
 	sharp_nt->enabled = false;
 
@@ -203,10 +201,8 @@ static int sharp_nt_panel_enable(struct drm_panel *panel)
 	if (sharp_nt->enabled)
 		return 0;
 
-	if (sharp_nt->backlight) {
-		sharp_nt->backlight->props.power = FB_BLANK_UNBLANK;
-		backlight_update_status(sharp_nt->backlight);
-	}
+	if (sharp_nt->backlight)
+		backlight_enable(sharp_nt->backlight);
 
 	sharp_nt->enabled = true;
 
-- 
2.11.0

[PATCH v16 08/10] drm/omapdrm: Use backlight_enable/disable helpers

[Meghana Madhyastha]

Use backlight_enable/disable helpers instead of changing
the property and calling backlight_update_status for cleaner
and simpler code and also to avoid repetitions.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index e065f7e10..0803f64cb 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -88,10 +88,8 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
 
 	gpiod_set_value_cansleep(ddata->enable_gpio, 1);
 
-	if (ddata->backlight) {
-		ddata->backlight->props.power = FB_BLANK_UNBLANK;
-		backlight_update_status(ddata->backlight);
-	}
+	if (ddata->backlight)
+		backlight_enable(ddata->backlight);
 
 	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
 
@@ -106,10 +104,8 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
 	if (!omapdss_device_is_enabled(dssdev))
 		return;
 
-	if (ddata->backlight) {
-		ddata->backlight->props.power = FB_BLANK_POWERDOWN;
-		backlight_update_status(ddata->backlight);
-	}
+	if (ddata->backlight)
+		backlight_disable(ddata->backlight);
 
 	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
 	regulator_disable(ddata->vcc_supply);
-- 
2.11.0

[PATCH v16 09/10] drm/panel: Use of_find_backlight helper

[Meghana Madhyastha]

Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 10 +++-------
 drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 +++-------
 drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 +++-------
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 4c1b29eec..a69b0530f 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -230,14 +230,10 @@ static int innolux_panel_add(struct innolux_panel *innolux)
 		innolux->enable_gpio = NULL;
 	}
 
-	np = of_parse_phandle(dev->of_node, "backlight", 0);
-	if (np) {
-		innolux->backlight = of_find_backlight_by_node(np);
-		of_node_put(np);
+	innolux->backlight = devm_of_find_backlight(np);
 
-		if (!innolux->backlight)
-			return -EPROBE_DEFER;
-	}
+	if (IS_ERR(innolux->backlight))
+		return PTR_ERR(innolux->backlight);
 
 	drm_panel_init(&innolux->base);
 	innolux->base.funcs = &innolux_panel_funcs;
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 1512ec4f3..d5450c9d9 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -329,14 +329,10 @@ static int sharp_panel_add(struct sharp_panel *sharp)
 	if (IS_ERR(sharp->supply))
 		return PTR_ERR(sharp->supply);
 
-	np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
-	if (np) {
-		sharp->backlight = of_find_backlight_by_node(np);
-		of_node_put(np);
+	sharp->backlight = devm_of_find_backlight(np);
 
-		if (!sharp->backlight)
-			return -EPROBE_DEFER;
-	}
+	if (IS_ERR(sharp->backlight))
+		return PTR_ERR(sharp->backlight);
 
 	drm_panel_init(&sharp->base);
 	sharp->base.funcs = &sharp_panel_funcs;
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index a6af3257f..db31d8268 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -273,14 +273,10 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
 		gpiod_set_value(sharp_nt->reset_gpio, 0);
 	}
 
-	np = of_parse_phandle(dev->of_node, "backlight", 0);
-	if (np) {
-		sharp_nt->backlight = of_find_backlight_by_node(np);
-		of_node_put(np);
+	sharp_nt->backlight = devm_of_find_backlight(np);
 
-		if (!sharp_nt->backlight)
-			return -EPROBE_DEFER;
-	}
+	if (IS_ERR(sharp_nt->backlight))
+		return PTR_ERR(sharp_nt->backlight);
 
 	drm_panel_init(&sharp_nt->base);
 	sharp_nt->base.funcs = &sharp_nt_panel_funcs;
-- 
2.11.0

[PATCH v16 10/10] drm/omapdrm: Use of_find_backlight helper

[Meghana Madhyastha]

Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index 0803f64cb..4a5e707a1 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -186,14 +186,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
 	if (IS_ERR(ddata->vcc_supply))
 		return PTR_ERR(ddata->vcc_supply);
 
-	bl_node = of_parse_phandle(node, "backlight", 0);
-	if (bl_node) {
-		ddata->backlight = of_find_backlight_by_node(bl_node);
-		of_node_put(bl_node);
+	ddata->backlight = of_find_backlight(bl_node);
 
-		if (!ddata->backlight)
-			return -EPROBE_DEFER;
-	}
+	if (IS_ERR(ddata->backlight))
+		return PTR_ERR(ddata->backlight);
 
 	r = of_get_display_timing(node, "panel-timing", &timing);
 	if (r) {
-- 
2.11.0

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Noralf Trønnes]

Den 16.01.2018 11.31, skrev Meghana Madhyastha:
> Add helper functions backlight_enable and backlight_disable to
> enable/disable a backlight device. These helper functions can
> then be used by different drm and tinydrm drivers to avoid
> repetition of code and also to enforce a uniform and consistent
> way to enable/disable a backlight device.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

checkpatch complains:
-:23: WARNING: Block comments should align the * on each line
-:45: ERROR: trailing whitespace

With that fixed:
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> ---
>   include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index af7003548..7b6a9a2a3 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -130,6 +130,36 @@ static inline int backlight_update_status(struct backlight_device *bd)
>   	return ret;
>   }
>   
> +/**
> +  * backlight_enable - Enable backlight
> +  * @bd: the backlight device to enable
> +  */
> +static inline int backlight_enable(struct backlight_device *bd)
> +{
> +	if (!bd)
> +		return 0;
> +
> +	bd->props.power = FB_BLANK_UNBLANK;
> +	bd->props.state &= ~BL_CORE_FBBLANK;
> +
> +	return backlight_update_status(bd);
> +}
> +
> +/**
> +  * backlight_disable - Disable backlight
> +  * @bd: the backlight device to disable
> +  */
> +static inline int backlight_disable(struct backlight_device *bd)
> +{
> +	if (!bd)
> +		return 0;
> +		
> +	bd->props.power = FB_BLANK_POWERDOWN;
> +	bd->props.state |= BL_CORE_FBBLANK;
> +
> +	return backlight_update_status(bd);
> +}
> +
>   extern struct backlight_device *backlight_device_register(const char *name,
>   	struct device *dev, void *devdata, const struct backlight_ops *ops,
>   	const struct backlight_properties *props);

Re: [PATCH v16 02/10] drm/tinydrm: Convert tinydrm_enable/disable_backlight to backlight_enable/disable

[Noralf Trønnes]

Den 16.01.2018 11.32, skrev Meghana Madhyastha:
> Remove tinydrm_enable/disable_backlight and let the callers call the
> more generic backlight_enable/disable helpers
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

This patch needs to be rebased on some recent changes to mipi-dbi.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> ---
>   drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 55 --------------------------
>   drivers/gpu/drm/tinydrm/mipi-dbi.c             |  4 +-
>   include/drm/tinydrm/tinydrm-helpers.h          |  2 -
>   3 files changed, 2 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> index bf96072d1..7326e1758 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -276,61 +276,6 @@ struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
>   }
>   EXPORT_SYMBOL(tinydrm_of_find_backlight);
>   
> -/**
> - * tinydrm_enable_backlight - Enable backlight helper
> - * @backlight: Backlight device
> - *
> - * Returns:
> - * Zero on success, negative error code on failure.
> - */
> -int tinydrm_enable_backlight(struct backlight_device *backlight)
> -{
> -	unsigned int old_state;
> -	int ret;
> -
> -	if (!backlight)
> -		return 0;
> -
> -	old_state = backlight->props.state;
> -	backlight->props.state &= ~BL_CORE_FBBLANK;
> -	DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
> -		      backlight->props.state);
> -
> -	ret = backlight_update_status(backlight);
> -	if (ret)
> -		DRM_ERROR("Failed to enable backlight %d\n", ret);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(tinydrm_enable_backlight);
> -
> -/**
> - * tinydrm_disable_backlight - Disable backlight helper
> - * @backlight: Backlight device
> - *
> - * Returns:
> - * Zero on success, negative error code on failure.
> - */
> -int tinydrm_disable_backlight(struct backlight_device *backlight)
> -{
> -	unsigned int old_state;
> -	int ret;
> -
> -	if (!backlight)
> -		return 0;
> -
> -	old_state = backlight->props.state;
> -	backlight->props.state |= BL_CORE_FBBLANK;
> -	DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
> -		      backlight->props.state);
> -	ret = backlight_update_status(backlight);
> -	if (ret)
> -		DRM_ERROR("Failed to disable backlight %d\n", ret);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(tinydrm_disable_backlight);
> -
>   #if IS_ENABLED(CONFIG_SPI)
>   
>   /**
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index aa6b6ce56..8c2cb1cf2 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -291,7 +291,7 @@ void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
>   	if (fb)
>   		fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
>   
> -	tinydrm_enable_backlight(mipi->backlight);
> +	backlight_enable(mipi->backlight);
>   }
>   EXPORT_SYMBOL(mipi_dbi_pipe_enable);
>   
> @@ -330,7 +330,7 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
>   	mipi->enabled = false;
>   
>   	if (mipi->backlight)
> -		tinydrm_disable_backlight(mipi->backlight);
> +		backlight_disable(mipi->backlight);
>   	else
>   		mipi_dbi_blank(mipi);
>   }
> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
> index d554ded60..f54fae03e 100644
> --- a/include/drm/tinydrm/tinydrm-helpers.h
> +++ b/include/drm/tinydrm/tinydrm-helpers.h
> @@ -47,8 +47,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>   			       struct drm_clip_rect *clip);
>   
>   struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
> -int tinydrm_enable_backlight(struct backlight_device *backlight);
> -int tinydrm_disable_backlight(struct backlight_device *backlight);
>   
>   size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
>   bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);

Re: [PATCH v16 03/10] video: backlight: Add of_find_backlight helper in backlight.c

[Noralf Trønnes]

Den 16.01.2018 11.33, skrev Meghana Madhyastha:
> Add of_find_backlight, a helper function which is a generic version
> of tinydrm_of_find_backlight that can be used by other drivers to avoid
> repetition of code and simplify things.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
> Changes in v16:
> -Add comment about brightness in of_find_backlight
>
>   drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
>   include/linux/backlight.h           | 19 ++++++++++++++++++
>   2 files changed, 59 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 8049e7656..7e4a5d77d 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -580,6 +580,46 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
>   EXPORT_SYMBOL(of_find_backlight_by_node);
>   #endif
>   
> +/**
> +  * of_find_backlight - Get backlight device
> +  * @dev: Device
> +  *
> +  * This function looks for a property named 'backlight' on the DT node
> +  * connected to @dev and looks up the backlight device.
> +  *
> +  * Call backlight_put() to drop the reference on the backlight device.
> +  * gpio_backlight uses brightness as power state during probe.

I was thinking the gpio_backlight issue to be a comment with the code, 
not the docs.
Isn't this sentence a bit confusing as part of the docs?

checkpatch complains:
-:21: WARNING: Block comments should align the * on each line
-:72: WARNING: Block comments should align the * on each line

Noralf.

> +  *
> +  * Returns:
> +  * A pointer to the backlight device if found.
> +  * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> +  * device is found.
> +  * NULL if there's no backlight property.
> +  */
> +struct backlight_device *of_find_backlight(struct device *dev)
> +{
> +	struct backlight_device *bd = NULL;
> +	struct device_node *np;
> +
> +	if (!dev)
> +		return NULL;
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +		np = of_parse_phandle(dev->of_node, "backlight", 0);
> +		if (np) {
> +			bd = of_find_backlight_by_node(np);
> +			of_node_put(np);
> +			if (!bd)
> +				return ERR_PTR(-EPROBE_DEFER);
> +			if (!bd->props.brightness)
> +				bd->props.brightness = bd->props.max_brightness;
> +		}
> +	}
> +
> +	return bd;
> +}
> +EXPORT_SYMBOL(of_find_backlight);
> +
>   static void __exit backlight_class_exit(void)
>   {
>   	class_destroy(backlight_class);
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7b6a9a2a3..32ea510da 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -160,6 +160,16 @@ static inline int backlight_disable(struct backlight_device *bd)
>   	return backlight_update_status(bd);
>   }
>   
> +/**
> +  * backlight_put - Drop backlight reference
> +  * @bd: the backlight device to put
> +  */
> +static inline void backlight_put(struct backlight_device *bd)
> +{
> +	if (bd)
> +		put_device(&bd->dev);
> +}
> +
>   extern struct backlight_device *backlight_device_register(const char *name,
>   	struct device *dev, void *devdata, const struct backlight_ops *ops,
>   	const struct backlight_properties *props);
> @@ -203,4 +213,13 @@ of_find_backlight_by_node(struct device_node *node)
>   }
>   #endif
>   
> +#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +struct backlight_device *of_find_backlight(struct device *dev);
> +#else
> +static inline struct backlight_device *of_find_backlight(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>   #endif

Re: [PATCH v16 04/10] drm/tinydrm: Replace tinydrm_of_find_backlight with of_find_backlight

[Noralf Trønnes]

Den 16.01.2018 11.34, skrev Meghana Madhyastha:
> Remove tinydrm_of_find_backlight from tinydrm-helpers.c. We now have
> a generic of_find_backlight defined in backlight.c. Let the callers
> of tinydrm_of_find_backlight call of_find_backlight.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

This patch needs rebasing, changes to mi0283qt.
You've missed st7735r.

I'd appreciate if you could also remove these from tinydrm/Kconfig:

     select BACKLIGHT_LCD_SUPPORT
     select BACKLIGHT_CLASS_DEVICE

It's an ugly hack that we don't need anymore thanks to your work!

With that fixed:
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> ---
>   drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --------------------------
>   drivers/gpu/drm/tinydrm/mi0283qt.c             |  3 +-
>   include/drm/tinydrm/tinydrm-helpers.h          |  2 --
>   3 files changed, 2 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> index 7326e1758..d1c3ce9ab 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -236,46 +236,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>   }
>   EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
>   
> -/**
> - * tinydrm_of_find_backlight - Find backlight device in device-tree
> - * @dev: Device
> - *
> - * This function looks for a DT node pointed to by a property named 'backlight'
> - * and uses of_find_backlight_by_node() to get the backlight device.
> - * Additionally if the brightness property is zero, it is set to
> - * max_brightness.
> - *
> - * Returns:
> - * NULL if there's no backlight property.
> - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
> - * is found.
> - * If the backlight device is found, a pointer to the structure is returned.
> - */
> -struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
> -{
> -	struct backlight_device *backlight;
> -	struct device_node *np;
> -
> -	np = of_parse_phandle(dev->of_node, "backlight", 0);
> -	if (!np)
> -		return NULL;
> -
> -	backlight = of_find_backlight_by_node(np);
> -	of_node_put(np);
> -
> -	if (!backlight)
> -		return ERR_PTR(-EPROBE_DEFER);
> -
> -	if (!backlight->props.brightness) {
> -		backlight->props.brightness = backlight->props.max_brightness;
> -		DRM_DEBUG_KMS("Backlight brightness set to %d\n",
> -			      backlight->props.brightness);
> -	}
> -
> -	return backlight;
> -}
> -EXPORT_SYMBOL(tinydrm_of_find_backlight);
> -
>   #if IS_ENABLED(CONFIG_SPI)
>   
>   /**
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index 674d40764..3f9065fb5 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -14,6 +14,7 @@
>   #include <drm/tinydrm/ili9341.h>
>   #include <drm/tinydrm/mipi-dbi.h>
>   #include <drm/tinydrm/tinydrm-helpers.h>
> +#include <linux/backlight.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/module.h>
> @@ -190,7 +191,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>   	if (IS_ERR(mipi->regulator))
>   		return PTR_ERR(mipi->regulator);
>   
> -	mipi->backlight = tinydrm_of_find_backlight(dev);
> +	mipi->backlight = of_find_backlight(dev);
>   	if (IS_ERR(mipi->backlight))
>   		return PTR_ERR(mipi->backlight);
>   
> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
> index f54fae03e..0a4ddbc04 100644
> --- a/include/drm/tinydrm/tinydrm-helpers.h
> +++ b/include/drm/tinydrm/tinydrm-helpers.h
> @@ -46,8 +46,6 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
>   void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>   			       struct drm_clip_rect *clip);
>   
> -struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
> -
>   size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
>   bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
>   int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,

Re: [PATCH v16 05/10] video: backlight: Add devres versions of of_find_backlight

[Noralf Trønnes]

Den 16.01.2018 11.34, skrev Meghana Madhyastha:
> Add devm_of_find_backlight and the corresponding release
> function because some drivers use devres versions of functions
> for acquiring device resources.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

checkpatch complains:
-:26: WARNING: Block comments should align the * on each line
This one is text so might as well fix it:
-:29: WARNING: line over 80 characters

With that fixed:
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> ---
>   drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
>   include/linux/backlight.h           |  7 +++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 7e4a5d77d..b3f76945f 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -620,6 +620,35 @@ struct backlight_device *of_find_backlight(struct device *dev)
>   }
>   EXPORT_SYMBOL(of_find_backlight);
>   
> +static void devm_backlight_put(void *data)
> +{
> +	backlight_put(data);
> +}
> +
> +/**
> +  * devm_of_find_backlight - Resource-managed of_find_backlight()
> +  * @dev: Device
> +  *
> +  * Device managed version of of_find_backlight(). The reference on the backlight
> +  * device is automatically dropped on driver detach.
> +  */
> +struct backlight_device *devm_of_find_backlight(struct device *dev)
> +{
> +	struct backlight_device *bd;
> +	int ret;
> +
> +	bd = of_find_backlight(dev);
> +	if (IS_ERR_OR_NULL(bd))
> +		return bd;
> +	ret = devm_add_action(dev, devm_backlight_put, bd);
> +	if (ret) {
> +		backlight_put(bd);
> +		return ERR_PTR(ret);
> +	}
> +	return bd;
> +}
> +EXPORT_SYMBOL(devm_of_find_backlight);
> +
>   static void __exit backlight_class_exit(void)
>   {
>   	class_destroy(backlight_class);
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 32ea510da..1d373f5a6 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -215,11 +215,18 @@ of_find_backlight_by_node(struct device_node *node)
>   
>   #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>   struct backlight_device *of_find_backlight(struct device *dev);
> +struct backlight_device *devm_of_find_backlight(struct device *dev);
>   #else
>   static inline struct backlight_device *of_find_backlight(struct device *dev)
>   {
>   	return NULL;
>   }
> +
> +static inline struct backlight_device *
> +devm_of_find_backlight(struct device *dev)
> +{
> +	return NULL;
> +}
>   #endif
>   
>   #endif

Re: [PATCH v16 06/10] drm/tinydrm: Call devres version of of_find_backlight

[Noralf Trønnes]

Den 16.01.2018 11.34, skrev Meghana Madhyastha:
> Call devm_of_find_backlight (the devres version) instead of
> of_find_backlight.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
>   drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index 3f9065fb5..4d8650bdc 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -191,7 +191,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>   	if (IS_ERR(mipi->regulator))
>   		return PTR_ERR(mipi->regulator);
>   
> -	mipi->backlight = of_find_backlight(dev);
> +	mipi->backlight = devm_of_find_backlight(dev);
>   	if (IS_ERR(mipi->backlight))
>   		return PTR_ERR(mipi->backlight);
>   

You've missed st7735r.

With that fixed:
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper

[Noralf Trønnes]

Den 16.01.2018 11.36, skrev Meghana Madhyastha:
> Replace of_find_backlight_by_node and of the code around it
> with of_find_backlight helper to avoid repetition of code.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
>   drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 10 +++-------
>   drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 +++-------
>   drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 +++-------
>   3 files changed, 9 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 4c1b29eec..a69b0530f 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -230,14 +230,10 @@ static int innolux_panel_add(struct innolux_panel *innolux)
>   		innolux->enable_gpio = NULL;
>   	}
>   
> -	np = of_parse_phandle(dev->of_node, "backlight", 0);
> -	if (np) {
> -		innolux->backlight = of_find_backlight_by_node(np);
> -		of_node_put(np);
> +	innolux->backlight = devm_of_find_backlight(np);

This driver isn't broken like tinydrm was, so it does put the backlight
device on cleanup like it should. So when you're using the devm_ version,
the result is a double put. Just remove the put_device() in
innolux_panel_add/del() and you're good.

I haven't checked the other drivers.

Noralf.

PS:
Give people a few days (one week?) to respond before respinning a new
version, so we avoid a fragmented discussion.

>   
> -		if (!innolux->backlight)
> -			return -EPROBE_DEFER;
> -	}
> +	if (IS_ERR(innolux->backlight))
> +		return PTR_ERR(innolux->backlight);
>   
>   	drm_panel_init(&innolux->base);
>   	innolux->base.funcs = &innolux_panel_funcs;
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 1512ec4f3..d5450c9d9 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -329,14 +329,10 @@ static int sharp_panel_add(struct sharp_panel *sharp)
>   	if (IS_ERR(sharp->supply))
>   		return PTR_ERR(sharp->supply);
>   
> -	np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
> -	if (np) {
> -		sharp->backlight = of_find_backlight_by_node(np);
> -		of_node_put(np);
> +	sharp->backlight = devm_of_find_backlight(np);
>   
> -		if (!sharp->backlight)
> -			return -EPROBE_DEFER;
> -	}
> +	if (IS_ERR(sharp->backlight))
> +		return PTR_ERR(sharp->backlight);
>   
>   	drm_panel_init(&sharp->base);
>   	sharp->base.funcs = &sharp_panel_funcs;
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> index a6af3257f..db31d8268 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> @@ -273,14 +273,10 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
>   		gpiod_set_value(sharp_nt->reset_gpio, 0);
>   	}
>   
> -	np = of_parse_phandle(dev->of_node, "backlight", 0);
> -	if (np) {
> -		sharp_nt->backlight = of_find_backlight_by_node(np);
> -		of_node_put(np);
> +	sharp_nt->backlight = devm_of_find_backlight(np);
>   
> -		if (!sharp_nt->backlight)
> -			return -EPROBE_DEFER;
> -	}
> +	if (IS_ERR(sharp_nt->backlight))
> +		return PTR_ERR(sharp_nt->backlight);
>   
>   	drm_panel_init(&sharp_nt->base);
>   	sharp_nt->base.funcs = &sharp_nt_panel_funcs;

Re: [PATCH v16 07/10] drm/panel: Use backlight_enable/disable helpers

[Noralf Trønnes]

Den 16.01.2018 11.35, skrev Meghana Madhyastha:
> Use backlight_enable/disable helpers instead of changing
> the property and calling backlight_update_status for cleaner
> and simpler code and also to avoid repetitions.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
>   drivers/gpu/drm/panel/panel-innolux-p079zca.c   |  6 ++----
>   drivers/gpu/drm/panel/panel-jdi-lt070me05000.c  |  6 ++----
>   drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 12 ++++--------
>   drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 12 ++++--------
>   4 files changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 6ba93449f..4c1b29eec 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -45,8 +45,7 @@ static int innolux_panel_disable(struct drm_panel *panel)
>   	if (!innolux->enabled)
>   		return 0;
>   
> -	innolux->backlight->props.power = FB_BLANK_POWERDOWN;
> -	backlight_update_status(innolux->backlight);
> +	backlight_disable(innolux->backlight);
>   
>   	err = mipi_dsi_dcs_set_display_off(innolux->link);
>   	if (err < 0)
> @@ -151,8 +150,7 @@ static int innolux_panel_enable(struct drm_panel *panel)
>   	if (innolux->enabled)
>   		return 0;
>   
> -	innolux->backlight->props.power = FB_BLANK_UNBLANK;
> -	ret = backlight_update_status(innolux->backlight);
> +	ret = backlight_enable(innolux->backlight);
>   	if (ret) {
>   		DRM_DEV_ERROR(panel->drm->dev,
>   			      "Failed to enable backlight %d\n", ret);
> diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> index 5b2340ef7..0a94ab79a 100644
> --- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> +++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> @@ -192,8 +192,7 @@ static int jdi_panel_disable(struct drm_panel *panel)
>   	if (!jdi->enabled)
>   		return 0;
>   
> -	jdi->backlight->props.power = FB_BLANK_POWERDOWN;
> -	backlight_update_status(jdi->backlight);
> +	backlight_disable(jdi->backlight);
>   
>   	jdi->enabled = false;
>   
> @@ -289,8 +288,7 @@ static int jdi_panel_enable(struct drm_panel *panel)
>   	if (jdi->enabled)
>   		return 0;
>   
> -	jdi->backlight->props.power = FB_BLANK_UNBLANK;
> -	backlight_update_status(jdi->backlight);
> +	backlight_enable(jdi->backlight);
>   
>   	jdi->enabled = true;
>   
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 3cce3ca19..1512ec4f3 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -96,10 +96,8 @@ static int sharp_panel_disable(struct drm_panel *panel)
>   	if (!sharp->enabled)
>   		return 0;
>   
> -	if (sharp->backlight) {
> -		sharp->backlight->props.power = FB_BLANK_POWERDOWN;
> -		backlight_update_status(sharp->backlight);
> -	}
> +	if (sharp->backlight)

No need for a NULL check, backlight_enable/disable() already does that.
The same goes for the rest of the patch.

Noralf.

> +		backlight_disable(sharp->backlight);
>   
>   	sharp->enabled = false;
>   
> @@ -263,10 +261,8 @@ static int sharp_panel_enable(struct drm_panel *panel)
>   	if (sharp->enabled)
>   		return 0;
>   
> -	if (sharp->backlight) {
> -		sharp->backlight->props.power = FB_BLANK_UNBLANK;
> -		backlight_update_status(sharp->backlight);
> -	}
> +	if (sharp->backlight)
> +		backlight_enable(sharp->backlight);
>   
>   	sharp->enabled = true;
>   
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> index 3aeb0bda4..a6af3257f 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> @@ -117,10 +117,8 @@ static int sharp_nt_panel_disable(struct drm_panel *panel)
>   	if (!sharp_nt->enabled)
>   		return 0;
>   
> -	if (sharp_nt->backlight) {
> -		sharp_nt->backlight->props.power = FB_BLANK_POWERDOWN;
> -		backlight_update_status(sharp_nt->backlight);
> -	}
> +	if (sharp_nt->backlight)
> +		backlight_disable(sharp_nt->backlight);
>   
>   	sharp_nt->enabled = false;
>   
> @@ -203,10 +201,8 @@ static int sharp_nt_panel_enable(struct drm_panel *panel)
>   	if (sharp_nt->enabled)
>   		return 0;
>   
> -	if (sharp_nt->backlight) {
> -		sharp_nt->backlight->props.power = FB_BLANK_UNBLANK;
> -		backlight_update_status(sharp_nt->backlight);
> -	}
> +	if (sharp_nt->backlight)
> +		backlight_enable(sharp_nt->backlight);
>   
>   	sharp_nt->enabled = true;
>   

Re: [PATCH v16 00/10] Add backlight helper functions

[Sean Paul]

On Tue, Jan 16, 2018 at 10:31:07AM +0000, Meghana Madhyastha wrote:
> Move drm helper functions from tinydrm-helpers to linux/backlight for
> ease of use by callers in other drivers.
> 
> Changes in v16:
> -Add a comment about setting brightness = max_brightness in of_find_backlight
> -Add dri-devel and linux-kernel mailing lists

I just took a pass through the set. Agreed on all of Noralf's review feedback
(which seems minor). Once that's resolved, feel free to add my R-b to the whole
thing. Thanks for sticking with this, almost there :-)

Sean

> 
> Meghana Madhyastha (10):
>   video: backlight: Add helpers to enable and disable backlight
>   drm/tinydrm: Convert tinydrm_enable/disable_backlight to
>     backlight_enable/disable
>   video: backlight: Add of_find_backlight helper in backlight.c
>   drm/tinydrm: Replace tinydrm_of_find_backlight with of_find_backlight
>   video: backlight: Add devres versions of of_find_backlight
>   drm/tinydrm: Call devres version of of_find_backlight
>   drm/panel: Use backlight_enable/disable helpers
>   drm/omapdrm: Use backlight_enable/disable helpers
>   drm/panel: Use of_find_backlight helper
>   drm/omapdrm: Use of_find_backlight helper
> 
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c    | 22 ++----
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 16 ++---
>  drivers/gpu/drm/panel/panel-jdi-lt070me05000.c  |  6 +-
>  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 22 ++----
>  drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 22 ++----
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c  | 95 -------------------------
>  drivers/gpu/drm/tinydrm/mi0283qt.c              |  3 +-
>  drivers/gpu/drm/tinydrm/mipi-dbi.c              |  4 +-
>  drivers/video/backlight/backlight.c             | 69 ++++++++++++++++++
>  include/drm/tinydrm/tinydrm-helpers.h           |  4 --
>  include/linux/backlight.h                       | 56 +++++++++++++++
>  11 files changed, 157 insertions(+), 162 deletions(-)
> 
> -- 
> 2.11.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Daniel Thompson]

On 16/01/18 10:31, Meghana Madhyastha wrote:
> Add helper functions backlight_enable and backlight_disable to
> enable/disable a backlight device. These helper functions can
> then be used by different drm and tinydrm drivers to avoid
> repetition of code and also to enforce a uniform and consistent
> way to enable/disable a backlight device.
> 
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

To be clear I don't disagree with anthing Daniel V. said about the 
horribly confused (and confusing) power states for backlight.

Nevertheless I don't recall seeing any response (positive or negative) 
to this post from v13:
https://www.spinics.net/lists/dri-devel/msg154459.html


Daniel.


> ---
>   include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index af7003548..7b6a9a2a3 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -130,6 +130,36 @@ static inline int backlight_update_status(struct backlight_device *bd)
>   	return ret;
>   }
>   
> +/**
> +  * backlight_enable - Enable backlight
> +  * @bd: the backlight device to enable
> +  */
> +static inline int backlight_enable(struct backlight_device *bd)
> +{
> +	if (!bd)
> +		return 0;
> +
> +	bd->props.power = FB_BLANK_UNBLANK;
> +	bd->props.state &= ~BL_CORE_FBBLANK;
> +
> +	return backlight_update_status(bd);
> +}
> +
> +/**
> +  * backlight_disable - Disable backlight
> +  * @bd: the backlight device to disable
> +  */
> +static inline int backlight_disable(struct backlight_device *bd)
> +{
> +	if (!bd)
> +		return 0;
> +		
> +	bd->props.power = FB_BLANK_POWERDOWN;
> +	bd->props.state |= BL_CORE_FBBLANK;
> +
> +	return backlight_update_status(bd);
> +}
> +
>   extern struct backlight_device *backlight_device_register(const char *name,
>   	struct device *dev, void *devdata, const struct backlight_ops *ops,
>   	const struct backlight_properties *props);
> 

Re: [PATCH v16 03/10] video: backlight: Add of_find_backlight helper in backlight.c

[Daniel Thompson]

On 16/01/18 10:33, Meghana Madhyastha wrote:
> Add of_find_backlight, a helper function which is a generic version
> of tinydrm_of_find_backlight that can be used by other drivers to avoid
> repetition of code and simplify things.
> 
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
> Changes in v16:
> -Add comment about brightness in of_find_backlight
> 
>   drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
>   include/linux/backlight.h           | 19 ++++++++++++++++++
>   2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 8049e7656..7e4a5d77d 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -580,6 +580,46 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
>   EXPORT_SYMBOL(of_find_backlight_by_node);
>   #endif
>   
> +/**
> +  * of_find_backlight - Get backlight device
> +  * @dev: Device
> +  *
> +  * This function looks for a property named 'backlight' on the DT node
> +  * connected to @dev and looks up the backlight device.
> +  *
> +  * Call backlight_put() to drop the reference on the backlight device.
> +  * gpio_backlight uses brightness as power state during probe.
> +  *
> +  * Returns:
> +  * A pointer to the backlight device if found.
> +  * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> +  * device is found.
> +  * NULL if there's no backlight property.
> +  */
> +struct backlight_device *of_find_backlight(struct device *dev)
> +{
> +	struct backlight_device *bd = NULL;
> +	struct device_node *np;
> +
> +	if (!dev)
> +		return NULL;
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +		np = of_parse_phandle(dev->of_node, "backlight", 0);
> +		if (np) {
> +			bd = of_find_backlight_by_node(np);
> +			of_node_put(np);
> +			if (!bd)
> +				return ERR_PTR(-EPROBE_DEFER);
> +			if (!bd->props.brightness)
> +				bd->props.brightness = bd->props.max_brightness;
> +		}
> +	}
> +
> +	return bd;
> +}
> +EXPORT_SYMBOL(of_find_backlight);
> +
>   static void __exit backlight_class_exit(void)
>   {
>   	class_destroy(backlight_class);
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7b6a9a2a3..32ea510da 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -160,6 +160,16 @@ static inline int backlight_disable(struct backlight_device *bd)
>   	return backlight_update_status(bd);
>   }
>   
> +/**
> +  * backlight_put - Drop backlight reference
> +  * @bd: the backlight device to put
> +  */
> +static inline void backlight_put(struct backlight_device *bd)
> +{
> +	if (bd)
> +		put_device(&bd->dev);
> +}
> +
>   extern struct backlight_device *backlight_device_register(const char *name,
>   	struct device *dev, void *devdata, const struct backlight_ops *ops,
>   	const struct backlight_properties *props);
> @@ -203,4 +213,13 @@ of_find_backlight_by_node(struct device_node *node)
>   }
>   #endif
>   
> +#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> +struct backlight_device *of_find_backlight(struct device *dev);
> +#else
> +static inline struct backlight_device *of_find_backlight(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>   #endif
> 

Re: [PATCH v16 05/10] video: backlight: Add devres versions of of_find_backlight

[Daniel Thompson]

On 16/01/18 10:34, Meghana Madhyastha wrote:
> Add devm_of_find_backlight and the corresponding release
> function because some drivers use devres versions of functions
> for acquiring device resources.
> 
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
>   drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
>   include/linux/backlight.h           |  7 +++++++
>   2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 7e4a5d77d..b3f76945f 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -620,6 +620,35 @@ struct backlight_device *of_find_backlight(struct device *dev)
>   }
>   EXPORT_SYMBOL(of_find_backlight);
>   
> +static void devm_backlight_put(void *data)
> +{
> +	backlight_put(data);

Shouldn't this be using devres_release()?


> +}
> +
> +/**
> +  * devm_of_find_backlight - Resource-managed of_find_backlight()
> +  * @dev: Device
> +  *
> +  * Device managed version of of_find_backlight(). The reference on the backlight
> +  * device is automatically dropped on driver detach.
> +  */
> +struct backlight_device *devm_of_find_backlight(struct device *dev)
> +{
> +	struct backlight_device *bd;
> +	int ret;
> +
> +	bd = of_find_backlight(dev);
> +	if (IS_ERR_OR_NULL(bd))
> +		return bd;
> +	ret = devm_add_action(dev, devm_backlight_put, bd);
> +	if (ret) {
> +		backlight_put(bd);
> +		return ERR_PTR(ret);
> +	}
> +	return bd;
> +}
> +EXPORT_SYMBOL(devm_of_find_backlight);
> +
>   static void __exit backlight_class_exit(void)
>   {
>   	class_destroy(backlight_class);
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 32ea510da..1d373f5a6 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -215,11 +215,18 @@ of_find_backlight_by_node(struct device_node *node)
>   
>   #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>   struct backlight_device *of_find_backlight(struct device *dev);
> +struct backlight_device *devm_of_find_backlight(struct device *dev);
>   #else
>   static inline struct backlight_device *of_find_backlight(struct device *dev)
>   {
>   	return NULL;
>   }
> +
> +static inline struct backlight_device *
> +devm_of_find_backlight(struct device *dev)
> +{
> +	return NULL;
> +}
>   #endif
>   
>   #endif
> 

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Daniel Vetter]

On Wed, Jan 17, 2018 at 6:00 PM, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
> On 16/01/18 10:31, Meghana Madhyastha wrote:
>>
>> Add helper functions backlight_enable and backlight_disable to
>> enable/disable a backlight device. These helper functions can
>> then be used by different drm and tinydrm drivers to avoid
>> repetition of code and also to enforce a uniform and consistent
>> way to enable/disable a backlight device.
>>
>> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
>
>
> To be clear I don't disagree with anthing Daniel V. said about the horribly
> confused (and confusing) power states for backlight.
>
> Nevertheless I don't recall seeing any response (positive or negative) to
> this post from v13:
> https://www.spinics.net/lists/dri-devel/msg154459.html

I think also adjusting the fb_blank bits in these new helpers is a
reasonable thing to do. Maybe with add a huge TODO comment that this
is all a bit sad ...
-Daniel

> Daniel.
>
>
>
>> ---
>>   include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index af7003548..7b6a9a2a3 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -130,6 +130,36 @@ static inline int backlight_update_status(struct
>> backlight_device *bd)
>>         return ret;
>>   }
>>   +/**
>> +  * backlight_enable - Enable backlight
>> +  * @bd: the backlight device to enable
>> +  */
>> +static inline int backlight_enable(struct backlight_device *bd)
>> +{
>> +       if (!bd)
>> +               return 0;
>> +
>> +       bd->props.power = FB_BLANK_UNBLANK;
>> +       bd->props.state &= ~BL_CORE_FBBLANK;
>> +
>> +       return backlight_update_status(bd);
>> +}
>> +
>> +/**
>> +  * backlight_disable - Disable backlight
>> +  * @bd: the backlight device to disable
>> +  */
>> +static inline int backlight_disable(struct backlight_device *bd)
>> +{
>> +       if (!bd)
>> +               return 0;
>> +
>> +       bd->props.power = FB_BLANK_POWERDOWN;
>> +       bd->props.state |= BL_CORE_FBBLANK;
>> +
>> +       return backlight_update_status(bd);
>> +}
>> +
>>   extern struct backlight_device *backlight_device_register(const char
>> *name,
>>         struct device *dev, void *devdata, const struct backlight_ops
>> *ops,
>>         const struct backlight_properties *props);
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Noralf Trønnes]

Den 17.01.2018 18.00, skrev Daniel Thompson:
>
>
> On 16/01/18 10:31, Meghana Madhyastha wrote:
>> Add helper functions backlight_enable and backlight_disable to
>> enable/disable a backlight device. These helper functions can
>> then be used by different drm and tinydrm drivers to avoid
>> repetition of code and also to enforce a uniform and consistent
>> way to enable/disable a backlight device.
>>
>> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
>
> To be clear I don't disagree with anthing Daniel V. said about the 
> horribly confused (and confusing) power states for backlight.
>
> Nevertheless I don't recall seeing any response (positive or negative) 
> to this post from v13:
> https://www.spinics.net/lists/dri-devel/msg154459.html
>

I see that Daniel V has answered while I was chasing this down, but anyways:

A grep suggests that omap1_bl is the only driver that only checks fb_blank.
All the other drivers check both fb_blank and power, a few check state. The
backlight fbdev notifier callback doesn't set power, but sets fb_blank and
state.

fb_blank was marked 'Due to be removed' 9 years ago, so it hasn't been
high priority.

So for completeness I guess it makes sense to set fb_blank.

Noralf.

$ grep -r -C10 "props\.fb_blank" .
./drivers/video/backlight/corgi_lcd.c-  if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
./drivers/video/backlight/corgi_lcd.c-
./drivers/video/backlight/corgi_lcd.c:  if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
--
./drivers/video/backlight/adp8860_bl.c- if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
./drivers/video/backlight/adp8860_bl.c-
./drivers/video/backlight/adp8860_bl.c: if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
--
./drivers/video/backlight/hp680_bl.c-   if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/hp680_bl.c-           intensity = 0;
./drivers/video/backlight/hp680_bl.c:   if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/hp680_bl.c-           intensity = 0;
--
./drivers/video/backlight/cr_bllcd.c-static int 
cr_backlight_set_intensity(struct backlight_device *bd)
./drivers/video/backlight/cr_bllcd.c-{
./drivers/video/backlight/cr_bllcd.c-   int intensity = 
bd->props.brightness;
./drivers/video/backlight/cr_bllcd.c-   u32 addr = gpio_bar + 
CRVML_PANEL_PORT;
./drivers/video/backlight/cr_bllcd.c-   u32 cur = inl(addr);
./drivers/video/backlight/cr_bllcd.c-
./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power == 
FB_BLANK_UNBLANK)
./drivers/video/backlight/cr_bllcd.c-           intensity = 
FB_BLANK_UNBLANK;
./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank == 
FB_BLANK_UNBLANK)
./drivers/video/backlight/cr_bllcd.c-           intensity = 
FB_BLANK_UNBLANK;
./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power == 
FB_BLANK_POWERDOWN)
./drivers/video/backlight/cr_bllcd.c-           intensity = 
FB_BLANK_POWERDOWN;
./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank == 
FB_BLANK_POWERDOWN)
./drivers/video/backlight/cr_bllcd.c-           intensity = 
FB_BLANK_POWERDOWN;
--
./drivers/video/backlight/max8925_bl.c- if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/max8925_bl.c-         brightness = 0;
./drivers/video/backlight/max8925_bl.c-
./drivers/video/backlight/max8925_bl.c: if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/max8925_bl.c-         brightness = 0;
./drivers/video/backlight/max8925_bl.c-
./drivers/video/backlight/max8925_bl.c- if (bl->props.state & 
BL_CORE_SUSPENDED)
./drivers/video/backlight/max8925_bl.c-         brightness = 0;
--
./drivers/video/backlight/lv5207lp.c-   if (backlight->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/lv5207lp.c: backlight->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/lv5207lp.c- backlight->props.state & 
(BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
./drivers/video/backlight/lv5207lp.c-           brightness = 0;
--
./drivers/video/backlight/lm3533_bl.c-  if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
./drivers/video/backlight/lm3533_bl.c:  if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
--
./drivers/video/backlight/omap1_bl.c-static int 
omapbl_update_status(struct backlight_device *dev)
./drivers/video/backlight/omap1_bl.c-{
./drivers/video/backlight/omap1_bl.c-   struct omap_backlight *bl = 
bl_get_data(dev);
./drivers/video/backlight/omap1_bl.c-
./drivers/video/backlight/omap1_bl.c-   if (bl->current_intensity != 
dev->props.brightness) {
./drivers/video/backlight/omap1_bl.c-           if (bl->powermode == 
FB_BLANK_UNBLANK)
./drivers/video/backlight/omap1_bl.c- 
omapbl_send_intensity(dev->props.brightness);
./drivers/video/backlight/omap1_bl.c- bl->current_intensity = 
dev->props.brightness;
./drivers/video/backlight/omap1_bl.c-   }
./drivers/video/backlight/omap1_bl.c-
./drivers/video/backlight/omap1_bl.c:   if (dev->props.fb_blank != 
bl->powermode)
./drivers/video/backlight/omap1_bl.c: omapbl_set_power(dev, 
dev->props.fb_blank);
./drivers/video/backlight/omap1_bl.c-
./drivers/video/backlight/omap1_bl.c-   return 0;
./drivers/video/backlight/omap1_bl.c-}
./drivers/video/backlight/omap1_bl.c-
--
./drivers/video/backlight/kb3886_bl.c-  if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
./drivers/video/backlight/kb3886_bl.c:  if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
--
./drivers/video/backlight/pwm_bl.c-     if (bl->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/pwm_bl.c:         bl->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/pwm_bl.c-         bl->props.state & 
BL_CORE_FBBLANK)
./drivers/video/backlight/pwm_bl.c-             brightness = 0;
--
./drivers/video/backlight/pm8941-wled.c-        if (bl->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/pm8941-wled.c: bl->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/pm8941-wled.c- bl->props.state & BL_CORE_FBBLANK)
./drivers/video/backlight/pm8941-wled.c-                val = 0;
--
./drivers/video/backlight/adp8870_bl.c- if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
./drivers/video/backlight/adp8870_bl.c-
./drivers/video/backlight/adp8870_bl.c: if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
--
./drivers/video/backlight/as3711_bl.c-  if (bl->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/as3711_bl.c:      bl->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/as3711_bl.c-      bl->props.state & 
(BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
./drivers/video/backlight/as3711_bl.c-          brightness = 0;
--
./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
./drivers/video/backlight/88pm860x_bl.c-
./drivers/video/backlight/88pm860x_bl.c:        if (bl->props.fb_blank 
!= FB_BLANK_UNBLANK)
./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
./drivers/video/backlight/88pm860x_bl.c-
./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.state & 
BL_CORE_SUSPENDED)
./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
--
./drivers/video/backlight/tps65217_bl.c-        if (bl->props.state & 
BL_CORE_SUSPENDED)
./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
./drivers/video/backlight/tps65217_bl.c-
./drivers/video/backlight/tps65217_bl.c-        if ((bl->props.power != 
FB_BLANK_UNBLANK) ||
./drivers/video/backlight/tps65217_bl.c: (bl->props.fb_blank != 
FB_BLANK_UNBLANK))
./drivers/video/backlight/tps65217_bl.c-                /* framebuffer 
in low power mode or blanking active */
./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
--
./drivers/video/backlight/adp5520_bl.c- if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
./drivers/video/backlight/adp5520_bl.c-
./drivers/video/backlight/adp5520_bl.c: if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
--
./drivers/video/backlight/wm831x_bl.c-  if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
./drivers/video/backlight/wm831x_bl.c-
./drivers/video/backlight/wm831x_bl.c:  if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
./drivers/video/backlight/wm831x_bl.c-
./drivers/video/backlight/wm831x_bl.c-  if (bl->props.state & 
BL_CORE_SUSPENDED)
./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
--
./drivers/video/backlight/gpio_backlight.c-     if (bl->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/gpio_backlight.c: bl->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/gpio_backlight.c- bl->props.state & 
(BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
./drivers/video/backlight/gpio_backlight.c-             brightness = 0;
--
./drivers/video/backlight/da903x_bl.c-  if (bl->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/da903x_bl.c-          brightness = 0;
./drivers/video/backlight/da903x_bl.c-
./drivers/video/backlight/da903x_bl.c:  if (bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/da903x_bl.c-          brightness = 0;
./drivers/video/backlight/da903x_bl.c-
./drivers/video/backlight/da903x_bl.c-  if (bl->props.state & 
BL_CORE_SUSPENDED)
./drivers/video/backlight/da903x_bl.c-          brightness = 0;
--
./drivers/video/backlight/locomolcd.c-  if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/locomolcd.c-          intensity = 0;
./drivers/video/backlight/locomolcd.c:  if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/locomolcd.c-          intensity = 0;
--
./drivers/video/backlight/bd6107.c-     if (backlight->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/bd6107.c: backlight->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/bd6107.c- backlight->props.state & 
(BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
./drivers/video/backlight/bd6107.c-             brightness = 0;
--
./drivers/video/backlight/backlight.c-                  if (fb_blank == 
FB_BLANK_UNBLANK &&
./drivers/video/backlight/backlight.c- !bd->fb_bl_on[node]) {
./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = true;
./drivers/video/backlight/backlight.c-                          if 
(!bd->use_count++) {
./drivers/video/backlight/backlight.c- bd->props.state &= ~BL_CORE_FBBLANK;
./drivers/video/backlight/backlight.c: bd->props.fb_blank = 
FB_BLANK_UNBLANK;
./drivers/video/backlight/backlight.c- backlight_update_status(bd);
./drivers/video/backlight/backlight.c-                          }
./drivers/video/backlight/backlight.c-                  } else if 
(fb_blank != FB_BLANK_UNBLANK &&
./drivers/video/backlight/backlight.c- bd->fb_bl_on[node]) {
./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = false;
./drivers/video/backlight/backlight.c-                          if 
(!(--bd->use_count)) {
./drivers/video/backlight/backlight.c- bd->props.state |= BL_CORE_FBBLANK;
./drivers/video/backlight/backlight.c: bd->props.fb_blank = fb_blank;
./drivers/video/backlight/backlight.c- backlight_update_status(bd);
--
./drivers/video/backlight/ep93xx_bl.c-  if (bl->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/backlight/ep93xx_bl.c:      bl->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/backlight/ep93xx_bl.c-          brightness = 0;
--
./drivers/video/backlight/jornada720_bl.c:      if ((bd->props.power != 
FB_BLANK_UNBLANK) || (bd->props.fb_blank != FB_BLANK_UNBLANK)) {
--
./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c:     if 
(dev->props.fb_blank == FB_BLANK_UNBLANK &&
./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- 
dev->props.power == FB_BLANK_UNBLANK)
./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level = 
dev->props.brightness;
./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c-     else
./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level = 0;
--
./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c: if 
(dev->props.fb_blank == FB_BLANK_UNBLANK &&
./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- 
dev->props.power == FB_BLANK_UNBLANK)
./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- 
level = dev->props.brightness;
./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- else
./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- 
level = 0;
--
./drivers/video/fbdev/aty/atyfb_base.c- if (bd->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/fbdev/aty/atyfb_base.c:     bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/fbdev/aty/atyfb_base.c-         level = 0;
./drivers/video/fbdev/aty/atyfb_base.c- else
./drivers/video/fbdev/aty/atyfb_base.c-         level = 
bd->props.brightness;
--
./drivers/video/fbdev/aty/aty128fb.c-   if (bd->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/fbdev/aty/aty128fb.c:       bd->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/video/fbdev/aty/aty128fb.c-       !par->lcd_on)
./drivers/video/fbdev/aty/aty128fb.c-           level = 0;
./drivers/video/fbdev/aty/aty128fb.c-   else
./drivers/video/fbdev/aty/aty128fb.c-           level = 
bd->props.brightness;
--
./drivers/video/fbdev/aty/radeon_backlight.c-        if (bd->props.power 
!= FB_BLANK_UNBLANK ||
./drivers/video/fbdev/aty/radeon_backlight.c: bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/fbdev/aty/radeon_backlight.c-           level = 0;
./drivers/video/fbdev/aty/radeon_backlight.c-   else
./drivers/video/fbdev/aty/radeon_backlight.c-           level = 
bd->props.brightness;
--
./drivers/video/fbdev/mx3fb.c-  if (bl->props.power != FB_BLANK_UNBLANK)
./drivers/video/fbdev/mx3fb.c-          brightness = 0;
./drivers/video/fbdev/mx3fb.c:  if (bl->props.fb_blank != FB_BLANK_UNBLANK)
./drivers/video/fbdev/mx3fb.c-          brightness = 0;
--
./drivers/video/fbdev/riva/fbdev.c-     if (bd->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/fbdev/riva/fbdev.c:         bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/fbdev/riva/fbdev.c-             level = 0;
./drivers/video/fbdev/riva/fbdev.c-     else
./drivers/video/fbdev/riva/fbdev.c-             level = 
bd->props.brightness;
--
./drivers/video/fbdev/atmel_lcdfb.c:    if (bl->props.fb_blank != 
sinfo->bl_power)
./drivers/video/fbdev/atmel_lcdfb.c:            power = bl->props.fb_blank;
./drivers/video/fbdev/atmel_lcdfb.c-    else if (bl->props.power != 
sinfo->bl_power)
./drivers/video/fbdev/atmel_lcdfb.c-            power = bl->props.power;
--
./drivers/video/fbdev/nvidia/nv_backlight.c-    if (bd->props.power != 
FB_BLANK_UNBLANK ||
./drivers/video/fbdev/nvidia/nv_backlight.c: bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/video/fbdev/nvidia/nv_backlight.c-            level = 0;
./drivers/video/fbdev/nvidia/nv_backlight.c-    else
./drivers/video/fbdev/nvidia/nv_backlight.c-            level = 
bd->props.brightness;
--
./drivers/macintosh/via-pmu-backlight.c-        if (bd->props.power != 
FB_BLANK_UNBLANK ||
./drivers/macintosh/via-pmu-backlight.c: bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/macintosh/via-pmu-backlight.c-                level = 0;
--
./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c:      if 
(dev->props.fb_blank == FB_BLANK_UNBLANK &&
./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- dev->props.power == 
FB_BLANK_UNBLANK)
./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level = 
dev->props.brightness;
./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c-      else
./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level = 0;
--
./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c:      if 
(dev->props.fb_blank == FB_BLANK_UNBLANK &&
./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- 
dev->props.power == FB_BLANK_UNBLANK)
./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level = 
dev->props.brightness;
./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c-      else
./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level = 0;
--
./drivers/staging/fbtft/fb_ssd1351.c-   on = (bd->props.power == 
FB_BLANK_UNBLANK) &&
./drivers/staging/fbtft/fb_ssd1351.c:        (bd->props.fb_blank == 
FB_BLANK_UNBLANK);
--
./drivers/staging/fbtft/fbtft-core.c-   if ((bd->props.power == 
FB_BLANK_UNBLANK) &&
./drivers/staging/fbtft/fbtft-core.c:       (bd->props.fb_blank == 
FB_BLANK_UNBLANK))
--
./drivers/staging/fbtft/fb_watterott.c- if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
./drivers/staging/fbtft/fb_watterott.c-
./drivers/staging/fbtft/fb_watterott.c: if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
--
./drivers/auxdisplay/ht16k33.c- if (bl->props.power != FB_BLANK_UNBLANK ||
./drivers/auxdisplay/ht16k33.c:     bl->props.fb_blank != 
FB_BLANK_UNBLANK ||
./drivers/auxdisplay/ht16k33.c-     bl->props.state & BL_CORE_FBBLANK || 
brightness == 0) {
./drivers/auxdisplay/ht16k33.c-         return ht16k33_display_off(priv);
--
./drivers/platform/x86/thinkpad_acpi.c: (bd->props.fb_blank == 
FB_BLANK_UNBLANK &&
./drivers/platform/x86/thinkpad_acpi.c-          bd->props.power == 
FB_BLANK_UNBLANK) ?
./drivers/platform/x86/thinkpad_acpi.c- bd->props.brightness : 0;
--
./drivers/platform/x86/acer-wmi.c-      if (bd->props.power != 
FB_BLANK_UNBLANK)
./drivers/platform/x86/acer-wmi.c-              intensity = 0;
./drivers/platform/x86/acer-wmi.c:      if (bd->props.fb_blank != 
FB_BLANK_UNBLANK)
./drivers/platform/x86/acer-wmi.c-              intensity = 0;

>
> Daniel.
>
>
>> ---
>>   include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index af7003548..7b6a9a2a3 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -130,6 +130,36 @@ static inline int backlight_update_status(struct 
>> backlight_device *bd)
>>       return ret;
>>   }
>>   +/**
>> +  * backlight_enable - Enable backlight
>> +  * @bd: the backlight device to enable
>> +  */
>> +static inline int backlight_enable(struct backlight_device *bd)
>> +{
>> +    if (!bd)
>> +        return 0;
>> +
>> +    bd->props.power = FB_BLANK_UNBLANK;
>> +    bd->props.state &= ~BL_CORE_FBBLANK;
>> +
>> +    return backlight_update_status(bd);
>> +}
>> +
>> +/**
>> +  * backlight_disable - Disable backlight
>> +  * @bd: the backlight device to disable
>> +  */
>> +static inline int backlight_disable(struct backlight_device *bd)
>> +{
>> +    if (!bd)
>> +        return 0;
>> +
>> +    bd->props.power = FB_BLANK_POWERDOWN;
>> +    bd->props.state |= BL_CORE_FBBLANK;
>> +
>> +    return backlight_update_status(bd);
>> +}
>> +
>>   extern struct backlight_device *backlight_device_register(const 
>> char *name,
>>       struct device *dev, void *devdata, const struct backlight_ops 
>> *ops,
>>       const struct backlight_properties *props);
>>

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Meghana Madhyastha]

On Wed, Jan 17, 2018 at 11:03:24PM +0100, Noralf Trønnes wrote:
> 
> Den 17.01.2018 18.00, skrev Daniel Thompson:
> >
> >
> >On 16/01/18 10:31, Meghana Madhyastha wrote:
> >>Add helper functions backlight_enable and backlight_disable to
> >>enable/disable a backlight device. These helper functions can
> >>then be used by different drm and tinydrm drivers to avoid
> >>repetition of code and also to enforce a uniform and consistent
> >>way to enable/disable a backlight device.
> >>
> >>Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> >
> >To be clear I don't disagree with anthing Daniel V. said about the
> >horribly confused (and confusing) power states for backlight.
> >
> >Nevertheless I don't recall seeing any response (positive or negative) to
> >this post from v13:
> >https://www.spinics.net/lists/dri-devel/msg154459.html
> >
> 
> I see that Daniel V has answered while I was chasing this down, but anyways:
> 
> A grep suggests that omap1_bl is the only driver that only checks fb_blank.
> All the other drivers check both fb_blank and power, a few check state. The
> backlight fbdev notifier callback doesn't set power, but sets fb_blank and
> state.
> 
> fb_blank was marked 'Due to be removed' 9 years ago, so it hasn't been
> high priority.
> 
> So for completeness I guess it makes sense to set fb_blank.

So if I understood correctly, the suggestion is to set fb_blank along
with power i.e something like this in backlight_enable.
        bd->props.power = FB_BLANK_UNBLANK;
+       bd->props.fb_blank = FB_BLANK_UNBLANK;
        bd->props.state &= ~BL_CORE_FBBLANK;

and set it to FB_BLANK_POWERDOWN in backlight_disable ?

Thanks and regards,
Meghana

> Noralf.
> 
> $ grep -r -C10 "props\.fb_blank" .
> ./drivers/video/backlight/corgi_lcd.c-  if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
> ./drivers/video/backlight/corgi_lcd.c-
> ./drivers/video/backlight/corgi_lcd.c:  if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
> --
> ./drivers/video/backlight/adp8860_bl.c- if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
> ./drivers/video/backlight/adp8860_bl.c-
> ./drivers/video/backlight/adp8860_bl.c: if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
> --
> ./drivers/video/backlight/hp680_bl.c-   if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/hp680_bl.c-           intensity = 0;
> ./drivers/video/backlight/hp680_bl.c:   if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/hp680_bl.c-           intensity = 0;
> --
> ./drivers/video/backlight/cr_bllcd.c-static int
> cr_backlight_set_intensity(struct backlight_device *bd)
> ./drivers/video/backlight/cr_bllcd.c-{
> ./drivers/video/backlight/cr_bllcd.c-   int intensity =
> bd->props.brightness;
> ./drivers/video/backlight/cr_bllcd.c-   u32 addr = gpio_bar +
> CRVML_PANEL_PORT;
> ./drivers/video/backlight/cr_bllcd.c-   u32 cur = inl(addr);
> ./drivers/video/backlight/cr_bllcd.c-
> ./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power ==
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/cr_bllcd.c-           intensity =
> FB_BLANK_UNBLANK;
> ./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank ==
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/cr_bllcd.c-           intensity =
> FB_BLANK_UNBLANK;
> ./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power ==
> FB_BLANK_POWERDOWN)
> ./drivers/video/backlight/cr_bllcd.c-           intensity =
> FB_BLANK_POWERDOWN;
> ./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank ==
> FB_BLANK_POWERDOWN)
> ./drivers/video/backlight/cr_bllcd.c-           intensity =
> FB_BLANK_POWERDOWN;
> --
> ./drivers/video/backlight/max8925_bl.c- if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> ./drivers/video/backlight/max8925_bl.c-
> ./drivers/video/backlight/max8925_bl.c: if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> ./drivers/video/backlight/max8925_bl.c-
> ./drivers/video/backlight/max8925_bl.c- if (bl->props.state &
> BL_CORE_SUSPENDED)
> ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> --
> ./drivers/video/backlight/lv5207lp.c-   if (backlight->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/lv5207lp.c: backlight->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/lv5207lp.c- backlight->props.state &
> (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> ./drivers/video/backlight/lv5207lp.c-           brightness = 0;
> --
> ./drivers/video/backlight/lm3533_bl.c-  if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
> ./drivers/video/backlight/lm3533_bl.c:  if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
> --
> ./drivers/video/backlight/omap1_bl.c-static int omapbl_update_status(struct
> backlight_device *dev)
> ./drivers/video/backlight/omap1_bl.c-{
> ./drivers/video/backlight/omap1_bl.c-   struct omap_backlight *bl =
> bl_get_data(dev);
> ./drivers/video/backlight/omap1_bl.c-
> ./drivers/video/backlight/omap1_bl.c-   if (bl->current_intensity !=
> dev->props.brightness) {
> ./drivers/video/backlight/omap1_bl.c-           if (bl->powermode ==
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/omap1_bl.c-
> omapbl_send_intensity(dev->props.brightness);
> ./drivers/video/backlight/omap1_bl.c- bl->current_intensity =
> dev->props.brightness;
> ./drivers/video/backlight/omap1_bl.c-   }
> ./drivers/video/backlight/omap1_bl.c-
> ./drivers/video/backlight/omap1_bl.c:   if (dev->props.fb_blank !=
> bl->powermode)
> ./drivers/video/backlight/omap1_bl.c: omapbl_set_power(dev,
> dev->props.fb_blank);
> ./drivers/video/backlight/omap1_bl.c-
> ./drivers/video/backlight/omap1_bl.c-   return 0;
> ./drivers/video/backlight/omap1_bl.c-}
> ./drivers/video/backlight/omap1_bl.c-
> --
> ./drivers/video/backlight/kb3886_bl.c-  if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
> ./drivers/video/backlight/kb3886_bl.c:  if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
> --
> ./drivers/video/backlight/pwm_bl.c-     if (bl->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/pwm_bl.c:         bl->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/pwm_bl.c-         bl->props.state &
> BL_CORE_FBBLANK)
> ./drivers/video/backlight/pwm_bl.c-             brightness = 0;
> --
> ./drivers/video/backlight/pm8941-wled.c-        if (bl->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/pm8941-wled.c: bl->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/pm8941-wled.c- bl->props.state & BL_CORE_FBBLANK)
> ./drivers/video/backlight/pm8941-wled.c-                val = 0;
> --
> ./drivers/video/backlight/adp8870_bl.c- if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
> ./drivers/video/backlight/adp8870_bl.c-
> ./drivers/video/backlight/adp8870_bl.c: if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
> --
> ./drivers/video/backlight/as3711_bl.c-  if (bl->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/as3711_bl.c:      bl->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/as3711_bl.c-      bl->props.state &
> (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> ./drivers/video/backlight/as3711_bl.c-          brightness = 0;
> --
> ./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> ./drivers/video/backlight/88pm860x_bl.c-
> ./drivers/video/backlight/88pm860x_bl.c:        if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> ./drivers/video/backlight/88pm860x_bl.c-
> ./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.state &
> BL_CORE_SUSPENDED)
> ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> --
> ./drivers/video/backlight/tps65217_bl.c-        if (bl->props.state &
> BL_CORE_SUSPENDED)
> ./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
> ./drivers/video/backlight/tps65217_bl.c-
> ./drivers/video/backlight/tps65217_bl.c-        if ((bl->props.power !=
> FB_BLANK_UNBLANK) ||
> ./drivers/video/backlight/tps65217_bl.c: (bl->props.fb_blank !=
> FB_BLANK_UNBLANK))
> ./drivers/video/backlight/tps65217_bl.c-                /* framebuffer in
> low power mode or blanking active */
> ./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
> --
> ./drivers/video/backlight/adp5520_bl.c- if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
> ./drivers/video/backlight/adp5520_bl.c-
> ./drivers/video/backlight/adp5520_bl.c: if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
> --
> ./drivers/video/backlight/wm831x_bl.c-  if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> ./drivers/video/backlight/wm831x_bl.c-
> ./drivers/video/backlight/wm831x_bl.c:  if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> ./drivers/video/backlight/wm831x_bl.c-
> ./drivers/video/backlight/wm831x_bl.c-  if (bl->props.state &
> BL_CORE_SUSPENDED)
> ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> --
> ./drivers/video/backlight/gpio_backlight.c-     if (bl->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/gpio_backlight.c: bl->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/gpio_backlight.c- bl->props.state &
> (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> ./drivers/video/backlight/gpio_backlight.c-             brightness = 0;
> --
> ./drivers/video/backlight/da903x_bl.c-  if (bl->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> ./drivers/video/backlight/da903x_bl.c-
> ./drivers/video/backlight/da903x_bl.c:  if (bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> ./drivers/video/backlight/da903x_bl.c-
> ./drivers/video/backlight/da903x_bl.c-  if (bl->props.state &
> BL_CORE_SUSPENDED)
> ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> --
> ./drivers/video/backlight/locomolcd.c-  if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/locomolcd.c-          intensity = 0;
> ./drivers/video/backlight/locomolcd.c:  if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/locomolcd.c-          intensity = 0;
> --
> ./drivers/video/backlight/bd6107.c-     if (backlight->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/bd6107.c: backlight->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/bd6107.c- backlight->props.state &
> (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> ./drivers/video/backlight/bd6107.c-             brightness = 0;
> --
> ./drivers/video/backlight/backlight.c-                  if (fb_blank ==
> FB_BLANK_UNBLANK &&
> ./drivers/video/backlight/backlight.c- !bd->fb_bl_on[node]) {
> ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = true;
> ./drivers/video/backlight/backlight.c-                          if
> (!bd->use_count++) {
> ./drivers/video/backlight/backlight.c- bd->props.state &= ~BL_CORE_FBBLANK;
> ./drivers/video/backlight/backlight.c: bd->props.fb_blank =
> FB_BLANK_UNBLANK;
> ./drivers/video/backlight/backlight.c- backlight_update_status(bd);
> ./drivers/video/backlight/backlight.c-                          }
> ./drivers/video/backlight/backlight.c-                  } else if (fb_blank
> != FB_BLANK_UNBLANK &&
> ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node]) {
> ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = false;
> ./drivers/video/backlight/backlight.c-                          if
> (!(--bd->use_count)) {
> ./drivers/video/backlight/backlight.c- bd->props.state |= BL_CORE_FBBLANK;
> ./drivers/video/backlight/backlight.c: bd->props.fb_blank = fb_blank;
> ./drivers/video/backlight/backlight.c- backlight_update_status(bd);
> --
> ./drivers/video/backlight/ep93xx_bl.c-  if (bl->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/backlight/ep93xx_bl.c:      bl->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/backlight/ep93xx_bl.c-          brightness = 0;
> --
> ./drivers/video/backlight/jornada720_bl.c:      if ((bd->props.power !=
> FB_BLANK_UNBLANK) || (bd->props.fb_blank != FB_BLANK_UNBLANK)) {
> --
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c:     if
> (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- dev->props.power
> == FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level =
> dev->props.brightness;
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c-     else
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level = 0;
> --
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c: if
> (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c-
> dev->props.power == FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- level =
> dev->props.brightness;
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- else
> ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- level =
> 0;
> --
> ./drivers/video/fbdev/aty/atyfb_base.c- if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/aty/atyfb_base.c:     bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/aty/atyfb_base.c-         level = 0;
> ./drivers/video/fbdev/aty/atyfb_base.c- else
> ./drivers/video/fbdev/aty/atyfb_base.c-         level =
> bd->props.brightness;
> --
> ./drivers/video/fbdev/aty/aty128fb.c-   if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/aty/aty128fb.c:       bd->props.fb_blank !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/aty/aty128fb.c-       !par->lcd_on)
> ./drivers/video/fbdev/aty/aty128fb.c-           level = 0;
> ./drivers/video/fbdev/aty/aty128fb.c-   else
> ./drivers/video/fbdev/aty/aty128fb.c-           level =
> bd->props.brightness;
> --
> ./drivers/video/fbdev/aty/radeon_backlight.c-        if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/aty/radeon_backlight.c: bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/aty/radeon_backlight.c-           level = 0;
> ./drivers/video/fbdev/aty/radeon_backlight.c-   else
> ./drivers/video/fbdev/aty/radeon_backlight.c-           level =
> bd->props.brightness;
> --
> ./drivers/video/fbdev/mx3fb.c-  if (bl->props.power != FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/mx3fb.c-          brightness = 0;
> ./drivers/video/fbdev/mx3fb.c:  if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/mx3fb.c-          brightness = 0;
> --
> ./drivers/video/fbdev/riva/fbdev.c-     if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/riva/fbdev.c:         bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/riva/fbdev.c-             level = 0;
> ./drivers/video/fbdev/riva/fbdev.c-     else
> ./drivers/video/fbdev/riva/fbdev.c-             level =
> bd->props.brightness;
> --
> ./drivers/video/fbdev/atmel_lcdfb.c:    if (bl->props.fb_blank !=
> sinfo->bl_power)
> ./drivers/video/fbdev/atmel_lcdfb.c:            power = bl->props.fb_blank;
> ./drivers/video/fbdev/atmel_lcdfb.c-    else if (bl->props.power !=
> sinfo->bl_power)
> ./drivers/video/fbdev/atmel_lcdfb.c-            power = bl->props.power;
> --
> ./drivers/video/fbdev/nvidia/nv_backlight.c-    if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/video/fbdev/nvidia/nv_backlight.c: bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/video/fbdev/nvidia/nv_backlight.c-            level = 0;
> ./drivers/video/fbdev/nvidia/nv_backlight.c-    else
> ./drivers/video/fbdev/nvidia/nv_backlight.c-            level =
> bd->props.brightness;
> --
> ./drivers/macintosh/via-pmu-backlight.c-        if (bd->props.power !=
> FB_BLANK_UNBLANK ||
> ./drivers/macintosh/via-pmu-backlight.c: bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/macintosh/via-pmu-backlight.c-                level = 0;
> --
> ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c:      if
> (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- dev->props.power ==
> FB_BLANK_UNBLANK)
> ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level =
> dev->props.brightness;
> ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c-      else
> ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level = 0;
> --
> ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c:      if
> (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- dev->props.power
> == FB_BLANK_UNBLANK)
> ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level =
> dev->props.brightness;
> ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c-      else
> ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level = 0;
> --
> ./drivers/staging/fbtft/fb_ssd1351.c-   on = (bd->props.power ==
> FB_BLANK_UNBLANK) &&
> ./drivers/staging/fbtft/fb_ssd1351.c:        (bd->props.fb_blank ==
> FB_BLANK_UNBLANK);
> --
> ./drivers/staging/fbtft/fbtft-core.c-   if ((bd->props.power ==
> FB_BLANK_UNBLANK) &&
> ./drivers/staging/fbtft/fbtft-core.c:       (bd->props.fb_blank ==
> FB_BLANK_UNBLANK))
> --
> ./drivers/staging/fbtft/fb_watterott.c- if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
> ./drivers/staging/fbtft/fb_watterott.c-
> ./drivers/staging/fbtft/fb_watterott.c: if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
> --
> ./drivers/auxdisplay/ht16k33.c- if (bl->props.power != FB_BLANK_UNBLANK ||
> ./drivers/auxdisplay/ht16k33.c:     bl->props.fb_blank != FB_BLANK_UNBLANK
> ||
> ./drivers/auxdisplay/ht16k33.c-     bl->props.state & BL_CORE_FBBLANK ||
> brightness == 0) {
> ./drivers/auxdisplay/ht16k33.c-         return ht16k33_display_off(priv);
> --
> ./drivers/platform/x86/thinkpad_acpi.c: (bd->props.fb_blank ==
> FB_BLANK_UNBLANK &&
> ./drivers/platform/x86/thinkpad_acpi.c-          bd->props.power ==
> FB_BLANK_UNBLANK) ?
> ./drivers/platform/x86/thinkpad_acpi.c- bd->props.brightness : 0;
> --
> ./drivers/platform/x86/acer-wmi.c-      if (bd->props.power !=
> FB_BLANK_UNBLANK)
> ./drivers/platform/x86/acer-wmi.c-              intensity = 0;
> ./drivers/platform/x86/acer-wmi.c:      if (bd->props.fb_blank !=
> FB_BLANK_UNBLANK)
> ./drivers/platform/x86/acer-wmi.c-              intensity = 0;
> 
> >
> >Daniel.
> >
> >
> >>---
> >>  include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
> >>  1 file changed, 30 insertions(+)
> >>
> >>diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >>index af7003548..7b6a9a2a3 100644
> >>--- a/include/linux/backlight.h
> >>+++ b/include/linux/backlight.h
> >>@@ -130,6 +130,36 @@ static inline int backlight_update_status(struct
> >>backlight_device *bd)
> >>      return ret;
> >>  }
> >>  +/**
> >>+  * backlight_enable - Enable backlight
> >>+  * @bd: the backlight device to enable
> >>+  */
> >>+static inline int backlight_enable(struct backlight_device *bd)
> >>+{
> >>+    if (!bd)
> >>+        return 0;
> >>+
> >>+    bd->props.power = FB_BLANK_UNBLANK;
> >>+    bd->props.state &= ~BL_CORE_FBBLANK;
> >>+
> >>+    return backlight_update_status(bd);
> >>+}
> >>+
> >>+/**
> >>+  * backlight_disable - Disable backlight
> >>+  * @bd: the backlight device to disable
> >>+  */
> >>+static inline int backlight_disable(struct backlight_device *bd)
> >>+{
> >>+    if (!bd)
> >>+        return 0;
> >>+
> >>+    bd->props.power = FB_BLANK_POWERDOWN;
> >>+    bd->props.state |= BL_CORE_FBBLANK;
> >>+
> >>+    return backlight_update_status(bd);
> >>+}
> >>+
> >>  extern struct backlight_device *backlight_device_register(const char
> >>*name,
> >>      struct device *dev, void *devdata, const struct backlight_ops
> >>*ops,
> >>      const struct backlight_properties *props);
> >>
> 

Re: [PATCH v16 05/10] video: backlight: Add devres versions of of_find_backlight

[Meghana Madhyastha]

On Wed, Jan 17, 2018 at 05:09:57PM +0000, Daniel Thompson wrote:
> On 16/01/18 10:34, Meghana Madhyastha wrote:
> >Add devm_of_find_backlight and the corresponding release
> >function because some drivers use devres versions of functions
> >for acquiring device resources.
> >
> >Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> >---
> >  drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
> >  include/linux/backlight.h           |  7 +++++++
> >  2 files changed, 36 insertions(+)
> >
> >diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> >index 7e4a5d77d..b3f76945f 100644
> >--- a/drivers/video/backlight/backlight.c
> >+++ b/drivers/video/backlight/backlight.c
> >@@ -620,6 +620,35 @@ struct backlight_device *of_find_backlight(struct device *dev)
> >  }
> >  EXPORT_SYMBOL(of_find_backlight);
> >+static void devm_backlight_put(void *data)
> >+{
> >+	backlight_put(data);
> 
> Shouldn't this be using devres_release()?

backlight_put calls put_device which essentially does a release right?
And looking at the code in other driver, looks like most of them use
put_device (to the best of my knowledge, correct me if I'm mistaken). 

Thanks and regards,
Meghana

> 
> >+}
> >+
> >+/**
> >+  * devm_of_find_backlight - Resource-managed of_find_backlight()
> >+  * @dev: Device
> >+  *
> >+  * Device managed version of of_find_backlight(). The reference on the backlight
> >+  * device is automatically dropped on driver detach.
> >+  */
> >+struct backlight_device *devm_of_find_backlight(struct device *dev)
> >+{
> >+	struct backlight_device *bd;
> >+	int ret;
> >+
> >+	bd = of_find_backlight(dev);
> >+	if (IS_ERR_OR_NULL(bd))
> >+		return bd;
> >+	ret = devm_add_action(dev, devm_backlight_put, bd);
> >+	if (ret) {
> >+		backlight_put(bd);
> >+		return ERR_PTR(ret);
> >+	}
> >+	return bd;
> >+}
> >+EXPORT_SYMBOL(devm_of_find_backlight);
> >+
> >  static void __exit backlight_class_exit(void)
> >  {
> >  	class_destroy(backlight_class);
> >diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >index 32ea510da..1d373f5a6 100644
> >--- a/include/linux/backlight.h
> >+++ b/include/linux/backlight.h
> >@@ -215,11 +215,18 @@ of_find_backlight_by_node(struct device_node *node)
> >  #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> >  struct backlight_device *of_find_backlight(struct device *dev);
> >+struct backlight_device *devm_of_find_backlight(struct device *dev);
> >  #else
> >  static inline struct backlight_device *of_find_backlight(struct device *dev)
> >  {
> >  	return NULL;
> >  }
> >+
> >+static inline struct backlight_device *
> >+devm_of_find_backlight(struct device *dev)
> >+{
> >+	return NULL;
> >+}
> >  #endif
> >  #endif
> >

Re: [PATCH v16 05/10] video: backlight: Add devres versions of of_find_backlight

[Daniel Thompson]

On Thu, Jan 18, 2018 at 04:32:26PM +0530, Meghana Madhyastha wrote:
> On Wed, Jan 17, 2018 at 05:09:57PM +0000, Daniel Thompson wrote:
> > On 16/01/18 10:34, Meghana Madhyastha wrote:
> > >Add devm_of_find_backlight and the corresponding release
> > >function because some drivers use devres versions of functions
> > >for acquiring device resources.
> > >
> > >Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> > >---
> > >  drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
> > >  include/linux/backlight.h           |  7 +++++++
> > >  2 files changed, 36 insertions(+)
> > >
> > >diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> > >index 7e4a5d77d..b3f76945f 100644
> > >--- a/drivers/video/backlight/backlight.c
> > >+++ b/drivers/video/backlight/backlight.c
> > >@@ -620,6 +620,35 @@ struct backlight_device *of_find_backlight(struct device *dev)
> > >  }
> > >  EXPORT_SYMBOL(of_find_backlight);
> > >+static void devm_backlight_put(void *data)
> > >+{
> > >+	backlight_put(data);
> > 
> > Shouldn't this be using devres_release()?
> 
> backlight_put calls put_device which essentially does a release right?
> And looking at the code in other driver, looks like most of them use
> put_device (to the best of my knowledge, correct me if I'm mistaken). 

Sorry, the name confused me. I mistakenly though this was API code like
devm_gpiod_put and devm_clk_put, rather than a static method.

Whilst its my fault for overlooking the "static" please could rename this to
devm_backlight_release(). I don't want to keep being confused every time
I re-read it!


Daniel.

> 
> Thanks and regards,
> Meghana
> 
> > 
> > >+}
> > >+
> > >+/**
> > >+  * devm_of_find_backlight - Resource-managed of_find_backlight()
> > >+  * @dev: Device
> > >+  *
> > >+  * Device managed version of of_find_backlight(). The reference on the backlight
> > >+  * device is automatically dropped on driver detach.
> > >+  */
> > >+struct backlight_device *devm_of_find_backlight(struct device *dev)
> > >+{
> > >+	struct backlight_device *bd;
> > >+	int ret;
> > >+
> > >+	bd = of_find_backlight(dev);
> > >+	if (IS_ERR_OR_NULL(bd))
> > >+		return bd;
> > >+	ret = devm_add_action(dev, devm_backlight_put, bd);
> > >+	if (ret) {
> > >+		backlight_put(bd);
> > >+		return ERR_PTR(ret);
> > >+	}
> > >+	return bd;
> > >+}
> > >+EXPORT_SYMBOL(devm_of_find_backlight);
> > >+
> > >  static void __exit backlight_class_exit(void)
> > >  {
> > >  	class_destroy(backlight_class);
> > >diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > >index 32ea510da..1d373f5a6 100644
> > >--- a/include/linux/backlight.h
> > >+++ b/include/linux/backlight.h
> > >@@ -215,11 +215,18 @@ of_find_backlight_by_node(struct device_node *node)
> > >  #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> > >  struct backlight_device *of_find_backlight(struct device *dev);
> > >+struct backlight_device *devm_of_find_backlight(struct device *dev);
> > >  #else
> > >  static inline struct backlight_device *of_find_backlight(struct device *dev)
> > >  {
> > >  	return NULL;
> > >  }
> > >+
> > >+static inline struct backlight_device *
> > >+devm_of_find_backlight(struct device *dev)
> > >+{
> > >+	return NULL;
> > >+}
> > >  #endif
> > >  #endif
> > >

Re: [PATCH v16 01/10] video: backlight: Add helpers to enable and disable backlight

[Daniel Thompson]

On Thu, Jan 18, 2018 at 04:29:23PM +0530, Meghana Madhyastha wrote:
> On Wed, Jan 17, 2018 at 11:03:24PM +0100, Noralf Trønnes wrote:
> > 
> > Den 17.01.2018 18.00, skrev Daniel Thompson:
> > >
> > >
> > >On 16/01/18 10:31, Meghana Madhyastha wrote:
> > >>Add helper functions backlight_enable and backlight_disable to
> > >>enable/disable a backlight device. These helper functions can
> > >>then be used by different drm and tinydrm drivers to avoid
> > >>repetition of code and also to enforce a uniform and consistent
> > >>way to enable/disable a backlight device.
> > >>
> > >>Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> > >
> > >To be clear I don't disagree with anthing Daniel V. said about the
> > >horribly confused (and confusing) power states for backlight.
> > >
> > >Nevertheless I don't recall seeing any response (positive or negative) to
> > >this post from v13:
> > >https://www.spinics.net/lists/dri-devel/msg154459.html
> > >
> > 
> > I see that Daniel V has answered while I was chasing this down, but anyways:
> > 
> > A grep suggests that omap1_bl is the only driver that only checks fb_blank.
> > All the other drivers check both fb_blank and power, a few check state. The
> > backlight fbdev notifier callback doesn't set power, but sets fb_blank and
> > state.
> > 
> > fb_blank was marked 'Due to be removed' 9 years ago, so it hasn't been
> > high priority.
> > 
> > So for completeness I guess it makes sense to set fb_blank.
> 
> So if I understood correctly, the suggestion is to set fb_blank along
> with power i.e something like this in backlight_enable.
>         bd->props.power = FB_BLANK_UNBLANK;
> +       bd->props.fb_blank = FB_BLANK_UNBLANK;
>         bd->props.state &= ~BL_CORE_FBBLANK;
> 
> and set it to FB_BLANK_POWERDOWN in backlight_disable ?

Yes please.

Note that strictly speaking it is not "along with power" it is "along with
clearing the BL_CORE_FBBLANK bit" since that is what fb_blank must be
consistent with.


Daniel.

> 
> Thanks and regards,
> Meghana
> 
> > Noralf.
> > 
> > $ grep -r -C10 "props\.fb_blank" .
> > ./drivers/video/backlight/corgi_lcd.c-  if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
> > ./drivers/video/backlight/corgi_lcd.c-
> > ./drivers/video/backlight/corgi_lcd.c:  if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/corgi_lcd.c-          intensity = 0;
> > --
> > ./drivers/video/backlight/adp8860_bl.c- if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
> > ./drivers/video/backlight/adp8860_bl.c-
> > ./drivers/video/backlight/adp8860_bl.c: if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp8860_bl.c-         brightness = 0;
> > --
> > ./drivers/video/backlight/hp680_bl.c-   if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/hp680_bl.c-           intensity = 0;
> > ./drivers/video/backlight/hp680_bl.c:   if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/hp680_bl.c-           intensity = 0;
> > --
> > ./drivers/video/backlight/cr_bllcd.c-static int
> > cr_backlight_set_intensity(struct backlight_device *bd)
> > ./drivers/video/backlight/cr_bllcd.c-{
> > ./drivers/video/backlight/cr_bllcd.c-   int intensity =
> > bd->props.brightness;
> > ./drivers/video/backlight/cr_bllcd.c-   u32 addr = gpio_bar +
> > CRVML_PANEL_PORT;
> > ./drivers/video/backlight/cr_bllcd.c-   u32 cur = inl(addr);
> > ./drivers/video/backlight/cr_bllcd.c-
> > ./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power ==
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/cr_bllcd.c-           intensity =
> > FB_BLANK_UNBLANK;
> > ./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank ==
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/cr_bllcd.c-           intensity =
> > FB_BLANK_UNBLANK;
> > ./drivers/video/backlight/cr_bllcd.c-   if (bd->props.power ==
> > FB_BLANK_POWERDOWN)
> > ./drivers/video/backlight/cr_bllcd.c-           intensity =
> > FB_BLANK_POWERDOWN;
> > ./drivers/video/backlight/cr_bllcd.c:   if (bd->props.fb_blank ==
> > FB_BLANK_POWERDOWN)
> > ./drivers/video/backlight/cr_bllcd.c-           intensity =
> > FB_BLANK_POWERDOWN;
> > --
> > ./drivers/video/backlight/max8925_bl.c- if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> > ./drivers/video/backlight/max8925_bl.c-
> > ./drivers/video/backlight/max8925_bl.c: if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> > ./drivers/video/backlight/max8925_bl.c-
> > ./drivers/video/backlight/max8925_bl.c- if (bl->props.state &
> > BL_CORE_SUSPENDED)
> > ./drivers/video/backlight/max8925_bl.c-         brightness = 0;
> > --
> > ./drivers/video/backlight/lv5207lp.c-   if (backlight->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/lv5207lp.c: backlight->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/lv5207lp.c- backlight->props.state &
> > (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > ./drivers/video/backlight/lv5207lp.c-           brightness = 0;
> > --
> > ./drivers/video/backlight/lm3533_bl.c-  if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
> > ./drivers/video/backlight/lm3533_bl.c:  if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/lm3533_bl.c-          brightness = 0;
> > --
> > ./drivers/video/backlight/omap1_bl.c-static int omapbl_update_status(struct
> > backlight_device *dev)
> > ./drivers/video/backlight/omap1_bl.c-{
> > ./drivers/video/backlight/omap1_bl.c-   struct omap_backlight *bl =
> > bl_get_data(dev);
> > ./drivers/video/backlight/omap1_bl.c-
> > ./drivers/video/backlight/omap1_bl.c-   if (bl->current_intensity !=
> > dev->props.brightness) {
> > ./drivers/video/backlight/omap1_bl.c-           if (bl->powermode ==
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/omap1_bl.c-
> > omapbl_send_intensity(dev->props.brightness);
> > ./drivers/video/backlight/omap1_bl.c- bl->current_intensity =
> > dev->props.brightness;
> > ./drivers/video/backlight/omap1_bl.c-   }
> > ./drivers/video/backlight/omap1_bl.c-
> > ./drivers/video/backlight/omap1_bl.c:   if (dev->props.fb_blank !=
> > bl->powermode)
> > ./drivers/video/backlight/omap1_bl.c: omapbl_set_power(dev,
> > dev->props.fb_blank);
> > ./drivers/video/backlight/omap1_bl.c-
> > ./drivers/video/backlight/omap1_bl.c-   return 0;
> > ./drivers/video/backlight/omap1_bl.c-}
> > ./drivers/video/backlight/omap1_bl.c-
> > --
> > ./drivers/video/backlight/kb3886_bl.c-  if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
> > ./drivers/video/backlight/kb3886_bl.c:  if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/kb3886_bl.c-          intensity = 0;
> > --
> > ./drivers/video/backlight/pwm_bl.c-     if (bl->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/pwm_bl.c:         bl->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/pwm_bl.c-         bl->props.state &
> > BL_CORE_FBBLANK)
> > ./drivers/video/backlight/pwm_bl.c-             brightness = 0;
> > --
> > ./drivers/video/backlight/pm8941-wled.c-        if (bl->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/pm8941-wled.c: bl->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/pm8941-wled.c- bl->props.state & BL_CORE_FBBLANK)
> > ./drivers/video/backlight/pm8941-wled.c-                val = 0;
> > --
> > ./drivers/video/backlight/adp8870_bl.c- if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
> > ./drivers/video/backlight/adp8870_bl.c-
> > ./drivers/video/backlight/adp8870_bl.c: if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp8870_bl.c-         brightness = 0;
> > --
> > ./drivers/video/backlight/as3711_bl.c-  if (bl->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/as3711_bl.c:      bl->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/as3711_bl.c-      bl->props.state &
> > (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > ./drivers/video/backlight/as3711_bl.c-          brightness = 0;
> > --
> > ./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> > ./drivers/video/backlight/88pm860x_bl.c-
> > ./drivers/video/backlight/88pm860x_bl.c:        if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> > ./drivers/video/backlight/88pm860x_bl.c-
> > ./drivers/video/backlight/88pm860x_bl.c-        if (bl->props.state &
> > BL_CORE_SUSPENDED)
> > ./drivers/video/backlight/88pm860x_bl.c-                brightness = 0;
> > --
> > ./drivers/video/backlight/tps65217_bl.c-        if (bl->props.state &
> > BL_CORE_SUSPENDED)
> > ./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
> > ./drivers/video/backlight/tps65217_bl.c-
> > ./drivers/video/backlight/tps65217_bl.c-        if ((bl->props.power !=
> > FB_BLANK_UNBLANK) ||
> > ./drivers/video/backlight/tps65217_bl.c: (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK))
> > ./drivers/video/backlight/tps65217_bl.c-                /* framebuffer in
> > low power mode or blanking active */
> > ./drivers/video/backlight/tps65217_bl.c-                brightness = 0;
> > --
> > ./drivers/video/backlight/adp5520_bl.c- if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
> > ./drivers/video/backlight/adp5520_bl.c-
> > ./drivers/video/backlight/adp5520_bl.c: if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/adp5520_bl.c-         brightness = 0;
> > --
> > ./drivers/video/backlight/wm831x_bl.c-  if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> > ./drivers/video/backlight/wm831x_bl.c-
> > ./drivers/video/backlight/wm831x_bl.c:  if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> > ./drivers/video/backlight/wm831x_bl.c-
> > ./drivers/video/backlight/wm831x_bl.c-  if (bl->props.state &
> > BL_CORE_SUSPENDED)
> > ./drivers/video/backlight/wm831x_bl.c-          brightness = 0;
> > --
> > ./drivers/video/backlight/gpio_backlight.c-     if (bl->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/gpio_backlight.c: bl->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/gpio_backlight.c- bl->props.state &
> > (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > ./drivers/video/backlight/gpio_backlight.c-             brightness = 0;
> > --
> > ./drivers/video/backlight/da903x_bl.c-  if (bl->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> > ./drivers/video/backlight/da903x_bl.c-
> > ./drivers/video/backlight/da903x_bl.c:  if (bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> > ./drivers/video/backlight/da903x_bl.c-
> > ./drivers/video/backlight/da903x_bl.c-  if (bl->props.state &
> > BL_CORE_SUSPENDED)
> > ./drivers/video/backlight/da903x_bl.c-          brightness = 0;
> > --
> > ./drivers/video/backlight/locomolcd.c-  if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/locomolcd.c-          intensity = 0;
> > ./drivers/video/backlight/locomolcd.c:  if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/locomolcd.c-          intensity = 0;
> > --
> > ./drivers/video/backlight/bd6107.c-     if (backlight->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/bd6107.c: backlight->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/bd6107.c- backlight->props.state &
> > (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > ./drivers/video/backlight/bd6107.c-             brightness = 0;
> > --
> > ./drivers/video/backlight/backlight.c-                  if (fb_blank ==
> > FB_BLANK_UNBLANK &&
> > ./drivers/video/backlight/backlight.c- !bd->fb_bl_on[node]) {
> > ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = true;
> > ./drivers/video/backlight/backlight.c-                          if
> > (!bd->use_count++) {
> > ./drivers/video/backlight/backlight.c- bd->props.state &= ~BL_CORE_FBBLANK;
> > ./drivers/video/backlight/backlight.c: bd->props.fb_blank =
> > FB_BLANK_UNBLANK;
> > ./drivers/video/backlight/backlight.c- backlight_update_status(bd);
> > ./drivers/video/backlight/backlight.c-                          }
> > ./drivers/video/backlight/backlight.c-                  } else if (fb_blank
> > != FB_BLANK_UNBLANK &&
> > ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node]) {
> > ./drivers/video/backlight/backlight.c- bd->fb_bl_on[node] = false;
> > ./drivers/video/backlight/backlight.c-                          if
> > (!(--bd->use_count)) {
> > ./drivers/video/backlight/backlight.c- bd->props.state |= BL_CORE_FBBLANK;
> > ./drivers/video/backlight/backlight.c: bd->props.fb_blank = fb_blank;
> > ./drivers/video/backlight/backlight.c- backlight_update_status(bd);
> > --
> > ./drivers/video/backlight/ep93xx_bl.c-  if (bl->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/backlight/ep93xx_bl.c:      bl->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/backlight/ep93xx_bl.c-          brightness = 0;
> > --
> > ./drivers/video/backlight/jornada720_bl.c:      if ((bd->props.power !=
> > FB_BLANK_UNBLANK) || (bd->props.fb_blank != FB_BLANK_UNBLANK)) {
> > --
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c:     if
> > (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- dev->props.power
> > == FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level =
> > dev->props.brightness;
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c-     else
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c- level = 0;
> > --
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c: if
> > (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c-
> > dev->props.power == FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- level =
> > dev->props.brightness;
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- else
> > ./drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c- level =
> > 0;
> > --
> > ./drivers/video/fbdev/aty/atyfb_base.c- if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/aty/atyfb_base.c:     bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/aty/atyfb_base.c-         level = 0;
> > ./drivers/video/fbdev/aty/atyfb_base.c- else
> > ./drivers/video/fbdev/aty/atyfb_base.c-         level =
> > bd->props.brightness;
> > --
> > ./drivers/video/fbdev/aty/aty128fb.c-   if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/aty/aty128fb.c:       bd->props.fb_blank !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/aty/aty128fb.c-       !par->lcd_on)
> > ./drivers/video/fbdev/aty/aty128fb.c-           level = 0;
> > ./drivers/video/fbdev/aty/aty128fb.c-   else
> > ./drivers/video/fbdev/aty/aty128fb.c-           level =
> > bd->props.brightness;
> > --
> > ./drivers/video/fbdev/aty/radeon_backlight.c-        if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/aty/radeon_backlight.c: bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/aty/radeon_backlight.c-           level = 0;
> > ./drivers/video/fbdev/aty/radeon_backlight.c-   else
> > ./drivers/video/fbdev/aty/radeon_backlight.c-           level =
> > bd->props.brightness;
> > --
> > ./drivers/video/fbdev/mx3fb.c-  if (bl->props.power != FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/mx3fb.c-          brightness = 0;
> > ./drivers/video/fbdev/mx3fb.c:  if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/mx3fb.c-          brightness = 0;
> > --
> > ./drivers/video/fbdev/riva/fbdev.c-     if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/riva/fbdev.c:         bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/riva/fbdev.c-             level = 0;
> > ./drivers/video/fbdev/riva/fbdev.c-     else
> > ./drivers/video/fbdev/riva/fbdev.c-             level =
> > bd->props.brightness;
> > --
> > ./drivers/video/fbdev/atmel_lcdfb.c:    if (bl->props.fb_blank !=
> > sinfo->bl_power)
> > ./drivers/video/fbdev/atmel_lcdfb.c:            power = bl->props.fb_blank;
> > ./drivers/video/fbdev/atmel_lcdfb.c-    else if (bl->props.power !=
> > sinfo->bl_power)
> > ./drivers/video/fbdev/atmel_lcdfb.c-            power = bl->props.power;
> > --
> > ./drivers/video/fbdev/nvidia/nv_backlight.c-    if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/video/fbdev/nvidia/nv_backlight.c: bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/video/fbdev/nvidia/nv_backlight.c-            level = 0;
> > ./drivers/video/fbdev/nvidia/nv_backlight.c-    else
> > ./drivers/video/fbdev/nvidia/nv_backlight.c-            level =
> > bd->props.brightness;
> > --
> > ./drivers/macintosh/via-pmu-backlight.c-        if (bd->props.power !=
> > FB_BLANK_UNBLANK ||
> > ./drivers/macintosh/via-pmu-backlight.c: bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/macintosh/via-pmu-backlight.c-                level = 0;
> > --
> > ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c:      if
> > (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- dev->props.power ==
> > FB_BLANK_UNBLANK)
> > ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level =
> > dev->props.brightness;
> > ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c-      else
> > ./drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c- level = 0;
> > --
> > ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c:      if
> > (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- dev->props.power
> > == FB_BLANK_UNBLANK)
> > ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level =
> > dev->props.brightness;
> > ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c-      else
> > ./drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c- level = 0;
> > --
> > ./drivers/staging/fbtft/fb_ssd1351.c-   on = (bd->props.power ==
> > FB_BLANK_UNBLANK) &&
> > ./drivers/staging/fbtft/fb_ssd1351.c:        (bd->props.fb_blank ==
> > FB_BLANK_UNBLANK);
> > --
> > ./drivers/staging/fbtft/fbtft-core.c-   if ((bd->props.power ==
> > FB_BLANK_UNBLANK) &&
> > ./drivers/staging/fbtft/fbtft-core.c:       (bd->props.fb_blank ==
> > FB_BLANK_UNBLANK))
> > --
> > ./drivers/staging/fbtft/fb_watterott.c- if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
> > ./drivers/staging/fbtft/fb_watterott.c-
> > ./drivers/staging/fbtft/fb_watterott.c: if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/staging/fbtft/fb_watterott.c-         brightness = 0;
> > --
> > ./drivers/auxdisplay/ht16k33.c- if (bl->props.power != FB_BLANK_UNBLANK ||
> > ./drivers/auxdisplay/ht16k33.c:     bl->props.fb_blank != FB_BLANK_UNBLANK
> > ||
> > ./drivers/auxdisplay/ht16k33.c-     bl->props.state & BL_CORE_FBBLANK ||
> > brightness == 0) {
> > ./drivers/auxdisplay/ht16k33.c-         return ht16k33_display_off(priv);
> > --
> > ./drivers/platform/x86/thinkpad_acpi.c: (bd->props.fb_blank ==
> > FB_BLANK_UNBLANK &&
> > ./drivers/platform/x86/thinkpad_acpi.c-          bd->props.power ==
> > FB_BLANK_UNBLANK) ?
> > ./drivers/platform/x86/thinkpad_acpi.c- bd->props.brightness : 0;
> > --
> > ./drivers/platform/x86/acer-wmi.c-      if (bd->props.power !=
> > FB_BLANK_UNBLANK)
> > ./drivers/platform/x86/acer-wmi.c-              intensity = 0;
> > ./drivers/platform/x86/acer-wmi.c:      if (bd->props.fb_blank !=
> > FB_BLANK_UNBLANK)
> > ./drivers/platform/x86/acer-wmi.c-              intensity = 0;
> > 
> > >
> > >Daniel.
> > >
> > >
> > >>---
> > >>  include/linux/backlight.h | 30 ++++++++++++++++++++++++++++++
> > >>  1 file changed, 30 insertions(+)
> > >>
> > >>diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > >>index af7003548..7b6a9a2a3 100644
> > >>--- a/include/linux/backlight.h
> > >>+++ b/include/linux/backlight.h
> > >>@@ -130,6 +130,36 @@ static inline int backlight_update_status(struct
> > >>backlight_device *bd)
> > >>      return ret;
> > >>  }
> > >>  +/**
> > >>+  * backlight_enable - Enable backlight
> > >>+  * @bd: the backlight device to enable
> > >>+  */
> > >>+static inline int backlight_enable(struct backlight_device *bd)
> > >>+{
> > >>+    if (!bd)
> > >>+        return 0;
> > >>+
> > >>+    bd->props.power = FB_BLANK_UNBLANK;
> > >>+    bd->props.state &= ~BL_CORE_FBBLANK;
> > >>+
> > >>+    return backlight_update_status(bd);
> > >>+}
> > >>+
> > >>+/**
> > >>+  * backlight_disable - Disable backlight
> > >>+  * @bd: the backlight device to disable
> > >>+  */
> > >>+static inline int backlight_disable(struct backlight_device *bd)
> > >>+{
> > >>+    if (!bd)
> > >>+        return 0;
> > >>+
> > >>+    bd->props.power = FB_BLANK_POWERDOWN;
> > >>+    bd->props.state |= BL_CORE_FBBLANK;
> > >>+
> > >>+    return backlight_update_status(bd);
> > >>+}
> > >>+
> > >>  extern struct backlight_device *backlight_device_register(const char
> > >>*name,
> > >>      struct device *dev, void *devdata, const struct backlight_ops
> > >>*ops,
> > >>      const struct backlight_properties *props);
> > >>
> > 

Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper

[Meghana Madhyastha]

On Tue, Jan 16, 2018 at 06:08:53PM +0100, Noralf Trønnes wrote:
> 
> Den 16.01.2018 11.36, skrev Meghana Madhyastha:
> >Replace of_find_backlight_by_node and of the code around it
> >with of_find_backlight helper to avoid repetition of code.
> >
> >Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> >---
> >  drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 10 +++-------
> >  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 +++-------
> >  drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 +++-------
> >  3 files changed, 9 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> >index 4c1b29eec..a69b0530f 100644
> >--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> >+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> >@@ -230,14 +230,10 @@ static int innolux_panel_add(struct innolux_panel *innolux)
> >  		innolux->enable_gpio = NULL;
> >  	}
> >-	np = of_parse_phandle(dev->of_node, "backlight", 0);
> >-	if (np) {
> >-		innolux->backlight = of_find_backlight_by_node(np);
> >-		of_node_put(np);
> >+	innolux->backlight = devm_of_find_backlight(np);
> 
> This driver isn't broken like tinydrm was, so it does put the backlight
> device on cleanup like it should. So when you're using the devm_ version,
> the result is a double put. Just remove the put_device() in
> innolux_panel_add/del() and you're good.

I have a quick question here. So devm_of_find_backlight automatically
does a put_device on detachment of the driver. However in this case,
put_device is called when panel_add is not successful (i.e when it
returns a negative value here). So is devm's release mechanism still
invoked here ?

	err = drm_panel_add(&innolux->base);
	if (err < 0)
		goto put_backlight;

	return 0;

	put_backlight:
		put_device(&innolux->backlight->dev);
	return err;

If so then I should change this to 
	err = drm_panel_add(&innolux->base)
	return err;

Thanks and regards,
Meghana

> I haven't checked the other drivers.
> 
> Noralf.
> 
> PS:
> Give people a few days (one week?) to respond before respinning a new
> version, so we avoid a fragmented discussion.
> 
> >-		if (!innolux->backlight)
> >-			return -EPROBE_DEFER;
> >-	}
> >+	if (IS_ERR(innolux->backlight))
> >+		return PTR_ERR(innolux->backlight);
> >  	drm_panel_init(&innolux->base);
> >  	innolux->base.funcs = &innolux_panel_funcs;
> >diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> >index 1512ec4f3..d5450c9d9 100644
> >--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> >+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> >@@ -329,14 +329,10 @@ static int sharp_panel_add(struct sharp_panel *sharp)
> >  	if (IS_ERR(sharp->supply))
> >  		return PTR_ERR(sharp->supply);
> >-	np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
> >-	if (np) {
> >-		sharp->backlight = of_find_backlight_by_node(np);
> >-		of_node_put(np);
> >+	sharp->backlight = devm_of_find_backlight(np);
> >-		if (!sharp->backlight)
> >-			return -EPROBE_DEFER;
> >-	}
> >+	if (IS_ERR(sharp->backlight))
> >+		return PTR_ERR(sharp->backlight);
> >  	drm_panel_init(&sharp->base);
> >  	sharp->base.funcs = &sharp_panel_funcs;
> >diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> >index a6af3257f..db31d8268 100644
> >--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> >+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> >@@ -273,14 +273,10 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
> >  		gpiod_set_value(sharp_nt->reset_gpio, 0);
> >  	}
> >-	np = of_parse_phandle(dev->of_node, "backlight", 0);
> >-	if (np) {
> >-		sharp_nt->backlight = of_find_backlight_by_node(np);
> >-		of_node_put(np);
> >+	sharp_nt->backlight = devm_of_find_backlight(np);
> >-		if (!sharp_nt->backlight)
> >-			return -EPROBE_DEFER;
> >-	}
> >+	if (IS_ERR(sharp_nt->backlight))
> >+		return PTR_ERR(sharp_nt->backlight);
> >  	drm_panel_init(&sharp_nt->base);
> >  	sharp_nt->base.funcs = &sharp_nt_panel_funcs;
> 

Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper

[Noralf Trønnes]

Den 18.01.2018 13.07, skrev Meghana Madhyastha:
> On Tue, Jan 16, 2018 at 06:08:53PM +0100, Noralf Trønnes wrote:
>> Den 16.01.2018 11.36, skrev Meghana Madhyastha:
>>> Replace of_find_backlight_by_node and of the code around it
>>> with of_find_backlight helper to avoid repetition of code.
>>>
>>> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
>>> ---
>>>   drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 10 +++-------
>>>   drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 +++-------
>>>   drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 +++-------
>>>   3 files changed, 9 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
>>> index 4c1b29eec..a69b0530f 100644
>>> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
>>> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
>>> @@ -230,14 +230,10 @@ static int innolux_panel_add(struct innolux_panel *innolux)
>>>   		innolux->enable_gpio = NULL;
>>>   	}
>>> -	np = of_parse_phandle(dev->of_node, "backlight", 0);
>>> -	if (np) {
>>> -		innolux->backlight = of_find_backlight_by_node(np);
>>> -		of_node_put(np);
>>> +	innolux->backlight = devm_of_find_backlight(np);
>> This driver isn't broken like tinydrm was, so it does put the backlight
>> device on cleanup like it should. So when you're using the devm_ version,
>> the result is a double put. Just remove the put_device() in
>> innolux_panel_add/del() and you're good.
> I have a quick question here. So devm_of_find_backlight automatically
> does a put_device on detachment of the driver. However in this case,
> put_device is called when panel_add is not successful (i.e when it
> returns a negative value here). So is devm's release mechanism still
> invoked here ?

As long as the error is propagated back up the chain, which it is in this
case, devres_release_all() is called and the resources are released.

Driver callchain passing up the error:
innolux_panel_driver.probe = innolux_panel_probe <- innolux_panel_add <- 
devm_of_find_backlight

This is mipi_dsi setting up the probe hook and calling into it's own 
version:

drivers/gpu/drm/drm_mipi_dsi.c:
int mipi_dsi_driver_register_full(struct mipi_dsi_driver *drv,
                   struct module *owner)
{
...
     if (drv->probe)
         drv->driver.probe = mipi_dsi_drv_probe;
...
}

static int mipi_dsi_drv_probe(struct device *dev)
{
     struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
     struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);

     return drv->probe(dsi);
}

This is the device-driver core that initiates the probing:

drivers/base/dd.c
static int really_probe(struct device *dev, struct device_driver *drv)
{
...
     } else if (drv->probe) {
         ret = drv->probe(dev);
         if (ret)
             goto probe_failed;
     }
...
probe_failed:
...
     devres_release_all(dev);
...
}

> 	err = drm_panel_add(&innolux->base);
> 	if (err < 0)
> 		goto put_backlight;
>
> 	return 0;
>
> 	put_backlight:
> 		put_device(&innolux->backlight->dev);
> 	return err;
>
> If so then I should change this to
> 	err = drm_panel_add(&innolux->base)
> 	return err;

Yes, and you can drop the variable:

         return drm_panel_add(&innolux->base);

Noralf.

>
> Thanks and regards,
> Meghana
>
>> I haven't checked the other drivers.
>>
>> Noralf.
>>
>> PS:
>> Give people a few days (one week?) to respond before respinning a new
>> version, so we avoid a fragmented discussion.
>>
>>> -		if (!innolux->backlight)
>>> -			return -EPROBE_DEFER;
>>> -	}
>>> +	if (IS_ERR(innolux->backlight))
>>> +		return PTR_ERR(innolux->backlight);
>>>   	drm_panel_init(&innolux->base);
>>>   	innolux->base.funcs = &innolux_panel_funcs;
>>> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
>>> index 1512ec4f3..d5450c9d9 100644
>>> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
>>> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
>>> @@ -329,14 +329,10 @@ static int sharp_panel_add(struct sharp_panel *sharp)
>>>   	if (IS_ERR(sharp->supply))
>>>   		return PTR_ERR(sharp->supply);
>>> -	np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
>>> -	if (np) {
>>> -		sharp->backlight = of_find_backlight_by_node(np);
>>> -		of_node_put(np);
>>> +	sharp->backlight = devm_of_find_backlight(np);
>>> -		if (!sharp->backlight)
>>> -			return -EPROBE_DEFER;
>>> -	}
>>> +	if (IS_ERR(sharp->backlight))
>>> +		return PTR_ERR(sharp->backlight);
>>>   	drm_panel_init(&sharp->base);
>>>   	sharp->base.funcs = &sharp_panel_funcs;
>>> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
>>> index a6af3257f..db31d8268 100644
>>> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
>>> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
>>> @@ -273,14 +273,10 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
>>>   		gpiod_set_value(sharp_nt->reset_gpio, 0);
>>>   	}
>>> -	np = of_parse_phandle(dev->of_node, "backlight", 0);
>>> -	if (np) {
>>> -		sharp_nt->backlight = of_find_backlight_by_node(np);
>>> -		of_node_put(np);
>>> +	sharp_nt->backlight = devm_of_find_backlight(np);
>>> -		if (!sharp_nt->backlight)
>>> -			return -EPROBE_DEFER;
>>> -	}
>>> +	if (IS_ERR(sharp_nt->backlight))
>>> +		return PTR_ERR(sharp_nt->backlight);
>>>   	drm_panel_init(&sharp_nt->base);
>>>   	sharp_nt->base.funcs = &sharp_nt_panel_funcs;