[Intel-gfx] [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW

[Paulo Zanoni]

2014-06-30 7:10 GMT-03:00 Jani Nikula <jani.nikula at linux.intel.com>:
> On Thu, 26 Jun 2014, Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
>> I'm sure this might affect Wayne, so, cc'ing him here.
>>
>> from my point of view this is right so:
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> Pushed to -fixes, thanks for the patch and review.
>
> BR,
> Jani.
>
>
>>
>>
>> On Tue, Jun 24, 2014 at 3:59 AM, <ville.syrjala at linux.intel.com> wrote:
>>
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> BDW signals the flip done interrupt immediately after the DSPSURF write
>>> when the plane is disabled. This is true even if we've already armed
>>> DSPCNTR to enable the plane at the next vblank. This causes major
>>> problems for our page flip code which relies on the flip done interrupts
>>> happening at vblank time.
>>>
>>> So what happens is that we enable the plane, and immediately allow
>>> userspace to submit a page flip. If the plane is still in the process
>>> of being enabled when the page flip is issued, the flip done gets
>>> signalled immediately. Our DSPSURFLIVE check catches this to prevent
>>> premature flip completion, but it also means that we don't get a flip
>>> done interrupt when the plane actually gets enabled, and so the page
>>> flip is never completed.
>>>
>>> Work around this by re-introducing blocking vblank waits on BDW
>>> whenever we enable the primary plane.
>>>
>>> I removed some of the vblank waits here:
>>>  commit 6304cd91e7f05f8802ea6f91287cac09741d9c46
>>>  Author: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>  Date:   Fri Apr 25 13:30:12 2014 +0300
>>>
>>>     drm/i915: Drop the excessive vblank waits from modeset codepaths
>>>
>>> To avoid these blocking vblank waits we should start using the vblank
>>> interrupt instead of the flip done interrupt to complete page flips.
>>> But that's material for another patch.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79354
>>> Tested-by: Guo Jinxian <jinxianx.guo at intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>>>  drivers/gpu/drm/i915/intel_sprite.c  | 8 ++++++++
>>>  2 files changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 9188fed..f92efc6 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct
>>> drm_i915_private *dev_priv,
>>>  static void intel_enable_primary_hw_plane(struct drm_i915_private
>>> *dev_priv,
>>>                                           enum plane plane, enum pipe pipe)
>>>  {
>>> +       struct drm_device *dev = dev_priv->dev;
>>>         struct intel_crtc *intel_crtc =
>>>                 to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>>>         int reg;
>>> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(struct
>>> drm_i915_private *dev_priv,
>>>
>>>         I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
>>>         intel_flush_primary_plane(dev_priv, plane);
>>> +
>>> +       /*
>>> +        * BDW signals flip done immediately if the plane
>>> +        * is disabled, even if the plane enable is already
>>> +        * armed to occur at the next vblank :(
>>> +        */
>>> +       if (IS_BROADWELL(dev))
>>> +               intel_wait_for_vblank(dev, intel_crtc->pipe);

This chunk triggers "WARN(ret == 0)" from drm_wait_one_vblank when
using HDMI on BDW.


>>>  }
>>>
>>>  /**
>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>> index 1b66ddc..9a17b4e 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crtc)
>>>         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>>
>>>         /*
>>> +        * BDW signals flip done immediately if the plane
>>> +        * is disabled, even if the plane enable is already
>>> +        * armed to occur at the next vblank :(
>>> +        */
>>> +       if (IS_BROADWELL(dev))
>>> +               intel_wait_for_vblank(dev, intel_crtc->pipe);
>>> +
>>> +       /*
>>>          * FIXME IPS should be fine as long as one plane is
>>>          * enabled, but in practice it seems to have problems
>>>          * when going from primary only to sprite only and vice
>>> --
>>> 1.8.5.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>
>>
>>
>> --
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni