[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAQKjZM+++P3ozLZZYEusYepamF0qdeuOe+thDb2BevLCsab_Q@mail.gmail.com>
Date: Fri, 27 Jun 2025 13:06:46 +0900
From: Inki Dae <daeinki@...il.com>
To: Kaustabh Chakraborty <kauschluss@...root.org>
Cc: Seung-Woo Kim <sw0312.kim@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>, Conor Dooley <conor@...nel.org>,
Ajay Kumar <ajaykumar.rs@...sung.com>, Akshu Agrawal <akshua@...il.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v3 2/3] drm/exynos: exynos7_drm_decon: fix call of decon_commit()
Hi,
2025년 6월 27일 (금) 오전 4:21, Kaustabh Chakraborty <kauschluss@...root.org>님이 작성:
>
> decon_commit() has a condition guard at the beginning:
>
> if (ctx->suspended)
> return;
>
> But, when it is being called from decon_atomic_enable(), ctx->suspended
> is still set to true, which prevents its execution. decon_commit() is
> vital for setting up display timing values, without which the display
> pipeline fails to function properly. Call the function after
> ctx->suspended is set to false as a fix.
Good observation. However, I think a more generic solution is needed.
>
> Cc: stable@...r.kernel.org
> Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver")
> Signed-off-by: Kaustabh Chakraborty <kauschluss@...root.org>
> ---
> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index f91daefa9d2bc5e314c279822047e60ee0d7ca99..43bcbe2e2917df43d7c2d27a9771e892628dd682 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -583,9 +583,9 @@ static void decon_atomic_enable(struct exynos_drm_crtc *crtc)
> if (test_and_clear_bit(0, &ctx->irq_flags))
> decon_enable_vblank(ctx->crtc);
>
> - decon_commit(ctx->crtc);
> -
> ctx->suspended = false;
> +
> + decon_commit(ctx->crtc);
There seem to be three possible solutions:
1. Remove all code related to ctx->suspended. If the pipeline flow is
properly managed as in the exynos5433_drm_decon.c module, checking the
ctx->suspended state may no longer be necessary.
2. Remove the ctx->suspended check from decon_commit(). Since the
runtime PM resume is already called before decon_commit() in
decon_atomic_enable(), the DECON controller should already be enabled
at the hardware level, and decon_commit() should work correctly.
3. Move the code that updates ctx->suspended from
decon_atomic_enable() and decon_atomic_disable() to
exynos7_decon_resume() and exynos7_decon_suspend(), respectively. The
decon_atomic_enable() function calls pm_runtime_resume_and_get(),
which ultimately triggers exynos7_decon_resume(). It would be more
appropriate to set ctx->suspended = false in the
exynos7_decon_resume() function, as this is the standard place to
handle hardware state changes and resume actions.
decon_atomic_enable() is responsible for requesting enablement of the
DECON controller, but actual hardware state transitions will be
handled within exynos7_decon_resume() and exynos7_decon_suspend().
Unfortunately, I do not have hardware to test this patch myself. Would
it be possible for you to try one of these approaches and verify the
behavior?
Option 1 would be the best solution if feasible.
Thanks,
Inki Dae
> }
>
> static void decon_atomic_disable(struct exynos_drm_crtc *crtc)
>
> --
> 2.49.0
>
>
Powered by blists - more mailing lists