[56/59] drm/todo: Update backlight todo
diff mbox series

Message ID 20190614203615.12639-57-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • prime doc polish and ... a few cleanups
Related show

Commit Message

Daniel Vetter June 14, 2019, 8:36 p.m. UTC
Basic helpers have been extracted, now there's a pile more todo still
across the entire tree.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/todo.rst | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

Comments

Sam Ravnborg June 14, 2019, 9:46 p.m. UTC | #1
On Fri, Jun 14, 2019 at 10:36:12PM +0200, Daniel Vetter wrote:
> Basic helpers have been extracted, now there's a pile more todo still
> across the entire tree.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks for taking care of this.

I have considered enhancing drm_panel so if properly configured
then backligt enable/disable are done by the core.
Will land some patches one day, lets take any discussion based on
that.

	Sam
Daniel Thompson June 21, 2019, 1:11 p.m. UTC | #2
On 14/06/2019 21:36, Daniel Vetter wrote:
> Basic helpers have been extracted, now there's a pile more todo still
> across the entire tree.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

No disagreement here... so FWIW:

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

> ---
>   Documentation/gpu/todo.rst | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 21a7b7800d3e..c4e7c0672a14 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -422,6 +422,19 @@ fit the available time.
>   
>   Contact: Daniel Vetter
>   
> +Backlight Refactoring
> +---------------------
> +
> +Backlight drivers have a triple enable/disable state, which is a bit overkill.
> +Plan to fix this:
> +
> +1. Roll out backlight_enable() and backlight_disable() helpers everywhere. This
> +   has started already.
> +2. In all, only look at one of the three status bits set by the above helpers.
> +3. Remove the other two status bits.
> +
> +Contact: Daniel Vetter
> +
>   Driver Specific
>   ===============
>   
> @@ -431,20 +444,6 @@ tinydrm
>   Tinydrm is the helper driver for really simple fb drivers. The goal is to make
>   those drivers as simple as possible, so lots of room for refactoring:
>   
> -- 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. Backlight helpers require a fair
> -  bit of reworking and refactoring. A simple example is the enabling of a backlight.
> -  Tinydrm has helpers for this. It would be good if other drivers can also use the
> -  helper. However, there are various cases we need to consider i.e different
> -  drivers seem to have different ways of enabling/disabling a backlight.
> -  We also need to consider the backlight drivers (like gpio_backlight). The situation
> -  is further complicated by the fact that the backlight is tied to fbdev
> -  via fb_notifier_callback() which has complicated logic. For further details, refer
> -  to the following discussion thread:
> -  https://groups.google.com/forum/#!topic/outreachy-kernel/8rBe30lwtdA
> -
>   - spi helpers, probably best put into spi core/helper code. Thierry said
>     the spi maintainer is fast&reactive, so shouldn't be a big issue.
>   
>

Patch
diff mbox series

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 21a7b7800d3e..c4e7c0672a14 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -422,6 +422,19 @@  fit the available time.
 
 Contact: Daniel Vetter
 
+Backlight Refactoring
+---------------------
+
+Backlight drivers have a triple enable/disable state, which is a bit overkill.
+Plan to fix this:
+
+1. Roll out backlight_enable() and backlight_disable() helpers everywhere. This
+   has started already.
+2. In all, only look at one of the three status bits set by the above helpers.
+3. Remove the other two status bits.
+
+Contact: Daniel Vetter
+
 Driver Specific
 ===============
 
@@ -431,20 +444,6 @@  tinydrm
 Tinydrm is the helper driver for really simple fb drivers. The goal is to make
 those drivers as simple as possible, so lots of room for refactoring:
 
-- 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. Backlight helpers require a fair
-  bit of reworking and refactoring. A simple example is the enabling of a backlight.
-  Tinydrm has helpers for this. It would be good if other drivers can also use the
-  helper. However, there are various cases we need to consider i.e different
-  drivers seem to have different ways of enabling/disabling a backlight.
-  We also need to consider the backlight drivers (like gpio_backlight). The situation
-  is further complicated by the fact that the backlight is tied to fbdev
-  via fb_notifier_callback() which has complicated logic. For further details, refer
-  to the following discussion thread:
-  https://groups.google.com/forum/#!topic/outreachy-kernel/8rBe30lwtdA
-
 - spi helpers, probably best put into spi core/helper code. Thierry said
   the spi maintainer is fast&reactive, so shouldn't be a big issue.