| Message ID | 1470260019-6173-7-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 03, 2016 at 02:33:39PM -0700, Rodrigo Vivi wrote: > DC state reset the frame counter that is a read-only register. > > So, besides blocking DC state on vblank let's restore the > drm crtc vblank counter to a place we know it is reliable. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 4efe20c..82d6896 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2759,7 +2759,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) > static void gen9_prepare_vblank(struct drm_device *dev, unsigned int pipe) > { > struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK); > + drm_crtc_vblank_sanitize_counter(crtc); I think this should be done within the platform power code. Otherwise something else might keep the system out of dc states, but then we might wreak havoc by calling this function here. -Daniel > } > > static void gen9_unprepare_vblank(struct drm_device *dev, unsigned int pipe) > -- > 2.4.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Aug 4, 2016 at 1:26 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Aug 03, 2016 at 02:33:39PM -0700, Rodrigo Vivi wrote: >> DC state reset the frame counter that is a read-only register. >> >> So, besides blocking DC state on vblank let's restore the >> drm crtc vblank counter to a place we know it is reliable. >> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index 4efe20c..82d6896 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -2759,7 +2759,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) >> static void gen9_prepare_vblank(struct drm_device *dev, unsigned int pipe) >> { >> struct drm_i915_private *dev_priv = to_i915(dev); >> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; >> intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK); >> + drm_crtc_vblank_sanitize_counter(crtc); > > I think this should be done within the platform power code. Otherwise > something else might keep the system out of dc states, but then we might > wreak havoc by calling this function here. This is safe here because DC is handled as a power_well... so as long as there is one domain holding its reference DC will be off. Also this needs to be in a place we are sure that vblanks are yet disabled. Now it comes back to that point on how to make sure that we only run 1 prepare pre-enable... multiple prepare/unprepares will keep trying to resync it useless and if we have that WARN_ON we will get flooded.... > -Daniel > >> } >> >> static void gen9_unprepare_vblank(struct drm_device *dev, unsigned int pipe) >> -- >> 2.4.3 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Aug 05, 2016 at 02:49:44PM -0700, Rodrigo Vivi wrote: > On Thu, Aug 4, 2016 at 1:26 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Aug 03, 2016 at 02:33:39PM -0700, Rodrigo Vivi wrote: > >> DC state reset the frame counter that is a read-only register. > >> > >> So, besides blocking DC state on vblank let's restore the > >> drm crtc vblank counter to a place we know it is reliable. > >> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> index 4efe20c..82d6896 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -2759,7 +2759,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) > >> static void gen9_prepare_vblank(struct drm_device *dev, unsigned int pipe) > >> { > >> struct drm_i915_private *dev_priv = to_i915(dev); > >> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > >> intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK); > >> + drm_crtc_vblank_sanitize_counter(crtc); > > > > I think this should be done within the platform power code. Otherwise > > something else might keep the system out of dc states, but then we might > > wreak havoc by calling this function here. > > This is safe here because DC is handled as a power_well... so as long > as there is one domain holding its reference DC will be off. > > Also this needs to be in a place we are sure that vblanks are yet disabled. > > Now it comes back to that point on how to make sure that we only run 1 > prepare pre-enable... > > multiple prepare/unprepares will keep trying to resync it useless and > if we have that WARN_ON we will get flooded.... Yeah, my comment was under the assumption that there can be multiple prepare/unprepare, and then it makes sense to reuse the refcounting we already have. Still not sure it'll make sense to implement prepare/unprepare refcounting in the vblank code, it'll be fairly tricky in there. And for use useless, since our power well code already has refcounting. -Daniel > > > -Daniel > > > >> } > >> > >> static void gen9_unprepare_vblank(struct drm_device *dev, unsigned int pipe) > >> -- > >> 2.4.3 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4efe20c..82d6896 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2759,7 +2759,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) static void gen9_prepare_vblank(struct drm_device *dev, unsigned int pipe) { struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; intel_display_power_get(dev_priv, POWER_DOMAIN_VBLANK); + drm_crtc_vblank_sanitize_counter(crtc); } static void gen9_unprepare_vblank(struct drm_device *dev, unsigned int pipe)
DC state reset the frame counter that is a read-only register. So, besides blocking DC state on vblank let's restore the drm crtc vblank counter to a place we know it is reliable. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 2 ++ 1 file changed, 2 insertions(+)