Re: [PATCH] drm/imagination: DRM_POWERVR should depend on ARCH_K3

[Geert Uytterhoeven]

Hi Maxime,

On Wed, Nov 29, 2023 at 10:23 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote:
> On Wed, Nov 29, 2023 at 09:58:12AM +0100, Geert Uytterhoeven wrote:
> > On Wed, Nov 29, 2023 at 9:35 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote:
> > > On Tue, Nov 28, 2023 at 08:16:18PM +0100, Geert Uytterhoeven wrote:
> > > > On Tue, Nov 28, 2023 at 8:03 PM Javier Martinez Canillas
> > > > <javierm@xxxxxxxxxx> wrote:
> > > > > Geert Uytterhoeven <geert+renesas@xxxxxxxxx> writes:
> > > > > > The Imagination Technologies PowerVR Series 6 GPU is currently only
> > > > > > supported on Texas Instruments K3 AM62x SoCs.  Hence add a dependency on
> > > > > > ARCH_K3, to prevent asking the user about this driver when configuring a
> > > > > > kernel without Texas Instruments K3 Multicore SoC support.
> > > > > >
> > > > > > Fixes: 4babef0708656c54 ("drm/imagination: Add skeleton PowerVR driver")
> > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> >
> > > > > In any case, I agree with you that restricting to only K3 makes sense.
> > > >
> > > > I am looking forward to adding || SOC_AM33XX || ARCH_RENESAS || ...,
> > > > eventually ;-)
> > >
> > > I disagree. This is to handle a generic IP, just like panfrost, lima, or
> > > etnaviv, and we certaintly don't want to maintain the Kconfig list of
> > > every possible architecture and SoC family it might or might not be
> > > found.
> >
> > While PowerVR is a generic IP, I believe it needs a non-generic
> > firmware, which is currently only available for AM62x SoCs.
>
> I'm not sure it's actually true, but let's consider it is. Then what? If
> the firmware isn't there and/or the DT bits too, then nothing will
> happen. We would have wasted a couple of 100kB on a system that is
> taking somewhere in the 100MB-10GB range, and that's pretty much it.

I am talking about posing the question to the user to enable the driver
or not.  Which applies to everyone who configures a kernel.

> If you have we take that patch in though, we have:
>
>   - To keep merging patches as firmwares become available.

You need to keep merging patches to update DT bindings, DTS,
SoC-specific drivers, the DRM driver itself, ... too.

>   - If we update linux-firmware only, then the driver is still not
>     loading even though it could.
>
>   - If we have gotten our firmware through some other mean, then the
>     driver is still not loading even though it could.

You will still need to update parts of the kernel, too.
As long as none of that has happened, asking about the PowerVR driver
on non-AM62x hardware is futile...

> It makes life harder for everyone: maintainers, users, devs, based on
> the state of some external project that might or might not be updated in
> sync.
>
> > Once it becomes truly generic, I'm happy to drop all platform
> > dependencies.  Until then, there is no point in asking everyone who
> > configures an arm64 kernel about this driver, unless they also enabled
> > K3 support.
>
> Whether it's truly generic, whatever that means, is irrelevant here.

It is.

BTW, playing the devil's advocate: why is there a dependency on ARM64?
PowerVR GPUs are also present on (at least) arm32 and Intel?
Oh, dropping that would expose this question to Linus, causing his
wrath to come down on you... ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds