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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
