diff mbox series

[13/23] drm/i915: Make hardware readout work on i915.

Message ID 20190920114235.22411-13-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [01/23] drm/i915/dp: Fix dsc bpp calculations, v2. | expand

Commit Message

Maarten Lankhorst Sept. 20, 2019, 11:42 a.m. UTC
Unfortunately I have no way to test this, but it should be correct
if the bios sets up bigjoiner in a sane way.

Skip iterating over bigjoiner slaves, only the master has the state we
care about.

Add the width of the bigjoiner slave to the reconstructed fb.

Hide the bigjoiner slave to userspace, and double the mode on bigjoiner
master.

And last, disable bigjoiner slave from primary if reconstruction fails.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 65 +++++++++++++++++++-
 1 file changed, 62 insertions(+), 3 deletions(-)

Comments

Matt Roper Sept. 27, 2019, 12:49 a.m. UTC | #1
On Fri, Sep 20, 2019 at 01:42:25PM +0200, Maarten Lankhorst wrote:
> Unfortunately I have no way to test this, but it should be correct
> if the bios sets up bigjoiner in a sane way.
> 
> Skip iterating over bigjoiner slaves, only the master has the state we
> care about.
> 
> Add the width of the bigjoiner slave to the reconstructed fb.
> 
> Hide the bigjoiner slave to userspace, and double the mode on bigjoiner
> master.
> 
> And last, disable bigjoiner slave from primary if reconstruction fails.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 65 +++++++++++++++++++-
>  1 file changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8c08c4914c9b..0424a378eb51 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3177,6 +3177,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	struct intel_plane *intel_plane = to_intel_plane(primary);
>  	struct intel_plane_state *intel_state =
>  		to_intel_plane_state(plane_state);
> +	struct intel_crtc_state *crtc_state =
> +		to_intel_crtc_state(intel_crtc->base.state);
>  	struct drm_framebuffer *fb;
>  
>  	if (!plane_config->fb)
> @@ -3199,7 +3201,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  		if (c == &intel_crtc->base)
>  			continue;
>  
> -		if (!to_intel_crtc(c)->active)
> +		if (!to_intel_crtc_state(c->state)->uapi.active)
>  			continue;
>  
>  		state = to_intel_plane_state(c->primary->state);
> @@ -3221,6 +3223,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	 * pretend the BIOS never had it enabled.
>  	 */
>  	intel_plane_disable_noatomic(intel_crtc, intel_plane);
> +	if (crtc_state->bigjoiner) {
> +		struct intel_crtc *slave =
> +			crtc_state->bigjoiner_linked_crtc;
> +
> +		intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary));
> +	}
>  
>  	return;
>  
> @@ -9918,6 +9926,7 @@ static void
>  skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  				 struct intel_initial_plane_config *plane_config)
>  {
> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> @@ -10021,6 +10030,18 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  	fb->height = ((val >> 16) & 0xffff) + 1;
>  	fb->width = ((val >> 0) & 0xffff) + 1;
>  
> +	/* add bigjoiner slave as well, if the fb stretches both */
> +	if (crtc_state->bigjoiner) {
> +		enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe;
> +
> +		if (fb->width == crtc_state->pipe_src_w &&

I just noticed that we don't seem to readout PLANE_POS or scaler state
when the scalers are in plane mode rather than pfit.  I guess we haven't
found the BIOS to use plane scaling in the past, but I wonder if that
might become something we have to worry about in bigjoiner cases?  I
could definitely imagine BIOS centering a framebuffer in the middle of a
panel (so it doesn't cover either pipe completely).

> +		    (I915_READ(PLANE_SURF(bigjoiner_pipe, plane_id)) & 0xfffff000) == plane_config->base) {
> +			val = I915_READ(PLANE_SIZE(crtc_state->bigjoiner_linked_crtc->pipe, plane_id));
> +			fb->height += ((val >> 16) & 0xfff) + 1;

On gen10 and below there were only 12 bits for height, but that bumped
up to 13 on gen11 (which are the platforms that have bigjoiner), so this
should be 0x1fff like below.  Although we should probably just mask with
0xffff to be consistent with the regular height/width readout farther up
(the extra bits are MBZ so it should be harmless to include them).

> +			fb->width += ((val >> 0) & 0x1fff) + 1;
> +		}
> +	}
> +
>  	val = I915_READ(PLANE_STRIDE(pipe, plane_id));
>  	stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0);
>  	fb->pitches[0] = (val & 0x3ff) * stride_mult;
> @@ -16814,7 +16835,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  
>  	/* Adjust the state of the output pipe according to whether we
>  	 * have active connectors/encoders. */
> -	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc))
> +	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
> +	    !crtc_state->bigjoiner_slave)
>  		intel_crtc_disable_noatomic(&crtc->base, ctx);
>  
>  	if (crtc_state->hw.active || HAS_GMCH(dev_priv)) {
> @@ -17136,6 +17158,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  		struct intel_plane *plane;
>  		int min_cdclk = 0;
>  
> +		if (crtc_state->bigjoiner_slave)
> +			continue;
> +
>  		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
>  		if (crtc_state->hw.active) {
>  			intel_mode_from_pipe_config(&crtc->base.mode, crtc_state);
> @@ -17144,7 +17169,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			intel_mode_from_pipe_config(&crtc_state->hw.adjusted_mode,
>  						    crtc_state);
>  			crtc_state->hw.mode = crtc->base.mode;
> -			WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
> +			if (!crtc_state->bigjoiner_slave)

This is always true since we added the 'continue' just above for slave
crtc's.



Matt

> +				WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
>  
>  			/*
>  			 * The initial mode needs to be set in order to keep
> @@ -17190,6 +17216,39 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  		intel_bw_crtc_update(bw_state, crtc_state);
>  		copy_hw_to_uapi_state(crtc_state);
>  		intel_pipe_config_sanity_check(dev_priv, crtc_state);
> +
> +		/* discard our incomplete slave state, copy it from master */
> +		if (crtc_state->bigjoiner && crtc_state->hw.active) {
> +			struct intel_crtc *slave = crtc_state->bigjoiner_linked_crtc;
> +			struct intel_crtc_state *slave_crtc_state =
> +				to_intel_crtc_state(slave->base.state);
> +
> +			copy_bigjoiner_crtc_state(slave_crtc_state, crtc_state);
> +			slave->base.mode = crtc->base.mode;
> +
> +			dev_priv->min_cdclk[slave->pipe] = min_cdclk;
> +			dev_priv->min_voltage_level[slave->pipe] =
> +				crtc_state->min_voltage_level;
> +
> +			for_each_intel_plane_on_crtc(&dev_priv->drm, slave, plane) {
> +				const struct intel_plane_state *plane_state =
> +					to_intel_plane_state(plane->base.state);
> +
> +				/*
> +				* FIXME don't have the fb yet, so can't
> +				* use intel_plane_data_rate() :(
> +				*/
> +				if (plane_state->base.visible)
> +					crtc_state->data_rate[plane->id] =
> +						4 * crtc_state->pixel_rate;
> +				else
> +					crtc_state->data_rate[plane->id] = 0;
> +			}
> +
> +			intel_bw_crtc_update(bw_state, slave_crtc_state);
> +			drm_calc_timestamping_constants(&slave->base,
> +							&slave_crtc_state->hw.adjusted_mode);
> +		}
>  	}
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8c08c4914c9b..0424a378eb51 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3177,6 +3177,8 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	struct intel_plane *intel_plane = to_intel_plane(primary);
 	struct intel_plane_state *intel_state =
 		to_intel_plane_state(plane_state);
+	struct intel_crtc_state *crtc_state =
+		to_intel_crtc_state(intel_crtc->base.state);
 	struct drm_framebuffer *fb;
 
 	if (!plane_config->fb)
@@ -3199,7 +3201,7 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 		if (c == &intel_crtc->base)
 			continue;
 
-		if (!to_intel_crtc(c)->active)
+		if (!to_intel_crtc_state(c->state)->uapi.active)
 			continue;
 
 		state = to_intel_plane_state(c->primary->state);
@@ -3221,6 +3223,12 @@  intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * pretend the BIOS never had it enabled.
 	 */
 	intel_plane_disable_noatomic(intel_crtc, intel_plane);
+	if (crtc_state->bigjoiner) {
+		struct intel_crtc *slave =
+			crtc_state->bigjoiner_linked_crtc;
+
+		intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary));
+	}
 
 	return;
 
@@ -9918,6 +9926,7 @@  static void
 skylake_get_initial_plane_config(struct intel_crtc *crtc,
 				 struct intel_initial_plane_config *plane_config)
 {
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
@@ -10021,6 +10030,18 @@  skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->height = ((val >> 16) & 0xffff) + 1;
 	fb->width = ((val >> 0) & 0xffff) + 1;
 
+	/* add bigjoiner slave as well, if the fb stretches both */
+	if (crtc_state->bigjoiner) {
+		enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe;
+
+		if (fb->width == crtc_state->pipe_src_w &&
+		    (I915_READ(PLANE_SURF(bigjoiner_pipe, plane_id)) & 0xfffff000) == plane_config->base) {
+			val = I915_READ(PLANE_SIZE(crtc_state->bigjoiner_linked_crtc->pipe, plane_id));
+			fb->height += ((val >> 16) & 0xfff) + 1;
+			fb->width += ((val >> 0) & 0x1fff) + 1;
+		}
+	}
+
 	val = I915_READ(PLANE_STRIDE(pipe, plane_id));
 	stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
@@ -16814,7 +16835,8 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc,
 
 	/* Adjust the state of the output pipe according to whether we
 	 * have active connectors/encoders. */
-	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc))
+	if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) &&
+	    !crtc_state->bigjoiner_slave)
 		intel_crtc_disable_noatomic(&crtc->base, ctx);
 
 	if (crtc_state->hw.active || HAS_GMCH(dev_priv)) {
@@ -17136,6 +17158,9 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		struct intel_plane *plane;
 		int min_cdclk = 0;
 
+		if (crtc_state->bigjoiner_slave)
+			continue;
+
 		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
 		if (crtc_state->hw.active) {
 			intel_mode_from_pipe_config(&crtc->base.mode, crtc_state);
@@ -17144,7 +17169,8 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			intel_mode_from_pipe_config(&crtc_state->hw.adjusted_mode,
 						    crtc_state);
 			crtc_state->hw.mode = crtc->base.mode;
-			WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
+			if (!crtc_state->bigjoiner_slave)
+				WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode));
 
 			/*
 			 * The initial mode needs to be set in order to keep
@@ -17190,6 +17216,39 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		intel_bw_crtc_update(bw_state, crtc_state);
 		copy_hw_to_uapi_state(crtc_state);
 		intel_pipe_config_sanity_check(dev_priv, crtc_state);
+
+		/* discard our incomplete slave state, copy it from master */
+		if (crtc_state->bigjoiner && crtc_state->hw.active) {
+			struct intel_crtc *slave = crtc_state->bigjoiner_linked_crtc;
+			struct intel_crtc_state *slave_crtc_state =
+				to_intel_crtc_state(slave->base.state);
+
+			copy_bigjoiner_crtc_state(slave_crtc_state, crtc_state);
+			slave->base.mode = crtc->base.mode;
+
+			dev_priv->min_cdclk[slave->pipe] = min_cdclk;
+			dev_priv->min_voltage_level[slave->pipe] =
+				crtc_state->min_voltage_level;
+
+			for_each_intel_plane_on_crtc(&dev_priv->drm, slave, plane) {
+				const struct intel_plane_state *plane_state =
+					to_intel_plane_state(plane->base.state);
+
+				/*
+				* FIXME don't have the fb yet, so can't
+				* use intel_plane_data_rate() :(
+				*/
+				if (plane_state->base.visible)
+					crtc_state->data_rate[plane->id] =
+						4 * crtc_state->pixel_rate;
+				else
+					crtc_state->data_rate[plane->id] = 0;
+			}
+
+			intel_bw_crtc_update(bw_state, slave_crtc_state);
+			drm_calc_timestamping_constants(&slave->base,
+							&slave_crtc_state->hw.adjusted_mode);
+		}
 	}
 }