Re: [PATCH 3/5] drm/atomic: Move drm_crtc_commit to drm_crtc_state, v2.

Op 30-08-17 om 15:09 schreef Laurent Pinchart:

Hi Maarten,

Thank you for the patch.

On Wednesday, 30 August 2017 15:17:50 EEST Maarten Lankhorst wrote:

...

Most code only cares about the current commit or previous commit. Fortuantely we already have a place to track those. Move it to drm_crtc_state where it belongs. :)

The per-crtc commit_list is kept for places where we have to look deeper than the current or previous commit for checking whether to stall on unpin. This is used in drm_atomic_helper_setup_commit and intel_has_pending_fb_unpin.

Changes since v1:

• Update kerneldoc for drm_crtc.commit_list. (danvet)

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

---

drivers/gpu/drm/drm_atomic.c | 7 --- drivers/gpu/drm/drm_atomic_helper.c | 92 +++++++++------------------------- include/drm/drm_atomic.h | 1 - include/drm/drm_crtc.h | 23 ++++++++-- 4 files changed, 42 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2fd383d7253a..2cce48f203e0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc, state->crtcs[i].state);

• if (state->crtcs[i].commit) {
  

• 	kfree(state->crtcs[i].commit->event);
  

• 	state->crtcs[i].commit->event = NULL;
  

• 	drm_crtc_commit_put(state->crtcs[i].commit);
  

• }
  

• state->crtcs[i].commit = NULL;
  

  state->crtcs[i].ptr = NULL; state->crtcs[i].state = NULL; }

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 11d0e94a2181..8ccb8b6536c0 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, struct drm_atomic_state *old_state) {

• struct drm_crtc_state *unused;

• struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; int i;

• for_each_new_crtc_in_state(old_state, crtc, unused, i) {

• struct drm_crtc_commit *commit = old_state->crtcs[i].commit;
  

• for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {

• struct drm_crtc_commit *commit = new_crtc_state->commit;
  

  int ret;

  if (!commit)

@@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state;

• struct drm_crtc_commit *commit; struct drm_plane *__plane, *plane = NULL; struct drm_plane_state *__plane_state, *plane_state = NULL; const struct drm_plane_helper_funcs *funcs;
• int i, j, n_planes = 0;

• int i, n_planes = 0;

  for_each_new_crtc_in_state(state, crtc, crtc_state, i) { if (drm_atomic_crtc_needs_modeset(crtc_state))

@@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device *dev, return -EINVAL;

/*

• * Don't do an async update if there is an outstanding commit modifying
  

• * TODO: Don't do an async update if there is an outstanding commit
  

modifying * the plane. This prevents our async update's changes from getting * overridden by a previous synchronous update's state. */

As mentioned in a comment to your previous patch, this is unrelated to $SUBJECT. You should mention and explain this change in the commit message (possibly splitting it out in a separate commit if you think that would make more sense, up to you).

I'm all for removing this, it doesn't work.. But I g uess it can be kept in for now and I'll drop this hunk.