Re: [PATCH] drm/i915/bdw: Simple IPS pixel_rate x 0.95 cdclk restriction

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, May 21, 2015 at 01:25:28PM +0300, Jani Nikula wrote:
> On Thu, 21 May 2015, Daniel Vetter <daniel@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.

> 
> 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@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@lists.freedesktop.org
> >> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> > > 
> >> > 
> >> 
> >> -- 
> >> Ville Syrjälä
> >> Intel OTC
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx