[PATCH 0/4] FBC frontbuffer tracking + MMAP_WC fixes

[PATCH 0/4] FBC frontbuffer tracking + MMAP_WC fixes

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Hi

This series can be reviewed in parallel with the other FBC patches
currently under review. The goal here is to fix all the mmap-wc
subtests of kms_frontbuffer_tracking that don't require any patches
that are in the other series.

Thanks,
Paulo

Paulo Zanoni (4):
  drm/i915: fix FBC frontbuffer tracking flushing code
  drm/i915: don't call intel_fbc_update() at intel_unpin_work_fn()
  drm/i915: don't disable FBC for pipe A when flipping pipe B
  drm/i915: special-case dirtyfb for frontbuffer tracking

 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/intel_display.c     |  7 ++-----
 drivers/gpu/drm/i915/intel_drv.h         |  2 +-
 drivers/gpu/drm/i915/intel_fbc.c         | 13 +++++++------
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 5 files changed, 12 insertions(+), 13 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/4] drm/i915: fix FBC frontbuffer tracking flushing code

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Due to the way busy_bits was handled, we were not doing any flushes if
we didn't previously get an invalidate. Since it's possible to get
flushes without an invalidate first, remove the busy_bits early
return.

So now that we don't have the busy_bits guard anymore we'll need the
origin check for the GTT tracking (we were not doing anything on GTT
flushes due to the GTT check at invalidate()).

As a last detail, since we can get multiple consecutive flushes,
disable FBC before updating it, otherwise intel_fbc_update() will just
keep FBC enabled instead of restarting it.

Notice that this does not fix any of the current IGT tests due to the
fact that we still have a few intel_fbc() calls at points where we
also have the frontbuffer tracking calls: we didn't fully convert to
frontbuffer tracking yet. Once we remove those calls and start relying
only on the frontbuffer tracking infrastructure we'll need this patch.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h         |  2 +-
 drivers/gpu/drm/i915/intel_fbc.c         | 13 +++++++------
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f4abce1..2437666 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1240,7 +1240,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 			  unsigned int frontbuffer_bits,
 			  enum fb_op_origin origin);
 void intel_fbc_flush(struct drm_i915_private *dev_priv,
-		     unsigned int frontbuffer_bits);
+		     unsigned int frontbuffer_bits, enum fb_op_origin origin);
 const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
 void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index c271af7..1f97fb5 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -884,22 +884,23 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 }
 
 void intel_fbc_flush(struct drm_i915_private *dev_priv,
-		     unsigned int frontbuffer_bits)
+		     unsigned int frontbuffer_bits, enum fb_op_origin origin)
 {
 	if (!dev_priv->fbc.enable_fbc)
 		return;
 
-	mutex_lock(&dev_priv->fbc.lock);
+	if (origin == ORIGIN_GTT)
+		return;
 
-	if (!dev_priv->fbc.busy_bits)
-		goto out;
+	mutex_lock(&dev_priv->fbc.lock);
 
 	dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
 
-	if (!dev_priv->fbc.busy_bits)
+	if (!dev_priv->fbc.busy_bits) {
+		__intel_fbc_disable(dev_priv);
 		__intel_fbc_update(dev_priv);
+	}
 
-out:
 	mutex_unlock(&dev_priv->fbc.lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 777b1d3..ac85357 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -129,7 +129,7 @@ static void intel_frontbuffer_flush(struct drm_device *dev,
 
 	intel_edp_drrs_flush(dev, frontbuffer_bits);
 	intel_psr_flush(dev, frontbuffer_bits, origin);
-	intel_fbc_flush(dev_priv, frontbuffer_bits);
+	intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
 }
 
 /**
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: fix FBC frontbuffer tracking flushing code

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 4170 bytes --]

On Tue, Jul 14, 2015 at 12:30 PM Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Due to the way busy_bits was handled, we were not doing any flushes if
> we didn't previously get an invalidate. Since it's possible to get
> flushes without an invalidate first, remove the busy_bits early
> return.
>
> So now that we don't have the busy_bits guard anymore we'll need the
> origin check for the GTT tracking (we were not doing anything on GTT
> flushes due to the GTT check at invalidate()).
>
> As a last detail, since we can get multiple consecutive flushes,
> disable FBC before updating it, otherwise intel_fbc_update() will just
> keep FBC enabled instead of restarting it.
>
> Notice that this does not fix any of the current IGT tests due to the
> fact that we still have a few intel_fbc() calls at points where we
> also have the frontbuffer tracking calls: we didn't fully convert to
> frontbuffer tracking yet. Once we remove those calls and start relying
> only on the frontbuffer tracking infrastructure we'll need this patch.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h         |  2 +-
>  drivers/gpu/drm/i915/intel_fbc.c         | 13 +++++++------
>  drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
>  3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f4abce1..2437666 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1240,7 +1240,7 @@ void intel_fbc_invalidate(struct drm_i915_private
> *dev_priv,
>                           unsigned int frontbuffer_bits,
>                           enum fb_op_origin origin);
>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
> -                    unsigned int frontbuffer_bits);
> +                    unsigned int frontbuffer_bits, enum fb_op_origin
> origin);
>  const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>  void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index c271af7..1f97fb5 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -884,22 +884,23 @@ void intel_fbc_invalidate(struct drm_i915_private
> *dev_priv,
>  }
>
>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
> -                    unsigned int frontbuffer_bits)
> +                    unsigned int frontbuffer_bits, enum fb_op_origin
> origin)
>  {
>         if (!dev_priv->fbc.enable_fbc)
>                 return;
>
> -       mutex_lock(&dev_priv->fbc.lock);
> +       if (origin == ORIGIN_GTT)
> +               return;
>
> -       if (!dev_priv->fbc.busy_bits)
> -               goto out;
> +       mutex_lock(&dev_priv->fbc.lock);
>
>         dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
>
> -       if (!dev_priv->fbc.busy_bits)
> +       if (!dev_priv->fbc.busy_bits) {
> +               __intel_fbc_disable(dev_priv);
>                 __intel_fbc_update(dev_priv);
>

are we going to kill fbc_update for good? ;)

But yes, we need to force the disable on flush so this is good.
I'd just add in a separated patch, but anyway feel free to use

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> +       }
>
> -out:
>         mutex_unlock(&dev_priv->fbc.lock);
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 777b1d3..ac85357 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -129,7 +129,7 @@ static void intel_frontbuffer_flush(struct drm_device
> *dev,
>
>         intel_edp_drrs_flush(dev, frontbuffer_bits);
>         intel_psr_flush(dev, frontbuffer_bits, origin);
> -       intel_fbc_flush(dev_priv, frontbuffer_bits);
> +       intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
>  }
>
>  /**
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 5519 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: fix FBC frontbuffer tracking flushing code

[Paulo Zanoni]

2015-07-30 20:37 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>
>
> On Tue, Jul 14, 2015 at 12:30 PM Paulo Zanoni <przanoni@gmail.com> wrote:
>>
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> Due to the way busy_bits was handled, we were not doing any flushes if
>> we didn't previously get an invalidate. Since it's possible to get
>> flushes without an invalidate first, remove the busy_bits early
>> return.
>>
>> So now that we don't have the busy_bits guard anymore we'll need the
>> origin check for the GTT tracking (we were not doing anything on GTT
>> flushes due to the GTT check at invalidate()).
>>
>> As a last detail, since we can get multiple consecutive flushes,
>> disable FBC before updating it, otherwise intel_fbc_update() will just
>> keep FBC enabled instead of restarting it.
>>
>> Notice that this does not fix any of the current IGT tests due to the
>> fact that we still have a few intel_fbc() calls at points where we
>> also have the frontbuffer tracking calls: we didn't fully convert to
>> frontbuffer tracking yet. Once we remove those calls and start relying
>> only on the frontbuffer tracking infrastructure we'll need this patch.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_drv.h         |  2 +-
>>  drivers/gpu/drm/i915/intel_fbc.c         | 13 +++++++------
>>  drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
>>  3 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index f4abce1..2437666 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1240,7 +1240,7 @@ void intel_fbc_invalidate(struct drm_i915_private
>> *dev_priv,
>>                           unsigned int frontbuffer_bits,
>>                           enum fb_op_origin origin);
>>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
>> -                    unsigned int frontbuffer_bits);
>> +                    unsigned int frontbuffer_bits, enum fb_op_origin
>> origin);
>>  const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>>  void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>> b/drivers/gpu/drm/i915/intel_fbc.c
>> index c271af7..1f97fb5 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -884,22 +884,23 @@ void intel_fbc_invalidate(struct drm_i915_private
>> *dev_priv,
>>  }
>>
>>  void intel_fbc_flush(struct drm_i915_private *dev_priv,
>> -                    unsigned int frontbuffer_bits)
>> +                    unsigned int frontbuffer_bits, enum fb_op_origin
>> origin)
>>  {
>>         if (!dev_priv->fbc.enable_fbc)
>>                 return;
>>
>> -       mutex_lock(&dev_priv->fbc.lock);
>> +       if (origin == ORIGIN_GTT)
>> +               return;
>>
>> -       if (!dev_priv->fbc.busy_bits)
>> -               goto out;
>> +       mutex_lock(&dev_priv->fbc.lock);
>>
>>         dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
>>
>> -       if (!dev_priv->fbc.busy_bits)
>> +       if (!dev_priv->fbc.busy_bits) {
>> +               __intel_fbc_disable(dev_priv);
>>                 __intel_fbc_update(dev_priv);
>
>
> are we going to kill fbc_update for good? ;)

It is part of the long term plan, but it requires many other changes
first, so I'm focusing on the bugs here.

>
> But yes, we need to force the disable on flush so this is good.
> I'd just add in a separated patch, but anyway feel free to use
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
>>
>> +       }
>>
>> -out:
>>         mutex_unlock(&dev_priv->fbc.lock);
>>  }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
>> b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> index 777b1d3..ac85357 100644
>> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> @@ -129,7 +129,7 @@ static void intel_frontbuffer_flush(struct drm_device
>> *dev,
>>
>>         intel_edp_drrs_flush(dev, frontbuffer_bits);
>>         intel_psr_flush(dev, frontbuffer_bits, origin);
>> -       intel_fbc_flush(dev_priv, frontbuffer_bits);
>> +       intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
>>  }
>>
>>  /**
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: don't call intel_fbc_update() at intel_unpin_work_fn()

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because intel_unpin_work_fn() already calls
intel_frontbuffer_flip_complete() which will call intel_fbc_flush()
which will call intel_fbc_update() when needed.

We couldn't fix this previously due to the fact that FBC was not
properly behaving as intended on frontbuffer flushes, but now that
this is fixed, we can remove the additional call.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad0fc6a..37b2528 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10765,15 +10765,12 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 		container_of(__work, struct intel_unpin_work, work);
 	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
 	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_plane *primary = crtc->base.primary;
 
 	mutex_lock(&dev->struct_mutex);
 	intel_unpin_fb_obj(work->old_fb, primary->state);
 	drm_gem_object_unreference(&work->pending_flip_obj->base);
 
-	intel_fbc_update(dev_priv);
-
 	if (work->flip_queued_req)
 		i915_gem_request_assign(&work->flip_queued_req, NULL);
 	mutex_unlock(&dev->struct_mutex);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: don't call intel_fbc_update() at intel_unpin_work_fn()

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 1873 bytes --]

On Tue, Jul 14, 2015 at 12:30 PM Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Because intel_unpin_work_fn() already calls
> intel_frontbuffer_flip_complete() which will call intel_fbc_flush()
> which will call intel_fbc_update() when needed.
>
> We couldn't fix this previously due to the fact that FBC was not
> properly behaving as intended on frontbuffer flushes, but now that
> this is fixed, we can remove the additional call.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ad0fc6a..37b2528 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10765,15 +10765,12 @@ static void intel_unpin_work_fn(struct
> work_struct *__work)
>                 container_of(__work, struct intel_unpin_work, work);
>         struct intel_crtc *crtc = to_intel_crtc(work->crtc);
>         struct drm_device *dev = crtc->base.dev;
> -       struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_plane *primary = crtc->base.primary;
>
>         mutex_lock(&dev->struct_mutex);
>         intel_unpin_fb_obj(work->old_fb, primary->state);
>         drm_gem_object_unreference(&work->pending_flip_obj->base);
>
> -       intel_fbc_update(dev_priv);
>

\o/ let's kill it!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> -
>         if (work->flip_queued_req)
>                 i915_gem_request_assign(&work->flip_queued_req, NULL);
>         mutex_unlock(&dev->struct_mutex);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 2894 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH igt] kms_frontbuffer_tracking: use the dirty ioctl after MMAP_WC calls

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We can't add this to igt_draw since igt_draw doesn't care whether it's
writing on a frontbuffer or not.

PS: the ENOSYS is for Kernels without the patch implementing the
IOCTL.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index e162e91..1c45429 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -826,6 +826,21 @@ static struct rect pat4_get_rect(struct fb_region *fb, int r)
 	return rect;
 }
 
+static void fb_dirty_ioctl(struct fb_region *fb, struct rect *rect)
+{
+	int rc;
+	drmModeClip clip = {
+		.x1 = rect->x,
+		.x2 = rect->x + rect->w,
+		.y1 = rect->y,
+		.y2 = rect->y + rect->h,
+	};
+
+	rc = drmModeDirtyFB(drm.fd, fb->fb->fb_id, &clip, 1);
+
+	igt_assert(rc == 0 || rc == -ENOSYS);
+}
+
 static void draw_rect(struct draw_pattern_info *pattern, struct fb_region *fb,
 		      enum igt_draw_method method, int r)
 {
@@ -834,6 +849,9 @@ static void draw_rect(struct draw_pattern_info *pattern, struct fb_region *fb,
 	igt_draw_rect_fb(drm.fd, drm.bufmgr, NULL, fb->fb, method,
 			 fb->x + rect.x, fb->y + rect.y,
 			 rect.w, rect.h, rect.color);
+
+	if (method == IGT_DRAW_MMAP_WC)
+		fb_dirty_ioctl(fb, &rect);
 }
 
 static void draw_rect_igt_fb(struct draw_pattern_info *pattern,
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: don't disable FBC for pipe A when flipping pipe B

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Use the appropriate call.

I know there's a discussion about whether we need this call here at
all, but removing the call means we'll only update FBC after we get
the page flip IRQ. So the user may only see the new frame a little
after it should. Let's wait just a little bit more before removing
this call since we can rely in the HW tracking for accurate flips.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 37b2528..6e3ba5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11544,7 +11544,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 			  to_intel_plane(primary)->frontbuffer_bit);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_fbc_disable(dev_priv);
+	intel_fbc_disable_crtc(intel_crtc);
 	intel_frontbuffer_flip_prepare(dev,
 				       to_intel_plane(primary)->frontbuffer_bit);
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: don't disable FBC for pipe A when flipping pipe B

[Daniel Vetter]

On Tue, Jul 14, 2015 at 04:29:13PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Use the appropriate call.
> 
> I know there's a discussion about whether we need this call here at
> all, but removing the call means we'll only update FBC after we get
> the page flip IRQ. So the user may only see the new frame a little
> after it should. Let's wait just a little bit more before removing
> this call since we can rely in the HW tracking for accurate flips.

Afaik fbc2 on g4x+ does correctly invalidate the fbc cache when flipping
the frontbuffer. This was needed for fbc1 which just didn't handle flips -
there we had to essentially disable and then re-enable fbc to make sure
the update was tracked correctly.

The other problem is correctly synchronizing the fence update (for hw gtt
tracking) against the flip. I'm not sure whether we still have races left
there, but simply updating the fence window before we flip should be all
that's required.
-Daniel

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 37b2528..6e3ba5f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11544,7 +11544,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  			  to_intel_plane(primary)->frontbuffer_bit);
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	intel_fbc_disable(dev_priv);
> +	intel_fbc_disable_crtc(intel_crtc);
>  	intel_frontbuffer_flip_prepare(dev,
>  				       to_intel_plane(primary)->frontbuffer_bit);
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: don't disable FBC for pipe A when flipping pipe B

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 2543 bytes --]

On Wed, Jul 15, 2015 at 5:31 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Jul 14, 2015 at 04:29:13PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Use the appropriate call.
>

Good!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> >
> > I know there's a discussion about whether we need this call here at
> > all, but removing the call means we'll only update FBC after we get
> > the page flip IRQ. So the user may only see the new frame a little
> > after it should. Let's wait just a little bit more before removing
> > this call since we can rely in the HW tracking for accurate flips.
>

I'm in favor of remove or at least move to frontbuffer prepare for flip..


>
> Afaik fbc2 on g4x+ does correctly invalidate the fbc cache when flipping
> the frontbuffer. This was needed for fbc1 which just didn't handle flips -
> there we had to essentially disable and then re-enable fbc to make sure
> the update was tracked correctly.
>
> The other problem is correctly synchronizing the fence update (for hw gtt
> tracking) against the flip. I'm not sure whether we still have races left
> there, but simply updating the fence window before we flip should be all
> that's required.


But anyway I'm also in favor of removing the old fbc...


> -Daniel
>
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > index 37b2528..6e3ba5f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11544,7 +11544,7 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
> >                         to_intel_plane(primary)->frontbuffer_bit);
> >       mutex_unlock(&dev->struct_mutex);
> >
> > -     intel_fbc_disable(dev_priv);
> > +     intel_fbc_disable_crtc(intel_crtc);
> >       intel_frontbuffer_flip_prepare(dev,
> >
> to_intel_plane(primary)->frontbuffer_bit);
> >
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 4251 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: special-case dirtyfb for frontbuffer tracking

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

First, an introduction. We currently have two types of GTT mmaps: the
"normal" old mmap, and the WC mmap. For frontbuffer-related features
that have automatic hardware tracking, only the non-WC mmap writes are
detected by the hardware. Since inside the Kernel both are treated as
ORIGIN_GTT, any features ignoring ORIGIN_GTT because of the hardware
tracking are destined to fail.

One of the special rules defined for the WC mmaps is that the user
should call the dirtyfb IOCTL after he is done using the pointers, so
that results in an intel_fb_obj_flush() call. The problem is that the
dirtyfb is passing ORIGIN_GTT, so it is being ignored by FBC - even
though the hardware tracking is not detecing the WC mmap operations.
So in order to fix that without having to give up the automatic
hardware tracking for GTT mmaps we transform the flush operation from
dirtyfb into a special operation: ORIGIN_DIRTYFB.

This commit fixes all the kms_frontbuffer_tracking subtests that
contain "fbc" and "mmap-wc" in their names and are currently failing
(for a total of 16 subtests).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf6761c..78d21c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -894,6 +894,7 @@ enum fb_op_origin {
 	ORIGIN_CPU,
 	ORIGIN_CS,
 	ORIGIN_FLIP,
+	ORIGIN_DIRTYFB,
 };
 
 struct i915_fbc {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6e3ba5f..cffaaf4a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14479,7 +14479,7 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 
 	mutex_lock(&dev->struct_mutex);
-	intel_fb_obj_flush(obj, false, ORIGIN_GTT);
+	intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
 	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: special-case dirtyfb for frontbuffer tracking

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 2712 bytes --]

Very good. I should've added this when adding the dirtyfb flush...
Thanks,

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



On Tue, Jul 14, 2015 at 12:30 PM Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> First, an introduction. We currently have two types of GTT mmaps: the
> "normal" old mmap, and the WC mmap. For frontbuffer-related features
> that have automatic hardware tracking, only the non-WC mmap writes are
> detected by the hardware. Since inside the Kernel both are treated as
> ORIGIN_GTT, any features ignoring ORIGIN_GTT because of the hardware
> tracking are destined to fail.
>
> One of the special rules defined for the WC mmaps is that the user
> should call the dirtyfb IOCTL after he is done using the pointers, so
> that results in an intel_fb_obj_flush() call. The problem is that the
> dirtyfb is passing ORIGIN_GTT, so it is being ignored by FBC - even
> though the hardware tracking is not detecing the WC mmap operations.
> So in order to fix that without having to give up the automatic
> hardware tracking for GTT mmaps we transform the flush operation from
> dirtyfb into a special operation: ORIGIN_DIRTYFB.
>
> This commit fixes all the kms_frontbuffer_tracking subtests that
> contain "fbc" and "mmap-wc" in their names and are currently failing
> (for a total of 16 subtests).
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 1 +
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index cf6761c..78d21c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -894,6 +894,7 @@ enum fb_op_origin {
>         ORIGIN_CPU,
>         ORIGIN_CS,
>         ORIGIN_FLIP,
> +       ORIGIN_DIRTYFB,
>  };
>
>  struct i915_fbc {
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6e3ba5f..cffaaf4a3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14479,7 +14479,7 @@ static int intel_user_framebuffer_dirty(struct
> drm_framebuffer *fb,
>         struct drm_i915_gem_object *obj = intel_fb->obj;
>
>         mutex_lock(&dev->struct_mutex);
> -       intel_fb_obj_flush(obj, false, ORIGIN_GTT);
> +       intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
>         mutex_unlock(&dev->struct_mutex);
>
>         return 0;
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 3654 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: special-case dirtyfb for frontbuffer tracking

[Daniel Vetter]

On Thu, Jul 30, 2015 at 11:48:12PM +0000, Rodrigo Vivi wrote:
> Very good. I should've added this when adding the dirtyfb flush...
> Thanks,
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Merged entire series, thanks.
-Daniel

> 
> 
> 
> On Tue, Jul 14, 2015 at 12:30 PM Paulo Zanoni <przanoni@gmail.com> wrote:
> 
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > First, an introduction. We currently have two types of GTT mmaps: the
> > "normal" old mmap, and the WC mmap. For frontbuffer-related features
> > that have automatic hardware tracking, only the non-WC mmap writes are
> > detected by the hardware. Since inside the Kernel both are treated as
> > ORIGIN_GTT, any features ignoring ORIGIN_GTT because of the hardware
> > tracking are destined to fail.
> >
> > One of the special rules defined for the WC mmaps is that the user
> > should call the dirtyfb IOCTL after he is done using the pointers, so
> > that results in an intel_fb_obj_flush() call. The problem is that the
> > dirtyfb is passing ORIGIN_GTT, so it is being ignored by FBC - even
> > though the hardware tracking is not detecing the WC mmap operations.
> > So in order to fix that without having to give up the automatic
> > hardware tracking for GTT mmaps we transform the flush operation from
> > dirtyfb into a special operation: ORIGIN_DIRTYFB.
> >
> > This commit fixes all the kms_frontbuffer_tracking subtests that
> > contain "fbc" and "mmap-wc" in their names and are currently failing
> > (for a total of 16 subtests).
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      | 1 +
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index cf6761c..78d21c1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -894,6 +894,7 @@ enum fb_op_origin {
> >         ORIGIN_CPU,
> >         ORIGIN_CS,
> >         ORIGIN_FLIP,
> > +       ORIGIN_DIRTYFB,
> >  };
> >
> >  struct i915_fbc {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 6e3ba5f..cffaaf4a3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14479,7 +14479,7 @@ static int intel_user_framebuffer_dirty(struct
> > drm_framebuffer *fb,
> >         struct drm_i915_gem_object *obj = intel_fb->obj;
> >
> >         mutex_lock(&dev->struct_mutex);
> > -       intel_fb_obj_flush(obj, false, ORIGIN_GTT);
> > +       intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
> >         mutex_unlock(&dev->struct_mutex);
> >
> >         return 0;
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx