[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