All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 15/24] drm/i915: Try to make bigjoiner work in atomic check, v2.
Date: Thu, 10 Oct 2019 14:42:20 +0200	[thread overview]
Message-ID: <2887353f-c30c-0e80-3338-4566661ed3da@linux.intel.com> (raw)
In-Reply-To: <20191008194022.GW1208@intel.com>

Op 08-10-2019 om 21:40 schreef Ville Syrjälä:
> On Fri, Oct 04, 2019 at 01:35:05PM +0200, Maarten Lankhorst wrote:
>> When the clock is higher than the dotclock, try with 2 pipes enabled.
>> If we can enable 2, then we will go into big joiner mode, and steal
>> the adjacent crtc.
>>
>> This only links the crtc's in software, no hardware or plane
>> programming is done yet. Blobs are also copied from the master's
>> crtc_state, so it doesn't depend at commit time on the other
>> crtc_state.
>>
>> Changes since v1:
>> - Rename pipe timings to transcoder timings, as they are now different.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_atomic.c   |  15 +-
>>  drivers/gpu/drm/i915/display/intel_atomic.h   |   3 +-
>>  drivers/gpu/drm/i915/display/intel_display.c  | 218 ++++++++++++++++--
>>  .../drm/i915/display/intel_display_types.h    |  11 +-
>>  drivers/gpu/drm/i915/display/intel_dp.c       |  25 +-
>>  5 files changed, 234 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
>> index 4783d7ff4fcf..a5b11bd9da68 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
>> @@ -228,25 +228,26 @@ void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state)
>>  	intel_crtc_put_color_blobs(crtc_state);
>>  }
>>  
>> -void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state)
>> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
>> +				 const struct intel_crtc_state *from_crtc_state)
>>  {
>>  	intel_crtc_put_color_blobs(crtc_state);
>>  
>> -	if (crtc_state->uapi.degamma_lut)
>> +	if (from_crtc_state->uapi.degamma_lut)
>>  		crtc_state->hw.degamma_lut =
>> -			drm_property_blob_get(crtc_state->uapi.degamma_lut);
>> +			drm_property_blob_get(from_crtc_state->uapi.degamma_lut);
>>  	else
>>  		crtc_state->hw.degamma_lut = NULL;
>>  
>> -	if (crtc_state->uapi.gamma_lut)
>> +	if (from_crtc_state->uapi.gamma_lut)
>>  		crtc_state->hw.gamma_lut =
>> -			drm_property_blob_get(crtc_state->uapi.gamma_lut);
>> +			drm_property_blob_get(from_crtc_state->uapi.gamma_lut);
>>  	else
>>  		crtc_state->hw.gamma_lut = NULL;
>>  
>> -	if (crtc_state->uapi.ctm)
>> +	if (from_crtc_state->uapi.ctm)
>>  		crtc_state->hw.ctm =
>> -			drm_property_blob_get(crtc_state->uapi.ctm);
>> +			drm_property_blob_get(from_crtc_state->uapi.ctm);
>>  	else
>>  		crtc_state->hw.ctm = NULL;
>>  }
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
>> index 42be91e0772a..8da84d64aa04 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
>> @@ -36,7 +36,8 @@ struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
>>  void intel_crtc_destroy_state(struct drm_crtc *crtc,
>>  			       struct drm_crtc_state *state);
>>  void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
>> -void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state);
>> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state,
>> +				 const struct intel_crtc_state *from_crtc_state);
>>  struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
>>  void intel_atomic_state_clear(struct drm_atomic_state *state);
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index caab8cfddcbd..c2b3c7b6f39b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -123,7 +123,7 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>>  static int intel_framebuffer_init(struct intel_framebuffer *ifb,
>>  				  struct drm_i915_gem_object *obj,
>>  				  struct drm_mode_fb_cmd2 *mode_cmd);
>> -static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state);
>> +static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
>>  static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
>>  static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
>>  					 const struct intel_link_m_n *m_n,
>> @@ -6308,7 +6308,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>>  	if (intel_crtc_has_dp_encoder(pipe_config))
>>  		intel_dp_set_m_n(pipe_config, M1_N1);
>>  
>> -	intel_set_pipe_timings(pipe_config);
>> +	intel_set_transcoder_timings(pipe_config);
>>  	intel_set_pipe_src_size(pipe_config);
>>  
>>  	if (pipe_config->has_pch_encoder) {
>> @@ -6435,7 +6435,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>>  		intel_dp_set_m_n(pipe_config, M1_N1);
>>  
>>  	if (!transcoder_is_dsi(cpu_transcoder))
>> -		intel_set_pipe_timings(pipe_config);
>> +		intel_set_transcoder_timings(pipe_config);
>>  
>>  	intel_set_pipe_src_size(pipe_config);
>>  
>> @@ -6838,7 +6838,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>>  	if (intel_crtc_has_dp_encoder(pipe_config))
>>  		intel_dp_set_m_n(pipe_config, M1_N1);
>>  
>> -	intel_set_pipe_timings(pipe_config);
>> +	intel_set_transcoder_timings(pipe_config);
>>  	intel_set_pipe_src_size(pipe_config);
>>  
>>  	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) {
>> @@ -6906,7 +6906,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>>  	if (intel_crtc_has_dp_encoder(pipe_config))
>>  		intel_dp_set_m_n(pipe_config, M1_N1);
>>  
>> -	intel_set_pipe_timings(pipe_config);
>> +	intel_set_transcoder_timings(pipe_config);
>>  	intel_set_pipe_src_size(pipe_config);
>>  
>>  	i9xx_set_pipeconf(pipe_config);
>> @@ -7396,7 +7396,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>>  				     struct intel_crtc_state *pipe_config)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> -	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
>> +	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
>>  	int clock_limit = dev_priv->max_dotclk_freq;
>>  
>>  	if (INTEL_GEN(dev_priv) < 4) {
>> @@ -7413,6 +7413,25 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>>  		}
>>  	}
>>  
>> +	/*
>> +	 * copy hw mode to transcoder mode.
>> +	 * This matters mostly for big joiner, which splits the mode in half.
>> +	 */
>> +	pipe_config->hw.transcoder_mode = pipe_config->hw.adjusted_mode;
>> +	if (pipe_config->bigjoiner) {
>> +		/* Make sure the crtc config is halved horizontally */
>> +		adjusted_mode->crtc_clock /= 2;
>> +		adjusted_mode->crtc_hdisplay /= 2;
>> +		adjusted_mode->crtc_hblank_start /= 2;
>> +		adjusted_mode->crtc_hblank_end /= 2;
>> +		adjusted_mode->crtc_hsync_start /= 2;
>> +		adjusted_mode->crtc_hsync_end /= 2;
>> +		adjusted_mode->crtc_htotal /= 2;
>> +		adjusted_mode->crtc_hskew /= 2;
>> +
>> +		pipe_config->pipe_src_w /= 2;
>> +	}
> Hmm. Oh right, we still need to keep the full timings since
> the joiner is between the pipe and transcoder :/
>
> We probably need a check to make sure the non-halved horiz
> timings are even.
>
> hskew we don't use so don't need to frob that.
>
> I wonder if there are other places that need the full values. Quick
> glance only really reveals encoder and audio code. Most encoder code
> I guess doesn't really matter since bigjoiner is DP only. And the DP
> code itself looks fine since this halving happens after
> .compute_confog(). And the audio code only uses crtc_clock on HDMI
> (not sure it should use it there either, but that's a separate issue).
>
>> +
>>  	if (adjusted_mode->crtc_clock > clock_limit) {
>>  		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
>>  			      adjusted_mode->crtc_clock, clock_limit,
>> @@ -8114,13 +8133,13 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
>>  	crtc_state->dpll_hw_state.dpll = dpll;
>>  }
>>  
>> -static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state)
>> +static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>>  {
>>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>  	enum pipe pipe = crtc->pipe;
>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>> -	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.transcoder_mode;
>>  	u32 crtc_vtotal, crtc_vblank_end;
>>  	int vsyncshift = 0;
>>  
>> @@ -8205,8 +8224,8 @@ static bool intel_pipe_is_interlaced(const struct intel_crtc_state *crtc_state)
>>  		return I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK;
>>  }
>>  
>> -static void intel_get_pipe_timings(struct intel_crtc *crtc,
>> -				   struct intel_crtc_state *pipe_config)
>> +static void intel_get_transcoder_timings(struct intel_crtc *crtc,
>> +					 struct intel_crtc_state *pipe_config)
> Missing the transcoder_mode readout + adjusted_mode/2 here?
>
>>  {
>>  	struct drm_device *dev = crtc->base.dev;
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>> @@ -8809,7 +8828,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>>  	if (INTEL_GEN(dev_priv) < 4)
>>  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
>>  
>> -	intel_get_pipe_timings(crtc, pipe_config);
>> +	intel_get_transcoder_timings(crtc, pipe_config);
>>  	intel_get_pipe_src_size(crtc, pipe_config);
>>  
>>  	i9xx_get_pfit_config(crtc, pipe_config);
>> @@ -10045,7 +10064,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>>  		pipe_config->pixel_multiplier = 1;
>>  	}
>>  
>> -	intel_get_pipe_timings(crtc, pipe_config);
>> +	intel_get_transcoder_timings(crtc, pipe_config);
>>  	intel_get_pipe_src_size(crtc, pipe_config);
>>  
>>  	ironlake_get_pfit_config(crtc, pipe_config);
>> @@ -10445,7 +10464,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>  	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
>>  	    INTEL_GEN(dev_priv) >= 11) {
>>  		haswell_get_ddi_port_state(crtc, pipe_config);
>> -		intel_get_pipe_timings(crtc, pipe_config);
>> +		intel_get_transcoder_timings(crtc, pipe_config);
>>  	}
>>  
>>  	intel_get_pipe_src_size(crtc, pipe_config);
>> @@ -11814,6 +11833,7 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
>>  
>>  	if (mode_changed && crtc_state->hw.enable &&
>>  	    dev_priv->display.crtc_compute_clock &&
>> +	    !crtc_state->bigjoiner_slave &&
>>  	    !WARN_ON(crtc_state->shared_dpll)) {
>>  		ret = dev_priv->display.crtc_compute_clock(crtc, crtc_state);
>>  		if (ret)
>> @@ -12278,7 +12298,7 @@ static void copy_uapi_to_hw_state(struct intel_crtc_state *crtc_state)
>>  	crtc_state->hw.active = crtc_state->uapi.active;
>>  	crtc_state->hw.mode = crtc_state->uapi.mode;
>>  	crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
>> -	intel_crtc_copy_color_blobs(crtc_state);
>> +	intel_crtc_copy_color_blobs(crtc_state, crtc_state);
>>  }
>>  
>>  static void copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
>> @@ -12286,7 +12306,48 @@ static void copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
>>  	crtc_state->uapi.enable = crtc_state->hw.enable;
>>  	crtc_state->uapi.active = crtc_state->hw.active;
>>  	crtc_state->uapi.mode = crtc_state->hw.mode;
>> -	crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
>> +	crtc_state->uapi.adjusted_mode = crtc_state->hw.transcoder_mode;
>> +}
>> +
>> +static int
>> +copy_bigjoiner_crtc_state(struct intel_crtc_state *crtc_state,
>> +			  const struct intel_crtc_state *from_crtc_state)
>> +{
>> +	struct intel_crtc_state *saved_state;
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>> +
>> +	saved_state = kmemdup(from_crtc_state, sizeof(*saved_state), GFP_KERNEL);
>> +	if (!saved_state)
>> +		return -ENOMEM;
>> +
>> +	saved_state->uapi = crtc_state->uapi;
>> +	saved_state->scaler_state = crtc_state->scaler_state;
>> +	saved_state->shared_dpll = crtc_state->shared_dpll;
>> +	saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
>> +	saved_state->crc_enabled = crtc_state->crc_enabled;
>> +
>> +	intel_crtc_free_hw_state(crtc_state);
>> +	memcpy(crtc_state, saved_state, sizeof(*crtc_state));
>> +	kfree(saved_state);
>> +
>> +	/* Re-init hw state */
>> +	memset(&crtc_state->hw, 0, sizeof(saved_state->hw));
>> +	crtc_state->hw.enable = from_crtc_state->hw.enable;
>> +	crtc_state->hw.active = from_crtc_state->hw.active;
>> +	crtc_state->hw.mode = from_crtc_state->hw.mode;
>> +	crtc_state->hw.adjusted_mode = from_crtc_state->hw.adjusted_mode;
>> +
>> +	/* Some fixups */
>> +	crtc_state->uapi.mode_changed = from_crtc_state->uapi.mode_changed;
>> +	crtc_state->uapi.connectors_changed = from_crtc_state->uapi.connectors_changed;
>> +	crtc_state->uapi.active_changed = from_crtc_state->uapi.active_changed;
>> +	crtc_state->nv12_planes = crtc_state->c8_planes = crtc_state->update_planes = 0;
>> +	crtc_state->bigjoiner_linked_crtc = to_intel_crtc(from_crtc_state->uapi.crtc);
>> +	crtc_state->bigjoiner_slave = true;
>> +	crtc_state->cpu_transcoder = (enum transcoder)crtc->pipe;
>> +	crtc_state->has_audio = false;
>> +
>> +	return 0;
>>  }
>>  
>>  static int
>> @@ -12459,7 +12520,7 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config)
>>  		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>>  
>>  	/* uapi wants a copy of the adjusted_mode for vblank bookkeeping */
>> -	pipe_config->uapi.adjusted_mode = pipe_config->hw.adjusted_mode;
>> +	pipe_config->uapi.adjusted_mode = pipe_config->hw.transcoder_mode;
>>  
>>  	return 0;
>>  }
>> @@ -13612,6 +13673,109 @@ static int intel_atomic_check_crtcs(struct intel_atomic_state *state)
>>  	return 0;
>>  }
>>  
>> +static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>> +	struct intel_crtc_state *old_crtc_state, *new_crtc_state, *slave_crtc_state, *master_crtc_state;
>> +	struct intel_crtc *crtc, *slave, *master;
>> +	int i, ret = 0;
>> +
>> +	if (INTEL_GEN(dev_priv) < 11)
>> +		return 0;
>> +
>> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>> +					    new_crtc_state, i) {
>> +		if (!old_crtc_state->bigjoiner_slave)
> I keep thinking that points at the slave. "is_bigjoiner_slave" would be
> a less confusing name perhaps.
>
>> +			continue;
>> +
>> +		if (crtc->pipe == PIPE_A) {
>> +			DRM_ERROR("Bigjoiner slave on pipe A?\n");
>> +			return -EINVAL;
>> +		}
> Dead code?
>
>> +
>> +		/* crtc staying in slave mode? */
>> +		if (!new_crtc_state->uapi.enable)
>> +			continue;
>> +
>> +		if (needs_modeset(new_crtc_state) || new_crtc_state->update_pipe) {
> Are these checks even needed? We've already determined we were a
> slave and the uapi crtc is now enabled, so we have to give the
> pipe back to the uapi user no matter what.
>
>> +			master = old_crtc_state->bigjoiner_linked_crtc;
>> +			master_crtc_state = intel_atomic_get_crtc_state(&state->base, master);
>> +			if (IS_ERR(master_crtc_state))
>> +				return PTR_ERR(master_crtc_state);
>> +
>> +			/*
>> +			 * Force modeset on master, to recalculate bigjoiner
>> +			 * state.
>> +			 *
>> +			 * If master_crtc_state was not part of the atomic commit,
>> +			 * we will fail because the master was not deconfigured,
>> +			 * but at least fail below to unify the checks.
>> +			 */
>> +			master_crtc_state->uapi.mode_changed = true;
>> +
>> +			ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
>> +			if (ret)
>> +				return ret;
>> +
>> +			ret = drm_atomic_add_affected_connectors(&state->base, &crtc->base);
>> +			if (ret)
>> +				return ret;
> This whole thing feels a bit late to be doing it. Can't we just do this
> stuff before .compute_config()? Then at least in theory the master could
> have a chance to reconfigure itself to not use the bigjoiner? I suppose
> there is nothing it really can do to make the configuration work, but
> I think it would still feel like the more natural order of things.
>
>> +		}
>> +	}
>> +
>> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>> +					    new_crtc_state, i) {
>> +		if (!new_crtc_state->uapi.enable || !new_crtc_state->bigjoiner) {
>> +			if (!old_crtc_state->bigjoiner)
>> +				continue;
> I'm having a hard time decoding that. What are we trying to do?
Break and merge the bigjoiner slave configs. Although I guess we can probably do this inside the pipe check now. I may have made it too complicated before. :)
>
>> +		}
>> +
>> +		if (!needs_modeset(new_crtc_state) && !new_crtc_state->update_pipe)
>> +			continue;
>> +
>> +		if (new_crtc_state->bigjoiner && !new_crtc_state->bigjoiner_slave) {
>> +			if (1 + crtc->pipe >= INTEL_NUM_PIPES(dev_priv)) {
>> +				DRM_DEBUG_KMS("Big joiner configuration requires CRTC + 1 to be used, doesn't exist\n");
>> +				return -EINVAL;
>> +			}
> I think we should be able to reject such a configuration already
> when we decided that we need to use the bigjoiner. Should probably
> write it with bitmasks to be future proof.
Yeah, see above. I think not future proofing is fine for now. :)
>> +
>> +			slave = new_crtc_state->bigjoiner_linked_crtc =
>> +				intel_get_crtc_for_pipe(dev_priv, crtc->pipe + 1);
>> +			slave_crtc_state = intel_atomic_get_crtc_state(&state->base, slave);
>> +			if (IS_ERR(slave_crtc_state))
>> +				return PTR_ERR(slave_crtc_state);
>> +
>> +			if (slave_crtc_state->uapi.enable) {
>> +				DRM_DEBUG_KMS("[CRTC:%d:%s] Big joiner configuration requires this CRTC to be unconfigured\n",
>> +					      slave->base.base.id, slave->base.name);
>> +				return -EINVAL;
>> +			} else {
>> +				DRM_DEBUG_KMS("[CRTC:%d:%s] Used as slave for big joiner\n",
>> +					      slave->base.base.id, slave->base.name);
>> +				ret = copy_bigjoiner_crtc_state(slave_crtc_state, new_crtc_state);
>> +			}
>> +		} else {
> I guess this means we're always looking to associate the bigjoiner
> master with the uapi crtc? I think in theory we should be able to
> handle the case where the slave is the uapi crtc as well. But maybe
> it doesn't matter so much as userspace probably picks crtcs in order
> anyway. Hmm, maybe it would even be hard to deal with that...
I think just doing the easy thing first makes it less likely to mess up. In theory we can do this, but I think it's not worth handling this special case.
>> +			master = new_crtc_state->bigjoiner_linked_crtc;
>> +			if (!master)
>> +				continue;
>> +
>> +			master_crtc_state = intel_atomic_get_crtc_state(&state->base, master);
>> +			if (IS_ERR(master_crtc_state))
>> +				return PTR_ERR(master_crtc_state);
>> +
>> +			if (!master_crtc_state->uapi.enable && !new_crtc_state->uapi.enable) {
>> +				DRM_DEBUG_KMS("[CRTC:%d:%s] Disabling slave from big joiner\n",
>> +					      crtc->base.base.id, crtc->base.name);
>> +				ret = clear_intel_crtc_state(new_crtc_state);
>> +			}
> This too feels like it could be done before we've compute anything.
>
> Something like this is what I was thinking earlier:
>
> detach_joiner() {
> 	master.bigjoiner = {};
> 	slave.bigjoiner = {};
> }
>
> setup_state_thing() {
> 	if (new_crtc_state->uapi.enable) {
> 		if (is_slave) {
> 			assert(needs_modeset(new_crtc_state));
> 			master = get_linked();
> 			master.modeset = true;
> 			detach_joiner();
> 		} else if (is_master) {
> 			slave = get_linked();
> 			if (needs_modeset(new_crtc_state)) {
> 				slave.modeset = true;
> 				detach_joiner();
> 			}
> 		}
> 		copy_uapi_to_hw(new_crtc_state);
> 	} else {
> 		if (is_master) {
> 			assert(needs_modeset(new_crtc_state));
> 			slave = get_linked();
> 			slave.modeset = true;
> 			detach_joiner();
> 		}
> 		if (!is_slave && needs_modeset(new_crtc_state))
> 			copy_uapi_to_hw(new_crtc_state);
> 	}
> }
>
>   drm_atomic_helper_check_modeset();
> + setup_state_thing();
Yeah, can do it immediately after in intel_crtc_check_fastset(), will fix the series. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-10-10 12:42 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-04 11:34 [PATCH 00/24] Enable bigjoiner support, second approach Maarten Lankhorst
2019-10-04 11:34 ` [PATCH 01/24] HAX to make DSC work on the icelake test system Maarten Lankhorst
2019-10-04 11:34 ` [PATCH 02/24] drm/i915: Fix for_each_intel_plane_mask definition Maarten Lankhorst
2019-10-04 13:14   ` Ville Syrjälä
2019-10-07 19:37   ` Matt Roper
2019-10-04 11:34 ` [PATCH 03/24] drm/i915: Introduce and use intel_atomic_crtc_state_for_each_plane_state Maarten Lankhorst
2019-10-04 13:18   ` Ville Syrjälä
2019-10-07 19:37   ` Matt Roper
2019-10-04 11:34 ` [PATCH 04/24] drm/i915: Remove cursor use of properties for coordinates Maarten Lankhorst
2019-10-04 13:22   ` Ville Syrjälä
2019-10-07 19:37   ` Matt Roper
2019-10-10 12:10     ` Maarten Lankhorst
2019-10-10 14:04     ` Maarten Lankhorst
2019-10-04 11:34 ` [PATCH 05/24] drm/i915: Use intel_plane_state in prepare and cleanup plane_fb Maarten Lankhorst
2019-10-04 13:23   ` Ville Syrjälä
2019-10-07 19:37   ` Matt Roper
2019-10-04 11:34 ` [PATCH 06/24] drm/i915: Remove begin/finish_crtc_commit, v4 Maarten Lankhorst
2019-10-07 19:43   ` Matt Roper
2019-10-04 11:34 ` [PATCH 07/24] drm/i915: Introduce intel_atomic_get_plane_state_after_check() Maarten Lankhorst
2019-10-08 17:03   ` Ville Syrjälä
2019-10-10 11:56     ` Maarten Lankhorst
2019-10-10 12:39       ` Ville Syrjälä
2019-10-10 13:01         ` Maarten Lankhorst
2019-10-04 11:34 ` [PATCH 08/24] drm/i915: Prepare to split crtc state in uapi and hw state Maarten Lankhorst
2019-10-08 17:06   ` Ville Syrjälä
2019-10-10 14:21     ` Maarten Lankhorst
2019-10-10 14:47       ` Ville Syrjälä
2019-10-14  8:20         ` Maarten Lankhorst
2019-10-04 11:34 ` [PATCH 09/24] drm/i915: Handle a few more cases for crtc hw/uapi split Maarten Lankhorst
2019-10-04 13:31   ` Ville Syrjälä
2019-10-04 15:51     ` Maarten Lankhorst
2019-10-04 15:56       ` Ville Syrjälä
2019-10-04 11:35 ` [PATCH 10/24] drm/i915: Complete crtc hw/uapi split, v2 Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 11/24] drm/i915: Preparation for plane split Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 12/24] drm/i915: Split plane hw and uapi state Maarten Lankhorst
2019-10-08 17:42   ` Ville Syrjälä
2019-10-09 12:13     ` Maarten Lankhorst
2019-10-09 12:23       ` Ville Syrjälä
2019-10-09 12:31         ` Maarten Lankhorst
2019-10-09 12:41           ` Ville Syrjälä
2019-10-09 12:58             ` Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 13/24] drm/i915: Stop using drm_atomic_helper_check_planes() Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 14/24] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v2 Maarten Lankhorst
2019-10-08 17:50   ` Ville Syrjälä
2019-10-04 11:35 ` [PATCH 15/24] drm/i915: Try to make bigjoiner work in atomic check, v2 Maarten Lankhorst
2019-10-08 19:40   ` Ville Syrjälä
2019-10-10 12:42     ` Maarten Lankhorst [this message]
2019-10-04 11:35 ` [PATCH 16/24] drm/i915: Enable big joiner support in enable and disable sequences Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 17/24] drm/i915: Make hardware readout work on i915 Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 18/24] drm/i915: Remove special case slave handling during hw programming Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 19/24] drm/i915: Link planes in a bigjoiner configuration, v2 Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 20/24] drm/i915: Add bigjoiner aware plane clipping checks Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 21/24] drm/i915: Ensure color blobs are copied to slave before planes are checked Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 22/24] drm/i915: Add intel_update_bigjoiner handling Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 23/24] drm/i915: Add debugfs dumping for bigjoiner, v2 Maarten Lankhorst
2019-10-04 11:35 ` [PATCH 24/24] semi-hax: drm/i915: Always verify ddb allocation Maarten Lankhorst
2019-10-04 14:23   ` [PATCH] " Maarten Lankhorst
2019-10-04 13:10 ` ✗ Fi.CI.BUILD: failure for Enable bigjoiner support, second approach Patchwork
2019-10-04 18:03 ` ✗ Fi.CI.BUILD: failure for Enable bigjoiner support, second approach. (rev2) Patchwork
2019-10-10 16:25 ` ✗ Fi.CI.BUILD: failure for Enable bigjoiner support, second approach. (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2887353f-c30c-0e80-3338-4566661ed3da@linux.intel.com \
    --to=maarten.lankhorst@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.