Re: [PATCH v2 0/8] Add AMD ISP4 driver

["Du, Bin" <bin.du@amd.com>]

Thanks Sultan.

On 8/22/2025 11:56 AM, Sultan Alsawaf wrote:
> On Fri, Aug 22, 2025 at 10:23:48AM +0800, Du, Bin wrote:
>> On 8/14/2025 2:53 PM, Sultan Alsawaf wrote:
>>> On Wed, Jun 18, 2025 at 05:19:51PM +0800, Bin Du wrote:
>>>> AMD ISP4 Key features:
>>>> - Processes bayer raw data from the connected sensor and output them to different YUV formats
>>>> - Downscale input image to different output image resolution
>>>> - Pipeline to do image processing on the input image including demosaic, denoise, 3A, etc
>>>
>>> BTW, another problem I have which I would love some help with: may I get the FW
>>> commands for setting basic 3A parameters?
>>>
>>> It seems like the default AE mode does frame-averaging, which is really
>>> unsuitable for video calls on the webcam. My face is really underexposed as a
>>> result during the daytime because there's a lot of ambient light in the
>>> background.
>>>
>>> The webcam on this laptop also has a very wide field of view, which makes my
>>> face appear small and shows too much of the background. This also exacerbates
>>> the AE problem.
>>>
>>> I'm thinking CMD_ID_SET_ZOOM would fix the FOV problem, and then either
>>> CMD_ID_AE_SET_MODE to change the AE mode or CMD_ID_AE_SET_REGION to set the AE
>>> ROI would fix the exposure problem. What do you think?
>>>
>>> Thanks,
>>> Sultan
>>
>> Thanks Sultan for the suggestion, sorry for the late response because we
>> spent some time internally to discuss the feasibility.
> 
> Thanks for looking into this!
> 
>> Yes, it's really good suggestion. Because current V4l2 doesn't have standard
>> ioctl for like region setting, to support it, besides adding FW command, new
>> customized ioctl also needs be added and no existing APP can benefit from
>> it. So our idea is not to add them to our current upstream driver, but we
>> would be really glad to help you to enable them locally with dedicated
>> thread, suppose it can help to improve the IQ with correct input setting
>> like the correct ROI region, but we aren't sure because we didn't do that
>> before on Linux and would really expect your test result and feedback.
> 
> I'm happy to help develop this and even help write the code. :)
> 

Thank you very much for your consistent help, that's invaluable.

> I think a lot of useful functionality can be put upstream just through V4L2,
> like V4L2_CID_EXPOSURE_METERING to control the AE mode.
> 

Yes, that's feasible, the consideration is if we add support to control 
AE mode, as a complete solution, then we have to add support to set 
again and exposure time in manual mode by FW command , besides that, 
extra work is also needed from QA to develop new testcases, all these 
together will cost more time. So, our current plan is first to upstream 
a version with basic functions, after that, we can submit incremental 
patches to add more features. Does it make sense?

> For advanced functions that don't have a standard V4L2 control, maybe we can set
> some defaults in the driver to improve the experience for front-facing cameras,
> i.e. webcams.
>> Depending on the features exposed by FW, maybe setting a ROI for AE isn't
> necessary. Is it possible for the FW to do face detection and set 3A according
> to the face landmarks/ROI?
> 

Oh, so sad, our FW doesn't embed algorithm to do face detection. :(

> Thanks,
> Sultan

-- 
Regards,
Bin