[1/2] mesa: make gallium swrast target optional

Submitted by Marco Felsch on June 13, 2019, 5:54 p.m. | Patch ID: 162184

Details

Message ID 20190613175442.26121-1-m.felsch@pengutronix.de
State Master Next
Commit 319d8ae8e19e29d2aff71b1ebef5154fe3c78bb3
Headers show

Commit Message

Marco Felsch June 13, 2019, 5:54 p.m.
Most of the time we are compiling for embedded targets which have
dedicated hardware combinations. Enabling swrast by default isn't a good
solution for such devices because if the hardware render node has an
issue or doesn't support a special format/request Mesa will fallback to
the software renderer. This will make it harder to debug performance
issues.

A better way is to let the user decide if a software renderer is
needed e.g. if the system has no hardware renderer or to have such a
fallback device. This way the user knows that the software renderer is
enabled.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v3
- rebased on current master-next branch

 meta/recipes-graphics/mesa/mesa.inc | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/meta/recipes-graphics/mesa/mesa.inc b/meta/recipes-graphics/mesa/mesa.inc
index 3ecfb8506c..a6d36cf21c 100644
--- a/meta/recipes-graphics/mesa/mesa.inc
+++ b/meta/recipes-graphics/mesa/mesa.inc
@@ -90,23 +90,27 @@  PACKAGECONFIG[egl] = "-Degl=true, -Degl=false"
 
 PACKAGECONFIG[etnaviv] = ""
 PACKAGECONFIG[kmsro] = ""
+PACKAGECONFIG[swrast] = ""
 
-GALLIUMDRIVERS = "swrast"
-GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv', ',etnaviv', '', d)}"
-GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'kmsro', ',kmsro', '', d)}"
+GALLIUMDRIVERS = ""
+GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'swrast', 'swrast', '', d)}"
+GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv', 'etnaviv', '', d)}"
+GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'kmsro', 'kmsro', '', d)}"
 
 # radeonsi requires LLVM
-GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600', ',radeonsi', '', d)}"
+GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'radeonsi', '', d)}"
 GALLIUMDRIVERS_LLVM33_ENABLED = "${@oe.utils.version_less_or_equal('MESA_LLVM_RELEASE', '3.2', False, len('${GALLIUMDRIVERS_LLVM33}') > 0, d)}"
-GALLIUMDRIVERS_LLVM = "r300,svga,nouveau${@',${GALLIUMDRIVERS_LLVM33}' if ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
+GALLIUMDRIVERS_LLVM = "r300 svga nouveau ${@'${GALLIUMDRIVERS_LLVM33}' if ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
 
 PACKAGECONFIG[r600] = ""
 
-GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG', 'gallium-llvm', ',${GALLIUMDRIVERS_LLVM}', '', d)}"
-GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG', 'r600', ',r600', '', d)}"
-GALLIUMDRIVERS_append = ",virgl"
+GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}"
+GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'r600', '', d)}"
+GALLIUMDRIVERS += "virgl"
+GALLIUMDRIVERS_MESON = "${@",".join("${GALLIUMDRIVERS}".split())}"
+
+PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS_MESON}, -Dgallium-drivers=''"
 
-PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS}, -Dgallium-drivers=''"
 MESA_LLVM_RELEASE ?= "8.0.0"
 PACKAGECONFIG[gallium-llvm] = "-Dllvm=true -Dshared-llvm=true, -Dllvm=false, llvm${MESA_LLVM_RELEASE} llvm-native \
                                ${@'elfutils' if ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"

Comments

Martin Jansa June 13, 2019, 6:17 p.m.
Looks like nice cleanup, but is someone still using llvm 3.2 or older?

With PACKAGECONFIG for almost each gallium driver it might be easier to get
rid
of GALLIUMDRIVERS_LLVM33, GALLIUMDRIVERS_LLVM33_ENABLED, GALLIUMDRIVERS_LLVM
variables and use just GALLIUMDRIVERS.

In worst case scenario people using llvm 3.2 or older will need to redefine
some of these PACKAGECONFIGs but that shouldn't be big issue considering
that they probably have different mesa recipe already.

People can then select right list of drivers with or without
using gallium-llvm as well.


On Thu, Jun 13, 2019 at 7:54 PM Marco Felsch <m.felsch@pengutronix.de>
wrote:

> Most of the time we are compiling for embedded targets which have
> dedicated hardware combinations. Enabling swrast by default isn't a good
> solution for such devices because if the hardware render node has an
> issue or doesn't support a special format/request Mesa will fallback to
> the software renderer. This will make it harder to debug performance
> issues.
>
> A better way is to let the user decide if a software renderer is
> needed e.g. if the system has no hardware renderer or to have such a
> fallback device. This way the user knows that the software renderer is
> enabled.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> v3
> - rebased on current master-next branch
>
>  meta/recipes-graphics/mesa/mesa.inc | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/meta/recipes-graphics/mesa/mesa.inc
> b/meta/recipes-graphics/mesa/mesa.inc
> index 3ecfb8506c..a6d36cf21c 100644
> --- a/meta/recipes-graphics/mesa/mesa.inc
> +++ b/meta/recipes-graphics/mesa/mesa.inc
> @@ -90,23 +90,27 @@ PACKAGECONFIG[egl] = "-Degl=true, -Degl=false"
>
>  PACKAGECONFIG[etnaviv] = ""
>  PACKAGECONFIG[kmsro] = ""
> +PACKAGECONFIG[swrast] = ""
>
> -GALLIUMDRIVERS = "swrast"
> -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv',
> ',etnaviv', '', d)}"
> -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'kmsro',
> ',kmsro', '', d)}"
> +GALLIUMDRIVERS = ""
> +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'swrast',
> 'swrast', '', d)}"
> +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv',
> 'etnaviv', '', d)}"
> +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'kmsro', 'kmsro',
> '', d)}"
>
>  # radeonsi requires LLVM
> -GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> ',radeonsi', '', d)}"
> +GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> 'radeonsi', '', d)}"
>  GALLIUMDRIVERS_LLVM33_ENABLED =
> "${@oe.utils.version_less_or_equal('MESA_LLVM_RELEASE', '3.2', False,
> len('${GALLIUMDRIVERS_LLVM33}') > 0, d)}"
> -GALLIUMDRIVERS_LLVM = "r300,svga,nouveau${@',${GALLIUMDRIVERS_LLVM33}' if
> ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> +GALLIUMDRIVERS_LLVM = "r300 svga nouveau ${@'${GALLIUMDRIVERS_LLVM33}' if
> ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
>
>  PACKAGECONFIG[r600] = ""
>
> -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG',
> 'gallium-llvm', ',${GALLIUMDRIVERS_LLVM}', '', d)}"
> -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> ',r600', '', d)}"
> -GALLIUMDRIVERS_append = ",virgl"
> +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'gallium-llvm',
> '${GALLIUMDRIVERS_LLVM}', '', d)}"
> +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'r600',
> '', d)}"
> +GALLIUMDRIVERS += "virgl"
> +GALLIUMDRIVERS_MESON = "${@",".join("${GALLIUMDRIVERS}".split())}"
> +
> +PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS_MESON},
> -Dgallium-drivers=''"
>
> -PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS},
> -Dgallium-drivers=''"
>  MESA_LLVM_RELEASE ?= "8.0.0"
>  PACKAGECONFIG[gallium-llvm] = "-Dllvm=true -Dshared-llvm=true,
> -Dllvm=false, llvm${MESA_LLVM_RELEASE} llvm-native \
>                                 ${@'elfutils' if
> ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> --
> 2.20.1
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
Marco Felsch June 13, 2019, 6:38 p.m.
Hi Martin,

On 19-06-13 20:17, Martin Jansa wrote:
> Looks like nice cleanup, but is someone still using llvm 3.2 or older?

I don't know but I learned to avoid breaking changes.

> With PACKAGECONFIG for almost each gallium driver it might be easier to get
> rid
> of GALLIUMDRIVERS_LLVM33, GALLIUMDRIVERS_LLVM33_ENABLED, GALLIUMDRIVERS_LLVM
> variables and use just GALLIUMDRIVERS.

Sure that will increase readability and drops unneeded variables :) But
can we add a follow up patch to address this?

> In worst case scenario people using llvm 3.2 or older will need to redefine
> some of these PACKAGECONFIGs but that shouldn't be big issue considering
> that they probably have different mesa recipe already.

I'm not that deep inside LLVM just googled it and saw that the current
version is 8.0.1 so maybe you are right.

> People can then select right list of drivers with or without
> using gallium-llvm as well.

As I wrote above we should address this by a seperate patch to keep the
diff as small as possible :)

Regards,
  Marco

> 
> 
> On Thu, Jun 13, 2019 at 7:54 PM Marco Felsch <m.felsch@pengutronix.de>
> wrote:
> 
> > Most of the time we are compiling for embedded targets which have
> > dedicated hardware combinations. Enabling swrast by default isn't a good
> > solution for such devices because if the hardware render node has an
> > issue or doesn't support a special format/request Mesa will fallback to
> > the software renderer. This will make it harder to debug performance
> > issues.
> >
> > A better way is to let the user decide if a software renderer is
> > needed e.g. if the system has no hardware renderer or to have such a
> > fallback device. This way the user knows that the software renderer is
> > enabled.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > v3
> > - rebased on current master-next branch
> >
> >  meta/recipes-graphics/mesa/mesa.inc | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/meta/recipes-graphics/mesa/mesa.inc
> > b/meta/recipes-graphics/mesa/mesa.inc
> > index 3ecfb8506c..a6d36cf21c 100644
> > --- a/meta/recipes-graphics/mesa/mesa.inc
> > +++ b/meta/recipes-graphics/mesa/mesa.inc
> > @@ -90,23 +90,27 @@ PACKAGECONFIG[egl] = "-Degl=true, -Degl=false"
> >
> >  PACKAGECONFIG[etnaviv] = ""
> >  PACKAGECONFIG[kmsro] = ""
> > +PACKAGECONFIG[swrast] = ""
> >
> > -GALLIUMDRIVERS = "swrast"
> > -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv',
> > ',etnaviv', '', d)}"
> > -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'kmsro',
> > ',kmsro', '', d)}"
> > +GALLIUMDRIVERS = ""
> > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'swrast',
> > 'swrast', '', d)}"
> > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv',
> > 'etnaviv', '', d)}"
> > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'kmsro', 'kmsro',
> > '', d)}"
> >
> >  # radeonsi requires LLVM
> > -GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > ',radeonsi', '', d)}"
> > +GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > 'radeonsi', '', d)}"
> >  GALLIUMDRIVERS_LLVM33_ENABLED =
> > "${@oe.utils.version_less_or_equal('MESA_LLVM_RELEASE', '3.2', False,
> > len('${GALLIUMDRIVERS_LLVM33}') > 0, d)}"
> > -GALLIUMDRIVERS_LLVM = "r300,svga,nouveau${@',${GALLIUMDRIVERS_LLVM33}' if
> > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> > +GALLIUMDRIVERS_LLVM = "r300 svga nouveau ${@'${GALLIUMDRIVERS_LLVM33}' if
> > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> >
> >  PACKAGECONFIG[r600] = ""
> >
> > -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG',
> > 'gallium-llvm', ',${GALLIUMDRIVERS_LLVM}', '', d)}"
> > -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > ',r600', '', d)}"
> > -GALLIUMDRIVERS_append = ",virgl"
> > +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'gallium-llvm',
> > '${GALLIUMDRIVERS_LLVM}', '', d)}"
> > +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'r600',
> > '', d)}"
> > +GALLIUMDRIVERS += "virgl"
> > +GALLIUMDRIVERS_MESON = "${@",".join("${GALLIUMDRIVERS}".split())}"
> > +
> > +PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS_MESON},
> > -Dgallium-drivers=''"
> >
> > -PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS},
> > -Dgallium-drivers=''"
> >  MESA_LLVM_RELEASE ?= "8.0.0"
> >  PACKAGECONFIG[gallium-llvm] = "-Dllvm=true -Dshared-llvm=true,
> > -Dllvm=false, llvm${MESA_LLVM_RELEASE} llvm-native \
> >                                 ${@'elfutils' if
> > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> > --
> > 2.20.1
> >
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> >
Martin Jansa June 13, 2019, 7:07 p.m.
Sure, follow-up patch is fine with me.

On Thu, Jun 13, 2019 at 8:38 PM Marco Felsch <m.felsch@pengutronix.de>
wrote:

> Hi Martin,
>
> On 19-06-13 20:17, Martin Jansa wrote:
> > Looks like nice cleanup, but is someone still using llvm 3.2 or older?
>
> I don't know but I learned to avoid breaking changes.
>
> > With PACKAGECONFIG for almost each gallium driver it might be easier to
> get
> > rid
> > of GALLIUMDRIVERS_LLVM33, GALLIUMDRIVERS_LLVM33_ENABLED,
> GALLIUMDRIVERS_LLVM
> > variables and use just GALLIUMDRIVERS.
>
> Sure that will increase readability and drops unneeded variables :) But
> can we add a follow up patch to address this?
>
> > In worst case scenario people using llvm 3.2 or older will need to
> redefine
> > some of these PACKAGECONFIGs but that shouldn't be big issue considering
> > that they probably have different mesa recipe already.
>
> I'm not that deep inside LLVM just googled it and saw that the current
> version is 8.0.1 so maybe you are right.
>
> > People can then select right list of drivers with or without
> > using gallium-llvm as well.
>
> As I wrote above we should address this by a seperate patch to keep the
> diff as small as possible :)
>
> Regards,
>   Marco
>
> >
> >
> > On Thu, Jun 13, 2019 at 7:54 PM Marco Felsch <m.felsch@pengutronix.de>
> > wrote:
> >
> > > Most of the time we are compiling for embedded targets which have
> > > dedicated hardware combinations. Enabling swrast by default isn't a
> good
> > > solution for such devices because if the hardware render node has an
> > > issue or doesn't support a special format/request Mesa will fallback to
> > > the software renderer. This will make it harder to debug performance
> > > issues.
> > >
> > > A better way is to let the user decide if a software renderer is
> > > needed e.g. if the system has no hardware renderer or to have such a
> > > fallback device. This way the user knows that the software renderer is
> > > enabled.
> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > > v3
> > > - rebased on current master-next branch
> > >
> > >  meta/recipes-graphics/mesa/mesa.inc | 22 +++++++++++++---------
> > >  1 file changed, 13 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/meta/recipes-graphics/mesa/mesa.inc
> > > b/meta/recipes-graphics/mesa/mesa.inc
> > > index 3ecfb8506c..a6d36cf21c 100644
> > > --- a/meta/recipes-graphics/mesa/mesa.inc
> > > +++ b/meta/recipes-graphics/mesa/mesa.inc
> > > @@ -90,23 +90,27 @@ PACKAGECONFIG[egl] = "-Degl=true, -Degl=false"
> > >
> > >  PACKAGECONFIG[etnaviv] = ""
> > >  PACKAGECONFIG[kmsro] = ""
> > > +PACKAGECONFIG[swrast] = ""
> > >
> > > -GALLIUMDRIVERS = "swrast"
> > > -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG',
> 'etnaviv',
> > > ',etnaviv', '', d)}"
> > > -GALLIUMDRIVERS_append ="${@bb.utils.contains('PACKAGECONFIG', 'kmsro',
> > > ',kmsro', '', d)}"
> > > +GALLIUMDRIVERS = ""
> > > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'swrast',
> > > 'swrast', '', d)}"
> > > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'etnaviv',
> > > 'etnaviv', '', d)}"
> > > +GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'kmsro',
> 'kmsro',
> > > '', d)}"
> > >
> > >  # radeonsi requires LLVM
> > > -GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > > ',radeonsi', '', d)}"
> > > +GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > > 'radeonsi', '', d)}"
> > >  GALLIUMDRIVERS_LLVM33_ENABLED =
> > > "${@oe.utils.version_less_or_equal('MESA_LLVM_RELEASE', '3.2', False,
> > > len('${GALLIUMDRIVERS_LLVM33}') > 0, d)}"
> > > -GALLIUMDRIVERS_LLVM =
> "r300,svga,nouveau${@',${GALLIUMDRIVERS_LLVM33}' if
> > > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> > > +GALLIUMDRIVERS_LLVM = "r300 svga nouveau
> ${@'${GALLIUMDRIVERS_LLVM33}' if
> > > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> > >
> > >  PACKAGECONFIG[r600] = ""
> > >
> > > -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG',
> > > 'gallium-llvm', ',${GALLIUMDRIVERS_LLVM}', '', d)}"
> > > -GALLIUMDRIVERS_append = "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> > > ',r600', '', d)}"
> > > -GALLIUMDRIVERS_append = ",virgl"
> > > +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG',
> 'gallium-llvm',
> > > '${GALLIUMDRIVERS_LLVM}', '', d)}"
> > > +GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'r600',
> 'r600',
> > > '', d)}"
> > > +GALLIUMDRIVERS += "virgl"
> > > +GALLIUMDRIVERS_MESON = "${@",".join("${GALLIUMDRIVERS}".split())}"
> > > +
> > > +PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS_MESON},
> > > -Dgallium-drivers=''"
> > >
> > > -PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS},
> > > -Dgallium-drivers=''"
> > >  MESA_LLVM_RELEASE ?= "8.0.0"
> > >  PACKAGECONFIG[gallium-llvm] = "-Dllvm=true -Dshared-llvm=true,
> > > -Dllvm=false, llvm${MESA_LLVM_RELEASE} llvm-native \
> > >                                 ${@'elfutils' if
> > > ${GALLIUMDRIVERS_LLVM33_ENABLED} else ''}"
> > > --
> > > 2.20.1
> > >
> > > --
> > > _______________________________________________
> > > Openembedded-core mailing list
> > > Openembedded-core@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> > >
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
Richard Purdie June 14, 2019, 10:55 a.m.
On Thu, 2019-06-13 at 19:54 +0200, Marco Felsch wrote:
> Most of the time we are compiling for embedded targets which have
> dedicated hardware combinations. Enabling swrast by default isn't a
> good
> solution for such devices because if the hardware render node has an
> issue or doesn't support a special format/request Mesa will fallback
> to
> the software renderer. This will make it harder to debug performance
> issues.
> 
> A better way is to let the user decide if a software renderer is
> needed e.g. if the system has no hardware renderer or to have such a
> fallback device. This way the user knows that the software renderer
> is
> enabled.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> v3
> - rebased on current master-next branch

I think this breaks the autobuilder:

https://autobuilder.yoctoproject.org/typhoon/#/builders/57/builds/694

Cheers,

Richard
Marco Felsch June 14, 2019, 12:04 p.m.
Hi Richard,

On 19-06-14 11:55, Richard Purdie wrote:
> On Thu, 2019-06-13 at 19:54 +0200, Marco Felsch wrote:
> > Most of the time we are compiling for embedded targets which have
> > dedicated hardware combinations. Enabling swrast by default isn't a
> > good
> > solution for such devices because if the hardware render node has an
> > issue or doesn't support a special format/request Mesa will fallback
> > to
> > the software renderer. This will make it harder to debug performance
> > issues.
> > 
> > A better way is to let the user decide if a software renderer is
> > needed e.g. if the system has no hardware renderer or to have such a
> > fallback device. This way the user knows that the software renderer
> > is
> > enabled.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > v3
> > - rebased on current master-next branch
> 
> I think this breaks the autobuilder:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/57/builds/694

thanks for covering that. Why didn't I get a email? Anyway thats
interessting. IMHO it isn't a good solution to rely on that fact that
the package have some 'random' default enabled drivers. Should we fix
the qemu configs or should I add:

PACKAGECONFIG_append_qemuall = " swrast"

Regards,
  Marco

> Cheers,
> 
> Richard
> 
>
Richard Purdie June 14, 2019, 12:11 p.m.
On Fri, 2019-06-14 at 14:04 +0200, Marco Felsch wrote:
> On 19-06-14 11:55, Richard Purdie wrote:
> > On Thu, 2019-06-13 at 19:54 +0200, Marco Felsch wrote:
> > > Most of the time we are compiling for embedded targets which have
> > > dedicated hardware combinations. Enabling swrast by default isn't
> > > a
> > > good
> > > solution for such devices because if the hardware render node has
> > > an
> > > issue or doesn't support a special format/request Mesa will
> > > fallback
> > > to
> > > the software renderer. This will make it harder to debug
> > > performance
> > > issues.
> > > 
> > > A better way is to let the user decide if a software renderer is
> > > needed e.g. if the system has no hardware renderer or to have
> > > such a
> > > fallback device. This way the user knows that the software
> > > renderer
> > > is
> > > enabled.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > > v3
> > > - rebased on current master-next branch
> > 
> > I think this breaks the autobuilder:
> > 
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/57/builds/694
> 
> thanks for covering that. Why didn't I get a email?

The patchtest emails come from quick tests. This is a test of full
builds on the autobuilder with batched together patches. The system has
no real knowledge of which patch causes which failure so its a manual
process. We don't have the infrastructure to run all patches through
the full autobuilder tests individually.

>  Anyway thats
> interessting. IMHO it isn't a good solution to rely on that fact that
> the package have some 'random' default enabled drivers. Should we fix
> the qemu configs or should I add:
> 
> PACKAGECONFIG_append_qemuall = " swrast"

The assumption is that swrast makes a good fallback and would be
available in most cases.

I suspect qemuall might not fix beaglebone-yocto or some of the
hardware BSPs.

Its also assumed these packages are shared amongst multiple machines
which may need different drivers.

I suspect this means the defaults should be on but I am happy to have
more PACKAGECONFIG options to control things for people who want to
customise/micro-optimise.

Cheers,

Richard
Marco Felsch June 14, 2019, 12:34 p.m.
On 19-06-14 13:11, richard.purdie@linuxfoundation.org wrote:
> On Fri, 2019-06-14 at 14:04 +0200, Marco Felsch wrote:
> > On 19-06-14 11:55, Richard Purdie wrote:
> > > On Thu, 2019-06-13 at 19:54 +0200, Marco Felsch wrote:
> > > > Most of the time we are compiling for embedded targets which have
> > > > dedicated hardware combinations. Enabling swrast by default isn't
> > > > a
> > > > good
> > > > solution for such devices because if the hardware render node has
> > > > an
> > > > issue or doesn't support a special format/request Mesa will
> > > > fallback
> > > > to
> > > > the software renderer. This will make it harder to debug
> > > > performance
> > > > issues.
> > > > 
> > > > A better way is to let the user decide if a software renderer is
> > > > needed e.g. if the system has no hardware renderer or to have
> > > > such a
> > > > fallback device. This way the user knows that the software
> > > > renderer
> > > > is
> > > > enabled.
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > > v3
> > > > - rebased on current master-next branch
> > > 
> > > I think this breaks the autobuilder:
> > > 
> > > https://autobuilder.yoctoproject.org/typhoon/#/builders/57/builds/694
> > 
> > thanks for covering that. Why didn't I get a email?
> 
> The patchtest emails come from quick tests. This is a test of full
> builds on the autobuilder with batched together patches. The system has
> no real knowledge of which patch causes which failure so its a manual
> process. We don't have the infrastructure to run all patches through
> the full autobuilder tests individually.
> 
> >  Anyway thats
> > interessting. IMHO it isn't a good solution to rely on that fact that
> > the package have some 'random' default enabled drivers. Should we fix
> > the qemu configs or should I add:
> > 
> > PACKAGECONFIG_append_qemuall = " swrast"
> 
> The assumption is that swrast makes a good fallback and would be
> available in most cases.

No I don't think that it is a good fallback, following my patch
description. If you are on a embedded device you want the hw-renderer
and not the sw one. A good example: After I updated my kernel, my
hw-renderer wasn't available anymore and my application didn't start.
Thats a very good indication that something went bad. With the swrast
enabled I wouldn't see that immediately.

> I suspect qemuall might not fix beaglebone-yocto or some of the
> hardware BSPs.

As I discribed in the patch description if you want that fallback you
have to enable it. IMHO this is the better way instead of to be a
'smart' package.

> Its also assumed these packages are shared amongst multiple machines
> which may need different drivers.
> 
> I suspect this means the defaults should be on but I am happy to have
> more PACKAGECONFIG options to control things for people who want to
> customise/micro-optimise.

Optimising wasn't my main motivation. Avoiding 'random' performace
issues after a upgrade (kernel, mesa) was my main motivation. Do you get
me?

Regards,
  Marco

> Cheers,
> 
> Richard
> 
> 
> 
>
Alexander Kanavin June 14, 2019, 12:45 p.m.
I tend to agree with Marco: I am not sure there is a use case for swrast
anymore, not even as a fallback. About the only thing it is useful for is
glxgears. On real hardware you want the real driver without fallbacks, on
qemu you want virgl.

I would rather replace swrast with virgl in the qemu machine configs.
Currently it's pulled in implicitly via the megadriver package which has
virgl included because it is enabled by default in the mesa recipe. From
what I can see beaglebone etc. are not affected by this.

Alex

On Fri, 14 Jun 2019 at 14:35, Marco Felsch <m.felsch@pengutronix.de> wrote:

> On 19-06-14 13:11, richard.purdie@linuxfoundation.org wrote:
> > On Fri, 2019-06-14 at 14:04 +0200, Marco Felsch wrote:
> > > On 19-06-14 11:55, Richard Purdie wrote:
> > > > On Thu, 2019-06-13 at 19:54 +0200, Marco Felsch wrote:
> > > > > Most of the time we are compiling for embedded targets which have
> > > > > dedicated hardware combinations. Enabling swrast by default isn't
> > > > > a
> > > > > good
> > > > > solution for such devices because if the hardware render node has
> > > > > an
> > > > > issue or doesn't support a special format/request Mesa will
> > > > > fallback
> > > > > to
> > > > > the software renderer. This will make it harder to debug
> > > > > performance
> > > > > issues.
> > > > >
> > > > > A better way is to let the user decide if a software renderer is
> > > > > needed e.g. if the system has no hardware renderer or to have
> > > > > such a
> > > > > fallback device. This way the user knows that the software
> > > > > renderer
> > > > > is
> > > > > enabled.
> > > > >
> > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > > ---
> > > > > v3
> > > > > - rebased on current master-next branch
> > > >
> > > > I think this breaks the autobuilder:
> > > >
> > > >
> https://autobuilder.yoctoproject.org/typhoon/#/builders/57/builds/694
> > >
> > > thanks for covering that. Why didn't I get a email?
> >
> > The patchtest emails come from quick tests. This is a test of full
> > builds on the autobuilder with batched together patches. The system has
> > no real knowledge of which patch causes which failure so its a manual
> > process. We don't have the infrastructure to run all patches through
> > the full autobuilder tests individually.
> >
> > >  Anyway thats
> > > interessting. IMHO it isn't a good solution to rely on that fact that
> > > the package have some 'random' default enabled drivers. Should we fix
> > > the qemu configs or should I add:
> > >
> > > PACKAGECONFIG_append_qemuall = " swrast"
> >
> > The assumption is that swrast makes a good fallback and would be
> > available in most cases.
>
> No I don't think that it is a good fallback, following my patch
> description. If you are on a embedded device you want the hw-renderer
> and not the sw one. A good example: After I updated my kernel, my
> hw-renderer wasn't available anymore and my application didn't start.
> Thats a very good indication that something went bad. With the swrast
> enabled I wouldn't see that immediately.
>
> > I suspect qemuall might not fix beaglebone-yocto or some of the
> > hardware BSPs.
>
> As I discribed in the patch description if you want that fallback you
> have to enable it. IMHO this is the better way instead of to be a
> 'smart' package.
>
> > Its also assumed these packages are shared amongst multiple machines
> > which may need different drivers.
> >
> > I suspect this means the defaults should be on but I am happy to have
> > more PACKAGECONFIG options to control things for people who want to
> > customise/micro-optimise.
>
> Optimising wasn't my main motivation. Avoiding 'random' performace
> issues after a upgrade (kernel, mesa) was my main motivation. Do you get
> me?
>
> Regards,
>   Marco
>
> > Cheers,
> >
> > Richard
> >
> >
> >
> >
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
Richard Purdie June 14, 2019, 12:50 p.m.
On Fri, 2019-06-14 at 14:34 +0200, Marco Felsch wrote:
> On 19-06-14 13:11, richard.purdie@linuxfoundation.org wrote:
> > On Fri, 2019-06-14 at 14:04 +0200, Marco Felsch wrote:
> > >  Anyway thats
> > > interessting. IMHO it isn't a good solution to rely on that fact
> > > that
> > > the package have some 'random' default enabled drivers. Should we
> > > fix
> > > the qemu configs or should I add:
> > > 
> > > PACKAGECONFIG_append_qemuall = " swrast"
> > 
> > The assumption is that swrast makes a good fallback and would be
> > available in most cases.
> 
> No I don't think that it is a good fallback, following my patch
> description. If you are on a embedded device you want the hw-renderer
> and not the sw one. A good example: After I updated my kernel, my
> hw-renderer wasn't available anymore and my application didn't start.
> Thats a very good indication that something went bad. With the swrast
> enabled I wouldn't see that immediately.

Alternatively, if swrast is available the system is more usable and you
can then have a more usable system to track down the problem and fix
it?

My point is you can argue that both ways.

I did read your commit message.

> > I suspect qemuall might not fix beaglebone-yocto or some of the
> > hardware BSPs.
> 
> As I discribed in the patch description if you want that fallback you
> have to enable it. IMHO this is the better way instead of to be a
> 'smart' package.

As things stand this patch will likely break a significant number of
BSPs. This isn't acceptable.

> > Its also assumed these packages are shared amongst multiple
> > machines
> > which may need different drivers.
> > 
> > I suspect this means the defaults should be on but I am happy to
> > have
> > more PACKAGECONFIG options to control things for people who want to
> > customise/micro-optimise.
> 
> Optimising wasn't my main motivation. Avoiding 'random' performace
> issues after a upgrade (kernel, mesa) was my main motivation. Do you
> get me?

I understand why you're doing it however I don't like the implications
of the change which mean breakage for a significant number of BSPs, and
the fact it breaks the way distros would use this recipe from a
packaging perspective as its not marked as machine specific, nor should
it be.

Cheers,

Richard
Alexander Kanavin June 14, 2019, 1:05 p.m.
I guess the patch needs a small tweak so that swrast remains enabled, but becomes optional. Then Marco can set packageconfig exactly as he wants.

The second patch is fine as it is, I think. Only qemu implements virgl device at the moment.

Alex

> On 14 Jun 2019, at 14.50, richard.purdie@linuxfoundation.org wrote:
> 
>> On Fri, 2019-06-14 at 14:34 +0200, Marco Felsch wrote:
>>> On 19-06-14 13:11, richard.purdie@linuxfoundation.org wrote:
>>>> On Fri, 2019-06-14 at 14:04 +0200, Marco Felsch wrote:
>>>> Anyway thats
>>>> interessting. IMHO it isn't a good solution to rely on that fact
>>>> that
>>>> the package have some 'random' default enabled drivers. Should we
>>>> fix
>>>> the qemu configs or should I add:
>>>> 
>>>> PACKAGECONFIG_append_qemuall = " swrast"
>>> 
>>> The assumption is that swrast makes a good fallback and would be
>>> available in most cases.
>> 
>> No I don't think that it is a good fallback, following my patch
>> description. If you are on a embedded device you want the hw-renderer
>> and not the sw one. A good example: After I updated my kernel, my
>> hw-renderer wasn't available anymore and my application didn't start.
>> Thats a very good indication that something went bad. With the swrast
>> enabled I wouldn't see that immediately.
> 
> Alternatively, if swrast is available the system is more usable and you
> can then have a more usable system to track down the problem and fix
> it?
> 
> My point is you can argue that both ways.
> 
> I did read your commit message.
> 
>>> I suspect qemuall might not fix beaglebone-yocto or some of the
>>> hardware BSPs.
>> 
>> As I discribed in the patch description if you want that fallback you
>> have to enable it. IMHO this is the better way instead of to be a
>> 'smart' package.
> 
> As things stand this patch will likely break a significant number of
> BSPs. This isn't acceptable.
> 
>>> Its also assumed these packages are shared amongst multiple
>>> machines
>>> which may need different drivers.
>>> 
>>> I suspect this means the defaults should be on but I am happy to
>>> have
>>> more PACKAGECONFIG options to control things for people who want to
>>> customise/micro-optimise.
>> 
>> Optimising wasn't my main motivation. Avoiding 'random' performace
>> issues after a upgrade (kernel, mesa) was my main motivation. Do you
>> get me?
> 
> I understand why you're doing it however I don't like the implications
> of the change which mean breakage for a significant number of BSPs, and
> the fact it breaks the way distros would use this recipe from a
> packaging perspective as its not marked as machine specific, nor should
> it be.
> 
> Cheers,
> 
> Richard
> 
> 
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Ross Burton June 14, 2019, 1:16 p.m.
On Fri, 14 Jun 2019 at 13:46, Alexander Kanavin <alex.kanavin@gmail.com> wrote:
> I would rather replace swrast with virgl in the qemu machine configs. Currently it's pulled in implicitly via the megadriver package which has virgl included because it is enabled by default in the mesa recipe. From what I can see beaglebone etc. are not affected by this.

So the failure is that swrast is no longer shipped by mesa but the
qemu BSP does this:

XSERVER ?= "xserver-xorg \
            ${@bb.utils.contains('DISTRO_FEATURES', 'opengl',
'mesa-driver-swrast xserver-xorg-extension-glx', '', d)} \
            xf86-video-fbdev \
            "

However as Alex says, we have virgl now.  Does this work on all qemus?
 Should we just change the XSERVER assignment to explicitly pull in
the megadriver instead?

The 'proper' swrast driver *is* terrible, lets not enabe it by default
now we have virgl for qemu.  If a BSP wants to use it then they can,
but it's not something we should enable out of the box.

Ross
Richard Purdie June 14, 2019, 1:26 p.m.
On Fri, 2019-06-14 at 15:05 +0200, Alexander Kanavin wrote:
> I guess the patch needs a small tweak so that swrast remains enabled,
> but becomes optional. Then Marco can set packageconfig exactly as he
> wants.

I guess if we can track down where the swrast dependency is coming and
and change things to avoid it, that would probably be ok.

I tend to think swrast is in a better state that it is in reality :(.

> The second patch is fine as it is, I think. Only qemu implements
> virgl device at the moment.

The second patch should be fine, I had to drop it as it depends on the
first though.

Cheers,

Richard
Alexander Kanavin June 14, 2019, 1:31 p.m.
I think the only qemus where we do anything mesa-related are kvm-enabled
x86 ones. So in that sense it doesn't matter which drivers are shipped for
other qemu targets.

Alex

On Fri, 14 Jun 2019 at 15:16, Burton, Ross <ross.burton@intel.com> wrote:

> On Fri, 14 Jun 2019 at 13:46, Alexander Kanavin <alex.kanavin@gmail.com>
> wrote:
> > I would rather replace swrast with virgl in the qemu machine configs.
> Currently it's pulled in implicitly via the megadriver package which has
> virgl included because it is enabled by default in the mesa recipe. From
> what I can see beaglebone etc. are not affected by this.
>
> So the failure is that swrast is no longer shipped by mesa but the
> qemu BSP does this:
>
> XSERVER ?= "xserver-xorg \
>             ${@bb.utils.contains('DISTRO_FEATURES', 'opengl',
> 'mesa-driver-swrast xserver-xorg-extension-glx', '', d)} \
>             xf86-video-fbdev \
>             "
>
> However as Alex says, we have virgl now.  Does this work on all qemus?
>  Should we just change the XSERVER assignment to explicitly pull in
> the megadriver instead?
>
> The 'proper' swrast driver *is* terrible, lets not enabe it by default
> now we have virgl for qemu.  If a BSP wants to use it then they can,
> but it's not something we should enable out of the box.
>
> Ross
>
Alexander Kanavin June 14, 2019, 1:33 p.m.
On Fri, 14 Jun 2019 at 15:26, <richard.purdie@linuxfoundation.org> wrote:

> I guess if we can track down where the swrast dependency is coming and
> and change things to avoid it, that would probably be ok.
>

As Ross said, qemu bsp has this:

XSERVER ?= "xserver-xorg \
            ${@bb.utils.contains('DISTRO_FEATURES', 'opengl',
'mesa-driver-swrast xserver-xorg-extension-glx', '', d)} \
            xf86-video-fbdev \
            "
Dropping the swrast dependency will not help though, as it is provided by
the mesa-megadriver package, which will continue to include swrast as long
as it is enabled in the mesa recipe. mesa-megadriver is also pulled in
through other dependencies.

Alex
Ross Burton June 14, 2019, 1:36 p.m.
On Fri, 14 Jun 2019 at 14:34, Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>
> On Fri, 14 Jun 2019 at 15:26, <richard.purdie@linuxfoundation.org> wrote:
>>
>> I guess if we can track down where the swrast dependency is coming and
>> and change things to avoid it, that would probably be ok.
>
>
> As Ross said, qemu bsp has this:
>
> XSERVER ?= "xserver-xorg \
>             ${@bb.utils.contains('DISTRO_FEATURES', 'opengl',
> 'mesa-driver-swrast xserver-xorg-extension-glx', '', d)} \
>             xf86-video-fbdev \
>             "
> Dropping the swrast dependency will not help though, as it is provided by the mesa-megadriver package, which will continue to include swrast as long as it is enabled in the mesa recipe. mesa-megadriver is also pulled in through other dependencies.

Won't that stop the build failure on the autobuilder though?

Ross
Alexander Kanavin June 14, 2019, 1:37 p.m.
On Fri, 14 Jun 2019 at 15:36, Burton, Ross <ross.burton@intel.com> wrote:

> > Dropping the swrast dependency will not help though, as it is provided
> by the mesa-megadriver package, which will continue to include swrast as
> long as it is enabled in the mesa recipe. mesa-megadriver is also pulled in
> through other dependencies.
>
> Won't that stop the build failure on the autobuilder though?
>

Yes, I think it would.
Marco Felsch June 17, 2019, 9:35 a.m.
On 19-06-14 15:37, Alexander Kanavin wrote:
> On Fri, 14 Jun 2019 at 15:36, Burton, Ross <ross.burton@intel.com> wrote:
> 
> > > Dropping the swrast dependency will not help though, as it is provided
> > by the mesa-megadriver package, which will continue to include swrast as
> > long as it is enabled in the mesa recipe. mesa-megadriver is also pulled in
> > through other dependencies.
> >
> > Won't that stop the build failure on the autobuilder though?
> >
> 
> Yes, I think it would.

So should I change that dependency to virgl since it is a qemu target?
Sorry but I can not see an agreement how we can change this in a 'good'
way for all parties.

My point of view is that it should not be enabled per default. Of course
I should fix the qemu config but I don't wanna fix each meta-layer.
Becuase I think that is the work of meta-layer maintainer.

Regards,
  Marco
Alexander Kanavin June 17, 2019, 11:21 a.m.
Yes please, I think that is the best way out for qemu.

Alex

On Mon, 17 Jun 2019 at 11:35, Marco Felsch <m.felsch@pengutronix.de> wrote:

> On 19-06-14 15:37, Alexander Kanavin wrote:
> > On Fri, 14 Jun 2019 at 15:36, Burton, Ross <ross.burton@intel.com>
> wrote:
> >
> > > > Dropping the swrast dependency will not help though, as it is
> provided
> > > by the mesa-megadriver package, which will continue to include swrast
> as
> > > long as it is enabled in the mesa recipe. mesa-megadriver is also
> pulled in
> > > through other dependencies.
> > >
> > > Won't that stop the build failure on the autobuilder though?
> > >
> >
> > Yes, I think it would.
>
> So should I change that dependency to virgl since it is a qemu target?
> Sorry but I can not see an agreement how we can change this in a 'good'
> way for all parties.
>
> My point of view is that it should not be enabled per default. Of course
> I should fix the qemu config but I don't wanna fix each meta-layer.
> Becuase I think that is the work of meta-layer maintainer.
>
> Regards,
>   Marco
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>