If we expose a render node for NPUs without rendering capabilities, the
userspace stack will offer it to compositors and applications for
rendering, which of course won't work.
Userspace is probably right in not questioning whether a render node
might not be capable of supporting rendering, so change it in the kernel
instead by exposing a /dev/accel node.
Before we bring the device up we don't know whether it is capable of
rendering or not (depends on the features of its blocks), so first try
to probe a rendering node, and if we find out that there is no rendering
hardware, abort and retry with an accel node.
Signed-off-by: Tomeu Vizoso <[email protected]>
Cc: Oded Gabbay <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6500f3999c5f..8e7dd23115f4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -11,6 +11,7 @@
#include <linux/platform_device.h>
#include <linux/uaccess.h>
+#include <drm/drm_accel.h>
#include <drm/drm_debugfs.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
@@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
};
-DEFINE_DRM_GEM_FOPS(fops);
+DEFINE_DRM_GEM_FOPS(render_fops);
+DEFINE_DRM_ACCEL_FOPS(accel_fops);
-static const struct drm_driver etnaviv_drm_driver = {
- .driver_features = DRIVER_GEM | DRIVER_RENDER,
+static struct drm_driver etnaviv_drm_driver = {
.open = etnaviv_open,
.postclose = etnaviv_postclose,
.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
@@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
#endif
.ioctls = etnaviv_ioctls,
.num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
- .fops = &fops,
.name = "etnaviv",
.desc = "etnaviv DRM",
.date = "20151214",
@@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
.minor = 4,
};
-/*
- * Platform driver:
- */
-static int etnaviv_bind(struct device *dev)
+static int etnaviv_bind_with_type(struct device *dev, u32 type)
{
struct etnaviv_drm_private *priv;
struct drm_device *drm;
+ bool is_compute_only = true;
int ret;
+ etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
+
+ if (type == DRIVER_RENDER)
+ etnaviv_drm_driver.fops = &render_fops;
+ else
+ etnaviv_drm_driver.fops = &accel_fops;
+
drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
if (IS_ERR(drm))
return PTR_ERR(drm);
@@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
load_gpu(drm);
+ for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
+ struct etnaviv_gpu *g = priv->gpu[i];
+
+ if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
+ is_compute_only = false;
+ }
+
+ if (type == DRIVER_RENDER && is_compute_only) {
+ ret = -EINVAL;
+ goto out_unbind;
+ }
+
ret = drm_dev_register(drm, 0);
if (ret)
goto out_unbind;
@@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
return ret;
}
+/*
+ * Platform driver:
+ */
+static int etnaviv_bind(struct device *dev)
+{
+ int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
+
+ if (ret == -EINVAL)
+ return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
+
+ return ret;
+}
+
static void etnaviv_unbind(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
--
2.44.0
Hi Tomeu,
>
> If we expose a render node for NPUs without rendering capabilities, the
> userspace stack will offer it to compositors and applications for
> rendering, which of course won't work.
>
> Userspace is probably right in not questioning whether a render node
> might not be capable of supporting rendering, so change it in the kernel
> instead by exposing a /dev/accel node.
>
> Before we bring the device up we don't know whether it is capable of
> rendering or not (depends on the features of its blocks), so first try
> to probe a rendering node, and if we find out that there is no rendering
> hardware, abort and retry with an accel node.
>
I really love this idea of moving away from a render node. What needs to be done
on the userspace side?
> Signed-off-by: Tomeu Vizoso <[email protected]>
> Cc: Oded Gabbay <[email protected]>
Reviewed-by: Christian Gmeiner <[email protected]>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..8e7dd23115f4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -11,6 +11,7 @@
> #include <linux/platform_device.h>
> #include <linux/uaccess.h>
>
> +#include <drm/drm_accel.h>
> #include <drm/drm_debugfs.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +DEFINE_DRM_GEM_FOPS(render_fops);
> +DEFINE_DRM_ACCEL_FOPS(accel_fops);
>
> -static const struct drm_driver etnaviv_drm_driver = {
> - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> +static struct drm_driver etnaviv_drm_driver = {
> .open = etnaviv_open,
> .postclose = etnaviv_postclose,
> .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> #endif
> .ioctls = etnaviv_ioctls,
> .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> - .fops = &fops,
> .name = "etnaviv",
> .desc = "etnaviv DRM",
> .date = "20151214",
> @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> .minor = 4,
> };
>
> -/*
> - * Platform driver:
> - */
> -static int etnaviv_bind(struct device *dev)
> +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> {
> struct etnaviv_drm_private *priv;
> struct drm_device *drm;
> + bool is_compute_only = true;
> int ret;
>
> + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> +
> + if (type == DRIVER_RENDER)
> + etnaviv_drm_driver.fops = &render_fops;
> + else
> + etnaviv_drm_driver.fops = &accel_fops;
> +
> drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
> if (IS_ERR(drm))
> return PTR_ERR(drm);
> @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
>
> load_gpu(drm);
>
> + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *g = priv->gpu[i];
> +
> + if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
> + is_compute_only = false;
> + }
> +
> + if (type == DRIVER_RENDER && is_compute_only) {
> + ret = -EINVAL;
> + goto out_unbind;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret)
> goto out_unbind;
> @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> return ret;
> }
>
> +/*
> + * Platform driver:
> + */
> +static int etnaviv_bind(struct device *dev)
> +{
> + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> +
> + if (ret == -EINVAL)
> + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> +
> + return ret;
> +}
> +
> static void etnaviv_unbind(struct device *dev)
> {
> struct drm_device *drm = dev_get_drvdata(dev);
> --
> 2.44.0
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
On 4/24/2024 12:37 AM, Tomeu Vizoso wrote:
> If we expose a render node for NPUs without rendering capabilities, the
> userspace stack will offer it to compositors and applications for
> rendering, which of course won't work.
>
> Userspace is probably right in not questioning whether a render node
> might not be capable of supporting rendering, so change it in the kernel
> instead by exposing a /dev/accel node.
>
> Before we bring the device up we don't know whether it is capable of
> rendering or not (depends on the features of its blocks), so first try
> to probe a rendering node, and if we find out that there is no rendering
> hardware, abort and retry with an accel node.
>
> Signed-off-by: Tomeu Vizoso <[email protected]>
> Cc: Oded Gabbay <[email protected]>
I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had
also previously mentioned they'd have opinions on what is Accel vs DRM.
This gets a nack from me in its current state. This is not a strong
nack, and I don't want to discourage you. I think there is a path forward.
The Accel subsystem documentation says that accel drivers will reside in
drivers/accel/ but this does not.
Also, the commit text for "accel: add dedicated minor for accelerator
devices" mentions -
"for drivers that
declare they handle compute accelerator, using a new driver feature
flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
driver feature is mutually exclusive with DRIVER_RENDER. Devices that
want to expose both graphics and compute device char files should be
handled by two drivers that are connected using the auxiliary bus
framework."
I don't see any of that happening here (two drivers connected by aux
bus, one in drivers/accel).
I think this is the first case we've had of a combo DRM/Accel usecase,
and so there isn't an existing example to refer you to on how to
structure things. I think you are going to be the first example where
we figure all of this out.
On a more implementation note, ioctls for Accel devices should not be
marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of
the code as possible trips over this.
-Jeff
On Thu, Apr 25, 2024 at 1:32 PM Christian Gmeiner
<[email protected]> wrote:
>
> Hi Tomeu,
>
> >
> > If we expose a render node for NPUs without rendering capabilities, the
> > userspace stack will offer it to compositors and applications for
> > rendering, which of course won't work.
> >
> > Userspace is probably right in not questioning whether a render node
> > might not be capable of supporting rendering, so change it in the kernel
> > instead by exposing a /dev/accel node.
> >
> > Before we bring the device up we don't know whether it is capable of
> > rendering or not (depends on the features of its blocks), so first try
> > to probe a rendering node, and if we find out that there is no rendering
> > hardware, abort and retry with an accel node.
> >
>
> I really love this idea of moving away from a render node. What needs to be done
> on the userspace side?
Doesn't seem that bad, here is a proof of concept:
https://gitlab.freedesktop.org/tomeu/mesa/-/tree/teflon-accel
Thanks for taking a look.
Tomeu
> > Signed-off-by: Tomeu Vizoso <[email protected]>
> > Cc: Oded Gabbay <[email protected]>
>
> Reviewed-by: Christian Gmeiner <[email protected]>
>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
> > 1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6500f3999c5f..8e7dd23115f4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -11,6 +11,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/uaccess.h>
> >
> > +#include <drm/drm_accel.h>
> > #include <drm/drm_debugfs.h>
> > #include <drm/drm_drv.h>
> > #include <drm/drm_file.h>
> > @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > };
> >
> > -DEFINE_DRM_GEM_FOPS(fops);
> > +DEFINE_DRM_GEM_FOPS(render_fops);
> > +DEFINE_DRM_ACCEL_FOPS(accel_fops);
> >
> > -static const struct drm_driver etnaviv_drm_driver = {
> > - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> > +static struct drm_driver etnaviv_drm_driver = {
> > .open = etnaviv_open,
> > .postclose = etnaviv_postclose,
> > .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> > #endif
> > .ioctls = etnaviv_ioctls,
> > .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> > - .fops = &fops,
> > .name = "etnaviv",
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .minor = 4,
> > };
> >
> > -/*
> > - * Platform driver:
> > - */
> > -static int etnaviv_bind(struct device *dev)
> > +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> > {
> > struct etnaviv_drm_private *priv;
> > struct drm_device *drm;
> > + bool is_compute_only = true;
> > int ret;
> >
> > + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> > +
> > + if (type == DRIVER_RENDER)
> > + etnaviv_drm_driver.fops = &render_fops;
> > + else
> > + etnaviv_drm_driver.fops = &accel_fops;
> > +
> > drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
> > if (IS_ERR(drm))
> > return PTR_ERR(drm);
> > @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
> >
> > load_gpu(drm);
> >
> > + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
> > + is_compute_only = false;
> > + }
> > +
> > + if (type == DRIVER_RENDER && is_compute_only) {
> > + ret = -EINVAL;
> > + goto out_unbind;
> > + }
> > +
> > ret = drm_dev_register(drm, 0);
> > if (ret)
> > goto out_unbind;
> > @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> > return ret;
> > }
> >
> > +/*
> > + * Platform driver:
> > + */
> > +static int etnaviv_bind(struct device *dev)
> > +{
> > + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> > +
> > + if (ret == -EINVAL)
> > + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> > +
> > + return ret;
> > +}
> > +
> > static void etnaviv_unbind(struct device *dev)
> > {
> > struct drm_device *drm = dev_get_drvdata(dev);
> > --
> > 2.44.0
> >
>
>
> --
> greets
> --
> Christian Gmeiner, MSc
>
> https://christian-gmeiner.info/privacypolicy
On Thu, Apr 25, 2024 at 8:59 PM Jeffrey Hugo <[email protected]> wrote:
>
> On 4/24/2024 12:37 AM, Tomeu Vizoso wrote:
> > If we expose a render node for NPUs without rendering capabilities, the
> > userspace stack will offer it to compositors and applications for
> > rendering, which of course won't work.
> >
> > Userspace is probably right in not questioning whether a render node
> > might not be capable of supporting rendering, so change it in the kernel
> > instead by exposing a /dev/accel node.
> >
> > Before we bring the device up we don't know whether it is capable of
> > rendering or not (depends on the features of its blocks), so first try
> > to probe a rendering node, and if we find out that there is no rendering
> > hardware, abort and retry with an accel node.
> >
> > Signed-off-by: Tomeu Vizoso <[email protected]>
> > Cc: Oded Gabbay <[email protected]>
>
> I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had
> also previously mentioned they'd have opinions on what is Accel vs DRM.
>
> This gets a nack from me in its current state. This is not a strong
> nack, and I don't want to discourage you. I think there is a path forward.
>
> The Accel subsystem documentation says that accel drivers will reside in
> drivers/accel/ but this does not.
Indeed, there is that code organization aspect.
> Also, the commit text for "accel: add dedicated minor for accelerator
> devices" mentions -
>
> "for drivers that
> declare they handle compute accelerator, using a new driver feature
> flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
> driver feature is mutually exclusive with DRIVER_RENDER. Devices that
> want to expose both graphics and compute device char files should be
> handled by two drivers that are connected using the auxiliary bus
> framework."
>
> I don't see any of that happening here (two drivers connected by aux
> bus, one in drivers/accel).
Well, the text refers to devices, not drivers. The case we are talking
about is a driver that wants to sometimes expose an accel node, and
sometimes a render node, depending on the hardware it is dealing with.
So there would either be a device exposing a single render node, or a
device exposing a single accel node.
Though by using the auxiliary bus we could in theory solve the code
organization problem mentioned above, I'm not quite seeing how to do
this in a clean way. The driver in /drivers/gpu/drm would have to be a
DRM driver that doesn't register a DRM device, but registers a device
in the auxiliary bus for the driver in /drivers/accel to bind to? Or
are you seeing some possibility that would fit better in the current
DRM framework?
> I think this is the first case we've had of a combo DRM/Accel usecase,
> and so there isn't an existing example to refer you to on how to
> structure things. I think you are going to be the first example where
> we figure all of this out.
Yep, I will be grateful for any ideas on how to structure this.
> On a more implementation note, ioctls for Accel devices should not be
> marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of
> the code as possible trips over this.
Indeed, thanks.
Cheers,
Tomeu
> -Jeff
Oded, Dave,
Do you have an opinion on this?
Thanks,
Tomeu
On Fri, Apr 26, 2024 at 8:10 AM Tomeu Vizoso <[email protected]> wrote:
>
> On Thu, Apr 25, 2024 at 8:59 PM Jeffrey Hugo <[email protected]> wrote:
> >
> > On 4/24/2024 12:37 AM, Tomeu Vizoso wrote:
> > > If we expose a render node for NPUs without rendering capabilities, the
> > > userspace stack will offer it to compositors and applications for
> > > rendering, which of course won't work.
> > >
> > > Userspace is probably right in not questioning whether a render node
> > > might not be capable of supporting rendering, so change it in the kernel
> > > instead by exposing a /dev/accel node.
> > >
> > > Before we bring the device up we don't know whether it is capable of
> > > rendering or not (depends on the features of its blocks), so first try
> > > to probe a rendering node, and if we find out that there is no rendering
> > > hardware, abort and retry with an accel node.
> > >
> > > Signed-off-by: Tomeu Vizoso <[email protected]>
> > > Cc: Oded Gabbay <[email protected]>
> >
> > I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had
> > also previously mentioned they'd have opinions on what is Accel vs DRM.
> >
> > This gets a nack from me in its current state. This is not a strong
> > nack, and I don't want to discourage you. I think there is a path forward.
> >
> > The Accel subsystem documentation says that accel drivers will reside in
> > drivers/accel/ but this does not.
>
> Indeed, there is that code organization aspect.
>
> > Also, the commit text for "accel: add dedicated minor for accelerator
> > devices" mentions -
> >
> > "for drivers that
> > declare they handle compute accelerator, using a new driver feature
> > flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
> > driver feature is mutually exclusive with DRIVER_RENDER. Devices that
> > want to expose both graphics and compute device char files should be
> > handled by two drivers that are connected using the auxiliary bus
> > framework."
> >
> > I don't see any of that happening here (two drivers connected by aux
> > bus, one in drivers/accel).
>
> Well, the text refers to devices, not drivers. The case we are talking
> about is a driver that wants to sometimes expose an accel node, and
> sometimes a render node, depending on the hardware it is dealing with.
> So there would either be a device exposing a single render node, or a
> device exposing a single accel node.
>
> Though by using the auxiliary bus we could in theory solve the code
> organization problem mentioned above, I'm not quite seeing how to do
> this in a clean way. The driver in /drivers/gpu/drm would have to be a
> DRM driver that doesn't register a DRM device, but registers a device
> in the auxiliary bus for the driver in /drivers/accel to bind to? Or
> are you seeing some possibility that would fit better in the current
> DRM framework?
>
> > I think this is the first case we've had of a combo DRM/Accel usecase,
> > and so there isn't an existing example to refer you to on how to
> > structure things. I think you are going to be the first example where
> > we figure all of this out.
>
> Yep, I will be grateful for any ideas on how to structure this.
>
> > On a more implementation note, ioctls for Accel devices should not be
> > marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of
> > the code as possible trips over this.
>
> Indeed, thanks.
>
> Cheers,
>
> Tomeu
>
> > -Jeff
On Thu, May 09, 2024 at 03:53:01PM +0200, Tomeu Vizoso wrote:
> Oded, Dave,
>
> Do you have an opinion on this?
>
> Thanks,
>
> Tomeu
Hi Tomeu,
Sorry for not replying earlier, I was down with Covid (again...).
To your question, I don't have an objection to what you are
suggesting. My personal view of accel is that it is an integral part of
DRM and therefore, if there is an *existing* drm driver that wants to
create an accel node, I'm not against it.
There is the question of why you want to expose an accel node, and
here I would like to hear Dave's and Sima's opinion on your suggested
solution as it may affect the direction of other drm drivers.
Thanks,
Oded.
p.s.
Please only use bottom-posting when replying, thanks :)
>
> On Fri, Apr 26, 2024 at 8:10 AM Tomeu Vizoso <[email protected]> wrote:
> >
> > On Thu, Apr 25, 2024 at 8:59 PM Jeffrey Hugo <[email protected]> wrote:
> > >
> > > On 4/24/2024 12:37 AM, Tomeu Vizoso wrote:
> > > > If we expose a render node for NPUs without rendering capabilities, the
> > > > userspace stack will offer it to compositors and applications for
> > > > rendering, which of course won't work.
> > > >
> > > > Userspace is probably right in not questioning whether a render node
> > > > might not be capable of supporting rendering, so change it in the kernel
> > > > instead by exposing a /dev/accel node.
> > > >
> > > > Before we bring the device up we don't know whether it is capable of
> > > > rendering or not (depends on the features of its blocks), so first try
> > > > to probe a rendering node, and if we find out that there is no rendering
> > > > hardware, abort and retry with an accel node.
> > > >
> > > > Signed-off-by: Tomeu Vizoso <[email protected]>
> > > > Cc: Oded Gabbay <[email protected]>
> > >
> > > I hope Oded chimes in as Accel maintainer. I think Airlie/Vetter had
> > > also previously mentioned they'd have opinions on what is Accel vs DRM.
> > >
> > > This gets a nack from me in its current state. This is not a strong
> > > nack, and I don't want to discourage you. I think there is a path forward.
> > >
> > > The Accel subsystem documentation says that accel drivers will reside in
> > > drivers/accel/ but this does not.
> >
> > Indeed, there is that code organization aspect.
> >
> > > Also, the commit text for "accel: add dedicated minor for accelerator
> > > devices" mentions -
> > >
> > > "for drivers that
> > > declare they handle compute accelerator, using a new driver feature
> > > flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
> > > driver feature is mutually exclusive with DRIVER_RENDER. Devices that
> > > want to expose both graphics and compute device char files should be
> > > handled by two drivers that are connected using the auxiliary bus
> > > framework."
> > >
> > > I don't see any of that happening here (two drivers connected by aux
> > > bus, one in drivers/accel).
> >
> > Well, the text refers to devices, not drivers. The case we are talking
> > about is a driver that wants to sometimes expose an accel node, and
> > sometimes a render node, depending on the hardware it is dealing with.
> > So there would either be a device exposing a single render node, or a
> > device exposing a single accel node.
> >
> > Though by using the auxiliary bus we could in theory solve the code
> > organization problem mentioned above, I'm not quite seeing how to do
> > this in a clean way. The driver in /drivers/gpu/drm would have to be a
> > DRM driver that doesn't register a DRM device, but registers a device
> > in the auxiliary bus for the driver in /drivers/accel to bind to? Or
> > are you seeing some possibility that would fit better in the current
> > DRM framework?
> >
> > > I think this is the first case we've had of a combo DRM/Accel usecase,
> > > and so there isn't an existing example to refer you to on how to
> > > structure things. I think you are going to be the first example where
> > > we figure all of this out.
> >
> > Yep, I will be grateful for any ideas on how to structure this.
> >
> > > On a more implementation note, ioctls for Accel devices should not be
> > > marked DRM_RENDER_ALLOW. Seems like your attempt to reuse as much of
> > > the code as possible trips over this.
> >
> > Indeed, thanks.
> >
> > Cheers,
> >
> > Tomeu
> >
> > > -Jeff
Hi Tomeu,
Am Mittwoch, dem 24.04.2024 um 08:37 +0200 schrieb Tomeu Vizoso:
> If we expose a render node for NPUs without rendering capabilities, the
> userspace stack will offer it to compositors and applications for
> rendering, which of course won't work.
>
> Userspace is probably right in not questioning whether a render node
> might not be capable of supporting rendering, so change it in the kernel
> instead by exposing a /dev/accel node.
>
> Before we bring the device up we don't know whether it is capable of
> rendering or not (depends on the features of its blocks), so first try
> to probe a rendering node, and if we find out that there is no rendering
> hardware, abort and retry with an accel node.
>
I thought about this for a while. My opinion is that this is the wrong
approach. We are adding another path to the kernel driver, potentially
complicating the userspace side, as now the NPU backend needs to look
for both render and accel nodes. While currently accel and drm are
pretty closely related and we can share most of the driver, it might
still be a maintenance hassle in the long run.
On the other hand we already have precedence of compute only DRM
devices exposing a render node: there are AMD GPUs that don't expose a
graphics queue and are thus not able to actually render graphics. Mesa
already handles this in part via the PIPE_CAP_GRAPHICS and I think we
should simply extend this to not offer a EGL display on screens without
that capability.
Regards,
Lucas
> Signed-off-by: Tomeu Vizoso <[email protected]>
> Cc: Oded Gabbay <[email protected]>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..8e7dd23115f4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -11,6 +11,7 @@
> #include <linux/platform_device.h>
> #include <linux/uaccess.h>
>
> +#include <drm/drm_accel.h>
> #include <drm/drm_debugfs.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +DEFINE_DRM_GEM_FOPS(render_fops);
> +DEFINE_DRM_ACCEL_FOPS(accel_fops);
>
> -static const struct drm_driver etnaviv_drm_driver = {
> - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> +static struct drm_driver etnaviv_drm_driver = {
> .open = etnaviv_open,
> .postclose = etnaviv_postclose,
> .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> #endif
> .ioctls = etnaviv_ioctls,
> .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> - .fops = &fops,
> .name = "etnaviv",
> .desc = "etnaviv DRM",
> .date = "20151214",
> @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> .minor = 4,
> };
>
> -/*
> - * Platform driver:
> - */
> -static int etnaviv_bind(struct device *dev)
> +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> {
> struct etnaviv_drm_private *priv;
> struct drm_device *drm;
> + bool is_compute_only = true;
> int ret;
>
> + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> +
> + if (type == DRIVER_RENDER)
> + etnaviv_drm_driver.fops = &render_fops;
> + else
> + etnaviv_drm_driver.fops = &accel_fops;
> +
> drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
> if (IS_ERR(drm))
> return PTR_ERR(drm);
> @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
>
> load_gpu(drm);
>
> + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *g = priv->gpu[i];
> +
> + if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
> + is_compute_only = false;
> + }
> +
> + if (type == DRIVER_RENDER && is_compute_only) {
> + ret = -EINVAL;
> + goto out_unbind;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret)
> goto out_unbind;
> @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> return ret;
> }
>
> +/*
> + * Platform driver:
> + */
> +static int etnaviv_bind(struct device *dev)
> +{
> + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> +
> + if (ret == -EINVAL)
> + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> +
> + return ret;
> +}
> +
> static void etnaviv_unbind(struct device *dev)
> {
> struct drm_device *drm = dev_get_drvdata(dev);
Hi Lucas,
On Fri, May 10, 2024 at 10:34 AM Lucas Stach <[email protected]> wrote:
>
> Hi Tomeu,
>
> Am Mittwoch, dem 24.04.2024 um 08:37 +0200 schrieb Tomeu Vizoso:
> > If we expose a render node for NPUs without rendering capabilities, the
> > userspace stack will offer it to compositors and applications for
> > rendering, which of course won't work.
> >
> > Userspace is probably right in not questioning whether a render node
> > might not be capable of supporting rendering, so change it in the kernel
> > instead by exposing a /dev/accel node.
> >
> > Before we bring the device up we don't know whether it is capable of
> > rendering or not (depends on the features of its blocks), so first try
> > to probe a rendering node, and if we find out that there is no rendering
> > hardware, abort and retry with an accel node.
> >
> I thought about this for a while. My opinion is that this is the wrong
> approach. We are adding another path to the kernel driver, potentially
> complicating the userspace side, as now the NPU backend needs to look
> for both render and accel nodes. While currently accel and drm are
> pretty closely related and we can share most of the driver, it might
> still be a maintenance hassle in the long run.
>
> On the other hand we already have precedence of compute only DRM
> devices exposing a render node: there are AMD GPUs that don't expose a
> graphics queue and are thus not able to actually render graphics. Mesa
> already handles this in part via the PIPE_CAP_GRAPHICS and I think we
> should simply extend this to not offer a EGL display on screens without
> that capability.
The problem with this is that the compositors I know don't loop over
/dev/dri files, trying to create EGL screens and moving to the next
one until they find one that works.
They take the first render node (unless a specific one has been
configured), and assumes it will be able to render with it.
To me it seems as if userspace expects that /dev/dri/renderD* devices
can be used for rendering and by breaking this assumption we would be
breaking existing software.
Which is what I understood to be the whole point behind the decision
to create a new device file hierarchy for accelerators. Or am I
missing something?
Adding Daniel Stone to CC in case he wants to give his opinion from
the compositor point of view.
Cheers,
Tomeu
> Regards,
> Lucas
>
> > Signed-off-by: Tomeu Vizoso <[email protected]>
> > Cc: Oded Gabbay <[email protected]>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
> > 1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6500f3999c5f..8e7dd23115f4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -11,6 +11,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/uaccess.h>
> >
> > +#include <drm/drm_accel.h>
> > #include <drm/drm_debugfs.h>
> > #include <drm/drm_drv.h>
> > #include <drm/drm_file.h>
> > @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > };
> >
> > -DEFINE_DRM_GEM_FOPS(fops);
> > +DEFINE_DRM_GEM_FOPS(render_fops);
> > +DEFINE_DRM_ACCEL_FOPS(accel_fops);
> >
> > -static const struct drm_driver etnaviv_drm_driver = {
> > - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> > +static struct drm_driver etnaviv_drm_driver = {
> > .open = etnaviv_open,
> > .postclose = etnaviv_postclose,
> > .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> > #endif
> > .ioctls = etnaviv_ioctls,
> > .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> > - .fops = &fops,
> > .name = "etnaviv",
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .minor = 4,
> > };
> >
> > -/*
> > - * Platform driver:
> > - */
> > -static int etnaviv_bind(struct device *dev)
> > +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> > {
> > struct etnaviv_drm_private *priv;
> > struct drm_device *drm;
> > + bool is_compute_only = true;
> > int ret;
> >
> > + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> > +
> > + if (type == DRIVER_RENDER)
> > + etnaviv_drm_driver.fops = &render_fops;
> > + else
> > + etnaviv_drm_driver.fops = &accel_fops;
> > +
> > drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
> > if (IS_ERR(drm))
> > return PTR_ERR(drm);
> > @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
> >
> > load_gpu(drm);
> >
> > + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
> > + is_compute_only = false;
> > + }
> > +
> > + if (type == DRIVER_RENDER && is_compute_only) {
> > + ret = -EINVAL;
> > + goto out_unbind;
> > + }
> > +
> > ret = drm_dev_register(drm, 0);
> > if (ret)
> > goto out_unbind;
> > @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> > return ret;
> > }
> >
> > +/*
> > + * Platform driver:
> > + */
> > +static int etnaviv_bind(struct device *dev)
> > +{
> > + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> > +
> > + if (ret == -EINVAL)
> > + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> > +
> > + return ret;
> > +}
> > +
> > static void etnaviv_unbind(struct device *dev)
> > {
> > struct drm_device *drm = dev_get_drvdata(dev);
>
On Fri, May 10, 2024 at 10:34 AM Lucas Stach <[email protected]> wrote:
>
> Hi Tomeu,
>
> Am Mittwoch, dem 24.04.2024 um 08:37 +0200 schrieb Tomeu Vizoso:
> > If we expose a render node for NPUs without rendering capabilities, the
> > userspace stack will offer it to compositors and applications for
> > rendering, which of course won't work.
> >
> > Userspace is probably right in not questioning whether a render node
> > might not be capable of supporting rendering, so change it in the kernel
> > instead by exposing a /dev/accel node.
> >
> > Before we bring the device up we don't know whether it is capable of
> > rendering or not (depends on the features of its blocks), so first try
> > to probe a rendering node, and if we find out that there is no rendering
> > hardware, abort and retry with an accel node.
> >
> I thought about this for a while. My opinion is that this is the wrong
> approach. We are adding another path to the kernel driver, potentially
> complicating the userspace side, as now the NPU backend needs to look
> for both render and accel nodes.
Forgot to mention in my earlier reply today that with the proposed
solution no changes are needed in the Gallium drivers, only in the
pipeloader component in Mesa, and in the Gallium frontends.
But those changes are needed anyway to support the upcoming
compute-only NPUs, such as Rockchip's.
These are the changes I needed to make to the userspace to go with
this kernel patch:
https://gitlab.freedesktop.org/tomeu/mesa/-/commit/6b0db4cce406c574d2b7710208df9c8bd1ab6345
Cheers,
Tomeu
> While currently accel and drm are
> pretty closely related and we can share most of the driver, it might
> still be a maintenance hassle in the long run.
>
> On the other hand we already have precedence of compute only DRM
> devices exposing a render node: there are AMD GPUs that don't expose a
> graphics queue and are thus not able to actually render graphics. Mesa
> already handles this in part via the PIPE_CAP_GRAPHICS and I think we
> should simply extend this to not offer a EGL display on screens without
> that capability.
>
> Regards,
> Lucas
>
> > Signed-off-by: Tomeu Vizoso <[email protected]>
> > Cc: Oded Gabbay <[email protected]>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++++++++++++++++++++++-----
> > 1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6500f3999c5f..8e7dd23115f4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -11,6 +11,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/uaccess.h>
> >
> > +#include <drm/drm_accel.h>
> > #include <drm/drm_debugfs.h>
> > #include <drm/drm_drv.h>
> > #include <drm/drm_file.h>
> > @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > };
> >
> > -DEFINE_DRM_GEM_FOPS(fops);
> > +DEFINE_DRM_GEM_FOPS(render_fops);
> > +DEFINE_DRM_ACCEL_FOPS(accel_fops);
> >
> > -static const struct drm_driver etnaviv_drm_driver = {
> > - .driver_features = DRIVER_GEM | DRIVER_RENDER,
> > +static struct drm_driver etnaviv_drm_driver = {
> > .open = etnaviv_open,
> > .postclose = etnaviv_postclose,
> > .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> > #endif
> > .ioctls = etnaviv_ioctls,
> > .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> > - .fops = &fops,
> > .name = "etnaviv",
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .minor = 4,
> > };
> >
> > -/*
> > - * Platform driver:
> > - */
> > -static int etnaviv_bind(struct device *dev)
> > +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> > {
> > struct etnaviv_drm_private *priv;
> > struct drm_device *drm;
> > + bool is_compute_only = true;
> > int ret;
> >
> > + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> > +
> > + if (type == DRIVER_RENDER)
> > + etnaviv_drm_driver.fops = &render_fops;
> > + else
> > + etnaviv_drm_driver.fops = &accel_fops;
> > +
> > drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
> > if (IS_ERR(drm))
> > return PTR_ERR(drm);
> > @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
> >
> > load_gpu(drm);
> >
> > + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g && (g->identity.minor_features8 & chipMinorFeatures8_COMPUTE_ONLY) == 0)
> > + is_compute_only = false;
> > + }
> > +
> > + if (type == DRIVER_RENDER && is_compute_only) {
> > + ret = -EINVAL;
> > + goto out_unbind;
> > + }
> > +
> > ret = drm_dev_register(drm, 0);
> > if (ret)
> > goto out_unbind;
> > @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> > return ret;
> > }
> >
> > +/*
> > + * Platform driver:
> > + */
> > +static int etnaviv_bind(struct device *dev)
> > +{
> > + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> > +
> > + if (ret == -EINVAL)
> > + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> > +
> > + return ret;
> > +}
> > +
> > static void etnaviv_unbind(struct device *dev)
> > {
> > struct drm_device *drm = dev_get_drvdata(dev);
>
Hi,
On Mon, 20 May 2024 at 08:39, Tomeu Vizoso <[email protected]> wrote:
> On Fri, May 10, 2024 at 10:34 AM Lucas Stach <l.stach@pengutronixde> wrote:
> > Am Mittwoch, dem 24.04.2024 um 08:37 +0200 schrieb Tomeu Vizoso:
> > > If we expose a render node for NPUs without rendering capabilities, the
> > > userspace stack will offer it to compositors and applications for
> > > rendering, which of course won't work.
> > >
> > > Userspace is probably right in not questioning whether a render node
> > > might not be capable of supporting rendering, so change it in the kernel
> > > instead by exposing a /dev/accel node.
> > >
> > > Before we bring the device up we don't know whether it is capable of
> > > rendering or not (depends on the features of its blocks), so first try
> > > to probe a rendering node, and if we find out that there is no rendering
> > > hardware, abort and retry with an accel node.
> >
> > On the other hand we already have precedence of compute only DRM
> > devices exposing a render node: there are AMD GPUs that don't expose a
> > graphics queue and are thus not able to actually render graphics. Mesa
> > already handles this in part via the PIPE_CAP_GRAPHICS and I think we
> > should simply extend this to not offer a EGL display on screens without
> > that capability.
>
> The problem with this is that the compositors I know don't loop over
> /dev/dri files, trying to create EGL screens and moving to the next
> one until they find one that works.
>
> They take the first render node (unless a specific one has been
> configured), and assumes it will be able to render with it.
>
> To me it seems as if userspace expects that /dev/dri/renderD* devices
> can be used for rendering and by breaking this assumption we would be
> breaking existing software.
Mm, it's sort of backwards from that. Compositors just take a
non-render DRM node for KMS, then ask GBM+EGL to instantiate a GPU
which can work with that. When run in headless mode, we don't take
render nodes directly, but instead just create an EGLDisplay or
VkPhysicalDevice and work backwards to a render node, rather than
selecting a render node and going from there.
So from that PoV I don't think it's really that harmful. The only
complication is in Mesa, where it would see an etnaviv/amdgpu/...
render node and potentially try to use it as a device. As long as Mesa
can correctly skip, there should be no userspace API implications.
That being said, I'm not entirely sure what the _benefit_ would be of
exposing a render node for a device which can't be used by any
'traditional' DRM consumers, i.e. GL/Vulkan/winsys.
Cheers,
Daniel
On Mon, May 20, 2024 at 1:19 PM Daniel Stone <[email protected]> wrote:
>
> Hi,
>
> On Mon, 20 May 2024 at 08:39, Tomeu Vizoso <[email protected]> wrote:
> > On Fri, May 10, 2024 at 10:34 AM Lucas Stach <[email protected]> wrote:
> > > Am Mittwoch, dem 24.04.2024 um 08:37 +0200 schrieb Tomeu Vizoso:
> > > > If we expose a render node for NPUs without rendering capabilities, the
> > > > userspace stack will offer it to compositors and applications for
> > > > rendering, which of course won't work.
> > > >
> > > > Userspace is probably right in not questioning whether a render node
> > > > might not be capable of supporting rendering, so change it in the kernel
> > > > instead by exposing a /dev/accel node.
> > > >
> > > > Before we bring the device up we don't know whether it is capable of
> > > > rendering or not (depends on the features of its blocks), so first try
> > > > to probe a rendering node, and if we find out that there is no rendering
> > > > hardware, abort and retry with an accel node.
> > >
> > > On the other hand we already have precedence of compute only DRM
> > > devices exposing a render node: there are AMD GPUs that don't expose a
> > > graphics queue and are thus not able to actually render graphics. Mesa
> > > already handles this in part via the PIPE_CAP_GRAPHICS and I think we
> > > should simply extend this to not offer a EGL display on screens without
> > > that capability.
> >
> > The problem with this is that the compositors I know don't loop over
> > /dev/dri files, trying to create EGL screens and moving to the next
> > one until they find one that works.
> >
> > They take the first render node (unless a specific one has been
> > configured), and assumes it will be able to render with it.
> >
> > To me it seems as if userspace expects that /dev/dri/renderD* devices
> > can be used for rendering and by breaking this assumption we would be
> > breaking existing software.
>
> Mm, it's sort of backwards from that. Compositors just take a
> non-render DRM node for KMS, then ask GBM+EGL to instantiate a GPU
> which can work with that. When run in headless mode, we don't take
> render nodes directly, but instead just create an EGLDisplay or
> VkPhysicalDevice and work backwards to a render node, rather than
> selecting a render node and going from there.
>
> So from that PoV I don't think it's really that harmful. The only
> complication is in Mesa, where it would see an etnaviv/amdgpu/...
> render node and potentially try to use it as a device. As long as Mesa
> can correctly skip, there should be no userspace API implications.
>
> That being said, I'm not entirely sure what the _benefit_ would be of
> exposing a render node for a device which can't be used by any
> 'traditional' DRM consumers, i.e. GL/Vulkan/winsys.
What I don't understand yet from Lucas proposal is how this isn't
going to break existing userspace.
I mean, even if we find a good way of having userspace skip
non-rendering render nodes, what about existing userspace that isn't
able to do that? Any updates to newer kernels are going to break them.
Regards,
Tomeu