[PATCH v3 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Meghana Madhyastha
meghana.madhyastha at gmail.com
Sat Sep 30 09:16:06 UTC 2017
On Fri, Sep 29, 2017 at 05:41:04PM +0200, Noralf Trønnes wrote:
>
> Den 29.09.2017 16.13, skrev Meghana Madhyastha:
> >On Fri, Sep 29, 2017 at 02:33:12PM +0200, Noralf Trønnes wrote:
> >>Den 29.09.2017 14.20, skrev Meghana Madhyastha:
> >>>On Fri, Sep 29, 2017 at 02:10:31PM +0200, Noralf Trønnes wrote:
> >>>>Den 29.09.2017 05.22, skrev Meghana Madhyastha:
> >>>>>On Thu, Sep 28, 2017 at 06:02:27PM +0200, Noralf Trønnes wrote:
> >>>>>>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 at gmail.com>
> >>>>>>>>---
> >>>>>>>>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);
>
> <snip>
>
> >>>>>>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
> >>>>>I didn't quite understand how we could use this fix for the current
> >>>>>case. Isn't this specific to the repaper driver dependency ?
> >>>>This is the solution Daniel mentions:
> >>>>
> >>>> menuconfig DRM
> >>>> tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
> >>>>support)"
> >>>> depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
> >>>>+ depends on (LCD_CLASS_DEVICE || LCD_CLASS_DEVICE=n)
> >>>I had tried this but it resulted in a recursive dependencies error.
> >>What's the error?
> >>Could this have something to do with tinydrm selecting backlight and
> >>depending on DRM?
> >>
> >>Noralf.
> >Looks like a case of circular dependencies. The exact error is this:
> >
> >scripts/kconfig/mconf Kconfig
> >drivers/gpu/drm/Kconfig:7:error: recursive dependency detected!
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/gpu/drm/Kconfig:7: symbol DRM depends on LCD_CLASS_DEVICE
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/video/backlight/Kconfig:16: symbol LCD_CLASS_DEVICE is
> >selected by FB_CLPS711X
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/video/fbdev/Kconfig:338: symbol FB_CLPS711X depends on FB
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/video/fbdev/Kconfig:5: symbol FB is selected by
> >DRM_KMS_FB_HELPER
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/gpu/drm/Kconfig:73: symbol DRM_KMS_FB_HELPER is selected by
> >DRM_FBDEV_EMULATION
> >For a resolution refer to Documentation/kbuild/kconfig-language.txt
> >subsection "Kconfig recursive dependency limitations"
> >drivers/gpu/drm/Kconfig:90: symbol DRM_FBDEV_EMULATION depends on
> >DRM
> >
> >Long story short, the dependency loop is as follows:
> >
> >DRM -> LCD_CLASS_DEVICE -> FB_CLPS711X -> FB -> DRM_KMS_FB_HELPER ->
> >DRM_FBDEV_EMULATION -> DRM
>
> I have messed up the options here, it should be BACKLIGHT_CLASS_DEVICE:
>
> menuconfig DRM
> depends on (BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n)
>
> It doesn't help with the recusrsive problem, but at least it makes sense:
>
> drivers/gpu/drm/Kconfig:7: symbol DRM depends on BACKLIGHT_CLASS_DEVICE
> drivers/video/backlight/Kconfig:158: symbol BACKLIGHT_CLASS_DEVICE is
> selected by DRM_RADEON
> drivers/gpu/drm/Kconfig:164: symbol DRM_RADEON depends on DRM
>
> I don't know how to solve this...
>
> Noralf.
Looks like select BACKLIGHT_CLASS_DEVICE and BACKLIGHT_LCD_SUPPORT
seemed to work (atleast when I replicated the configurations from the
kbuild support). Maybe that is because we are not forcing the
dependencies when we do a select ?
Thanks and regards,
Meghana
More information about the dri-devel
mailing list