On Fri, May 22, 2015 at 01:34:53PM +0100, Daniel Stone wrote:
Add a blob property tracking the current mode to the CRTC state, and ensure it is properly updated and referenced.
Signed-off-by: Daniel Stone daniels@collabora.com Tested-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/drm_atomic.c | 62 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 11 ++++--- drivers/gpu/drm/drm_crtc.c | 3 +- include/drm/drm_crtc.h | 3 ++ 4 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 67c251f..63fc58a 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -301,6 +301,8 @@ EXPORT_SYMBOL(drm_atomic_get_crtc_state); int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, struct drm_display_mode *mode) {
- struct drm_mode_modeinfo umode;
- /* Early return for no change. */ if (!mode && !state->enable) return 0;
@@ -308,9 +310,20 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, memcmp(&state->mode, mode, sizeof(*mode)) == 0) return 0;
if (state->mode_blob)
drm_property_unreference_blob(state->mode_blob);
state->mode_blob = NULL; state->mode_changed = true;
if (mode) {
drm_mode_convert_to_umode(&umode, mode);
state->mode_blob =
drm_property_create_blob(state->crtc->dev,
sizeof(umode),
&umode);
if (!state->mode_blob)
return -ENOMEM;
IS_ERR and PTR_ERR? Dan Carpenter will catch this as soon as I'll apply ;-)
- drm_mode_copy(&state->mode, mode); state->enable = true; } else {
@@ -330,6 +343,55 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, } EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc);
+/**
- drm_atomic_set_mode_prop_for_crtc - set mode for CRTC
- @state: the CRTC whose incoming state to update
- @blob: pointer to blob property to use for mode
- Set a mode (originating from a blob property) on the desired CRTC state.
- This function will take a reference on the blob property for the CRTC state,
- and release the reference held on the state's existing mode property, if any
- was set.
- */
+int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
struct drm_property_blob *blob)
+{
- if (blob == state->mode_blob)
return 0;
- if (state->mode_blob)
drm_property_unreference_blob(state->mode_blob);
- state->mode_blob = NULL;
- if (blob) {
if (blob->length != sizeof(struct drm_mode_modeinfo) ||
drm_mode_convert_umode(&state->mode,
(const struct drm_mode_modeinfo *)
blob->data))
return -EINVAL;
state->mode_blob = drm_property_reference_blob(blob);
state->mode_changed = true;
state->enable = true;
- } else {
memset(&state->mode, 0, sizeof(state->mode));
if (state->enable)
state->mode_changed = true;
Same comment as in the previous patch: mode_changed isnt' the core atomic code's business.
state->enable = false;
state->active = false;
- }
- if (state->enable)
DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
state->mode.name, state);
- else
DRM_DEBUG_ATOMIC("Set [NOMODE] for CRTC state %p\n",
state);
- return 0;
+} +EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
/**
- drm_atomic_crtc_set_property - set property on CRTC
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 70ce243..5d596d9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2027,6 +2027,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_dpms); */ void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc) {
- if (crtc->state && crtc->state->mode_blob)
kfree(crtc->state); crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);drm_property_unreference_blob(crtc->state->mode_blob);
@@ -2048,6 +2050,8 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, { memcpy(state, crtc->state, sizeof(*state));
- if (state->mode_blob)
state->mode_changed = false; state->active_changed = false; state->planes_changed = false;drm_property_reference_blob(state->mode_blob);
@@ -2090,11 +2094,8 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) {
- /*
* This is currently a placeholder so that drivers that subclass the
* state will automatically do the right thing if code is ever added
* to this function.
*/
- if (state->mode_blob)
drm_property_unreference_blob(state->mode_blob);
} EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9263b22..ee87045 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1961,7 +1961,8 @@ int drm_mode_getcrtc(struct drm_device *dev, crtc_resp->x = crtc->primary->state->src_x >> 16; crtc_resp->y = crtc->primary->state->src_y >> 16; if (crtc->state->enable) {
drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
memcpy(&crtc_resp->mode, &crtc->state->mode_blob->data,
sizeof(crtc_resp->mode));
Why this change? While transitioning I'd expect that some driver (probably the one with hw state readout) will get this wrong, imo safer to keep looking at crtc->state->mode.
lgtm otherwise. -Daniel
crtc_resp->mode_valid = 1; } else {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 72b60db..c54fa4a 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -299,6 +299,9 @@ struct drm_crtc_state {
struct drm_display_mode mode;
/* blob property to expose current mode to atomic userspace */
struct drm_property_blob *mode_blob;
struct drm_pending_vblank_event *event;
struct drm_atomic_state *state;
-- 2.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel