Some configurations require additional actions when all windows are
disabled to keep DECON operating correctly. Programming a zero-sized window
in ->atomic_update() leaves the plane logically enabled and can bypass
those disable semantics.

Treat a fully off-screen plane as not visible and take the explicit disable
path.

Implementation details:
- exynos_plane_mode_set(): if computed actual_w/actual_h is zero, mark
  state->visible = false and return early.
- exynos_plane_atomic_check(): if !visible, skip further checks and
  return 0.
- exynos_plane_atomic_update(): if !visible, call ->disable_plane();
  otherwise call ->update_plane().

No functional change for visible planes; off-screen planes are now cleanly
disabled, ensuring the disable hooks run consistently.

Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index 7c3aa77186d3..842974154d79 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -91,6 +91,11 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state)
 	actual_w = exynos_plane_get_size(crtc_x, crtc_w, mode->hdisplay);
 	actual_h = exynos_plane_get_size(crtc_y, crtc_h, mode->vdisplay);
 
+	if (!actual_w || !actual_h) {
+		state->visible = false;
+		return;
+	}
+
 	if (crtc_x < 0) {
 		if (actual_w)
 			src_x += ((-crtc_x) * exynos_state->h_ratio) >> 16;
@@ -244,6 +249,9 @@ static int exynos_plane_atomic_check(struct drm_plane *plane,
 	/* translate state into exynos_state */
 	exynos_plane_mode_set(exynos_state);
 
+	if (!new_plane_state->visible)
+		return 0;
+
 	ret = exynos_drm_plane_check_format(exynos_plane->config, exynos_state);
 	if (ret)
 		return ret;
@@ -263,8 +271,10 @@ static void exynos_plane_atomic_update(struct drm_plane *plane,
 	if (!new_state->crtc)
 		return;
 
-	if (exynos_crtc->ops->update_plane)
+	if (new_state->visible && exynos_crtc->ops->update_plane)
 		exynos_crtc->ops->update_plane(exynos_crtc, exynos_plane);
+	else if (exynos_crtc->ops->disable_plane)
+		exynos_crtc->ops->disable_plane(exynos_crtc, exynos_plane);
 }
 
 static void exynos_plane_atomic_disable(struct drm_plane *plane,
-- 
2.34.1

Thanks for contribution,

2025년 9월 29일 (월) 오후 1:29, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작성:
>
> Some configurations require additional actions when all windows are
> disabled to keep DECON operating correctly. Programming a zero-sized window
> in ->atomic_update() leaves the plane logically enabled and can bypass
> those disable semantics.
>
> Treat a fully off-screen plane as not visible and take the explicit disable
> path.
>
> Implementation details:
> - exynos_plane_mode_set(): if computed actual_w/actual_h is zero, mark
>   state->visible = false and return early.
> - exynos_plane_atomic_check(): if !visible, skip further checks and
>   return 0.
> - exynos_plane_atomic_update(): if !visible, call ->disable_plane();
>   otherwise call ->update_plane().
>
> No functional change for visible planes; off-screen planes are now cleanly
> disabled, ensuring the disable hooks run consistently.
>
> Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index 7c3aa77186d3..842974154d79 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -91,6 +91,11 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state)
>         actual_w = exynos_plane_get_size(crtc_x, crtc_w, mode->hdisplay);
>         actual_h = exynos_plane_get_size(crtc_y, crtc_h, mode->vdisplay);
>
> +       if (!actual_w || !actual_h) {
> +               state->visible = false;

The state->visible field in the DRM atomic framework is set to true
only when the following conditions are met:
- Both state->crtc and state->fb are present (having only one of them
results in an error).
- src_w/src_h and crtc_w/crtc_h are non-zero.
- The source rectangle does not exceed the framebuffer bounds (e.g.,
src_x + src_w <= fb->width).
- Rotation and clipping checks pass successfully.

However, this patch modifies the state->visible value within
vendor-specific code. Doing so can be problematic because it overrides
a field that is managed by the DRM atomic framework. Even if it
currently works, it may lead to unexpected behavior in the future.

For example, if the DRM atomic framework sets visible = true after
validating the above conditions and begins processing certain logic,
but the vendor driver later changes it to false, the framework may
still assume the variable remains true, resulting in inconsistent
states.

Turning off a plane when it doesn’t need to be displayed is a good
idea I think. You might consider contributing this behavior upstream
so it can be properly handled within the DRM atomic framework itself.

Thanks,
Inki Dae

> +               return;
> +       }
> +
>         if (crtc_x < 0) {
>                 if (actual_w)
>                         src_x += ((-crtc_x) * exynos_state->h_ratio) >> 16;
> @@ -244,6 +249,9 @@ static int exynos_plane_atomic_check(struct drm_plane *plane,
>         /* translate state into exynos_state */
>         exynos_plane_mode_set(exynos_state);
>
> +       if (!new_plane_state->visible)
> +               return 0;
> +
>         ret = exynos_drm_plane_check_format(exynos_plane->config, exynos_state);
>         if (ret)
>                 return ret;
> @@ -263,8 +271,10 @@ static void exynos_plane_atomic_update(struct drm_plane *plane,
>         if (!new_state->crtc)
>                 return;
>
> -       if (exynos_crtc->ops->update_plane)
> +       if (new_state->visible && exynos_crtc->ops->update_plane)
>                 exynos_crtc->ops->update_plane(exynos_crtc, exynos_plane);
> +       else if (exynos_crtc->ops->disable_plane)
> +               exynos_crtc->ops->disable_plane(exynos_crtc, exynos_plane);
>  }
>
>  static void exynos_plane_atomic_disable(struct drm_plane *plane,
> --
> 2.34.1
>
>

> -----Original Message-----
> From: Inki Dae <daeinki@gmail.com>
> Sent: Monday, November 10, 2025 11:24 AM
> To: Hoyoung Lee <hy_fifty.lee@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park
> <kyungmin.park@samsung.com>; David Airlie <airlied@gmail.com>; Simona
> Vetter <simona@ffwll.ch>; Krzysztof Kozlowski <krzk@kernel.org>; Alim
> Akhtar <alim.akhtar@samsung.com>; dri-devel@lists.freedesktop.org; linux-
> arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] drm/exynos: plane: Disable fully off-screen
> planes instead of zero-sized update
> 
> Thanks for contribution,
> 
> 2025년 9월 29일 (월) 오후 1:29, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작
> 성:
> >
> > Some configurations require additional actions when all windows are
> > disabled to keep DECON operating correctly. Programming a zero-sized
> > window in ->atomic_update() leaves the plane logically enabled and can
> > bypass those disable semantics.
> >
> > Treat a fully off-screen plane as not visible and take the explicit
> > disable path.
> >
> > Implementation details:
> > - exynos_plane_mode_set(): if computed actual_w/actual_h is zero, mark
> >   state->visible = false and return early.
> > - exynos_plane_atomic_check(): if !visible, skip further checks and
> >   return 0.
> > - exynos_plane_atomic_update(): if !visible, call ->disable_plane();
> >   otherwise call ->update_plane().
> >
> > No functional change for visible planes; off-screen planes are now
> > cleanly disabled, ensuring the disable hooks run consistently.
> >
> > Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > index 7c3aa77186d3..842974154d79 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > @@ -91,6 +91,11 @@ static void exynos_plane_mode_set(struct
> exynos_drm_plane_state *exynos_state)
> >         actual_w = exynos_plane_get_size(crtc_x, crtc_w, mode->hdisplay);
> >         actual_h = exynos_plane_get_size(crtc_y, crtc_h,
> > mode->vdisplay);
> >
> > +       if (!actual_w || !actual_h) {
> > +               state->visible = false;
> 
> The state->visible field in the DRM atomic framework is set to true only
> when the following conditions are met:
> - Both state->crtc and state->fb are present (having only one of them
> results in an error).
> - src_w/src_h and crtc_w/crtc_h are non-zero.
> - The source rectangle does not exceed the framebuffer bounds (e.g., src_x
> + src_w <= fb->width).
> - Rotation and clipping checks pass successfully.
> 
> However, this patch modifies the state->visible value within vendor-
> specific code. Doing so can be problematic because it overrides a field
> that is managed by the DRM atomic framework. Even if it currently works,
> it may lead to unexpected behavior in the future.
> 
> For example, if the DRM atomic framework sets visible = true after
> validating the above conditions and begins processing certain logic, but
> the vendor driver later changes it to false, the framework may still
> assume the variable remains true, resulting in inconsistent states.
> 
> Turning off a plane when it doesn’t need to be displayed is a good idea I
> think. You might consider contributing this behavior upstream so it can be
> properly handled within the DRM atomic framework itself.
> 
> Thanks,
> Inki Dae
> 
> > +               return;
> > +       }
> > +
> >         if (crtc_x < 0) {
> >                 if (actual_w)
> >                         src_x += ((-crtc_x) * exynos_state->h_ratio)
> > >> 16; @@ -244,6 +249,9 @@ static int exynos_plane_atomic_check(struct
> drm_plane *plane,
> >         /* translate state into exynos_state */
> >         exynos_plane_mode_set(exynos_state);
> >
> > +       if (!new_plane_state->visible)
> > +               return 0;
> > +
> >         ret = exynos_drm_plane_check_format(exynos_plane->config,
> exynos_state);
> >         if (ret)
> >                 return ret;
> > @@ -263,8 +271,10 @@ static void exynos_plane_atomic_update(struct
> drm_plane *plane,
> >         if (!new_state->crtc)
> >                 return;
> >
> > -       if (exynos_crtc->ops->update_plane)
> > +       if (new_state->visible && exynos_crtc->ops->update_plane)
> >                 exynos_crtc->ops->update_plane(exynos_crtc,
> > exynos_plane);
> > +       else if (exynos_crtc->ops->disable_plane)
> > +               exynos_crtc->ops->disable_plane(exynos_crtc,
> > + exynos_plane);
> >  }
> >
> >  static void exynos_plane_atomic_disable(struct drm_plane *plane,
> > --
> > 2.34.1
> >
> >

Hi Inki,
Thanks for the review.

I agree that mutating state->visible value in vendor code is risky.
Rather than touching the DRM atomic framework or that field, how about we add a driver-private flag in Exynos(e.g. exynos_drm_plane_state->visible) and use that instead?
If this approach sounds reasonable to you, I’ll spin a v2 along these lines.

BRs,
Hoyoung Lee

Hi Hoyoung,

2025년 11월 12일 (수) 오전 11:44, <hy_fifty.lee@samsung.com>님이 작성:
>
> > -----Original Message-----
> > From: Inki Dae <daeinki@gmail.com>
> > Sent: Monday, November 10, 2025 11:24 AM
> > To: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park
> > <kyungmin.park@samsung.com>; David Airlie <airlied@gmail.com>; Simona
> > Vetter <simona@ffwll.ch>; Krzysztof Kozlowski <krzk@kernel.org>; Alim
> > Akhtar <alim.akhtar@samsung.com>; dri-devel@lists.freedesktop.org; linux-
> > arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/3] drm/exynos: plane: Disable fully off-screen
> > planes instead of zero-sized update
> >
> > Thanks for contribution,
> >
> > 2025년 9월 29일 (월) 오후 1:29, Hoyoung Lee <hy_fifty.lee@samsung.com>님이 작
> > 성:
> > >
> > > Some configurations require additional actions when all windows are
> > > disabled to keep DECON operating correctly. Programming a zero-sized
> > > window in ->atomic_update() leaves the plane logically enabled and can
> > > bypass those disable semantics.
> > >
> > > Treat a fully off-screen plane as not visible and take the explicit
> > > disable path.
> > >
> > > Implementation details:
> > > - exynos_plane_mode_set(): if computed actual_w/actual_h is zero, mark
> > >   state->visible = false and return early.
> > > - exynos_plane_atomic_check(): if !visible, skip further checks and
> > >   return 0.
> > > - exynos_plane_atomic_update(): if !visible, call ->disable_plane();
> > >   otherwise call ->update_plane().
> > >
> > > No functional change for visible planes; off-screen planes are now
> > > cleanly disabled, ensuring the disable hooks run consistently.
> > >
> > > Signed-off-by: Hoyoung Lee <hy_fifty.lee@samsung.com>
> > > ---
> > >  drivers/gpu/drm/exynos/exynos_drm_plane.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > > b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > > index 7c3aa77186d3..842974154d79 100644
> > > --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > > +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> > > @@ -91,6 +91,11 @@ static void exynos_plane_mode_set(struct
> > exynos_drm_plane_state *exynos_state)
> > >         actual_w = exynos_plane_get_size(crtc_x, crtc_w, mode->hdisplay);
> > >         actual_h = exynos_plane_get_size(crtc_y, crtc_h,
> > > mode->vdisplay);
> > >
> > > +       if (!actual_w || !actual_h) {
> > > +               state->visible = false;
> >
> > The state->visible field in the DRM atomic framework is set to true only
> > when the following conditions are met:
> > - Both state->crtc and state->fb are present (having only one of them
> > results in an error).
> > - src_w/src_h and crtc_w/crtc_h are non-zero.
> > - The source rectangle does not exceed the framebuffer bounds (e.g., src_x
> > + src_w <= fb->width).
> > - Rotation and clipping checks pass successfully.
> >
> > However, this patch modifies the state->visible value within vendor-
> > specific code. Doing so can be problematic because it overrides a field
> > that is managed by the DRM atomic framework. Even if it currently works,
> > it may lead to unexpected behavior in the future.
> >
> > For example, if the DRM atomic framework sets visible = true after
> > validating the above conditions and begins processing certain logic, but
> > the vendor driver later changes it to false, the framework may still
> > assume the variable remains true, resulting in inconsistent states.
> >
> > Turning off a plane when it doesn’t need to be displayed is a good idea I
> > think. You might consider contributing this behavior upstream so it can be
> > properly handled within the DRM atomic framework itself.
> >
> > Thanks,
> > Inki Dae
> >
> > > +               return;
> > > +       }
> > > +
> > >         if (crtc_x < 0) {
> > >                 if (actual_w)
> > >                         src_x += ((-crtc_x) * exynos_state->h_ratio)
> > > >> 16; @@ -244,6 +249,9 @@ static int exynos_plane_atomic_check(struct
> > drm_plane *plane,
> > >         /* translate state into exynos_state */
> > >         exynos_plane_mode_set(exynos_state);
> > >
> > > +       if (!new_plane_state->visible)
> > > +               return 0;
> > > +
> > >         ret = exynos_drm_plane_check_format(exynos_plane->config,
> > exynos_state);
> > >         if (ret)
> > >                 return ret;
> > > @@ -263,8 +271,10 @@ static void exynos_plane_atomic_update(struct
> > drm_plane *plane,
> > >         if (!new_state->crtc)
> > >                 return;
> > >
> > > -       if (exynos_crtc->ops->update_plane)
> > > +       if (new_state->visible && exynos_crtc->ops->update_plane)
> > >                 exynos_crtc->ops->update_plane(exynos_crtc,
> > > exynos_plane);
> > > +       else if (exynos_crtc->ops->disable_plane)
> > > +               exynos_crtc->ops->disable_plane(exynos_crtc,
> > > + exynos_plane);
> > >  }
> > >
> > >  static void exynos_plane_atomic_disable(struct drm_plane *plane,
> > > --
> > > 2.34.1
> > >
> > >
>
> Hi Inki,
> Thanks for the review.
>
> I agree that mutating state->visible value in vendor code is risky.
> Rather than touching the DRM atomic framework or that field, how about we add a driver-private flag in Exynos(e.g. exynos_drm_plane_state->visible) and use that instead?
> If this approach sounds reasonable to you, I’ll spin a v2 along these lines.
>

For now, it is fine to add a driver-private flag and apply the change
only to Exynos. However, I believe this feature can also be generally
applied to other SoCs whose display controllers support multiple
hardware overlays, not just Exynos. Therefore, it would be ideal if
this could eventually be integrated into the DRM atomic framework so
that other SoCs may also take advantage of it in the future.

Thanks,
Inki Dae

> BRs,
> Hoyoung Lee
>
>
>