linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs
@ 2019-06-24 20:31 Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 1/4] MAINTAINERS: Add entry for stable backlight sysfs ABI documentation Matthias Kaehlcke
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-24 20:31 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz
  Cc: linux-pwm, dri-devel, linux-fbdev, linux-kernel,
	Enric Balletbo i Serra, Douglas Anderson, Brian Norris,
	Pavel Machek, Jacek Anaszewski, Matthias Kaehlcke

Backlight brightness curves can have different shapes. The two main
types are linear and non-linear curves. The human eye doesn't
perceive linearly increasing/decreasing brightness as linear (see
also 88ba95bedb79 "backlight: pwm_bl: Compute brightness of LED
linearly to human eye"), hence many backlights use non-linear (often
logarithmic) brightness curves. The type of curve is currently opaque
to userspace, so userspace often relies on more or less reliable
heuristics (like the number of brightness levels) to decide whether
to treat a backlight device as linear or non-linear.

Export the type of the brightness curve via a new sysfs attribute.

Matthias Kaehlcke (4):
  MAINTAINERS: Add entry for stable backlight sysfs ABI documentation
  backlight: Expose brightness curve type through sysfs
  backlight: pwm_bl: Set scale type for CIE 1931 curves
  backlight: pwm_bl: Set scale type for brightness curves specified in
    the DT

 .../ABI/testing/sysfs-class-backlight         | 32 +++++++++++++++++
 MAINTAINERS                                   |  2 ++
 drivers/video/backlight/backlight.c           | 21 +++++++++++
 drivers/video/backlight/pwm_bl.c              | 35 ++++++++++++++++++-
 include/linux/backlight.h                     | 10 ++++++
 5 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-backlight

-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] MAINTAINERS: Add entry for stable backlight sysfs ABI documentation
  2019-06-24 20:31 [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
@ 2019-06-24 20:31 ` Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-24 20:31 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz
  Cc: linux-pwm, dri-devel, linux-fbdev, linux-kernel,
	Enric Balletbo i Serra, Douglas Anderson, Brian Norris,
	Pavel Machek, Jacek Anaszewski, Matthias Kaehlcke

Add an entry for the stable backlight sysfs ABI to the MAINTAINERS
file.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Changes in v2:
- added Daniel's 'Acked-by' tag
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 57f496cff999..d51e74340870 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2857,6 +2857,7 @@ F:	drivers/video/backlight/
 F:	include/linux/backlight.h
 F:	include/linux/pwm_backlight.h
 F:	Documentation/devicetree/bindings/leds/backlight
+F:	Documentation/ABI/stable/sysfs-class-backlight
 
 BATMAN ADVANCED
 M:	Marek Lindner <mareklindner@neomailbox.ch>
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs
  2019-06-24 20:31 [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 1/4] MAINTAINERS: Add entry for stable backlight sysfs ABI documentation Matthias Kaehlcke
@ 2019-06-24 20:31 ` Matthias Kaehlcke
  2019-06-26 14:56   ` Pavel Machek
  2019-06-24 20:31 ` [PATCH v2 3/4] backlight: pwm_bl: Set scale type for CIE 1931 curves Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT Matthias Kaehlcke
  3 siblings, 1 reply; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-24 20:31 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz
  Cc: linux-pwm, dri-devel, linux-fbdev, linux-kernel,
	Enric Balletbo i Serra, Douglas Anderson, Brian Norris,
	Pavel Machek, Jacek Anaszewski, Matthias Kaehlcke

Backlight brightness curves can have different shapes. The two main
types are linear and non-linear curves. The human eye doesn't
perceive linearly increasing/decreasing brightness as linear (see
also 88ba95bedb79 "backlight: pwm_bl: Compute brightness of LED
linearly to human eye"), hence many backlights use non-linear (often
logarithmic) brightness curves. The type of curve currently is opaque
to userspace, so userspace often uses more or less reliable heuristics
(like the number of brightness levels) to decide whether to treat a
backlight device as linear or non-linear.

Export the type of the brightness curve via the new sysfs attribute
'scale'. The value of the attribute may be a simple string like
'linear' or 'non-linear', or a composite string similar to
'compatible' strings of the device tree. A composite string consists
of different elements separated by commas, starting with the
most-detailed description and ending with the least-detailed one. An
example for a composite string is "cie-1931,perceptual,non-linear"
This brightness curve was generated with the CIE 1931 algorithm, it
is perceptually linear, but not actually linear in terms of the
emitted light. If userspace doesn't know about 'cie-1931' or
'perceptual' it should at least be able to interpret the 'non-linear'
part.

For devices that don't provide information about the scale of their
brightness curve the value of the 'scale' attribute is 'unknown'.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- changed order of brightness scale enums, explicitly make 'unknown' zero
- minor update of commit message
- deleted excess blank line after 'backlight_scale_types'
- s/curves/curve/ in sysfs doc
---
 .../ABI/testing/sysfs-class-backlight         | 32 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 drivers/video/backlight/backlight.c           | 21 ++++++++++++
 include/linux/backlight.h                     | 10 ++++++
 4 files changed, 64 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-backlight

diff --git a/Documentation/ABI/testing/sysfs-class-backlight b/Documentation/ABI/testing/sysfs-class-backlight
new file mode 100644
index 000000000000..da1ce9e5c55a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-backlight
@@ -0,0 +1,32 @@
+What:		/sys/class/backlight/<backlight>/scale
+Date:		June 2019
+KernelVersion:	5.4
+Contact:	Daniel Thompson <daniel.thompson@linaro.org>
+Description:
+		Description of the scale of the brightness curve. The
+		description consists of one or more elements separated by
+		commas, from the most detailed to the least detailed
+		description.
+
+		Possible values are:
+
+		unknown
+		  The scale of the brightness curve is unknown.
+
+		linear
+		  The brightness changes linearly in terms of the emitted
+		  light, changes are perceived as non-linear by the human eye.
+
+		non-linear
+		  The brightness changes non-linearly in terms of the emitted
+		  light, changes might be perceived as linear by the human eye.
+
+		perceptual,non-linear
+		  The brightness changes non-linearly in terms of the emitted
+		  light, changes should be perceived as linear by the human eye.
+
+		cie-1931,perceptual,non-linear
+		  The brightness curve was calculated with the CIE 1931
+		  algorithm. Brightness changes non-linearly in terms of the
+		  emitted light, changes should be perceived as linear by the
+		  human eye.
diff --git a/MAINTAINERS b/MAINTAINERS
index d51e74340870..c46812510ba5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2858,6 +2858,7 @@ F:	include/linux/backlight.h
 F:	include/linux/pwm_backlight.h
 F:	Documentation/devicetree/bindings/leds/backlight
 F:	Documentation/ABI/stable/sysfs-class-backlight
+F:	Documentation/ABI/testing/sysfs-class-backlight
 
 BATMAN ADVANCED
 M:	Marek Lindner <mareklindner@neomailbox.ch>
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 1ef8b6fd62ac..86612ec42ed0 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -32,6 +32,14 @@ static const char *const backlight_types[] = {
 	[BACKLIGHT_FIRMWARE] = "firmware",
 };
 
+static const char *const backlight_scale_types[] = {
+	[BACKLIGHT_SCALE_UNKNOWN]	= "unknown",
+	[BACKLIGHT_SCALE_LINEAR]	= "linear",
+	[BACKLIGHT_SCALE_NON_LINEAR]	= "non-linear",
+	[BACKLIGHT_SCALE_PERCEPTUAL]	= "perceptual,non-linear",
+	[BACKLIGHT_SCALE_CIE1931]	= "cie-1931,perceptual,non-linear",
+};
+
 #if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
 			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
 /* This callback gets called when something important happens inside a
@@ -246,6 +254,18 @@ static ssize_t actual_brightness_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(actual_brightness);
 
+static ssize_t scale_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct backlight_device *bd = to_backlight_device(dev);
+
+	if (WARN_ON(bd->props.scale > BACKLIGHT_SCALE_NON_LINEAR))
+		return sprintf(buf, "unknown\n");
+
+	return sprintf(buf, "%s\n", backlight_scale_types[bd->props.scale]);
+}
+static DEVICE_ATTR_RO(scale);
+
 static struct class *backlight_class;
 
 #ifdef CONFIG_PM_SLEEP
@@ -292,6 +312,7 @@ static struct attribute *bl_device_attrs[] = {
 	&dev_attr_brightness.attr,
 	&dev_attr_actual_brightness.attr,
 	&dev_attr_max_brightness.attr,
+	&dev_attr_scale.attr,
 	&dev_attr_type.attr,
 	NULL,
 };
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 0b5897446dca..94d3559cd968 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -46,6 +46,14 @@ enum backlight_notification {
 	BACKLIGHT_UNREGISTERED,
 };
 
+enum backlight_scale {
+	BACKLIGHT_SCALE_UNKNOWN = 0,
+	BACKLIGHT_SCALE_LINEAR,
+	BACKLIGHT_SCALE_NON_LINEAR,	/* needed for backwards compatibility */
+	BACKLIGHT_SCALE_PERCEPTUAL,
+	BACKLIGHT_SCALE_CIE1931,
+};
+
 struct backlight_device;
 struct fb_info;
 
@@ -80,6 +88,8 @@ struct backlight_properties {
 	enum backlight_type type;
 	/* Flags used to signal drivers of state changes */
 	unsigned int state;
+	/* Type of the brightness scale (linear, non-linear, ...) */
+	enum backlight_scale scale;
 
 #define BL_CORE_SUSPENDED	(1 << 0)	/* backlight is suspended */
 #define BL_CORE_FBBLANK		(1 << 1)	/* backlight is under an fb blank event */
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] backlight: pwm_bl: Set scale type for CIE 1931 curves
  2019-06-24 20:31 [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 1/4] MAINTAINERS: Add entry for stable backlight sysfs ABI documentation Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
@ 2019-06-24 20:31 ` Matthias Kaehlcke
  2019-06-24 20:31 ` [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT Matthias Kaehlcke
  3 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-24 20:31 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz
  Cc: linux-pwm, dri-devel, linux-fbdev, linux-kernel,
	Enric Balletbo i Serra, Douglas Anderson, Brian Norris,
	Pavel Machek, Jacek Anaszewski, Matthias Kaehlcke

For backlight curves calculated with the CIE 1931 algorithm set
the brightness scale type property accordingly. This makes the
scale type available to userspace via the 'scale' sysfs attribute.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Changes in v2:
- added Enric's 'Tested-by' tag
- added Daniel's 'Acked-by' tag
---
 drivers/video/backlight/pwm_bl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index fb45f866b923..f067fe7aa35d 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -553,6 +553,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		goto err_alloc;
 	}
 
+	memset(&props, 0, sizeof(struct backlight_properties));
+
 	if (data->levels) {
 		/*
 		 * For the DT case, only when brightness levels is defined
@@ -591,6 +593,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 			pb->levels = data->levels;
 		}
+
+		props.scale = BACKLIGHT_SCALE_CIE1931;
 	} else {
 		/*
 		 * That only happens for the non-DT case, where platform data
@@ -601,7 +605,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 	pb->lth_brightness = data->lth_brightness * (state.period / pb->scale);
 
-	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = data->max_brightness;
 	bl = backlight_device_register(dev_name(&pdev->dev), &pdev->dev, pb,
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT
  2019-06-24 20:31 [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
                   ` (2 preceding siblings ...)
  2019-06-24 20:31 ` [PATCH v2 3/4] backlight: pwm_bl: Set scale type for CIE 1931 curves Matthias Kaehlcke
@ 2019-06-24 20:31 ` Matthias Kaehlcke
  2019-06-26 14:56   ` Pavel Machek
  3 siblings, 1 reply; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-24 20:31 UTC (permalink / raw)
  To: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz
  Cc: linux-pwm, dri-devel, linux-fbdev, linux-kernel,
	Enric Balletbo i Serra, Douglas Anderson, Brian Norris,
	Pavel Machek, Jacek Anaszewski, Matthias Kaehlcke

Check if a brightness curve specified in the device tree is linear or
not and set the corresponding property accordingly. This makes the
scale type available to userspace via the 'scale' sysfs attribute.

To determine if a curve is linear it is compared to a interpolated linear
curve between min and max brightness. The curve is considered linear if
no value deviates more than +/-5% of ${brightness_range} from their
interpolated value.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
Changes in v2:
- use 128 (power of two) instead of 100 as factor for the slope
- add comment about max quantization error
- added Daniel's 'Acked-by' tag
---
 drivers/video/backlight/pwm_bl.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index f067fe7aa35d..2297fb4af49d 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -404,6 +404,31 @@ int pwm_backlight_brightness_default(struct device *dev,
 }
 #endif
 
+static bool pwm_backlight_is_linear(struct platform_pwm_backlight_data *data)
+{
+	unsigned int nlevels = data->max_brightness + 1;
+	unsigned int min_val = data->levels[0];
+	unsigned int max_val = data->levels[nlevels - 1];
+	/*
+	 * Multiplying by 128 means that even in pathological cases such
+	 * as (max_val - min_val) == nlevels the error at max_val is less
+	 * than 1%.
+	 */
+	unsigned int slope = (128 * (max_val - min_val)) / nlevels;
+	unsigned int margin = (max_val - min_val) / 20; /* 5% */
+	int i;
+
+	for (i = 1; i < nlevels; i++) {
+		unsigned int linear_value = min_val + ((i * slope) / 128);
+		unsigned int delta = abs(linear_value - data->levels[i]);
+
+		if (delta > margin)
+			return false;
+	}
+
+	return true;
+}
+
 static int pwm_backlight_initial_power_state(const struct pwm_bl_data *pb)
 {
 	struct device_node *node = pb->dev->of_node;
@@ -567,6 +592,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 			pb->levels = data->levels;
 		}
+
+		if (pwm_backlight_is_linear(data))
+			props.scale = BACKLIGHT_SCALE_LINEAR;
+		else
+			props.scale = BACKLIGHT_SCALE_NON_LINEAR;
 	} else if (!data->max_brightness) {
 		/*
 		 * If no brightness levels are provided and max_brightness is
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs
  2019-06-24 20:31 ` [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
@ 2019-06-26 14:56   ` Pavel Machek
  2019-06-28  8:34     ` Daniel Thompson
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Machek @ 2019-06-26 14:56 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

Hi!

> Export the type of the brightness curve via the new sysfs attribute
> 'scale'. The value of the attribute may be a simple string like
> 'linear' or 'non-linear', or a composite string similar to
> 'compatible' strings of the device tree. A composite string consists
> of different elements separated by commas, starting with the
> most-detailed description and ending with the least-detailed one. An
> example for a composite string is "cie-1931,perceptual,non-linear"
> This brightness curve was generated with the CIE 1931 algorithm, it
> is perceptually linear, but not actually linear in terms of the
> emitted light. If userspace doesn't know about 'cie-1931' or
> 'perceptual' it should at least be able to interpret the 'non-linear'
> part.

I'm not sure the comma-separated thing is a good idea. If it is, it should 
go to the Documentation, not to changelog.

> +What:		/sys/class/backlight/<backlight>/scale
> +Date:		June 2019
> +KernelVersion:	5.4
> +Contact:	Daniel Thompson <daniel.thompson@linaro.org>
> +Description:
> +		Description of the scale of the brightness curve. The
> +		description consists of one or more elements separated by
> +		commas, from the most detailed to the least detailed
> +		description.
> +
> +		Possible values are:
> +
> +		unknown
> +		  The scale of the brightness curve is unknown.
> +
> +		linear
> +		  The brightness changes linearly in terms of the emitted
> +		  light, changes are perceived as non-linear by the human eye.
> +
> +		non-linear
> +		  The brightness changes non-linearly in terms of the emitted
> +		  light, changes might be perceived as linear by the human eye.

non-linear is not too useful as described.

> +		perceptual,non-linear
> +		  The brightness changes non-linearly in terms of the emitted
> +		  light, changes should be perceived as linear by the human eye.
> +
> +		cie-1931,perceptual,non-linear
> +		  The brightness curve was calculated with the CIE 1931
> +		  algorithm. Brightness changes non-linearly in terms of the
> +		  emitted light, changes should be perceived as linear by the
> +		  human eye.

Is it useful to know difference between perceptual, and cie-1931?

Would it be useful to export absolute values in some well-known units?

If I'm in dark room, I may want 100mW/m^2 of backlight... And it would
be nice if I could set same backlight intensity on all my devices easily.

								Pavel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT
  2019-06-24 20:31 ` [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT Matthias Kaehlcke
@ 2019-06-26 14:56   ` Pavel Machek
  2019-06-28  7:55     ` Daniel Thompson
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Machek @ 2019-06-26 14:56 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Thierry Reding, Lee Jones, Daniel Thompson, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

On Mon 2019-06-24 13:31:13, Matthias Kaehlcke wrote:
> Check if a brightness curve specified in the device tree is linear or
> not and set the corresponding property accordingly. This makes the
> scale type available to userspace via the 'scale' sysfs attribute.
> 
> To determine if a curve is linear it is compared to a interpolated linear
> curve between min and max brightness. The curve is considered linear if
> no value deviates more than +/-5% of ${brightness_range} from their
> interpolated value.

I don't think this works. Some hardware does takes brightness in perceptual units,
converting it in the LED controller.

									Pavel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT
  2019-06-26 14:56   ` Pavel Machek
@ 2019-06-28  7:55     ` Daniel Thompson
  2019-06-28  8:18       ` Pavel Machek
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2019-06-28  7:55 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Matthias Kaehlcke, Thierry Reding, Lee Jones, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

On Wed, Jun 26, 2019 at 04:56:18PM +0200, Pavel Machek wrote:
> On Mon 2019-06-24 13:31:13, Matthias Kaehlcke wrote:
> > Check if a brightness curve specified in the device tree is linear or
> > not and set the corresponding property accordingly. This makes the
> > scale type available to userspace via the 'scale' sysfs attribute.
> > 
> > To determine if a curve is linear it is compared to a interpolated linear
> > curve between min and max brightness. The curve is considered linear if
> > no value deviates more than +/-5% of ${brightness_range} from their
> > interpolated value.
> 
> I don't think this works. Some hardware does takes brightness in perceptual units,
> converting it in the LED controller.

This check is exclusive to PWM backlights so I'd like to double check
that you are thinking specifically of hardware that takes it's signal
from the PWM and works in perceptual units?

I don't recall any examples being offered when we reviewed the
auto-generated CIE tables (although since that can be overriden by DT it
was not of the same gravity and this example).


Daniel.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT
  2019-06-28  7:55     ` Daniel Thompson
@ 2019-06-28  8:18       ` Pavel Machek
  0 siblings, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2019-06-28  8:18 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Matthias Kaehlcke, Thierry Reding, Lee Jones, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

On Fri 2019-06-28 08:55:16, Daniel Thompson wrote:
> On Wed, Jun 26, 2019 at 04:56:18PM +0200, Pavel Machek wrote:
> > On Mon 2019-06-24 13:31:13, Matthias Kaehlcke wrote:
> > > Check if a brightness curve specified in the device tree is linear or
> > > not and set the corresponding property accordingly. This makes the
> > > scale type available to userspace via the 'scale' sysfs attribute.
> > > 
> > > To determine if a curve is linear it is compared to a interpolated linear
> > > curve between min and max brightness. The curve is considered linear if
> > > no value deviates more than +/-5% of ${brightness_range} from their
> > > interpolated value.
> > 
> > I don't think this works. Some hardware does takes brightness in perceptual units,
> > converting it in the LED controller.
> 
> This check is exclusive to PWM backlights so I'd like to double check
> that you are thinking specifically of hardware that takes it's signal
> from the PWM and works in perceptual units?

I missed that details. Taking PWM input then converting it to
perceptual units would indeed be strange.

Sorry,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs
  2019-06-26 14:56   ` Pavel Machek
@ 2019-06-28  8:34     ` Daniel Thompson
  2019-07-01 16:55       ` Matthias Kaehlcke
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2019-06-28  8:34 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Matthias Kaehlcke, Thierry Reding, Lee Jones, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

On Wed, Jun 26, 2019 at 04:56:11PM +0200, Pavel Machek wrote:
> Hi!
> 
> > Export the type of the brightness curve via the new sysfs attribute
> > 'scale'. The value of the attribute may be a simple string like
> > 'linear' or 'non-linear', or a composite string similar to
> > 'compatible' strings of the device tree. A composite string consists
> > of different elements separated by commas, starting with the
> > most-detailed description and ending with the least-detailed one. An
> > example for a composite string is "cie-1931,perceptual,non-linear"
> > This brightness curve was generated with the CIE 1931 algorithm, it
> > is perceptually linear, but not actually linear in terms of the
> > emitted light. If userspace doesn't know about 'cie-1931' or
> > 'perceptual' it should at least be able to interpret the 'non-linear'
> > part.
> 
> I'm not sure the comma-separated thing is a good idea. If it is, it should 
> go to the Documentation, not to changelog.

So I viewed the comma-separated thing as allow us to describe facts about
the scale used.

In particular I suspect that some controllers will be non-linear *and*
non-perceptual and that some userspaces, particularly those that animate
backlight changes, may care enough about the difference to ask us to add
another fact to the set that describes that scale.

Having said that I do share your concern that the comma-separated list
is overengineered and that all userspaces will end up implementing
something like:

if (strstr("non-linear", scale) {
  mode = PERCEPTUAL;
} else if (strstr("unknown", scale) {
  mode = use_existing_hueristic();
} else {
  mode = LINEAR;
}


> > +What:		/sys/class/backlight/<backlight>/scale
> > +Date:		June 2019
> > +KernelVersion:	5.4
> > +Contact:	Daniel Thompson <daniel.thompson@linaro.org>
> > +Description:
> > +		Description of the scale of the brightness curve. The
> > +		description consists of one or more elements separated by
> > +		commas, from the most detailed to the least detailed
> > +		description.
> > +
> > +		Possible values are:
> > +
> > +		unknown
> > +		  The scale of the brightness curve is unknown.
> > +
> > +		linear
> > +		  The brightness changes linearly in terms of the emitted
> > +		  light, changes are perceived as non-linear by the human eye.
> > +
> > +		non-linear
> > +		  The brightness changes non-linearly in terms of the emitted
> > +		  light, changes might be perceived as linear by the human eye.
> 
> non-linear is not too useful as described.

Agree.

The idea is that allows a userspace with simple backlight needs to
simple map the brightness property directly to a slider using the
approach above without worrying about perceptual or (possible future)
logarithmic scales. Such an approach won't be perfect but it
probably won't feel horrible for the user either.

Arguably the descriptions should move away from the raw factual
approach and describe what advise the kernel of offering the
userspace.


> > +		perceptual,non-linear
> > +		  The brightness changes non-linearly in terms of the emitted
> > +		  light, changes should be perceived as linear by the human eye.
> > +
> > +		cie-1931,perceptual,non-linear
> > +		  The brightness curve was calculated with the CIE 1931
> > +		  algorithm. Brightness changes non-linearly in terms of the
> > +		  emitted light, changes should be perceived as linear by the
> > +		  human eye.
> 
> Is it useful to know difference between perceptual, and cie-1931?

Depends how assertive the userspaces are!

If they follow the "fix kernel bugs in the kernel" mantra rather than
implement workarounds and heuristics then I suspect it would not be used
much.


> Would it be useful to export absolute values in some well-known units?
> 
> If I'm in dark room, I may want 100mW/m^2 of backlight... And it would
> be nice if I could set same backlight intensity on all my devices
> easily.

I'm a little sceptical that we could calibrate an absolute scale on
enough devices for such a property to be useful. I think it would be
"unknown" on almost every system.


Daniel.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs
  2019-06-28  8:34     ` Daniel Thompson
@ 2019-07-01 16:55       ` Matthias Kaehlcke
  0 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-07-01 16:55 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Pavel Machek, Thierry Reding, Lee Jones, Jingoo Han,
	Bartlomiej Zolnierkiewicz, linux-pwm, dri-devel, linux-fbdev,
	linux-kernel, Enric Balletbo i Serra, Douglas Anderson,
	Brian Norris, Jacek Anaszewski

Hi,

On Fri, Jun 28, 2019 at 09:34:52AM +0100, Daniel Thompson wrote:
> On Wed, Jun 26, 2019 at 04:56:11PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > Export the type of the brightness curve via the new sysfs attribute
> > > 'scale'. The value of the attribute may be a simple string like
> > > 'linear' or 'non-linear', or a composite string similar to
> > > 'compatible' strings of the device tree. A composite string consists
> > > of different elements separated by commas, starting with the
> > > most-detailed description and ending with the least-detailed one. An
> > > example for a composite string is "cie-1931,perceptual,non-linear"
> > > This brightness curve was generated with the CIE 1931 algorithm, it
> > > is perceptually linear, but not actually linear in terms of the
> > > emitted light. If userspace doesn't know about 'cie-1931' or
> > > 'perceptual' it should at least be able to interpret the 'non-linear'
> > > part.
> > 
> > I'm not sure the comma-separated thing is a good idea. If it is, it should 
> > go to the Documentation, not to changelog.
> 
> So I viewed the comma-separated thing as allow us to describe facts about
> the scale used.
> 
> In particular I suspect that some controllers will be non-linear *and*
> non-perceptual and that some userspaces, particularly those that animate
> backlight changes, may care enough about the difference to ask us to add
> another fact to the set that describes that scale.
> 
> Having said that I do share your concern that the comma-separated list
> is overengineered and that all userspaces will end up implementing
> something like:
> 
> if (strstr("non-linear", scale) {
>   mode = PERCEPTUAL;
> } else if (strstr("unknown", scale) {
>   mode = use_existing_hueristic();
> } else {
>   mode = LINEAR;
> }

I agree that this is not unlikely ...

So let's just make it 'linear', 'non-linear' and 'unknown'?

> > > +What:		/sys/class/backlight/<backlight>/scale
> > > +Date:		June 2019
> > > +KernelVersion:	5.4
> > > +Contact:	Daniel Thompson <daniel.thompson@linaro.org>
> > > +Description:
> > > +		Description of the scale of the brightness curve. The
> > > +		description consists of one or more elements separated by
> > > +		commas, from the most detailed to the least detailed
> > > +		description.
> > > +
> > > +		Possible values are:
> > > +
> > > +		unknown
> > > +		  The scale of the brightness curve is unknown.
> > > +
> > > +		linear
> > > +		  The brightness changes linearly in terms of the emitted
> > > +		  light, changes are perceived as non-linear by the human eye.
> > > +
> > > +		non-linear
> > > +		  The brightness changes non-linearly in terms of the emitted
> > > +		  light, changes might be perceived as linear by the human eye.
> > 
> > non-linear is not too useful as described.
> 
> Agree.
> 
> The idea is that allows a userspace with simple backlight needs to
> simple map the brightness property directly to a slider using the
> approach above without worrying about perceptual or (possible future)
> logarithmic scales. Such an approach won't be perfect but it
> probably won't feel horrible for the user either.
> 
> Arguably the descriptions should move away from the raw factual
> approach and describe what advise the kernel of offering the
> userspace.

ok, I'll change it in the next revision

> > > +		perceptual,non-linear
> > > +		  The brightness changes non-linearly in terms of the emitted
> > > +		  light, changes should be perceived as linear by the human eye.
> > > +
> > > +		cie-1931,perceptual,non-linear
> > > +		  The brightness curve was calculated with the CIE 1931
> > > +		  algorithm. Brightness changes non-linearly in terms of the
> > > +		  emitted light, changes should be perceived as linear by the
> > > +		  human eye.
> > 
> > Is it useful to know difference between perceptual, and cie-1931?
> 
> Depends how assertive the userspaces are!
> 
> If they follow the "fix kernel bugs in the kernel" mantra rather than
> implement workarounds and heuristics then I suspect it would not be used
> much.
> 
> 
> > Would it be useful to export absolute values in some well-known units?
> > 
> > If I'm in dark room, I may want 100mW/m^2 of backlight... And it would
> > be nice if I could set same backlight intensity on all my devices
> > easily.
> 
> I'm a little sceptical that we could calibrate an absolute scale on
> enough devices for such a property to be useful. I think it would be
> "unknown" on almost every system.

I share your scepticism and would expect most devices to remain
"unknown"

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-07-01 16:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 20:31 [PATCH v2 0/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
2019-06-24 20:31 ` [PATCH v2 1/4] MAINTAINERS: Add entry for stable backlight sysfs ABI documentation Matthias Kaehlcke
2019-06-24 20:31 ` [PATCH v2 2/4] backlight: Expose brightness curve type through sysfs Matthias Kaehlcke
2019-06-26 14:56   ` Pavel Machek
2019-06-28  8:34     ` Daniel Thompson
2019-07-01 16:55       ` Matthias Kaehlcke
2019-06-24 20:31 ` [PATCH v2 3/4] backlight: pwm_bl: Set scale type for CIE 1931 curves Matthias Kaehlcke
2019-06-24 20:31 ` [PATCH v2 4/4] backlight: pwm_bl: Set scale type for brightness curves specified in the DT Matthias Kaehlcke
2019-06-26 14:56   ` Pavel Machek
2019-06-28  7:55     ` Daniel Thompson
2019-06-28  8:18       ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).