From acf24a395c5a9290189b080383564437101d411c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 29 Jul 2014 15:33:05 +0200 Subject: drm/plane-helper: transitional atomic plane helpers Converting a driver to the atomic interface can be a daunting undertaking. One of the prerequisites is to have full universal planes support. To make that transition a bit easier this patch provides plane helpers which use the new atomic helper callbacks just only for the plane changes. This way the plane update functionality can be tested without being forced to convert everything at once. Of course a real atomic update capable driver will implement the all plane properties through the atomic interface, so these helpers are mostly transitional. But they can be used to enable proper universal plane support, especially once the crtc helpers have also been adapted. v2: Use ->atomic_duplicate_state if available. v3: Don't forget to call ->atomic_destroy_state if available. v4: Fixup kerneldoc, reported by Paulo. v5: Extract a common plane_commit helper and fix some bugs in the plane_state setup of the plane_disable implementation. v6: Fix issues with the cleanup of the old fb. Since transitional helpers can be mixed we need to assume that the old fb has been set up by a legacy path (e.g. set_config or page_flip when the primary plane is converted to use these functions already). Hence pass an additional old_fb parameter to plane_commit to do that cleanup work correctly. v7: - Fix spurious WARNING (crtc helpers really love to disable stuff harder) and fix array index bonghits. - Correctly handle the lack of plane->state object, necessary for transitional use. - Don't indicate failure if drm_vblank_get doesn't work - that's expected when the pipe is in dpms off mode. v8: Review from Sean: - s/fail/out/ to make the meaning of a label more clear. - spelling fix in the commit message. Cc: Paulo Zanoni Cc: Sean Paul Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_plane_helper.c | 172 ++++++++++++++++++++++++++++++++++++- 1 file changed, 171 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_plane_helper.c') diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 827ec1a3040b..a202b89e14eb 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -27,7 +27,7 @@ #include #include #include -#include +#include #define SUBPIXEL_MASK 0xffff @@ -369,3 +369,173 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs); } EXPORT_SYMBOL(drm_crtc_init); + +static int +plane_commit(struct drm_plane *plane, struct drm_plane_state *plane_state, + struct drm_framebuffer *old_fb) +{ + struct drm_plane_helper_funcs *plane_funcs; + struct drm_crtc *crtc[2]; + struct drm_crtc_helper_funcs *crtc_funcs[2]; + int i, ret = 0; + + plane_funcs = plane->helper_private; + + /* Since this is a transitional helper we can't assume that plane->state + * is always valid. Hence we need to use plane->crtc instead of + * plane->state->crtc as the old crtc. */ + crtc[0] = plane->crtc; + crtc[1] = crtc[0] != plane_state->crtc ? plane_state->crtc : NULL; + + for (i = 0; i < 2; i++) + crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL; + + if (plane_funcs->atomic_check) { + ret = plane_funcs->atomic_check(plane, plane_state); + if (ret) + goto out; + } + + if (plane_funcs->prepare_fb && plane_state->fb) { + ret = plane_funcs->prepare_fb(plane, plane_state->fb); + if (ret) + goto out; + } + + /* Point of no return, commit sw state. */ + swap(plane->state, plane_state); + + for (i = 0; i < 2; i++) { + if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin) + crtc_funcs[i]->atomic_begin(crtc[i]); + } + + plane_funcs->atomic_update(plane); + + for (i = 0; i < 2; i++) { + if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush) + crtc_funcs[i]->atomic_flush(crtc[i]); + } + + for (i = 0; i < 2; i++) { + if (!crtc[i]) + continue; + + /* There's no other way to figure out whether the crtc is running. */ + ret = drm_crtc_vblank_get(crtc[i]); + if (ret == 0) { + drm_crtc_wait_one_vblank(crtc[i]); + drm_crtc_vblank_put(crtc[i]); + } + + ret = 0; + } + + if (plane_funcs->cleanup_fb && old_fb) + plane_funcs->cleanup_fb(plane, old_fb); +out: + if (plane_state) { + if (plane->funcs->atomic_destroy_state) + plane->funcs->atomic_destroy_state(plane, plane_state); + else + kfree(plane_state); + } + + return ret; +} + +/** + * drm_plane_helper_update() - Helper for primary plane update + * @plane: plane object to update + * @crtc: owning CRTC of owning plane + * @fb: framebuffer to flip onto plane + * @crtc_x: x offset of primary plane on crtc + * @crtc_y: y offset of primary plane on crtc + * @crtc_w: width of primary plane rectangle on crtc + * @crtc_h: height of primary plane rectangle on crtc + * @src_x: x offset of @fb for panning + * @src_y: y offset of @fb for panning + * @src_w: width of source rectangle in @fb + * @src_h: height of source rectangle in @fb + * + * Provides a default plane update handler using the atomic plane update + * functions. It is fully left to the driver to check plane constraints and + * handle corner-cases like a fully occluded or otherwise invisible plane. + * + * This is useful for piecewise transitioning of a driver to the atomic helpers. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_plane_state *plane_state; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + plane_state = kmemdup(plane->state, sizeof(*plane_state), + GFP_KERNEL); + else + plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); + if (!plane_state) + return -ENOMEM; + + plane_state->crtc = crtc; + plane_state->fb = fb; + plane_state->crtc_x = crtc_x; + plane_state->crtc_y = crtc_y; + plane_state->crtc_h = crtc_h; + plane_state->crtc_w = crtc_w; + plane_state->src_x = src_x; + plane_state->src_y = src_y; + plane_state->src_h = src_h; + plane_state->src_w = src_w; + + return plane_commit(plane, plane_state, plane->fb); +} +EXPORT_SYMBOL(drm_plane_helper_update); + +/** + * drm_plane_helper_disable() - Helper for primary plane disable + * @plane: plane to disable + * + * Provides a default plane disable handler using the atomic plane update + * functions. It is fully left to the driver to check plane constraints and + * handle corner-cases like a fully occluded or otherwise invisible plane. + * + * This is useful for piecewise transitioning of a driver to the atomic helpers. + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_plane_helper_disable(struct drm_plane *plane) +{ + struct drm_plane_state *plane_state; + + /* crtc helpers love to call disable functions for already disabled hw + * functions. So cope with that. */ + if (!plane->crtc) + return 0; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + plane_state = kmemdup(plane->state, sizeof(*plane_state), + GFP_KERNEL); + else + plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); + if (!plane_state) + return -ENOMEM; + + plane_state->crtc = NULL; + plane_state->fb = NULL; + + return plane_commit(plane, plane_state, plane->fb); +} +EXPORT_SYMBOL(drm_plane_helper_disable); -- cgit v1.2.3 From 2f324b42b7b24a48fe3f8a7af60ec3c9024255fa Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 29 Oct 2014 11:13:47 +0100 Subject: drm/crtc-helper: Transitional functions using atomic plane helpers These two functions allow drivers to reuse their atomic plane helpers functions for the primary plane to implement the interfaces required by the crtc helpers for the legacy ->set_config callback. This is purely transitional and won't be used once the driver is fully converted. But it allows partial conversions to the atomic plane helpers which are functional. v2: - Use ->atomic_duplicate_state if available. - Don't forget to run crtc_funcs->atomic_check. v3: Shift source coordinates correctly for 16.16 fixed point. v4: Don't forget to call ->atomic_destroy_state if available. v5: Fixup kerneldoc. v6: Reuse the plane_commit function from the transitional plane helpers to avoid too much duplication. v7: - Remove some stale comment. - Correctly handle the lack of plane->state object, necessary for transitional use. v8: Fixup an embarrassing h/vdisplay mixup. Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c | 110 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_plane_helper.c | 10 ++-- include/drm/drm_crtc.h | 4 ++ include/drm/drm_crtc_helper.h | 7 +++ include/drm/drm_plane_helper.h | 4 ++ 5 files changed, 130 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/drm_plane_helper.c') diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 6c65a0a28fbd..95ecbb131053 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -38,6 +38,7 @@ #include #include #include +#include #include MODULE_AUTHOR("David Airlie, Jesse Barnes"); @@ -888,3 +889,112 @@ void drm_helper_resume_force_mode(struct drm_device *dev) drm_modeset_unlock_all(dev); } EXPORT_SYMBOL(drm_helper_resume_force_mode); + +/** + * drm_helper_crtc_mode_set - mode_set implementation for atomic plane helpers + * @crtc: DRM CRTC + * @mode: DRM display mode which userspace requested + * @adjusted_mode: DRM display mode adjusted by ->mode_fixup callbacks + * @x: x offset of the CRTC scanout area on the underlying framebuffer + * @y: y offset of the CRTC scanout area on the underlying framebuffer + * @old_fb: previous framebuffer + * + * This function implements a callback useable as the ->mode_set callback + * required by the crtc helpers. Besides the atomic plane helper functions for + * the primary plane the driver must also provide the ->mode_set_nofb callback + * to set up the crtc. + * + * This is a transitional helper useful for converting drivers to the atomic + * interfaces. + */ +int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode, int x, int y, + struct drm_framebuffer *old_fb) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + int ret; + + if (crtc->funcs->atomic_duplicate_state) + crtc_state = crtc->funcs->atomic_duplicate_state(crtc); + else if (crtc->state) + crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), + GFP_KERNEL); + else + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL); + if (!crtc_state) + return -ENOMEM; + + crtc_state->enable = true; + crtc_state->planes_changed = true; + drm_mode_copy(&crtc_state->mode, mode); + drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode); + + if (crtc_funcs->atomic_check) { + ret = crtc_funcs->atomic_check(crtc, crtc_state); + if (ret) { + kfree(crtc_state); + + return ret; + } + } + + swap(crtc->state, crtc_state); + + crtc_funcs->mode_set_nofb(crtc); + + if (crtc_state) { + if (crtc->funcs->atomic_destroy_state) + crtc->funcs->atomic_destroy_state(crtc, crtc_state); + else + kfree(crtc_state); + } + + return drm_helper_crtc_mode_set_base(crtc, x, y, old_fb); +} +EXPORT_SYMBOL(drm_helper_crtc_mode_set); + +/** + * drm_helper_crtc_mode_set_base - mode_set_base implementation for atomic plane helpers + * @crtc: DRM CRTC + * @x: x offset of the CRTC scanout area on the underlying framebuffer + * @y: y offset of the CRTC scanout area on the underlying framebuffer + * @old_fb: previous framebuffer + * + * This function implements a callback useable as the ->mode_set_base used + * required by the crtc helpers. The driver must provide the atomic plane helper + * functions for the primary plane. + * + * This is a transitional helper useful for converting drivers to the atomic + * interfaces. + */ +int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, + struct drm_framebuffer *old_fb) +{ + struct drm_plane_state *plane_state; + struct drm_plane *plane = crtc->primary; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + plane_state = kmemdup(plane->state, sizeof(*plane_state), + GFP_KERNEL); + else + plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); + if (!plane_state) + return -ENOMEM; + + plane_state->crtc = crtc; + plane_state->fb = crtc->primary->fb; + plane_state->crtc_x = 0; + plane_state->crtc_y = 0; + plane_state->crtc_h = crtc->mode.vdisplay; + plane_state->crtc_w = crtc->mode.hdisplay; + plane_state->src_x = x << 16; + plane_state->src_y = y << 16; + plane_state->src_h = crtc->mode.vdisplay << 16; + plane_state->src_w = crtc->mode.hdisplay << 16; + + return drm_plane_helper_commit(plane, plane_state, old_fb); +} +EXPORT_SYMBOL(drm_helper_crtc_mode_set_base); diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index a202b89e14eb..a5a295872fbb 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -370,9 +370,9 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, } EXPORT_SYMBOL(drm_crtc_init); -static int -plane_commit(struct drm_plane *plane, struct drm_plane_state *plane_state, - struct drm_framebuffer *old_fb) +int drm_plane_helper_commit(struct drm_plane *plane, + struct drm_plane_state *plane_state, + struct drm_framebuffer *old_fb) { struct drm_plane_helper_funcs *plane_funcs; struct drm_crtc *crtc[2]; @@ -497,7 +497,7 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, plane_state->src_h = src_h; plane_state->src_w = src_w; - return plane_commit(plane, plane_state, plane->fb); + return drm_plane_helper_commit(plane, plane_state, plane->fb); } EXPORT_SYMBOL(drm_plane_helper_update); @@ -536,6 +536,6 @@ int drm_plane_helper_disable(struct drm_plane *plane) plane_state->crtc = NULL; plane_state->fb = NULL; - return plane_commit(plane, plane_state, plane->fb); + return drm_plane_helper_commit(plane, plane_state, plane->fb); } EXPORT_SYMBOL(drm_plane_helper_disable); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 56147409408d..53c8638592d4 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -230,6 +230,7 @@ struct drm_atomic_state; * struct drm_crtc_state - mutable CRTC state * @enable: whether the CRTC should be enabled, gates all other state * @planes_changed: for use by helpers and drivers when computing state updates + * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings * @mode: current mode timings * @event: optional pointer to a DRM event to signal upon completion of the * state update @@ -241,6 +242,9 @@ struct drm_crtc_state { /* computed state bits used by helpers and drivers */ bool planes_changed : 1; + /* adjusted_mode: for use by helpers and drivers */ + struct drm_display_mode adjusted_mode; + struct drm_display_mode mode; struct drm_pending_vblank_event *event; diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index adec48b27aa5..7adbb65ea8ae 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -68,6 +68,7 @@ struct drm_crtc_helper_funcs { int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, int x, int y, struct drm_framebuffer *old_fb); + void (*mode_set_nofb)(struct drm_crtc *crtc); /* Move the crtc on the current fb to the given position *optional* */ int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, @@ -167,6 +168,12 @@ static inline void drm_connector_helper_add(struct drm_connector *connector, extern void drm_helper_resume_force_mode(struct drm_device *dev); +int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode, int x, int y, + struct drm_framebuffer *old_fb); +int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, + struct drm_framebuffer *old_fb); + /* drm_probe_helper.c */ extern int drm_helper_probe_single_connector_modes(struct drm_connector *connector, uint32_t maxX, diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index 92bceccaa62a..c48f14d88690 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -103,4 +103,8 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_w, uint32_t src_h); int drm_plane_helper_disable(struct drm_plane *plane); +/* For use by drm_crtc_helper.c */ +int drm_plane_helper_commit(struct drm_plane *plane, + struct drm_plane_state *plane_state, + struct drm_framebuffer *old_fb); #endif -- cgit v1.2.3 From 3150c7d0c686ffee9e414ccce705c34ebcbf4e30 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 6 Nov 2014 20:53:29 +0100 Subject: drm: Docbook integration and over sections for all the new helpers In all cases the text requires that new drivers are converted to the atomic interfaces. v2: Add overview for state handling. v3: Review from Sean: Some spelling fixes and drop the misguided hunk to remove rgba8888 from the plane helpers compat list. Cc: Sean Paul Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 20 +++++++++++++++++++- drivers/gpu/drm/drm_atomic_helper.c | 36 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 20 ++++++++++++++++++++ drivers/gpu/drm/drm_plane_helper.c | 26 ++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_plane_helper.c') diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 748513b34025..f276c2cf806b 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2323,9 +2323,26 @@ void intel_crt_init(struct drm_device *dev) + + Atomic Modeset Helper Functions Reference + + Overview +!Pdrivers/gpu/drm/drm_atomic_helper.c overview + + + Implementing Asynchronous Atomic Commit +!Pdrivers/gpu/drm/drm_atomic_helper.c implementing async commit + + + Atomic State Reset and Initialization +!Pdrivers/gpu/drm/drm_atomic_helper.c atomic state reset and initialization + +!Edrivers/gpu/drm/drm_atomic_helper.c + Modeset Helper Functions Reference !Edrivers/gpu/drm/drm_crtc_helper.c +!Pdrivers/gpu/drm/drm_crtc_helper.c overview Output Probing Helper Functions Reference @@ -2379,7 +2396,8 @@ void intel_crt_init(struct drm_device *dev) Plane Helper Reference -!Edrivers/gpu/drm/drm_plane_helper.c Plane Helpers +!Edrivers/gpu/drm/drm_plane_helper.c +!Pdrivers/gpu/drm/drm_plane_helper.c overview diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2c38bda217ec..2b1db0c12fdc 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -32,6 +32,27 @@ #include #include +/** + * DOC: overview + * + * This helper library provides implementations of check and commit functions on + * top of the CRTC modeset helper callbacks and the plane helper callbacks. It + * also provides convenience implementations for the atomic state handling + * callbacks for drivers which don't need to subclass the drm core structures to + * add their own additional internal state. + * + * This library also provides default implementations for the check callback in + * drm_atomic_helper_check and for the commit callback with + * drm_atomic_helper_commit. But the individual stages and callbacks are expose + * to allow drivers to mix and match and e.g. use the plane helpers only + * together with a driver private modeset implementation. + * + * This library also provides implementations for all the legacy driver + * interfaces on top of the atomic interface. See drm_atomic_helper_set_config, + * drm_atomic_helper_disable_plane, drm_atomic_helper_disable_plane and the + * various functions to implement set_property callbacks. New drivers must not + * implement these functions themselves but must use the provided helpers. + */ static void drm_atomic_helper_plane_changed(struct drm_atomic_state *state, struct drm_plane_state *plane_state, @@ -1707,6 +1728,21 @@ backoff: } EXPORT_SYMBOL(drm_atomic_helper_page_flip); +/** + * DOC: atomic state reset and initialization + * + * Both the drm core and the atomic helpers assume that there is always the full + * and correct atomic software state for all connectors, CRTCs and planes + * available. Which is a bit a problem on driver load and also after system + * suspend. One way to solve this is to have a hardware state read-out + * infrastructure which reconstructs the full software state (e.g. the i915 + * driver). + * + * The simpler solution is to just reset the software state to everything off, + * which is easiest to do by calling drm_mode_config_reset(). To facilitate this + * the atomic helpers provide default reset implementations for all hooks. + */ + /** * drm_atomic_helper_crtc_reset - default ->reset hook for CRTCs * @crtc: drm CRTC diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 46728a8ac622..33195e9adaab 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -41,6 +41,26 @@ #include #include +/** + * DOC: overview + * + * The CRTC modeset helper library provides a default set_config implementation + * in drm_crtc_helper_set_config(). Plus a few other convenience functions using + * the same callbacks which drivers can use to e.g. restore the modeset + * configuration on resume with drm_helper_resume_force_mode(). + * + * The driver callbacks are mostly compatible with the atomic modeset helpers, + * except for the handling of the primary plane: Atomic helpers require that the + * primary plane is implemented as a real standalone plane and not directly tied + * to the CRTC state. For easier transition this library provides functions to + * implement the old semantics required by the CRTC helpers using the new plane + * and atomic helper callbacks. + * + * Drivers are strongly urged to convert to the atomic helpers (by way of first + * converting to the plane helpers). New drivers must not use these functions + * but need to implement the atomic interface instead, potentially using the + * atomic helpers for that. + */ MODULE_AUTHOR("David Airlie, Jesse Barnes"); MODULE_DESCRIPTION("DRM KMS helper"); MODULE_LICENSE("GPL and additional rights"); diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index a5a295872fbb..fa56bb5da6c3 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -31,6 +31,32 @@ #define SUBPIXEL_MASK 0xffff +/** + * DOC: overview + * + * This helper library has two parts. The first part has support to implement + * primary plane support on top of the normal CRTC configuration interface. + * Since the legacy ->set_config interface ties the primary plane together with + * the CRTC state this does not allow userspace to disable the primary plane + * itself. To avoid too much duplicated code use + * drm_plane_helper_check_update() which can be used to enforce the same + * restrictions as primary planes had thus. The default primary plane only + * expose XRBG8888 and ARGB8888 as valid pixel formats for the attached + * framebuffer. + * + * Drivers are highly recommended to implement proper support for primary + * planes, and newly merged drivers must not rely upon these transitional + * helpers. + * + * The second part also implements transitional helpers which allow drivers to + * gradually switch to the atomic helper infrastructure for plane updates. Once + * that switch is complete drivers shouldn't use these any longer, instead using + * the proper legacy implementations for update and disable plane hooks provided + * by the atomic helpers. + * + * Again drivers are strongly urged to switch to the new interfaces. + */ + /* * This is the minimal list of formats that seem to be safe for modeset use * with all current DRM drivers. Most hardware can actually support more -- cgit v1.2.3 From 321ebf04dc7ab5c54d658f93db0ffe35277664ab Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 4 Nov 2014 22:57:27 +0100 Subject: drm/atomic: Refcounting for plane_state->fb So my original plan was that the drm core refcounts framebuffers like with the legacy ioctls. But that doesn't work for a bunch of reasons: - State objects might live longer than until the next fb change happens for a plane. For example delayed cleanup work only happens _after_ the pageflip ioctl has completed. So this definitely doesn't work without the plane state holding its own references. - The other issue is transition from legacy to atomic implementations, where the driver works under a mix of both worlds. Which means legacy paths might not properly update the ->fb pointer under plane->state->fb. Which is a bit a problem when then someone comes around and _does_ try to clean it up when it's long gone. The second issue is just a bit a transition bug, since drivers should update plane->state->fb in all the paths that aren't converted yet. But a bit more robustness for the transition can't hurt - we pull similar tricks with cleaning up the old fb in the transitional helpers already. The pattern for drivers that transition is if (plane->state) drm_atomic_set_fb_for_plane(plane->state, plane->fb); inserted after the fb update has logically completed at the end of ->set_config (or ->set_base/mode_set if using the crtc helpers), ->page_flip, ->update_plane or any other entry point which updates plane->fb. v2: Update kerneldoc - copypasta fail. v3: Fix spelling in the commit message (Sean). Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 28 ++++++++++++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 25 +++++++++++++++++++------ drivers/gpu/drm/drm_crtc_helper.c | 7 ++++--- drivers/gpu/drm/drm_plane_helper.c | 14 +++++++------- include/drm/drm_atomic.h | 2 ++ 5 files changed, 60 insertions(+), 16 deletions(-) (limited to 'drivers/gpu/drm/drm_plane_helper.c') diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index ad15a88c0f74..ed991ba66e21 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -367,6 +367,34 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, } EXPORT_SYMBOL(drm_atomic_set_crtc_for_plane); +/** + * drm_atomic_set_fb_for_plane - set crtc for plane + * @plane_state: atomic state object for the plane + * @fb: fb to use for the plane + * + * Changing the assigned framebuffer for a plane requires us to grab a reference + * to the new fb and drop the reference to the old fb, if there is one. This + * function takes care of all these details besides updating the pointer in the + * state object itself. + */ +void +drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, + struct drm_framebuffer *fb) +{ + if (plane_state->fb) + drm_framebuffer_unreference(plane_state->fb); + if (fb) + drm_framebuffer_reference(fb); + plane_state->fb = fb; + + if (fb) + DRM_DEBUG_KMS("Set [FB:%d] for plane state %p\n", + fb->base.id, plane_state); + else + DRM_DEBUG_KMS("Set [NOFB] for plane state %p\n", plane_state); +} +EXPORT_SYMBOL(drm_atomic_set_fb_for_plane); + /** * drm_atomic_set_crtc_for_connector - set crtc for connector * @conn_state: atomic state object for the connector diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 2b1db0c12fdc..ca839bd9bb0d 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1182,7 +1182,7 @@ retry: ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); if (ret != 0) goto fail; - plane_state->fb = fb; + drm_atomic_set_fb_for_plane(plane_state, fb); plane_state->crtc_x = crtc_x; plane_state->crtc_y = crtc_y; plane_state->crtc_h = crtc_h; @@ -1250,7 +1250,7 @@ retry: ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); if (ret != 0) goto fail; - plane_state->fb = NULL; + drm_atomic_set_fb_for_plane(plane_state, NULL); plane_state->crtc_x = 0; plane_state->crtc_y = 0; plane_state->crtc_h = 0; @@ -1422,7 +1422,7 @@ retry: ret = drm_atomic_set_crtc_for_plane(primary_state, crtc); if (ret != 0) goto fail; - primary_state->fb = set->fb; + drm_atomic_set_fb_for_plane(primary_state, set->fb); primary_state->crtc_x = 0; primary_state->crtc_y = 0; primary_state->crtc_h = set->mode->vdisplay; @@ -1694,7 +1694,7 @@ retry: ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); if (ret != 0) goto fail; - plane_state->fb = fb; + drm_atomic_set_fb_for_plane(plane_state, fb); ret = drm_atomic_async_commit(state); if (ret != 0) @@ -1808,6 +1808,9 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state); */ void drm_atomic_helper_plane_reset(struct drm_plane *plane) { + if (plane->state && plane->state->fb) + drm_framebuffer_unreference(plane->state->fb); + kfree(plane->state); plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL); } @@ -1823,10 +1826,17 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_reset); struct drm_plane_state * drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane) { + struct drm_plane_state *state; + if (WARN_ON(!plane->state)) return NULL; - return kmemdup(plane->state, sizeof(*plane->state), GFP_KERNEL); + state = kmemdup(plane->state, sizeof(*plane->state), GFP_KERNEL); + + if (state && state->fb) + drm_framebuffer_reference(state->fb); + + return state; } EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state); @@ -1839,8 +1849,11 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state); * subclassed plane state structure. */ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, - struct drm_plane_state *state) + struct drm_plane_state *state) { + if (state->fb) + drm_framebuffer_unreference(state->fb); + kfree(state); } EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state); diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 33195e9adaab..d552708409de 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -34,11 +34,13 @@ #include #include +#include #include #include #include #include #include +#include #include /** @@ -998,15 +1000,14 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, if (plane->funcs->atomic_duplicate_state) plane_state = plane->funcs->atomic_duplicate_state(plane); else if (plane->state) - plane_state = kmemdup(plane->state, sizeof(*plane_state), - GFP_KERNEL); + plane_state = drm_atomic_helper_plane_duplicate_state(plane); else plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); if (!plane_state) return -ENOMEM; plane_state->crtc = crtc; - plane_state->fb = crtc->primary->fb; + drm_atomic_set_fb_for_plane(plane_state, crtc->primary->fb); plane_state->crtc_x = 0; plane_state->crtc_y = 0; plane_state->crtc_h = crtc->mode.vdisplay; diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index fa56bb5da6c3..d99c452b0563 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -27,7 +27,9 @@ #include #include #include +#include #include +#include #define SUBPIXEL_MASK 0xffff @@ -464,7 +466,7 @@ out: if (plane->funcs->atomic_destroy_state) plane->funcs->atomic_destroy_state(plane, plane_state); else - kfree(plane_state); + drm_atomic_helper_plane_destroy_state(plane, plane_state); } return ret; @@ -505,15 +507,14 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, if (plane->funcs->atomic_duplicate_state) plane_state = plane->funcs->atomic_duplicate_state(plane); else if (plane->state) - plane_state = kmemdup(plane->state, sizeof(*plane_state), - GFP_KERNEL); + plane_state = drm_atomic_helper_plane_duplicate_state(plane); else plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); if (!plane_state) return -ENOMEM; plane_state->crtc = crtc; - plane_state->fb = fb; + drm_atomic_set_fb_for_plane(plane_state, fb); plane_state->crtc_x = crtc_x; plane_state->crtc_y = crtc_y; plane_state->crtc_h = crtc_h; @@ -552,15 +553,14 @@ int drm_plane_helper_disable(struct drm_plane *plane) if (plane->funcs->atomic_duplicate_state) plane_state = plane->funcs->atomic_duplicate_state(plane); else if (plane->state) - plane_state = kmemdup(plane->state, sizeof(*plane_state), - GFP_KERNEL); + plane_state = drm_atomic_helper_plane_duplicate_state(plane); else plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL); if (!plane_state) return -ENOMEM; plane_state->crtc = NULL; - plane_state->fb = NULL; + drm_atomic_set_fb_for_plane(plane_state, NULL); return drm_plane_helper_commit(plane, plane_state, plane->fb); } diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 5bb15f550c42..9d919168bc11 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -46,6 +46,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, int __must_check drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, struct drm_crtc *crtc); +void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, + struct drm_framebuffer *fb); int __must_check drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, struct drm_crtc *crtc); -- cgit v1.2.3