Reviewed-by: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>
> ---
>   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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to