| Message ID | 1410543789-10815-5-git-send-email-vandana.kannan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 12, 2014 at 11:13:07PM +0530, Vandana Kannan wrote: > Calls to switch between high and low refresh rates based on calls made to > fb_invalidate and fb_flush. > > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> A few comments below. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++++ > drivers/gpu/drm/i915/intel_dp.c | 51 ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > 3 files changed, 59 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 6d6214a..3bf1732 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9146,6 +9146,11 @@ static void intel_mark_fb_busy(struct drm_device *dev, > intel_increase_pllclock(dev, pipe); > if (ring && intel_fbc_enabled(dev)) > ring->fbc_dirty = true; > + > + if (ring) > + intel_edp_drrs_invalidate(dev, frontbuffer_bits); > + else > + intel_edp_drrs_flush(dev, frontbuffer_bits); I think the integration here isn't quite right. I've written a small patch to add a bit more documentation about how the frontbuffer tracking should work. See http://people.freedesktop.org/~danvet/drm/drmI915.html#idp72543008 So I think we need two cases here: - Mark DRRS as busy with intel_edp_drrs_busy. You can place it into intel_mark_fb_busy since that will be called for all 3 events (invalidate, flush, flip) as needed. But the place right now is the wrong spot since it's in the pipe loop so means you're function gets called 3 times. And with the below item I think it's better to move this call out of fb_mark_busy anyway. - Rearm the idle timer. This should _only_ be done for flush&flip, with a delay (100ms like in your patch sounds good) and _after_ you've cranked up the refresh rate to high. So checking for "ring" here isn't the right thing, we instead need to add an additional call at the end of intel_frontbuffer_flush. Or just add an "arm-idle-work" boolean to the _busy function and call it from intel_fb_obj_invalidate with false and intel_frontbuffer_flush with true. > } > } > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index d50249e..bd0415b 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4949,6 +4949,57 @@ unlock: > mutex_unlock(&dev_priv->drrs.mutex); > } > > +void intel_edp_drrs_invalidate(struct drm_device *dev, > + unsigned frontbuffer_bits) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_crtc *crtc; > + enum pipe pipe; > + > + mutex_lock(&dev_priv->drrs.mutex); > + if (!dev_priv->drrs.dp) { > + mutex_unlock(&dev_priv->drrs.mutex); > + return; > + } > + > + crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc; Dereferencing this pointer chain is a bit risk since this function gets called without modeset locks held. It should work due to ordering, but I'd prefer if we compute the DRRS pipe in drrs_enable and store it in dev_prive->drrs.pipe. > + pipe = to_intel_crtc(crtc)->pipe; > + > + if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) > + intel_dp_set_drrs_state(dev_priv->dev, > + dev_priv->drrs.dp->attached_connector->panel. > + fixed_mode->vrefresh); > + > + frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe); > + > + dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits; > + mutex_unlock(&dev_priv->drrs.mutex); > +} > + > +void intel_edp_drrs_flush(struct drm_device *dev, > + unsigned frontbuffer_bits) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_crtc *crtc; > + enum pipe pipe; > + > + mutex_lock(&dev_priv->drrs.mutex); > + if (!dev_priv->drrs.dp) { > + mutex_unlock(&dev_priv->drrs.mutex); > + return; > + } > + > + crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc; > + pipe = to_intel_crtc(crtc)->pipe; > + dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits; > + > + if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR && > + !dev_priv->psr.busy_frontbuffer_bits) > + schedule_delayed_work(&dev_priv->drrs.work, > + msecs_to_jiffies(100)); > + mutex_unlock(&dev_priv->drrs.mutex); > +} > + > static struct drm_display_mode * > intel_dp_drrs_init(struct intel_connector *intel_connector, > struct drm_display_mode *fixed_mode) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a728cc8..380bfe4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -929,6 +929,9 @@ void intel_edp_psr_init(struct drm_device *dev); > > void intel_edp_drrs_enable(struct intel_dp *intel_dp); > void intel_edp_drrs_disable(struct intel_dp *intel_dp); > +void intel_edp_drrs_invalidate(struct drm_device *dev, > + unsigned frontbuffer_bits); > +void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits); > > int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd); > void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector); > -- > 2.0.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6d6214a..3bf1732 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9146,6 +9146,11 @@ static void intel_mark_fb_busy(struct drm_device *dev, intel_increase_pllclock(dev, pipe); if (ring && intel_fbc_enabled(dev)) ring->fbc_dirty = true; + + if (ring) + intel_edp_drrs_invalidate(dev, frontbuffer_bits); + else + intel_edp_drrs_flush(dev, frontbuffer_bits); } } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d50249e..bd0415b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4949,6 +4949,57 @@ unlock: mutex_unlock(&dev_priv->drrs.mutex); } +void intel_edp_drrs_invalidate(struct drm_device *dev, + unsigned frontbuffer_bits) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc; + enum pipe pipe; + + mutex_lock(&dev_priv->drrs.mutex); + if (!dev_priv->drrs.dp) { + mutex_unlock(&dev_priv->drrs.mutex); + return; + } + + crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc; + pipe = to_intel_crtc(crtc)->pipe; + + if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) + intel_dp_set_drrs_state(dev_priv->dev, + dev_priv->drrs.dp->attached_connector->panel. + fixed_mode->vrefresh); + + frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe); + + dev_priv->drrs.busy_frontbuffer_bits |= frontbuffer_bits; + mutex_unlock(&dev_priv->drrs.mutex); +} + +void intel_edp_drrs_flush(struct drm_device *dev, + unsigned frontbuffer_bits) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc; + enum pipe pipe; + + mutex_lock(&dev_priv->drrs.mutex); + if (!dev_priv->drrs.dp) { + mutex_unlock(&dev_priv->drrs.mutex); + return; + } + + crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc; + pipe = to_intel_crtc(crtc)->pipe; + dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits; + + if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR && + !dev_priv->psr.busy_frontbuffer_bits) + schedule_delayed_work(&dev_priv->drrs.work, + msecs_to_jiffies(100)); + mutex_unlock(&dev_priv->drrs.mutex); +} + static struct drm_display_mode * intel_dp_drrs_init(struct intel_connector *intel_connector, struct drm_display_mode *fixed_mode) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a728cc8..380bfe4 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -929,6 +929,9 @@ void intel_edp_psr_init(struct drm_device *dev); void intel_edp_drrs_enable(struct intel_dp *intel_dp); void intel_edp_drrs_disable(struct intel_dp *intel_dp); +void intel_edp_drrs_invalidate(struct drm_device *dev, + unsigned frontbuffer_bits); +void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits); int intel_dp_handle_hpd_irq(struct intel_digital_port *digport, bool long_hpd); void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
Calls to switch between high and low refresh rates based on calls made to fb_invalidate and fb_flush. Signed-off-by: Vandana Kannan <vandana.kannan@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 ++++ drivers/gpu/drm/i915/intel_dp.c | 51 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 +++ 3 files changed, 59 insertions(+)