2023-01-16 23:03:31

by Richard Acayan

[permalink] [raw]
Subject: [RFC PATCH v3 0/3] drm/mipi-dsi: 16-bit Brightness Endianness Fix

Changes since v2 ([email protected]):
- patch vtdr6130 to use _large (3/3)
- remove Change-Id again (1/3)
- change patch subject (1-2/3)
- correct function name in patch description (2/3)
- add Tested-by tags (1-2/3)

Changes since v1 ([email protected]):
- move 16-bit brightness handling to new functions and revert API
change (1/2)
- remove Change-Id in compliance with checkpatch.pl (1/2)
- separate panel driver changes (2/2)

This series adds proper support for 16-bit MIPI DSI brightness and
cleans up existing panel drivers with 16-bit brightness.

Both setting and getting works on an external S6E3FA7
(max_brightness = 1023) panel driver with the first patch.

Even though this originally fixed both 8-bit and 16-bit brightness, the
8-bit fix is omitted for now because it affects ~13 panels that would
need to be tested:

panel-asus-z00t-tm5p5-n35596.c
panel-boe-bf060y8m-aj0.c
panel-dsi-cm.c
panel-ebbg-ft8719.c
panel-jdi-fhd-r63452.c
panel-jdi-lt070me05000.c
panel-novatek-nt35510.c
panel-novatek-nt35560.c
panel-orisetech-otm8009a.c
panel-raydium-rm67191.c
panel-samsung-s6e63j0x03.c
panel-sony-acx565akm.c
panel-sony-tulip-truly-nt35521.c

Daniel Mentz (1):
drm/mipi-dsi: Fix byte order of 16-bit DCS set/get brightness

Richard Acayan (2):
drm/panel: sofef00: Use 16-bit brightness function
drm/panel: vtdr6130: Use 16-bit brightness function

drivers/gpu/drm/drm_mipi_dsi.c | 52 +++++++++++++++++++
drivers/gpu/drm/panel/panel-samsung-sofef00.c | 9 +---
.../gpu/drm/panel/panel-visionox-vtdr6130.c | 5 +-
include/drm/drm_mipi_dsi.h | 4 ++
4 files changed, 59 insertions(+), 11 deletions(-)

--
2.39.0


2023-01-16 23:04:03

by Richard Acayan

[permalink] [raw]
Subject: [RFC PATCH v3 3/3] drm/panel: vtdr6130: Use 16-bit brightness function

This panel communicates brightness in big endian. This is not a quirk of
the panels themselves, but rather, a part of the MIPI standard. Use the
new mipi_dsi_dcs_set_display_brightness_large() function that properly
handles 16-bit brightness instead of bypassing the brightness functions
entirely.

Signed-off-by: Richard Acayan <[email protected]>
---
drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
index f9a6abc1e121..1092075b31a5 100644
--- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
+++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
@@ -243,12 +243,9 @@ static int visionox_vtdr6130_bl_update_status(struct backlight_device *bl)
{
struct mipi_dsi_device *dsi = bl_get_data(bl);
u16 brightness = backlight_get_brightness(bl);
- /* Panel needs big-endian order of brightness value */
- u8 payload[2] = { brightness >> 8, brightness & 0xff };
int ret;

- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
- payload, sizeof(payload));
+ mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
if (ret < 0)
return ret;

--
2.39.0

2023-01-16 23:05:20

by Richard Acayan

[permalink] [raw]
Subject: [RFC PATCH v3 1/3] drm/mipi-dsi: Fix byte order of 16-bit DCS set/get brightness

From: Daniel Mentz <[email protected]>

The MIPI DCS specification demands that brightness values are sent in
big endian byte order. It also states that one parameter (i.e. one byte)
shall be sent/received for 8 bit wide values, and two parameters shall
be used for values that are between 9 and 16 bits wide.

Add new functions to properly handle 16-bit brightness in big endian,
since the two 8- and 16-bit cases are distinct from each other.

Fixes: 1a9d759331b8 ("drm/dsi: Implement DCS set/get display brightness")
Signed-off-by: Daniel Mentz <[email protected]>
Link: https://android.googlesource.com/kernel/msm/+/754affd62d0ee268c686c53169b1dbb7deac8550
[richard: fix 16-bit brightness_get]
[richard: use separate functions instead of switch/case]
[richard: split into 16-bit component]
Signed-off-by: Richard Acayan <[email protected]>
Tested-by: Caleb Connolly <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 52 ++++++++++++++++++++++++++++++++++
include/drm/drm_mipi_dsi.h | 4 +++
2 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 497ef4b6a90a..4bc15fbd009d 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -1224,6 +1224,58 @@ int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
}
EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness);

+/**
+ * mipi_dsi_dcs_set_display_brightness_large() - sets the 16-bit brightness value
+ * of the display
+ * @dsi: DSI peripheral device
+ * @brightness: brightness value
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int mipi_dsi_dcs_set_display_brightness_large(struct mipi_dsi_device *dsi,
+ u16 brightness)
+{
+ u8 payload[2] = { brightness >> 8, brightness & 0xff };
+ ssize_t err;
+
+ err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+ payload, sizeof(payload));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness_large);
+
+/**
+ * mipi_dsi_dcs_get_display_brightness_large() - gets the current 16-bit
+ * brightness value of the display
+ * @dsi: DSI peripheral device
+ * @brightness: brightness value
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
+ u16 *brightness)
+{
+ u8 brightness_be[2];
+ ssize_t err;
+
+ err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS,
+ brightness_be, sizeof(brightness_be));
+ if (err <= 0) {
+ if (err == 0)
+ err = -ENODATA;
+
+ return err;
+ }
+
+ *brightness = (brightness_be[0] << 8) | brightness_be[1];
+
+ return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_large);
+
static int mipi_dsi_drv_probe(struct device *dev)
{
struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 4f503d99f668..16f30975b22b 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -296,6 +296,10 @@ int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
u16 brightness);
int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
u16 *brightness);
+int mipi_dsi_dcs_set_display_brightness_large(struct mipi_dsi_device *dsi,
+ u16 brightness);
+int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
+ u16 *brightness);

/**
* mipi_dsi_generic_write_seq - transmit data using a generic write packet
--
2.39.0

2023-01-18 11:09:55

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFC PATCH v3 1/3] drm/mipi-dsi: Fix byte order of 16-bit DCS set/get brightness

On 16/01/2023 23:49, Richard Acayan wrote:
> From: Daniel Mentz <[email protected]>
>
> The MIPI DCS specification demands that brightness values are sent in
> big endian byte order. It also states that one parameter (i.e. one byte)
> shall be sent/received for 8 bit wide values, and two parameters shall
> be used for values that are between 9 and 16 bits wide.
>
> Add new functions to properly handle 16-bit brightness in big endian,
> since the two 8- and 16-bit cases are distinct from each other.
>
> Fixes: 1a9d759331b8 ("drm/dsi: Implement DCS set/get display brightness")
> Signed-off-by: Daniel Mentz <[email protected]>
> Link: https://android.googlesource.com/kernel/msm/+/754affd62d0ee268c686c53169b1dbb7deac8550
> [richard: fix 16-bit brightness_get]
> [richard: use separate functions instead of switch/case]
> [richard: split into 16-bit component]
> Signed-off-by: Richard Acayan <[email protected]>
> Tested-by: Caleb Connolly <[email protected]>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 52 ++++++++++++++++++++++++++++++++++
> include/drm/drm_mipi_dsi.h | 4 +++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 497ef4b6a90a..4bc15fbd009d 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -1224,6 +1224,58 @@ int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
> }
> EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness);
>
> +/**
> + * mipi_dsi_dcs_set_display_brightness_large() - sets the 16-bit brightness value
> + * of the display
> + * @dsi: DSI peripheral device
> + * @brightness: brightness value
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_set_display_brightness_large(struct mipi_dsi_device *dsi,
> + u16 brightness)
> +{
> + u8 payload[2] = { brightness >> 8, brightness & 0xff };
> + ssize_t err;
> +
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
> + payload, sizeof(payload));
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness_large);
> +
> +/**
> + * mipi_dsi_dcs_get_display_brightness_large() - gets the current 16-bit
> + * brightness value of the display
> + * @dsi: DSI peripheral device
> + * @brightness: brightness value
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
> + u16 *brightness)
> +{
> + u8 brightness_be[2];
> + ssize_t err;
> +
> + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS,
> + brightness_be, sizeof(brightness_be));
> + if (err <= 0) {
> + if (err == 0)
> + err = -ENODATA;
> +
> + return err;
> + }
> +
> + *brightness = (brightness_be[0] << 8) | brightness_be[1];
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_large);
> +
> static int mipi_dsi_drv_probe(struct device *dev)
> {
> struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 4f503d99f668..16f30975b22b 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -296,6 +296,10 @@ int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
> u16 brightness);
> int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
> u16 *brightness);
> +int mipi_dsi_dcs_set_display_brightness_large(struct mipi_dsi_device *dsi,
> + u16 brightness);
> +int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi,
> + u16 *brightness);
>
> /**
> * mipi_dsi_generic_write_seq - transmit data using a generic write packet

Reviewed-by: Neil Armstrong <[email protected]>

2023-01-18 11:13:19

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFC PATCH v3 3/3] drm/panel: vtdr6130: Use 16-bit brightness function

On 16/01/2023 23:49, Richard Acayan wrote:
> This panel communicates brightness in big endian. This is not a quirk of
> the panels themselves, but rather, a part of the MIPI standard. Use the
> new mipi_dsi_dcs_set_display_brightness_large() function that properly
> handles 16-bit brightness instead of bypassing the brightness functions
> entirely.
>
> Signed-off-by: Richard Acayan <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-visionox-vtdr6130.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> index f9a6abc1e121..1092075b31a5 100644
> --- a/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> +++ b/drivers/gpu/drm/panel/panel-visionox-vtdr6130.c
> @@ -243,12 +243,9 @@ static int visionox_vtdr6130_bl_update_status(struct backlight_device *bl)
> {
> struct mipi_dsi_device *dsi = bl_get_data(bl);
> u16 brightness = backlight_get_brightness(bl);
> - /* Panel needs big-endian order of brightness value */
> - u8 payload[2] = { brightness >> 8, brightness & 0xff };
> int ret;
>
> - ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
> - payload, sizeof(payload));
> + mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> if (ret < 0)
> return ret;
>

Reviewed-by: Neil Armstrong <[email protected]>
Tested-by: Neil Armstrong <[email protected]> # on SM8550-MTP

Thanks !

2023-01-19 07:18:14

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC PATCH v3 0/3] drm/mipi-dsi: 16-bit Brightness Endianness Fix

Hi Richard.
On Mon, Jan 16, 2023 at 05:49:06PM -0500, Richard Acayan wrote:
> Changes since v2 ([email protected]):
> - patch vtdr6130 to use _large (3/3)
> - remove Change-Id again (1/3)
> - change patch subject (1-2/3)
> - correct function name in patch description (2/3)
> - add Tested-by tags (1-2/3)
>
> Changes since v1 ([email protected]):
> - move 16-bit brightness handling to new functions and revert API
> change (1/2)
> - remove Change-Id in compliance with checkpatch.pl (1/2)
> - separate panel driver changes (2/2)
>
> This series adds proper support for 16-bit MIPI DSI brightness and
> cleans up existing panel drivers with 16-bit brightness.

The series is:
Reviewed-by: Sam Ravnborg <[email protected]>

Neil - I hope you can land this in drm-misc.

Sam

2023-01-19 07:53:30

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFC PATCH v3 0/3] drm/mipi-dsi: 16-bit Brightness Endianness Fix

On 19/01/2023 08:14, Sam Ravnborg wrote:
> Hi Richard.
> On Mon, Jan 16, 2023 at 05:49:06PM -0500, Richard Acayan wrote:
>> Changes since v2 ([email protected]):
>> - patch vtdr6130 to use _large (3/3)
>> - remove Change-Id again (1/3)
>> - change patch subject (1-2/3)
>> - correct function name in patch description (2/3)
>> - add Tested-by tags (1-2/3)
>>
>> Changes since v1 ([email protected]):
>> - move 16-bit brightness handling to new functions and revert API
>> change (1/2)
>> - remove Change-Id in compliance with checkpatch.pl (1/2)
>> - separate panel driver changes (2/2)
>>
>> This series adds proper support for 16-bit MIPI DSI brightness and
>> cleans up existing panel drivers with 16-bit brightness.
>
> The series is:
> Reviewed-by: Sam Ravnborg <[email protected]>
>
> Neil - I hope you can land this in drm-misc.

Sure, done !

Thanks,
Neil

>
> Sam

2023-01-19 07:56:23

by Neil Armstrong

[permalink] [raw]
Subject: Re: [RFC PATCH v3 0/3] drm/mipi-dsi: 16-bit Brightness Endianness Fix

Hi,

On Mon, 16 Jan 2023 17:49:06 -0500, Richard Acayan wrote:
> Changes since v2 ([email protected]):
> - patch vtdr6130 to use _large (3/3)
> - remove Change-Id again (1/3)
> - change patch subject (1-2/3)
> - correct function name in patch description (2/3)
> - add Tested-by tags (1-2/3)
>
> [...]

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)

[1/3] drm/mipi-dsi: Fix byte order of 16-bit DCS set/get brightness
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=c9d27c6be518b4ef2966d9564654ef99292ea1b3
[2/3] drm/panel: sofef00: Use 16-bit brightness function
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=fd40749a4f62a03d0aebe6eb446ea84a9901795a
[3/3] drm/panel: vtdr6130: Use 16-bit brightness function
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=9402cde9347eca050e14ea9e47270e84a6899162

--
Neil