Re: [PATCH v3 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Den 28.09.2017 16.08, skrev Daniel Vetter:
On Thu, Sep 28, 2017 at 02:44:34PM +0530, Meghana Madhyastha wrote:
Rename tinydrm_of_find_backlight to drm_of_find_backlight
and move it into drm_of.c from tinydrm-helpers.c. This is
because other drivers in the drm subsystem might need to call
this function. In that case and otherwise, it is better from
an organizational point of view to move it into drm_of.c along
with the other _of.c functions.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@xxxxxxxxx>
---
Changes in v3:
-Change it back to a single patch from two patches in v2

  drivers/gpu/drm/drm_of.c                       | 44 ++++++++++++++++++++++++++
  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -----------------------
  drivers/gpu/drm/tinydrm/mi0283qt.c             |  3 +-
  include/drm/drm_of.h                           |  1 +
  include/drm/tinydrm/tinydrm-helpers.h          |  1 -
  5 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 8dafbdf..d878d3a 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -1,6 +1,7 @@
  #include <linux/component.h>
  #include <linux/export.h>
  #include <linux/list.h>
+#include <linux/backlight.h>
  #include <linux/of_graph.h>
  #include <drm/drmP.h>
  #include <drm/drm_bridge.h>
@@ -260,3 +261,46 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
  	return ret;
  }
  EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
+
+/**
+ * drm_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.
+ *
+ * Note: It is the responsibility of the caller to call put_device() when
+ * releasing the resource.
+ *
+ * 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 *drm_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(drm_of_find_backlight);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bd6cce0..cd4c6a5 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -237,46 +237,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);
-
-/**
   * tinydrm_enable_backlight - Enable backlight helper
   * @backlight: Backlight device
   *
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7e5bb7d..5e3d635 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -12,6 +12,7 @@
  #include <drm/tinydrm/ili9341.h>
  #include <drm/tinydrm/mipi-dbi.h>
  #include <drm/tinydrm/tinydrm-helpers.h>
+#include <drm/drm_of.h>
  #include <linux/delay.h>
  #include <linux/gpio/consumer.h>
  #include <linux/module.h>
@@ -189,7 +190,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 = drm_of_find_backlight(dev);
  	if (IS_ERR(mipi->backlight))
  		return PTR_ERR(mipi->backlight);
diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
index 104dd51..e8fba5b 100644
--- a/include/drm/drm_of.h
+++ b/include/drm/drm_of.h
@@ -29,6 +29,7 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
  				int port, int endpoint,
  				struct drm_panel **panel,
  				struct drm_bridge **bridge);
+struct backlight_device *drm_of_find_backlight(struct device *dev);
  #else
  static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
  						  struct device_node *port)
Minor detail I missed: The dummy version for the #else here is missing.
Not a problem for tinydrm, but might be an issue for a driver where
CONFIG_OF is optional.

Yeah, we need a dummy version. tinydrm can in theory be used on non-DT
platforms. The only resource that isn't available there is backlight
since there is no platform or ACPI way of describing that dependency.

Another problem I discovered when trying to compile this, is that if
DRM is builtin and LCD_CLASS_DEVICE is a module:

drivers/gpu/drm/drm_of.c:292: undefined reference to `of_find_backlight_by_node'

I see that I solved this in tinydrm by just selecting
BACKLIGHT_LCD_SUPPORT and LCD_CLASS_DEVICE. I'm not aware of a way to
force LCD_CLASS_DEVICE to be builtin if DRM is builtin.

Arnd fixed a similar type of dependecy in the repaper driver by forcing
repaper to be a module if necessary. To be honest, I don't know why his
fix works:
https://github.com/torvalds/linux/commit/8312a3fe84b96e30a010fa20f933606d976ac002

Another way of solving this is to put the function in
drivers/gpu/drm/drm_backlight.c and add a DRM_BACKLIGHT konfig option
that does the selection.

Noralf.

Otherwise I think this patch looks good.
-Daniel

diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index d554ded..e40ef2d 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -46,7 +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);
  int tinydrm_enable_backlight(struct backlight_device *backlight);
  int tinydrm_disable_backlight(struct backlight_device *backlight);
--
2.7.4


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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux