| Message ID | 1432298094-22239-11-git-send-email-daniels@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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) > + drm_property_unreference_blob(crtc->state->mode_blob); > kfree(crtc->state); > crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL); > > @@ -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) > + drm_property_reference_blob(state->mode_blob); > state->mode_changed = false; > state->active_changed = false; > state->planes_changed = false; > @@ -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
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; + 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; + 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) + drm_property_unreference_blob(crtc->state->mode_blob); kfree(crtc->state); crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL); @@ -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) + drm_property_reference_blob(state->mode_blob); state->mode_changed = false; state->active_changed = false; state->planes_changed = false; @@ -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)); 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;