Received-SPF: pass (google.com: domain of linux-kernel+bounces-212186-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 12 Jun 2024 23:01:31 +0300
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Genes Lists <lists@sapience.com>, linux-kernel@vger.kernel.org,
	mchehab@kernel.org, hverkuil-cisco@xs4all.nl, wentong.wu@intel.com,
	linux-media@vger.kernel.org, linux-acpi@vger.kernel.org,
	"regressions@lists.linux.dev" <regressions@lists.linux.dev>
Subject: Re: [PATCH 1/1] ACPI: scan: Ignore Dell XPS 9320 camera graph port
 nodes
Message-ID: <20240612200131.GQ28989@pendragon.ideasonboard.com>
References: <4b387b4d-f778-4891-9f07-df5fc0a093cd@redhat.com>
 <ZmmQLt7wB-yGQBTw@kekkonen.localdomain>
 <CAJZ5v0ii3WFQRPdfHeeW4M9kXSWDVxxxy02zThcf25mjNwqDAw@mail.gmail.com>
 <ZmmT56Cyvb2FCyav@kekkonen.localdomain>
 <CAJZ5v0hOBggQR_=uA3VuhruQnZihVxHHovpTz4=qfcbiSunsYw@mail.gmail.com>
 <ZmmY3he9vfWVWU3I@kekkonen.localdomain>
 <CAJZ5v0j7HTfg1wY+B+7vhE6tBKPVHMuu_MsFHjaLK70VS_cNEw@mail.gmail.com>
 <ZmnnFueL-Cgw5Eqp@kekkonen.localdomain>
 <CAJZ5v0gtK9yusimCOVV2dGkQWDwQ6=r=vfbgC-eE60Cg-5wk_Q@mail.gmail.com>
 <ZmnrtIEla9R24egi@kekkonen.localdomain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZmnrtIEla9R24egi@kekkonen.localdomain>

On Wed, Jun 12, 2024 at 06:40:52PM +0000, Sakari Ailus wrote:
> On Wed, Jun 12, 2024 at 08:29:21PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Jun 12, 2024 at 8:21 PM Sakari Ailus wrote:
> > > On Wed, Jun 12, 2024 at 03:06:53PM +0200, Rafael J. Wysocki wrote:
> > > > On Wed, Jun 12, 2024 at 2:47 PM Sakari Ailus wrote:
> > > > > 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?
> 
> 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).

Are you aware of any IPU6-based devices, apart from chromebooks, that
have correct ACPI tables for the camera ?

> 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.
> 
> > > 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?

-- 
Regards,

Laurent Pinchart