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

Noralf Trønnes noralf at tronnes.org
Fri Sep 29 15:41:04 UTC 2017


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.



More information about the dri-devel mailing list