[PATCH v1 07/43] dm: acpi: Add support for the NHLT table

[Simon Glass]

Hi Bin,

On Mon, 13 Jul 2020 at 00:10, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Jul 13, 2020 at 3:37 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 7 Jul 2020 at 21:25, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 6 Jul 2020 at 18:22, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 2 Jul 2020 at 22:33, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass <sjg at chromium.org> 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 <sjg at chromium.org>
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > >  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