I had hoped to make this a full "nuclear pageflip" enablement series, but I didn't have as much time to work on this today as I hoped, so I still need to finish up the final integration with the DRM core atomic code. Several of my early patches are working and relatively straightforward, so I figured I'd post these as a preliminary patchset; hopefully I'll have the rest of the patches necessary to expose "nuclear pageflip" (i.e., single-crtc, plane-only updates) ready in the next day or so.
This series does add (and then use) a transitional helper for planes' .set_property() handler, similar to what we have for .update_plane() and .disable_plane(). The transitional helper can be used by drivers that are starting to implement the atomic driver entrypoints, but aren't ready to switch over to full atomic functionality yet.
Note that the i915-specific patches here depend on Ander's recent series which hasn't been merged yet: [PATCH 0/7] Make drm_crtc->state match pipe_config
Cc: dri-devel@lists.freedesktop.org
Matt Roper (6): drm/i915: Move rotation from intel_plane to intel_plane_state drm/i915: Consolidate plane handler vtables drm/plane-helper: Add transitional helper for .set_plane(). drm/plane-helper: Fix transitional helper kerneldocs drm/i915: Add .atomic_{get,set}_property() entrypoints to planes drm/i915: Replace intel_set_property() with transitional helper
drivers/gpu/drm/drm_plane_helper.c | 109 +++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_atomic_plane.c | 84 +++++++++++++++++++++-- drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 22 ++++-- drivers/gpu/drm/i915/intel_fbc.c | 4 +- drivers/gpu/drm/i915/intel_sprite.c | 65 ++++++------------ include/drm/drm_plane_helper.h | 3 + 7 files changed, 268 insertions(+), 82 deletions(-)
Add a transitional helper for planes' .set_property() entrypoint, similar to what we already have for .update_plane() and .disable_plane(). This allows drivers that are still in the process of transitioning to implement and exercise the .atomic_set_property() driver entrypoint that will eventually be used by atomic.
Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/drm_plane_helper.c | 105 +++++++++++++++++++++++++++++++++++++ include/drm/drm_plane_helper.h | 3 ++ 2 files changed, 108 insertions(+)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index f24c4cf..1b1e927 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -578,3 +578,108 @@ int drm_plane_helper_disable(struct drm_plane *plane) return drm_plane_helper_commit(plane, plane_state, plane->fb); } EXPORT_SYMBOL(drm_plane_helper_disable); + +/** + * drm_plane_helper_set_property() - Transitional helper for property updates + * @plane: plane object to update + * @prop: property to update + * @val: value to set property to + * + * Provides a default plane property handler using the atomic plane update + * functions. Drivers in the process of transitioning to atomic should + * replace their plane.set_property() entrypoint with this function and + * then implement the plane.atomic_set_property() entrypoint. + * + * This is useful for piecewise transitioning of a driver to the atomic helpers. + * + * Note that this helper assumes that no hardware programming should be + * performed if the plane is disabled. When the plane is not attached to a + * crtc, we just swap in the validated plane state and return; there's no + * call to atomic_begin()/atomic_update()/atomic_flush(). + * + * RETURNS: + * Zero on success, error code on failure + */ +int drm_plane_helper_set_property(struct drm_plane *plane, + struct drm_property *prop, + uint64_t val) +{ + struct drm_plane_state *plane_state; + struct drm_plane_helper_funcs *plane_funcs = plane->helper_private; + struct drm_crtc_helper_funcs *crtc_funcs; + int ret; + + /* + * Drivers may not have an .atomic_set_property() handler if they have + * no driver-specific properties, but they generally wouldn't setup a + * .set_property() handler (pointing to this function) for the plane in + * that case either; throw a warning since this is probably a mistake. + */ + if (WARN_ON(!plane->funcs->atomic_set_property)) + return -EINVAL; + + if (plane->funcs->atomic_duplicate_state) + plane_state = plane->funcs->atomic_duplicate_state(plane); + else if (plane->state) + 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->plane = plane; + + /* + * Update the state object with the new property value we want to + * set. If the property is not recognized as a driver-specific property, + * -EINVAL will be returned here. + */ + ret = plane->funcs->atomic_set_property(plane, plane_state, prop, val); + if (ret) + goto out; + + /* + * Check that the updated plane state is actually programmable (e.g., + * doesn't violate hardware constraints). + */ + if (plane_funcs->atomic_check) { + ret = plane_funcs->atomic_check(plane, plane_state); + if (ret) + goto out; + } + + /* Point of no return, commit sw state. */ + swap(plane->state, plane_state); + + /* + * If the plane is disabled, we're done. No hardware programming is + * attempted when the plane is disabled. + */ + if (!plane->crtc) + goto out; + + /* Start hardware transaction to commit new state to hardware */ + crtc_funcs = plane->crtc->helper_private; + if (crtc_funcs && crtc_funcs->atomic_begin) + crtc_funcs->atomic_begin(plane->crtc); + plane_funcs->atomic_update(plane, plane_state); + if (crtc_funcs && crtc_funcs->atomic_flush) + crtc_funcs->atomic_flush(plane->crtc); + + /* + * Since we're not using full atomic yet, we still need to update the shadow + * property value that's managed by the DRM core since that's where the + * property values will be read back from. + */ + drm_object_property_set_value(&plane->base, prop, val); + +out: + if (plane_state) { + if (plane->funcs->atomic_destroy_state) + plane->funcs->atomic_destroy_state(plane, plane_state); + else + drm_atomic_helper_plane_destroy_state(plane, plane_state); + } + + return ret; +} +EXPORT_SYMBOL(drm_plane_helper_set_property); diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index a185392..4a56961 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -107,6 +107,9 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h); int drm_plane_helper_disable(struct drm_plane *plane); +int drm_plane_helper_set_property(struct drm_plane *plane, + struct drm_property *prop, + uint64_t val);
/* For use by drm_crtc_helper.c */ int drm_plane_helper_commit(struct drm_plane *plane,
On Thu, Jan 15, 2015 at 06:34:21PM -0800, Matt Roper wrote:
Add a transitional helper for planes' .set_property() entrypoint, similar to what we already have for .update_plane() and .disable_plane(). This allows drivers that are still in the process of transitioning to implement and exercise the .atomic_set_property() driver entrypoint that will eventually be used by atomic.
Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matt Roper matthew.d.roper@intel.com
Since i915 is the only driver with an interest in this (I presume at least) maybe we should make that much fuzz about this here and just move this function into i915 private code?
At least the problem I see with too fancy transitional helpers is that they'll grow stale fast. Which will happen with this one here as soon as we move rotation into drm_plane_state, and will happen every time we add something there. And I expect a lot of common core properties, e.g. for Z order or blending modes. -Daniel
drivers/gpu/drm/drm_plane_helper.c | 105 +++++++++++++++++++++++++++++++++++++ include/drm/drm_plane_helper.h | 3 ++ 2 files changed, 108 insertions(+)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index f24c4cf..1b1e927 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -578,3 +578,108 @@ int drm_plane_helper_disable(struct drm_plane *plane) return drm_plane_helper_commit(plane, plane_state, plane->fb); } EXPORT_SYMBOL(drm_plane_helper_disable);
+/**
- drm_plane_helper_set_property() - Transitional helper for property updates
- @plane: plane object to update
- @prop: property to update
- @val: value to set property to
- Provides a default plane property handler using the atomic plane update
- functions. Drivers in the process of transitioning to atomic should
- replace their plane.set_property() entrypoint with this function and
- then implement the plane.atomic_set_property() entrypoint.
- This is useful for piecewise transitioning of a driver to the atomic helpers.
- Note that this helper assumes that no hardware programming should be
- performed if the plane is disabled. When the plane is not attached to a
- crtc, we just swap in the validated plane state and return; there's no
- call to atomic_begin()/atomic_update()/atomic_flush().
- RETURNS:
- Zero on success, error code on failure
- */
+int drm_plane_helper_set_property(struct drm_plane *plane,
struct drm_property *prop,uint64_t val)+{
- struct drm_plane_state *plane_state;
- struct drm_plane_helper_funcs *plane_funcs = plane->helper_private;
- struct drm_crtc_helper_funcs *crtc_funcs;
- int ret;
- /*
* Drivers may not have an .atomic_set_property() handler if they have* no driver-specific properties, but they generally wouldn't setup a* .set_property() handler (pointing to this function) for the plane in* that case either; throw a warning since this is probably a mistake.*/- if (WARN_ON(!plane->funcs->atomic_set_property))
return -EINVAL;- if (plane->funcs->atomic_duplicate_state)
plane_state = plane->funcs->atomic_duplicate_state(plane);- else if (plane->state)
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->plane = plane;
- /*
* Update the state object with the new property value we want to* set. If the property is not recognized as a driver-specific property,* -EINVAL will be returned here.*/- ret = plane->funcs->atomic_set_property(plane, plane_state, prop, val);
- if (ret)
goto out;- /*
* Check that the updated plane state is actually programmable (e.g.,* doesn't violate hardware constraints).*/- if (plane_funcs->atomic_check) {
ret = plane_funcs->atomic_check(plane, plane_state);if (ret)goto out;- }
- /* Point of no return, commit sw state. */
- swap(plane->state, plane_state);
- /*
* If the plane is disabled, we're done. No hardware programming is* attempted when the plane is disabled.*/- if (!plane->crtc)
goto out;- /* Start hardware transaction to commit new state to hardware */
- crtc_funcs = plane->crtc->helper_private;
- if (crtc_funcs && crtc_funcs->atomic_begin)
crtc_funcs->atomic_begin(plane->crtc);- plane_funcs->atomic_update(plane, plane_state);
- if (crtc_funcs && crtc_funcs->atomic_flush)
crtc_funcs->atomic_flush(plane->crtc);- /*
* Since we're not using full atomic yet, we still need to update the shadow* property value that's managed by the DRM core since that's where the* property values will be read back from.*/- drm_object_property_set_value(&plane->base, prop, val);
+out:
- if (plane_state) {
if (plane->funcs->atomic_destroy_state)plane->funcs->atomic_destroy_state(plane, plane_state);elsedrm_atomic_helper_plane_destroy_state(plane, plane_state);- }
- return ret;
+} +EXPORT_SYMBOL(drm_plane_helper_set_property); diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h index a185392..4a56961 100644 --- a/include/drm/drm_plane_helper.h +++ b/include/drm/drm_plane_helper.h @@ -107,6 +107,9 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h); int drm_plane_helper_disable(struct drm_plane *plane); +int drm_plane_helper_set_property(struct drm_plane *plane,
struct drm_property *prop,uint64_t val);/* For use by drm_crtc_helper.c */ int drm_plane_helper_commit(struct drm_plane *plane, -- 1.8.5.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
drm_plane_helper_{update,disable} are not specific to primary planes; fix some copy/paste summaries to avoid confusion.
Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/drm_plane_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 1b1e927..3e7e26f 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -484,7 +484,7 @@ out: }
/** - * drm_plane_helper_update() - Helper for primary plane update + * drm_plane_helper_update() - Transitional helper for plane update * @plane: plane object to update * @crtc: owning CRTC of owning plane * @fb: framebuffer to flip onto plane @@ -541,7 +541,7 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, EXPORT_SYMBOL(drm_plane_helper_update);
/** - * drm_plane_helper_disable() - Helper for primary plane disable + * drm_plane_helper_disable() - Transitional helper for plane disable * @plane: plane to disable * * Provides a default plane disable handler using the atomic plane update
On Thu, Jan 15, 2015 at 06:34:22PM -0800, Matt Roper wrote:
drm_plane_helper_{update,disable} are not specific to primary planes; fix some copy/paste summaries to avoid confusion.
Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matt Roper matthew.d.roper@intel.com
Applied to my topic/atomic-core branch.
Thanks, Daniel
drivers/gpu/drm/drm_plane_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 1b1e927..3e7e26f 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -484,7 +484,7 @@ out: }
/**
- drm_plane_helper_update() - Helper for primary plane update
- drm_plane_helper_update() - Transitional helper for plane update
- @plane: plane object to update
- @crtc: owning CRTC of owning plane
- @fb: framebuffer to flip onto plane
@@ -541,7 +541,7 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, EXPORT_SYMBOL(drm_plane_helper_update);
/**
- drm_plane_helper_disable() - Helper for primary plane disable
- drm_plane_helper_disable() - Transitional helper for plane disable
- @plane: plane to disable
- Provides a default plane disable handler using the atomic plane update
-- 1.8.5.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Matt Roper