[PATCH 5/7] drm/i915: Prepare for atomic plane helpers

Bob Paauwe bob.j.paauwe at intel.com
Thu Nov 13 11:46:25 PST 2014


On Thu, 13 Nov 2014 10:43:24 -0800
Matt Roper <matthew.d.roper at intel.com> wrote:

> Add the new driver entrypoints that will be called by the atomic plane
> helpers.
> 
> This patch does not actually switch over to the new plane helpers yet,
> so there should be no functional change here.  Also note that although
> plane programming was already split into check/prepare/commit steps,
> some of the semantics of those individual functions will need to change
> slightly when we do make the jump so that they match the behavior the
> plane helpers expect.
> 
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile        |   1 +
>  drivers/gpu/drm/i915/intel_atomic.c  | 220 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  20 +++-
>  drivers/gpu/drm/i915/intel_drv.h     |  15 +++
>  drivers/gpu/drm/i915/intel_sprite.c  |  10 ++
>  5 files changed, 265 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_atomic.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 891e584..8b67617 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -63,6 +63,7 @@ i915-y += dvo_ch7017.o \
>  	  dvo_ns2501.o \
>  	  dvo_sil164.o \
>  	  dvo_tfp410.o \
> +	  intel_atomic.o \
>  	  intel_crt.o \
>  	  intel_ddi.o \
>  	  intel_dp.o \
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> new file mode 100644
> index 0000000..2e9b56e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -0,0 +1,220 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_plane_helper.h>
> +#include "intel_drv.h"
> +
> +/**
> + * 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.
> + */
> +struct drm_plane_state *
> +intel_plane_duplicate_state(struct drm_plane *plane)
> +{
> +	struct intel_plane_state *state;
> +
> +	if (plane->state)
> +		state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL);
> +	else
> +		state = kzalloc(sizeof(*state), GFP_KERNEL);
> +
> +	if (state && state->base.fb)
> +		drm_framebuffer_reference(state->base.fb);
> +
> +	return &state->base;
> +}
> +
> +/**
> + * intel_plane_destroy_state - destroy plane state
> + * @plane: drm plane
> + *
> + * Allocates and returns a copy of the plane state (both common and
> + * Intel-specific) for the specified plane.

Comment should be updated for destroy.

> + */
> +void
> +intel_plane_destroy_state(struct drm_plane *plane,
> +			  struct drm_plane_state *state)
> +{
> +	drm_atomic_helper_plane_destroy_state(plane, state);
> +}
> +
> +
> +/**
> + * intel_crtc_atomic_begin - Begins an atomic commit on a CRTC
> + * @crtc: drm crtc
> + *
> + * Prepares to write registers associated with the atomic commit of a CRTC
> + * by using vblank evasion to ensure that all register writes happen within
> + * the same vblank period.
> + */
> +void intel_crtc_atomic_begin(struct drm_crtc *crtc)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	intel_pipe_update_start(intel_crtc, &intel_crtc->atomic_vbl_count);
> +}
> +
> +/**
> + * intel_crtc_atomic_flush - Finishes an atomic commit on a CRTC
> + * @crtc: drm crtc
> + *
> + * Concludes the writing of registers for an atomic commit of a CRTC.
> + */
> +void intel_crtc_atomic_flush(struct drm_crtc *crtc)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	intel_pipe_update_end(intel_crtc, intel_crtc->atomic_vbl_count);
> +}
> +
> +static int intel_prepare_fb(struct drm_plane *plane,
> +			    struct drm_framebuffer *fb)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +	struct drm_i915_gem_object *old_obj = intel_plane->obj;
> +	enum pipe pipe = intel_plane->pipe;
> +	unsigned front_bits = 0;
> +	int ret = 0;
> +
> +	switch (plane->type) {
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		front_bits = INTEL_FRONTBUFFER_PRIMARY(pipe);
> +
> +		if (plane->crtc) {
> +			intel_crtc_wait_for_pending_flips(plane->crtc);
> +			if (intel_crtc_has_pending_flip(plane->crtc)) {
> +				DRM_ERROR("pipe is still busy with an old pageflip\n");
> +				return -EBUSY;
> +			}
> +		}
> +
> +		break;
> +	case DRM_PLANE_TYPE_OVERLAY:
> +		front_bits = INTEL_FRONTBUFFER_SPRITE(pipe);
> +		break;
> +	case DRM_PLANE_TYPE_CURSOR:
> +		front_bits = INTEL_FRONTBUFFER_CURSOR(pipe);
> +		break;
> +	}
> +
> +	mutex_lock(&dev->struct_mutex);
> +
> +	/* Note that this will apply the VT-d workaround for scanouts,
> +	 * which is more restrictive than required for sprites. (The
> +	 * primary plane requires 256KiB alignment with 64 PTE padding,
> +	 * the sprite planes only require 128KiB alignment and 32 PTE
> +	 * padding.
> +	 */
> +	ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
> +	if (ret == 0)
> +		i915_gem_track_fb(old_obj, obj, front_bits);
> +
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> +	    INTEL_INFO(dev)->cursor_needs_physical) {
> +		int align = IS_I830(dev) ? 16 * 1024 : 256;
> +
> +		ret = i915_gem_object_attach_phys(obj, align);
> +		if (ret)
> +			DRM_DEBUG_KMS("failed to attach phys object\n");
> +	}
> +
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	return ret;
> +}
> +
> +static void intel_cleanup_fb(struct drm_plane *plane,
> +			     struct drm_framebuffer *fb)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +
> +	mutex_lock(&dev->struct_mutex);
> +	intel_unpin_fb_obj(obj);
> +	mutex_unlock(&dev->struct_mutex);
> +}
> +
> +static int intel_plane_atomic_check(struct drm_plane *plane,
> +				    struct drm_plane_state *state)
> +{
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
> +	struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> +	/* Disabling a plane is always okay */
> +	if (state->fb == NULL)
> +		return 0;
> +
> +	/*
> +	 * The original src/dest coordinates are stored in state->base, but
> +	 * we want to keep another copy internal to our driver that we can
> +	 * clip/modify ourselves.
> +	 */
> +	intel_state->src.x1 = state->src_x;
> +	intel_state->src.y1 = state->src_y;
> +	intel_state->src.x2 = state->src_x + state->src_w;
> +	intel_state->src.y2 = state->src_y + state->src_h;
> +	intel_state->dst.x1 = state->crtc_x;
> +	intel_state->dst.y1 = state->crtc_y;
> +	intel_state->dst.x2 = state->crtc_x + state->crtc_w;
> +	intel_state->dst.y2 = state->crtc_y + state->crtc_h;
> +
> +	/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
> +	if (intel_crtc) {
> +		intel_state->clip.x1 = 0;
> +		intel_state->clip.y1 = 0;
> +		intel_state->clip.x2 =
> +			intel_crtc->active ? intel_crtc->config.pipe_src_w : 0;
> +		intel_state->clip.y2 =
> +			intel_crtc->active ? intel_crtc->config.pipe_src_h : 0;

Should this be using intel_crtc->new_config here?  I'm not entierly
clear on how the crtc config/new_config would tie into the atomic state.

> +	}
> +
> +	return intel_plane->check_plane(plane, intel_state);
> +}
> +
> +static void intel_plane_atomic_update(struct drm_plane *plane)
> +{
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_plane_state *intel_state =
> +		to_intel_plane_state(plane->state);
> +
> +	if (!plane->state->fb)
> +		intel_plane_disable(plane);
> +	else
> +		intel_plane->commit_plane(plane, intel_state);
> +}
> +
> +const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> +	.prepare_fb = intel_prepare_fb,
> +	.cleanup_fb = intel_cleanup_fb,
> +	.atomic_check = intel_plane_atomic_check,
> +	.atomic_update = intel_plane_atomic_update,
> +};
> +
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2d4ef04..32a2b12 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9705,6 +9705,8 @@ out_hang:
>  static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
>  	.load_lut = intel_crtc_load_lut,
> +	.atomic_begin = intel_crtc_atomic_begin,
> +	.atomic_flush = intel_crtc_atomic_flush,
>  };
>  
>  /**
> @@ -11747,6 +11749,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  void intel_plane_destroy(struct drm_plane *plane)
>  {
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	intel_plane_destroy_state(plane, plane->state);
>  	drm_plane_cleanup(plane);
>  	kfree(intel_plane);
>  }
> @@ -11755,7 +11758,10 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = {
>  	.update_plane = intel_primary_plane_setplane,
>  	.disable_plane = intel_primary_plane_disable,
>  	.destroy = intel_plane_destroy,
> -	.set_property = intel_plane_set_property
> +	.set_property = intel_plane_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
> +
>  };
>  
>  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> @@ -11769,11 +11775,14 @@ 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);
>  	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)
>  		primary->plane = !pipe;
>  
> @@ -11802,6 +11811,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  				primary->rotation);
>  	}
>  
> +	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
> +
>  	return &primary->base;
>  }
>  
> @@ -11957,6 +11968,8 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
>  	.disable_plane = intel_cursor_plane_disable,
>  	.destroy = intel_plane_destroy,
>  	.set_property = intel_plane_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
>  };
>  
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> @@ -11968,11 +11981,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	if (cursor == NULL)
>  		return NULL;
>  
> +	cursor->base.state = intel_plane_duplicate_state(&cursor->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;
>  
>  	drm_universal_plane_init(dev, &cursor->base, 0,
>  				 &intel_cursor_plane_funcs,
> @@ -11992,6 +12008,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  				cursor->rotation);
>  	}
>  
> +	drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs);
> +
>  	return &cursor->base;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 49358c8..1882de1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -466,6 +466,8 @@ struct intel_crtc {
>  
>  	int scanline_offset;
>  	struct intel_mmio_flip mmio_flip;
> +
> +	uint32_t atomic_vbl_count;
>  };
>  
>  struct intel_plane_wm_parameters {
> @@ -506,6 +508,10 @@ struct intel_plane {
>  			     uint32_t src_w, uint32_t src_h);
>  	void (*disable_plane)(struct drm_plane *plane,
>  			      struct drm_crtc *crtc);
> +	int (*check_plane)(struct drm_plane *plane,
> +			   struct intel_plane_state *state);
> +	void (*commit_plane)(struct drm_plane *plane,
> +			     struct intel_plane_state *state);
>  	int (*update_colorkey)(struct drm_plane *plane,
>  			       struct drm_intel_sprite_colorkey *key);
>  	void (*get_colorkey)(struct drm_plane *plane,
> @@ -536,6 +542,7 @@ struct cxsr_latency {
>  #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
>  #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
>  #define to_intel_plane(x) container_of(x, struct intel_plane, base)
> +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base)
>  #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
>  
>  struct intel_hdmi {
> @@ -1195,4 +1202,12 @@ int intel_sprite_plane_disable(struct drm_plane *plane);
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_device *dev);
>  
> +/* intel_atomic.c */
> +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);
> +void intel_crtc_atomic_begin(struct drm_crtc *crtc);
> +void intel_crtc_atomic_flush(struct drm_crtc *crtc);
> +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
> +
>  #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9e6a72a..d3677c3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -33,6 +33,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_rect.h>
> +#include <drm/drm_plane_helper.h>
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> @@ -1573,6 +1574,8 @@ static const struct drm_plane_funcs intel_plane_funcs = {
>  	.disable_plane = intel_sprite_plane_disable,
>  	.destroy = intel_plane_destroy,
>  	.set_property = intel_plane_set_property,
> +	.atomic_duplicate_state = intel_plane_duplicate_state,
> +	.atomic_destroy_state = intel_plane_destroy_state,
>  };
>  
>  static uint32_t ilk_plane_formats[] = {
> @@ -1634,6 +1637,9 @@ 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);
> +
>  	switch (INTEL_INFO(dev)->gen) {
>  	case 5:
>  	case 6:
> @@ -1704,6 +1710,8 @@ 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);
>  	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
>  				       &intel_plane_funcs,
> @@ -1725,6 +1733,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  					   dev->mode_config.rotation_property,
>  					   intel_plane->rotation);
>  
> +	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> +
>   out:
>  	return ret;
>  }



More information about the dri-devel mailing list