Op 22-06-15 om 17:21 schreef Daniel Vetter:
> On Fri, Jun 19, 2015 at 10:02:27AM +0200, Maarten Lankhorst wrote:
>> Now that we read out the full atomic state we can force fastboot without 
>> hacks.
>> The only thing that we worry about is preventing a modeset. This can be 
>> easily
>> done by calculating if sw state matches hw state, with exception for pfit and
>> pipe size. Since the original fastboot code only touched pipe size and panel
>> fitter we keep those, and compare full sw state against hw state.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_params.c   |   5 -
>>  drivers/gpu/drm/i915/intel_display.c | 271 
>> ++++++++++++++++++++++-------------
>>  drivers/gpu/drm/i915/intel_fbdev.c   |  10 +-
>>  3 files changed, 169 insertions(+), 117 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c 
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 8ac5a1b29ac0..9b344a956ba6 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -41,7 +41,6 @@ struct i915_params i915 __read_mostly = {
>>      .preliminary_hw_support = 
>> IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
>>      .disable_power_well = 1,
>>      .enable_ips = 1,
>> -    .fastboot = 0,
>>      .prefault_disable = 0,
>>      .load_detect_test = 0,
>>      .reset = true,
>> @@ -137,10 +136,6 @@ MODULE_PARM_DESC(disable_power_well,
>>  module_param_named(enable_ips, i915.enable_ips, int, 0600);
>>  MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
>>  
>> -module_param_named(fastboot, i915.fastboot, bool, 0600);
>> -MODULE_PARM_DESC(fastboot,
>> -    "Try to skip unnecessary mode sets at boot time (default: false)");
>> -
>>  module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 
>> 0600);
>>  MODULE_PARM_DESC(prefault_disable,
>>      "Disable page prefaulting for pread/pwrite/reloc (default:false). "
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 9b2acc7b4e29..de975ef09e23 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -110,6 +110,7 @@ static void skl_init_scalers(struct drm_device *dev, 
>> struct intel_crtc *intel_cr
>>  static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
>>                         int num_connectors);
>>  static void intel_pre_disable_primary(struct drm_crtc *crtc);
>> +static void skylake_pfit_enable(struct intel_crtc *crtc);
>>  static struct drm_atomic_state *
>>  intel_modeset_setup_hw_state(struct drm_device *dev, bool force_restore);
>>  
>> @@ -3289,14 +3290,17 @@ static bool intel_crtc_has_pending_flip(struct 
>> drm_crtc *crtc)
>>      return pending;
>>  }
>>  
>> -static void intel_update_pipe_size(struct intel_crtc *crtc)
>> +static void intel_update_fastboot(struct intel_crtc *crtc,
>> +                              struct intel_crtc_state *old_crtc_state)
> That's not a useful rename imo - updating pfit/pipe_size clearly tells us
> what's going on, fastboot is much more a marketing thing really. And I
> expect that eventually we'll have to grow a lot more update code to fix up
> fastboot.
>
>>  {
>>      struct drm_device *dev = crtc->base.dev;
>>      struct drm_i915_private *dev_priv = dev->dev_private;
>> -    const struct drm_display_mode *adjusted_mode;
>> +    struct intel_crtc_state *pipe_config =
>> +            to_intel_crtc_state(crtc->base.state);
>>  
>> -    if (!i915.fastboot)
>> -            return;
>> +    DRM_DEBUG_KMS("Applying fastboot quirks, %ix%i -> %ix%i\n",
>> +                  old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
>> +                  pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>>  
>>      /*
>>       * Update pipe size and adjust fitter if needed: the reason for this is
>> @@ -3305,27 +3309,27 @@ static void intel_update_pipe_size(struct intel_crtc 
>> *crtc)
>>       * fastboot case, we'll flip, but if we don't update the pipesrc and
>>       * pfit state, we'll end up with a big fb scanned out into the wrong
>>       * sized surface.
>> -     *
>> -     * To fix this properly, we need to hoist the checks up into
>> -     * compute_mode_changes (or above), check the actual pfit state and
>> -     * whether the platform allows pfit disable with pipe active, and only
>> -     * then update the pipesrc and pfit state, even on the flip path.
>>       */
>>  
>> -    adjusted_mode = &crtc->config->base.adjusted_mode;
>> -
>>      I915_WRITE(PIPESRC(crtc->pipe),
>> -               ((adjusted_mode->crtc_hdisplay - 1) << 16) |
>> -               (adjusted_mode->crtc_vdisplay - 1));
>> -    if (!crtc->config->pch_pfit.enabled &&
>> -        (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
>> -         intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
>> +               ((pipe_config->pipe_src_w - 1) << 16) |
>> +               (pipe_config->pipe_src_h - 1));
>> +
>> +    /* on skylake this is done by detaching scalers */
>> +    if (INTEL_INFO(dev)->gen == 9) {
>> +            skl_detach_scalers(crtc);
>> +
>> +            if (pipe_config->pch_pfit.enabled)
>> +                    skylake_pfit_enable(crtc);
>> +    }
>> +    else if (!pipe_config->pch_pfit.enabled &&
>> +        old_crtc_state->pch_pfit.enabled) {
>> +            DRM_DEBUG_KMS("Disabling panel fitter\n");
>> +
>>              I915_WRITE(PF_CTL(crtc->pipe), 0);
>>              I915_WRITE(PF_WIN_POS(crtc->pipe), 0);
>>              I915_WRITE(PF_WIN_SZ(crtc->pipe), 0);
>>      }
>> -    crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
>> -    crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
>>  }
>>  
>>  static void intel_fdi_normal_train(struct drm_crtc *crtc)
>> @@ -12244,19 +12248,6 @@ encoder_retry:
>>      DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
>>                    base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>>  
>> -    /* Check if we need to force a modeset */
>> -    if (pipe_config->has_audio !=
>> -        to_intel_crtc_state(crtc->state)->has_audio) {
>> -            pipe_config->base.mode_changed = true;
>> -            ret = drm_atomic_add_affected_planes(state, crtc);
>> -    }
>> -
>> -    /*
>> -     * Note we have an issue here with infoframes: current code
>> -     * only updates them on the full mode set path per hw
>> -     * requirements.  So here we should be checking for any
>> -     * required changes and forcing a mode set.
>> -     */
>>  fail:
>>      return ret;
>>  }
>> @@ -12500,29 +12491,21 @@ intel_pipe_config_compare(struct drm_device *dev,
>>                                    DRM_MODE_FLAG_NVSYNC);
>>      }
>>  
>> -    PIPE_CONF_CHECK_I(pipe_src_w);
>> -    PIPE_CONF_CHECK_I(pipe_src_h);
>> -
>> -    /*
>> -     * FIXME: BIOS likes to set up a cloned config with lvds+external
>> -     * screen. Since we don't yet re-compute the pipe config when moving
>> -     * just the lvds port away to another pipe the sw tracking won't match.
>> -     *
>> -     * Proper atomic modesets with recomputed global state will fix this.
>> -     * Until then just don't check gmch state for inherited modes.
>> -     */
>>      if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_INHERITED_MODE)) {
> This seems like a separate change - with recomputing state on all pipes
> unconditionally we should be able to drop this?
We repurpose inherited mode here to skip some checks on first boot that will be 
patched up on first atomic commit.

>> -            PIPE_CONF_CHECK_I(gmch_pfit.control);
>> -            /* pfit ratios are autocomputed by the hw on gen4+ */
>> -            if (INTEL_INFO(dev)->gen < 4)
>> -                    PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
>> -            PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
>> -    }
>> +            PIPE_CONF_CHECK_I(pipe_src_w);
>> +            PIPE_CONF_CHECK_I(pipe_src_h);
>> +
>> +            PIPE_CONF_CHECK_I(pch_pfit.enabled);
>> +            if (current_config->pch_pfit.enabled) {
>> +                    PIPE_CONF_CHECK_I(pch_pfit.pos);
>> +                    PIPE_CONF_CHECK_I(pch_pfit.size);
>>  
>> -    PIPE_CONF_CHECK_I(pch_pfit.enabled);
>> -    if (current_config->pch_pfit.enabled) {
>> -            PIPE_CONF_CHECK_I(pch_pfit.pos);
>> -            PIPE_CONF_CHECK_I(pch_pfit.size);
>> +                    PIPE_CONF_CHECK_I(gmch_pfit.control);
>> +                    /* pfit ratios are autocomputed by the hw on gen4+ */
>> +                    if (INTEL_INFO(dev)->gen < 4)
>> +                            PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
>> +                    PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
>> +            }
>>      }
>>  
>>      PIPE_CONF_CHECK_I(scaler_state.scaler_id);
>> @@ -13057,26 +13040,18 @@ intel_modeset_compute_config(struct 
>> drm_atomic_state *state)
>>              return ret;
>>  
>>      for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> -            if (!crtc_state->enable) {
>> -                    if (needs_modeset(crtc_state))
>> -                            any_ms = true;
>> +            if (!needs_modeset(crtc_state))
>>                      continue;
>> -            }
>>  
>> -            if (!needs_modeset(crtc_state)) {
>> -                    ret = drm_atomic_add_affected_connectors(state, crtc);
>> -                    if (ret)
>> -                            return ret;
>> -            }
>> +            any_ms = true;
>> +            if (!crtc_state->enable)
>> +                    continue;
>>  
>>              ret = intel_modeset_pipe_config(crtc,
>>                                      to_intel_crtc_state(crtc_state));
>>              if (ret)
>>                      return ret;
>>  
>> -            if (needs_modeset(crtc_state))
>> -                    any_ms = true;
>> -
>>              intel_dump_pipe_config(to_intel_crtc(crtc),
>>                                     to_intel_crtc_state(crtc_state),
>>                                     "[modeset]");
>> @@ -13147,7 +13122,10 @@ static int __intel_set_mode(struct drm_atomic_state 
>> *state)
>>              if (needs_modeset(crtc->state) && crtc->state->active) {
>>                      update_scanline_offset(to_intel_crtc(crtc));
>>                      dev_priv->display.crtc_enable(crtc);
>> -            }
>> +            } else if (to_intel_crtc_state(crtc_state)->quirks &
>> +                       PIPE_CONFIG_QUIRK_INHERITED_MODE)
>> +                    /* force hw readout check */
>> +                    any_ms = true;
> Imo this should be a separate patch since forcing a modeset check after
> the initial takeover is a good thing. Also maybe do a
> s/any_ms/do_modeset_checks/ for clarity of what this does.
It's used for other reasons too, like updating power domains when crtc states 
change. So do_modeset_checks would be misleading.
>>  
>>              drm_atomic_helper_commit_planes_on_crtc(crtc_state);
>>      }
>> @@ -13430,8 +13408,6 @@ static int intel_crtc_set_config(struct drm_mode_set 
>> *set)
>>      if (ret)
>>              goto out;
>>  
>> -    intel_update_pipe_size(to_intel_crtc(set->crtc));
>> -
>>      ret = intel_set_mode_checked(state);
>>      if (ret) {
>>              DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
>> @@ -13718,10 +13694,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>>      if (!crtc->state->active)
>>              return;
>>  
>> -    if (state->visible)
>> -            /* FIXME: kill this fastboot hack */
>> -            intel_update_pipe_size(intel_crtc);
>> -
>>      dev_priv->display.update_primary_plane(crtc, fb, crtc->x, crtc->y);
>>  }
>>  
>> @@ -13741,6 +13713,8 @@ static void intel_begin_crtc_commit(struct drm_crtc 
>> *crtc,
>>      struct drm_device *dev = crtc->dev;
>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>      struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +    struct intel_crtc_state *old_intel_state =
>> +            to_intel_crtc_state(old_crtc_state);
>>  
>>      if (!needs_modeset(crtc->state))
>>              intel_pre_plane_update(intel_crtc);
>> @@ -13756,7 +13730,10 @@ static void intel_begin_crtc_commit(struct drm_crtc 
>> *crtc,
>>                      intel_pipe_update_start(intel_crtc,
>>                                              
>> &intel_crtc->atomic.start_vbl_count);
>>  
>> -    if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
>> +    if (!needs_modeset(crtc->state) && crtc->state->active &&
>> +        old_intel_state->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE)
>> +            intel_update_fastboot(intel_crtc, old_intel_state);
> Nack, fastboot only for the initial mode is not what I want. We should be
> able to speed up _any_ modeset where we just disable the pfit. Allowing
> fast pfit removal unconditionally will also allow us to easily test this
> part of fastboot with an igt, which I'd also like to see.
You're right, but I think because this needs more infrastructure. I only want 
to convert fastboot at first.
For MST we might also need a variant which only performs a pipe modeset, rather 
than a full modeset.
Still there are cases where we will need to force a modeset regardless, for 
example when device frequency
changes.

In either case this probably requires a lot of code may differ on each 
platform. :(

>> +    else if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
>>              skl_detach_scalers(intel_crtc);
> Also I think it'd be clearer if we have state flags (like for the pageflip
> stuff) in crtc_state to enable these fixup functions on an as-needed
> basis.
I think this call should be moved to the plane commit function instead, with a 
single
detach_scaler to disable the relevant scaler, if the scaler's not used 
afterwards.

fastboot updates would disable it for the crtc, eliminating the need to call it 
separately.

>>  }
>>  
>> @@ -15096,6 +15073,114 @@ intel_check_plane_mapping(struct intel_crtc *crtc)
>>      return true;
>>  }
>>  
>> +static bool
>> +intel_compare_m_n(unsigned int m, unsigned int n,
>> +              unsigned int m2, unsigned int n2)
>> +{
>> +    if (m == m2 && n == n2)
>> +            return true;
>> +
>> +    if (!m || !n || !m2 || !n2)
>> +            return false;
>> +
>> +    if (m > m2) {
>> +            while (m > m2) {
>> +                    m >>= 1;
>> +                    n >>= 1;
>> +            }
>> +    } else if (m < m2) {
>> +            while (m < m2) {
>> +                    m2 >>= 1;
>> +                    n2 >>= 1;
>> +            }
>> +    }
>> +
>> +    return m == m2 && n == n2;
>> +}
>> +
>> +static bool
>> +intel_compare_link_m_n(const struct intel_link_m_n *m_n,
>> +                   const struct intel_link_m_n *m2_n2)
>> +{
>> +    if (m_n->tu == m2_n2->tu &&
>> +        intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
>> +                          m2_n2->gmch_m, m2_n2->gmch_n) &&
>> +        intel_compare_m_n(m_n->link_m, m_n->link_n,
>> +                          m2_n2->link_m, m2_n2->link_n))
>> +            return true;
>> +
>> +    return false;
>> +}
>> +
>> +static void intel_fastboot_calc_modeset(struct intel_crtc *crtc,
>> +                                    struct intel_crtc_state *hw_state,
>> +                                    struct intel_crtc_state *pipe_config)
>> +{
>> +    struct drm_device *dev = crtc->base.dev;
>> +    struct intel_crtc_state sw_state;
>> +
>> +    if (needs_modeset(&pipe_config->base)) {
>> +            DRM_DEBUG_KMS("Skipping fastboot checks, modeset forced\n");
>> +            return;
>> +    }
>> +
>> +    memset(&sw_state, 0, sizeof(sw_state));
>> +    sw_state.base = pipe_config->base;
>> +    sw_state.scaler_state = pipe_config->scaler_state;
>> +    sw_state.shared_dpll = pipe_config->shared_dpll;
>> +    sw_state.dpll_hw_state = pipe_config->dpll_hw_state;
>> +    sw_state.ddi_pll_sel = pipe_config->ddi_pll_sel;
>> +    intel_modeset_pipe_config(&crtc->base, &sw_state);
>> +    sw_state.quirks = hw_state->quirks & PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS;
>> +
>> +    if (intel_compare_link_m_n(&sw_state.fdi_m_n, &hw_state->fdi_m_n))
>> +            sw_state.fdi_m_n = hw_state->fdi_m_n;
>> +
>> +    if (INTEL_INFO(dev)->gen < 8) {
>> +            if (intel_compare_link_m_n(&sw_state.dp_m_n,
>> +                                       &hw_state->dp_m_n))
>> +                    sw_state.dp_m_n = hw_state->dp_m_n;
>> +
>> +            if (intel_compare_link_m_n(&sw_state.dp_m2_n2,
>> +                                       &hw_state->dp_m2_n2))
>> +                    sw_state.dp_m2_n2 = hw_state->dp_m2_n2;
>> +    } else {
>> +            if (intel_compare_link_m_n(&sw_state.dp_m_n,
>> +                                       &hw_state->dp_m_n)) {
>> +                    sw_state.dp_m_n = hw_state->dp_m_n;
>> +                    hw_state->dp_m2_n2 = sw_state.dp_m2_n2;
>> +            } else if (intel_compare_link_m_n(&sw_state.dp_m2_n2,
>> +                                              &hw_state->dp_m_n)) {
>> +                    sw_state.dp_m2_n2 = hw_state->dp_m_n;
>> +
>> +                    hw_state->dp_m_n = sw_state.dp_m_n;
>> +                    hw_state->dp_m2_n2 = sw_state.dp_m2_n2;
>> +            }
>> +    }
> Not too happy with splitting up the modeset state compare code like this.
> If we add enum mode { EXACT, COMPATIBLE } to intel_pipe_config_compare we
> could tie things together well and have all the pipe config code grouped
> in one place. But then the _compare() is a bit misleading since it'd also
> update the sw_state, so maybe better to call it compare_and_ajust.
Or pass a bool adjust, since checking state after a modeset needs to match 
exactly,
and no adjustment can be done.

> The macros would then either copy the hw_state to sw_state (if it's some
> intermediate state we don't care about) or compare it (with maybe some
> adjustments made). But I'd really like to have all that compare/adjust
> logic in one place if possible.
>
>> +
>> +    /* Check if we need to force a modeset */
>> +    if (!intel_pipe_config_compare(dev, &sw_state, hw_state, true)) {
>> +            DRM_INFO("[CRTC:%i] sw state incompatible, forcing modeset\n",
>> +                     crtc->base.base.id);
>> +            pipe_config->base.mode_changed = true;
>> +    } else {
>> +            DRM_INFO("[CRTC:%i] sw state and hw state fastboot 
>> compatible\n",
>> +                     crtc->base.base.id);
>> +
>> +            *pipe_config = sw_state;
>> +            intel_dump_pipe_config(crtc, pipe_config,
>> +                                   "[new fastboot state]");
>> +
>> +            /* prevent a modeset by patching up mode struct */
>> +
>> +            hw_state->base.mode.type = pipe_config->base.mode.type =
>> +            hw_state->base.adjusted_mode.type = 
>> pipe_config->base.adjusted_mode.type;
>> +
>> +            /* clock readout can be approximate, just copy it. */
>> +            hw_state->base.mode.clock = pipe_config->base.mode.clock = 
>> pipe_config->base.adjusted_mode.clock;
> Why do we still need this? If we use intel_pipe_config_compare to compute
> mode_changed then we hopefully don't need to hack around things any more.
> I guess this is where we need your plan to split out connector_changed
> from mode_changed and then replace the mode_changed computed by the
> helpers by what intel_pipe_config_compare things is justified.
My display has a slightly different clock and mode->type in edid. Patching this 
up prevented the modeset.
There can also be other reasons for modeset, like CDCLK changes.

>> +    }
>> +}
>> +
>>  static void intel_sanitize_crtc(struct intel_crtc *crtc,
>>                              struct intel_crtc_state *pipe_config,
>>                              bool force_restore)
>> @@ -15135,37 +15220,11 @@ static void intel_sanitize_crtc(struct intel_crtc 
>> *crtc,
>>              pipe_config->base.active = !!enable;
>>      }
>>  
>> -    if (hw_state->quirks & PIPE_CONFIG_QUIRK_WRONG_PLANE) {
>> -            if (force_restore || i915.fastboot)
>> -                    pipe_config->base.mode_changed = true;
>> -            else
>> -                    pipe_config->base.active = false;
>> -    }
>> -
>> -    /* XXX: This is needs to be modified for making fastboot possible
>> -     * by default without requiring a modeset.
>> -     */
>> -    if (hw_state->base.active && pipe_config->base.active) {
>> -            struct intel_crtc_state sw_state;
>> -
>> -            memset(&sw_state, 0, sizeof(sw_state));
>> -            sw_state.base = pipe_config->base;
>> -            sw_state.scaler_state = pipe_config->scaler_state;
>> -            sw_state.shared_dpll = pipe_config->shared_dpll;
>> -            sw_state.dpll_hw_state = pipe_config->dpll_hw_state;
>> -            sw_state.ddi_pll_sel = pipe_config->ddi_pll_sel;
>> -
>> -            intel_modeset_pipe_config(&crtc->base, &sw_state);
>> -
>> -            /* Check if we need to force a modeset */
>> -            if (!intel_pipe_config_compare(dev, &sw_state, hw_state, 
>> !i915.fastboot)) {
>> -                    DRM_DEBUG_KMS("sw and hw state differ, forcing 
>> modeset\n");
>> -                    pipe_config->base.mode_changed = true;
>> -            } else {
>> -                    *pipe_config = sw_state;
>> -            }
>> -    }
>> +    if (hw_state->quirks & PIPE_CONFIG_QUIRK_WRONG_PLANE)
>> +            pipe_config->base.mode_changed = true;
>>  
>> +    else if (hw_state->base.active && pipe_config->base.active)
>> +            intel_fastboot_calc_modeset(crtc, hw_state, pipe_config);
>>  
>>      if (dev_priv->quirks & QUIRK_PIPEA_FORCE && !hw_state->base.active &&
>>          crtc->pipe == PIPE_A && !pipe_config->base.active) {
>> @@ -15584,6 +15643,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev, 
>> bool force_restore)
>>              crtc_state->planes_changed = false;
>>  
>>              if (crtc->state->enable) {
>> +                    memset(&crtc->mode, 0, sizeof(crtc->mode));
>>                      intel_mode_from_pipe_config(&crtc->mode,
>>                              to_intel_crtc_state(crtc->state));
>>                      intel_mode_from_pipe_config(&crtc->state->adjusted_mode,
>> @@ -15678,9 +15738,12 @@ void intel_modeset_gem_init(struct drm_device *dev, 
>> struct drm_atomic_state *sta
>>              int j;
>>              struct drm_plane *primary = c->primary;
>>  
>> +            if (!c->state->active)
>> +                    continue;
>> +
>>              obj = intel_fb_obj(primary->state->fb);
>>              if (obj == NULL)
>> -                    goto disable;
>> +                    continue;
>>  
>>              mutex_lock(&dev->struct_mutex);
>>              ret = intel_pin_and_fence_fb_obj(primary, primary->state->fb,
>> @@ -15715,6 +15778,8 @@ void intel_modeset_gem_init(struct drm_device *dev, 
>> struct drm_atomic_state *sta
>>               */
>>  
>>  disable:
>> +            DRM_DEBUG_KMS("[CRTC:%i] No fb or forced inactive, 
>> disabling.\n",
>> +                          c->base.id);
>>              crtc_state->active = crtc_state->enable = false;
>>  
>>              ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
>> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
>> b/drivers/gpu/drm/i915/intel_fbdev.c
>> index 6ac4990a0c11..2c494d78652a 100644
>> --- a/drivers/gpu/drm/i915/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
>> @@ -486,15 +486,10 @@ retry:
>>                       * config, not the input mode, which is what crtc->mode
>>                       * usually contains. But since our current fastboot
>>                       * code puts a mode derived from the post-pfit timings
>> -                     * into crtc->mode this works out correctly. We don't
>> -                     * use hwmode anywhere right now, so use it for this
>> -                     * since the fb helper layer wants a pointer to
>> -                     * something we own.
>> +                     * into crtc->mode this works out correctly.
>>                       */
>>                      DRM_DEBUG_KMS("looking for current mode on connector 
>> %s\n",
>>                                    connector->name);
>> -                    intel_mode_from_pipe_config(&encoder->crtc->hwmode,
>> -                                                
>> to_intel_crtc(encoder->crtc)->config);
>>                      modes[i] = &encoder->crtc->hwmode;
>>              }
>>              crtcs[i] = new_crtc;
>> @@ -584,9 +579,6 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>>      struct intel_crtc *intel_crtc;
>>      unsigned int max_size = 0;
>>  
>> -    if (!i915.fastboot)
>> -            return false;
> I think it'd be useful to split the fbdev part of the fastboot removal out
> into a separate patch. Or do I miss a depency here? Assuming ofc that we
> enable the pfit trick for modesets in general.
>
I think you did miss a dependency. Not competely sure though..

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to