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

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

[Geert Uytterhoeven]

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@glider.be>
---
 drivers/gpu/drm/imagination/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
index 3bfa2ac212dccb73..af492dbd9afd4ed9 100644
--- a/drivers/gpu/drm/imagination/Kconfig
+++ b/drivers/gpu/drm/imagination/Kconfig
@@ -6,6 +6,7 @@ config DRM_POWERVR
 	depends on ARM64
 	depends on DRM
 	depends on PM
+	depends on ARCH_K3 || COMPILE_TEST
 	select DRM_EXEC
 	select DRM_GEM_SHMEM_HELPER
 	select DRM_SCHED
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Javier Martinez Canillas]

Geert Uytterhoeven <geert+renesas@glider.be> writes:

Hello Geert,

> 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@glider.be>
> ---

Indeed. Although I wonder what is the supposed policy since for example
the DRM_PANFROST symbol only depends on ARM || ARM64 and others such as
DRM_ETNAVIV don't even have an SoC or architecture dependency.

In any case, I agree with you that restricting to only K3 makes sense.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Javier,

On Tue, Nov 28, 2023 at 8:03 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > ---
>
> Indeed. Although I wonder what is the supposed policy since for example
> the DRM_PANFROST symbol only depends on ARM || ARM64 and others such as

I think ARM Mali is sufficiently ubiquitous on ARM/ARM64 systems to
have just an ARM/ARM64 dependency...

> DRM_ETNAVIV don't even have an SoC or architecture dependency.

Vivante GPUs are found in DTS files on at least 4 architectures.
Might be worthwhile to add some dependencies, though...

> 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 ;-)

> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Javier Martinez Canillas]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Javier,
>
> On Tue, Nov 28, 2023 at 8:03 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
>> > ---
>>
>> Indeed. Although I wonder what is the supposed policy since for example
>> the DRM_PANFROST symbol only depends on ARM || ARM64 and others such as
>
> I think ARM Mali is sufficiently ubiquitous on ARM/ARM64 systems to
> have just an ARM/ARM64 dependency...
>

Fair.

>> DRM_ETNAVIV don't even have an SoC or architecture dependency.
>
> Vivante GPUs are found in DTS files on at least 4 architectures.
> Might be worthwhile to add some dependencies, though...
>

Yeah, that's what I was thinking.

>> 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 ;-)
>

Same! :)

>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1956 bytes --]

Hi,

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@redhat.com> wrote:
> > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > > ---
> >
> > Indeed. Although I wonder what is the supposed policy since for example
> > the DRM_PANFROST symbol only depends on ARM || ARM64 and others such as
> 
> I think ARM Mali is sufficiently ubiquitous on ARM/ARM64 systems to
> have just an ARM/ARM64 dependency...
> 
> > DRM_ETNAVIV don't even have an SoC or architecture dependency.
> 
> Vivante GPUs are found in DTS files on at least 4 architectures.
> Might be worthwhile to add some dependencies, though...
> 
> > 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.

GPUs supposed to be handled are spread across 4 architectures (x86,
riscv, arm, arm64, mips?), and in arm/arm64 alone we have at least 5
platforms that might use it (allwinner, ti, mediatek, renesas, rockchip)

It didn't make sense for panfrost, or etnaviv. It doesn't make sense for
that driver either. Especially for something that olddefconfig can
handle just fine.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

Hello Maxime,

> Hi,
>
> 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@redhat.com> wrote:
>> > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
>> > > ---
>> >
>> > Indeed. Although I wonder what is the supposed policy since for example
>> > the DRM_PANFROST symbol only depends on ARM || ARM64 and others such as
>> 
>> I think ARM Mali is sufficiently ubiquitous on ARM/ARM64 systems to
>> have just an ARM/ARM64 dependency...
>> 
>> > DRM_ETNAVIV don't even have an SoC or architecture dependency.
>> 
>> Vivante GPUs are found in DTS files on at least 4 architectures.
>> Might be worthwhile to add some dependencies, though...
>> 
>> > 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.
>

Thanks for the clarification. Then the policy is to have a depends on
ARCH_$FOO if the IP block is tied to a particular SoC or SoC family ?

For example, DRM_V3D has:

depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST

If the IP block is generic and could be integrated with any SoC, then it
should not have a dependency as you said.

> GPUs supposed to be handled are spread across 4 architectures (x86,
> riscv, arm, arm64, mips?), and in arm/arm64 alone we have at least 5
> platforms that might use it (allwinner, ti, mediatek, renesas, rockchip)
>
> It didn't make sense for panfrost, or etnaviv. It doesn't make sense for
> that driver either. Especially for something that olddefconfig can
> handle just fine.
>

I think then that we should drop the arch and SoC dependency for these GPU
drivers and just leave the symbols they really depend on (e.g: DRM, MMU) ?

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Maxime,

On Wed, Nov 29, 2023 at 9:35 AM Maxime Ripard <mripard@kernel.org> 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@redhat.com> wrote:
> > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Javier Martinez Canillas]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

Hello Geert,

> Hi Maxime,
>
> On Wed, Nov 29, 2023 at 9:35 AM Maxime Ripard <mripard@kernel.org> 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@redhat.com> wrote:
>> > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
>
>> > > 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.
> 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.
>

That's true but it will require a Kconfig patch every time that there is a
design with a different SoC using this generic IP.

So when should be added? Once there's an upstream DTS that has a GPU device?
Once there's a firmware for it in linux-firmware?

I like the guideline of not having a depends on for generic IP and only have
for IP that can only be used in designs with a SoC from the same vendor.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Javier,

On Wed, Nov 29, 2023 at 10:13 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > On Wed, Nov 29, 2023 at 9:35 AM Maxime Ripard <mripard@kernel.org> 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@redhat.com> wrote:
> >> > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> >
> >> > > 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.
> > 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.
>
> That's true but it will require a Kconfig patch every time that there is a
> design with a different SoC using this generic IP.

It also requires a DT bindings patch, to add a new compatible value,
plus whatever missing properties for SoC integration (e.g. resets).
And a DTS integration patch.
And patches for various on-SoC resources (e.g. clocks).
And perhaps a DRM driver update.

> So when should be added? Once there's an upstream DTS that has a GPU device?
> Once there's a firmware for it in linux-firmware?

It can be added when handling the above.  As all patches should be
tested, the firmware must be available first.

When critical mass is reached, platform dependencies can be dropped.
I do hope that will happen rather sooner than later!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2544 bytes --]

On Wed, Nov 29, 2023 at 09:58:12AM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Wed, Nov 29, 2023 at 9:35 AM Maxime Ripard <mripard@kernel.org> 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@redhat.com> wrote:
> > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> 
> > > > 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.

If you have we take that patch in though, we have:

  - To keep merging patches as firmwares become available.

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

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.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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@kernel.org> 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@kernel.org> 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@redhat.com> wrote:
> > > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> >
> > > > > 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@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 5246 bytes --]

On Wed, Nov 29, 2023 at 11:10:51AM +0100, Geert Uytterhoeven wrote:
> On Wed, Nov 29, 2023 at 10:23 AM Maxime Ripard <mripard@kernel.org> 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@kernel.org> 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@redhat.com> wrote:
> > > > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > >
> > > > > > 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 just asked, it's not true in most cases. There's some exceptions
(GX6250 for example) that could require different firmwares depending on
the SoC it's used in, but it's not the case here.

> > 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 that user doesn't use a defconfig, doesn't use any variant of
*defconfig make target. Plus, the driver still isn't enabled by default.

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

The DT binding and DRM driver is already there, the SoC specific drivers
are probably already there by the time you reach GPU enablement, and the
DT doesn't have to be upstream.

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

Not really, no. We can use the AM62x as an example. The only thing that
was required to enable the driver (excluding the driver itself of
course) was a single DT patch, without anything you've mentioned so far.

> As long as none of that has happened, asking about the PowerVR driver
> on non-AM62x hardware is futile...

Maybe. Just like asking whether you want to enable the UMS driver on
platforms that don't have a USB controller. Or, closer to us, whether
you want to enable AMDGPU on platforms without a PCIe bus.

We *never* do that.

If only because we can't. We don't have a per-SoC Kconfig option, so
even if we were to merge your patch, we would still ask about the
PowerVR driver on, let's say, the AM69 that isn't an AM62x and is just
as futile according to you. Or for the TDA4VM, or...

The other reason is that it's just impossible to maintain. You wouldn't
expect everyone, once they got their USB support in, to amend the
Kconfig dependencies for every USB driver out there, would you?

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

I would welcome any patch extending that requirement, or droping that
requirement.

> Oh, dropping that would expose this question to Linus, causing his
> wrath to come down on you... ;-)

Don't threaten me with a good time.

Also, it's already the case for AMDGPU or etnaviv, so I'm not sure what
Linus would have to say about it exactly.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Maxime,

On Wed, Nov 29, 2023 at 11:50 AM Maxime Ripard <mripard@kernel.org> wrote:
> On Wed, Nov 29, 2023 at 11:10:51AM +0100, Geert Uytterhoeven wrote:
> > On Wed, Nov 29, 2023 at 10:23 AM Maxime Ripard <mripard@kernel.org> 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@kernel.org> 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@redhat.com> wrote:
> > > > > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > > >
> > > > > > > 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 just asked, it's not true in most cases. There's some exceptions
> (GX6250 for example) that could require different firmwares depending on
> the SoC it's used in, but it's not the case here.

OK, please tell me how to use it on e.g. R-Car Gen3.

> > > 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 that user doesn't use a defconfig, doesn't use any variant of
> *defconfig make target. Plus, the driver still isn't enabled by default.
>
> > > 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.
>
> The DT binding and DRM driver is already there, the SoC specific drivers

The DT binding only lists "ti,am62-gpu" with "img,img-axe" as a fallback.

> are probably already there by the time you reach GPU enablement, and the
> DT doesn't have to be upstream.

And getting it upstream requires updating the bindings...

> > >   - 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.
>
> Not really, no. We can use the AM62x as an example. The only thing that
> was required to enable the driver (excluding the driver itself of
> course) was a single DT patch, without anything you've mentioned so far.

Who added:

Documentation/devicetree/bindings/gpu/img,powervr.yaml-          - ti,am62-gpu
Documentation/devicetree/bindings/gpu/img,powervr.yaml:      - const:
img,img-axe # IMG AXE GPU model/revision is fully discoverable

?

> > As long as none of that has happened, asking about the PowerVR driver
> > on non-AM62x hardware is futile...
>
> Maybe. Just like asking whether you want to enable the UMS driver on
> platforms that don't have a USB controller. Or, closer to us, whether
> you want to enable AMDGPU on platforms without a PCIe bus.
>
> We *never* do that.

Thanks for not checking ;-)

    if USB
    [...]
    source "drivers/usb/storage/Kconfig"

    config DRM_AMDGPU
            tristate "AMD GPU"
            depends on DRM && PCI && MMU

> If only because we can't. We don't have a per-SoC Kconfig option, so
> even if we were to merge your patch, we would still ask about the
> PowerVR driver on, let's say, the AM69 that isn't an AM62x and is just
> as futile according to you. Or for the TDA4VM, or...

That's why we use per-family options.  It's the next best thing we have.

> The other reason is that it's just impossible to maintain. You wouldn't
> expect everyone, once they got their USB support in, to amend the
> Kconfig dependencies for every USB driver out there, would you?

USB devices are (usually) truly generic, and can be plugged in
everywhere you see a USB port.

> > > 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?
>
> I would welcome any patch extending that requirement, or droping that
> requirement.
>
> > Oh, dropping that would expose this question to Linus, causing his
> > wrath to come down on you... ;-)
>
> Don't threaten me with a good time.
>
> Also, it's already the case for AMDGPU or etnaviv, so I'm not sure what
> Linus would have to say about it exactly.

AMDGPU is a PCI device, and can be plugged everywhere you see a PCI
slot.  Etnaviv could indeed use some dependencies...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 8385 bytes --]

On Wed, Nov 29, 2023 at 12:08:17PM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Wed, Nov 29, 2023 at 11:50 AM Maxime Ripard <mripard@kernel.org> wrote:
> > On Wed, Nov 29, 2023 at 11:10:51AM +0100, Geert Uytterhoeven wrote:
> > > On Wed, Nov 29, 2023 at 10:23 AM Maxime Ripard <mripard@kernel.org> 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@kernel.org> 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@redhat.com> wrote:
> > > > > > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > > > >
> > > > > > > > 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 just asked, it's not true in most cases. There's some exceptions
> > (GX6250 for example) that could require different firmwares depending on
> > the SoC it's used in, but it's not the case here.
> 
> OK, please tell me how to use it on e.g. R-Car Gen3.

I'm not very familiar with the R-Car family of SoCs.

However, if I'm to trust that page: https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs

None of them feature the same GPU than the AM62x, so that question is
completely different?

> > > > 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 that user doesn't use a defconfig, doesn't use any variant of
> > *defconfig make target. Plus, the driver still isn't enabled by default.
> >
> > > > 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.
> >
> > The DT binding and DRM driver is already there, the SoC specific drivers
> 
> The DT binding only lists "ti,am62-gpu" with "img,img-axe" as a fallback.

Sure. And the driver matches on img,img-axe, so it would probe fine even
with a different first compatible.

> > are probably already there by the time you reach GPU enablement, and the
> > DT doesn't have to be upstream.
> 
> And getting it upstream requires updating the bindings...

Right. And you still don't have to put it upstream, so the binding isn't
a requirement either.

> > > >   - 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.
> >
> > Not really, no. We can use the AM62x as an example. The only thing that
> > was required to enable the driver (excluding the driver itself of
> > course) was a single DT patch, without anything you've mentioned so far.
> 
> Who added:
> 
> Documentation/devicetree/bindings/gpu/img,powervr.yaml-          - ti,am62-gpu
> Documentation/devicetree/bindings/gpu/img,powervr.yaml:      - const:
> img,img-axe # IMG AXE GPU model/revision is fully discoverable
> 
> ?

Which is a formality, part of the upstreaming process, but not required
at all from a technical point of view to make a driver probe.

> > > As long as none of that has happened, asking about the PowerVR driver
> > > on non-AM62x hardware is futile...
> >
> > Maybe. Just like asking whether you want to enable the UMS driver on
> > platforms that don't have a USB controller. Or, closer to us, whether
> > you want to enable AMDGPU on platforms without a PCIe bus.
> >
> > We *never* do that.
> 
> Thanks for not checking ;-)
> 
>     if USB
>     [...]
>     source "drivers/usb/storage/Kconfig"
> 
>     config DRM_AMDGPU
>             tristate "AMD GPU"
>             depends on DRM && PCI && MMU

I'm not seeing any platform Kconfig option there.

Most importantly, you were arguing that the GPU should be enabled only
on systems where the GPU is in the SoC, with a firmware in
linux-firmware, and the DT bits in.

And you're now making it equivalent to "the framework handling that
device is compiled in", which I fully agree with: of course a USB device
driver should only be compiled if the USB framework is there.

But "having the framework compiled" and "a controller is functional on a
platform" is completely different, and you know that very well otherwise
you wouldn't have sent that patch in the first place.

> > If only because we can't. We don't have a per-SoC Kconfig option, so
> > even if we were to merge your patch, we would still ask about the
> > PowerVR driver on, let's say, the AM69 that isn't an AM62x and is just
> > as futile according to you. Or for the TDA4VM, or...
> 
> That's why we use per-family options.  It's the next best thing we have.
> 
> > The other reason is that it's just impossible to maintain. You wouldn't
> > expect everyone, once they got their USB support in, to amend the
> > Kconfig dependencies for every USB driver out there, would you?
> 
> USB devices are (usually) truly generic, and can be plugged in
> everywhere you see a USB port.
> 
> > > > 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?
> >
> > I would welcome any patch extending that requirement, or droping that
> > requirement.
> >
> > > Oh, dropping that would expose this question to Linus, causing his
> > > wrath to come down on you... ;-)
> >
> > Don't threaten me with a good time.
> >
> > Also, it's already the case for AMDGPU or etnaviv, so I'm not sure what
> > Linus would have to say about it exactly.
> 
> AMDGPU is a PCI device, and can be plugged everywhere you see a PCI
> slot.  Etnaviv could indeed use some dependencies...

It might be plugged in any PCIe slot. It will not work with any PCIe
controller.

Anyway, there's no point in discussing it further. We're at the point of
sending blank threats so it's not super productive anyway.

As far as I'm concerned, and if there's no new actual technical
argument,

NAK.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Geert Uytterhoeven]

Hi Maxime,

On Wed, Nov 29, 2023 at 12:34 PM Maxime Ripard <mripard@kernel.org> wrote:
> On Wed, Nov 29, 2023 at 12:08:17PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Nov 29, 2023 at 11:50 AM Maxime Ripard <mripard@kernel.org> wrote:
> > > On Wed, Nov 29, 2023 at 11:10:51AM +0100, Geert Uytterhoeven wrote:
> > > > On Wed, Nov 29, 2023 at 10:23 AM Maxime Ripard <mripard@kernel.org> 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@kernel.org> 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@redhat.com> wrote:
> > > > > > > > > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > > > > >
> > > > > > > > > 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 just asked, it's not true in most cases. There's some exceptions
> > > (GX6250 for example) that could require different firmwares depending on
> > > the SoC it's used in, but it's not the case here.
> >
> > OK, please tell me how to use it on e.g. R-Car Gen3.
>
> I'm not very familiar with the R-Car family of SoCs.
>
> However, if I'm to trust that page: https://www.renesas.com/us/en/products/automotive-products/automotive-system-chips-socs
>
> None of them feature the same GPU than the AM62x, so that question is
> completely different?

According to the documentation I have, they contain PowerVR Series6XT
or Series7XE GPUs.

DRM_POWERVR claims to support "Imagination Technologies PowerVR
(Series 6 or later) or IMG GPU".

> > > > > 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 that user doesn't use a defconfig, doesn't use any variant of
> > > *defconfig make target. Plus, the driver still isn't enabled by default.
> > >
> > > > > 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.
> > >
> > > The DT binding and DRM driver is already there, the SoC specific drivers
> >
> > The DT binding only lists "ti,am62-gpu" with "img,img-axe" as a fallback.
>
> Sure. And the driver matches on img,img-axe, so it would probe fine even
> with a different first compatible.
>
> > > are probably already there by the time you reach GPU enablement, and the
> > > DT doesn't have to be upstream.
> >
> > And getting it upstream requires updating the bindings...
>
> Right. And you still don't have to put it upstream, so the binding isn't
> a requirement either.

Oh, how we love out-of-tree...

> > > > >   - 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.
> > >
> > > Not really, no. We can use the AM62x as an example. The only thing that
> > > was required to enable the driver (excluding the driver itself of
> > > course) was a single DT patch, without anything you've mentioned so far.
> >
> > Who added:
> >
> > Documentation/devicetree/bindings/gpu/img,powervr.yaml-          - ti,am62-gpu
> > Documentation/devicetree/bindings/gpu/img,powervr.yaml:      - const:
> > img,img-axe # IMG AXE GPU model/revision is fully discoverable
> >
> > ?
>
> Which is a formality, part of the upstreaming process, but not required
> at all from a technical point of view to make a driver probe.
>
> > > > As long as none of that has happened, asking about the PowerVR driver
> > > > on non-AM62x hardware is futile...
> > >
> > > Maybe. Just like asking whether you want to enable the UMS driver on
> > > platforms that don't have a USB controller. Or, closer to us, whether
> > > you want to enable AMDGPU on platforms without a PCIe bus.
> > >
> > > We *never* do that.
> >
> > Thanks for not checking ;-)
> >
> >     if USB
> >     [...]
> >     source "drivers/usb/storage/Kconfig"
> >
> >     config DRM_AMDGPU
> >             tristate "AMD GPU"
> >             depends on DRM && PCI && MMU
>
> I'm not seeing any platform Kconfig option there.

There is no need for platform dependencies here because USB and PCI
are better gatekeepers.

> Most importantly, you were arguing that the GPU should be enabled only

s/enabled/visible/

> on systems where the GPU is in the SoC, with a firmware in
> linux-firmware, and the DT bits in.

For now, because it is really only supported on AM62x
If that claim is false, please tell me on which other platform it works.

> And you're now making it equivalent to "the framework handling that
> device is compiled in", which I fully agree with: of course a USB device
> driver should only be compiled if the USB framework is there.
>
> But "having the framework compiled" and "a controller is functional on a
> platform" is completely different, and you know that very well otherwise
> you wouldn't have sent that patch in the first place.

DRM is the framework.
DRM_POWERVR is a driver for hardware that can only be used
(for now) on a very limited platform subset.

> > AMDGPU is a PCI device, and can be plugged everywhere you see a PCI
> > slot.  Etnaviv could indeed use some dependencies...
>
> It might be plugged in any PCIe slot. It will not work with any PCIe
> controller.

Why not?

> Anyway, there's no point in discussing it further. We're at the point of
> sending blank threats so it's not super productive anyway.
>
> As far as I'm concerned, and if there's no new actual technical
> argument,

Linus has stated multiple times he does not want to see Kconfig
options that do not make sense and/or cannot be sued.  Such options
are wasting everyone's time.

> NAK.

Oh well...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Nishanth Menon]

On 19:29-20231128, Geert Uytterhoeven wrote:
> 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")

Minor nitpick here - 12 char sha.. otherwise:

Reviewed-by: Nishanth Menon <nm@ti.com>

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/gpu/drm/imagination/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
> index 3bfa2ac212dccb73..af492dbd9afd4ed9 100644
> --- a/drivers/gpu/drm/imagination/Kconfig
> +++ b/drivers/gpu/drm/imagination/Kconfig
> @@ -6,6 +6,7 @@ config DRM_POWERVR
>  	depends on ARM64
>  	depends on DRM
>  	depends on PM
> +	depends on ARCH_K3 || COMPILE_TEST
>  	select DRM_EXEC
>  	select DRM_GEM_SHMEM_HELPER
>  	select DRM_SCHED
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel