[Intel-gfx] [PATCH v3 07/20] drm/i915: Rework plane readout.
Daniel Vetter
daniel at ffwll.ch
Tue Jul 14 02:53:28 PDT 2015
On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote:
> All non-primary planes get disabled during hw readout,
> this reduces complexity and means not having to do some plane
> visibility checks during the first commit.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
I still think calling readout_plane_state from the hw state readout code
is the wrong approach. Instead we should consolidate all the plane
readout code exclusively into a new intel_plane_readout_hw_state helper
which is called only from driver load paths. That should also contain the
fb/gem_bo reconstruction loop.
For the other 2 users of this code (lid notifiery and resume) we should
just force an unconditional plane restore by setting
crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work,
once atomic has settled a bit more.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 7 ---
> drivers/gpu/drm/i915/intel_display.c | 86 ++++--------------------------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 3 files changed, 8 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index b92b8581efc2..dcf4fb458649 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -98,13 +98,6 @@ int intel_atomic_check(struct drm_device *dev,
> return -EINVAL;
> }
>
> - if (crtc_state &&
> - crtc_state->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> - ret = drm_atomic_add_affected_planes(state, &nuclear_crtc->base);
> - if (ret)
> - return ret;
> - }
> -
> ret = drm_atomic_helper_check_planes(dev, state);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e4d8acc63823..118187dc76be 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11783,34 +11783,6 @@ static bool check_encoder_cloning(struct drm_atomic_state *state,
> return true;
> }
>
> -static void intel_crtc_check_initial_planes(struct drm_crtc *crtc,
> - struct drm_crtc_state *crtc_state)
> -{
> - struct intel_crtc_state *pipe_config =
> - to_intel_crtc_state(crtc_state);
> - struct drm_plane *p;
> - unsigned visible_mask = 0;
> -
> - drm_for_each_plane_mask(p, crtc->dev, crtc_state->plane_mask) {
> - struct drm_plane_state *plane_state =
> - drm_atomic_get_existing_plane_state(crtc_state->state, p);
> -
> - if (WARN_ON(!plane_state))
> - continue;
> -
> - if (!plane_state->fb)
> - crtc_state->plane_mask &=
> - ~(1 << drm_plane_index(p));
> - else if (to_intel_plane_state(plane_state)->visible)
> - visible_mask |= 1 << drm_plane_index(p);
> - }
> -
> - if (!visible_mask)
> - return;
> -
> - pipe_config->quirks &= ~PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -}
> -
> static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_crtc_state *crtc_state)
> {
> @@ -11832,10 +11804,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> "[CRTC:%i] mismatch between state->active(%i) and crtc->active(%i)\n",
> idx, crtc->state->active, intel_crtc->active);
>
> - /* plane mask is fixed up after all initial planes are calculated */
> - if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES)
> - intel_crtc_check_initial_planes(crtc, crtc_state);
> -
> if (mode_changed && !crtc_state->active)
> intel_crtc->atomic.update_wm_post = true;
>
> @@ -13188,19 +13156,6 @@ intel_modeset_compute_config(struct drm_atomic_state *state)
> continue;
> }
>
> - if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> - ret = drm_atomic_add_affected_planes(state, crtc);
> - if (ret)
> - return ret;
> -
> - /*
> - * We ought to handle i915.fastboot here.
> - * If no modeset is required and the primary plane has
> - * a fb, update the members of crtc_state as needed,
> - * and run the necessary updates during vblank evasion.
> - */
> - }
> -
> modeset = needs_modeset(crtc_state);
> recalc = pipe_config->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE;
>
> @@ -15460,47 +15415,22 @@ static void readout_plane_state(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state)
> {
> struct intel_plane *p;
> - struct drm_plane_state *drm_plane_state;
> + struct intel_plane_state *plane_state;
> bool active = crtc_state->base.active;
>
> - if (active) {
> - crtc_state->quirks |= PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -
> - /* apply to previous sw state too */
> - to_intel_crtc_state(crtc->base.state)->quirks |=
> - PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> - }
> -
> for_each_intel_plane(crtc->base.dev, p) {
> - bool visible = active;
> -
> if (crtc->pipe != p->pipe)
> continue;
>
> - drm_plane_state = p->base.state;
> -
> - /* Plane scaler state is not touched here. The first atomic
> - * commit will restore all plane scalers to its old state.
> - */
> + plane_state = to_intel_plane_state(p->base.state);
>
> - if (active && p->base.type == DRM_PLANE_TYPE_PRIMARY) {
> - visible = primary_get_hw_state(crtc);
> - to_intel_plane_state(drm_plane_state)->visible = visible;
> - } else {
> - /*
> - * unknown state, assume it's off to force a transition
> - * to on when calculating state changes.
> - */
> - to_intel_plane_state(drm_plane_state)->visible = false;
> - }
> + if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
> + plane_state->visible = primary_get_hw_state(crtc);
> + else {
> + if (active)
> + p->disable_plane(&p->base, &crtc->base);
>
> - if (visible) {
> - crtc_state->base.plane_mask |=
> - 1 << drm_plane_index(&p->base);
> - } else if (crtc_state->base.state) {
> - /* Make this unconditional for atomic hw readout. */
> - crtc_state->base.plane_mask &=
> - ~(1 << drm_plane_index(&p->base));
> + plane_state->visible = false;
> }
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 09e3581c8441..2c23900b491f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -341,7 +341,6 @@ struct intel_crtc_state {
> */
> #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (1<<0) /* unreliable sync mode.flags */
> #define PIPE_CONFIG_QUIRK_INHERITED_MODE (1<<1) /* mode inherited from firmware */
> -#define PIPE_CONFIG_QUIRK_INITIAL_PLANES (1<<2) /* planes are in unknown state */
> unsigned long quirks;
>
> /* Pipe source size (ie. panel fitter input size)
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list