Re: [PATCH v12] platform/x86: Add AMD ISP platform config for OV05C10

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Hans,

On Wed, May 07, 2025 at 11:13:18PM +0200, Hans de Goede wrote:
> Hi Sakari,
> 
> On 6-May-25 5:37 PM, Sakari Ailus wrote:
> > Hi Pratap,
> > 
> > On Mon, May 05, 2025 at 01:11:26PM -0400, Pratap Nirujogi wrote:
> >> ISP device specific configuration is not available in ACPI. Add
> >> swnode graph to configure the missing device properties for the
> >> OV05C10 camera device supported on amdisp platform.
> >>
> >> Add support to create i2c-client dynamically when amdisp i2c
> >> adapter is available.
> >>
> >> Co-developed-by: Benjamin Chan <benjamin.chan@amd.com>
> >> Signed-off-by: Benjamin Chan <benjamin.chan@amd.com>
> >> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> >> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
> >> ---
> 
> <snip>
> 
> >> +/*
> >> + * Remote endpoint AMD ISP node definition. No properties defined for
> >> + * remote endpoint node for OV05C10.
> > 
> > How will this scale? Can you use other sensors with this ISP? Although if
> > you get little from firmware, there's not much you can do. That being said,
> > switching to DisCo for Imaging could be an easier step in this case.
> 
> Note I've already talked to AMD about the way the camera setup
> is currently being described in ACPI tables is suboptimal and
> how they really should use proper ACPI description using e.g.
> a _CRS with an I2cSerialBus resource for the sensor.

That's one thing, yes, but it's not enough to get rid of the board code.

> 
> Although I must admit I did not bring up the ACPI DisCo for imaging
> spec as something to also look at for future generations.

I think we should really try to get rid of the board code the raw cameras
on ACPI systems currently depend on, in future systems, instead of just
reducing it a little bit. MIPI DisCo for Imaging enables that.

I guess you're not very familiar with Intel-based ChromeOS systems in this
area? Maybe largely because they work out of the box. And there's no board
code for these systems in the kernel. These are based (albeit I'm not quite
sure about the latest ones) on older Linux-based definitions whereas newer
MIPI DisCo for Imaging spec is OS-independent.

> 
> Note that there currently is hw shipping using the somewhat
> broken ACPI sensor description this glue driver binds to,
> so we're stuck with dealing with these ACPI tables as they
> are already out there in the wild.

I agree, there's little that can be done at this point.

> 
> But yes for future hw generations it would be good to have
> a better description of the hw in ACPI.

-- 
Regards,

Sakari Ailus