| Message ID | 20181101150605.18235-8-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Program SKL+ watermarks/ddb more carefully | expand |
On Thu, Nov 01, 2018 at 05:05:58PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The plane color correction registers are single buffered. So > ideally we would write them at the start of vblank just after the > double buffered plane registers have been latched. Since we have > no convenient way to do that for now let's at least move the > single buffered register writes to happen after the double > buffered registers have been written. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 455b2d0cbaa6..84c5f532fba5 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -718,8 +718,6 @@ vlv_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - vlv_update_clrc(plane_state); > - > I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > plane_state->color_plane[0].stride); > I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > @@ -747,6 +745,8 @@ vlv_update_plane(struct intel_plane *plane, > I915_WRITE_FW(SPSURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > > + vlv_update_clrc(plane_state); > + > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Nov 01, 2018 at 05:05:58PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The plane color correction registers are single buffered. So > ideally we would write them at the start of vblank just after the > double buffered plane registers have been latched. Since we have > no convenient way to do that for now let's at least move the > single buffered register writes to happen after the double > buffered registers have been written. Should we move this all the way out of the vblank evasion? I realize that vlv is only two registers total so it's not a big deal, but I know Uma is working on the plane color management stuff for later platforms where we have a bunch of registers to write, so maybe we should setup the callsite now? On a similar note, I notice our single-buffered pipe-level color management registers are written before evasion right now...should we move that to after the evasion as well? Matt > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 455b2d0cbaa6..84c5f532fba5 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -718,8 +718,6 @@ vlv_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - vlv_update_clrc(plane_state); > - > I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > plane_state->color_plane[0].stride); > I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > @@ -747,6 +745,8 @@ vlv_update_plane(struct intel_plane *plane, > I915_WRITE_FW(SPSURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > > + vlv_update_clrc(plane_state); > + > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Nov 08, 2018 at 02:06:52PM -0800, Matt Roper wrote: > On Thu, Nov 01, 2018 at 05:05:58PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The plane color correction registers are single buffered. So > > ideally we would write them at the start of vblank just after the > > double buffered plane registers have been latched. Since we have > > no convenient way to do that for now let's at least move the > > single buffered register writes to happen after the double > > buffered registers have been written. > > Should we move this all the way out of the vblank evasion? I realize > that vlv is only two registers total so it's not a big deal, but I know > Uma is working on the plane color management stuff for later platforms > where we have a bunch of registers to write, so maybe we should setup > the callsite now? I didn't want to pile on too much work in this series. For the plane color management we might need to think how to do this properly as otherwise it's may end up being too ugly to actually use. I also have a branch somewhere with fp16 scanout support, and on ivb that requires playing around with the plane gamma as well. So that could be another natural point when we might come up with a better mechanism for single buffered registers. Although I'm not sure we can land fp16 any time soon though since there is no userspace currently. I implemented it just so that I could test the higher precision pipe gamma modes. > > On a similar note, I notice our single-buffered pipe-level color > management registers are written before evasion right now...should we > move that to after the evasion as well? Yes. I have that actually implemented on a branch that reworks the pipe color management stuff quite a bit. I'm actually moving it to happen after the vblank waits in the sequence, but I didn't add any kind of proper vblank worker etc. so I expect it's still likely to tear :( But at least it's a bit closer to where it really should be. I'm planning to post that series after this stuff lands as there is a slight dependency on the update_planes stuff and whatnot.
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 455b2d0cbaa6..84c5f532fba5 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -718,8 +718,6 @@ vlv_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - vlv_update_clrc(plane_state); - I915_WRITE_FW(SPSTRIDE(pipe, plane_id), plane_state->color_plane[0].stride); I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); @@ -747,6 +745,8 @@ vlv_update_plane(struct intel_plane *plane, I915_WRITE_FW(SPSURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + sprsurf_offset); + vlv_update_clrc(plane_state); + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); }