[PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[Benjamin Berg]

On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT
mode is reported. Decode it correctly rather than warning about an
unexpected multi mode status to be reported.

Signed-off-by: Benjamin Berg <bberg@redhat.com>
Cc: Peter FP1 Zhang <zhangfp1@lenovo.com
Cc: Lyude <lyude@redhat.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 91fab1a13a6d..d23c84222e9f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
 			      TP_ACPI_MULTI_MODE_FLAT |
 			      TP_ACPI_MULTI_MODE_TABLET |
 			      TP_ACPI_MULTI_MODE_STAND |
 			      TP_ACPI_MULTI_MODE_TENT;
 		break;
 	case 3:
 		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
 			      TP_ACPI_MULTI_MODE_FLAT;
 		break;
 	case 4:
-		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
-			      TP_ACPI_MULTI_MODE_TABLET |
-			      TP_ACPI_MULTI_MODE_STAND |
-			      TP_ACPI_MULTI_MODE_TENT;
-		break;
 	case 5:
+		/* In mode 4, FLAT is not specified as a valid mode. However,
+		 * it can be seen at least on the X1 Yoga 2nd Generation.
+		 */
 		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
 			      TP_ACPI_MULTI_MODE_FLAT |
 			      TP_ACPI_MULTI_MODE_TABLET |
 			      TP_ACPI_MULTI_MODE_STAND |
 			      TP_ACPI_MULTI_MODE_TENT;
 		break;
 	default:
 		pr_err("Unknown multi mode status type %d with value 0x%04X, please report this to %s\n",
 		       type, value, TPACPI_MAIL);
 		return 0;
-- 
2.13.6

Re: [PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[Andy Shevchenko]

On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com> wrote:
> On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT
> mode is reported. Decode it correctly rather than warning about an
> unexpected multi mode status to be reported.

If we can't check on all available hardware, better to avoid "doing
for all" fixes.
I suppose DMI match can help here.

It would be nice to hear from Henrique and others as well.


>         case 4:
> -               valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> -                             TP_ACPI_MULTI_MODE_TABLET |
> -                             TP_ACPI_MULTI_MODE_STAND |
> -                             TP_ACPI_MULTI_MODE_TENT;
> -               break;

The common practice is to put
/* fallthrough */
instead.

>         case 5:

> +               /* In mode 4, FLAT is not specified as a valid mode. However,
> +                * it can be seen at least on the X1 Yoga 2nd Generation.
> +                */

We don't use network subsystem style of comments.

>                 valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
>                               TP_ACPI_MULTI_MODE_FLAT |
>                               TP_ACPI_MULTI_MODE_TABLET |
>                               TP_ACPI_MULTI_MODE_STAND |
>                               TP_ACPI_MULTI_MODE_TENT;
>                 break;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[Benjamin Berg]

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

On Tue, 2017-11-14 at 18:58 +0200, Andy Shevchenko wrote:
> On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com>
> wrote:
> > On the X1 Yoga 2nd Generation and most likely other notebooks the
> > FLAT
> > mode is reported. Decode it correctly rather than warning about an
> > unexpected multi mode status to be reported.
> 
> If we can't check on all available hardware, better to avoid "doing
> for all" fixes.
> I suppose DMI match can help here.

The documentation I saw suggests that the flat mode cannot be detected
by machines reporting a multi mode status of type 4. This is why the
original patch excluded the FLAT mode in the list of valid modes on
those devices.

I think it is safe to simply assume that all laptops reporting type 4
can actually detect the flat state. Doing so will not affect actual
state reporting on machines that may not be able to detect it. And I
don't think that the information whether the flat state can be detected
or not is interesting enough to bother with DMI matches.

Benjamin

> It would be nice to hear from Henrique and others as well.
> 
> 
> >         case 4:
> > -               valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> > -                             TP_ACPI_MULTI_MODE_TABLET |
> > -                             TP_ACPI_MULTI_MODE_STAND |
> > -                             TP_ACPI_MULTI_MODE_TENT;
> > -               break;
> 
> The common practice is to put
> /* fallthrough */
> instead.
> 
> >         case 5:
> > +               /* In mode 4, FLAT is not specified as a valid
> > mode. However,
> > +                * it can be seen at least on the X1 Yoga 2nd
> > Generation.
> > +                */
> 
> We don't use network subsystem style of comments.
> 
> >                 valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> >                               TP_ACPI_MULTI_MODE_FLAT |
> >                               TP_ACPI_MULTI_MODE_TABLET |
> >                               TP_ACPI_MULTI_MODE_STAND |
> >                               TP_ACPI_MULTI_MODE_TENT;
> >                 break;
> 
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[Lyude Paul]

lgtm.

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2017-11-14 at 17:14 +0100, Benjamin Berg wrote:
> On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT
> mode is reported. Decode it correctly rather than warning about an
> unexpected multi mode status to be reported.
> 
> Signed-off-by: Benjamin Berg <bberg@redhat.com>
> Cc: Peter FP1 Zhang <zhangfp1@lenovo.com
> Cc: Lyude <lyude@redhat.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index 91fab1a13a6d..d23c84222e9f 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int
> *has_tablet_mode)
>  			      TP_ACPI_MULTI_MODE_FLAT |
>  			      TP_ACPI_MULTI_MODE_TABLET |
>  			      TP_ACPI_MULTI_MODE_STAND |
>  			      TP_ACPI_MULTI_MODE_TENT;
>  		break;
>  	case 3:
>  		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
>  			      TP_ACPI_MULTI_MODE_FLAT;
>  		break;
>  	case 4:
> -		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> -			      TP_ACPI_MULTI_MODE_TABLET |
> -			      TP_ACPI_MULTI_MODE_STAND |
> -			      TP_ACPI_MULTI_MODE_TENT;
> -		break;
>  	case 5:
> +		/* In mode 4, FLAT is not specified as a valid mode.
> However,
> +		 * it can be seen at least on the X1 Yoga 2nd Generation.
> +		 */
>  		valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
>  			      TP_ACPI_MULTI_MODE_FLAT |
>  			      TP_ACPI_MULTI_MODE_TABLET |
>  			      TP_ACPI_MULTI_MODE_STAND |
>  			      TP_ACPI_MULTI_MODE_TENT;
>  		break;
>  	default:
>  		pr_err("Unknown multi mode status type %d with value
> 0x%04X, please report this to %s\n",
>  		       type, value, TPACPI_MAIL);
>  		return 0;

Re: [PATCH] platform/x86: thinkpad_acpi: Accept flat mode for type 4 multi mode status

[Andy Shevchenko]

On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com> wrote:
> On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT
> mode is reported. Decode it correctly rather than warning about an
> unexpected multi mode status to be reported.
>
> Signed-off-by: Benjamin Berg <bberg@redhat.com>
> Cc: Peter FP1 Zhang <zhangfp1@lenovo.com
> Cc: Lyude <lyude@redhat.com>

Applied to my review and testing queue, thanks!

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 91fab1a13a6d..d23c84222e9f 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
>                               TP_ACPI_MULTI_MODE_FLAT |
>                               TP_ACPI_MULTI_MODE_TABLET |
>                               TP_ACPI_MULTI_MODE_STAND |
>                               TP_ACPI_MULTI_MODE_TENT;
>                 break;
>         case 3:
>                 valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
>                               TP_ACPI_MULTI_MODE_FLAT;
>                 break;
>         case 4:
> -               valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> -                             TP_ACPI_MULTI_MODE_TABLET |
> -                             TP_ACPI_MULTI_MODE_STAND |
> -                             TP_ACPI_MULTI_MODE_TENT;
> -               break;
>         case 5:
> +               /* In mode 4, FLAT is not specified as a valid mode. However,
> +                * it can be seen at least on the X1 Yoga 2nd Generation.
> +                */
>                 valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
>                               TP_ACPI_MULTI_MODE_FLAT |
>                               TP_ACPI_MULTI_MODE_TABLET |
>                               TP_ACPI_MULTI_MODE_STAND |
>                               TP_ACPI_MULTI_MODE_TENT;
>                 break;
>         default:
>                 pr_err("Unknown multi mode status type %d with value 0x%04X, please report this to %s\n",
>                        type, value, TPACPI_MAIL);
>                 return 0;
> --
> 2.13.6
>



-- 
With Best Regards,
Andy Shevchenko