| Message ID | 20240919-exynosdrm-decon-v1-0-6c5861c1cb04@disroot.org |
|---|---|
| Headers | show |
| Series | Samsung Exynos 7870 DECON driver support | expand |
On 19/09/2024 17:11, Kaustabh Chakraborty wrote: > decon_commit() gets called during atomic_enable. At this stage, DECON is > suspended, and thus the function refuses to run. Fix the suspended > condition checking in decon_commit(). > > Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org> > --- If this is a fix, then you miss fixes tag and cc-stable. However the explanation seems just incomplete. This looked like a intentional code, so you should explain really why original approach was wrong. Best regards, Krzysztof
On 2024-09-20 12:40, Krzysztof Kozlowski wrote: > On 19/09/2024 17:11, Kaustabh Chakraborty wrote: >> decon_commit() gets called during atomic_enable. At this stage, DECON is >> suspended, and thus the function refuses to run. Fix the suspended >> condition checking in decon_commit(). >> >> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org> >> --- > > If this is a fix, then you miss fixes tag and cc-stable. However the > explanation seems just incomplete. This looked like a intentional code, > so you should explain really why original approach was wrong. Fixes: 96976c3d9aff ("drm/exynos: Add DECON driver") Now that I read the commit description of the above commit, which mentions that the DECON driver is based on the FIMD driver, I think it makes more sense to rewrite the suspend logic exactly as done in the FIMD driver. Will do it in v2. Here's a commit description which may be better suited, let me know: A flag variable in struct decon_context, called 'suspended' is set to false at the end of decon_atomic_enable() and is set back to true at the end of decon_atomic_disable(). Functions called in decon_atomic_enable(), such as decon_enable_vblank() and decon_commit() are guarded by suspend condition checking, where it refuses to proceed if 'suspended' is set to true. Since 'suspended' isn't set to true until the end of the calling function, the called functions aren't even executed. The original commit, 96976c3d9aff ("drm/exynos: Add DECON driver") implementing the DECON driver, is based on the FIMD driver, but changes the suspend flag logic which causes this issue. Implement the suspend logic present in FIMD, which changes the flag at the beginning of atomic_enable and atomic_disable instead. > > Best regards, > Krzysztof
This patch series aims at adding support for Exynos7870's DECON in the Exynos7 DECON driver. It introduces a driver data struct so that support for DECON on other SoCs can be added to it in the future. It also fixes a few bugs in the driver, such as functions recieving bad pointers. Tested on Samsung Galaxy J7 Prime and Samsung Galaxy A2 Core. Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org> --- Kaustabh Chakraborty (6): drm/exynos: exynos7_drm_decon: fix uninitialized crtc reference in functions drm/exynos: exynos7_drm_decon: fix suspended condition in decon_commit() drm/exynos: exynos7_drm_decon: fix ideal_clk by converting it to Hz drm/exynos: exynos7_drm_decon: properly clear channels during bind drm/exynos: exynos7_drm_decon: add driver data and support for Exynos7870 dt-bindings: display: samsung,exynos7-decon: add exynos7870 compatible .../display/samsung/samsung,exynos7-decon.yaml | 4 +- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 124 +++++++++++++-------- drivers/gpu/drm/exynos/regs-decon7.h | 15 ++- 3 files changed, 90 insertions(+), 53 deletions(-) --- base-commit: 4f3e012d4cfd1d9bf837870c961f462ca9f23ebe change-id: 20240917-exynosdrm-decon-4c228dd1d2bf Best regards,