On Wed, Oct 12, 2016 at 03:28:14PM +0200, Maarten Lankhorst wrote: > Caching is not required, drm_atomic_crtc_state_for_each_plane_state > can be used to inspect all plane_states that are assigned to the > current crtc_state, so we can just recalculate every time. > > Signed-off-by: Maarten Lankhorst <[email protected]> > --- > drivers/gpu/drm/i915/intel_pm.c | 27 ++++++++++++--------------- > 1 file changed, 12 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 6af1587e9d84..b96a899c899d 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -31,6 +31,7 @@ > #include "intel_drv.h" > #include "../../../platform/x86/intel_ips.h" > #include <linux/module.h> > +#include <drm/drm_atomic_helper.h> > > /** > * DOC: RC6 > @@ -3242,18 +3243,17 @@ skl_get_total_relative_data_rate(struct > intel_crtc_state *intel_cstate) > struct drm_crtc *crtc = cstate->crtc; > struct drm_device *dev = crtc->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - const struct drm_plane *plane; > + struct drm_plane *plane; > const struct intel_plane *intel_plane; > - struct drm_plane_state *pstate; > + const struct drm_plane_state *pstate; > unsigned int rate, total_data_rate = 0; > int id; > - int i; > > if (WARN_ON(!state)) > return 0; > > /* Calculate and cache data rate for each plane */ > - for_each_plane_in_state(state, plane, pstate, i) { > + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, cstate) { > id = skl_wm_plane_id(to_intel_plane(plane)); > intel_plane = to_intel_plane(plane); > > @@ -3356,7 +3356,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane; > struct drm_plane *plane; > - struct drm_plane_state *pstate; > + const struct drm_plane_state *pstate; > enum pipe pipe = intel_crtc->pipe; > struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; > uint16_t alloc_size, start, cursor_blocks; > @@ -3392,14 +3392,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > alloc_size -= cursor_blocks; > > /* 1. Allocate the mininum required blocks for each active plane */ > - for_each_plane_in_state(state, plane, pstate, i) { > + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, > &cstate->base) { > intel_plane = to_intel_plane(plane); > id = skl_wm_plane_id(intel_plane); > > if (intel_plane->pipe != pipe) > continue; > > - if (!to_intel_plane_state(pstate)->base.visible) { > + if (!pstate->visible) { > minimum[id] = 0; > y_minimum[id] = 0; > continue; > @@ -3948,7 +3948,7 @@ skl_ddb_add_affected_planes(struct intel_crtc_state > *cstate) > > WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); > > - drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) { > + drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) {
Is this change necessary? Any plane that differs between the two masks should already be part of our state, so I don't think this changes the behavior at all. The original 'crtc->state->plane_mask' form is closer to the drm_atomic_add_affected_planes() that this function is modeled after so my slight preference would be to leave it alone for consistency. Aside from that, this patch is Reviewed-by: Matt Roper <[email protected]> I remember when I first wrote an early version of this code it was just doing a drm_atomic_get_plane_state() on each plane unconditionally, which was non-optimal. When I reworked it, I wasn't aware of drm_atomic_crtc_state_for_each_plane_state (or maybe it didn't exist yet), so I used caching as an alternative. But the new approach is much better so I'm glad we can get rid of the caching. Matt > id = skl_wm_plane_id(to_intel_plane(plane)); > > if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][id], > @@ -4063,14 +4063,12 @@ skl_print_wm_changes(const struct drm_atomic_state > *state) > to_intel_atomic_state(state); > const struct drm_crtc *crtc; > const struct drm_crtc_state *cstate; > - const struct drm_plane *plane; > const struct intel_plane *intel_plane; > - const struct drm_plane_state *pstate; > const struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb; > const struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb; > enum pipe pipe; > int id; > - int i, j; > + int i; > > for_each_crtc_in_state(state, crtc, cstate, i) { > if (!crtc->state) > @@ -4078,10 +4076,9 @@ skl_print_wm_changes(const struct drm_atomic_state > *state) > > pipe = to_intel_crtc(crtc)->pipe; > > - for_each_plane_in_state(state, plane, pstate, j) { > + for_each_intel_plane_on_crtc(dev, to_intel_crtc(crtc), > intel_plane) { > const struct skl_ddb_entry *old, *new; > > - intel_plane = to_intel_plane(plane); > id = skl_wm_plane_id(intel_plane); > old = &old_ddb->plane[pipe][id]; > new = &new_ddb->plane[pipe][id]; > @@ -4094,13 +4091,13 @@ skl_print_wm_changes(const struct drm_atomic_state > *state) > > if (id != PLANE_CURSOR) { > DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb (%d > - %d) -> (%d - %d)\n", > - plane->base.id, id + 1, > + intel_plane->base.base.id, id > + 1, > pipe_name(pipe), > old->start, old->end, > new->start, new->end); > } else { > DRM_DEBUG_ATOMIC("[PLANE:%d:cursor %c] ddb (%d > - %d) -> (%d - %d)\n", > - plane->base.id, > + intel_plane->base.base.id, > pipe_name(pipe), > old->start, old->end, > new->start, new->end); > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
