On 02/02/2015 09:49 AM, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:56PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
To prepare for framebuffer modifiers, move tiling definition from the
object into the framebuffer. Move in a way that framebuffer tiling is
now used for display while object tiling remains for fencing.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 7 +++---
drivers/gpu/drm/i915/intel_sprite.c | 26 ++++++++++----------
4 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4425e86..e22afbe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- switch (obj->tiling_mode) {
+ switch (to_intel_framebuffer(fb)->tiling_mode) {
case I915_TILING_NONE:
Imo we should just look at fb->modifier[0] and flip over all the enums. A
bit more invasive, but we also don't need to change all the platform code
at once - set_tiling already guarantees that no one can modify the bo
tiling mode when there's an fb object using it. Which means we can change
the code over at leasure.
What do you mean by "flip over"?
To make places which need to get fb tiling format use fb->modifier[0]
directly rather than have them mapped to tiling enums at fb init?
@@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->event = event;
work->crtc = crtc;
work->old_fb_obj = intel_fb_obj(old_fb);
+ work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
Hm, that's actually an interesting bugfix - currently userspace could be
sneaky and destroy the old fb immediately after the flip completes and the
change the tiling of the underlying object before the unpin work had a
chance to run (needs some fudgin with rt prios to starve workers to make
this work though).
Imo the right fix is to hold a reference onto the fb and not the
underlying gem object. With that tiling is guaranteed not to change.
Ok I'll pull it out in a separate patch.
INIT_WORK(&work->work, intel_unpin_work_fn);
ret = drm_crtc_vblank_get(crtc);
@@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (IS_VALLEYVIEW(dev)) {
ring = &dev_priv->ring[BCS];
- if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
+ if (to_intel_framebuffer(fb)->tiling_mode !=
+ work->old_tiling_mode)
/* vlv: DISPLAY_FLIP fails to change tiling */
ring = NULL;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
@@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
/* we only need to pin inside GTT if cursor is non-phy */
mutex_lock(&dev->struct_mutex);
- if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
+ if (!INTEL_INFO(dev)->cursor_needs_physical &&
+ to_intel_framebuffer(fb)->tiling_mode) {
DRM_DEBUG_KMS("cursor cannot be tiled\n");
ret = -EINVAL;
}
@@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
intel_fb->obj = obj;
intel_fb->obj->framebuffer_references++;
+ intel_fb->tiling_mode = obj->tiling_mode;
One side-effect of using fb->modifier[0] is that if the modifier flag is
_not_ set, we need to reconstruct this field from obj->tiling_mode here.
Otoh if it is set this code here should check that fb->modifier and
obj->tiling_mode are consistent.
Perhaps best to split this change out as a prep patch, like you've done
with the code for the initial framebuffer.
If I understood correctly what you meant in the first quote then yes.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
