Re: [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2)

On Thu, Nov 13, 2014 at 02:51:32PM -0800, Matt Roper 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.

v2:

• Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel)
• Fix a copy/paste comment mistake (Bob)

Signed-off-by: Matt Roper matthew.d.roper@intel.com

Actually, I don't think this is quite ready to go yet. I must have had too much debug spam turned on before and missed the important messages in the kernel log, but it looks like we're still calling something that can sleep (in the frontbuffer tracking code?) while irqs are disabled for vblank evasion.

I'll have to look into this some more tomorrow and decide what an appropriate fix is.

Matt

---

drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.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_plane.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 891e584..9f5c57d 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_plane.o \
  

  intel_crt.o \ intel_ddi.o \ intel_dp.o \

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c new file mode 100644 index 0000000..7c039bf --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic_plane.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
• • Destroys the plane state (both common and Intel-specific) for the
• • specified plane.
• */

+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;
  

• }
• 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;

}

1.8.5.1