All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Tomi Valkeinen" <tomi.valkeinen+renesas@ideasonboard.com>
Cc: devicetree@vger.kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, robh@kernel.org,
	wsa+renesas@sang-engineering.com
Subject: Re: [PATCH v2 3/5] drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver
Date: Fri, 15 May 2026 08:30:57 +0000	[thread overview]
Message-ID: <20260515083057.C5FB2C2BCB8@smtp.kernel.org> (raw)
In-Reply-To: <20260515-rcar-du-dsc-v2-3-f6b9240a1240@ideasonboard.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Sleeping functions called from atomic bridge callbacks (Subsystem Guide Violation).
- [Medium] Resource leak of the remote bridge module reference.
- [Low] Kconfig logic error defeats COMPILE_TEST support.
- [Low] Unbalanced clock management due to ignored initialization errors.
--

commit bddd7270b2102e27ba00c2ae1f77dd8387606431
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver

The Renesas DSC Display Stream Compression is a bridge embedded in the
Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to
8k or 400 Mpixel/s. Add rudimentary driver to act as a pass-through
bridge for DSI1.

> diff --git a/drivers/gpu/drm/renesas/rcar-du/Kconfig b/drivers/gpu/drm/renesas/rcar-du/Kconfig
> index 840305fdeb49a..a51f996d35375 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/renesas/rcar-du/Kconfig
> @@ -33,6 +33,19 @@ config DRM_RCAR_DW_HDMI
>  	help
>  	  Enable support for R-Car Gen3 or RZ/G2 internal HDMI encoder.
>  
> +config DRM_RCAR_USE_DSC
> +	bool "R-Car DU DSC Encoder Support"
> +	depends on DRM_BRIDGE && OF
> +	depends on DRM_RCAR_DU || COMPILE_TEST
> +	default DRM_RCAR_DU
> +	help
> +	  Enable support for the R-Car Display Unit embedded DSC encoder.
> +
> +config DRM_RCAR_DSC
> +	def_tristate DRM_RCAR_DU
> +	depends on DRM_RCAR_USE_DSC
> +	select RESET_CONTROLLER

Since DRM_RCAR_DSC lacks a user prompt and uses def_tristate DRM_RCAR_DU,
doesn't it silently evaluate to 'n' during cross-architecture compile tests
where DRM_RCAR_DU=n?

This seems to prevent rcar_dsc.c from being compiled even when COMPILE_TEST
is enabled in DRM_RCAR_USE_DSC.

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c
> new file mode 100644
> index 0000000000000..2ab5a4bb099c3
> --- /dev/null
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c
[ ... ]
> +static void rcar_dsc_atomic_enable(struct drm_bridge *bridge,
> +				   struct drm_atomic_state *state)
> +{
> +	struct rcar_dsc *dsc = bridge_to_rcar_dsc(bridge);
> +
> +	WARN_ON(clk_prepare_enable(dsc->clk));

Does this sequence execute in atomic context?

Looking at the DRM subsystem guidelines, bridge atomic enable/disable
callbacks run in atomic context where sleeping is forbidden. Since
clk_prepare_enable() acquires a mutex internally, can this trigger a
scheduling while atomic warning?

Also, if clk_prepare_enable() fails, the failure is recorded by WARN_ON()
but execution continues. Later, the pipeline teardown will unconditionally
call clk_disable_unprepare().

Can this lead to unbalanced clock disable warnings from the Common Clock
Framework since the clock was never actually enabled?

> +	WARN_ON(reset_control_deassert(dsc->rst));
> +}
> +
> +static void rcar_dsc_atomic_disable(struct drm_bridge *bridge,
> +				    struct drm_atomic_state *state)
> +{
> +	struct rcar_dsc *dsc = bridge_to_rcar_dsc(bridge);
> +
> +	reset_control_assert(dsc->rst);
> +	clk_disable_unprepare(dsc->clk);
> +}
[ ... ]
> +static int rcar_dsc_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	dsc->bridge.next_bridge = of_drm_find_and_get_bridge(remote);
> +	of_node_put(remote);
> +	if (!dsc->bridge.next_bridge)
> +		return -EPROBE_DEFER;
> +
> +	dsc->bridge.of_node = dev->of_node;
> +
> +	return devm_drm_bridge_add(dev, &dsc->bridge);
> +}

Does this code leak the remote bridge module reference?

The call to of_drm_find_and_get_bridge() acquires a reference to the
bridge that requires a matching drm_bridge_put(). If devm_drm_bridge_add()
fails, the reference doesn't appear to be released.

Additionally, there doesn't seem to be a .remove callback or a registered
devm action to release the reference when the driver unbinds. Would using
devm_drm_of_get_bridge() resolve this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-rcar-du-dsc-v2-0-f6b9240a1240@ideasonboard.com?part=3

  reply	other threads:[~2026-05-15  8:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  7:56 [PATCH v2 0/5] drm/rcar-du: Add support for DSI pipelines with DSC Tomi Valkeinen
2026-05-15  7:56 ` [PATCH v2 1/5] clk: renesas: r8a779g0: Add DSC clock Tomi Valkeinen
2026-05-15  7:56 ` [PATCH v2 2/5] dt-bindings: display: bridge: Document Renesas R-Car V4H DSC bindings Tomi Valkeinen
2026-05-15 17:32   ` Conor Dooley
2026-06-10 23:18     ` Laurent Pinchart
2026-06-12 10:43     ` Tomi Valkeinen
2026-06-12 16:09       ` Conor Dooley
2026-06-12 17:01         ` Laurent Pinchart
2026-05-15  7:56 ` [PATCH v2 3/5] drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver Tomi Valkeinen
2026-05-15  8:30   ` sashiko-bot [this message]
2026-05-15  7:56 ` [PATCH v2 4/5] drm/rcar-du: dsi: Support DSC in the pipeline Tomi Valkeinen
2026-05-15  7:56 ` [PATCH v2 5/5] arm64: dts: renesas: Add Renesas R-Car V4H DSC Tomi Valkeinen
2026-05-15  8:01 ` [PATCH v2 0/5] drm/rcar-du: Add support for DSI pipelines with DSC Geert Uytterhoeven

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=20260515083057.C5FB2C2BCB8@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tomi.valkeinen+renesas@ideasonboard.com \
    --cc=wsa+renesas@sang-engineering.com \
    /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.