[PATCH 3/3] drm/etnaviv: export client GPU usage statistics via fdinfo

[Tvrtko Ursulin]

On 16/09/2022 10:50, Lucas Stach wrote:
> Hi Tvrtko,
> 
> Am Freitag, dem 16.09.2022 um 10:31 +0100 schrieb Tvrtko Ursulin:
>> Hi Lucas,
>>
>> On 08/09/2022 19:10, Lucas Stach wrote:
>>> This exposes a accumulated GPU active time per client via the
>>> fdinfo infrastructure.
>>>
>>> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
>>> ---
>>>    drivers/gpu/drm/etnaviv/etnaviv_drv.c | 38 ++++++++++++++++++++++++++-
>>>    1 file changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>> index b69edb40ae2a..11b1f11fcb58 100644
>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>>> @@ -22,6 +22,7 @@
>>>    #include "etnaviv_gem.h"
>>>    #include "etnaviv_mmu.h"
>>>    #include "etnaviv_perfmon.h"
>>> +#include "common.xml.h"
>>>    
>>>    /*
>>>     * DRM operations:
>>> @@ -471,7 +472,42 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>>>    	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>>>    };
>>>    
>>> -DEFINE_DRM_GEM_FOPS(fops);
>>> +static void etnaviv_fop_show_fdinfo(struct seq_file *m, struct file *f)
>>> +{
>>> +	struct drm_file *file = f->private_data;
>>> +	struct drm_device *dev = file->minor->dev;
>>> +	struct etnaviv_drm_private *priv = dev->dev_private;
>>> +	struct etnaviv_file_private *ctx = file->driver_priv;
>>> +	struct drm_printer p = drm_seq_file_printer(m);
>>
>> Any specific reason not to use seq_printf directly? (May be my ignorance.)
>>
> Not really, I just followed what msm was doing here.

Right, no strong feelings either way I just did not see the need to wrap 
it for this use case.

>>> +	int i;
>>> +
>>> +	drm_printf(&p, "drm-driver:\t%s\n", dev->driver->name);
>>> +	drm_printf(&p, "drm-client-id:\t%u\n", ctx->id);
>>> +
>>> +	for (i = 0; i < ETNA_MAX_PIPES; i++) {
>>> +                struct etnaviv_gpu *gpu = priv->gpu[i];
>>> +		char engine[8];
>>> +		int cur = 0;
>>
>> Alignment renders odd in my client.
> 
> I'll check that, might have messed up here.
>>
>>> +
>>> +		if (!gpu)
>>> +			continue;
>>
>> I'd stick a comment in here to the effect of "For text output format
>> description please see drm-usage-stats.rst!".
>>
>> Just to leave a breadcrumb putting some restraint on adding vendor
>> specific things which may be already covered by the common spec. To help
>> with common tools in the future as much as possible.
> 
> Yea, it was pretty to clear to me that we want the common format as
> much as possible when writing the patches, but it's a good idea to add
> a pointer for the future reader.

Thanks!

>>> +
>>> +		if (gpu->identity.features & chipFeatures_PIPE_2D)
>>> +			cur = snprintf(engine, sizeof(engine), "2D");
>>> +		if (gpu->identity.features & chipFeatures_PIPE_3D)
>>> +			cur = snprintf(engine + cur, sizeof(engine) - cur,
>>> +				       "%s3D", cur ? "/" : "");
>>> +
>>> +		drm_printf(&p, "drm-engine-%s:\t%llu ns\n", engine,
>>> +			   ctx->sched_entity[i].elapsed_ns);
>>
>> Two questions:
>>
>> 1)
>> So you have max four pipes, each can be either only 2d, 3d, or 2d/3d?
>> Can you have multiple of the same like 2x 3D? If you do, have you
>> considered exporting one drm-engine-% together with drm-engine-capacity-
>> for it?
>>
> The four pipes is a arbitrary driver limit. Etnaviv is a bit special in
> that it collects all Vivante GPUs present in a system into a single DRM
> device, each of those GPUs can be either 2D, 3D or a combined core with
> both 2D and 3D capabilities. In theory there could be multiple GPUs of
> each kind, but for now all real SoC designs we've come across only had
> a single one of each kind.
> 
> When we add support for a SoC that has multiple GPUs of one kind, I
> think exposing drm-engine-capacity, together with hooking them up to
> the load balancing in the scheduler is the right thing to do.
> 
>> 2)
>> Have you tried my, yet unmerged, vendor agnostic gputop tool with your
>> changes?
>>
>> https://patchwork.freedesktop.org/series/102175/
>>
>> It would be interesting to know if it works.
>>
> Yes, I did when working on this series. I had some crashes related to
> (I believe) double frees in the DRM client discovery, which I hadn't
> had time to investigate further. Seems there is a race, as I couldn't
> reproduce the crash when running with valgrind.
> 
> Other than that, the tool works for exposing the per-client GPU load on
> etnaviv.

Cool, at least some success.

Out of curiosity what is the planned consumer in etnaviv landscape?

Regards,

Tvrtko