Hi Rafael, On Wed, Jun 12, 2024 at 09:12:59PM +0200, Rafael J. Wysocki wrote: > Hi Sakari, > > On Wed, Jun 12, 2024 at 9:07 PM Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > Hi Rafael, > > > > On Wed, Jun 12, 2024 at 08:41:43PM +0200, Rafael J. Wysocki wrote: > > > Hi Sakari, > > > > > > On Wed, Jun 12, 2024 at 8:33 PM Sakari Ailus > > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > > > Hi Rafael, > > > > > > > > On Wed, Jun 12, 2024 at 05:26:46PM +0200, Rafael J. Wysocki wrote: > > > > > Hi, > > > > > > > > > > On Wed, Jun 12, 2024 at 4:30 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > On 6/12/24 3:06 PM, Rafael J. Wysocki wrote: > > > > > > > Hi Sakari, > > > > > > > > > > > > > > On Wed, Jun 12, 2024 at 2:47 PM Sakari Ailus > > > > > > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > >> > > > > > > >> Hi Rafael, > > > > > > >> > > > > > > >> On Wed, Jun 12, 2024 at 02:32:26PM +0200, Rafael J. Wysocki wrote: > > > > > > >>>>>>> I just hit the same problem on another Dell laptop. It seems that > > > > > > >>>>>>> all Dell laptops with IPU6 camera from the Tiger Lake, Alder Lake > > > > > > >>>>>>> and Raptor Lake generations suffer from this problem. > > > > > > >>>>>>> > > > > > > >>>>>>> So instead of playing whack a mole with DMI matches we should > > > > > > >>>>>>> simply disable ACPI MIPI DISCO support on all Dell laptops > > > > > > >>>>>>> with those CPUs. I'm preparing a fix for this to replace > > > > > > >>>>>>> the DMI matching now. > > > > > > >>>>>> > > > > > > >>>>>> DisCo for Imaging support shouldn't be dropped on these systems, and this > > > > > > >>>>>> isn't what your patch does either. Instead the ACPI graph port nodes (as > > > > > > >>>>>> per Linux specific definitions) are simply dropped, i.e. this isn't related > > > > > > >>>>>> to DisCo for Imaging at all. > > > > > > >>>>> > > > > > > >>>>> So it looks like the changelog of that patch could be improved, right? > > > > > > >>>> > > > > > > >>>> Well, yes. The reason the function is in the file is that nearly all camera > > > > > > >>>> related parsing is located there, not that it would be related to DisCo for > > > > > > >>>> Imaging as such. > > > > > > >>> > > > > > > >>> So IIUC the camera graph port nodes are created by default with the > > > > > > >>> help of the firmware-supplied information, but if that is defective a > > > > > > >>> quirk can be added to skip the creation of those ports in which case > > > > > > >>> they will be created elsewhere. > > > > > > >>> > > > > > > >>> Is this correct? > > > > > > >> > > > > > > >> Yes. > > > > > > > > > > > > > > So it would be good to add a comment to this effect to > > > > > > > acpi_nondev_subnode_extract() where acpi_graph_ignore_port() is > > > > > > > called. > > > > > > > > > > > > > > And there is a somewhat tangential question that occurred to me: If > > > > > > > the nodes are created elsewhere when acpi_graph_ignore_port() is true, > > > > > > > why is it necessary to consult the platform firmware for the > > > > > > > information on them at all? Wouldn't it be better to simply always > > > > > > > create them elsewhere? > > > > > > > > > > > > That is a good question. The ACPI MIPI DISCO specification is an > > > > > > attempt standardize how MIPI cameras and their sensors are described > > > > > > in ACPI. > > > > > > > > > > > > But this is not actually being used by any Windows drivers atm. The windows > > > > > > drivers rely on their own custom ACPI data which gets translated into > > > > > > standard Linux device-properties by: drivers/media/pci/intel/ipu-bridge.c > > > > > > > > > > > > and so far AFAIK there are 0 laptops where there actually is 100% functional > > > > > > ACPI MIPI information. I believe that some work is in place to get correct > > > > > > usable ACPI MIPI information in place in the ACPI tables of some Meteor Lake > > > > > > laptops. But I believe that there too it does not work yet with the BIOS > > > > > > version with which current Windows models are shipping. It is being fixed > > > > > > for systems which have Linux support from the vendor but I suspect that > > > > > > on other models if ACPI MIPI DISCO information is there it will not > > > > > > necessarily be reliable because AFAICT Windows does not actually use it. > > > > > > > > > > > > And TBH this has me worried about camera support for Meteor Lake devices > > > > > > going forward. We really need to have 1 reliable source of truth here and > > > > > > using information which is ignored by Windows does not seem like the best > > > > > > source to use. > > > > > > > > > > > > Sakari I know you have been pushing for MIPI camera descriptions under > > > > > > ACPI to move to a standardized format and I can see how that is a good > > > > > > thing, but atm it seems to mainly cause things to break and before > > > > > > the ACPI MIPI DISCO support landed in 6.8 we did not have these issues, > > > > > > since the information used by the ipu-bridge code does seem to be correct. > > > > > > > > > > Well, if Windows doesn't use this information, it is almost guaranteed > > > > > to be garbage. > > > > > > > > No ACPI DSDT in production systems uses DisCo for Imaging as of now at > > > > least to my knowledge. > > > > > > > > > > > > > > So maybe it would be better to make acpi_graph_ignore_port() return > > > > > true by default and false only when the information is known to be > > > > > valid. IOW, whitelist things instead of adding blacklist entries in > > > > > perpetuum. > > > > > > > > What could be gained from this? > > > > > > Generally speaking, fewer headaches for people trying to support Linux > > > on Intel client platforms. > > > > I don't think that is the case here. > > > > I'd like to reiterate that none of the issues there have been so far > > (including with Dell laptops) have been related to DisCo for Imaging. > > Well, they were (or are) related to firmware issues that cause systems > to fail to boot if triggered until they get blacklisted in > acpi_graph_ignore_port(). This is the first time I hear about a boot failure due to incorrect camera description (on production systems). Could you point me to where this has happened? -- Kind regards, Sakari Ailus
