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@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); > > <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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel