[igt-dev] [RFC 1/3] intel_gpu_top: User friendly device listing

[Zbigniew Kempczyński]

On Tue, Nov 10, 2020 at 11:52:50AM +0000, Tvrtko Ursulin wrote:
> 
> On 10/11/2020 11:46, Zbigniew Kempczyński wrote:
> 
> [snip]
> 
> > > > > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > > > > index 298defa4e6ed..5230472d2af4 100644
> > > > > --- a/tools/intel_gpu_top.c
> > > > > +++ b/tools/intel_gpu_top.c
> > > > > @@ -1313,7 +1313,6 @@ int main(int argc, char **argv)
> > > > >    	unsigned int i;
> > > > >    	int ret = 0, ch;
> > > > >    	bool list_device = false;
> > > > > -	enum igt_devices_print_type printtype = IGT_PRINT_SIMPLE;
> > > > >    	char *pmu_device, *opt_device = NULL;
> > > > >    	struct igt_device_card card;
> > > > > @@ -1388,7 +1387,7 @@ int main(int argc, char **argv)
> > > > >    	igt_devices_scan(false);
> > > > >    	if (list_device) {
> > > > > -		igt_devices_print(printtype);
> > > > > +		igt_devices_print(IGT_PRINT_USER);
> > > > 
> > > > I would add at least possibility to use simple view to suggest
> > > > how to use pci/sys filter. With USER print format we see only
> > > 
> > > You mean the blurb printed out by igt_device_print_filter_types (currently
> > > printed as part of help text) or something else?
> > 
> > Yes. We know lsgpu and intel_gpu_top selection is same and you're
> > also printing filter syntax on 'intel_gpu_top -h'. But without
> > using 'lsgpu -s' you won't guess how to specify sys:... filter.
> 
> My thinking is that typical intel_gpu_top user wouldn't want/need to concern
> themselves with different filters but just copy&paste what was listed from
> intel_gpu_top -L. In which case..
> 
> > > 
> > > > drm paths. But I won't insist for that, using drm selection
> > > > is ok for me.
> > > 
> > > Good point actually, intel_gpu_top should probably default to "--pci"
> > > listing type since it monitors GPUs with no notion of DRM master/render.
> > > 
> > > > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > > 
> > > Thanks! Let me know if you agree with "--pci" by default for intel_gpu_top
> > > and if it is okay to keep the r-b?
> > 
> > If it is not problem you could provide --pci and --sysfs too to be compliant
> > with lsgpu switches. But there are minor nits and you can keep r-b with
> > the code as it is.
> 
> ... I did not see the value of supporting --drm/--pci/--sysfs from
> intel_gpu_top.
> 
> So "--pci" by default should "JustWork" and make end users happy. As long as
> the help text is clear enough what needs to be copy&pasted, which may need
> improvement. If I am not missing something else.
> 
> Regards,
> 
> Tvrtko

Ok. Probably you're right and users won't need any other selectors.
If they will we will change the code. 

--
Zbigniew