Re: [dunfell][PATCH v2] ti-graphics: gpu enable and move all platforms to ddk 1.15


Denys Dmytriyenko
 

On Tue, Mar 29, 2022 at 04:48:43PM -0500, Etheridge, Darren wrote:
On 3/25/2022 5:36 PM, Andrew Davis wrote:
On 3/25/22 5:30 PM, Andrew F. Davis via lists.yoctoproject.org wrote:
On 3/25/22 5:06 PM, Denys Dmytriyenko wrote:
On Fri, Mar 25, 2022 at 04:54:56PM -0500, Andrew F. Davis via
lists.yoctoproject.org wrote:
On 3/25/22 4:38 PM, Denys Dmytriyenko wrote:
On Fri, Mar 25, 2022 at 04:21:38PM -0500, Andrew Davis wrote:
On 3/25/22 3:10 PM, Denys Dmytriyenko wrote:
On Wed, Mar 23, 2022 at 02:37:07PM -0500, Darren Etheridge wrote:
Enable the GPU for am62xx and j721s2 and use IMG DDK 1.15

Migrate Imagination DDK 1.13 to DDK 1.15 for J721e
Overall looks good, please see inline below.


Signed-off-by: Darren Etheridge <detheridge@...>
---

rename from recipes-bsp/powervr-drivers/ti-img-rogue-driver_1.13.5776728.bb
rename to recipes-bsp/powervr-drivers/ti-img-rogue-driver_1.15.6133109.bb
index a05de0f2..fbff6c51 100644
--- a/recipes-bsp/powervr-drivers/ti-img-rogue-driver_1.13.5776728.bb
+++ b/recipes-bsp/powervr-drivers/ti-img-rogue-driver_1.15.6133109.bb
@@ -7,17 +7,17 @@ inherit module features_check
  REQUIRED_MACHINE_FEATURES = "gpu"
-MACHINE_KERNEL_PR_append = "b"
+MACHINE_KERNEL_PR_append = "a"
  PR = "${MACHINE_KERNEL_PR}"
  PACKAGE_ARCH = "${MACHINE_ARCH}"
-COMPATIBLE_MACHINE = "j7"
+COMPATIBLE_MACHINE = "j7-evm|j721s2-evm|am62xx"
  DEPENDS = "virtual/kernel"
  PROVIDES = "virtual/gpudriver"
-BRANCH = "1.13-5776728/linux-k5.10"
+BRANCH = "linuxws/dunfell/k5.10/${PV}"
  SRC_URI = " \
git://git.ti.com/graphics/ti-img-rogue-driver.git;branch=${BRANCH}
\
@@ -26,15 +26,19 @@ SRC_URI = " \
  S = "${WORKDIR}/git"
-SRCREV = "35a25875ae8738f82c7cabc6b077ef992b0cca84"
+SRCREV = "ee0674adccac16f5b2f7cb8d5d05948706080cb5"
-PVR_SOC = "j721e_linux"
I was actually thinking of keeping PVR_SOC variable and moving it to
corresponding machine configs.

PVR_SOC is a bit of a legacy name, especially since PVR is now IMG.
PVR or PowerVR name is still used as an overall umbrella for all of
Imagination's graphics, vision and AI chips, including SGX
and Rogue/RGX:
https://en.wikipedia.org/wiki/PowerVR

On the wiki page:

These GPUs are no longer called PowerVR, they are called IMG.[58]
For their next gen GPUs they are distancing themselves from
the PVR name,
the GPU in AM62xx is one such next gen GPU, so it's already
not correct here
as is.
So, then why AM62xx is being added to Rogue driver/DDK? Should
there be a
separate Albiorix driver and DDK then?

We asked IMG very nicely to not fork the DDK for each new gen of
device (like
what happened with SGX -> Rogue), so now we have he same driver code base
supporting multiple generations of GPU. The name "Rogue" just
stuck around.
We should probably rename some of this stuff to also be more generic.



Thinking on this, the mapping between SoC family and the
internal names
like "RGX_BVNC" and "TARGET_PRODUCT" are specific to the
version of this
driver. For instance in the next DDK I may want the target name to
go from "am62_linux" to "axb_128_linux", I would have to
change things here (update the SRCREV) AND in the machine config.
1. I totally agree that "axb_128_linux" makes more sense
than "am62_linux".
2. Changes like that happen very rarely.
3. You can call it PVR_MODEL or PVR_PRODUCT if you want,
instead of PVR_SOC.


Mapping here feels like the right spot to me. I'd even argue the same
for OPTEEMACHINE and the like, should go in the
optee.bbappends with the
rest of our platform specific recipe fixups, etc.
The number of overrides in the recipe will keep on
growing, as each new
platform will need to add own config. That's the whole
point of the machine
configuration file to have those defined centrally.

The goal is to have a recipe as machine-agnostic and
clean, as possible. Do
not overwhelm it with tons of conditionals like that - any
machine-specific
configuration should be set in the machine config file.

Having all the fixups related to a package inside that
package's definition sounds
more central to me, and easier to reason about. But I can
see the argument both
ways.

Maybe the better solution would be to splitup this(and the
SGX) recipe. So we
get a package per GPU type. It's already how the bins are
organized/shipped. Then
we just pick the right GPU package for our SoC in
arago-prefs.inc. (again like
we already do to select between SGX/RGX)
Yeah, I think keeping each series of GPU (SGX, Rogue,
Albiorix) in their own
separate recipes would be fine. Or are you suggesting
splitting even further
into separate recipes for SGX530 vs SGX540, etc?

I'm suggesting even further. The driver/package for SGX530 is
not compatible
and conflicts with the driver package for SGX540. So lets not
have a single named
package (ti-sgx-ddk-um) that can have very different contents,
that's just confusing.

Oh, and not separate recipes, separate packages, they can all be provided
by the same one or two recipes.

ti-sgx-530-um
ti-sgx-544-um
ti-axe1-16m-um
etc..

Then:

PREFERRED_PROVIDER_virtual/libgles2:am62xx = "ti-axe1-16m-um"
...

Andrew
Can we merge for Dunfell and then improve significantly for
Kirkstone, or do I still need to make changes here for Dunfell?
This is fine with me for Dunfell.

--
Denys

Join meta-ti@lists.yoctoproject.org to automatically receive all group messages.