| Message ID | 20180817082405.755-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Keep physical cursors pinned while in use | expand |
On Fri, Aug 17, 2018 at 09:24:05AM +0100, Chris Wilson wrote: > The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the > physical object coherent with GTT") relies on that once we allocated a > cursor we would have coherent, zero overhead access to the scanout plane > holding the cursor. That is we could then do the very frequent cursor > updates X enjoys with no indirection or kernel involvement. However, > that all hinges on the GGTT mmap of the cursor being pinned and not > require refaulting on each access -- handling such a page fault likely > requires the busy GGTT to be rearranged causing a stall. A very simple > fix is then to handle the physical cursor exactly like other cursors and > keep its vma pinned while active. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600 I guess this wasn't the thing we wanted. But seems quite harmless to me anyway, so Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> in case you still want to land it. > References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 592b847db88e..d47ec9fd4af4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12966,8 +12966,11 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state) > INTEL_INFO(dev_priv)->cursor_needs_physical) { > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > const int align = intel_cursor_alignment(dev_priv); > + int err; > > - return i915_gem_object_attach_phys(obj, align); > + err = i915_gem_object_attach_phys(obj, align); > + if (err) > + return err; > } > > vma = intel_pin_and_fence_fb_obj(fb, > -- > 2.18.0
Quoting Ville Syrjälä (2018-08-22 13:35:52) > On Fri, Aug 17, 2018 at 09:24:05AM +0100, Chris Wilson wrote: > > The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the > > physical object coherent with GTT") relies on that once we allocated a > > cursor we would have coherent, zero overhead access to the scanout plane > > holding the cursor. That is we could then do the very frequent cursor > > updates X enjoys with no indirection or kernel involvement. However, > > that all hinges on the GGTT mmap of the cursor being pinned and not > > require refaulting on each access -- handling such a page fault likely > > requires the busy GGTT to be rearranged causing a stall. A very simple > > fix is then to handle the physical cursor exactly like other cursors and > > keep its vma pinned while active. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600 > > I guess this wasn't the thing we wanted. But seems quite harmless to > me anyway, so It ties neatly in with the ggtt map being used for updates, so I think it's harmless enough. If I could just get a contiguous page out of shmemfs, I could follow up with the removal of phys_object. :| > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > in case you still want to land it. But without the bugzilla since that bug is occurring without cursor updates, so is even more bizarre. -Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 592b847db88e..d47ec9fd4af4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12966,8 +12966,11 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state) INTEL_INFO(dev_priv)->cursor_needs_physical) { struct drm_i915_gem_object *obj = intel_fb_obj(fb); const int align = intel_cursor_alignment(dev_priv); + int err; - return i915_gem_object_attach_phys(obj, align); + err = i915_gem_object_attach_phys(obj, align); + if (err) + return err; } vma = intel_pin_and_fence_fb_obj(fb,
The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT") relies on that once we allocated a cursor we would have coherent, zero overhead access to the scanout plane holding the cursor. That is we could then do the very frequent cursor updates X enjoys with no indirection or kernel involvement. However, that all hinges on the GGTT mmap of the cursor being pinned and not require refaulting on each access -- handling such a page fault likely requires the busy GGTT to be rearranged causing a stall. A very simple fix is then to handle the physical cursor exactly like other cursors and keep its vma pinned while active. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600 References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)