RE: [PATCH v9 0/7] Add AMD ISP4 driver

[Du, Bin]

[AMD Official Use Only - AMD Internal Distribution Only]

Thanks, Sakari

>-----Original Message-----
>From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>Sent: Thursday, March 12, 2026 6:18 AM
>To: Du, Bin <Bin.Du@xxxxxxx>
>Cc: mchehab@xxxxxxxxxx; hverkuil@xxxxxxxxx;
>laurent.pinchart+renesas@xxxxxxxxxxxxxxxx; bryan.odonoghue@xxxxxxxxxx;
>prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
>linux-kernel@xxxxxxxxxxxxxxx; sultan@xxxxxxxxxxxxxxx; Nirujogi, Pratap
><Pratap.Nirujogi@xxxxxxx>; Chan, Benjamin (Koon Pan)
><Benjamin.Chan@xxxxxxx>; Li, King <King.Li@xxxxxxx>;
>gjorgji.rosikopulos@xxxxxxx; Jawich, Phil <Phil.Jawich@xxxxxxx>; Antony,
>Dominic <Dominic.Antony@xxxxxxx>; Limonciello, Mario
><Mario.Limonciello@xxxxxxx>; Gong, Richard <Richard.Gong@xxxxxxx>;
>Tsao, Anson <anson.tsao@xxxxxxx>
>Subject: Re: [PATCH v9 0/7] Add AMD ISP4 driver
>
>Hi Bin,
>
>On Mon, Mar 02, 2026 at 03:30:13PM +0800, Bin Du wrote:
>> Hello,
>>
>> AMD ISP4 is the AMD image processing gen 4 which can be found in HP
>> ZBook Ultra G1a 14 inch Mobile Workstation PC (Ryzen AI Max 300
>> Series)
>> (https://ubuntu.com/certified/202411-36043)
>> This patch series introduces the initial driver support for the AMD ISP4.
>
>This seems to fail to compile on LLVM. Could you check <URL:https://linux-
>media.pages.freedesktop.org/-/users/sailus/-
>/jobs/94941990/artifacts/junit/llvm.err.txt>

This issue was not observed in my local build or the media CI build with earlier versions. I am unsure whether there have been any updates to the media CI tests. Nevertheless, I will make the necessary modifications to ensure compatibility across all compiler versions.

>and also the smatch warning here
><URL:https://linux-media.pages.freedesktop.org/-/users/sailus/-
>/jobs/94955201/artifacts/junit/test-smatch.err.txt>?
>

Based on following code, I believe it's a false negative, by line 462, 'ele' is either not added to the queue or has been removed by isp4if_rm_cmd_from_cmdq() called at line 453 or by isp4sd_fw_resp_cmd_done(). This appears to be a smatch limitation due to list operations being split across helper functions.

static int isp4if_send_fw_cmd()
{
...
err_dequeue_ele:
        /*
         * Try to remove the command from the queue. If that fails, then it
         * means the response thread is currently using the object, and we need
         * to use the refcount to avoid a use-after-free by either side.
         */
        if (ele && isp4if_rm_cmd_from_cmdq(ispif, seq_num, cmd_id))
                goto free_ele;

put_ele_ref:
        /* Don't free the command if we didn't put the last reference */
        if (ele && atomic_dec_return(&ele->refcnt))
                ele = NULL;

free_ele:
        kfree(ele); // smatch gives warning, '&ele->list' not removed from list

}

>There may be other issues, the Media CI system right now isn't very vocal about
>them.
>
>--
>Kind regards,
>
>Sakari Ailus