On Thu, May 21, 2015 at 3:35 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Thu, May 21, 2015 at 01:25:28PM +0300, Jani Nikula wrote:
>> On Thu, 21 May 2015, Daniel Vetter <daniel at ffwll.ch> wrote:
>> > On Wed, May 20, 2015 at 11:50:17PM +0300, Ville Syrjälä wrote:
>> >> On Wed, May 20, 2015 at 08:38:25PM +0000, Vivi, Rodrigo wrote:
>> >> > On Wed, 2015-05-20 at 20:26 +0300, Ville Syrjälä wrote:
>> >> > > On Wed, May 20, 2015 at 09:42:57AM -0700, Rodrigo Vivi wrote:
>> >> > > > Broadwell Workaround : Do not enable IPS when the pipe pixel rate is
>> >> > > > greater than
>> >> > > > 95% of the CDCLK frequency. The pipe pixel rate is the port pixel rate
>> >> > > > multiplied by the pipe scaler down scale amount.
>> >> > >
>> >> > > Or just merge my patch doing the same thing?
>> >> >
>> >> > I was trying to make the simple possible since latest one that I was
>> >> > looking at was depending on a series of many patches. It was caching the
>> >> > max cdclk, etc...
>> >>
>> >> Well the latter one is the one we want (well actually we want to go
>> >> further than that, but have to start somewhere). The only thing blocking
>> >> the patches is lack of review so the solution seems easy enough to me ;)
>> >
>> > We need a minimal patch for -fixes+cc: stable since this is blowing up on
>> > real-world bdws out there already. Jani?
>>
>> Already?! *gasp*! https://bugs.freedesktop.org/show_bug.cgi?id=83497
>>
>> Point me at a patch with r-b, t-b and supposedly not regressing, and
>> I'll merge.
>
> The minimal fix is i915.enable_ips=0. But I won't send such a patch.
I'm almost considering this more seriously. The flickering on console
isn't caused by this 95% of cdclk but by the other limitation where
IPS needs at least one plane enabled while it is on.... There are some
cases like on modeset that we shortcut !visible setting DSPCNTRL to 0.
At this moment we might have no plane really enabled and ips enabled
so it doesn't recover and the flicker starts.
One w/a would be double vblank_waits we have and call disable_ips when
setting dspcntrl to 0. This minimized the flickers a lot here for me,
but not 100% of the time.
And even worst on 3.14 custom kernel that I was looking at as well
where I couldn''t find yet where we are disabling planes without
disabling ips...
please let me know if you have any idea...
>
>>
>> BR,
>> Jani.
>>
>>
>>
>> > -Daniel
>> >
>> >>
>> >> >
>> >> > But now I looked again your first one and saw the history there that got
>> >> > merged but it was removed due to unclaimed registers...
>> >> >
>> >> > Now I see that my patch and your original one aren't so different... I'm
>> >> > sorry...
>> >> >
>> >> > So I was telling Paulo that with my patch I can't get Unclaimed messages
>> >> > anymore and when double cheking I noticed that runing pm_rpm rte with no
>> >> > patch I get the "Unclaimed register detected before reading register
>> >> > 0x130040"
>> >> >
>> >> > So I believe your first patch is good to get merged. I believe that
>> >> > "Unclaimed reg" that Paulo saw was something else.
>> >> >
>> >> > So, back to yours...
>> >> >
>> >> > >
>> >> > > >
>> >> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> >> > > > ---
>> >> > > > drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>> >> > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
>> >> > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>> >> > > > 3 files changed, 13 insertions(+), 2 deletions(-)
>> >> > > >
>> >> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> >> > > > index 9d2d6fb..dd8ef51 100644
>> >> > > > --- a/drivers/gpu/drm/i915/intel_display.c
>> >> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> >> > > > @@ -112,6 +112,7 @@ static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
>> >> > > > int num_connectors);
>> >> > > > static void intel_crtc_enable_planes(struct drm_crtc *crtc);
>> >> > > > static void intel_crtc_disable_planes(struct drm_crtc *crtc);
>> >> > > > +static int broadwell_get_display_clock_speed(struct drm_device *dev);
>> >> > > >
>> >> > > > static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
>> >> > > > {
>> >> > > > @@ -4948,6 +4949,14 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>> >> > > > /* IPS only exists on ULT machines and is tied to pipe A. */
>> >> > > > static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
>> >> > > > {
>> >> > > > + struct drm_device *dev = crtc->base.dev;
>> >> > > > +
>> >> > > > + if (IS_BROADWELL(dev) && ilk_pipe_pixel_rate(dev, &crtc->base) >
>> >> > > > + broadwell_get_display_clock_speed(dev) * 95/100) {
>> >> > > > + DRM_DEBUG_KMS("IPS avoided because pixel rate is greater than 95 percent of CDCLK\n");
>> >> > > > + return false;
>> >> > > > + }
>> >> > > > +
>> >> > > > return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
>> >> > > > }
>> >> > > >
>> >> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> >> > > > index 47bc729..c59fa3f 100644
>> >> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
>> >> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> >> > > > @@ -1341,6 +1341,8 @@ void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
>> >> > > > /* intel_pm.c */
>> >> > > > void intel_init_clock_gating(struct drm_device *dev);
>> >> > > > void intel_suspend_hw(struct drm_device *dev);
>> >> > > > +uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>> >> > > > + struct drm_crtc *crtc);
>> >> > > > int ilk_wm_max_level(const struct drm_device *dev);
>> >> > > > void intel_update_watermarks(struct drm_crtc *crtc);
>> >> > > > void intel_update_sprite_watermarks(struct drm_plane *plane,
>> >> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> >> > > > index ce1d079..0afeae8 100644
>> >> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
>> >> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
>> >> > > > @@ -1434,8 +1434,8 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
>> >> > > > I915_WRITE(FW_BLC, fwater_lo);
>> >> > > > }
>> >> > > >
>> >> > > > -static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>> >> > > > - struct drm_crtc *crtc)
>> >> > > > +uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>> >> > > > + struct drm_crtc *crtc)
>> >> > > > {
>> >> > > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> >> > > > uint32_t pixel_rate;
>> >> > > > --
>> >> > > > 2.1.0
>> >> > > >
>> >> > > > _______________________________________________
>> >> > > > Intel-gfx mailing list
>> >> > > > Intel-gfx at lists.freedesktop.org
>> >> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> > >
>> >> >
>> >>
>> >> --
>> >> Ville Syrjälä
>> >> Intel OTC
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Daniel Vetter
>> > Software Engineer, Intel Corporation
>> > http://blog.ffwll.ch
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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