Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Mon, 13 Jul 2020 at 00:10, Bin Meng wrote:
>
> Hi Simon,
>
> On Mon, Jul 13, 2020 at 3:37 AM Simon Glass wrote:
> >
> > Hi Bin,
> >
> > On Tue, 7 Jul 2020 at 21:25, Simon Glass wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 6 Jul 2020 at 18:22, Bin Meng wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT)
> > > > > > > > > > > table describes the
> > > > > > > > > > > audio codecs and connections in a system. Various devices
> > > > > > > > > > > can contribute
> > > > > > > > > > > information to produce the table.
> > > > > > > > > > >
> > > > > > > > > > > Add core support for this, based on a structure which is
> > > > > > > > > > > built up through
> > > > > > > > > > > calls to the driver.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Simon Glass
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > drivers/core/acpi.c | 15 +++
> > > > > > > > > > > include/dm/acpi.h | 26 ++
> > > > > > > > > > > 2 files changed, 41 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > > > + METHOD_SETUP_NHLT,
> > > > > > > > > >
> > > > > > > > > > Do we really need to promote this to be an ACPI core
> > > > > > > > > > method? Can we
> > > > > > > > > > reuse the SSDT/DSDT one?
> > > > > > > > >
> > > > > > > > > I don't think so. Those two are for a particular purpose. In
> > > > > > > > > fact NHLT
> > > > > > > > > is generated while doing SSDT I think. The idea is that
> > > > > > > > > drivers that
> > > > > > > > > want to contribute to NHLT can do so. But we cannot use the
> > > > > > > > > SSDT
> > > > > > > > > mechanism since each driver contributes only a part of the
> > > > > > > > > info, and
> > > > > > > > > we need something else to bring it all together.
> > > > > > > >
> > > > > > > > Will there be only one device that sets up the NHLT info?
> > > > > > >
> > > > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > > > suppose it depends on the audio codecs present.
> > > > > >
> > > > > > Could we make this method to be provided by the codec device,
> > > > > > instead
> > > > > > of a generic ACPI core method?
> > > > >
> > > > > The codec device does implement this. See the drivers where they
> > > > > actually implement the NHLT method.
> > > > >
> > > > > This is definitely an ACPI-specific thing, so I think we need core
> > > > > support for iterating through drivers that want to provide this info.
> > > >
> > > > My concern is that this is not generic enough to promote this to ACPI
> > > > core.
> > > >
> > > > I wanted to have something like this:
> > > >
> > > > Create a codec uclass driver, and in the code uclass driver, create an
> > > > op that is used to set up the NHLT infor if ACPI_GEN is on.
> > >
> > > We already have UCLASS_SOUND so could add it to sound_ops. But that
> > > seems weird to me since this is not an operation to play a sound. We
> > > do this with GIPOs and IRQs but in that case the operation returns
> > > some data. Here we are asking the driver to add some data to a table.
> > > I'm just not sure it makes sense.
> > >
> > > What do you think?
> >
> > Let me know what you think about this as I'd like to finish this series.
>
> I haven't looked into the details of NHLT but I trust your
> consideration so let's leave this as it is.
I think both approaches require bending in some direction due to the
fact that the data structure is 'built up' during the calls, but not
added to ACPI until all calls are done.
I'll try a little series to switch it the other way so we can compare it.
Regards,
Simon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon,
On Mon, Jul 13, 2020 at 3:37 AM Simon Glass wrote:
>
> Hi Bin,
>
> On Tue, 7 Jul 2020 at 21:25, Simon Glass wrote:
> >
> > Hi Bin,
> >
> > On Mon, 6 Jul 2020 at 18:22, Bin Meng wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Bin,
> > > > > > > >
> > > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Simon,
> > > > > > > > >
> > > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table
> > > > > > > > > > describes the
> > > > > > > > > > audio codecs and connections in a system. Various devices
> > > > > > > > > > can contribute
> > > > > > > > > > information to produce the table.
> > > > > > > > > >
> > > > > > > > > > Add core support for this, based on a structure which is
> > > > > > > > > > built up through
> > > > > > > > > > calls to the driver.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > drivers/core/acpi.c | 15 +++
> > > > > > > > > > include/dm/acpi.h | 26 ++
> > > > > > > > > > 2 files changed, 41 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > > + METHOD_SETUP_NHLT,
> > > > > > > > >
> > > > > > > > > Do we really need to promote this to be an ACPI core method?
> > > > > > > > > Can we
> > > > > > > > > reuse the SSDT/DSDT one?
> > > > > > > >
> > > > > > > > I don't think so. Those two are for a particular purpose. In
> > > > > > > > fact NHLT
> > > > > > > > is generated while doing SSDT I think. The idea is that drivers
> > > > > > > > that
> > > > > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > > > > mechanism since each driver contributes only a part of the
> > > > > > > > info, and
> > > > > > > > we need something else to bring it all together.
> > > > > > >
> > > > > > > Will there be only one device that sets up the NHLT info?
> > > > > >
> > > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > > suppose it depends on the audio codecs present.
> > > > >
> > > > > Could we make this method to be provided by the codec device, instead
> > > > > of a generic ACPI core method?
> > > >
> > > > The codec device does implement this. See the drivers where they
> > > > actually implement the NHLT method.
> > > >
> > > > This is definitely an ACPI-specific thing, so I think we need core
> > > > support for iterating through drivers that want to provide this info.
> > >
> > > My concern is that this is not generic enough to promote this to ACPI
> > > core.
> > >
> > > I wanted to have something like this:
> > >
> > > Create a codec uclass driver, and in the code uclass driver, create an
> > > op that is used to set up the NHLT infor if ACPI_GEN is on.
> >
> > We already have UCLASS_SOUND so could add it to sound_ops. But that
> > seems weird to me since this is not an operation to play a sound. We
> > do this with GIPOs and IRQs but in that case the operation returns
> > some data. Here we are asking the driver to add some data to a table.
> > I'm just not sure it makes sense.
> >
> > What do you think?
>
> Let me know what you think about this as I'd like to finish this series.
I haven't looked into the details of NHLT but I trust your
consideration so let's leave this as it is.
Regards,
Bin
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Tue, 7 Jul 2020 at 21:25, Simon Glass wrote:
>
> Hi Bin,
>
> On Mon, 6 Jul 2020 at 18:22, Bin Meng wrote:
> >
> > Hi Simon,
> >
> > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass wrote:
> > >
> > > Hi Bin,
> > >
> > > On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table
> > > > > > > > > describes the
> > > > > > > > > audio codecs and connections in a system. Various devices can
> > > > > > > > > contribute
> > > > > > > > > information to produce the table.
> > > > > > > > >
> > > > > > > > > Add core support for this, based on a structure which is
> > > > > > > > > built up through
> > > > > > > > > calls to the driver.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > drivers/core/acpi.c | 15 +++
> > > > > > > > > include/dm/acpi.h | 26 ++
> > > > > > > > > 2 files changed, 41 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > + METHOD_SETUP_NHLT,
> > > > > > > >
> > > > > > > > Do we really need to promote this to be an ACPI core method?
> > > > > > > > Can we
> > > > > > > > reuse the SSDT/DSDT one?
> > > > > > >
> > > > > > > I don't think so. Those two are for a particular purpose. In fact
> > > > > > > NHLT
> > > > > > > is generated while doing SSDT I think. The idea is that drivers
> > > > > > > that
> > > > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > > > mechanism since each driver contributes only a part of the info,
> > > > > > > and
> > > > > > > we need something else to bring it all together.
> > > > > >
> > > > > > Will there be only one device that sets up the NHLT info?
> > > > >
> > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > suppose it depends on the audio codecs present.
> > > >
> > > > Could we make this method to be provided by the codec device, instead
> > > > of a generic ACPI core method?
> > >
> > > The codec device does implement this. See the drivers where they
> > > actually implement the NHLT method.
> > >
> > > This is definitely an ACPI-specific thing, so I think we need core
> > > support for iterating through drivers that want to provide this info.
> >
> > My concern is that this is not generic enough to promote this to ACPI core.
> >
> > I wanted to have something like this:
> >
> > Create a codec uclass driver, and in the code uclass driver, create an
> > op that is used to set up the NHLT infor if ACPI_GEN is on.
>
> We already have UCLASS_SOUND so could add it to sound_ops. But that
> seems weird to me since this is not an operation to play a sound. We
> do this with GIPOs and IRQs but in that case the operation returns
> some data. Here we are asking the driver to add some data to a table.
> I'm just not sure it makes sense.
>
> What do you think?
Let me know what you think about this as I'd like to finish this series.
Regards,
SImon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Mon, 6 Jul 2020 at 18:22, Bin Meng wrote:
>
> Hi Simon,
>
> On Tue, Jul 7, 2020 at 3:22 AM Simon Glass wrote:
> >
> > Hi Bin,
> >
> > On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table
> > > > > > > > describes the
> > > > > > > > audio codecs and connections in a system. Various devices can
> > > > > > > > contribute
> > > > > > > > information to produce the table.
> > > > > > > >
> > > > > > > > Add core support for this, based on a structure which is built
> > > > > > > > up through
> > > > > > > > calls to the driver.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass
> > > > > > > > ---
> > > > > > > >
> > > > > > > > drivers/core/acpi.c | 15 +++
> > > > > > > > include/dm/acpi.h | 26 ++
> > > > > > > > 2 files changed, 41 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > + METHOD_SETUP_NHLT,
> > > > > > >
> > > > > > > Do we really need to promote this to be an ACPI core method? Can
> > > > > > > we
> > > > > > > reuse the SSDT/DSDT one?
> > > > > >
> > > > > > I don't think so. Those two are for a particular purpose. In fact
> > > > > > NHLT
> > > > > > is generated while doing SSDT I think. The idea is that drivers that
> > > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > > mechanism since each driver contributes only a part of the info, and
> > > > > > we need something else to bring it all together.
> > > > >
> > > > > Will there be only one device that sets up the NHLT info?
> > > >
> > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > suppose it depends on the audio codecs present.
> > >
> > > Could we make this method to be provided by the codec device, instead
> > > of a generic ACPI core method?
> >
> > The codec device does implement this. See the drivers where they
> > actually implement the NHLT method.
> >
> > This is definitely an ACPI-specific thing, so I think we need core
> > support for iterating through drivers that want to provide this info.
>
> My concern is that this is not generic enough to promote this to ACPI core.
>
> I wanted to have something like this:
>
> Create a codec uclass driver, and in the code uclass driver, create an
> op that is used to set up the NHLT infor if ACPI_GEN is on.
We already have UCLASS_SOUND so could add it to sound_ops. But that
seems weird to me since this is not an operation to play a sound. We
do this with GIPOs and IRQs but in that case the operation returns
some data. Here we are asking the driver to add some data to a table.
I'm just not sure it makes sense.
What do you think?
Regards,
SImon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon,
On Tue, Jul 7, 2020 at 3:22 AM Simon Glass wrote:
>
> Hi Bin,
>
> On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
> >
> > Hi Simon,
> >
> > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
> > >
> > > Hi Bin,
> > >
> > > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > > wrote:
> > > > > > >
> > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table
> > > > > > > describes the
> > > > > > > audio codecs and connections in a system. Various devices can
> > > > > > > contribute
> > > > > > > information to produce the table.
> > > > > > >
> > > > > > > Add core support for this, based on a structure which is built up
> > > > > > > through
> > > > > > > calls to the driver.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass
> > > > > > > ---
> > > > > > >
> > > > > > > drivers/core/acpi.c | 15 +++
> > > > > > > include/dm/acpi.h | 26 ++
> > > > > > > 2 files changed, 41 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > --- a/drivers/core/acpi.c
> > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > METHOD_WRITE_TABLES,
> > > > > > > METHOD_FILL_SSDT,
> > > > > > > METHOD_INJECT_DSDT,
> > > > > > > + METHOD_SETUP_NHLT,
> > > > > >
> > > > > > Do we really need to promote this to be an ACPI core method? Can we
> > > > > > reuse the SSDT/DSDT one?
> > > > >
> > > > > I don't think so. Those two are for a particular purpose. In fact NHLT
> > > > > is generated while doing SSDT I think. The idea is that drivers that
> > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > mechanism since each driver contributes only a part of the info, and
> > > > > we need something else to bring it all together.
> > > >
> > > > Will there be only one device that sets up the NHLT info?
> > >
> > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > suppose it depends on the audio codecs present.
> >
> > Could we make this method to be provided by the codec device, instead
> > of a generic ACPI core method?
>
> The codec device does implement this. See the drivers where they
> actually implement the NHLT method.
>
> This is definitely an ACPI-specific thing, so I think we need core
> support for iterating through drivers that want to provide this info.
My concern is that this is not generic enough to promote this to ACPI core.
I wanted to have something like this:
Create a codec uclass driver, and in the code uclass driver, create an
op that is used to set up the NHLT infor if ACPI_GEN is on.
Regards,
Bin
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Thu, 2 Jul 2020 at 22:33, Bin Meng wrote:
>
> Hi Simon,
>
> On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
> >
> > Hi Bin,
> >
> > On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass
> > > > > wrote:
> > > > > >
> > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table
> > > > > > describes the
> > > > > > audio codecs and connections in a system. Various devices can
> > > > > > contribute
> > > > > > information to produce the table.
> > > > > >
> > > > > > Add core support for this, based on a structure which is built up
> > > > > > through
> > > > > > calls to the driver.
> > > > > >
> > > > > > Signed-off-by: Simon Glass
> > > > > > ---
> > > > > >
> > > > > > drivers/core/acpi.c | 15 +++
> > > > > > include/dm/acpi.h | 26 ++
> > > > > > 2 files changed, 41 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > --- a/drivers/core/acpi.c
> > > > > > +++ b/drivers/core/acpi.c
> > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > METHOD_WRITE_TABLES,
> > > > > > METHOD_FILL_SSDT,
> > > > > > METHOD_INJECT_DSDT,
> > > > > > + METHOD_SETUP_NHLT,
> > > > >
> > > > > Do we really need to promote this to be an ACPI core method? Can we
> > > > > reuse the SSDT/DSDT one?
> > > >
> > > > I don't think so. Those two are for a particular purpose. In fact NHLT
> > > > is generated while doing SSDT I think. The idea is that drivers that
> > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > mechanism since each driver contributes only a part of the info, and
> > > > we need something else to bring it all together.
> > >
> > > Will there be only one device that sets up the NHLT info?
> >
> > WIth coral it is two devices. I'm not sure of the maximum, but I
> > suppose it depends on the audio codecs present.
>
> Could we make this method to be provided by the codec device, instead
> of a generic ACPI core method?
The codec device does implement this. See the drivers where they
actually implement the NHLT method.
This is definitely an ACPI-specific thing, so I think we need core
support for iterating through drivers that want to provide this info.
Regards,
Simon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon,
On Fri, Jul 3, 2020 at 11:50 AM Simon Glass wrote:
>
> Hi Bin,
>
> On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
> >
> > Hi Simon,
> >
> > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass wrote:
> > > > >
> > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes
> > > > > the
> > > > > audio codecs and connections in a system. Various devices can
> > > > > contribute
> > > > > information to produce the table.
> > > > >
> > > > > Add core support for this, based on a structure which is built up
> > > > > through
> > > > > calls to the driver.
> > > > >
> > > > > Signed-off-by: Simon Glass
> > > > > ---
> > > > >
> > > > > drivers/core/acpi.c | 15 +++
> > > > > include/dm/acpi.h | 26 ++
> > > > > 2 files changed, 41 insertions(+)
> > > > >
> > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > index ea304a3067..a5053fec6f 100644
> > > > > --- a/drivers/core/acpi.c
> > > > > +++ b/drivers/core/acpi.c
> > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > METHOD_WRITE_TABLES,
> > > > > METHOD_FILL_SSDT,
> > > > > METHOD_INJECT_DSDT,
> > > > > + METHOD_SETUP_NHLT,
> > > >
> > > > Do we really need to promote this to be an ACPI core method? Can we
> > > > reuse the SSDT/DSDT one?
> > >
> > > I don't think so. Those two are for a particular purpose. In fact NHLT
> > > is generated while doing SSDT I think. The idea is that drivers that
> > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > mechanism since each driver contributes only a part of the info, and
> > > we need something else to bring it all together.
> >
> > Will there be only one device that sets up the NHLT info?
>
> WIth coral it is two devices. I'm not sure of the maximum, but I
> suppose it depends on the audio codecs present.
Could we make this method to be provided by the codec device, instead
of a generic ACPI core method?
Regards,
Bin
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Thu, 2 Jul 2020 at 18:54, Bin Meng wrote:
>
> Hi Simon,
>
> On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
> >
> > Hi Bin,
> >
> > On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass wrote:
> > > >
> > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes
> > > > the
> > > > audio codecs and connections in a system. Various devices can contribute
> > > > information to produce the table.
> > > >
> > > > Add core support for this, based on a structure which is built up
> > > > through
> > > > calls to the driver.
> > > >
> > > > Signed-off-by: Simon Glass
> > > > ---
> > > >
> > > > drivers/core/acpi.c | 15 +++
> > > > include/dm/acpi.h | 26 ++
> > > > 2 files changed, 41 insertions(+)
> > > >
> > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > index ea304a3067..a5053fec6f 100644
> > > > --- a/drivers/core/acpi.c
> > > > +++ b/drivers/core/acpi.c
> > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > METHOD_WRITE_TABLES,
> > > > METHOD_FILL_SSDT,
> > > > METHOD_INJECT_DSDT,
> > > > + METHOD_SETUP_NHLT,
> > >
> > > Do we really need to promote this to be an ACPI core method? Can we
> > > reuse the SSDT/DSDT one?
> >
> > I don't think so. Those two are for a particular purpose. In fact NHLT
> > is generated while doing SSDT I think. The idea is that drivers that
> > want to contribute to NHLT can do so. But we cannot use the SSDT
> > mechanism since each driver contributes only a part of the info, and
> > we need something else to bring it all together.
>
> Will there be only one device that sets up the NHLT info?
WIth coral it is two devices. I'm not sure of the maximum, but I
suppose it depends on the audio codecs present.
Regards,
Simon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon,
On Fri, Jul 3, 2020 at 8:46 AM Simon Glass wrote:
>
> Hi Bin,
>
> On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
> >
> > Hi Simon,
> >
> > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass wrote:
> > >
> > > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the
> > > audio codecs and connections in a system. Various devices can contribute
> > > information to produce the table.
> > >
> > > Add core support for this, based on a structure which is built up through
> > > calls to the driver.
> > >
> > > Signed-off-by: Simon Glass
> > > ---
> > >
> > > drivers/core/acpi.c | 15 +++
> > > include/dm/acpi.h | 26 ++
> > > 2 files changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > index ea304a3067..a5053fec6f 100644
> > > --- a/drivers/core/acpi.c
> > > +++ b/drivers/core/acpi.c
> > > @@ -31,6 +31,7 @@ enum method_t {
> > > METHOD_WRITE_TABLES,
> > > METHOD_FILL_SSDT,
> > > METHOD_INJECT_DSDT,
> > > + METHOD_SETUP_NHLT,
> >
> > Do we really need to promote this to be an ACPI core method? Can we
> > reuse the SSDT/DSDT one?
>
> I don't think so. Those two are for a particular purpose. In fact NHLT
> is generated while doing SSDT I think. The idea is that drivers that
> want to contribute to NHLT can do so. But we cannot use the SSDT
> mechanism since each driver contributes only a part of the info, and
> we need something else to bring it all together.
Will there be only one device that sets up the NHLT info?
Regards,
Bin
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Bin,
On Mon, 29 Jun 2020 at 20:49, Bin Meng wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 11:57 AM Simon Glass wrote:
> >
> > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the
> > audio codecs and connections in a system. Various devices can contribute
> > information to produce the table.
> >
> > Add core support for this, based on a structure which is built up through
> > calls to the driver.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > drivers/core/acpi.c | 15 +++
> > include/dm/acpi.h | 26 ++
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > index ea304a3067..a5053fec6f 100644
> > --- a/drivers/core/acpi.c
> > +++ b/drivers/core/acpi.c
> > @@ -31,6 +31,7 @@ enum method_t {
> > METHOD_WRITE_TABLES,
> > METHOD_FILL_SSDT,
> > METHOD_INJECT_DSDT,
> > + METHOD_SETUP_NHLT,
>
> Do we really need to promote this to be an ACPI core method? Can we
> reuse the SSDT/DSDT one?
I don't think so. Those two are for a particular purpose. In fact NHLT
is generated while doing SSDT I think. The idea is that drivers that
want to contribute to NHLT can do so. But we cannot use the SSDT
mechanism since each driver contributes only a part of the info, and
we need something else to bring it all together.
Regards,
Simon
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon,
On Mon, Jun 15, 2020 at 11:57 AM Simon Glass wrote:
>
> The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the
> audio codecs and connections in a system. Various devices can contribute
> information to produce the table.
>
> Add core support for this, based on a structure which is built up through
> calls to the driver.
>
> Signed-off-by: Simon Glass
> ---
>
> drivers/core/acpi.c | 15 +++
> include/dm/acpi.h | 26 ++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> index ea304a3067..a5053fec6f 100644
> --- a/drivers/core/acpi.c
> +++ b/drivers/core/acpi.c
> @@ -31,6 +31,7 @@ enum method_t {
> METHOD_WRITE_TABLES,
> METHOD_FILL_SSDT,
> METHOD_INJECT_DSDT,
> + METHOD_SETUP_NHLT,
Do we really need to promote this to be an ACPI core method? Can we
reuse the SSDT/DSDT one?
> };
>
> /* Prototype for all methods */
> @@ -239,6 +240,8 @@ acpi_method acpi_get_method(struct udevice *dev, enum
> method_t method)
> return aops->fill_ssdt;
> case METHOD_INJECT_DSDT:
> return aops->inject_dsdt;
> + case METHOD_SETUP_NHLT:
> + return aops->setup_nhlt;
> }
> }
>
> @@ -325,3 +328,15 @@ int acpi_write_dev_tables(struct acpi_ctx *ctx)
>
> return ret;
> }
> +
> +int acpi_setup_nhlt(struct acpi_ctx *ctx, struct nhlt *nhlt)
> +{
> + int ret;
> +
> + log_debug("Setup NHLT\n");
> + ctx->nhlt = nhlt;
> + ret = acpi_recurse_method(ctx, dm_root(), METHOD_SETUP_NHLT,
> TYPE_NONE);
> + log_debug("Setup finished, err=%d\n", ret);
> +
> + return ret;
> +}
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index b6308b9fa6..a7f8e10ee2 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -27,6 +27,8 @@
>
> #if !defined(__ACPI__)
>
> +struct nhlt;
> +
> /** enum acpi_dump_option - selects what ACPI information to dump */
> enum acpi_dump_option {
> ACPI_DUMP_LIST, /* Just the list of items */
> @@ -44,6 +46,9 @@ enum acpi_dump_option {
> * adding a new table. The RSDP holds pointers to the RSDT and XSDT.
> * @rsdt: Pointer to the Root System Description Table
> * @xsdt: Pointer to the Extended System Description Table
> + * @nhlt: Intel Non-High-Definition-Audio Link Table (NHLT) pointer, used to
> + * build up information that audio codecs need to provide in the NHLT
> ACPI
> + * table
> * @len_stack: Stack of 'length' words to fix up later
> * @ltop: Points to current top of stack (0 = empty)
> */
> @@ -53,6 +58,7 @@ struct acpi_ctx {
> struct acpi_rsdp *rsdp;
> struct acpi_rsdt *rsdt;
> struct acpi_xsdt *xsdt;
> + struct nhlt *nhlt;
> char *len_stack[ACPIGEN_LENSTACK_SIZE];
> int ltop;
> };
> @@ -106,6 +112,15 @@ struct acpi_ops {
> * @return 0 if OK, -ve on error
> */
> int (*inject_dsdt)(const struct udevice *dev, struct acpi_ctx *ctx);
> +
> + /**
> +* setup_nhlt() - Set up audio information for this device
> +*
> +* The method can add information to ctx->nhlt if it likes
> +*
> +* @return 0 if OK, -ENODATA if nothing to add, -ve on error
> +*/
> + int (*setup_nhlt)(const struct udevice *dev, struct acpi_ctx *ctx);
> };
>
> #define device_get_acpi_ops(dev) ((dev)->driver->acpi_ops)
> @@ -170,6 +185,17 @@ int acpi_fill_ssdt(struct acpi_ctx *ctx);
> */
> int acpi_inject_dsdt(struct acpi_ctx *ctx);
>
> +/**
> + * acpi_setup_nhlt() - Set up audio information
> + *
> + * This is called to set up the nhlt information for all devices.
> + *
> + * @ctx: ACPI context to use
> + * @nhlt: Pointer to nhlt information to add to
> + * @return 0 if OK, -ve on error
> + */
> +int acpi_setup_nhlt(struct acpi_ctx *ctx, struct nhlt *nhlt);
> +
> /**
> * acpi_dump_items() - Dump out the collected ACPI items
> *
> --
Regards,
Bin
Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table
Hi Simon, -"Simon Glass" schrieb: - > Betreff: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table > > The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the > audio codecs and connections in a system. Various devices can contribute > information to produce the table. > > Add core support for this, based on a structure which is built up through > calls to the driver. > > Signed-off-by: Simon Glass > --- > > drivers/core/acpi.c | 15 +++ > include/dm/acpi.h | 26 ++ > 2 files changed, 41 insertions(+) Reviewed-by: Wolfgang Wallner
[PATCH v1 07/43] dm: acpi: Add support for the NHLT table
The Intel Non-High-Definition-Audio Link Table (NHLT) table describes the
audio codecs and connections in a system. Various devices can contribute
information to produce the table.
Add core support for this, based on a structure which is built up through
calls to the driver.
Signed-off-by: Simon Glass
---
drivers/core/acpi.c | 15 +++
include/dm/acpi.h | 26 ++
2 files changed, 41 insertions(+)
diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
index ea304a3067..a5053fec6f 100644
--- a/drivers/core/acpi.c
+++ b/drivers/core/acpi.c
@@ -31,6 +31,7 @@ enum method_t {
METHOD_WRITE_TABLES,
METHOD_FILL_SSDT,
METHOD_INJECT_DSDT,
+ METHOD_SETUP_NHLT,
};
/* Prototype for all methods */
@@ -239,6 +240,8 @@ acpi_method acpi_get_method(struct udevice *dev, enum
method_t method)
return aops->fill_ssdt;
case METHOD_INJECT_DSDT:
return aops->inject_dsdt;
+ case METHOD_SETUP_NHLT:
+ return aops->setup_nhlt;
}
}
@@ -325,3 +328,15 @@ int acpi_write_dev_tables(struct acpi_ctx *ctx)
return ret;
}
+
+int acpi_setup_nhlt(struct acpi_ctx *ctx, struct nhlt *nhlt)
+{
+ int ret;
+
+ log_debug("Setup NHLT\n");
+ ctx->nhlt = nhlt;
+ ret = acpi_recurse_method(ctx, dm_root(), METHOD_SETUP_NHLT, TYPE_NONE);
+ log_debug("Setup finished, err=%d\n", ret);
+
+ return ret;
+}
diff --git a/include/dm/acpi.h b/include/dm/acpi.h
index b6308b9fa6..a7f8e10ee2 100644
--- a/include/dm/acpi.h
+++ b/include/dm/acpi.h
@@ -27,6 +27,8 @@
#if !defined(__ACPI__)
+struct nhlt;
+
/** enum acpi_dump_option - selects what ACPI information to dump */
enum acpi_dump_option {
ACPI_DUMP_LIST, /* Just the list of items */
@@ -44,6 +46,9 @@ enum acpi_dump_option {
* adding a new table. The RSDP holds pointers to the RSDT and XSDT.
* @rsdt: Pointer to the Root System Description Table
* @xsdt: Pointer to the Extended System Description Table
+ * @nhlt: Intel Non-High-Definition-Audio Link Table (NHLT) pointer, used to
+ * build up information that audio codecs need to provide in the NHLT ACPI
+ * table
* @len_stack: Stack of 'length' words to fix up later
* @ltop: Points to current top of stack (0 = empty)
*/
@@ -53,6 +58,7 @@ struct acpi_ctx {
struct acpi_rsdp *rsdp;
struct acpi_rsdt *rsdt;
struct acpi_xsdt *xsdt;
+ struct nhlt *nhlt;
char *len_stack[ACPIGEN_LENSTACK_SIZE];
int ltop;
};
@@ -106,6 +112,15 @@ struct acpi_ops {
* @return 0 if OK, -ve on error
*/
int (*inject_dsdt)(const struct udevice *dev, struct acpi_ctx *ctx);
+
+ /**
+* setup_nhlt() - Set up audio information for this device
+*
+* The method can add information to ctx->nhlt if it likes
+*
+* @return 0 if OK, -ENODATA if nothing to add, -ve on error
+*/
+ int (*setup_nhlt)(const struct udevice *dev, struct acpi_ctx *ctx);
};
#define device_get_acpi_ops(dev) ((dev)->driver->acpi_ops)
@@ -170,6 +185,17 @@ int acpi_fill_ssdt(struct acpi_ctx *ctx);
*/
int acpi_inject_dsdt(struct acpi_ctx *ctx);
+/**
+ * acpi_setup_nhlt() - Set up audio information
+ *
+ * This is called to set up the nhlt information for all devices.
+ *
+ * @ctx: ACPI context to use
+ * @nhlt: Pointer to nhlt information to add to
+ * @return 0 if OK, -ve on error
+ */
+int acpi_setup_nhlt(struct acpi_ctx *ctx, struct nhlt *nhlt);
+
/**
* acpi_dump_items() - Dump out the collected ACPI items
*
--
2.27.0.290.gba653c62da-goog
