The point behind standardizing properties into core drm state structures is also that internal code looks prettiers. Take advantage of that and set rotation directly in the fbdev atomic code.
Cc: Rob Clark robdclark@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index bd6d4ab27512..0ac0fcc9b0d2 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -360,11 +360,7 @@ retry: goto fail; }
- ret = drm_atomic_plane_set_property(plane, plane_state, - dev->mode_config.rotation_property, - BIT(DRM_ROTATE_0)); - if (ret != 0) - goto fail; + plane_state->rotation = BIT(DRM_ROTATE_0);
/* disable non-primary: */ if (plane->type == DRM_PLANE_TYPE_PRIMARY)
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..
we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
OCD-align function parameters while at it.
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
+ mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb; + mode_set->x = var->xoffset; mode_set->y = var->yoffset;
@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
- return 0;
fail: + for(i = 0; i < fb_helper->crtc_count; i++) { + struct drm_mode_set *mode_set; + struct drm_plane *plane; + + mode_set = &fb_helper->crtc_info[i].mode_set; + plane = mode_set->crtc->primary; + + if (ret == 0) { + struct drm_framebuffer *new_fb = plane->state->fb; + + if (new_fb) + drm_framebuffer_reference(new_fb); + plane->fb = new_fb; + plane->crtc = plane->state->crtc; + + if (plane->old_fb) + drm_framebuffer_unreference(plane->old_fb); + } + plane->old_fb = NULL; + } + if (ret == -EDEADLK) goto backoff;
- drm_atomic_state_free(state); + if (ret != 0) + drm_atomic_state_free(state);
return ret;
On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter daniel.vetter@ffwll.ch wrote:
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
oh, whoops
OCD-align function parameters while at it.
did you mean to drop that line from the commit msg, or include an extra hunk that you missed?
at any rate,
Reviewed-by: Rob Clark robdclark@gmail.com
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;mode_set->x = var->xoffset; mode_set->y = var->yoffset;@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
return 0;fail:
for(i = 0; i < fb_helper->crtc_count; i++) {struct drm_mode_set *mode_set;struct drm_plane *plane;mode_set = &fb_helper->crtc_info[i].mode_set;plane = mode_set->crtc->primary;if (ret == 0) {struct drm_framebuffer *new_fb = plane->state->fb;if (new_fb)drm_framebuffer_reference(new_fb);plane->fb = new_fb;plane->crtc = plane->state->crtc;if (plane->old_fb)drm_framebuffer_unreference(plane->old_fb);}plane->old_fb = NULL;}if (ret == -EDEADLK) goto backoff;
drm_atomic_state_free(state);
if (ret != 0)drm_atomic_state_free(state); return ret;-- 2.5.1
Thanks!
Tested-by: Rodrigo Vivi rodrigo.vivi@intel.com
On Fri, Oct 16, 2015 at 9:35 AM Rob Clark robdclark@gmail.com wrote:
On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter daniel.vetter@ffwll.ch wrote:
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
oh, whoops
OCD-align function parameters while at it.
did you mean to drop that line from the commit msg, or include an extra hunk that you missed?
at any rate,
Reviewed-by: Rob Clark robdclark@gmail.com
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c
index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
mode_set->crtc->primary->old_fb =mode_set->crtc->primary->fb;
mode_set->x = var->xoffset; mode_set->y = var->yoffset;@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
return 0;fail:
for(i = 0; i < fb_helper->crtc_count; i++) {struct drm_mode_set *mode_set;struct drm_plane *plane;mode_set = &fb_helper->crtc_info[i].mode_set;plane = mode_set->crtc->primary;if (ret == 0) {struct drm_framebuffer *new_fb =plane->state->fb;
if (new_fb)drm_framebuffer_reference(new_fb);plane->fb = new_fb;plane->crtc = plane->state->crtc;if (plane->old_fb)drm_framebuffer_unreference(plane->old_fb);
}plane->old_fb = NULL;}if (ret == -EDEADLK) goto backoff;
drm_atomic_state_free(state);
if (ret != 0)drm_atomic_state_free(state); return ret;-- 2.5.1
On Fri, Oct 16, 2015 at 04:48:12PM +0000, Rodrigo Vivi wrote:
Thanks!
Tested-by: Rodrigo Vivi rodrigo.vivi@intel.com
On Fri, Oct 16, 2015 at 9:35 AM Rob Clark robdclark@gmail.com wrote:
On Fri, Oct 16, 2015 at 12:23 PM, Daniel Vetter daniel.vetter@ffwll.ch wrote:
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
oh, whoops
OCD-align function parameters while at it.
did you mean to drop that line from the commit msg, or include an extra hunk that you missed?
git add fail, so fixed that and applied the patches, thanks for the review and testing. -Daniel
at any rate,
Reviewed-by: Rob Clark robdclark@gmail.com
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c
index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
mode_set->crtc->primary->old_fb =mode_set->crtc->primary->fb;
mode_set->x = var->xoffset; mode_set->y = var->yoffset;@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
return 0;fail:
for(i = 0; i < fb_helper->crtc_count; i++) {struct drm_mode_set *mode_set;struct drm_plane *plane;mode_set = &fb_helper->crtc_info[i].mode_set;plane = mode_set->crtc->primary;if (ret == 0) {struct drm_framebuffer *new_fb =plane->state->fb;
if (new_fb)drm_framebuffer_reference(new_fb);plane->fb = new_fb;plane->crtc = plane->state->crtc;if (plane->old_fb)drm_framebuffer_unreference(plane->old_fb);
}plane->old_fb = NULL;}if (ret == -EDEADLK) goto backoff;
drm_atomic_state_free(state);
if (ret != 0)drm_atomic_state_free(state); return ret;-- 2.5.1
On Fri, Oct 16, 2015 at 06:23:14PM +0200, Daniel Vetter wrote:
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
OCD-align function parameters while at it.
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;- mode_set->x = var->xoffset; mode_set->y = var->yoffset;
@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
- return 0;
fail:
Time to rename the label too?
- for(i = 0; i < fb_helper->crtc_count; i++) {
struct drm_mode_set *mode_set;struct drm_plane *plane;mode_set = &fb_helper->crtc_info[i].mode_set;plane = mode_set->crtc->primary;if (ret == 0) {struct drm_framebuffer *new_fb = plane->state->fb;if (new_fb)drm_framebuffer_reference(new_fb);plane->fb = new_fb;plane->crtc = plane->state->crtc;if (plane->old_fb)drm_framebuffer_unreference(plane->old_fb);}plane->old_fb = NULL;- }
- if (ret == -EDEADLK) goto backoff;
- drm_atomic_state_free(state);
if (ret != 0)
drm_atomic_state_free(state);return ret;
-- 2.5.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Oct 16, 2015 at 07:48:37PM +0300, Ville Syrjälä wrote:
On Fri, Oct 16, 2015 at 06:23:14PM +0200, Daniel Vetter wrote:
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
OCD-align function parameters while at it.
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0ac0fcc9b0d2..b2cf28dd90fe 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb;- mode_set->x = var->xoffset; mode_set->y = var->yoffset;
@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
- return 0;
fail:
Time to rename the label too?
Other atomic functions use fail: too. I guess we could do a large-scale rename to out: or something ... -Daniel
- for(i = 0; i < fb_helper->crtc_count; i++) {
struct drm_mode_set *mode_set;struct drm_plane *plane;mode_set = &fb_helper->crtc_info[i].mode_set;plane = mode_set->crtc->primary;if (ret == 0) {struct drm_framebuffer *new_fb = plane->state->fb;if (new_fb)drm_framebuffer_reference(new_fb);plane->fb = new_fb;plane->crtc = plane->state->crtc;if (plane->old_fb)drm_framebuffer_unreference(plane->old_fb);}plane->old_fb = NULL;- }
- if (ret == -EDEADLK) goto backoff;
- drm_atomic_state_free(state);
if (ret != 0)
drm_atomic_state_free(state);return ret;
-- 2.5.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
In
commit bbb1e52402b2a288b09ae37e8182599931c7e9df Author: Rob Clark robdclark@gmail.com Date: Tue Aug 25 15:35:58 2015 -0400
drm/fb-helper: atomic restore_fbdev_mode()..
we've forgotten to do the plane->old_fb refcount dance for pan_display_atomic, which can result in refcount leaks if the current configuration is not from fbcon. Which apparently can happen when vt-switching - fbcon does a pan first before a set_par.
OCD-align function parameters while at it.
v2: Actually git add the OCD.
Cc: Rob Clark robdclark@gmail.com Cc: Rodrigo Vivi rodrigo.vivi@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92483 Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0ac0fcc9b0d2..e673c13c7391 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1231,7 +1231,7 @@ int drm_fb_helper_set_par(struct fb_info *info) EXPORT_SYMBOL(drm_fb_helper_set_par);
static int pan_display_atomic(struct fb_var_screeninfo *var, - struct fb_info *info) + struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; struct drm_device *dev = fb_helper->dev; @@ -1249,6 +1249,8 @@ retry:
mode_set = &fb_helper->crtc_info[i].mode_set;
+ mode_set->crtc->primary->old_fb = mode_set->crtc->primary->fb; + mode_set->x = var->xoffset; mode_set->y = var->yoffset;
@@ -1264,13 +1266,34 @@ retry: info->var.xoffset = var->xoffset; info->var.yoffset = var->yoffset;
- return 0;
fail: + for(i = 0; i < fb_helper->crtc_count; i++) { + struct drm_mode_set *mode_set; + struct drm_plane *plane; + + mode_set = &fb_helper->crtc_info[i].mode_set; + plane = mode_set->crtc->primary; + + if (ret == 0) { + struct drm_framebuffer *new_fb = plane->state->fb; + + if (new_fb) + drm_framebuffer_reference(new_fb); + plane->fb = new_fb; + plane->crtc = plane->state->crtc; + + if (plane->old_fb) + drm_framebuffer_unreference(plane->old_fb); + } + plane->old_fb = NULL; + } + if (ret == -EDEADLK) goto backoff;
- drm_atomic_state_free(state); + if (ret != 0) + drm_atomic_state_free(state);
return ret;
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Rob Clark -
Rodrigo Vivi -
Ville Syrjälä