[jerrysxie]

No description provided.

[williampMSFT]

I think we should consider not supporting 'protocol' at all - the boot protocol stuff only actually makes sense if there is a higher-level protocol (i.e. USB) that can enumerate sub-devices. You can't put a combination keyboard+mouse into boot mode because the boot protocol requires that your reports not have report IDs - you have to have a separate keyboard device and mouse device, and on HID-PCC I think that effectively translates to needing a separate HID-PCC region for each individual device, which might not make sense

[ascheng1]

Agree. I would recommend not using HID-over-I2C as starting point. Instead, use HID-over-I3C spec as the starting point. Idle and protocol are not supported in HID-over-I3C or HID-over-SPI spec from MSFT. There is no reason to bring them back for HID-over-eSPI.

Also, Wake is not a separate message. When HID DEVICE is in SLEEP state, it is allowed to generate input HID report.

[kurtjd]

This part I'm a bit confused about, but I could be missing something. I'm having trouble thinking of when this would be necessary.

For example, a hardware eSPI controller that automatically converts CPU read/writes to the MMIO region into peripheral channel transactions (which I'm modeling in QEMU), shouldn't need a protocol layer on top of this.

Say the PCC type 3 region is 1024 bytes in size and the host wants to write a command and payload that takes up all that space. It would look like the host CPU doing, say, multiple 64 bit writes to respective offsets in that shared region.

The eSPI controller would convert those into 8 byte peripheral channel packets (inefficient yes, but otherwise requires some form of buffering/staging as mentioend in the line below). But that peripheral channel packet contains the offset in the address space already, so when the EC's eSPI target receives it, it knows which offset to place it in its mirror of the shared memory.

Then, the host ringing the doorbell is just a way to tell the EC that the whole command is now in the shared memory, but it's already been ordered correctly since that information is inherent in the eSPI spec.

Essentially, it seems to me the eSPI spec already has ordering built in since all peripheral channel requests contain src/dst addresses/offsets, so I'm having trouble understanding when another protocol needs to be added on top to handle this.

[ascheng1]

IIRC, HID Report descriptor is capable of specifying up to 64KB report. Not saying that EC current has usage will need a large size HID report, the fragment protocol layer was introduced when we implement HID over generic SPI and the report size was bigger than the HW transmission buffer size. It was either breaking up the large report to multiple fragments or ask the HID device to break the large report into multiple reports.

If we are willing to limit the maximum report size (to a fraction of the peripheral channel, assuming that we want multiple shared usage on the channel), punting the large data support to next layer up is a possiblility.

[ascheng1]

CFU reference should be in a separate optional section. CFU makes more sense for a standalone physical HID device with FW just for HID. On EC, HID device is a logical function, while it is possible to use CFU to update just the HID FW, theoretically. It is likely that some FW update will need changes beyond the HID FW (e.g. share FW). In those cases, CFU via the HID device is not appropriate, as the changes goes beyond the HID DEVICE scope.

[ascheng1]

Do we still want to support interrupt-and-pull/read? It slows down the transaction significantly, and keep CPU busier than necessary. HID-over-I3C "currently" does not. Is there any technical reason to keep the slower method?

[kurtjd]

I think the main technical reason here for supporting the slower method would be in the case of an embedded controller with more constrained memory than usual. In that case, you may not want to reserve an additional chunk of memory for the type 4 region if you need as much as possible for your application.

In this case, the EC can't just asynchronously write a payload into the shared type 3 memory since the host might be simultaneously writing a request. So, the EC would first need to interrupt the host to tell it to send a request, then ownership of the type 3 region transfers to the EC and it can safely write its payload as a response.

Though would there really be a case where an EC is so memory constrained that this is necessary in practice? Not entirely sure.

[ascheng1]

The flip side is if Type 4 based async input report does not result is measurable benefit (e.g. because the read is from MMIO, and not actual wires), then it would be better to just support Type 3 PCC based HID Input report (to simplify driver/validation a bit).

[kurtjd]

Hm, could you clarify a bit? PCC is bus agnostic, but on hardware where the eSPI controller translates host MMIO accesses into Peripheral Channel transactions, those accesses still involve actual transfers over the wire, right?

With a dedicated Type 4 space, after the EC prepares the payload and notifies the host, the host just needs to read that payload.

With only a Type 3 space, after the EC notifies the host, the host first needs to write a fetch request. The EC then processes that request and makes the response available, which the host reads. Ignoring doorbells, status checks, and acknowledgments, this adds a command transfer and EC command processing before the payload read, compared with Type 4.

So, I’d expect additional overhead even though the accesses appear as MMIO to the host CPU. But as you mentioned, measuring whether that overhead meaningfully affects performance would help determine whether Type 4’s additional complexity is justified.

[ascheng1]

True that MMIO are still going through wires. My main comment is whether there is an opportunity to simplify. Functionally, there is no problem to have both HIDI3C style (~type 4 PCC) and HIDI2C style (~type 3 PCC) ways of implementing input report.

HID input report is the only type of report that need Type 4 space. The point is regardless of type 3 or type 4 PCC, DEVICE should only generate the notification AFTER it knows there is something (an input report) that it needs to send to the HOST. I can't think of a input report case where DEVICE will generate a notification without knowing what data to give to HOST. (i.e. the response should be created before the notification is sent). Please help point out my blind spot, my experience are mainly from HID input devices (e.g. a touchpad controller will know there is a valid finger HID input report ready before generating an interrupt/notification).

[kurtjd]

Right, I think we agree that the EC should already know what payload it wants to send before notifying the host. My concern is about where it can safely publish that payload, rather than when it prepares it.

With Type 3, the host may own the shared memory when the input report becomes ready, so the EC can’t immediately write the report into that region without risking a race with a host command.

For example, the host might be writing an output report when the EC wants to publish an input report. Having the EC check the PCC Command Complete bit wouldn’t prevent this: when that bit is set, the host owns the region and is free to begin writing a new command. When the bit is clear, technically the EC then has ownership over the memory, but in this case that would mean the host is expecting a response to a previously sent command, so the host might inadvertently interpret the notification payload as a response.

My understanding is that the straightforward way to preserve Type 3 ownership semantics is for the EC to notify the host, then have the host send a fetch command and hand ownership to the EC. The EC can then safely write the prepared report as the response. That still requires an additional request/response exchange, even though the report was ready before the notification.

With an available Type 4 slot, the EC can publish the report before notifying the host, avoiding that fetch exchange. If there’s a way to achieve the same thing with only Type 3 using the existing PCC synchronization semantics, then that could be viable, I just haven’t identified one yet. If not, then I suppose we would have to weigh the complexity of adding additional synchronization vs the complexity of just using a Type 4 space.

@williampMSFT Do you have any additional insight here?

[williampMSFT]

I don't totally follow this thread, so I'm going to restate what I think you're all saying, please bugcheck me if I'm misunderstanding:

I think Anton is saying unless we have a compelling reason not to, we might want to consider picking one of the two options (type-3 only/"HID-I2C style" vs type3+4/"HID-I3C style") and only supporting that one option for simplicity.

It seems like the tradeoff between the two options is between performance and memory usage - I2C-style uses less memory but is expected to be a bit less performant (we have to pass control of the single buffer between the EC and host a couple more times when unsolicited input reports happen to signal readiness to accept the report), and I3C-style uses more memory (we have to pay for a second memory-mapped region) but lets us avoid some of that synchronization cost and parallelize handling of host-initiated commands and unsolicited input reports (sort-of; in the eSPI case it does ultimately go over the same wires).

3+4 is also a bit more complex to implement, but supporting 3 and 3+4 is by definition more complex than just supporting 3+4 so optimizing for minimizing complexity still pushes us toward a single option regardless of what that option is.

Unless we think both ends of the perf/memory usage tradeoff is significant, maybe it is worth just picking one and going with that? I don't think we have data on what the actual performance penalty of the type-3-only approach is or on how resource-constrained ECs are going to be, so perhaps next steps are to collect some data there to make an informed decision? I unfortunately don't currently have a working hid-i3c setup to test on, though

[kurtjd]

Yes that looks like it matches my understanding.

Though additionally, if we do decide to support only one of these options, I believe what @ascheng1 is saying (correct me if I'm wrong), is it might be best to choose "Type 3 only" always (regardless of memory constraints) due to the fact that it would likely be less complex to implement. Which I would agree with assuming we are willing to accept the performance penalty (though again, I'd agree we need to measure how significant this penalty is in practice) and not want to introduce further synchronization methods to offset this potential performance penalty (which could potentially introduce additional complexity that we are trying to avoid).

Though @williampMSFT, based on our discussion offline, it does seem to be that the memory-constraint issue might actually be a non-factor. Since either way the EC needs to stage its notification payloads somewhere, and it might not make much of a difference if that is in an officially set-aside Type 4 region or some other internal buffer that the EC uses. So, it might really come down to complexity and performance dictating what we go with, and not so much memory constraints.