Op 15-12-16 om 16:02 schreef Ville Syrjälä: > On Thu, Dec 15, 2016 at 03:56:03PM +0100, Maarten Lankhorst wrote: >> Op 12-12-16 om 21:35 schreef [email protected]: >>> From: Ville Syrjälä <[email protected]> >>> >>> In a lot of place we wish to know which planes on the crtc are actually >>> visible, or how many of them there are. Let's start tracking that in a >>> bitmask in the crtc state. >>> >>> We already track enabled planes (ie. ones with an fb and crtc specified by >>> the user) but that's not quite the same thing as enabled planes may >>> still end up being invisible due to clipping and whatnot. >>> >>> Signed-off-by: Ville Syrjälä <[email protected]> >>> --- >>> drivers/gpu/drm/i915/intel_atomic_plane.c | 6 +++ >>> drivers/gpu/drm/i915/intel_display.c | 79 >>> +++++++++++++++++++++---------- >>> drivers/gpu/drm/i915/intel_drv.h | 3 ++ >>> 3 files changed, 64 insertions(+), 24 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c >>> b/drivers/gpu/drm/i915/intel_atomic_plane.c >>> index dbe9fb41ae53..7ec12edda4d4 100644 >>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c >>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c >>> @@ -181,6 +181,12 @@ static int intel_plane_atomic_check(struct drm_plane >>> *plane, >>> if (ret) >>> return ret; >>> >>> + /* FIXME pre-g4x don't work like this */ >>> + if (intel_state->base.visible) >>> + crtc_state->active_planes |= BIT(intel_plane->id); >>> + else >>> + crtc_state->active_planes &= ~BIT(intel_plane->id); >>> + >>> return intel_plane_atomic_calc_changes(&crtc_state->base, state); >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c >>> b/drivers/gpu/drm/i915/intel_display.c >>> index bc1af87789bc..3f027341b0f3 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -2741,6 +2741,29 @@ update_state_fb(struct drm_plane *plane) >>> } >>> >>> static void >>> +intel_set_plane_visible(struct intel_crtc_state *crtc_state, >>> + struct intel_plane_state *plane_state, >>> + bool visible) >>> +{ >>> + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); >>> + >>> + plane_state->base.visible = visible; >>> + >>> + /* FIXME pre-g4x don't work like this */ >>> + if (visible) { >>> + crtc_state->base.plane_mask |= >>> BIT(drm_plane_index(&plane->base)); >>> + crtc_state->active_planes |= BIT(plane->id); >>> + } else { >>> + crtc_state->base.plane_mask &= >>> ~BIT(drm_plane_index(&plane->base)); >>> + crtc_state->active_planes &= ~BIT(plane->id); >>> + } >>> + >>> + DRM_DEBUG_KMS("%s active planes 0x%x\n", >>> + crtc_state->base.crtc->name, >>> + crtc_state->active_planes); >>> +} >>> + >>> +static void >>> intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, >>> struct intel_initial_plane_config *plane_config) >>> { >>> @@ -2798,8 +2821,9 @@ intel_find_initial_plane_obj(struct intel_crtc >>> *intel_crtc, >>> * simplest solution is to just disable the primary plane now and >>> * pretend the BIOS never had it enabled. >>> */ >>> - to_intel_plane_state(plane_state)->base.visible = false; >>> - crtc_state->plane_mask &= ~(1 << drm_plane_index(primary)); >>> + intel_set_plane_visible(to_intel_crtc_state(crtc_state), >>> + to_intel_plane_state(plane_state), >>> + false); >>> intel_pre_disable_primary_noatomic(&intel_crtc->base); >>> intel_plane->disable_plane(primary, &intel_crtc->base); >>> >>> @@ -2826,7 +2850,11 @@ intel_find_initial_plane_obj(struct intel_crtc >>> *intel_crtc, >>> drm_framebuffer_reference(fb); >>> primary->fb = primary->state->fb = fb; >>> primary->crtc = primary->state->crtc = &intel_crtc->base; >>> - intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary)); >>> + >>> + intel_set_plane_visible(to_intel_crtc_state(intel_crtc->base.state), >>> + to_intel_plane_state(primary->state), >>> + true); >>> + >>> atomic_or(to_intel_plane(primary)->frontbuffer_bit, >>> &obj->frontbuffer_bits); >>> } >>> @@ -12440,11 +12468,11 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state); >>> struct drm_crtc *crtc = crtc_state->crtc; >>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >>> - struct drm_plane *plane = plane_state->plane; >>> + struct intel_plane *plane = to_intel_plane(plane_state->plane); >>> struct drm_device *dev = crtc->dev; >>> struct drm_i915_private *dev_priv = to_i915(dev); >>> struct intel_plane_state *old_plane_state = >>> - to_intel_plane_state(plane->state); >>> + to_intel_plane_state(plane->base.state); >>> bool mode_changed = needs_modeset(crtc_state); >>> bool was_crtc_enabled = crtc->state->active; >>> bool is_crtc_enabled = crtc_state->active; >>> @@ -12452,7 +12480,7 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> struct drm_framebuffer *fb = plane_state->fb; >>> int ret; >>> >>> - if (INTEL_GEN(dev_priv) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) { >>> + if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) { >>> ret = skl_update_scaler_plane( >>> to_intel_crtc_state(crtc_state), >>> to_intel_plane_state(plane_state)); >>> @@ -12476,8 +12504,10 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> * per-plane wm computation to the .check_plane() hook, and >>> * only combine the results from all planes in the current place? >>> */ >>> - if (!is_crtc_enabled) >>> + if (!is_crtc_enabled) { >>> to_intel_plane_state(plane_state)->base.visible = visible = >>> false; >>> + to_intel_crtc_state(crtc_state)->active_planes &= >>> ~BIT(plane->id); >>> + } >>> >>> if (!was_visible && !visible) >>> return 0; >>> @@ -12489,13 +12519,12 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> turn_on = visible && (!was_visible || mode_changed); >>> >>> DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n", >>> - intel_crtc->base.base.id, >>> - intel_crtc->base.name, >>> - plane->base.id, plane->name, >>> + intel_crtc->base.base.id, intel_crtc->base.name, >>> + plane->base.base.id, plane->base.name, >>> fb ? fb->base.id : -1); >>> >>> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms >>> %i\n", >>> - plane->base.id, plane->name, >>> + plane->base.base.id, plane->base.name, >>> was_visible, visible, >>> turn_off, turn_on, mode_changed); >>> >>> @@ -12503,15 +12532,15 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> pipe_config->update_wm_pre = true; >>> >>> /* must disable cxsr around plane enable/disable */ >>> - if (plane->type != DRM_PLANE_TYPE_CURSOR) >>> + if (plane->id != PLANE_CURSOR) >>> pipe_config->disable_cxsr = true; >>> } else if (turn_off) { >>> pipe_config->update_wm_post = true; >>> >>> /* must disable cxsr around plane enable/disable */ >>> - if (plane->type != DRM_PLANE_TYPE_CURSOR) >>> + if (plane->id != PLANE_CURSOR) >>> pipe_config->disable_cxsr = true; >>> - } else if (intel_wm_need_update(plane, plane_state)) { >>> + } else if (intel_wm_need_update(&plane->base, plane_state)) { >>> /* FIXME bollocks */ >>> pipe_config->update_wm_pre = true; >>> pipe_config->update_wm_post = true; >>> @@ -12523,7 +12552,7 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true; >>> >>> if (visible || was_visible) >>> - pipe_config->fb_bits |= to_intel_plane(plane)->frontbuffer_bit; >>> + pipe_config->fb_bits |= plane->frontbuffer_bit; >>> >>> /* >>> * WaCxSRDisabledForSpriteScaling:ivb >>> @@ -12531,7 +12560,7 @@ int intel_plane_atomic_calc_changes(struct >>> drm_crtc_state *crtc_state, >>> * cstate->update_wm was already set above, so this flag will >>> * take effect when we commit and program watermarks. >>> */ >>> - if (plane->type == DRM_PLANE_TYPE_OVERLAY && IS_IVYBRIDGE(dev_priv) && >>> + if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) && >>> needs_scaling(to_intel_plane_state(plane_state)) && >>> !needs_scaling(old_plane_state)) >>> pipe_config->disable_lp_wm = true; >>> @@ -16876,15 +16905,14 @@ static bool primary_get_hw_state(struct >>> intel_plane *plane) >>> /* FIXME read out full plane state for all planes */ >>> static void readout_plane_state(struct intel_crtc *crtc) >>> { >>> - struct drm_plane *primary = crtc->base.primary; >>> - struct intel_plane_state *plane_state = >>> - to_intel_plane_state(primary->state); >>> + struct intel_plane *primary = to_intel_plane(crtc->base.primary); >>> + bool visible; >>> >>> - plane_state->base.visible = crtc->active && >>> - primary_get_hw_state(to_intel_plane(primary)); >>> + visible = crtc->active && primary_get_hw_state(primary); >>> >>> - if (plane_state->base.visible) >>> - crtc->base.state->plane_mask |= 1 << drm_plane_index(primary); >>> + intel_set_plane_visible(to_intel_crtc_state(crtc->base.state), >>> + to_intel_plane_state(primary->base.state), >>> + visible); >>> } >>> >>> static void intel_modeset_readout_hw_state(struct drm_device *dev) >>> @@ -17171,7 +17199,10 @@ void intel_modeset_gem_init(struct drm_device *dev) >>> c->primary->fb = NULL; >>> c->primary->crtc = c->primary->state->crtc = NULL; >>> update_state_fb(c->primary); >>> - c->state->plane_mask &= ~(1 << >>> drm_plane_index(c->primary)); >>> + >>> + intel_set_plane_visible(to_intel_crtc_state(c->state), >>> + >>> to_intel_plane_state(c->primary->state), >>> + false); >>> } >>> } >>> } >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h >>> b/drivers/gpu/drm/i915/intel_drv.h >>> index 8f4ddca0f521..20ba8f48bc3b 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -669,6 +669,9 @@ struct intel_crtc_state { >>> >>> /* Gamma mode programmed on the pipe */ >>> uint32_t gamma_mode; >>> + >>> + /* bitmask of visible planes (enum plane_id) */ >>> + u8 active_planes; >> Can we change this to uint32_t 1<<drm_plane_index like the other masks? >> >> plane_mask, connector_mask and encoder_mask use 1 << index. >> Perhaps name it active_planes_mask too. > The way I use it wouldn't lend itself well to that. I don't have any > plane structs around in a bunch of the places where this gets used. > Do you use this outside of 13/14? If not please drop the patch from this series. :)
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
