Re: [PATCH v1 2/3] drm: Add API for capturing frame CRCs

[Thierry Reding]

On Thu, Jun 23, 2016 at 10:24:46AM +0200, Tomeu Vizoso wrote:
> On 23 June 2016 at 10:21, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> > On Wed, 22 Jun 2016, Daniel Vetter <daniel@xxxxxxxx> wrote:
> >> On Wed, Jun 22, 2016 at 4:31 PM, Thierry Reding
> >> <thierry.reding@xxxxxxxxx> wrote:
> >>> Perhaps another way to avoid that would be to put the two files into a
> >>> separate directory, as in:
> >>>
> >>>         /sys/kernel/debug/dri/<minor>/crtc-<pipe>/crc/
> >>>         +-- control
> >>>         +-- data
> >>>
> >>> That's slightly on the deeply nested side, but on the other hand it
> >>> nicely uses the filesystem for namespacing, which is what filesystems
> >>> are really good at.
> >>
> >> crtc-<index>/crc/(control|data) sounds great.
> >
> > Side note, we should eventually do the same for sink CRCs, but I guess
> > under the connectors. i915 currently has a special cased version for eDP
> > (named "i915_sink_crc_eDP1"...), reading the data from DPCD.
> 
> Was hoping we could just add one more source to this interface to expose those.

I don't think that would be easy to achieve. You'd have to determine
that a sink source is connected to the CRTC and then ajdust the list
of possible sources dynamically.

For connectors we already have separate directories in debugfs and a
sink can easily be found from the connector it is attached to.

It's also possible, though I don't know of any that do so currently,
that eventually sinks will support several types (i.e. "sources") of
CRC, which will further complicate to represent this in the CRTC's
list of CRC sources.

And it may also be useful to have both CRCs at the same time. The eDP
specification for example defines a very specific polynomial that is
used to compute the CRC, whereas not all display hardware uses a CRC
algorithm that's documented.

Finally, the data that will be checksummed by the CRTC isn't necessarily
the same as that arriving at the sink.

Thierry

Attachment: signature.asc
Description: PGP signature