2023-03-04 19:46:44

by Maximilian Luz

[permalink] [raw]
Subject: [PATCH 0/3] platform/surface: Add support for tablet-mode switch on Surface Pro 9

This series adds support for the tablet-mode switch on the Surface Pro
9. On that device, the posture subsystem (POS) can be used to query the
state of the Type-Cover (detached, laptop-mode, flipped-back, ...) and
receive notifications when it changes. We can use this to implement a
tablet mode switch, extending the driver already in use on the Surface
Laptop Studio.

More specifically, the posture subsystem allows for different posture
sources, identified by a numerical ID, each of which can have different
states. At the moment, however, the tablet-mode switch driver using the
POS subsystem only supports the Surface Laptop Studio, and support for
that is hard-coded.

To support the Surface Pro 9, we therefore need to adapt the driver to
properly disambiguate between different posture sources (patch 1) and
then implement the respective functions for the source ID representing
the Type-Cover (patch 2). Finally, we also need to add the respective
device to the Surface Pro 9 entry in the aggregator registry, to allow
the driver to load (patch 3).

Maximilian Luz (3):
platform/surface: aggregator_tabletsw: Properly handle different
posture source IDs
platform/surface: aggregator_tabletsw: Add support for Type-Cover
posture source
platform/surface: aggregator_registry: Add support for tablet-mode
switch on Surface Pro 9

.../surface/surface_aggregator_registry.c | 2 +-
.../surface/surface_aggregator_tabletsw.c | 180 ++++++++++++++----
2 files changed, 142 insertions(+), 40 deletions(-)

--
2.39.2



2023-03-04 19:46:47

by Maximilian Luz

[permalink] [raw]
Subject: [PATCH 1/3] platform/surface: aggregator_tabletsw: Properly handle different posture source IDs

The device posture subsystem (POS) can provide different posture
sources. Different sources can provide different posture states and
sources can be identified by their ID.

For example, screen posture of the Surface Laptop Studio (SLS), which is
currently the only supported source, uses a source ID of 0x03. The
Surface Pro 9 uses the same subsystem for its Type-Cover, however,
provides different states for that under the ID 0x00.

To eventually support the Surface Pro 9 and potential future devices, we
need to properly disambiguate between source IDs. Therefore, add the
source ID to the state we carry and determine the tablet-mode state (as
well as state names) based on that.

Signed-off-by: Maximilian Luz <[email protected]>
---
.../surface/surface_aggregator_tabletsw.c | 123 ++++++++++++------
1 file changed, 84 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index 9fed800c7cc0..e8682f52558f 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -20,16 +20,23 @@

struct ssam_tablet_sw;

+struct ssam_tablet_sw_state {
+ u32 source;
+ u32 state;
+};
+
struct ssam_tablet_sw_ops {
- int (*get_state)(struct ssam_tablet_sw *sw, u32 *state);
- const char *(*state_name)(struct ssam_tablet_sw *sw, u32 state);
- bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw, u32 state);
+ int (*get_state)(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state);
+ const char *(*state_name)(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state);
+ bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state);
};

struct ssam_tablet_sw {
struct ssam_device *sdev;

- u32 state;
+ struct ssam_tablet_sw_state state;
struct work_struct update_work;
struct input_dev *mode_switch;

@@ -45,9 +52,11 @@ struct ssam_tablet_sw_desc {

struct {
u32 (*notify)(struct ssam_event_notifier *nf, const struct ssam_event *event);
- int (*get_state)(struct ssam_tablet_sw *sw, u32 *state);
- const char *(*state_name)(struct ssam_tablet_sw *sw, u32 state);
- bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw, u32 state);
+ int (*get_state)(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state);
+ const char *(*state_name)(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state);
+ bool (*state_is_tablet_mode)(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state);
} ops;

struct {
@@ -61,7 +70,7 @@ struct ssam_tablet_sw_desc {
static ssize_t state_show(struct device *dev, struct device_attribute *attr, char *buf)
{
struct ssam_tablet_sw *sw = dev_get_drvdata(dev);
- const char *state = sw->ops.state_name(sw, sw->state);
+ const char *state = sw->ops.state_name(sw, &sw->state);

return sysfs_emit(buf, "%s\n", state);
}
@@ -79,19 +88,19 @@ static const struct attribute_group ssam_tablet_sw_group = {
static void ssam_tablet_sw_update_workfn(struct work_struct *work)
{
struct ssam_tablet_sw *sw = container_of(work, struct ssam_tablet_sw, update_work);
+ struct ssam_tablet_sw_state state;
int tablet, status;
- u32 state;

status = sw->ops.get_state(sw, &state);
if (status)
return;

- if (sw->state == state)
+ if (sw->state.source == state.source && sw->state.state == state.state)
return;
sw->state = state;

/* Send SW_TABLET_MODE event. */
- tablet = sw->ops.state_is_tablet_mode(sw, state);
+ tablet = sw->ops.state_is_tablet_mode(sw, &state);
input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);
input_sync(sw->mode_switch);
}
@@ -146,7 +155,7 @@ static int ssam_tablet_sw_probe(struct ssam_device *sdev)
sw->mode_switch->id.bustype = BUS_HOST;
sw->mode_switch->dev.parent = &sdev->dev;

- tablet = sw->ops.state_is_tablet_mode(sw, sw->state);
+ tablet = sw->ops.state_is_tablet_mode(sw, &sw->state);
input_set_capability(sw->mode_switch, EV_SW, SW_TABLET_MODE);
input_report_switch(sw->mode_switch, SW_TABLET_MODE, tablet);

@@ -203,9 +212,10 @@ enum ssam_kip_cover_state {
SSAM_KIP_COVER_STATE_FOLDED_BACK = 0x05,
};

-static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state)
{
- switch (state) {
+ switch (state->state) {
case SSAM_KIP_COVER_STATE_DISCONNECTED:
return "disconnected";

@@ -222,14 +232,15 @@ static const char *ssam_kip_cover_state_name(struct ssam_tablet_sw *sw, u32 stat
return "folded-back";

default:
- dev_warn(&sw->sdev->dev, "unknown KIP cover state: %u\n", state);
+ dev_warn(&sw->sdev->dev, "unknown KIP cover state: %u\n", state->state);
return "<unknown>";
}
}

-static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 state)
+static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state)
{
- switch (state) {
+ switch (state->state) {
case SSAM_KIP_COVER_STATE_DISCONNECTED:
case SSAM_KIP_COVER_STATE_FOLDED_CANVAS:
case SSAM_KIP_COVER_STATE_FOLDED_BACK:
@@ -240,7 +251,7 @@ static bool ssam_kip_cover_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 s
return false;

default:
- dev_warn(&sw->sdev->dev, "unknown KIP cover state: %d\n", sw->state);
+ dev_warn(&sw->sdev->dev, "unknown KIP cover state: %d\n", state->state);
return true;
}
}
@@ -252,7 +263,7 @@ SSAM_DEFINE_SYNC_REQUEST_R(__ssam_kip_get_cover_state, u8, {
.instance_id = 0x00,
});

-static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, u32 *state)
+static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state)
{
int status;
u8 raw;
@@ -263,7 +274,8 @@ static int ssam_kip_get_cover_state(struct ssam_tablet_sw *sw, u32 *state)
return status;
}

- *state = raw;
+ state->source = 0; /* Unused for KIP switch. */
+ state->state = raw;
return 0;
}

@@ -312,11 +324,15 @@ MODULE_PARM_DESC(tablet_mode_in_slate_state, "Enable tablet mode in slate device
#define SSAM_EVENT_POS_CID_POSTURE_CHANGED 0x03
#define SSAM_POS_MAX_SOURCES 4

-enum ssam_pos_state {
- SSAM_POS_POSTURE_LID_CLOSED = 0x00,
- SSAM_POS_POSTURE_LAPTOP = 0x01,
- SSAM_POS_POSTURE_SLATE = 0x02,
- SSAM_POS_POSTURE_TABLET = 0x03,
+enum ssam_pos_source_id {
+ SSAM_POS_SOURCE_SLS = 0x03,
+};
+
+enum ssam_pos_state_sls {
+ SSAM_POS_SLS_LID_CLOSED = 0x00,
+ SSAM_POS_SLS_LAPTOP = 0x01,
+ SSAM_POS_SLS_SLATE = 0x02,
+ SSAM_POS_SLS_TABLET = 0x03,
};

struct ssam_sources_list {
@@ -324,42 +340,68 @@ struct ssam_sources_list {
__le32 id[SSAM_POS_MAX_SOURCES];
} __packed;

-static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_pos_state_name_sls(struct ssam_tablet_sw *sw, u32 state)
{
switch (state) {
- case SSAM_POS_POSTURE_LID_CLOSED:
+ case SSAM_POS_SLS_LID_CLOSED:
return "closed";

- case SSAM_POS_POSTURE_LAPTOP:
+ case SSAM_POS_SLS_LAPTOP:
return "laptop";

- case SSAM_POS_POSTURE_SLATE:
+ case SSAM_POS_SLS_SLATE:
return "slate";

- case SSAM_POS_POSTURE_TABLET:
+ case SSAM_POS_SLS_TABLET:
return "tablet";

default:
- dev_warn(&sw->sdev->dev, "unknown device posture: %u\n", state);
+ dev_warn(&sw->sdev->dev, "unknown device posture for SLS: %u\n", state);
return "<unknown>";
}
}

-static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw, u32 state)
+static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state)
+{
+ switch (state->source) {
+ case SSAM_POS_SOURCE_SLS:
+ return ssam_pos_state_name_sls(sw, state->state);
+
+ default:
+ dev_warn(&sw->sdev->dev, "unknown device posture source: %u\n", state->source);
+ return "<unknown>";
+ }
+}
+
+static bool ssam_pos_state_is_tablet_mode_sls(struct ssam_tablet_sw *sw, u32 state)
{
switch (state) {
- case SSAM_POS_POSTURE_LAPTOP:
- case SSAM_POS_POSTURE_LID_CLOSED:
+ case SSAM_POS_SLS_LAPTOP:
+ case SSAM_POS_SLS_LID_CLOSED:
return false;

- case SSAM_POS_POSTURE_SLATE:
+ case SSAM_POS_SLS_SLATE:
return tablet_mode_in_slate_state;

- case SSAM_POS_POSTURE_TABLET:
+ case SSAM_POS_SLS_TABLET:
return true;

default:
- dev_warn(&sw->sdev->dev, "unknown device posture: %u\n", state);
+ dev_warn(&sw->sdev->dev, "unknown device posture for SLS: %u\n", state);
+ return true;
+ }
+}
+
+static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw,
+ const struct ssam_tablet_sw_state *state)
+{
+ switch (state->source) {
+ case SSAM_POS_SOURCE_SLS:
+ return ssam_pos_state_is_tablet_mode_sls(sw, state->state);
+
+ default:
+ dev_warn(&sw->sdev->dev, "unknown device posture source: %u\n", state->source);
return true;
}
}
@@ -450,9 +492,10 @@ static int ssam_pos_get_posture_for_source(struct ssam_tablet_sw *sw, u32 source
return 0;
}

-static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, u32 *state)
+static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, struct ssam_tablet_sw_state *state)
{
u32 source_id;
+ u32 source_state;
int status;

status = ssam_pos_get_source(sw, &source_id);
@@ -461,13 +504,15 @@ static int ssam_pos_get_posture(struct ssam_tablet_sw *sw, u32 *state)
return status;
}

- status = ssam_pos_get_posture_for_source(sw, source_id, state);
+ status = ssam_pos_get_posture_for_source(sw, source_id, &source_state);
if (status) {
dev_err(&sw->sdev->dev, "failed to get posture value for source %u: %d\n",
source_id, status);
return status;
}

+ state->source = source_id;
+ state->state = source_state;
return 0;
}

--
2.39.2


2023-03-04 19:46:50

by Maximilian Luz

[permalink] [raw]
Subject: [PATCH 2/3] platform/surface: aggregator_tabletsw: Add support for Type-Cover posture source

Implement support for the Type-Cover posture source (ID 0x00), found on
the Surface Pro 9.

Signed-off-by: Maximilian Luz <[email protected]>
---
.../surface/surface_aggregator_tabletsw.c | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)

diff --git a/drivers/platform/surface/surface_aggregator_tabletsw.c b/drivers/platform/surface/surface_aggregator_tabletsw.c
index e8682f52558f..8f52b62d1c19 100644
--- a/drivers/platform/surface/surface_aggregator_tabletsw.c
+++ b/drivers/platform/surface/surface_aggregator_tabletsw.c
@@ -325,9 +325,18 @@ MODULE_PARM_DESC(tablet_mode_in_slate_state, "Enable tablet mode in slate device
#define SSAM_POS_MAX_SOURCES 4

enum ssam_pos_source_id {
+ SSAM_POS_SOURCE_COVER = 0x00,
SSAM_POS_SOURCE_SLS = 0x03,
};

+enum ssam_pos_state_cover {
+ SSAM_POS_COVER_DISCONNECTED = 0x01,
+ SSAM_POS_COVER_CLOSED = 0x02,
+ SSAM_POS_COVER_LAPTOP = 0x03,
+ SSAM_POS_COVER_FOLDED_CANVAS = 0x04,
+ SSAM_POS_COVER_FOLDED_BACK = 0x05,
+};
+
enum ssam_pos_state_sls {
SSAM_POS_SLS_LID_CLOSED = 0x00,
SSAM_POS_SLS_LAPTOP = 0x01,
@@ -340,6 +349,30 @@ struct ssam_sources_list {
__le32 id[SSAM_POS_MAX_SOURCES];
} __packed;

+static const char *ssam_pos_state_name_cover(struct ssam_tablet_sw *sw, u32 state)
+{
+ switch (state) {
+ case SSAM_POS_COVER_DISCONNECTED:
+ return "disconnected";
+
+ case SSAM_POS_COVER_CLOSED:
+ return "closed";
+
+ case SSAM_POS_COVER_LAPTOP:
+ return "laptop";
+
+ case SSAM_POS_COVER_FOLDED_CANVAS:
+ return "folded-canvas";
+
+ case SSAM_POS_COVER_FOLDED_BACK:
+ return "folded-back";
+
+ default:
+ dev_warn(&sw->sdev->dev, "unknown device posture for type-cover: %u\n", state);
+ return "<unknown>";
+ }
+}
+
static const char *ssam_pos_state_name_sls(struct ssam_tablet_sw *sw, u32 state)
{
switch (state) {
@@ -365,6 +398,9 @@ static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
const struct ssam_tablet_sw_state *state)
{
switch (state->source) {
+ case SSAM_POS_SOURCE_COVER:
+ return ssam_pos_state_name_cover(sw, state->state);
+
case SSAM_POS_SOURCE_SLS:
return ssam_pos_state_name_sls(sw, state->state);

@@ -374,6 +410,24 @@ static const char *ssam_pos_state_name(struct ssam_tablet_sw *sw,
}
}

+static bool ssam_pos_state_is_tablet_mode_cover(struct ssam_tablet_sw *sw, u32 state)
+{
+ switch (state) {
+ case SSAM_POS_COVER_DISCONNECTED:
+ case SSAM_POS_COVER_FOLDED_CANVAS:
+ case SSAM_POS_COVER_FOLDED_BACK:
+ return true;
+
+ case SSAM_POS_COVER_CLOSED:
+ case SSAM_POS_COVER_LAPTOP:
+ return false;
+
+ default:
+ dev_warn(&sw->sdev->dev, "unknown device posture for type-cover: %u\n", state);
+ return true;
+ }
+}
+
static bool ssam_pos_state_is_tablet_mode_sls(struct ssam_tablet_sw *sw, u32 state)
{
switch (state) {
@@ -397,6 +451,9 @@ static bool ssam_pos_state_is_tablet_mode(struct ssam_tablet_sw *sw,
const struct ssam_tablet_sw_state *state)
{
switch (state->source) {
+ case SSAM_POS_SOURCE_COVER:
+ return ssam_pos_state_is_tablet_mode_cover(sw, state->state);
+
case SSAM_POS_SOURCE_SLS:
return ssam_pos_state_is_tablet_mode_sls(sw, state->state);

--
2.39.2


2023-03-04 19:46:53

by Maximilian Luz

[permalink] [raw]
Subject: [PATCH 3/3] platform/surface: aggregator_registry: Add support for tablet-mode switch on Surface Pro 9

Add support for the POS-subsystem tablet-mode switch used on the Surface
Pro 9.

Signed-off-by: Maximilian Luz <[email protected]>
---
drivers/platform/surface/surface_aggregator_registry.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 296f72d52e6a..0fe5be539652 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -305,7 +305,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
&ssam_node_bat_ac,
&ssam_node_bat_main,
&ssam_node_tmp_pprof,
- /* TODO: Tablet mode switch (via POS subsystem) */
+ &ssam_node_pos_tablet_switch,
&ssam_node_hid_kip_keyboard,
&ssam_node_hid_kip_penstash,
&ssam_node_hid_kip_touchpad,
--
2.39.2


2023-03-16 13:48:51

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 0/3] platform/surface: Add support for tablet-mode switch on Surface Pro 9

Hi,

On 3/4/23 20:46, Maximilian Luz wrote:
> This series adds support for the tablet-mode switch on the Surface Pro
> 9. On that device, the posture subsystem (POS) can be used to query the
> state of the Type-Cover (detached, laptop-mode, flipped-back, ...) and
> receive notifications when it changes. We can use this to implement a
> tablet mode switch, extending the driver already in use on the Surface
> Laptop Studio.
>
> More specifically, the posture subsystem allows for different posture
> sources, identified by a numerical ID, each of which can have different
> states. At the moment, however, the tablet-mode switch driver using the
> POS subsystem only supports the Surface Laptop Studio, and support for
> that is hard-coded.
>
> To support the Surface Pro 9, we therefore need to adapt the driver to
> properly disambiguate between different posture sources (patch 1) and
> then implement the respective functions for the source ID representing
> the Type-Cover (patch 2). Finally, we also need to add the respective
> device to the Surface Pro 9 entry in the aggregator registry, to allow
> the driver to load (patch 3).
>
> Maximilian Luz (3):
> platform/surface: aggregator_tabletsw: Properly handle different
> posture source IDs
> platform/surface: aggregator_tabletsw: Add support for Type-Cover
> posture source
> platform/surface: aggregator_registry: Add support for tablet-mode
> switch on Surface Pro 9
>
> .../surface/surface_aggregator_registry.c | 2 +-
> .../surface/surface_aggregator_tabletsw.c | 180 ++++++++++++++----
> 2 files changed, 142 insertions(+), 40 deletions(-)


Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans