[PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

AceLan Kao
From: "Chia-Lin Kao (AceLan)" <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1865402

[Impact]
The get_scalar_status command was provided by ODM engineer and was newly
introduced to determinate whether scalar is responsible for the backlight
adjustment. That guy asked the scalar vendor to add this command and the
command works as expected. But there is already a command in the Dell AIO
scalar UART command spec. which can do the same thing since 2015, and the
newly introduced get_scalar_status command is undocumented. To prevent
from introducing regression in the future and to align with what Windows
uses, replace this command with get_display_mode command.

[Fix]
Replace get_scalar_status command with get_display_mode command.

[Test]
Verified on new Dell AIO platforms.

[Regression Potential]
Low, the command is already in the spec. since 2015.

Chia-Lin Kao (AceLan) (1):
  UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode
    command

 drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
 drivers/platform/x86/dell-uart-backlight.h |  7 +-
 2 files changed, 52 insertions(+), 46 deletions(-)

--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][unstable][OEM-5.10] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode command

AceLan Kao
From: "Chia-Lin Kao (AceLan)" <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1865402

ODM asks us to use get_display_mode command to confirm the scalar's
behavior, and Windows use this command, too.
To align the behavior with Windows, remove get_scalar_status command and
replace it with get_display_mode.

Signed-off-by: Chia-Lin Kao (AceLan) <[hidden email]>
---
 drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
 drivers/platform/x86/dell-uart-backlight.h |  7 +-
 2 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/dell-uart-backlight.c b/drivers/platform/x86/dell-uart-backlight.c
index 5bcda55a972a..4406fe678c81 100644
--- a/drivers/platform/x86/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell-uart-backlight.c
@@ -57,18 +57,6 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .cmd = {0x6A, 0x06, 0x8F},
  .tx_len = 3,
  },
- /*
- * Get Scalar Status: Tool uses this command to check if scalar IC controls brightness.
- * Command: 0x6A 0x1F 0x8F (Length:3 Type: 0x0A, Cmd:0x1F Checksum:0x76)
- * Return data: 0x04 0x1F Data checksum
- * (Data = 0: scalar cannot adjust brightness, Data = 1: scalar can adjust brightness)
- */
- [DELL_UART_GET_SCALAR] = {
- .cmd = {0x6A,0x1F,0x76},
- .ret = {0x04,0x1F,0x00,0x00},
- .tx_len = 3,
- .rx_len = 4,
- },
  /*
  * Get Brightness level: Application uses this command for scaler to
  *                       get brightness.
@@ -115,6 +103,21 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .tx_len = 4,
  .rx_len = 3,
  },
+ /*
+ * Get display mode: Application uses this command to get scaler
+ *     display mode.
+ * Command: 0x6A 0x10 0x85 (Length:3 Type: 0x0A, Cmd:0x10)
+ * Return data: 0x04 0x10 Data checksum
+ * (Length:4 Cmd:0x10 Data: mode checksum: SUM
+ * mode =0 if PC mode
+ * mode =1 if AV(HDMI) mode
+ */
+ [DELL_UART_GET_DISPLAY_MODE] = {
+ .cmd = {0x6A, 0x10, 0x85},
+ .ret = {0x04, 0x10, 0x00, 0x00},
+ .tx_len = 3,
+ .rx_len = 4,
+ },
 };
 
 static const struct dmi_system_id dell_uart_backlight_alpha_platform[] = {
@@ -313,37 +316,6 @@ static int dell_uart_update_status(struct backlight_device *bd)
  return 0;
 }
 
-static int dell_uart_get_scalar_status(struct dell_uart_backlight *dell_pdata)
-{
- struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_SCALAR];
- struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
- int rx_len;
- int status = 0, retry = 50;
-
- dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
-
- if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
- pr_debug("Failed to get mutex_lock");
- return 0;
- }
-
- dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
- do {
- rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
- if (rx_len == 0)
- msleep(100);
- } while (rx_len == 0 && --retry);
-
- mutex_unlock(&dell_pdata->brightness_mutex);
-
- dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
-
- if (rx_len == 4)
- status = (unsigned int)bl_cmd->ret[2];
-
- return status;
-}
-
 static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
 {
  struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_FIRMWARE_VER];
@@ -383,6 +355,36 @@ static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
  return rx_len;
 }
 
+static int dell_uart_get_display_mode(struct dell_uart_backlight *dell_pdata)
+{
+ struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_DISPLAY_MODE];
+ struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
+ int rx_len;
+ int status = 0, retry = 10;
+
+ do {
+ dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
+
+ if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
+ pr_debug("Failed to get mutex_lock");
+ return 0;
+ }
+
+ dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
+ rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
+
+ mutex_unlock(&dell_pdata->brightness_mutex);
+
+ dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
+ mdelay(1000);
+ } while (rx_len == 0 && --retry);
+
+ if (rx_len == 4)
+ status = ((unsigned int)bl_cmd->ret[2] == PC_MODE);
+
+ return status;
+}
+
 static const struct backlight_ops dell_uart_backlight_ops = {
  .get_brightness = dell_uart_get_brightness,
  .update_status = dell_uart_update_status,
@@ -430,13 +432,12 @@ static int dell_uart_bl_add(struct acpi_device *dev)
  return -ENODEV;
  }
  }
- else if (!dell_uart_get_scalar_status(dell_pdata)) {
+ else if (!dell_uart_get_display_mode(dell_pdata)) {
  pr_debug("Scalar is not in charge of brightness adjustment.\n");
  kfree_sensitive(dell_pdata);
  return -ENODEV;
  }
  }
- dell_uart_show_firmware_ver(dell_pdata);
 
  memset(&props, 0, sizeof(struct backlight_properties));
  props.type = BACKLIGHT_PLATFORM;
diff --git a/drivers/platform/x86/dell-uart-backlight.h b/drivers/platform/x86/dell-uart-backlight.h
index e4fe74fae6a8..5c732d362d4b 100644
--- a/drivers/platform/x86/dell-uart-backlight.h
+++ b/drivers/platform/x86/dell-uart-backlight.h
@@ -20,10 +20,15 @@
 
 enum {
  DELL_UART_GET_FIRMWARE_VER,
- DELL_UART_GET_SCALAR,
  DELL_UART_GET_BRIGHTNESS,
  DELL_UART_SET_BRIGHTNESS,
  DELL_UART_SET_BACKLIGHT_POWER,
+ DELL_UART_GET_DISPLAY_MODE,
+};
+
+enum {
+ PC_MODE,
+ AV_MODE,
 };
 
 struct dell_uart_bl_cmd {
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][F] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode command

AceLan Kao
In reply to this post by AceLan Kao
From: "Chia-Lin Kao (AceLan)" <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1865402

ODM asks us to use get_display_mode command to confirm the scalar's
behavior, and Windows use this command, too.
To align the behavior with Windows, remove get_scalar_status command and
replace it with get_display_mode.

Signed-off-by: Chia-Lin Kao (AceLan) <[hidden email]>
---
 drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
 drivers/platform/x86/dell-uart-backlight.h |  7 +-
 2 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/dell-uart-backlight.c b/drivers/platform/x86/dell-uart-backlight.c
index bddc4f228bf9..aa08974900ea 100644
--- a/drivers/platform/x86/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell-uart-backlight.c
@@ -57,18 +57,6 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .cmd = {0x6A, 0x06, 0x8F},
  .tx_len = 3,
  },
- /*
- * Get Scalar Status: Tool uses this command to check if scalar IC controls brightness.
- * Command: 0x6A 0x1F 0x8F (Length:3 Type: 0x0A, Cmd:0x1F Checksum:0x76)
- * Return data: 0x04 0x1F Data checksum
- * (Data = 0: scalar cannot adjust brightness, Data = 1: scalar can adjust brightness)
- */
- [DELL_UART_GET_SCALAR] = {
- .cmd = {0x6A,0x1F,0x76},
- .ret = {0x04,0x1F,0x00,0x00},
- .tx_len = 3,
- .rx_len = 4,
- },
  /*
  * Get Brightness level: Application uses this command for scaler to
  *                       get brightness.
@@ -115,6 +103,21 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .tx_len = 4,
  .rx_len = 3,
  },
+ /*
+ * Get display mode: Application uses this command to get scaler
+ *     display mode.
+ * Command: 0x6A 0x10 0x85 (Length:3 Type: 0x0A, Cmd:0x10)
+ * Return data: 0x04 0x10 Data checksum
+ * (Length:4 Cmd:0x10 Data: mode checksum: SUM
+ * mode =0 if PC mode
+ * mode =1 if AV(HDMI) mode
+ */
+ [DELL_UART_GET_DISPLAY_MODE] = {
+ .cmd = {0x6A, 0x10, 0x85},
+ .ret = {0x04, 0x10, 0x00, 0x00},
+ .tx_len = 3,
+ .rx_len = 4,
+ },
 };
 
 static const struct dmi_system_id dell_uart_backlight_alpha_platform[] __initconst = {
@@ -313,37 +316,6 @@ static int dell_uart_update_status(struct backlight_device *bd)
  return 0;
 }
 
-static int dell_uart_get_scalar_status(struct dell_uart_backlight *dell_pdata)
-{
- struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_SCALAR];
- struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
- int rx_len;
- int status = 0, retry = 50;
-
- dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
-
- if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
- pr_debug("Failed to get mutex_lock");
- return 0;
- }
-
- dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
- do {
- rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
- if (rx_len == 0)
- msleep(100);
- } while (rx_len == 0 && --retry);
-
- mutex_unlock(&dell_pdata->brightness_mutex);
-
- dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
-
- if (rx_len == 4)
- status = (unsigned int)bl_cmd->ret[2];
-
- return status;
-}
-
 static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
 {
  struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_FIRMWARE_VER];
@@ -383,6 +355,36 @@ static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
  return rx_len;
 }
 
+static int dell_uart_get_display_mode(struct dell_uart_backlight *dell_pdata)
+{
+ struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_DISPLAY_MODE];
+ struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
+ int rx_len;
+ int status = 0, retry = 10;
+
+ do {
+ dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
+
+ if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
+ pr_debug("Failed to get mutex_lock");
+ return 0;
+ }
+
+ dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
+ rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
+
+ mutex_unlock(&dell_pdata->brightness_mutex);
+
+ dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
+ mdelay(1000);
+ } while (rx_len == 0 && --retry);
+
+ if (rx_len == 4)
+ status = ((unsigned int)bl_cmd->ret[2] == PC_MODE);
+
+ return status;
+}
+
 static const struct backlight_ops dell_uart_backlight_ops = {
  .get_brightness = dell_uart_get_brightness,
  .update_status = dell_uart_update_status,
@@ -430,13 +432,12 @@ static int dell_uart_bl_add(struct acpi_device *dev)
  return -ENODEV;
  }
  }
- else if (!dell_uart_get_scalar_status(dell_pdata)) {
+ else if (!dell_uart_get_display_mode(dell_pdata)) {
  pr_debug("Scalar is not in charge of brightness adjustment.\n");
  kzfree(dell_pdata);
  return -ENODEV;
  }
  }
- dell_uart_show_firmware_ver(dell_pdata);
 
  memset(&props, 0, sizeof(struct backlight_properties));
  props.type = BACKLIGHT_PLATFORM;
diff --git a/drivers/platform/x86/dell-uart-backlight.h b/drivers/platform/x86/dell-uart-backlight.h
index e4fe74fae6a8..5c732d362d4b 100644
--- a/drivers/platform/x86/dell-uart-backlight.h
+++ b/drivers/platform/x86/dell-uart-backlight.h
@@ -20,10 +20,15 @@
 
 enum {
  DELL_UART_GET_FIRMWARE_VER,
- DELL_UART_GET_SCALAR,
  DELL_UART_GET_BRIGHTNESS,
  DELL_UART_SET_BRIGHTNESS,
  DELL_UART_SET_BACKLIGHT_POWER,
+ DELL_UART_GET_DISPLAY_MODE,
+};
+
+enum {
+ PC_MODE,
+ AV_MODE,
 };
 
 struct dell_uart_bl_cmd {
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][G] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode command

AceLan Kao
In reply to this post by AceLan Kao
From: "Chia-Lin Kao (AceLan)" <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1865402

ODM asks us to use get_display_mode command to confirm the scalar's
behavior, and Windows use this command, too.
To align the behavior with Windows, remove get_scalar_status command and
replace it with get_display_mode.

Signed-off-by: Chia-Lin Kao (AceLan) <[hidden email]>
---
 drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
 drivers/platform/x86/dell-uart-backlight.h |  7 +-
 2 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/dell-uart-backlight.c b/drivers/platform/x86/dell-uart-backlight.c
index beb6618f13b8..f16bb9b52a02 100644
--- a/drivers/platform/x86/dell-uart-backlight.c
+++ b/drivers/platform/x86/dell-uart-backlight.c
@@ -57,18 +57,6 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .cmd = {0x6A, 0x06, 0x8F},
  .tx_len = 3,
  },
- /*
- * Get Scalar Status: Tool uses this command to check if scalar IC controls brightness.
- * Command: 0x6A 0x1F 0x8F (Length:3 Type: 0x0A, Cmd:0x1F Checksum:0x76)
- * Return data: 0x04 0x1F Data checksum
- * (Data = 0: scalar cannot adjust brightness, Data = 1: scalar can adjust brightness)
- */
- [DELL_UART_GET_SCALAR] = {
- .cmd = {0x6A,0x1F,0x76},
- .ret = {0x04,0x1F,0x00,0x00},
- .tx_len = 3,
- .rx_len = 4,
- },
  /*
  * Get Brightness level: Application uses this command for scaler to
  *                       get brightness.
@@ -115,6 +103,21 @@ static struct dell_uart_bl_cmd uart_cmd[] = {
  .tx_len = 4,
  .rx_len = 3,
  },
+ /*
+ * Get display mode: Application uses this command to get scaler
+ *     display mode.
+ * Command: 0x6A 0x10 0x85 (Length:3 Type: 0x0A, Cmd:0x10)
+ * Return data: 0x04 0x10 Data checksum
+ * (Length:4 Cmd:0x10 Data: mode checksum: SUM
+ * mode =0 if PC mode
+ * mode =1 if AV(HDMI) mode
+ */
+ [DELL_UART_GET_DISPLAY_MODE] = {
+ .cmd = {0x6A, 0x10, 0x85},
+ .ret = {0x04, 0x10, 0x00, 0x00},
+ .tx_len = 3,
+ .rx_len = 4,
+ },
 };
 
 static const struct dmi_system_id dell_uart_backlight_alpha_platform[] = {
@@ -313,37 +316,6 @@ static int dell_uart_update_status(struct backlight_device *bd)
  return 0;
 }
 
-static int dell_uart_get_scalar_status(struct dell_uart_backlight *dell_pdata)
-{
- struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_SCALAR];
- struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
- int rx_len;
- int status = 0, retry = 50;
-
- dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
-
- if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
- pr_debug("Failed to get mutex_lock");
- return 0;
- }
-
- dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
- do {
- rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
- if (rx_len == 0)
- msleep(100);
- } while (rx_len == 0 && --retry);
-
- mutex_unlock(&dell_pdata->brightness_mutex);
-
- dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
-
- if (rx_len == 4)
- status = (unsigned int)bl_cmd->ret[2];
-
- return status;
-}
-
 static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
 {
  struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_FIRMWARE_VER];
@@ -383,6 +355,36 @@ static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
  return rx_len;
 }
 
+static int dell_uart_get_display_mode(struct dell_uart_backlight *dell_pdata)
+{
+ struct dell_uart_bl_cmd *bl_cmd = &uart_cmd[DELL_UART_GET_DISPLAY_MODE];
+ struct uart_8250_port *uart = serial8250_get_port(dell_pdata->line);
+ int rx_len;
+ int status = 0, retry = 10;
+
+ do {
+ dell_uart_dump_cmd(__func__, "tx: ", bl_cmd->cmd, bl_cmd->tx_len);
+
+ if (mutex_lock_killable(&dell_pdata->brightness_mutex) < 0) {
+ pr_debug("Failed to get mutex_lock");
+ return 0;
+ }
+
+ dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
+ rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
+
+ mutex_unlock(&dell_pdata->brightness_mutex);
+
+ dell_uart_dump_cmd(__func__, "rx: ", bl_cmd->ret, rx_len);
+ mdelay(1000);
+ } while (rx_len == 0 && --retry);
+
+ if (rx_len == 4)
+ status = ((unsigned int)bl_cmd->ret[2] == PC_MODE);
+
+ return status;
+}
+
 static const struct backlight_ops dell_uart_backlight_ops = {
  .get_brightness = dell_uart_get_brightness,
  .update_status = dell_uart_update_status,
@@ -430,13 +432,12 @@ static int dell_uart_bl_add(struct acpi_device *dev)
  return -ENODEV;
  }
  }
- else if (!dell_uart_get_scalar_status(dell_pdata)) {
+ else if (!dell_uart_get_display_mode(dell_pdata)) {
  pr_debug("Scalar is not in charge of brightness adjustment.\n");
  kzfree(dell_pdata);
  return -ENODEV;
  }
  }
- dell_uart_show_firmware_ver(dell_pdata);
 
  memset(&props, 0, sizeof(struct backlight_properties));
  props.type = BACKLIGHT_PLATFORM;
diff --git a/drivers/platform/x86/dell-uart-backlight.h b/drivers/platform/x86/dell-uart-backlight.h
index e4fe74fae6a8..5c732d362d4b 100644
--- a/drivers/platform/x86/dell-uart-backlight.h
+++ b/drivers/platform/x86/dell-uart-backlight.h
@@ -20,10 +20,15 @@
 
 enum {
  DELL_UART_GET_FIRMWARE_VER,
- DELL_UART_GET_SCALAR,
  DELL_UART_GET_BRIGHTNESS,
  DELL_UART_SET_BRIGHTNESS,
  DELL_UART_SET_BACKLIGHT_POWER,
+ DELL_UART_GET_DISPLAY_MODE,
+};
+
+enum {
+ PC_MODE,
+ AV_MODE,
 };
 
 struct dell_uart_bl_cmd {
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED [OEM-5.10] Re: [PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

Timo Aaltonen-6
In reply to this post by AceLan Kao
On 17.12.2020 10.08, AceLan Kao wrote:

> From: "Chia-Lin Kao (AceLan)" <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1865402
>
> [Impact]
> The get_scalar_status command was provided by ODM engineer and was newly
> introduced to determinate whether scalar is responsible for the backlight
> adjustment. That guy asked the scalar vendor to add this command and the
> command works as expected. But there is already a command in the Dell AIO
> scalar UART command spec. which can do the same thing since 2015, and the
> newly introduced get_scalar_status command is undocumented. To prevent
> from introducing regression in the future and to align with what Windows
> uses, replace this command with get_display_mode command.
>
> [Fix]
> Replace get_scalar_status command with get_display_mode command.
>
> [Test]
> Verified on new Dell AIO platforms.
>
> [Regression Potential]
> Low, the command is already in the spec. since 2015.
>
> Chia-Lin Kao (AceLan) (1):
>    UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode
>      command
>
>   drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
>   drivers/platform/x86/dell-uart-backlight.h |  7 +-
>   2 files changed, 52 insertions(+), 46 deletions(-)
>

applied to oem-5.10, thanks

--
t

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

Stefan Bader-2
In reply to this post by AceLan Kao
On 17.12.20 09:08, AceLan Kao wrote:

> From: "Chia-Lin Kao (AceLan)" <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1865402
>
> [Impact]
> The get_scalar_status command was provided by ODM engineer and was newly
> introduced to determinate whether scalar is responsible for the backlight
> adjustment. That guy asked the scalar vendor to add this command and the
> command works as expected. But there is already a command in the Dell AIO
> scalar UART command spec. which can do the same thing since 2015, and the
> newly introduced get_scalar_status command is undocumented. To prevent
> from introducing regression in the future and to align with what Windows
> uses, replace this command with get_display_mode command.
>
> [Fix]
> Replace get_scalar_status command with get_display_mode command.
>
> [Test]
> Verified on new Dell AIO platforms.
>
> [Regression Potential]
> Low, the command is already in the spec. since 2015.
That does not necessarily mean it works. I am probably rather hesitant here
because I got burned a very long time ago when trying to fix some ACPI backlight
issue. This started to become a nightmare ping-ponging between fixing new
hardware while breaking older on in turn. Or the other way round.
So, as simple as it looks, imo, this should be upstream with some settle time
before going into a stable kernel.

-Stefan
>
> Chia-Lin Kao (AceLan) (1):
>   UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode
>     command
>
>  drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
>  drivers/platform/x86/dell-uart-backlight.h |  7 +-
>  2 files changed, 52 insertions(+), 46 deletions(-)
>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

AceLan Kao
Yes, we are planning to upstream this driver, but it's required to
re-write the driver with serdev.
I got some troubles while re-writing it and then got distracted by
other bugs, so the plan has been suspended for quite a long time.

Currently we're enabling new AIOs, and require this updated function.
At least this fix is needed in the OEM-5.10 kernel for shipment, but
it would be better that 5.4 kernel could include this fix, too.
I'll resume the upstream plan as soon as possible.

Stefan Bader <[hidden email]> 於 2021年1月15日 週五 下午4:45寫道:

>
> On 17.12.20 09:08, AceLan Kao wrote:
> > From: "Chia-Lin Kao (AceLan)" <[hidden email]>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1865402
> >
> > [Impact]
> > The get_scalar_status command was provided by ODM engineer and was newly
> > introduced to determinate whether scalar is responsible for the backlight
> > adjustment. That guy asked the scalar vendor to add this command and the
> > command works as expected. But there is already a command in the Dell AIO
> > scalar UART command spec. which can do the same thing since 2015, and the
> > newly introduced get_scalar_status command is undocumented. To prevent
> > from introducing regression in the future and to align with what Windows
> > uses, replace this command with get_display_mode command.
> >
> > [Fix]
> > Replace get_scalar_status command with get_display_mode command.
> >
> > [Test]
> > Verified on new Dell AIO platforms.
> >
> > [Regression Potential]
> > Low, the command is already in the spec. since 2015.
>
> That does not necessarily mean it works. I am probably rather hesitant here
> because I got burned a very long time ago when trying to fix some ACPI backlight
> issue. This started to become a nightmare ping-ponging between fixing new
> hardware while breaking older on in turn. Or the other way round.
> So, as simple as it looks, imo, this should be upstream with some settle time
> before going into a stable kernel.
>
> -Stefan
> >
> > Chia-Lin Kao (AceLan) (1):
> >   UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode
> >     command
> >
> >  drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
> >  drivers/platform/x86/dell-uart-backlight.h |  7 +-
> >  2 files changed, 52 insertions(+), 46 deletions(-)
> >
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED (unstable/5.11): [PATCH 1/1][SRU][unstable][OEM-5.10] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode command

Andrea Righi
In reply to this post by AceLan Kao
On Thu, Dec 17, 2020 at 04:08:58PM +0800, AceLan Kao wrote:

> From: "Chia-Lin Kao (AceLan)" <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1865402
>
> ODM asks us to use get_display_mode command to confirm the scalar's
> behavior, and Windows use this command, too.
> To align the behavior with Windows, remove get_scalar_status command and
> replace it with get_display_mode.
>
> Signed-off-by: Chia-Lin Kao (AceLan) <[hidden email]>

Applied to unstable/5.11.

Thanks,
-Andrea

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK[F/G]: [PATCH 0/1][SRU][F][G][unstable][OEM-5.10] Stop using get_scalar_status command in Dell AIO uart backlight driver Edit

Stefan Bader-2
In reply to this post by AceLan Kao
On 17.12.20 09:08, AceLan Kao wrote:

> From: "Chia-Lin Kao (AceLan)" <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1865402
>
> [Impact]
> The get_scalar_status command was provided by ODM engineer and was newly
> introduced to determinate whether scalar is responsible for the backlight
> adjustment. That guy asked the scalar vendor to add this command and the
> command works as expected. But there is already a command in the Dell AIO
> scalar UART command spec. which can do the same thing since 2015, and the
> newly introduced get_scalar_status command is undocumented. To prevent
> from introducing regression in the future and to align with what Windows
> uses, replace this command with get_display_mode command.
>
> [Fix]
> Replace get_scalar_status command with get_display_mode command.
>
> [Test]
> Verified on new Dell AIO platforms.
>
> [Regression Potential]
> Low, the command is already in the spec. since 2015.
>
> Chia-Lin Kao (AceLan) (1):
>    UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode
>      command
>
>   drivers/platform/x86/dell-uart-backlight.c | 91 +++++++++++-----------
>   drivers/platform/x86/dell-uart-backlight.h |  7 +-
>   2 files changed, 52 insertions(+), 46 deletions(-)
>
This is still not even in linux-next. As such not really appears to be SRU material.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

OpenPGP_signature (849 bytes) Download Attachment