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
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 <[email protected]>
---
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
Remove tinydrm_enable/disable_backlight and let the callers call the
more generic backlight_enable/disable helpers
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Call devm_of_find_backlight (the devres version) instead of
of_find_backlight.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
checkpatch complains:
-:23: WARNING: Block comments should align the * on each line
-:45: ERROR: trailing whitespace
With that fixed:
Reviewed-by: Noralf Trønnes <[email protected]>
> ---
> 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);
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 <[email protected]>
This patch needs to be rebased on some recent changes to mipi-dbi.
Reviewed-by: Noralf Trønnes <[email protected]>
> ---
> 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);
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 <[email protected]>
> ---
> 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
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 <[email protected]>
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 <[email protected]>
> ---
> 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,
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 <[email protected]>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> ---
> 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;
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 <[email protected]>
> ---
> 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;
>
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
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 <[email protected]>
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);
>
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 <[email protected]>
Acked-by: Daniel Thompson <[email protected]>
> ---
> 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
>
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 <[email protected]>
> ---
> 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
>
On Wed, Jan 17, 2018 at 6:00 PM, Daniel Thompson
<[email protected]> 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 <[email protected]>
>
>
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
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 <[email protected]>
>
> 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);
>>
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 <[email protected]>
> >
> >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);
> >>
>
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 <[email protected]>
> >---
> > 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
> >
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 <[email protected]>
> > >
> > >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);
> > >>
> >
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 <[email protected]>
> > >---
> > > 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
> > >
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 <[email protected]>
> >---
> > 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;
>
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 <[email protected]>
>>> ---
>>> 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;