All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mesa: make gallium swrast target optional
@ 2019-06-13 17:54 Marco Felsch
  2019-06-13 17:54 ` [PATCH 2/2] mesa: make gallium virgl " Marco Felsch
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Marco Felsch @ 2019-06-13 17:54 UTC (permalink / raw)
  To: openembedded-core; +Cc: kernel

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



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] mesa: make gallium virgl target optional
  2019-06-13 17:54 [PATCH 1/2] mesa: make gallium swrast target optional Marco Felsch
@ 2019-06-13 17:54 ` Marco Felsch
  2019-06-13 18:17 ` [PATCH 1/2] mesa: make gallium swrast " Martin Jansa
  2019-06-14 10:55 ` Richard Purdie
  2 siblings, 0 replies; 20+ messages in thread
From: Marco Felsch @ 2019-06-13 17:54 UTC (permalink / raw)
  To: openembedded-core; +Cc: kernel

Just as for the swrast target it isn't a good solution to enable the
target per default since we are compiling for embedded targets most of
the time. Make this target optional for all hardware targets except
qemu to save resources.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v3:
- rebased on current master-next branch
- add PACKAGECONFIG_append_qemuall to avoid breaking changes

 meta/recipes-graphics/mesa/mesa.inc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-graphics/mesa/mesa.inc b/meta/recipes-graphics/mesa/mesa.inc
index a6d36cf21c..6ffcfb7ea5 100644
--- a/meta/recipes-graphics/mesa/mesa.inc
+++ b/meta/recipes-graphics/mesa/mesa.inc
@@ -91,6 +91,9 @@ PACKAGECONFIG[egl] = "-Degl=true, -Degl=false"
 PACKAGECONFIG[etnaviv] = ""
 PACKAGECONFIG[kmsro] = ""
 PACKAGECONFIG[swrast] = ""
+PACKAGECONFIG[r600] = ""
+PACKAGECONFIG[virgl] = ""
+PACKAGECONFIG_append_qemuall = " virgl"
 
 GALLIUMDRIVERS = ""
 GALLIUMDRIVERS +="${@bb.utils.contains('PACKAGECONFIG', 'swrast', 'swrast', '', d)}"
@@ -102,11 +105,9 @@ GALLIUMDRIVERS_LLVM33 = "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'radeonsi
 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 ''}"
 
-PACKAGECONFIG[r600] = ""
-
 GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}"
 GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'r600', 'r600', '', d)}"
-GALLIUMDRIVERS += "virgl"
+GALLIUMDRIVERS += "${@bb.utils.contains('PACKAGECONFIG', 'virgl', 'virgl', '', d)}"
 GALLIUMDRIVERS_MESON = "${@",".join("${GALLIUMDRIVERS}".split())}"
 
 PACKAGECONFIG[gallium] = "-Dgallium-drivers=${GALLIUMDRIVERS_MESON}, -Dgallium-drivers=''"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-13 17:54 [PATCH 1/2] mesa: make gallium swrast target optional Marco Felsch
  2019-06-13 17:54 ` [PATCH 2/2] mesa: make gallium virgl " Marco Felsch
@ 2019-06-13 18:17 ` Martin Jansa
  2019-06-13 18:38   ` Marco Felsch
  2019-06-14 10:55 ` Richard Purdie
  2 siblings, 1 reply; 20+ messages in thread
From: Martin Jansa @ 2019-06-13 18:17 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 4333 bytes --]

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
>

[-- Attachment #2: Type: text/html, Size: 5814 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-13 18:17 ` [PATCH 1/2] mesa: make gallium swrast " Martin Jansa
@ 2019-06-13 18:38   ` Marco Felsch
  2019-06-13 19:07     ` Martin Jansa
  0 siblings, 1 reply; 20+ messages in thread
From: Marco Felsch @ 2019-06-13 18:38 UTC (permalink / raw)
  To: Martin Jansa; +Cc: kernel, Patches and discussions about the oe-core layer

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 |


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-13 18:38   ` Marco Felsch
@ 2019-06-13 19:07     ` Martin Jansa
  0 siblings, 0 replies; 20+ messages in thread
From: Martin Jansa @ 2019-06-13 19:07 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 5737 bytes --]

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 |
>

[-- Attachment #2: Type: text/html, Size: 8083 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-13 17:54 [PATCH 1/2] mesa: make gallium swrast target optional Marco Felsch
  2019-06-13 17:54 ` [PATCH 2/2] mesa: make gallium virgl " Marco Felsch
  2019-06-13 18:17 ` [PATCH 1/2] mesa: make gallium swrast " Martin Jansa
@ 2019-06-14 10:55 ` Richard Purdie
  2019-06-14 12:04   ` Marco Felsch
  2 siblings, 1 reply; 20+ messages in thread
From: Richard Purdie @ 2019-06-14 10:55 UTC (permalink / raw)
  To: Marco Felsch, openembedded-core; +Cc: kernel

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



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 10:55 ` Richard Purdie
@ 2019-06-14 12:04   ` Marco Felsch
  2019-06-14 12:11     ` richard.purdie
  0 siblings, 1 reply; 20+ messages in thread
From: Marco Felsch @ 2019-06-14 12:04 UTC (permalink / raw)
  To: Richard Purdie; +Cc: kernel, openembedded-core

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
> 
> 

-- 
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 |


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:04   ` Marco Felsch
@ 2019-06-14 12:11     ` richard.purdie
  2019-06-14 12:34       ` Marco Felsch
  0 siblings, 1 reply; 20+ messages in thread
From: richard.purdie @ 2019-06-14 12:11 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, openembedded-core

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





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:11     ` richard.purdie
@ 2019-06-14 12:34       ` Marco Felsch
  2019-06-14 12:45         ` Alexander Kanavin
  2019-06-14 12:50         ` richard.purdie
  0 siblings, 2 replies; 20+ messages in thread
From: Marco Felsch @ 2019-06-14 12:34 UTC (permalink / raw)
  To: richard.purdie; +Cc: kernel, openembedded-core

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 |


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:34       ` Marco Felsch
@ 2019-06-14 12:45         ` Alexander Kanavin
  2019-06-14 13:16           ` Burton, Ross
  2019-06-14 12:50         ` richard.purdie
  1 sibling, 1 reply; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-14 12:45 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, OE-core

[-- Attachment #1: Type: text/plain, Size: 4461 bytes --]

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
>

[-- Attachment #2: Type: text/html, Size: 6138 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:34       ` Marco Felsch
  2019-06-14 12:45         ` Alexander Kanavin
@ 2019-06-14 12:50         ` richard.purdie
  2019-06-14 13:05           ` Alexander Kanavin
  1 sibling, 1 reply; 20+ messages in thread
From: richard.purdie @ 2019-06-14 12:50 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, openembedded-core

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





^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:50         ` richard.purdie
@ 2019-06-14 13:05           ` Alexander Kanavin
  2019-06-14 13:26             ` richard.purdie
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-14 13:05 UTC (permalink / raw)
  To: richard.purdie; +Cc: kernel, openembedded-core

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 12:45         ` Alexander Kanavin
@ 2019-06-14 13:16           ` Burton, Ross
  2019-06-14 13:31             ` Alexander Kanavin
  0 siblings, 1 reply; 20+ messages in thread
From: Burton, Ross @ 2019-06-14 13:16 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: kernel, OE-core

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:05           ` Alexander Kanavin
@ 2019-06-14 13:26             ` richard.purdie
  2019-06-14 13:33               ` Alexander Kanavin
  0 siblings, 1 reply; 20+ messages in thread
From: richard.purdie @ 2019-06-14 13:26 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: kernel, openembedded-core

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



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:16           ` Burton, Ross
@ 2019-06-14 13:31             ` Alexander Kanavin
  0 siblings, 0 replies; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-14 13:31 UTC (permalink / raw)
  To: Burton, Ross; +Cc: kernel, OE-core

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

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
>

[-- Attachment #2: Type: text/html, Size: 1831 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:26             ` richard.purdie
@ 2019-06-14 13:33               ` Alexander Kanavin
  2019-06-14 13:36                 ` Burton, Ross
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-14 13:33 UTC (permalink / raw)
  To: Richard Purdie; +Cc: kernel, OE-core

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 1137 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:33               ` Alexander Kanavin
@ 2019-06-14 13:36                 ` Burton, Ross
  2019-06-14 13:37                   ` Alexander Kanavin
  0 siblings, 1 reply; 20+ messages in thread
From: Burton, Ross @ 2019-06-14 13:36 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: kernel, OE-core

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:36                 ` Burton, Ross
@ 2019-06-14 13:37                   ` Alexander Kanavin
  2019-06-17  9:35                     ` Marco Felsch
  0 siblings, 1 reply; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-14 13:37 UTC (permalink / raw)
  To: Burton, Ross; +Cc: kernel, OE-core

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 720 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-14 13:37                   ` Alexander Kanavin
@ 2019-06-17  9:35                     ` Marco Felsch
  2019-06-17 11:21                       ` Alexander Kanavin
  0 siblings, 1 reply; 20+ messages in thread
From: Marco Felsch @ 2019-06-17  9:35 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: kernel, OE-core

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 |


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] mesa: make gallium swrast target optional
  2019-06-17  9:35                     ` Marco Felsch
@ 2019-06-17 11:21                       ` Alexander Kanavin
  0 siblings, 0 replies; 20+ messages in thread
From: Alexander Kanavin @ 2019-06-17 11:21 UTC (permalink / raw)
  To: Marco Felsch; +Cc: kernel, OE-core

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

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 |
>

[-- Attachment #2: Type: text/html, Size: 2047 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-06-17 11:21 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 17:54 [PATCH 1/2] mesa: make gallium swrast target optional Marco Felsch
2019-06-13 17:54 ` [PATCH 2/2] mesa: make gallium virgl " Marco Felsch
2019-06-13 18:17 ` [PATCH 1/2] mesa: make gallium swrast " Martin Jansa
2019-06-13 18:38   ` Marco Felsch
2019-06-13 19:07     ` Martin Jansa
2019-06-14 10:55 ` Richard Purdie
2019-06-14 12:04   ` Marco Felsch
2019-06-14 12:11     ` richard.purdie
2019-06-14 12:34       ` Marco Felsch
2019-06-14 12:45         ` Alexander Kanavin
2019-06-14 13:16           ` Burton, Ross
2019-06-14 13:31             ` Alexander Kanavin
2019-06-14 12:50         ` richard.purdie
2019-06-14 13:05           ` Alexander Kanavin
2019-06-14 13:26             ` richard.purdie
2019-06-14 13:33               ` Alexander Kanavin
2019-06-14 13:36                 ` Burton, Ross
2019-06-14 13:37                   ` Alexander Kanavin
2019-06-17  9:35                     ` Marco Felsch
2019-06-17 11:21                       ` Alexander Kanavin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.