[PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Mohan Kumar]

Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
ARM64 based platforms as Intel PCI driver was used for registering the
sound card.

Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f3053e7018fe..abdf0ff469d7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -812,6 +812,7 @@ CONFIG_LOGO=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_ALOOP=m
+CONFIG_SND_HDA_INTEL=m
 CONFIG_SND_HDA_TEGRA=m
 CONFIG_SND_HDA_CODEC_HDMI=m
 CONFIG_SND_SOC=y
-- 
2.17.1

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 17/01/2023 19:16, Mohan Kumar wrote:
> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
> ARM64 based platforms as Intel PCI driver was used for registering the
> sound card.

It's not a part of SoC, not a common device used during debugging or
development, so I don't think it is reasonable to enable it. We do not
enable driver just because someone uses them. Otherwise please clarify
which board has this device embedded (not pluggable by user, but embedded).

Best regards,
Krzysztof

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Mohan Kumar D]

On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 17/01/2023 19:16, Mohan Kumar wrote:
>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>> ARM64 based platforms as Intel PCI driver was used for registering the
>> sound card.
> It's not a part of SoC, not a common device used during debugging or
> development, so I don't think it is reasonable to enable it. We do not
> enable driver just because someone uses them. Otherwise please clarify
> which board has this device embedded (not pluggable by user, but embedded).
This change is required for enabling HDA sound registration for Nvidia 
discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
>
> Best regards,
> Krzysztof
>

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 18/01/2023 12:46, Mohan Kumar D wrote:
> 
> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>> sound card.
>> It's not a part of SoC, not a common device used during debugging or
>> development, so I don't think it is reasonable to enable it. We do not
>> enable driver just because someone uses them. Otherwise please clarify
>> which board has this device embedded (not pluggable by user, but embedded).
> This change is required for enabling HDA sound registration for Nvidia 
> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.

You can plug anything to PCI slot and we do not enable every such PCI
adapter.

Best regards,
Krzysztof

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Mohan Kumar D]

On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 18/01/2023 12:46, Mohan Kumar D wrote:
>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>>> sound card.
>>> It's not a part of SoC, not a common device used during debugging or
>>> development, so I don't think it is reasonable to enable it. We do not
>>> enable driver just because someone uses them. Otherwise please clarify
>>> which board has this device embedded (not pluggable by user, but embedded).
>> This change is required for enabling HDA sound registration for Nvidia
>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
> You can plug anything to PCI slot and we do not enable every such PCI
> adapter.
Without this config enabled, the Intel hda audio driver won't be built 
and dGPU won't be able to register sound card. Do you have any 
suggestion here?
>
> Best regards,
> Krzysztof
>

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 20/01/2023 06:48, Mohan Kumar D wrote:
> 
> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 18/01/2023 12:46, Mohan Kumar D wrote:
>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>>>> sound card.
>>>> It's not a part of SoC, not a common device used during debugging or
>>>> development, so I don't think it is reasonable to enable it. We do not
>>>> enable driver just because someone uses them. Otherwise please clarify
>>>> which board has this device embedded (not pluggable by user, but embedded).
>>> This change is required for enabling HDA sound registration for Nvidia
>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
>> You can plug anything to PCI slot and we do not enable every such PCI
>> adapter.
> Without this config enabled, the Intel hda audio driver won't be built 
> and dGPU won't be able to register sound card. Do you have any 
> suggestion here?

Without hundreds of other drivers they also won't be built and won't be
usable. Anyway, this is just defconfig, so it does not matter. You can
always enable it in your setup, why is this a problem?

Again, we do not enable drivers for every PCI card.

Best regards,
Krzysztof

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Catalin Marinas]

On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
> On 20/01/2023 06:48, Mohan Kumar D wrote:
> > On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
> >> External email: Use caution opening links or attachments
> >> On 18/01/2023 12:46, Mohan Kumar D wrote:
> >>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
> >>>> External email: Use caution opening links or attachments
> >>>> On 17/01/2023 19:16, Mohan Kumar wrote:
> >>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
> >>>>> ARM64 based platforms as Intel PCI driver was used for registering the
> >>>>> sound card.
> >>>> It's not a part of SoC, not a common device used during debugging or
> >>>> development, so I don't think it is reasonable to enable it. We do not
> >>>> enable driver just because someone uses them. Otherwise please clarify
> >>>> which board has this device embedded (not pluggable by user, but embedded).
> >>> This change is required for enabling HDA sound registration for Nvidia
> >>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
> >> You can plug anything to PCI slot and we do not enable every such PCI
> >> adapter.
> > Without this config enabled, the Intel hda audio driver won't be built 
> > and dGPU won't be able to register sound card. Do you have any 
> > suggestion here?
> 
> Without hundreds of other drivers they also won't be built and won't be
> usable. Anyway, this is just defconfig, so it does not matter. You can
> always enable it in your setup, why is this a problem?
> 
> Again, we do not enable drivers for every PCI card.

I don't think we have any set rules for what goes in a defconfig. If one
has a real use-case, we tend to enable stuff in defconfig, especially if
it's a module.

-- 
Catalin

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 20/01/2023 17:56, Catalin Marinas wrote:
> On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
>> On 20/01/2023 06:48, Mohan Kumar D wrote:
>>> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
>>>> External email: Use caution opening links or attachments
>>>> On 18/01/2023 12:46, Mohan Kumar D wrote:
>>>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>>>>>> sound card.
>>>>>> It's not a part of SoC, not a common device used during debugging or
>>>>>> development, so I don't think it is reasonable to enable it. We do not
>>>>>> enable driver just because someone uses them. Otherwise please clarify
>>>>>> which board has this device embedded (not pluggable by user, but embedded).
>>>>> This change is required for enabling HDA sound registration for Nvidia
>>>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
>>>> You can plug anything to PCI slot and we do not enable every such PCI
>>>> adapter.
>>> Without this config enabled, the Intel hda audio driver won't be built 
>>> and dGPU won't be able to register sound card. Do you have any 
>>> suggestion here?
>>
>> Without hundreds of other drivers they also won't be built and won't be
>> usable. Anyway, this is just defconfig, so it does not matter. You can
>> always enable it in your setup, why is this a problem?
>>
>> Again, we do not enable drivers for every PCI card.
> 
> I don't think we have any set rules for what goes in a defconfig. If one
> has a real use-case, we tend to enable stuff in defconfig, especially if
> it's a module.

There will be always an use case for every PCI and USB card. It's not
related to storage or networking which could justify device bringup
(rootfs). It's really not needed for any board operation. defconfig is
not for marketing products but for our development and reference platforms.

The only argument behind this change is "I have a PCI card and I want it
in defconfig", but why it has to be in defconfig in the first place?
There is no reason. This is not distro...

Best regards,
Krzysztof

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]

On Fri, Jan 20, 2023 at 06:00:25PM +0100, Krzysztof Kozlowski wrote:
> On 20/01/2023 17:56, Catalin Marinas wrote:
> > On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
> >> On 20/01/2023 06:48, Mohan Kumar D wrote:
> >>> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
> >>>> External email: Use caution opening links or attachments
> >>>> On 18/01/2023 12:46, Mohan Kumar D wrote:
> >>>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
> >>>>>> External email: Use caution opening links or attachments
> >>>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
> >>>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
> >>>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
> >>>>>>> sound card.
> >>>>>> It's not a part of SoC, not a common device used during debugging or
> >>>>>> development, so I don't think it is reasonable to enable it. We do not
> >>>>>> enable driver just because someone uses them. Otherwise please clarify
> >>>>>> which board has this device embedded (not pluggable by user, but embedded).
> >>>>> This change is required for enabling HDA sound registration for Nvidia
> >>>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
> >>>> You can plug anything to PCI slot and we do not enable every such PCI
> >>>> adapter.
> >>> Without this config enabled, the Intel hda audio driver won't be built 
> >>> and dGPU won't be able to register sound card. Do you have any 
> >>> suggestion here?
> >>
> >> Without hundreds of other drivers they also won't be built and won't be
> >> usable. Anyway, this is just defconfig, so it does not matter. You can
> >> always enable it in your setup, why is this a problem?
> >>
> >> Again, we do not enable drivers for every PCI card.
> > 
> > I don't think we have any set rules for what goes in a defconfig. If one
> > has a real use-case, we tend to enable stuff in defconfig, especially if
> > it's a module.
> 
> There will be always an use case for every PCI and USB card. It's not
> related to storage or networking which could justify device bringup
> (rootfs). It's really not needed for any board operation. defconfig is
> not for marketing products but for our development and reference platforms.

If defconfig were only for boot-critical drivers, it's terribly bloated
as it is. We enable things like multimedia, infrared and audio. None of
those are critical to booting a system. Heck, we also enable most of
DRM/KMS, which are useful for boot on consumer devices, but are rarely
critical on development and reference platforms.

Besides, a PCI board can be considered a development platform depending
on who you are.

I've always looked at defconfig as more of a guideline as to what's a
useful baseline configuration for an architecture.

> The only argument behind this change is "I have a PCI card and I want it
> in defconfig", but why it has to be in defconfig in the first place?
> There is no reason. This is not distro...

That's highly subjective and honestly that argument can go in both
directions. People can, after all, start from an allnoconfig and then
work their way up to something that's usable on their particular device.
Or they could start from an allmodconfig and work their way down.

The point of defconfig is to give you something that's somewhere between
the two extremes. Obviously if we start enabling everything, it defeats
that purpose. If we prohibit the enablement of new options, we equally
limit its usefulness.

Thierry

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

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 23/01/2023 16:58, Thierry Reding wrote:
> On Fri, Jan 20, 2023 at 06:00:25PM +0100, Krzysztof Kozlowski wrote:
>> On 20/01/2023 17:56, Catalin Marinas wrote:
>>> On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
>>>> On 20/01/2023 06:48, Mohan Kumar D wrote:
>>>>> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>> On 18/01/2023 12:46, Mohan Kumar D wrote:
>>>>>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>>>>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>>>>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>>>>>>>> sound card.
>>>>>>>> It's not a part of SoC, not a common device used during debugging or
>>>>>>>> development, so I don't think it is reasonable to enable it. We do not
>>>>>>>> enable driver just because someone uses them. Otherwise please clarify
>>>>>>>> which board has this device embedded (not pluggable by user, but embedded).
>>>>>>> This change is required for enabling HDA sound registration for Nvidia
>>>>>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
>>>>>> You can plug anything to PCI slot and we do not enable every such PCI
>>>>>> adapter.
>>>>> Without this config enabled, the Intel hda audio driver won't be built 
>>>>> and dGPU won't be able to register sound card. Do you have any 
>>>>> suggestion here?
>>>>
>>>> Without hundreds of other drivers they also won't be built and won't be
>>>> usable. Anyway, this is just defconfig, so it does not matter. You can
>>>> always enable it in your setup, why is this a problem?
>>>>
>>>> Again, we do not enable drivers for every PCI card.
>>>
>>> I don't think we have any set rules for what goes in a defconfig. If one
>>> has a real use-case, we tend to enable stuff in defconfig, especially if
>>> it's a module.
>>
>> There will be always an use case for every PCI and USB card. It's not
>> related to storage or networking which could justify device bringup
>> (rootfs). It's really not needed for any board operation. defconfig is
>> not for marketing products but for our development and reference platforms.
> 
> If defconfig were only for boot-critical drivers, it's terribly bloated

We enable drivers for devices present in our platforms. Everything which
is on such platforms. For pluggable USB/PCI/whatever third-party
devices, then comes the argument as boot-related.

> as it is. We enable things like multimedia, infrared and audio. None of
> those are critical to booting a system. Heck, we also enable most of
> DRM/KMS, which are useful for boot on consumer devices, but are rarely
> critical on development and reference platforms.
> 
> Besides, a PCI board can be considered a development platform depending
> on who you are.
> 
> I've always looked at defconfig as more of a guideline as to what's a
> useful baseline configuration for an architecture.

Yep and this one here is nowhere near that architecture. It's pluggable
card, not related to hardware nor arm64 (If I understood correctly). Why
you do not enable it on x86? Or multi_v7? or hundreds of other defconfigs?

> 
>> The only argument behind this change is "I have a PCI card and I want it
>> in defconfig", but why it has to be in defconfig in the first place?
>> There is no reason. This is not distro...
> 
> That's highly subjective and honestly that argument can go in both
> directions. People can, after all, start from an allnoconfig and then
> work their way up to something that's usable on their particular device.
> Or they could start from an allmodconfig and work their way down.

I am sorry, but adding new stuff does not require arguments against.
Adding new stuff requires argument for it. You reverse the argumentation
that I need to find proves that we do not need it in mainline platforms,
if I got your response correctly.

> 
> The point of defconfig is to give you something that's somewhere between
> the two extremes. Obviously if we start enabling everything, it defeats
> that purpose. If we prohibit the enablement of new options, we equally
> limit its usefulness.

I don't think we discuss the same thing. There are no extremes here at
all. The patch is about enabling arm64-unrelated PCI pluggable device,
just because it came from @nvidia.com author. If you think some PCI
pluggable 3rd party device is suitable for defconfig, I will bring
hundreds of other drivers I am also plugging over PCI to my boards, just
because I want some audio.

It's not reasonable path...

Best regards,
Krzysztof

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 6827 bytes --]

On Mon, Jan 23, 2023 at 05:05:08PM +0100, Krzysztof Kozlowski wrote:
> On 23/01/2023 16:58, Thierry Reding wrote:
> > On Fri, Jan 20, 2023 at 06:00:25PM +0100, Krzysztof Kozlowski wrote:
> >> On 20/01/2023 17:56, Catalin Marinas wrote:
> >>> On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 20/01/2023 06:48, Mohan Kumar D wrote:
> >>>>> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
> >>>>>> External email: Use caution opening links or attachments
> >>>>>> On 18/01/2023 12:46, Mohan Kumar D wrote:
> >>>>>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
> >>>>>>>> External email: Use caution opening links or attachments
> >>>>>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
> >>>>>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
> >>>>>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
> >>>>>>>>> sound card.
> >>>>>>>> It's not a part of SoC, not a common device used during debugging or
> >>>>>>>> development, so I don't think it is reasonable to enable it. We do not
> >>>>>>>> enable driver just because someone uses them. Otherwise please clarify
> >>>>>>>> which board has this device embedded (not pluggable by user, but embedded).
> >>>>>>> This change is required for enabling HDA sound registration for Nvidia
> >>>>>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
> >>>>>> You can plug anything to PCI slot and we do not enable every such PCI
> >>>>>> adapter.
> >>>>> Without this config enabled, the Intel hda audio driver won't be built 
> >>>>> and dGPU won't be able to register sound card. Do you have any 
> >>>>> suggestion here?
> >>>>
> >>>> Without hundreds of other drivers they also won't be built and won't be
> >>>> usable. Anyway, this is just defconfig, so it does not matter. You can
> >>>> always enable it in your setup, why is this a problem?
> >>>>
> >>>> Again, we do not enable drivers for every PCI card.
> >>>
> >>> I don't think we have any set rules for what goes in a defconfig. If one
> >>> has a real use-case, we tend to enable stuff in defconfig, especially if
> >>> it's a module.
> >>
> >> There will be always an use case for every PCI and USB card. It's not
> >> related to storage or networking which could justify device bringup
> >> (rootfs). It's really not needed for any board operation. defconfig is
> >> not for marketing products but for our development and reference platforms.
> > 
> > If defconfig were only for boot-critical drivers, it's terribly bloated
> 
> We enable drivers for devices present in our platforms. Everything which
> is on such platforms. For pluggable USB/PCI/whatever third-party
> devices, then comes the argument as boot-related.
> 
> > as it is. We enable things like multimedia, infrared and audio. None of
> > those are critical to booting a system. Heck, we also enable most of
> > DRM/KMS, which are useful for boot on consumer devices, but are rarely
> > critical on development and reference platforms.
> > 
> > Besides, a PCI board can be considered a development platform depending
> > on who you are.
> > 
> > I've always looked at defconfig as more of a guideline as to what's a
> > useful baseline configuration for an architecture.
> 
> Yep and this one here is nowhere near that architecture. It's pluggable
> card, not related to hardware nor arm64 (If I understood correctly). Why
> you do not enable it on x86? Or multi_v7? or hundreds of other defconfigs?

My understanding is that the code behind SND_HDA_INTEL, despite the
name, is in fact generic PCI HDA code and is compatible with a number of
non-Intel devices. It's also nothing x86-specific because this code
works on any PCI-enabled platform (it's for driving generic PCI HDA
controllers).

It's quite common for people to hook up PCI cards on NVIDIA Jetson
platforms (which are arm64). Often these will be NVIDIA GPUs, but they
can be any brand of GPU, really (SND_HDA_INTEL supports hardware from a
large number of vendors).

> >> The only argument behind this change is "I have a PCI card and I want it
> >> in defconfig", but why it has to be in defconfig in the first place?
> >> There is no reason. This is not distro...
> > 
> > That's highly subjective and honestly that argument can go in both
> > directions. People can, after all, start from an allnoconfig and then
> > work their way up to something that's usable on their particular device.
> > Or they could start from an allmodconfig and work their way down.
> 
> I am sorry, but adding new stuff does not require arguments against.
> Adding new stuff requires argument for it. You reverse the argumentation
> that I need to find proves that we do not need it in mainline platforms,
> if I got your response correctly.

Adding new stuff also doesn't require arguments in favour. At least
that's not been my experience in kernel development. If somebody spends
the time to write a driver, we don't tend to ask proof of why it's
useful, we trust that they wouldn't have spent the effort if they didn't
think it worth it.

Besides, this is to enable support for these various HDA controllers
that can be plugged into Jetson platforms, which very well are mainline
platforms.

> > The point of defconfig is to give you something that's somewhere between
> > the two extremes. Obviously if we start enabling everything, it defeats
> > that purpose. If we prohibit the enablement of new options, we equally
> > limit its usefulness.
> 
> I don't think we discuss the same thing. There are no extremes here at
> all. The patch is about enabling arm64-unrelated PCI pluggable device,
> just because it came from @nvidia.com author. If you think some PCI

I think you're making this into something it's not. What would be the
benefit for NVIDIA to enable something that is unused? If there's
consensus that only boot-critical pieces should go into defconfig, then
that's fine with me. Currently that's not the case, so painting this as
some sort of marketing stunt is hardly fair.

> pluggable 3rd party device is suitable for defconfig, I will bring
> hundreds of other drivers I am also plugging over PCI to my boards, just
> because I want some audio.
> 
> It's not reasonable path...

Well, then what is a reasonable path? We've had per-platform defconfigs
on 32-bit ARM and people weren't happy about those. But if the single
defconfig on 64-bit ARM doesn't generate a configuration that is useful
for many people, then we don't need it. People could then just keep
their own, local default configurations.

Surely I'm not the only one that gets annoyed if I have to tweak config
and rebuild multiple times until I have the right configuration.

Thierry

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

Re: [PATCH v2] arm64: defconfig: Enable HDA INTEL config for ARM64

[Krzysztof Kozlowski]

On 23/01/2023 18:26, Thierry Reding wrote:
> On Mon, Jan 23, 2023 at 05:05:08PM +0100, Krzysztof Kozlowski wrote:
>> On 23/01/2023 16:58, Thierry Reding wrote:
>>> On Fri, Jan 20, 2023 at 06:00:25PM +0100, Krzysztof Kozlowski wrote:
>>>> On 20/01/2023 17:56, Catalin Marinas wrote:
>>>>> On Fri, Jan 20, 2023 at 07:20:01AM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 20/01/2023 06:48, Mohan Kumar D wrote:
>>>>>>> On 18-01-2023 18:06, Krzysztof Kozlowski wrote:
>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>> On 18/01/2023 12:46, Mohan Kumar D wrote:
>>>>>>>>> On 18-01-2023 13:04, Krzysztof Kozlowski wrote:
>>>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>>> On 17/01/2023 19:16, Mohan Kumar wrote:
>>>>>>>>>>> Enable CONFIG_SND_HDA_INTEL for NVIDIA PCI based graphics sound card for
>>>>>>>>>>> ARM64 based platforms as Intel PCI driver was used for registering the
>>>>>>>>>>> sound card.
>>>>>>>>>> It's not a part of SoC, not a common device used during debugging or
>>>>>>>>>> development, so I don't think it is reasonable to enable it. We do not
>>>>>>>>>> enable driver just because someone uses them. Otherwise please clarify
>>>>>>>>>> which board has this device embedded (not pluggable by user, but embedded).
>>>>>>>>> This change is required for enabling HDA sound registration for Nvidia
>>>>>>>>> discrete GPU cards based on PCI and pluggable to Nvidia Jetson Platforms.
>>>>>>>> You can plug anything to PCI slot and we do not enable every such PCI
>>>>>>>> adapter.
>>>>>>> Without this config enabled, the Intel hda audio driver won't be built 
>>>>>>> and dGPU won't be able to register sound card. Do you have any 
>>>>>>> suggestion here?
>>>>>>
>>>>>> Without hundreds of other drivers they also won't be built and won't be
>>>>>> usable. Anyway, this is just defconfig, so it does not matter. You can
>>>>>> always enable it in your setup, why is this a problem?
>>>>>>
>>>>>> Again, we do not enable drivers for every PCI card.
>>>>>
>>>>> I don't think we have any set rules for what goes in a defconfig. If one
>>>>> has a real use-case, we tend to enable stuff in defconfig, especially if
>>>>> it's a module.
>>>>
>>>> There will be always an use case for every PCI and USB card. It's not
>>>> related to storage or networking which could justify device bringup
>>>> (rootfs). It's really not needed for any board operation. defconfig is
>>>> not for marketing products but for our development and reference platforms.
>>>
>>> If defconfig were only for boot-critical drivers, it's terribly bloated
>>
>> We enable drivers for devices present in our platforms. Everything which
>> is on such platforms. For pluggable USB/PCI/whatever third-party
>> devices, then comes the argument as boot-related.
>>
>>> as it is. We enable things like multimedia, infrared and audio. None of
>>> those are critical to booting a system. Heck, we also enable most of
>>> DRM/KMS, which are useful for boot on consumer devices, but are rarely
>>> critical on development and reference platforms.
>>>
>>> Besides, a PCI board can be considered a development platform depending
>>> on who you are.
>>>
>>> I've always looked at defconfig as more of a guideline as to what's a
>>> useful baseline configuration for an architecture.
>>
>> Yep and this one here is nowhere near that architecture. It's pluggable
>> card, not related to hardware nor arm64 (If I understood correctly). Why
>> you do not enable it on x86? Or multi_v7? or hundreds of other defconfigs?
> 
> My understanding is that the code behind SND_HDA_INTEL, despite the
> name, is in fact generic PCI HDA code and is compatible with a number of
> non-Intel devices. It's also nothing x86-specific because this code
> works on any PCI-enabled platform (it's for driving generic PCI HDA
> controllers).
> 
> It's quite common for people to hook up PCI cards on NVIDIA Jetson
> platforms (which are arm64). Often these will be NVIDIA GPUs, but they
> can be any brand of GPU, really (SND_HDA_INTEL supports hardware from a
> large number of vendors).

Like with every other board with PCI or miniPCI slots. You can hook up
there anything and we do not enable all these possible devices.

> 
>>>> The only argument behind this change is "I have a PCI card and I want it
>>>> in defconfig", but why it has to be in defconfig in the first place?
>>>> There is no reason. This is not distro...
>>>
>>> That's highly subjective and honestly that argument can go in both
>>> directions. People can, after all, start from an allnoconfig and then
>>> work their way up to something that's usable on their particular device.
>>> Or they could start from an allmodconfig and work their way down.
>>
>> I am sorry, but adding new stuff does not require arguments against.
>> Adding new stuff requires argument for it. You reverse the argumentation
>> that I need to find proves that we do not need it in mainline platforms,
>> if I got your response correctly.
> 
> Adding new stuff also doesn't require arguments in favour. At least
> that's not been my experience in kernel development. If somebody spends
> the time to write a driver, we don't tend to ask proof of why it's
> useful, we trust that they wouldn't have spent the effort if they didn't
> think it worth it.

We need that argument and submitter provides - I have a device for it.
That's the argument for including driver in the kernel. That's not an
argument good enough for including in defconfig, though.

> Besides, this is to enable support for these various HDA controllers
> that can be plugged into Jetson platforms, which very well are mainline
> platforms.
> 
>>> The point of defconfig is to give you something that's somewhere between
>>> the two extremes. Obviously if we start enabling everything, it defeats
>>> that purpose. If we prohibit the enablement of new options, we equally
>>> limit its usefulness.
>>
>> I don't think we discuss the same thing. There are no extremes here at
>> all. The patch is about enabling arm64-unrelated PCI pluggable device,
>> just because it came from @nvidia.com author. If you think some PCI
> 
> I think you're making this into something it's not. What would be the
> benefit for NVIDIA to enable something that is unused? If there's
> consensus that only boot-critical pieces should go into defconfig, then
> that's fine with me. Currently that's not the case, so painting this as
> some sort of marketing stunt is hardly fair.

As I said - no one here  speaks about boot-critical pieces. We enable
everything which is on the boards. Everything.

Now for fully user pluggable, common devices - like USB, PCI - we enable
stuff relevant to the boot.

> 
>> pluggable 3rd party device is suitable for defconfig, I will bring
>> hundreds of other drivers I am also plugging over PCI to my boards, just
>> because I want some audio.
>>
>> It's not reasonable path...
> 
> Well, then what is a reasonable path? We've had per-platform defconfigs
> on 32-bit ARM and people weren't happy about those. But if the single
> defconfig on 64-bit ARM doesn't generate a configuration that is useful
> for many people, then we don't need it. People could then just keep
> their own, local default configurations.

And what is the problem with it? It's just defconfig, so what's the big
deal for them to have out of tree patch?

For me it is big deal because I am using and building defconfig all the
time. It uses my electricity, so directly my money. It uses my time,
which is even more important than money.

If anyone wants user-pluggable devices, please add fragments or one more
defconfig, so I won't be affected by such drivers.

> 
> Surely I'm not the only one that gets annoyed if I have to tweak config
> and rebuild multiple times until I have the right configuration.
> 
> Thierry

Best regards,
Krzysztof