Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx> On 01/22/2015 02:35 AM, Matt Roper wrote: > Runtime state that can be manipulated via properties should now go in > intel_plane_state/drm_plane_state so that it can be tracked as part of > an atomic transaction. > > We add a new 'intel_create_plane_state' function so that the proper > initial value for this property (and future properties) doesn't have to > be repeated at each plane initialization site. > > v2: > - Stick rotation in common drm_plane_state rather than > intel_plane_state. (Daniel) > - Add intel_create_plane_state() to consolidate the places where we > have to set initial state values. (Ander) > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 45 ++++++++++++++++++++++++------- > drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++----------- > drivers/gpu/drm/i915/intel_drv.h | 8 +++++- > drivers/gpu/drm/i915/intel_fbc.c | 2 +- > drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++---------- > 5 files changed, 75 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 4027fc0..d9d4306 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -37,31 +37,58 @@ > #include "intel_drv.h" > > /** > + * intel_create_plane_state - create plane state object > + * @plane: drm plane > + * > + * Allocates a fresh plane state for the given plane and sets some of > + * the state values to sensible initial values. > + * > + * Returns: A newly allocated plane state, or NULL on failure > + */ > +struct intel_plane_state * > +intel_create_plane_state(struct drm_plane *plane) > +{ > + struct intel_plane_state *state; > + > + state = kzalloc(sizeof(*state), GFP_KERNEL); > + if (!state) > + return NULL; > + > + state->base.plane = plane; > + state->base.rotation = BIT(DRM_ROTATE_0); > + > + return state; > +} > + > +/** > * intel_plane_duplicate_state - duplicate plane state > * @plane: drm plane > * > * Allocates and returns a copy of the plane state (both common and > * Intel-specific) for the specified plane. > * > - * Returns: The newly allocated plane state, or NULL or failure. > + * Returns: The newly allocated plane state, or NULL on failure. > */ > struct drm_plane_state * > intel_plane_duplicate_state(struct drm_plane *plane) > { > - struct intel_plane_state *state; > + struct drm_plane_state *state; > + struct intel_plane_state *intel_state; > > - if (plane->state) > - state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL); > + if (WARN_ON(!plane->state)) > + intel_state = intel_create_plane_state(plane); > else > - state = kzalloc(sizeof(*state), GFP_KERNEL); > + intel_state = kmemdup(plane->state, sizeof(*intel_state), > + GFP_KERNEL); > > - if (!state) > + if (!intel_state) > return NULL; > > - if (state->base.fb) > - drm_framebuffer_reference(state->base.fb); > + state = &intel_state->base; > + if (state->fb) > + drm_framebuffer_reference(state->fb); > > - return &state->base; > + return state; > } > > /** > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 01dc80b..db42824 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2560,7 +2560,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > intel_crtc->dspaddr_offset = linear_offset; > } > > - if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) { > + if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) { > dspcntr |= DISPPLANE_ROTATE_180; > > x += (intel_crtc->config->pipe_src_w - 1); > @@ -2662,7 +2662,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > pixel_size, > fb->pitches[0]); > linear_offset -= intel_crtc->dspaddr_offset; > - if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) { > + if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) { > dspcntr |= DISPPLANE_ROTATE_180; > > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { > @@ -2759,7 +2759,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, > } > > plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE; > - if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) > + if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) > plane_ctl |= PLANE_CTL_ROTATE_180; > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > @@ -8332,7 +8332,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base) > cntl |= CURSOR_PIPE_CSC_ENABLE; > } > > - if (to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) > + if (crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) > cntl |= CURSOR_ROTATE_180; > > if (intel_crtc->cursor_cntl != cntl) { > @@ -8394,7 +8394,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, > > /* ILK+ do this automagically */ > if (HAS_GMCH_DISPLAY(dev) && > - to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) { > + crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) { > base += (intel_crtc->cursor_height * > intel_crtc->cursor_width - 1) * 4; > } > @@ -11846,7 +11846,6 @@ intel_check_primary_plane(struct drm_plane *plane, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *crtc = state->base.crtc; > struct intel_crtc *intel_crtc; > - struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > struct drm_rect *src = &state->src; > @@ -11880,7 +11879,7 @@ intel_check_primary_plane(struct drm_plane *plane, > if (intel_crtc->primary_enabled && > INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > dev_priv->fbc.plane == intel_crtc->plane && > - intel_plane->rotation != BIT(DRM_ROTATE_0)) { > + state->base.rotation != BIT(DRM_ROTATE_0)) { > intel_crtc->atomic.disable_fbc = true; > } > > @@ -12064,6 +12063,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > int pipe) > { > struct intel_plane *primary; > + struct intel_plane_state *state; > const uint32_t *intel_primary_formats; > int num_formats; > > @@ -12071,17 +12071,17 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > if (primary == NULL) > return NULL; > > - primary->base.state = intel_plane_duplicate_state(&primary->base); > - if (primary->base.state == NULL) { > + state = intel_create_plane_state(&primary->base); > + if (!state) { > kfree(primary); > return NULL; > } > + primary->base.state = &state->base; > > primary->can_scale = false; > primary->max_downscale = 1; > primary->pipe = pipe; > primary->plane = pipe; > - primary->rotation = BIT(DRM_ROTATE_0); > primary->check_plane = intel_check_primary_plane; > primary->commit_plane = intel_commit_primary_plane; > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > @@ -12109,7 +12109,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > if (dev->mode_config.rotation_property) > drm_object_attach_property(&primary->base.base, > dev->mode_config.rotation_property, > - primary->rotation); > + state->base.rotation); > } > > drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs); > @@ -12237,22 +12237,23 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > int pipe) > { > struct intel_plane *cursor; > + struct intel_plane_state *state; > > cursor = kzalloc(sizeof(*cursor), GFP_KERNEL); > if (cursor == NULL) > return NULL; > > - cursor->base.state = intel_plane_duplicate_state(&cursor->base); > - if (cursor->base.state == NULL) { > + state = intel_create_plane_state(&cursor->base); > + if (!state) { > kfree(cursor); > return NULL; > } > + cursor->base.state = &state->base; > > cursor->can_scale = false; > cursor->max_downscale = 1; > cursor->pipe = pipe; > cursor->plane = pipe; > - cursor->rotation = BIT(DRM_ROTATE_0); > cursor->check_plane = intel_check_cursor_plane; > cursor->commit_plane = intel_commit_cursor_plane; > > @@ -12271,7 +12272,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > if (dev->mode_config.rotation_property) > drm_object_attach_property(&cursor->base.base, > dev->mode_config.rotation_property, > - cursor->rotation); > + state->base.rotation); > } > > drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index e957d4d..b0562e4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -509,7 +509,6 @@ struct intel_plane { > struct drm_i915_gem_object *obj; > bool can_scale; > int max_downscale; > - unsigned int rotation; > > /* Since we need to change the watermarks before/after > * enabling/disabling the planes, we need to store the parameters here > @@ -518,6 +517,12 @@ struct intel_plane { > */ > struct intel_plane_wm_parameters wm; > > + /* > + * NOTE: Do not place new plane state fields here (e.g., when adding > + * new plane properties). New runtime state should now be placed in > + * the intel_plane_state structure and accessed via drm_plane->state. > + */ > + > void (*update_plane)(struct drm_plane *plane, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > @@ -1230,6 +1235,7 @@ void intel_pre_disable_primary(struct drm_crtc *crtc); > void intel_tv_init(struct drm_device *dev); > > /* intel_atomic.c */ > +struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); > 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); > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index ed9a012..624d1d9 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -595,7 +595,7 @@ void intel_fbc_update(struct drm_device *dev) > goto out_disable; > } > if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > - to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) { > + crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) { > if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) > DRM_DEBUG_KMS("Rotation unsupported, disabling\n"); > goto out_disable; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 0a6094e..ba85439 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -256,7 +256,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, > default: > BUG(); > } > - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) > + if (drm_plane->state->rotation == BIT(DRM_ROTATE_180)) > plane_ctl |= PLANE_CTL_ROTATE_180; > > plane_ctl |= PLANE_CTL_ENABLE; > @@ -493,7 +493,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, > fb->pitches[0]); > linear_offset -= sprsurf_offset; > > - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) { > + if (dplane->state->rotation == BIT(DRM_ROTATE_180)) { > sprctl |= SP_ROTATE_180; > > x += src_w; > @@ -684,7 +684,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > pixel_size, fb->pitches[0]); > linear_offset -= sprsurf_offset; > > - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) { > + if (plane->state->rotation == BIT(DRM_ROTATE_180)) { > sprctl |= SPRITE_ROTATE_180; > > /* HSW and BDW does this automagically in hardware */ > @@ -884,7 +884,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > pixel_size, fb->pitches[0]); > linear_offset -= dvssurf_offset; > > - if (intel_plane->rotation == BIT(DRM_ROTATE_180)) { > + if (plane->state->rotation == BIT(DRM_ROTATE_180)) { > dvscntr |= DVS_ROTATE_180; > > x += src_w; > @@ -1125,7 +1125,7 @@ intel_check_sprite_plane(struct drm_plane *plane, > min_scale = intel_plane->can_scale ? 1 : (1 << 16); > > drm_rect_rotate(src, fb->width << 16, fb->height << 16, > - intel_plane->rotation); > + state->base.rotation); > > hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale); > BUG_ON(hscale < 0); > @@ -1166,7 +1166,7 @@ intel_check_sprite_plane(struct drm_plane *plane, > drm_rect_height(dst) * vscale - drm_rect_height(src)); > > drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, > - intel_plane->rotation); > + state->base.rotation); > > /* sanity check to make sure the src viewport wasn't enlarged */ > WARN_ON(src->x1 < (int) state->base.src_x || > @@ -1367,7 +1367,6 @@ int intel_plane_set_property(struct drm_plane *plane, > uint64_t val) > { > struct drm_device *dev = plane->dev; > - struct intel_plane *intel_plane = to_intel_plane(plane); > uint64_t old_val; > int ret = -ENOENT; > > @@ -1376,14 +1375,14 @@ int intel_plane_set_property(struct drm_plane *plane, > if (hweight32(val & 0xf) != 1) > return -EINVAL; > > - if (intel_plane->rotation == val) > + if (plane->state->rotation == val) > return 0; > > - old_val = intel_plane->rotation; > - intel_plane->rotation = val; > + old_val = plane->state->rotation; > + plane->state->rotation = val; > ret = intel_plane_restore(plane); > if (ret) > - intel_plane->rotation = old_val; > + plane->state->rotation = old_val; > } > > return ret; > @@ -1457,6 +1456,7 @@ int > intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > { > struct intel_plane *intel_plane; > + struct intel_plane_state *state; > unsigned long possible_crtcs; > const uint32_t *plane_formats; > int num_plane_formats; > @@ -1469,12 +1469,12 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > if (!intel_plane) > return -ENOMEM; > > - intel_plane->base.state = > - intel_plane_duplicate_state(&intel_plane->base); > - if (intel_plane->base.state == NULL) { > + state = intel_create_plane_state(&intel_plane->base); > + if (!state) { > kfree(intel_plane); > return -ENOMEM; > } > + intel_plane->base.state = &state->base; > > switch (INTEL_INFO(dev)->gen) { > case 5: > @@ -1545,7 +1545,6 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > > intel_plane->pipe = pipe; > intel_plane->plane = plane; > - intel_plane->rotation = BIT(DRM_ROTATE_0); > intel_plane->check_plane = intel_check_sprite_plane; > intel_plane->commit_plane = intel_commit_sprite_plane; > possible_crtcs = (1 << pipe); > @@ -1567,7 +1566,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > if (dev->mode_config.rotation_property) > drm_object_attach_property(&intel_plane->base.base, > dev->mode_config.rotation_property, > - intel_plane->rotation); > + state->base.rotation); > > drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs); > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx