On Sat, Sep 30, 2017 at 09:04:57PM +0200, Noralf Trønnes wrote:
>
> Den 30.09.2017 19.12, skrev Meghana Madhyastha:
> >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 v6:
> >-Move function declarations to linux/backlight.h to resolve
> > module dependency issues.
> >
> > drivers/gpu/drm/drm_of.c | 44 ++++++++++++++++++++++++++
> > drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -----------------------
> > drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
> > include/drm/tinydrm/tinydrm-helpers.h | 1 -
> > include/linux/backlight.h | 11 +++++++
> > 5 files changed, 56 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..682b4db 100644
> >--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> >+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> >@@ -189,7 +189,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/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);
> >diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >index 5f2fd61..47a095e 100644
> >--- a/include/linux/backlight.h
> >+++ b/include/linux/backlight.h
> >@@ -172,4 +172,15 @@ of_find_backlight_by_node(struct device_node *node)
> > }
> > #endif
> >+#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> >+struct backlight_device *
> >+drm_of_find_backlight(struct device *dev);
> >+#else
> >+static inline struct backlight_device *
> >+drm_of_find_backlight(struct device *dev)
> >+{
> >+ return NULL;
> >+}
> >+#endif
> >+
> > #endif
>
> This isn't what I meant, you only change the of_find_backlight_by_node()
> declaration/definition. It should be like this:
>
> #if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> struct backlight_device *of_find_backlight_by_node(struct device_node
> *node);
> #else
> static inline struct backlight_device *
> of_find_backlight_by_node(struct device_node *node)
> {
> return NULL;
> }
> #endif
I see. So basically add the function declaration of drm_of_find_backlight
and devm_drm_of_find_backlight in drm_of.h and this function declaration
need not be inside the #ifdef CONFIG / #else part?
So there need not be a dummy version of drm_of_find_backlight here
because we are anyway taking care of that in of_find_backliht by node?
Sorry for the many versions and thank you for the clarifications.
Regards,
Meghana
> And this change needs to be a patch on it's own. The backlight subsystem
> has it's own maintainer that will review this change.
> It's best to send him/them the whole patchset so they see the context.
>
> Noralf.
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
