[Intel-gfx] [PATCH] drm/i915: Prepare for atomic plane helpers (v7)
Ander Conselvan de Oliveira
conselvan2 at gmail.com
Fri Dec 12 04:52:39 PST 2014
On 12/12/2014 01:54 AM, 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)
>
> v3:
> - Use prepare/cleanup functions that we've already factored out
> - Use newly refactored pre_commit/commit/post_commit to avoid sleeping
> during vblank evasion
>
> v4:
> - Rebase to latest di-nightly requires adding an 'old_state' parameter
> to atomic_update;
>
> v5:
> - Must have botched a rebase somewhere and lost some work. Restore
> state 'dirty' flag to let begin/end code know which planes to
> run the pre_commit/post_commit hooks for. This would have actually
> shown up as broken in the next commit rather than this one.
>
> v6:
> - Squash kerneldoc patch into this one.
> - Previous patches have now already taken care of most of the
> infrastructure that used to be in this patch. All we're adding here
> now is some thin wrappers.
>
> v7:
> - Check return of intel_plane_duplicate_state() for allocation
> failures.
>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
> Documentation/DocBook/drm.tmpl | 5 +
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 150 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 32 ++++++-
> drivers/gpu/drm/i915/intel_drv.h | 8 ++
> drivers/gpu/drm/i915/intel_sprite.c | 12 +++
> 6 files changed, 207 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 25c23ca..7c5207b 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -3932,6 +3932,11 @@ int num_ioctls;</synopsis>
> </para>
> </sect2>
> <sect2>
> + <title>Atomic Plane Helpers</title>
> +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers
> +!Idrivers/gpu/drm/i915/intel_atomic_plane.c
> + </sect2>
> + <sect2>
> <title>Output Probing</title>
> <para>
> This section covers output probing and related infrastructure like the
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3cf70a6..4a6ced7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -65,6 +65,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..286fec8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -0,0 +1,150 @@
> +/*
> + * 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.
> + */
> +
> +/**
> + * DOC: atomic plane helper support
> + *
> + * The functions here are used by the atomic plane helper functions to
> + * implement legacy plane updates (i.e., drm_plane->update_plane() and
> + * drm_plane->disable_plane()). This allows plane updates to use the
> + * atomic state infrastructure and perform plane updates as separate
> + * prepare/check/commit/cleanup steps.
> + */
> +
> +#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.
> + *
> + * Returns: The newly allocated plane state, or NULL or failure.
> + */
> +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)
> + return NULL;
> +
> + if (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);
> +}
> +
> +static int intel_plane_atomic_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + struct drm_crtc *crtc = state->crtc;
> + struct intel_crtc *intel_crtc;
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> + crtc = crtc ? crtc : plane->crtc;
> + intel_crtc = to_intel_crtc(crtc);
> +
> + /*
> + * 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 */
> + 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;
> +
> + /*
> + * Disabling a plane is always okay; we just need to update
> + * fb tracking in a special way since cleanup_fb() won't
> + * get called by the plane helpers.
> + */
> + if (state->fb == NULL && plane->state->fb != NULL) {
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking as a special case
> + */
> + intel_crtc->atomic.track_fbs |= (1 << drm_plane_index(plane));
> + }
> +
> + return intel_plane->check_plane(plane, intel_state);
> +}
> +
> +static void intel_plane_atomic_update(struct drm_plane *plane,
> + struct drm_plane_state *old_state)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct intel_plane_state *intel_state =
> + to_intel_plane_state(plane->state);
> +
> + /* Don't disable an already disabled plane */
> + if (!plane->state->fb && !old_state->fb)
> + return;
> +
> + intel_plane->commit_plane(plane, intel_state);
> +}
> +
> +const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> + .prepare_fb = intel_prepare_plane_fb,
> + .cleanup_fb = intel_cleanup_plane_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 030cf93..f839115 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -98,6 +98,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
> const struct intel_crtc_config *pipe_config);
> static void chv_prepare_pll(struct intel_crtc *crtc,
> const struct intel_crtc_config *pipe_config);
> +static void intel_begin_crtc_commit(struct drm_crtc *crtc);
> +static void intel_finish_crtc_commit(struct drm_crtc *crtc);
>
> static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
> {
> @@ -9794,6 +9796,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_begin_crtc_commit,
> + .atomic_flush = intel_finish_crtc_commit,
> };
>
> /**
> @@ -12011,6 +12015,7 @@ intel_disable_plane(struct drm_plane *plane)
> 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);
> }
> @@ -12019,13 +12024,17 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = {
> .update_plane = intel_update_plane,
> .disable_plane = intel_disable_plane,
> .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,
> int pipe)
> {
> struct intel_plane *primary;
> + struct intel_plane_state *state;
> const uint32_t *intel_primary_formats;
> int num_formats;
>
> @@ -12033,6 +12042,13 @@ 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) {
> + kfree(primary);
> + return NULL;
> + }
> +
> + state = to_intel_plane_state(primary->base.state);
You set the value of state, but then that variable is never used in that
function.
> primary->can_scale = false;
> primary->max_downscale = 1;
> primary->pipe = pipe;
> @@ -12068,6 +12084,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;
> }
>
> @@ -12188,17 +12206,27 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> .disable_plane = intel_disable_plane,
> .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,
> 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) {
> + kfree(cursor);
> + return NULL;
> + }
> +
> + state = to_intel_plane_state(cursor->base.state);
Same here.
> cursor->can_scale = false;
> cursor->max_downscale = 1;
> cursor->pipe = pipe;
> @@ -12225,6 +12253,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 2523315..7bd8ad9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -437,6 +437,7 @@ struct intel_crtc_atomic_commit {
> bool disable_fbc;
> bool pre_disable_primary;
> bool update_wm;
> + unsigned track_fbs;
>
> /* Sleepable operations to perform after commit */
> unsigned fb_bits;
> @@ -575,6 +576,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 {
> @@ -1253,4 +1255,10 @@ void intel_pre_disable_primary(struct drm_crtc *crtc);
> /* 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);
> +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 14c0b88..df41c89 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"
> @@ -1390,6 +1391,8 @@ static const struct drm_plane_funcs intel_plane_funcs = {
Not necessarily something that needs to be done with this series, but
would be nice to rename this struct to intel_sprite_plane_funcs. That
would match better with intel_primary_plane_funcs and
intel_cursor_plane_funcs.
Ander
> .disable_plane = intel_disable_plane,
> .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[] = {
> @@ -1451,6 +1454,13 @@ 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) {
> + kfree(intel_plane);
> + return -ENOMEM;
> + }
> +
> switch (INTEL_INFO(dev)->gen) {
> case 5:
> case 6:
> @@ -1544,6 +1554,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 Intel-gfx
mailing list