On Tue, Mar 03, 2026 at 05:20:09PM -0500, Frank Li wrote:
> On Tue, Mar 03, 2026 at 10:22:02PM +0100, Marco Felsch wrote:
>> On 26-03-03, Frank Li wrote:
>>> From: Frank Li (AI-BOT) <[email protected]>
>>>
>>> AI bot review and may be useless.
>>
>> Hi Frank,
>>
>> albeit I'm very open to new technology, I would appreciate it if your
>> AI-BOT is used internally first till you're convinced that it reports
>> real issues instead of false-positives.
>>
>> Regards,
>> Marco
>>
>>>> +static u32 *
>>>> +imx93_pdfc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
>>>> + struct drm_bridge_state
>>>> *bridge_state,
>>>> + struct drm_crtc_state *crtc_state,
>>>> + struct drm_connector_state
>>>> *conn_state,
>>>> + u32 output_fmt,
>>>> + unsigned int *num_input_fmts)
>>>> +{
>>>> + struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
>>>> + u32 *input_fmts;
>>>> +
>>>> + *num_input_fmts = 0;
>>>> +
>>>> + input_fmts = kmalloc_obj(*input_fmts);
>>>> + if (!input_fmts)
>>>> + return NULL;
>>>
>
>> + input_fmts = kmalloc_obj(*input_fmts);
>
> AI:
>
> Actually, this looks incorrect. kmalloc_obj() allocates based on the type
> of the pointer argument. Here you're passing *input_fmts (a u32), not
> input_fmts (a u32 *).
This comment is false. Per kmalloc_obj()'s kerneldoc, it's first argument
is "Variable or type to allocate". In this particular case, the argument
should be "*input_fmts (a u32)".
/**
* kmalloc_obj - Allocate a single instance of the given type
* @VAR_OR_TYPE: Variable or type to allocate.
* @GFP: GFP flags for the allocation.
*
* Returns: newly allocated pointer to a @VAR_OR_TYPE on success, or NULL
* on failure.
*/
#define kmalloc_obj(VAR_OR_TYPE, ...) \
__alloc_objs(kmalloc, default_gfp(__VA_ARGS__), typeof(VAR_OR_TYPE), 1)
> Should be:
>
> + input_fmts = kmalloc_array(*num_input_fmts, sizeof(*input_fmts),
> + GFP_KERNEL);
No. "input_fmts = kmalloc_obj(*input_fmts);" is just fine.
>
> suppose you use kmalloc_objs()
No. We should use kmalloc_obj(), not kmalloc_objs().
>
> Frank
>
>>> Missing kfree(input_fmts) in error path if the switch statement
>>> or subsequent logic fails. Consider allocating a fixed-size array
>>> or using devm_kzalloc() instead.
>>>
>>>> + *num_input_fmts = 1;
>>>> +
>>>> + if (!imx93_pdfc_bus_output_fmt_supported(output_fmt)) {
>>>> + dev_dbg(pdfc->dev, "No valid output bus-fmt detected, fallback
>>>> to MEDIA_BUS_FMT_RGB888_1X24\n");
>>>
>>> Line exceeds 80 characters (97 chars). Break into two lines.
>>>
>>>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>> + return input_fmts;
>>>> + }
>>>> +
>>>> + switch (output_fmt) {
>>>> + case MEDIA_BUS_FMT_RGB888_1X24:
>>>> + case MEDIA_BUS_FMT_RGB565_1X16:
>>>> + input_fmts[0] = output_fmt;
>>>> + break;
>>>> + case MEDIA_BUS_FMT_RGB666_1X18:
>>>> + case MEDIA_BUS_FMT_FIXED:
>>>> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>> + break;
>>>> + }
>>>
>>> Switch statement lacks default case. Add default case to handle
>>> unexpected format values explicitly.
>>>
>>>> +static int imx93_pdfc_bridge_atomic_enable(struct drm_bridge *bridge,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct imx93_pdfc *pdfc = bridge_to_imx93_pdfc(bridge);
>>>> + const struct drm_bridge_state *bridge_state;
>>>> + unsigned int mask = PARALLEL_DISP_FORMAT;
>>>> + unsigned int val;
>>>> +
>>>> + bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
>>>> +
>>>> + switch (bridge_state->output_bus_cfg.format) {
>>>> + case MEDIA_BUS_FMT_RGB888_1X24:
>>>> + case MEDIA_BUS_FMT_FIXED:
>>>> + val = FORMAT_RGB888_TO_RGB888;
>>>> + if (pdfc->phy_bus_width == 18) {
>>>> + /*
>>>> + * Can be valid if physical bus limitation exists,
>>>> + * therefore use dev_dbg().
>>>> + */
>>>> + dev_dbg(pdfc->dev, "Truncate two LSBs from each
>>>> color\n");
>>>> + val = FORMAT_RGB888_TO_RGB666;
>>>> + }
>>>> + break;
>>>> + case MEDIA_BUS_FMT_RGB666_1X18:
>>>> + val = FORMAT_RGB888_TO_RGB666;
>>>> +
>>>
>>
>> --
>> #gernperDu
>> #CallMeByMyFirstName
>>
>> Pengutronix e.K. | |
>> Steuerwalder Str. 21 | https://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
--
Regards,
Liu Ying
