| Message ID | 1436397921-3399-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6761
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 343/343 343/343
BYT -3 287/287 284/287
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads PASS(1) FAIL(1)
*BYT igt@gem_persistent_relocs@normal PASS(1) FAIL(1)
*BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
2015-07-08 20:25 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>: > This fbdev restore mode was another corner case that was now > calling frontbuffer flip and flush and making we miss > screen updates with PSR enabled. > > So let's also add the invalidate hack here while we don't have > a reliable dirty fbdev op. > > v2: As pointed by Paulo: removed seg fault risk, used fb_helper > when possible and put brackets on if. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> I just realized the other places that call intel_fb_obj_invalidate() on this file have a huge FIXME comment. With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> You/Daniel may also want to add: Testcase: igt/kms_fbcon_fbt/psr > --- > drivers/gpu/drm/i915/intel_fbdev.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 44c9ccc..fe176d8 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -825,11 +825,20 @@ void intel_fbdev_restore_mode(struct drm_device *dev) > { > int ret; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_fbdev *ifbdev = dev_priv->fbdev; > + struct drm_fb_helper *fb_helper; > > - if (!dev_priv->fbdev) > + if (!ifbdev) > return; > > - ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); > - if (ret) > + fb_helper = &ifbdev->helper; > + > + ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); > + if (ret) { > DRM_DEBUG("failed to restore crtc mode\n"); > + } else { > + mutex_lock(&fb_helper->dev->struct_mutex); > + intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > + mutex_unlock(&fb_helper->dev->struct_mutex); > + } > } > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Those "FIXME" are for atomic fb ops callbacks what I don't believe that applies for this case. -----Original Message----- From: Paulo Zanoni [mailto:przanoni@gmail.com] Sent: Thursday, July 09, 2015 11:54 AM To: Vivi, Rodrigo Cc: Intel Graphics Development; Zanoni, Paulo R Subject: Re: [Intel-gfx] [PATCH] drm/i915: fbdev restore mode needs to invalidate frontbuffer 2015-07-08 20:25 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>: > This fbdev restore mode was another corner case that was now calling > frontbuffer flip and flush and making we miss screen updates with PSR > enabled. > > So let's also add the invalidate hack here while we don't have a > reliable dirty fbdev op. > > v2: As pointed by Paulo: removed seg fault risk, used fb_helper > when possible and put brackets on if. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> I just realized the other places that call intel_fb_obj_invalidate() on this file have a huge FIXME comment. With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> You/Daniel may also want to add: Testcase: igt/kms_fbcon_fbt/psr > --- > drivers/gpu/drm/i915/intel_fbdev.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > b/drivers/gpu/drm/i915/intel_fbdev.c > index 44c9ccc..fe176d8 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -825,11 +825,20 @@ void intel_fbdev_restore_mode(struct drm_device > *dev) { > int ret; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_fbdev *ifbdev = dev_priv->fbdev; > + struct drm_fb_helper *fb_helper; > > - if (!dev_priv->fbdev) > + if (!ifbdev) > return; > > - ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); > - if (ret) > + fb_helper = &ifbdev->helper; > + > + ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); > + if (ret) { > DRM_DEBUG("failed to restore crtc mode\n"); > + } else { > + mutex_lock(&fb_helper->dev->struct_mutex); > + intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); > + mutex_unlock(&fb_helper->dev->struct_mutex); > + } > } > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni
On Thu, Jul 09, 2015 at 07:00:34PM +0000, Vivi, Rodrigo wrote:
> Those "FIXME" are for atomic fb ops callbacks what I don't believe that applies for this case.
Yeah intel_fbdev_restore_mode can't be called from atomic context, no need
for a FIXME here.
-Daniel
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 44c9ccc..fe176d8 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -825,11 +825,20 @@ void intel_fbdev_restore_mode(struct drm_device *dev) { int ret; struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_fbdev *ifbdev = dev_priv->fbdev; + struct drm_fb_helper *fb_helper; - if (!dev_priv->fbdev) + if (!ifbdev) return; - ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper); - if (ret) + fb_helper = &ifbdev->helper; + + ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); + if (ret) { DRM_DEBUG("failed to restore crtc mode\n"); + } else { + mutex_lock(&fb_helper->dev->struct_mutex); + intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); + mutex_unlock(&fb_helper->dev->struct_mutex); + } }
This fbdev restore mode was another corner case that was now calling frontbuffer flip and flush and making we miss screen updates with PSR enabled. So let's also add the invalidate hack here while we don't have a reliable dirty fbdev op. v2: As pointed by Paulo: removed seg fault risk, used fb_helper when possible and put brackets on if. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_fbdev.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)