Op 11-06-15 om 03:37 schreef Matt Roper:
> On Thu, Jun 04, 2015 at 02:47:48PM +0200, Maarten Lankhorst wrote:
>> Read out the initial state, and add a quirk to force disable all plane
>> that were initially active. Use this to disable planes during modeset
>> too.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_atomic.c  |   7 ++
>>  drivers/gpu/drm/i915/intel_display.c | 139 
>> +++++++++++++++++++++++++----------
>>  drivers/gpu/drm/i915/intel_drv.h     |   7 ++
>>  3 files changed, 114 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 8447a1fef332..5627df2807b0 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -96,6 +96,13 @@ int intel_atomic_check(struct drm_device *dev,
>>              return -EINVAL;
>>      }
>>  
>> +    if (crtc_state &&
>> +        crtc_state->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE) {
>> +            ret = drm_atomic_add_affected_planes(state, 
>> &nuclear_crtc->base);
>> +            if (ret)
>> +                    return ret;
>> +    }
>> +
>>      ret = drm_atomic_helper_check_planes(dev, state);
>>      if (ret)
>>              return ret;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index b72724121f57..3b5d23692935 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4830,11 +4830,20 @@ static void intel_crtc_enable_planes(struct drm_crtc 
>> *crtc)
>>      intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
>>  }
>>  
>> +static void intel_disable_planes_on_crtc(struct drm_crtc *c)
>> +{
>> +    struct intel_crtc *crtc = to_intel_crtc(c);
>> +    struct drm_plane *plane;
>> +
>> +    drm_for_each_plane_mask(plane, crtc->base.dev,
>> +                            crtc->atomic.force_disabled_planes)
>> +            to_intel_plane(plane)->disable_plane(plane, c);
>> +}
>> +
>>  static void intel_crtc_disable_planes(struct drm_crtc *crtc)
>>  {
>>      struct drm_device *dev = crtc->dev;
>>      struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -    struct intel_plane *intel_plane;
>>      int pipe = intel_crtc->pipe;
>>  
>>      intel_crtc_wait_for_pending_flips(crtc);
>> @@ -4842,14 +4851,7 @@ static void intel_crtc_disable_planes(struct drm_crtc 
>> *crtc)
>>      intel_pre_disable_primary(crtc);
>>  
>>      intel_crtc_dpms_overlay_disable(intel_crtc);
>> -    for_each_intel_plane(dev, intel_plane) {
>> -            if (intel_plane->pipe == pipe) {
>> -                    struct drm_crtc *from = intel_plane->base.crtc;
>> -
>> -                    intel_plane->disable_plane(&intel_plane->base,
>> -                                               from ?: crtc);
>> -            }
>> -    }
>> +    intel_disable_planes_on_crtc(crtc);
>>  
>>      /*
>>       * FIXME: Once we grow proper nuclear flip support out of this we need
>> @@ -11554,6 +11556,21 @@ int intel_plane_atomic_calc_changes(struct 
>> drm_crtc_state *crtc_state,
>>      if (!intel_crtc->active || mode_changed)
>>              return 0;
>>  
>> +    if (to_intel_crtc_state(crtc_state)->quirks &
>> +        PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
>> +            if (!plane_state->crtc) {
>> +                    intel_crtc->atomic.force_disabled_planes |=
>> +                            1 << drm_plane_index(plane);
>> +                    crtc_state->plane_mask &=
>> +                            ~(1 << drm_plane_index(plane));
>> +            }
>> +
>> +            /* Initial state for sprite planes is unknown,
>> +             * no need to update sprite watermarks */
>> +            if (plane->type == DRM_PLANE_TYPE_OVERLAY)
>> +                    mode_changed = true;
>> +    }
>> +
>>      was_visible = old_plane_state->visible;
>>      visible = to_intel_plane_state(plane_state)->visible;
>>  
>> @@ -11633,7 +11650,7 @@ int intel_plane_atomic_calc_changes(struct 
>> drm_crtc_state *crtc_state,
>>                      intel_crtc->atomic.fb_bits |=
>>                          INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
>>  
>> -            if (turn_off && is_crtc_enabled) {
>> +            if (turn_off && !mode_changed) {
>>                      intel_crtc->atomic.wait_vblank = true;
>>                      intel_crtc->atomic.update_sprite_watermarks |=
>>                              1 << i;
>> @@ -11714,6 +11731,11 @@ static int intel_crtc_atomic_check(struct drm_crtc 
>> *crtc,
>>              "[CRTC:%i] mismatch between state->active(%i) and 
>> crtc->active(%i)\n",
>>              idx, crtc->state->active, intel_crtc->active);
>>  
>> +    /* plane mask is fixed up after all initial planes are calculated */
>> +    pipe_config->quirks &= ~PIPE_CONFIG_QUIRK_INITIAL_PLANES;
>> +    if (mode_changed)
>> +            intel_crtc->atomic.force_disabled_planes = 
>> crtc->state->plane_mask;
>> +
>>      if (mode_changed && crtc_state->enable &&
>>          dev_priv->display.crtc_compute_clock &&
>>          !WARN_ON(pipe_config->shared_dpll != DPLL_ID_PRIVATE)) {
>> @@ -12883,6 +12905,13 @@ intel_modeset_compute_config(struct 
>> drm_atomic_state *state)
>>              return ret;
>>  
>>      for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> +            if (to_intel_crtc_state(crtc_state)->quirks &
>> +                PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
>> +                    ret = drm_atomic_add_affected_planes(state, crtc);
>> +                    if (ret)
>> +                            return ret;
>> +            }
>> +
>>              if (!needs_modeset(crtc_state))
>>                      continue;
>>  
>> @@ -13529,13 +13558,17 @@ static void intel_begin_crtc_commit(struct 
>> drm_crtc *crtc)
>>      intel_runtime_pm_get(dev_priv);
>>  
>>      /* Perform vblank evasion around commit operation */
>> -    if (crtc->state->active && !needs_modeset(crtc->state))
>> +    if (crtc->state->active && !needs_modeset(crtc->state)) {
>>              intel_crtc->atomic.evade =
>>                      intel_pipe_update_start(intel_crtc,
>>                                              
>> &intel_crtc->atomic.start_vbl_count);
>>  
>> +            if (intel_crtc->atomic.force_disabled_planes)
>> +                    intel_disable_planes_on_crtc(crtc);
>> +    }
>> +
>>      if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
>> -            skl_detach_scalers(intel_crtc);
>> +                skl_detach_scalers(intel_crtc);
> Unintentional change from tabs to spaces on this line?
Oops.

>>  }
>>  
>>  static void intel_finish_crtc_commit(struct drm_crtc *crtc)
>> @@ -15040,14 +15073,63 @@ void i915_redisable_vga(struct drm_device *dev)
>>      i915_redisable_vga_power_on(dev);
>>  }
>>  
>> -static bool primary_get_hw_state(struct intel_crtc *crtc)
>> +static bool intel_read_hw_plane_state(struct intel_crtc *crtc,
>> +                                  struct intel_plane *intel_plane)
>>  {
>> -    struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>> +    struct drm_device *dev = crtc->base.dev;
>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>>  
>> -    if (!crtc->base.enabled)
>> -            return false;
>> +    switch (intel_plane->base.type) {
>> +    case DRM_PLANE_TYPE_PRIMARY:
>> +            return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>> +
>> +    case DRM_PLANE_TYPE_CURSOR:
>> +            if (IS_845G(dev) || IS_I865G(dev))
>> +                    return I915_READ(_CURACNTR) & CURSOR_ENABLE;
>> +            else
>> +                    return I915_READ(CURCNTR(crtc->plane)) & CURSOR_MODE;
>> +
> If we're not trying to seamlessly inherit the cursor image, I'm not sure
> I understand what the benefit of figuring out whether the cursor is
> actually on or not is.  Just assuming true, as we do for sprites, would
> be enough to make the cursor always turn off, right?
Yeah, when reworking this patch to apply after modeset revert I fixed that, but 
kept cursor readout since I wrote it anyway.

Also got me rid of tracking force disabled planes. :-)
>> +    default:
>> +            return true;
>> +    }
>> +}
>> +
>> +static int readout_plane_state(struct drm_atomic_state *state,
>> +                           struct intel_crtc *crtc,
>> +                           struct intel_crtc_state *crtc_state)
>> +{
>> +    struct intel_plane *p;
>> +    struct drm_plane_state *drm_plane_state;
>> +    bool active = crtc_state->base.active;
>> +
>> +    if (active) {
>> +            crtc_state->quirks |= PIPE_CONFIG_QUIRK_INITIAL_PLANES;
>>  
>> -    return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>> +            /* apply to previous sw state too */
>> +            to_intel_crtc_state(crtc->base.state)->quirks |=
>> +                    PIPE_CONFIG_QUIRK_INITIAL_PLANES;
>> +    }
>> +
>> +    for_each_intel_plane(state->dev, p) {
>> +            if (crtc->plane != p->plane)
>> +                    continue;
> Was this supposed to be comparing ->pipe rather than ->plane?  For
> primary and cursor planes I believe this works out okay since ->plane
> and ->pipe are basically the same (except for pre-gen4 FBC swapping),
> but for sprite planes, the ->plane pointer indicates whether it's the
> first, second, third, etc. sprite of that specific CRTC.
>
> As written, I think you'll only be operating on the first sprite plane
> of CRTC 0, the second sprite plane of CRTC 1, etc.
Ah thanks, that would explain it. :)

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

Reply via email to