| Message ID | 20180919135644.14182-5-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gen11: Implement planar format support. | expand |
On Wed, Sep 19, 2018 at 03:56:33PM +0200, Maarten Lankhorst wrote: > drm_atomic_helper_commit_planes_on_crtc calls begin_commit, > then plane_update hooks, then flush_commit. Because we keep our own > visibility tracking through plane_state->visible there's no need to > rely on the atomic hooks for this. > > By explicitly writing our own helper, we can update visible planes > as needed, which is useful to make NV12 support work as intended. > > Changes since v1: > - Reword commit message. (Matt Roper) > - Rename to intel_update_planes_on_crtc(). (Matt) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> This patch does leave drm_atomic_helper_commit_planes_on_crtc() unused by any driver. Not sure if we need to follow up with a patch to remove it as dead code, or whether we anticipate other drivers wanting to use it soon. Matt > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 41 ++++++++++++----------- > drivers/gpu/drm/i915/intel_display.c | 10 ++++-- > drivers/gpu/drm/i915/intel_drv.h | 4 +++ > 3 files changed, 33 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index aabebe0d2e9b..e067fb531a29 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -164,29 +164,33 @@ static int intel_plane_atomic_check(struct drm_plane *plane, > to_intel_plane_state(new_plane_state)); > } > > -static void intel_plane_atomic_update(struct drm_plane *plane, > - struct drm_plane_state *old_state) > +void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > + struct intel_crtc *crtc, > + struct intel_crtc_state *old_crtc_state, > + struct intel_crtc_state *new_crtc_state) > { > - struct intel_atomic_state *state = to_intel_atomic_state(old_state->state); > - struct intel_plane *intel_plane = to_intel_plane(plane); > - const struct intel_plane_state *new_plane_state = > - intel_atomic_get_new_plane_state(state, intel_plane); > - struct drm_crtc *crtc = new_plane_state->base.crtc ?: old_state->crtc; > + struct intel_plane_state *new_plane_state; > + struct intel_plane *plane; > + u32 update_mask; > + int i; > + > + update_mask = old_crtc_state->active_planes; > + update_mask |= new_crtc_state->active_planes; > > - if (new_plane_state->base.visible) { > - const struct intel_crtc_state *new_crtc_state = > - intel_atomic_get_new_crtc_state(state, to_intel_crtc(crtc)); > + for_each_new_intel_plane_in_state(old_state, plane, new_plane_state, i) { > + if (crtc->pipe != plane->pipe || > + !(update_mask & BIT(plane->id))) > + continue; > > - trace_intel_update_plane(plane, > - to_intel_crtc(crtc)); > + if (new_plane_state->base.visible) { > + trace_intel_update_plane(&plane->base, crtc); > > - intel_plane->update_plane(intel_plane, > - new_crtc_state, new_plane_state); > - } else { > - trace_intel_disable_plane(plane, > - to_intel_crtc(crtc)); > + plane->update_plane(plane, new_crtc_state, new_plane_state); > + } else { > + trace_intel_disable_plane(&plane->base, crtc); > > - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); > + plane->disable_plane(plane, crtc); > + } > } > } > > @@ -194,7 +198,6 @@ const struct drm_plane_helper_funcs intel_plane_helper_funcs = { > .prepare_fb = intel_prepare_plane_fb, > .cleanup_fb = intel_cleanup_plane_fb, > .atomic_check = intel_plane_atomic_check, > - .atomic_update = intel_plane_atomic_update, > }; > > /** > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f7982f18b8fe..3975f3af4fb4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10813,8 +10813,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > } > > static const struct drm_crtc_helper_funcs intel_helper_funcs = { > - .atomic_begin = intel_begin_crtc_commit, > - .atomic_flush = intel_finish_crtc_commit, > .atomic_check = intel_crtc_atomic_check, > }; > > @@ -12483,6 +12481,7 @@ static void intel_update_crtc(struct drm_crtc *crtc, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *old_intel_cstate = to_intel_crtc_state(old_crtc_state); > struct intel_crtc_state *pipe_config = to_intel_crtc_state(new_crtc_state); > bool modeset = needs_modeset(new_crtc_state); > struct intel_plane_state *new_plane_state = > @@ -12503,7 +12502,12 @@ static void intel_update_crtc(struct drm_crtc *crtc, > if (new_plane_state) > intel_fbc_enable(intel_crtc, pipe_config, new_plane_state); > > - drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); > + intel_begin_crtc_commit(crtc, old_crtc_state); > + > + intel_update_planes_on_crtc(to_intel_atomic_state(state), intel_crtc, > + old_intel_cstate, pipe_config); > + > + intel_finish_crtc_commit(crtc, old_crtc_state); > } > > static void intel_update_crtcs(struct drm_atomic_state *state) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index bf1c38728a59..8073a85d7178 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2188,6 +2188,10 @@ struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); > void intel_plane_destroy_state(struct drm_plane *plane, > struct drm_plane_state *state); > extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > +void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, > + struct intel_crtc *crtc, > + struct intel_crtc_state *old_crtc_state, > + struct intel_crtc_state *new_crtc_state); > int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state, > struct intel_crtc_state *crtc_state, > const struct intel_plane_state *old_plane_state, > -- > 2.18.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index aabebe0d2e9b..e067fb531a29 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -164,29 +164,33 @@ static int intel_plane_atomic_check(struct drm_plane *plane, to_intel_plane_state(new_plane_state)); } -static void intel_plane_atomic_update(struct drm_plane *plane, - struct drm_plane_state *old_state) +void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, + struct intel_crtc *crtc, + struct intel_crtc_state *old_crtc_state, + struct intel_crtc_state *new_crtc_state) { - struct intel_atomic_state *state = to_intel_atomic_state(old_state->state); - struct intel_plane *intel_plane = to_intel_plane(plane); - const struct intel_plane_state *new_plane_state = - intel_atomic_get_new_plane_state(state, intel_plane); - struct drm_crtc *crtc = new_plane_state->base.crtc ?: old_state->crtc; + struct intel_plane_state *new_plane_state; + struct intel_plane *plane; + u32 update_mask; + int i; + + update_mask = old_crtc_state->active_planes; + update_mask |= new_crtc_state->active_planes; - if (new_plane_state->base.visible) { - const struct intel_crtc_state *new_crtc_state = - intel_atomic_get_new_crtc_state(state, to_intel_crtc(crtc)); + for_each_new_intel_plane_in_state(old_state, plane, new_plane_state, i) { + if (crtc->pipe != plane->pipe || + !(update_mask & BIT(plane->id))) + continue; - trace_intel_update_plane(plane, - to_intel_crtc(crtc)); + if (new_plane_state->base.visible) { + trace_intel_update_plane(&plane->base, crtc); - intel_plane->update_plane(intel_plane, - new_crtc_state, new_plane_state); - } else { - trace_intel_disable_plane(plane, - to_intel_crtc(crtc)); + plane->update_plane(plane, new_crtc_state, new_plane_state); + } else { + trace_intel_disable_plane(&plane->base, crtc); - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); + plane->disable_plane(plane, crtc); + } } } @@ -194,7 +198,6 @@ const struct drm_plane_helper_funcs intel_plane_helper_funcs = { .prepare_fb = intel_prepare_plane_fb, .cleanup_fb = intel_cleanup_plane_fb, .atomic_check = intel_plane_atomic_check, - .atomic_update = intel_plane_atomic_update, }; /** diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f7982f18b8fe..3975f3af4fb4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10813,8 +10813,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, } static const struct drm_crtc_helper_funcs intel_helper_funcs = { - .atomic_begin = intel_begin_crtc_commit, - .atomic_flush = intel_finish_crtc_commit, .atomic_check = intel_crtc_atomic_check, }; @@ -12483,6 +12481,7 @@ static void intel_update_crtc(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *old_intel_cstate = to_intel_crtc_state(old_crtc_state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(new_crtc_state); bool modeset = needs_modeset(new_crtc_state); struct intel_plane_state *new_plane_state = @@ -12503,7 +12502,12 @@ static void intel_update_crtc(struct drm_crtc *crtc, if (new_plane_state) intel_fbc_enable(intel_crtc, pipe_config, new_plane_state); - drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); + intel_begin_crtc_commit(crtc, old_crtc_state); + + intel_update_planes_on_crtc(to_intel_atomic_state(state), intel_crtc, + old_intel_cstate, pipe_config); + + intel_finish_crtc_commit(crtc, old_crtc_state); } static void intel_update_crtcs(struct drm_atomic_state *state) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bf1c38728a59..8073a85d7178 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2188,6 +2188,10 @@ struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; +void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, + struct intel_crtc *crtc, + struct intel_crtc_state *old_crtc_state, + struct intel_crtc_state *new_crtc_state); int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *crtc_state, const struct intel_plane_state *old_plane_state,
drm_atomic_helper_commit_planes_on_crtc calls begin_commit, then plane_update hooks, then flush_commit. Because we keep our own visibility tracking through plane_state->visible there's no need to rely on the atomic hooks for this. By explicitly writing our own helper, we can update visible planes as needed, which is useful to make NV12 support work as intended. Changes since v1: - Reword commit message. (Matt Roper) - Rename to intel_update_planes_on_crtc(). (Matt) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 41 ++++++++++++----------- drivers/gpu/drm/i915/intel_display.c | 10 ++++-- drivers/gpu/drm/i915/intel_drv.h | 4 +++ 3 files changed, 33 insertions(+), 22 deletions(-)