Re: [PATCH 6/6] drm/tidss: Implement struct drm_plane_helper_funcs.atomic_enable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <tzimmermann@xxxxxxx>
---
  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.valkeinen@xxxxxxxxxxxxxxxx>

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux