On 09/02/2023 17:41, Thomas Zimmermann wrote:
Enable the primary plane for tidss hardware via atomic_enable.
Atomic helpers invoke this callback only when the plane becomes
active.

Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
---
  drivers/gpu/drm/tidss/tidss_plane.c | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/tidss/tidss_plane.c 
b/drivers/gpu/drm/tidss/tidss_plane.c
index 0b12405edb47..6bdd6e4a955a 100644
--- a/drivers/gpu/drm/tidss/tidss_plane.c
+++ b/drivers/gpu/drm/tidss/tidss_plane.c
@@ -124,6 +124,16 @@ static void tidss_plane_atomic_update(struct drm_plane 
*plane,
        hw_videoport = to_tidss_crtc(new_state->crtc)->hw_videoport;
dispc_plane_setup(tidss->dispc, tplane->hw_plane_id, new_state, hw_videoport);
+}
+
+static void tidss_plane_atomic_enable(struct drm_plane *plane,
+                                     struct drm_atomic_state *state)
+{
+       struct drm_device *ddev = plane->dev;
+       struct tidss_device *tidss = to_tidss(ddev);
+       struct tidss_plane *tplane = to_tidss_plane(plane);
+
+       dev_dbg(ddev->dev, "%s\n", __func__);
dispc_plane_enable(tidss->dispc, tplane->hw_plane_id, true);
  }
@@ -151,6 +161,7 @@ static void drm_plane_destroy(struct drm_plane *plane)
  static const struct drm_plane_helper_funcs tidss_plane_helper_funcs = {
        .atomic_check = tidss_plane_atomic_check,
        .atomic_update = tidss_plane_atomic_update,
+       .atomic_enable = tidss_plane_atomic_enable,
        .atomic_disable = tidss_plane_atomic_disable,
  };

I haven't tested this, but looks fine to me.

Reviewed-by: Tomi Valkeinen <tomi.valkei...@ideasonboard.com>

One thought, though, is that we still do dispc_plane_enable(false) in tidss_plane_atomic_update() when the plane is not visible. Not a problem, but it would be nice to only enable/disable the plane inside atomic_enable/disable.

Or maybe in cases like this the driver should only use atomic_update, and do all the enabling and disabling there...

 Tomi

Reply via email to