| Message ID | 20230510-feature-ts_virtobj_patch-v5-2-ff6b5c4db693@wolfvision.net |
|---|---|
| State | New |
| Headers | show |
| Series | Input: support overlay objects on touchscreens | expand |
Hi Javier, Thank you for continuing to drive this high-quality work. On Tue, Oct 17, 2023 at 01:00:08PM +0200, Javier Carrasco wrote: > The overlay-touchscreen object defines an area within the touchscreen > where touch events are reported and their coordinates get converted to > the overlay origin. This object avoids getting events from areas that > are physically hidden by overlay frames. > > For touchscreens where overlay buttons on the touchscreen surface are > provided, the overlay-buttons object contains a node for every button > and the key event that should be reported when pressed. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > --- > .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++ > 1 file changed, 143 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > index 431c13335c40..5c58eb79ee9a 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > @@ -87,6 +87,129 @@ properties: > touchscreen-y-plate-ohms: > description: Resistance of the Y-plate in Ohms > > + overlay-touchscreen: > + description: Clipped touchscreen area > + > + This object can be used to describe a frame that restricts the area > + within touch events are reported, ignoring the events that occur outside > + this area. This is of special interest if the touchscreen is shipped > + with a physical overlay on top of it with a frame that hides some part > + of the original touchscreen area. > + > + The x-origin and y-origin properties of this object define the offset of > + a new origin from where the touchscreen events are referenced. > + This offset is applied to the events accordingly. The x-size and y-size > + properties define the size of the overlay-touchscreen (effective area). > + > + The following example shows the new touchscreen area and the new origin > + (0',0') for the touch events generated by the device. > + > + Touchscreen (full area) > + ┌────────────────────────────────────────┐ > + │ ┌───────────────────────────────┐ │ > + │ │ │ │ > + │ ├ y-size │ │ > + │ │ │ │ > + │ │ overlay-touchscreen │ │ > + │ │ │ │ > + │ │ │ │ > + │ │ x-size │ │ > + │ ┌└──────────────┴────────────────┘ │ > + │(0',0') │ > + ┌└────────────────────────────────────────┘ > + (0,0) > + > + where (0',0') = (0+x-origin,0+y-origin) > + > + type: object > + $ref: '#/$defs/overlay-node' > + unevaluatedProperties: false > + > + required: > + - x-origin > + - y-origin > + - x-size > + - y-size > + > + overlay-buttons: > + description: list of nodes defining the buttons on the touchscreen > + > + This object can be used to describe buttons on the touchscreen area, > + reporting the touch events on their surface as key events instead of > + the original touch events. > + > + This is of special interest if the touchscreen is shipped with a > + physical overlay on top of it where a number of buttons with some > + predefined functionality are printed. In that case a specific behavior > + is expected from those buttons instead of raw touch events. > + > + The overlay-buttons properties define a per-button area as well as an > + origin relative to the real touchscreen origin. Touch events within the > + button area are reported as the key event defined in the linux,code > + property. Given that the key events do not provide coordinates, the > + button origin is only used to place the button area on the touchscreen > + surface. Any event outside the overlay-buttons object is reported as a > + touch event with no coordinate transformation. > + > + The following example shows a touchscreen with a single button on it > + > + Touchscreen (full area) > + ┌───────────────────────────────────┐ > + │ │ > + │ │ > + │ ┌─────────┐ │ > + │ │button 0 │ │ > + │ │KEY_POWER│ │ > + │ └─────────┘ │ > + │ │ > + │ │ > + ┌└───────────────────────────────────┘ > + (0,0) > + > + The overlay-buttons object can be combined with the overlay-touchscreen > + object as shown in the following example. In that case only the events > + within the overlay-touchscreen object are reported as touch events. > + > + Touchscreen (full area) > + ┌─────────┬──────────────────────────────┐ > + │ │ │ > + │ │ ┌───────────────────────┐ │ > + │ button 0│ │ │ │ > + │KEY_POWER│ │ │ │ > + │ │ │ │ │ > + ├─────────┤ │ overlay-touchscreen │ │ > + │ │ │ │ │ > + │ │ │ │ │ > + │ button 1│ │ │ │ > + │ KEY_INFO│ ┌└───────────────────────┘ │ > + │ │(0',0') │ > + ┌└─────────┴──────────────────────────────┘ > + (0,0) > + > + type: object I am still confused why the buttons need to live under an 'overlay-buttons' parent node, which seems like an imaginary boundary. In my view, the touch surface comprises the following types of rectangular areas: 1. A touchscreen, wherein granular coordinates and pressure are reported. 2. A momentary button, wherein pressure is quantized into a binary value (press or release), and coordinates are ignored. Any contact that falls outside of (1) and (2) is presumed to be part of a border or matting, and is hence ignored. Areas (1) and (2) exist in the same "plane", so why can they not reside under the same parent node? The following seems much more representative of the actual hardware we intend to describe in the device tree: touchscreen { compatible = "..."; reg = <...>; /* raw coordinates reported here */ touch-area-1 { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; }; /* a button */ touch-area-2a { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; linux,code = <KEY_POWER>; }; /* another button */ touch-area-2b { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; linux,code = <KEY_INFO>; }; }; With this method, the driver merely stores a list head. The parsing code then walks the client device node; for each touch* child encountered, it allocates memory for a structure of five members, and adds it to the list. The event handling code then simply iterates through the list and checks if the coordinates reported by the hardware fall within each rectangle. If so, and the keycode in the list element is equal to KEY_RESERVED (zero), we assume the rectangle is of type (1); the coordinates are passed to touchscreen_report_pos() and the pressure is reported as well. If the keycode is not equal to KEY_RESERVED (e.g. KEY_POWER), we assume the rectangle is of type (2); input_report_key() is called instead and the coordinates are ignored altogether. Instead, the existing implementation has two separate structures, one of which inherits the other. Its corresponding properties are then parsed in a separate function to account for the fact that the two structures exist at different layers in the heirarchy. My argument is that all nodes can be parsed in the same way, without having to understand whether they represent case (1) or (2). The existing method also has to count the nodes; this would not be necessary with a linked list. Can you help me understand why the buttons must be "wrapped" in their own node? As I mention in v2, this isn't a deal breaker necessarily, but I'd like to understand the reasoning behind what I interpret as additional complexity, and a degree of separation from a natural description of the touch surface. The only reason I can think to insert the 'overlay-buttons' parent is in case we want to specify a property within this node that applies to all buttons, but this does not exist in the current implementation, nor do I see a compelling reason to do so in the future. > + > + patternProperties: > + '^button-': > + type: object > + description: > + Each button (key) is represented as a sub-node. > + $ref: '#/$defs/overlay-node' > + unevaluatedProperties: false > + > + properties: > + label: > + $ref: /schemas/types.yaml#/definitions/string > + description: descriptive name of the button > + > + linux,code: true > + > + required: > + - linux,code > + - x-origin > + - y-origin > + - x-size > + - y-size > + > dependencies: > touchscreen-size-x: [ touchscreen-size-y ] > touchscreen-size-y: [ touchscreen-size-x ] > @@ -94,3 +217,23 @@ dependencies: > touchscreen-y-mm: [ touchscreen-x-mm ] > > additionalProperties: true > + > +$defs: > + overlay-node: > + type: object > + properties: > + x-origin: > + description: horizontal origin of the node area > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + y-origin: > + description: vertical origin of the node area > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + x-size: > + description: horizontal resolution of the node area > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + y-size: > + description: vertical resolution of the node area > + $ref: /schemas/types.yaml#/definitions/uint32 > > -- > 2.39.2 > Kind regards, Jeff LaBundy
Hi Jeff, On 26.10.23 16:46, Jeff LaBundy wrote: > Hi Javier, > > Thank you for continuing to drive this high-quality work. > > On Tue, Oct 17, 2023 at 01:00:08PM +0200, Javier Carrasco wrote: >> The overlay-touchscreen object defines an area within the touchscreen >> where touch events are reported and their coordinates get converted to >> the overlay origin. This object avoids getting events from areas that >> are physically hidden by overlay frames. >> >> For touchscreens where overlay buttons on the touchscreen surface are >> provided, the overlay-buttons object contains a node for every button >> and the key event that should be reported when pressed. >> >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> >> --- >> .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++ >> 1 file changed, 143 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml >> index 431c13335c40..5c58eb79ee9a 100644 >> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml >> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml >> @@ -87,6 +87,129 @@ properties: >> touchscreen-y-plate-ohms: >> description: Resistance of the Y-plate in Ohms >> >> + overlay-touchscreen: >> + description: Clipped touchscreen area >> + >> + This object can be used to describe a frame that restricts the area >> + within touch events are reported, ignoring the events that occur outside >> + this area. This is of special interest if the touchscreen is shipped >> + with a physical overlay on top of it with a frame that hides some part >> + of the original touchscreen area. >> + >> + The x-origin and y-origin properties of this object define the offset of >> + a new origin from where the touchscreen events are referenced. >> + This offset is applied to the events accordingly. The x-size and y-size >> + properties define the size of the overlay-touchscreen (effective area). >> + >> + The following example shows the new touchscreen area and the new origin >> + (0',0') for the touch events generated by the device. >> + >> + Touchscreen (full area) >> + ┌────────────────────────────────────────┐ >> + │ ┌───────────────────────────────┐ │ >> + │ │ │ │ >> + │ ├ y-size │ │ >> + │ │ │ │ >> + │ │ overlay-touchscreen │ │ >> + │ │ │ │ >> + │ │ │ │ >> + │ │ x-size │ │ >> + │ ┌└──────────────┴────────────────┘ │ >> + │(0',0') │ >> + ┌└────────────────────────────────────────┘ >> + (0,0) >> + >> + where (0',0') = (0+x-origin,0+y-origin) >> + >> + type: object >> + $ref: '#/$defs/overlay-node' >> + unevaluatedProperties: false >> + >> + required: >> + - x-origin >> + - y-origin >> + - x-size >> + - y-size >> + >> + overlay-buttons: >> + description: list of nodes defining the buttons on the touchscreen >> + >> + This object can be used to describe buttons on the touchscreen area, >> + reporting the touch events on their surface as key events instead of >> + the original touch events. >> + >> + This is of special interest if the touchscreen is shipped with a >> + physical overlay on top of it where a number of buttons with some >> + predefined functionality are printed. In that case a specific behavior >> + is expected from those buttons instead of raw touch events. >> + >> + The overlay-buttons properties define a per-button area as well as an >> + origin relative to the real touchscreen origin. Touch events within the >> + button area are reported as the key event defined in the linux,code >> + property. Given that the key events do not provide coordinates, the >> + button origin is only used to place the button area on the touchscreen >> + surface. Any event outside the overlay-buttons object is reported as a >> + touch event with no coordinate transformation. >> + >> + The following example shows a touchscreen with a single button on it >> + >> + Touchscreen (full area) >> + ┌───────────────────────────────────┐ >> + │ │ >> + │ │ >> + │ ┌─────────┐ │ >> + │ │button 0 │ │ >> + │ │KEY_POWER│ │ >> + │ └─────────┘ │ >> + │ │ >> + │ │ >> + ┌└───────────────────────────────────┘ >> + (0,0) >> + >> + The overlay-buttons object can be combined with the overlay-touchscreen >> + object as shown in the following example. In that case only the events >> + within the overlay-touchscreen object are reported as touch events. >> + >> + Touchscreen (full area) >> + ┌─────────┬──────────────────────────────┐ >> + │ │ │ >> + │ │ ┌───────────────────────┐ │ >> + │ button 0│ │ │ │ >> + │KEY_POWER│ │ │ │ >> + │ │ │ │ │ >> + ├─────────┤ │ overlay-touchscreen │ │ >> + │ │ │ │ │ >> + │ │ │ │ │ >> + │ button 1│ │ │ │ >> + │ KEY_INFO│ ┌└───────────────────────┘ │ >> + │ │(0',0') │ >> + ┌└─────────┴──────────────────────────────┘ >> + (0,0) >> + >> + type: object > > I am still confused why the buttons need to live under an 'overlay-buttons' > parent node, which seems like an imaginary boundary. In my view, the touch > surface comprises the following types of rectangular areas: > > 1. A touchscreen, wherein granular coordinates and pressure are reported. > 2. A momentary button, wherein pressure is quantized into a binary value > (press or release), and coordinates are ignored. > > Any contact that falls outside of (1) and (2) is presumed to be part of a > border or matting, and is hence ignored. > > Areas (1) and (2) exist in the same "plane", so why can they not reside > under the same parent node? The following seems much more representative > of the actual hardware we intend to describe in the device tree: > > touchscreen { > compatible = "..."; > reg = <...>; > > /* raw coordinates reported here */ > touch-area-1 { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > }; > > /* a button */ > touch-area-2a { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > linux,code = <KEY_POWER>; > }; > > /* another button */ > touch-area-2b { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > linux,code = <KEY_INFO>; > }; > }; > Now that I am working on the approach you suggested, I see that some things can get slightly more complicated. I still think that it is worth a try, but I would like to discuss a couple of points. The node parsing is not that simple anymore because the touch-area nodes are only surrounded by the touchscreen node. Theoretically they could be even be defined with other properties in between. The current approach only needs to find the overlay-buttons parent and iterate over all the inner nodes(simply by calling device_get_named_child_node() and fwnode_for_each_child_node() the parsing is achieved in two lines + error checking). So maybe even if we opt for the single-object approach, an overlay node to group all the touch-areas could simplify the parsing. Or did you have a different approach in mind? Your example would turn into this one: touchscreen { compatible = "..."; reg = <...>; touch-overlay { /* raw coordinates reported here */ touch-area-1 { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; }; /* a button */ touch-area-2a { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; linux,code = <KEY_POWER>; }; /* another button */ touch-area-2b { x-origin = <...>; y-origin = <...>; x-size = <...>; y-size = <...>; linux,code = <KEY_INFO>; }; }; }; In my opinion it looks cleaner as well because you are defining a physical object: the overlay. > With this method, the driver merely stores a list head. The parsing code > then walks the client device node; for each touch* child encountered, it > allocates memory for a structure of five members, and adds it to the list. > The button objects do not only store the keycode, but also the slot and if they are pressed or not. I could allocate memory for these members as well, but maybe an additional struct with the button-specific members set to NULL for the touch areas with keycode = KEY_RESERVED would make sense. I don't know if that's adding too much overhead for two members though. > The event handling code then simply iterates through the list and checks > if the coordinates reported by the hardware fall within each rectangle. If > so, and the keycode in the list element is equal to KEY_RESERVED (zero), > we assume the rectangle is of type (1); the coordinates are passed to > touchscreen_report_pos() and the pressure is reported as well. There is another case to consider that might make the iteration less optimal, but I don't think it will be critical. A button could be defined inside an overlay-touchscreen (no keycode) area. Given that the other way round (a touchscreen inside a button) does not make much sense, the buttons have a higher priority. Let's take your example: imagine that your third area is a button inside the first one. We have to iterate through the whole list until we are sure we checked if there are buttons in the given position, but keeping in mind that the first object already has the right coordinates to handle the touch event. Your approach even allows for multiple no-key areas and we do not know if there are buttons when we iterate (there could be none). Therefore some iterations could be unnecessary, but this is probably an edge case that would cost at most a couple of extra iterations compared to a two-list approach. I will keep on working on the next version with a single list while we clarify these points, so maybe we can save an iteration. > Kind regards, > Jeff LaBundy Best regards, Javier Carrasco
Hi Javier, I am so sorry for the delayed response. On Thu, Nov 23, 2023 at 08:48:56PM +0100, Javier Carrasco wrote: > Hi Jeff, > > On 26.10.23 16:46, Jeff LaBundy wrote: > > Hi Javier, > > > > Thank you for continuing to drive this high-quality work. > > > > On Tue, Oct 17, 2023 at 01:00:08PM +0200, Javier Carrasco wrote: > >> The overlay-touchscreen object defines an area within the touchscreen > >> where touch events are reported and their coordinates get converted to > >> the overlay origin. This object avoids getting events from areas that > >> are physically hidden by overlay frames. > >> > >> For touchscreens where overlay buttons on the touchscreen surface are > >> provided, the overlay-buttons object contains a node for every button > >> and the key event that should be reported when pressed. > >> > >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > >> --- > >> .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++ > >> 1 file changed, 143 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > >> index 431c13335c40..5c58eb79ee9a 100644 > >> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > >> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml > >> @@ -87,6 +87,129 @@ properties: > >> touchscreen-y-plate-ohms: > >> description: Resistance of the Y-plate in Ohms > >> > >> + overlay-touchscreen: > >> + description: Clipped touchscreen area > >> + > >> + This object can be used to describe a frame that restricts the area > >> + within touch events are reported, ignoring the events that occur outside > >> + this area. This is of special interest if the touchscreen is shipped > >> + with a physical overlay on top of it with a frame that hides some part > >> + of the original touchscreen area. > >> + > >> + The x-origin and y-origin properties of this object define the offset of > >> + a new origin from where the touchscreen events are referenced. > >> + This offset is applied to the events accordingly. The x-size and y-size > >> + properties define the size of the overlay-touchscreen (effective area). > >> + > >> + The following example shows the new touchscreen area and the new origin > >> + (0',0') for the touch events generated by the device. > >> + > >> + Touchscreen (full area) > >> + ┌────────────────────────────────────────┐ > >> + │ ┌───────────────────────────────┐ │ > >> + │ │ │ │ > >> + │ ├ y-size │ │ > >> + │ │ │ │ > >> + │ │ overlay-touchscreen │ │ > >> + │ │ │ │ > >> + │ │ │ │ > >> + │ │ x-size │ │ > >> + │ ┌└──────────────┴────────────────┘ │ > >> + │(0',0') │ > >> + ┌└────────────────────────────────────────┘ > >> + (0,0) > >> + > >> + where (0',0') = (0+x-origin,0+y-origin) > >> + > >> + type: object > >> + $ref: '#/$defs/overlay-node' > >> + unevaluatedProperties: false > >> + > >> + required: > >> + - x-origin > >> + - y-origin > >> + - x-size > >> + - y-size > >> + > >> + overlay-buttons: > >> + description: list of nodes defining the buttons on the touchscreen > >> + > >> + This object can be used to describe buttons on the touchscreen area, > >> + reporting the touch events on their surface as key events instead of > >> + the original touch events. > >> + > >> + This is of special interest if the touchscreen is shipped with a > >> + physical overlay on top of it where a number of buttons with some > >> + predefined functionality are printed. In that case a specific behavior > >> + is expected from those buttons instead of raw touch events. > >> + > >> + The overlay-buttons properties define a per-button area as well as an > >> + origin relative to the real touchscreen origin. Touch events within the > >> + button area are reported as the key event defined in the linux,code > >> + property. Given that the key events do not provide coordinates, the > >> + button origin is only used to place the button area on the touchscreen > >> + surface. Any event outside the overlay-buttons object is reported as a > >> + touch event with no coordinate transformation. > >> + > >> + The following example shows a touchscreen with a single button on it > >> + > >> + Touchscreen (full area) > >> + ┌───────────────────────────────────┐ > >> + │ │ > >> + │ │ > >> + │ ┌─────────┐ │ > >> + │ │button 0 │ │ > >> + │ │KEY_POWER│ │ > >> + │ └─────────┘ │ > >> + │ │ > >> + │ │ > >> + ┌└───────────────────────────────────┘ > >> + (0,0) > >> + > >> + The overlay-buttons object can be combined with the overlay-touchscreen > >> + object as shown in the following example. In that case only the events > >> + within the overlay-touchscreen object are reported as touch events. > >> + > >> + Touchscreen (full area) > >> + ┌─────────┬──────────────────────────────┐ > >> + │ │ │ > >> + │ │ ┌───────────────────────┐ │ > >> + │ button 0│ │ │ │ > >> + │KEY_POWER│ │ │ │ > >> + │ │ │ │ │ > >> + ├─────────┤ │ overlay-touchscreen │ │ > >> + │ │ │ │ │ > >> + │ │ │ │ │ > >> + │ button 1│ │ │ │ > >> + │ KEY_INFO│ ┌└───────────────────────┘ │ > >> + │ │(0',0') │ > >> + ┌└─────────┴──────────────────────────────┘ > >> + (0,0) > >> + > >> + type: object > > > > I am still confused why the buttons need to live under an 'overlay-buttons' > > parent node, which seems like an imaginary boundary. In my view, the touch > > surface comprises the following types of rectangular areas: > > > > 1. A touchscreen, wherein granular coordinates and pressure are reported. > > 2. A momentary button, wherein pressure is quantized into a binary value > > (press or release), and coordinates are ignored. > > > > Any contact that falls outside of (1) and (2) is presumed to be part of a > > border or matting, and is hence ignored. > > > > Areas (1) and (2) exist in the same "plane", so why can they not reside > > under the same parent node? The following seems much more representative > > of the actual hardware we intend to describe in the device tree: > > > > touchscreen { > > compatible = "..."; > > reg = <...>; > > > > /* raw coordinates reported here */ > > touch-area-1 { > > x-origin = <...>; > > y-origin = <...>; > > x-size = <...>; > > y-size = <...>; > > }; > > > > /* a button */ > > touch-area-2a { > > x-origin = <...>; > > y-origin = <...>; > > x-size = <...>; > > y-size = <...>; > > linux,code = <KEY_POWER>; > > }; > > > > /* another button */ > > touch-area-2b { > > x-origin = <...>; > > y-origin = <...>; > > x-size = <...>; > > y-size = <...>; > > linux,code = <KEY_INFO>; > > }; > > }; > > > Now that I am working on the approach you suggested, I see that some > things can get slightly more complicated. I still think that it is worth > a try, but I would like to discuss a couple of points. > > The node parsing is not that simple anymore because the touch-area nodes > are only surrounded by the touchscreen node. Theoretically they could be > even be defined with other properties in between. The current approach > only needs to find the overlay-buttons parent and iterate over all the > inner nodes(simply by calling device_get_named_child_node() and > fwnode_for_each_child_node() the parsing is achieved in two lines + > error checking). So maybe even if we opt for the single-object approach, > an overlay node to group all the touch-areas could simplify the parsing. > Or did you have a different approach in mind? Your example would turn > into this one: > > touchscreen { > compatible = "..."; > reg = <...>; > > touch-overlay { > /* raw coordinates reported here */ > touch-area-1 { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > }; > > /* a button */ > touch-area-2a { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > linux,code = <KEY_POWER>; > }; > > /* another button */ > touch-area-2b { > x-origin = <...>; > y-origin = <...>; > x-size = <...>; > y-size = <...>; > linux,code = <KEY_INFO>; > }; > }; > }; > In my opinion it looks cleaner as well because you are defining a > physical object: the overlay. I like this idea. My original example assumes one would include any node that contains some magic substring (e.g. "touch") in the node name, but thinking about this more, that may be a bit presumptive. It seems safer to wrap all of the children in one newly introduced node as you have done here. > > > With this method, the driver merely stores a list head. The parsing code > > then walks the client device node; for each touch* child encountered, it > > allocates memory for a structure of five members, and adds it to the list. > > > The button objects do not only store the keycode, but also the slot and > if they are pressed or not. I could allocate memory for these members as > well, but maybe an additional struct with the button-specific members > set to NULL for the touch areas with keycode = KEY_RESERVED would make > sense. I don't know if that's adding too much overhead for two members > though. It's still not clear to me why your code is responsible for storing button state; that's the job of the input subsystem. Your code is only responsible for reporting instantaneous state after you are told something interesting happened (e.g. interrupt). The input core is responsible for determining whether the most recently reported state is different than the last. > > > The event handling code then simply iterates through the list and checks > > if the coordinates reported by the hardware fall within each rectangle. If > > so, and the keycode in the list element is equal to KEY_RESERVED (zero), > > we assume the rectangle is of type (1); the coordinates are passed to > > touchscreen_report_pos() and the pressure is reported as well. > > There is another case to consider that might make the iteration less > optimal, but I don't think it will be critical. > > A button could be defined inside an overlay-touchscreen (no keycode) > area. Given that the other way round (a touchscreen inside a button) > does not make much sense, the buttons have a higher priority. > > Let's take your example: imagine that your third area > is a button inside the first one. We have to iterate through the whole > list until we are sure we checked if there are buttons in the given > position, but keeping in mind that the first object already has the > right coordinates to handle the touch event. Your approach even allows > for multiple no-key areas and we do not know if there are buttons when > we iterate (there could be none). > Therefore some iterations could be unnecessary, but this is probably an > edge case that would cost at most a couple of extra iterations compared > to a two-list approach. I think we need to model the overlay as having only two dimensions, with nothing "on top of" or "inside" anything else. For this case of a button inside a touch surface, with the latter making a square doughnut shape of sorts, the 'touch-overlay' node would have five children: two tall rectangles (left and right), two shorter rectanges (above and below the button), and then finally a button in the center. Stated another way, the 'touch-overlay' node shall support an infinite number of infinitesimally small rectangles which comprise the entire touch surface. It shall be possible for a contact to be in zero rectangles, but impossible for any contact to be in more than one rectangle. I appreciate that the devil is in the details, but here we are defining the interface, independent of the implementation. > > I will keep on working on the next version with a single list while we > clarify these points, so maybe we can save an iteration. I see there is a v6 now; I'll take a look at that next. Thanks again for the productive discussion! > > Kind regards, > > Jeff LaBundy > > Best regards, > Javier Carrasco Kind regards, Jeff LaBundy
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml index 431c13335c40..5c58eb79ee9a 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml @@ -87,6 +87,129 @@ properties: touchscreen-y-plate-ohms: description: Resistance of the Y-plate in Ohms + overlay-touchscreen: + description: Clipped touchscreen area + + This object can be used to describe a frame that restricts the area + within touch events are reported, ignoring the events that occur outside + this area. This is of special interest if the touchscreen is shipped + with a physical overlay on top of it with a frame that hides some part + of the original touchscreen area. + + The x-origin and y-origin properties of this object define the offset of + a new origin from where the touchscreen events are referenced. + This offset is applied to the events accordingly. The x-size and y-size + properties define the size of the overlay-touchscreen (effective area). + + The following example shows the new touchscreen area and the new origin + (0',0') for the touch events generated by the device. + + Touchscreen (full area) + ┌────────────────────────────────────────┐ + │ ┌───────────────────────────────┐ │ + │ │ │ │ + │ ├ y-size │ │ + │ │ │ │ + │ │ overlay-touchscreen │ │ + │ │ │ │ + │ │ │ │ + │ │ x-size │ │ + │ ┌└──────────────┴────────────────┘ │ + │(0',0') │ + ┌└────────────────────────────────────────┘ + (0,0) + + where (0',0') = (0+x-origin,0+y-origin) + + type: object + $ref: '#/$defs/overlay-node' + unevaluatedProperties: false + + required: + - x-origin + - y-origin + - x-size + - y-size + + overlay-buttons: + description: list of nodes defining the buttons on the touchscreen + + This object can be used to describe buttons on the touchscreen area, + reporting the touch events on their surface as key events instead of + the original touch events. + + This is of special interest if the touchscreen is shipped with a + physical overlay on top of it where a number of buttons with some + predefined functionality are printed. In that case a specific behavior + is expected from those buttons instead of raw touch events. + + The overlay-buttons properties define a per-button area as well as an + origin relative to the real touchscreen origin. Touch events within the + button area are reported as the key event defined in the linux,code + property. Given that the key events do not provide coordinates, the + button origin is only used to place the button area on the touchscreen + surface. Any event outside the overlay-buttons object is reported as a + touch event with no coordinate transformation. + + The following example shows a touchscreen with a single button on it + + Touchscreen (full area) + ┌───────────────────────────────────┐ + │ │ + │ │ + │ ┌─────────┐ │ + │ │button 0 │ │ + │ │KEY_POWER│ │ + │ └─────────┘ │ + │ │ + │ │ + ┌└───────────────────────────────────┘ + (0,0) + + The overlay-buttons object can be combined with the overlay-touchscreen + object as shown in the following example. In that case only the events + within the overlay-touchscreen object are reported as touch events. + + Touchscreen (full area) + ┌─────────┬──────────────────────────────┐ + │ │ │ + │ │ ┌───────────────────────┐ │ + │ button 0│ │ │ │ + │KEY_POWER│ │ │ │ + │ │ │ │ │ + ├─────────┤ │ overlay-touchscreen │ │ + │ │ │ │ │ + │ │ │ │ │ + │ button 1│ │ │ │ + │ KEY_INFO│ ┌└───────────────────────┘ │ + │ │(0',0') │ + ┌└─────────┴──────────────────────────────┘ + (0,0) + + type: object + + patternProperties: + '^button-': + type: object + description: + Each button (key) is represented as a sub-node. + $ref: '#/$defs/overlay-node' + unevaluatedProperties: false + + properties: + label: + $ref: /schemas/types.yaml#/definitions/string + description: descriptive name of the button + + linux,code: true + + required: + - linux,code + - x-origin + - y-origin + - x-size + - y-size + dependencies: touchscreen-size-x: [ touchscreen-size-y ] touchscreen-size-y: [ touchscreen-size-x ] @@ -94,3 +217,23 @@ dependencies: touchscreen-y-mm: [ touchscreen-x-mm ] additionalProperties: true + +$defs: + overlay-node: + type: object + properties: + x-origin: + description: horizontal origin of the node area + $ref: /schemas/types.yaml#/definitions/uint32 + + y-origin: + description: vertical origin of the node area + $ref: /schemas/types.yaml#/definitions/uint32 + + x-size: + description: horizontal resolution of the node area + $ref: /schemas/types.yaml#/definitions/uint32 + + y-size: + description: vertical resolution of the node area + $ref: /schemas/types.yaml#/definitions/uint32
The overlay-touchscreen object defines an area within the touchscreen where touch events are reported and their coordinates get converted to the overlay origin. This object avoids getting events from areas that are physically hidden by overlay frames. For touchscreens where overlay buttons on the touchscreen surface are provided, the overlay-buttons object contains a node for every button and the key event that should be reported when pressed. Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> --- .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++ 1 file changed, 143 insertions(+)