drm/i915: Set connector_state->connector using the helper.
Subsystems (6)
Maarten Lankhorst
maintainerPATCH v2 1/6 drm/i915: Set connector_state->connector using the helper.
The atomic helper sets connector_state->connector, which the i915 code didn't. This will become a problem when we start using it.
Signed-off-by Maarten Lankhorst <[email protected]>
Acked-by Thierry Reding <[email protected]>
--- drivers/gpu/drm/i915/intel_display.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +2 -4
@@ -6477,13 +6477,11 @@ static void intel_connector_check_state(struct intel_connector *connector)int intel_connector_init (struct intel_connector *connector) { -struct drm_connector_state *connector_state; +drm_atomic_helper_connector_reset (&connector->base); - connector_state =kzalloc (sizeof *connector_state,GFP_KERNEL ); -if (!connector_state) +if (!connector->base.state)return -ENOMEM ; - connector->base.state = connector_state;return 0 ; }
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH v2 2/6 drm/atomic: Add __drm_atomic_helper_connector_reset, v2.
This is useful for drivers that subclass connector_state, like tegra.
Changes since v1:
- Docbook updates.
Signed-off-by Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/drm_atomic_helper.c | 30 ++++++++++++++++++++++++++---- include/drm/drm_atomic_helper.h | 2 ++ 2 files changed, 28 insertions(+), 4 deletions(-)
drivers/gpu/drm/drm_atomic_helper.c +26 -4
@@ -2607,6 +2607,28 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,EXPORT_SYMBOL (drm_atomic_helper_plane_destroy_state);/** +* __drm_atomic_helper_connector_reset - reset state on connector +* @connector: drm connector +* @conn_state: connector state to assign +* +* Initializes the newly allocated @conn_state and assigns it to +* #connector ->state, usually required when initializing the drivers +* or when called from the ->reset hook. +* +* This is useful for drivers that subclass the connector state. +*/ +void +__drm_atomic_helper_connector_reset (struct drm_connector *connector, +struct drm_connector_state *conn_state) +{ +if (conn_state) + conn_state->connector = connector; + + connector->state = conn_state; +} +EXPORT_SYMBOL (__drm_atomic_helper_connector_reset); + +/** * drm_atomic_helper_connector_reset - default ->reset hook for connectors * @connector: drm connector *
@@ -2616,11 +2638,11 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);*/ void drm_atomic_helper_connector_reset (struct drm_connector *connector) { -kfree (connector->state); - connector->state =kzalloc (sizeof (*connector->state),GFP_KERNEL ); +struct drm_connector_state *conn_state = +kzalloc (sizeof (*conn_state),GFP_KERNEL ); -if (connector->state) - connector->state->connector = connector; +kfree (connector->state); +__drm_atomic_helper_connector_reset (connector, conn_state); }EXPORT_SYMBOL (drm_atomic_helper_connector_reset);
include/drm/drm_atomic_helper.h +2 -0
@@ -126,6 +126,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,void drm_atomic_helper_plane_destroy_state (struct drm_plane *plane,struct drm_plane_state *state); +void __drm_atomic_helper_connector_reset (struct drm_connector *connector, +struct drm_connector_state *conn_state);void drm_atomic_helper_connector_reset (struct drm_connector *connector); void __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v2 2/6] drm/atomic: Add __drm_atomic_helper_connector_reset, v2.
Quoted 29 lines · Maarten Lankhorst
This is useful for drivers that subclass
connector_state, like tegra.Changes since v1:
- Docbook updates.Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/drm_atomic_helper.c | 30 ++++++++++++++++++++++++++---- include/drm/drm_atomic_helper.h | 2 ++ 2 files changed, 28 insertions(+), 4 deletions(-)drivers/gpu/drm/drm_atomic_helper.c +10
-0@@ -2607,6 +2607,28 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,EXPORT_SYMBOL (drm_atomic_helper_plane_destroy_state);/** +* __drm_atomic_helper_connector_reset - reset state on connector +* @connector: drm connector +* @conn_state: connector state to assign +* +* Initializes the newly allocated @conn_state and assigns it to +* #connector ->state, usually required when initializing the drivers +* or when called from the ->reset hook. +* +* This is useful for drivers that subclass the connector state. +*/
drivers/gpu/drm/drm_atomic_helper.c void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, · line 2629
Quoted 10 lines
drivers/gpu/drm/drm_atomic_helper.c +10
-0+void +__drm_atomic_helper_connector_reset (struct drm_connector *connector, +struct drm_connector_state *conn_state) +{ +if (conn_state) + conn_state->connector = connector; + + connector->state = conn_state; +} +EXPORT_SYMBOL (__drm_atomic_helper_connector_reset);
It's a bit strange that we don't have __*reset functions for crtc/plane now, especially since that would uncover that probably we should move cleaning up the old state into this helper (to share more code). That can be done by simply calling ->atomic_state_destroy. Callers would then only allocate a suitable struct, call this func and init any driver-private state.
But this here won't hurt, so merged it.
-Daniel
Quoted 41 lines
drivers/gpu/drm/drm_atomic_helper.c +6
-4+ +/** * drm_atomic_helper_connector_reset - default ->reset hook for connectors * @connector: drm connector * @@ -2616,11 +2638,11 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);*/ void drm_atomic_helper_connector_reset (struct drm_connector *connector) { -kfree (connector->state); - connector->state =kzalloc (sizeof (*connector->state),GFP_KERNEL ); +struct drm_connector_state *conn_state = +kzalloc (sizeof (*conn_state),GFP_KERNEL ); -if (connector->state) - connector->state->connector = connector; +kfree (connector->state); +__drm_atomic_helper_connector_reset (connector, conn_state); }EXPORT_SYMBOL (drm_atomic_helper_connector_reset);include/drm/drm_atomic_helper.h +2
-0@@ -126,6 +126,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,void drm_atomic_helper_plane_destroy_state (struct drm_plane *plane,struct drm_plane_state *state); +void __drm_atomic_helper_connector_reset (struct drm_connector *connector, +struct drm_connector_state *conn_state);void drm_atomic_helper_connector_reset (struct drm_connector *connector); void __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,Signature · 6 lines
2.1.0 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Maarten Lankhorst
maintainerPATCH v2 3/6 drm/tegra: Use __drm_atomic_helper_reset_connector for subclassing connector state, v2.
In reply to Maarten LankhorstChanges since v1:
- Do not reset if state allocation fails.
Signed-off-by Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/tegra/dsi.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/gpu/drm/tegra/dsi.c +6 -7
@@ -745,14 +745,13 @@ static void tegra_dsi_soft_reset(struct tegra_dsi *dsi)static void tegra_dsi_connector_reset (struct drm_connector *connector) { -struct tegra_dsi_state *state; - -kfree (connector->state); - connector->state =NULL ; +struct tegra_dsi_state *state = +kzalloc (sizeof (*state),GFP_KERNEL ); - state =kzalloc (sizeof (*state),GFP_KERNEL ); -if (state) - connector->state = &state->base; +if (state) { +kfree (connector->state); +__drm_atomic_helper_connector_reset (connector, &state->base); + } } static struct drm_connector_state *
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH v2 4/6 drm/atomic: add connector mask to drm_crtc_state.
In reply to Maarten LankhorstIt can be useful to iterate over connectors without grabbing connection_mutex. It can also be used to see how many connectors are on a crtc without iterating over the list.
Signed-off-by Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/drm_atomic.c | 11 +++++++++++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 14 insertions(+)
drivers/gpu/drm/drm_atomic.c +11 -0
@@ -1063,10 +1063,21 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, {struct drm_crtc_state *crtc_state; +if (conn_state->crtc && conn_state->crtc != crtc) { + crtc_state =drm_atomic_get_existing_crtc_state (conn_state->state, + conn_state->crtc); + + crtc_state->connector_mask &= + ~(1 <<drm_connector_index (conn_state->connector)); + } +if (crtc) { crtc_state =drm_atomic_get_crtc_state (conn_state->state, crtc);if (IS_ERR (crtc_state))return PTR_ERR (crtc_state); + + crtc_state->connector_mask |= +1 <<drm_connector_index (conn_state->connector); } conn_state->crtc = crtc;
include/drm/drm_crtc.h +3 -0
@@ -301,6 +301,7 @@ struct drm_plane_helper_funcs;* @active_changed: crtc_state->active has been toggled. * @connectors_changed: connectors to this crtc have been updated * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes +* @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @last_vblank_count: for helpers and drivers to capture the vblank of the * update to ensure framebuffer cleanup isn't done too early * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
@@ -334,6 +335,8 @@ struct drm_crtc_state {*/ u32 plane_mask; +u32 connector_mask; +/* last_vblank_count: for vblank waits before cleanup */ u32 last_vblank_count;
Signature · 6 lines
2.1.0 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/6] drm/atomic: add connector mask to drm_crtc_state.
Quoted 5 lines · Maarten Lankhorst
It can be useful to iterate over connectors without grabbing
connection_mutex. It can also be used to see how many connectors are on a crtc without iterating over the list.Signed-off-by Maarten Lankhorst <[email protected]>
Merged up to this patch here, thanks.
-Daniel
Quoted 59 lines
--- drivers/gpu/drm/drm_atomic.c | 11 +++++++++++ include/drm/drm_crtc.h | 3 +++ 2 files changed, 14 insertions(+)drivers/gpu/drm/drm_atomic.c +11
-0@@ -1063,10 +1063,21 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, {struct drm_crtc_state *crtc_state; +if (conn_state->crtc && conn_state->crtc != crtc) { + crtc_state =drm_atomic_get_existing_crtc_state (conn_state->state, + conn_state->crtc); + + crtc_state->connector_mask &= + ~(1 <<drm_connector_index (conn_state->connector)); + } +if (crtc) { crtc_state =drm_atomic_get_crtc_state (conn_state->state, crtc);if (IS_ERR (crtc_state))return PTR_ERR (crtc_state); + + crtc_state->connector_mask |= +1 <<drm_connector_index (conn_state->connector); } conn_state->crtc = crtc;include/drm/drm_crtc.h +3
-0@@ -301,6 +301,7 @@ struct drm_plane_helper_funcs;* @active_changed: crtc_state->active has been toggled. * @connectors_changed: connectors to this crtc have been updated * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes +* @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @last_vblank_count: for helpers and drivers to capture the vblank of the * update to ensure framebuffer cleanup isn't done too early * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings @@ -334,6 +335,8 @@ struct drm_crtc_state {*/ u32 plane_mask; +u32 connector_mask; +/* last_vblank_count: for vblank waits before cleanup */ u32 last_vblank_count;Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH v2 5/6 drm/i915: Update connector_mask during readout.
In reply to Maarten LankhorstSigned-off-by Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_display.c +8 -3
@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the
@@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) {struct intel_connector *connector;struct drm_device *dev = encoder->base.dev; +struct drm_crtc *crtc = encoder->base.crtc;bool active = false;/* We need to check both for a crtc link (meaning that the * encoder is active and trying to read from a pipe) and the * pipe itself being active. */ -bool has_active_crtc = encoder->base.crtc && -to_intel_crtc (encoder->base.crtc)->active; +bool has_active_crtc = crtc && crtc->state->active;for_each_intel_connector (dev, connector) {if (connector->base.encoder != &encoder->base)continue ; active = true; -break ; +if (!has_active_crtc) +break ; + + crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base); }if (active && !has_active_crtc) {
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Quoted 32 lines · Maarten Lankhorst
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/gpu/drm/i915/intel_display.c +3
-2@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) {struct intel_connector *connector;struct drm_device *dev = encoder->base.dev; +struct drm_crtc *crtc = encoder->base.crtc;bool active = false;/* We need to check both for a crtc link (meaning that the * encoder is active and trying to read from a pipe) and the * pipe itself being active. */ -bool has_active_crtc = encoder->base.crtc && -to_intel_crtc (encoder->base.crtc)->active; +bool has_active_crtc = crtc && crtc->state->active;for_each_intel_connector (dev, connector) {
drivers/gpu/drm/i915/intel_display.c static void intel_sanitize_encoder(struct intel_encoder *encoder) · line 15477
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +5
-1if (connector->base.encoder != &encoder->base)continue ; active = true; -break ; +if (!has_active_crtc) +break ; + + crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base);
I still think this is the wrong place. Imo this should be done in intel_modeset_update_connector_atomic_state. It'd be great if we could somehow share the logic with drm_atomic_set_crtc_for_connector even, but that's probably over the top.
-Daniel
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +0
-0}if (active && !has_active_crtc) {Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerRe: [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Op 05-01-16 om 09:35 schreef Daniel Vetter:
Quoted 36 lines · Maarten Lankhorst
Quoted 35 lines
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/gpu/drm/i915/intel_display.c +3
-2@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) {struct intel_connector *connector;struct drm_device *dev = encoder->base.dev; +struct drm_crtc *crtc = encoder->base.crtc;bool active = false;/* We need to check both for a crtc link (meaning that the * encoder is active and trying to read from a pipe) and the * pipe itself being active. */ -bool has_active_crtc = encoder->base.crtc && -to_intel_crtc (encoder->base.crtc)->active; +bool has_active_crtc = crtc && crtc->state->active;for_each_intel_connector (dev, connector) {if (connector->base.encoder != &encoder->base)continue ;
Quoted 12 lines
drivers/gpu/drm/i915/intel_display.c static void intel_sanitize_encoder(struct intel_encoder *encoder) · line 15477
Quoted 7 lines
drivers/gpu/drm/i915/intel_display.c +5
-1active = true; -break ; +if (!has_active_crtc) +break ; + + crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base);I still think this is the wrong place. Imo this should be done in
intel_modeset_update_connector_atomic_state. It'd be great if we could somehow share the logic withdrm_atomic_set_crtc_for_connectoreven, but that's probably over the top.
No it should be done sooner. I want to be able use it anywhere in the .crtc_disable calls without worrying about it..
~Maarten
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Quoted 49 lines · Maarten Lankhorst
Op 05-01-16 om 09:35 schreef Daniel Vetter:
Quoted 36 lines · Maarten Lankhorst
Quoted 35 lines
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/gpu/drm/i915/intel_display.c +3
-2@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) {struct intel_connector *connector;struct drm_device *dev = encoder->base.dev; +struct drm_crtc *crtc = encoder->base.crtc;bool active = false;/* We need to check both for a crtc link (meaning that the * encoder is active and trying to read from a pipe) and the * pipe itself being active. */ -bool has_active_crtc = encoder->base.crtc && -to_intel_crtc (encoder->base.crtc)->active; +bool has_active_crtc = crtc && crtc->state->active;for_each_intel_connector (dev, connector) {if (connector->base.encoder != &encoder->base)continue ;Quoted 12 lines
drivers/gpu/drm/i915/intel_display.c static void intel_sanitize_encoder(struct intel_encoder *encoder) · line 15477
Quoted 7 lines
drivers/gpu/drm/i915/intel_display.c +5
-1active = true; -break ; +if (!has_active_crtc) +break ; + + crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base);I still think this is the wrong place. Imo this should be done in
intel_modeset_update_connector_atomic_state. It'd be great if we could somehow share the logic withdrm_atomic_set_crtc_for_connectoreven, but that's probably over the top.
Quoted 2 lines
No it should be done sooner. I want to be able use it anywhere in the .crtc_disable calls without worrying about it..
Well I don't want to split things up all over. Atm our state recover is a complete mess, and we need to start recovering some order in it. Updating related things at completely different places without even a comment stating why that's required is imo a no-go.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Maarten Lankhorst
maintainerRe: [PATCH v2 5/6] drm/i915: Update connector_mask during readout.
Op 05-01-16 om 10:10 schreef Daniel Vetter:
Quoted 50 lines · Maarten Lankhorst
Quoted 49 lines
Op 05-01-16 om 09:35 schreef Daniel Vetter:
Quoted 48 lines · Maarten Lankhorst
Quoted 32 lines
Signed-off-by Maarten Lankhorst <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/gpu/drm/i915/intel_display.c +3
-2@@ -15421,6 +15421,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -15456,20 +15457,24 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder) {struct intel_connector *connector;struct drm_device *dev = encoder->base.dev; +struct drm_crtc *crtc = encoder->base.crtc;bool active = false;/* We need to check both for a crtc link (meaning that the * encoder is active and trying to read from a pipe) and the * pipe itself being active. */ -bool has_active_crtc = encoder->base.crtc && -to_intel_crtc (encoder->base.crtc)->active; +bool has_active_crtc = crtc && crtc->state->active;for_each_intel_connector (dev, connector) {drivers/gpu/drm/i915/intel_display.c static void intel_sanitize_encoder(struct intel_encoder *encoder) · line 15477
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +5
-1if (connector->base.encoder != &encoder->base)continue ; active = true; -break ; +if (!has_active_crtc) +break ; + + crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base);I still think this is the wrong place. Imo this should be done in
intel_modeset_update_connector_atomic_state. It'd be great if we could somehow share the logic withdrm_atomic_set_crtc_for_connectoreven, but that's probably over the top.
Quoted 6 lines
Quoted 2 lines
No it should be done sooner. I want to be able use it anywhere in the .crtc_disable calls without worrying about it..
Well I don't want to split things up all over. Atm our state recover is a complete mess, and we need to start recovering some order in it. Updating related things at completely different places without even a comment stating why that's required is imo a no-go.
Ok so if I resubmit with a comment in this hunk it will be acceptable?
~Maarten
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH 5/6 drm/i915: Update connector_mask during readout, v2.
In reply to Daniel Vetterdrm/i915: Update connector_mask during readout, v2.
The connector_mask may be used any time during the non-atomic .crtc_disable which is called before the full atomic state is set up and needs to be accurate for that reason.
Changes since v1:
- Update connector_mask in readout_hw_state and add a comment.
Signed-off-by Maarten Lankhorst <[email protected]>
---
drivers/gpu/drm/i915/intel_display.c +16 -1
@@ -15602,6 +15602,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)WARN_ON (drm_atomic_set_mode_for_crtc (crtc->base.state,NULL ) <0 ); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask =0 ;/* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the
@@ -15828,7 +15829,21 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)for_each_intel_connector (dev, connector) {if (connector->get_hw_state (connector)) { connector->base.dpms =DRM_MODE_DPMS_ON ; - connector->base.encoder = &connector->encoder->base; + + encoder = connector->encoder; + connector->base.encoder = &encoder->base; + +if (encoder->base.crtc && + encoder->base.crtc->state->active) { +/* +* This has to be done during hardware readout +* because anything calling .crtc_disable may +* rely on the connector_mask being accurate. +*/ + encoder->base.crtc->state->connector_mask |= +1 <<drm_connector_index (&connector->base); + } + }else { connector->base.dpms =DRM_MODE_DPMS_OFF ; connector->base.encoder =NULL ;
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst
maintainerPATCH v2 6/6 drm/atomic: Remove drm_atomic_connectors_for_crtc.
In reply to Maarten LankhorstNow that connector_mask is reliable there's no need for this function any more.
Signed-off-by Maarten Lankhorst <[email protected]>
--- drivers/gpu/drm/drm_atomic.c | 30 ------------------------------ drivers/gpu/drm/drm_atomic_helper.c | 10 ++++------ drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- include/drm/drm_atomic.h | 4 ---- 4 files changed, 5 insertions(+), 41 deletions(-)
drivers/gpu/drm/drm_atomic.c +0 -30
@@ -1183,36 +1183,6 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,EXPORT_SYMBOL (drm_atomic_add_affected_planes);/** -* drm_atomic_connectors_for_crtc - count number of connected outputs -* @state: atomic state -* @crtc: DRM crtc -* -* This function counts all connectors which will be connected to @crtc -* according to @state. Useful to recompute the enable state for @crtc. -*/ -int -drm_atomic_connectors_for_crtc (struct drm_atomic_state *state, -struct drm_crtc *crtc) -{ -struct drm_connector *connector; -struct drm_connector_state *conn_state; - -int i, num_connected_connectors =0 ; - -for_each_connector_in_state (state, connector, conn_state, i) { -if (conn_state->crtc == crtc) - num_connected_connectors++; - } - -DRM_DEBUG_ATOMIC ("State %p has %i connectors for [CRTC:%d:%s]\n" , - state, num_connected_connectors, - crtc->base.id, crtc->name); - -return num_connected_connectors; -} -EXPORT_SYMBOL (drm_atomic_connectors_for_crtc); - -/** * drm_atomic_legacy_backoff - locking backoff for legacy ioctls * @state: atomic state *
drivers/gpu/drm/drm_atomic_helper.c +4 -6
@@ -464,7 +464,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,* crtc only changed its mode but has the same set of connectors. */ for_each_crtc_in_state (state, crtc, crtc_state, i) { -int num_connectors; +bool has_connectors = + !!crtc_state->connector_mask;/* * We must set ->active_changed after walking connectors for
@@ -493,10 +494,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,if (ret !=0 )return ret; - num_connectors =drm_atomic_connectors_for_crtc (state, - crtc); - -if (crtc_state->enable != !!num_connectors) { +if (crtc_state->enable != has_connectors) {DRM_DEBUG_ATOMIC ("[CRTC:%d:%s] enabled/connectors mismatch\n" , crtc->base.id, crtc->name);
@@ -1755,7 +1753,7 @@ static int update_output_state(struct drm_atomic_state *state,if (crtc == set->crtc)continue ; -if (!drm_atomic_connectors_for_crtc (state, crtc)) { +if (!crtc_state->connector_mask) { ret =drm_atomic_set_mode_prop_for_crtc (crtc_state,NULL );if (ret <0 )
drivers/gpu/drm/vc4/vc4_crtc.c +1 -1
@@ -327,7 +327,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,/* The pixelvalve can only feed one encoder (and encoders are * 1:1 with connectors.) */ -if (drm_atomic_connectors_for_crtc (state->state, crtc) >1 ) +if (hweight32 (state->connector_mask) >1 )return -EINVAL ;drm_atomic_crtc_state_for_each_plane (plane, state) {
include/drm/drm_atomic.h +0 -4
@@ -130,10 +130,6 @@ 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); -void drm_atomic_legacy_backoff (struct drm_atomic_state *state);void
Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v2 6/6] drm/atomic: Remove drm_atomic_connectors_for_crtc.
Quoted 4 lines · Maarten Lankhorst
Now that
connector_maskis reliable there's no need for this function any more.Signed-off-by Maarten Lankhorst <[email protected]>
Since this doesn't touch i915 I figured I can merge this one too. So except for the previous i915 patch it's now all in drm-misc. -Daniel
Quoted 118 lines
--- drivers/gpu/drm/drm_atomic.c | 30 ------------------------------ drivers/gpu/drm/drm_atomic_helper.c | 10 ++++------ drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- include/drm/drm_atomic.h | 4 ---- 4 files changed, 5 insertions(+), 41 deletions(-)drivers/gpu/drm/drm_atomic.c +0
-30@@ -1183,36 +1183,6 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,EXPORT_SYMBOL (drm_atomic_add_affected_planes);/** -* drm_atomic_connectors_for_crtc - count number of connected outputs -* @state: atomic state -* @crtc: DRM crtc -* -* This function counts all connectors which will be connected to @crtc -* according to @state. Useful to recompute the enable state for @crtc. -*/ -int -drm_atomic_connectors_for_crtc (struct drm_atomic_state *state, -struct drm_crtc *crtc) -{ -struct drm_connector *connector; -struct drm_connector_state *conn_state; - -int i, num_connected_connectors =0 ; - -for_each_connector_in_state (state, connector, conn_state, i) { -if (conn_state->crtc == crtc) - num_connected_connectors++; - } - -DRM_DEBUG_ATOMIC ("State %p has %i connectors for [CRTC:%d:%s]\n" , - state, num_connected_connectors, - crtc->base.id, crtc->name); - -return num_connected_connectors; -} -EXPORT_SYMBOL (drm_atomic_connectors_for_crtc); - -/** * drm_atomic_legacy_backoff - locking backoff for legacy ioctls * @state: atomic state * drivers/gpu/drm/drm_atomic_helper.c +4
-6@@ -464,7 +464,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,* crtc only changed its mode but has the same set of connectors. */ for_each_crtc_in_state (state, crtc, crtc_state, i) { -int num_connectors; +bool has_connectors = + !!crtc_state->connector_mask;/* * We must set ->active_changed after walking connectors for @@ -493,10 +494,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,if (ret !=0 )return ret; - num_connectors =drm_atomic_connectors_for_crtc (state, - crtc); - -if (crtc_state->enable != !!num_connectors) { +if (crtc_state->enable != has_connectors) {DRM_DEBUG_ATOMIC ("[CRTC:%d:%s] enabled/connectors mismatch\n" , crtc->base.id, crtc->name);@@ -1755,7 +1753,7 @@ static int update_output_state(struct drm_atomic_state *state,if (crtc == set->crtc)continue ; -if (!drm_atomic_connectors_for_crtc (state, crtc)) { +if (!crtc_state->connector_mask) { ret =drm_atomic_set_mode_prop_for_crtc (crtc_state,NULL );if (ret <0 )drivers/gpu/drm/vc4/vc4_crtc.c +1
-1@@ -327,7 +327,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,/* The pixelvalve can only feed one encoder (and encoders are * 1:1 with connectors.) */ -if (drm_atomic_connectors_for_crtc (state->state, crtc) >1 ) +if (hweight32 (state->connector_mask) >1 )return -EINVAL ;drm_atomic_crtc_state_for_each_plane (plane, state) {include/drm/drm_atomic.h +0
-4@@ -130,10 +130,6 @@ 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); -void drm_atomic_legacy_backoff (struct drm_atomic_state *state);void Signature · 6 lines
2.1.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ warning: Fi.CI.BAT
In reply to Maarten Lankhorst== Summary ==
== Summary ==
Built on c1e9dc2dcb577438a6350c7f1cb36ba8ad0e1dfd drm-intel-nightly: 2016y-01m-04d-09h-35m-16s UTC integration manifest
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-WARN (skl-i5k-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
dmesg-warn -> PASS (ilk-hp8440p)
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (bsw-nuc-2)
dmesg-warn -> PASS (hsw-brixbox)
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS (snb-x220t)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> DMESG-WARN (snb-x220t)
Subgroup read-crc-pipe-a:
dmesg-warn -> PASS (snb-x220t)
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (skl-i7k-2)bdw-nuci7 total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6 bsw-nuc-2 total:135 pass:113 dwarn:2 dfail:0 fail:0 skip:20 hsw-brixbox total:135 pass:127 dwarn:1 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4 hsw-xps12 total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6 skl-i5k-2 total:135 pass:123 dwarn:4 dfail:0 fail:0 skip:8 skl-i7k-2 total:135 pass:125 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12 snb-x220t total:135 pass:122 dwarn:1 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_1067/
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx