All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denys Dmytriyenko <denis@denix.org>
To: afd@ti.com
Cc: Darren Etheridge <detheridge@ti.com>,
	meta-ti@lists.yoctoproject.org, reatmon@ti.com
Subject: Re: [meta-ti][dunfell][PATCH v2] ti-graphics: gpu enable and move all platforms to ddk 1.15
Date: Tue, 29 Mar 2022 01:21:58 -0400	[thread overview]
Message-ID: <20220329052158.GT23554@denix.org> (raw)
In-Reply-To: <1c4a7e86-9d2b-eaf0-f2bb-be04109e4f43@ti.com>

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

Think of it from another perspective - if you keep all machine overrides for 
one varibale in the same recipe, a minor change to one specific override there 
will make the recipe as "modified" for all platforms and all builds would have 
to be redone.

But if that one variable is set in the machine config to a specific value, 
changing it for one platform won't affect other platforms and you'd only need 
to rebuild that changed platform.

For example - let's add a new .dtbo for am65xx-evm...

In first case you would modify an override in the kernel recipe:
recipes-kernel/linux/linux-ti-staging_5.10.bb:

 KERNEL_DEVICETREE_am64xx-evm = "..."
-KERNEL_DEVICETREE_am65xx-evm = "devtree.dtb overlay1.dtbo"
+KERNEL_DEVICETREE_am65xx-evm = "devtree.dtb overlay1.dtbo overlay2.dtbo"
 KERNEL_DEVICETREE_j721e-evm = "..."

So, you modified the kernel recipes, so now all platforms would have to 
rebuild the kernel and everything that depends on it. With all the variants 
(RT, non-RT, debug), brands and configs (nightly, RC, etc), that ends up in 
a lot of rebuilds in the farm (could be hundreds of rebuilds, if you count 
all permutations)!

In the second case, the .dtbo is added in the machine config:
conf/machine/am65xx-evm.conf:
-KERNEL_DEVICETREE = "devtree.dtb overlay1.dtbo"
+KERNEL_DEVICETREE = "devtree.dtb overlay1.dtbo overlay2.dtbo"

And the kernel recipe wasn't touched, so only am65xx-evm needs to be rebuilt. 
Even with permutations, it's just several rebuilds of the same platform.

The other point is that changes to the same variable more often happen one 
platform at a time (add a .dtbo, change defconfig or add a config fragment, 
even add a new platform), instead of changing globally for all platforms at 
the same time...


PS. There are some advanced Yocto techniques to remedy the above rebuilding 
of dependency tree to some extent, such as hash equivalence and such, but 
that's still a bit cutting edge, somewhat unstable and might not be ready for 
production yet.

-- 
Denys


  parent reply	other threads:[~2022-03-29  5:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23 19:37 [meta-ti][dunfell][PATCH v2] ti-graphics: gpu enable and move all platforms to ddk 1.15 Darren Etheridge
2022-03-25 20:10 ` Denys Dmytriyenko
2022-03-25 21:21   ` Andrew Davis
2022-03-25 21:38     ` Denys Dmytriyenko
2022-03-25 21:54       ` Andrew Davis
2022-03-25 22:06         ` Denys Dmytriyenko
2022-03-25 22:30           ` Andrew Davis
     [not found]           ` <16DFC054FC61A9B3.21339@lists.yoctoproject.org>
2022-03-25 22:36             ` Andrew Davis
2022-03-29  5:28               ` Denys Dmytriyenko
2022-03-29 21:48               ` Etheridge, Darren
2022-03-29 23:26                 ` Denys Dmytriyenko
2022-03-30 12:40                   ` Ryan Eatmon
2022-03-29  5:21         ` Denys Dmytriyenko [this message]
2022-04-30 18:02 ` Denys Dmytriyenko
     [not found] ` <16EABE932EA7F3D7.11702@lists.yoctoproject.org>
2022-05-02  2:57   ` Denys Dmytriyenko
2022-05-02  2:58   ` Denys Dmytriyenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220329052158.GT23554@denix.org \
    --to=denis@denix.org \
    --cc=afd@ti.com \
    --cc=detheridge@ti.com \
    --cc=meta-ti@lists.yoctoproject.org \
    --cc=reatmon@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.