diff mbox series

[7/8] drm/mipi-dbi: Remove ->enabled

Message ID 20200612160056.2082681-7-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series [1/8] drm/atomic-helper: reset vblank on crtc reset | expand

Commit Message

Daniel Vetter June 12, 2020, 4 p.m. UTC
The atomic helpers try really hard to not lose track of things,
duplicating enabled tracking in the driver is at best confusing.
Double-enabling or disabling is a bug in atomic helpers.

In the fb_dirty function we can just assume that the fb always exists,
simple display pipe helpers guarantee that the crtc is only enabled
together with the output, so we always have a primary plane around.

Now in the update function we need to be a notch more careful, since
that can also get called when the crtc is off. And we don't want to
upload frames when that's the case, so filter that out too.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Lechner <david@lechnology.com>
---
 drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
 drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
 drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
 include/drm/drm_mipi_dbi.h     |  5 -----
 4 files changed, 12 insertions(+), 32 deletions(-)

Comments

Noralf Trønnes June 13, 2020, 1:42 p.m. UTC | #1
Den 12.06.2020 18.00, skrev Daniel Vetter:
> The atomic helpers try really hard to not lose track of things,
> duplicating enabled tracking in the driver is at best confusing.
> Double-enabling or disabling is a bug in atomic helpers.
> 
> In the fb_dirty function we can just assume that the fb always exists,
> simple display pipe helpers guarantee that the crtc is only enabled
> together with the output, so we always have a primary plane around.
> 
> Now in the update function we need to be a notch more careful, since
> that can also get called when the crtc is off. And we don't want to
> upload frames when that's the case, so filter that out too.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Lechner <david@lechnology.com>
> ---

Thanks for fixing this.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
David Lechner June 13, 2020, 6:47 p.m. UTC | #2
On 6/12/20 11:00 AM, Daniel Vetter wrote:
> The atomic helpers try really hard to not lose track of things,
> duplicating enabled tracking in the driver is at best confusing.
> Double-enabling or disabling is a bug in atomic helpers.
> 
> In the fb_dirty function we can just assume that the fb always exists,
> simple display pipe helpers guarantee that the crtc is only enabled
> together with the output, so we always have a primary plane around.
> 
> Now in the update function we need to be a notch more careful, since
> that can also get called when the crtc is off. And we don't want to
> upload frames when that's the case, so filter that out too.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Lechner <david@lechnology.com>
> ---

Acked-by: David Lechner <david@lechnology.com>
Emil Velikov June 15, 2020, 9:31 p.m. UTC | #3
Hi Daniel,

On Fri, 12 Jun 2020 at 17:01, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> The atomic helpers try really hard to not lose track of things,
> duplicating enabled tracking in the driver is at best confusing.
> Double-enabling or disabling is a bug in atomic helpers.
>
> In the fb_dirty function we can just assume that the fb always exists,
> simple display pipe helpers guarantee that the crtc is only enabled
> together with the output, so we always have a primary plane around.
>
> Now in the update function we need to be a notch more careful, since
> that can also get called when the crtc is off. And we don't want to
> upload frames when that's the case, so filter that out too.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Lechner <david@lechnology.com>
> ---
>  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
>  drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
>  drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
>  include/drm/drm_mipi_dbi.h     |  5 -----
>  4 files changed, 12 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index fd8d672972a9..79532b9a324a 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
>         bool full;
>         void *tr;
>
> -       if (!dbidev->enabled)
> +       if (WARN_ON(!fb))
>                 return;
>
AFAICT no other driver has such WARN_ON. Let's drop that - it is
pretty confusing and misleading as-is.

With that, patches 7/8 and 8/8 are:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

-Emil
Daniel Vetter June 16, 2020, 6:50 a.m. UTC | #4
On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> Hi Daniel,
>
> On Fri, 12 Jun 2020 at 17:01, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > The atomic helpers try really hard to not lose track of things,
> > duplicating enabled tracking in the driver is at best confusing.
> > Double-enabling or disabling is a bug in atomic helpers.
> >
> > In the fb_dirty function we can just assume that the fb always exists,
> > simple display pipe helpers guarantee that the crtc is only enabled
> > together with the output, so we always have a primary plane around.
> >
> > Now in the update function we need to be a notch more careful, since
> > that can also get called when the crtc is off. And we don't want to
> > upload frames when that's the case, so filter that out too.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: David Lechner <david@lechnology.com>
> > ---
> >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
> >  drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
> >  drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
> >  include/drm/drm_mipi_dbi.h     |  5 -----
> >  4 files changed, 12 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> > index fd8d672972a9..79532b9a324a 100644
> > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> >         bool full;
> >         void *tr;
> >
> > -       if (!dbidev->enabled)
> > +       if (WARN_ON(!fb))
> >                 return;
> >
> AFAICT no other driver has such WARN_ON. Let's drop that - it is
> pretty confusing and misleading as-is.

Yeah, this is a helper library which might be used wrongly by drivers.
That's why I put it in - if you don't put all the various calls
together correctly, this should at least catch one case. So really
would like to keep this, can I convince you?
-Daniel

> With that, patches 7/8 and 8/8 are:
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>
> -Emil
Emil Velikov June 16, 2020, 1:54 p.m. UTC | #5
On Tue, 16 Jun 2020 at 07:50, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > Hi Daniel,
> >
> > On Fri, 12 Jun 2020 at 17:01, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > The atomic helpers try really hard to not lose track of things,
> > > duplicating enabled tracking in the driver is at best confusing.
> > > Double-enabling or disabling is a bug in atomic helpers.
> > >
> > > In the fb_dirty function we can just assume that the fb always exists,
> > > simple display pipe helpers guarantee that the crtc is only enabled
> > > together with the output, so we always have a primary plane around.
> > >
> > > Now in the update function we need to be a notch more careful, since
> > > that can also get called when the crtc is off. And we don't want to
> > > upload frames when that's the case, so filter that out too.
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Cc: David Lechner <david@lechnology.com>
> > > ---
> > >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
> > >  drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
> > >  drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
> > >  include/drm/drm_mipi_dbi.h     |  5 -----
> > >  4 files changed, 12 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> > > index fd8d672972a9..79532b9a324a 100644
> > > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> > >         bool full;
> > >         void *tr;
> > >
> > > -       if (!dbidev->enabled)
> > > +       if (WARN_ON(!fb))
> > >                 return;
> > >
> > AFAICT no other driver has such WARN_ON. Let's drop that - it is
> > pretty confusing and misleading as-is.
>
> Yeah, this is a helper library which might be used wrongly by drivers.
> That's why I put it in - if you don't put all the various calls
> together correctly, this should at least catch one case. So really
> would like to keep this, can I convince you?

There are plenty of similar places where a drm library/helper can be
misused, lacking a WARN. Nevertheless - sure feel free to keep it.

-Emil
Daniel Vetter June 16, 2020, 5:16 p.m. UTC | #6
On Tue, Jun 16, 2020 at 3:57 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Tue, 16 Jun 2020 at 07:50, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > >
> > > Hi Daniel,
> > >
> > > On Fri, 12 Jun 2020 at 17:01, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > >
> > > > The atomic helpers try really hard to not lose track of things,
> > > > duplicating enabled tracking in the driver is at best confusing.
> > > > Double-enabling or disabling is a bug in atomic helpers.
> > > >
> > > > In the fb_dirty function we can just assume that the fb always exists,
> > > > simple display pipe helpers guarantee that the crtc is only enabled
> > > > together with the output, so we always have a primary plane around.
> > > >
> > > > Now in the update function we need to be a notch more careful, since
> > > > that can also get called when the crtc is off. And we don't want to
> > > > upload frames when that's the case, so filter that out too.
> > > >
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Cc: Maxime Ripard <mripard@kernel.org>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: David Airlie <airlied@linux.ie>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Cc: David Lechner <david@lechnology.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
> > > >  drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
> > > >  drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
> > > >  include/drm/drm_mipi_dbi.h     |  5 -----
> > > >  4 files changed, 12 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > index fd8d672972a9..79532b9a324a 100644
> > > > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > > > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> > > >         bool full;
> > > >         void *tr;
> > > >
> > > > -       if (!dbidev->enabled)
> > > > +       if (WARN_ON(!fb))
> > > >                 return;
> > > >
> > > AFAICT no other driver has such WARN_ON. Let's drop that - it is
> > > pretty confusing and misleading as-is.
> >
> > Yeah, this is a helper library which might be used wrongly by drivers.
> > That's why I put it in - if you don't put all the various calls
> > together correctly, this should at least catch one case. So really
> > would like to keep this, can I convince you?
>
> There are plenty of similar places where a drm library/helper can be
> misused, lacking a WARN. Nevertheless - sure feel free to keep it.

Yeah I agree, we can't check for everything. Personally I think a
check is warranted in two conditions:
- drivers got it wrong, and the WARNING helps catch driver-bugs we've
seen in the wild. Not really the case here
- drivers do check something as defensive programming, but it's an
invariant enforced by higher levels or helpers. Those I like to
convert to WARNING so that other driver authors learn that this should
never happen. This is such a case imo, I removed a bunch of fb checks
from drivers here.

But yeah I think we should only add WARNING checks if this is actually
something people have gotten wrong, otherwise there's just too many of
them, distracting from the code.
-Daniel
Daniel Vetter June 24, 2020, 7:18 a.m. UTC | #7
On Tue, Jun 16, 2020 at 07:16:45PM +0200, Daniel Vetter wrote:
> On Tue, Jun 16, 2020 at 3:57 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Tue, 16 Jun 2020 at 07:50, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > On Mon, Jun 15, 2020 at 11:35 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > > >
> > > > Hi Daniel,
> > > >
> > > > On Fri, 12 Jun 2020 at 17:01, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > > >
> > > > > The atomic helpers try really hard to not lose track of things,
> > > > > duplicating enabled tracking in the driver is at best confusing.
> > > > > Double-enabling or disabling is a bug in atomic helpers.
> > > > >
> > > > > In the fb_dirty function we can just assume that the fb always exists,
> > > > > simple display pipe helpers guarantee that the crtc is only enabled
> > > > > together with the output, so we always have a primary plane around.
> > > > >
> > > > > Now in the update function we need to be a notch more careful, since
> > > > > that can also get called when the crtc is off. And we don't want to
> > > > > upload frames when that's the case, so filter that out too.
> > > > >
> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Cc: Maxime Ripard <mripard@kernel.org>
> > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > > Cc: David Airlie <airlied@linux.ie>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Cc: David Lechner <david@lechnology.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_mipi_dbi.c | 16 ++++++----------
> > > > >  drivers/gpu/drm/tiny/ili9225.c | 12 +++---------
> > > > >  drivers/gpu/drm/tiny/st7586.c  | 11 +++--------
> > > > >  include/drm/drm_mipi_dbi.h     |  5 -----
> > > > >  4 files changed, 12 insertions(+), 32 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > > index fd8d672972a9..79532b9a324a 100644
> > > > > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > > > > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > > > > @@ -268,7 +268,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
> > > > >         bool full;
> > > > >         void *tr;
> > > > >
> > > > > -       if (!dbidev->enabled)
> > > > > +       if (WARN_ON(!fb))
> > > > >                 return;
> > > > >
> > > > AFAICT no other driver has such WARN_ON. Let's drop that - it is
> > > > pretty confusing and misleading as-is.
> > >
> > > Yeah, this is a helper library which might be used wrongly by drivers.
> > > That's why I put it in - if you don't put all the various calls
> > > together correctly, this should at least catch one case. So really
> > > would like to keep this, can I convince you?
> >
> > There are plenty of similar places where a drm library/helper can be
> > misused, lacking a WARN. Nevertheless - sure feel free to keep it.
> 
> Yeah I agree, we can't check for everything. Personally I think a
> check is warranted in two conditions:
> - drivers got it wrong, and the WARNING helps catch driver-bugs we've
> seen in the wild. Not really the case here
> - drivers do check something as defensive programming, but it's an
> invariant enforced by higher levels or helpers. Those I like to
> convert to WARNING so that other driver authors learn that this should
> never happen. This is such a case imo, I removed a bunch of fb checks
> from drivers here.
> 
> But yeah I think we should only add WARNING checks if this is actually
> something people have gotten wrong, otherwise there's just too many of
> them, distracting from the code.

Merged this patch here too, thanks everyone for reviewing.
-Daniel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index fd8d672972a9..79532b9a324a 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -268,7 +268,7 @@  static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	bool full;
 	void *tr;
 
-	if (!dbidev->enabled)
+	if (WARN_ON(!fb))
 		return;
 
 	if (!drm_dev_enter(fb->dev, &idx))
@@ -314,6 +314,9 @@  void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_plane_state *state = pipe->plane.state;
 	struct drm_rect rect;
 
+	if (!pipe->crtc.state->active)
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		mipi_dbi_fb_dirty(state->fb, &rect);
 }
@@ -325,9 +328,8 @@  EXPORT_SYMBOL(mipi_dbi_pipe_update);
  * @crtc_state: CRTC state
  * @plane_state: Plane state
  *
- * This function sets &mipi_dbi->enabled, flushes the whole framebuffer and
- * enables the backlight. Drivers can use this in their
- * &drm_simple_display_pipe_funcs->enable callback.
+ * Flushes the whole framebuffer and enables the backlight. Drivers can use this
+ * in their &drm_simple_display_pipe_funcs->enable callback.
  *
  * Note: Drivers which don't use mipi_dbi_pipe_update() because they have custom
  * framebuffer flushing, can't use this function since they both use the same
@@ -349,7 +351,6 @@  void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 	if (!drm_dev_enter(&dbidev->drm, &idx))
 		return;
 
-	dbidev->enabled = true;
 	mipi_dbi_fb_dirty(fb, &rect);
 	backlight_enable(dbidev->backlight);
 
@@ -390,13 +391,8 @@  void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
 
-	if (!dbidev->enabled)
-		return;
-
 	DRM_DEBUG_KMS("\n");
 
-	dbidev->enabled = false;
-
 	if (dbidev->backlight)
 		backlight_disable(dbidev->backlight);
 	else
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 16400064320f..97a77262d791 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -89,9 +89,6 @@  static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	bool full;
 	void *tr;
 
-	if (!dbidev->enabled)
-		return;
-
 	if (!drm_dev_enter(fb->dev, &idx))
 		return;
 
@@ -167,6 +164,9 @@  static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_plane_state *state = pipe->plane.state;
 	struct drm_rect rect;
 
+	if (!pipe->crtc.state->active)
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		ili9225_fb_dirty(state->fb, &rect);
 }
@@ -275,7 +275,6 @@  static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	ili9225_command(dbi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
 
-	dbidev->enabled = true;
 	ili9225_fb_dirty(fb, &rect);
 out_exit:
 	drm_dev_exit(idx);
@@ -295,16 +294,11 @@  static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
 	 * unplug.
 	 */
 
-	if (!dbidev->enabled)
-		return;
-
 	ili9225_command(dbi, ILI9225_DISPLAY_CONTROL_1, 0x0000);
 	msleep(50);
 	ili9225_command(dbi, ILI9225_POWER_CONTROL_2, 0x0007);
 	msleep(50);
 	ili9225_command(dbi, ILI9225_POWER_CONTROL_1, 0x0a02);
-
-	dbidev->enabled = false;
 }
 
 static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 *cmd, u8 *par,
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 1311e5df8721..d05de03891f8 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -118,9 +118,6 @@  static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	struct mipi_dbi *dbi = &dbidev->dbi;
 	int start, end, idx, ret = 0;
 
-	if (!dbidev->enabled)
-		return;
-
 	if (!drm_dev_enter(fb->dev, &idx))
 		return;
 
@@ -161,6 +158,9 @@  static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_plane_state *state = pipe->plane.state;
 	struct drm_rect rect;
 
+	if (!pipe->crtc.state->active)
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		st7586_fb_dirty(state->fb, &rect);
 }
@@ -237,7 +237,6 @@  static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	msleep(100);
 
-	dbidev->enabled = true;
 	st7586_fb_dirty(fb, &rect);
 
 	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
@@ -258,11 +257,7 @@  static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
 
 	DRM_DEBUG_KMS("\n");
 
-	if (!dbidev->enabled)
-		return;
-
 	mipi_dbi_command(&dbidev->dbi, MIPI_DCS_SET_DISPLAY_OFF);
-	dbidev->enabled = false;
 }
 
 static const u32 st7586_formats[] = {
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 4d0e49c0ed2c..c2827ceaba0d 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -94,11 +94,6 @@  struct mipi_dbi_dev {
 	 */
 	struct drm_display_mode mode;
 
-	/**
-	 * @enabled: Pipeline is enabled
-	 */
-	bool enabled;
-
 	/**
 	 * @tx_buf: Buffer used for transfer (copy clip rect area)
 	 */