[PATCH v2] drm/tinydrm: Move backlight helpers to a separate file

Noralf Trønnes noralf at tronnes.org
Mon Sep 25 16:31:58 UTC 2017


Den 25.09.2017 16.56, skrev Noralf Trønnes:
> Hi Meghana,
>
>
> Den 22.09.2017 17.09, skrev Meghana Madhyastha:
>> Move backlight helpers from tinydrm-helpers.c to
>> tinydrm-backlight.c. This is because it is organizationally
>> simpler to understand and advantageous to group functions
>> performing a similar function to a separate file as opposed to
>> having one helper file with heteregenous helper functions.
>>
>> Signed-off-by: Meghana Madhyastha <meghana.madhyastha at gmail.com>
>> ---
>
> I don't think there is much gain in just moving the code like this.
>
> The idea is to add a drm_backlight helper that can be useful for all
> DRM drivers that use the backlight subsystem.
>

The full path to that helper would be:
drivers/gpu/drm/drm_backlight.c

> This is what the TODO says:
> https://dri.freedesktop.org/docs/drm/gpu/todo.html#tinydrm
>
> - backlight helpers, probably best to put them into a new 
> drm_backlight.c.
>   This is because drivers/video is de-facto unmaintained. We could also
>   move drivers/video/backlight to drivers/gpu/backlight and take it all
>   over within drm-misc, but that’s more work.
>
> There is also this discussion to take into account:
> KMS backlight ABI proposition
> https://lists.freedesktop.org/archives/dri-devel/2017-February/133206.html 
>
>
> I don't remember what came out of that discussion.
>
> Noralf.
>
>> Changes in v2:
>>   -Improved commit message by explaining why the changes were made.
>>
>>   drivers/gpu/drm/tinydrm/core/Makefile            |   2 +-
>>   drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c | 103 
>> +++++++++++++++++++++++
>>   drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c   |  94 
>> ---------------------
>>   drivers/gpu/drm/tinydrm/mi0283qt.c               |   1 +
>>   drivers/gpu/drm/tinydrm/mipi-dbi.c               |   1 +
>>   include/drm/tinydrm/tinydrm-backlight.h          |  18 ++++
>>   6 files changed, 124 insertions(+), 95 deletions(-)
>>   create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
>>   create mode 100644 include/drm/tinydrm/tinydrm-backlight.h
>>
>> diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
>> b/drivers/gpu/drm/tinydrm/core/Makefile
>> index fb221e6..389ca7a 100644
>> --- a/drivers/gpu/drm/tinydrm/core/Makefile
>> +++ b/drivers/gpu/drm/tinydrm/core/Makefile
>> @@ -1,3 +1,3 @@
>> -tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
>> +tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-backlight.o 
>> tinydrm-helpers.o
>>     obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
>> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c 
>> b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
>> new file mode 100644
>> index 0000000..dc6f17d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
>> @@ -0,0 +1,103 @@
>> +#include <linux/backlight.h>
>> +#include <linux/dma-buf.h>
>> +#include <linux/pm.h>
>> +#include <linux/swab.h>
>> +
>> +#include <drm/tinydrm/tinydrm.h>
>> +#include <drm/tinydrm/tinydrm-backlight.h>
>> +
>> +/**
>> + * 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
>> + *
>> + * Returns:
>> + * Zero on success, negative error code on failure.
>> + */
>> +int tinydrm_enable_backlight(struct backlight_device *backlight)
>> +{
>> +    unsigned int old_state;
>> +    int ret;
>> +
>> +    if (!backlight)
>> +        return 0;
>> +
>> +    old_state = backlight->props.state;
>> +    backlight->props.state &= ~BL_CORE_FBBLANK;
>> +    DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
>> +              backlight->props.state);
>> +
>> +    ret = backlight_update_status(backlight);
>> +    if (ret)
>> +        DRM_ERROR("Failed to enable backlight %d\n", ret);
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(tinydrm_enable_backlight);
>> +
>> +/**
>> + * tinydrm_disable_backlight - Disable backlight helper
>> + * @backlight: Backlight device
>> + *
>> + * Returns:
>> + * Zero on success, negative error code on failure.
>> + */
>> +int tinydrm_disable_backlight(struct backlight_device *backlight)
>> +{
>> +    unsigned int old_state;
>> +    int ret;
>> +
>> +    if (!backlight)
>> +        return 0;
>> +
>> +    old_state = backlight->props.state;
>> +    backlight->props.state |= BL_CORE_FBBLANK;
>> +    DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
>> +              backlight->props.state);
>> +    ret = backlight_update_status(backlight);
>> +    if (ret)
>> +        DRM_ERROR("Failed to disable backlight %d\n", ret);
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(tinydrm_disable_backlight);
>> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
>> b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> index bd6cce0..ee8ad8c 100644
>> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>> @@ -236,100 +236,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
>> - *
>> - * Returns:
>> - * Zero on success, negative error code on failure.
>> - */
>> -int tinydrm_enable_backlight(struct backlight_device *backlight)
>> -{
>> -    unsigned int old_state;
>> -    int ret;
>> -
>> -    if (!backlight)
>> -        return 0;
>> -
>> -    old_state = backlight->props.state;
>> -    backlight->props.state &= ~BL_CORE_FBBLANK;
>> -    DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
>> -              backlight->props.state);
>> -
>> -    ret = backlight_update_status(backlight);
>> -    if (ret)
>> -        DRM_ERROR("Failed to enable backlight %d\n", ret);
>> -
>> -    return ret;
>> -}
>> -EXPORT_SYMBOL(tinydrm_enable_backlight);
>> -
>> -/**
>> - * tinydrm_disable_backlight - Disable backlight helper
>> - * @backlight: Backlight device
>> - *
>> - * Returns:
>> - * Zero on success, negative error code on failure.
>> - */
>> -int tinydrm_disable_backlight(struct backlight_device *backlight)
>> -{
>> -    unsigned int old_state;
>> -    int ret;
>> -
>> -    if (!backlight)
>> -        return 0;
>> -
>> -    old_state = backlight->props.state;
>> -    backlight->props.state |= BL_CORE_FBBLANK;
>> -    DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
>> -              backlight->props.state);
>> -    ret = backlight_update_status(backlight);
>> -    if (ret)
>> -        DRM_ERROR("Failed to disable backlight %d\n", ret);
>> -
>> -    return ret;
>> -}
>> -EXPORT_SYMBOL(tinydrm_disable_backlight);
>>     #if IS_ENABLED(CONFIG_SPI)
>>   diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
>> b/drivers/gpu/drm/tinydrm/mi0283qt.c
>> index 7e5bb7d..c161d45 100644
>> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
>> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
>> @@ -12,6 +12,7 @@
>>   #include <drm/tinydrm/ili9341.h>
>>   #include <drm/tinydrm/mipi-dbi.h>
>>   #include <drm/tinydrm/tinydrm-helpers.h>
>> +#include <drm/tinydrm/tinydrm-backlight.h>
>>   #include <linux/delay.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/module.h>
>> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
>> b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> index 2caeabc..dc84f26 100644
>> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> @@ -11,6 +11,7 @@
>>     #include <drm/tinydrm/mipi-dbi.h>
>>   #include <drm/tinydrm/tinydrm-helpers.h>
>> +#include <drm/tinydrm/tinydrm-backlight.h>
>>   #include <linux/debugfs.h>
>>   #include <linux/dma-buf.h>
>>   #include <linux/gpio/consumer.h>
>> diff --git a/include/drm/tinydrm/tinydrm-backlight.h 
>> b/include/drm/tinydrm/tinydrm-backlight.h
>> new file mode 100644
>> index 0000000..6a7b6d5
>> --- /dev/null
>> +++ b/include/drm/tinydrm/tinydrm-backlight.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Copyright (C) 2016 Noralf Trønnes
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef __LINUX_TINYDRM_BACKLIGHT_H
>> +#define __LINUX_TINYDRM_BACKLIGHT_H
>> +
>> +struct backlight_device;
>> +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);
>> +
>> +#endif /* __LINUX_TINYDRM_BACKLIGHT_H */
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>



More information about the dri-devel mailing list