drm/vkms: totally reworked crc data tracking

The crc computation worker needs to be able to get at some data
structures and framebuffer mappings, while potentially more atomic
updates are going on. The solution thus far is to copy relevant bits
around, but that's very tedious.

Here's a new approach, which tries to be more clever, but relies on a
few not-so-obvious things:
- crtc_state is always updated when a plane_state changes. Therefore
  we can just stuff plane_state pointers into a crtc_state. That
  solves the problem of easily getting at the needed plane_states.
- with the flushing changes from previous patches the above also holds
  without races due to the next atomic update being a bit eager with
  cleaning up pending work - we always wait for all crc work items to
  complete before unmapping framebuffers.
- we also need to make sure that the hrtimer fires off the right
  worker. Keep a new distinct crc_state pointer, under the
  vkms_output->lock protection for this. Note that crtc->state is
  updated very early in the atomic commit, way before we arm the
  vblank event - the vblank event should always match the buffers we
  use to compute the crc. This also solves an issue in the hrtimer,
  where we've accessed drm_crtc->state without holding the right locks
  (we held none - oops).
- in the worker itself we can then just access the plane states we
  need, again solving a bunch of ordering and locking issues.
  Accessing plane->state requires locks, accessing the private
  vkms_crtc_state->active_planes pointer only requires that the memory
  doesn't get freed too early.

The idea behind vkms_crtc_state->active_planes is that this would
contain all visible planes, in z-order, as a first step towards a more
generic blending implementation.

Note that this patch also fixes races between prepare_fb/cleanup_fb
and the crc worker accessing ->vaddr.

Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190606222751.32567-10-daniel.vetter@ffwll.ch

1 files changed, 55 insertions, 5 deletions

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index d3d95b1e7292..7e2a081f3764 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 
 #include "vkms_drv.h"
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_probe_helper.h>
 
@@ -9,7 +10,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 	struct vkms_output *output = container_of(timer, struct vkms_output,
 						  vblank_hrtimer);
 	struct drm_crtc *crtc = &output->crtc;
-	struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
+	struct vkms_crtc_state *state;
 	u64 ret_overrun;
 	bool ret;
 
@@ -23,6 +24,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
 	if (!ret)
 		DRM_ERROR("vkms failure on handling vblank");
 
+	state = output->crc_state;
 	if (state && output->crc_enabled) {
 		u64 frame = drm_crtc_accurate_vblank_count(crtc);
 
@@ -124,10 +126,9 @@ static void vkms_atomic_crtc_destroy_state(struct drm_crtc *crtc,
 
 	__drm_atomic_helper_crtc_destroy_state(state);
 
-	if (vkms_state) {
-		WARN_ON(work_pending(&vkms_state->crc_work));
-		kfree(vkms_state);
-	}
+	WARN_ON(work_pending(&vkms_state->crc_work));
+	kfree(vkms_state->active_planes);
+	kfree(vkms_state);
 }
 
 static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
@@ -157,6 +158,52 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 	.verify_crc_source	= vkms_verify_crc_source,
 };
 
+static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
+				  struct drm_crtc_state *state)
+{
+	struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(state);
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	int i = 0, ret;
+
+	if (vkms_state->active_planes)
+		return 0;
+
+	ret = drm_atomic_add_affected_planes(state->state, crtc);
+	if (ret < 0)
+		return ret;
+
+	drm_for_each_plane_mask(plane, crtc->dev, state->plane_mask) {
+		plane_state = drm_atomic_get_existing_plane_state(state->state,
+								  plane);
+		WARN_ON(!plane_state);
+
+		if (!plane_state->visible)
+			continue;
+
+		i++;
+	}
+
+	vkms_state->active_planes = kcalloc(i, sizeof(plane), GFP_KERNEL);
+	if (!vkms_state->active_planes)
+		return -ENOMEM;
+	vkms_state->num_active_planes = i;
+
+	i = 0;
+	drm_for_each_plane_mask(plane, crtc->dev, state->plane_mask) {
+		plane_state = drm_atomic_get_existing_plane_state(state->state,
+								  plane);
+
+		if (!plane_state->visible)
+			continue;
+
+		vkms_state->active_planes[i++] =
+			to_vkms_plane_state(plane_state);
+	}
+
+	return 0;
+}
+
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_state)
 {
@@ -198,10 +245,13 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
 		crtc->state->event = NULL;
 	}
 
+	vkms_output->crc_state = to_vkms_crtc_state(crtc->state);
+
 	spin_unlock_irq(&vkms_output->lock);
 }
 
 static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+	.atomic_check	= vkms_crtc_atomic_check,
 	.atomic_begin	= vkms_crtc_atomic_begin,
 	.atomic_flush	= vkms_crtc_atomic_flush,
 	.atomic_enable	= vkms_crtc_atomic_enable,