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 <[email protected]>
> >>>>>>>>---
> >>>>>>>>
> >>>>>>>>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
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#14580): 
https://lists.yoctoproject.org/g/meta-ti/message/14580
Mute This Topic: https://lists.yoctoproject.org/mt/89983769/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to