[PATCH 1/1][SRU][B][E][F][OEM-OSP1-B] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add get_display_mode command
Thadeu Lima de Souza Cascardo
cascardo at canonical.com
Fri Mar 6 14:34:22 UTC 2020
This doesn't apply in bionic.
Also, see some inline comments.
On Wed, Mar 04, 2020 at 02:18:16PM +0800, AceLan Kao wrote:
> 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: AceLan Kao <acelan.kao at canonical.com>
> ---
> drivers/platform/x86/dell-uart-backlight.c | 90 +++++++++++-----------
> drivers/platform/x86/dell-uart-backlight.h | 7 +-
> 2 files changed, 52 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-uart-backlight.c b/drivers/platform/x86/dell-uart-backlight.c
> index 9b2b219f3b8f..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,36 +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;
> -
> - 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);
> - msleep(100);
> - 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);
> - } while (rx_len == 0 && --retry);
> -
> - 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];
> @@ -382,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);
Above, a msleep(100) was being used after the write, before the read. Isn't
this necessary for this command?
And now it's a mdelay(1000). One second doing some busy wait between retries.
Can it really take like 10s before a response? If 1s is okay, an msleep(100)
would be better here. And comparing this to bionic's version (well, really the
other command), you only retry reads. Is retrying writes followed by reads
okay? Or doesn't it make it more prone to failures?
Cascardo.
> + } 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,
> @@ -429,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.17.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list