[PATCH v3 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Daniel Vetter
daniel at ffwll.ch
Mon Oct 2 07:47:18 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...
You can't mix depends and select on the same symbol. More or less. So
either all depends or all selects, but select only works well if the
option is a top-level one (otherwise you also need to select _all_ its
dependencies).
And yes backlight is known to be an especially bad horror show in Kconfig,
unfortunately no one yet managed to clean up the mess :-/
So correct fix is to check out what all the other drivers do and then do
the same (and hope there's a clear majority).
Also adding Jani, who looked at the backlight Kconfig mess in the past.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list