LinuxLists.cc - [PATCH v17 00/10] Add backlight helper functions

[permalink] [raw]

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

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]>
---
changes in v17:
-set fb_blank along with clearing the BL_CORE_FBBLANK bit

include/linux/backlight.h | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index af7003548..ace825e2c 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -130,6 +130,38 @@ 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.fb_blank = 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.fb_blank = 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

2018-01-19 10:41:37

[permalink] [raw]

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

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

Signed-off-by: Meghana Madhyastha <[email protected]>
---
changes in v17:
-rebase with drm-misc-next

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 75dd65c57..9e903812b 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -286,7 +286,7 @@ void mipi_dbi_enable_flush(struct mipi_dbi *mipi)
if (fb)
fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);

- tinydrm_enable_backlight(mipi->backlight);
+ backlight_enable(mipi->backlight);
}
EXPORT_SYMBOL(mipi_dbi_enable_flush);

@@ -325,7 +325,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

2018-01-19 10:43:12

[permalink] [raw]

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

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 v17:
-rebase with drm-misc-next
-convert st7735r callers from tinydrm specific helpers
to new generic backlight helpers
-remove select BACKLIGHT_LCD_SUPPORT
and select BACKLIGHT_CLASS_DEVICE from
tinydrm/Kconfig

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

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 8049e7656..553bf5c48 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -580,6 +580,49 @@ 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.
+ *
+ * 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);
+ /*
+ * Note: gpio_backlight uses brightness as
+ * power state during probe
+ */
+ 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 ace825e2c..ddc9bade4 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -162,6 +162,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);
@@ -205,4 +215,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

2018-01-19 10:44:43

[permalink] [raw]

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

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. Also, remove
select BACKLIGHT_LCD_SUPPORT and select BACKLIGHT_CLASS_DEVICE from
tinydrm/Kconfig as it is a hack that is no longer needed.

Signed-off-by: Meghana Madhyastha <[email protected]>
---
changes in v17:
-rebase with drm-misc-next
-convert st7735r callers from tinydrm specific helpers
to new generic backlight helpers
-remove select BACKLIGHT_LCD_SUPPORT
and select BACKLIGHT_CLASS_DEVICE from
tinydrm/Kconfig

drivers/gpu/drm/tinydrm/Kconfig | 2 --
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --------------------------
drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +-
drivers/gpu/drm/tinydrm/st7735r.c | 3 +-
include/drm/tinydrm/tinydrm-helpers.h | 2 --
5 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index b0e567d41..13339be84 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -3,8 +3,6 @@ menuconfig DRM_TINYDRM
depends on DRM
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
- select BACKLIGHT_LCD_SUPPORT
- select BACKLIGHT_CLASS_DEVICE
help
Choose this option if you have a tinydrm supported display.
If M is selected the module will be called tinydrm.
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 79cb5af5a..a8aafce36 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -9,6 +9,7 @@
* (at your option) any later version.
*/

+#include <linux/backlight.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
#include <linux/module.h>
@@ -195,7 +196,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/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 08b4fb18e..2e6b7b8ec 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -5,6 +5,7 @@
* Copyright 2017 David Lechner <[email protected]>
*/

+#include <linux/backlight.h>
#include <linux/delay.h>
#include <linux/dma-buf.h>
#include <linux/gpio/consumer.h>
@@ -163,7 +164,7 @@ static int st7735r_probe(struct spi_device *spi)
return PTR_ERR(dc);
}

- 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

2018-01-19 10:45:30

[permalink] [raw]

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

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]>
---
changes in v17:
-fix checkpath errors/warnings
-rename devm_backlight_put to
devm_backlight_release

drivers/video/backlight/backlight.c | 30 ++++++++++++++++++++++++++++++
include/linux/backlight.h | 7 +++++++
2 files changed, 37 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 553bf5c48..deb824bef 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -623,6 +623,36 @@ struct backlight_device *of_find_backlight(struct device *dev)
}
EXPORT_SYMBOL(of_find_backlight);

+static void devm_backlight_release(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_release, 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 ddc9bade4..2baab6f38 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -217,11 +217,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

2018-01-19 10:47:31

[permalink] [raw]

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

Call devm_of_find_backlight (the devres version) instead of
of_find_backlight.

Signed-off-by: Meghana Madhyastha <[email protected]>
---
changes in v17:
-convert st7735r callers from tinydrm specific helpers
to new generic backlight helpers

drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
drivers/gpu/drm/tinydrm/st7735r.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index a8aafce36..d8ed6e6f8 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -196,7 +196,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);

diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 2e6b7b8ec..67d197ecf 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -164,7 +164,7 @@ static int st7735r_probe(struct spi_device *spi)
return PTR_ERR(dc);
}

- 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

2018-01-19 10:47:40

[permalink] [raw]

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

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]>
---
changes in v17:
-remove redundant NULL check

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 | 10 ++--------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 ++--------
4 files changed, 8 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..072c0fc79 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -96,10 +96,7 @@ 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);
- }
+ backlight_disable(sharp->backlight);

sharp->enabled = false;

@@ -263,10 +260,7 @@ 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);
- }
+ 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..8a5137963 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -117,10 +117,7 @@ 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);
- }
+ backlight_disable(sharp_nt->backlight);

sharp_nt->enabled = false;

@@ -203,10 +200,7 @@ 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);
- }
+ backlight_enable(sharp_nt->backlight);

sharp_nt->enabled = true;

--
2.11.0

2018-01-19 10:48:23

[permalink] [raw]

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

2018-01-19 10:49:24

[permalink] [raw]

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

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]>
---
changes in v17:
-remove put_device() to avoid double put
as we are using the devm version

drivers/gpu/drm/panel/panel-innolux-p079zca.c | 23 ++++-------------------
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 25 ++++---------------------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 25 ++++---------------------
3 files changed, 12 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 4c1b29eec..059f4af1a 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -230,37 +230,22 @@ 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;
innolux->base.dev = &innolux->link->dev;

- err = drm_panel_add(&innolux->base);
- if (err < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- put_device(&innolux->backlight->dev);
-
- return err;
+ return drm_panel_add(&innolux->base);
}

static void innolux_panel_del(struct innolux_panel *innolux)
{
if (innolux->base.dev)
drm_panel_remove(&innolux->base);
-
- put_device(&innolux->backlight->dev);
}

static int innolux_panel_probe(struct mipi_dsi_device *dsi)
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 072c0fc79..85279d224 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -327,30 +327,16 @@ 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;
sharp->base.dev = &sharp->link1->dev;

- err = drm_panel_add(&sharp->base);
- if (err < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- if (sharp->backlight)
- put_device(&sharp->backlight->dev);
-
- return err;
+ return drm_panel_add(&sharp->base);
}

static void sharp_panel_del(struct sharp_panel *sharp)
@@ -358,9 +344,6 @@ static void sharp_panel_del(struct sharp_panel *sharp)
if (sharp->base.dev)
drm_panel_remove(&sharp->base);

- if (sharp->backlight)
- put_device(&sharp->backlight->dev);
-
if (sharp->link2)
put_device(&sharp->link2->dev);
}
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index 8a5137963..b634ec887 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -271,39 +271,22 @@ 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;
sharp_nt->base.dev = &sharp_nt->dsi->dev;

- ret = drm_panel_add(&sharp_nt->base);
- if (ret < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- if (sharp_nt->backlight)
- put_device(&sharp_nt->backlight->dev);
-
- return ret;
+ return drm_panel_add(&sharp_nt->base);
}

static void sharp_nt_panel_del(struct sharp_nt_panel *sharp_nt)
{
if (sharp_nt->base.dev)
drm_panel_remove(&sharp_nt->base);
-
- if (sharp_nt->backlight)
- put_device(&sharp_nt->backlight->dev);
}

static int sharp_nt_panel_probe(struct mipi_dsi_device *dsi)
--
2.11.0

2018-01-19 10:49:37

[permalink] [raw]

Subject: [PATCH v17 10/10] drm/omapdrm: Use of_find_backlight helper

2018-01-19 11:05:11

[permalink] [raw]

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

On Fri, Jan 19, 2018 at 10:39:16AM +0000, 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]>

Acked-by: Daniel Thompson <[email protected]>

BTW Lee J. looks PRs and merges for the backlight tree, I'll leave it
to Lee and whoevers-DRM-tree-this-would-naturally-land-in to decide the
merge path.

Daniel.

> ---
> changes in v17:
> -set fb_blank along with clearing the BL_CORE_FBBLANK bit
>
> include/linux/backlight.h | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index af7003548..ace825e2c 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -130,6 +130,38 @@ 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.fb_blank = 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.fb_blank = 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
>

2018-01-19 11:07:20

[permalink] [raw]

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

On Fri, Jan 19, 2018 at 10:42:15AM +0000, 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]>

Didn't I already ack this one?

> ---
> changes in v17:
> -rebase with drm-misc-next
> -convert st7735r callers from tinydrm specific helpers
> to new generic backlight helpers
> -remove select BACKLIGHT_LCD_SUPPORT
> and select BACKLIGHT_CLASS_DEVICE from
> tinydrm/Kconfig
>
> drivers/video/backlight/backlight.c | 43 +++++++++++++++++++++++++++++++++++++
> include/linux/backlight.h | 19 ++++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 8049e7656..553bf5c48 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -580,6 +580,49 @@ 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.
> + *
> + * 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);
> + /*
> + * Note: gpio_backlight uses brightness as
> + * power state during probe
> + */
> + 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 ace825e2c..ddc9bade4 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -162,6 +162,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);
> @@ -205,4 +215,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
>

2018-01-19 11:08:42

[permalink] [raw]

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

On Fri, Jan 19, 2018 at 10:44:33AM +0000, 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]>

Acked-by: Daniel Thompson <[email protected]>

> ---
> changes in v17:
> -fix checkpath errors/warnings
> -rename devm_backlight_put to
> devm_backlight_release
>
> drivers/video/backlight/backlight.c | 30 ++++++++++++++++++++++++++++++
> include/linux/backlight.h | 7 +++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 553bf5c48..deb824bef 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -623,6 +623,36 @@ struct backlight_device *of_find_backlight(struct device *dev)
> }
> EXPORT_SYMBOL(of_find_backlight);
>
> +static void devm_backlight_release(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_release, 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 ddc9bade4..2baab6f38 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -217,11 +217,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
>

2018-01-19 11:20:29

[permalink] [raw]

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

On Fri, Jan 19, 2018 at 10:37:58AM +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 v17:
> -set fb_blank along with clearing the BL_CORE_FBBLANK bit
> -rebase with drm-misc-next
> -fix checkpath errors/warnings
> -Make the gpio_backlight issue appear as a comment in the
> code and not as part of the docs
> -rebase with drm-misc-next
> -convert st7735r callers from tinydrm specific helpers
> to new generic backlight helpers
> -remove select BACKLIGHT_LCD_SUPPORT and select
> BACKLIGHT_CLASS_DEVICE from tinydrm/Kconfig
> -fix checkpath errors/warnings
> -rename devm_backlight_put to
> devm_backlight_release
> -remove redundant NULL check
> -remove put_device() to avoid double put
> as we are using the devm version
>
> 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

So, I've acked all the backlight bits (if there's a v18 please include
the acks in the posting).

I've read the other patches and I *like* them. Switching to
backlight_enable()/disable() looks like a good way to ensure
drivers don't interact with all those flags in unexpected ways
(so its a good contribution to eventually cleaning them up).

Daniel.

>
> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 32 ++-------
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 29 ++------
> drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 +-
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 35 ++-------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 35 ++-------
> drivers/gpu/drm/tinydrm/Kconfig | 2 -
> 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/gpu/drm/tinydrm/st7735r.c | 3 +-
> drivers/video/backlight/backlight.c | 73 +++++++++++++++++++
> include/drm/tinydrm/tinydrm-helpers.h | 4 --
> include/linux/backlight.h | 58 +++++++++++++++
> 13 files changed, 164 insertions(+), 215 deletions(-)
>
> --
> 2.11.0
>

2018-01-19 14:27:00

by Lee Jones

[permalink] [raw]

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

On Fri, 19 Jan 2018, Daniel Thompson wrote:

> On Fri, Jan 19, 2018 at 10:39:16AM +0000, 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]>
>
> Acked-by: Daniel Thompson <[email protected]>
>
> BTW Lee J. looks PRs and merges for the backlight tree, I'll leave it
> to Lee and whoevers-DRM-tree-this-would-naturally-land-in to decide the
> merge path.

So the choices are; either I take the set and send out a pull-request
which can be subsequently merged into the DRM repo, OR they are
applied into an immutable branch in the DRM tree and I can pull that
in instead.

Don't really mind which.

--
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-01-21 18:36:10

[permalink] [raw]

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

Den 19.01.2018 12.05, skrev Daniel Thompson:
> On Fri, Jan 19, 2018 at 10:42:15AM +0000, 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]>
> Didn't I already ack this one?

Meghana,

You are supposed to pick up the r-b/acks you get and add them when you send
a new version. This way the reviewer knows which patches still need
attention.

Here's a good overview of the acks/r-b's you've received:
https://patchwork.freedesktop.org/project/dri-devel/patches/?submitter=17149

Beware that r-b/ack to the cover letter doesn't show up in patchwork, like
Sean did on the last version putting his r-b on the whole series.
The same goes for someone putting an r-b in one of the patches stating that
it applies to whole series, patchwork thinks it only applies to this one
patch.

Noralf.

>
>> ---
>> changes in v17:
>> -rebase with drm-misc-next
>> -convert st7735r callers from tinydrm specific helpers
>> to new generic backlight helpers
>> -remove select BACKLIGHT_LCD_SUPPORT
>> and select BACKLIGHT_CLASS_DEVICE from
>> tinydrm/Kconfig
>>
>> drivers/video/backlight/backlight.c | 43 +++++++++++++++++++++++++++++++++++++
>> include/linux/backlight.h | 19 ++++++++++++++++
>> 2 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>> index 8049e7656..553bf5c48 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -580,6 +580,49 @@ 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.
>> + *
>> + * 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);
>> + /*
>> + * Note: gpio_backlight uses brightness as
>> + * power state during probe
>> + */
>> + 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 ace825e2c..ddc9bade4 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -162,6 +162,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);
>> @@ -205,4 +215,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
>>

2018-01-21 18:39:49

[permalink] [raw]

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

Den 19.01.2018 11.47, 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]>
> ---
> changes in v17:
> -remove put_device() to avoid double put
> as we are using the devm version
>
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 23 ++++-------------------
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 25 ++++---------------------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 25 ++++---------------------
> 3 files changed, 12 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 4c1b29eec..059f4af1a 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -230,37 +230,22 @@ 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);

There are several errors in these two last patches, like this one:

/home/pi/dim-build/workdirs/drm-misc/linux-linus/drivers/gpu/drm/panel/panel-innolux-p079zca.c:
In function 'innolux_panel_add':
/home/pi/dim-build/workdirs/drm-misc/linux-linus/drivers/gpu/drm/panel/panel-innolux-p079zca.c:233:46:
warning: passing argument 1 of 'devm_of_find_backlight' from
incompatible pointer type
innolux->backlight = devm_of_find_backlight(np);

Instead of tracking down the build dependencies to get this driver built,
you can instead use these defconfigs and all DRM drivers should be built:
https://cgit.freedesktop.org/drm/drm-tip/tree/?h=rerere-cache

But not all drivers are built on every architecture.

Noralf.

>
> - 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;
> innolux->base.dev = &innolux->link->dev;
>
> - err = drm_panel_add(&innolux->base);
> - if (err < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - put_device(&innolux->backlight->dev);
> -
> - return err;
> + return drm_panel_add(&innolux->base);
> }
>
> static void innolux_panel_del(struct innolux_panel *innolux)
> {
> if (innolux->base.dev)
> drm_panel_remove(&innolux->base);
> -
> - put_device(&innolux->backlight->dev);
> }
>
> static int innolux_panel_probe(struct mipi_dsi_device *dsi)
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 072c0fc79..85279d224 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -327,30 +327,16 @@ 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;
> sharp->base.dev = &sharp->link1->dev;
>
> - err = drm_panel_add(&sharp->base);
> - if (err < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - if (sharp->backlight)
> - put_device(&sharp->backlight->dev);
> -
> - return err;
> + return drm_panel_add(&sharp->base);
> }
>
> static void sharp_panel_del(struct sharp_panel *sharp)
> @@ -358,9 +344,6 @@ static void sharp_panel_del(struct sharp_panel *sharp)
> if (sharp->base.dev)
> drm_panel_remove(&sharp->base);
>
> - if (sharp->backlight)
> - put_device(&sharp->backlight->dev);
> -
> if (sharp->link2)
> put_device(&sharp->link2->dev);
> }
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> index 8a5137963..b634ec887 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> @@ -271,39 +271,22 @@ 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;
> sharp_nt->base.dev = &sharp_nt->dsi->dev;
>
> - ret = drm_panel_add(&sharp_nt->base);
> - if (ret < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - if (sharp_nt->backlight)
> - put_device(&sharp_nt->backlight->dev);
> -
> - return ret;
> + return drm_panel_add(&sharp_nt->base);
> }
>
> static void sharp_nt_panel_del(struct sharp_nt_panel *sharp_nt)
> {
> if (sharp_nt->base.dev)
> drm_panel_remove(&sharp_nt->base);
> -
> - if (sharp_nt->backlight)
> - put_device(&sharp_nt->backlight->dev);
> }
>
> static int sharp_nt_panel_probe(struct mipi_dsi_device *dsi)

2018-01-21 18:43:40

[permalink] [raw]

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

Den 19.01.2018 11.46, skrev Meghana Madhyastha:
> Call devm_of_find_backlight (the devres version) instead of
> of_find_backlight.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---

I already put my r-b on this one in the previous version, but now also:

Tested-by: Noralf Trønnes <[email protected]>

I did also test with the backlight subsystem disabled.

> changes in v17:
> -convert st7735r callers from tinydrm specific helpers
> to new generic backlight helpers
>
> drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
> drivers/gpu/drm/tinydrm/st7735r.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index a8aafce36..d8ed6e6f8 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -196,7 +196,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);
>
> diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
> index 2e6b7b8ec..67d197ecf 100644
> --- a/drivers/gpu/drm/tinydrm/st7735r.c
> +++ b/drivers/gpu/drm/tinydrm/st7735r.c
> @@ -164,7 +164,7 @@ static int st7735r_probe(struct spi_device *spi)
> return PTR_ERR(dc);
> }
>
> - mipi->backlight = of_find_backlight(dev);
> + mipi->backlight = devm_of_find_backlight(dev);
> if (IS_ERR(mipi->backlight))
> return PTR_ERR(mipi->backlight);
>

2018-01-21 18:45:17