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

[Lionel Landwerlin]

On 13/11/17 09:14, Tvrtko Ursulin wrote:
>
> 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.
>
> ?
>
> Tvrtko
>
I guess that works too. What layout would fit for the other bits of 
topology though? :

topology/slice_mask
topology/slice0/subslice_mask
topology/slice0/subslice0/eu_mask
topology/slice0/subslice1/eu_mask
topology/slice0/subslice2/eu_mask
topology/slice1/subslice_mask
...
topology/slice2/subslice_mask
...

Or something flatter but then you could need lines in the files to split 
things by slice/subslice.

-
Lionel