2015-07-07 20:28 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
> 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.
So when I saw patches 6 and 7 I decided to investigate how fbcon
interacts with frontbuffer tracking. One thing that I notice is that,
without this patch, if I kill the display manager, I'll have a bunch
of flushes without an invalidate when returning to fbcon. And I
suppose we don't want PSR/FBC enabled on fbcon, so this patch seems to
fix a bug.
By the way, I think we can try to simulate this "kill display manager"
bug on IGT. We could try to close the DRM device and then check if
FBC/PSR stopped. I guess it's probably easier to create a new IGT test
for that.
More below.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index a76cebc..ae9b809 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -831,11 +831,18 @@ 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 = &ifbdev->helper;
Can't this ifbdev->helper assignment segfault? Shouldn't we assign
this pointer just after the !ifbdev check below?
>
> - if (!dev_priv->fbdev)
> + if (!ifbdev)
> return;
>
> - ret = drm_fb_helper_restore_fbdev_mode_unlocked(&dev_priv->fbdev->helper);
> + ret = drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper);
You could have used fb_helper here :)
> if (ret)
> DRM_DEBUG("failed to restore crtc mode\n");
My OCD tells me to request you to add the brackets on the "if" too.
Documentation/CodingStyle:168
> + 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
