lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <348086ac-b5bc-4ca9-9e5b-82106d319eeb@ti.com>
Date: Wed, 22 Oct 2025 19:25:10 +0530
From: Devarsh Thakkar <devarsht@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
        Jyri Sarha
	<jyri.sarha@....fi>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
CC: <dri-devel@...ts.freedesktop.org>,
        Laurent Pinchart
	<laurent.pinchart@...asonboard.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] drm/tidss: Add some support for splash-screen

On 08/09/25 14:43, Tomi Valkeinen wrote:
> Currently when the driver's probe is called, we do a full DSS reset. If
> the bootloader has set up a splash-screen, the reset will disable the
> video output, and after that it may still take time until the display is
> usable (all the kernel modules have been loaded) and even more time
> until the userspace is able to use the display.
> 
> If fbdev is enabled, in a perfect case tidss would take over the fb
> memory set up by the bootloader, and use that memory for tidss's fbdev,
> thus retaining the splash-screen. However, we're not there yet.
> 
> As a partial solution, this patch changes the driver so that the driver
> will not reset (or change) the DSS registers until tidss_runtime_get()
> is called when the display is being set up (because of fbdev modesetting
> or modesetting from the userspace).
> 
> This is achieved in two parts:
> 
> 1. Probe
> 
> At probe time, in dispc_init_hw(), we check if the DSS is idle
> (videoports disabled). If yes, do a reset and continue as before. If
> not, we know that there's a splash-screen, and we set the
> 'tidss->boot_enabled_vp_mask' field to reflect the enabled VPs.
> 
> We then enable the corresponding VP clocks (to ensure they stay on), set
> the IRQENABLE to 0 to make sure we won't get any interrupts, and then
> exit leaving the fclk and VP clocks enabled, and the runtime PM status
> active.
> 
> 2. Runtime get
> 
> Later, when the tidss_runtime_get() is called the first time, we check
> the 'boot_enabled_vp_mask'. If set, we know that we have the
> splash-screen showing on the screen, and thus the clocks are enabled and
> runtime PM status is active. This indicates that
> pm_runtime_resume_and_get() call just before in tidss_runtime_get() did
> not cause a runtime_resume callback to get called, so we need to do that
> manually.
> 
> We call dispc_splash_fini() which essentially returns the DSS into the
> state where it would be in a non-splash-screen case: dispc_splash_fini()
> will do a DSS reset, manually call the runtime_resume callback, and then
> call clk_disable_unprepare() and pm_runtime_put_noidle() to counter the
> actions at probe time.
> 
> Finally 'boot_enabled_vp_mask' is set to zero to mark that we're no
> longer in the "splash-screen mode".
> 
> A note about fbdev emulation:
> 
> If fbdev emulation is enabled in the DRM, tidss will set up an fbdev.
> This will cause a modeset, and the blank framebuffer from tidss's fbdev
> will be shown instead of the splash-screen.
> 
> I see two improvements to this: either we should memcpy the pixel data
> from the bootloader's splash-screen to the new fbdev buffer, or the
> fbdev could use the splash-screen directly as its buffer. I have done
> some hacks for the former, but I'm not sure how to implement either of
> these properly.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>

Reviewed-by: Devarsh Thakkar <devarsht@...com>

Regards
Devarsh

> ---
> Changes in v2:
> - Rebased to latest drm-misc-next, and fixed the issues with the new
>    register access macros
> - Clarified the patch description a bit
> - Dropped the second patch "drm/tidss: Remove early fb", as it's already
>    in upstream
> - Link to v1: https://lore.kernel.org/r/20250416-tidss-splash-v1-0-4ff396eb5008@ideasonboard.com
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c | 127 ++++++++++++++++++++++++++++++++----
>   drivers/gpu/drm/tidss/tidss_dispc.h |   5 ++
>   drivers/gpu/drm/tidss/tidss_drv.c   |  29 +++++++-
>   drivers/gpu/drm/tidss/tidss_drv.h   |   2 +
>   4 files changed, 148 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 7c8c15a5c39b..1bf208cb227b 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -2800,6 +2800,11 @@ void dispc_vp_setup(struct dispc_device *dispc, u32 hw_videoport,
>   	dispc_vp_set_color_mgmt(dispc, hw_videoport, state, newmodeset);
>   }
>   
> +static bool dispc_is_idle(struct dispc_device *dispc)
> +{
> +	return REG_GET(dispc, DSS_SYSSTATUS, DSS_SYSSTATUS_DISPC_IDLE_STATUS);
> +}
> +
>   int dispc_runtime_suspend(struct dispc_device *dispc)
>   {
>   	dev_dbg(dispc->dev, "suspend\n");
> @@ -2940,7 +2945,7 @@ static int dispc_softreset(struct dispc_device *dispc)
>   	return 0;
>   }
>   
> -static int dispc_init_hw(struct dispc_device *dispc)
> +int dispc_init_hw(struct dispc_device *dispc)
>   {
>   	struct device *dev = dispc->dev;
>   	int ret;
> @@ -2957,15 +2962,70 @@ static int dispc_init_hw(struct dispc_device *dispc)
>   		goto err_runtime_suspend;
>   	}
>   
> -	ret = dispc_softreset(dispc);
> -	if (ret)
> -		goto err_clk_disable;
> +	dispc->tidss->boot_enabled_vp_mask = 0;
>   
> -	clk_disable_unprepare(dispc->fclk);
> -	ret = pm_runtime_set_suspended(dev);
> -	if (ret) {
> -		dev_err(dev, "Failed to set DSS PM to suspended\n");
> -		return ret;
> +	if (dispc_is_idle(dispc)) {
> +		ret = dispc_softreset(dispc);
> +		if (ret)
> +			goto err_clk_disable;
> +	} else {
> +		for (u32 vp_idx = 0; vp_idx < dispc->feat->num_vps; vp_idx++) {
> +			bool enabled = VP_REG_GET(dispc, vp_idx,
> +						  DISPC_VP_CONTROL,
> +						  DISPC_VP_CONTROL_ENABLE_MASK);
> +
> +			if (!enabled)
> +				continue;
> +
> +			dispc->tidss->boot_enabled_vp_mask |= BIT(vp_idx);
> +
> +			/* Keep the VP clk enabled */
> +			ret = clk_prepare_enable(dispc->vp_clk[vp_idx]);
> +			if (ret) {
> +				while (vp_idx--) {
> +					if (!(dispc->tidss->boot_enabled_vp_mask &
> +					      BIT(vp_idx)))
> +						continue;
> +
> +					clk_disable_unprepare(dispc->vp_clk[vp_idx]);
> +				}
> +
> +				goto err_clk_disable;
> +			}
> +		}
> +
> +		if (!dispc->tidss->boot_enabled_vp_mask) {
> +			dev_warn(dev,
> +				 "Bad HW state: DSS not idle but no VPs are enabled. Resetting.\n");
> +			ret = dispc_softreset(dispc);
> +			if (ret)
> +				goto err_clk_disable;
> +		} else {
> +			/*
> +			 * Do basic HW init here, which won't interrupt the
> +			 * splash-screen.
> +			 */
> +
> +			/* Ensure we won't get interrupts */
> +			dispc_set_irqenable(dispc, 0);
> +			dispc_read_and_clear_irqstatus(dispc);
> +		}
> +	}
> +
> +	if (dispc->tidss->boot_enabled_vp_mask) {
> +		dev_dbg(dev,
> +			"Bootloader splash-screen detected, leaving DSS active.\n");
> +		pm_runtime_get_noresume(dev);
> +	} else {
> +		/*
> +		 * No splash-screen. Disable the clock and set DSS back to suspended.
> +		 */
> +		clk_disable_unprepare(dispc->fclk);
> +		ret = pm_runtime_set_suspended(dev);
> +		if (ret) {
> +			dev_err(dev, "Failed to set DSS PM to suspended\n");
> +			return ret;
> +		}
>   	}
>   
>   	return 0;
> @@ -2983,6 +3043,28 @@ static int dispc_init_hw(struct dispc_device *dispc)
>   	return ret;
>   }
>   
> +void dispc_init_hw_cleanup(struct dispc_device *dispc)
> +{
> +	if (!dispc->tidss->boot_enabled_vp_mask)
> +		return;
> +
> +	/*
> +	 * Call clk_disable_unprepare() to counter the clk_prepare_enable() we
> +	 * did in the dispc_init_hw().
> +	 */
> +	clk_disable_unprepare(dispc->fclk);
> +
> +	for (u32 vp_idx = 0; vp_idx < dispc->feat->num_vps; vp_idx++) {
> +		if (dispc->tidss->boot_enabled_vp_mask & BIT(vp_idx))
> +			clk_disable_unprepare(dispc->vp_clk[vp_idx]);
> +	}
> +
> +	/* counter the pm_runtime_get_noresume() */
> +	pm_runtime_put_noidle(dispc->dev);
> +
> +	dispc->tidss->boot_enabled_vp_mask = 0;
> +}
> +
>   int dispc_init(struct tidss_device *tidss)
>   {
>   	struct device *dev = tidss->dev;
> @@ -3092,11 +3174,30 @@ int dispc_init(struct tidss_device *tidss)
>   	of_property_read_u32(dispc->dev->of_node, "max-memory-bandwidth",
>   			     &dispc->memory_bandwidth_limit);
>   
> -	r = dispc_init_hw(dispc);
> -	if (r)
> -		return r;
> -
>   	tidss->dispc = dispc;
>   
>   	return 0;
>   }
> +
> +void dispc_splash_fini(struct dispc_device *dispc)
> +{
> +	if (WARN_ON(!dispc->tidss->boot_enabled_vp_mask))
> +		return;
> +
> +	/*
> +	 * Do a reset now, to clean up the bootloader setup, as we're about to
> +	 * do a modeset.
> +	 */
> +	dispc_softreset(dispc);
> +
> +	/*
> +	 * Call resume manually. This will clk_prepare_enable() the fclk, and do
> +	 * the common basic HW configuration.
> +	 */
> +	dispc_runtime_resume(dispc);
> +
> +	/*
> +	 * Revert the enables and gets we did in dispc_init_hw() at probe time.
> +	 */
> +	dispc_init_hw_cleanup(dispc);
> +}
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 60c1b400eb89..647ed013e080 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -152,4 +152,9 @@ const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len);
>   int dispc_init(struct tidss_device *tidss);
>   void dispc_remove(struct tidss_device *tidss);
>   
> +int dispc_init_hw(struct dispc_device *dispc);
> +void dispc_init_hw_cleanup(struct dispc_device *dispc);
> +
> +void dispc_splash_fini(struct dispc_device *dispc);
> +
>   #endif
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 27d9a8fd541f..eb6a57b8f682 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -36,8 +36,22 @@ int tidss_runtime_get(struct tidss_device *tidss)
>   	dev_dbg(tidss->dev, "%s\n", __func__);
>   
>   	r = pm_runtime_resume_and_get(tidss->dev);
> -	WARN_ON(r < 0);
> -	return r;
> +	if (WARN_ON(r < 0))
> +		return r;
> +
> +	if (tidss->boot_enabled_vp_mask) {
> +		/*
> +		 * If 'boot_enabled_vp_mask' is set, it means that the DSS is
> +		 * enabled and bootloader splash-screen is still on the screen,
> +		 * using bootloader's DSS HW config.
> +		 *
> +		 * This is the first time the driver is about to use the HW, and
> +		 * we need to do some cleanup and initial setup.
> +		 */
> +		dispc_splash_fini(tidss->dispc);
> +	}
> +
> +	return 0;
>   }
>   
>   void tidss_runtime_put(struct tidss_device *tidss)
> @@ -149,6 +163,12 @@ static int tidss_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> +	ret = dispc_init_hw(tidss->dispc);
> +	if (ret) {
> +		dev_err(dev, "failed to initialize dispc HW: %d\n", ret);
> +		return ret;
> +	}
> +
>   	ret = tidss_oldi_init(tidss);
>   	if (ret)
>   		return dev_err_probe(dev, ret, "failed to init OLDI\n");
> @@ -211,6 +231,7 @@ static int tidss_probe(struct platform_device *pdev)
>   	tidss_irq_uninstall(ddev);
>   
>   err_runtime_suspend:
> +
>   #ifndef CONFIG_PM
>   	dispc_runtime_suspend(tidss->dispc);
>   #endif
> @@ -219,6 +240,8 @@ static int tidss_probe(struct platform_device *pdev)
>   
>   	tidss_oldi_deinit(tidss);
>   
> +	dispc_init_hw_cleanup(tidss->dispc);
> +
>   	return ret;
>   }
>   
> @@ -245,6 +268,8 @@ static void tidss_remove(struct platform_device *pdev)
>   
>   	tidss_oldi_deinit(tidss);
>   
> +	dispc_init_hw_cleanup(tidss->dispc);
> +
>   	/* devm allocated dispc goes away with the dev so mark it NULL */
>   	dispc_remove(tidss);
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
> index 84454a4855d1..c23091be2b60 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -39,6 +39,8 @@ struct tidss_device {
>   	/* protects the irq masks field and irqenable/irqstatus registers */
>   	spinlock_t irq_lock;
>   	dispc_irq_t irq_mask;	/* enabled irqs */
> +
> +	u32 boot_enabled_vp_mask;
>   };
>   
>   #define to_tidss(__dev) container_of(__dev, struct tidss_device, ddev)
> 
> ---
> base-commit: 685e8dae19df73d5400734ee5ad9e96470f9c0b4
> change-id: 20250416-tidss-splash-3d8f328b7160
> 
> Best regards,


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ