From de28d0212a3ea7b92a9795cd0e5bfc7d15214677 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Tue, 19 May 2015 16:41:01 +0200 Subject: drm/atomic: add commit_planes_on_crtc helper drm_atomic_helper_commit_planes calls all atomic_begin's first, then updates all planes, finally calling atomic_flush. Some drivers may want to things like disabling irq's from their atomic_begin, in which case a second call to atomic_begin will splat. By using commit_planes_on_crtc on each crtc in the atomic state they'll evade that issue. Signed-off-by: Maarten Lankhorst [danvet: Extend kerneldoc a bit as discussed with Maarten on irc.] [danvet: Squash in fixup to check for crtc_funcs in all places. Reported by Dan Carpenter.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 62 +++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic_helper.h | 1 + 2 files changed, 63 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index b82ef6262469..424a98bfa686 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1127,6 +1127,10 @@ EXPORT_SYMBOL(drm_atomic_helper_prepare_planes); * * It still requires the global state object @old_state to know which planes and * crtcs need to be updated though. + * + * Note that this function does all plane updates across all CRTCs in one step. + * If the hardware can't support this approach look at + * drm_atomic_helper_commit_planes_on_crtc() instead. */ void drm_atomic_helper_commit_planes(struct drm_device *dev, struct drm_atomic_state *old_state) @@ -1180,6 +1184,64 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_helper_commit_planes); +/** + * drm_atomic_helper_commit_planes_on_crtc - commit plane state for a crtc + * @old_crtc_state: atomic state object with the old crtc state + * + * This function commits the new plane state using the plane and atomic helper + * functions for planes on the specific crtc. It assumes that the atomic state + * has already been pushed into the relevant object state pointers, since this + * step can no longer fail. + * + * This function is useful when plane updates should be done crtc-by-crtc + * instead of one global step like drm_atomic_helper_commit_planes() does. + * + * This function can only be savely used when planes are not allowed to move + * between different CRTCs because this function doesn't handle inter-CRTC + * depencies. Callers need to ensure that either no such depencies exist, + * resolve them through ordering of commit calls or through some other means. + */ +void +drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state) +{ + const struct drm_crtc_helper_funcs *crtc_funcs; + struct drm_crtc *crtc = old_crtc_state->crtc; + struct drm_atomic_state *old_state = old_crtc_state->state; + struct drm_plane *plane; + unsigned plane_mask; + + plane_mask = old_crtc_state->plane_mask; + plane_mask |= crtc->state->plane_mask; + + crtc_funcs = crtc->helper_private; + if (crtc_funcs && crtc_funcs->atomic_begin) + crtc_funcs->atomic_begin(crtc); + + drm_for_each_plane_mask(plane, crtc->dev, plane_mask) { + struct drm_plane_state *old_plane_state = + drm_atomic_get_existing_plane_state(old_state, plane); + const struct drm_plane_helper_funcs *plane_funcs; + + plane_funcs = plane->helper_private; + + if (!old_plane_state || !plane_funcs) + continue; + + WARN_ON(plane->state->crtc && plane->state->crtc != crtc); + + if (drm_atomic_plane_disabling(plane, old_plane_state) && + plane_funcs->atomic_disable) + plane_funcs->atomic_disable(plane, old_plane_state); + else if (plane->state->crtc || + drm_atomic_plane_disabling(plane, old_plane_state)) + plane_funcs->atomic_update(plane, old_plane_state); + } + + if (crtc_funcs && crtc_funcs->atomic_flush) + crtc_funcs->atomic_flush(crtc); +} +EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc); + /** * drm_atomic_helper_cleanup_planes - cleanup plane resources after commit * @dev: DRM device diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 6ee0ee5b6143..cc1fee8a12d0 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -58,6 +58,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, struct drm_atomic_state *state); void drm_atomic_helper_cleanup_planes(struct drm_device *dev, struct drm_atomic_state *old_state); +void drm_atomic_helper_commit_planes_on_crtc(struct drm_crtc_state *old_crtc_state); void drm_atomic_helper_swap_state(struct drm_device *dev, struct drm_atomic_state *state); -- cgit v1.2.3-59-g8ed1b From e01e9f75a0c4e6cdbf1f139e37e9161408e49b7c Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Tue, 19 May 2015 16:41:02 +0200 Subject: drm/atomic: add drm_atomic_add_affected_planes This is a convenience function to add all planes for a crtc, similar to add_affected_connectors. This will be used in drm_atomic_helper_check_modeset, but drivers can call it too when they need to recalculate all state. Signed-off-by: Maarten Lankhorst [danvet: Amend kerneldoc a bit.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 39 +++++++++++++++++++++++++++++++++++++++ include/drm/drm_atomic.h | 4 ++++ 2 files changed, 43 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index cd1b16b25716..3134f506b762 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -955,6 +955,45 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_add_affected_connectors); +/** + * drm_atomic_add_affected_planes - add planes for crtc + * @state: atomic state + * @crtc: DRM crtc + * + * This function walks the current configuration and adds all planes + * currently used by @crtc to the atomic configuration @state. This is useful + * when an atomic commit also needs to check all currently enabled plane on + * @crtc, e.g. when changing the mode. It's also useful when re-enabling a CRTC + * to avoid special code to force-enable all planes. + * + * Since acquiring a plane state will always also acquire the w/w mutex of the + * current CRTC for that plane (if there is any) adding all the plane states for + * a CRTC will not reduce parallism of atomic updates. + * + * Returns: + * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK + * then the w/w mutex code has detected a deadlock and the entire atomic + * sequence must be restarted. All other errors are fatal. + */ +int +drm_atomic_add_affected_planes(struct drm_atomic_state *state, + struct drm_crtc *crtc) +{ + struct drm_plane *plane; + + WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); + + drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) { + struct drm_plane_state *plane_state = + drm_atomic_get_plane_state(state, plane); + + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + } + return 0; +} +EXPORT_SYMBOL(drm_atomic_add_affected_planes); + /** * drm_atomic_connectors_for_crtc - count number of connected outputs * @state: atomic state diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index f0d3a7387d99..e89db0c377ba 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -120,6 +120,10 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, int __must_check drm_atomic_add_affected_connectors(struct drm_atomic_state *state, struct drm_crtc *crtc); +int __must_check +drm_atomic_add_affected_planes(struct drm_atomic_state *state, + struct drm_crtc *crtc); + int drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, struct drm_crtc *crtc); -- cgit v1.2.3-59-g8ed1b From 57744aa7cfb5969c5a0621f4cfa45bb83d391064 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Tue, 19 May 2015 16:41:03 +0200 Subject: drm/atomic: add all affected planes in drm_atomic_helper_check_modeset Drivers may need to recalculate plane state when a modeset occurs, not reliably adding them might cause hard to debug bugs. Signed-off-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 424a98bfa686..21259e70adee 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -429,6 +429,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret != 0) return ret; + ret = drm_atomic_add_affected_planes(state, crtc); + if (ret != 0) + return ret; + num_connectors = drm_atomic_connectors_for_crtc(state, crtc); -- cgit v1.2.3-59-g8ed1b From 862e686ce40299250d21ba425ea5078e947397ff Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Thu, 21 May 2015 11:03:16 +0530 Subject: drm: bridge: Allow daisy chaining of bridges Allow drm_bridge objects to link to each other in order to form an encoder chain. The requirement for creating a chain of bridges comes because the MSM drm driver uses up its encoder and bridge objects for blocks within the SoC itself. There isn't anything left to use if the SoC display output is connected to an external encoder IC. Having an additional bridge connected to the existing bridge helps here. In general, it is possible for platforms to have multiple devices between the encoder and the connector/panel that require some sort of configuration. We create drm bridge helper functions corresponding to each op in 'drm_bridge_funcs'. These helpers call the corresponding 'drm_bridge_funcs' op for the entire chain of bridges. These helpers are used internally by drm_atomic_helper.c and drm_crtc_helper.c. The drm_bridge_enable/pre_enable helpers execute enable/pre_enable ops of the bridge closet to the encoder, and proceed until the last bridge in the chain is enabled. The same holds for drm_bridge_mode_set/mode_fixup helpers. The drm_bridge_disable/post_disable helpers disable the last bridge in the chain first, and proceed until the first bridge in the chain is disabled. drm_bridge_attach() remains the same. As before, the driver calling this function should make sure it has set the links correctly. The order in which the bridges are connected to each other determines the order in which the calls are made. One requirement is that every bridge in the chain should point the parent encoder object. This is required since bridge drivers expect a valid encoder pointer in drm_bridge. For example, consider a chain where an encoder's output is connected to bridge1, and bridge1's output is connected to bridge2: /* Like before, attach bridge to an encoder */ bridge1->encoder = encoder; ret = drm_bridge_attach(dev, bridge1); .. /* * set the first bridge's 'next' bridge to bridge2, set its encoder * as bridge1's encoder */ bridge1->next = bridge2 bridge2->encoder = bridge1->encoder; ret = drm_bridge_attach(dev, bridge2); ... ... This method of bridge chaining isn't intrusive and existing drivers that use drm_bridge will behave the same way as before. The bridge helpers also cleans up the atomic and crtc helper files a bit. Reviewed-by: Jani Nikula Reviewed-by: Rob Clark Reviewed-by: Daniel Vetter Signed-off-by: Archit Taneja Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 29 +++---- drivers/gpu/drm/drm_bridge.c | 147 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 54 +++++-------- include/drm/drm_crtc.h | 14 ++++ 4 files changed, 191 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 21259e70adee..a64bacdcf263 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -283,14 +283,11 @@ mode_fixup(struct drm_atomic_state *state) if (!funcs) continue; - if (encoder->bridge && encoder->bridge->funcs->mode_fixup) { - ret = encoder->bridge->funcs->mode_fixup( - encoder->bridge, &crtc_state->mode, - &crtc_state->adjusted_mode); - if (!ret) { - DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); - return -EINVAL; - } + ret = drm_bridge_mode_fixup(encoder->bridge, &crtc_state->mode, + &crtc_state->adjusted_mode); + if (!ret) { + DRM_DEBUG_ATOMIC("Bridge fixup failed\n"); + return -EINVAL; } if (funcs->atomic_check) { @@ -587,8 +584,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) * Each encoder has at most one connector (since we always steal * it away), so we won't call disable hooks twice. */ - if (encoder->bridge) - encoder->bridge->funcs->disable(encoder->bridge); + drm_bridge_disable(encoder->bridge); /* Right function depends upon target state. */ if (connector->state->crtc && funcs->prepare) @@ -598,8 +594,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) else funcs->dpms(encoder, DRM_MODE_DPMS_OFF); - if (encoder->bridge) - encoder->bridge->funcs->post_disable(encoder->bridge); + drm_bridge_post_disable(encoder->bridge); } for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { @@ -741,9 +736,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) if (funcs->mode_set) funcs->mode_set(encoder, mode, adjusted_mode); - if (encoder->bridge && encoder->bridge->funcs->mode_set) - encoder->bridge->funcs->mode_set(encoder->bridge, - mode, adjusted_mode); + drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode); } } @@ -839,16 +832,14 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, * Each encoder has at most one connector (since we always steal * it away), so we won't call enable hooks twice. */ - if (encoder->bridge) - encoder->bridge->funcs->pre_enable(encoder->bridge); + drm_bridge_pre_enable(encoder->bridge); if (funcs->enable) funcs->enable(encoder); else funcs->commit(encoder); - if (encoder->bridge) - encoder->bridge->funcs->enable(encoder->bridge); + drm_bridge_enable(encoder->bridge); } } EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables); diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index eaa5790c2a6f..c3a85ce5a7c4 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -66,6 +66,153 @@ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_attach); +/** + * drm_bridge_mode_fixup - fixup proposed mode for all bridges in the + * encoder chain + * @bridge: bridge control structure + * @mode: desired mode to be set for the bridge + * @adjusted_mode: updated mode that works for this bridge + * + * Calls 'mode_fixup' drm_bridge_funcs op for all the bridges in the + * encoder chain, starting from the first bridge to the last. + * + * Note: the bridge passed should be the one closest to the encoder + * + * RETURNS: + * true on success, false on failure + */ +bool drm_bridge_mode_fixup(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + bool ret = true; + + if (!bridge) + return true; + + if (bridge->funcs->mode_fixup) + ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode); + + ret = ret && drm_bridge_mode_fixup(bridge->next, mode, adjusted_mode); + + return ret; +} +EXPORT_SYMBOL(drm_bridge_mode_fixup); + +/** + * drm_bridge_disable - calls 'disable' drm_bridge_funcs op for all + * bridges in the encoder chain. + * @bridge: bridge control structure + * + * Calls 'disable' drm_bridge_funcs op for all the bridges in the encoder + * chain, starting from the last bridge to the first. These are called before + * calling the encoder's prepare op. + * + * Note: the bridge passed should be the one closest to the encoder + */ +void drm_bridge_disable(struct drm_bridge *bridge) +{ + if (!bridge) + return; + + drm_bridge_disable(bridge->next); + + bridge->funcs->disable(bridge); +} +EXPORT_SYMBOL(drm_bridge_disable); + +/** + * drm_bridge_post_disable - calls 'post_disable' drm_bridge_funcs op for + * all bridges in the encoder chain. + * @bridge: bridge control structure + * + * Calls 'post_disable' drm_bridge_funcs op for all the bridges in the + * encoder chain, starting from the first bridge to the last. These are called + * after completing the encoder's prepare op. + * + * Note: the bridge passed should be the one closest to the encoder + */ +void drm_bridge_post_disable(struct drm_bridge *bridge) +{ + if (!bridge) + return; + + bridge->funcs->post_disable(bridge); + + drm_bridge_post_disable(bridge->next); +} +EXPORT_SYMBOL(drm_bridge_post_disable); + +/** + * drm_bridge_mode_set - set proposed mode for all bridges in the + * encoder chain + * @bridge: bridge control structure + * @mode: desired mode to be set for the bridge + * @adjusted_mode: updated mode that works for this bridge + * + * Calls 'mode_set' drm_bridge_funcs op for all the bridges in the + * encoder chain, starting from the first bridge to the last. + * + * Note: the bridge passed should be the one closest to the encoder + */ +void drm_bridge_mode_set(struct drm_bridge *bridge, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + if (!bridge) + return; + + if (bridge->funcs->mode_set) + bridge->funcs->mode_set(bridge, mode, adjusted_mode); + + drm_bridge_mode_set(bridge->next, mode, adjusted_mode); +} +EXPORT_SYMBOL(drm_bridge_mode_set); + +/** + * drm_bridge_pre_enable - calls 'pre_enable' drm_bridge_funcs op for all + * bridges in the encoder chain. + * @bridge: bridge control structure + * + * Calls 'pre_enable' drm_bridge_funcs op for all the bridges in the encoder + * chain, starting from the last bridge to the first. These are called + * before calling the encoder's commit op. + * + * Note: the bridge passed should be the one closest to the encoder + */ +void drm_bridge_pre_enable(struct drm_bridge *bridge) +{ + if (!bridge) + return; + + drm_bridge_pre_enable(bridge->next); + + bridge->funcs->pre_enable(bridge); +} +EXPORT_SYMBOL(drm_bridge_pre_enable); + +/** + * drm_bridge_enable - calls 'enable' drm_bridge_funcs op for all bridges + * in the encoder chain. + * @bridge: bridge control structure + * + * Calls 'enable' drm_bridge_funcs op for all the bridges in the encoder + * chain, starting from the first bridge to the last. These are called + * after completing the encoder's commit op. + * + * Note that the bridge passed should be the one closest to the encoder + */ +void drm_bridge_enable(struct drm_bridge *bridge) +{ + if (!bridge) + return; + + bridge->funcs->enable(bridge); + + drm_bridge_enable(bridge->next); +} +EXPORT_SYMBOL(drm_bridge_enable); + #ifdef CONFIG_OF struct drm_bridge *of_drm_find_bridge(struct device_node *np) { diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index d727b73fba3a..f2f42b17967a 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -163,16 +163,14 @@ drm_encoder_disable(struct drm_encoder *encoder) { const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private; - if (encoder->bridge) - encoder->bridge->funcs->disable(encoder->bridge); + drm_bridge_disable(encoder->bridge); if (encoder_funcs->disable) (*encoder_funcs->disable)(encoder); else (*encoder_funcs->dpms)(encoder, DRM_MODE_DPMS_OFF); - if (encoder->bridge) - encoder->bridge->funcs->post_disable(encoder->bridge); + drm_bridge_post_disable(encoder->bridge); } static void __drm_helper_disable_unused_functions(struct drm_device *dev) @@ -312,13 +310,11 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue; - if (encoder->bridge && encoder->bridge->funcs->mode_fixup) { - ret = encoder->bridge->funcs->mode_fixup( - encoder->bridge, mode, adjusted_mode); - if (!ret) { - DRM_DEBUG_KMS("Bridge fixup failed\n"); - goto done; - } + ret = drm_bridge_mode_fixup(encoder->bridge, + mode, adjusted_mode); + if (!ret) { + DRM_DEBUG_KMS("Bridge fixup failed\n"); + goto done; } encoder_funcs = encoder->helper_private; @@ -343,15 +339,13 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue; - if (encoder->bridge) - encoder->bridge->funcs->disable(encoder->bridge); + drm_bridge_disable(encoder->bridge); encoder_funcs = encoder->helper_private; /* Disable the encoders as the first thing we do. */ encoder_funcs->prepare(encoder); - if (encoder->bridge) - encoder->bridge->funcs->post_disable(encoder->bridge); + drm_bridge_post_disable(encoder->bridge); } drm_crtc_prepare_encoders(dev); @@ -376,9 +370,7 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, encoder_funcs = encoder->helper_private; encoder_funcs->mode_set(encoder, mode, adjusted_mode); - if (encoder->bridge && encoder->bridge->funcs->mode_set) - encoder->bridge->funcs->mode_set(encoder->bridge, mode, - adjusted_mode); + drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode); } /* Now enable the clocks, plane, pipe, and connectors that we set up. */ @@ -389,14 +381,12 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, if (encoder->crtc != crtc) continue; - if (encoder->bridge) - encoder->bridge->funcs->pre_enable(encoder->bridge); + drm_bridge_pre_enable(encoder->bridge); encoder_funcs = encoder->helper_private; encoder_funcs->commit(encoder); - if (encoder->bridge) - encoder->bridge->funcs->enable(encoder->bridge); + drm_bridge_enable(encoder->bridge); } /* Calculate and store various constants which @@ -735,23 +725,19 @@ static void drm_helper_encoder_dpms(struct drm_encoder *encoder, int mode) struct drm_bridge *bridge = encoder->bridge; const struct drm_encoder_helper_funcs *encoder_funcs; - if (bridge) { - if (mode == DRM_MODE_DPMS_ON) - bridge->funcs->pre_enable(bridge); - else - bridge->funcs->disable(bridge); - } + if (mode == DRM_MODE_DPMS_ON) + drm_bridge_pre_enable(bridge); + else + drm_bridge_disable(bridge); encoder_funcs = encoder->helper_private; if (encoder_funcs->dpms) encoder_funcs->dpms(encoder, mode); - if (bridge) { - if (mode == DRM_MODE_DPMS_ON) - bridge->funcs->enable(bridge); - else - bridge->funcs->post_disable(bridge); - } + if (mode == DRM_MODE_DPMS_ON) + drm_bridge_enable(bridge); + else + drm_bridge_post_disable(bridge); } static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index bff25b0cada9..dace1b635685 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -903,6 +903,8 @@ struct drm_bridge_funcs { /** * struct drm_bridge - central DRM bridge control structure * @dev: DRM device this bridge belongs to + * @encoder: encoder to which this bridge is connected + * @next: the next bridge in the encoder chain * @of_node: device node pointer to the bridge * @list: to keep track of all added bridges * @base: base mode object @@ -912,6 +914,7 @@ struct drm_bridge_funcs { struct drm_bridge { struct drm_device *dev; struct drm_encoder *encoder; + struct drm_bridge *next; #ifdef CONFIG_OF struct device_node *of_node; #endif @@ -1247,6 +1250,17 @@ extern void drm_bridge_remove(struct drm_bridge *bridge); extern struct drm_bridge *of_drm_find_bridge(struct device_node *np); extern int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge); +bool drm_bridge_mode_fixup(struct drm_bridge *bridge, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); +void drm_bridge_disable(struct drm_bridge *bridge); +void drm_bridge_post_disable(struct drm_bridge *bridge); +void drm_bridge_mode_set(struct drm_bridge *bridge, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode); +void drm_bridge_pre_enable(struct drm_bridge *bridge); +void drm_bridge_enable(struct drm_bridge *bridge); + extern int drm_encoder_init(struct drm_device *dev, struct drm_encoder *encoder, const struct drm_encoder_funcs *funcs, -- cgit v1.2.3-59-g8ed1b From 2331b4e476e0dc705825c8f5462fa1274b6d3b97 Mon Sep 17 00:00:00 2001 From: Archit Taneja Date: Thu, 21 May 2015 11:03:17 +0530 Subject: drm/DocBook: Add more drm_bridge documentation Add DOC sections giving an overview of drm_bridge and how to fill up the drm_bridge_funcs ops. Add these to drm.tpml in DocBook. Add headerdocs for funcs in drm_bridge.c that don't have them yet. Signed-off-by: Archit Taneja [danvet: Amend kerneldoc as discussed with Archit.] Signed-off-by: Daniel Vetter --- Documentation/DocBook/drm.tmpl | 12 ++++++ drivers/gpu/drm/drm_bridge.c | 95 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 7c68ecc800a1..5d9d851d8623 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2439,6 +2439,18 @@ void intel_crt_init(struct drm_device *dev) Tile group !Pdrivers/gpu/drm/drm_crtc.c Tile group + + Bridges + + Overview +!Pdrivers/gpu/drm/drm_bridge.c overview + + + Default bridge callback sequence +!Pdrivers/gpu/drm/drm_bridge.c bridge callbacks + +!Edrivers/gpu/drm/drm_bridge.c + diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c3a85ce5a7c4..6b8f7211e543 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -28,9 +28,42 @@ #include "drm/drmP.h" +/** + * DOC: overview + * + * drm_bridge represents a device that hangs on to an encoder. These are handy + * when a regular drm_encoder entity isn't enough to represent the entire + * encoder chain. + * + * A bridge is always associated to a single drm_encoder at a time, but can be + * either connected to it directly, or through an intermediate bridge: + * + * encoder ---> bridge B ---> bridge A + * + * Here, the output of the encoder feeds to bridge B, and that furthers feeds to + * bridge A. + * + * The driver using the bridge is responsible to make the associations between + * the encoder and bridges. Once these links are made, the bridges will + * participate along with encoder functions to perform mode_set/enable/disable + * through the ops provided in drm_bridge_funcs. + * + * drm_bridge, like drm_panel, aren't drm_mode_object entities like planes, + * crtcs, encoders or connectors. They just provide additional hooks to get the + * desired output at the end of the encoder chain. + */ + static DEFINE_MUTEX(bridge_lock); static LIST_HEAD(bridge_list); +/** + * drm_bridge_add - add the given bridge to the global bridge list + * + * @bridge: bridge control structure + * + * RETURNS: + * Unconditionally returns Zero. + */ int drm_bridge_add(struct drm_bridge *bridge) { mutex_lock(&bridge_lock); @@ -41,6 +74,11 @@ int drm_bridge_add(struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_add); +/** + * drm_bridge_remove - remove the given bridge from the global bridge list + * + * @bridge: bridge control structure + */ void drm_bridge_remove(struct drm_bridge *bridge) { mutex_lock(&bridge_lock); @@ -49,6 +87,21 @@ void drm_bridge_remove(struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_remove); +/** + * drm_bridge_attach - associate given bridge to our DRM device + * + * @dev: DRM device + * @bridge: bridge control structure + * + * called by a kms driver to link one of our encoder/bridge to the given + * bridge. + * + * Note that setting up links between the bridge and our encoder/bridge + * objects needs to be handled by the kms driver itself + * + * RETURNS: + * Zero on success, error code on failure + */ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge) { if (!dev || !bridge) @@ -66,6 +119,39 @@ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge) } EXPORT_SYMBOL(drm_bridge_attach); +/** + * DOC: bridge callbacks + * + * The drm_bridge_funcs ops are populated by the bridge driver. The drm + * internals(atomic and crtc helpers) use the helpers defined in drm_bridge.c + * These helpers call a specific drm_bridge_funcs op for all the bridges + * during encoder configuration. + * + * When creating a bridge driver, one can implement drm_bridge_funcs op with + * the help of these rough rules: + * + * pre_enable: this contains things needed to be done for the bridge before + * its clock and timings are enabled by its source. For a bridge, its source + * is generally the encoder or bridge just before it in the encoder chain. + * + * enable: this contains things needed to be done for the bridge once its + * source is enabled. In other words, enable is called once the source is + * ready with clock and timing needed by the bridge. + * + * disable: this contains things needed to be done for the bridge assuming + * that its source is still enabled, i.e. clock and timings are still on. + * + * post_disable: this contains things needed to be done for the bridge once + * its source is disabled, i.e. once clocks and timings are off. + * + * mode_fixup: this should fixup the given mode for the bridge. It is called + * after the encoder's mode fixup. mode_fixup can also reject a mode completely + * if it's unsuitable for the hardware. + * + * mode_set: this sets up the mode for the bridge. It assumes that its source + * (an encoder or a bridge) has set the mode too. + */ + /** * drm_bridge_mode_fixup - fixup proposed mode for all bridges in the * encoder chain @@ -214,6 +300,15 @@ void drm_bridge_enable(struct drm_bridge *bridge) EXPORT_SYMBOL(drm_bridge_enable); #ifdef CONFIG_OF +/** + * of_drm_find_bridge - find the bridge corresponding to the device node in + * the global bridge list + * + * @np: device node + * + * RETURNS: + * drm_bridge control struct on success, NULL on failure + */ struct drm_bridge *of_drm_find_bridge(struct device_node *np) { struct drm_bridge *bridge; -- cgit v1.2.3-59-g8ed1b From f102c16ebbeb40ba2f2a7ba4703ed3e2fc013c68 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:44 +0100 Subject: drm: kerneldoc fixes for blob properties Change '@param foo' to '@foo:' to fit kerneldoc style. 672cb1d6ae mistakenly added an extra parameter to the kerneldoc for drm_property_unreference_blob which wasn't actually present. Signed-off-by: Daniel Stone Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4059f065c854..7e5085fe4b75 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4270,7 +4270,7 @@ EXPORT_SYMBOL(drm_property_create_blob); * * Internal free function for blob properties; must not be used directly. * - * @param kref Reference + * @kref: Reference */ static void drm_property_free_blob(struct kref *kref) { @@ -4290,7 +4290,7 @@ static void drm_property_free_blob(struct kref *kref) * * Drop a reference on a blob property. May free the object. * - * @param blob Pointer to blob property + * @blob: Pointer to blob property */ void drm_property_unreference_blob(struct drm_property_blob *blob) { @@ -4318,8 +4318,7 @@ EXPORT_SYMBOL(drm_property_unreference_blob); * Drop a reference on a blob property. May free the object. This must be * called with blob_lock held. * - * @param dev Device the blob was created on - * @param blob Pointer to blob property + * @blob: Pointer to blob property */ static void drm_property_unreference_blob_locked(struct drm_property_blob *blob) { @@ -4336,7 +4335,7 @@ static void drm_property_unreference_blob_locked(struct drm_property_blob *blob) * * Take a new reference on an existing blob property. * - * @param blob Pointer to blob property + * @blob: Pointer to blob property */ struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob) { -- cgit v1.2.3-59-g8ed1b From 9f658b7b62e7aefc1ee067136126eca3f58cabfd Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:45 +0100 Subject: drm/crtc_helper: Replace open-coded CRTC state helpers Rather than open-coding our own CRTC state helpers, use the atomic helpers added in f5e7840b0c, and make our freeing behaviour consistent as well. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c | 42 ++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index f2f42b17967a..9297a61a7c95 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -927,14 +927,15 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod 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 + else { crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL); - if (!crtc_state) - return -ENOMEM; - crtc_state->crtc = crtc; + if (!crtc_state) + return -ENOMEM; + if (crtc->state) + __drm_atomic_helper_crtc_duplicate_state(crtc, crtc_state); + else + crtc_state->crtc = crtc; + } crtc_state->enable = true; crtc_state->planes_changed = true; @@ -944,30 +945,25 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod if (crtc_funcs->atomic_check) { ret = crtc_funcs->atomic_check(crtc, crtc_state); - if (ret) { - if (crtc->funcs->atomic_destroy_state) { - crtc->funcs->atomic_destroy_state(crtc, - crtc_state); - } else { - kfree(crtc_state); - } - - return ret; - } + if (ret) + goto out; } 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); + ret = drm_helper_crtc_mode_set_base(crtc, x, y, old_fb); + +out: + if (crtc->funcs->atomic_destroy_state) + crtc->funcs->atomic_destroy_state(crtc, crtc_state); + else { + __drm_atomic_helper_crtc_destroy_state(crtc, crtc_state); + kfree(crtc_state); } - return drm_helper_crtc_mode_set_base(crtc, x, y, old_fb); + return ret; } EXPORT_SYMBOL(drm_helper_crtc_mode_set); -- cgit v1.2.3-59-g8ed1b From 7dec9a9648f825a0698fd875d2834b597f122bd6 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:47 +0100 Subject: drm/mode: Validate modes inside drm_crtc_convert_umode The only user of convert_umode was also performing mode validation, so do that in the same place. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 7e5085fe4b75..baad4e856006 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1785,11 +1785,15 @@ static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out, static int drm_crtc_convert_umode(struct drm_display_mode *out, const struct drm_mode_modeinfo *in) { - if (in->clock > INT_MAX || in->vrefresh > INT_MAX) - return -ERANGE; + int ret = -EINVAL; + + if (in->clock > INT_MAX || in->vrefresh > INT_MAX) { + ret = -ERANGE; + goto out; + } if ((in->flags & DRM_MODE_FLAG_3D_MASK) > DRM_MODE_FLAG_3D_MAX) - return -EINVAL; + goto out; out->clock = in->clock; out->hdisplay = in->hdisplay; @@ -1808,7 +1812,14 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out, strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); out->name[DRM_DISPLAY_MODE_LEN-1] = 0; - return 0; + out->status = drm_mode_validate_basic(out); + if (out->status != MODE_OK) + goto out; + + ret = 0; + +out: + return ret; } /** @@ -2821,12 +2832,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, goto out; } - mode->status = drm_mode_validate_basic(mode); - if (mode->status != MODE_OK) { - ret = -EINVAL; - goto out; - } - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); /* -- cgit v1.2.3-59-g8ed1b From 934a8a899a7275ed187810fe9a15a93397e88c6b Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:48 +0100 Subject: drm/mode: Unstatic kernel-userspace mode conversion Move the drm_display_mode <-> drm_mode_modeinfo conversion functions from drm_crtc.c to drm_modes.c, and make them non-static so that others can use them. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 95 ++------------------------------------------- drivers/gpu/drm/drm_modes.c | 87 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_modes.h | 4 ++ 3 files changed, 95 insertions(+), 91 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index baad4e856006..f6b0332aad7f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1735,93 +1735,6 @@ void drm_reinit_primary_mode_group(struct drm_device *dev) } EXPORT_SYMBOL(drm_reinit_primary_mode_group); -/** - * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo - * @out: drm_mode_modeinfo struct to return to the user - * @in: drm_display_mode to use - * - * Convert a drm_display_mode into a drm_mode_modeinfo structure to return to - * the user. - */ -static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out, - const struct drm_display_mode *in) -{ - WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX || - in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX || - in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX || - in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX || - in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX, - "timing values too large for mode info\n"); - - out->clock = in->clock; - out->hdisplay = in->hdisplay; - out->hsync_start = in->hsync_start; - out->hsync_end = in->hsync_end; - out->htotal = in->htotal; - out->hskew = in->hskew; - out->vdisplay = in->vdisplay; - out->vsync_start = in->vsync_start; - out->vsync_end = in->vsync_end; - out->vtotal = in->vtotal; - out->vscan = in->vscan; - out->vrefresh = in->vrefresh; - out->flags = in->flags; - out->type = in->type; - strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); - out->name[DRM_DISPLAY_MODE_LEN-1] = 0; -} - -/** - * drm_crtc_convert_umode - convert a modeinfo into a drm_display_mode - * @out: drm_display_mode to return to the user - * @in: drm_mode_modeinfo to use - * - * Convert a drm_mode_modeinfo into a drm_display_mode structure to return to - * the caller. - * - * Returns: - * Zero on success, negative errno on failure. - */ -static int drm_crtc_convert_umode(struct drm_display_mode *out, - const struct drm_mode_modeinfo *in) -{ - int ret = -EINVAL; - - if (in->clock > INT_MAX || in->vrefresh > INT_MAX) { - ret = -ERANGE; - goto out; - } - - if ((in->flags & DRM_MODE_FLAG_3D_MASK) > DRM_MODE_FLAG_3D_MAX) - goto out; - - out->clock = in->clock; - out->hdisplay = in->hdisplay; - out->hsync_start = in->hsync_start; - out->hsync_end = in->hsync_end; - out->htotal = in->htotal; - out->hskew = in->hskew; - out->vdisplay = in->vdisplay; - out->vsync_start = in->vsync_start; - out->vsync_end = in->vsync_end; - out->vtotal = in->vtotal; - out->vscan = in->vscan; - out->vrefresh = in->vrefresh; - out->flags = in->flags; - out->type = in->type; - strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); - out->name[DRM_DISPLAY_MODE_LEN-1] = 0; - - out->status = drm_mode_validate_basic(out); - if (out->status != MODE_OK) - goto out; - - ret = 0; - -out: - return ret; -} - /** * drm_mode_getresources - get graphics configuration * @dev: drm device for the ioctl @@ -2048,7 +1961,7 @@ 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_crtc_convert_to_umode(&crtc_resp->mode, &crtc->state->mode); + drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode); crtc_resp->mode_valid = 1; } else { @@ -2058,7 +1971,7 @@ int drm_mode_getcrtc(struct drm_device *dev, crtc_resp->x = crtc->x; crtc_resp->y = crtc->y; if (crtc->enabled) { - drm_crtc_convert_to_umode(&crtc_resp->mode, &crtc->mode); + drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode); crtc_resp->mode_valid = 1; } else { @@ -2215,7 +2128,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if (!drm_mode_expose_to_userspace(mode, file_priv)) continue; - drm_crtc_convert_to_umode(&u_mode, mode); + drm_mode_convert_to_umode(&u_mode, mode); if (copy_to_user(mode_ptr + copied, &u_mode, sizeof(u_mode))) { ret = -EFAULT; @@ -2826,7 +2739,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, goto out; } - ret = drm_crtc_convert_umode(mode, &crtc_req->mode); + ret = drm_mode_convert_umode(mode, &crtc_req->mode); if (ret) { DRM_DEBUG_KMS("Invalid mode\n"); goto out; diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 213b11ea69b5..cd74a0953f42 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1405,3 +1405,90 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, return mode; } EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode); + +/** + * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo + * @out: drm_mode_modeinfo struct to return to the user + * @in: drm_display_mode to use + * + * Convert a drm_display_mode into a drm_mode_modeinfo structure to return to + * the user. + */ +void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, + const struct drm_display_mode *in) +{ + WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX || + in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX || + in->hskew > USHRT_MAX || in->vdisplay > USHRT_MAX || + in->vsync_start > USHRT_MAX || in->vsync_end > USHRT_MAX || + in->vtotal > USHRT_MAX || in->vscan > USHRT_MAX, + "timing values too large for mode info\n"); + + out->clock = in->clock; + out->hdisplay = in->hdisplay; + out->hsync_start = in->hsync_start; + out->hsync_end = in->hsync_end; + out->htotal = in->htotal; + out->hskew = in->hskew; + out->vdisplay = in->vdisplay; + out->vsync_start = in->vsync_start; + out->vsync_end = in->vsync_end; + out->vtotal = in->vtotal; + out->vscan = in->vscan; + out->vrefresh = in->vrefresh; + out->flags = in->flags; + out->type = in->type; + strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); + out->name[DRM_DISPLAY_MODE_LEN-1] = 0; +} + +/** + * drm_crtc_convert_umode - convert a modeinfo into a drm_display_mode + * @out: drm_display_mode to return to the user + * @in: drm_mode_modeinfo to use + * + * Convert a drm_mode_modeinfo into a drm_display_mode structure to return to + * the caller. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_mode_convert_umode(struct drm_display_mode *out, + const struct drm_mode_modeinfo *in) +{ + int ret = -EINVAL; + + if (in->clock > INT_MAX || in->vrefresh > INT_MAX) { + ret = -ERANGE; + goto out; + } + + if ((in->flags & DRM_MODE_FLAG_3D_MASK) > DRM_MODE_FLAG_3D_MAX) + goto out; + + out->clock = in->clock; + out->hdisplay = in->hdisplay; + out->hsync_start = in->hsync_start; + out->hsync_end = in->hsync_end; + out->htotal = in->htotal; + out->hskew = in->hskew; + out->vdisplay = in->vdisplay; + out->vsync_start = in->vsync_start; + out->vsync_end = in->vsync_end; + out->vtotal = in->vtotal; + out->vscan = in->vscan; + out->vrefresh = in->vrefresh; + out->flags = in->flags; + out->type = in->type; + strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); + out->name[DRM_DISPLAY_MODE_LEN-1] = 0; + + out->status = drm_mode_validate_basic(out); + if (out->status != MODE_OK) + goto out; + + ret = 0; + +out: + return ret; +} \ No newline at end of file diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 0616188c7801..08a8cac9e555 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -182,6 +182,10 @@ struct drm_cmdline_mode; struct drm_display_mode *drm_mode_create(struct drm_device *dev); void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode); +void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, + const struct drm_display_mode *in); +int drm_mode_convert_umode(struct drm_display_mode *out, + const struct drm_mode_modeinfo *in); void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); -- cgit v1.2.3-59-g8ed1b From 99531d9bb76c649df15311c717deefdff7cc5b7b Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:49 +0100 Subject: drm: Allow creating blob properties without copy Make the data parameter to drm_property_create_blob optional; if omitted, the copy will be skipped and the data will be empty. Signed-off-by: Daniel Stone Reviewed-by: Maarten Lankhorst Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f6b0332aad7f..0844108e19e0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4145,6 +4145,16 @@ done: return ret; } +/** + * drm_property_create_blob - Create new blob property + * + * Creates a new blob property for a specified DRM device, optionally + * copying data. + * + * @dev: DRM device to create property for + * @length: Length to allocate for blob data + * @data: If specified, copies data into blob + */ struct drm_property_blob * drm_property_create_blob(struct drm_device *dev, size_t length, const void *data) @@ -4152,7 +4162,7 @@ drm_property_create_blob(struct drm_device *dev, size_t length, struct drm_property_blob *blob; int ret; - if (!length || !data) + if (!length) return NULL; blob = kzalloc(sizeof(struct drm_property_blob)+length, GFP_KERNEL); @@ -4162,7 +4172,8 @@ drm_property_create_blob(struct drm_device *dev, size_t length, blob->length = length; blob->dev = dev; - memcpy(blob->data, data, length); + if (data) + memcpy(blob->data, data, length); mutex_lock(&dev->mode_config.blob_lock); -- cgit v1.2.3-59-g8ed1b From 10e8cb7e79391071b950a28a4d85790dd38fb714 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:50 +0100 Subject: drm: Return error value from blob creation Change drm_property_create_blob to return an ERR_PTR-encoded error on failure, so we can pass the failure reason down. Signed-off-by: Daniel Stone Cc: Maarten Lankhorst Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0844108e19e0..f661589b1dea 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4154,6 +4154,10 @@ done: * @dev: DRM device to create property for * @length: Length to allocate for blob data * @data: If specified, copies data into blob + * + * Returns: + * New blob property with a single reference on success, or an ERR_PTR + * value on failure. */ struct drm_property_blob * drm_property_create_blob(struct drm_device *dev, size_t length, @@ -4163,11 +4167,11 @@ drm_property_create_blob(struct drm_device *dev, size_t length, int ret; if (!length) - return NULL; + return ERR_PTR(-EINVAL); blob = kzalloc(sizeof(struct drm_property_blob)+length, GFP_KERNEL); if (!blob) - return NULL; + return ERR_PTR(-ENOMEM); blob->length = length; blob->dev = dev; @@ -4181,7 +4185,7 @@ drm_property_create_blob(struct drm_device *dev, size_t length, if (ret) { kfree(blob); mutex_unlock(&dev->mode_config.blob_lock); - return NULL; + return ERR_PTR(-EINVAL); } kref_init(&blob->refcount); @@ -4370,8 +4374,8 @@ static int drm_property_replace_global_blob(struct drm_device *dev, if (length && data) { new_blob = drm_property_create_blob(dev, length, data); - if (!new_blob) - return -EINVAL; + if (IS_ERR(new_blob)) + return PTR_ERR(new_blob); } /* This does not need to be synchronised with blob_lock, as the -- cgit v1.2.3-59-g8ed1b From e2f5d2ea479b9b2619965d43db70939589afe43a Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 22 May 2015 13:34:51 +0100 Subject: drm/mode: Add user blob-creation ioctl Add an ioctl which allows users to create blob properties from supplied data. Currently this only supports modes, creating a drm_display_mode from the userspace drm_mode_modeinfo. v2: Removed size/type checks. Rebased on new patches to allow error propagation from create_blob, as well as avoiding double-allocation. Signed-off-by: Daniel Stone Reviewed-by: Maarten Lankhorst Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 139 +++++++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_fops.c | 5 +- drivers/gpu/drm/drm_ioctl.c | 2 + include/drm/drmP.h | 4 ++ include/drm/drm_crtc.h | 9 ++- include/uapi/drm/drm.h | 2 + include/uapi/drm/drm_mode.h | 20 +++++++ 7 files changed, 176 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f661589b1dea..e548c50edc94 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4173,6 +4173,9 @@ drm_property_create_blob(struct drm_device *dev, size_t length, if (!blob) return ERR_PTR(-ENOMEM); + /* This must be explicitly initialised, so we can safely call list_del + * on it in the removal handler, even if it isn't in a file list. */ + INIT_LIST_HEAD(&blob->head_file); blob->length = length; blob->dev = dev; @@ -4190,7 +4193,8 @@ drm_property_create_blob(struct drm_device *dev, size_t length, kref_init(&blob->refcount); - list_add_tail(&blob->head, &dev->mode_config.property_blob_list); + list_add_tail(&blob->head_global, + &dev->mode_config.property_blob_list); mutex_unlock(&dev->mode_config.blob_lock); @@ -4212,7 +4216,8 @@ static void drm_property_free_blob(struct kref *kref) WARN_ON(!mutex_is_locked(&blob->dev->mode_config.blob_lock)); - list_del(&blob->head); + list_del(&blob->head_global); + list_del(&blob->head_file); drm_mode_object_put(blob->dev, &blob->base); kfree(blob); @@ -4263,6 +4268,26 @@ static void drm_property_unreference_blob_locked(struct drm_property_blob *blob) kref_put(&blob->refcount, drm_property_free_blob); } +/** + * drm_property_destroy_user_blobs - destroy all blobs created by this client + * @dev: DRM device + * @file_priv: destroy all blobs owned by this file handle + */ +void drm_property_destroy_user_blobs(struct drm_device *dev, + struct drm_file *file_priv) +{ + struct drm_property_blob *blob, *bt; + + mutex_lock(&dev->mode_config.blob_lock); + + list_for_each_entry_safe(blob, bt, &file_priv->blobs, head_file) { + list_del_init(&blob->head_file); + drm_property_unreference_blob_locked(blob); + } + + mutex_unlock(&dev->mode_config.blob_lock); +} + /** * drm_property_reference_blob - Take a reference on an existing property * @@ -4452,6 +4477,114 @@ done: return ret; } +/** + * drm_mode_createblob_ioctl - create a new blob property + * @dev: DRM device + * @data: ioctl data + * @file_priv: DRM file info + * + * This function creates a new blob property with user-defined values. In order + * to give us sensible validation and checking when creating, rather than at + * every potential use, we also require a type to be provided upfront. + * + * Called by the user via ioctl. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_mode_createblob_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ + struct drm_mode_create_blob *out_resp = data; + struct drm_property_blob *blob; + void __user *blob_ptr; + int ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + blob = drm_property_create_blob(dev, out_resp->length, NULL); + if (IS_ERR(blob)) + return PTR_ERR(blob); + + blob_ptr = (void __user *)(unsigned long)out_resp->data; + if (copy_from_user(blob->data, blob_ptr, out_resp->length)) { + ret = -EFAULT; + goto out_blob; + } + + /* Dropping the lock between create_blob and our access here is safe + * as only the same file_priv can remove the blob; at this point, it is + * not associated with any file_priv. */ + mutex_lock(&dev->mode_config.blob_lock); + out_resp->blob_id = blob->base.id; + list_add_tail(&file_priv->blobs, &blob->head_file); + mutex_unlock(&dev->mode_config.blob_lock); + + return 0; + +out_blob: + drm_property_unreference_blob(blob); + return ret; +} + +/** + * drm_mode_destroyblob_ioctl - destroy a user blob property + * @dev: DRM device + * @data: ioctl data + * @file_priv: DRM file info + * + * Destroy an existing user-defined blob property. + * + * Called by the user via ioctl. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_mode_destroyblob_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ + struct drm_mode_destroy_blob *out_resp = data; + struct drm_property_blob *blob = NULL, *bt; + bool found = false; + int ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + mutex_lock(&dev->mode_config.blob_lock); + blob = __drm_property_lookup_blob(dev, out_resp->blob_id); + if (!blob) { + ret = -ENOENT; + goto err; + } + + /* Ensure the property was actually created by this user. */ + list_for_each_entry(bt, &file_priv->blobs, head_file) { + if (bt == blob) { + found = true; + break; + } + } + + if (!found) { + ret = -EPERM; + goto err; + } + + /* We must drop head_file here, because we may not be the last + * reference on the blob. */ + list_del_init(&blob->head_file); + drm_property_unreference_blob_locked(blob); + mutex_unlock(&dev->mode_config.blob_lock); + + return 0; + +err: + mutex_unlock(&dev->mode_config.blob_lock); + return ret; +} + /** * drm_mode_connector_set_path_property - set tile property on connector * @connector: connector to set property on. @@ -5655,7 +5788,7 @@ void drm_mode_config_cleanup(struct drm_device *dev) } list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list, - head) { + head_global) { drm_property_unreference_blob(blob); } diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 0f6a5c8801e3..c59ce4d0ef75 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -167,6 +167,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) INIT_LIST_HEAD(&priv->lhead); INIT_LIST_HEAD(&priv->fbs); mutex_init(&priv->fbs_lock); + INIT_LIST_HEAD(&priv->blobs); INIT_LIST_HEAD(&priv->event_list); init_waitqueue_head(&priv->event_wait); priv->event_space = 4096; /* set aside 4k for event buffer */ @@ -405,8 +406,10 @@ int drm_release(struct inode *inode, struct file *filp) drm_events_release(file_priv); - if (drm_core_check_feature(dev, DRIVER_MODESET)) + if (drm_core_check_feature(dev, DRIVER_MODESET)) { drm_fb_release(file_priv); + drm_property_destroy_user_blobs(dev, file_priv); + } if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_release(dev, file_priv); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 266dcd6cdf3b..9bac1b7479af 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -641,6 +641,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB, drm_mode_destroyblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), }; #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls ) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index df6d9970d9a4..9fa6366f47c2 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -326,6 +326,10 @@ struct drm_file { struct list_head fbs; struct mutex fbs_lock; + /** User-created blob properties; this retains a reference on the + * property. */ + struct list_head blobs; + wait_queue_head_t event_wait; struct list_head event_list; int event_space; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index dace1b635685..72b60dbe0891 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -218,7 +218,8 @@ struct drm_property_blob { struct drm_mode_object base; struct drm_device *dev; struct kref refcount; - struct list_head head; + struct list_head head_global; + struct list_head head_file; size_t length; unsigned char data[]; }; @@ -1315,6 +1316,8 @@ extern const char *drm_get_dvi_i_select_name(int val); extern const char *drm_get_tv_subconnector_name(int val); extern const char *drm_get_tv_select_name(int val); extern void drm_fb_release(struct drm_file *file_priv); +extern void drm_property_destroy_user_blobs(struct drm_device *dev, + struct drm_file *file_priv); extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct drm_mode_group *group); extern void drm_mode_group_destroy(struct drm_mode_group *group); extern void drm_reinit_primary_mode_group(struct drm_device *dev); @@ -1460,6 +1463,10 @@ extern int drm_mode_getproperty_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_mode_getblob_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +extern int drm_mode_createblob_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv); +extern int drm_mode_destroyblob_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv); extern int drm_mode_connector_property_set_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int drm_mode_getencoder(struct drm_device *dev, diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index ff6ef62d084b..3801584a0c53 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -786,6 +786,8 @@ struct drm_prime_handle { #define DRM_IOCTL_MODE_OBJ_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_obj_set_property) #define DRM_IOCTL_MODE_CURSOR2 DRM_IOWR(0xBB, struct drm_mode_cursor2) #define DRM_IOCTL_MODE_ATOMIC DRM_IOWR(0xBC, struct drm_mode_atomic) +#define DRM_IOCTL_MODE_CREATEPROPBLOB DRM_IOWR(0xBD, struct drm_mode_create_blob) +#define DRM_IOCTL_MODE_DESTROYPROPBLOB DRM_IOWR(0xBE, struct drm_mode_destroy_blob) /** * Device specific ioctls should only be in their respective headers diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index dbeba949462a..359107ab629e 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -558,4 +558,24 @@ struct drm_mode_atomic { __u64 user_data; }; +/** + * Create a new 'blob' data property, copying length bytes from data pointer, + * and returning new blob ID. + */ +struct drm_mode_create_blob { + /** Pointer to data to copy. */ + __u64 data; + /** Length of data to copy. */ + __u32 length; + /** Return: new property ID. */ + __u32 blob_id; +}; + +/** + * Destroy a user-created blob property. + */ +struct drm_mode_destroy_blob { + __u32 blob_id; +}; + #endif -- cgit v1.2.3-59-g8ed1b From da9b2a381a32c2a287f99e4be2f372587c53ef14 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Mon, 25 May 2015 19:11:49 +0100 Subject: drm: Retain reference to blob properties in lookup When we look up a blob property, make sure we retain a reference to the blob for the lifetime. v2: Use DRM_MODE_PROP_BLOB, not PROP_OBJECT + OBJECT_BLOB. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e548c50edc94..2de6edf821c7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4775,7 +4775,8 @@ void drm_property_change_valid_put(struct drm_property *property, if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) { if (property->values[0] == DRM_MODE_OBJECT_FB) drm_framebuffer_unreference(obj_to_fb(ref)); - } + } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) + drm_property_unreference_blob(obj_to_blob(ref)); } /** -- cgit v1.2.3-59-g8ed1b From bbe16a40e23a65626904aa22fbfc3240a65d21d1 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 20 May 2015 16:53:53 +0200 Subject: drm: check for garbage in unused addfb2 fields Unfortunately old userspace didn't clear this properly, but since we've added fb modifiers that's fixed. Checking properly that unused fields is important for abi extensions, and just right now there's a bunch of discussions going on about how exactly the additional aux planes for render compression should be specified. So let's first make sure that the values in those additional fields can be indeed used. Cc: Thierry Reding Testcase: igt/kms_addfb/unused-* Reviewed-by: Thierry Reding Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2de6edf821c7..098c94e53fdf 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3249,6 +3249,32 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } } + for (i = num_planes; i < 4; i++) { + if (r->modifier[i]) { + DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i); + return -EINVAL; + } + + /* Pre-FB_MODIFIERS userspace didn't clear the structs properly. */ + if (!(r->flags & DRM_MODE_FB_MODIFIERS)) + continue; + + if (r->handles[i]) { + DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i); + return -EINVAL; + } + + if (r->pitches[i]) { + DRM_DEBUG_KMS("non-zero pitch for unused plane %d\n", i); + return -EINVAL; + } + + if (r->offsets[i]) { + DRM_DEBUG_KMS("non-zero offset for unused plane %d\n", i); + return -EINVAL; + } + } + return 0; } -- cgit v1.2.3-59-g8ed1b From 819364da20fd914aba2fd03e95ee0467286752f5 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Tue, 26 May 2015 14:36:48 +0100 Subject: drm: Add drm_atomic_set_mode_for_crtc Add a new helper, to be used later for blob property management, that sets the mode for a CRTC state, as well as updating the CRTC enable/active state at the same time. v2: Do not touch active/mode_changed in CRTC state. Document return value. Remove stray drm_atomic_set_mode_prop_for_crtc declaration. v3: Remove i915 changes, and leave it directly bashing crtc_state->mode for the meantime. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 36 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++++--- drivers/gpu/drm/drm_crtc_helper.c | 5 +++-- include/drm/drm_atomic.h | 3 +++ 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 3134f506b762..e749287390a3 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -289,6 +289,42 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_atomic_get_crtc_state); +/** + * drm_atomic_set_mode_for_crtc - set mode for CRTC + * @state: the CRTC whose incoming state to update + * @mode: kernel-internal mode to use for the CRTC, or NULL to disable + * + * Set a mode (originating from the kernel) on the desired CRTC state. Does + * not change any other state properties, including enable, active, or + * mode_changed. + * + * RETURNS: + * Zero on success, error code on failure. Cannot return -EDEADLK. + */ +int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, + struct drm_display_mode *mode) +{ + /* Early return for no change. */ + if (mode && memcmp(&state->mode, mode, sizeof(*mode)) == 0) + return 0; + + if (mode) { + drm_mode_copy(&state->mode, mode); + state->enable = true; + DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n", + mode->name, state); + } else { + memset(&state->mode, 0, sizeof(state->mode)); + state->enable = false; + DRM_DEBUG_ATOMIC("Set [NOMODE] for CRTC state %p\n", + state); + } + + return 0; +} +EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); + + /** * drm_atomic_crtc_set_property - set property on CRTC * @crtc: the drm CRTC to set a property on diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index a64bacdcf263..e69d4847e237 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1607,7 +1607,10 @@ retry: WARN_ON(set->fb); WARN_ON(set->num_connectors); - crtc_state->enable = false; + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); + if (ret != 0) + goto fail; + crtc_state->active = false; ret = drm_atomic_set_crtc_for_plane(primary_state, NULL); @@ -1622,9 +1625,11 @@ retry: WARN_ON(!set->fb); WARN_ON(!set->num_connectors); - crtc_state->enable = true; + ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode); + if (ret != 0) + goto fail; + crtc_state->active = true; - drm_mode_copy(&crtc_state->mode, set->mode); ret = drm_atomic_set_crtc_for_plane(primary_state, crtc); if (ret != 0) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 9297a61a7c95..393114df88a3 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -937,10 +937,11 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod crtc_state->crtc = crtc; } - crtc_state->enable = true; crtc_state->planes_changed = true; crtc_state->mode_changed = true; - drm_mode_copy(&crtc_state->mode, mode); + ret = drm_atomic_set_mode_for_crtc(crtc_state, mode); + if (ret) + goto out; drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode); if (crtc_funcs->atomic_check) { diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index e89db0c377ba..1e8c61f23294 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -109,6 +109,9 @@ drm_atomic_get_existing_connector_state(struct drm_atomic_state *state, return state->connector_states[index]; } +int __must_check +drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, + struct drm_display_mode *mode); int __must_check drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, struct drm_crtc *crtc); -- cgit v1.2.3-59-g8ed1b From 99cf4a29fa24461bbfe22125967188a18383eb5c Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Mon, 25 May 2015 19:11:51 +0100 Subject: drm/atomic: Add current-mode blob to CRTC state Add a blob property tracking the current mode to the CRTC state, and ensure it is properly updated and referenced. v2: Continue using crtc_state->mode inside getcrtc, instead of reading out the mode blob. Use IS_ERR and PTR_ERR from create_blob. Move set_mode_prop_for_crtc to later patch where it actually gets used. Enforce !!state->enable == !!state->mode_blob inside drm_atomic_crtc_check. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 32 +++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++----- include/drm/drm_crtc.h | 3 +++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index e749287390a3..327ccc71012f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -304,11 +304,25 @@ 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 && memcmp(&state->mode, mode, sizeof(*mode)) == 0) return 0; + if (state->mode_blob) + drm_property_unreference_blob(state->mode_blob); + state->mode_blob = NULL; + if (mode) { + drm_mode_convert_to_umode(&umode, mode); + state->mode_blob = + drm_property_create_blob(state->crtc->dev, + sizeof(umode), + &umode); + if (IS_ERR(state->mode_blob)) + return PTR_ERR(state->mode_blob); + drm_mode_copy(&state->mode, mode); state->enable = true; DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n", @@ -324,7 +338,6 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, } EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); - /** * drm_atomic_crtc_set_property - set property on CRTC * @crtc: the drm CRTC to set a property on @@ -410,6 +423,23 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, return -EINVAL; } + /* The state->enable vs. state->mode_blob checks can be WARN_ON, + * as this is a kernel-internal detail that userspace should never + * be able to trigger. */ + if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) && + WARN_ON(state->enable && !state->mode_blob)) { + DRM_DEBUG_ATOMIC("[CRTC:%d] enabled without mode blob\n", + crtc->base.id); + return -EINVAL; + } + + if (drm_core_check_feature(crtc->dev, DRIVER_ATOMIC) && + WARN_ON(!state->enable && state->mode_blob)) { + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled with mode blob\n", + crtc->base.id); + return -EINVAL; + } + return 0; } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index e69d4847e237..a900858fa265 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2044,6 +2044,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); @@ -2065,6 +2067,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; @@ -2107,11 +2111,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/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 72b60dbe0891..c54fa4add792 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; -- cgit v1.2.3-59-g8ed1b From 955f3c334f0fb2b843efad5cc6d3b7e141e9d666 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Mon, 25 May 2015 19:11:52 +0100 Subject: drm/atomic: Add MODE_ID property Atomic modesetting: now with modesetting support. v2: Moved drm_atomic_set_mode_prop_for_crtc from previous patch; removed state->active fiddling, documented return code. Changed property type to DRM_MODE_PROP_BLOB. Signed-off-by: Daniel Stone Tested-by: Sean Paul Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 57 +++++++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_crtc.c | 8 +++++++ include/drm/drm_atomic.h | 3 +++ include/drm/drm_crtc.h | 1 + 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 327ccc71012f..c7e59b074e62 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -338,6 +338,51 @@ 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. + * + * RETURNS: + * Zero on success, error code on failure. Cannot return -EDEADLK. + */ +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->enable = true; + DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n", + state->mode.name, state); + } else { + memset(&state->mode, 0, sizeof(state->mode)); + state->enable = false; + 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 * @crtc: the drm CRTC to set a property on @@ -360,10 +405,18 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, { struct drm_device *dev = crtc->dev; struct drm_mode_config *config = &dev->mode_config; + int ret; - /* FIXME: Mode prop is missing, which also controls ->enable. */ if (property == config->prop_active) state->active = val; + else if (property == config->prop_mode_id) { + struct drm_property_blob *mode = + drm_property_lookup_blob(dev, val); + ret = drm_atomic_set_mode_prop_for_crtc(state, mode); + if (mode) + drm_property_unreference_blob(mode); + return ret; + } else if (crtc->funcs->atomic_set_property) return crtc->funcs->atomic_set_property(crtc, state, property, val); else @@ -388,6 +441,8 @@ int drm_atomic_crtc_get_property(struct drm_crtc *crtc, if (property == config->prop_active) *val = state->active; + else if (property == config->prop_mode_id) + *val = (state->mode_blob) ? state->mode_blob->base.id : 0; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 098c94e53fdf..77f87b23a6e7 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -688,6 +688,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&crtc->base, config->prop_active, 0); + drm_object_attach_property(&crtc->base, config->prop_mode_id, 0); } return 0; @@ -1454,6 +1455,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_active = prop; + prop = drm_property_create(dev, + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB, + "MODE_ID", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.prop_mode_id = prop; + return 0; } diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 1e8c61f23294..55f46049e4a0 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -113,6 +113,9 @@ int __must_check drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, struct drm_display_mode *mode); int __must_check +drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, + struct drm_property_blob *blob); +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, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c54fa4add792..3b4d8a4a23fb 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1146,6 +1146,7 @@ struct drm_mode_config { struct drm_property *prop_fb_id; struct drm_property *prop_crtc_id; struct drm_property *prop_active; + struct drm_property *prop_mode_id; /* DVI-I properties */ struct drm_property *dvi_i_subconnector_property; -- cgit v1.2.3-59-g8ed1b From 60f207a5b6d8f23c2e8388b415e8d5c7311cc79d Mon Sep 17 00:00:00 2001 From: Andrey Ryabinin Date: Mon, 25 May 2015 13:29:44 +0300 Subject: drm/atomic: fix out of bounds read in for_each_*_in_state helpers for_each_*_in_state validate array index after access to array elements, thus perform out of bounds read. Fix this by validating index in the first place and read array element iff validation was successful. Fixes: df63b9994eaf ("drm/atomic: Add for_each_{connector,crtc,plane}_in_state helper macros") Signed-off-by: Andrey Ryabinin Signed-off-by: Daniel Vetter --- include/drm/drm_atomic.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 55f46049e4a0..1bbfedf466b9 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -142,26 +142,26 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state); #define for_each_connector_in_state(state, connector, connector_state, __i) \ for ((__i) = 0; \ - (connector) = (state)->connectors[__i], \ - (connector_state) = (state)->connector_states[__i], \ - (__i) < (state)->num_connector; \ + (__i) < (state)->num_connector && \ + ((connector) = (state)->connectors[__i], \ + (connector_state) = (state)->connector_states[__i], 1); \ (__i)++) \ if (connector) #define for_each_crtc_in_state(state, crtc, crtc_state, __i) \ for ((__i) = 0; \ - (crtc) = (state)->crtcs[__i], \ - (crtc_state) = (state)->crtc_states[__i], \ - (__i) < (state)->dev->mode_config.num_crtc; \ + (__i) < (state)->dev->mode_config.num_crtc && \ + ((crtc) = (state)->crtcs[__i], \ + (crtc_state) = (state)->crtc_states[__i], 1); \ (__i)++) \ if (crtc_state) -#define for_each_plane_in_state(state, plane, plane_state, __i) \ - for ((__i) = 0; \ - (plane) = (state)->planes[__i], \ - (plane_state) = (state)->plane_states[__i], \ - (__i) < (state)->dev->mode_config.num_total_plane; \ - (__i)++) \ +#define for_each_plane_in_state(state, plane, plane_state, __i) \ + for ((__i) = 0; \ + (__i) < (state)->dev->mode_config.num_total_plane && \ + ((plane) = (state)->planes[__i], \ + (plane_state) = (state)->plane_states[__i], 1); \ + (__i)++) \ if (plane_state) #endif /* DRM_ATOMIC_H_ */ -- cgit v1.2.3-59-g8ed1b From 5ceecb2fa720c01a97e6fa2353c2b8f1e8d69f1f Mon Sep 17 00:00:00 2001 From: Michel Dänzer Date: Tue, 26 May 2015 17:53:38 +0900 Subject: drm: Fix off-by-one in vblank hardware counter wraparound handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dev->max_vblank_count contains the largest value that can be represented by the hardware counter. When the hardware counter wraps around, we have to add that value + 1 to get the same value as if the hardware counter didn't wrap around. Signed-off-by: Michel Dänzer Reviewed-by: Christian König Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 1967e7fc9805..6f02b4b560b8 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -130,7 +130,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) /* * Interrupts were disabled prior to this call, so deal with counter * wrap if needed. - * NOTE! It's possible we lost a full dev->max_vblank_count events + * NOTE! It's possible we lost a full dev->max_vblank_count + 1 events * here if the register is small or we had vblank interrupts off for * a long time. * @@ -147,7 +147,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) /* Deal with counter wrap */ diff = cur_vblank - vblank->last; if (cur_vblank < vblank->last) { - diff += dev->max_vblank_count; + diff += dev->max_vblank_count + 1; DRM_DEBUG("last_vblank[%d]=0x%x, cur_vblank=0x%x => diff=0x%x\n", crtc, vblank->last, cur_vblank, diff); -- cgit v1.2.3-59-g8ed1b