Re: [PATCH 1/3] drm/i915: Move watermark handling to intel_wm.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 27, 2015 at 07:47:46PM +0100, Daniel Vetter wrote:
> On Fri, Feb 27, 2015 at 08:21:07PM +0200, Ville Syrjälä wrote:
> > On Fri, Feb 27, 2015 at 08:54:19AM -0800, Matt Roper wrote:
> > > Move watermark handling from intel_pm.c to intel_wm.c and add a little
> > > bit of kerneldoc to exported functions.  We also add a new
> > > intel_init_wm() function to setup memory timing information and
> > > initialize the relevant watermark vfuncs.
> > 
> > How about delaying gratious code movement until a bunch of the in flight
> > WM stuff has landed? This is just going to cause rebase pains all over.
> 
> Hand-editing diffs works reasonable well ime for these kinds of extraction
> patches. At least that's how I've done it with -internal. And git rebase
> can sometimes help too if you crank up the rename detection threshold.

All pointless extra work. Moving the code doesn't make it any better so
I fail to see why it needs to be rushed in.

> -Daniel
> 
> > 
> > > 
> > > Jira: VIZ-4612
> > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > > ---
> > >  drivers/gpu/drm/i915/Makefile        |    3 +-
> > >  drivers/gpu/drm/i915/intel_display.c |    1 +
> > >  drivers/gpu/drm/i915/intel_drv.h     |    3 +
> > >  drivers/gpu/drm/i915/intel_pm.c      | 3450 ---------------------------------
> > >  drivers/gpu/drm/i915/intel_wm.c      | 3525 ++++++++++++++++++++++++++++++++++
> > >  5 files changed, 3531 insertions(+), 3451 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/intel_wm.c
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index f025e7f..5fd2257 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -55,7 +55,8 @@ i915-y += intel_audio.o \
> > >  	  intel_overlay.o \
> > >  	  intel_psr.o \
> > >  	  intel_sideband.o \
> > > -	  intel_sprite.o
> > > +	  intel_sprite.o \
> > > +	  intel_wm.o
> > >  i915-$(CONFIG_ACPI)		+= intel_acpi.o intel_opregion.o
> > >  i915-$(CONFIG_DRM_I915_FBDEV)	+= intel_fbdev.o
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index fad5f76..370c173 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13213,6 +13213,7 @@ void intel_modeset_init(struct drm_device *dev)
> > >  	intel_init_quirks(dev);
> > >  
> > >  	intel_init_pm(dev);
> > > +	intel_init_wm(dev);
> > >  
> > >  	if (INTEL_INFO(dev)->num_pipes == 0)
> > >  		return;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 1fb1529..995d030 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1258,6 +1258,9 @@ void intel_pre_disable_primary(struct drm_crtc *crtc);
> > >  /* intel_tv.c */
> > >  void intel_tv_init(struct drm_device *dev);
> > >  
> > > +/* intel_wm.c */
> > > +void intel_init_wm(struct drm_device *dev);
> > > +
> > >  /* intel_atomic.c */
> > >  int intel_atomic_check(struct drm_device *dev,
> > >  		       struct drm_atomic_state *state);
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 7dcb5b6..f906b05 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -94,3402 +94,6 @@ static void skl_init_clock_gating(struct drm_device *dev)
> > >  			   GEN8_LQSC_RO_PERF_DIS);
> > >  }
> > >  
> > > -static void i915_pineview_get_mem_freq(struct drm_device *dev)
> > > -{
> > > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > > -	u32 tmp;
> > > -
> > > -	tmp = I915_READ(CLKCFG);
> > > -
> > > -	switch (tmp & CLKCFG_FSB_MASK) {
> > > -	case CLKCFG_FSB_533:
> > > -		dev_priv->fsb_freq = 533; /* 133*4 */
> > > -		break;
> > > -	case CLKCFG_FSB_800:
> > > -		dev_priv->fsb_freq = 800; /* 200*4 */
> > > -		break;
> > > -	case CLKCFG_FSB_667:
> > > -		dev_priv->fsb_freq =  667; /* 167*4 */
> > > -		break;
> > > -	case CLKCFG_FSB_400:
> > > -		dev_priv->fsb_freq = 400; /* 100*4 */
> > > -		break;
> > > -	}
> > > -
> > > -	switch (tmp & CLKCFG_MEM_MASK) {
> > > -	case CLKCFG_MEM_533:
> > > -		dev_priv->mem_freq = 533;
> > > -		break;
> > > -	case CLKCFG_MEM_667:
> > > -		dev_priv->mem_freq = 667;
> > > -		break;
> > > -	case CLKCFG_MEM_800:
> > > -		dev_priv->mem_freq = 800;
> > > -		break;
> > > -	}
> > > -
> > > -	/* detect pineview DDR3 setting */
> > > -	tmp = I915_READ(CSHRDDR3CTL);
> > > -	dev_priv->is_ddr3 = (tmp & CSHRDDR3CTL_DDR3) ? 1 : 0;
> > > -}
> > > -
> > > -static void i915_ironlake_get_mem_freq(struct drm_device *dev)
> > > -{
> > > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > > -	u16 ddrpll, csipll;
> > > -
> > > -	ddrpll = I915_READ16(DDRMPLL1);
> > > -	csipll = I915_READ16(CSIPLL0);
> > > -
> > > -	switch (ddrpll & 0xff) {
> > > -	case 0xc:
> > > -		dev_priv->mem_freq = 800;
> > > -		break;
> > > -	case 0x10:
> > > -		dev_priv->mem_freq = 1066;
> > > -		break;
> > > -	case 0x14:
> > > -		dev_priv->mem_freq = 1333;
> > > -		break;
> > > -	case 0x18:
> > > -		dev_priv->mem_freq = 1600;
> > > -		break;
> > > -	default:
> > > -		DRM_DEBUG_DRIVER("unknown memory frequency 0x%02x\n",
> > > -				 ddrpll & 0xff);
> > > -		dev_priv->mem_freq = 0;
> > > -		break;
> > > -	}
> > > -
> > > -	dev_priv->ips.r_t = dev_priv->mem_freq;
> > > -
> > > -	switch (csipll & 0x3ff) {
> > > -	case 0x00c:
> > > -		dev_priv->fsb_freq = 3200;
> > > -		break;
> > > -	case 0x00e:
> > > -		dev_priv->fsb_freq = 3733;
> > > -		break;
> > > -	case 0x010:
> > > -		dev_priv->fsb_freq = 4266;
> > > -		break;
> > > -	case 0x012:
> > > -		dev_priv->fsb_freq = 4800;
> > > -		break;
> > > -	case 0x014:
> > > -		dev_priv->fsb_freq = 5333;
> > > -		break;
> > > -	case 0x016:
> > > -		dev_priv->fsb_freq = 5866;
> > > -		break;
> > > -	case 0x018:
> > > -		dev_priv->fsb_freq = 6400;
> > > -		break;
> > > -	default:
> > > -		DRM_DEBUG_DRIVER("unknown fsb frequency 0x%04x\n",
> > > -				 csipll & 0x3ff);
> > > -		dev_priv->fsb_freq = 0;
> > > -		break;
> > > -	}
> > > -
> > > -	if (dev_priv->fsb_freq == 3200) {
> > > -		dev_priv->ips.c_m = 0;
> > > -	} else if (dev_priv->fsb_freq > 3200 && dev_priv->fsb_freq <= 4800) {
> > > -		dev_priv->ips.c_m = 1;
> > > -	} else {
> > > -		dev_priv->ips.c_m = 2;
> > > -	}
> > > -}
> > 
> > I don't think these two functions should be moved.
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux