On Wed, Mar 13, 2013 at 06:31:16PM +0000, Chris Wilson wrote: > On Wed, Mar 13, 2013 at 11:21:00AM -0700, Ben Widawsky wrote: > > Requested by Daniel. > > > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 9 +------ > > drivers/gpu/drm/i915/i915_drv.c | 48 ++++++++++++++++++------------------ > > drivers/gpu/drm/i915/i915_drv.h | 4 +-- > > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > > drivers/gpu/drm/i915/intel_display.c | 19 +++++++------- > > drivers/gpu/drm/i915/intel_fb.c | 2 +- > > drivers/gpu/drm/i915/intel_panel.c | 2 +- > > 7 files changed, 40 insertions(+), 47 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index e16099b..ebcfe2e 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1630,14 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > mutex_init(&dev_priv->rps.hw_lock); > > mutex_init(&dev_priv->modeset_restore_lock); > > > > - if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) > > - dev_priv->num_pipe = 3; > > - else if (IS_MOBILE(dev) || !IS_GEN2(dev)) > > - dev_priv->num_pipe = 2; > > - else > > - dev_priv->num_pipe = 1; > > - > > - ret = drm_vblank_init(dev, dev_priv->num_pipe); > > + ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes); > > if (ret) > > goto out_gem_unload; > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 1ebed96..0849651 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -136,74 +136,74 @@ extern int intel_agp_enabled; > > .driver_data = (unsigned long) info } > > > > static const struct intel_device_info intel_i830_info = { > > - .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, > > + .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 830gm > > > static const struct intel_device_info intel_i85x_info = { > > - .gen = 2, .is_i85x = 1, .is_mobile = 1, > > + .gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 1, > 2 pipes for 855gm > > > static const struct intel_device_info intel_i915g_info = { > > - .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, > > + .gen = 3, .is_i915g = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 915g > > > static const struct intel_device_info intel_i945g_info = { > > - .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, > > + .gen = 3, .has_hotplug = 1, .cursor_needs_physical = 1, .num_pipes = 1, > 2 pipes for 945g > > > static const struct intel_device_info intel_i965g_info = { > > - .gen = 4, .is_broadwater = 1, > > + .gen = 4, .is_broadwater = 1, .num_pipes = 1, > 2 pipes for 965g > > > > static const struct intel_device_info intel_g33_info = { > > - .gen = 3, .is_g33 = 1, > > + .gen = 3, .is_g33 = 1, .num_pipes = 1, > 2 pipes for g33 > > > .need_gfx_hws = 1, .has_hotplug = 1, > > .has_overlay = 1, > > }; > > > > static const struct intel_device_info intel_g45_info = { > > - .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, > > + .gen = 4, .is_g4x = 1, .need_gfx_hws = 1, .num_pipes = 1, > 2 pipes for g45 > > > static const struct intel_device_info intel_ironlake_d_info = { > > - .gen = 5, > > + .gen = 5, .num_pipes = 1, > 2 pipes for ilk-d > > > static const struct intel_device_info intel_sandybridge_d_info = { > > - .gen = 6, > > + .gen = 6, .num_pipes = 1, > 2 pipes for snb-d > > > static const struct intel_device_info intel_valleyview_d_info = { > > - .gen = 7, > > + .gen = 7, .num_pipes = 3, > 2? > > > @@ -341,6 +341,7 @@ struct drm_i915_gt_funcs { > > > > struct intel_device_info { > > u32 display_mmio_offset; > > + u8 num_pipes:3; > A hole! > > > u8 gen; > > u8 is_mobile:1; > > u8 is_i85x:1; > > @@ -912,7 +913,6 @@ typedef struct drm_i915_private { > > struct work_struct hotplug_work; > > bool enable_hotplug_processing; > > > > - int num_pipe; > > int num_pch_pll; > > > > unsigned long cfb_size; > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 2139714..b860f0b 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -254,10 +254,9 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, > > struct timeval *vblank_time, > > unsigned flags) > > { > > - struct drm_i915_private *dev_priv = dev->dev_private; > > struct drm_crtc *crtc; > > > > - if (pipe < 0 || pipe >= dev_priv->num_pipe) { > > + if (pipe < 0 || pipe >= INTEL_INFO(dev)->num_pipes) { > num_pipes is unsigned so pipe<0 is a redundant compare > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 502cb28..23379e7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2335,10 +2335,10 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > > return 0; > > } > > > > - if(intel_crtc->plane > dev_priv->num_pipe) { > > + if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { > > >= ? > > -- > Chris Wilson, Intel Open Source Technology Centre Thanks for detailed review. Sloppy code is a result of working while at a conference. I'll apply these fixes locally presently. -- Ben Widawsky, Intel Open Source Technology Center