| Message ID | 20230104130308.3467806-2-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panfrost: Expose memory usage stats through fdinfo | expand |
On Wed, Jan 04, 2023 at 02:03:05PM +0100, Boris Brezillon wrote: > Provide a dummy show_fdinfo() implementation exposing drm-driver and > drm-client-id. More stats will be added soon. > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 2fa5afe21288..6ee43559fc14 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -515,7 +515,22 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > }; > > -DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops); > +static void panfrost_show_fdinfo(struct seq_file *m, struct file *f) > +{ > + struct drm_file *file = f->private_data; > + struct panfrost_file_priv *panfrost_priv = file->driver_priv; > + > + seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); > + seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context); I think at this point we really need to not just have a document that says what this should look like, but drm infrastructure with shared code. Drivers all inventing their fdinfo really doesn't seem like a great idea to me. -Daniel > +} > + > +static const struct file_operations panfrost_drm_driver_fops = { > + .owner = THIS_MODULE, > + DRM_GEM_FOPS, > +#ifdef CONFIG_PROC_FS > + .show_fdinfo = panfrost_show_fdinfo, > +#endif > +}; > > /* > * Panfrost driver version: > -- > 2.38.1 >
Hi Daniel, On Thu, 5 Jan 2023 16:31:49 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Jan 04, 2023 at 02:03:05PM +0100, Boris Brezillon wrote: > > Provide a dummy show_fdinfo() implementation exposing drm-driver and > > drm-client-id. More stats will be added soon. > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_drv.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index 2fa5afe21288..6ee43559fc14 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -515,7 +515,22 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > > }; > > > > -DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops); > > +static void panfrost_show_fdinfo(struct seq_file *m, struct file *f) > > +{ > > + struct drm_file *file = f->private_data; > > + struct panfrost_file_priv *panfrost_priv = file->driver_priv; > > + > > + seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); > > + seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context); > > I think at this point we really need to not just have a document that says > what this should look like, but drm infrastructure with shared code. > Drivers all inventing their fdinfo really doesn't seem like a great idea > to me. Okay. I'm just curious how far you want to go with this common infrastructure? Are we talking about having a generic helper printing the pretty generic drm-{driver,client-id} props and letting the driver prints its driver specific properties, or do you also want to standardize/automate printing of some drm-memory/drm-engine props too? Regards, Boris
On Mon, 9 Jan 2023 at 09:34, Boris Brezillon <boris.brezillon@collabora.com> wrote: > > Hi Daniel, > > On Thu, 5 Jan 2023 16:31:49 +0100 > Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Wed, Jan 04, 2023 at 02:03:05PM +0100, Boris Brezillon wrote: > > > Provide a dummy show_fdinfo() implementation exposing drm-driver and > > > drm-client-id. More stats will be added soon. > > > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 17 ++++++++++++++++- > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > index 2fa5afe21288..6ee43559fc14 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > @@ -515,7 +515,22 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > > > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > > > }; > > > > > > -DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops); > > > +static void panfrost_show_fdinfo(struct seq_file *m, struct file *f) > > > +{ > > > + struct drm_file *file = f->private_data; > > > + struct panfrost_file_priv *panfrost_priv = file->driver_priv; > > > + > > > + seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); > > > + seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context); > > > > I think at this point we really need to not just have a document that says > > what this should look like, but drm infrastructure with shared code. > > Drivers all inventing their fdinfo really doesn't seem like a great idea > > to me. > > Okay. I'm just curious how far you want to go with this common > infrastructure? Are we talking about having a generic helper printing > the pretty generic drm-{driver,client-id} props and letting the driver > prints its driver specific properties, or do you also want to > standardize/automate printing of some drm-memory/drm-engine props too? I think we should standardized what's used by multiple drivers at least. It might be a bit tough for the memory/engine props, because there's really not much standard stuff there yet (e.g. for memory I'm still hoping for cgroups work, for engines we should probably base this on drm_sched_entity and maybe untie that somewhat from sched itself for i915-sched and fw sched and whatever there is). But as usual, try to be reasonable with standard code :-) -Daniel
On Mon, 9 Jan 2023 11:17:49 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, 9 Jan 2023 at 09:34, Boris Brezillon > <boris.brezillon@collabora.com> wrote: > > > > Hi Daniel, > > > > On Thu, 5 Jan 2023 16:31:49 +0100 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > On Wed, Jan 04, 2023 at 02:03:05PM +0100, Boris Brezillon wrote: > > > > Provide a dummy show_fdinfo() implementation exposing drm-driver and > > > > drm-client-id. More stats will be added soon. > > > > > > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > > > --- > > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 17 ++++++++++++++++- > > > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > > index 2fa5afe21288..6ee43559fc14 100644 > > > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > > @@ -515,7 +515,22 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > > > > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > > > > }; > > > > > > > > -DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops); > > > > +static void panfrost_show_fdinfo(struct seq_file *m, struct file *f) > > > > +{ > > > > + struct drm_file *file = f->private_data; > > > > + struct panfrost_file_priv *panfrost_priv = file->driver_priv; > > > > + > > > > + seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); > > > > + seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context); > > > > > > I think at this point we really need to not just have a document that says > > > what this should look like, but drm infrastructure with shared code. > > > Drivers all inventing their fdinfo really doesn't seem like a great idea > > > to me. > > > > Okay. I'm just curious how far you want to go with this common > > infrastructure? Are we talking about having a generic helper printing > > the pretty generic drm-{driver,client-id} props and letting the driver > > prints its driver specific properties, or do you also want to > > standardize/automate printing of some drm-memory/drm-engine props too? > > I think we should standardized what's used by multiple drivers at > least. It might be a bit tough for the memory/engine props, because > there's really not much standard stuff there yet (e.g. for memory I'm > still hoping for cgroups work, for engines we should probably base > this on drm_sched_entity and maybe untie that somewhat from sched > itself for i915-sched and fw sched and whatever there is). Good, didn't want to be drawn in endless discussions about what should be standardized and what shouldn't anyway. So I'll start with drm-{driver,client-id}. For the client-id, we'll probably need some sort of unique-id stored at the drm_file level (ida-based?), unless you want to leave that to drivers too.
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 2fa5afe21288..6ee43559fc14 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -515,7 +515,22 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), }; -DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops); +static void panfrost_show_fdinfo(struct seq_file *m, struct file *f) +{ + struct drm_file *file = f->private_data; + struct panfrost_file_priv *panfrost_priv = file->driver_priv; + + seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); + seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context); +} + +static const struct file_operations panfrost_drm_driver_fops = { + .owner = THIS_MODULE, + DRM_GEM_FOPS, +#ifdef CONFIG_PROC_FS + .show_fdinfo = panfrost_show_fdinfo, +#endif +}; /* * Panfrost driver version:
Provide a dummy show_fdinfo() implementation exposing drm-driver and drm-client-id. More stats will be added soon. Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_drv.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)