| Message ID | 1363208772-24714-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 13, 2013 at 02:06:12PM -0700, Ben Widawsky wrote: > v2: Move check to the top (Chris) > Add BUG_ON for !ivybridge, since it's all we support for now (Ben) > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 52203fd..8e7908b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) > i915_ironlake_get_mem_freq(dev); > > /* For FIFO watermark updates */ > - if (HAS_PCH_SPLIT(dev)) { > + if (HAS_PCH_NOP(dev)) { > + BUG_ON(!IS_IVYBRIDGE(dev)); > + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; > + dev_priv->display.update_wm = NULL; > + dev_priv->display.update_sprite_wm = NULL; > + } else if (HAS_PCH_SPLIT(dev)) { > if (IS_GEN5(dev)) { > if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) > dev_priv->display.update_wm = ironlake_update_wm; > @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) > dev_priv->display.init_clock_gating = gen6_init_clock_gating; > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > - if (SNB_READ_WM0_LATENCY()) { > + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { > dev_priv->display.update_wm = ivybridge_update_wm; > dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; > } else { I'm confused why we need this patch here - update_wm functions should only be called when we have a pipe. If there's any caller left I think we should fix those up, not paper over it here. And imo it's ok to have non-NULL vfuncs here (or anywhere else, e.g. pageflips) as long as we don't call them. After all the num_pips/PCH_NOP checks are here to make this as unobtrusive as possible. -Daniel
On Sun, Mar 17, 2013 at 10:02:44PM +0100, Daniel Vetter wrote: > On Wed, Mar 13, 2013 at 02:06:12PM -0700, Ben Widawsky wrote: > > v2: Move check to the top (Chris) > > Add BUG_ON for !ivybridge, since it's all we support for now (Ben) > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 52203fd..8e7908b 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) > > i915_ironlake_get_mem_freq(dev); > > > > /* For FIFO watermark updates */ > > - if (HAS_PCH_SPLIT(dev)) { > > + if (HAS_PCH_NOP(dev)) { > > + BUG_ON(!IS_IVYBRIDGE(dev)); > > + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; > > + dev_priv->display.update_wm = NULL; > > + dev_priv->display.update_sprite_wm = NULL; > > + } else if (HAS_PCH_SPLIT(dev)) { > > if (IS_GEN5(dev)) { > > if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) > > dev_priv->display.update_wm = ironlake_update_wm; > > @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) > > dev_priv->display.init_clock_gating = gen6_init_clock_gating; > > } else if (IS_IVYBRIDGE(dev)) { > > /* FIXME: detect B0+ stepping and use auto training */ > > - if (SNB_READ_WM0_LATENCY()) { > > + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { > > dev_priv->display.update_wm = ivybridge_update_wm; > > dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; > > } else { > > I'm confused why we need this patch here - update_wm functions should only > be called when we have a pipe. If there's any caller left I think we > should fix those up, not paper over it here. > > And imo it's ok to have non-NULL vfuncs here (or anywhere else, e.g. > pageflips) as long as we don't call them. After all the num_pips/PCH_NOP > checks are here to make this as unobtrusive as possible. > -Daniel I think you're right, so I've dropped this patch entirely.
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 52203fd..8e7908b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4153,7 +4153,12 @@ void intel_init_pm(struct drm_device *dev) i915_ironlake_get_mem_freq(dev); /* For FIFO watermark updates */ - if (HAS_PCH_SPLIT(dev)) { + if (HAS_PCH_NOP(dev)) { + BUG_ON(!IS_IVYBRIDGE(dev)); + dev_priv->display.init_clock_gating = ivybridge_init_clock_gating; + dev_priv->display.update_wm = NULL; + dev_priv->display.update_sprite_wm = NULL; + } else if (HAS_PCH_SPLIT(dev)) { if (IS_GEN5(dev)) { if (I915_READ(MLTR_ILK) & ILK_SRLT_MASK) dev_priv->display.update_wm = ironlake_update_wm; @@ -4175,7 +4180,7 @@ void intel_init_pm(struct drm_device *dev) dev_priv->display.init_clock_gating = gen6_init_clock_gating; } else if (IS_IVYBRIDGE(dev)) { /* FIXME: detect B0+ stepping and use auto training */ - if (SNB_READ_WM0_LATENCY()) { + if (SNB_READ_WM0_LATENCY() && !HAS_PCH_NOP(dev)) { dev_priv->display.update_wm = ivybridge_update_wm; dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm; } else {
v2: Move check to the top (Chris) Add BUG_ON for !ivybridge, since it's all we support for now (Ben) Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_pm.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)