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

[Andrew Davis <afd@ti.com>]

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@ti.com>
>>>>>>> ---
>>>>>>>
>>>>>>> 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


> Andrew
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14562): https://lists.yoctoproject.org/g/meta-ti/message/14562
> Mute This Topic: https://lists.yoctoproject.org/mt/89983769/3619733
> Group Owner: meta-ti+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub [afd@ti.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>