2018-01-24 16:33:10

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 00/10] Add backlight helper functions

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

Changes in v19:
-Changed to devm version of of_find_backlight in omapdrm (patch 10)
-removed assigning pdev->dev to variable dev in omapdrm (patch 10)

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 | 36 ++--------
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 30 ++------
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 +-
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 38 ++--------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 37 ++--------
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, 165 insertions(+), 224 deletions(-)

--
2.11.0



2018-01-24 16:35:04

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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-24 16:36:49

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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-24 16:37:42

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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-24 16:37:50

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
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 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-24 16:39:20

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 06/10] drm/tinydrm: Call devres version of of_find_backlight

Call devm_of_find_backlight (the devres version) instead of
of_find_backlight.

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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-24 16:39:23

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
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-24 16:40:31

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
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 | 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-24 16:41:28

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 08/10] drm/omapdrm: 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.

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index e065f7e10..ac9596251 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -87,11 +87,7 @@ 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);
- }
+ backlight_enable(ddata->backlight);

dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;

@@ -106,10 +102,7 @@ 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);
- }
+ backlight_disable(ddata->backlight);

gpiod_set_value_cansleep(ddata->enable_gpio, 0);
regulator_disable(ddata->vcc_supply);
--
2.11.0


2018-01-24 16:41:37

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 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.

Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
Signed-off-by: Meghana Madhyastha <[email protected]>
---
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 24 ++++-----------------
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 28 +++++--------------------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 27 ++++--------------------
3 files changed, 13 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 4c1b29eec..57df39b5c 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -215,7 +215,6 @@ MODULE_DEVICE_TABLE(of, innolux_of_match);
static int innolux_panel_add(struct innolux_panel *innolux)
{
struct device *dev = &innolux->link->dev;
- struct device_node *np;
int err;

innolux->supply = devm_regulator_get(dev, "power");
@@ -230,37 +229,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(dev);

- 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..6bf8730f1 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -318,8 +318,7 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);

static int sharp_panel_add(struct sharp_panel *sharp)
{
- struct device_node *np;
- int err;
+ struct device *dev = &sharp->link1->dev;

sharp->mode = &default_mode;

@@ -327,30 +326,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(dev);

- 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 +343,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..494aa9b16 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -253,8 +253,6 @@ static const struct drm_panel_funcs sharp_nt_panel_funcs = {
static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
{
struct device *dev = &sharp_nt->dsi->dev;
- struct device_node *np;
- int ret;

sharp_nt->mode = &default_mode;

@@ -271,39 +269,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(dev);

- 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-24 16:42:20

by Meghana Madhyastha

[permalink] [raw]
Subject: [PATCH v19 10/10] drm/omapdrm: 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 v19:
-Changed to devm version of of_find_backlight in omapdrm (patch 10)
-removed assigning pdev->dev to variable dev in omapdrm (patch 10)

drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index ac9596251..b4a4006a2 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -157,7 +157,6 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
{
struct panel_drv_data *ddata = platform_get_drvdata(pdev);
struct device_node *node = pdev->dev.of_node;
- struct device_node *bl_node;
struct omap_dss_device *in;
int r;
struct display_timing timing;
@@ -183,19 +182,15 @@ 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 = devm_of_find_backlight(&pdev->dev);

- 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) {
dev_err(&pdev->dev, "failed to get video timing\n");
- goto error_free_backlight;
+ return r;
}

videomode_from_timing(&timing, &ddata->vm);
@@ -203,19 +198,12 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
in = omapdss_of_find_source_for_first_ep(node);
if (IS_ERR(in)) {
dev_err(&pdev->dev, "failed to find video source\n");
- r = PTR_ERR(in);
- goto error_free_backlight;
+ return PTR_ERR(in);
}

ddata->in = in;

return 0;
-
-error_free_backlight:
- if (ddata->backlight)
- put_device(&ddata->backlight->dev);
-
- return r;
}

static int panel_dpi_probe(struct platform_device *pdev)
@@ -270,9 +258,6 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)

omap_dss_put_device(in);

- if (ddata->backlight)
- put_device(&ddata->backlight->dev);
-
return 0;
}

--
2.11.0


2018-01-24 19:24:33

by Sean Paul

[permalink] [raw]
Subject: Re: [PATCH v19 10/10] drm/omapdrm: Use of_find_backlight helper

On Wed, Jan 24, 2018 at 04:41:38PM +0000, Meghana Madhyastha wrote:
> 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 v19:
> -Changed to devm version of of_find_backlight in omapdrm (patch 10)
> -removed assigning pdev->dev to variable dev in omapdrm (patch 10)
>
> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 25 +++++--------------------
> 1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index ac9596251..b4a4006a2 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -157,7 +157,6 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> {
> struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> struct device_node *node = pdev->dev.of_node;
> - struct device_node *bl_node;
> struct omap_dss_device *in;
> int r;
> struct display_timing timing;
> @@ -183,19 +182,15 @@ 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 = devm_of_find_backlight(&pdev->dev);
>
> - if (!ddata->backlight)
> - return -EPROBE_DEFER;
> - }
> + if (IS_ERR(ddata->backlight))

AFAICT, devm_of_find_backlight can return NULL. As such, you should be checking
IS_ERR_OR_NULL here instead of just IS_ERR. Looks like you also made this
same mistake in other patches in the series, so please fix those up as well.

Sean

> + return PTR_ERR(ddata->backlight);
>
> r = of_get_display_timing(node, "panel-timing", &timing);
> if (r) {
> dev_err(&pdev->dev, "failed to get video timing\n");
> - goto error_free_backlight;
> + return r;
> }
>
> videomode_from_timing(&timing, &ddata->vm);
> @@ -203,19 +198,12 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> in = omapdss_of_find_source_for_first_ep(node);
> if (IS_ERR(in)) {
> dev_err(&pdev->dev, "failed to find video source\n");
> - r = PTR_ERR(in);
> - goto error_free_backlight;
> + return PTR_ERR(in);
> }
>
> ddata->in = in;
>
> return 0;
> -
> -error_free_backlight:
> - if (ddata->backlight)
> - put_device(&ddata->backlight->dev);
> -
> - return r;
> }
>
> static int panel_dpi_probe(struct platform_device *pdev)
> @@ -270,9 +258,6 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)
>
> omap_dss_put_device(in);
>
> - if (ddata->backlight)
> - put_device(&ddata->backlight->dev);
> -
> return 0;
> }
>
> --
> 2.11.0
>

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

2018-01-24 19:27:26

by Sean Paul

[permalink] [raw]
Subject: Re: [PATCH v19 10/10] drm/omapdrm: Use of_find_backlight helper

On Wed, Jan 24, 2018 at 02:23:46PM -0500, Sean Paul wrote:
> On Wed, Jan 24, 2018 at 04:41:38PM +0000, Meghana Madhyastha wrote:
> > 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 v19:
> > -Changed to devm version of of_find_backlight in omapdrm (patch 10)
> > -removed assigning pdev->dev to variable dev in omapdrm (patch 10)
> >
> > drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 25 +++++--------------------
> > 1 file changed, 5 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> > index ac9596251..b4a4006a2 100644
> > --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> > @@ -157,7 +157,6 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> > {
> > struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> > struct device_node *node = pdev->dev.of_node;
> > - struct device_node *bl_node;
> > struct omap_dss_device *in;
> > int r;
> > struct display_timing timing;
> > @@ -183,19 +182,15 @@ 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 = devm_of_find_backlight(&pdev->dev);
> >
> > - if (!ddata->backlight)
> > - return -EPROBE_DEFER;
> > - }
> > + if (IS_ERR(ddata->backlight))
>
> AFAICT, devm_of_find_backlight can return NULL. As such, you should be checking
> IS_ERR_OR_NULL here instead of just IS_ERR. Looks like you also made this
> same mistake in other patches in the series, so please fix those up as well.
>

annnd I should spend more time reading the context of the code. Looks like
backlight is optional here, so this is a valid code path. I'm sorry about that.

So,

Reviewed-by: Sean Paul <[email protected]>

I think every other patch has R-b, so if no one objects in the next day or so,
I'll apply this to drm-misc-next

Sean


> Sean
>
> > + return PTR_ERR(ddata->backlight);
> >
> > r = of_get_display_timing(node, "panel-timing", &timing);
> > if (r) {
> > dev_err(&pdev->dev, "failed to get video timing\n");
> > - goto error_free_backlight;
> > + return r;
> > }
> >
> > videomode_from_timing(&timing, &ddata->vm);
> > @@ -203,19 +198,12 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> > in = omapdss_of_find_source_for_first_ep(node);
> > if (IS_ERR(in)) {
> > dev_err(&pdev->dev, "failed to find video source\n");
> > - r = PTR_ERR(in);
> > - goto error_free_backlight;
> > + return PTR_ERR(in);
> > }
> >
> > ddata->in = in;
> >
> > return 0;
> > -
> > -error_free_backlight:
> > - if (ddata->backlight)
> > - put_device(&ddata->backlight->dev);
> > -
> > - return r;
> > }
> >
> > static int panel_dpi_probe(struct platform_device *pdev)
> > @@ -270,9 +258,6 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)
> >
> > omap_dss_put_device(in);
> >
> > - if (ddata->backlight)
> > - put_device(&ddata->backlight->dev);
> > -
> > return 0;
> > }
> >
> > --
> > 2.11.0
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS

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

2018-01-24 20:14:13

by Noralf Trønnes

[permalink] [raw]
Subject: Re: [PATCH v19 10/10] drm/omapdrm: Use of_find_backlight helper


Den 24.01.2018 17.41, 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]>
> ---

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

> Changes in v19:
> -Changed to devm version of of_find_backlight in omapdrm (patch 10)
> -removed assigning pdev->dev to variable dev in omapdrm (patch 10)
>
> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 25 +++++--------------------
> 1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index ac9596251..b4a4006a2 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -157,7 +157,6 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> {
> struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> struct device_node *node = pdev->dev.of_node;
> - struct device_node *bl_node;
> struct omap_dss_device *in;
> int r;
> struct display_timing timing;
> @@ -183,19 +182,15 @@ 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 = devm_of_find_backlight(&pdev->dev);
>
> - 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) {
> dev_err(&pdev->dev, "failed to get video timing\n");
> - goto error_free_backlight;
> + return r;
> }
>
> videomode_from_timing(&timing, &ddata->vm);
> @@ -203,19 +198,12 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> in = omapdss_of_find_source_for_first_ep(node);
> if (IS_ERR(in)) {
> dev_err(&pdev->dev, "failed to find video source\n");
> - r = PTR_ERR(in);
> - goto error_free_backlight;
> + return PTR_ERR(in);
> }
>
> ddata->in = in;
>
> return 0;
> -
> -error_free_backlight:
> - if (ddata->backlight)
> - put_device(&ddata->backlight->dev);
> -
> - return r;
> }
>
> static int panel_dpi_probe(struct platform_device *pdev)
> @@ -270,9 +258,6 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)
>
> omap_dss_put_device(in);
>
> - if (ddata->backlight)
> - put_device(&ddata->backlight->dev);
> -
> return 0;
> }
>


2018-01-25 11:29:29

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v19 10/10] drm/omapdrm: Use of_find_backlight helper

On Wed, Jan 24, 2018 at 02:26:48PM -0500, Sean Paul wrote:
> So,
>
> Reviewed-by: Sean Paul <[email protected]>
>
> I think every other patch has R-b, so if no one objects in the next day or so,
> I'll apply this to drm-misc-next

No problems on my side... but please make sure Lee J. gets a PR.


Daniel.

2018-01-25 14:10:05

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v19 07/10] drm/panel: Use backlight_enable/disable helpers

On Wed, Jan 24, 2018 at 04:39:27PM +0000, Meghana Madhyastha wrote:
> 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.
>
> Reviewed-by: Noralf Trønnes <[email protected]>
> Reviewed-by: Sean Paul<[email protected]>
> 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 | 10 ++--------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 ++--------
> 4 files changed, 8 insertions(+), 24 deletions(-)

Acked-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (792.00 B)
signature.asc (849.00 B)
Download all attachments

2018-01-25 14:10:32

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v19 09/10] drm/panel: Use of_find_backlight helper

On Wed, Jan 24, 2018 at 04:40:51PM +0000, Meghana Madhyastha wrote:
> Replace of_find_backlight_by_node and of the code around it
> with of_find_backlight helper to avoid repetition of code.
>
> Reviewed-by: Noralf Trønnes <[email protected]>
> Reviewed-by: Sean Paul<[email protected]>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 24 ++++-----------------
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 28 +++++--------------------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 27 ++++--------------------
> 3 files changed, 13 insertions(+), 66 deletions(-)

Acked-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (723.00 B)
signature.asc (849.00 B)
Download all attachments

2018-01-25 14:38:19

by Thierry Reding

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

On Wed, Jan 24, 2018 at 04:35:30PM +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.
>
> Acked-by: Daniel Thompson <[email protected]>
> Reviewed-by: Noralf Trønnes <[email protected]>
> Reviewed-by: Sean Paul<[email protected]>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
> 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)

Just a nit: there isn't really anything OF specific about this. It would
be if this took a struct device_node * as parameter. So technically you
could implement this for ACPI or whatever other firmware interface as
well. So I think a more idiomatic name for this would be:

struct backlight_device *backlight_get(struct device *dev);

which would match the:

void backlight_put(struct backlight_device *bd);

and be consistent with how other resources are obtained. Not a big deal,
though. It can always be changed later on.

Reviewed-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (2.20 kB)
signature.asc (849.00 B)
Download all attachments

2018-01-25 14:39:27

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v19 00/10] Add backlight helper functions

On Wed, Jan 24, 2018 at 04:32:26PM +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 v19:
> -Changed to devm version of of_find_backlight in omapdrm (patch 10)
> -removed assigning pdev->dev to variable dev in omapdrm (patch 10)
>
> 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 | 36 ++--------
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 30 ++------
> drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 +-
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 38 ++--------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 37 ++--------
> 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, 165 insertions(+), 224 deletions(-)

Thanks for doing this, this has been a long-standing issue!

Thierry


Attachments:
(No filename) (1.96 kB)
signature.asc (849.00 B)
Download all attachments

2018-01-26 09:49:34

by Lee Jones

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

On Wed, 24 Jan 2018, 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.
>
> Acked-by: Daniel Thompson <[email protected]>
> Reviewed-by: Noralf Trønnes <[email protected]>
> Reviewed-by: Sean Paul<[email protected]>
> Signed-off-by: Meghana Madhyastha <[email protected]>

Nit: These should be in chronological order.

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

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

2018-01-26 17:46:48

by Randy Dunlap

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

On 01/26/2018 01:48 AM, Lee Jones wrote:
> On Wed, 24 Jan 2018, 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.
>>
>> Acked-by: Daniel Thompson <[email protected]>
>> Reviewed-by: Noralf Trønnes <[email protected]>
>> Reviewed-by: Sean Paul<[email protected]>
>> Signed-off-by: Meghana Madhyastha <[email protected]>
>
> Nit: These should be in chronological order.

Hi,
Where does that tidbit of information come from?
I have never heard or read that.

Thanks.

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


--
~Randy

2018-01-29 09:12:46

by Lee Jones

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

On Fri, 26 Jan 2018, Randy Dunlap wrote:

> On 01/26/2018 01:48 AM, Lee Jones wrote:
> > On Wed, 24 Jan 2018, 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.
> >>
> >> Acked-by: Daniel Thompson <[email protected]>
> >> Reviewed-by: Noralf Trønnes <[email protected]>
> >> Reviewed-by: Sean Paul<[email protected]>
> >> Signed-off-by: Meghana Madhyastha <[email protected]>
> >
> > Nit: These should be in chronological order.
>
> Where does that tidbit of information come from?
> I have never heard or read that.

Not sure it is documented anywhere. It appeared to be the widely
used, most sensible approach, so I adopted it a few years ago.

This method provides us with information which would otherwise be
absent; including description of the patch submission/acceptance path
and an idea of who did what, when.

For example:

Original Author sign-off
Original Co-author sign-off
[Additional contributions: rebase, API changes, fix-ups]
Re-worker's sign-off
Tester's tested-by
Reviewer's acked-by/reviewed-by
Level-2 Maintainer sign-off
Level-1 Maintainer sign-off

Are you aware of a more functional/practical/useful method?

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

2018-01-29 14:44:45

by Sean Paul

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

On Mon, Jan 29, 2018 at 4:11 AM, Lee Jones <[email protected]> wrote:
> On Fri, 26 Jan 2018, Randy Dunlap wrote:
>
>> On 01/26/2018 01:48 AM, Lee Jones wrote:
>> > On Wed, 24 Jan 2018, 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.
>> >>
>> >> Acked-by: Daniel Thompson <[email protected]>
>> >> Reviewed-by: Noralf Trønnes <[email protected]>
>> >> Reviewed-by: Sean Paul<[email protected]>
>> >> Signed-off-by: Meghana Madhyastha <[email protected]>
>> >
>> > Nit: These should be in chronological order.
>>
>> Where does that tidbit of information come from?
>> I have never heard or read that.
>
> Not sure it is documented anywhere. It appeared to be the widely
> used, most sensible approach, so I adopted it a few years ago.
>
> This method provides us with information which would otherwise be
> absent; including description of the patch submission/acceptance path
> and an idea of who did what, when.
>
> For example:
>
> Original Author sign-off
> Original Co-author sign-off
> [Additional contributions: rebase, API changes, fix-ups]
> Re-worker's sign-off
> Tester's tested-by
> Reviewer's acked-by/reviewed-by
> Level-2 Maintainer sign-off
> Level-1 Maintainer sign-off
>
> Are you aware of a more functional/practical/useful method?


Hi Lee,
Our tooling adds a Link: tag to the commit message pointing back to
mailing list archives. This provides more detailed patch provenance in
addition to reducing the burden on contributors to order tags
correctly (I had never heard of this as a requirement either). Check
out [1] if you're interested.

Sean

[1]- https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html

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

2018-01-30 02:31:39

by Randy Dunlap

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

On 01/29/2018 01:11 AM, Lee Jones wrote:
> On Fri, 26 Jan 2018, Randy Dunlap wrote:
>
>> On 01/26/2018 01:48 AM, Lee Jones wrote:
>>> On Wed, 24 Jan 2018, 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.
>>>>
>>>> Acked-by: Daniel Thompson <[email protected]>
>>>> Reviewed-by: Noralf Trønnes <[email protected]>
>>>> Reviewed-by: Sean Paul<[email protected]>
>>>> Signed-off-by: Meghana Madhyastha <[email protected]>
>>>
>>> Nit: These should be in chronological order.
>>
>> Where does that tidbit of information come from?
>> I have never heard or read that.
>
> Not sure it is documented anywhere. It appeared to be the widely
> used, most sensible approach, so I adopted it a few years ago.
>
> This method provides us with information which would otherwise be
> absent; including description of the patch submission/acceptance path
> and an idea of who did what, when.
>
> For example:
>
> Original Author sign-off
> Original Co-author sign-off
> [Additional contributions: rebase, API changes, fix-ups]
> Re-worker's sign-off
> Tester's tested-by
> Reviewer's acked-by/reviewed-by
> Level-2 Maintainer sign-off
> Level-1 Maintainer sign-off
>
> Are you aware of a more functional/practical/useful method?

No, I'm not.

Do you do anything else with this chronological order? E.g., do you verify
that someone actually did Ack or Test or Review a patch?

Sean: that dim tool looks interesting. I'll check it out.

thanks,
--
~Randy

2018-01-30 09:03:31

by Lee Jones

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

On Mon, 29 Jan 2018, Sean Paul wrote:
> On Mon, Jan 29, 2018 at 4:11 AM, Lee Jones <[email protected]> wrote:
> > On Fri, 26 Jan 2018, Randy Dunlap wrote:
> >
> >> On 01/26/2018 01:48 AM, Lee Jones wrote:
> >> > On Wed, 24 Jan 2018, 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.
> >> >>
> >> >> Acked-by: Daniel Thompson <[email protected]>
> >> >> Reviewed-by: Noralf Trønnes <[email protected]>
> >> >> Reviewed-by: Sean Paul<[email protected]>
> >> >> Signed-off-by: Meghana Madhyastha <[email protected]>
> >> >
> >> > Nit: These should be in chronological order.
> >>
> >> Where does that tidbit of information come from?
> >> I have never heard or read that.
> >
> > Not sure it is documented anywhere. It appeared to be the widely
> > used, most sensible approach, so I adopted it a few years ago.
> >
> > This method provides us with information which would otherwise be
> > absent; including description of the patch submission/acceptance path
> > and an idea of who did what, when.
> >
> > For example:
> >
> > Original Author sign-off
> > Original Co-author sign-off
> > [Additional contributions: rebase, API changes, fix-ups]
> > Re-worker's sign-off
> > Tester's tested-by
> > Reviewer's acked-by/reviewed-by
> > Level-2 Maintainer sign-off
> > Level-1 Maintainer sign-off
> >
> > Are you aware of a more functional/practical/useful method?
>
> Our tooling adds a Link: tag to the commit message pointing back to
> mailing list archives. This provides more detailed patch provenance in

I've been avoiding using tooling for Maintainership for a long time.
I really do not want to go via that route. The simpler the better as
far as I'm concerned (KISS [no, I'm not coming on to you!]).

Also, a ML link only provides you with information on the final
iteration of the submitted patch, and still does not provide
information on who did what, when.

> addition to reducing the burden on contributors to order tags

It's not really much of a burden. Just keep putting the tags at the
bottom when you collect them. Fortunately this is most developer's
default pattern, so I do not have to bring this up much.

> correctly (I had never heard of this as a requirement either). Check
> out [1] if you're interested.
>
> Sean
>
> [1]- https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html
>
> >

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

2018-02-20 16:18:16

by Sean Paul

[permalink] [raw]
Subject: Re: [PATCH v19 00/10] Add backlight helper functions

On Wed, Jan 24, 2018 at 11:32 AM, Meghana Madhyastha
<[email protected]> wrote:
> Move drm helper functions from tinydrm-helpers to linux/backlight for
> ease of use by callers in other drivers.
>

It was a long time coming, but this has finally been applied to
-misc-next. Thanks for sticking with it!

Sean

> Changes in v19:
> -Changed to devm version of of_find_backlight in omapdrm (patch 10)
> -removed assigning pdev->dev to variable dev in omapdrm (patch 10)
>
> 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 | 36 ++--------
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 30 ++------
> drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 +-
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 38 ++--------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 37 ++--------
> 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, 165 insertions(+), 224 deletions(-)
>
> --
> 2.11.0
>