diff mbox

[v7,4/4] drm/dp: Track MST link bandwidth

Message ID 1492753893-3748-5-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan April 21, 2017, 5:51 a.m. UTC
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

Use the added helpers to track MST link bandwidth for atomic modesets.
Link bw is acquired in the ->atomic_check() phase when CRTCs are being
enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
Similarly, link bw is released during ->atomic_check() with the connector
helper callback ->atomic_release() when CRTCs are disabled.

v5: Implement connector->atomic_check() in place of atomic_release()
v4: Return an int from intel_dp_mst_atomic_release() (Maarten)
v3:
 Handled the case where ->atomic_release() is called more than once
 during an atomic_check() for the same state.
v2:
 Squashed atomic_release() implementation and caller (Daniel)
 Fixed logic for connector-crtc switching case (Daniel)
 Fixed logic for suspend-resume case.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Harry Wentland <Harry.wentland@amd.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 57 +++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

Comments

Maarten Lankhorst April 25, 2017, 7:51 a.m. UTC | #1
On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
>
> Use the added helpers to track MST link bandwidth for atomic modesets.
> Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> Similarly, link bw is released during ->atomic_check() with the connector
> helper callback ->atomic_release() when CRTCs are disabled.
>
> v5: Implement connector->atomic_check() in place of atomic_release()
> v4: Return an int from intel_dp_mst_atomic_release() (Maarten)
> v3:
>  Handled the case where ->atomic_release() is called more than once
>  during an atomic_check() for the same state.
> v2:
>  Squashed atomic_release() implementation and caller (Daniel)
>  Fixed logic for connector-crtc switching case (Daniel)
>  Fixed logic for suspend-resume case.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Harry Wentland <Harry.wentland@amd.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 57 +++++++++++++++++++++++++++++++++----
>  1 file changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 5af22a7..20c557c 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct intel_connector *connector =
>  		to_intel_connector(conn_state->connector);
> -	struct drm_atomic_state *state;
> +	struct drm_atomic_state *state = pipe_config->base.state;
>  	int bpp;
> -	int lane_count, slots;
> +	int lane_count, slots = 0;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int mst_pbn;
>  
> @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	pipe_config->pipe_bpp = bpp;
>  	pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
>  
> -	state = pipe_config->base.state;
> -
>  	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))
>  		pipe_config->has_audio = true;
> -	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  
> +	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  	pipe_config->pbn = mst_pbn;
> -	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);
>  
>  	intel_link_compute_m_n(bpp, lane_count,
>  			       adjusted_mode->crtc_clock,
>  			       pipe_config->port_clock,
>  			       &pipe_config->dp_m_n);
>  
> +	if (pipe_config->base.active) {
Is this check needed? If so it needs a comment as to why.

I know that the output won't actually turn on, but if the crtc is configured vcpi slots should be allocated if possible.

Else the following will fail in the wrong place:
1. Configure CRTC with a valid mode and DP-MST connector, but set active=false. connectors_changed = true, mode_changed = true. Not enough VCPI slots to enable all crtc's at the same time now, but this returns OK.
2. Commit with active property set to true. There are too few vcpi slots to make this work, this atomic commit returns -EINVAL. This might happen on a different crtc than the newly configured one in stap 1, if they were all previously set to !active.

> +		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
> +					      connector->port, mst_pbn);
> +		if (slots < 0) {
> +			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
> +			return false;
> +		}
> +	}
>  	pipe_config->dp_m_n.tu = slots;
>  
>  	return true;
> +}
>  
> +static inline bool release_resources(struct drm_crtc_state *crtc_state)
> +{
> +	return (crtc_state->connectors_changed ||
> +		crtc_state->mode_changed ||
> +		(crtc_state->active_changed && !crtc_state->active));
> +}
> +
> +static int intel_dp_mst_atomic_check(struct drm_connector *connector,
> +		struct drm_connector_state *new_conn_state)
> +{
> +	struct drm_atomic_state *state = new_conn_state->state;
> +	struct drm_connector_state *old_conn_state;
> +	struct drm_crtc *old_crtc;
> +	struct drm_crtc_state *crtc_state;
> +	int slots, ret = 0;
> +
> +	old_conn_state = drm_atomic_get_old_connector_state(state, connector);
> +	old_crtc = old_conn_state->crtc;
> +	if (!old_crtc)
> +		return 0;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);
> +	slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;
> +
> +	if (release_resources(crtc_state) && slots > 0) {
> +		struct drm_dp_mst_topology_mgr *mgr;
> +		struct drm_encoder *old_encoder;
> +
> +		old_encoder = old_conn_state->best_encoder;
> +		mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;
> +
> +		ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);
> +		if (ret)
> +			DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret);
> +		else
> +			to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0;
> +	}
> +	return ret;
>  }
>  
>  static void intel_mst_disable_dp(struct intel_encoder *encoder,
> @@ -378,6 +422,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun
>  	.mode_valid = intel_dp_mst_mode_valid,
>  	.atomic_best_encoder = intel_mst_atomic_best_encoder,
>  	.best_encoder = intel_mst_best_encoder,
> +	.atomic_check = intel_dp_mst_atomic_check,
>  };
>  
>  static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)
Dhinakaran Pandiyan April 26, 2017, 11:48 p.m. UTC | #2
On Tue, 2017-04-25 at 09:51 +0200, Maarten Lankhorst wrote:
> On 21-04-17 07:51, Dhinakaran Pandiyan wrote:

> > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

> >

> > Use the added helpers to track MST link bandwidth for atomic modesets.

> > Link bw is acquired in the ->atomic_check() phase when CRTCs are being

> > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().

> > Similarly, link bw is released during ->atomic_check() with the connector

> > helper callback ->atomic_release() when CRTCs are disabled.

> >

> > v5: Implement connector->atomic_check() in place of atomic_release()

> > v4: Return an int from intel_dp_mst_atomic_release() (Maarten)

> > v3:

> >  Handled the case where ->atomic_release() is called more than once

> >  during an atomic_check() for the same state.

> > v2:

> >  Squashed atomic_release() implementation and caller (Daniel)

> >  Fixed logic for connector-crtc switching case (Daniel)

> >  Fixed logic for suspend-resume case.

> >

> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> > Cc: Archit Taneja <architt@codeaurora.org>

> > Cc: Chris Wilson <chris@chris-wilson.co.uk>

> > Cc: Harry Wentland <Harry.wentland@amd.com>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_dp_mst.c | 57 +++++++++++++++++++++++++++++++++----

> >  1 file changed, 51 insertions(+), 6 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c

> > index 5af22a7..20c557c 100644

> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c

> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c

> > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,

> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;

> >  	struct intel_connector *connector =

> >  		to_intel_connector(conn_state->connector);

> > -	struct drm_atomic_state *state;

> > +	struct drm_atomic_state *state = pipe_config->base.state;

> >  	int bpp;

> > -	int lane_count, slots;

> > +	int lane_count, slots = 0;

> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;

> >  	int mst_pbn;

> >  

> > @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,

> >  	pipe_config->pipe_bpp = bpp;

> >  	pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);

> >  

> > -	state = pipe_config->base.state;

> > -

> >  	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))

> >  		pipe_config->has_audio = true;

> > -	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);

> >  

> > +	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);

> >  	pipe_config->pbn = mst_pbn;

> > -	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);

> >  

> >  	intel_link_compute_m_n(bpp, lane_count,

> >  			       adjusted_mode->crtc_clock,

> >  			       pipe_config->port_clock,

> >  			       &pipe_config->dp_m_n);

> >  

> > +	if (pipe_config->base.active) {

> Is this check needed? If so it needs a comment as to why.

> 

> I know that the output won't actually turn on, but if the crtc is configured vcpi slots should be allocated if possible.

> 

> Else the following will fail in the wrong place:

> 1. Configure CRTC with a valid mode and DP-MST connector, but set active=false. connectors_changed = true, mode_changed = true. Not enough VCPI slots to enable all crtc's at the same time now, but this returns OK.

> 2. Commit with active property set to true. There are too few vcpi slots to make this work, this atomic commit returns -EINVAL. This might happen on a different crtc than the newly configured one in stap 1, if they were all previously set to !active.

> 

Thanks for the review. 
 
I am not sure if it's supposed to fail earlier i.e, when active=false,
connectors_changed = true, mode_changed = true.

In your example, 
1. We have enough vcpi slots for crtc1 and crtc2- for both active=false
and enable=true.
2. Now, crtc3 is also configured with active=false and it does not have
enough bandwidth to be active along with crtc1 and crtc2. 
3. You are saying the problem is that if there is a commit with crtc3,
crtc2, crtc1, in that order, setting active=true, then the commit will
fail?


I don't know how likely this scenario is, but what if userspace is not
going to set crtc1 and crtc2 active=true at all and we return -EINVAL in
Step2 for crtc3?


iiuc we disable pipes, drop payload allocations, disable shared_dpll
when active goes from true->false, even if enable is still true. For eg.
this happens when I 'xset dpms force standby' without these patches. If
we are dropping resources when active goes true->false with enable=true,
shouldn't we also acquire only when active changes from false->true ?


-DK

> > +		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,

> > +					      connector->port, mst_pbn);

> > +		if (slots < 0) {

> > +			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);

> > +			return false;

> > +		}

> > +	}

> >  	pipe_config->dp_m_n.tu = slots;

> >  

> >  	return true;

> > +}

> >  

> > +static inline bool release_resources(struct drm_crtc_state *crtc_state)

> > +{

> > +	return (crtc_state->connectors_changed ||

> > +		crtc_state->mode_changed ||

> > +		(crtc_state->active_changed && !crtc_state->active));

> > +}

> > +

> > +static int intel_dp_mst_atomic_check(struct drm_connector *connector,

> > +		struct drm_connector_state *new_conn_state)

> > +{

> > +	struct drm_atomic_state *state = new_conn_state->state;

> > +	struct drm_connector_state *old_conn_state;

> > +	struct drm_crtc *old_crtc;

> > +	struct drm_crtc_state *crtc_state;

> > +	int slots, ret = 0;

> > +

> > +	old_conn_state = drm_atomic_get_old_connector_state(state, connector);

> > +	old_crtc = old_conn_state->crtc;

> > +	if (!old_crtc)

> > +		return 0;

> > +

> > +	crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);

> > +	slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;

> > +

> > +	if (release_resources(crtc_state) && slots > 0) {

> > +		struct drm_dp_mst_topology_mgr *mgr;

> > +		struct drm_encoder *old_encoder;

> > +

> > +		old_encoder = old_conn_state->best_encoder;

> > +		mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;

> > +

> > +		ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);

> > +		if (ret)

> > +			DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret);

> > +		else

> > +			to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0;

> > +	}

> > +	return ret;

> >  }

> >  

> >  static void intel_mst_disable_dp(struct intel_encoder *encoder,

> > @@ -378,6 +422,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun

> >  	.mode_valid = intel_dp_mst_mode_valid,

> >  	.atomic_best_encoder = intel_mst_atomic_best_encoder,

> >  	.best_encoder = intel_mst_best_encoder,

> > +	.atomic_check = intel_dp_mst_atomic_check,

> >  };

> >  

> >  static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)

> 

>
Dhinakaran Pandiyan April 28, 2017, 12:32 a.m. UTC | #3
On Tue, 2017-04-25 at 09:51 +0200, Maarten Lankhorst wrote:
> On 21-04-17 07:51, Dhinakaran Pandiyan wrote:

> > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

> >

> > Use the added helpers to track MST link bandwidth for atomic modesets.

> > Link bw is acquired in the ->atomic_check() phase when CRTCs are being

> > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().

> > Similarly, link bw is released during ->atomic_check() with the connector

> > helper callback ->atomic_release() when CRTCs are disabled.

> >

> > v5: Implement connector->atomic_check() in place of atomic_release()

> > v4: Return an int from intel_dp_mst_atomic_release() (Maarten)

> > v3:

> >  Handled the case where ->atomic_release() is called more than once

> >  during an atomic_check() for the same state.

> > v2:

> >  Squashed atomic_release() implementation and caller (Daniel)

> >  Fixed logic for connector-crtc switching case (Daniel)

> >  Fixed logic for suspend-resume case.

> >

> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> > Cc: Archit Taneja <architt@codeaurora.org>

> > Cc: Chris Wilson <chris@chris-wilson.co.uk>

> > Cc: Harry Wentland <Harry.wentland@amd.com>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  drivers/gpu/drm/i915/intel_dp_mst.c | 57 +++++++++++++++++++++++++++++++++----

> >  1 file changed, 51 insertions(+), 6 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c

> > index 5af22a7..20c557c 100644

> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c

> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c

> > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,

> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;

> >  	struct intel_connector *connector =

> >  		to_intel_connector(conn_state->connector);

> > -	struct drm_atomic_state *state;

> > +	struct drm_atomic_state *state = pipe_config->base.state;

> >  	int bpp;

> > -	int lane_count, slots;

> > +	int lane_count, slots = 0;

> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;

> >  	int mst_pbn;

> >  

> > @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,

> >  	pipe_config->pipe_bpp = bpp;

> >  	pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);

> >  

> > -	state = pipe_config->base.state;

> > -

> >  	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))

> >  		pipe_config->has_audio = true;

> > -	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);

> >  

> > +	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);

> >  	pipe_config->pbn = mst_pbn;

> > -	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);

> >  

> >  	intel_link_compute_m_n(bpp, lane_count,

> >  			       adjusted_mode->crtc_clock,

> >  			       pipe_config->port_clock,

> >  			       &pipe_config->dp_m_n);

> >  

> > +	if (pipe_config->base.active) {

> Is this check needed? If so it needs a comment as to why.

> 

> I know that the output won't actually turn on, but if the crtc is configured vcpi slots should be allocated if possible.

> 

> Else the following will fail in the wrong place:

> 1. Configure CRTC with a valid mode and DP-MST connector, but set active=false. connectors_changed = true, mode_changed = true. Not enough VCPI slots to enable all crtc's at the same time now, but this returns OK.

> 2. Commit with active property set to true. There are too few vcpi slots to make this work, this atomic commit returns -EINVAL. This might happen on a different crtc than the newly configured one in stap 1, if they were all previously set to !active.

> 


I gave this more thought, what you are recommending is not allocating
vcpi for active false->true but instead doing it only for enable
false->true. The problem I see is encoder->compute_config() is called to
configure the link for a modeset with active=false->true transition. If
we don't want to allocate vcpi slots in this case, we should not be
doing other config. computations as well i.e., not call
encoder->compute_config() at all for active_changed. This means we
should complete this FIXME in intel_atomic_check()

 /* FIXME: For only active_changed we shouldn't need to do any
                 * state recomputation at all. */ 

Otoh if you want me to remove 'if (pipe_config->base.active)', we can do
this - 

a) allocate slots unconditionally in compute_config()
        intel_link_compute_m_n(bpp, lane_count,
                               adjusted_mode->crtc_clock,
                               pipe_config->port_clock,
                               &pipe_config->dp_m_n);

+       slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
+                                             connector->port, mst_pbn);
+       if (slots < 0) {
+               DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
+               return false;
+       }
        pipe_config->dp_m_n.tu = slots;

b) release slots on connectors_changed||mode_changed||active_changed. 
 
+       if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) {
+               struct drm_dp_mst_topology_mgr *mgr;
+               struct drm_encoder *old_encoder;
+
+               old_encoder = old_conn_state->best_encoder;
+               mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;
+
+               ret = drm_dp_atomic_release_vcpi_slots(state, mgr,
slots);

Full patch - https://pastebin.ubuntu.com/24469988/
This assumes encoder->compute_config() will be called only once for a
state.

-DK



> > +		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,

> > +					      connector->port, mst_pbn);

> > +		if (slots < 0) {

> > +			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);

> > +			return false;

> > +		}

> > +	}

> >  	pipe_config->dp_m_n.tu = slots;

> >  

> >  	return true;

> > +}

> >  

> > +static inline bool release_resources(struct drm_crtc_state *crtc_state)

> > +{

> > +	return (crtc_state->connectors_changed ||

> > +		crtc_state->mode_changed ||

> > +		(crtc_state->active_changed && !crtc_state->active));

> > +}

> > +

> > +static int intel_dp_mst_atomic_check(struct drm_connector *connector,

> > +		struct drm_connector_state *new_conn_state)

> > +{

> > +	struct drm_atomic_state *state = new_conn_state->state;

> > +	struct drm_connector_state *old_conn_state;

> > +	struct drm_crtc *old_crtc;

> > +	struct drm_crtc_state *crtc_state;

> > +	int slots, ret = 0;

> > +

> > +	old_conn_state = drm_atomic_get_old_connector_state(state, connector);

> > +	old_crtc = old_conn_state->crtc;

> > +	if (!old_crtc)

> > +		return 0;

> > +

> > +	crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);

> > +	slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;

> > +

> > +	if (release_resources(crtc_state) && slots > 0) {

> > +		struct drm_dp_mst_topology_mgr *mgr;

> > +		struct drm_encoder *old_encoder;

> > +

> > +		old_encoder = old_conn_state->best_encoder;

> > +		mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;

> > +

> > +		ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);

> > +		if (ret)

> > +			DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret);

> > +		else

> > +			to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0;

> > +	}

> > +	return ret;

> >  }

> >  

> >  static void intel_mst_disable_dp(struct intel_encoder *encoder,

> > @@ -378,6 +422,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun

> >  	.mode_valid = intel_dp_mst_mode_valid,

> >  	.atomic_best_encoder = intel_mst_atomic_best_encoder,

> >  	.best_encoder = intel_mst_best_encoder,

> > +	.atomic_check = intel_dp_mst_atomic_check,

> >  };

> >  

> >  static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)

> 

> 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5af22a7..20c557c 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -39,9 +39,9 @@  static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
-	struct drm_atomic_state *state;
+	struct drm_atomic_state *state = pipe_config->base.state;
 	int bpp;
-	int lane_count, slots;
+	int lane_count, slots = 0;
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	int mst_pbn;
 
@@ -63,24 +63,68 @@  static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	pipe_config->pipe_bpp = bpp;
 	pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
 
-	state = pipe_config->base.state;
-
 	if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port))
 		pipe_config->has_audio = true;
-	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
 
+	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
 	pipe_config->pbn = mst_pbn;
-	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);
 
 	intel_link_compute_m_n(bpp, lane_count,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n);
 
+	if (pipe_config->base.active) {
+		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
+					      connector->port, mst_pbn);
+		if (slots < 0) {
+			DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
+			return false;
+		}
+	}
 	pipe_config->dp_m_n.tu = slots;
 
 	return true;
+}
 
+static inline bool release_resources(struct drm_crtc_state *crtc_state)
+{
+	return (crtc_state->connectors_changed ||
+		crtc_state->mode_changed ||
+		(crtc_state->active_changed && !crtc_state->active));
+}
+
+static int intel_dp_mst_atomic_check(struct drm_connector *connector,
+		struct drm_connector_state *new_conn_state)
+{
+	struct drm_atomic_state *state = new_conn_state->state;
+	struct drm_connector_state *old_conn_state;
+	struct drm_crtc *old_crtc;
+	struct drm_crtc_state *crtc_state;
+	int slots, ret = 0;
+
+	old_conn_state = drm_atomic_get_old_connector_state(state, connector);
+	old_crtc = old_conn_state->crtc;
+	if (!old_crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);
+	slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;
+
+	if (release_resources(crtc_state) && slots > 0) {
+		struct drm_dp_mst_topology_mgr *mgr;
+		struct drm_encoder *old_encoder;
+
+		old_encoder = old_conn_state->best_encoder;
+		mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr;
+
+		ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);
+		if (ret)
+			DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret);
+		else
+			to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0;
+	}
+	return ret;
 }
 
 static void intel_mst_disable_dp(struct intel_encoder *encoder,
@@ -378,6 +422,7 @@  static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun
 	.mode_valid = intel_dp_mst_mode_valid,
 	.atomic_best_encoder = intel_mst_atomic_best_encoder,
 	.best_encoder = intel_mst_best_encoder,
+	.atomic_check = intel_dp_mst_atomic_check,
 };
 
 static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)