[Intel-gfx] [PATCH 4/4] drm/i915: expose rcs topology through discovery uAPI

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-13 09:14:15)
> 
> On 10/11/2017 18:29, Lionel Landwerlin wrote:
> > On 10/11/17 16:47, Chris Wilson wrote:
> >> Quoting Lionel Landwerlin (2017-11-10 16:37:33)
> >>> On 09/11/17 17:34, Tvrtko Ursulin wrote:
> >>>> On 08/11/2017 16:22, Lionel Landwerlin wrote:
> >>>> But in general would it be feasible to define and name the returned
> >>>> data more precisely? Like:
> >>>>
> >>>> struct drm_engine_rcs_engine_info {
> >>>>      ...
> >>>>      /existing_stuff/
> >>>>      ...
> >>>>
> >>>> #define HAS_TOPOLOGY
> >>>>      u32 flags;
> >>>>
> >>>>      struct {
> >>>>          u32 this;
> >>>>          u32 that;
> >>>>          ...
> >>>>          u8 mask[SOME_FUTURE_PROOF_NUMBER];
> >>>>      } slice_topology;
> >>>>
> >>>>      struct {
> >>>>          u32 this;
> >>>>          u32 that;
> >>>>          ...
> >>>>          u8 mask[SOME_FUTURE_PROOF_NUMBER];
> >>>>      } subslice_topology;
> >>>>
> >>>>      struct {
> >>>>          u32 this;
> >>>>          u32 that;
> >>>>          ...
> >>>>          u8 mask[SOME_FUTURE_PROOF_NUMBER];
> >>>>      } eu_topology;
> >>>> };
> >>> I'm pretty sure we'll need to expose more than these 3 properties here
> >>> (slice/subslice/eus) soon.
> >>> Some of the components residing in the subslice could be of interest.
> >>> So I'm reluctant to have all of this within a single struct which we
> >>> can't change the size of.
> >> The struct size doesn't have to be fixed, just reported. The underlying
> >> question is can we construct an interface that is flexible enough?
> >>
> >> Something along the lines of perf (GL even) where the output format is
> >> constructed by request from the user, then we only need to declare how
> >> long each field will be, get to the user allocate the buffer, then fill
> >> on the second pass.
> >>
> >> Alternatively we output some ASN string?
> >>
> >> I don't want to overengineer, but at the same time this looks to be the
> >> perfect excuse to require enough flexibility to cater for future
> >> complexity without going too meta. :)
> >> -Chris
> >>
> > Heh, so one ioctl to get the string, another ioctl to get the data?
> > And a list of enum for all the properties you can list?
> > 
> > Unrelated question, have you considered ASN to describe the error state 
> > layout?
> 
> Or we go with sysfs, plain and simple?
> 
> $ cat $i915root/engine/vcs0/info
> hevc
> 
> $ cat $i915root/engine/vcs1/instance
> 1
> 
> $ cat $i915root/engine/rcs0/class
> render
> 
> ...
> 
> $i915root/gpu/topology/slice_mask
> 
> Should be able to design to avoid issues with extensibility and avoids 
> the need to come up with complex binary structures or even adding new 
> protocols like the ASN mentioned above.

That wins the flexibility argument hands down. (It is so flexible we have
to plan ahead to get the hierarchy right. We can even version it with a
$i915root/version. Except people will add it to their scripts and notice
if paths change. It has the benefit of being too convenient ;)

The only counter is, ugh that's a lot of work to gather the info on
userspace driver load. But everybody can cat their gpu details.
-Chris