On 2023/5/25 下午 04:50, Jani Nikula wrote:
On Wed, 24 May 2023, Simon Ser<contact@xxxxxxxxxxx> wrote:
On Tuesday, May 16th, 2023 at 07:39, Alan Liu<HaoPing.Liu@xxxxxxx> wrote:
To address this problem, since modern display control hardware is able to
calculate the CRC checksum of the display content, we are thinking of a
feature to let userspace specify a region of interest (ROI) on display, and
we can utilize the hardware to calculate the CRC checksum as frames scanned
out, and finally, provide the checksum for userspace for validation purpose.
In this case, since the icons themselves are often fixed over static
backgrounds, the CRC of the ROI pixels can be known in advance. So one of the
usage of ROI and corresponding CRC result is that as users know the CRC
checksum of the tell-tales in advance, at runtime they can retrieve the CRC
value from kernel for validation as frames are scanned out.
We implement this feature and call it secure display.
I's strongly advise *not* using the word "secure" here. "Secure" is over-loaded
with so many different meanings, as a result it's super-unclear what a KMS
property name "SECURE_FOO" would do. As an example, some people use "secure" to
refer to Digital Restrictions Management. Something like "CHECKSUM_REGION"
would much better describe the feature you want to implement IMHO.
Agreed.
Thanks, we are fine with calling this feature "CHECKSUM_REGION".
On naming, I also think "ROI" is confusing. Nobody's going to know what
it means without looking it up. I think just "region" is much better,
and "of interest" goes without saying. (Why would you specify a region
unless it was "of interest"?)
"Region" seems to be too simple. We'd like to know if "Critical Region"
or "Checksum Region" is ok. (Although we are going to use
checksum_region as the feature name.)
Also, please note that IGT already extracts CRCs for testing purposes. Maybe
there's an opportunity to use the same uAPI here.
It's debugfs, so probably not suitable for uAPI, but there's already a
bunch of crtc infrastructure in drm level to make that happen. Would
seem odd to add two different ways to gather CRCs with no common code.
Yeah, we need a proper uAPI other than debugfs for this feature in
product stage. We'll take a look at the legacy code and it needs more
reading.
Just checking, we're talking about CRCs computed at some stage of the
display pipeline in the source, not on the sink, right?
Yes, but in the future we may want to extend this feature to support CRC
validation on the sink side.
What's the algorithm for the CRCs? Vendor specific? Is the idea that the
userspace is able to compute it and compare, or snapshot multiple CRCs
from kernel and compare them against each other? If the former, then I
assume the userspace is going to be vendor specific too.
The idea is not for userspace to compute CRC and compare, since after
the pipe doing pixel processing, the image data is different from the
one in the framebuffer. Even users compute CRC on their own by the same
algorithm, the result will is different. Currently the idea is that
before the car is sold to the customer, venders can compute the
tell-tale icons' CRC by the display pipe and save it. At runtime these
pre-saved CRC is used to compare with the CRC from the pipe to make sure
the icon is displayed properly. As a result, userspace should not be
worried about the CRC algorithm, and we'll update this part in the API
documentation.
What about limitations in the dimensions/location of the region? What
Userspace should not submit a region out of the displayable region,
because there is nothing to display and needs protection. We'll update
this in API documentation.
about future compatibility, e.g. if you're interested in *a* region,
surely you might be interested in multiple regions in the future...?
(Not saying this should be implemented now, but would be nice to have
some vague idea how to extend this.)
Yes, we are interested in more regions in the future. We can have
current implementation and add a region_number property later, and for
now the number is 1 by default.
BR,
Jani.
Thank you for all your comments. We'll update the naming and the API
documentation as mentioned above. Also, we will move the new properties'
create and attach API definition to DRM and send the new version of
patch series.
Please let us know if any further comments.
Best Regards,
Alan
