[Intel-gfx] [PATCH 1/6] drm/i915: Move rotation from intel_plane to intel_plane_state
Matt Roper
matthew.d.roper at intel.com
Tue Jan 20 07:26:20 PST 2015
On Tue, Jan 20, 2015 at 10:44:06AM +0100, Daniel Vetter wrote:
> On Thu, Jan 15, 2015 at 06:34:19PM -0800, Matt Roper wrote:
> > Runtime state that can be manipulated via properties should now go in
> > intel_plane_state instead so that it can be tracked as part of an atomic
> > transaction.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>
> Imo we should move this to drm_plane_state so that fb helpers or and the
> drm atomic ioctl could do the decoding in shared code instead of
> duplicating things all over. But we probably want to do that once the i915
> conversion (at least on the plane side) has settled a bit.
> -Daniel
I considered this, but I wasn't sure how it would shake out if some
drivers have converted to atomic (and store their rotation in plane
state) and other drivers haven't converted (and still store it in some
driver-specific area). If drivers haven't converted to atomic, the
atomic ioctl wouldn't be a concern, but it seems like the fb helpers
might have more difficulty figuring out whether it could trust rotation
values or not. From a quick cscope, it looks like exynos and omap at
least have some rotation support and I didn't want to deal with them
until I had the Intel work in good shape.
Matt
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++------------
> > drivers/gpu/drm/i915/intel_drv.h | 10 +++++++++-
> > drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
> > drivers/gpu/drm/i915/intel_sprite.c | 33 ++++++++++++++++++++-------------
> > 4 files changed, 56 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b6c4667..84b0b4d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2435,6 +2435,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> > {
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(crtc->primary->state);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > struct drm_i915_gem_object *obj;
> > int plane = intel_crtc->plane;
> > @@ -2532,7 +2534,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 (state->rotation == BIT(DRM_ROTATE_180)) {
> > dspcntr |= DISPPLANE_ROTATE_180;
> >
> > x += (intel_crtc->config->pipe_src_w - 1);
> > @@ -2568,6 +2570,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(crtc->primary->state);
> > struct drm_i915_gem_object *obj;
> > int plane = intel_crtc->plane;
> > unsigned long linear_offset;
> > @@ -2634,7 +2638,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 (state->rotation == BIT(DRM_ROTATE_180)) {
> > dspcntr |= DISPPLANE_ROTATE_180;
> >
> > if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> > @@ -2673,6 +2677,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(crtc->primary->state);
> > struct intel_framebuffer *intel_fb;
> > struct drm_i915_gem_object *obj;
> > int pipe = intel_crtc->pipe;
> > @@ -2731,7 +2737,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 (state->rotation == BIT(DRM_ROTATE_180))
> > plane_ctl |= PLANE_CTL_ROTATE_180;
> >
> > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> > @@ -8216,6 +8222,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(crtc->cursor->state);
> > int pipe = intel_crtc->pipe;
> > uint32_t cntl;
> >
> > @@ -8242,7 +8250,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 (state->rotation == BIT(DRM_ROTATE_180))
> > cntl |= CURSOR_ROTATE_180;
> >
> > if (intel_crtc->cursor_cntl != cntl) {
> > @@ -8265,6 +8273,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(crtc->cursor->state);
> > int pipe = intel_crtc->pipe;
> > int x = crtc->cursor_x;
> > int y = crtc->cursor_y;
> > @@ -8303,8 +8313,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> > I915_WRITE(CURPOS(pipe), pos);
> >
> > /* ILK+ do this automagically */
> > - if (HAS_GMCH_DISPLAY(dev) &&
> > - to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) {
> > + if (HAS_GMCH_DISPLAY(dev) && state->rotation == BIT(DRM_ROTATE_180)) {
> > base += (intel_crtc->cursor_height *
> > intel_crtc->cursor_width - 1) * 4;
> > }
> > @@ -11756,7 +11765,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;
> > @@ -11790,7 +11798,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->rotation != BIT(DRM_ROTATE_0)) {
> > intel_crtc->atomic.disable_fbc = true;
> > }
> >
> > @@ -11975,6 +11983,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;
> >
> > @@ -11987,12 +11996,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> > kfree(primary);
> > return NULL;
> > }
> > + state = to_intel_plane_state(primary->base.state);
> >
> > primary->can_scale = false;
> > primary->max_downscale = 1;
> > primary->pipe = pipe;
> > primary->plane = pipe;
> > - primary->rotation = BIT(DRM_ROTATE_0);
> > + state->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)
> > @@ -12020,7 +12030,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->rotation);
> > }
> >
> > drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
> > @@ -12148,6 +12158,7 @@ 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)
> > @@ -12158,12 +12169,13 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> > kfree(cursor);
> > return NULL;
> > }
> > + state = to_intel_plane_state(cursor->base.state);
> >
> > cursor->can_scale = false;
> > cursor->max_downscale = 1;
> > cursor->pipe = pipe;
> > cursor->plane = pipe;
> > - cursor->rotation = BIT(DRM_ROTATE_0);
> > + state->rotation = BIT(DRM_ROTATE_0);
> > cursor->check_plane = intel_check_cursor_plane;
> > cursor->commit_plane = intel_commit_cursor_plane;
> >
> > @@ -12182,7 +12194,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->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 c8c0b7f..918cce2 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -250,6 +250,9 @@ struct intel_plane_state {
> > struct drm_rect clip;
> > bool visible;
> >
> > + /* Intel-specific plane properties */
> > + unsigned int rotation;
> > +
> > /*
> > * used only for sprite planes to determine when to implicitly
> > * enable/disable the primary plane
> > @@ -509,7 +512,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 +520,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,
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 5b1d7c4..3e87454 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -496,6 +496,7 @@ void intel_fbc_update(struct drm_device *dev)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct drm_crtc *crtc = NULL, *tmp_crtc;
> > struct intel_crtc *intel_crtc;
> > + struct intel_plane_state *primary_state;
> > struct drm_framebuffer *fb;
> > struct drm_i915_gem_object *obj;
> > const struct drm_display_mode *adjusted_mode;
> > @@ -540,6 +541,7 @@ void intel_fbc_update(struct drm_device *dev)
> > }
> >
> > intel_crtc = to_intel_crtc(crtc);
> > + primary_state = to_intel_plane_state(crtc->primary->state);
> > fb = crtc->primary->fb;
> > obj = intel_fb_obj(fb);
> > adjusted_mode = &intel_crtc->config->base.adjusted_mode;
> > @@ -595,7 +597,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)) {
> > + 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..140c5b7 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -187,6 +187,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> > struct drm_device *dev = drm_plane->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> > + struct intel_plane_state *state =
> > + to_intel_plane_state(drm_plane->state);
> > const int pipe = intel_plane->pipe;
> > const int plane = intel_plane->plane + 1;
> > u32 plane_ctl, stride;
> > @@ -256,7 +258,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> > default:
> > BUG();
> > }
> > - if (intel_plane->rotation == BIT(DRM_ROTATE_180))
> > + if (state->rotation == BIT(DRM_ROTATE_180))
> > plane_ctl |= PLANE_CTL_ROTATE_180;
> >
> > plane_ctl |= PLANE_CTL_ENABLE;
> > @@ -407,6 +409,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> > struct drm_device *dev = dplane->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_plane *intel_plane = to_intel_plane(dplane);
> > + struct intel_plane_state *state = to_intel_plane_state(dplane->state);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > int pipe = intel_plane->pipe;
> > int plane = intel_plane->plane;
> > @@ -493,7 +496,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 (state->rotation == BIT(DRM_ROTATE_180)) {
> > sprctl |= SP_ROTATE_180;
> >
> > x += src_w;
> > @@ -608,6 +611,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > struct drm_device *dev = plane->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > + struct intel_plane_state *state = to_intel_plane_state(plane->state);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > int pipe = intel_plane->pipe;
> > u32 sprctl, sprscale = 0;
> > @@ -684,7 +688,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 (state->rotation == BIT(DRM_ROTATE_180)) {
> > sprctl |= SPRITE_ROTATE_180;
> >
> > /* HSW and BDW does this automagically in hardware */
> > @@ -813,6 +817,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > struct drm_device *dev = plane->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > + struct intel_plane_state *state = to_intel_plane_state(plane->state);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > int pipe = intel_plane->pipe;
> > unsigned long dvssurf_offset, linear_offset;
> > @@ -884,7 +889,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 (state->rotation == BIT(DRM_ROTATE_180)) {
> > dvscntr |= DVS_ROTATE_180;
> >
> > x += src_w;
> > @@ -1125,7 +1130,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->rotation);
> >
> > hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
> > BUG_ON(hscale < 0);
> > @@ -1166,7 +1171,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->rotation);
> >
> > /* sanity check to make sure the src viewport wasn't enlarged */
> > WARN_ON(src->x1 < (int) state->base.src_x ||
> > @@ -1367,7 +1372,7 @@ 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);
> > + struct intel_plane_state *state = to_intel_plane_state(plane->state);
> > uint64_t old_val;
> > int ret = -ENOENT;
> >
> > @@ -1376,14 +1381,14 @@ int intel_plane_set_property(struct drm_plane *plane,
> > if (hweight32(val & 0xf) != 1)
> > return -EINVAL;
> >
> > - if (intel_plane->rotation == val)
> > + if (state->rotation == val)
> > return 0;
> >
> > - old_val = intel_plane->rotation;
> > - intel_plane->rotation = val;
> > + old_val = state->rotation;
> > + state->rotation = val;
> > ret = intel_plane_restore(plane);
> > if (ret)
> > - intel_plane->rotation = old_val;
> > + state->rotation = old_val;
> > }
> >
> > return ret;
> > @@ -1457,6 +1462,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;
> > @@ -1475,6 +1481,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> > kfree(intel_plane);
> > return -ENOMEM;
> > }
> > + state = to_intel_plane_state(intel_plane->base.state);
> >
> > switch (INTEL_INFO(dev)->gen) {
> > case 5:
> > @@ -1545,7 +1552,7 @@ 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);
> > + state->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 +1574,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->rotation);
> >
> > drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> >
> > --
> > 1.8.5.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
More information about the Intel-gfx
mailing list