Hi Rafael, On Wed, Jun 12, 2024 at 08:50:57PM +0200, Rafael J. Wysocki wrote: > Hi Sakari, > > On Wed, Jun 12, 2024 at 8:41 PM Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > Hi Rafael, > > > > On Wed, Jun 12, 2024 at 08:29:21PM +0200, Rafael J. Wysocki wrote: > > > Hi Sakari, > > > > > > On Wed, Jun 12, 2024 at 8:21 PM Sakari Ailus > > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > > > Hi Rafael, > > > > > > > > On Wed, Jun 12, 2024 at 03:06:53PM +0200, 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? > > > > > > > > Simple answer: for the same reason why in general system specific > > > > information comes from ACPI and not from platform data compiled into the > > > > kernel. > > > > > > > > Of course this is technically possible but it does not scale. > > > > > > While I agree in general, in this particular case the platform data > > > compiled into the kernel needs to be present anyway, at least > > > apparently, in case the data coming from the platform firmware is > > > invalid. > > > > > > So we need to do 3 things: compile in the platform data into the > > > kernel and expect the platform firmware to provide the necessary > > > information, and add quirks for the systems where it is known invalid. > > > > > > Isn't this a bit too much? > > > > Isn't this pretty much how ACPI works currently? > > No, we don't need to put platform data into the kernel for every bit > of information that can be retrieved from the platform firmware via > ACPI. > > The vast majority of information in the ACPI tables is actually > correct and if quirks are needed, they usually are limited in scope. > > Where it breaks is when the ACPI tables are not sufficiently validated > by OEMs which mostly happens when the data in question are not needed > to pass some sort of certification or admission tests. > > Which unfortunately is related to whether or not Windows uses those data. > > > We can support systems that contain correct DSDT description of cameras > > without platform data. I was, until recently, only aware of Dell XPS 9315 > > that has incorrect camera description and that based on recent findings > > seems to extend to other Dell systems with IPU6 (Hans's patches have the > > details). > > > > Still this is not a reason to break systems that have correct camera > > description and expect the users to report them so they can be listed as > > such. > > Well, what do you mean by "break". I thought that platform data > needed to support them were built into the kernel, weren't they? If you add a whitelist for systems where the port aren't just dropped, that is bound to break camera on a lot of current systems that depend on the said port nodes. > > > > > > > > On laptops shipped with Windows some additional information is also available > > > > from ACPI via custom objects but a lot of information is just hard coded into > > > > the IPU bridge as well as the INT3472 driver. > > > > > > Well, that's how it goes. > > > > Yes, but is it desirable? > > No, it is not desirable, but the way to address it is to convince the > Windows people to stop doing this and use standard-defined data from > the ACPI tables instead. It cannot be addressed by Linux unilaterally > trying to do the right thing, because there are OEMs who don't care > about Linux. I don't disagree with that as such but it's not really the matter here, is it? Historically, some systems were amended with special "Linux support" which I believe is what these Dell systems have. That was done because the IPU bridge driver did not exist yet. I frankly don't think it was ever tested on Linux either. -- Kind regards, Sakari Ailus
