[Intel-gfx] [PATCH v11 5/6] drm/i915: add query uAPI

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-01-24 12:03:46)
> 
> On 23/01/2018 14:17, Lionel Landwerlin wrote:
> > Hi all,
> > 
> > I've been trying to expose some information to userspace about the fused 
> > parts of the GPU.
> > This is the 4th attempt at getting this upstream, here are the previous 
> > ones :
> >      https://patchwork.freedesktop.org/patch/185959/
> >      https://patchwork.freedesktop.org/series/33436/
> >      https://patchwork.freedesktop.org/series/33950/
> > 
> > This last iteration was based upon some direction by Daniel : 
> > https://lists.freedesktop.org/archives/dri-devel/2017-December/160162.html
> > There was, I think, a fair point about having this working in 
> > environments where sysfs (mechanism used in the 3rd iteration) is not 
> > available (containers).
> > 
> > Following some discussion on IRC, it seems Joonas would like this 
> > rewritten in a such way that we essentially drop the generic mechanism 
> > introduced in this patch, and instead go for an additional ioctl() on 
> > the drm fd just for querying the state of a fused part of 
> > slice/subslice/eus.
> > The proposal is to have a single struct like :
> > 
> > struct drm_i915_topology {
> >     /* All field are in/out */
> >     int slice;
> >     int subslice;
> >     int eu;
> > 
> >     int enabled;
> > };
> > 
> > You would let the slice field to -1 and then the kernel would fill it 
> > with the max slice value. Same for subslice (with a valid slice value) 
> > and eu.
> > When querying with slice = 0, and all other fields to -1, the kernel 
> > would fill the enabled value with 0 or 1.
> > Essentially that would mean that an application wanting to query the 
> > state of all of the EUs would have to go through them one by one (which 
> > would be about ~100 ioctl() on SKL GT4 for example).
> > 
> > Apart from the fact that we'll probably end up adding another ioctl() 
> > for engine discovery, I don't have any problem with what Joonas is 
> > proposing.
> > It's just a bit annoying this comes up on the 4th rewrite.
> > I really wouldn't like to rewrite this one more time and get turned down 
> > because this isn't to the taste of one of the reviewer.
> > So my question is : Is everybody happy with what Joonas is proposing?
> > Anybody in favor of having a generic mechanism?
> 
> I am not very keen on this counter-proposal for two reasons.
> 
> First is that I think is a bit inelegant to have to query so many times 
> just to get the full topology out. If this ends in some library, we may 
> end up running this on every trivial client startup.
> 
> Secondly, it is kind of dispatcher in it's own right. Since the 
> operation mode will depend on the combination of field values. As such a 
> generic, or at least a more explicit, dispatcher, like the proposed 
> i915_query_ioctl sounds cleaner to me.
> 
> I take the point we can't guess how many other users we will have for it 
> in the future. So there is a little bit of a risk of adding something 
> generic which won't be used a lot in the future.
> 
> Because apart from the three queries Lionel needs, I would be adding an 
> engine info query and potentially, depending on userspace interest, 
> engine queue depths. So that would be a maximum of five queries I am 
> aware of would use the generic framework. Maybe too little, or maybe 
> good enough for a start?

Another use case would be a single shot method to gather all GETPARAMs.

There's a lot of too'ing and fro'ing at the start of mesa trying to
determine all of the kernel's capabilities, which more or less come down
to a long series of parsing GETPARAM results. Bundling all of those up
into a single ioctl seems attractive to me (bonus for it being properly
defined and not a compat nightmare).
-Chris