Message-ID: <2a9ba94e-7985-4ba9-88c6-45b8cf4d001f@amd.com>
Date: Thu, 8 May 2025 19:13:24 -0400
From: "Nirujogi, Pratap" <pnirujog@....com>
To: Sakari Ailus <sakari.ailus@....fi>, Hans de Goede <hdegoede@...hat.com>
Cc: Pratap Nirujogi <pratap.nirujogi@....com>, W_Armin@....de,
 ilpo.jarvinen@...ux.intel.com, mario.limonciello@....com,
 platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
 benjamin.chan@....com, bin.du@....com, gjorgji.rosikopulos@....com,
 king.li@....com, dantony@....com
Subject: Re: [PATCH v12] platform/x86: Add AMD ISP platform config for OV05C10



On 5/8/2025 3:17 AM, Sakari Ailus wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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@....com>
>>>> Signed-off-by: Benjamin Chan <benjamin.chan@....com>
>>>> Reviewed-by: Mario Limonciello <mario.limonciello@....com>
>>>> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
>>>> Reviewed-by: Armin Wolf <W_Armin@....de>
>>>> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@....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.
> 
Hi Sakari, Hans,

Thanks for your support and understanding. We will work internally and 
ensure our future models adheres to MIPI DisCo Imaging spec.

Thanks,
Pratap

> --
> Regards,
> 
> Sakari Ailus

Powered by blists - more mailing lists