All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liu Ying <victor.liu@nxp.com>
To: sashiko-reviews@lists.linux.dev
Cc: imx@lists.linux.dev, Frank.Li@kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v12] drm/bridge: imx: Add i.MX93 parallel display format configuration support
Date: Wed, 22 Jul 2026 17:45:26 +0800	[thread overview]
Message-ID: <amCRNuuPkr7fON6k@raspi> (raw)
In-Reply-To: <20260722082359.E14521F000E9@smtp.kernel.org>

On Wed, Jul 22, 2026 at 08:23:59AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Returning a fallback format for unsupported output formats breaks DRM bridge format negotiation.
> --
> 
> --- Patch [1]: [PATCH v12] drm/bridge: imx: Add i.MX93 parallel display format configuration support ---
> commit 62def9c6e85686d63c6204b71b54383955f8f9d1
> Author: Liu Ying <victor.liu@nxp.com>
> drm/bridge: imx: Add i.MX93 parallel display format configuration support
> 
> This commit introduces a new DRM bridge driver to support parallel display
> format configuration on the NXP i.MX93 and i.MX91 SoCs. It handles format
> conversions for the MEDIAMIX BLK-CTRL DISPLAY_MUX register.
> 
> > diff --git a/drivers/gpu/drm/bridge/imx/imx93-pdfc.c b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
> > new file mode 100644
> > index 0000000000000..b084409d22ef2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx/imx93-pdfc.c
> [ ... ]
> > +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;
> > +
> > +	*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");
> > +		input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > +		return input_fmts;
> > +	}
> 
> [Severity: High]
> Does returning a fallback format here break DRM bridge format negotiation?
> 
> When the downstream bridge requests an unsupported output format first,
> drm_atomic_bridge_chain_select_bus_fmts() queries atomic_get_input_bus_fmts()
> to check if the format is supported. By returning MEDIA_BUS_FMT_RGB888_1X24
> instead of 0 formats, this driver falsely claims it can support the requested
> output_fmt.
> 
> The core then proceeds with the unsupported format, which is correctly
> rejected by imx93_pdfc_bridge_atomic_check(), aborting the modeset with
> -EINVAL instead of gracefully negotiating a fallback.

Makes sense.  The reported issue was introduced by v8.  My earlier v5 does
the negotiation correctly.

> 
> Should this instead set *num_input_fmts = 0, free the array, and return NULL
> when the output format is unsupported?

No need to free any array.  Just need to set *num_input_fmts = 0 and then
return NULL when the output format is unsupported.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260722-v6-18-topic-imx93-parallel-display-v12-1-a59dbb4dbc13@nxp.com?part=1

-- 
Regards,
Liu Ying

      reply	other threads:[~2026-07-22  9:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22  8:12 [PATCH v12] drm/bridge: imx: Add i.MX93 parallel display format configuration support Liu Ying
2026-07-22  8:23 ` sashiko-bot
2026-07-22  9:45   ` Liu Ying [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amCRNuuPkr7fON6k@raspi \
    --to=victor.liu@nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=imx@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.